[PATCH 0/4] dt-bindings: gpio: aspeed,sgpio: Miscellaneous cleanups

[PATCH 0/4] dt-bindings: gpio: aspeed,sgpio: Miscellaneous cleanups

[Andrew Jeffery]

Hello,

This short series fixes some SGPIO-related devicetree warnings currently
emitted by `make dtbs_check` for Aspeed devicetrees.

One change documents `#interrupt-cells` and a subseqent change makes
it required. The property should have been both documented and marked
as required from the start. As the change is technically not backwards
compatible, I've split it such that we can debate the required status
separately.

Please review!

Andrew

---
Andrew Jeffery (4):
      dt-bindings: gpio: aspeed,sgpio: Order properties by DTS style
      dt-bindings: gpio: aspeed,sgpio: Specify gpio-line-names
      dt-bindings: gpio: aspeed,sgpio: Specify #interrupt-cells
      dt-bindings: gpio: aspeed,sgpio: Require #interrupt-cells

 .../devicetree/bindings/gpio/aspeed,sgpio.yaml     | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240528-dt-warnings-gpio-sgpio-interrupt-cells-685a0efd2c0b

Best regards,
-- 
Andrew Jeffery <andrew@codeconstruct.com.au>

[PATCH 1/4] dt-bindings: gpio: aspeed,sgpio: Order properties by DTS style

[Andrew Jeffery]

Tidy up the list of required properties and the example node by ordering
the properties in terms of the DTS coding style.

Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
 Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
index 46bb121360dc..6b15a3a3fb66 100644
--- a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
@@ -51,12 +51,12 @@ properties:
 required:
   - compatible
   - reg
-  - gpio-controller
-  - '#gpio-cells'
+  - clocks
   - interrupts
   - interrupt-controller
+  - gpio-controller
+  - '#gpio-cells'
   - ngpios
-  - clocks
   - bus-frequency
 
 additionalProperties: false
@@ -65,13 +65,13 @@ examples:
   - |
     #include <dt-bindings/clock/aspeed-clock.h>
     sgpio: sgpio@1e780200 {
-        #gpio-cells = <2>;
         compatible = "aspeed,ast2500-sgpio";
-        gpio-controller;
-        interrupts = <40>;
         reg = <0x1e780200 0x0100>;
         clocks = <&syscon ASPEED_CLK_APB>;
+        interrupts = <40>;
         interrupt-controller;
+        gpio-controller;
+        #gpio-cells = <2>;
         ngpios = <80>;
         bus-frequency = <12000000>;
     };

-- 
2.39.2

[PATCH 2/4] dt-bindings: gpio: aspeed,sgpio: Specify gpio-line-names

[Andrew Jeffery]

Some devicetrees specify gpio-line-names in the sgpio node despite it
not being defined by the binding. It's a reasonable thing to do, so
define the property to squash warnings such as:

```
arch/arm/boot/dts/aspeed/aspeed-bmc-vegman-rx20.dtb: sgpio@1e780200: 'gpio-line-names' does not match any of the regexes: 'pinctrl-[0-9]+'
```

Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
 Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
index 6b15a3a3fb66..fe27b5d01e80 100644
--- a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
@@ -33,6 +33,11 @@ properties:
 
   gpio-controller: true
 
+  # Each SGPIO is represented as a pair of input and output GPIOs
+  gpio-line-names:
+    minItems: 160
+    maxItems: 256
+
   '#gpio-cells':
     const: 2
 

-- 
2.39.2

[PATCH 3/4] dt-bindings: gpio: aspeed,sgpio: Specify #interrupt-cells

[Andrew Jeffery]

Squash warnings such as:

```
/home/andrew/src/kernel.org/linux/build.aspeed_g5/arch/arm/boot/dts/aspeed/aspeed-ast2500-evb.dtb: sgpio@1e780200: '#interrupt-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
```

Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
 Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
index fe27b5d01e80..02c02ef97565 100644
--- a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
@@ -46,6 +46,9 @@ properties:
 
   interrupt-controller: true
 
+  '#interrupt-cells':
+    const: 2
+
   clocks:
     maxItems: 1
 
@@ -75,6 +78,7 @@ examples:
         clocks = <&syscon ASPEED_CLK_APB>;
         interrupts = <40>;
         interrupt-controller;
+        #interrupt-cells = <2>;
         gpio-controller;
         #gpio-cells = <2>;
         ngpios = <80>;

-- 
2.39.2

[PATCH 4/4] dt-bindings: gpio: aspeed,sgpio: Require #interrupt-cells

[Andrew Jeffery]

It shouldn't have been the case that it wasn't required. The kernel
devicetrees already specified it where compatible nodes were defined,
and u-boot pulls in the kernel devicetrees, so this should have minimal
practical impact.

Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
 Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
index 02c02ef97565..433b50bd5484 100644
--- a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
@@ -62,6 +62,7 @@ required:
   - clocks
   - interrupts
   - interrupt-controller
+  - '#interrupt-cells'
   - gpio-controller
   - '#gpio-cells'
   - ngpios

-- 
2.39.2

Re: [PATCH 1/4] dt-bindings: gpio: aspeed,sgpio: Order properties by DTS style

[Krzysztof Kozlowski]

On 29/05/2024 07:13, Andrew Jeffery wrote:
> Tidy up the list of required properties and the example node by ordering
> the properties in terms of the DTS coding style.
> 
> Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
> ---
>  Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> index 46bb121360dc..6b15a3a3fb66 100644
> --- a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> @@ -51,12 +51,12 @@ properties:
>  required:
>    - compatible
>    - reg
> -  - gpio-controller
> -  - '#gpio-cells'
> +  - clocks
>    - interrupts
>    - interrupt-controller
> +  - gpio-controller
> +  - '#gpio-cells'
>    - ngpios
> -  - clocks
>    - bus-frequency

No, this should have the same order as properties are listed.

Don't change it.

>  
>  additionalProperties: false
> @@ -65,13 +65,13 @@ examples:
>    - |
>      #include <dt-bindings/clock/aspeed-clock.h>
>      sgpio: sgpio@1e780200 {
> -        #gpio-cells = <2>;
>          compatible = "aspeed,ast2500-sgpio";
> -        gpio-controller;
> -        interrupts = <40>;
>          reg = <0x1e780200 0x0100>;
>          clocks = <&syscon ASPEED_CLK_APB>;
> +        interrupts = <40>;
>          interrupt-controller;
> +        gpio-controller;
> +        #gpio-cells = <2>;

That's just example. I don't find this change useful but churn.

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: gpio: aspeed,sgpio: Specify gpio-line-names

[Krzysztof Kozlowski]

On 29/05/2024 07:13, Andrew Jeffery wrote:
> Some devicetrees specify gpio-line-names in the sgpio node despite it
> not being defined by the binding. It's a reasonable thing to do, so
> define the property to squash warnings such as:
> 
> ```

No need for ```

This is just commit log. Not markdown.


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 3/4] dt-bindings: gpio: aspeed,sgpio: Specify #interrupt-cells

[Krzysztof Kozlowski]

On 29/05/2024 07:13, Andrew Jeffery wrote:
> Squash warnings such as:
> 
> ```
> /home/andrew/src/kernel.org/linux/build.aspeed_g5/arch/arm/boot/dts/aspeed/aspeed-ast2500-evb.dtb: sgpio@1e780200: '#interrupt-cells' does not match any of the regexes: 'pinctrl-[0-9]+'

Please trim the log from unnecessary path components.

Best regards,
Krzysztof

Re: [PATCH 4/4] dt-bindings: gpio: aspeed,sgpio: Require #interrupt-cells

[Krzysztof Kozlowski]

On 29/05/2024 07:13, Andrew Jeffery wrote:
> It shouldn't have been the case that it wasn't required. The kernel
> devicetrees already specified it where compatible nodes were defined,
> and u-boot pulls in the kernel devicetrees, so this should have minimal
> practical impact.
> 

This should be squashed with previous patch.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: gpio: aspeed,sgpio: Order properties by DTS style

[Andrew Jeffery]

On Wed, 2024-05-29 at 09:26 +0200, Krzysztof Kozlowski wrote:
> On 29/05/2024 07:13, Andrew Jeffery wrote:
> > Tidy up the list of required properties and the example node by ordering
> > the properties in terms of the DTS coding style.
> > 
> > Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
> > ---
> >  Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> > index 46bb121360dc..6b15a3a3fb66 100644
> > --- a/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> > +++ b/Documentation/devicetree/bindings/gpio/aspeed,sgpio.yaml
> > @@ -51,12 +51,12 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > -  - gpio-controller
> > -  - '#gpio-cells'
> > +  - clocks
> >    - interrupts
> >    - interrupt-controller
> > +  - gpio-controller
> > +  - '#gpio-cells'
> >    - ngpios
> > -  - clocks
> >    - bus-frequency
> 
> No, this should have the same order as properties are listed.
> 
> Don't change it.

Ack.

> 
> >  
> >  additionalProperties: false
> > @@ -65,13 +65,13 @@ examples:
> >    - |
> >      #include <dt-bindings/clock/aspeed-clock.h>
> >      sgpio: sgpio@1e780200 {
> > -        #gpio-cells = <2>;
> >          compatible = "aspeed,ast2500-sgpio";
> > -        gpio-controller;
> > -        interrupts = <40>;
> >          reg = <0x1e780200 0x0100>;
> >          clocks = <&syscon ASPEED_CLK_APB>;
> > +        interrupts = <40>;
> >          interrupt-controller;
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> 
> That's just example. I don't find this change useful but churn.

Sure, I'll drop the patch.

Thanks,

Andrew

Re: [PATCH 2/4] dt-bindings: gpio: aspeed,sgpio: Specify gpio-line-names

[Andrew Jeffery]

On Wed, 2024-05-29 at 09:27 +0200, Krzysztof Kozlowski wrote:
> On 29/05/2024 07:13, Andrew Jeffery wrote:
> > Some devicetrees specify gpio-line-names in the sgpio node despite it
> > not being defined by the binding. It's a reasonable thing to do, so
> > define the property to squash warnings such as:
> > 
> > ```
> 
> No need for ```
> 
> This is just commit log. Not markdown.

I'm going to do a v2 anyway, so I'll drop the markup.

> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks,

Andrew

Re: [PATCH 3/4] dt-bindings: gpio: aspeed,sgpio: Specify #interrupt-cells

[Andrew Jeffery]

On Wed, 2024-05-29 at 09:28 +0200, Krzysztof Kozlowski wrote:
> On 29/05/2024 07:13, Andrew Jeffery wrote:
> > Squash warnings such as:
> > 
> > ```
> > /home/andrew/src/kernel.org/linux/build.aspeed_g5/arch/arm/boot/dts/aspeed/aspeed-ast2500-evb.dtb: sgpio@1e780200: '#interrupt-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Please trim the log from unnecessary path components.

Ack.

Re: [PATCH 4/4] dt-bindings: gpio: aspeed,sgpio: Require #interrupt-cells

[Andrew Jeffery]

On Wed, 2024-05-29 at 09:28 +0200, Krzysztof Kozlowski wrote:
> On 29/05/2024 07:13, Andrew Jeffery wrote:
> > It shouldn't have been the case that it wasn't required. The kernel
> > devicetrees already specified it where compatible nodes were defined,
> > and u-boot pulls in the kernel devicetrees, so this should have minimal
> > practical impact.
> > 
> 
> This should be squashed with previous patch.
> 

Sure, will do.

Andrew