Received-SPF: pass (google.com: domain of linux-kernel+bounces-145041-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <CAE5UKNqufWZfKLAXLcpBYKQpJEVt6jPD4Xtr=Nesh34VkNOETg@mail.gmail.com>
 <81e1b870-37f9-4ef2-9a3d-87015f32546b@kl.wtf>
In-Reply-To: <81e1b870-37f9-4ef2-9a3d-87015f32546b@kl.wtf>
From: Radoslaw Biernacki <biernacki@google.com>
Date: Mon, 15 Apr 2024 14:10:41 +0200
Message-ID: <CAM4=RnJAe1wzi_+Nif0wiSeqC5GR6Aavz97mY7tm=hFu3mijhg@mail.gmail.com>
Subject: Re: [PATCH v2] HID: i2c-hid: wait for i2c touchpad deep-sleep to
 power-up transition
To: Kenny Levinsen <kl@kl.wtf>
Cc: lma@chromium.org, benjamin.tissoires@redhat.com, dianders@chromium.org, 
	dtor@chromium.org, hdegoede@redhat.com, jikos@kernel.org, 
	johan+linaro@kernel.org, johan@kernel.org, kai.heng.feng@canonical.com, 
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, mripard@kernel.org, 
	rad@chromium.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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=C2=B5s, 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
>