* [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
@ 2018-10-01 3:53 Kai-Heng Feng
2018-10-01 7:00 ` Benjamin Tissoires
2018-10-03 9:10 ` Jiri Kosina
0 siblings, 2 replies; 5+ messages in thread
From: Kai-Heng Feng @ 2018-10-01 3:53 UTC (permalink / raw)
To: jikos
Cc: benjamin.tissoires, hdegoede, linux-input, linux-kernel, Kai-Heng Feng
Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
although it works in display manager.
During user logging, the display manager exits, close the HID device,
then the device gets runtime suspended and powered off. The desktop
session begins shortly after, opens the HID device, then the device gets
runtime resumed and powered on.
If the trasition from display manager to desktop sesesion is fast, the
touchpanel cannot switch from powered off to powered on in short
timeframe. So add a small delay to workaround the issue.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/hid/i2c-hid/i2c-hid.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index f3076659361a..ff5682cc1bce 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
if (ret)
dev_err(&client->dev, "failed to change power setting.\n");
+ else
+ msleep(20);
set_pwr_exit:
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
2018-10-01 3:53 [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device Kai-Heng Feng
@ 2018-10-01 7:00 ` Benjamin Tissoires
2018-10-01 7:13 ` Kai Heng Feng
2018-10-03 9:10 ` Jiri Kosina
1 sibling, 1 reply; 5+ messages in thread
From: Benjamin Tissoires @ 2018-10-01 7:00 UTC (permalink / raw)
To: Kai Heng Feng; +Cc: Jiri Kosina, Hans de Goede, open list:HID CORE LAYER, lkml
On Mon, Oct 1, 2018 at 5:53 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
> although it works in display manager.
>
> During user logging, the display manager exits, close the HID device,
> then the device gets runtime suspended and powered off. The desktop
> session begins shortly after, opens the HID device, then the device gets
> runtime resumed and powered on.
>
> If the trasition from display manager to desktop sesesion is fast, the
> touchpanel cannot switch from powered off to powered on in short
> timeframe. So add a small delay to workaround the issue.
I think you want something similar to
https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/commit/?h=for-next&id=807588ac92018bde88a1958f546438e840eb0158
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index f3076659361a..ff5682cc1bce 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
>
> if (ret)
> dev_err(&client->dev, "failed to change power setting.\n");
> + else
> + msleep(20);
We shouldn't have this error in the first place, so adding a timeout
will just add a band-aid on top of an other issue.
I really think the solution from Anisse would work in your case and
will be nicer.
Cheers,
Benjamin
>
> set_pwr_exit:
> return ret;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
2018-10-01 7:00 ` Benjamin Tissoires
@ 2018-10-01 7:13 ` Kai Heng Feng
0 siblings, 0 replies; 5+ messages in thread
From: Kai Heng Feng @ 2018-10-01 7:13 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Hans de Goede, open list:HID CORE LAYER, lkml
> On Oct 1, 2018, at 3:00 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>
> On Mon, Oct 1, 2018 at 5:53 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>> Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
>> although it works in display manager.
>>
>> During user logging, the display manager exits, close the HID device,
>> then the device gets runtime suspended and powered off. The desktop
>> session begins shortly after, opens the HID device, then the device gets
>> runtime resumed and powered on.
>>
>> If the trasition from display manager to desktop sesesion is fast, the
>> touchpanel cannot switch from powered off to powered on in short
>> timeframe. So add a small delay to workaround the issue.
>
> I think you want something similar to
> https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/commit/?h=for-next&id=807588ac92018bde88a1958f546438e840eb0158
>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> drivers/hid/i2c-hid/i2c-hid.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index f3076659361a..ff5682cc1bce 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client
>> *client, int power_state)
>>
>> if (ret)
>> dev_err(&client->dev, "failed to change power setting.\n");
>> + else
>> + msleep(20);
>
> We shouldn't have this error in the first place, so adding a timeout
> will just add a band-aid on top of an other issue.
>
> I really think the solution from Anisse would work in your case and
> will be nicer.
The Hantick one requires 2.5 seconds delay, the Raydium one requires much
less.
Also it doesn't have the input shift issue.
So disabling runtime PM entirely is a little overkill in this case.
Maybe there's a better approach?
Kai-Heng
>
> Cheers,
> Benjamin
>
>> set_pwr_exit:
>> return ret;
>> --
>> 2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
2018-10-01 3:53 [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device Kai-Heng Feng
2018-10-01 7:00 ` Benjamin Tissoires
@ 2018-10-03 9:10 ` Jiri Kosina
2018-10-03 9:21 ` Kai Heng Feng
1 sibling, 1 reply; 5+ messages in thread
From: Jiri Kosina @ 2018-10-03 9:10 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: benjamin.tissoires, hdegoede, linux-input, linux-kernel
On Mon, 1 Oct 2018, Kai-Heng Feng wrote:
> Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
> although it works in display manager.
>
> During user logging, the display manager exits, close the HID device,
> then the device gets runtime suspended and powered off. The desktop
> session begins shortly after, opens the HID device, then the device gets
> runtime resumed and powered on.
>
> If the trasition from display manager to desktop sesesion is fast, the
> touchpanel cannot switch from powered off to powered on in short
> timeframe. So add a small delay to workaround the issue.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index f3076659361a..ff5682cc1bce 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
>
> if (ret)
> dev_err(&client->dev, "failed to change power setting.\n");
> + else
> + msleep(20);
In case there really is no other way around it, at least please add
comment explaining why this ugly msleep() is there. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
2018-10-03 9:10 ` Jiri Kosina
@ 2018-10-03 9:21 ` Kai Heng Feng
0 siblings, 0 replies; 5+ messages in thread
From: Kai Heng Feng @ 2018-10-03 9:21 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, Hans de Goede, linux-input, linux-kernel
> On Oct 3, 2018, at 5:10 PM, Jiri Kosina <jikos@kernel.org> wrote:
>
> On Mon, 1 Oct 2018, Kai-Heng Feng wrote:
>
>> Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
>> although it works in display manager.
>>
>> During user logging, the display manager exits, close the HID device,
>> then the device gets runtime suspended and powered off. The desktop
>> session begins shortly after, opens the HID device, then the device gets
>> runtime resumed and powered on.
>>
>> If the trasition from display manager to desktop sesesion is fast, the
>> touchpanel cannot switch from powered off to powered on in short
>> timeframe. So add a small delay to workaround the issue.
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> drivers/hid/i2c-hid/i2c-hid.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index f3076659361a..ff5682cc1bce 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
>>
>> if (ret)
>> dev_err(&client->dev, "failed to change power setting.\n");
>> + else
>> + msleep(20);
>
> In case there really is no other way around it, at least please add
> comment explaining why this ugly msleep() is there. Thanks,
I am working on a better approach for this issue, I’ll send a v2 soon.
Kai-Heng
>
> --
> Jiri Kosina
> SUSE Labs
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-03 9:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-01 3:53 [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device Kai-Heng Feng
2018-10-01 7:00 ` Benjamin Tissoires
2018-10-01 7:13 ` Kai Heng Feng
2018-10-03 9:10 ` Jiri Kosina
2018-10-03 9:21 ` Kai Heng Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).