2020-09-04 09:14:57

by penghao

[permalink] [raw]
Subject: [PATCH v3] HID: quirks: Add USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for BYD zhaoxin notebook

Add a USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for the BYD zhaoxin notebook.
This notebook come with usb touchpad. And we would like to disable touchpad
wakeup on this notebook by default.

Signed-off-by: Penghao <[email protected]>
---

Changes since v2:
- Add changes

Changes since v1:
- Add the entries sorted by vendor ID and product ID.

drivers/usb/core/quirks.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 7c1198f80c23..fffe1f7f1098 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -393,6 +393,10 @@ static const struct usb_device_id usb_quirk_list[] = {
/* Generic RTL8153 based ethernet adapters */
{ USB_DEVICE(0x0bda, 0x8153), .driver_info = USB_QUIRK_NO_LPM },

+ /* SONiX USB DEVICE Touchpad */
+ { USB_DEVICE(0x0c45, 0x7056), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* Action Semiconductor flash disk */
{ USB_DEVICE(0x10d6, 0x2200), .driver_info =
USB_QUIRK_STRING_FETCH_255 },
--
2.11.0




2020-09-04 10:07:05

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v3] HID: quirks: Add USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for BYD zhaoxin notebook

Hi,

On 9/4/20 11:13 AM, Penghao wrote:
> Add a USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for the BYD zhaoxin notebook.
> This notebook come with usb touchpad. And we would like to disable touchpad
> wakeup on this notebook by default.

You are still not explaining why this is necessary ?

And you did not answer my question if this touchpad is using hid-multitouch
either ?

So NACK (rejection) from me until both questions are answered. The commi
message for a quirk should always explain why a quirk is necessary and
"we would like to disable touchpad wakeup on this notebook by default"
does not explain why you want to do that.

Regards,

Hans



>
> Signed-off-by: Penghao <[email protected]>
> ---
>
> Changes since v2:
> - Add changes
>
> Changes since v1:
> - Add the entries sorted by vendor ID and product ID.
>
> drivers/usb/core/quirks.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 7c1198f80c23..fffe1f7f1098 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -393,6 +393,10 @@ static const struct usb_device_id usb_quirk_list[] = {
> /* Generic RTL8153 based ethernet adapters */
> { USB_DEVICE(0x0bda, 0x8153), .driver_info = USB_QUIRK_NO_LPM },
>
> + /* SONiX USB DEVICE Touchpad */
> + { USB_DEVICE(0x0c45, 0x7056), .driver_info =
> + USB_QUIRK_IGNORE_REMOTE_WAKEUP },
> +
> /* Action Semiconductor flash disk */
> { USB_DEVICE(0x10d6, 0x2200), .driver_info =
> USB_QUIRK_STRING_FETCH_255 },
>

2020-09-04 10:42:46

by penghao

[permalink] [raw]
Subject: Re: [PATCH v3] HID: quirks: Add USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for BYD zhaoxin notebook

Hi, Hans

Thanks for the review. And sorry for I'm not really familiar with
mailing list, so didn't reply early.

Actually the reason why I submit this patch, is the vendor ask we do
this, they do not want enable touchpad wakeup.

And yes, this device use hid-multitouch.


在 2020/9/4 下午6:05, Hans de Goede 写道:
> Hi,
>
> On 9/4/20 11:13 AM, Penghao wrote:
>> Add a USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for the BYD zhaoxin notebook.
>> This notebook come with usb touchpad. And we would like to disable
>> touchpad
>> wakeup on this notebook by default.
>
> You are still not explaining why this is necessary ?
>
> And you did not answer my question if this touchpad is using hid-multitouch
> either ?
>
> So NACK (rejection) from me until both questions are answered. The commi
> message for a quirk should always explain why a quirk is necessary and
> "we would like to disable touchpad wakeup on this notebook by default"
> does not explain why you want to do that.
>
> Regards,
>
> Hans
>
>
>
>>
>> Signed-off-by: Penghao <[email protected]>
>> ---
>>
>> Changes since v2:
>>   - Add changes
>>
>> Changes since v1:
>>   - Add the entries sorted by vendor ID and product ID.
>>
>>   drivers/usb/core/quirks.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
>> index 7c1198f80c23..fffe1f7f1098 100644
>> --- a/drivers/usb/core/quirks.c
>> +++ b/drivers/usb/core/quirks.c
>> @@ -393,6 +393,10 @@ static const struct usb_device_id
>> usb_quirk_list[] = {
>>       /* Generic RTL8153 based ethernet adapters */
>>       { USB_DEVICE(0x0bda, 0x8153), .driver_info = USB_QUIRK_NO_LPM },
>>   +    /* SONiX USB DEVICE Touchpad */
>> +    { USB_DEVICE(0x0c45, 0x7056), .driver_info =
>> +            USB_QUIRK_IGNORE_REMOTE_WAKEUP },
>> +
>>       /* Action Semiconductor flash disk */
>>       { USB_DEVICE(0x10d6, 0x2200), .driver_info =
>>               USB_QUIRK_STRING_FETCH_255 },
>>
>
>
>
>
>


2020-09-04 10:45:38

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3] HID: quirks: Add USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for BYD zhaoxin notebook

On Fri, Sep 04, 2020 at 06:38:21PM +0800, PengHao wrote:
> Hi, Hans
>
> Thanks for the review. And sorry for I'm not really familiar with
> mailing list, so didn't reply early.
>
> Actually the reason why I submit this patch, is the vendor ask we do
> this, they do not want enable touchpad wakeup.

That says _what_ they want to do, but we need to know _why_ this is the
case.

What is wrong with the existing touchpad wakeup functionality that is
broken on this device? Why does this quirk have to be added, what goes
wrong if it is not added?

Please put that information in the changelog text.

thanks,

greg k-h

2020-09-04 11:31:25

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v3] HID: quirks: Add USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for BYD zhaoxin notebook

Hi,

On 9/4/20 12:38 PM, PengHao wrote:
> Hi, Hans
>
> Thanks for the review. And sorry for I'm not really familiar with
> mailing list, so didn't reply early.
>
> Actually the reason why I submit this patch, is the vendor ask we do
> this, they do not want enable touchpad wakeup.
>
> And yes, this device use hid-multitouch.

Right, I already suspected that. I guess they might be seeing
a similar issue as what I am seeing on the Asus T101HA.

Here when closed the lid comes to close to the touchpad and
it starts sending spurious events, resulting in immediate
wakeup from suspend.

I have a workaround patch for that issue here:

https://github.com/jwrdegoede/linux-sunxi/commit/cb3a4c23b94bc3646953d00cea183d4e8fddeea0

As the commit message explains that patch is not yet
ready for upstream. It should probably be modified to
only disable the touchpad this way when remote-wakeup is
disabled (at which point it mainly becomes a power-saving
feature).

So if the BYD zhaoxin notebook has the same issue, then
adding a quirk to disable remote-wakeup is likely still the
right thing to do. But first I would like to understand the
problem better.

Regards,

Hans



>
>
> 在 2020/9/4 下午6:05, Hans de Goede 写道:
>> Hi,
>>
>> On 9/4/20 11:13 AM, Penghao wrote:
>>> Add a USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for the BYD zhaoxin notebook.
>>> This notebook come with usb touchpad. And we would like to disable
>>> touchpad
>>> wakeup on this notebook by default.
>>
>> You are still not explaining why this is necessary ?
>>
>> And you did not answer my question if this touchpad is using hid-multitouch
>> either ?
>>
>> So NACK (rejection) from me until both questions are answered. The commi
>> message for a quirk should always explain why a quirk is necessary and
>> "we would like to disable touchpad wakeup on this notebook by default"
>> does not explain why you want to do that.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>>
>>> Signed-off-by: Penghao <[email protected]>
>>> ---
>>>
>>> Changes since v2:
>>>   - Add changes
>>>
>>> Changes since v1:
>>>   - Add the entries sorted by vendor ID and product ID.
>>>
>>>   drivers/usb/core/quirks.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
>>> index 7c1198f80c23..fffe1f7f1098 100644
>>> --- a/drivers/usb/core/quirks.c
>>> +++ b/drivers/usb/core/quirks.c
>>> @@ -393,6 +393,10 @@ static const struct usb_device_id
>>> usb_quirk_list[] = {
>>>       /* Generic RTL8153 based ethernet adapters */
>>>       { USB_DEVICE(0x0bda, 0x8153), .driver_info = USB_QUIRK_NO_LPM },
>>>   +    /* SONiX USB DEVICE Touchpad */
>>> +    { USB_DEVICE(0x0c45, 0x7056), .driver_info =
>>> +            USB_QUIRK_IGNORE_REMOTE_WAKEUP },
>>> +
>>>       /* Action Semiconductor flash disk */
>>>       { USB_DEVICE(0x10d6, 0x2200), .driver_info =
>>>               USB_QUIRK_STRING_FETCH_255 },
>>>
>>
>>
>>
>>
>>
>
>

2020-09-04 13:56:07

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3] HID: quirks: Add USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for BYD zhaoxin notebook

On Fri, Sep 04, 2020 at 05:13:22PM +0800, Penghao wrote:
> Add a USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk for the BYD zhaoxin notebook.
> This notebook come with usb touchpad. And we would like to disable touchpad
> wakeup on this notebook by default.
>
> Signed-off-by: Penghao <[email protected]>
> ---
>
> Changes since v2:
> - Add changes
>
> Changes since v1:
> - Add the entries sorted by vendor ID and product ID.
>
> drivers/usb/core/quirks.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 7c1198f80c23..fffe1f7f1098 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -393,6 +393,10 @@ static const struct usb_device_id usb_quirk_list[] = {
> /* Generic RTL8153 based ethernet adapters */
> { USB_DEVICE(0x0bda, 0x8153), .driver_info = USB_QUIRK_NO_LPM },
>
> + /* SONiX USB DEVICE Touchpad */
> + { USB_DEVICE(0x0c45, 0x7056), .driver_info =
> + USB_QUIRK_IGNORE_REMOTE_WAKEUP },
> +

Why is the subject line "HID:"? This is a USB core quirk, not a
hid-specific one.

thanks,

greg k-h