Am 2022-01-11 17:52, schrieb Adam Ford:
On Tue, Jan 11, 2022 at 9:31 AM Michael Walle <[email protected]> wrote:
Hi,
Am 2022-01-11 15:20, schrieb Adam Ford:
> On Tue, Jan 4, 2022 at 2:32 AM Michael Walle <[email protected]> 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 <[email protected]>
>> > ---
>> > 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.
but plat->init_type seems to change during runtime (by
usb_setup_ehci_gadget()) and with this patch applied, it doesn't
do that anymore.
> 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?
I'd start by seeing if/when usb_setup_ehci_gadget() is called and
see if plat->init_type changes. If its not called, is it dead code
then?
Sorry I just noticed this, while reviewing how this particular driver
works, because I still like to find a working OTG driver in u-boot.
I don't have any imx boards.
-michael
