diff mbox

[v2,1/3] input: touchscreen: edt-ft5x06: don't make device a wakeup source by default

Message ID 20180517090552.5704-2-daniel@zonque.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Mack May 17, 2018, 9:05 a.m. UTC
Allow configuring the device as wakeup source through device properties, as
not all platforms want to wake up on touch screen activity.

The I2C core automatically reads the "wakeup-source" DT property to
configure a device's wakeup capability, and board supports files can set
I2C_CLIENT_WAKE in the flags.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt | 3 +++
 drivers/input/touchscreen/edt-ft5x06.c                             | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Rob Herring (Arm) May 22, 2018, 5:54 p.m. UTC | #1
On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
> Allow configuring the device as wakeup source through device properties, as
> not all platforms want to wake up on touch screen activity.
> 
> The I2C core automatically reads the "wakeup-source" DT property to
> configure a device's wakeup capability, and board supports files can set
> I2C_CLIENT_WAKE in the flags.

This will break wake-up on working systems. Looks like mostly i.MX, but 
there's one AM437x board. If that board doesn't care, then it is up to 
Shawn.

Rob
Daniel Mack May 23, 2018, 8:27 a.m. UTC | #2
On Tuesday, May 22, 2018 07:54 PM, Rob Herring wrote:
> On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
>> Allow configuring the device as wakeup source through device properties, as
>> not all platforms want to wake up on touch screen activity.
>>
>> The I2C core automatically reads the "wakeup-source" DT property to
>> configure a device's wakeup capability, and board supports files can set
>> I2C_CLIENT_WAKE in the flags.
> 
> This will break wake-up on working systems. Looks like mostly i.MX, but
> there's one AM437x board. If that board doesn't care, then it is up to
> Shawn.

I added the property to the dts files, but as Dmitry pointed out, I 
missed some. Sorry for that.


Thanks,
Daniel
Rob Herring (Arm) May 23, 2018, 2:45 p.m. UTC | #3
On Wed, May 23, 2018 at 3:27 AM, Daniel Mack <daniel@zonque.org> wrote:
> On Tuesday, May 22, 2018 07:54 PM, Rob Herring wrote:
>>
>> On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
>>>
>>> Allow configuring the device as wakeup source through device properties,
>>> as
>>> not all platforms want to wake up on touch screen activity.
>>>
>>> The I2C core automatically reads the "wakeup-source" DT property to
>>> configure a device's wakeup capability, and board supports files can set
>>> I2C_CLIENT_WAKE in the flags.
>>
>>
>> This will break wake-up on working systems. Looks like mostly i.MX, but
>> there's one AM437x board. If that board doesn't care, then it is up to
>> Shawn.
>
>
> I added the property to the dts files, but as Dmitry pointed out, I missed
> some. Sorry for that.

Just adding the property to dts files doesn't fix the compatibility
problem. If a user uses an existing dtb (before this change) with a
new kernel (after this change), then wakeup will stop working.

Rob
Dmitry Torokhov May 24, 2018, 11:17 p.m. UTC | #4
On Wed, May 23, 2018 at 09:45:05AM -0500, Rob Herring wrote:
> On Wed, May 23, 2018 at 3:27 AM, Daniel Mack <daniel@zonque.org> wrote:
> > On Tuesday, May 22, 2018 07:54 PM, Rob Herring wrote:
> >>
> >> On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
> >>>
> >>> Allow configuring the device as wakeup source through device properties,
> >>> as
> >>> not all platforms want to wake up on touch screen activity.
> >>>
> >>> The I2C core automatically reads the "wakeup-source" DT property to
> >>> configure a device's wakeup capability, and board supports files can set
> >>> I2C_CLIENT_WAKE in the flags.
> >>
> >>
> >> This will break wake-up on working systems. Looks like mostly i.MX, but
> >> there's one AM437x board. If that board doesn't care, then it is up to
> >> Shawn.
> >
> >
> > I added the property to the dts files, but as Dmitry pointed out, I missed
> > some. Sorry for that.
> 
> Just adding the property to dts files doesn't fix the compatibility
> problem. If a user uses an existing dtb (before this change) with a
> new kernel (after this change), then wakeup will stop working.

Is this a practical problem though? Do we know of any products with
this touch panel that use DTS not distributed with the kernel?
Rob Herring (Arm) May 25, 2018, 3:52 p.m. UTC | #5
On Thu, May 24, 2018 at 6:17 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, May 23, 2018 at 09:45:05AM -0500, Rob Herring wrote:
>> On Wed, May 23, 2018 at 3:27 AM, Daniel Mack <daniel@zonque.org> wrote:
>> > On Tuesday, May 22, 2018 07:54 PM, Rob Herring wrote:
>> >>
>> >> On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
>> >>>
>> >>> Allow configuring the device as wakeup source through device properties,
>> >>> as
>> >>> not all platforms want to wake up on touch screen activity.
>> >>>
>> >>> The I2C core automatically reads the "wakeup-source" DT property to
>> >>> configure a device's wakeup capability, and board supports files can set
>> >>> I2C_CLIENT_WAKE in the flags.
>> >>
>> >>
>> >> This will break wake-up on working systems. Looks like mostly i.MX, but
>> >> there's one AM437x board. If that board doesn't care, then it is up to
>> >> Shawn.
>> >
>> >
>> > I added the property to the dts files, but as Dmitry pointed out, I missed
>> > some. Sorry for that.
>>
>> Just adding the property to dts files doesn't fix the compatibility
>> problem. If a user uses an existing dtb (before this change) with a
>> new kernel (after this change), then wakeup will stop working.
>
> Is this a practical problem though? Do we know of any products with
> this touch panel that use DTS not distributed with the kernel?

Distribution of dts with kernel is irrelevant. It is how the dtb's are
handled that matters. You still need to ask in tree users.

I can't have any way of knowing what DTs may exist as there is only
one upstream repository of dts files. There's not really a good
solution on this to avoid breaking users, so we'll just have to see if
anyone (besides the known users) complains.

Rob
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 025cf8c9324a..83f792d4d88c 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -52,6 +52,8 @@  Optional properties:
  - touchscreen-inverted-y  : See touchscreen.txt
  - touchscreen-swapped-x-y : See touchscreen.txt
 
+ - wakeup-source: touchscreen acts as wakeup source
+
 Example:
 	polytouch: edt-ft5x06@38 {
 		compatible = "edt,edt-ft5406", "edt,edt-ft5x06";
@@ -62,4 +64,5 @@  Example:
 		interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
 		reset-gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
 		wake-gpios = <&gpio4 9 GPIO_ACTIVE_HIGH>;
+		wakeup-source;
 	};
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 5bf63f76ddda..e18a2f215500 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1007,7 +1007,6 @@  static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		goto err_remove_attrs;
 
 	edt_ft5x06_ts_prepare_debugfs(tsdata, dev_driver_string(&client->dev));
-	device_init_wakeup(&client->dev, 1);
 
 	dev_dbg(&client->dev,
 		"EDT FT5x06 initialized: IRQ %d, WAKE pin %d, Reset pin %d.\n",