[v2,0/3] HID: i2c-hid: Probe and wake device with HID descriptor fetch

Message

Kenny Levinsen April 23, 2024, 12:07 p.m. UTC

This revises my previous patch[0] to add the sleep STM chips seem to
require as per discussion on the original patch from Lukasz and
Radoslaw[1]. I had initially tried without as it had not previously been
needed in the similar logic in our resume path, but it would appear that
this was simply luck as the affected device was woken up in that case by
"noise" from other sources.

To reiterate, the idea is to add the retry that Lukasz and Radoslaw
discovered was necessary, but do away with the dummy smbus probe and
instead just let HID descriptor fetch retry as needed, aligning more
with the existing retry logic used after resume while saving some noise
on the bus and speeding up initialization a tiny bit.

I added Co-developed-by tags, I hope that's appropriate. We should await
an ACK from Lukasz on it fixing their hardware quirk.

[0]: https://lore.kernel.org/all/[e-mail]/
[1]: https://lore.kernel.org/all/[e-mail]/

Comments

Łukasz Majczak April 24, 2024, 10:34 a.m. UTC | #1

On Tue, Apr 23, 2024 at 2:26 PM Kenny Levinsen <[e-mail]> wrote:
>
> This revises my previous patch[0] to add the sleep STM chips seem to
> require as per discussion on the original patch from Lukasz and
> Radoslaw[1]. I had initially tried without as it had not previously been
> needed in the similar logic in our resume path, but it would appear that
> this was simply luck as the affected device was woken up in that case by
> "noise" from other sources.
>
> To reiterate, the idea is to add the retry that Lukasz and Radoslaw
> discovered was necessary, but do away with the dummy smbus probe and
> instead just let HID descriptor fetch retry as needed, aligning more
> with the existing retry logic used after resume while saving some noise
> on the bus and speeding up initialization a tiny bit.
>
> I added Co-developed-by tags, I hope that's appropriate. We should await
> an ACK from Lukasz on it fixing their hardware quirk.
>
> [0]: https://lore.kernel.org/all/[e-mail]/
> [1]: https://lore.kernel.org/all/[e-mail]/
>
Hi Kenny,

Your solution works as it should - I have tested it on my Eve with
enabled debugs and
the retries works as expected with power-on, reboot and suspend/resume paths.

I have also disabled cros_ec_i2c driver to be 100% sure it doesn't do
any i2c transactions on the bus
and again the touchpad initialized successfully (with a retry) on all paths.

So you can add:
Tested-by: Lukasz Majczak <[e-mail]>
Reviewed-by: Lukasz Majczak <[e-mail]>

Thank you Kenny for your work :)

Best regards,
Lukasz