Match i2c_device_id when using DT ids through ACPI

Linux supports instantiating devices using devicetree ids from ACPI by setting the id to PRP0001 and adding the devicetree compatible string in _DSD properties. This is described in Documentation/acpi/enumeration.txt.

I've tried to use this feature using custom ACPI tables and one issue I encountered is that a lot of i2c device drivers are written for DT only and expect a valid i2c_device_id *id parameter to their probe function but this is always NULL for the PRP0001 case. Others call of_match_device in order to determine their exact model number and that will also fail.

Drivers normally do this in order to differentiate between minor model numbers supported by the same driver, for example hmc5883 versus hmc5983.

For ACPI it is common to add calls to acpi_match_device in order to do such differentiation. It might make sense to call some form of acpi_of_match_device except no such function is currently exported. I think it makes a lot of sense to refactor the current acpi_of_match_device to return the of_device_id and export it.

This is in PATCH 1. After exposing this function it could be called from each driver that needs to support multiple models through PRP0001. I guess the alternative to exposing acpi_of_match_device would be for driver code to poke at acpi_device->data.of_compatible?

Going further it might make sense to attempt to fetch the i2c_device_id when instantiating through PRP0001. This already happens automatically for devicetree and it makes a lot of sense it would work through ACPI with DT ids. Patch 2 hacks an additional search in i2c register code. This makes some drivers "just work" without manually handling the "ACPI with DT ids" case.

For P1 it might make sense to change the parameters around so that acpi_of_match_device will have the same signature as of_match_device.

The purpose of these changes would be to make more drivers easier to instantiate through ACPI firmware. It's possible that I misunderstood and this problem already has a different solution.

Crestez Dan Leonard (2):
acpi: Expose acpi_of_match_device
i2c: Pass i2c_device_id to probe func when using DT ids through ACPI

 drivers/acpi/bus.c     | 13 +++++++------
 drivers/i2c/i2c-core.c | 26 +++++++++++++++++++++++---
 include/linux/acpi.h   |  8 ++++++++
 3 files changed, 38 insertions(+), 9 deletions(-)

2.5.5

This can be used by device drivers as the equivalent of of_match_device when they are instantiated through ACPI using devicetree IDs. This is described in Documentation/acpi/enumeration.txt

---
 drivers/acpi/bus.c   | 13 +++++++------
 include/linux/acpi.h |  8 ++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

index 27367d4..b366bb2 100644
@@ -574,18 +574,18 @@ struct acpi_device *acpi_companion_match(const struct device *dev)
  * identifiers and a _DSD object with the "compatible" property, use that
  * property to match against the given list of identifiers.
  */
-static bool acpi_of_match_device(struct acpi_device *adev,
-				 const struct of_device_id *of_match_table)
+const struct of_device_id* acpi_of_match_device(const struct acpi_device *adev,
+						const struct of_device_id *of_match_table)
 {
 	const union acpi_object *of_compatible, *obj;
 	int i, nval;
 
 	if (!adev)
-		return false;
+		return NULL;
 
 	of_compatible = adev->data.of_compatible;
 	if (!of_match_table || !of_compatible)
-		return false;
+		return NULL;
 
 	if (of_compatible->type == ACPI_TYPE_PACKAGE) {
 		nval = of_compatible->package.count;

@@ -600,11 +600,12 @@ static bool acpi_of_match_device(struct acpi_device *adev,
 
 		for (id = of_match_table; id->compatible[0]; id++)
 			if (!strcasecmp(obj->string.pointer, id->compatible))
-				return true;
+				return id;
 	}
 
-	return false;
+	return NULL;
 }
+EXPORT_SYMBOL(acpi_of_match_device);
 
 static bool __acpi_match_device_cls(const struct acpi_device_id *id,
 				    struct acpi_hardware_id *hwid)

index d4a3cb2..b8cefca 100644
@@ -524,6 +524,8 @@ extern int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
 
 const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
 					       const struct device *dev);
+const struct of_device_id *acpi_of_match_device(const struct acpi_device *dev,
+						const struct of_device_id *ids);
 
 extern bool acpi_driver_match_device(struct device *dev,
 				     const struct device_driver *drv);

@@ -649,6 +651,12 @@ static inline const struct acpi_device_id *acpi_match_device(
 	return NULL;
 }
 
+const struct of_device_id *acpi_of_match_device(const struct acpi_device *dev,
+						const struct acpi_device_id *ids);
+{
+	return NULL;
+}
+
 static inline bool acpi_driver_match_device(struct device *dev,
 					    const struct device_driver *drv)
 {

2.5.5

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.

---
 drivers/i2c/i2c-core.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

index 3ffeb6c..c899108 100644
@@ -581,17 +581,23 @@ static inline int acpi_i2c_install_space_handler(struct i2c_adapter *adapter)
 
 /* ------------------------------------------------------------------------- */
 
-static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
-						const struct i2c_client *client)
+static const struct i2c_device_id *i2c_match_id_name(const struct i2c_device_id *id,
+						     const char *id_name)
 {
 	while (id->name[0]) {
-		if (strcmp(client->name, id->name) == 0)
+		if (strcmp(id_name, id->name) == 0)
 			return id;
 		id++;
 	}
 	return NULL;
 }
 
+static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
+						const struct i2c_client *client)
+{
+	return i2c_match_id_name(id, client->name);
+}
+
 static int i2c_device_match(struct device *dev, struct device_driver *drv)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);

@@ -767,6 +773,7 @@ static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
+	const struct i2c_device_id *i2c_device_id;
 	int status;
 
 	if (!client)

@@ -826,6 +833,19 @@ static int i2c_device_probe(struct device *dev)
 	if (status == -EPROBE_DEFER)
 		goto err_clear_wakeup_irq;
 
+	i2c_device_id = i2c_match_id(driver->id_table, client);
+#ifdef CONFIG_ACPI
+	if (!i2c_device_id) {
+		const char *id_name;
+		const struct of_device_id *ofid;
+
+		ofid = acpi_of_match_device(ACPI_COMPANION(&client->dev),
+					    driver->driver.of_match_table);
+		id_name = strchr(ofid->name, ',');
+		id_name = id_name ? id_name + 1 : ofid->name;
+		i2c_device_id = i2c_match_id_name(driver->id_table, id_name);
+	}
+#endif
 	status = driver->probe(client, i2c_match_id(driver->id_table, client));
 	if (status)
 		goto err_detach_pm_domain;

2.5.5

This can be used by device drivers as the equivalent of of_match_device when they are instantiated through ACPI using devicetree IDs. This is described in Documentation/acpi/enumeration.txt

---
 drivers/acpi/bus.c   | 13 +++++++------
 include/linux/acpi.h |  8 ++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

Sending the actual patches separately to [email protected] because I used --cc-cover without --to-cover.

index 27367d4..b366bb2 100644
@@ -574,18 +574,18 @@ struct acpi_device *acpi_companion_match(const

struct device *dev)
* identifiers and a _DSD object with the "compatible" property, use that
* property to match against the given list of identifiers.
*/
-static bool acpi_of_match_device(struct acpi_device *adev,
- const struct of_device_id *of_match_table)
+const struct of_device_id* acpi_of_match_device(const struct acpi_device *adev,
+ const struct of_device_id *of_match_table)
{
const union acpi_object *of_compatible, *obj;
int i, nval;
if (!adev)
- return false;
+ return NULL;
of_compatible = adev->data.of_compatible;
if (!of_match_table || !of_compatible)
- return false;
+ return NULL;
if (of_compatible->type == ACPI_TYPE_PACKAGE) {
nval = of_compatible->package.count;
@@ -600,11 +600,12 @@ static bool acpi_of_match_device(struct acpi_device *adev,
for (id = of_match_table; id->compatible[0]; id++)
if (!strcasecmp(obj->string.pointer, id->compatible))
- return true;
+ return id;
}
- return false;
+ return NULL;
}
+EXPORT_SYMBOL(acpi_of_match_device);
static bool __acpi_match_device_cls(const struct acpi_device_id *id,
struct acpi_hardware_id *hwid)

index d4a3cb2..b8cefca 100644
@@ -524,6 +524,8 @@ extern int acpi_nvs_for_each_region(int

(*func)(__u64, __u64, void *),
const struct acpi_device_id *acpi_match_device(const struct
acpi_device_id *ids,
const struct device *dev);
+const struct of_device_id *acpi_of_match_device(const struct acpi_device *dev,
+ const struct of_device_id *ids);
extern bool acpi_driver_match_device(struct device *dev,
const struct device_driver *drv);
@@ -649,6 +651,12 @@ static inline const struct acpi_device_id *acpi_match_device(
return NULL;
}
+const struct of_device_id *acpi_of_match_device(const struct
acpi_device *dev,
+ const struct acpi_device_id *ids);
+{
+ return NULL;
+}
+
static inline bool acpi_driver_match_device(struct device *dev,
const struct device_driver *drv)
{

2.5.5

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.

---
 drivers/i2c/i2c-core.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

Sending the actual patches separately to [email protected] because I used --cc-cover without --to-cover.

index 3ffeb6c..c899108 100644
@@ -581,17 +581,23 @@ static inline int

acpi_i2c_install_space_handler(struct i2c_adapter *adapter)
/*
------------------------------------------------------------------------- */
-static const struct i2c_device_id *i2c_match_id(const struct
i2c_device_id *id,
- const struct i2c_client *client)
+static const struct i2c_device_id *i2c_match_id_name(const struct i2c_device_id *id,
+ const char *id_name)
{
while (id->name[0]) {
- if (strcmp(client->name, id->name) == 0)
+ if (strcmp(id_name, id->name) == 0)
return id;
id++;
}
return NULL;
}
+static const struct i2c_device_id *i2c_match_id(const struct
i2c_device_id *id,
+ const struct i2c_client *client)
+{
+ return i2c_match_id_name(id, client->name);
+}
+
static int i2c_device_match(struct device *dev, struct device_driver *drv) {
struct i2c_client *client = i2c_verify_client(dev);
@@ -767,6 +773,7 @@ static int i2c_device_probe(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
struct i2c_driver *driver;
+ const struct i2c_device_id *i2c_device_id;
int status;
if (!client)
@@ -826,6 +833,19 @@ static int i2c_device_probe(struct device *dev)
if (status == -EPROBE_DEFER)
goto err_clear_wakeup_irq;
+ i2c_device_id = i2c_match_id(driver->id_table, client);
+#ifdef CONFIG_ACPI
+ if (!i2c_device_id) {
+ const char *id_name;
+ const struct of_device_id *ofid;
+
+ ofid = acpi_of_match_device(ACPI_COMPANION(&client->dev),
+ driver->driver.of_match_table);
+ id_name = strchr(ofid->name, ',');
+ id_name = id_name ? id_name + 1 : ofid->name;
+ i2c_device_id = i2c_match_id_name(driver->id_table, id_name);
+ }
+#endif
status = driver->probe(client, i2c_match_id(driver->id_table, client)); if (status)
goto err_detach_pm_domain;

2.5.5

It would be good to CC the actual patches to the same set of addresses as this message.

Thanks,
Rafael