Re: [PATCH] pwm: removes period check from pwm_apply_state()

[Uwe Kleine-König]

Hello,

On Fri, Aug 05, 2022 at 03:41:25PM +0530, Tamseel Shams wrote:
> There may be situation when PWM is exported using sysfs,
> but at that point PWM period is not set. At this situation
> if we issue a system suspend, it calls pwm_class_suspend
> which in turn calls pwm_apply_state, where PWM period value is
> checked which returns an invalid argument error casuing Kernel
> to panic. So, check for PWM period value is removed so as to
> fix the kernel panic observed during suspend.

This looks and sounds wrong. One thing I would accept is:

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 0e042410f6b9..075bbcdad6c1 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -557,8 +557,8 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 	 */
 	might_sleep();
 
-	if (!pwm || !state || !state->period ||
-	    state->duty_cycle > state->period)
+	if (!pwm || !state || state->enabled && (!state->period ||
+	    state->duty_cycle > state->period))
 		return -EINVAL;
 
 	chip = pwm->chip;

That is, don't refuse calling pwm_apply_state() for state->period = 0
and even state->duty_cycle > state->period if the PWM is not enabled.

But anyhow, even without that the kernel should not panic. So I ask you
to research and provide some more info about the problem. (Which
hardware does it affect? Where does it panic? ...)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature