Received-SPF: pass (google.com: domain of linux-kernel+bounces-164943-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <26070c7a-4005-4bb4-b4af-779bfc415dea@kl.wtf>
Date: Wed, 1 May 2024 07:24:08 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v3 1/3] HID: i2c-hid: Rely on HID descriptor fetch to
 probe
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <jikos@kernel.org>, Dmitry Torokhov <dtor@chromium.org>,
 Benjamin Tissoires <benjamin.tissoires@redhat.com>,
 Douglas Anderson <dianders@chromium.org>, Hans de Goede
 <hdegoede@redhat.com>, Maxime Ripard <mripard@kernel.org>,
 Kai-Heng Feng <kai.heng.feng@canonical.com>,
 Johan Hovold <johan+linaro@kernel.org>, linux-input@vger.kernel.org,
 linux-kernel@vger.kernel.org, Radoslaw Biernacki <rad@chromium.org>,
 Lukasz Majczak <lma@chromium.org>
References: <20240426225739.2166-1-kl@kl.wtf>
 <20240426225739.2166-2-kl@kl.wtf> <ZixvUNooESC02cJK@google.com>
 <5aa9f745-7f6a-4873-90ba-79c55335905c@kl.wtf> <ZjFli4zOalXkDWx_@google.com>
Content-Language: en-US
From: Kenny Levinsen <kl@kl.wtf>
In-Reply-To: <ZjFli4zOalXkDWx_@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 4/30/24 11:41 PM, Dmitry Torokhov wrote:
> I actually believe there is. On Chromebooks we may source components
> from several vendors and use them in our devices. The components
> are electrically compatible with each other, have exactly the same
> connector, and therefore interchangeable. Because of that at probe time
> we do not quite know if the device is there at given address, or not
> (i.e. the touchpad could be from a different vendor and listening on
> another address) and we need to make a quick determination whether we
> should continue with probe or not.

Maybe I should clarify what I meant: All I2C operations start with the 
master writing the slave address to the bus. When a slave reads its own 
address off the bus, it pulls the data line low to ACK. If no device is 
present on the bus with the specified address, the line stays high which 
is a NACK. This means that on the bus level, we have a clear error 
condition specifically for no device with the specified address being 
present on the bus.

Whether the operation used is a dummy read or our first actual write 
should not matter - if the address is not acknowledged, the device is 
not present (or able to talk I2C). The problem lies in whether this "no 
device present on bus" error is reported clearly to us: Some drivers use 
-ENXIO specifically for this, some use it also for NACKs on written 
data, some report it but use other return codes for it, etc.

Even if we stick to the smbus probe in the long run, if we get to the 
point where we can rely on the error codes from I2C drivers we would be 
able to correctly log and propagate other error classes like bus errors 
or I2C driver issues which are all currently silenced as "nothing at 
address" by the smbus probe.

> I am not sure we can fully unify what Windows does and what Linux does,
> mainly because our firmwares are different (I think Windows devices do a
> lot more device discovery in firmware, Chrome OS historically tried to
> limit amount of code in its firmware). We also need to make sure it
> works on non-ACPI systems/ARM.

Good point. My main focus is also quirky behaviors we have added to 
replicate Windows behavior, the smbus probe just stood out in my bus traces.

I already sent 
https://lore.kernel.org/all/20240429233924.6453-1-kl@kl.wtf/ which goes 
back to improving the bus probe.