From: Guenter Roeck <groeck@google.com>
To: Paul Menzel <pmenzel@molgen.mpg.de>
Cc: Dmitry Torokhov <dtor@chromium.org>,
Wolfram Sang <wsa@kernel.org>, Rob Herring <robh+dt@kernel.org>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Tim Wawrzynczak <twawrzynczak@chromium.org>,
coreboot@coreboot.org, Matt DeVillier <matt.devillier@gmail.com>,
Felix Singer <felixsinger@posteo.net>,
Benson Leung <bleung@chromium.org>,
Justin TerAvest <teravest@chromium.org>,
Guenter Roeck <groeck@chromium.org>,
linux-i2c@vger.kernel.org,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE"
<devicetree@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-acpi@vger.kernel.org, Sangwon Jee <jeesw@melfas.com>
Subject: Re: [PATCH] CHROMIUM: i2c: Add device property for probing
Date: Tue, 21 Dec 2021 12:35:47 -0800 [thread overview]
Message-ID: <CABXOdTfB4M8AcCOVERpQwddr_N09gpKF67FxRO32S4M9JUaYEQ@mail.gmail.com> (raw)
In-Reply-To: <8a7fad1b-b34d-88db-2f6b-462303fe03d9@molgen.mpg.de>
On Tue, Dec 21, 2021 at 11:42 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Guenter, dear Dmitry,
>
>
> Am 21.12.21 um 17:47 schrieb Guenter Roeck:
> > On Mon, Dec 20, 2021 at 1:49 PM Dmitry Torokhov <dtor@chromium.org> wrote:
>
> >> On Mon, Dec 20, 2021 at 1:07 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> >>>
> >>> From: Furquan Shaikh <furquan@google.com>
>
> >>> Google Chromebooks are often built with devices sourced from different
> >>> vendors. These need to be probed. To deal with this, the firmware – in
> >>> this case coreboot – tags such optional devices accordingly – I think
> >>> this is commit fbf2c79b (drivers/i2c/generic: Add config for marking
> >>> device as probed) – and Chromium OS’ Linux kernel has the patch at hand
> >>> applied to act accordingly. Right after the merge, Dmitry created a
> >>> revert, which was actively discussed for two days but wasn’t applied.
> >>> That means, millions of devices shipped with such a firmware and Linux
> >>> kernel. To support these devices with upstream Linux kernel, is there an
> >>> alternative to applying the patch to the Linux kernel, and to support
> >>> the shipped devices?
> >>
> >> *sigh* I should have pushed harder, but I see it managed to
> >> proliferate even into our newer kernels. Not having this patch should
> >> not cause any problems, it can only hurt, because the i2c core has no
> >> idea how to power up and reset the device properly. The only downside
> >> of not having this patch is that we may have devices in sysfs that are
> >> not connected to actual hardware. They do now cause any problems and
> >> is how we have been shipping ARM-based devices where we also dual- and
> >> triple-source components. However if we were to have a device that
> >> switches between several addresses (let's say device in bootloader
> >> mode uses 0x10 address and in normal mode 0x20) this "probing" may
> >> result in device not being detected at all.
>
> On google/sarien, the (upstream) Linux kernel sometimes detects the
> Melfas touchscreen and sometimes not, but in never works. When it’s
> detected, the errors below are still shown.
>
> ```
> $ grep i2c voidlinux-linux-5.13.19-messages.txt
> [ 9.392598] i2c i2c-7: 2/2 memory slots populated (from DMI)
> [ 9.393108] i2c i2c-7: Successfully instantiated SPD at 0x50
> [ 9.622151] input: MELFAS MIP4 Touchscreen as
> /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-8/i2c-MLFS0000:00/input/input6
> [ 9.657964] cr50_i2c i2c-GOOG0005:00: cr50 TPM 2.0 (i2c 0x50 irq 114
> id 0x28)
> [ 9.662309] elan_i2c i2c-ELAN0000:00: supply vcc not found, using
> dummy regulator
> [ 9.773244] elan_i2c i2c-ELAN0000:00: Elan Touchpad: Module ID:
> 0x00d6, Firmware: 0x0005, Sample: 0x0009, IAP: 0x0001
> [ 9.773349] input: Elan Touchpad as
> /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-9/i2c-ELAN0000:00/input/input7
> [ 10.820307] i2c_designware i2c_designware.0: controller timed out
> [ 10.820359] mip4_ts i2c-MLFS0000:00: mip4_i2c_xfer - i2c_transfer
> failed: -110 (-110)
> [ 11.844523] i2c_designware i2c_designware.0: controller timed out
> [ 11.844635] mip4_ts i2c-MLFS0000:00: mip4_i2c_xfer - i2c_transfer
> failed: -110 (-110)
> [ 12.868376] i2c_designware i2c_designware.0: controller timed out
> [ 12.868488] mip4_ts i2c-MLFS0000:00: mip4_i2c_xfer - i2c_transfer
> failed: -110 (-110)
> [ 12.868570] mip4_ts i2c-MLFS0000:00: Failed to read packet info: -110
> ```
>
> Is that related to the probing stuff?
>
Difficult to say without further testing. I can see two possible
problems: The device may sometimes not be seen because it is powered
off, and/or interrupt handling may not work properly. You could apply
the patch (commit 11cd1bd03f75 in chromeos-5.15) and see if it
improves the situation. I would also suggest applying commit
b4b55381e5cf ("CHROMIUM: Input: elants_i2c: Default to low level
interrupt for Chromebooks") from chromeos-4.19.
Guenter
> >> If we wanted to do this correctly, coreboot would have to implement
> >> full power and reset control and also add drivers for I2C controllers
> >> to be able to communicate with peripherals, and then adjust _STA
> >> methods to report "not present" when the device is indeed absent. And
> >> note that even in this case we would have issues with "morphing
> >> devices", so coreboot would also need to know how to reset device out
> >> of bootloader mode, and maybe flash firmware so device can work in
> >> normal mode.
>
> What do you mean by “bootloader mode”? coreboot also cannot flash
> anything. That’s up to the payload, and even there support for flashing
> is rare.
>
> Duncan wrote something about the ACPI _STA method idea, that ASL(?) and
> I2C do not go well together.
>
> >> However coreboot does (or did?) not want to add code to handle i2c
> >> controllers, and would like to push this knowledge to the kernel. And
> >> the kernel does know how to handle peripherals properly, but that
> >> knowledge lies in individual drivers, not i2c core.
>
> Excuse my ignorance, can you give an example driver? Does the Melfas
> touchscreen driver (`drivers/input/touchscreen/melfas_mip4.c`) support it?
>
> >> We should remove "linux,probed" from coreboot and not propagate to
> >> newer Chrome OS kernels, and keep it away from upstream.
> >
> > Revert from chromeos-5.15 is at
> > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3350347.
> > Everyone please feel free to comment there.
>
> Guenther, thank you for your quick response. Note, that neither Furquan,
> nor Aaron, nor Duncan work at Google anymore, so won’t comment.
> Hopefully, others from the Chromium OS/coreboot folks can chime in.
>
>
> Kind regards,
>
> Paul
next prev parent reply other threads:[~2021-12-21 20:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-20 21:06 [PATCH] CHROMIUM: i2c: Add device property for probing Paul Menzel
2021-12-20 21:49 ` Dmitry Torokhov
2021-12-21 16:47 ` Guenter Roeck
2021-12-21 19:42 ` Paul Menzel
2021-12-21 20:35 ` Guenter Roeck [this message]
2021-12-22 1:45 ` Dmitry Torokhov
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=CABXOdTfB4M8AcCOVERpQwddr_N09gpKF67FxRO32S4M9JUaYEQ@mail.gmail.com \
--to=groeck@google.com \
--cc=bleung@chromium.org \
--cc=coreboot@coreboot.org \
--cc=devicetree@vger.kernel.org \
--cc=dtor@chromium.org \
--cc=felixsinger@posteo.net \
--cc=gregkh@linuxfoundation.org \
--cc=groeck@chromium.org \
--cc=jeesw@melfas.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matt.devillier@gmail.com \
--cc=mika.westerberg@linux.intel.com \
--cc=pmenzel@molgen.mpg.de \
--cc=robh+dt@kernel.org \
--cc=teravest@chromium.org \
--cc=twawrzynczak@chromium.org \
--cc=wsa@kernel.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 read-only IMAP folder(s) and NNTP newsgroup(s).