linux-kernel - Re: [PATCH v2] HID: i2c-hid: wait for i2c touchpad deep-sleep to power-up transition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM4=RnJAe1wzi_+Nif0wiSeqC5GR6Aavz97mY7tm=hFu3mijhg@mail.gmail.com>
Date: Mon, 15 Apr 2024 14:10:41 +0200
From: Radoslaw Biernacki <biernacki@...gle.com>
To: Kenny Levinsen <kl@...wtf>
Cc: lma@...omium.org, benjamin.tissoires@...hat.com, dianders@...omium.org, 
	dtor@...omium.org, hdegoede@...hat.com, jikos@...nel.org, 
	johan+linaro@...nel.org, johan@...nel.org, kai.heng.feng@...onical.com, 
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, mripard@...nel.org, 
	rad@...omium.org
Subject: Re: [PATCH v2] HID: i2c-hid: wait for i2c touchpad deep-sleep to
 power-up transition

Hi Kenny,

> If the device can enter deep-sleep arbitrarily, then we presumably also
> have problems in i2c_hid_output_raw_report() and
> i2c_hid_get_raw_report() which could happen after the device has gone to
> sleep from inactivity. These places would also need EREMOTEIO retry logic.

It does not enter deep-sleep arbitrarily and therefore it is not a problem with
other communication patterns.
The design which was chosen back in the day, to save the battery power
is event based
instead of level based (some HW line). Therefore to avoid power leak
we need to request
low power state (to prevent power leak in case the kernel will not
boot up soon).

Basically we need to take out the device from deep state logic by message,
without knowing if the device is on the bus or it is on the bus but
not responding.

>
> All these places should have the same sleeping behavior as they are
> working around the same device glitch. I imagine the client ACK timeout
> is longer than your required 400µs, in which case you don't need any
> sleep on retry at all, as is the case in the current i2c_hid_set_power()
> implementation.
>
> However, as we litter retry-code all over the place, Johan's suggestion
> about doing this in the I2C driver does become a bit more relevant...
>
> Best regards,
> Kenny Levinsen
>