Re: [PATCH v2 3/6] dt-bindings: gpio: gpio-cdns: convert to YAML

[Harshit Shah <hshah@axiado.com>]

On 6/15/2025 11:05 PM, Krzysztof Kozlowski wrote:
> On 16/06/2025 06:31, Harshit Shah wrote:
>> Convert Cadence family GPIO controller bindings to DT schema.
>>
>> Changes during conversion:
>>     - update the naming as per the other files.
> You made it entirely different than every other file and review.
Got it. I will update it to "cdns,gpio.yaml"
>
>>     - add gpio maintainers
> Not really, you need to find someone interested in that hardware, not
> subsystem maintainers.
Understood. I have connected with the original author and I will update 
the same.
>
>> Signed-off-by: Harshit Shah <hshah@axiado.com>
>> ---
>>   .../devicetree/bindings/gpio/cdns,gpio.txt         | 43 ------------
>>   .../devicetree/bindings/gpio/gpio-cdns.yaml        | 81 ++++++++++++++++++++++
> Previous filename was correct.
Got it. I will update it to "cdns,gpio.yaml"
>
>>   2 files changed, 81 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
>> deleted file mode 100644
>> index 706ef00f5c64951bb29c79a5541db4397e8b2733..0000000000000000000000000000000000000000
>> --- a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -Cadence GPIO controller bindings
>> -
>>
>> +properties:
>> +  compatible:
>> +    const: cdns,gpio-r1p02
>> +
>> +  reg:
>> +    minItems: 1
> Why this is min and unconstrained...
Make sense. I will update it to maxItems: 1
>
>> +
>> +  clocks:
>> +    maxItems: 1
> But this is max?
>
> maxItems: 1 in both cases (so clocks are correct, but why writing
> similar things entirely different?).
Yes will keep it to maxItems: 1
>
>> +
>> +  ngpios:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Number of GPIO lines supported, maximum 32.
>> +
>> +  gpio-controller: true
>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +    description: |
>> +      - First cell is the GPIO line number.
>> +      - Second cell is flags as defined in <dt-bindings/gpio/gpio.h>,
>> +        only GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW supported.
>> +
>> +  interrupt-controller: true
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +    description: |
>> +      - First cell is the GPIO line number used as IRQ.
>> +      - Second cell is the trigger type, as defined in
>> +        <dt-bindings/interrupt-controller/irq.h>.
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - gpio-controller
>> +  - "#gpio-cells"
>> +
>> +if:
>> +  required: [interrupt-controller]
>> +then:
>> +  required:
>> +    - interrupts
>> +    - "#interrupt-cells"
> Drop last one, core schema requires it.
Got it. I will remove the "#interrupt-cells".
>
>
> Best regards,
> Krzysztof

Thank you for the review, I will update as per the above comments.

I have verified that the dt_binding_check is passing without errors.
However, check_patch.pl is giving warning about the DT binding docs and includes.

checkpatch.pl: dev/null:10: WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst
checkpatch.pl: Documentation/devicetree/bindings/gpio/gpio-cdns.yaml:-1: WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst

I am unsure about how to split files as suggested, could you please advise?