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

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

[Rafał Miłecki]

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>
---
V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
---
 .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
 .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/brcm,cru.yaml b/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
index fc1317ab3226..28ac60acf4ac 100644
--- a/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
+++ b/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
@@ -32,13 +32,13 @@ properties:
   "#size-cells":
     const: 1
 
-  pinctrl:
-    $ref: ../pinctrl/brcm,ns-pinmux.yaml
-
 patternProperties:
   '^clock-controller@[a-f0-9]+$':
     $ref: ../clock/brcm,iproc-clocks.yaml
 
+  '^pin-controller@[a-f0-9]+$':
+    $ref: ../pinctrl/brcm,ns-pinmux.yaml
+
   '^thermal@[a-f0-9]+$':
     $ref: ../thermal/brcm,ns-thermal.yaml
 
@@ -73,9 +73,10 @@ examples:
                                  "iprocfast", "sata1", "sata2";
         };
 
-        pinctrl {
+        pin-controller@1c0 {
             compatible = "brcm,bcm4708-pinmux";
-            offset = <0x1c0>;
+            reg = <0x1c0 0x24>;
+            reg-names = "cru_gpio_control";
         };
 
         thermal@2c0 {
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";
-- 
2.26.2

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

[Rafał Miłecki]

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>
---
V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
---
 .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
 .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/brcm,cru.yaml b/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
index fc1317ab3226..28ac60acf4ac 100644
--- a/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
+++ b/Documentation/devicetree/bindings/mfd/brcm,cru.yaml
@@ -32,13 +32,13 @@ properties:
   "#size-cells":
     const: 1
 
-  pinctrl:
-    $ref: ../pinctrl/brcm,ns-pinmux.yaml
-
 patternProperties:
   '^clock-controller@[a-f0-9]+$':
     $ref: ../clock/brcm,iproc-clocks.yaml
 
+  '^pin-controller@[a-f0-9]+$':
+    $ref: ../pinctrl/brcm,ns-pinmux.yaml
+
   '^thermal@[a-f0-9]+$':
     $ref: ../thermal/brcm,ns-thermal.yaml
 
@@ -73,9 +73,10 @@ examples:
                                  "iprocfast", "sata1", "sata2";
         };
 
-        pinctrl {
+        pin-controller@1c0 {
             compatible = "brcm,bcm4708-pinmux";
-            offset = <0x1c0>;
+            reg = <0x1c0 0x24>;
+            reg-names = "cru_gpio_control";
         };
 
         thermal@2c0 {
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";
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 linux-pinctrl 2/2] Revert "pinctrl: bcm: ns: support updated DT binding as syscon subnode"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This reverts commit a49d784d5a8272d0f63c448fe8dc69e589db006e.

The updated binding was wrong / invalid and has been reverted. There
isn't any upstream kernel DTS using it and Broadcom isn't known to use
it neither. There is close to zero chance this will cause regression for
anyone.

Actually in-kernel bcm5301x.dtsi still uses the old good binding and so
it's broken since the driver update. This revert fixes it.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/bcm/pinctrl-ns.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-ns.c b/drivers/pinctrl/bcm/pinctrl-ns.c
index e79690bd8b85..d7f8175d2c1c 100644
--- a/drivers/pinctrl/bcm/pinctrl-ns.c
+++ b/drivers/pinctrl/bcm/pinctrl-ns.c
@@ -5,7 +5,6 @@
 
 #include <linux/err.h>
 #include <linux/io.h>
-#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -13,7 +12,6 @@
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 #include <linux/platform_device.h>
-#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #define FLAG_BCM4708		BIT(1)
@@ -24,8 +22,7 @@ struct ns_pinctrl {
 	struct device *dev;
 	unsigned int chipset_flag;
 	struct pinctrl_dev *pctldev;
-	struct regmap *regmap;
-	u32 offset;
+	void __iomem *base;
 
 	struct pinctrl_desc pctldesc;
 	struct ns_pinctrl_group *groups;
@@ -232,9 +229,9 @@ static int ns_pinctrl_set_mux(struct pinctrl_dev *pctrl_dev,
 		unset |= BIT(pin_number);
 	}
 
-	regmap_read(ns_pinctrl->regmap, ns_pinctrl->offset, &tmp);
+	tmp = readl(ns_pinctrl->base);
 	tmp &= ~unset;
-	regmap_write(ns_pinctrl->regmap, ns_pinctrl->offset, tmp);
+	writel(tmp, ns_pinctrl->base);
 
 	return 0;
 }
@@ -266,13 +263,13 @@ static const struct of_device_id ns_pinctrl_of_match_table[] = {
 static int ns_pinctrl_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
 	const struct of_device_id *of_id;
 	struct ns_pinctrl *ns_pinctrl;
 	struct pinctrl_desc *pctldesc;
 	struct pinctrl_pin_desc *pin;
 	struct ns_pinctrl_group *group;
 	struct ns_pinctrl_function *function;
+	struct resource *res;
 	int i;
 
 	ns_pinctrl = devm_kzalloc(dev, sizeof(*ns_pinctrl), GFP_KERNEL);
@@ -290,18 +287,12 @@ static int ns_pinctrl_probe(struct platform_device *pdev)
 		return -EINVAL;
 	ns_pinctrl->chipset_flag = (uintptr_t)of_id->data;
 
-	ns_pinctrl->regmap = syscon_node_to_regmap(of_get_parent(np));
-	if (IS_ERR(ns_pinctrl->regmap)) {
-		int err = PTR_ERR(ns_pinctrl->regmap);
-
-		dev_err(dev, "Failed to map pinctrl regs: %d\n", err);
-
-		return err;
-	}
-
-	if (of_property_read_u32(np, "offset", &ns_pinctrl->offset)) {
-		dev_err(dev, "Failed to get register offset\n");
-		return -ENOENT;
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+					   "cru_gpio_control");
+	ns_pinctrl->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ns_pinctrl->base)) {
+		dev_err(dev, "Failed to map pinctrl regs\n");
+		return PTR_ERR(ns_pinctrl->base);
 	}
 
 	memcpy(pctldesc, &ns_pinctrl_desc, sizeof(*pctldesc));
-- 
2.26.2

[PATCH V2 linux-pinctrl 2/2] Revert "pinctrl: bcm: ns: support updated DT binding as syscon subnode"

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This reverts commit a49d784d5a8272d0f63c448fe8dc69e589db006e.

The updated binding was wrong / invalid and has been reverted. There
isn't any upstream kernel DTS using it and Broadcom isn't known to use
it neither. There is close to zero chance this will cause regression for
anyone.

Actually in-kernel bcm5301x.dtsi still uses the old good binding and so
it's broken since the driver update. This revert fixes it.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/bcm/pinctrl-ns.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-ns.c b/drivers/pinctrl/bcm/pinctrl-ns.c
index e79690bd8b85..d7f8175d2c1c 100644
--- a/drivers/pinctrl/bcm/pinctrl-ns.c
+++ b/drivers/pinctrl/bcm/pinctrl-ns.c
@@ -5,7 +5,6 @@
 
 #include <linux/err.h>
 #include <linux/io.h>
-#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -13,7 +12,6 @@
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 #include <linux/platform_device.h>
-#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #define FLAG_BCM4708		BIT(1)
@@ -24,8 +22,7 @@ struct ns_pinctrl {
 	struct device *dev;
 	unsigned int chipset_flag;
 	struct pinctrl_dev *pctldev;
-	struct regmap *regmap;
-	u32 offset;
+	void __iomem *base;
 
 	struct pinctrl_desc pctldesc;
 	struct ns_pinctrl_group *groups;
@@ -232,9 +229,9 @@ static int ns_pinctrl_set_mux(struct pinctrl_dev *pctrl_dev,
 		unset |= BIT(pin_number);
 	}
 
-	regmap_read(ns_pinctrl->regmap, ns_pinctrl->offset, &tmp);
+	tmp = readl(ns_pinctrl->base);
 	tmp &= ~unset;
-	regmap_write(ns_pinctrl->regmap, ns_pinctrl->offset, tmp);
+	writel(tmp, ns_pinctrl->base);
 
 	return 0;
 }
@@ -266,13 +263,13 @@ static const struct of_device_id ns_pinctrl_of_match_table[] = {
 static int ns_pinctrl_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
 	const struct of_device_id *of_id;
 	struct ns_pinctrl *ns_pinctrl;
 	struct pinctrl_desc *pctldesc;
 	struct pinctrl_pin_desc *pin;
 	struct ns_pinctrl_group *group;
 	struct ns_pinctrl_function *function;
+	struct resource *res;
 	int i;
 
 	ns_pinctrl = devm_kzalloc(dev, sizeof(*ns_pinctrl), GFP_KERNEL);
@@ -290,18 +287,12 @@ static int ns_pinctrl_probe(struct platform_device *pdev)
 		return -EINVAL;
 	ns_pinctrl->chipset_flag = (uintptr_t)of_id->data;
 
-	ns_pinctrl->regmap = syscon_node_to_regmap(of_get_parent(np));
-	if (IS_ERR(ns_pinctrl->regmap)) {
-		int err = PTR_ERR(ns_pinctrl->regmap);
-
-		dev_err(dev, "Failed to map pinctrl regs: %d\n", err);
-
-		return err;
-	}
-
-	if (of_property_read_u32(np, "offset", &ns_pinctrl->offset)) {
-		dev_err(dev, "Failed to get register offset\n");
-		return -ENOENT;
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+					   "cru_gpio_control");
+	ns_pinctrl->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ns_pinctrl->base)) {
+		dev_err(dev, "Failed to map pinctrl regs\n");
+		return PTR_ERR(ns_pinctrl->base);
 	}
 
 	memcpy(pctldesc, &ns_pinctrl_desc, sizeof(*pctldesc));
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rob Herring]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

On Fri, 08 Oct 2021 17:39:38 +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>
> ---
> V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
> ---
>  .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
>  .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
>  2 files changed, 19 insertions(+), 15 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/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/1538413

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.

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

[Rob Herring]

[-- Attachment #1: Type: text/plain, Size: 2240 bytes --]

On Fri, 08 Oct 2021 17:39:38 +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>
> ---
> V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
> ---
>  .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
>  .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
>  2 files changed, 19 insertions(+), 15 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/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/1538413

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.



[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rafał Miłecki]

On 08.10.2021 21:48, Rob Herring wrote:
> On Fri, 08 Oct 2021 17:39:38 +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>
>> ---
>> V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
>> ---
>>   .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
>>   .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
>>   2 files changed, 19 insertions(+), 15 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/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

It's warning we already have and not something introduced by this
revert.

As a revert this commit should introduce as little non-revert changes
as possible. I'm planning to improve that example later in a separated
commit.

Can you take a look at this commit despite your bot warning, please?

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

[Rafał Miłecki]

On 08.10.2021 21:48, Rob Herring wrote:
> On Fri, 08 Oct 2021 17:39:38 +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>
>> ---
>> V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
>> ---
>>   .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
>>   .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
>>   2 files changed, 19 insertions(+), 15 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/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

It's warning we already have and not something introduced by this
revert.

As a revert this commit should introduce as little non-revert changes
as possible. I'm planning to improve that example later in a separated
commit.

Can you take a look at this commit despite your bot warning, please?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rob Herring]

On Fri, Oct 08, 2021 at 10:08:46PM +0200, Rafał Miłecki wrote:
> On 08.10.2021 21:48, Rob Herring wrote:
> > On Fri, 08 Oct 2021 17:39:38 +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>
> > > ---
> > > V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
> > > ---
> > >   .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
> > >   .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
> > >   2 files changed, 19 insertions(+), 15 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/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
> 
> It's warning we already have and not something introduced by this
> revert.

We don't allow warnings, so you're saying this one was warning before?

> As a revert this commit should introduce as little non-revert changes
> as possible. I'm planning to improve that example later in a separated
> commit.

You have to fix the warning as part of this series.

> 
> Can you take a look at this commit despite your bot warning, please?

If 'CRU' is a hardware block, then I don't think most of the revert is 
right to use 'simple-bus'. 

If you want to just replace 'offset' with 'reg', then I'd be happy to 
see that. I've been pushing child nodes of syscons in that direction.

Rob

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

[Rob Herring]

On Fri, Oct 08, 2021 at 10:08:46PM +0200, Rafał Miłecki wrote:
> On 08.10.2021 21:48, Rob Herring wrote:
> > On Fri, 08 Oct 2021 17:39:38 +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>
> > > ---
> > > V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
> > > ---
> > >   .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
> > >   .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
> > >   2 files changed, 19 insertions(+), 15 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/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
> 
> It's warning we already have and not something introduced by this
> revert.

We don't allow warnings, so you're saying this one was warning before?

> As a revert this commit should introduce as little non-revert changes
> as possible. I'm planning to improve that example later in a separated
> commit.

You have to fix the warning as part of this series.

> 
> Can you take a look at this commit despite your bot warning, please?

If 'CRU' is a hardware block, then I don't think most of the revert is 
right to use 'simple-bus'. 

If you want to just replace 'offset' with 'reg', then I'd be happy to 
see that. I've been pushing child nodes of syscons in that direction.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rafał Miłecki]

On 08.10.2021 22:49, Rob Herring wrote:
> On Fri, Oct 08, 2021 at 10:08:46PM +0200, Rafał Miłecki wrote:
>> On 08.10.2021 21:48, Rob Herring wrote:
>>> On Fri, 08 Oct 2021 17:39:38 +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>
>>>> ---
>>>> V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
>>>> ---
>>>>    .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
>>>>    .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
>>>>    2 files changed, 19 insertions(+), 15 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/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
>>
>> It's warning we already have and not something introduced by this
>> revert.
> 
> We don't allow warnings, so you're saying this one was warning before?

Yes.


>> As a revert this commit should introduce as little non-revert changes
>> as possible. I'm planning to improve that example later in a separated
>> commit.
> 
> You have to fix the warning as part of this series.

OK


>> Can you take a look at this commit despite your bot warning, please?
> 
> If 'CRU' is a hardware block, then I don't think most of the revert is
> right to use 'simple-bus'.
> 
> If you want to just replace 'offset' with 'reg', then I'd be happy to
> see that. I've been pushing child nodes of syscons in that direction.

Well, it's a revert, not a fix, it goes from totally wrong to semi-wrong
;) Let me send V3.

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

[Rafał Miłecki]

On 08.10.2021 22:49, Rob Herring wrote:
> On Fri, Oct 08, 2021 at 10:08:46PM +0200, Rafał Miłecki wrote:
>> On 08.10.2021 21:48, Rob Herring wrote:
>>> On Fri, 08 Oct 2021 17:39:38 +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>
>>>> ---
>>>> V2: Update brcm,cru.yaml to avoid new yamllint warnings/errors
>>>> ---
>>>>    .../devicetree/bindings/mfd/brcm,cru.yaml     | 11 +++++----
>>>>    .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 23 +++++++++++--------
>>>>    2 files changed, 19 insertions(+), 15 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/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
>>
>> It's warning we already have and not something introduced by this
>> revert.
> 
> We don't allow warnings, so you're saying this one was warning before?

Yes.


>> As a revert this commit should introduce as little non-revert changes
>> as possible. I'm planning to improve that example later in a separated
>> commit.
> 
> You have to fix the warning as part of this series.

OK


>> Can you take a look at this commit despite your bot warning, please?
> 
> If 'CRU' is a hardware block, then I don't think most of the revert is
> right to use 'simple-bus'.
> 
> If you want to just replace 'offset' with 'reg', then I'd be happy to
> see that. I've been pushing child nodes of syscons in that direction.

Well, it's a revert, not a fix, it goes from totally wrong to semi-wrong
;) Let me send V3.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel