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?
