Re: [PATCH 1/4 v2] mfd: add STw481x driver

From: Mark Brown
Date: Mon Sep 16 2013 - 09:51:29 EST


On Mon, Sep 16, 2013 at 02:44:35PM +0200, Linus Walleij wrote:

> I've tried to fix this for DT-only I2C devices
> and this very driver was the reason.

> But a tiresome regression due to drivers relying on this
> i2c_device_id not being NULL and inability to remove it from the I2C
> core without refactoring the world ensued, see:
> commit c80f52847c50109ca248c22efbf71ff10553dca4

Oh, that was the change...

> Reverted in:
> commit 661f6c1cd926c6c973e03c6b5151d161f3a666ed

> For this reason I think:
> http://marc.info/?l=linux-next&m=137148411231784&w=2

> I have tentatively given up getting pure DT I2C drivers
> to probe, I don't think I have the whole picture, but
> Wolfram has serious doubts about this and say we have
> to be careful ....

> Wolfram, do you have some ideas on how we should
> proceed or ar you happy with merging this as-is?

I'd have expected that it should be possible to change things such that
the change in the core doesn't produce any change in behaviour for
existing drivers. Can we not change the patch so that i2c_match_id()
copes with getting a NULL id_table? Something like this:

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 29d3f04..61087ea 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -67,6 +67,9 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver);
static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
const struct i2c_client *client)
{
+ if (!id)
+ return NULL;
+
while (id->name[0]) {
if (strcmp(client->name, id->name) == 0)
return id;
@@ -92,11 +95,8 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
return 1;

driver = to_i2c_driver(drv);
- /* match on an id table if there is one */
- if (driver->id_table)
- return i2c_match_id(driver->id_table, client) != NULL;

- return 0;
+ return i2c_match_id(driver->id_table, client) != NULL;
}


@@ -246,7 +246,7 @@ static int i2c_device_probe(struct device *dev)
return 0;

driver = to_i2c_driver(dev->driver);
- if (!driver->probe || !driver->id_table)
+ if (!driver->probe)
return -ENODEV;
client->driver = driver;
if (!device_can_wakeup(&client->dev))

Attachment: signature.asc
Description: Digital signature