Received-SPF: pass (google.com: domain of linux-kernel+bounces-159081-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <836b5226-a6e9-49af-981b-365aa5df1fcb@kl.wtf>
Date: Thu, 25 Apr 2024 21:36:42 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v2 1/3] HID: i2c-hid: Rely on HID descriptor fetch to
 probe
To: Doug Anderson <dianders@chromium.org>
Cc: Jiri Kosina <jikos@kernel.org>, Dmitry Torokhov <dtor@chromium.org>,
 Benjamin Tissoires <benjamin.tissoires@redhat.com>,
 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: <20240423122518.34811-1-kl@kl.wtf>
 <20240423122518.34811-2-kl@kl.wtf>
 <CAD=FV=Xr6NsW085Sc+NhVmGDOn-zCCQ65CMNce_DsHxtXUgm9w@mail.gmail.com>
Content-Language: en-US
From: Kenny Levinsen <kl@kl.wtf>
In-Reply-To: <CAD=FV=Xr6NsW085Sc+NhVmGDOn-zCCQ65CMNce_DsHxtXUgm9w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 4/24/24 7:00 PM, Doug Anderson wrote:
> I worry a little bit about keying just off of -EREMOTEIO. If I'm
> skimming the code properly it's up to the different i2c bus controller
> to decide which error code to return here. Looking at, for instance,
> "i2c-qcom-geni.c", I see:
>
> [NACK] = {-ENXIO, "NACK: slv unresponsive, check its power/reset-ln"},


Hmm, good point. I decided to go through the drivers and check their 
behavior on NACK, and based on my quick glance I found (insert accuracy 
disclaimer):

- 52 drivers emitting ENXIO
- 14 drivers emitting EREMOTEIO
- 11 driver emitting EIO
- 5 drivers emitting ETIMEDOUT
- 1 driver emitting EAGAIN
- 1 driver emitting I2C_ERR_BERR (???)

So just EREMOTEIO is definitely not good enough. Looking at the drivers, 
it seems like the majority of drivers emitting generic error codes could 
just as well emit ENXIO on NACK. Room for improvement.

> Maybe we should just use dev_dbg() in all cases here when we fail to
> fetch the descriptor? ...otherwise I think some boards will start
> getting a noisy error message.

I'm okay with that. I don't like hiding a useful error message, but the 
smbus probe would also have hidden bus errors.

I'll send a v3 with just dev_dbg, then if I (or someone else) end up 
aligning more i2c drivers on their NACK error we can go to the stricter 
check and incentivize the drivers to give meaningful error values...