From: Kenny Levinsen <kl@kl.wtf>
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>
Subject: Re: [PATCH v2 1/3] HID: i2c-hid: Rely on HID descriptor fetch to probe
Date: Thu, 25 Apr 2024 21:36:42 +0200 [thread overview]
Message-ID: <836b5226-a6e9-49af-981b-365aa5df1fcb@kl.wtf> (raw)
In-Reply-To: <CAD=FV=Xr6NsW085Sc+NhVmGDOn-zCCQ65CMNce_DsHxtXUgm9w@mail.gmail.com>
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...
next prev parent reply other threads:[~2024-04-25 19:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-23 12:07 [PATCH v2 0/3] HID: i2c-hid: Probe and wake device with HID descriptor fetch Kenny Levinsen
2024-04-23 12:07 ` [PATCH v2 1/3] HID: i2c-hid: Rely on HID descriptor fetch to probe Kenny Levinsen
2024-04-24 17:00 ` Doug Anderson
2024-04-25 19:36 ` Kenny Levinsen [this message]
2024-04-23 12:07 ` [PATCH v2 2/3] HID: i2c-hid: Retry HID descriptor read to wake up STM devices Kenny Levinsen
2024-04-23 12:07 ` [PATCH v2 3/3] HID: i2c-hid: Align i2c_hid_set_power() retry with HID descriptor read Kenny Levinsen
2024-04-24 10:34 ` [PATCH v2 0/3] HID: i2c-hid: Probe and wake device with HID descriptor fetch Łukasz Majczak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=836b5226-a6e9-49af-981b-365aa5df1fcb@kl.wtf \
--to=kl@kl.wtf \
--cc=benjamin.tissoires@redhat.com \
--cc=dianders@chromium.org \
--cc=dtor@chromium.org \
--cc=hdegoede@redhat.com \
--cc=jikos@kernel.org \
--cc=johan+linaro@kernel.org \
--cc=kai.heng.feng@canonical.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lma@chromium.org \
--cc=mripard@kernel.org \
--cc=rad@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).