From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Kenny Levinsen <kl@kl.wtf>
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>
Subject: Re: [PATCH v3 1/3] HID: i2c-hid: Rely on HID descriptor fetch to probe
Date: Tue, 30 Apr 2024 14:41:31 -0700 [thread overview]
Message-ID: <ZjFli4zOalXkDWx_@google.com> (raw)
In-Reply-To: <5aa9f745-7f6a-4873-90ba-79c55335905c@kl.wtf>
On Sat, Apr 27, 2024 at 03:20:07PM +0200, Kenny Levinsen wrote:
> On 4/27/24 5:21 AM, Dmitry Torokhov wrote:
> > I really think we should differentiate the cases "we do not know if
> > there is a device" vs "we do known that there is a device and we have
> > strong expectation of what that device is, and we do not expect
> > communication to fail".
>
> My reasoning was that there is no difference between looking for address
> acknowledge on a probe read vs. a real command. Unfortunately, I ran into
> some issues with error code consistency that Doug highlighted...
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.
Once we decided that the probe should continue we commit to it and all
subsequent errors from the device should be treated as unexpected errors
worthy of logging. On resume we do not expect hardware configuration to
change, so again when resuming we also treat errors as errors and log
them and fail resume.
>
> Considering that the smbus probe bails on *any* error, it's really only
> ACK'd address + NACK'd register that remains, and I thought it maybe
> wouldn't be too harmful to just always have a debug log as suggested.
> However, I would still like *more* good errors by being specific about the
> error condition, and I plan to send some patches to get the number of
> drivers sending ENXIO up so we can comfortably rely on it in a future
> i2c-hid patch.
>
> If you don't think it's acceptable to leave this as a pure debug print for
> now, I'll send a patch with just a minor clean-up and Łukasz' delays - then
> I'll try this again later when the driver situation has improved. I've been
> rapid-firing revisions, so I'll await an ACK this time... :)
>
> ---
>
> For some context, I started looking at the i2c-hid driver after a recent
> regression around assumed Windows behavior, and found that the actual
> behavior differed a lot from our assumptions. Windows gets the job done
> notably quicker, with fewer messages and with shorter albeit differently
> placed delays.
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.
Thanks.
--
Dmitry
next prev parent reply other threads:[~2024-04-30 21:41 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 22:47 [PATCH v3 0/3] HID: i2c-hid: Probe and wake device with HID descriptor fetch Kenny Levinsen
2024-04-26 22:47 ` [PATCH v3 1/3] HID: i2c-hid: Rely on HID descriptor fetch to probe Kenny Levinsen
2024-04-27 3:21 ` Dmitry Torokhov
2024-04-27 13:20 ` Kenny Levinsen
2024-04-30 21:41 ` Dmitry Torokhov [this message]
2024-05-01 5:24 ` Kenny Levinsen
2024-05-01 19:09 ` Dmitry Torokhov
2024-05-01 23:09 ` Kenny Levinsen
2024-04-26 22:47 ` [PATCH v3 2/3] HID: i2c-hid: Retry HID descriptor read to wake up STM devices Kenny Levinsen
2024-04-26 22:47 ` [PATCH v3 3/3] HID: i2c-hid: Align i2c_hid_set_power() retry with HID descriptor read Kenny Levinsen
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=ZjFli4zOalXkDWx_@google.com \
--to=dmitry.torokhov@gmail.com \
--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=kl@kl.wtf \
--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).