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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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