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

[sashiko-bot@kernel.org]

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