linux-kernel - Re: [RFC v2 2/2] i2c: Pass i2c_device_id to probe func when using DT ids through ACPI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 10 Jun 2016 18:57:36 +0300
From:	Crestez Dan Leonard <leonard.crestez@...el.com>
To:	Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:	linux-acpi@...r.kernel.org,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Len Brown <lenb@...nel.org>, linux-i2c@...r.kernel.org,
	Wolfram Sang <wsa@...-dreams.de>, linux-kernel@...r.kernel.org,
	Irina Tirdea <irina.tirdea@...el.com>,
	Octavian Purdila <octavian.purdila@...el.com>,
	Daniel Baluta <daniel.baluta@...el.com>
Subject: Re: [RFC v2 2/2] i2c: Pass i2c_device_id to probe func when using DT
 ids through ACPI

On 06/10/2016 09:32 AM, Mika Westerberg wrote:
> On Thu, Jun 09, 2016 at 04:06:03PM +0300, Crestez Dan Leonard wrote:
>> When devices are instatiated through devicetree the i2c_client->name is
>> set to the compatible string with company name stripped out. This is
>> then matched to the i2c_device_id table to pass the device_id to the
>> probe function. This id parameter is used by some device drivers to
>> differentiate between model numbers.
>>
>> When using ACPI this id parameter is NULL and the driver usually needs
>> to do ACPI-specific differentiation.
>>
>> This patch attempts to find a valid i2c_device_id when using ACPI with
>> DT-like compatible strings.
> 
> So I don't really understand why it would be good idea to pass
> i2c_device_id for devices which are matched against their ACPI/DT
> tables. Apparently DT is already doing that so maybe there is some
> reason.
> 
> Anyway, why not fill in the device name when it is first enumerated
> if it uses DT compatible property? Just like DT does.
> 
This automatic matching of i2c_device_id works for devicetree because
of_i2c_register_device sets i2c_board_info.type to the compatible string
with the vendor prefix removed. For I2C devices described via ACPI the
i2c_board_info.type string is set to the ACPI device name. This ends up
something like "PRP0001:00".

This could be changed in acpi_i2c_get_info to use the of_compatible
string from DSD if present. Is that what you mean? That would work and
it would be cleaner than my patch. Something like this:

diff --git drivers/i2c/i2c-core.c drivers/i2c/i2c-core.c
index 1e0ef9b..ba2fe7f 100644
--- drivers/i2c/i2c-core.c
+++ drivers/i2c/i2c-core.c
@@ -181,7 +181,24 @@ static int acpi_i2c_get_info(struct acpi_device *adev,

        acpi_dev_free_resource_list(&resource_list);

-       strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+       /*
+        * If we have a DT id set info.type to the first compatible
string with
+        * the vendor prefix stripped. This is similar to of_modalias_node
+        */
+       if (adev->data.of_compatible) {
+               const union acpi_object *obj;
+               const char *str, *chr;
+
+               obj = adev->data.of_compatible;
+               if (obj->type == ACPI_TYPE_PACKAGE)
+                       obj = obj->package.elements;
+               str = obj->string.pointer;
+               chr = strchr(str, ',');
+               if (chr)
+                       str = chr + 1;
+               strlcpy(info->type, str, sizeof(info->type));
+       } else
+               strlcpy(info->type, dev_name(&adev->dev),
sizeof(info->type));

        return 0;
 }

The biggest concern is that this would change the i2c device name
between kernel versions. Is that acceptable?