Re: [PATCH v1 2/2] pinctrl: qcom: Allow arming the wakeup_enable bit from pin config

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] pinctrl: qcom: missing hardware capability check in wakeup-enable get()
--

commit b3082e25a0d303b051061eac8a2191e7673c4106
Author: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

pinctrl: qcom: Allow arming the wakeup_enable bit from pin config

This commit adds a "wakeup-enable" pin configuration parameter to arm the
wakeup_enable bit without taking over the pin mux. It folds open-coded
accesses into a helper shared between GPIO IRQ requests and pin config. Pins
not described as wakeup-capable by the SoC are rejected.

> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 27f06bbc113c9..cb12f8b906007 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
[ ... ]
> @@ -354,6 +402,16 @@ static int msm_config_group_get(struct pinctrl_dev *pctldev,
>  
>  	g = &pctrl->soc->groups[group];
>  
> +	if (param == MSM_PINCONF_WAKEUP_ENABLE) {
> +		if (!g->intr_wakeup_present_bit)
> +			return -EOPNOTSUPP;
> +
> +		arg = msm_readl_intr_cfg(pctrl, g) & BIT(g->intr_wakeup_enable_bit);
> +		*config = pinconf_to_config_packed(param, !!arg);

[Severity: Low]
Does this code miss a hardware capability check before returning the
configuration?

In the setter msm_pin_set_wakeup_enable(), the code correctly verifies that
the specific pin has the capability by checking the dynamic hardware presence
bit:

    intr_cfg = msm_readl_intr_cfg(pctrl, g);
    if (intr_cfg & BIT(g->intr_wakeup_present_bit)) {

However, this getter msm_config_group_get() only checks the static bit offset
g->intr_wakeup_present_bit from the SoC struct and fails to read the register
to check the actual dynamic hardware capability.

Can querying wakeup-enable on a pin that is dynamically not wakeup-capable
succeed and return unverified, meaningless register state from the enable_bit
offset, exposing inconsistent capabilities to userspace?

> +
> +		return 0;
> +	}
> +
>  	ret = msm_config_reg(pctrl, g, param, &mask, &bit);
>  	if (ret < 0)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-tlmm_wakeup_gpio_support-v1-0-c6d6f37a7e51@oss.qualcomm.com?part=2