dt-bindings: usb: microchip,usb5744: Add support for configurable board reset delays

[PATCH] dt-bindings: usb: microchip,usb5744: Add support for configurable board reset delays

Posted by Radhey Shyam Pandey 6 months, 2 weeks ago

Introduce 'reset-delay-us' and 'power-on-delay-us' properties. Default
delays in datasheet are not good enough for Xilinx Kria KR260 Robotics
Starter Kit (and others) so there is a need to program board specific
reset and power on delay via DT.

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
Taken reference from mdio.yaml[1]
[1]: Documentation/devicetree/bindings/net/mdio.yaml
---
 .../devicetree/bindings/usb/microchip,usb5744.yaml   | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
index c68c04da3399..94a2bebd32da 100644
--- a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
+++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
@@ -52,6 +52,16 @@ properties:
     description:
       phandle of an usb hub connected via i2c bus.
 
+  reset-delay-us:
+    description:
+      RESET pulse width in microseconds.
+
+  power-on-delay-us:
+    description:
+      Delay after power on in microseconds. USB5744 Hub requires some
+      delay after reset to get to state when SMBus configuration
+      data is going to be accepted.
+
 required:
   - compatible
   - reg
@@ -69,6 +79,8 @@ allOf:
         vdd2-supply: false
         peer-hub: false
         i2c-bus: false
+        reset-delay-us: false
+        power-on-delay-us: false
     else:
       $ref: /schemas/usb/usb-device.yaml
 

base-commit: 4f27f06ec12190c7c62c722e99ab6243dea81a94
-- 
2.34.1

Re: [PATCH] dt-bindings: usb: microchip,usb5744: Add support for configurable board reset delays

Posted by Krzysztof Kozlowski 6 months, 1 week ago

On 05/06/2025 20:29, Radhey Shyam Pandey wrote:
> Introduce 'reset-delay-us' and 'power-on-delay-us' properties. Default
> delays in datasheet are not good enough for Xilinx Kria KR260 Robotics
> Starter Kit (and others) so there is a need to program board specific
> reset and power on delay via DT.
> 
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> Taken reference from mdio.yaml[1]
> [1]: Documentation/devicetree/bindings/net/mdio.yaml
> ---
>  .../devicetree/bindings/usb/microchip,usb5744.yaml   | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> index c68c04da3399..94a2bebd32da 100644
> --- a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> @@ -52,6 +52,16 @@ properties:
>      description:
>        phandle of an usb hub connected via i2c bus.
>  
> +  reset-delay-us:
> +    description:
> +      RESET pulse width in microseconds.

I don't understand - there is no user for this in USB. Why do we need an
ABI if no one ever uses it (and commit msg should clearly explain that)?

> +
> +  power-on-delay-us:

No user here, either. Plus I just wonder if you are mixing here RC
delays or regulator ramp delays, because datasheet does not mention any
delay.

Can you point me to datasheet page explaining these delays?



Best regards,
Krzysztof

Re: [PATCH] dt-bindings: usb: microchip,usb5744: Add support for configurable board reset delays

Posted by Michal Simek 6 months, 1 week ago


On 6/13/25 10:47, Krzysztof Kozlowski wrote:
> On 05/06/2025 20:29, Radhey Shyam Pandey wrote:
>> Introduce 'reset-delay-us' and 'power-on-delay-us' properties. Default
>> delays in datasheet are not good enough for Xilinx Kria KR260 Robotics
>> Starter Kit (and others) so there is a need to program board specific
>> reset and power on delay via DT.
>>
>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>> ---
>> Taken reference from mdio.yaml[1]
>> [1]: Documentation/devicetree/bindings/net/mdio.yaml
>> ---
>>   .../devicetree/bindings/usb/microchip,usb5744.yaml   | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>> index c68c04da3399..94a2bebd32da 100644
>> --- a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>> @@ -52,6 +52,16 @@ properties:
>>       description:
>>         phandle of an usb hub connected via i2c bus.
>>   
>> +  reset-delay-us:
>> +    description:
>> +      RESET pulse width in microseconds.
> 
> I don't understand - there is no user for this in USB. Why do we need an
> ABI if no one ever uses it (and commit msg should clearly explain that)?
> 
>> +
>> +  power-on-delay-us:
> 
> No user here, either. Plus I just wonder if you are mixing here RC
> delays or regulator ramp delays, because datasheet does not mention any
> delay.
> 
> Can you point me to datasheet page explaining these delays?

Here is datasheet.

https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/USB5744-Data-Sheet-DS00001855.pdf

10.6.3 chapter

minimum assert time is 5us this corresponds to reset-delay-us and then there is 
requirement for minimum 1ms for configuration strap.
On Kria platforms 5us and 1ms is used in U-Boot usb onboard driver but that 
times needs to be extended to be able to configure hub over i2c.

Linux current has hardcoded value 0us/10ms.

  33 static const struct onboard_dev_pdata microchip_usb5744_data = {
  34         .reset_us = 0,
  35         .power_on_delay_us = 10000,
  36         .num_supplies = 2,
  37         .supply_names = { "vdd", "vdd2" },
  38         .is_hub = true,
  39 };

Thanks,
Michal

Re: [PATCH] dt-bindings: usb: microchip,usb5744: Add support for configurable board reset delays

Posted by Krzysztof Kozlowski 6 months, 1 week ago

On 13/06/2025 11:13, Michal Simek wrote:
> 
> 
> On 6/13/25 10:47, Krzysztof Kozlowski wrote:
>> On 05/06/2025 20:29, Radhey Shyam Pandey wrote:
>>> Introduce 'reset-delay-us' and 'power-on-delay-us' properties. Default
>>> delays in datasheet are not good enough for Xilinx Kria KR260 Robotics
>>> Starter Kit (and others) so there is a need to program board specific
>>> reset and power on delay via DT.
>>>
>>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>>> ---
>>> Taken reference from mdio.yaml[1]
>>> [1]: Documentation/devicetree/bindings/net/mdio.yaml
>>> ---
>>>   .../devicetree/bindings/usb/microchip,usb5744.yaml   | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>> index c68c04da3399..94a2bebd32da 100644
>>> --- a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>> @@ -52,6 +52,16 @@ properties:
>>>       description:
>>>         phandle of an usb hub connected via i2c bus.
>>>   
>>> +  reset-delay-us:
>>> +    description:
>>> +      RESET pulse width in microseconds.
>>
>> I don't understand - there is no user for this in USB. Why do we need an
>> ABI if no one ever uses it (and commit msg should clearly explain that)?

This still needs solving.

>>
>>> +
>>> +  power-on-delay-us:
>>
>> No user here, either. Plus I just wonder if you are mixing here RC
>> delays or regulator ramp delays, because datasheet does not mention any
>> delay.
>>
>> Can you point me to datasheet page explaining these delays?
> 
> Here is datasheet.
> 
> https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/USB5744-Data-Sheet-DS00001855.pdf
> 
> 10.6.3 chapter
> 
> minimum assert time is 5us this corresponds to reset-delay-us and then there is 
> requirement for minimum 1ms for configuration strap.
> On Kria platforms 5us and 1ms is used in U-Boot usb onboard driver but that 
> times needs to be extended to be able to configure hub over i2c.


And why minimums don't work? Because of some RC circuitry? If so, we
have bindings for GPIO delays as well.

Or regulator-ramp-delay... but I already said about these two.



Best regards,
Krzysztof

Re: [PATCH] dt-bindings: usb: microchip,usb5744: Add support for configurable board reset delays

Posted by Michal Simek 6 months, 1 week ago


On 6/13/25 13:12, Krzysztof Kozlowski wrote:
> On 13/06/2025 11:13, Michal Simek wrote:
>>
>>
>> On 6/13/25 10:47, Krzysztof Kozlowski wrote:
>>> On 05/06/2025 20:29, Radhey Shyam Pandey wrote:
>>>> Introduce 'reset-delay-us' and 'power-on-delay-us' properties. Default
>>>> delays in datasheet are not good enough for Xilinx Kria KR260 Robotics
>>>> Starter Kit (and others) so there is a need to program board specific
>>>> reset and power on delay via DT.
>>>>
>>>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>>>> ---
>>>> Taken reference from mdio.yaml[1]
>>>> [1]: Documentation/devicetree/bindings/net/mdio.yaml
>>>> ---
>>>>    .../devicetree/bindings/usb/microchip,usb5744.yaml   | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>> index c68c04da3399..94a2bebd32da 100644
>>>> --- a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>> @@ -52,6 +52,16 @@ properties:
>>>>        description:
>>>>          phandle of an usb hub connected via i2c bus.
>>>>    
>>>> +  reset-delay-us:
>>>> +    description:
>>>> +      RESET pulse width in microseconds.
>>>
>>> I don't understand - there is no user for this in USB. Why do we need an
>>> ABI if no one ever uses it (and commit msg should clearly explain that)?
> 
> This still needs solving.

Let's solve discussion below first.

>>>
>>>> +
>>>> +  power-on-delay-us:
>>>
>>> No user here, either. Plus I just wonder if you are mixing here RC
>>> delays or regulator ramp delays, because datasheet does not mention any
>>> delay.
>>>
>>> Can you point me to datasheet page explaining these delays?
>>
>> Here is datasheet.
>>
>> https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/USB5744-Data-Sheet-DS00001855.pdf
>>
>> 10.6.3 chapter
>>
>> minimum assert time is 5us this corresponds to reset-delay-us and then there is
>> requirement for minimum 1ms for configuration strap.
>> On Kria platforms 5us and 1ms is used in U-Boot usb onboard driver but that
>> times needs to be extended to be able to configure hub over i2c.
> 
> 
> And why minimums don't work? Because of some RC circuitry? If so, we
> have bindings for GPIO delays as well.

There is slg7xl45106 device which is likely adding delays.
But maybe this is something what can be validated on different board without slg 
device to see differences.
Let me look at gpio delays too.


> Or regulator-ramp-delay... but I already said about these two.

Not this one because there is physical regulator to wait for. Second time is 
configuration strap one and power-on-delay is likely misleading and should be 
renamed in the driver.

Thanks,
Michal

Re: [PATCH] dt-bindings: usb: microchip,usb5744: Add support for configurable board reset delays

Posted by Michal Simek 6 months, 1 week ago


On 6/5/25 20:29, Radhey Shyam Pandey wrote:
> Introduce 'reset-delay-us' and 'power-on-delay-us' properties. Default
> delays in datasheet are not good enough for Xilinx Kria KR260 Robotics
> Starter Kit (and others) so there is a need to program board specific
> reset and power on delay via DT.
> 
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> Taken reference from mdio.yaml[1]
> [1]: Documentation/devicetree/bindings/net/mdio.yaml
> ---
>   .../devicetree/bindings/usb/microchip,usb5744.yaml   | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> index c68c04da3399..94a2bebd32da 100644
> --- a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> @@ -52,6 +52,16 @@ properties:
>       description:
>         phandle of an usb hub connected via i2c bus.
>   
> +  reset-delay-us:
> +    description:
> +      RESET pulse width in microseconds.
> +
> +  power-on-delay-us:
> +    description:
> +      Delay after power on in microseconds. USB5744 Hub requires some
> +      delay after reset to get to state when SMBus configuration
> +      data is going to be accepted.
> +
>   required:
>     - compatible
>     - reg
> @@ -69,6 +79,8 @@ allOf:
>           vdd2-supply: false
>           peer-hub: false
>           i2c-bus: false
> +        reset-delay-us: false
> +        power-on-delay-us: false
>       else:
>         $ref: /schemas/usb/usb-device.yaml
>   
> 
> base-commit: 4f27f06ec12190c7c62c722e99ab6243dea81a94

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal