* [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
@ 2017-07-01 10:03 Hans de Goede
2017-07-03 11:10 ` Andy Shevchenko
0 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2017-07-01 10:03 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
Mika Westerberg
Cc: Hans de Goede, linux-i2c
ACPI video devices get tagged by the kernel with the custom LNXVIDEO
HID so that normal pnp-id matching can be used and are handled by the
acpi-video driver.
Sometimes the ACPI nodes describing these contain a SERIAL_TYPE_I2C ACPI
resource. Before this commit the presence of this resource would cause the
i2c-core to create a /sys/bus/i2c/devices/i2c-LNXVIDEO:00 device for this
with a modalias of: "i2c:LNXVIDEO:00".
There is no i2c driver for this custom HID, the acpi-video driver binds
directly to the ACPI device /sys/bus/acpi/devices/LNXVIDEO\:00 which has
a modalias of "acpi:LNXVIDEO:" .
Not only is the creation of an i2c-client for this undesirable, it is
actually causing problems. This weird pseudo-resource claims an i2c
speed of 100KHz and typically points to the i2c bus which is used by the
touchscreen controller. Some touchscreen controllers only work properly at
400KHz, at 100KHz they cause errors like these:
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
silead_ts i2c-MSSL1680:00: Registers clear error -11
This commit makes the i2c-core ignore LNXVIDEO compatible ACPI devices
which has 2 positive results:
1) The bogus i2c-client for these is no longer created.
2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the pseudo
i2c-resouce and properly returns 400KHz as speed for the touchscreen
i2c bus, fixing the touchscreen not working on various devies.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/i2c/i2c-core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 82576aaccc90..4334ee1116fe 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -154,11 +154,22 @@ static int i2c_acpi_do_lookup(struct acpi_device *adev,
struct i2c_board_info *info = lookup->info;
struct list_head resource_list;
int ret;
+ static const struct acpi_device_id video_device_ids[] = {
+ { ACPI_VIDEO_HID, 0 },
+ {}
+ };
if (acpi_bus_get_status(adev) || !adev->status.present ||
acpi_device_enumerated(adev))
return -EINVAL;
+ /*
+ * ACPI video acpi_devices, which are handled by the acpi-video driver
+ * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore these.
+ */
+ if (acpi_match_device_ids(adev, video_device_ids) == 0)
+ return -ENODEV;
+
memset(info, 0, sizeof(*info));
lookup->device_handle = acpi_device_handle(adev);
--
2.13.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-01 10:03 Hans de Goede
@ 2017-07-03 11:10 ` Andy Shevchenko
2017-07-03 15:04 ` Hans de Goede
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2017-07-03 11:10 UTC (permalink / raw)
To: Hans de Goede, Jarkko Nikula, Wolfram Sang, Len Brown,
Mika Westerberg
Cc: linux-i2c
On Sat, 2017-07-01 at 12:03 +0200, Hans de Goede wrote:
> ACPI video devices get tagged by the kernel with the custom LNXVIDEO
> HID so that normal pnp-id matching can be used and are handled by the
> acpi-video driver.
>
> Sometimes the ACPI nodes describing these contain a SERIAL_TYPE_I2C
> ACPI
> resource. Before this commit the presence of this resource would cause
> the
> i2c-core to create a /sys/bus/i2c/devices/i2c-LNXVIDEO:00 device for
> this
> with a modalias of: "i2c:LNXVIDEO:00".
>
> There is no i2c driver for this custom HID, the acpi-video driver
> binds
> directly to the ACPI device /sys/bus/acpi/devices/LNXVIDEO\:00 which
> has
> a modalias of "acpi:LNXVIDEO:" .
>
> Not only is the creation of an i2c-client for this undesirable, it is
> actually causing problems. This weird pseudo-resource claims an i2c
> speed of 100KHz and typically points to the i2c bus which is used by
> the
> touchscreen controller. Some touchscreen controllers only work
> properly at
> 400KHz, at 100KHz they cause errors like these:
>
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> silead_ts i2c-MSSL1680:00: Registers clear error -11
>
> This commit makes the i2c-core ignore LNXVIDEO compatible ACPI devices
> which has 2 positive results:
>
> 1) The bogus i2c-client for these is no longer created.
> 2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the pseudo
> i2c-resouce and properly returns 400KHz as speed for the touchscreen
> i2c bus, fixing the touchscreen not working on various devies.
Should it have Fixes tag?
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/i2c/i2c-core.c | 11 +++++++++++
Shouldn't you rebase it on top of i2c-next? The file is called i2c-core-
acpi.c nowadays.
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 82576aaccc90..4334ee1116fe 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -154,11 +154,22 @@ static int i2c_acpi_do_lookup(struct acpi_device
> *adev,
> struct i2c_board_info *info = lookup->info;
> struct list_head resource_list;
> int ret;
> + static const struct acpi_device_id video_device_ids[] = {
> + { ACPI_VIDEO_HID, 0 },
> + {}
> + };
>
> if (acpi_bus_get_status(adev) || !adev->status.present ||
> acpi_device_enumerated(adev))
> return -EINVAL;
>
> + /*
> + * ACPI video acpi_devices, which are handled by the acpi-
> video driver
> + * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore
> these.
> + */
> + if (acpi_match_device_ids(adev, video_device_ids) == 0)
> + return -ENODEV;
> +
> memset(info, 0, sizeof(*info));
> lookup->device_handle = acpi_device_handle(adev);
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-03 11:10 ` Andy Shevchenko
@ 2017-07-03 15:04 ` Hans de Goede
2017-07-03 15:48 ` Andy Shevchenko
0 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2017-07-03 15:04 UTC (permalink / raw)
To: Andy Shevchenko, Jarkko Nikula, Wolfram Sang, Len Brown,
Mika Westerberg
Cc: linux-i2c
Hi,
On 03-07-17 13:10, Andy Shevchenko wrote:
> On Sat, 2017-07-01 at 12:03 +0200, Hans de Goede wrote:
>> ACPI video devices get tagged by the kernel with the custom LNXVIDEO
>> HID so that normal pnp-id matching can be used and are handled by the
>> acpi-video driver.
>>
>> Sometimes the ACPI nodes describing these contain a SERIAL_TYPE_I2C
>> ACPI
>> resource. Before this commit the presence of this resource would cause
>> the
>> i2c-core to create a /sys/bus/i2c/devices/i2c-LNXVIDEO:00 device for
>> this
>> with a modalias of: "i2c:LNXVIDEO:00".
>>
>> There is no i2c driver for this custom HID, the acpi-video driver
>> binds
>> directly to the ACPI device /sys/bus/acpi/devices/LNXVIDEO\:00 which
>> has
>> a modalias of "acpi:LNXVIDEO:" .
>>
>> Not only is the creation of an i2c-client for this undesirable, it is
>> actually causing problems. This weird pseudo-resource claims an i2c
>> speed of 100KHz and typically points to the i2c bus which is used by
>> the
>> touchscreen controller. Some touchscreen controllers only work
>> properly at
>> 400KHz, at 100KHz they cause errors like these:
>>
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> silead_ts i2c-MSSL1680:00: Registers clear error -11
>>
>> This commit makes the i2c-core ignore LNXVIDEO compatible ACPI devices
>> which has 2 positive results:
>>
>> 1) The bogus i2c-client for these is no longer created.
>> 2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the pseudo
>> i2c-resouce and properly returns 400KHz as speed for the touchscreen
>> i2c bus, fixing the touchscreen not working on various devies.
>
> Should it have Fixes tag?
Well it is a fix, but it does not fix one specific commit, so no
I don't think it should.
>
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/i2c/i2c-core.c | 11 +++++++++++
>
> Shouldn't you rebase it on top of i2c-next? The file is called i2c-core-
> acpi.c nowadays.
Ah, ok, I will rebase and send a new version.
Regards,
Hans
>
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 82576aaccc90..4334ee1116fe 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -154,11 +154,22 @@ static int i2c_acpi_do_lookup(struct acpi_device
>> *adev,
>> struct i2c_board_info *info = lookup->info;
>> struct list_head resource_list;
>> int ret;
>> + static const struct acpi_device_id video_device_ids[] = {
>> + { ACPI_VIDEO_HID, 0 },
>> + {}
>> + };
>>
>> if (acpi_bus_get_status(adev) || !adev->status.present ||
>> acpi_device_enumerated(adev))
>> return -EINVAL;
>>
>> + /*
>> + * ACPI video acpi_devices, which are handled by the acpi-
>> video driver
>> + * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore
>> these.
>> + */
>> + if (acpi_match_device_ids(adev, video_device_ids) == 0)
>> + return -ENODEV;
>> +
>> memset(info, 0, sizeof(*info));
>> lookup->device_handle = acpi_device_handle(adev);
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-03 15:04 ` Hans de Goede
@ 2017-07-03 15:48 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-07-03 15:48 UTC (permalink / raw)
To: Hans de Goede, Jarkko Nikula, Wolfram Sang, Len Brown,
Mika Westerberg
Cc: linux-i2c
On Mon, 2017-07-03 at 17:04 +0200, Hans de Goede wrote:
> On 03-07-17 13:10, Andy Shevchenko wrote:
> > On Sat, 2017-07-01 at 12:03 +0200, Hans de Goede wrote:
> > > This commit makes the i2c-core ignore LNXVIDEO compatible ACPI
> > > devices
> > > which has 2 positive results:
> > >
> > > 1) The bogus i2c-client for these is no longer created.
> > > 2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the
> > > pseudo
> > > i2c-resouce and properly returns 400KHz as speed for the
> > > touchscreen
> > > i2c bus, fixing the touchscreen not working on various devies.
> >
> > Should it have Fixes tag?
>
> Well it is a fix, but it does not fix one specific commit, so no
> I don't think it should.
One may put several Fixes tags to make it clear that it applies on top
of the tree that has all of them (in the given sequence AFAIR).
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
@ 2017-07-04 12:46 Hans de Goede
2017-07-04 12:47 ` Hans de Goede
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Hans de Goede @ 2017-07-04 12:46 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
Mika Westerberg
Cc: Hans de Goede, linux-i2c
ACPI video devices get tagged by the kernel with the custom LNXVIDEO
HID so that normal pnp-id matching can be used and are handled by the
acpi-video driver.
Sometimes the ACPI nodes describing these contain a SERIAL_TYPE_I2C ACPI
resource. Before this commit the presence of this resource would cause the
i2c-core to create a /sys/bus/i2c/devices/i2c-LNXVIDEO:00 device for this
with a modalias of: "i2c:LNXVIDEO:00".
There is no i2c driver for this custom HID, the acpi-video driver binds
directly to the ACPI device /sys/bus/acpi/devices/LNXVIDEO\:00 which has
a modalias of "acpi:LNXVIDEO:" .
Not only is the creation of an i2c-client for this undesirable, it is
actually causing problems. This weird pseudo-resource claims an i2c
speed of 100KHz and typically points to the i2c bus which is used by the
touchscreen controller. Some touchscreen controllers only work properly at
400KHz, at 100KHz they cause errors like these:
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
silead_ts i2c-MSSL1680:00: Registers clear error -11
This commit makes the i2c-core ignore LNXVIDEO compatible ACPI devices
which has 2 positive results:
1) The bogus i2c-client for these is no longer created.
2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the pseudo
i2c-resouce and properly returns 400KHz as speed for the touchscreen
i2c bus, fixing the touchscreen not working on various devies.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rebase on top of linux-i2c/for-next
---
drivers/i2c/i2c-core-acpi.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 052005579ed6..bda281293d28 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -82,11 +82,22 @@ static int i2c_acpi_do_lookup(struct acpi_device *adev,
struct i2c_board_info *info = lookup->info;
struct list_head resource_list;
int ret;
+ static const struct acpi_device_id video_device_ids[] = {
+ { ACPI_VIDEO_HID, 0 },
+ {}
+ };
if (acpi_bus_get_status(adev) || !adev->status.present ||
acpi_device_enumerated(adev))
return -EINVAL;
+ /*
+ * ACPI video acpi_devices, which are handled by the acpi-video driver
+ * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore these.
+ */
+ if (acpi_match_device_ids(adev, video_device_ids) == 0)
+ return -ENODEV;
+
memset(info, 0, sizeof(*info));
lookup->device_handle = acpi_device_handle(adev);
--
2.13.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-04 12:46 [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices Hans de Goede
@ 2017-07-04 12:47 ` Hans de Goede
2017-07-04 12:49 ` Wolfram Sang
2017-07-04 12:49 ` Andy Shevchenko
2 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2017-07-04 12:47 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
Mika Westerberg
Cc: linux-i2c
Hi all,
I forgot to add "v2" to the subject, but this is a rebased v2,
which should apply cleanly on next.
Regards.
Hans
On 04-07-17 14:46, Hans de Goede wrote:
> ACPI video devices get tagged by the kernel with the custom LNXVIDEO
> HID so that normal pnp-id matching can be used and are handled by the
> acpi-video driver.
>
> Sometimes the ACPI nodes describing these contain a SERIAL_TYPE_I2C ACPI
> resource. Before this commit the presence of this resource would cause the
> i2c-core to create a /sys/bus/i2c/devices/i2c-LNXVIDEO:00 device for this
> with a modalias of: "i2c:LNXVIDEO:00".
>
> There is no i2c driver for this custom HID, the acpi-video driver binds
> directly to the ACPI device /sys/bus/acpi/devices/LNXVIDEO\:00 which has
> a modalias of "acpi:LNXVIDEO:" .
>
> Not only is the creation of an i2c-client for this undesirable, it is
> actually causing problems. This weird pseudo-resource claims an i2c
> speed of 100KHz and typically points to the i2c bus which is used by the
> touchscreen controller. Some touchscreen controllers only work properly at
> 400KHz, at 100KHz they cause errors like these:
>
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> silead_ts i2c-MSSL1680:00: Registers clear error -11
>
> This commit makes the i2c-core ignore LNXVIDEO compatible ACPI devices
> which has 2 positive results:
>
> 1) The bogus i2c-client for these is no longer created.
> 2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the pseudo
> i2c-resouce and properly returns 400KHz as speed for the touchscreen
> i2c bus, fixing the touchscreen not working on various devies.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Rebase on top of linux-i2c/for-next
> ---
> drivers/i2c/i2c-core-acpi.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index 052005579ed6..bda281293d28 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -82,11 +82,22 @@ static int i2c_acpi_do_lookup(struct acpi_device *adev,
> struct i2c_board_info *info = lookup->info;
> struct list_head resource_list;
> int ret;
> + static const struct acpi_device_id video_device_ids[] = {
> + { ACPI_VIDEO_HID, 0 },
> + {}
> + };
>
> if (acpi_bus_get_status(adev) || !adev->status.present ||
> acpi_device_enumerated(adev))
> return -EINVAL;
>
> + /*
> + * ACPI video acpi_devices, which are handled by the acpi-video driver
> + * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore these.
> + */
> + if (acpi_match_device_ids(adev, video_device_ids) == 0)
> + return -ENODEV;
> +
> memset(info, 0, sizeof(*info));
> lookup->device_handle = acpi_device_handle(adev);
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-04 12:46 [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices Hans de Goede
2017-07-04 12:47 ` Hans de Goede
@ 2017-07-04 12:49 ` Wolfram Sang
2017-07-04 12:50 ` Hans de Goede
2017-07-04 12:49 ` Andy Shevchenko
2 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2017-07-04 12:49 UTC (permalink / raw)
To: Hans de Goede
Cc: Jarkko Nikula, Len Brown, Andy Shevchenko, Mika Westerberg,
linux-i2c
[-- Attachment #1: Type: text/plain, Size: 134 bytes --]
> + static const struct acpi_device_id video_device_ids[] = {
What about naming the array 'ignored_device_ids' to be more generic?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-04 12:46 [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices Hans de Goede
2017-07-04 12:47 ` Hans de Goede
2017-07-04 12:49 ` Wolfram Sang
@ 2017-07-04 12:49 ` Andy Shevchenko
2017-07-04 12:51 ` Hans de Goede
2 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2017-07-04 12:49 UTC (permalink / raw)
To: Hans de Goede, Jarkko Nikula, Wolfram Sang, Len Brown,
Mika Westerberg
Cc: linux-i2c
On Tue, 2017-07-04 at 14:46 +0200, Hans de Goede wrote:
> ACPI video devices get tagged by the kernel with the custom LNXVIDEO
> HID so that normal pnp-id matching can be used and are handled by the
> acpi-video driver.
>
> Sometimes the ACPI nodes describing these contain a SERIAL_TYPE_I2C
> ACPI
> resource. Before this commit the presence of this resource would cause
> the
> i2c-core to create a /sys/bus/i2c/devices/i2c-LNXVIDEO:00 device for
> this
> with a modalias of: "i2c:LNXVIDEO:00".
>
> There is no i2c driver for this custom HID, the acpi-video driver
> binds
> directly to the ACPI device /sys/bus/acpi/devices/LNXVIDEO\:00 which
> has
> a modalias of "acpi:LNXVIDEO:" .
>
> Not only is the creation of an i2c-client for this undesirable, it is
> actually causing problems. This weird pseudo-resource claims an i2c
> speed of 100KHz and typically points to the i2c bus which is used by
> the
> touchscreen controller. Some touchscreen controllers only work
> properly at
> 400KHz, at 100KHz they cause errors like these:
>
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
> silead_ts i2c-MSSL1680:00: Registers clear error -11
>
> This commit makes the i2c-core ignore LNXVIDEO compatible ACPI devices
> which has 2 positive results:
>
> 1) The bogus i2c-client for these is no longer created.
> 2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the pseudo
> i2c-resouce and properly returns 400KHz as speed for the touchscreen
> i2c bus, fixing the touchscreen not working on various devies.
>
Thanks for fixing this.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
One comment below.
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Rebase on top of linux-i2c/for-next
> ---
> drivers/i2c/i2c-core-acpi.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index 052005579ed6..bda281293d28 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -82,11 +82,22 @@ static int i2c_acpi_do_lookup(struct acpi_device
> *adev,
> struct i2c_board_info *info = lookup->info;
> struct list_head resource_list;
> int ret;
> + static const struct acpi_device_id video_device_ids[] = {
> + { ACPI_VIDEO_HID, 0 },
> + {}
> + };
Can we move this out of the function body?
>
> if (acpi_bus_get_status(adev) || !adev->status.present ||
> acpi_device_enumerated(adev))
> return -EINVAL;
>
> + /*
> + * ACPI video acpi_devices, which are handled by the acpi-
> video driver
> + * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore
> these.
> + */
> + if (acpi_match_device_ids(adev, video_device_ids) == 0)
> + return -ENODEV;
> +
> memset(info, 0, sizeof(*info));
> lookup->device_handle = acpi_device_handle(adev);
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-04 12:49 ` Wolfram Sang
@ 2017-07-04 12:50 ` Hans de Goede
0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2017-07-04 12:50 UTC (permalink / raw)
To: Wolfram Sang
Cc: Jarkko Nikula, Len Brown, Andy Shevchenko, Mika Westerberg,
linux-i2c
Hi,
On 04-07-17 14:49, Wolfram Sang wrote:
>
>> + static const struct acpi_device_id video_device_ids[] = {
>
> What about naming the array 'ignored_device_ids' to be more generic?
Sure I can do that, any other remarks before I send a v3 ?
Regards,
Hans
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices
2017-07-04 12:49 ` Andy Shevchenko
@ 2017-07-04 12:51 ` Hans de Goede
0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2017-07-04 12:51 UTC (permalink / raw)
To: Andy Shevchenko, Jarkko Nikula, Wolfram Sang, Len Brown,
Mika Westerberg
Cc: linux-i2c
Hi,
On 04-07-17 14:49, Andy Shevchenko wrote:
> On Tue, 2017-07-04 at 14:46 +0200, Hans de Goede wrote:
>> ACPI video devices get tagged by the kernel with the custom LNXVIDEO
>> HID so that normal pnp-id matching can be used and are handled by the
>> acpi-video driver.
>>
>> Sometimes the ACPI nodes describing these contain a SERIAL_TYPE_I2C
>> ACPI
>> resource. Before this commit the presence of this resource would cause
>> the
>> i2c-core to create a /sys/bus/i2c/devices/i2c-LNXVIDEO:00 device for
>> this
>> with a modalias of: "i2c:LNXVIDEO:00".
>>
>> There is no i2c driver for this custom HID, the acpi-video driver
>> binds
>> directly to the ACPI device /sys/bus/acpi/devices/LNXVIDEO\:00 which
>> has
>> a modalias of "acpi:LNXVIDEO:" .
>>
>> Not only is the creation of an i2c-client for this undesirable, it is
>> actually causing problems. This weird pseudo-resource claims an i2c
>> speed of 100KHz and typically points to the i2c bus which is used by
>> the
>> touchscreen controller. Some touchscreen controllers only work
>> properly at
>> 400KHz, at 100KHz they cause errors like these:
>>
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> i2c_designware 80860F41:03: i2c_dw_handle_tx_abort: lost arbitration
>> silead_ts i2c-MSSL1680:00: Registers clear error -11
>>
>> This commit makes the i2c-core ignore LNXVIDEO compatible ACPI devices
>> which has 2 positive results:
>>
>> 1) The bogus i2c-client for these is no longer created.
>> 2) i2c_acpi_lookup_speed now ignores the 100KHz speed from the pseudo
>> i2c-resouce and properly returns 400KHz as speed for the touchscreen
>> i2c bus, fixing the touchscreen not working on various devies.
>>
>
> Thanks for fixing this.
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thank you.
>
> One comment below.
>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Rebase on top of linux-i2c/for-next
>> ---
>> drivers/i2c/i2c-core-acpi.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
>> index 052005579ed6..bda281293d28 100644
>> --- a/drivers/i2c/i2c-core-acpi.c
>> +++ b/drivers/i2c/i2c-core-acpi.c
>> @@ -82,11 +82,22 @@ static int i2c_acpi_do_lookup(struct acpi_device
>> *adev,
>> struct i2c_board_info *info = lookup->info;
>> struct list_head resource_list;
>> int ret;
>
>> + static const struct acpi_device_id video_device_ids[] = {
>> + { ACPI_VIDEO_HID, 0 },
>> + {}
>> + };
>
> Can we move this out of the function body?
Sure, will fix for v3.
Regards,
Hans
>
>>
>> if (acpi_bus_get_status(adev) || !adev->status.present ||
>> acpi_device_enumerated(adev))
>> return -EINVAL;
>>
>> + /*
>> + * ACPI video acpi_devices, which are handled by the acpi-
>> video driver
>> + * sometimes contain a SERIAL_TYPE_I2C ACPI resource, ignore
>> these.
>> + */
>> + if (acpi_match_device_ids(adev, video_device_ids) == 0)
>> + return -ENODEV;
>> +
>> memset(info, 0, sizeof(*info));
>> lookup->device_handle = acpi_device_handle(adev);
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-07-04 12:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-04 12:46 [PATCH] i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices Hans de Goede
2017-07-04 12:47 ` Hans de Goede
2017-07-04 12:49 ` Wolfram Sang
2017-07-04 12:50 ` Hans de Goede
2017-07-04 12:49 ` Andy Shevchenko
2017-07-04 12:51 ` Hans de Goede
-- strict thread matches above, loose matches on Subject: below --
2017-07-01 10:03 Hans de Goede
2017-07-03 11:10 ` Andy Shevchenko
2017-07-03 15:04 ` Hans de Goede
2017-07-03 15:48 ` Andy Shevchenko
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).