On Tue, Jan 11, 2022 at 9:31 AM Michael Walle <mich...@walle.cc> wrote: > > Hi, > > Am 2022-01-11 15:20, schrieb Adam Ford: > > On Tue, Jan 4, 2022 at 2:32 AM Michael Walle <mich...@walle.cc> wrote: > >> > The imx8mm and imx8mn appear compatible with imx7d-usb > >> > flags in the OTG driver. If the dr_mode is defined as > >> > host or peripheral, the device appears to operate correctly, > >> > however the auto host/peripheral detection results in an error. > >> > > >> > The solution isn't just adding checks for imx8mm and imx8mn to > >> > the check for imx7, because the USB clock needs to be running > >> > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > >> > > >> > The init_type in both priv and plat data are the same, so it doesn't > >> > make sense to configure the data in the plat data and copy the > >> > data to priv when priv can be configured directly. Instead, rename > >> > ehci_usb_of_to_plat to ehci_usb_dr_mode and call it from the > >> > probe functions after the clocks are enabled, but before the data > >> > is required. > >> > > >> > With that added, the additional checks for imx8mm and imx8mn will > >> > allow reading the register to automatically determine the state > >> > (host or device) of the OTG controller. > >> > > >> > Signed-off-by: Adam Ford <aford...@gmail.com> > >> > --- > >> > V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it > >> > from the probe after the clocks are enabled, but before > >> > the data is needed. > >> > > >> > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > >> > index 1bd6147c76..f2a34b7f06 100644 > >> > --- a/drivers/usb/host/ehci-mx6.c > >> > +++ b/drivers/usb/host/ehci-mx6.c > >> > >> .. > >> > >> > @@ -639,10 +639,8 @@ static int mx6_parse_dt_addrs(struct udevice *dev) > >> > > >> > static int ehci_usb_probe(struct udevice *dev) > >> > { > >> > - struct usb_plat *plat = dev_get_plat(dev); > >> > struct usb_ehci *ehci = dev_read_addr_ptr(dev); > >> > struct ehci_mx6_priv_data *priv = dev_get_priv(dev); > >> > - enum usb_init_type type = plat->init_type; > >> > struct ehci_hccr *hccr; > >> > struct ehci_hcor *hcor; > >> > int ret; > >> > @@ -660,8 +658,6 @@ static int ehci_usb_probe(struct udevice *dev) > >> > return ret; > >> > > >> > priv->ehci = ehci; > >> > - priv->init_type = type; > >> > > > > Michael, > > > >> I'm not sure this is correct. There is also this: > >> https://elixir.bootlin.com/u-boot/v2022.01-rc4/source/drivers/usb/host/usb-uclass.c#L407 > >> > > > > Further down in the code, you should see: > > priv->phy_type = usb_get_phy_mode(dev_ofnode(dev)); > > But that is just fetching the mode from the device tree, the > usb_setup_ehci_gadget() referenced above, change the mode during > runtime by writing the plat->init_type and reprobing the device. > > >> Which won't work anymore. usb_setup_ehci_gadget() is called from > >> usb_gadget_register_driver() in ci_udc.c. The latter is the one used > >> on the imx, right? But I might be wrong. I'm still trying to figure > >> out how this all works together, because I also try to get OTG > >> running on the dwc3 driver. It looks like the ci_udc.c is special > >> here, and I wonder how a transition to UCLASS_USB_GADGET_GENERIC > >> might look like for this driver. > > > > This driver really isn't changing anything, it's just an order of > > operations. > > It doesn't use the plat->init_type at all anymore, no?
>From what I could tell, the only thing that plat->init_type did was to set priv->init_type. The priv structure appears to do most of the work. > > > Previously there was a separate that was being called to > > determine the init_type as being either a peripheral or host. If it > > wasn't explicitly set in the device tree, the helper function would > > query a register, however, on the imx8mm and imx8mn platforms, the > > clocks were not running which resulted in a hang. What I've done is > > simply move the helper function into the probe and have it run after > > the clocks start. In theory the register values will be the same as > > they would be with the help function still in place. > > > > Both Tim Harvey and I have tested on an imx8mm and Fabio tested it on > > an imx7. For me, the peripheral mode worked when the ID pin of the > > USB-OTG was not grounded. When it was grounded, the system corrected > > identified it as a host and I was able to mount a USB drive. > > Again, I'm missing something here, but the only user of > usb_setup_ehci_gadget() is usb/gadget/ci_udc.c. I can't speak to what those functions do. What are you suggesting that I do differently? adam > > -michael