From: Furquan Shaikh <[email protected]>
Dear Linux folks,
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?
Kind regards,
Paul
[1]: https://review.coreboot.org/c/coreboot/+/16742/
[2]: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1104997
--------------- 8< -------------------------- >8 ---------------
Add a new device property to indicate if an I2C device should be
probed before being added. If this property is present and set
then the I2C core layer will use i2c_new_probed_device() instead
of i2c_new_device().
This can be used to provide devices in ACPI or DT that may not be
present on the board. For example, multiple trackpad vendors can
be supported on a single board with a unified firmware image this
way by having their device address be probed before being added.
This property is styled after the PCI Host Bridge probe property
(bindings/pci/host-generic-pci.txt:linuxk,pci-probe-only) and is
a linux specific directive to alter device probing.
BUG=b:110013532
TEST=tested on soraka with 4.14 kernel:
1) add "linux,probed=1" device property to the touchscreen ACPI device
on soraka and ensure that the device is probed before being added.
tested on yorp with 4.14 kernel:
1) I2C devices without the "linux,probed=1" device property are still
functional.
Original-change-Id: I9cf689f7b75ef445c1f0e9f7ec143fa695eb398e
Original-signed-off-by: Duncan Laurie <[email protected]>
Original-reviewed-on: https://chromium-review.googlesource.com/388767
Original-reviewed-by: Benson Leung <[email protected]>
Change-Id: I54015fe102f2834f6a094d9e650c166a0cc0583b
Signed-off-by: Furquan Shaikh <[email protected]>
Reviewed-on: https://chromium-review.googlesource.com/1100544
Commit-Ready: Furquan Shaikh <[email protected]>
Tested-by: Furquan Shaikh <[email protected]>
Reviewed-by: Justin TerAvest <[email protected]>
Conflicts:
drivers/i2c/i2c-core-of.c
[rebase419(groeck): Context conflicts]
Signed-off-by: Guenter Roeck <[email protected]>
[rebase510(groeck): Context conflicts]
Signed-off-by: Guenter Roeck <[email protected]>
Change-Id: Id10adae00b381e62813fd6b8ce7c6c50f140c31b
---
Documentation/devicetree/bindings/i2c/i2c.txt | 5 +++++
drivers/i2c/i2c-core-acpi.c | 12 +++++++++++-
drivers/i2c/i2c-core-of.c | 10 +++++++++-
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index b864916e087f..4921ee57f4c1 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -133,6 +133,11 @@ wants to support one of the below features, it should adapt these bindings.
- wakeup-source
device can be used as a wakeup source.
+- linux,probed
+ If this property is present, then the I2C device will be
+ probed before being added using i2c_new_scanned_device, else
+ linux will instantiate the I2C device normally.
+
Binding may contain optional "interrupts" property, describing interrupts
used by the device. I2C core will assign "irq" interrupt (or the very first
interrupt if not using interrupt names) as primary interrupt for the slave.
diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 92c1cc07ed46..c970d99e4438 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -254,10 +254,20 @@ static void i2c_acpi_register_device(struct i2c_adapter *adapter,
struct acpi_device *adev,
struct i2c_board_info *info)
{
+ struct i2c_client *client;
+
adev->power.flags.ignore_parent = true;
acpi_device_set_enumerated(adev);
- if (IS_ERR(i2c_new_client_device(adapter, info)))
+ if (!acpi_dev_get_property(adev, "linux,probed", ACPI_TYPE_ANY, NULL)) {
+ unsigned short addrs[] = { info->addr, I2C_CLIENT_END };
+
+ client = i2c_new_scanned_device(adapter, info, addrs, NULL);
+ } else {
+ client = i2c_new_client_device(adapter, info);
+ }
+
+ if (IS_ERR(client))
adev->power.flags.ignore_parent = false;
}
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 3ed74aa4b44b..fd375ce38a9e 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -75,7 +75,15 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
if (ret)
return ERR_PTR(ret);
- client = i2c_new_client_device(adap, &info);
+ /* Allow device property to enable probing before init */
+ if (of_get_property(node, "linux,probed", NULL)) {
+ unsigned short addrs[] = { info.addr, I2C_CLIENT_END };
+
+ client = i2c_new_scanned_device(adap, &info, addrs, NULL);
+ } else {
+ client = i2c_new_client_device(adap, &info);
+ }
+
if (IS_ERR(client))
dev_err(&adap->dev, "of_i2c: Failure registering %pOF\n", node);
--
2.34.1
Hi Paul,
On Mon, Dec 20, 2021 at 1:07 PM Paul Menzel <[email protected]> wrote:
>
> From: Furquan Shaikh <[email protected]>
>
> Dear Linux folks,
>
>
> 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.
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.
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.
We should remove "linux,probed" from coreboot and not propagate to
newer Chrome OS kernels, and keep it away from upstream.
Thanks,
Dmitry
On Mon, Dec 20, 2021 at 1:49 PM Dmitry Torokhov <[email protected]> wrote:
>
> Hi Paul,
>
> On Mon, Dec 20, 2021 at 1:07 PM Paul Menzel <[email protected]> wrote:
> >
> > From: Furquan Shaikh <[email protected]>
> >
> > Dear Linux folks,
> >
> >
> > 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.
>
> 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.
>
> 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.
>
> 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.
Guenter
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 <[email protected]> wrote:
>> On Mon, Dec 20, 2021 at 1:07 PM Paul Menzel <[email protected]> wrote:
>>>
>>> From: Furquan Shaikh <[email protected]>
>>> 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?
>> 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
On Tue, Dec 21, 2021 at 11:42 AM Paul Menzel <[email protected]> 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 <[email protected]> wrote:
>
> >> On Mon, Dec 20, 2021 at 1:07 PM Paul Menzel <[email protected]> wrote:
> >>>
> >>> From: Furquan Shaikh <[email protected]>
>
> >>> 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
On Tue, Dec 21, 2021 at 11:42 AM Paul Menzel <[email protected]> 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 <[email protected]> wrote:
>
> >> On Mon, Dec 20, 2021 at 1:07 PM Paul Menzel <[email protected]> wrote:
> >>>
> >>> From: Furquan Shaikh <[email protected]>
>
> >>> 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?
So what happens is that there are Melfas and Elan (i2c-hid)
touchscreens described in DSDT, both sharing the same reset line. So
you get Melfas identified, and then i2c-hid comes along, does not find
anything, and powers off/resets Melfas controller. Unfortunately
coreboot does not implement logic for shared power resources here, so
when I2C core thinks that it is powering off Elan it in fact powers
off an unrelated device.
I wonder if we should offer a language to better describe dual-sourced
components/shared power resources so we would not get into this
situation. I am still curious what powers up the devices in case we
use
i2c_new_scanned_device() path. Is it simply a happenstance? I have a
Sarien with Melfas, I guess I can play with it a bit...
OK, I guess we've built more stuff on top of linux,probed and we will
have to have it if we want to support current Google firmware. We need
to have much stronger wording about what linux,probed property really
means (i.e. we expect the device to be powered up by the firmware,
executing proper power up timing sequence, and not being a "morphing"
device) for this to work reliably.
>
> >> 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”?
Modern touch controllers and other peripherals are pretty complex.
They typically have R0 firmware (bootloader) and RW main firmware, and
the controller might end up being in "bootloader" mode if RW firmware
gets corrupted or because of power surge, or something else. Some
controllers (for example Atmel) use different addresses on the bus
when in bootloader mode vs normal 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.
There would need to be arbitration between OS and firmware when
accessing I2C controllers in this case so indeed this would be hard.
Maybe coreboot would have to pre-scan the peripherals before booting
the OS.
>
> >> 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?
Melfas has handling for its reset line, as well as Elan touch
controllers (elants_i2c.c), Atmel (atmel_mxt_ts.c), Raydium, etc.
Thanks,
Dmitry