On 04/04/2017 06:41 AM, Dmitry Torokhov wrote:
On Mon, Apr 03, 2017 at 01:53:53PM -0700, Brian Norris wrote:
On Mon, Apr 03, 2017 at 11:43:36AM -0700, Dmitry Torokhov wrote:
On Sun, Apr 02, 2017 at 08:07:39AM +0800, Jeffy Chen wrote:
Report wakeup events when process events.
Signed-off-by: Jeffy Chen <jeffy.chen@xxxxxxxxxxxxxx>
Changes in v2:
Remove unneeded dts changes.
drivers/input/keyboard/cros_ec_keyb.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c
index 6a250d6..a93d55f 100644
@@ -286,6 +286,9 @@ static int cros_ec_keyb_work(struct
+ if (device_may_wakeup(ckdev->dev))
+ pm_wakeup_event(ckdev->dev, 0);
@@ -632,6 +635,12 @@ static int cros_ec_keyb_probe(struct
+ err = device_init_wakeup(dev, 1);
I would prefer if we did not mark cros_ec devices as wakeup sources
unconditionally. Your original patch series was better (except it
to parse the "wakeup-source" property that you introduced.
I'm curious, why is this keyboard device different than any other
device? I see several other drivers in drivers/input/keyboard/ that
unconditional 'device_init_wakeup(..., 1)'. Keyboards tend to be wakeup
If we did something before it does not mean we should continue doing
this forever. I think providing an option to mark device as wakeup
capable should be left to the platform.
Also, what's the idea behind sub-devices vs. the main cros-ec device
wakeups? Right now, we have this in drivers/mfd/cros_ec.c:
static irqreturn_t ec_irq_thread(int irq, void *data)
struct cros_ec_device *ec_dev = data;
ret = cros_ec_get_next_event(ec_dev);
if (ret > 0)
But now, we're going to start double-reporting wakeups? Is that
hmmm, it looks like the i2c-based ec also do this, but through i2c-core:
No, and not always (below).
I think we have a similar overlap with the RTC driver (which is being
[PATCH v3 3/4] rtc: cros-ec: add cros-ec-rtc driver.
except that also goes through the trouble of enabling/disabling
wakeup for the
EC IRQ. It seems to me (though I haven't dug in thoroughly) like the
main MFD shouldn't really be doing the wakeup reporting at all, and we
should depend on the sub-devices to do this. (i.e., the current patchset
is a step in the right direction, but it's not 100%.)
Anyway, I could be wrong about the above, but I think we should make
sure there's a consistent answer across the drivers tree.
Hm, it appears we have quite a mess. SPI-based EC declares entire EC as
wakeup source (unconditionally I must add; we do mention "wakeup-source"
in binding document at least). I2C-based EC does not call
device_init_wakeup() at all, presumably that is what caused the calls to
be added into sub-drivers.
we have this https://chromium-review.googlesource.com/c/372399/ in cros 4.4 kernel, but somehow not upstream.
We need to resolve this one way or another. You probably do not want to
wake up any time you move your device (accelerometer or other sensors),
so I would try to move this property into individual devices, and try to
come up with a reasonable binding.
right, we do have a issue about gyro sensor break
it would be better if we move wakeup codes to sub drivers. and if you do
this, it would also solve the original issue of this patchset ;)