Hi Johan, On Tue, Oct 13, 2015 at 02:39:48PM +0200, Johan Hovold wrote: > On Mon, Oct 12, 2015 at 09:13:32PM +0200, Sylvain Rochet wrote: > > This patch adds wake up support to GPIO rotary encoders. > > > > Signed-off-by: Sylvain Rochet <sylvain.rochet@xxxxxxxxxxxx> > > Reviewed-by: Johan Hovold <johan@xxxxxxxxxx> > > Hmm. I have not yet reviewed the changes you did in v4. Woops, sorry, I forgot to remove it while rebasing. > > --- > > drivers/input/misc/rotary_encoder.c | 39 +++++++++++++++++++++++++++++++++++++ > > include/linux/rotary_encoder.h | 1 + > > 2 files changed, 40 insertions(+) > > > > diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c > > index f27f81e..0d86dc4 100644 > > --- a/drivers/input/misc/rotary_encoder.c > > +++ b/drivers/input/misc/rotary_encoder.c > > > @@ -280,6 +283,10 @@ static int rotary_encoder_probe(struct platform_device *pdev) > > goto exit_free_irq_b; > > } > > > > + device_set_wakeup_capable(&pdev->dev, true); > > You should continue to use the platform data to determine whether the > device is capable of wakeup or not. > > > + if (pdata->wakeup_source) > > + device_wakeup_enable(&pdev->dev); > > + > > Just stick to > > device_init_wakeup(&pdev->dev, pdata->wakeup_source); There is unfortunately no or poor documentation on how "wakeup-source" DT property should behave. We have no clue if wake up should be enabled by default, which is what device_init_wakeup() is doing. If "wakeup-source" is just a flag to determine whether the device is capable of wakeup or not then it should probably not change the behavior and wake up should probably be off by default, thus the right way would be to call device_set_wakeup_capable(&pdev->dev, pdata->wakeup_source); device_init_wakeup() is a bit confusing, its introductory comment says that "By default, most devices should leave wakeup disabled." but it actually enables it by default. In this case we are the exception "The exceptions are devices that everyone expects to be wakeup sources: keyboards, …" but it only adds more confusion, we are the exception, but by default we are not. Anyway, I don't really care and I will resend with device_init_wakeup() because wakeup support enabled by default is what I need. Thank you very much :-) Cheers, Sylvain -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
