Messages in this thread | | | From | "m.shams" <> | Subject | RE: [PATCH] pwm: removes period check from pwm_apply_state() | Date | Mon, 8 Aug 2022 19:47:03 +0530 |
| |
Hi Uwe,
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.
By this do you mean doing it following way?
if (!pwm || !state || (pwm && !state->period) || (pwm && state->duty_cycle > state->period)) return -EINVAL;
> 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? ...)
Observing Kernel panic in exynos SoC when we issue system suspend. Following is the snippet of error:
# echo mem > /sys/power/state [ 29.224784] 010: Kernel panic - not syncing: pwm pwmchip0: dpm_run_callback failure [ 29.240134] 010: Call trace: [ 29.242993] 010: dump_backtrace+0x0/0x1b8 [ 29.247067] 010: show_stack+0x24/0x30 [ 29.250793] 010: dump_stack+0xb8/0x114 [ 29.254606] 010: panic+0x180/0x398 [ 29.258073] 010: dpm_run_callback+0x270/0x278 [ 29.262493] 010: __device_suspend+0x15c/0x628 [ 29.266913] 010: dpm_suspend+0x124/0x3b0 [ 29.270899] 010: dpm_suspend_start+0xa0/0xa8 [ 29.275233] 010: suspend_devices_and_enter+0x110/0x968 [ 29.280433] 010: pm_suspend+0x308/0x3d8 [ 29.284333] 010: state_store+0x8c/0x110 [ 29.288233] 010: kobj_attr_store+0x14/0x28 [ 29.292393] 010: sysfs_kf_write+0x5c/0x78 [ 29.296466] 010: kernfs_fop_write+0x10c/0x220 [ 29.300886] 010: __vfs_write+0x48/0x90 [ 29.304699] 010: vfs_write+0xb8/0x1c0 [ 29.308426] 010: ksys_write+0x74/0x100 [ 29.312240] 010: __arm64_sys_write+0x24/0x30 [ 29.316573] 010: el0_svc_handler+0x110/0x1b8 [ 29.320906] 010: el0_svc+0x8/0x1bc [ 29.324374] 010: SMP: stopping secondary CPUs [ 29.328711] 010: Kernel Offset: disabled [ 29.332607] 010: CPU features: 0x0002,00006008 [ 29.337026] 010: Memory Limit: none [ 29.343949] 010: Rebooting in 1 seconds.. [ 30.344539] 010: Disabling non-boot CPUs ...
Thanks & Regards, Tamseel
| |