[PATCH] dt-bindings: pinctrl: aspeed: Drop referenced nodes in examples

Rob Herring posted 1 patch 4 years ago
.../pinctrl/aspeed,ast2500-pinctrl.yaml       | 81 ++++---------------
1 file changed, 16 insertions(+), 65 deletions(-)
[PATCH] dt-bindings: pinctrl: aspeed: Drop referenced nodes in examples
Posted by Rob Herring 4 years ago
The additional nodes in the example referenced from the pinctrl node
'aspeed,external-nodes' properties are either incorrect (aspeed,ast2500-lpc)
or not documented with a schema (aspeed,ast2500-gfx). There's no need to
show these nodes as part of the pinctrl example, so just remove them.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../pinctrl/aspeed,ast2500-pinctrl.yaml       | 81 ++++---------------
 1 file changed, 16 insertions(+), 65 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
index 7c25c8d51116..9db904a528ee 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
@@ -76,73 +76,24 @@ additionalProperties: false
 examples:
   - |
     #include <dt-bindings/clock/aspeed-clock.h>
-    apb {
-        compatible = "simple-bus";
-        #address-cells = <1>;
-        #size-cells = <1>;
-        ranges;
-
-        syscon: scu@1e6e2000 {
-            compatible = "aspeed,ast2500-scu", "syscon", "simple-mfd";
-            reg = <0x1e6e2000 0x1a8>;
-            #clock-cells = <1>;
-            #reset-cells = <1>;
-
-            pinctrl: pinctrl {
-                compatible = "aspeed,ast2500-pinctrl";
-                aspeed,external-nodes = <&gfx>, <&lhc>;
-
-                pinctrl_i2c3_default: i2c3_default {
-                    function = "I2C3";
-                    groups = "I2C3";
-                };
-
-                pinctrl_gpioh0_unbiased_default: gpioh0 {
-                    pins = "A18";
-                    bias-disable;
-                };
+    scu@1e6e2000 {
+        compatible = "aspeed,ast2500-scu", "syscon", "simple-mfd";
+        reg = <0x1e6e2000 0x1a8>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+
+        pinctrl: pinctrl {
+            compatible = "aspeed,ast2500-pinctrl";
+            aspeed,external-nodes = <&gfx>, <&lhc>;
+
+            pinctrl_i2c3_default: i2c3_default {
+                function = "I2C3";
+                groups = "I2C3";
             };
-        };
-
-        gfx: display@1e6e6000 {
-            compatible = "aspeed,ast2500-gfx", "syscon";
-            reg = <0x1e6e6000 0x1000>;
-            reg-io-width = <4>;
-            clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
-            resets = <&syscon ASPEED_RESET_CRT1>;
-            interrupts = <0x19>;
-            syscon = <&syscon>;
-            memory-region = <&gfx_memory>;
-        };
-    };
-
-    lpc: lpc@1e789000 {
-        compatible = "aspeed,ast2500-lpc", "simple-mfd";
-        reg = <0x1e789000 0x1000>;
-
-        #address-cells = <1>;
-        #size-cells = <1>;
-        ranges = <0x0 0x1e789000 0x1000>;
-
-        lpc_host: lpc-host@80 {
-            compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon";
-            reg = <0x80 0x1e0>;
-            reg-io-width = <4>;
 
-            #address-cells = <1>;
-            #size-cells = <1>;
-            ranges = <0x0 0x80 0x1e0>;
-
-            lhc: lhc@20 {
-                   compatible = "aspeed,ast2500-lhc";
-                   reg = <0x20 0x24>, <0x48 0x8>;
+            pinctrl_gpioh0_unbiased_default: gpioh0 {
+                pins = "A18";
+                bias-disable;
             };
         };
     };
-
-    gfx_memory: framebuffer {
-        size = <0x01000000>;
-        alignment = <0x01000000>;
-        compatible = "shared-dma-pool";
-        reusable;
-    };
-- 
2.32.0
Re: [PATCH] dt-bindings: pinctrl: aspeed: Drop referenced nodes in examples
Posted by Linus Walleij 4 years ago
On Fri, Apr 22, 2022 at 9:21 PM Rob Herring <robh@kernel.org> wrote:

> The additional nodes in the example referenced from the pinctrl node
> 'aspeed,external-nodes' properties are either incorrect (aspeed,ast2500-lpc)
> or not documented with a schema (aspeed,ast2500-gfx). There's no need to
> show these nodes as part of the pinctrl example, so just remove them.
>
> Signed-off-by: Rob Herring <robh@kernel.org>

Patch applied. Concerns about lost examples can be solved
with incremental patches on top adding more schema.

Yours,
Linus Walleij
Re: [PATCH] dt-bindings: pinctrl: aspeed: Drop referenced nodes in examples
Posted by Joel Stanley 4 years ago
On Fri, 22 Apr 2022 at 19:21, Rob Herring <robh@kernel.org> wrote:
>
> The additional nodes in the example referenced from the pinctrl node
> 'aspeed,external-nodes' properties are either incorrect (aspeed,ast2500-lpc)
> or not documented with a schema (aspeed,ast2500-gfx). There's no need to
> show these nodes as part of the pinctrl example, so just remove them.
>
> Signed-off-by: Rob Herring <robh@kernel.org>

Nak.

This removes the information on how to use the bindings. Surely we
prefer to over document rather than under document?


> ---
>  .../pinctrl/aspeed,ast2500-pinctrl.yaml       | 81 ++++---------------
>  1 file changed, 16 insertions(+), 65 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> index 7c25c8d51116..9db904a528ee 100644
> --- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> @@ -76,73 +76,24 @@ additionalProperties: false
>  examples:
>    - |
>      #include <dt-bindings/clock/aspeed-clock.h>
> -    apb {
> -        compatible = "simple-bus";
> -        #address-cells = <1>;
> -        #size-cells = <1>;
> -        ranges;
> -
> -        syscon: scu@1e6e2000 {
> -            compatible = "aspeed,ast2500-scu", "syscon", "simple-mfd";
> -            reg = <0x1e6e2000 0x1a8>;
> -            #clock-cells = <1>;
> -            #reset-cells = <1>;
> -
> -            pinctrl: pinctrl {
> -                compatible = "aspeed,ast2500-pinctrl";
> -                aspeed,external-nodes = <&gfx>, <&lhc>;
> -
> -                pinctrl_i2c3_default: i2c3_default {
> -                    function = "I2C3";
> -                    groups = "I2C3";
> -                };
> -
> -                pinctrl_gpioh0_unbiased_default: gpioh0 {
> -                    pins = "A18";
> -                    bias-disable;
> -                };
> +    scu@1e6e2000 {
> +        compatible = "aspeed,ast2500-scu", "syscon", "simple-mfd";
> +        reg = <0x1e6e2000 0x1a8>;
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;
> +
> +        pinctrl: pinctrl {
> +            compatible = "aspeed,ast2500-pinctrl";
> +            aspeed,external-nodes = <&gfx>, <&lhc>;
> +
> +            pinctrl_i2c3_default: i2c3_default {
> +                function = "I2C3";
> +                groups = "I2C3";
>              };
> -        };
> -
> -        gfx: display@1e6e6000 {
> -            compatible = "aspeed,ast2500-gfx", "syscon";
> -            reg = <0x1e6e6000 0x1000>;
> -            reg-io-width = <4>;
> -            clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
> -            resets = <&syscon ASPEED_RESET_CRT1>;
> -            interrupts = <0x19>;
> -            syscon = <&syscon>;
> -            memory-region = <&gfx_memory>;
> -        };
> -    };
> -
> -    lpc: lpc@1e789000 {
> -        compatible = "aspeed,ast2500-lpc", "simple-mfd";
> -        reg = <0x1e789000 0x1000>;
> -
> -        #address-cells = <1>;
> -        #size-cells = <1>;
> -        ranges = <0x0 0x1e789000 0x1000>;
> -
> -        lpc_host: lpc-host@80 {
> -            compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon";
> -            reg = <0x80 0x1e0>;
> -            reg-io-width = <4>;
>
> -            #address-cells = <1>;
> -            #size-cells = <1>;
> -            ranges = <0x0 0x80 0x1e0>;
> -
> -            lhc: lhc@20 {
> -                   compatible = "aspeed,ast2500-lhc";
> -                   reg = <0x20 0x24>, <0x48 0x8>;
> +            pinctrl_gpioh0_unbiased_default: gpioh0 {
> +                pins = "A18";
> +                bias-disable;
>              };
>          };
>      };
> -
> -    gfx_memory: framebuffer {
> -        size = <0x01000000>;
> -        alignment = <0x01000000>;
> -        compatible = "shared-dma-pool";
> -        reusable;
> -    };
> --
> 2.32.0
>
Re: [PATCH] dt-bindings: pinctrl: aspeed: Drop referenced nodes in examples
Posted by Krzysztof Kozlowski 4 years ago
On 27/04/2022 10:40, Joel Stanley wrote:
> On Fri, 22 Apr 2022 at 19:21, Rob Herring <robh@kernel.org> wrote:
>>
>> The additional nodes in the example referenced from the pinctrl node
>> 'aspeed,external-nodes' properties are either incorrect (aspeed,ast2500-lpc)
>> or not documented with a schema (aspeed,ast2500-gfx). There's no need to
>> show these nodes as part of the pinctrl example, so just remove them.
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
> 
> Nak.
> 
> This removes the information on how to use the bindings. Surely we
> prefer to over document rather than under document?

There is no need to document consumers of generic providers, like
syscon, clock or pinctrl. These are already well documented. The
examples of consumers here do not bring any additional value.

Best regards,
Krzysztof
Re: [PATCH] dt-bindings: pinctrl: aspeed: Drop referenced nodes in examples
Posted by Rob Herring 4 years ago
On Wed, Apr 27, 2022 at 08:40:31AM +0000, Joel Stanley wrote:
> On Fri, 22 Apr 2022 at 19:21, Rob Herring <robh@kernel.org> wrote:
> >
> > The additional nodes in the example referenced from the pinctrl node
> > 'aspeed,external-nodes' properties are either incorrect (aspeed,ast2500-lpc)
> > or not documented with a schema (aspeed,ast2500-gfx). There's no need to
> > show these nodes as part of the pinctrl example, so just remove them.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> 
> Nak.

I welcome patches that add schemas for the undocumented compatibles 
instead. Otherwise, I will be turning on this check by default and 
nagging people to fix them.

> This removes the information on how to use the bindings. Surely we
> prefer to over document rather than under document?

How is what the 'gfx' and 'lpc' nodes contain relevant to how the 
pinctrl binding works? If a user wants to know, then they should go look 
at the aspeed,ast2500-lpc/aspeed,ast2500-gfx bindings and their 
examples. Which brings up my secondary issue which is having the same 
example multiple times. It is multiple chances for errors (that I end 
up fixing).

How do we know the example is even correct without any schema checks? 
The 'framebuffer' node is not in a valid location is the most obvious 
thing I see.

Rob