[PATCH 0/5] pwm: fix overflow, truncation and error-path reference leaks

[PATCH 0/5] pwm: fix overflow, truncation and error-path reference leaks

[shao.mingyin]

Five independent defects in four PWM drivers, found while reviewing
linux-next next-20261030.  Two are 32-bit overflows that make the
hardware readback report a wrong period or duty cycle, one is a 16-bit
truncation of the on/off counters, and two are missing clock or
runtime-PM cleanups on error paths.  All are reachable from the PWM
sysfs interface or from the hardware readback path.

The series is ordered by impact and is independent patch-by-patch; each
patch applies on its own and taking a subset is fine.  Patches 1 and 4
both touch pwm-dwc-core.c but different callbacks (dwc_pwm_get_state()
and dwc_pwm_apply()), so they can be applied separately.

  1/5 pwm: dwc: Fix 32 bit overflow when computing duty/period in get_state
  2/5 pwm: airoha: Fix 32 bit overflow when converting duty ticks to ns
  3/5 pwm: adp5585: Clamp on/off counters to the 16 bit register range
  4/5 pwm: dwc: Balance the runtime PM reference when configuring fails
  5/5 pwm: rockchip: Fix pclk leak in get_state error path

Shao Mingyin (5):
  pwm: dwc: Fix 32 bit overflow when computing duty/period in get_state
  pwm: airoha: Fix 32 bit overflow when converting duty ticks to ns
  pwm: adp5585: Clamp on/off counters to the 16 bit register range
  pwm: dwc: Balance the runtime PM reference when configuring fails
  pwm: rockchip: Fix pclk leak in get_state error path

 drivers/pwm/pwm-adp5585.c  |  7 +++++++
 drivers/pwm/pwm-airoha.c   |  8 ++++----
 drivers/pwm/pwm-dwc-core.c | 12 ++++++++----
 drivers/pwm/pwm-rockchip.c |  6 +++++-
 4 files changed, 24 insertions(+), 9 deletions(-)

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 0/5] pwm: fix overflow, truncation and error-path reference leaks

[shao.mingyin]

Five independent defects in four PWM drivers, found while reviewing
linux-next next-20261030.  Two are 32-bit overflows that make the
hardware readback report a wrong period or duty cycle, one is a 16-bit
truncation of the on/off counters, and two are missing clock or
runtime-PM cleanups on error paths.  All are reachable from the PWM
sysfs interface or from the hardware readback path.

The series is ordered by impact and is independent patch-by-patch; each
patch applies on its own and taking a subset is fine.  Patches 1 and 4
both touch pwm-dwc-core.c but different callbacks (dwc_pwm_get_state()
and dwc_pwm_apply()), so they can be applied separately.

  1/5 pwm: dwc: Fix 32 bit overflow when computing duty/period in get_state
  2/5 pwm: airoha: Fix 32 bit overflow when converting duty ticks to ns
  3/5 pwm: adp5585: Clamp on/off counters to the 16 bit register range
  4/5 pwm: dwc: Balance the runtime PM reference when configuring fails
  5/5 pwm: rockchip: Fix pclk leak in get_state error path

Shao Mingyin (5):
  pwm: dwc: Fix 32 bit overflow when computing duty/period in get_state
  pwm: airoha: Fix 32 bit overflow when converting duty ticks to ns
  pwm: adp5585: Clamp on/off counters to the 16 bit register range
  pwm: dwc: Balance the runtime PM reference when configuring fails
  pwm: rockchip: Fix pclk leak in get_state error path

 drivers/pwm/pwm-adp5585.c  |  7 +++++++
 drivers/pwm/pwm-airoha.c   |  8 ++++----
 drivers/pwm/pwm-dwc-core.c | 12 ++++++++----
 drivers/pwm/pwm-rockchip.c |  6 +++++-
 4 files changed, 24 insertions(+), 9 deletions(-)

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

[PATCH 1/5] pwm: dwc: Fix 32 bit overflow when computing duty/period in get_state

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

dwc_pwm_get_state() computes duty and period with
"duty = (ld + 1) * dwc->clk_ns".  ld and ld2 are u32 and dwc->clk_ns is
an unsigned int, so the multiplication is carried out in 32 bit and wraps
modulo 2^32 before the (already truncated) result is assigned to the u64
duty/period variables.  For load counts above ~2^32 / clk_ns the reported
duty cycle and period are wrong.

Cast the load counts to u64 before multiplying.

Fixes: 4aff152d14c14 ("pwm: dwc: add PWM bit unset in get_state call")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---

diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index 6dabec93a3c6..104393c499f0 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -137,11 +137,11 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * based on the timer load-count only.
 	 */
 	if (ctrl & DWC_TIM_CTRL_PWM) {
-		duty = (ld + 1) * dwc->clk_ns;
-		period = (ld2 + 1)  * dwc->clk_ns;
+		duty = ((u64)ld + 1) * dwc->clk_ns;
+		period = ((u64)ld2 + 1)  * dwc->clk_ns;
 		period += duty;
 	} else {
-		duty = (ld + 1) * dwc->clk_ns;
+		duty = ((u64)ld + 1) * dwc->clk_ns;
 		period = duty * 2;
 	}
 
-- 
2.47.0

[PATCH 2/5] pwm: airoha: Fix 32 bit overflow when converting duty ticks to ns

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

airoha_pwm_get_duty_ns_from_ticks() computes "duty_tick * period_ns"
where both operands are u32.  The result is passed to DIV_U64_ROUND_UP()
as a u64, but the multiplication itself is performed in 32 bit and wraps
modulo 2^32.  period_ns can be as large as AIROHA_PWM_PERIOD_MAX_NS (1 s)
and duty_tick up to AIROHA_PWM_DUTY_FULL (255), so the product exceeds
u32 and the reported duty cycle is wrong.

Cast duty_tick to u64 before multiplying.

Fixes: b55bbc2872eb ("pwm: airoha: Add support for EN7581 SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---

diff --git a/drivers/pwm/pwm-airoha.c b/drivers/pwm/pwm-airoha.c
index 0cf6b011520a..702c9cfc0e37 100644
--- a/drivers/pwm/pwm-airoha.c
+++ b/drivers/pwm/pwm-airoha.c
@@ -152,9 +152,9 @@ static u32 airoha_pwm_get_duty_ns_from_ticks(u32 period_tick, u32 duty_tick)
 	/*
-	 * Overflow can't occur in multiplication as duty_tick is just 8 bit
-	 * and period_ns is clamped to AIROHA_PWM_PERIOD_MAX_NS and fit in a
-	 * u64.
+	 * period_ns is at most AIROHA_PWM_PERIOD_MAX_NS (1 s), which fits in
+	 * a u32, but duty_tick * period_ns can exceed 32 bits, so promote
+	 * duty_tick to u64 before multiplying.
 	 */
-	return DIV_U64_ROUND_UP(duty_tick * period_ns, AIROHA_PWM_DUTY_FULL);
+	return DIV_U64_ROUND_UP((u64)duty_tick * period_ns, AIROHA_PWM_DUTY_FULL);
 }
 
 static int airoha_pwm_get_bucket(struct airoha_pwm *pc, int bucket,
-- 
2.47.0

[PATCH 3/5] pwm: adp5585: Clamp on/off counters to the 16 bit register range

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

pwm_adp5585_apply() computes the on and off counter values as u32 and
writes them through cpu_to_le16() into 16 bit registers.  The period is
only clamped to ADP5585_PWM_MAX_PERIOD_NS which is 2 * 0xffff oscillator
ticks, so for duty cycles close to a full period the on counter can
exceed 0xffff and is silently truncated by cpu_to_le16(), programming a
wrong duty cycle.

Clamp both counters to the representable range before writing them.
This is a saturation, not an exact mapping: when the on counter is
clamped the programmed period can be shorter than the requested one.

Fixes: e9b503879fd2 ("pwm: adp5585: Add Analog Devices ADP5585 support")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---

diff --git a/drivers/pwm/pwm-adp5585.c b/drivers/pwm/pwm-adp5585.c
index 9a1dbba75bc1..da83fcd35e1d 100644
--- a/drivers/pwm/pwm-adp5585.c
+++ b/drivers/pwm/pwm-adp5585.c
@@ -96,6 +96,13 @@ static int pwm_adp5585_apply(struct pwm_chip *chip,
 	on = div_u64(duty_cycle, NSEC_PER_SEC / ADP5585_PWM_OSC_FREQ_HZ);
 	off = div_u64(period, NSEC_PER_SEC / ADP5585_PWM_OSC_FREQ_HZ) - on;
 
+	/*
+	 * The on/off registers are 16 bits wide, clamp the counter values so
+	 * that cpu_to_le16() below can't silently truncate them.
+	 */
+	on = min_t(u32, on, 0xffff);
+	off = min_t(u32, off, 0xffff);
+
 	val = cpu_to_le16(off);
 	ret = regmap_bulk_write(regmap, info->pwm_offt_low, &val, 2);
 	if (ret)
-- 
2.47.0

Re: [PATCH 3/5] pwm: adp5585: Clamp on/off counters to the 16 bit register range

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] pwm: adp5585: clamp on before computing off in
  pwm_adp5585_apply()
--

commit 636e863f3fd6ed7ec6fcc05f5eed5bd16bc080a9
Author: Shao Mingyin <shao.mingyin@zte.com.cn>

pwm: adp5585: Clamp on/off counters to the 16 bit register range

This commit clamps the on and off counter values in pwm_adp5585_apply() to
0xffff before writing them to the 16-bit hardware registers.

> diff --git a/drivers/pwm/pwm-adp5585.c b/drivers/pwm/pwm-adp5585.c
> index 9a1dbba75bc14..da83fcd35e1d7 100644
> --- a/drivers/pwm/pwm-adp5585.c
> +++ b/drivers/pwm/pwm-adp5585.c
> @@ -96,6 +96,13 @@ static int pwm_adp5585_apply(struct pwm_chip *chip,
>  	on = div_u64(duty_cycle, NSEC_PER_SEC / ADP5585_PWM_OSC_FREQ_HZ);
>  	off = div_u64(period, NSEC_PER_SEC / ADP5585_PWM_OSC_FREQ_HZ) - on;
>  
> +	/*
> +	 * The on/off registers are 16 bits wide, clamp the counter values so
> +	 * that cpu_to_le16() below can't silently truncate them.
> +	 */
> +	on = min_t(u32, on, 0xffff);
> +	off = min_t(u32, off, 0xffff);
> +
>  	val = cpu_to_le16(off);
>  	ret = regmap_bulk_write(regmap, info->pwm_offt_low, &val, 2);
>  	if (ret)

[Severity: Medium]
Should on be clamped to 0xffff before subtracting it to compute off?

The hardware PWM period is the sum of the on and off counters:

drivers/pwm/pwm-adp5585.c:pwm_adp5585_get_state() {
	...
	state->duty_cycle = on * (NSEC_PER_SEC / ADP5585_PWM_OSC_FREQ_HZ);
	state->period = (on + off) * (NSEC_PER_SEC / ADP5585_PWM_OSC_FREQ_HZ);
	...
}

When duty_cycle exceeds 0xffff oscillator ticks (65,535,000 ns), subtracting
the unclamped on value from the period ticks and then clamping on to 0xffff
drops (unclamped_on - 0xffff) ticks from the total programmed period (and
triggers a warning in pwm_apply_debug() under CONFIG_PWM_DEBUG if the same
period was previously configured with a smaller duty_cycle).  Because period
is already clamped to 2 * 0xffff ticks, clamping on before computing off
preserves the requested period and guarantees off <= 0xffff whenever on is
saturated.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107000724697MsR-XmE4V-ADPPMr3wGom@zte.com.cn?part=3

Re: [PATCH 3/5] pwm: adp5585: Clamp on/off counters to the 16 bit register range

[shao.mingyin]

> Should on be clamped to 0xffff before subtracting it to compute off?

The period loss is real, but clamping on before computing off is not
correct here: it turns high duty-cycle requests into 50%.

The period is on + off and both counters are limited to 0xffff ticks.
When on saturates, the current code keeps the requested off time, so the
output stays close to the requested duty and only the period is
shortened:

  period 131070 us, duty 131070 us (100%):
    current:  on=65535 off=0      -> 65535 us,  100%
    proposed: on=65535 off=65535  -> 131070 us, 50%

  period 131070 us, duty 131000 us (99.947%):
    current:  on=65535 off=70     -> 65605 us,  99.893%
    proposed: on=65535 off=65535  -> 131070 us, 50%

Above 50% duty, keeping the off phase is much closer to the request than
keeping the period; 100% is a flat line where the period has no meaning.
The pwm_apply_debug() "best available period" warning is a heuristic and
does not justify a 50% duty error.

I'll keep the current ordering.  I can add a comment explaining why on is
clamped after off if that helps.

[PATCH 4/5] pwm: dwc: Balance the runtime PM reference when configuring fails

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

dwc_pwm_apply() takes a runtime PM reference with pm_runtime_get_sync()
when transitioning a channel from disabled to enabled and returns the
result of __dwc_pwm_configure_timer() directly.  If that call fails (e.g.
with -ERANGE for an out-of-range duty cycle or period) the reference is
never put, so the device never reaches the suspended state again and the
usage counter keeps growing.

Put the reference when the configuration fails on the enable transition.

Fixes: 721ee1884883 ("pwm: dwc: split pci out of core driver")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---

diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index 6dabec93a3c6..cfeefae68969 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -99,6 +99,7 @@ static int dwc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			 const struct pwm_state *state)
 {
 	struct dwc_pwm *dwc = to_dwc_pwm(chip);
+	int ret;
 
 	if (state->polarity != PWM_POLARITY_INVERSED)
 		return -EINVAL;
@@ -106,7 +107,10 @@ static int dwc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (state->enabled) {
 		if (!pwm->state.enabled)
 			pm_runtime_get_sync(pwmchip_parent(chip));
-		return __dwc_pwm_configure_timer(dwc, pwm, state);
+		ret = __dwc_pwm_configure_timer(dwc, pwm, state);
+		if (ret && !pwm->state.enabled)
+			pm_runtime_put_sync(pwmchip_parent(chip));
+		return ret;
 	} else {
 		if (pwm->state.enabled) {
 			__dwc_pwm_set_enable(dwc, pwm->hwpwm, false);
-- 
2.47.0

Re: [PATCH 4/5] pwm: dwc: Balance the runtime PM reference when configuring fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] pwm: dwc: inaccurate Fixes tag for dwc_pwm_apply() runtime PM
  leak
--

commit db856b4aa6941fc2e987c693ae9e69e8f32d25a5
Author: Shao Mingyin <shao.mingyin@zte.com.cn>

> pwm: dwc: Balance the runtime PM reference when configuring fails
>
> dwc_pwm_apply() takes a runtime PM reference with pm_runtime_get_sync()
> when transitioning a channel from disabled to enabled and returns the
> result of __dwc_pwm_configure_timer() directly.  If that call fails (e.g.
> with -ERANGE for an out-of-range duty cycle or period) the reference is
> never put, so the device never reaches the suspended state again and the
> usage counter keeps growing.
>
> Put the reference when the configuration fails on the enable transition.
>
> Fixes: 721ee1884883 ("pwm: dwc: split pci out of core driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>

[Severity: Low]
721ee1884883 ("pwm: dwc: split pci out of core driver") only moved
dwc_pwm_apply() from drivers/pwm/pwm-dwc.c to drivers/pwm/pwm-dwc-core.c.
Should the Fixes tag reference the commit that originally introduced the
unbalanced pm_runtime_get_sync() instead:

  Fixes: 1ed2b3fca645 ("pwm: Add DesignWare PWM Controller Driver")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107000724697MsR-XmE4V-ADPPMr3wGom@zte.com.cn?part=4

Re: [PATCH 4/5] pwm: dwc: Balance the runtime PM reference when configuring fails

[shao.mingyin]

> Should the Fixes tag reference the commit that originally introduced
> the unbalanced pm_runtime_get_sync() instead:
>
>   Fixes: 1ed2b3fca645 ("pwm: Add DesignWare PWM Controller Driver")

Agreed.  721ee1884883 only moved dwc_pwm_apply() from pwm-dwc.c to
pwm-dwc-core.c; 1ed2b3fca645 already has

  pm_runtime_get_sync(chip->dev);
  return __dwc_pwm_configure_timer(dwc, pwm, state);

I'll send a v2 with

  Fixes: 1ed2b3fca645 ("pwm: Add DesignWare PWM Controller Driver")

[PATCH 5/5] pwm: rockchip: Fix pclk leak in get_state error path

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

rockchip_pwm_get_state() enables pc->pclk and then pc->clk.  If
clk_enable(pc->clk) fails the function returns immediately, leaving
pc->pclk enabled.  The clock therefore stays on after a failed readback,
and its enable count grows with every retry; unbinding the driver then
hits "Unpreparing enabled" in the clock core.

Disable pc->pclk on the error path.

Fixes: 11be938ae003 ("pwm: rockchip: Enable clock before calling clk_get_rate()")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 050ca8aad0ca..17cf1e28a3c8 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -76,7 +76,7 @@ static int rockchip_pwm_get_state(struct pwm_chip *chip,
 
 	ret = clk_enable(pc->clk);
 	if (ret)
-		return ret;
+		goto err_clk;
 
 	clk_rate = clk_get_rate(pc->clk);
 
@@ -100,6 +100,10 @@ static int rockchip_pwm_get_state(struct pwm_chip *chip,
 	clk_disable(pc->pclk);
 
 	return 0;
+
+err_clk:
+	clk_disable(pc->pclk);
+	return ret;
 }
 
 static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-- 
2.47.0