[PATCH v2] pinctrl: intel: move PWM base computation past feature check

Stepan Ionichev posted 1 patch 1 week ago
There is a newer version of this series
drivers/pinctrl/intel/pinctrl-intel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH v2] pinctrl: intel: move PWM base computation past feature check
Posted by Stepan Ionichev 1 week ago
intel_pinctrl_probe_pwm() computes the PWM register base unconditionally
at the start of the function:

	void __iomem *base = community->regs + capability_offset + 4;

	if (!(community->features & PINCTRL_FEATURE_PWM))
		return 0;
	if (!IS_REACHABLE(CONFIG_PWM_LPSS))
		return 0;

	chip = devm_pwm_lpss_probe(pctrl->dev, base, &info);

For communities without CAPLIST_ID_PWM, capability_offset is the
uninitialised value left in the per-community array on the stack;
forming base = regs + capability_offset + 4 from that value reads
indeterminate state even though the result is never dereferenced
(the feature check returns first).

Split the declaration from the assignment so the value is only
computed after PINCTRL_FEATURE_PWM and the CONFIG_PWM_LPSS
reachability check have passed. base is then the same single use
that devm_pwm_lpss_probe() already consumes, so no other call sites
are affected.

Suggested-by: Andy Shevchenko <andy@kernel.org>
Link: https://lore.kernel.org/linux-gpio/aglu5jy5SbW9Wjwj@ashevche-desk.local/
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
Changes since v1:
- Drop the array zero-initialisation approach.
- Move the base assignment past the PINCTRL_FEATURE_PWM and
  CONFIG_PWM_LPSS checks instead, per Andy's review.
- Drop the Fixes: tag (per Andy: this is a cleanup, not a fix).

v1: https://lore.kernel.org/linux-gpio/20260515150049.33761-1-sozdayvek@gmail.com/

 drivers/pinctrl/intel/pinctrl-intel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 97bf5ec78..2e2526e01 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1556,13 +1556,13 @@ static int intel_pinctrl_probe_pwm(struct intel_pinctrl *pctrl,
 				   struct intel_community *community,
 				   unsigned short capability_offset)
 {
-	void __iomem *base = community->regs + capability_offset + 4;
 	static const struct pwm_lpss_boardinfo info = {
 		.clk_rate = 19200000,
 		.npwm = 1,
 		.base_unit_bits = 22,
 	};
 	struct pwm_chip *chip;
+	void __iomem *base;
 
 	if (!(community->features & PINCTRL_FEATURE_PWM))
 		return 0;
@@ -1570,6 +1570,7 @@ static int intel_pinctrl_probe_pwm(struct intel_pinctrl *pctrl,
 	if (!IS_REACHABLE(CONFIG_PWM_LPSS))
 		return 0;
 
+	base = community->regs + capability_offset + 4;
 	chip = devm_pwm_lpss_probe(pctrl->dev, base, &info);
 	return PTR_ERR_OR_ZERO(chip);
 }
-- 
2.43.0
Re: [PATCH v2] pinctrl: intel: move PWM base computation past feature check
Posted by Andy Shevchenko 1 week ago
On Sun, May 17, 2026 at 6:40 PM Stepan Ionichev <sozdayvek@gmail.com> wrote:
>
> intel_pinctrl_probe_pwm() computes the PWM register base unconditionally
> at the start of the function:
>
>         void __iomem *base = community->regs + capability_offset + 4;
>
>         if (!(community->features & PINCTRL_FEATURE_PWM))
>                 return 0;
>         if (!IS_REACHABLE(CONFIG_PWM_LPSS))
>                 return 0;
>
>         chip = devm_pwm_lpss_probe(pctrl->dev, base, &info);
>
> For communities without CAPLIST_ID_PWM, capability_offset is the
> uninitialised value left in the per-community array on the stack;
> forming base = regs + capability_offset + 4 from that value reads
> indeterminate state even though the result is never dereferenced
> (the feature check returns first).
>
> Split the declaration from the assignment so the value is only
> computed after PINCTRL_FEATURE_PWM and the CONFIG_PWM_LPSS
> reachability check have passed. base is then the same single use
> that devm_pwm_lpss_probe() already consumes, so no other call sites
> are affected.

The commit message is too long for this smaller clean up. Just say
that we want to tidy up the code for better maintenance (it's the only
reason I see for this patch), otherwise you also need to prove that
compiler does some bad things.

...

Code wise okay.

-- 
With Best Regards,
Andy Shevchenko