DWC3 host support

DWC3 host support

[Angus Ainslie]

Hi Michal,

I recently rebased my librem5 tree onto the latest u-boot-imx branch and 
the dwc3 host mode stopped working.

I bisected it down to this commit:

142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for 
usb3-phy PHY configuration

Reverting that commit allows usb host mode to work on the librem5 again.

Should this initialization go into an SOC specific glue_configure 
function ?

Is the imx8mq.dtsi missing something that will keep usb host working 
with this patch ?

Does this break usb host on other imx8mq devices ?

Thanks
Angus

Re: DWC3 host support

[Angus Ainslie]

On 2022-07-16 06:02, Angus Ainslie wrote:
> Hi Michal,
> 
> I recently rebased my librem5 tree onto the latest u-boot-imx branch
> and the dwc3 host mode stopped working.
> 

Further debugging indicates that the "U-Boot XHCI Host Controller" is 
detected but downstream USB 2 hubs and storage devices are not.

> I bisected it down to this commit:
> 
> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for
> usb3-phy PHY configuration
> 
> Reverting that commit allows usb host mode to work on the librem5 
> again.
> 
> Should this initialization go into an SOC specific glue_configure 
> function ?
> 
> Is the imx8mq.dtsi missing something that will keep usb host working
> with this patch ?
> 
> Does this break usb host on other imx8mq devices ?
> 
> Thanks
> Angus

Re: DWC3 host support

[Marek Vasut]

On 7/16/22 15:02, Angus Ainslie wrote:
> Hi Michal,
> 
> I recently rebased my librem5 tree onto the latest u-boot-imx branch and 
> the dwc3 host mode stopped working.
> 
> I bisected it down to this commit:
> 
> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for 
> usb3-phy PHY configuration
> 
> Reverting that commit allows usb host mode to work on the librem5 again.
> 
> Should this initialization go into an SOC specific glue_configure 
> function ?
> 
> Is the imx8mq.dtsi missing something that will keep usb host working 
> with this patch ?
> 
> Does this break usb host on other imx8mq devices ?

Wasn't this fixed by:
868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
?

Re: DWC3 host support

[Angus Ainslie]

On 2022-07-16 11:37, Marek Vasut wrote:
> On 7/16/22 15:02, Angus Ainslie wrote:
>> Hi Michal,
>> 
>> I recently rebased my librem5 tree onto the latest u-boot-imx branch 
>> and the dwc3 host mode stopped working.
>> 
>> I bisected it down to this commit:
>> 
>> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for 
>> usb3-phy PHY configuration
>> 
>> Reverting that commit allows usb host mode to work on the librem5 
>> again.
>> 
>> Should this initialization go into an SOC specific glue_configure 
>> function ?
>> 
>> Is the imx8mq.dtsi missing something that will keep usb host working 
>> with this patch ?
>> 
>> Does this break usb host on other imx8mq devices ?
> 
> Wasn't this fixed by:
> 868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
> ?

I've got that in my tree and it still fails to probe the USB2 hub and 
USB 2 storage.

Angus

Re: DWC3 host support

[Marek Vasut]

On 7/17/22 05:00, Angus Ainslie wrote:
> On 2022-07-16 11:37, Marek Vasut wrote:
>> On 7/16/22 15:02, Angus Ainslie wrote:
>>> Hi Michal,
>>>
>>> I recently rebased my librem5 tree onto the latest u-boot-imx branch 
>>> and the dwc3 host mode stopped working.
>>>
>>> I bisected it down to this commit:
>>>
>>> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for 
>>> usb3-phy PHY configuration
>>>
>>> Reverting that commit allows usb host mode to work on the librem5 again.
>>>
>>> Should this initialization go into an SOC specific glue_configure 
>>> function ?
>>>
>>> Is the imx8mq.dtsi missing something that will keep usb host working 
>>> with this patch ?
>>>
>>> Does this break usb host on other imx8mq devices ?
>>
>> Wasn't this fixed by:
>> 868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
>> ?
> 
> I've got that in my tree and it still fails to probe the USB2 hub and 
> USB 2 storage.

I assume you do have CONFIG_PHY_IMX8MQ_USB enabled ?

What does generic_phy_get_by_name() return for you in 
drivers/usb/dwc3/dwc3-generic.c ?

Re: DWC3 host support

[Michal Simek]

On 7/17/22 17:23, Marek Vasut wrote:
> On 7/17/22 05:00, Angus Ainslie wrote:
>> On 2022-07-16 11:37, Marek Vasut wrote:
>>> On 7/16/22 15:02, Angus Ainslie wrote:
>>>> Hi Michal,
>>>>
>>>> I recently rebased my librem5 tree onto the latest u-boot-imx branch
>>>> and the dwc3 host mode stopped working.
>>>>
>>>> I bisected it down to this commit:
>>>>
>>>> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for
>>>> usb3-phy PHY configuration
>>>>
>>>> Reverting that commit allows usb host mode to work on the librem5 again.
>>>>
>>>> Should this initialization go into an SOC specific glue_configure
>>>> function ?
>>>>
>>>> Is the imx8mq.dtsi missing something that will keep usb host working
>>>> with this patch ?
>>>>
>>>> Does this break usb host on other imx8mq devices ?
>>>
>>> Wasn't this fixed by:
>>> 868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
>>> ?
>>
>> I've got that in my tree and it still fails to probe the USB2 hub and
>> USB 2 storage.
> 
> I assume you do have CONFIG_PHY_IMX8MQ_USB enabled ?
> 
> What does generic_phy_get_by_name() return for you in
> drivers/usb/dwc3/dwc3-generic.c ?

As Marek said there was one patch which fixes origin patch which didn't handle 
all the error cases properly. We need to know return value from 
generic_phy_get_by_name(), also if you still have usb3-phy in DT (as is in 
imx8mq.dtsi) with phy DT status enabled and enabled  phy driver 
(CONFIG_PHY_IMX8MQ_USB).

Thanks,
Michal

Re: DWC3 host support

[Angus Ainslie]

Hi Marek/Michal,

On 2022-07-18 01:13, Michal Simek wrote:
> On 7/17/22 17:23, Marek Vasut wrote:
>> On 7/17/22 05:00, Angus Ainslie wrote:
>>> On 2022-07-16 11:37, Marek Vasut wrote:
>>>> On 7/16/22 15:02, Angus Ainslie wrote:
>>>>> Hi Michal,
>>>>> 
>>>>> I recently rebased my librem5 tree onto the latest u-boot-imx 
>>>>> branch
>>>>> and the dwc3 host mode stopped working.
>>>>> 
>>>>> I bisected it down to this commit:
>>>>> 
>>>>> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for
>>>>> usb3-phy PHY configuration
>>>>> 
>>>>> Reverting that commit allows usb host mode to work on the librem5 
>>>>> again.
>>>>> 
>>>>> Should this initialization go into an SOC specific glue_configure
>>>>> function ?
>>>>> 
>>>>> Is the imx8mq.dtsi missing something that will keep usb host 
>>>>> working
>>>>> with this patch ?
>>>>> 
>>>>> Does this break usb host on other imx8mq devices ?
>>>> 
>>>> Wasn't this fixed by:
>>>> 868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
>>>> ?
>>> 
>>> I've got that in my tree and it still fails to probe the USB2 hub and
>>> USB 2 storage.
>> 
>> I assume you do have CONFIG_PHY_IMX8MQ_USB enabled ?
>> 
>> What does generic_phy_get_by_name() return for you in
>> drivers/usb/dwc3/dwc3-generic.c ?
> 
> As Marek said there was one patch which fixes origin patch which
> didn't handle all the error cases properly. We need to know return
> value from generic_phy_get_by_name(), also if you still have usb3-phy
> in DT (as is in imx8mq.dtsi) with phy DT status enabled and enabled
> phy driver (CONFIG_PHY_IMX8MQ_USB).
> 

I use the imx8mq.dtsi from the u-boot-imx tree that includes the 
usb3-phy

https://source.denx.de/u-boot/custodians/u-boot-imx/-/blob/master/arch/arm/dts/imx8mq.dtsi#L1421

CONFIG_PHY_IMX8MQ_USB is defined in my defconfig

https://source.puri.sm/angus.ainslie/uboot-imx/-/blob/upstream/librem5-uart2/configs/librem5_defconfig#L114

Here is the error path I think you're talking about

https://source.puri.sm/angus.ainslie/uboot-imx/-/blob/upstream/librem5-uart2/drivers/usb/dwc3/dwc3-generic.c#L474

I modified dwc3-generic.c to print the return value

@@ -475,6 +477,8 @@ static int dwc3_glue_probe(struct udevice *dev)
                 phy.dev = NULL;
         }

+       debug("phy3 initialized %d %s\n", ret, phy.dev ? phy.dev->name : 
"null");
+
         glue->regs = dev_read_addr(dev);

Here's the output from the boot

U-Boot 2022.07-rc5-00014-g329f8a8ae05-dirty (Jul 18 2022 - 06:59:45 
-0700)

CPU:   Freescale i.MX8MQ rev2.0 1500 MHz (running at 1000 MHz)
CPU:   Commercial temperature grade (0C to 95C) at 35C
Reset cause: POR
Model: Purism Librem 5r4
DRAM:  3 GiB
dwc3_glue_bind: subnode name: port@0
dwc3_glue_bind: dr_mode: OTG or Peripheral
dwc3_glue_bind: subnode name: port@1
dwc3_glue_bind: dr_mode: OTG or Peripheral
dwc3_glue_bind: subnode name: hub@1
dwc3_glue_bind: dr_mode: HOST
clk_register: failed to get <NULL> device (parent of ckil)
clk_register: failed to get <NULL> device (parent of clock-osc-27m)
clk_register: failed to get <NULL> device (parent of sys1_pll)
clk_register: failed to get <NULL> device (parent of sys2_pll)
clk_register: failed to get <NULL> device (parent of sys3_pll)
Enabling regulator-hub
tps65982 boot successful
Core:  192 devices, 25 uclasses, devicetree: separate
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
Loading Environment from MMC... OK
In:    serial
Out:   serial
Err:   serial
Board name: librem5
Board rev:  4
vol_down_key_pressed : 1
Net:   No ethernet found.
Hit any key to stop autoboot:  0
u-boot=> usb start
starting USB...
Bus hub@1: generic_phy_get_by_name(dev=00000000ffb19830, name=usb3-phy, 
phy=00000000ffb08460)
generic_phy_get_by_index_nodev(node=usb@38200000, index=1, 
phy=00000000ffb08460)
generic_phy_xlate_offs_flags(phy=00000000ffb08460)
phy3 initialized 0 usb-phy@382f0040
phy3 powered on
dwc3_glue_probe finished
generic_phy_get_by_index_nodev(node=usb@38200000, index=0, 
phy=00000000ffb2b1e0)
generic_phy_xlate_offs_flags(phy=00000000ffb2b1e0)
generic_phy_get_by_index_nodev(node=usb@38200000, index=1, 
phy=00000000ffb2b1f0)
generic_phy_xlate_offs_flags(phy=00000000ffb2b1f0)
dwc3-generic-host hub@1: Event buf 00000000ffb2b340 dma ffb2b340 length 
256
Register 2000140 NbrPorts 2
Starting the controller
USB XHCI 1.10
scanning bus hub@1 for devices... cannot reset port 1!?
1 USB Device(s) found
        scanning usb for storage devices... 0 Storage Device(s) found
u-boot=> usb info
1: Hub,  USB Revision 3.0
  - U-Boot XHCI Host Controller
  - Class: Hub
  - PacketSize: 512  Configurations: 1
  - Vendor: 0x0000  Product 0x0000 Version 1.0
    Configuration: 1
    - Interfaces: 1 Self Powered 0mA
      Interface: 0
      - Alternate Setting 0, Endpoints: 1
      - Class Hub
      - Endpoint 1 In Interrupt MaxPacket 8 Interval 255ms

Thanks
Angus

> Thanks,
> Michal

Re: DWC3 host support

[Angus Ainslie]

Hi,

On 2022-07-18 01:13, Michal Simek wrote:
> On 7/17/22 17:23, Marek Vasut wrote:
>> On 7/17/22 05:00, Angus Ainslie wrote:
>>> On 2022-07-16 11:37, Marek Vasut wrote:
>>>> On 7/16/22 15:02, Angus Ainslie wrote:
>>>>> Hi Michal,
>>>>> 
>>>>> I recently rebased my librem5 tree onto the latest u-boot-imx 
>>>>> branch
>>>>> and the dwc3 host mode stopped working.
>>>>> 
>>>>> I bisected it down to this commit:
>>>>> 
>>>>> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for
>>>>> usb3-phy PHY configuration
>>>>> 
>>>>> Reverting that commit allows usb host mode to work on the librem5 
>>>>> again.
>>>>> 
>>>>> Should this initialization go into an SOC specific glue_configure
>>>>> function ?
>>>>> 
>>>>> Is the imx8mq.dtsi missing something that will keep usb host 
>>>>> working
>>>>> with this patch ?
>>>>> 
>>>>> Does this break usb host on other imx8mq devices ?
>>>> 
>>>> Wasn't this fixed by:
>>>> 868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
>>>> ?
>>> 
>>> I've got that in my tree and it still fails to probe the USB2 hub and
>>> USB 2 storage.
>> 
>> I assume you do have CONFIG_PHY_IMX8MQ_USB enabled ?
>> 
>> What does generic_phy_get_by_name() return for you in
>> drivers/usb/dwc3/dwc3-generic.c ?
> 
> As Marek said there was one patch which fixes origin patch which
> didn't handle all the error cases properly. We need to know return
> value from generic_phy_get_by_name(), also if you still have usb3-phy
> in DT (as is in imx8mq.dtsi) with phy DT status enabled and enabled
> phy driver (CONFIG_PHY_IMX8MQ_USB).
> 

Removing the usb3 phy definition also "fixes" it

--- a/arch/arm/dts/imx8mq-librem5-r4.dts
+++ b/arch/arm/dts/imx8mq-librem5-r4.dts
@@ -33,3 +33,8 @@
  &proximity {
         proximity-near-level = <10>;
  };
+
+&usb_dwc3_1 {
+       phys = <&usb3_phy1>;
+       phy-names = "usb2-phy";
+};

Thanks
Angus

> Thanks,
> Michal

Re: DWC3 host support

[Michal Simek]

Hi,

On 7/18/22 18:23, Angus Ainslie wrote:
> Hi,
> 
> On 2022-07-18 01:13, Michal Simek wrote:
>> On 7/17/22 17:23, Marek Vasut wrote:
>>> On 7/17/22 05:00, Angus Ainslie wrote:
>>>> On 2022-07-16 11:37, Marek Vasut wrote:
>>>>> On 7/16/22 15:02, Angus Ainslie wrote:
>>>>>> Hi Michal,
>>>>>>
>>>>>> I recently rebased my librem5 tree onto the latest u-boot-imx branch
>>>>>> and the dwc3 host mode stopped working.
>>>>>>
>>>>>> I bisected it down to this commit:
>>>>>>
>>>>>> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support for
>>>>>> usb3-phy PHY configuration
>>>>>>
>>>>>> Reverting that commit allows usb host mode to work on the librem5 again.
>>>>>>
>>>>>> Should this initialization go into an SOC specific glue_configure
>>>>>> function ?
>>>>>>
>>>>>> Is the imx8mq.dtsi missing something that will keep usb host working
>>>>>> with this patch ?
>>>>>>
>>>>>> Does this break usb host on other imx8mq devices ?
>>>>>
>>>>> Wasn't this fixed by:
>>>>> 868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
>>>>> ?
>>>>
>>>> I've got that in my tree and it still fails to probe the USB2 hub and
>>>> USB 2 storage.
>>>
>>> I assume you do have CONFIG_PHY_IMX8MQ_USB enabled ?
>>>
>>> What does generic_phy_get_by_name() return for you in
>>> drivers/usb/dwc3/dwc3-generic.c ?
>>
>> As Marek said there was one patch which fixes origin patch which
>> didn't handle all the error cases properly. We need to know return
>> value from generic_phy_get_by_name(), also if you still have usb3-phy
>> in DT (as is in imx8mq.dtsi) with phy DT status enabled and enabled
>> phy driver (CONFIG_PHY_IMX8MQ_USB).
>>
> 
> Removing the usb3 phy definition also "fixes" it
> 
> --- a/arch/arm/dts/imx8mq-librem5-r4.dts
> +++ b/arch/arm/dts/imx8mq-librem5-r4.dts
> @@ -33,3 +33,8 @@
>   &proximity {
>          proximity-near-level = <10>;
>   };
> +
> +&usb_dwc3_1 {
> +       phys = <&usb3_phy1>;
> +       phy-names = "usb2-phy";
> +};

Log shows that phy initialization is called properly but it looks like that 
initialization itself has issue which should be the issue with phy driver.

DWC3 driver is calling just phy_init and phy_power_on which are quite small 2 
functions in phy-imx8mq-usb.c.

Did usb3 work before 142d50fbce7c364a74f5e8204dba491b9f066e6c?
I have no idea how imx is working but I have added to CC author of this phy driver.

I see small differences between Linux and U-Boot drivers.

  99         value &= ~PHY_CTRL0_SSC_RANGE_MASK;
100         value |= PHY_CTRL0_SSC_RANGE_4003PPM;

And in power_on I see this in u-boot

129         /* Disable rx term override */
130         value = readl(imx_phy->base + PHY_CTRL6);
131         value &= ~PHY_CTRL6_RXTERM_OVERRIDE_SEL;
132         writel(value, imx_phy->base + PHY_CTRL6);

And Linux driver is handling regulators. Do you use any regulator?

If usb3.0 works in Linux I think it should be easier for you to track this down 
but there are some small differences in the U-Boot driver which can be the 
reason why it is failing on your board.

Thanks,
Michal

Re: DWC3 host support

[Angus Ainslie]

On 2022-07-18 23:47, Michal Simek wrote:
> Hi,
> 
> On 7/18/22 18:23, Angus Ainslie wrote:
>> Hi,
>> 
>> On 2022-07-18 01:13, Michal Simek wrote:
>>> On 7/17/22 17:23, Marek Vasut wrote:
>>>> On 7/17/22 05:00, Angus Ainslie wrote:
>>>>> On 2022-07-16 11:37, Marek Vasut wrote:
>>>>>> On 7/16/22 15:02, Angus Ainslie wrote:
>>>>>>> Hi Michal,
>>>>>>> 
>>>>>>> I recently rebased my librem5 tree onto the latest u-boot-imx 
>>>>>>> branch
>>>>>>> and the dwc3 host mode stopped working.
>>>>>>> 
>>>>>>> I bisected it down to this commit:
>>>>>>> 
>>>>>>> 142d50fbce7c364a74f5e8204dba491b9f066e6c usb: dwc3: Add support 
>>>>>>> for
>>>>>>> usb3-phy PHY configuration
>>>>>>> 
>>>>>>> Reverting that commit allows usb host mode to work on the librem5 
>>>>>>> again.
>>>>>>> 
>>>>>>> Should this initialization go into an SOC specific glue_configure
>>>>>>> function ?
>>>>>>> 
>>>>>>> Is the imx8mq.dtsi missing something that will keep usb host 
>>>>>>> working
>>>>>>> with this patch ?
>>>>>>> 
>>>>>>> Does this break usb host on other imx8mq devices ?
>>>>>> 
>>>>>> Wasn't this fixed by:
>>>>>> 868d58f69c ("usb: dwc3: Fix non-usb3 configurations")
>>>>>> ?
>>>>> 
>>>>> I've got that in my tree and it still fails to probe the USB2 hub 
>>>>> and
>>>>> USB 2 storage.
>>>> 
>>>> I assume you do have CONFIG_PHY_IMX8MQ_USB enabled ?
>>>> 
>>>> What does generic_phy_get_by_name() return for you in
>>>> drivers/usb/dwc3/dwc3-generic.c ?
>>> 
>>> As Marek said there was one patch which fixes origin patch which
>>> didn't handle all the error cases properly. We need to know return
>>> value from generic_phy_get_by_name(), also if you still have usb3-phy
>>> in DT (as is in imx8mq.dtsi) with phy DT status enabled and enabled
>>> phy driver (CONFIG_PHY_IMX8MQ_USB).
>>> 
>> 
>> Removing the usb3 phy definition also "fixes" it
>> 
>> --- a/arch/arm/dts/imx8mq-librem5-r4.dts
>> +++ b/arch/arm/dts/imx8mq-librem5-r4.dts
>> @@ -33,3 +33,8 @@
>>   &proximity {
>>          proximity-near-level = <10>;
>>   };
>> +
>> +&usb_dwc3_1 {
>> +       phys = <&usb3_phy1>;
>> +       phy-names = "usb2-phy";
>> +};
> 
> Log shows that phy initialization is called properly but it looks like
> that initialization itself has issue which should be the issue with
> phy driver.
> 
> DWC3 driver is calling just phy_init and phy_power_on which are quite
> small 2 functions in phy-imx8mq-usb.c.
> 
> Did usb3 work before 142d50fbce7c364a74f5e8204dba491b9f066e6c?
> I have no idea how imx is working but I have added to CC author of
> this phy driver.
> 

We don't use USB3 on that port so no idea. The only thing connected to 
that port is a USB 2 hub and then downstream devices from the hub. The 
USB 2 hub did work without 142d50fbce7c364a74f5e8204dba491b9f066e6c.

> I see small differences between Linux and U-Boot drivers.
> 
>  99         value &= ~PHY_CTRL0_SSC_RANGE_MASK;
> 100         value |= PHY_CTRL0_SSC_RANGE_4003PPM;
> 
> And in power_on I see this in u-boot
> 
> 129         /* Disable rx term override */
> 130         value = readl(imx_phy->base + PHY_CTRL6);
> 131         value &= ~PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> 132         writel(value, imx_phy->base + PHY_CTRL6);
> 
> And Linux driver is handling regulators. Do you use any regulator?

There isn't a regulator specifically for the PHY. There is one for the 
hub and it gets enabled before the phy_init functions.

> 
> If usb3.0 works in Linux I think it should be easier for you to track
> this down but there are some small differences in the U-Boot driver
> which can be the reason why it is failing on your board.

Thanks
Angus

> 
> Thanks,
> Michal