On Mon, Feb 22, 2016 at 11:46:39AM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 17, 2016 at 02:19:26PM +0100, Manfred Schlaegl wrote:
> > If the pwm can sleep defer actions to it using a worker.
> > A similar approach was used in leds-pwm (c971ff185)
> >
> > Trigger:
> > On a Freescale i.MX53 based board we ran into "BUG: scheduling while
> > atomic" because input_inject_event locks interrupts, but
> > imx_pwm_config_v2 sleeps.
> >
> > Tested on Freescale i.MX53 SoC with 4.5-rc1 and 4.1.
> >
> > Unmodified applicable to
> > * 4.5-rc4
> > * 4.4.1 (stable)
> > * 4.3.5 (stable)
> > * 4.1.18 (longterm)
> >
> > Modified applicable to
> > * 3.18.27 (longterm)
> >
> > Signed-off-by: Manfred Schlaegl <manfred.schlaegl@xxxxxx>
> > ---
> > drivers/input/misc/pwm-beeper.c | 62 +++++++++++++++++++++++++++++------------
> > 1 file changed, 44 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> > index f2261ab..c160b5e 100644
> > --- a/drivers/input/misc/pwm-beeper.c
> > +++ b/drivers/input/misc/pwm-beeper.c
> > @@ -20,21 +20,42 @@
> > #include <linux/platform_device.h>
> > #include <linux/pwm.h>
> > #include <linux/slab.h>
> > +#include <linux/workqueue.h>
> >
> > struct pwm_beeper {
> > struct input_dev *input;
> > struct pwm_device *pwm;
> > + struct work_struct work;
> > unsigned long period;
> > + bool can_sleep;
>
> I wonder if it is not better to always schedule work, regardless of
> whether PWM may sleep or not.
I agree with Dmitry. Users of the PWM API should always assume that
calls to the PWM API might sleep. Conditionalizing on pwm_can_sleep()
isn't a good idea, since that function is scheduled to be removed. In
fact it's been returning true unconditionally since v4.5, so the fast
path is dead code anyway.
Thierry
Attachment:
signature.asc
Description: PGP signature
