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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a50aaebb-eb44-4e49-aea0-7b7aa86cd157@axiado.com>
Date: Wed, 18 Jun 2025 21:09:44 +0000
From: Harshit Shah <hshah@...ado.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
	<brgl@...ev.pl>, Arnd Bergmann <arnd@...db.de>, Catalin Marinas
	<catalin.marinas@....com>, Will Deacon <will@...nel.org>
CC: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-gpio@...r.kernel.org"
	<linux-gpio@...r.kernel.org>, "soc@...ts.linux.dev" <soc@...ts.linux.dev>
Subject: Re: [PATCH v2 3/6] dt-bindings: gpio: gpio-cdns: convert to YAML

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@...ado.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?