Re: [PATCH v2] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

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@d0018100 {

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@18140 {

Best regards,
Krzysztof