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

From: Krzysztof Kozlowski
Date: Tue Dec 12 2023 - 03:40:40 EST


On 12/12/2023 07:22, Anand Moon wrote:
> Hi Krzysztof,
>
> On Fri, 8 Dec 2023 at 17:47, Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>>
>> On 08/12/2023 12:19, Anand Moon wrote:
>>> Hi Krzysztof,
>>>
>>> On Fri, 8 Dec 2023 at 13:14, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> -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