diff mbox series

[linux-pinctrl,1/2] Revert "dt-bindings: pinctrl: bcm4708-pinmux: rework binding to use syscon"

Message ID 20211008082932.1370-1-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show
Series [linux-pinctrl,1/2] Revert "dt-bindings: pinctrl: bcm4708-pinmux: rework binding to use syscon" | expand

Commit Message

Rafał Miłecki Oct. 8, 2021, 8:29 a.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

This reverts commit 2ae80900f239484069569380e1fc4340fd6e0089.

My rework was unneeded & wrong. It replaced a clear & correct "reg"
property usage with a custom "offset" one.

Back then I didn't understand how to properly handle CRU block binding.
I heard / read about syscon and tried to use it in a totally invalid
way. That change also missed Rob's review (obviously).

Northstar's pin controller is a simple consistent hardware block that
can be cleanly mapped using a 0x24 long reg space.

Since the rework commit there wasn't any follow up modifying in-kernel
DTS files to use the new binding. Broadcom also isn't known to use that
bugged binding. There is close to zero chance this revert may actually
cause problems / regressions.

This commit is a simple revert. Example binding may (should) be updated
/ cleaned up but that can be handled separately.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
 1 file changed, 13 insertions(+), 10 deletions(-)

Comments

Rob Herring (Arm) Oct. 8, 2021, 1:49 p.m. UTC | #1
On Fri, 08 Oct 2021 10:29:31 +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This reverts commit 2ae80900f239484069569380e1fc4340fd6e0089.
> 
> My rework was unneeded & wrong. It replaced a clear & correct "reg"
> property usage with a custom "offset" one.
> 
> Back then I didn't understand how to properly handle CRU block binding.
> I heard / read about syscon and tried to use it in a totally invalid
> way. That change also missed Rob's review (obviously).
> 
> Northstar's pin controller is a simple consistent hardware block that
> can be cleanly mapped using a 0x24 long reg space.
> 
> Since the rework commit there wasn't any follow up modifying in-kernel
> DTS files to use the new binding. Broadcom also isn't known to use that
> bugged binding. There is close to zero chance this revert may actually
> cause problems / regressions.
> 
> This commit is a simple revert. Example binding may (should) be updated
> / cleaned up but that can be handled separately.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml: cru-bus@1800c100: pinctrl: 'reg' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml: cru-bus@1800c100: pinctrl: 'reg-names' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml: cru-bus@1800c100: pinctrl: 'offset' does not match any of the regexes: '-pins$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml: pinctrl: 'reg' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml: pinctrl: 'reg-names' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml: pinctrl: 'offset' does not match any of the regexes: '-pins$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.example.dt.yaml: cru@1800c100: $nodename:0: 'cru@1800c100' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/simple-bus.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1538264

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
index 470aff599c27..78600a8fe403 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
@@ -17,9 +17,6 @@  description:
 
   A list of pins varies across chipsets so few bindings are available.
 
-  Node of the pinmux must be nested in the CRU (Central Resource Unit) "syscon"
-  node.
-
 properties:
   compatible:
     enum:
@@ -27,10 +24,11 @@  properties:
       - brcm,bcm4709-pinmux
       - brcm,bcm53012-pinmux
 
-  offset:
-    description: offset of pin registers in the CRU block
+  reg:
     maxItems: 1
-    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  reg-names:
+    const: cru_gpio_control
 
 patternProperties:
   '-pins$':
@@ -72,19 +70,24 @@  allOf:
                         uart1_grp ]
 
 required:
-  - offset
+  - reg
+  - reg-names
 
 additionalProperties: false
 
 examples:
   - |
     cru@1800c100 {
-        compatible = "syscon", "simple-mfd";
+        compatible = "simple-bus";
         reg = <0x1800c100 0x1a4>;
+        ranges;
+        #address-cells = <1>;
+        #size-cells = <1>;
 
-        pinctrl {
+        pin-controller@1c0 {
             compatible = "brcm,bcm4708-pinmux";
-            offset = <0xc0>;
+            reg = <0x1c0 0x24>;
+            reg-names = "cru_gpio_control";
 
             spi-pins {
                 function = "spi";