diff mbox series

[v6,1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

Message ID 20231204144429.45197-2-linux.amoon@gmail.com
State Superseded
Headers show
Series [v6,1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub | expand

Commit Message

Anand Moon Dec. 4, 2023, 2:44 p.m. UTC
Add the binding example for the USB3.1 Genesys Logic GL3523
integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
hub.

For onboard hub controllers that support USB 3.x and USB 2.0 hubs
with shared resets and power supplies, this property is used to identify
the hubs with which these are shared.

GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves
power to the USB HUB, it uses 5V power regulator.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
V6: fix the description of the regulators
Updated the commit message for regulator updates.
add reviewed by Conor Dooley
[1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/
v5: upgrade peer-hub description : Conor Dooley
[0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram]
v4: Fix the description of peer-hub and update the commit message.
Schematics of the Odroid N2+
https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf
V3: fix the dt_binding_check error, added new example for Genesys GL3523
v2: added Genesys GL3523 binding
v1: none
---
 .../bindings/usb/genesys,gl850g.yaml          | 65 +++++++++++++++++--
 1 file changed, 61 insertions(+), 4 deletions(-)

Comments

Rob Herring (Arm) Dec. 6, 2023, 1:53 p.m. UTC | #1
On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote:
> Add the binding example for the USB3.1 Genesys Logic GL3523
> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> hub.
> 
> For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> with shared resets and power supplies, this property is used to identify
> the hubs with which these are shared.
> 
> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves
> power to the USB HUB, it uses 5V power regulator.
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> V6: fix the description of the regulators
> Updated the commit message for regulator updates.
> add reviewed by Conor Dooley
> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/
> v5: upgrade peer-hub description : Conor Dooley
> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram]
> v4: Fix the description of peer-hub and update the commit message.
> Schematics of the Odroid N2+
> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf
> V3: fix the dt_binding_check error, added new example for Genesys GL3523
> v2: added Genesys GL3523 binding
> v1: none
> ---
>  .../bindings/usb/genesys,gl850g.yaml          | 65 +++++++++++++++++--
>  1 file changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> index ee08b9c3721f..c6f63a69396d 100644
> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
>  maintainers:
>    - Icenowy Zheng <uwu@icenowy.me>
>  
> -allOf:
> -  - $ref: usb-device.yaml#
> -
>  properties:
>    compatible:
>      enum:
> @@ -27,12 +24,46 @@ properties:
>  
>    vdd-supply:
>      description:
> -      the regulator that provides 3.3V core power to the hub.
> +      The regulator that provides 3.3V or 5.0V core power to the hub.
> +
> +  peer-hub:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> +      with shared resets and power supplies, this property is used to identify
> +      the hubs with which these are shared.
>  
>  required:
>    - compatible
>    - reg
>  
> +allOf:
> +  - $ref: usb-device.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - usb5e3,608
> +    then:
> +      properties:
> +        peer-hub: false
> +        vdd-supply: false
> +        reset-gpios: true
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - usb5e3,610
> +              - usb5e3,620
> +    then:
> +      properties:
> +        peer-hub: true
> +        vdd-supply: true
> +        reset-gpios: true

No need for this if schema. The default is they are allowed.

Rob
Anand Moon Dec. 6, 2023, 5:14 p.m. UTC | #2
Hi Rob,

On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote:
> > Add the binding example for the USB3.1 Genesys Logic GL3523
> > integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> > hub.
> >
> > For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> > with shared resets and power supplies, this property is used to identify
> > the hubs with which these are shared.
> >
> > GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves
> > power to the USB HUB, it uses 5V power regulator.
> >
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > V6: fix the description of the regulators
> > Updated the commit message for regulator updates.
> > add reviewed by Conor Dooley
> > [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/
> > v5: upgrade peer-hub description : Conor Dooley
> > [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram]
> > v4: Fix the description of peer-hub and update the commit message.
> > Schematics of the Odroid N2+
> > https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf
> > V3: fix the dt_binding_check error, added new example for Genesys GL3523
> > v2: added Genesys GL3523 binding
> > v1: none
> > ---
> >  .../bindings/usb/genesys,gl850g.yaml          | 65 +++++++++++++++++--
> >  1 file changed, 61 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > index ee08b9c3721f..c6f63a69396d 100644
> > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
> >  maintainers:
> >    - Icenowy Zheng <uwu@icenowy.me>
> >
> > -allOf:
> > -  - $ref: usb-device.yaml#
> > -
> >  properties:
> >    compatible:
> >      enum:
> > @@ -27,12 +24,46 @@ properties:
> >
> >    vdd-supply:
> >      description:
> > -      the regulator that provides 3.3V core power to the hub.
> > +      The regulator that provides 3.3V or 5.0V core power to the hub.
> > +
> > +  peer-hub:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> > +      with shared resets and power supplies, this property is used to identify
> > +      the hubs with which these are shared.
> >
> >  required:
> >    - compatible
> >    - reg
> >
> > +allOf:
> > +  - $ref: usb-device.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - usb5e3,608
> > +    then:
> > +      properties:
> > +        peer-hub: false
> > +        vdd-supply: false
> > +        reset-gpios: true
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - usb5e3,610
> > +              - usb5e3,620
> > +    then:
> > +      properties:
> > +        peer-hub: true
> > +        vdd-supply: true
> > +        reset-gpios: true
>
> No need for this if schema. The default is they are allowed.
>

If I move reset-gpios to required, I observe the below warning.

  DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
/home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
hub@1: 'reset-gpio' is a required property
        from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
  DTC_CHK Documentation/devicetree/bindings/usb/mediatek,musb.example.dtb
  DTC_CHK Documentation/devicetree/bindings/usb/usb251xb.example.dtb
  DTC_CHK Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb
/home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/genesys,gl850g.example.dtb:
hub@1: 'reset-gpio' is a required property
        from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
  DTC_CHK Documentation/devicetree/bindings/usb/brcm,bdc.example.dtb
/home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/genesys,gl850g.example.dtb:
hub@1: 'reset-gpio' is a required property
        from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
  DTC_CHK Documentation/devicetree/bindings/usb/xlnx,usb2.example.dtb
/home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/genesys,gl850g.example.dtb:
hub@2: 'reset-gpio' is a required property
        from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
  DTC_CHK Documentation/devicetree/bindings/usb/starfive,jh7110-usb.example.dtb

> Rob

Thanks
-Anand
Krzysztof Kozlowski Dec. 7, 2023, 8:30 a.m. UTC | #3
On 06/12/2023 18:14, Anand Moon wrote:
> Hi Rob,
> 
> On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote:
>>
>> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote:
>>> Add the binding example for the USB3.1 Genesys Logic GL3523
>>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
>>> hub.
>>>
>>> For onboard hub controllers that support USB 3.x and USB 2.0 hubs
>>> with shared resets and power supplies, this property is used to identify
>>> the hubs with which these are shared.
>>>
>>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves
>>> power to the USB HUB, it uses 5V power regulator.
>>>
>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> ---
>>> V6: fix the description of the regulators
>>> Updated the commit message for regulator updates.
>>> add reviewed by Conor Dooley
>>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/
>>> v5: upgrade peer-hub description : Conor Dooley
>>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram]
>>> v4: Fix the description of peer-hub and update the commit message.
>>> Schematics of the Odroid N2+
>>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf
>>> V3: fix the dt_binding_check error, added new example for Genesys GL3523
>>> v2: added Genesys GL3523 binding
>>> v1: none
>>> ---
>>>  .../bindings/usb/genesys,gl850g.yaml          | 65 +++++++++++++++++--
>>>  1 file changed, 61 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
>>> index ee08b9c3721f..c6f63a69396d 100644
>>> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
>>> @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
>>>  maintainers:
>>>    - Icenowy Zheng <uwu@icenowy.me>
>>>
>>> -allOf:
>>> -  - $ref: usb-device.yaml#
>>> -
>>>  properties:
>>>    compatible:
>>>      enum:
>>> @@ -27,12 +24,46 @@ properties:
>>>
>>>    vdd-supply:
>>>      description:
>>> -      the regulator that provides 3.3V core power to the hub.
>>> +      The regulator that provides 3.3V or 5.0V core power to the hub.
>>> +
>>> +  peer-hub:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description:
>>> +      For onboard hub controllers that support USB 3.x and USB 2.0 hubs
>>> +      with shared resets and power supplies, this property is used to identify
>>> +      the hubs with which these are shared.
>>>
>>>  required:
>>>    - compatible
>>>    - reg
>>>
>>> +allOf:
>>> +  - $ref: usb-device.yaml#
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - usb5e3,608
>>> +    then:
>>> +      properties:
>>> +        peer-hub: false
>>> +        vdd-supply: false
>>> +        reset-gpios: true
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - usb5e3,610
>>> +              - usb5e3,620
>>> +    then:
>>> +      properties:
>>> +        peer-hub: true
>>> +        vdd-supply: true
>>> +        reset-gpios: true
>>
>> No need for this if schema. The default is they are allowed.
>>
> 
> If I move reset-gpios to required, I observe the below warning.
> 
>   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> hub@1: 'reset-gpio' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#

Where are the properties defined? If you open the binding you see:
nowhere. You cannot define properties in some variant with "true".
Please define all of them in top-level and only narrow/constrain when
applicable.


Best regards,
Krzysztof
Anand Moon Dec. 7, 2023, 12:33 p.m. UTC | #4
Hi Krzysztof

On Thu, 7 Dec 2023 at 14:00, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 06/12/2023 18:14, Anand Moon wrote:
> > Hi Rob,
> >
> > On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote:
> >>
> >> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote:
> >>> Add the binding example for the USB3.1 Genesys Logic GL3523
> >>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> >>> hub.
> >>>
> >>> For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> >>> with shared resets and power supplies, this property is used to identify
> >>> the hubs with which these are shared.
> >>>
> >>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves
> >>> power to the USB HUB, it uses 5V power regulator.
> >>>
> >>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >>> ---
> >>> V6: fix the description of the regulators
> >>> Updated the commit message for regulator updates.
> >>> add reviewed by Conor Dooley
> >>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/
> >>> v5: upgrade peer-hub description : Conor Dooley
> >>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram]
> >>> v4: Fix the description of peer-hub and update the commit message.
> >>> Schematics of the Odroid N2+
> >>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf
> >>> V3: fix the dt_binding_check error, added new example for Genesys GL3523
> >>> v2: added Genesys GL3523 binding
> >>> v1: none
> >>> ---
> >>>  .../bindings/usb/genesys,gl850g.yaml          | 65 +++++++++++++++++--
> >>>  1 file changed, 61 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> >>> index ee08b9c3721f..c6f63a69396d 100644
> >>> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> >>> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> >>> @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
> >>>  maintainers:
> >>>    - Icenowy Zheng <uwu@icenowy.me>
> >>>
> >>> -allOf:
> >>> -  - $ref: usb-device.yaml#
> >>> -
> >>>  properties:
> >>>    compatible:
> >>>      enum:
> >>> @@ -27,12 +24,46 @@ properties:
> >>>
> >>>    vdd-supply:
> >>>      description:
> >>> -      the regulator that provides 3.3V core power to the hub.
> >>> +      The regulator that provides 3.3V or 5.0V core power to the hub.
> >>> +
> >>> +  peer-hub:
> >>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>> +    description:
> >>> +      For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> >>> +      with shared resets and power supplies, this property is used to identify
> >>> +      the hubs with which these are shared.
> >>>
> >>>  required:
> >>>    - compatible
> >>>    - reg
> >>>
> >>> +allOf:
> >>> +  - $ref: usb-device.yaml#
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - usb5e3,608
> >>> +    then:
> >>> +      properties:
> >>> +        peer-hub: false
> >>> +        vdd-supply: false
> >>> +        reset-gpios: true
> >>> +
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - usb5e3,610
> >>> +              - usb5e3,620
> >>> +    then:
> >>> +      properties:
> >>> +        peer-hub: true
> >>> +        vdd-supply: true
> >>> +        reset-gpios: true
> >>
> >> No need for this if schema. The default is they are allowed.
> >>
> >
> > If I move reset-gpios to required, I observe the below warning.
> >
> >   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
> > /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> > hub@1: 'reset-gpio' is a required property
> >         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
>
> Where are the properties defined? If you open the binding you see:
> nowhere. You cannot define properties in some variant with "true".
> Please define all of them in top-level and only narrow/constrain when
> applicable.
>
What I meant is the example below, required meant applicable for both
the binding
But it shows me the above warning.

required:
  - compatible
  - reg
  - reset-gpio

allOf:
  - $ref: usb-device.yaml#
  - if:
      properties:
        compatible:
          contains:
            enum:
              - usb5e3,608
    then:
      properties:
        peer-hub: false
        vdd-supply: false

  - if:
      properties:
        compatible:
          contains:
            enum:
              - usb5e3,610
              - usb5e3,620
    then:
      properties:
        peer-hub: true
        vdd-supply: true

additionalProperties: false

>
> Best regards,
> Krzysztof
>

Thanks
-Anand
Krzysztof Kozlowski Dec. 7, 2023, 12:40 p.m. UTC | #5
On 07/12/2023 13:33, Anand Moon wrote:
> Hi Krzysztof
> 
> On Thu, 7 Dec 2023 at 14:00, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 06/12/2023 18:14, Anand Moon wrote:
>>> Hi Rob,
>>>
>>> On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote:
>>>>
>>>> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote:
>>>>> Add the binding example for the USB3.1 Genesys Logic GL3523
>>>>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
>>>>> hub.
>>>>>
>>>>> For onboard hub controllers that support USB 3.x and USB 2.0 hubs
>>>>> with shared resets and power supplies, this property is used to identify
>>>>> the hubs with which these are shared.
>>>>>
>>>>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves
>>>>> power to the USB HUB, it uses 5V power regulator.
>>>>>
>>>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>> ---
>>>>> V6: fix the description of the regulators
>>>>> Updated the commit message for regulator updates.
>>>>> add reviewed by Conor Dooley
>>>>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/
>>>>> v5: upgrade peer-hub description : Conor Dooley
>>>>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram]
>>>>> v4: Fix the description of peer-hub and update the commit message.
>>>>> Schematics of the Odroid N2+
>>>>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf
>>>>> V3: fix the dt_binding_check error, added new example for Genesys GL3523
>>>>> v2: added Genesys GL3523 binding
>>>>> v1: none
>>>>> ---
>>>>>  .../bindings/usb/genesys,gl850g.yaml          | 65 +++++++++++++++++--
>>>>>  1 file changed, 61 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
>>>>> index ee08b9c3721f..c6f63a69396d 100644
>>>>> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
>>>>> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
>>>>> @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
>>>>>  maintainers:
>>>>>    - Icenowy Zheng <uwu@icenowy.me>
>>>>>
>>>>> -allOf:
>>>>> -  - $ref: usb-device.yaml#
>>>>> -
>>>>>  properties:
>>>>>    compatible:
>>>>>      enum:
>>>>> @@ -27,12 +24,46 @@ properties:
>>>>>
>>>>>    vdd-supply:
>>>>>      description:
>>>>> -      the regulator that provides 3.3V core power to the hub.
>>>>> +      The regulator that provides 3.3V or 5.0V core power to the hub.
>>>>> +
>>>>> +  peer-hub:
>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>>>> +    description:
>>>>> +      For onboard hub controllers that support USB 3.x and USB 2.0 hubs
>>>>> +      with shared resets and power supplies, this property is used to identify
>>>>> +      the hubs with which these are shared.
>>>>>
>>>>>  required:
>>>>>    - compatible
>>>>>    - reg
>>>>>
>>>>> +allOf:
>>>>> +  - $ref: usb-device.yaml#
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          contains:
>>>>> +            enum:
>>>>> +              - usb5e3,608
>>>>> +    then:
>>>>> +      properties:
>>>>> +        peer-hub: false
>>>>> +        vdd-supply: false
>>>>> +        reset-gpios: true
>>>>> +
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          contains:
>>>>> +            enum:
>>>>> +              - usb5e3,610
>>>>> +              - usb5e3,620
>>>>> +    then:
>>>>> +      properties:
>>>>> +        peer-hub: true
>>>>> +        vdd-supply: true
>>>>> +        reset-gpios: true
>>>>
>>>> No need for this if schema. The default is they are allowed.
>>>>
>>>
>>> If I move reset-gpios to required, I observe the below warning.
>>>
>>>   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
>>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
>>> hub@1: 'reset-gpio' is a required property
>>>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
>>
>> Where are the properties defined? If you open the binding you see:
>> nowhere. You cannot define properties in some variant with "true".
>> Please define all of them in top-level and only narrow/constrain when
>> applicable.
>>
> What I meant is the example below, required meant applicable for both
> the binding
> But it shows me the above warning.

My explanation stands... So again:

>> Please define all of them in top-level and only narrow/constrain when
>> applicable.

Best regards,
Krzysztof
Anand Moon Dec. 8, 2023, 12:24 a.m. UTC | #6
Hi Krzysztof,

On Thu, 7 Dec 2023 at 18:11, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 07/12/2023 13:33, Anand Moon wrote:
> > Hi Krzysztof
> >
> > On Thu, 7 Dec 2023 at 14:00, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 06/12/2023 18:14, Anand Moon wrote:
> >>> Hi Rob,
> >>>
> >>> On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote:
> >>>>
> >>>> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote:
> >>>>> Add the binding example for the USB3.1 Genesys Logic GL3523
> >>>>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> >>>>> hub.
> >>>>>
> >>>>> For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> >>>>> with shared resets and power supplies, this property is used to identify
> >>>>> the hubs with which these are shared.
> >>>>>
> >>>>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves
> >>>>> power to the USB HUB, it uses 5V power regulator.
> >>>>>
> >>>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >>>>> ---
> >>>>> V6: fix the description of the regulators
> >>>>> Updated the commit message for regulator updates.
> >>>>> add reviewed by Conor Dooley
> >>>>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/
> >>>>> v5: upgrade peer-hub description : Conor Dooley
> >>>>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram]
> >>>>> v4: Fix the description of peer-hub and update the commit message.
> >>>>> Schematics of the Odroid N2+
> >>>>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf
> >>>>> V3: fix the dt_binding_check error, added new example for Genesys GL3523
> >>>>> v2: added Genesys GL3523 binding
> >>>>> v1: none
> >>>>> ---
> >>>>>  .../bindings/usb/genesys,gl850g.yaml          | 65 +++++++++++++++++--
> >>>>>  1 file changed, 61 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> >>>>> index ee08b9c3721f..c6f63a69396d 100644
> >>>>> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> >>>>> @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
> >>>>>  maintainers:
> >>>>>    - Icenowy Zheng <uwu@icenowy.me>
> >>>>>
> >>>>> -allOf:
> >>>>> -  - $ref: usb-device.yaml#
> >>>>> -
> >>>>>  properties:
> >>>>>    compatible:
> >>>>>      enum:
> >>>>> @@ -27,12 +24,46 @@ properties:
> >>>>>
> >>>>>    vdd-supply:
> >>>>>      description:
> >>>>> -      the regulator that provides 3.3V core power to the hub.
> >>>>> +      The regulator that provides 3.3V or 5.0V core power to the hub.
> >>>>> +
> >>>>> +  peer-hub:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>>>> +    description:
> >>>>> +      For onboard hub controllers that support USB 3.x and USB 2.0 hubs
> >>>>> +      with shared resets and power supplies, this property is used to identify
> >>>>> +      the hubs with which these are shared.
> >>>>>
> >>>>>  required:
> >>>>>    - compatible
> >>>>>    - reg
> >>>>>
> >>>>> +allOf:
> >>>>> +  - $ref: usb-device.yaml#
> >>>>> +  - if:
> >>>>> +      properties:
> >>>>> +        compatible:
> >>>>> +          contains:
> >>>>> +            enum:
> >>>>> +              - usb5e3,608
> >>>>> +    then:
> >>>>> +      properties:
> >>>>> +        peer-hub: false
> >>>>> +        vdd-supply: false
> >>>>> +        reset-gpios: true
> >>>>> +
> >>>>> +  - if:
> >>>>> +      properties:
> >>>>> +        compatible:
> >>>>> +          contains:
> >>>>> +            enum:
> >>>>> +              - usb5e3,610
> >>>>> +              - usb5e3,620
> >>>>> +    then:
> >>>>> +      properties:
> >>>>> +        peer-hub: true
> >>>>> +        vdd-supply: true
> >>>>> +        reset-gpios: true
> >>>>
> >>>> No need for this if schema. The default is they are allowed.
> >>>>
> >>>
> >>> If I move reset-gpios to required, I observe the below warning.
> >>>
> >>>   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
> >>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> >>> hub@1: 'reset-gpio' is a required property
> >>>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> >>
> >> Where are the properties defined? If you open the binding you see:
> >> nowhere. You cannot define properties in some variant with "true".
> >> Please define all of them in top-level and only narrow/constrain when
> >> applicable.
> >>
> > What I meant is the example below, required meant applicable for both
> > the binding
> > But it shows me the above warning.
>
> My explanation stands... So again:
>
> >> Please define all of them in top-level and only narrow/constrain when
> >> applicable.
>
Apologies, But I have tried this multiple times but have not been able
to fix the device tree warning
I have verified that example
Documentation/devicetree/bindings/usb/genesys,gl850g.example.dts
generate is correct

required:
  - compatible
  - reg
  - reset-gpio
  - peer-hub
  - vdd-supply

allOf:
  - $ref: usb-device.yaml#
  - if:
      properties:
        compatible:
          contains:
            enum:
              - usb5e3,608
    then:
      properties:
        peer-hub: false
        vdd-supply: false

  - if:
      properties:
        compatible:
          contains:
            enum:
              - usb5e3,610
              - usb5e3,620
    then:
      properties:
        peer-hub: true
        vdd-supply: true

additionalProperties: false

> Best regards,
> Krzysztof
>

Thanks
-Anand
Krzysztof Kozlowski Dec. 8, 2023, 7:44 a.m. UTC | #7
On 08/12/2023 01:24, Anand Moon wrote:
>>>>>
>>>>> If I move reset-gpios to required, I observe the below warning.
>>>>>
>>>>>   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
>>>>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
>>>>> hub@1: 'reset-gpio' is a required property
>>>>>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
>>>>
>>>> Where are the properties defined? If you open the binding you see:
>>>> nowhere. You cannot define properties in some variant with "true".
>>>> Please define all of them in top-level and only narrow/constrain when
>>>> applicable.
>>>>
>>> What I meant is the example below, required meant applicable for both
>>> the binding
>>> But it shows me the above warning.
>>
>> My explanation stands... So again:
>>
>>>> Please define all of them in top-level and only narrow/constrain when
>>>> applicable.
>>
> Apologies, But I have tried this multiple times but have not been able
> to fix the device tree warning

Did you document all properties in top-level "properties:" block?


Best regards,
Krzysztof
Anand Moon Dec. 8, 2023, 11:19 a.m. UTC | #8
Hi Krzysztof,

On Fri, 8 Dec 2023 at 13:14, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/12/2023 01:24, Anand Moon wrote:
> >>>>>
> >>>>> If I move reset-gpios to required, I observe the below warning.
> >>>>>
> >>>>>   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
> >>>>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> >>>>> hub@1: 'reset-gpio' is a required property
> >>>>>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> >>>>
> >>>> Where are the properties defined? If you open the binding you see:
> >>>> nowhere. You cannot define properties in some variant with "true".
> >>>> Please define all of them in top-level and only narrow/constrain when
> >>>> applicable.
> >>>>
> >>> What I meant is the example below, required meant applicable for both
> >>> the binding
> >>> But it shows me the above warning.
> >>
> >> My explanation stands... So again:
> >>
> >>>> Please define all of them in top-level and only narrow/constrain when
> >>>> applicable.
> >>
> > Apologies, But I have tried this multiple times but have not been able
> > to fix the device tree warning
>
> Did you document all properties in top-level "properties:" block?
>
Yes, I have,

Can you suggest a couple of examples to follow?
I looked at some of the YAML files but could not fix my issue.

Thanks
-Anand
Krzysztof Kozlowski Dec. 8, 2023, 12:17 p.m. UTC | #9
On 08/12/2023 12:19, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Fri, 8 Dec 2023 at 13:14, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 08/12/2023 01:24, Anand Moon wrote:
>>>>>>>
>>>>>>> If I move reset-gpios to required, I observe the below warning.
>>>>>>>
>>>>>>>   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
>>>>>>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
>>>>>>> hub@1: 'reset-gpio' is a required property
>>>>>>>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
>>>>>>
>>>>>> Where are the properties defined? If you open the binding you see:
>>>>>> nowhere. You cannot define properties in some variant with "true".
>>>>>> Please define all of them in top-level and only narrow/constrain when
>>>>>> applicable.
>>>>>>
>>>>> What I meant is the example below, required meant applicable for both
>>>>> the binding
>>>>> But it shows me the above warning.
>>>>
>>>> My explanation stands... So again:
>>>>
>>>>>> Please define all of them in top-level and only narrow/constrain when
>>>>>> applicable.
>>>>
>>> Apologies, But I have tried this multiple times but have not been able
>>> to fix the device tree warning
>>
>> Did you document all properties in top-level "properties:" block?
>>
> Yes, I have,
> 
> Can you suggest a couple of examples to follow?
> I looked at some of the YAML files but could not fix my issue.

99% of bindings. Look also at example-schema.

You can also attach here complete patch for fast look / short review.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 12, 2023, 8:40 a.m. UTC | #10
On 12/12/2023 07:22, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Fri, 8 Dec 2023 at 17:47, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 08/12/2023 12:19, Anand Moon wrote:
>>> Hi Krzysztof,
>>>
>>> On Fri, 8 Dec 2023 at 13:14, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 08/12/2023 01:24, Anand Moon wrote:
>>>>>>>>>
>>>>>>>>> If I move reset-gpios to required, I observe the below warning.
>>>>>>>>>
>>>>>>>>>   DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
>>>>>>>>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
>>>>>>>>> hub@1: 'reset-gpio' is a required property
>>>>>>>>>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
>>>>>>>>
>>>>>>>> Where are the properties defined? If you open the binding you see:
>>>>>>>> nowhere. You cannot define properties in some variant with "true".
>>>>>>>> Please define all of them in top-level and only narrow/constrain when
>>>>>>>> applicable.
>>>>>>>>
>>>>>>> What I meant is the example below, required meant applicable for both
>>>>>>> the binding
>>>>>>> But it shows me the above warning.
>>>>>>
>>>>>> My explanation stands... So again:
>>>>>>
>>>>>>>> Please define all of them in top-level and only narrow/constrain when
>>>>>>>> applicable.
>>>>>>
>>>>> Apologies, But I have tried this multiple times but have not been able
>>>>> to fix the device tree warning
>>>>
>>>> Did you document all properties in top-level "properties:" block?
>>>>
>>> Yes, I have,
>>>
>>> Can you suggest a couple of examples to follow?
>>> I looked at some of the YAML files but could not fix my issue.
>>
>> 99% of bindings. Look also at example-schema.
>>
>> You can also attach here complete patch for fast look / short review.
>>
> 
> Please find the modified patch, I have tried a few things but none
> resolve the binding warning.
> I am not able to debug this.

And where is the warning or the issue? Can you describe what problem do
you have?

> 
> -Thanks
> Anand
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> index ee08b9c3721f..7f75fa3c1945 100644
> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
>  maintainers:
>    - Icenowy Zheng <uwu@icenowy.me>
> 
> -allOf:
> -  - $ref: usb-device.yaml#
> -
>  properties:
>    compatible:
>      enum:
> @@ -27,11 +24,47 @@ properties:
> 
>    vdd-supply:
>      description:
> -      the regulator that provides 3.3V core power to the hub.
> +      The regulator that provides 3.3V or 5.0V core power to the hub.
> +
> +  peer-hub:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      For onboard hub controllers that support USB 3.x and USB 2.0 hubs with
> +      shared resets and power supplies, this property is used to identify the
> +      hubs with which these are shared.
> 
>  required:
>    - compatible
>    - reg
> +  - vdd-supply
> +  - reset-gpios
> +  - peer-hub
> +
> +allOf:
> +  - $ref: usb-device.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - usb5e3,608
> +    then:
> +      properties:
> +        peer-hub: false
> +        vdd-supply: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - usb5e3,610
> +              - usb5e3,620
> +    then:
> +      properties:
> +        peer-hub: true
> +        vdd-supply: true

Drop this if:, redundant.


Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 12, 2023, 11:52 a.m. UTC | #11
On 12/12/2023 12:37, Anand Moon wrote:
> 
> Here is the list of warnings I observed with this patch
> 
>   DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra186-xusb.example.dtb
> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> hub@1: 'vdd-supply' is a required property

You always require the property, but it is not valid for some devices.
Just require it only where it is applicable (in if:then: clause).


>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> hub@1: 'reset-gpios' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> hub@1: 'peer-hub' is a required property

...

>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - usb5e3,610
>>> +              - usb5e3,620
>>> +    then:
>>> +      properties:
>>> +        peer-hub: true
>>> +        vdd-supply: true
>>
>> Drop this if:, redundant.
>>
> No, this does not resolve the above issue.

It shouldn't resolve it, not related.


Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 12, 2023, 1:09 p.m. UTC | #12
On 12/12/2023 13:51, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Tue, 12 Dec 2023 at 17:22, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 12/12/2023 12:37, Anand Moon wrote:
>>>
>>> Here is the list of warnings I observed with this patch
>>>
>>>   DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra186-xusb.example.dtb
>>> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
>>> hub@1: 'vdd-supply' is a required property
>>
>> You always require the property, but it is not valid for some devices.
>> Just require it only where it is applicable (in if:then: clause).
>>
> I had already done this check many times before.

I don't ask you to check. I ask you to change the code.

> my v6 original patch was doing the same and it passed all the tests
> but since I updated the required field it not parsing correctly.

Your original v6 patch was different. I don't understand what you are
trying to achieve. Or rather: how is it different, that my simple advice
above does not work for you  (as in the past you reply with some really
unrelated sentence).

Best regards,
Krzysztof
Anand Moon April 4, 2024, 4:27 a.m. UTC | #13
Hi Krzysztof,

On Tue, 12 Dec 2023 at 18:47, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Krzysztof,
>
> On Tue, 12 Dec 2023 at 18:39, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 12/12/2023 13:51, Anand Moon wrote:
> > > Hi Krzysztof,
> > >
> > > On Tue, 12 Dec 2023 at 17:22, Krzysztof Kozlowski
> > > <krzysztof.kozlowski@linaro.org> wrote:
> > >>
> > >> On 12/12/2023 12:37, Anand Moon wrote:
> > >>>
> > >>> Here is the list of warnings I observed with this patch
> > >>>
> > >>>   DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra186-xusb.example.dtb
> > >>> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> > >>> hub@1: 'vdd-supply' is a required property
> > >>
> > >> You always require the property, but it is not valid for some devices.
> > >> Just require it only where it is applicable (in if:then: clause).
> > >>
> > > I had already done this check many times before.
> >
> > I don't ask you to check. I ask you to change the code.
> >
> I have tried this and it's not working for me.
>
> > > my v6 original patch was doing the same and it passed all the tests
> > > but since I updated the required field it not parsing correctly.
> >
> > Your original v6 patch was different. I don't understand what you are
> > trying to achieve. Or rather: how is it different, that my simple advice
> > above does not work for you  (as in the past you reply with some really
> > unrelated sentence).
> >
> Ok, It's my poor English grammar, thanks for your review comments.
>
> > Best regards,
> > Krzysztof
> >

Any reason this device tree binding got removed,I cannot find this file
Can not find the commit which removed this file.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/usb?h=v6.9-rc2

Thanks
-Anand
Krzysztof Kozlowski April 4, 2024, 6:12 a.m. UTC | #14
On 04/04/2024 06:27, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Tue, 12 Dec 2023 at 18:47, Anand Moon <linux.amoon@gmail.com> wrote:
>>
>> Hi Krzysztof,
>>
>> On Tue, 12 Dec 2023 at 18:39, Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 12/12/2023 13:51, Anand Moon wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On Tue, 12 Dec 2023 at 17:22, Krzysztof Kozlowski
>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>
>>>>> On 12/12/2023 12:37, Anand Moon wrote:
>>>>>>
>>>>>> Here is the list of warnings I observed with this patch
>>>>>>
>>>>>>   DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra186-xusb.example.dtb
>>>>>> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
>>>>>> hub@1: 'vdd-supply' is a required property
>>>>>
>>>>> You always require the property, but it is not valid for some devices.
>>>>> Just require it only where it is applicable (in if:then: clause).
>>>>>
>>>> I had already done this check many times before.
>>>
>>> I don't ask you to check. I ask you to change the code.
>>>
>> I have tried this and it's not working for me.
>>
>>>> my v6 original patch was doing the same and it passed all the tests
>>>> but since I updated the required field it not parsing correctly.
>>>
>>> Your original v6 patch was different. I don't understand what you are
>>> trying to achieve. Or rather: how is it different, that my simple advice
>>> above does not work for you  (as in the past you reply with some really
>>> unrelated sentence).
>>>
>> Ok, It's my poor English grammar, thanks for your review comments.
>>
>>> Best regards,
>>> Krzysztof
>>>
> 
> Any reason this device tree binding got removed,I cannot find this file
> Can not find the commit which removed this file.

Use git log.

Best regards,
Krzysztof
Anand Moon April 4, 2024, 6:32 a.m. UTC | #15
Hi,

On Thu, 4 Apr 2024 at 11:42, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/04/2024 06:27, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > On Tue, 12 Dec 2023 at 18:47, Anand Moon <linux.amoon@gmail.com> wrote:
> >>
> >> Hi Krzysztof,
> >>
> >> On Tue, 12 Dec 2023 at 18:39, Krzysztof Kozlowski
> >> <krzysztof.kozlowski@linaro.org> wrote:
> >>>
> >>> On 12/12/2023 13:51, Anand Moon wrote:
> >>>> Hi Krzysztof,
> >>>>
> >>>> On Tue, 12 Dec 2023 at 17:22, Krzysztof Kozlowski
> >>>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>>
> >>>>> On 12/12/2023 12:37, Anand Moon wrote:
> >>>>>>
> >>>>>> Here is the list of warnings I observed with this patch
> >>>>>>
> >>>>>>   DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra186-xusb.example.dtb
> >>>>>> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> >>>>>> hub@1: 'vdd-supply' is a required property
> >>>>>
> >>>>> You always require the property, but it is not valid for some devices.
> >>>>> Just require it only where it is applicable (in if:then: clause).
> >>>>>
> >>>> I had already done this check many times before.
> >>>
> >>> I don't ask you to check. I ask you to change the code.
> >>>
> >> I have tried this and it's not working for me.
> >>
> >>>> my v6 original patch was doing the same and it passed all the tests
> >>>> but since I updated the required field it not parsing correctly.
> >>>
> >>> Your original v6 patch was different. I don't understand what you are
> >>> trying to achieve. Or rather: how is it different, that my simple advice
> >>> above does not work for you  (as in the past you reply with some really
> >>> unrelated sentence).
> >>>
> >> Ok, It's my poor English grammar, thanks for your review comments.
> >>
> >>> Best regards,
> >>> Krzysztof
> >>>
> >
> > Any reason this device tree binding got removed,I cannot find this file
> > Can not find the commit which removed this file.
>
> Use git log.
>
> Best regards,
> Krzysztof
>
Anand Moon April 4, 2024, 6:34 a.m. UTC | #16
Hi Krzysztof,

On Thu, 4 Apr 2024 at 12:02, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi,
>
> On Thu, 4 Apr 2024 at 11:42, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 04/04/2024 06:27, Anand Moon wrote:
> > > Hi Krzysztof,
> > >
> > > On Tue, 12 Dec 2023 at 18:47, Anand Moon <linux.amoon@gmail.com> wrote:
> > >>
> > >> Hi Krzysztof,
> > >>
> > >> On Tue, 12 Dec 2023 at 18:39, Krzysztof Kozlowski
> > >> <krzysztof.kozlowski@linaro.org> wrote:
> > >>>
> > >>> On 12/12/2023 13:51, Anand Moon wrote:
> > >>>> Hi Krzysztof,
> > >>>>
> > >>>> On Tue, 12 Dec 2023 at 17:22, Krzysztof Kozlowski
> > >>>> <krzysztof.kozlowski@linaro.org> wrote:
> > >>>>>
> > >>>>> On 12/12/2023 12:37, Anand Moon wrote:
> > >>>>>>
> > >>>>>> Here is the list of warnings I observed with this patch
> > >>>>>>
> > >>>>>>   DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra186-xusb.example.dtb
> > >>>>>> /home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> > >>>>>> hub@1: 'vdd-supply' is a required property
> > >>>>>
> > >>>>> You always require the property, but it is not valid for some devices.
> > >>>>> Just require it only where it is applicable (in if:then: clause).
> > >>>>>
> > >>>> I had already done this check many times before.
> > >>>
> > >>> I don't ask you to check. I ask you to change the code.
> > >>>
> > >> I have tried this and it's not working for me.
> > >>
> > >>>> my v6 original patch was doing the same and it passed all the tests
> > >>>> but since I updated the required field it not parsing correctly.
> > >>>
> > >>> Your original v6 patch was different. I don't understand what you are
> > >>> trying to achieve. Or rather: how is it different, that my simple advice
> > >>> above does not work for you  (as in the past you reply with some really
> > >>> unrelated sentence).
> > >>>
> > >> Ok, It's my poor English grammar, thanks for your review comments.
> > >>
> > >>> Best regards,
> > >>> Krzysztof
> > >>>
> > >
> > > Any reason this device tree binding got removed,I cannot find this file
> > > Can not find the commit which removed this file.
> >
> > Use git log.
> >

I got confused with the file name and my local changes.

> > Best regards,
> > Krzysztof
> >
Thanks
-Anand
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
index ee08b9c3721f..c6f63a69396d 100644
--- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
+++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
@@ -9,9 +9,6 @@  title: Genesys Logic USB hub controller
 maintainers:
   - Icenowy Zheng <uwu@icenowy.me>
 
-allOf:
-  - $ref: usb-device.yaml#
-
 properties:
   compatible:
     enum:
@@ -27,12 +24,46 @@  properties:
 
   vdd-supply:
     description:
-      the regulator that provides 3.3V core power to the hub.
+      The regulator that provides 3.3V or 5.0V core power to the hub.
+
+  peer-hub:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      For onboard hub controllers that support USB 3.x and USB 2.0 hubs
+      with shared resets and power supplies, this property is used to identify
+      the hubs with which these are shared.
 
 required:
   - compatible
   - reg
 
+allOf:
+  - $ref: usb-device.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - usb5e3,608
+    then:
+      properties:
+        peer-hub: false
+        vdd-supply: false
+        reset-gpios: true
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - usb5e3,610
+              - usb5e3,620
+    then:
+      properties:
+        peer-hub: true
+        vdd-supply: true
+        reset-gpios: true
+
 additionalProperties: false
 
 examples:
@@ -49,3 +80,29 @@  examples:
             reset-gpios = <&pio 7 2 GPIO_ACTIVE_LOW>;
         };
     };
+
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    usb {
+        dr_mode = "host";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        /* 2.0 hub on port 1 */
+        hub_2_0: hub@1 {
+            compatible = "usb5e3,610";
+            reg = <1>;
+            peer-hub = <&hub_3_0>;
+            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&vcc_5v>;
+        };
+
+        /* 3.1 hub on port 4 */
+        hub_3_0: hub@2 {
+            compatible = "usb5e3,620";
+            reg = <2>;
+            peer-hub = <&hub_2_0>;
+            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&vcc_5v>;
+        };
+    };