[v1] pwm: microchip-core: fix incorrect comparison with max period

Commit Message

Conor Dooley Jan. 22, 2025, 2:42 p.m. UTC

From: Conor Dooley <[e-mail]>

In mchp_core_pwm_apply_locked(), if hw_period_steps is equal to its max,
an error is reported and .apply fails. The max value is actually a
permitted value however, and so this check can fail where multiple
channels are enabled.

For example, the first channel to be configured requests a period that
sets hw_period_steps to the maximum value, and when a second channel
is enabled the driver reads hw_period_steps back from the hardware and
finds it to be the maximum possible value, triggering the warning on a
permitted value. The value to be avoided is 255 (PERIOD_STEPS_MAX + 1),
as that will produce undesired behaviour, so test for greater than,
rather than equal to.

Fixes: 2bf7ecf7b4ff ("pwm: add microchip soft ip corePWM driver")
CC: [e-mail]
Signed-off-by: Conor Dooley <[e-mail]>
---
CC: Conor Dooley <[e-mail]>
CC: Daire McNamara <[e-mail]>
CC: Uwe Kleine-König <[e-mail]>
CC: Thierry Reding <[e-mail]>
CC: [e-mail]
CC: [e-mail]
CC: [e-mail]
---
 drivers/pwm/pwm-microchip-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Uwe Kleine-König Jan. 23, 2025, 3:15 p.m. UTC | #1

Hello Conor,

On Wed, Jan 22, 2025 at 02:42:56PM +0000, Conor Dooley wrote:
> From: Conor Dooley <[e-mail]>
> 
> In mchp_core_pwm_apply_locked(), if hw_period_steps is equal to its max,
> an error is reported and .apply fails. The max value is actually a
> permitted value however, and so this check can fail where multiple
> channels are enabled.
> 
> For example, the first channel to be configured requests a period that
> sets hw_period_steps to the maximum value, and when a second channel
> is enabled the driver reads hw_period_steps back from the hardware and
> finds it to be the maximum possible value, triggering the warning on a
> permitted value. The value to be avoided is 255 (PERIOD_STEPS_MAX + 1),
> as that will produce undesired behaviour, so test for greater than,
> rather than equal to.
> 
> Fixes: 2bf7ecf7b4ff ("pwm: add microchip soft ip corePWM driver")
> CC: [e-mail]
> Signed-off-by: Conor Dooley <[e-mail]>

Applied to

	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/fixes

which I intend to send to Linus next week.

Best regards
Uwe

Patch

diff --git a/drivers/pwm/pwm-microchip-core.c b/drivers/pwm/pwm-microchip-core.c
index c1f2287b8e97..12821b4bbf97 100644
--- a/drivers/pwm/pwm-microchip-core.c
+++ b/drivers/pwm/pwm-microchip-core.c
@@ -327,7 +327,7 @@  static int mchp_core_pwm_apply_locked(struct pwm_chip *chip, struct pwm_device *
 		 * mchp_core_pwm_calc_period().
 		 * The period is locked and we cannot change this, so we abort.
 		 */
-		if (hw_period_steps == MCHPCOREPWM_PERIOD_STEPS_MAX)
+		if (hw_period_steps > MCHPCOREPWM_PERIOD_STEPS_MAX)
 			return -EINVAL;
 
 		prescale = hw_prescale;