linux-kernel - Re: [PATCH v2] dt-bindings: gpio: gpio-mvebu: convert txt binding 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] [day] [month] [year] [list]

Message-ID: <59d23590-0b1e-39f0-80f1-d875081a276c@linaro.org>
Date:   Wed, 11 May 2022 17:55:55 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Chris Packham <chris.packham@...iedtelesis.co.nz>,
        linus.walleij@...aro.org, brgl@...ev.pl, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, thierry.reding@...il.com,
        u.kleine-koenig@...gutronix.de, lee.jones@...aro.org
Cc:     linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v2] dt-bindings: gpio: gpio-mvebu: convert txt binding to
 YAML

On 11/05/2022 03:37, Chris Packham wrote:
> Convert the existing device tree binding to YAML format.
> 
> The old binding listed the interrupt-controller and related properties
> as required but there are sufficiently many existing usages without it
> that the YAML binding does not make the interrupt properties required.
> 
> The offset and marvell,pwm-offset properties weren't in the old binding
> and are added to the YAML binding. The offset property is required when
> the marvell,armada-8k-gpio compatible is used.

These properties do not look correct. It's some hacky design. As I see
in the driver, there is no reason to model the gpio under the syscon at
all. The GPIO has its own address space, which is for example in
armada-ap80x.dtsi 0x6f4000+0x1040.

Instead of describing it as a separate device under that address,
someone created a syscon node for entire address space, put the GPIO as
a fake child and added some new property "offset" indicating address
offset. Wait, what, why?

Why this cannot be a child of SoC, just like all other nodes are?

Since this is a conversion and offset was never previously accepted in
the bindings, it has to go to separate patch where you will need to get
Rob's ack on documenting offset.

(...)

> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +
> +unevaluatedProperties: true
> +
> +examples:
> +  - |
> +      gpio@...18100 {

Wrong indentation. See example schema.

> +        compatible = "marvell,armadaxp-gpio", "marvell,orion-gpio";
> +        reg = <0xd0018100 0x40>, <0xd0018800 0x30>;
> +        ngpios = <32>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        interrupt-controller;
> +        #interrupt-cells = <2>;
> +        interrupts = <16>, <17>, <18>, <19>;
> +      };
> +
> +  - |
> +      gpio@...40 {

Best regards,
Krzysztof