[PATCH 0/5] pinctrl: allow storing pins, groups & functions in DT

[PATCH 0/5] pinctrl: allow storing pins, groups & functions in DT

[Rafał Miłecki]

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

A week ago I sent
[PATCH RFC] dt-bindings: pinctrl: support specifying pins
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211110231436.8866-1-zajec5@gmail.com/

From short discussion in that thread it seems that using DT to store
pinctrl pins, groups & functions may be an option. I'd like to ask for
reviewing my patchset implementing that.

Please note it's about describing hardware elements and not actual
programming way. It may be used with pinctrl-single.c one day but it's
designed as a generic solution for data.

Patches 1-4 are for linux-pinctrl.git. Patch 5 I found worth including
as DT big example. It can go through Linus with Florian's Ack or I can
send it to Florian later.

Rafał Miłecki (5):
  dt-bindings: pinctrl: support specifying pins, groups & functions
  dt-bindings: pinctrl: brcm,ns-pinmux: extend example
  pinctrl: add helpers reading pins, groups & functions from DT
  pinctrl: bcm: pinctrl-ns: supoprt DT specified pins, groups &
    functions
  ARM: dts: BCM5301X: add pinctrl pins, groups & functions

 .../bindings/pinctrl/brcm,ns-pinmux.yaml      |  29 ++++-
 .../devicetree/bindings/pinctrl/pinctrl.yaml  |  50 +++++++
 arch/arm/boot/dts/bcm4709.dtsi                |  74 +++++++++++
 arch/arm/boot/dts/bcm47094.dtsi               |  11 +-
 arch/arm/boot/dts/bcm5301x.dtsi               | 123 ++++++++++++++++++
 drivers/pinctrl/bcm/pinctrl-ns.c              |  90 ++++++++-----
 drivers/pinctrl/core.c                        |  89 +++++++++++++
 drivers/pinctrl/core.h                        |   5 +
 drivers/pinctrl/pinmux.c                      |  43 ++++++
 drivers/pinctrl/pinmux.h                      |   2 +
 10 files changed, 475 insertions(+), 41 deletions(-)

-- 
2.31.1


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

[PATCH 1/5] dt-bindings: pinctrl: support specifying pins, groups & functions

[Rafał Miłecki]

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

This binding change is meant to introduce a generic way of describing
pinctrl blocks details. Every pinmux block is expected to have:
1. Named pins
2. Named groups containing one or more pins
3. Named functions referencing one or more groups

It doesn't describe how hw should be programmed. That is still binding
specific.

This commit describes syntax for "pins", "groups" & "functions" nodes in
a standard pinctrl binding. Every above node allows specifying its
entries and it's done using subnodes because:
1. It's required with reg = <n> ("pins")
2. It's generic & extendable (hw specific properties can be added)

Pins are number based so they use reg = <n>. It's also required as
binding needs to allow gaps. It's to avoid hacks like:
pins = <"foo" "-ENODEV" "-ENODEV" "-ENODEV" "bar">;
(for hw with pins 0 and 4 present).

Subnodes also allow storing hw specific pin/group/function
configuration. While it would be possible to have:
groups {
    foo-pins = <0 1>;
    bar-pins = <2 3>;
};
that doesn't allow hw specific quirks.

Introduced design allows e.g.:
groups {
    foo {
        pins = <0 1>;
        vendor,magic = <0xbeaf>;
    };
};

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/pinctrl/pinctrl.yaml  | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
index d471563119a9..1a99920e94ef 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
@@ -42,4 +42,54 @@ properties:
       This property can be set either globally for the pin controller or in
       child nodes for individual pin group control.
 
+  pins:
+    type: object
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^pin@[0-9a-z]+$":
+        type: object
+
+        properties:
+          reg:
+            description: Pin number
+
+          label:
+            description: Pin name
+            $ref: /schemas/types.yaml#/definitions/string
+
+        additionalProperties: false
+
+  groups:
+    type: object
+
+    patternProperties:
+      "^.*$":
+        type: object
+        description: Group identified by node name
+
+        properties:
+          pins:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            description: Array of pins belonging to this group
+
+  functions:
+    type: object
+
+    patternProperties:
+      "^.*$":
+        type: object
+        description: Function identified by node name
+
+        properties:
+          groups:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+            description: Array of pins groups used by this function
+
 additionalProperties: true
-- 
2.31.1


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

[PATCH 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Rafał Miłecki]

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

pinctrl bindings allow specifying pins, groups & functions now. Put some
entries in binding example to help writing DTS files.

Specify pins, groups & functions in the example.

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

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
index 8d1e5b1cdd5f..154119981ad9 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
@@ -74,7 +74,7 @@ required:
   - reg
   - reg-names
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
@@ -83,6 +83,33 @@ examples:
         reg = <0x1800c1c0 0x24>;
         reg-names = "cru_gpio_control";
 
+        pins {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            pin@4 {
+                reg = <4>;
+                label = "i2c_scl";
+            };
+
+            pin@5 {
+                reg = <5>;
+                label = "i2c_sda";
+            };
+        };
+
+        groups {
+            i2c_grp: i2c_grp {
+                pins = <4 5>;
+            };
+        };
+
+        functions {
+            i2c {
+                groups = <&i2c_grp>;
+            };
+        };
+
         spi-pins {
             function = "spi";
             groups = "spi_grp";
-- 
2.31.1


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

[PATCH 3/5] pinctrl: add helpers reading pins, groups & functions from DT

[Rafał Miłecki]

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

DT binding allows specifying pins, groups & functions now. That allows
storing them in DT instead of hardcoding in drivers.

Introduce helpers based on CONFIG_GENERIC_PINCONF,
CONFIG_GENERIC_PINCTRL_GROUPS and CONFIG_GENERIC_PINMUX_FUNCTIONS for
parsing that info into pinctrl generic structures.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/core.c   | 89 ++++++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/core.h   |  5 +++
 drivers/pinctrl/pinmux.c | 43 +++++++++++++++++++
 drivers/pinctrl/pinmux.h |  2 +
 4 files changed, 139 insertions(+)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index ffe39336fcac..8f6ed8488313 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -515,8 +515,97 @@ void pinctrl_remove_gpio_range(struct pinctrl_dev *pctldev,
 }
 EXPORT_SYMBOL_GPL(pinctrl_remove_gpio_range);
 
+int pinctrl_generic_get_dt_pins(struct pinctrl_desc *pctldesc,
+				struct device *dev)
+{
+	struct pinctrl_pin_desc *descs;
+	struct device_node *pins;
+	struct device_node *np;
+	int err = 0;
+	int i = 0;
+
+	pins = of_get_child_by_name(dev->of_node, "pins");
+	if (!pins) {
+		dev_err(dev, "failed to find \"pins\" DT node\n");
+		err = -ENOENT;
+		goto err_out;
+	}
+
+	pctldesc->npins = of_get_available_child_count(pins);
+
+	descs = devm_kcalloc(dev, pctldesc->npins, sizeof(*descs), GFP_KERNEL);
+	if (!descs) {
+		err = -ENOMEM;
+		goto err_put_node;
+	}
+
+	for_each_available_child_of_node(pins, np) {
+		if (of_property_read_u32(np, "reg", &descs[i].number)) {
+			dev_err(dev, "missing \"reg\" property in %pOF\n", np);
+			err = -ENOENT;
+			goto err_put_node;
+		}
+
+		if (of_property_read_string(np, "label", &descs[i].name)) {
+			dev_err(dev, "missing \"label\" property in %pOF\n", np);
+			err = -ENOENT;
+			goto err_put_node;
+		}
+
+		i++;
+	}
+
+	pctldesc->pins = descs;
+
+err_put_node:
+	of_node_put(pins);
+err_out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_get_dt_pins);
+
 #ifdef CONFIG_GENERIC_PINCTRL_GROUPS
 
+int pinctrl_generic_get_dt_groups(struct pinctrl_dev *pctldev)
+{
+	struct device *dev = pctldev->dev;
+	struct device_node *groups;
+	struct device_node *np;
+	int err = 0;
+
+	groups = of_get_child_by_name(dev->of_node, "groups");
+	if (!groups) {
+		dev_err(dev, "failed to find \"groups\" DT node\n");
+		err = -ENOENT;
+		goto err_out;
+	}
+
+	for_each_available_child_of_node(groups, np) {
+		int num_pins;
+		u32 *pins;
+
+		num_pins = of_property_count_u32_elems(np, "pins");
+		pins = devm_kmalloc_array(dev, num_pins, sizeof(*pins), GFP_KERNEL);
+		if (!pins) {
+			err = -ENOMEM;
+			goto err_put_node;
+		}
+
+		if (of_property_read_u32_array(np, "pins", pins, num_pins)) {
+			err = -EIO;
+			goto err_put_node;
+		}
+
+		pinctrl_generic_add_group(pctldev, np->name, pins, num_pins, np);
+	}
+
+err_put_node:
+	of_node_put(groups);
+err_out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_get_dt_groups);
+
 /**
  * pinctrl_generic_get_group_count() - returns the number of pin groups
  * @pctldev: pin controller device
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 840103c40c14..59661d4d4cc7 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -182,6 +182,9 @@ struct pinctrl_maps {
 	unsigned num_maps;
 };
 
+int pinctrl_generic_get_dt_pins(struct pinctrl_desc *pctldesc,
+				struct device *dev);
+
 #ifdef CONFIG_GENERIC_PINCTRL_GROUPS
 
 /**
@@ -198,6 +201,8 @@ struct group_desc {
 	void *data;
 };
 
+int pinctrl_generic_get_dt_groups(struct pinctrl_dev *pctldev);
+
 int pinctrl_generic_get_group_count(struct pinctrl_dev *pctldev);
 
 const char *pinctrl_generic_get_group_name(struct pinctrl_dev *pctldev,
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 6cdbd9ccf2f0..5e34bd3135f5 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -24,6 +24,7 @@
 #include <linux/string.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
+#include <linux/of.h>
 #include <linux/pinctrl/machine.h>
 #include <linux/pinctrl/pinmux.h>
 #include "core.h"
@@ -788,6 +789,48 @@ void pinmux_init_device_debugfs(struct dentry *devroot,
 
 #ifdef CONFIG_GENERIC_PINMUX_FUNCTIONS
 
+int pinmux_generic_get_dt_functions(struct pinctrl_dev *pctldev)
+{
+	struct device *dev = pctldev->dev;
+	struct device_node *functions;
+	struct device_node *np;
+	int err = 0;
+
+	functions = of_get_child_by_name(dev->of_node, "functions");
+	if (!functions) {
+		dev_err(dev, "failed to find \"functions\" DT node\n");
+		err = -ENOENT;
+		goto err_out;
+	}
+
+	for_each_available_child_of_node(functions, np) {
+		int num_groups = of_count_phandle_with_args(np, "groups", NULL);
+		struct of_phandle_iterator it;
+		const char **groups;
+		int ret;
+		int i;
+
+		groups = devm_kmalloc_array(dev, num_groups, sizeof(*groups), GFP_KERNEL);
+		if (!groups) {
+			err = -ENOMEM;
+			goto err_put_node;
+		}
+
+		i = 0;
+		of_for_each_phandle(&it, ret, np, "groups", NULL, 0) {
+			groups[i++] = it.node->name;
+		}
+
+		pinmux_generic_add_function(pctldev, np->name, groups, num_groups, np);
+	}
+
+err_put_node:
+	of_node_put(functions);
+err_out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(pinmux_generic_get_dt_functions);
+
 /**
  * pinmux_generic_get_function_count() - returns number of functions
  * @pctldev: pin controller device
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 78c3a31be882..ca69025fce46 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -134,6 +134,8 @@ struct function_desc {
 	void *data;
 };
 
+int pinmux_generic_get_dt_functions(struct pinctrl_dev *pctldev);
+
 int pinmux_generic_get_function_count(struct pinctrl_dev *pctldev);
 
 const char *
-- 
2.31.1


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

[PATCH 4/5] pinctrl: bcm: pinctrl-ns: supoprt DT specified pins, groups & functions

[Rafał Miłecki]

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

It's now possible to specify hardware pins, groups & functions in DT
instead of hardcoding that info in a driver. Use pinctrl subsystem
helpers to extract that info from DT.

Keep hardcoded data as fallback method.

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

diff --git a/drivers/pinctrl/bcm/pinctrl-ns.c b/drivers/pinctrl/bcm/pinctrl-ns.c
index 0897041b5ef1..48e77ff25d9d 100644
--- a/drivers/pinctrl/bcm/pinctrl-ns.c
+++ b/drivers/pinctrl/bcm/pinctrl-ns.c
@@ -213,7 +213,11 @@ static int ns_pinctrl_probe(struct platform_device *pdev)
 	struct ns_pinctrl *ns_pinctrl;
 	struct pinctrl_desc *pctldesc;
 	struct pinctrl_pin_desc *pin;
+	struct device_node *functions;
+	struct device_node *groups;
+	struct device_node *pins;
 	struct resource *res;
+	int err;
 	int i;
 
 	ns_pinctrl = devm_kzalloc(dev, sizeof(*ns_pinctrl), GFP_KERNEL);
@@ -243,19 +247,27 @@ static int ns_pinctrl_probe(struct platform_device *pdev)
 
 	/* Set pinctrl properties */
 
-	pctldesc->pins = devm_kcalloc(dev, ARRAY_SIZE(ns_pinctrl_pins),
-				      sizeof(struct pinctrl_pin_desc),
-				      GFP_KERNEL);
-	if (!pctldesc->pins)
-		return -ENOMEM;
-	for (i = 0, pin = (struct pinctrl_pin_desc *)&pctldesc->pins[0];
-	     i < ARRAY_SIZE(ns_pinctrl_pins); i++) {
-		const struct pinctrl_pin_desc *src = &ns_pinctrl_pins[i];
-		unsigned int chipsets = (uintptr_t)src->drv_data;
-
-		if (chipsets & ns_pinctrl->chipset_flag) {
-			memcpy(pin++, src, sizeof(*src));
-			pctldesc->npins++;
+	pins = of_get_child_by_name(dev->of_node, "pins");
+	if (pins) {
+		err = pinctrl_generic_get_dt_pins(pctldesc, dev);
+		of_node_put(pins);
+		if (err)
+			return err;
+	} else {
+		pctldesc->pins = devm_kcalloc(dev, ARRAY_SIZE(ns_pinctrl_pins),
+					sizeof(struct pinctrl_pin_desc),
+					GFP_KERNEL);
+		if (!pctldesc->pins)
+			return -ENOMEM;
+		for (i = 0, pin = (struct pinctrl_pin_desc *)&pctldesc->pins[0];
+		     i < ARRAY_SIZE(ns_pinctrl_pins); i++) {
+			const struct pinctrl_pin_desc *src = &ns_pinctrl_pins[i];
+			unsigned int chipsets = (uintptr_t)src->drv_data;
+
+			if (chipsets & ns_pinctrl->chipset_flag) {
+				memcpy(pin++, src, sizeof(*src));
+				pctldesc->npins++;
+			}
 		}
 	}
 
@@ -267,25 +279,43 @@ static int ns_pinctrl_probe(struct platform_device *pdev)
 		return PTR_ERR(ns_pinctrl->pctldev);
 	}
 
-	for (i = 0; i < ARRAY_SIZE(ns_pinctrl_groups); i++) {
-		const struct ns_pinctrl_group *group = &ns_pinctrl_groups[i];
-
-		if (!(group->chipsets & ns_pinctrl->chipset_flag))
-			continue;
-
-		pinctrl_generic_add_group(ns_pinctrl->pctldev, group->name,
-					  group->pins, group->num_pins, NULL);
+	groups = of_get_child_by_name(dev->of_node, "groups");
+	if (groups) {
+		err = pinctrl_generic_get_dt_groups(ns_pinctrl->pctldev);
+		of_node_put(groups);
+		if (err)
+			return err;
+	} else {
+		for (i = 0; i < ARRAY_SIZE(ns_pinctrl_groups); i++) {
+			const struct ns_pinctrl_group *group = &ns_pinctrl_groups[i];
+
+			if (!(group->chipsets & ns_pinctrl->chipset_flag))
+				continue;
+
+			pinctrl_generic_add_group(ns_pinctrl->pctldev,
+						  group->name, group->pins,
+						  group->num_pins, NULL);
+		}
 	}
 
-	for (i = 0; i < ARRAY_SIZE(ns_pinctrl_functions); i++) {
-		const struct ns_pinctrl_function *function = &ns_pinctrl_functions[i];
-
-		if (!(function->chipsets & ns_pinctrl->chipset_flag))
-			continue;
-
-		pinmux_generic_add_function(ns_pinctrl->pctldev, function->name,
-					    function->groups,
-					    function->num_groups, NULL);
+	functions = of_get_child_by_name(dev->of_node, "functions");
+	if (functions) {
+		err = pinmux_generic_get_dt_functions(ns_pinctrl->pctldev);
+		of_node_put(functions);
+		if (err)
+			return err;
+	} else {
+		for (i = 0; i < ARRAY_SIZE(ns_pinctrl_functions); i++) {
+			const struct ns_pinctrl_function *function = &ns_pinctrl_functions[i];
+
+			if (!(function->chipsets & ns_pinctrl->chipset_flag))
+				continue;
+
+			pinmux_generic_add_function(ns_pinctrl->pctldev,
+						    function->name,
+						    function->groups,
+						    function->num_groups, NULL);
+		}
 	}
 
 	return 0;
-- 
2.31.1


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

[PATCH 5/5] ARM: dts: BCM5301X: add pinctrl pins, groups & functions

[Rafał Miłecki]

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

They can now be described in DT so do that.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm/boot/dts/bcm4709.dtsi  |  74 +++++++++++++++++++
 arch/arm/boot/dts/bcm47094.dtsi |  11 +--
 arch/arm/boot/dts/bcm5301x.dtsi | 123 ++++++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/bcm4709.dtsi b/arch/arm/boot/dts/bcm4709.dtsi
index cba3d910bed8..ba4700a85772 100644
--- a/arch/arm/boot/dts/bcm4709.dtsi
+++ b/arch/arm/boot/dts/bcm4709.dtsi
@@ -10,6 +10,80 @@ &uart0 {
 	status = "okay";
 };
 
+&pinctrl {
+	compatible = "brcm,bcm4709-pinmux";
+
+	pins {
+		reg@6 {
+			reg = <6>;
+			label = "mdc";
+		};
+
+		reg@7 {
+			reg = <7>;
+			label = "mdio";
+		};
+
+		reg@10 {
+			reg = <16>;
+			label = "uart2_rx";
+		};
+
+		reg@11 {
+			reg = <17>;
+			label = "uart2_tx";
+		};
+
+		/* TODO
+		 * reg@ {
+		 *	label = "xtal_out";
+		 * };
+		 */
+
+		reg@16 {
+			reg = <22>;
+			label = "sdio_pwr";
+		};
+
+		reg@17 {
+			reg = <23>;
+			label = "sdio_en_1p8v";
+		};
+	};
+
+	groups {
+		mdio_grp: mdio_grp {
+			pins = <6 7>;
+		};
+
+		uart2_grp: uart2_grp {
+			pins = <16 17>;
+		};
+
+		sdio_pwr_grp: sdio_pwr_grp {
+			pins = <22>;
+		};
+
+		sdio_1p8v_grp: sdio_1p8v_grp {
+			pins = <23>;
+		};
+	};
+
+	functions {
+		mdio {
+			groups = <&mdio_grp>;
+		};
+
+		uart2 {
+			groups = <&uart2_grp>;
+		};
+
+		sdio {
+			groups = <&sdio_pwr_grp &sdio_1p8v_grp>;
+		};
+	};
+};
+
 &srab {
 	compatible = "brcm,bcm53012-srab", "brcm,bcm5301x-srab";
 };
diff --git a/arch/arm/boot/dts/bcm47094.dtsi b/arch/arm/boot/dts/bcm47094.dtsi
index 6282363313e1..239c1c1b0268 100644
--- a/arch/arm/boot/dts/bcm47094.dtsi
+++ b/arch/arm/boot/dts/bcm47094.dtsi
@@ -3,14 +3,12 @@
  * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
  */
 
-#include "bcm4708.dtsi"
+#include "bcm4709.dtsi"
 
 / {
 };
 
 &pinctrl {
-	compatible = "brcm,bcm4709-pinmux";
-
 	pinmux_mdio: mdio-pins {
 		groups = "mdio_grp";
 		function = "mdio";
@@ -21,11 +19,4 @@ &usb3_phy {
 	compatible = "brcm,ns-bx-usb3-phy";
 };
 
-&uart0 {
-	clock-frequency = <125000000>;
-	status = "okay";
-};
 
-&srab {
-	compatible = "brcm,bcm53012-srab", "brcm,bcm5301x-srab";
-};
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index d4f355015e3c..31c6a3dbba30 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -473,6 +473,129 @@ pinmux_uart1: uart1-pins {
 					groups = "uart1_grp";
 					function = "uart1";
 				};
+
+				pins {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					pin@0 {
+						reg = <0>;
+						label = "spi_clk";
+					};
+
+					pin@1 {
+						reg = <1>;
+						label = "spi_ss";
+					};
+
+					pin@2 {
+						reg = <2>;
+						label = "spi_mosi";
+					};
+
+					pin@3 {
+						reg = <3>;
+						label = "spi_miso";
+					};
+
+					pin@4 {
+						reg = <4>;
+						label = "i2c_scl";
+					};
+
+					pin@5 {
+						reg = <5>;
+						label = "i2c_sda";
+					};
+
+					pin@8 {
+						reg = <8>;
+						label = "pwm0";
+					};
+
+					pin@9 {
+						reg = <9>;
+						label = "pwm1";
+					};
+
+					pin@a {
+						reg = <10>;
+						label = "pwm2";
+					};
+
+					pin@b {
+						reg = <11>;
+						label = "pwm3";
+					};
+
+					pin@c {
+						reg = <12>;
+						label = "uart1_rx";
+					};
+
+					pin@d {
+						reg = <13>;
+						label = "uart1_tx";
+					};
+
+					pin@e {
+						reg = <14>;
+						label = "uart1_cts";
+					};
+
+					pin@f {
+						reg = <15>;
+						label = "uart1_rts";
+					};
+				};
+
+				groups {
+					spi_grp: spi_grp {
+						pins = <0 1 2 3>;
+					};
+
+					i2c_grp: i2c_grp {
+						pins = <4 5>;
+					};
+
+					pwm0_grp: pwm0_grp {
+						pins = <8>;
+					};
+
+					pwm1_grp: pwm1_grp {
+						pins = <9>;
+					};
+
+					pwm2_grp: pwm2_grp {
+						pins = <10>;
+					};
+
+					pwm3_grp: pwm3_grp {
+						pins = <11>;
+					};
+
+					uart1_grp: uart1_grp {
+						pins = <12 13 14 15>;
+					};
+				};
+
+				functions {
+					spi {
+						groups = <&spi_grp>;
+					};
+
+					i2c {
+						groups = <&i2c_grp>;
+					};
+
+					pwm {
+						groups = <&pwm0_grp &pwm1_grp &pwm2_grp &pwm3_grp>;
+					};
+
+					uart1 {
+						groups = <&uart1_grp>;
+					};
+				};
 			};
 
 			thermal: thermal@2c0 {
-- 
2.31.1


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

Re: [PATCH 0/5] pinctrl: allow storing pins, groups & functions in DT

[Andy Shevchenko]

On Thu, Nov 18, 2021 at 3:22 PM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> From: Rafał Miłecki <rafal@milecki.pl>
>
> A week ago I sent
> [PATCH RFC] dt-bindings: pinctrl: support specifying pins
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211110231436.8866-1-zajec5@gmail.com/
>
> From short discussion in that thread it seems that using DT to store
> pinctrl pins, groups & functions may be an option. I'd like to ask for
> reviewing my patchset implementing that.
>
> Please note it's about describing hardware elements and not actual
> programming way. It may be used with pinctrl-single.c one day but it's
> designed as a generic solution for data.
>
> Patches 1-4 are for linux-pinctrl.git. Patch 5 I found worth including
> as DT big example. It can go through Linus with Florian's Ack or I can
> send it to Florian later.

DT is not the only one,

So, I would like to see some kind of roadmap / vision on how this can
be useful in the ACPI case.


-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH 3/5] pinctrl: add helpers reading pins, groups & functions from DT

[Andy Shevchenko]

On Thu, Nov 18, 2021 at 3:22 PM Rafał Miłecki <zajec5@gmail.com> wrote:

...

> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c

> +#include <linux/of.h>

I don't like this. This shows not thought through the design of the series.

What I rather expect is a proper interfacing layer that you fill with
options that can be provided by corresponding underlying
implementation, e.g. DT.

Moreover, before doing this you probably would need to refactor the
pin control core to get rid of DT specifics, i.e. abstract them away
first.

-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH 0/5] pinctrl: allow storing pins, groups & functions in DT

[Rafał Miłecki]

On 18.11.2021 14:52, Andy Shevchenko wrote:
> DT is not the only one,
> 
> So, I would like to see some kind of roadmap / vision on how this can
> be useful in the ACPI case.

Unfortunately I don't do any ACPI development and I also have zero ACPI
experience. I'm not sure how to proceed with this. Does ACPI have some
tables that get translated into DT-like form? Can I emulate that and
test somehow?

Anyone else willing to help on the ACPI part?

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

Re: [PATCH 3/5] pinctrl: add helpers reading pins, groups & functions from DT

[Rafał Miłecki]

On 18.11.2021 14:57, Andy Shevchenko wrote:
> On Thu, Nov 18, 2021 at 3:22 PM Rafał Miłecki <zajec5@gmail.com> wrote:
> 
> ...
> 
>> --- a/drivers/pinctrl/pinmux.c
>> +++ b/drivers/pinctrl/pinmux.c
> 
>> +#include <linux/of.h>
> 
> I don't like this. This shows not thought through the design of the series.
> 
> What I rather expect is a proper interfacing layer that you fill with
> options that can be provided by corresponding underlying
> implementation, e.g. DT.
> 
> Moreover, before doing this you probably would need to refactor the
> pin control core to get rid of DT specifics, i.e. abstract them away
> first.

Ouch, it seems like pinctrl got into a tricky state. As I understand it
we need some abstraction layer between DT and pinctrl but noone is
working on it? Does it mean we should consider pinctrl core frozen until
it's refactored?

It's quite inconvenient for me as I'm not sure if I can handle such
heavy pinctrl refactoring while at the same time I'd like to add
those small features to it.

Can you point to an example of extra interfacing layer that could be
used as a reference for what you expect for pinctrl one, please? Some
solution in another Linux's subsystem?

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

Re: [PATCH 3/5] pinctrl: add helpers reading pins, groups & functions from DT

[Andy Shevchenko]

On Thu, Nov 18, 2021 at 4:17 PM Rafał Miłecki <zajec5@gmail.com> wrote:
> On 18.11.2021 14:57, Andy Shevchenko wrote:
> > On Thu, Nov 18, 2021 at 3:22 PM Rafał Miłecki <zajec5@gmail.com> wrote:

...

> >> +#include <linux/of.h>
> >
> > I don't like this. This shows not thought through the design of the series.
> >
> > What I rather expect is a proper interfacing layer that you fill with
> > options that can be provided by corresponding underlying
> > implementation, e.g. DT.
> >
> > Moreover, before doing this you probably would need to refactor the
> > pin control core to get rid of DT specifics, i.e. abstract them away
> > first.
>
> Ouch, it seems like pinctrl got into a tricky state. As I understand it
> we need some abstraction layer between DT and pinctrl but noone is
> working on it?

Seems so to me.
To be more specific, I'm talking about these:

  struct pinctrl_ops {
    ...
    int (*dt_node_to_map)...
    void (*dt_free_map)...
  }

and this:

  struct pinctrl {
    ...
    struct list_head dt_maps;
 }

In the latter case it is almost related to renaming dt_maps to
something like fw_maps.
In both cases it is about decoupling DT stuff out from the pin control core.

So, with something like pinctrl_fw_ops to be introduced, the DT stuff
moved to drivers/pinctrl/devicetree.c.

> Does it mean we should consider pinctrl core frozen until
> it's refactored?

Solutions which are related to pin control core without keeping non-DT
providers in mind will be NAKed by me, definitely. Of course it can be
overridden by maintainers.

> It's quite inconvenient for me as I'm not sure if I can handle such
> heavy pinctrl refactoring while at the same time I'd like to add
> those small features to it.

Which effectively means "let give somebody else even more burden and problems".

> Can you point to an example of extra interfacing layer that could be
> used as a reference for what you expect for pinctrl one, please? Some
> solution in another Linux's subsystem?

GPIOLIB decoupled this.
Another example (not sure if it's good enough here) is the fwnode API
(see fwnode ops).

-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH 0/5] pinctrl: allow storing pins, groups & functions in DT

[Andy Shevchenko]

On Thu, Nov 18, 2021 at 3:59 PM Rafał Miłecki <zajec5@gmail.com> wrote:
> On 18.11.2021 14:52, Andy Shevchenko wrote:
> > DT is not the only one,
> >
> > So, I would like to see some kind of roadmap / vision on how this can
> > be useful in the ACPI case.
>
> Unfortunately I don't do any ACPI development and I also have zero ACPI
> experience. I'm not sure how to proceed with this. Does ACPI have some
> tables that get translated into DT-like form? Can I emulate that and
> test somehow?
>
> Anyone else willing to help on the ACPI part?

As I elaborated in the subthread of patch 3 you don't need to care
about ACPI _implementation_, only about _design_ that will give no
obstacles to have the ACPI part be implemented. I may do it in case
somebody prepares internal APIs and data structures for the
interaction with FW (descriptions). That said, you don't need to know
the ACPI details at all. Ask questions in case you are in doubt, I
would be happy to help.

-- 
With Best Regards,
Andy Shevchenko

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

Re: [PATCH 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Rob Herring]

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

On Thu, 18 Nov 2021 14:21:49 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> pinctrl bindings allow specifying pins, groups & functions now. Put some
> entries in binding example to help writing DTS files.
> 
> Specify pins, groups & functions in the example.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../bindings/pinctrl/brcm,ns-pinmux.yaml      | 29 ++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 

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:
schemas/pinctrl/brcm,ns-pinmux.yaml: ignoring, error in schema: 
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml: 'additionalProperties' is a required property
	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml: ignoring, error in schema: 
warning: no schema found in file: ./Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml
Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.example.dt.yaml:0:0: /example-0/pin-controller@1800c1c0: failed to match any schema with compatible: ['brcm,bcm4708-pinmux']
make[1]: *** Deleting file 'Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml'
schemas/pinctrl/brcm,ns-pinmux.yaml: ignoring, error in schema: 
Traceback (most recent call last):
  File "/usr/local/bin/dt-validate", line 170, in <module>
    sg.check_trees(filename, testtree)
  File "/usr/local/bin/dt-validate", line 119, in check_trees
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 105, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/local/bin/dt-validate", line 49, in check_node
    errors = sorted(dtschema.DTValidator(schema).iter_errors(node), key=lambda e: e.linecol)
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 766, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 224, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 25, in patternProperties
    yield from validator.descend(
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 240, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 766, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 224, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 298, in ref
    yield from validator.descend(instance, resolved)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 240, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 766, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 214, in iter_errors
    scope = id_of(_schema)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 90, in _id_of
    return schema.get("$id", "")
AttributeError: 'NoneType' object has no attribute 'get'
make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/mfd/brcm,cru.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1413: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

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

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 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Rafał Miłecki]

On 18.11.2021 23:09, Rob Herring wrote:
> schemas/pinctrl/brcm,ns-pinmux.yaml: ignoring, error in schema:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/brcm,ns-pinmux.yaml: 'additionalProperties' is a required property
> 	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
> 	from schema $id: http://devicetree.org/meta-schemas/base.yaml#

I forgot to mention this patch is based on top of the
[PATCH 2/2] dt-bindings: pinctrl: use pinctrl.yaml
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211110165720.30242-2-zajec5@gmail.com/

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

Re: [PATCH 3/5] pinctrl: add helpers reading pins, groups & functions from DT

[Linus Walleij]

On Thu, Nov 18, 2021 at 2:58 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Nov 18, 2021 at 3:22 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> > +#include <linux/of.h>
>
> I don't like this. This shows not thought through the design of the series.
>
> What I rather expect is a proper interfacing layer that you fill with
> options that can be provided by corresponding underlying
> implementation, e.g. DT.

I agree, the DT parts should land in pinctrl/devicetree.c in the
end with an opt-in for ACPI & board files. I don't know if we
can use software nodes fully instead of board file-specific
helpers at this point, it seems to have
grown pretty competent.

Yours,
Linus Walleij

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

Re: [PATCH 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Tony Lindgren]

Hi,

200* Rafał Miłecki <zajec5@gmail.com> [211118 13:30]:
> @@ -83,6 +83,33 @@ examples:
>          reg = <0x1800c1c0 0x24>;
>          reg-names = "cru_gpio_control";
>  
> +        pins {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            pin@4 {
> +                reg = <4>;
> +                label = "i2c_scl";
> +            };
> +
> +            pin@5 {
> +                reg = <5>;
> +                label = "i2c_sda";
> +            };
> +        };

The reg property should indicate the hardware offset from the device base
address. The reg values above for 4 and 5 seem to be indexed instead :)
Please update to use real register offsets from the 0x1800c1c0 base
instead. If a reg offset + bit offset are needed, the #address-cells or
#pinctrl-cells can be used.

The main problem using an index is that you need to keep it in sync
between the dts and device driver. And if a new SoC variant adds an entry
between the registers, you end up having to renumber the index.

Regards,

Tony


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

Re: [PATCH 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Rafał Miłecki]

On 23.11.2021 08:38, Tony Lindgren wrote:
> 200* Rafał Miłecki <zajec5@gmail.com> [211118 13:30]:
>> @@ -83,6 +83,33 @@ examples:
>>           reg = <0x1800c1c0 0x24>;
>>           reg-names = "cru_gpio_control";
>>   
>> +        pins {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            pin@4 {
>> +                reg = <4>;
>> +                label = "i2c_scl";
>> +            };
>> +
>> +            pin@5 {
>> +                reg = <5>;
>> +                label = "i2c_sda";
>> +            };
>> +        };
> 
> The reg property should indicate the hardware offset from the device base
> address. The reg values above for 4 and 5 seem to be indexed instead :)
> Please update to use real register offsets from the 0x1800c1c0 base
> instead. If a reg offset + bit offset are needed, the #address-cells or
> #pinctrl-cells can be used.

It's true those are "reg"s are PINS indexes and not actual hw registers.
That concept of changing "reg" context is nothing new here however. It's
used in many other places.

Some examples:

1. net/mdio-mux.yaml
"reg" is used for "The sub-bus number" (not actual hw register)

2. usb/usb-device.yaml
"reg" is used for USB port index on USB hub

3. spi/spi-controller.yaml
"reg" is used for "Chip select used by the device."

4. mtd/partitions/partition.yaml
"reg" is used for "partition's offset and size within the flash"

Does it mean above "reg" usages are all incorrect and binding "reg" in
such way is deprecated? This is something totally new to me, can you
confirm that, please?


> The main problem using an index is that you need to keep it in sync
> between the dts and device driver. And if a new SoC variant adds an entry
> between the registers, you end up having to renumber the index.

I don't understand that part. That index ("reg" value in above example)
is actual PIN number. It's expected to match hw datasheets. Order
doesn't matter there.

If new hw revision adds PIN 2, I could just add pin@2 { ... };

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

Re: [PATCH 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Rafał Miłecki]

On 23.11.2021 08:56, Rafał Miłecki wrote:
> On 23.11.2021 08:38, Tony Lindgren wrote:
>> The main problem using an index is that you need to keep it in sync
>> between the dts and device driver. And if a new SoC variant adds an entry
>> between the registers, you end up having to renumber the index.
> 
> I don't understand that part. That index ("reg" value in above example)
> is actual PIN number. It's expected to match hw datasheets. Order
> doesn't matter there.
> 
> If new hw revision adds PIN 2, I could just add pin@2 { ... };

Actually if you check
[PATCH 5/5] ARM: dts: BCM5301X: add pinctrl pins, groups & functions

you can see that first Northstar SoCs (BCM4708, BCM47081) didn't have
PINs 6, 7, 16, 17, 22 and 23 exposed. I define them only for
later-released Northstar SoC BCM4709 (bcm4709.dtsi).

That works just fine on my hw here.

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

Re: [PATCH 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Tony Lindgren]

* Rafał Miłecki <zajec5@gmail.com> [211123 07:56]:
> Does it mean above "reg" usages are all incorrect and binding "reg" in
> such way is deprecated? This is something totally new to me, can you
> confirm that, please?

Here you have a device with multiple control register instances at
various register offsets. Using index here makes as much sense as
the old interrupt number defines we used to have but got rid of.

Please don't use an index to address them. Index makes sense when
there is no real offset to use, like a SPI chip select, or a bit
offset inside the register like a GPIO instance bit.

Regards,

Tony

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

Re: [PATCH 2/5] dt-bindings: pinctrl: brcm,ns-pinmux: extend example

[Rafał Miłecki]

On 23.11.2021 10:15, Tony Lindgren wrote:
> * Rafał Miłecki <zajec5@gmail.com> [211123 07:56]:
>> Does it mean above "reg" usages are all incorrect and binding "reg" in
>> such way is deprecated? This is something totally new to me, can you
>> confirm that, please?
> 
> Here you have a device with multiple control register instances at
> various register offsets. Using index here makes as much sense as
> the old interrupt number defines we used to have but got rid of.
> 
> Please don't use an index to address them. Index makes sense when
> there is no real offset to use, like a SPI chip select, or a bit
> offset inside the register like a GPIO instance bit.

I think I'll simply trust you on this as there seems to be some thin
line I can't really see. It may be however worth documenting somehwere
what's the rule for changing "reg" context. So that in future less
experienced developers (like me) don't bother maintainers with such
bad concepts.

What I understood from your e-mail is that it's a matter of "reg" usage
in a hardware block binding. If reg contains "multiple control register
instances" (I understand it as reg space size > 0x4) then children nodes
may use "reg" only as address in that register space.

My above understanding doesn't fit however what I see in various
controllers.

*****

Example 1:

usb/generic-xhci.yaml / usb/usb-xhci.yaml

That block binding covers *multiple* registers, see:
reg = <0xf0931000 0x8c8>;

However its children nodes are indexed and use "reg", see:
hub@1 {
     compatible = "usb5e3,610";
     reg = <1>;
};

*****

Example 2:

spi/spi-controller.yaml

That binding uses "fsl,imx28-spi" as example. Its binding covers
*multiple* registers, see:
reg = <0x80010000 0x2000>;

However its children nodes are indexed and use "reg", see:
display@0 {
     compatible = "lg,lg4573";
     spi-max-frequency = <1000000>;
     reg = <0>;
};

*****

So it appears my understanding is wrong somewhere. It seems to be a bit
tricky to get things right so I'd really appreciate some documentation
on that.

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