Re: [PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
> On Jan 29, 2019, at 18:05, Benjamin Tissoires
> wrote:
>
> On Mon, Jan 21, 2019 at 4:30 AM Kai-Heng Feng
> wrote:
>>
>>
>>
>>> On Jan 18, 2019, at 23:50, Benjamin Tissoires
>>> wrote:
>>>
>>> Hi Kai-Heng,
>>>
>>> On Mon, Jan 7, 2019 at 8:24 AM Kai-Heng Feng
>>> wrote:
While using Elan touchpads, the message floods:
[ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete
report (14/65535)
Though the message flood is annoying, the device it self works without
any issue. I suspect that the device in question takes too much time to
pull the IRQ back to high after I2C host has done reading its data.
Since the host receives all useful data, let's ignore the input report
when there's no data.
Signed-off-by: Kai-Heng Feng
>>>
>>> Thanks for the v3.
>>>
>>> This patch has just been brought to my attention, and I have one
>>> question before applying it:
>>> are those messages (without your patch) occurring all the time, or
>>> just after resume?
>>
>> All the time.
>>
>> The touchpad works fine though. The entire report is 0xff, so it can be
>> safely ignored.
>
> Couple of things:
> - I have forgotten to tell you that I have applied this patch in
> for-5.1/i2c-hid, it will be pushed to linux-next today.
Thanks!
> - Dell told me that a BIOS fix was solving the issue. We can still
> quarry the patch, but there should not be a strong need for it when
> users upgrade their BIOS.
Multiple platforms are affected by this issue [1], so the patch can still be
really helpful.
[1] https://bugs.launchpad.net/bugs/1784152
Kai-Heng
>
> Cheers,
> Benjamin
>
>>
>>>
>>> I wonder if the pm_suspend delay we talked about in the other thread
>>> would fix that issue in a cleaner way.
>>
>> I’ve replied in another thread, unfortunately it can’t.
>>
>> We can introduce msleep() between each commands though, but I don’t
>> think it’s good either.
>>
>> Kai-Heng
>>
>>>
>>> Cheers,
>>> Benjamin
>>>
---
v3:
Fix compiler error/warnings.
v2:
Use dev_warn_once() to warn the user about the situation.
drivers/hid/i2c-hid/i2c-hid-core.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
b/drivers/hid/i2c-hid/i2c-hid-core.c
index 8555ce7e737b..2f940c1de616 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -50,6 +50,7 @@
#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
#define I2C_HID_QUIRK_NO_RUNTIME_PMBIT(2)
#define I2C_HID_QUIRK_DELAY_AFTER_SLEEPBIT(3)
+#define I2C_HID_QUIRK_BOGUS_IRQBIT(4)
/* flags */
#define I2C_HID_STARTED0
@@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
{ USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
I2C_HID_QUIRK_NO_RUNTIME_PM },
+ { USB_VENDOR_ID_ELAN, HID_ANY_ID,
+I2C_HID_QUIRK_BOGUS_IRQ },
{ 0, 0 }
};
@@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
return;
}
+ if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0x) {
+ dev_warn_once(>client->dev, "%s: IRQ triggered but "
+ "there's no data\n", __func__);
+ return;
+ }
+
if ((ret_size > size) || (ret_size < 2)) {
dev_err(>client->dev, "%s: incomplete report
(%d/%d)\n",
__func__, size, ret_size);
--
2.17.1
Re: [PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
On Mon, Jan 21, 2019 at 4:30 AM Kai-Heng Feng
wrote:
>
>
>
> > On Jan 18, 2019, at 23:50, Benjamin Tissoires
> > wrote:
> >
> > Hi Kai-Heng,
> >
> > On Mon, Jan 7, 2019 at 8:24 AM Kai-Heng Feng
> > wrote:
> >>
> >> While using Elan touchpads, the message floods:
> >> [ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete
> >> report (14/65535)
> >>
> >> Though the message flood is annoying, the device it self works without
> >> any issue. I suspect that the device in question takes too much time to
> >> pull the IRQ back to high after I2C host has done reading its data.
> >>
> >> Since the host receives all useful data, let's ignore the input report
> >> when there's no data.
> >>
> >> Signed-off-by: Kai-Heng Feng
> >
> > Thanks for the v3.
> >
> > This patch has just been brought to my attention, and I have one
> > question before applying it:
> > are those messages (without your patch) occurring all the time, or
> > just after resume?
>
> All the time.
>
> The touchpad works fine though. The entire report is 0xff, so it can be
> safely ignored.
Couple of things:
- I have forgotten to tell you that I have applied this patch in
for-5.1/i2c-hid, it will be pushed to linux-next today.
- Dell told me that a BIOS fix was solving the issue. We can still
quarry the patch, but there should not be a strong need for it when
users upgrade their BIOS.
Cheers,
Benjamin
>
> >
> > I wonder if the pm_suspend delay we talked about in the other thread
> > would fix that issue in a cleaner way.
>
> I’ve replied in another thread, unfortunately it can’t.
>
> We can introduce msleep() between each commands though, but I don’t
> think it’s good either.
>
> Kai-Heng
>
> >
> > Cheers,
> > Benjamin
> >
> >> ---
> >> v3:
> >> Fix compiler error/warnings.
> >>
> >> v2:
> >> Use dev_warn_once() to warn the user about the situation.
> >>
> >> drivers/hid/i2c-hid/i2c-hid-core.c | 9 +
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
> >> b/drivers/hid/i2c-hid/i2c-hid-core.c
> >> index 8555ce7e737b..2f940c1de616 100644
> >> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> >> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> >> @@ -50,6 +50,7 @@
> >> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> >> #define I2C_HID_QUIRK_NO_RUNTIME_PMBIT(2)
> >> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEPBIT(3)
> >> +#define I2C_HID_QUIRK_BOGUS_IRQBIT(4)
> >>
> >> /* flags */
> >> #define I2C_HID_STARTED0
> >> @@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
> >>I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
> >>{ USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
> >>I2C_HID_QUIRK_NO_RUNTIME_PM },
> >> + { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> >> +I2C_HID_QUIRK_BOGUS_IRQ },
> >>{ 0, 0 }
> >> };
> >>
> >> @@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
> >>return;
> >>}
> >>
> >> + if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0x) {
> >> + dev_warn_once(>client->dev, "%s: IRQ triggered but "
> >> + "there's no data\n", __func__);
> >> + return;
> >> + }
> >> +
> >>if ((ret_size > size) || (ret_size < 2)) {
> >>dev_err(>client->dev, "%s: incomplete report
> >> (%d/%d)\n",
> >>__func__, size, ret_size);
> >> --
> >> 2.17.1
> >>
>
Re: [PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
> On Jan 18, 2019, at 23:50, Benjamin Tissoires
> wrote:
>
> Hi Kai-Heng,
>
> On Mon, Jan 7, 2019 at 8:24 AM Kai-Heng Feng
> wrote:
>>
>> While using Elan touchpads, the message floods:
>> [ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete report
>> (14/65535)
>>
>> Though the message flood is annoying, the device it self works without
>> any issue. I suspect that the device in question takes too much time to
>> pull the IRQ back to high after I2C host has done reading its data.
>>
>> Since the host receives all useful data, let's ignore the input report
>> when there's no data.
>>
>> Signed-off-by: Kai-Heng Feng
>
> Thanks for the v3.
>
> This patch has just been brought to my attention, and I have one
> question before applying it:
> are those messages (without your patch) occurring all the time, or
> just after resume?
All the time.
The touchpad works fine though. The entire report is 0xff, so it can be
safely ignored.
>
> I wonder if the pm_suspend delay we talked about in the other thread
> would fix that issue in a cleaner way.
I’ve replied in another thread, unfortunately it can’t.
We can introduce msleep() between each commands though, but I don’t
think it’s good either.
Kai-Heng
>
> Cheers,
> Benjamin
>
>> ---
>> v3:
>> Fix compiler error/warnings.
>>
>> v2:
>> Use dev_warn_once() to warn the user about the situation.
>>
>> drivers/hid/i2c-hid/i2c-hid-core.c | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
>> b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index 8555ce7e737b..2f940c1de616 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -50,6 +50,7 @@
>> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
>> #define I2C_HID_QUIRK_NO_RUNTIME_PMBIT(2)
>> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEPBIT(3)
>> +#define I2C_HID_QUIRK_BOGUS_IRQBIT(4)
>>
>> /* flags */
>> #define I2C_HID_STARTED0
>> @@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
>>I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
>>{ USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
>>I2C_HID_QUIRK_NO_RUNTIME_PM },
>> + { USB_VENDOR_ID_ELAN, HID_ANY_ID,
>> +I2C_HID_QUIRK_BOGUS_IRQ },
>>{ 0, 0 }
>> };
>>
>> @@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
>>return;
>>}
>>
>> + if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0x) {
>> + dev_warn_once(>client->dev, "%s: IRQ triggered but "
>> + "there's no data\n", __func__);
>> + return;
>> + }
>> +
>>if ((ret_size > size) || (ret_size < 2)) {
>>dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
>>__func__, size, ret_size);
>> --
>> 2.17.1
>>
Re: [PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
Hi Kai-Heng,
On Mon, Jan 7, 2019 at 8:24 AM Kai-Heng Feng
wrote:
>
> While using Elan touchpads, the message floods:
> [ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete report
> (14/65535)
>
> Though the message flood is annoying, the device it self works without
> any issue. I suspect that the device in question takes too much time to
> pull the IRQ back to high after I2C host has done reading its data.
>
> Since the host receives all useful data, let's ignore the input report
> when there's no data.
>
> Signed-off-by: Kai-Heng Feng
Thanks for the v3.
This patch has just been brought to my attention, and I have one
question before applying it:
are those messages (without your patch) occurring all the time, or
just after resume?
I wonder if the pm_suspend delay we talked about in the other thread
would fix that issue in a cleaner way.
Cheers,
Benjamin
> ---
> v3:
> Fix compiler error/warnings.
>
> v2:
> Use dev_warn_once() to warn the user about the situation.
>
> drivers/hid/i2c-hid/i2c-hid-core.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
> b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 8555ce7e737b..2f940c1de616 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -50,6 +50,7 @@
> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> #define I2C_HID_QUIRK_NO_RUNTIME_PMBIT(2)
> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEPBIT(3)
> +#define I2C_HID_QUIRK_BOGUS_IRQBIT(4)
>
> /* flags */
> #define I2C_HID_STARTED0
> @@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
> { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> +I2C_HID_QUIRK_BOGUS_IRQ },
> { 0, 0 }
> };
>
> @@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
> return;
> }
>
> + if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0x) {
> + dev_warn_once(>client->dev, "%s: IRQ triggered but "
> + "there's no data\n", __func__);
> + return;
> + }
> +
> if ((ret_size > size) || (ret_size < 2)) {
> dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
> __func__, size, ret_size);
> --
> 2.17.1
>
[PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
While using Elan touchpads, the message floods:
[ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete report
(14/65535)
Though the message flood is annoying, the device it self works without
any issue. I suspect that the device in question takes too much time to
pull the IRQ back to high after I2C host has done reading its data.
Since the host receives all useful data, let's ignore the input report
when there's no data.
Signed-off-by: Kai-Heng Feng
---
v3:
Fix compiler error/warnings.
v2:
Use dev_warn_once() to warn the user about the situation.
drivers/hid/i2c-hid/i2c-hid-core.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
b/drivers/hid/i2c-hid/i2c-hid-core.c
index 8555ce7e737b..2f940c1de616 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -50,6 +50,7 @@
#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
#define I2C_HID_QUIRK_NO_RUNTIME_PMBIT(2)
#define I2C_HID_QUIRK_DELAY_AFTER_SLEEPBIT(3)
+#define I2C_HID_QUIRK_BOGUS_IRQBIT(4)
/* flags */
#define I2C_HID_STARTED0
@@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
{ USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
I2C_HID_QUIRK_NO_RUNTIME_PM },
+ { USB_VENDOR_ID_ELAN, HID_ANY_ID,
+I2C_HID_QUIRK_BOGUS_IRQ },
{ 0, 0 }
};
@@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
return;
}
+ if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0x) {
+ dev_warn_once(>client->dev, "%s: IRQ triggered but "
+ "there's no data\n", __func__);
+ return;
+ }
+
if ((ret_size > size) || (ret_size < 2)) {
dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
__func__, size, ret_size);
--
2.17.1
