Hello Johan,

On Thu, Jan 29, 2015 at 11:49:53AM +0100, Johan Hovold wrote:

On Tue, Jan 13, 2015 at 04:51:02PM +0100, Sylvain Rochet wrote:

quoted

This patch add wake up support for rotary encoders.
New DT property: rotary-encoder,wakeup

Signed-off-by: Sylvain Rochet <redacted>
---
 .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
 Documentation/input/rotary-encoder.txt             |  1 +
 drivers/input/misc/rotary_encoder.c                | 36 ++++++++++++++++++++++
 include/linux/rotary_encoder.h                     |  1 +
 4 files changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
index 3315495..468d545 100644
--- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
+++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt

@@ -15,6 +15,7 @@ Optional properties:
 - rotary-encoder,rollover: Automatic rollove when the rotary value becomes
   greater than the specified steps or smaller than 0. For absolute axis only.
 - rotary-encoder,half-period: Makes the driver work on half-period mode.
+- rotary-encoder,wakeup: Boolean, rotary encoder can wake-up the system.

As Dmitry already mentioned this should probably just be
"wakeup-source".

Yeah, I think so.

quoted

+static SIMPLE_DEV_PM_OPS(rotary_encoder_pm_ops, rotary_encoder_suspend, rotary_encoder_resume);

Please break this line to stay within 80 cols.

I saw that afterward, I waited the answer about DT property to propose a 
v2. Fixed.

quoted

+	int wakeup;			/* configure the rotary-encoder as a wake-up source */

This should be bool.

Ditto. Fixed.

I also suggest you rename the variable wakeup_source and drop the
verbose comment.

I agree, done !

Looks good otherwise.

Thank for the review, v2 sent :)


Sylvain