Received-SPF: pass (google.com: domain of linux-kernel+bounces-155241-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <25f3ac4f-0c0d-4183-93f1-e7069579420b@kl.wtf>
Date: Tue, 23 Apr 2024 15:01:50 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v2] HID: i2c-hid: wait for i2c touchpad deep-sleep to
 power-up transition
To: =?UTF-8?Q?=C5=81ukasz_Majczak?= <lma@chromium.org>
Cc: Johan Hovold <johan@kernel.org>, benjamin.tissoires@redhat.com,
 dianders@chromium.org, dtor@chromium.org, hdegoede@redhat.com,
 jikos@kernel.org, johan+linaro@kernel.org, kai.heng.feng@canonical.com,
 linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
 mripard@kernel.org, rad@chromium.org
References: <CAE5UKNqufWZfKLAXLcpBYKQpJEVt6jPD4Xtr=Nesh34VkNOETg@mail.gmail.com>
 <81e1b870-37f9-4ef2-9a3d-87015f32546b@kl.wtf>
 <Zh0qKeI-YPDE-NVT@hovoldconsulting.com>
 <535f3756-80d9-4599-bf73-a8785d18c399@kl.wtf>
 <CAE5UKNqPA4SnnXyaB7Hwk0kcKMMQ_DUuxogDphnnvSGP8g1nAQ@mail.gmail.com>
Content-Language: en-US
From: Kenny Levinsen <kl@kl.wtf>
In-Reply-To: <CAE5UKNqPA4SnnXyaB7Hwk0kcKMMQ_DUuxogDphnnvSGP8g1nAQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 4/23/24 1:32 PM, Łukasz Majczak wrote:
> Unfortunately, your fix doesn't help in our case as there is no sleep
> before the second call to
> i2c_hid_fetch_hid_descriptor().

Yeah, I checked with a logic analyzer and only see ~50µs delay from the 
I2C timeout, and 50 is according to my quick math less than the 400 you 
mention is the requirement. That means that the current resume path also 
lacked a sleep, as it tried power commands in immediate succession.

I have made a v2 with your sleeps added, and added you as Co-developed-by.

Link: https://lore.kernel.org/all/20240423122518.34811-1-kl@kl.wtf/

> Saying more, this STM exposes two i2c addresses (connected physically
> to the same bus)
> one is the HID interface and the other one gives an access to the base
> firmware and is
> served by cros_ec_i2c driver and actually thanks to this driver,
> touchpad is woken up because
> In the resume path cros_ec_i2c "starts talking" through the i2c bus
> thus generating a wakeup
> interrupt.

Ah, that explains why you did not find an issue with the resume path. In 
the patch-series I sent, the boot (hid descriptor fetch) and resume 
(power on) wake-up paths are fully aligned so neither have to rely on 
such "adjacent drivers" waking up the i2c-hid device.