[PATCH v3 0/7] pinctrl: Add generic pinctrl for board-level mux chips

[PATCH v3 0/7] pinctrl: Add generic pinctrl for board-level mux chips

[Frank Li]

Add a generic pinctrl binding for board-level pinmux chips that are
controlled through the multiplexer subsystem.

On some boards, especially development boards, external mux chips are used
to switch SoC signals between different peripherals (e.g. MMC and UART).
The mux select lines are often driven by a GPIO expander over I2C,
as illustrated below:

        ┌──────┐      ┌─────┐
        │ SOC  │      │     │    ┌───────┐
        │      │      │     │───►│ MMC   │
        │      │      │ MUX │    └───────┘
        │      ├─────►│     │    ┌───────┐
        │      │      │     │───►│ UART  │
        │      │      └─────┘    └───────┘
        │      │         ▲
        │      │    ┌────┴──────────────┐
        │ I2C  ├───►│ GPIO Expander     │
        └──────┘    └───────────────────┘

Traditionally, gpio-hog is used to configure the onboard mux at boot.
However, the GPIO expander may probe later than consumer devices such as
MMC. As a result, the MUX might not be configured when the peripheral
driver probes, leading to initialization failures or data transfer errors.

Introduce a generic pinctrl binding that models the board-level MUX as a
pin control provider and builds proper device links between the MUX, its
GPIO controller, and peripheral devices. This ensures correct probe
ordering and reliable mux configuration.

The implementation leverages the standard multiplexer subsystem, which
provides broad support for onboard mux controllers and avoids the need for
per-driver custom MUX handling

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v3:
- collect rob's review tag for binding
- extend and use pinctrl_generic_pins_function_dt_node_to_map()
- add judgement about
commit 2243a87d90b42eb38bc281957df3e57c712b5e56
"pinctrl: avoid duplicated calling enable_pinmux_setting for a pin"

which call pinmux_disable_setting() before pinmux_enable_setting() when
switch state. It is actually what wanted. Previous remove .disable() to
avoid hardware glitch when switch state.

New .release_mux() call intent just release software resource, like lock,
don't touch hardware register. No glitch involve. Comments already added

Linus Walleij:
  I hope this answer all of your questions. If I missed, let me know

- Link to v2: https://lore.kernel.org/r/20260225-pinctrl-mux-v2-0-1436a25fa454@nxp.com

Changes in v2:
- Add release_mux callback,
  test insmod/rmmod, mux_state_(de)select() called.
- Link to v1: https://lore.kernel.org/r/20260219-pinctrl-mux-v1-0-678d21637788@nxp.com

---
Frank Li (7):
      mux: add devm_mux_control_get_from_np() to get mux from child node
      dt-bindings: pinctrl: Add generic pinctrl for board-level mux chips
      pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()
      pinctrl: add optional .release_mux() callback
      pinctrl: add generic board-level pinctrl driver using mux framework
      arm64: dts: imx8mp-evk: add board-level mux for CAN2 and MICFIL
      arm64: dts: imx8mp-evk: add flexcan2 overlay file

 .../bindings/pinctrl/pinctrl-multiplexer.yaml      |  57 +++++++
 .../devicetree/bindings/pinctrl/pinctrl.yaml       |   2 +-
 arch/arm64/boot/dts/freescale/Makefile             |   4 +
 .../boot/dts/freescale/imx8mp-evk-flexcan2.dtso    |  15 ++
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts       |  23 ++-
 drivers/mux/core.c                                 |  40 +++--
 drivers/pinctrl/Kconfig                            |   9 ++
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinconf.h                          |  21 ++-
 drivers/pinctrl/pinctrl-generic-mux.c              | 178 +++++++++++++++++++++
 drivers/pinctrl/pinctrl-generic.c                  |  41 +++--
 drivers/pinctrl/pinmux.c                           |   5 +
 include/linux/mux/consumer.h                       |  16 +-
 include/linux/pinctrl/pinmux.h                     |   5 +
 14 files changed, 373 insertions(+), 44 deletions(-)
---
base-commit: ff76d257e86235eb07ef33db8644a517c48d1c3f
change-id: 20260213-pinctrl-mux-df9c5b661540

Best regards,
--
Frank Li <Frank.Li@nxp.com>

[PATCH v3 1/7] mux: add devm_mux_control_get_from_np() to get mux from child node

[Frank Li]

Add new API devm_mux_control_get_from_np() to retrieve a mux control from
a specified child device node.

Make devm_mux_control_get() call devm_mux_control_get_from_np() with a NULL
node parameter, which defaults to using the device's own of_node.

Support the following DT schema:

pinctrl@0 {
    uart-func {
            mux-state = <&mux_chip 0>;
    };

    spi-func {
            mux-state = <&mux_chip 1>;
    };
};

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/mux/core.c           | 40 ++++++++++++++++++++++++----------------
 include/linux/mux/consumer.h | 16 ++++++++++++----
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index a3840fe0995fe0125432d34edd8ab0f2cd1a6e9a..bdd959389b4ee1b0b8a7367fadf2c148c8f2f0b1 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -522,13 +522,15 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
  * @mux_name: The name identifying the mux-control.
  * @state: Pointer to where the requested state is returned, or NULL when
  *         the required multiplexer states are handled by other means.
+ * @node: the device nodes, use dev->of_node if it is NULL.
  *
  * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
  */
 static struct mux_control *mux_get(struct device *dev, const char *mux_name,
-				   unsigned int *state)
+				   unsigned int *state,
+				   struct device_node *node)
 {
-	struct device_node *np = dev->of_node;
+	struct device_node *np = node ? node : dev->of_node;
 	struct of_phandle_args args;
 	struct mux_chip *mux_chip;
 	unsigned int controller;
@@ -617,7 +619,7 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
  */
 struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
 {
-	return mux_get(dev, mux_name, NULL);
+	return mux_get(dev, mux_name, NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(mux_control_get);
 
@@ -641,15 +643,17 @@ static void devm_mux_control_release(struct device *dev, void *res)
 }
 
 /**
- * devm_mux_control_get() - Get the mux-control for a device, with resource
- *			    management.
+ * devm_mux_control_get_from_np() - Get the mux-control for a device, with
+ *				    resource management.
  * @dev: The device that needs a mux-control.
  * @mux_name: The name identifying the mux-control.
+ * @np: the device nodes, use dev->of_node if it is NULL.
  *
  * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
  */
-struct mux_control *devm_mux_control_get(struct device *dev,
-					 const char *mux_name)
+struct mux_control *
+devm_mux_control_get_from_np(struct device *dev, const char *mux_name,
+			     struct device_node *np)
 {
 	struct mux_control **ptr, *mux;
 
@@ -668,16 +672,18 @@ struct mux_control *devm_mux_control_get(struct device *dev,
 
 	return mux;
 }
-EXPORT_SYMBOL_GPL(devm_mux_control_get);
+EXPORT_SYMBOL_GPL(devm_mux_control_get_from_np);
 
 /*
  * mux_state_get() - Get the mux-state for a device.
  * @dev: The device that needs a mux-state.
  * @mux_name: The name identifying the mux-state.
+ * @np: the device nodes, use dev->of_node if it is NULL.
  *
  * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno.
  */
-static struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
+static struct mux_state *
+mux_state_get(struct device *dev, const char *mux_name, struct device_node *np)
 {
 	struct mux_state *mstate;
 
@@ -685,7 +691,7 @@ static struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
 	if (!mstate)
 		return ERR_PTR(-ENOMEM);
 
-	mstate->mux = mux_get(dev, mux_name, &mstate->state);
+	mstate->mux = mux_get(dev, mux_name, &mstate->state, np);
 	if (IS_ERR(mstate->mux)) {
 		int err = PTR_ERR(mstate->mux);
 
@@ -716,15 +722,17 @@ static void devm_mux_state_release(struct device *dev, void *res)
 }
 
 /**
- * devm_mux_state_get() - Get the mux-state for a device, with resource
- *			  management.
+ * devm_mux_state_get_from_np() - Get the mux-state for a device, with resource
+ *				  management.
  * @dev: The device that needs a mux-control.
  * @mux_name: The name identifying the mux-control.
+ * @np: the device nodes, use dev->of_node if it is NULL.
  *
  * Return: Pointer to the mux-state, or an ERR_PTR with a negative errno.
  */
-struct mux_state *devm_mux_state_get(struct device *dev,
-				     const char *mux_name)
+struct mux_state *
+devm_mux_state_get_from_np(struct device *dev, const char *mux_name,
+			   struct device_node *np)
 {
 	struct mux_state **ptr, *mstate;
 
@@ -732,7 +740,7 @@ struct mux_state *devm_mux_state_get(struct device *dev,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	mstate = mux_state_get(dev, mux_name);
+	mstate = mux_state_get(dev, mux_name, np);
 	if (IS_ERR(mstate)) {
 		devres_free(ptr);
 		return mstate;
@@ -743,7 +751,7 @@ struct mux_state *devm_mux_state_get(struct device *dev,
 
 	return mstate;
 }
-EXPORT_SYMBOL_GPL(devm_mux_state_get);
+EXPORT_SYMBOL_GPL(devm_mux_state_get_from_np);
 
 /*
  * Using subsys_initcall instead of module_init here to try to ensure - for
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index 2e25c838f8312532040441ee618424b76378aad7..6300e091035323dd6158d52a55a109d43ef120aa 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -56,9 +56,17 @@ int mux_state_deselect(struct mux_state *mstate);
 struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
 void mux_control_put(struct mux_control *mux);
 
-struct mux_control *devm_mux_control_get(struct device *dev,
-					 const char *mux_name);
-struct mux_state *devm_mux_state_get(struct device *dev,
-				     const char *mux_name);
+struct mux_control *
+devm_mux_control_get_from_np(struct device *dev, const char *mux_name,
+			     struct device_node *np);
+
+#define devm_mux_control_get(dev, mux_name)		\
+	devm_mux_control_get_from_np(dev, mux_name, NULL)
+
+struct mux_state *
+devm_mux_state_get_from_np(struct device *dev, const char *mux_name,
+			   struct device_node *np);
+#define devm_mux_state_get(dev, mux_name)		\
+	devm_mux_state_get_from_np(dev, mux_name, NULL)
 
 #endif /* _LINUX_MUX_CONSUMER_H */

-- 
2.43.0

[PATCH v3 2/7] dt-bindings: pinctrl: Add generic pinctrl for board-level mux chips

[Frank Li]

Add a generic pinctrl binding for board-level pinmux chips that are
controlled through the multiplexer subsystem.

On some boards, especially development boards, external mux chips are used
to switch SoC signals between different peripherals (e.g. MMC and UART).
The mux select lines are often driven by a GPIO expander over I2C,
as illustrated below:

	┌──────┐      ┌─────┐
	│ SOC  │      │     │    ┌───────┐
	│      │      │     │───►│ MMC   │
	│      │      │ MUX │    └───────┘
	│      ├─────►│     │    ┌───────┐
	│      │      │     │───►│ UART  │
	│      │      └─────┘    └───────┘
	│      │         ▲
	│      │    ┌────┴──────────────┐
	│ I2C  ├───►│ GPIO Expander     │
	└──────┘    └───────────────────┘

Traditionally, gpio-hog is used to configure the onboard mux at boot.
However, the GPIO expander may probe later than consumer devices such as
MMC. As a result, the MUX might not be configured when the peripheral
driver probes, leading to initialization failures or data transfer errors.

Introduce a generic pinctrl binding that models the board-level MUX as a
pin control provider and builds proper device links between the MUX, its
GPIO controller, and peripheral devices. This ensures correct probe
ordering and reliable mux configuration.

The implementation leverages the standard multiplexer subsystem, which
provides broad support for onboard mux controllers and avoids the need for
per-driver custom MUX handling.

Allow pinctrl-* pattern as node name because this pinctrl device have not
reg property.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3:
- collect rob's reviewed-by tag.

change in v2:
 - change descriptions for device, not for driver
 - add missed additionalProperties: false
---
 .../bindings/pinctrl/pinctrl-multiplexer.yaml      | 57 ++++++++++++++++++++++
 .../devicetree/bindings/pinctrl/pinctrl.yaml       |  2 +-
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-multiplexer.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-multiplexer.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..2b0385ed879b70b24ca9c39b098c3840d08d7482
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-multiplexer.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/pinctrl-multiplexer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic pinctrl device for on-board MUX Chips
+
+maintainers:
+  - Frank Li <Frank.Li@nxp.com>
+
+description:
+  Generic pinctrl device for on-board MUX Chips, which switch SoC signals
+  between different peripherals (e.g. MMC and UART).
+
+  The MUX select lines are often driven by a I2C GPIO expander.
+
+properties:
+  compatible:
+    const: pinctrl-multiplexer
+
+patternProperties:
+  '-grp$':
+    type: object
+    additionalProperties: false
+    properties:
+      mux-states:
+        maxItems: 1
+
+    required:
+      - mux-states
+
+required:
+  - compatible
+
+allOf:
+  - $ref: pinctrl.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    pinctrl-mux {
+        compatible = "pinctrl-multiplexer";
+
+        uart-grp {
+            mux-states = <&mux 0>;
+        };
+
+        spi-grp {
+            mux-states = <&mux 1>;
+        };
+
+        i2c-grp {
+            mux-states = <&mux 2>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
index 290438826c507ec6725f486d18cf686aa7c35e67..20176bf3074757de30f208e69b968a6bd6125273 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
@@ -27,7 +27,7 @@ description: |
 
 properties:
   $nodename:
-    pattern: "^(pinctrl|pinmux)(@[0-9a-f]+)?$"
+    pattern: "^(pinctrl|pinmux)(@[0-9a-f]+|-[a-z0-9]+)?$"
 
   "#pinctrl-cells":
     description: >

-- 
2.43.0

[PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Frank Li]

Introduce __pinctrl_generic_pins_function_dt_node_to_map() to allow
passing private data and skip_npins to pinmux_generic_add_function().

The 'skip_npins' to skip parse pins in dts because on boards MUX control
chip switch the whole group together, so needn't handle each pins.

Keep pinctrl_generic_pins_function_dt_node_to_map() as a wrapper
calling the new helper with a NULL argument to preserve backward
compatibility.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v3
- new patch
---
 drivers/pinctrl/pinconf.h         | 21 ++++++++++++++++----
 drivers/pinctrl/pinctrl-generic.c | 41 +++++++++++++++++++++++----------------
 2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index 2880adef476e68950ffdd540ea42cdee6a16ec27..d6d22c088a144de0180314826b8cb8c528290970 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -162,10 +162,23 @@ pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
 #endif
 
 #if defined(CONFIG_GENERIC_PINCTRL) && defined (CONFIG_OF)
-int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
-						 struct device_node *np,
-						 struct pinctrl_map **maps,
-						 unsigned int *num_maps);
+int __pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
+						   struct device_node *np,
+						   struct pinctrl_map **maps,
+						   unsigned int *num_maps,
+						   void *func_data,
+						   bool skip_npins);
+static inline int
+pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
+					     struct device_node *np,
+					     struct pinctrl_map **maps,
+					     unsigned int *num_maps)
+{
+	return __pinctrl_generic_pins_function_dt_node_to_map(pctldev, np, maps,
+							      num_maps, NULL,
+							      false);
+}
+
 #else
 static inline int
 pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index efb39c6a670331775855efdc8566102b5c6202ef..dc575fbcc7103bebded49d8268764531f349a8c9 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -24,21 +24,24 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
 							   unsigned int *num_maps,
 							   unsigned int *num_reserved_maps,
 							   const char **group_names,
-							   unsigned int ngroups)
+							   unsigned int ngroups,
+							   bool skip_npins)
 {
 	struct device *dev = pctldev->dev;
 	const char **functions;
 	const char *group_name;
 	unsigned long *configs;
 	unsigned int num_configs, pin, *pins;
-	int npins, ret, reserve = 1;
+	int npins = 0, ret, reserve = 1;
 
-	npins = of_property_count_u32_elems(np, "pins");
+	if (!skip_npins) {
+		npins = of_property_count_u32_elems(np, "pins");
 
-	if (npins < 1) {
-		dev_err(dev, "invalid pinctrl group %pOFn.%pOFn %d\n",
-			parent, np, npins);
-		return npins;
+		if (npins < 1) {
+			dev_err(dev, "invalid pinctrl group %pOFn.%pOFn %d\n",
+				parent, np, npins);
+			return npins;
+		}
 	}
 
 	group_name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn", parent, np);
@@ -110,10 +113,12 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
  * and functions, create them in addition to parsing pinconf properties and
  * adding mappings.
  */
-int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
-						 struct device_node *np,
-						 struct pinctrl_map **maps,
-						 unsigned int *num_maps)
+int __pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
+						   struct device_node *np,
+						   struct pinctrl_map **maps,
+						   unsigned int *num_maps,
+						   void *func_data,
+						   bool skip_npins)
 {
 	struct device *dev = pctldev->dev;
 	struct device_node *child_np;
@@ -129,7 +134,7 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
 	 * Check if this is actually the pins node, or a parent containing
 	 * multiple pins nodes.
 	 */
-	if (!of_property_present(np, "pins"))
+	if (!of_property_present(np, "pins") && !skip_npins)
 		goto parent;
 
 	group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
@@ -140,13 +145,14 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
 							      maps, num_maps,
 							      &num_reserved_maps,
 							      group_names,
-							      ngroups);
+							      ngroups,
+							      skip_npins);
 	if (ret) {
 		pinctrl_utils_free_map(pctldev, *maps, *num_maps);
 		return dev_err_probe(dev, ret, "error figuring out mappings for %s\n", np->name);
 	}
 
-	ret = pinmux_generic_add_function(pctldev, np->name, group_names, 1, NULL);
+	ret = pinmux_generic_add_function(pctldev, np->name, group_names, 1, func_data);
 	if (ret < 0) {
 		pinctrl_utils_free_map(pctldev, *maps, *num_maps);
 		return dev_err_probe(dev, ret, "error adding function %s\n", np->name);
@@ -168,7 +174,8 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
 								      maps, num_maps,
 								      &num_reserved_maps,
 								      group_names,
-								      ngroups);
+								      ngroups,
+								      skip_npins);
 		if (ret) {
 			pinctrl_utils_free_map(pctldev, *maps, *num_maps);
 			return dev_err_probe(dev, ret, "error figuring out mappings for %s\n",
@@ -178,7 +185,7 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
 		ngroups++;
 	}
 
-	ret = pinmux_generic_add_function(pctldev, np->name, group_names, ngroups, NULL);
+	ret = pinmux_generic_add_function(pctldev, np->name, group_names, ngroups, func_data);
 	if (ret < 0) {
 		pinctrl_utils_free_map(pctldev, *maps, *num_maps);
 		return dev_err_probe(dev, ret, "error adding function %s\n", np->name);
@@ -186,4 +193,4 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(pinctrl_generic_pins_function_dt_node_to_map);
+EXPORT_SYMBOL_GPL(__pinctrl_generic_pins_function_dt_node_to_map);

-- 
2.43.0

[PATCH v3 4/7] pinctrl: add optional .release_mux() callback

[Frank Li]

Add an optional .release_mux() callback to struct pinmux_ops.

Some drivers acquire additional resources in .set_mux(), such as software
locks. These resources may need to be released when the mux function is no
longer active. Introducing a dedicated .release_mux() callback allows
drivers to clean up such resources.

The callback is optional and does not affect existing drivers.

Commit 2243a87d90b42 ("pinctrl: avoid duplicated calling
enable_pinmux_setting for a pin") removed the .disable() callback
to resolve two issues:

  1. desc->mux_usecount increasing monotonically
  2. Hardware glitches caused by repeated .disable()/.enable() calls

Adding .release_mux() does not reintroduce those problems. The callback is
intended only for releasing driver-side resources (e.g. locks) and must not
modify hardware registers.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3
- Add judgement about 2243a87d90b42 ("pinctrl: avoid duplicated calling
enable_pinmux_setting for a pin") in commit message.
---
 drivers/pinctrl/pinmux.c       | 5 +++++
 include/linux/pinctrl/pinmux.h | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 3a8dd184ba3d670e01a890427e19af59b65eb813..c705bc182266c596c4e6c820f5e3ffcadbbb2838 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -517,6 +517,7 @@ void pinmux_disable_setting(const struct pinctrl_setting *setting)
 {
 	struct pinctrl_dev *pctldev = setting->pctldev;
 	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
+	const struct pinmux_ops *ops = pctldev->desc->pmxops;
 	int ret = 0;
 	const unsigned int *pins = NULL;
 	unsigned int num_pins = 0;
@@ -563,6 +564,10 @@ void pinmux_disable_setting(const struct pinctrl_setting *setting)
 				 pins[i], desc->name, gname);
 		}
 	}
+
+	if (ops->release_mux)
+		ops->release_mux(pctldev, setting->data.mux.func,
+				 setting->data.mux.group);
 }
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/pinctrl/pinmux.h b/include/linux/pinctrl/pinmux.h
index 094bbe2fd6fd5ea3c5fdf5b6d6d9a7639700b50b..77664937eeb273eef440988c4cf833dbc6f10758 100644
--- a/include/linux/pinctrl/pinmux.h
+++ b/include/linux/pinctrl/pinmux.h
@@ -51,6 +51,8 @@ struct pinctrl_gpio_range;
  *	are handled by the pinmux subsystem. The @func_selector selects a
  *	certain function whereas @group_selector selects a certain set of pins
  *	to be used. On simple controllers the latter argument may be ignored
+ * @release_mux: Release software resources acquired by @set_mux. This callback
+ *	must not change hardware state to avoid glitches when switching mux.
  * @gpio_request_enable: requests and enables GPIO on a certain pin.
  *	Implement this only if you can mux every pin individually as GPIO. The
  *	affected GPIO range is passed along with an offset(pin number) into that
@@ -80,6 +82,9 @@ struct pinmux_ops {
 				  unsigned int selector);
 	int (*set_mux) (struct pinctrl_dev *pctldev, unsigned int func_selector,
 			unsigned int group_selector);
+	void (*release_mux) (struct pinctrl_dev *pctldev,
+			     unsigned int func_selector,
+			     unsigned int group_selector);
 	int (*gpio_request_enable) (struct pinctrl_dev *pctldev,
 				    struct pinctrl_gpio_range *range,
 				    unsigned int offset);

-- 
2.43.0

[PATCH v3 5/7] pinctrl: add generic board-level pinctrl driver using mux framework

[Frank Li]

Many boards use on-board mux chips (often controlled by GPIOs from an I2C
expander) to switch shared signals between peripherals.

Add a generic pinctrl driver built on top of the mux framework to
centralize mux handling and avoid probe ordering issues. Keep board-level
routing out of individual drivers and supports boot-time only mux
selection.

Ensure correct probe ordering, especially when the GPIO expander is probed
later.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3:
- use pinctrl_generic_pins_function_dt_node_to_map() and
pinctrl_utils_free_map().

change in v2:
- fix copywrite by add nxp
- fix if (!*map) check
- add release_mux to call mux_state_deselect()
---
 drivers/pinctrl/Kconfig               |   9 ++
 drivers/pinctrl/Makefile              |   1 +
 drivers/pinctrl/pinctrl-generic-mux.c | 178 ++++++++++++++++++++++++++++++++++
 3 files changed, 188 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index afecd9407f5354f5b92223f8cd80d2f7a08f8e7d..b6d4755e67510786c34f890c5e7a3fcf0adf45e4 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -274,6 +274,15 @@ config PINCTRL_GEMINI
 	select GENERIC_PINCONF
 	select MFD_SYSCON
 
+config PINCTRL_GENERIC_MUX
+	tristate "Generic Pinctrl driver by using multiplexer"
+	depends on MULTIPLEXER
+	select PINMUX
+	select GENERIC_PINCTRL
+	help
+          Generic pinctrl driver by MULTIPLEXER framework to control on
+          board pin selection.
+
 config PINCTRL_INGENIC
 	bool "Pinctrl driver for the Ingenic JZ47xx SoCs"
 	default MACH_INGENIC
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f7d5d5f76d0c8becc0aa1d77c68b6ced924ea264..fcd1703440d24579636e8ddb6cbd83a0a982dfb7 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PINCTRL_EQUILIBRIUM)   += pinctrl-equilibrium.o
 obj-$(CONFIG_PINCTRL_EP93XX)	+= pinctrl-ep93xx.o
 obj-$(CONFIG_PINCTRL_EYEQ5)	+= pinctrl-eyeq5.o
 obj-$(CONFIG_PINCTRL_GEMINI)	+= pinctrl-gemini.o
+obj-$(CONFIG_PINCTRL_GENERIC_MUX) += pinctrl-generic-mux.o
 obj-$(CONFIG_PINCTRL_INGENIC)	+= pinctrl-ingenic.o
 obj-$(CONFIG_PINCTRL_K210)	+= pinctrl-k210.o
 obj-$(CONFIG_PINCTRL_K230)	+= pinctrl-k230.o
diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
new file mode 100644
index 0000000000000000000000000000000000000000..fd23b04e9bcdc008ee20bf0a35d6b0365dd2d2d9
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-generic-mux.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic Pin Control Driver for Board-Level Mux Chips
+ * Copyright 2026 NXP
+ */
+
+#include <linux/cleanup.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/mutex.h>
+#include <linux/mux/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/slab.h>
+
+#include "core.h"
+#include "pinconf.h"
+#include "pinmux.h"
+#include "pinctrl-utils.h"
+
+struct mux_pin_function {
+	struct mux_state *mux_state;
+};
+
+struct mux_pinctrl {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+
+	/* mutex protect [pinctrl|pinmux]_generic functions */
+	struct mutex lock;
+	int cur_select;
+};
+
+static int
+mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
+			  struct device_node *np_config,
+			  struct pinctrl_map **map, unsigned int *num_maps)
+{
+	struct mux_pin_function *function;
+
+	function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
+	if (!function)
+		return -ENOMEM;
+
+	function->mux_state = devm_mux_state_get_from_np(pctldev->dev, NULL, np_config);
+	if (IS_ERR(function->mux_state))
+		return PTR_ERR(function->mux_state);
+
+	return __pinctrl_generic_pins_function_dt_node_to_map(pctldev, np_config, map,
+							      num_maps, function,
+							      true);
+}
+
+static const struct pinctrl_ops mux_pinctrl_ops = {
+	.get_groups_count = pinctrl_generic_get_group_count,
+	.get_group_name = pinctrl_generic_get_group_name,
+	.get_group_pins = pinctrl_generic_get_group_pins,
+	.dt_node_to_map = mux_pinmux_dt_node_to_map,
+	.dt_free_map = pinctrl_utils_free_map,
+};
+
+static int mux_pinmux_set_mux(struct pinctrl_dev *pctldev,
+			      unsigned int func_selector,
+			      unsigned int group_selector)
+{
+	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct function_desc *function;
+	struct mux_pin_function *func;
+	int ret;
+
+	guard(mutex)(&mpctl->lock);
+
+	function = pinmux_generic_get_function(pctldev, func_selector);
+	func = function->data;
+
+	if (mpctl->cur_select == func_selector)
+		return 0;
+
+	if (mpctl->cur_select >= 0 && mpctl->cur_select != func_selector)
+		return -EINVAL;
+
+	ret = mux_state_select(func->mux_state);
+	if (ret)
+		return ret;
+
+	mpctl->cur_select = func_selector;
+
+	return 0;
+}
+
+static void mux_pinmux_release_mux(struct pinctrl_dev *pctldev,
+				   unsigned int func_selector,
+				   unsigned int group_selector)
+{
+	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct function_desc *function;
+	struct mux_pin_function *func;
+
+	guard(mutex)(&mpctl->lock);
+
+	function = pinmux_generic_get_function(pctldev, func_selector);
+	func = function->data;
+
+	mux_state_deselect(func->mux_state);
+
+	mpctl->cur_select = -1;
+}
+
+static const struct pinmux_ops mux_pinmux_ops = {
+	.get_functions_count = pinmux_generic_get_function_count,
+	.get_function_name = pinmux_generic_get_function_name,
+	.get_function_groups = pinmux_generic_get_function_groups,
+	.set_mux = mux_pinmux_set_mux,
+	.release_mux = mux_pinmux_release_mux,
+};
+
+static int mux_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mux_pinctrl *mpctl;
+	struct pinctrl_desc *pctl_desc;
+	int ret;
+
+	mpctl = devm_kzalloc(dev, sizeof(*mpctl), GFP_KERNEL);
+	if (!mpctl)
+		return -ENOMEM;
+
+	mpctl->dev = dev;
+	mpctl->cur_select = -1;
+
+	platform_set_drvdata(pdev, mpctl);
+
+	pctl_desc = devm_kzalloc(dev, sizeof(*pctl_desc), GFP_KERNEL);
+	if (!pctl_desc)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(dev, &mpctl->lock);
+	if (ret)
+		return ret;
+
+	pctl_desc->name = dev_name(dev);
+	pctl_desc->owner = THIS_MODULE;
+	pctl_desc->pctlops = &mux_pinctrl_ops;
+	pctl_desc->pmxops = &mux_pinmux_ops;
+
+	ret = devm_pinctrl_register_and_init(dev, pctl_desc, mpctl,
+					     &mpctl->pctl);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
+
+	ret = pinctrl_enable(mpctl->pctl);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
+
+	return 0;
+}
+
+static const struct of_device_id mux_pinctrl_of_match[] = {
+	{ .compatible = "pinctrl-multiplexer" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mux_pinctrl_of_match);
+
+static struct platform_driver mux_pinctrl_driver = {
+	.driver = {
+		.name = "generic-pinctrl-mux",
+		.of_match_table = mux_pinctrl_of_match,
+	},
+	.probe = mux_pinctrl_probe,
+};
+module_platform_driver(mux_pinctrl_driver);
+
+MODULE_AUTHOR("Frank Li <Frank.Li@nxp.com>");
+MODULE_DESCRIPTION("Generic Pin Control Driver for Board-Level Mux Chips");
+MODULE_LICENSE("GPL");
+

-- 
2.43.0

[PATCH v3 6/7] arm64: dts: imx8mp-evk: add board-level mux for CAN2 and MICFIL

[Frank Li]

The board integrates an on-board mux to route shared signals to either
CAN2 or PDM (MICFIL). The mux is controlled by a GPIO.

Add a pinctrl-based multiplexer node to describe this routing and ensure
proper probe ordering of the dependent devices.

Previously, MICFIL operation implicitly depended on the default level of
PCA6416 GPIO3. After adding the pinctrl-multiplexer, make the dependency
explicit.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3
- none
change in v2
- update commit message to show why need update PDM MICIFL.
---
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index b256be710ea1281465f5cecc7a3b979f2c068e43..1341ee27239fd41a26117adc9023524ce50420a7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -50,6 +50,25 @@ status {
 		};
 	};
 
+	can_mux: mux-controller-0 {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+		#mux-state-cells = <1>;
+		mux-gpios = <&pca6416 3 GPIO_ACTIVE_HIGH>;
+	};
+
+	can_mux_pinctrl: pinctrl-gpiomux {
+		compatible = "pinctrl-multiplexer";
+
+		can_fun: can-grp {
+			mux-states = <&can_mux 1>;
+		};
+
+		pdm_fun: pdm-grp {
+			mux-states = <&can_mux 0>;
+		};
+	};
+
 	memory@40000000 {
 		device_type = "memory";
 		reg = <0x0 0x40000000 0 0xc0000000>,
@@ -446,7 +465,7 @@ &flexcan1 {
 
 &flexcan2 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_flexcan2>;
+	pinctrl-0 = <&pinctrl_flexcan2>, <&can_fun>;
 	phys = <&flexcan_phy 1>;
 	status = "disabled";/* can2 pin conflict with pdm */
 };
@@ -712,7 +731,7 @@ &lcdif3 {
 &micfil {
 	#sound-dai-cells = <0>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_pdm>;
+	pinctrl-0 = <&pinctrl_pdm>, <&pdm_fun>;
 	assigned-clocks = <&clk IMX8MP_CLK_PDM>;
 	assigned-clock-parents = <&clk IMX8MP_AUDIO_PLL1_OUT>;
 	assigned-clock-rates = <196608000>;

-- 
2.43.0

[PATCH v3 7/7] arm64: dts: imx8mp-evk: add flexcan2 overlay file

[Frank Li]

Add flexcan2 overlay file, which enable flexcan2 node and disable micfil
node.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3
- none
---
 arch/arm64/boot/dts/freescale/Makefile                 |  4 ++++
 arch/arm64/boot/dts/freescale/imx8mp-evk-flexcan2.dtso | 15 +++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 700bab4d3e6001fe6cf460fcb09cfe57acc77e36..bd377191a68a6167d5f9a65184d19c789a4223ee 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -233,6 +233,10 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk3.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-picoitx.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-edm-g-wb.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
+
+imx8mp-evk-flexcan2-dtbs += imx8mp-evk.dtb imx8mp-evk-flexcan2.dtbo
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk-flexcan2.dtb
+
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-frdm.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-hummingboard-mate.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mp-hummingboard-pro.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk-flexcan2.dtso b/arch/arm64/boot/dts/freescale/imx8mp-evk-flexcan2.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..f7d2674c45f72353a20300300e98c8a1eba4a2a6
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk-flexcan2.dtso
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2026 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+&flexcan2 {
+        status = "okay"; /* can2 pin conflict with pdm */
+};
+
+&micfil {
+        status = "disabled";
+};

-- 
2.43.0

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Linus Walleij]

On Wed, Mar 11, 2026 at 8:08 PM Frank Li <Frank.Li@nxp.com> wrote:

> Introduce __pinctrl_generic_pins_function_dt_node_to_map() to allow
> passing private data and skip_npins to pinmux_generic_add_function().
>
> The 'skip_npins' to skip parse pins in dts because on boards MUX control
> chip switch the whole group together, so needn't handle each pins.
>
> Keep pinctrl_generic_pins_function_dt_node_to_map() as a wrapper
> calling the new helper with a NULL argument to preserve backward
> compatibility.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Ad attested by several reviews, the pinctrl subsystem maintainer
strongly dislikes any use of __double_underscore_function_names().

The reason I dislike it is because it is ambiguous.

For example there are __compiler_intrinsics such as
__iomem and all the stuff from <linux/compiler_types.h>.

Then there are __non_atomics such as __set_bit().

This means __inner_function() just adds to this mess and creates
a big confusion for the mind.

That said: in this case you're just adding a parameter, just add
the parameter and change all of the in-tree users to pass false
or whatever you need, these is just one (1) in-tree user anyway.

Yours,
Linus Walleij

Re: [PATCH v3 4/7] pinctrl: add optional .release_mux() callback

[Linus Walleij]

On Wed, Mar 11, 2026 at 8:08 PM Frank Li <Frank.Li@nxp.com> wrote:

> Add an optional .release_mux() callback to struct pinmux_ops.
>
> Some drivers acquire additional resources in .set_mux(), such as software
> locks. These resources may need to be released when the mux function is no
> longer active. Introducing a dedicated .release_mux() callback allows
> drivers to clean up such resources.
>
> The callback is optional and does not affect existing drivers.
>
> Commit 2243a87d90b42 ("pinctrl: avoid duplicated calling
> enable_pinmux_setting for a pin") removed the .disable() callback
> to resolve two issues:
>
>   1. desc->mux_usecount increasing monotonically
>   2. Hardware glitches caused by repeated .disable()/.enable() calls
>
> Adding .release_mux() does not reintroduce those problems. The callback is
> intended only for releasing driver-side resources (e.g. locks) and must not
> modify hardware registers.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

OK fair enough, I think I'm convinced about this now, also it has
a nice pairing with set_mux(). Let's see if someone else has comments,
I might apply v4 after fixing the generic function issue in patch 2.

Yours,
Linus Walleij

Re: [PATCH v3 0/7] pinctrl: Add generic pinctrl for board-level mux chips

[Linus Walleij]

On Wed, Mar 11, 2026 at 8:08 PM Frank Li <Frank.Li@nxp.com> wrote:

> Add a generic pinctrl binding for board-level pinmux chips that are
> controlled through the multiplexer subsystem.

I really like this version, I had a minor comment on refactoring the
generic helper instead of wrapping it (plus some random ramblings
from my side, sorry). With this fixed I feel I can apply v4.

Yours,
Linus Walleij

Re: [PATCH v3 6/7] arm64: dts: imx8mp-evk: add board-level mux for CAN2 and MICFIL

[Ahmad Fatoum]

On 3/11/26 8:08 PM, Frank Li wrote:
> The board integrates an on-board mux to route shared signals to either
> CAN2 or PDM (MICFIL). The mux is controlled by a GPIO.
> 
> Add a pinctrl-based multiplexer node to describe this routing and ensure
> proper probe ordering of the dependent devices.
> 
> Previously, MICFIL operation implicitly depended on the default level of
> PCA6416 GPIO3. After adding the pinctrl-multiplexer, make the dependency
> explicit.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

I really like this new abstraction.

> +	can_mux_pinctrl: pinctrl-gpiomux {
> +		compatible = "pinctrl-multiplexer";
> +
> +		can_fun: can-grp {
> +			mux-states = <&can_mux 1>;
> +		};
> +
> +		pdm_fun: pdm-grp {
> +			mux-states = <&can_mux 0>;
> +		};
> +	};

Just to make sure: If both &flexcan2 and &micfil were enabled at the
same time, the existing pinctrl clash detection will also work for this
new pinctrl-multiplexer and point that out, right?

Thanks,
Ahmad

> +
>  	memory@40000000 {
>  		device_type = "memory";
>  		reg = <0x0 0x40000000 0 0xc0000000>,
> @@ -446,7 +465,7 @@ &flexcan1 {
>  
>  &flexcan2 {
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&pinctrl_flexcan2>;
> +	pinctrl-0 = <&pinctrl_flexcan2>, <&can_fun>;
>  	phys = <&flexcan_phy 1>;
>  	status = "disabled";/* can2 pin conflict with pdm */
>  };
> @@ -712,7 +731,7 @@ &lcdif3 {
>  &micfil {
>  	#sound-dai-cells = <0>;
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&pinctrl_pdm>;
> +	pinctrl-0 = <&pinctrl_pdm>, <&pdm_fun>;
>  	assigned-clocks = <&clk IMX8MP_CLK_PDM>;
>  	assigned-clock-parents = <&clk IMX8MP_AUDIO_PLL1_OUT>;
>  	assigned-clock-rates = <196608000>;
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Frank Li]

On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:
> On Wed, Mar 11, 2026 at 8:08 PM Frank Li <Frank.Li@nxp.com> wrote:
>
> > Introduce __pinctrl_generic_pins_function_dt_node_to_map() to allow
> > passing private data and skip_npins to pinmux_generic_add_function().
> >
> > The 'skip_npins' to skip parse pins in dts because on boards MUX control
> > chip switch the whole group together, so needn't handle each pins.
> >
> > Keep pinctrl_generic_pins_function_dt_node_to_map() as a wrapper
> > calling the new helper with a NULL argument to preserve backward
> > compatibility.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> Ad attested by several reviews, the pinctrl subsystem maintainer
> strongly dislikes any use of __double_underscore_function_names().
>
> The reason I dislike it is because it is ambiguous.
>
> For example there are __compiler_intrinsics such as
> __iomem and all the stuff from <linux/compiler_types.h>.
>
> Then there are __non_atomics such as __set_bit().
>
> This means __inner_function() just adds to this mess and creates
> a big confusion for the mind.
>
> That said: in this case you're just adding a parameter, just add
> the parameter and change all of the in-tree users to pass false
> or whatever you need, these is just one (1) in-tree user anyway.

pinctrl_generic_pins_function_dt_node_to_map() directly feed to
.dt_node_to_map() callback, add parameter will impact too much.

If don't like __funciton_name(), can we use
pinctrl_generic_pins_function_dt_node_to_map_ext() or other name

Frank

>
> Yours,
> Linus Walleij

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Linus Walleij]

On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote:
> On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:

> > That said: in this case you're just adding a parameter, just add
> > the parameter and change all of the in-tree users to pass false
> > or whatever you need, these is just one (1) in-tree user anyway.
>
> pinctrl_generic_pins_function_dt_node_to_map() directly feed to
> .dt_node_to_map() callback, add parameter will impact too much.

Why do you say that. It already has many parameters, one more
or less doesn't matter. It's not like this call is performance-critical.
Just change the users.

Yours,
Linus Walleij

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Frank Li]

On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote:
> On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote:
> > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:
>
> > > That said: in this case you're just adding a parameter, just add
> > > the parameter and change all of the in-tree users to pass false
> > > or whatever you need, these is just one (1) in-tree user anyway.
> >
> > pinctrl_generic_pins_function_dt_node_to_map() directly feed to
> > .dt_node_to_map() callback, add parameter will impact too much.
>
> Why do you say that. It already has many parameters, one more
> or less doesn't matter. It's not like this call is performance-critical.
> Just change the users.

In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c,
	.dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map;

pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s
declear.

So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map()
Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in
pinconf.h.

If add two parameter in .dt_node_to_map(), need change all functions, which
.dt_node_to_map = xxx_to_map(). and OF core part.

Frank

>
> Yours,
> Linus Walleij

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Frank Li]

On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote:
> On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote:
> > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote:
> > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:
> >
> > > > That said: in this case you're just adding a parameter, just add
> > > > the parameter and change all of the in-tree users to pass false
> > > > or whatever you need, these is just one (1) in-tree user anyway.
> > >
> > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to
> > > .dt_node_to_map() callback, add parameter will impact too much.
> >
> > Why do you say that. It already has many parameters, one more
> > or less doesn't matter. It's not like this call is performance-critical.
> > Just change the users.
>
> In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c,
> 	.dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map;
>
> pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s
> declear.
>
> So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map()
> Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in
> pinconf.h.
>
> If add two parameter in .dt_node_to_map(), need change all functions, which
> .dt_node_to_map = xxx_to_map(). and OF core part.

Linus Walleij:
	Is my explain clear enough? I am preparing respin it?

	is okay use wrap function
	pinctrl_generic_pins_function_dt_node_to_map_ext()?

Frank
>
> Frank
>
> >
> > Yours,
> > Linus Walleij

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Conor Dooley]

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

On Tue, Mar 24, 2026 at 04:16:10PM -0400, Frank Li wrote:
> On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote:
> > On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote:
> > > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote:
> > > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:
> > >
> > > > > That said: in this case you're just adding a parameter, just add
> > > > > the parameter and change all of the in-tree users to pass false
> > > > > or whatever you need, these is just one (1) in-tree user anyway.
> > > >
> > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to
> > > > .dt_node_to_map() callback, add parameter will impact too much.
> > >
> > > Why do you say that. It already has many parameters, one more
> > > or less doesn't matter. It's not like this call is performance-critical.
> > > Just change the users.
> >
> > In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c,
> > 	.dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map;
> >
> > pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s
> > declear.
> >
> > So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map()
> > Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in
> > pinconf.h.
> >
> > If add two parameter in .dt_node_to_map(), need change all functions, which
> > .dt_node_to_map = xxx_to_map(). and OF core part.
> 
> Linus Walleij:
> 	Is my explain clear enough? I am preparing respin it?
> 
> 	is okay use wrap function
> 	pinctrl_generic_pins_function_dt_node_to_map_ext()?

I don't understand this patch. The function is called
pinctrl_generic_pins_function_dt_node_to_map(). You have no pins.
You're adding a parameter to make a function with *pins* in its name not
use pins. The new function doesn't use pins but has pins in the name.
At the very least function names should not be misleading.

I was going to suggest pulling out the relevant portions and creating
some helpers that could be used by multiple different-but-similar
functions, but I don't actually even think that there's much in common.
Most damningly I think, you don't actually read either the functions or
pins properties at all and neither are permitted by your binding.
So turns out you use neither pins or functions...

You don't actually have any of these properties which runs counter to the
goal of the function, which is parsing. With this in mind, it feels to me
like you're trying way too hard to make use of a generic function when the
right thing to do is probably just have an entirely custom function.
Maybe that's a custom implementation in your driver, or a new function
here, but I think writing that will highlight just how little of the
code would be shared between the existing function and what your
use-case needs: no pin configuration stuff, no reading of the devicetree
other than the node names and no dealing with the label pointing to the
"wrong" place.

I recently bought a spacemit k1 board to go and write a sister function
to pinctrl_generic_pins_function_dt_node_to_map() that deals with pins
and groups (because that's a pretty common pattern).
I would be calling that pinctrl_generic_pinmux_dt_node_to_map(),
because it that's the property it deals with. I have honestly got no
idea what to call one for this situation since you don't have any of the
properties in pinmux-node.yaml. Maybe that's a sign.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Frank Li]

On Wed, Mar 25, 2026 at 10:33:05AM +0000, Conor Dooley wrote:
> On Tue, Mar 24, 2026 at 04:16:10PM -0400, Frank Li wrote:
> > On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote:
> > > On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote:
> > > > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote:
> > > > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:
> > > >
> > > > > > That said: in this case you're just adding a parameter, just add
> > > > > > the parameter and change all of the in-tree users to pass false
> > > > > > or whatever you need, these is just one (1) in-tree user anyway.
> > > > >
> > > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to
> > > > > .dt_node_to_map() callback, add parameter will impact too much.
> > > >
> > > > Why do you say that. It already has many parameters, one more
> > > > or less doesn't matter. It's not like this call is performance-critical.
> > > > Just change the users.
> > >
> > > In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c,
> > > 	.dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map;
> > >
> > > pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s
> > > declear.
> > >
> > > So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map()
> > > Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in
> > > pinconf.h.
> > >
> > > If add two parameter in .dt_node_to_map(), need change all functions, which
> > > .dt_node_to_map = xxx_to_map(). and OF core part.
> >
> > Linus Walleij:
> > 	Is my explain clear enough? I am preparing respin it?
> >
> > 	is okay use wrap function
> > 	pinctrl_generic_pins_function_dt_node_to_map_ext()?
>
> I don't understand this patch. The function is called
> pinctrl_generic_pins_function_dt_node_to_map(). You have no pins.
> You're adding a parameter to make a function with *pins* in its name not
> use pins. The new function doesn't use pins but has pins in the name.
> At the very least function names should not be misleading.
>
> I was going to suggest pulling out the relevant portions and creating
> some helpers that could be used by multiple different-but-similar
> functions, but I don't actually even think that there's much in common.
> Most damningly I think, you don't actually read either the functions or
> pins properties at all and neither are permitted by your binding.
> So turns out you use neither pins or functions...
>
> You don't actually have any of these properties which runs counter to the
> goal of the function, which is parsing. With this in mind, it feels to me
> like you're trying way too hard to make use of a generic function when the
> right thing to do is probably just have an entirely custom function.
> Maybe that's a custom implementation in your driver, or a new function
> here, but I think writing that will highlight just how little of the
> code would be shared between the existing function and what your
> use-case needs: no pin configuration stuff, no reading of the devicetree
> other than the node names and no dealing with the label pointing to the
> "wrong" place.
>
> I recently bought a spacemit k1 board to go and write a sister function
> to pinctrl_generic_pins_function_dt_node_to_map() that deals with pins
> and groups (because that's a pretty common pattern).
> I would be calling that pinctrl_generic_pinmux_dt_node_to_map(),
> because it that's the property it deals with. I have honestly got no
> idea what to call one for this situation since you don't have any of the
> properties in pinmux-node.yaml. Maybe that's a sign.

At v2, I implemented customize dt_node_to_map(), Linus Walleij think it is
too similar with pinctrl_generic_pins_function_dt_node_to_map(), so ask me
to enhanance and reuse pinctrl_generic_pins_function_dt_node_to_map().

Frank
>
> Cheers,
> Conor.

Re: [PATCH v3 3/7] pinctrl: pinctrl-generic: add __pinctrl_generic_pins_function_dt_node_to_map()

[Conor Dooley]

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

On Wed, Mar 25, 2026 at 11:09:20AM -0400, Frank Li wrote:
> On Wed, Mar 25, 2026 at 10:33:05AM +0000, Conor Dooley wrote:
> > On Tue, Mar 24, 2026 at 04:16:10PM -0400, Frank Li wrote:
> > > On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote:
> > > > On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote:
> > > > > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote:
> > > > > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:
> > > > >
> > > > > > > That said: in this case you're just adding a parameter, just add
> > > > > > > the parameter and change all of the in-tree users to pass false
> > > > > > > or whatever you need, these is just one (1) in-tree user anyway.
> > > > > >
> > > > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to
> > > > > > .dt_node_to_map() callback, add parameter will impact too much.
> > > > >
> > > > > Why do you say that. It already has many parameters, one more
> > > > > or less doesn't matter. It's not like this call is performance-critical.
> > > > > Just change the users.
> > > >
> > > > In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c,
> > > > 	.dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map;
> > > >
> > > > pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s
> > > > declear.
> > > >
> > > > So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map()
> > > > Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in
> > > > pinconf.h.
> > > >
> > > > If add two parameter in .dt_node_to_map(), need change all functions, which
> > > > .dt_node_to_map = xxx_to_map(). and OF core part.
> > >
> > > Linus Walleij:
> > > 	Is my explain clear enough? I am preparing respin it?
> > >
> > > 	is okay use wrap function
> > > 	pinctrl_generic_pins_function_dt_node_to_map_ext()?
> >
> > I don't understand this patch. The function is called
> > pinctrl_generic_pins_function_dt_node_to_map(). You have no pins.
> > You're adding a parameter to make a function with *pins* in its name not
> > use pins. The new function doesn't use pins but has pins in the name.
> > At the very least function names should not be misleading.
> >
> > I was going to suggest pulling out the relevant portions and creating
> > some helpers that could be used by multiple different-but-similar
> > functions, but I don't actually even think that there's much in common.
> > Most damningly I think, you don't actually read either the functions or
> > pins properties at all and neither are permitted by your binding.
> > So turns out you use neither pins or functions...
> >
> > You don't actually have any of these properties which runs counter to the
> > goal of the function, which is parsing. With this in mind, it feels to me
> > like you're trying way too hard to make use of a generic function when the
> > right thing to do is probably just have an entirely custom function.
> > Maybe that's a custom implementation in your driver, or a new function
> > here, but I think writing that will highlight just how little of the
> > code would be shared between the existing function and what your
> > use-case needs: no pin configuration stuff, no reading of the devicetree
> > other than the node names and no dealing with the label pointing to the
> > "wrong" place.
> >
> > I recently bought a spacemit k1 board to go and write a sister function
> > to pinctrl_generic_pins_function_dt_node_to_map() that deals with pins
> > and groups (because that's a pretty common pattern).
> > I would be calling that pinctrl_generic_pinmux_dt_node_to_map(),
> > because it that's the property it deals with. I have honestly got no
> > idea what to call one for this situation since you don't have any of the
> > properties in pinmux-node.yaml. Maybe that's a sign.
> 
> At v2, I implemented customize dt_node_to_map(), Linus Walleij think it is
> too similar with pinctrl_generic_pins_function_dt_node_to_map(), so ask me
> to enhanance and reuse pinctrl_generic_pins_function_dt_node_to_map().

Sure, and he's right that there's a lot similar. Everything you want to
do, other than looking at the mux state, is something that
pinctrl_generic_pins_function_dt_node_to_map() does. But bastardising
a function that's explicitly about reading pins and functions properties
to do things that have _neither_ is not a good implementation of that
review feedback.

If you're going to make something generic, the right level to hook in
IMO is at the pinctrl_generic_pins_function_dt_subnode_to_map() level,
I already know that 95% of the code in that function is identical to
what will be used for the spacemit k1 that uses the pinmux property
and changing its API is a lot easier than changing the API of something
that is written to match the dt_node_to_map callback.
You don't even benefit from the extra functionality that
pinctrl_generic_pins_function_dt_node_to_map() provides, because you
don't have ambiguity about where the phandle you're parsing from points.
In your case, it has to be the group node.

pinctrl_generic_pins_function_dt_subnode_to_map() could probably be
split into two parts, one that does the dt parsing portion and a second
portion that does the mapping to kernel data structures. The first
portion of that is the for loop. The second portion is everything after
the for loop and the bit that names the groups. IOW, do something like
what I am pasting here, and you create your own function that wraps
pinctrl_generic_to_map() in the way you want. I personally think this is
more tasteful than what you've done, and more importantly I am pretty
sure that this is what's needed to be able to maximise code reuse for
the devices that use pinmux. I didn't compile this or anything, it's
just speculative.

diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index efb39c6a67033..7f02af6d9f3e4 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -17,56 +17,30 @@
 #include "pinctrl-utils.h"
 #include "pinmux.h"
 
-static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *pctldev,
-							   struct device_node *parent,
-							   struct device_node *np,
-							   struct pinctrl_map **maps,
-							   unsigned int *num_maps,
-							   unsigned int *num_reserved_maps,
-							   const char **group_names,
-							   unsigned int ngroups)
+static int pinctrl_generic_to_map(struct pinctrl_dev *pctldev,
+				  struct device_node *parent,
+				  struct device_node *np,
+				  struct pinctrl_map **maps,
+				  unsigned int *num_maps,
+				  unsigned int *num_reserved_maps,
+				  const char **group_names,
+				  unsigned int ngroups,
+				  const char **functions,
+				  unsigned int *pins,
+				  void *function_data)
 {
 	struct device *dev = pctldev->dev;
-	const char **functions;
 	const char *group_name;
 	unsigned long *configs;
-	unsigned int num_configs, pin, *pins;
+	unsigned int num_configs;
 	int npins, ret, reserve = 1;
 
-	npins = of_property_count_u32_elems(np, "pins");
-
-	if (npins < 1) {
-		dev_err(dev, "invalid pinctrl group %pOFn.%pOFn %d\n",
-			parent, np, npins);
-		return npins;
-	}
-
 	group_name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn", parent, np);
 	if (!group_name)
 		return -ENOMEM;
 
 	group_names[ngroups] = group_name;
 
-	pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
-	if (!pins)
-		return -ENOMEM;
-
-	functions = devm_kcalloc(dev, npins, sizeof(*functions), GFP_KERNEL);
-	if (!functions)
-		return -ENOMEM;
-
-	for (int i = 0; i < npins; i++) {
-		ret = of_property_read_u32_index(np, "pins", i, &pin);
-		if (ret)
-			return ret;
-
-		pins[i] = pin;
-
-		ret = of_property_read_string(np, "function", &functions[i]);
-		if (ret)
-			return ret;
-	}
-
 	ret = pinctrl_utils_reserve_map(pctldev, maps, num_reserved_maps, num_maps, reserve);
 	if (ret)
 		return ret;
@@ -101,6 +75,52 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
 		return ret;
 
 	return 0;
+}
+
+static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *pctldev,
+							   struct device_node *parent,
+							   struct device_node *np,
+							   struct pinctrl_map **maps,
+							   unsigned int *num_maps,
+							   unsigned int *num_reserved_maps,
+							   const char **group_names,
+							   unsigned int ngroups)
+{
+	struct device *dev = pctldev->dev;
+	const char **functions;
+	unsigned int pin, *pins;
+	int npins, ret;
+
+	npins = of_property_count_u32_elems(np, "pins");
+
+	if (npins < 1) {
+		dev_err(dev, "invalid pinctrl group %pOFn.%pOFn %d\n",
+			parent, np, npins);
+		return npins;
+	}
+
+	pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
+	if (!pins)
+		return -ENOMEM;
+
+	functions = devm_kcalloc(dev, npins, sizeof(*functions), GFP_KERNEL);
+	if (!functions)
+		return -ENOMEM;
+
+	for (int i = 0; i < npins; i++) {
+		ret = of_property_read_u32_index(np, "pins", i, &pin);
+		if (ret)
+			return ret;
+
+		pins[i] = pin;
+
+		ret = of_property_read_string(np, "function", &functions[i]);
+		if (ret)
+			return ret;
+	}
+	return pinctrl_generic_to_map(pctldev, parent, np, maps, num_maps,
+				  num_reserved_maps, group_names, ngroups,
+				  functions, pins, NULL);
 };
 
 /*



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]