[PATCH 0/2] pinctrl: qcom: Allow eGPIO ownership independent of the mux function

[PATCH 0/2] pinctrl: qcom: Allow eGPIO ownership independent of the mux function

[Pankaj Patil]

eGPIO capable TLMM pins can be handed over to a remote subsystem,
which then drives the pin instead of the application processor. The
eGPIO present bit describes whether a pin is capable of being owned
remotely and the enable bit selects the owner; neither is tied to the
pin's mux function.

The only way to express this today is to select the virtual "egpio"
function which transfers the ownership to remote io whereas the 
single-valued "function" property e.g - gpio, qup0_se0 claim the
ownership for APPS. There is no way to describe both the pin's normal mux
function and enable remote ownership where as the hardware programming
guide(HPG) mentions no such exclusion. Boards that need a pin muxed to
it's real function while the remote subsystem drives it is permitted
behavior according to the HPG.

This series adds a "qcom,remote-owned" boolean flag to the TLMM pin
configuration, describing ownership on its own. Its presence
hands the listed pins to their remote owner, and its absence leaves the
ownership to APPS.

cam_sensor_suspend_rst4: cam-sensor-suspend-rst4 {
		/* RESET REAR */
		config {
			pins = "gpio239";
			bias-pull-down; /* PULL DOWN */
			drive-strength = <12>; /* 12 MA */
			output-low;
			qcom,remote-owned;
		};

		mux {
			pins = "gpio239";
			function = "gpio";
		};
	};

Pinconf-generic emits the mux mapping ahead of the config
mapping, the flag is applied after msm_pinmux_set_mux() has
claimed the pin, so the combination above ends up muxed to gpio and owned
remotely.

Signed-off-by: Pankaj Patil <pankaj.patil@oss.qualcomm.com>
---
Pankaj Patil (2):
      dt-bindings: pinctrl: qcom: Add qcom,remote-owned for ownership update
      pinctrl: qcom: Add qcom,remote-owned pin configuration support

 .../bindings/pinctrl/qcom,tlmm-common.yaml         |  7 ++
 drivers/pinctrl/qcom/pinctrl-msm.c                 | 81 +++++++++++++++++++---
 2 files changed, 80 insertions(+), 8 deletions(-)
---
base-commit: d6bcfc94eb4da6e089c5473970cd96727292fd97
change-id: 20261102-egpio_fix-ddf4c2a4f24e

Best regards,
-- 
Pankaj Patil <pankaj.patil@oss.qualcomm.com>

[PATCH 1/2] dt-bindings: pinctrl: qcom: Add qcom,remote-owned for ownership update

[Pankaj Patil]

Add "qcom,remote-owned" boolean flag for egpio capable pins since they can
be owned by remote island io independently of the selected mux function
even if function is non-egpio.

Presently egpio is treated as a mux function level decision where as
hardware programming guide(hpg) specifies it as pin conf level decision
i.e setting egpio bit is independent of selected mux function, the
driver/apps claims the ownership of the pin if it's egpio capable in
case of mux function being specified as anything other than egpio.

Signed-off-by: Pankaj Patil <pankaj.patil@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml
index aec72e8c0621..d7ea3eb4e124 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml
@@ -87,6 +87,13 @@ $defs:
       output-high: true
       output-low: true
 
+      qcom,remote-owned:
+        type: boolean
+        description:
+          Hand the specified pins over to their remote eGPIO owner. Only
+          valid for eGPIO capable pins, and leaves the mux function
+          untouched.
+
     allOf:
       - $ref: pincfg-node.yaml#
       - $ref: pinmux-node.yaml#

-- 
2.34.1

[PATCH 2/2] pinctrl: qcom: Add qcom,remote-owned pin configuration support

[Pankaj Patil]

The eGPIO support added was incomplete since pin ownership is independent
of the selected mux function: the eGPIO present bit only describes
whether a pin is capable of being owned remotely, and the enable bit
selects the owner - 1 sets the owner as APPS and 0 sets the owner as
remote io. At present it's a mux level decision by setting the function
as "egpio", APPS releases the ownership to remote io and in process
cannot use it for any other function.

Add a driver specific pinconf parameter enabling the "qcom,remote-owned"
dt property support. pinconf-generic falls back to the default value of
1 whenever the property carries no value, and an absent property is
skipped entirely. Since pinconf-generic emits the mux mapping ahead of
the config mapping, this is applied after msm_pinmux_set_mux() has
set the pin ownership, giving the intended result for a state that
specifies both a function and remote-owned.

The hardware programming guide(hpg) explicitly mentions that the egpio
enable bit can be set independently of whatever the function select bits
are set to on targets which require shared ownership while the APPS
continues to use it as normal mux function.

msm_gpio_dbg_show_one() printed "egpio" and returned early whenever the
enable bit was clear, which was sufficient while that state could only be
reached by muxing to the egpio function. Ownership is no longer tied to
the mux, so report it as a suffix on the regular line and keep the
direction, value, mux, drive strength and bias visible. Pins muxed to the
egpio function reach the same register state and so print the full line
too; their mux field reflects the previously selected function.

Fixes: bebc49c1e5f6 ("pinctrl: qcom: Add egpio feature support")
Signed-off-by: Pankaj Patil <pankaj.patil@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 81 ++++++++++++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 32aa7b919e24..4d96260f9909 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -48,6 +48,31 @@ struct msm_pin_state {
 	bool saved;
 };
 
+/*
+ * Driver specific pinconf parameter to hand a pin over to its eGPIO owner
+ * without muxing the TLMM away to the virtual eGPIO function. The eGPIO
+ * present bit only describes whether a pin is capable of being owned
+ * remotely; ownership itself is independent of the selected mux function,
+ * so a pin can keep its normal function selected while being owned by the
+ * remote subsystem.
+ *
+ * The "remote-owned" property is a boolean flag: its mere presence releases
+ * the pin to the remote owner. The default value below is what makes a
+ * valueless property come out as "release", as pinconf-generic falls back to
+ * it whenever the property carries no value.
+ */
+#define PIN_CONFIG_REMOTE_OWNED	(PIN_CONFIG_END + 1)
+
+static const struct pinconf_generic_params msm_gpio_bindings[] = {
+	{ "qcom,remote-owned", PIN_CONFIG_REMOTE_OWNED, 1 },
+};
+
+#ifdef CONFIG_DEBUG_FS
+static const struct pin_config_item msm_conf_items[] = {
+	PCONFDUMP(PIN_CONFIG_REMOTE_OWNED, "qcom,remote-owned", NULL, false),
+};
+#endif
+
 /**
  * struct msm_pinctrl - state for a pinctrl-msm device
  * @dev:            device handle.
@@ -328,6 +353,10 @@ static int msm_config_reg(struct msm_pinctrl *pctrl,
 		*bit = g->oe_bit;
 		*mask = 1;
 		break;
+	case PIN_CONFIG_REMOTE_OWNED:
+		*bit = g->egpio_enable;
+		*mask = 1;
+		break;
 	default:
 		return -ENOTSUPP;
 	}
@@ -424,6 +453,17 @@ static int msm_config_group_get(struct pinctrl_dev *pctldev,
 		if (!arg)
 			return -EINVAL;
 		break;
+	case PIN_CONFIG_REMOTE_OWNED:
+		/* Pin is not eGPIO capable */
+		if (!pctrl->soc->egpio_func || !(val & BIT(g->egpio_present)))
+			return -ENOTSUPP;
+
+		/* The enable bit is cleared when the remote owns the pin */
+		if (arg)
+			return -EINVAL;
+
+		arg = 1;
+		break;
 	default:
 		return -ENOTSUPP;
 	}
@@ -536,6 +576,29 @@ static int msm_config_group_set(struct pinctrl_dev *pctldev,
 		case PIN_CONFIG_OUTPUT_ENABLE:
 			arg = !!arg;
 			break;
+		case PIN_CONFIG_REMOTE_OWNED:
+			if (!pctrl->soc->egpio_func)
+				return -ENOTSUPP;
+
+			/*
+			 * Ownership may only be handed over on eGPIO capable
+			 * pins and is released by clearing the enable bit, so
+			 * this is applied here rather than through the generic
+			 * read-modify-write below.
+			 */
+			raw_spin_lock_irqsave(&pctrl->lock, flags);
+			val = msm_readl_ctl(pctrl, g);
+			if (!(val & BIT(g->egpio_present))) {
+				raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+				dev_err(pctrl->dev, "%s is not eGPIO capable\n",
+					g->grp.name);
+				return -ENOTSUPP;
+			}
+
+			val &= ~BIT(g->egpio_enable);
+			msm_writel_ctl(val, pctrl, g);
+			raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+			continue;
 		default:
 			dev_err(pctrl->dev, "Unsupported config parameter: %x\n",
 				param);
@@ -676,7 +739,7 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
 	int drive;
 	int pull;
 	int val;
-	int egpio_enable;
+	bool remote_owned;
 	u32 ctl_reg, io_reg;
 
 	static const char * const pulls_keeper[] = {
@@ -703,20 +766,15 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
 	func = (ctl_reg >> g->mux_bit) & 7;
 	drive = (ctl_reg >> g->drv_bit) & 7;
 	pull = (ctl_reg >> g->pull_bit) & 3;
-	egpio_enable = 0;
+	remote_owned = false;
 	if (pctrl->soc->egpio_func && ctl_reg & BIT(g->egpio_present))
-		egpio_enable = !(ctl_reg & BIT(g->egpio_enable));
+		remote_owned = !(ctl_reg & BIT(g->egpio_enable));
 
 	if (is_out)
 		val = !!(io_reg & BIT(g->out_bit));
 	else
 		val = !!(io_reg & BIT(g->in_bit));
 
-	if (egpio_enable) {
-		seq_printf(s, " %-8s: egpio\n", g->grp.name);
-		return;
-	}
-
 	seq_printf(s, " %-8s: %-3s", g->grp.name, is_out ? "out" : "in");
 	seq_printf(s, " %-4s func%d", str_high_low(val), func);
 	seq_printf(s, " %dmA", msm_regval_to_drive(drive));
@@ -724,6 +782,8 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
 		seq_printf(s, " %s", pulls_no_keeper[pull]);
 	else
 		seq_printf(s, " %s", pulls_keeper[pull]);
+	if (remote_owned)
+		seq_puts(s, " remote-owned");
 	seq_puts(s, "\n");
 }
 
@@ -1778,6 +1838,11 @@ int msm_pinctrl_probe(struct platform_device *pdev,
 	pctrl->desc.name = dev_name(&pdev->dev);
 	pctrl->desc.pins = pctrl->soc->pins;
 	pctrl->desc.npins = pctrl->soc->npins;
+	pctrl->desc.num_custom_params = ARRAY_SIZE(msm_gpio_bindings);
+	pctrl->desc.custom_params = msm_gpio_bindings;
+#ifdef CONFIG_DEBUG_FS
+	pctrl->desc.custom_conf_items = msm_conf_items;
+#endif
 
 	ret = devm_pinctrl_register_and_init(&pdev->dev, &pctrl->desc,
 					     pctrl, &pctrl->pctrl);

-- 
2.34.1

Re: [PATCH 2/2] pinctrl: qcom: Add qcom,remote-owned pin configuration support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] pinctrl: qcom: incorrect pull-up bias reporting in
  msm_config_group_get() [1]
--

commit d1d6e2282f19bdb42f5fdbf3729ed8109d4061fb
Author: Pankaj Patil <pankaj.patil@oss.qualcomm.com>

pinctrl: qcom: Add qcom,remote-owned pin configuration support

This commit adds support for the "qcom,remote-owned" pinconf property to
hand pin ownership over to a remote subsystem without changing the
selected mux function, and updates debugfs output to report remote
ownership alongside the regular pin state.

> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 32aa7b919e24a..4d96260f99090 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c

[ ... ]

> @@ -424,6 +453,17 @@ static int msm_config_group_get(struct pinctrl_dev *pctldev,
>  		if (!arg)
>  			return -EINVAL;
>  		break;
> +	case PIN_CONFIG_REMOTE_OWNED:
> +		/* Pin is not eGPIO capable */
> +		if (!pctrl->soc->egpio_func || !(val & BIT(g->egpio_present)))
> +			return -ENOTSUPP;
> +
> +		/* The enable bit is cleared when the remote owns the pin */
> +		if (arg)
> +			return -EINVAL;
> +
> +		arg = 1;
> +		break;
>  	default:
>  		return -ENOTSUPP;
>  	}

[Severity: Medium]
This problem wasn't introduced by this patch, but in msm_config_group_get(),
does the PIN_CONFIG_BIAS_PULL_UP case [1] need to check that
g->i2c_pull_bit is non-zero before testing BIT(g->i2c_pull_bit), just as
msm_config_reg() and msm_config_group_set() do?

drivers/pinctrl/qcom/pinctrl-msm.c:msm_config_group_get() {
    ...
	case PIN_CONFIG_BIAS_PULL_UP:
		if (pctrl->soc->pull_no_keeper)
			arg = arg == MSM_PULL_UP_NO_KEEPER;
		else if (arg & BIT(g->i2c_pull_bit))
			arg = MSM_I2C_STRONG_PULL_UP;
		else
			arg = arg == MSM_PULL_UP;
    ...
}

For pins without an I2C strong pull-up bit, g->i2c_pull_bit is 0, so
arg & BIT(g->i2c_pull_bit) tests bit 0 of the bias field.  Since both
MSM_PULL_DOWN (1) and MSM_PULL_UP (3) have bit 0 set, querying
PIN_CONFIG_BIAS_PULL_UP (for example via debugfs) on such a pin when
pull-down or standard pull-up is configured falsely reports
MSM_I2C_STRONG_PULL_UP (2200 ohms).

[1] https://sashiko.dev/#/bug/linux-42fe82f8-2ab3-4bdf-9d87-6f46c976e49a

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-egpio_fix-v1-0-569d220a4748@oss.qualcomm.com?part=2