[PATCH 0/3] pinctrl: single: bit-per-mux DT flexibility, probe robustness, and consistent pinconf offsets

[PATCH 0/3] pinctrl: single: bit-per-mux DT flexibility, probe robustness, and consistent pinconf offsets

[Billy Tsai]

This series updates pinctrl-single to behave more predictably on
bit-per-mux platforms by making its DT interface more flexible, its probe
path more tolerant of pre-reserved resources, and its pin configuration
register addressing consistent with pinmux.
It extends the driver to accept a per-pin <pin_index func_sel> style
description for bit-per-mux users while keeping the existing
pinctrl-single,bits binding as the preferred input when available. It also
relaxes probe failure when the I/O memory region cannot be reserved
exclusively, allowing initialization to proceed with a warning on systems
where that region is already reserved. Finally, it aligns pinconf register
offset computation with the pinmux logic so that both muxing and pin
configuration access the same register offsets, avoiding incorrect pinconf
operations on bit-per-mux configurations.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
Billy Tsai (3):
      pinctrl: single: add per-pin binding support for bit-per-mux
      pinctrl: single: Allow probe to continue if mem region busy
      pinctrl: single: unify pinconf offset mapping with pinmux

 drivers/pinctrl/pinctrl-single.c | 150 ++++++++++++++++++++++++++++-----------
 1 file changed, 110 insertions(+), 40 deletions(-)
---
base-commit: dd9b004b7ff3289fb7bae35130c0a5c0537266af
change-id: 20251222-upstream_pinctrl_single-99e8df1fe2b9

Best regards,
-- 
Billy Tsai <billy_tsai@aspeedtech.com>

[PATCH 1/3] pinctrl: single: add per-pin binding support for bit-per-mux

[Billy Tsai]

Add support for binding where bit-per-mux users specify pins as
<pin_index func_sel> pairs. Prefer explicit bits binding when present,
but fall back to the new per-pin binding for improved flexibility.
This approach is intended to adapt to hardware with a regular register
layout, where pin functions are arranged with a fixed stride. For example,
the function of pin 0 is controlled by bits [3:0] at offset 0, the
function of pin 1 by bits [7:4] at the same offset, and so on.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/pinctrl/pinctrl-single.c | 132 +++++++++++++++++++++++++++++----------
 1 file changed, 100 insertions(+), 32 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 998f23d6c317..757c22cc09f3 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1041,29 +1041,81 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 			break;
 		}
 
-		offset = pinctrl_spec.args[0];
-		vals[found].reg = pcs->base + offset;
+		/*
+		 * For legacy (non bit-per-mux) users the first cell is the
+		 * register offset and the second (and optional third) cell is
+		 * the value to be written.
+		 *
+		 * For bit-per-mux users we want a simpler binding where the
+		 * first cell is the pin index and the second cell is the
+		 * function selector. Translate that into register offset,
+		 * value and mask here so the rest of the driver can stay
+		 * Register based.
+		 */
+		if (!pcs->bits_per_mux) {
+			offset = pinctrl_spec.args[0];
+			vals[found].reg = pcs->base + offset;
 
-		switch (pinctrl_spec.args_count) {
-		case 2:
-			vals[found].val = pinctrl_spec.args[1];
-			break;
-		case 3:
-			vals[found].val = (pinctrl_spec.args[1] | pinctrl_spec.args[2]);
-			break;
-		}
+			switch (pinctrl_spec.args_count) {
+			case 2:
+				vals[found].val = pinctrl_spec.args[1];
+				break;
+			case 3:
+				vals[found].val = (pinctrl_spec.args[1] |
+						   pinctrl_spec.args[2]);
+				break;
+			}
 
-		dev_dbg(pcs->dev, "%pOFn index: 0x%x value: 0x%x\n",
-			pinctrl_spec.np, offset, vals[found].val);
+			dev_dbg(pcs->dev, "%pOFn offset: 0x%x value: 0x%x\n",
+				 pinctrl_spec.np, offset, vals[found].val);
 
-		pin = pcs_get_pin_by_offset(pcs, offset);
-		if (pin < 0) {
-			dev_err(pcs->dev,
-				"could not add functions for %pOFn %ux\n",
-				np, offset);
-			break;
+			pin = pcs_get_pin_by_offset(pcs, offset);
+			if (pin < 0) {
+				dev_err(pcs->dev,
+					"could not add functions for %pOFn %ux\n",
+					np, offset);
+				break;
+			}
+			pins[found++] = pin;
+		} else {
+			unsigned int pin_index, func_sel;
+			unsigned int shift, mask, val;
+
+			/* Expect <pin_index func_sel> for bit-per-mux users. */
+			if (pinctrl_spec.args_count < 2) {
+				dev_err(pcs->dev,
+					"invalid args_count for bit-per-mux spec: %i\n",
+					pinctrl_spec.args_count);
+				break;
+			}
+
+			pin_index = pinctrl_spec.args[0];
+			func_sel = pinctrl_spec.args[1];
+
+			if (pin_index >= pcs->desc.npins) {
+				dev_err(pcs->dev,
+					"pin index out of range for %pOFn: %u (npins %u)\n",
+					np, pin_index, pcs->desc.npins);
+				break;
+			}
+
+			offset = pcs_pin_reg_offset_get(pcs, pin_index);
+			shift = pcs_pin_shift_reg_get(pcs, pin_index);
+
+			mask = pcs->fmask << shift;
+			val = (func_sel << shift) & mask;
+
+			vals[found].reg = pcs->base + offset;
+			vals[found].val = val;
+			vals[found].mask = mask;
+
+			dev_dbg(pcs->dev,
+				 "%pOFn pin: %u offset: 0x%x func: 0x%x val: 0x%x mask: 0x%x\n",
+				 pinctrl_spec.np, pin_index, offset,
+				 func_sel, val, mask);
+
+			pins[found++] = pin_index;
 		}
-		pins[found++] = pin;
 	}
 
 	pgnames[0] = np->name;
@@ -1280,21 +1332,37 @@ static int pcs_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 
 	if (pcs->bits_per_mux) {
-		ret = pcs_parse_bits_in_pinctrl_entry(pcs, np_config, map,
-				num_maps, pgnames);
-		if (ret < 0) {
-			dev_err(pcs->dev, "no pins entries for %pOFn\n",
-				np_config);
-			goto free_pgnames;
+		/*
+		 * For bit-per-mux users there are two possible bindings:
+		 * - pinctrl-single,bits: offset/value/mask triples
+		 * - pinctrl-single,pins: <pin_index func_sel> pairs
+		 *
+		 * Prefer the explicit bits binding when present so existing
+		 * users keep their current behaviour, otherwise fall back
+		 * to the per-pin binding.
+		 */
+		if (of_find_property(np_config, "pinctrl-single,bits", NULL)) {
+			ret = pcs_parse_bits_in_pinctrl_entry(pcs, np_config,
+							      map, num_maps,
+							      pgnames);
+		} else if (of_find_property(np_config,
+					    "pinctrl-single,pins", NULL)) {
+			ret = pcs_parse_one_pinctrl_entry(pcs, np_config, map,
+							  num_maps, pgnames);
+		} else {
+			ret = -EINVAL;
 		}
 	} else {
-		ret = pcs_parse_one_pinctrl_entry(pcs, np_config, map,
-				num_maps, pgnames);
-		if (ret < 0) {
-			dev_err(pcs->dev, "no pins entries for %pOFn\n",
-				np_config);
-			goto free_pgnames;
-		}
+		if (of_find_property(np_config, "pinctrl-single,pins", NULL))
+			ret = pcs_parse_one_pinctrl_entry(pcs, np_config, map,
+							  num_maps, pgnames);
+		else
+			ret = -EINVAL;
+	}
+
+	if (ret < 0) {
+		dev_err(pcs->dev, "no pins entries for %pOFn\n", np_config);
+		goto free_pgnames;
 	}
 
 	return 0;

-- 
2.34.1

[PATCH 2/3] pinctrl: single: Allow probe to continue if mem region busy

[Billy Tsai]

Skip exclusive memory region reservation failure during probe and
continue initialization with a warning. This enables support for
systems where the memory region may already be reserved, improving
probe robustness.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/pinctrl/pinctrl-single.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 757c22cc09f3..e65ae737b4c5 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1910,13 +1910,13 @@ static int pcs_probe(struct platform_device *pdev)
 
 	pcs->res = devm_request_mem_region(pcs->dev, res->start,
 			resource_size(res), DRIVER_NAME);
-	if (!pcs->res) {
-		dev_err(pcs->dev, "could not get mem_region\n");
-		return -EBUSY;
-	}
+	if (!pcs->res)
+		dev_warn(pcs->dev, "mem_region busy, continuing without reservation\n");
+	else
+		res = pcs->res;
 
-	pcs->size = resource_size(pcs->res);
-	pcs->base = devm_ioremap(pcs->dev, pcs->res->start, pcs->size);
+	pcs->size = resource_size(res);
+	pcs->base = devm_ioremap(pcs->dev, res->start, pcs->size);
 	if (!pcs->base) {
 		dev_err(pcs->dev, "could not ioremap\n");
 		return -ENODEV;

-- 
2.34.1

[PATCH 3/3] pinctrl: single: unify pinconf offset mapping with pinmux

[Billy Tsai]

Use the same register offset calculation for pinconf as pinmux to
properly handle bit-per-mux configurations. Ensures consistent and
correct offset mapping for pin configuration operations.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/pinctrl/pinctrl-single.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index e65ae737b4c5..aaf830315c5d 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -505,7 +505,8 @@ static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
 			continue;
 		}
 
-		offset = pin * (pcs->width / BITS_PER_BYTE);
+		/* Use the same offset mapping as pinmux (handles bit-per-mux) */
+		offset = pcs_pin_reg_offset_get(pcs, pin);
 		data = pcs->read(pcs->base + offset) & func->conf[i].mask;
 		switch (func->conf[i].param) {
 		/* 4 parameters */
@@ -573,7 +574,8 @@ static int pcs_pinconf_set(struct pinctrl_dev *pctldev,
 			if (param != func->conf[i].param)
 				continue;
 
-			offset = pin * (pcs->width / BITS_PER_BYTE);
+			/* Use the same offset mapping as pinmux (handles bit-per-mux) */
+			offset = pcs_pin_reg_offset_get(pcs, pin);
 			data = pcs->read(pcs->base + offset);
 			arg = pinconf_to_config_argument(configs[j]);
 			switch (param) {

-- 
2.34.1

Re: [PATCH 0/3] pinctrl: single: bit-per-mux DT flexibility, probe robustness, and consistent pinconf offsets

[Andrew Jeffery]

Hi Billy,

On Mon, 2025-12-22 at 20:04 +0800, Billy Tsai wrote:
> This series updates pinctrl-single to behave more predictably on
> bit-per-mux platforms by making its DT interface more flexible, its probe
> path more tolerant of pre-reserved resources, and its pin configuration
> register addressing consistent with pinmux.

Can you provide some more context here? For instance, this is motivated
by the AST2700 - can you talk a bit more about why its design needs
these changes?

> It extends the driver to accept a per-pin <pin_index func_sel> style
> description for bit-per-mux users while keeping the existing
> pinctrl-single,bits binding as the preferred input when available. It also
> relaxes probe failure when the I/O memory region cannot be reserved
> exclusively, allowing initialization to proceed with a warning on systems
> where that region is already reserved.
> 

Can you unpack what's going on here in the context of the target soc?

Andrew

>  Finally, it aligns pinconf register
> offset computation with the pinmux logic so that both muxing and pin
> configuration access the same register offsets, avoiding incorrect pinconf
> operations on bit-per-mux configurations.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
> Billy Tsai (3):
>       pinctrl: single: add per-pin binding support for bit-per-mux
>       pinctrl: single: Allow probe to continue if mem region busy
>       pinctrl: single: unify pinconf offset mapping with pinmux
> 
>  drivers/pinctrl/pinctrl-single.c | 150 ++++++++++++++++++++++++++++-----------
>  1 file changed, 110 insertions(+), 40 deletions(-)
> ---
> base-commit: dd9b004b7ff3289fb7bae35130c0a5c0537266af
> change-id: 20251222-upstream_pinctrl_single-99e8df1fe2b9
> 
> Best regards,

Re: [PATCH 0/3] pinctrl: single: bit-per-mux DT flexibility, probe robustness, and consistent pinconf offsets

[Billy Tsai]

> > This series updates pinctrl-single to behave more predictably on
> > bit-per-mux platforms by making its DT interface more flexible, its probe
> > path more tolerant of pre-reserved resources, and its pin configuration
> > register addressing consistent with pinmux.
 
> Can you provide some more context here? For instance, this is motivated
> by the AST2700 - can you talk a bit more about why its design needs
> these changes?
 
> > It extends the driver to accept a per-pin <pin_index func_sel> style
> > description for bit-per-mux users while keeping the existing
> > pinctrl-single,bits binding as the preferred input when available. It also
> > relaxes probe failure when the I/O memory region cannot be reserved
> > exclusively, allowing initialization to proceed with a warning on systems
> > where that region is already reserved.
> >
 
> Can you unpack what's going on here in the context of the target soc?
 
Hi Andrew,
 
This series is mainly extending pinctrl-single to match how AST2700 uses the
pinmux and pinconf registers. There are three parts:
 
1. Per-pin DT binding for bit-per-mux
In the current driver, bit-per-mux users can only describe pinmux via
pinctrl-single,bits (<offset, value, mask>). On AST2700 the pinmux register
layout is contiguous per pin:
pin 0 maps to 0x400[3:0], pin 1 maps to 0x400[7:4], pin 2 maps to 0x400[11:8], and so on.
For that layout, describing each pin by <pin_index func_sel> is more direct and readable
than crafting offset/mask/value tuples.
Example:
 
pinctrl-single,pins = <
    0 2   // pin 0 to function 2
    1 3   // pin 1 to function 3
    2 0   // pin 2 to function 0
>;
This change lets the driver accept that per-pin encoding while still
preferring the existing pinctrl-single,bits binding for compatibility.
 
2. Unify pinconf offset mapping with pinmux
Today pinconf offset calculation assumes a linear per-register width even if
bit-per-mux/function-mask is in use. On AST2700, pinconf follows the same
bit-per-pin layout as pinmux. For example,
drive strength: pin 0 at 0x4c0[1:0], pin 1 at 0x4c0[3:2], etc.
Pulldown: pin 0 at 0x480[0], pin 1 at 0x480[1], etc.
Unifying pinconf offset mapping with the pinmux logic makes the behavior correct
and easier to understand.
 
3. Probe with busy memory region
On AST2700 boards the SCU/pinctrl registers are often reserved in the DT by
a top-level “syscon” node or by firmware. devm_request_mem_region() returns
-EBUSY even though the region is valid and should be shared. Without this
change, pinctrl-single fails to probe and all pinmux setup is lost. The patch
just downgrades that case to a warning and continues.
 
Thanks
Best regards,
Billy Tsai