[PATCH 0/9] i2c: Relax mandatory I2C ID table passing (+ some creep)

[PATCH 0/9] i2c: Relax mandatory I2C ID table passing (+ some creep)

[Lee Jones]

Hi Wolfram,
  
As previously discussed I believe it should be okay for an I2C device
driver _not_ supply an I2C ID table to match to.  The I2C subsystem
should be able to match via other means, such as via OF tables.  The
blocking factor during our previous conversation was to keep
registering via sysfs up and running.  This set does that.
  
After thinking more deeply about the problem, it occurred to me that
any I2C device driver which uses the sysfs method and issues an
of_match_device() would also fail their probe().  Bolted on to this
set is a new, more generic way for these devices to match against
either of the I2C/OF tables.

I hope this ticks all of your boxes.

v2:
  - Removal of ACPI support (this is really an OF issue).
  - Add a new .probe2( with will seamlessly replace
  - Supply a warning on devices matching via OF without a suitable compatible
  - Remove unified match_device() - bad idea as it subverts type-safe behaviour
  - Provide examples of the kind of clean-up possible after this set.
    - I already have the full support from the maintainer of these drivers =;-) 

Kind regards,
Lee

 drivers/i2c/i2c-core.c      | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 drivers/mfd/88pm860x-core.c |  5 ++---
 drivers/mfd/as3722.c        | 12 ++----------
 drivers/of/device.c         | 19 ++++++++++++++++++-
 include/linux/i2c.h         | 29 ++++++++++++++++++++++++++++-
 5 files changed, 114 insertions(+), 24 deletions(-)

[PATCH 1/9] i2c: Add pointer dereference protection to i2c_match_id()

[Lee Jones]

Here we're providing dereference protection for i2c_match_id(), which
saves us having to do it each time it's called.  We're also stripping
out the (now) needless checks in i2c_device_match().  This patch paves
the way for other, similar code trimming.

Acked-by: Grant Likely <grant.likely@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/i2c/i2c-core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 7c7f4b8..d3802dc 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -82,6 +82,9 @@ void i2c_transfer_trace_unreg(void)
 static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 						const struct i2c_client *client)
 {
+	if (!(id && client))
+		return NULL;
+
 	while (id->name[0]) {
 		if (strcmp(client->name, id->name) == 0)
 			return id;
@@ -95,8 +98,6 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
 
-	if (!client)
-		return 0;
 
 	/* Attempt an OF style match */
 	if (of_driver_match_device(dev, drv))
@@ -107,9 +108,10 @@ 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;
+
+	/* Finally an I2C match */
+	if (i2c_match_id(driver->id_table, client))
+		return 1;
 
 	return 0;
 }
-- 
1.8.3.2

[PATCH 2/9] i2c: Add the ability to match device to compatible string without an of_node

[Lee Jones]

A great deal of I2C devices are currently matched via DT node name, and
as such the compatible naming convention of '<vendor>,<device>' has gone
somewhat awry - some nodes don't supply one, some supply an arbitrary
string and others the correct device name with an arbitrary vendor prefix.

In an effort to correct this problem we have to supply a mechanism to
match a device by compatible string AND by simple device name.  This
function strips off the '<vendor>,' part of a supplied compatible string
and attempts to match without it.

The plan is to remove this function once all of the compatible strings
for each device have been brought into line.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/i2c/i2c-core.c | 25 +++++++++++++++++++++++++
 include/linux/i2c.h    | 10 ++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d3802dc..face8f9 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1090,6 +1090,31 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
 	return i2c_verify_adapter(dev);
 }
 EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
+
+const struct of_device_id
+*i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
+				  struct device *dev)
+{
+	const struct i2c_client *client = i2c_verify_client(dev);
+	const char *name;
+
+	if (!(client && matches))
+		return NULL;
+
+	for (; matches->compatible[0]; matches++) {
+		name = strchr(matches->compatible, ',');
+		if (!name)
+			name = matches->compatible;
+		else
+			name++;
+
+		if (!strnicmp(client->name, name, strlen(client->name)))
+			return matches;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(i2c_of_match_device_strip_vendor);
 #else
 static void of_i2c_register_devices(struct i2c_adapter *adap) { }
 #endif /* CONFIG_OF */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b556e0a..f7ec75b 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -564,6 +564,9 @@ extern struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
 /* must call put_device() when done with returned i2c_adapter device */
 extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
 
+extern const struct of_device_id
+*i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
+				  struct device *dev);
 #else
 
 static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
@@ -575,6 +578,13 @@ static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node
 {
 	return NULL;
 }
+
+const struct of_device_id
+*i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
+				  struct device *dev)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 #endif /* _LINUX_I2C_H */
-- 
1.8.3.2

[PATCH 3/9] i2c: Match using traditional OF methods, then by vendor-less compatible strings

[Lee Jones]

This function provides a single call for all I2C devices which need to
match firstly using traditional OF means i.e by of_node, then if that
fails we attempt to match using the supplied I2C client name with a
list of supplied compatible strings with the '<vendor>,' string
removed.  The latter is required due to the unruly naming conventions
used currently by I2C devices.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/i2c/i2c-core.c | 13 +++++++++++++
 include/linux/i2c.h    |  9 +++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index face8f9..80a2f2e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1115,6 +1115,19 @@ const struct of_device_id
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(i2c_of_match_device_strip_vendor);
+
+const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches, struct device *dev)
+{
+	const struct of_device_id *match;
+
+	match = of_match_device(matches, dev);
+	if (match)
+		return match;
+
+	return i2c_of_match_device_strip_vendor(matches, dev);
+}
+EXPORT_SYMBOL_GPL(i2c_of_match_device);
 #else
 static void of_i2c_register_devices(struct i2c_adapter *adap) { }
 #endif /* CONFIG_OF */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index f7ec75b..ab5866f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -565,6 +565,9 @@ extern struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
 extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
 
 extern const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches, struct device *dev);
+
+extern const struct of_device_id
 *i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
 				  struct device *dev);
 #else
@@ -580,6 +583,12 @@ static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node
 }
 
 const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches, struct device *dev)
+{
+	return NULL;
+}
+
+const struct of_device_id
 *i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
 				  struct device *dev)
 {
-- 
1.8.3.2

[PATCH 4/9] i2c: Make I2C ID tables non-mandatory for DT'ed devices

[Lee Jones]

Currently the I2C framework insists on devices supplying an I2C ID
table.  Many of the devices which do so unnecessarily adding quite a
few wasted lines to kernel code.  This patch allows drivers a means
to 'not' supply the aforementioned table and match on DT match tables
instead.

Acked-by: Grant Likely <grant.likely@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/i2c/i2c-core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 80a2f2e..3507087 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -100,7 +100,7 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
 
 
 	/* Attempt an OF style match */
-	if (of_driver_match_device(dev, drv))
+	if (i2c_of_match_device(drv->of_match_table, dev))
 		return 1;
 
 	/* Then ACPI style match */
@@ -268,7 +268,15 @@ 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 -EINVAL;
+
+	/*
+	 * An I2C ID table is not mandatory, if and only if, a suitable Device
+	 * Tree match table entry is supplied for the probing device.
+	 */
+	if (!driver->id_table &&
+	    !i2c_of_match_device(dev->driver->of_match_table, dev))
 		return -ENODEV;
 
 	if (!device_can_wakeup(&client->dev))
-- 
1.8.3.2

[PATCH 5/9] i2c: Export i2c_match_id() for direct use by device drivers

[Lee Jones]

When there was no other way to match a I2C device to driver i2c_match_id()
was exclusively used.  However, now there are other types of tables which
are commonly supplied, matching on an i2c_device_id table is used less
frequently.  Instead of _always_ calling i2c_match_id() from within the
framework, we only need to do so from drivers which have no other way of
matching.  This patch makes i2c_match_id() available to the aforementioned
device drivers.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/i2c/i2c-core.c | 3 ++-
 include/linux/i2c.h    | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3507087..3fe8232 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -79,7 +79,7 @@ void i2c_transfer_trace_unreg(void)
 
 /* ------------------------------------------------------------------------- */
 
-static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
+const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 						const struct i2c_client *client)
 {
 	if (!(id && client))
@@ -92,6 +92,7 @@ static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 	}
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(i2c_match_id);
 
 static int i2c_device_match(struct device *dev, struct device_driver *drv)
 {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index ab5866f..9d36f86 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -229,6 +229,8 @@ struct i2c_client {
 
 extern struct i2c_client *i2c_verify_client(struct device *dev);
 extern struct i2c_adapter *i2c_verify_adapter(struct device *dev);
+extern const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
+						const struct i2c_client *client);
 
 static inline struct i2c_client *kobj_to_i2c_client(struct kobject *kobj)
 {
-- 
1.8.3.2

[PATCH 6/9] i2c: Provide a temporary .probe2() call-back type

[Lee Jones]

This will aid the seamless removal of the current probe()'s, more
commonly unused than used second parameter.  Most I2C drivers can
simply switch over to the new interface, others which have DT
support can use its own matching instead and others can call
i2c_match_id() themselves.  This brings I2C's device probe method
into line with other similar interfaces in the kernel and prevents
the requirement to pass an i2c_device_id table.

Suggested-by: Grant Likely <grant.likely@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/i2c/i2c-core.c | 12 +++++++++---
 include/linux/i2c.h    |  8 +++++++-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3fe8232..7047ea9 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -269,8 +269,6 @@ static int i2c_device_probe(struct device *dev)
 		return 0;
 
 	driver = to_i2c_driver(dev->driver);
-	if (!driver->probe)
-		return -EINVAL;
 
 	/*
 	 * An I2C ID table is not mandatory, if and only if, a suitable Device
@@ -286,7 +284,15 @@ static int i2c_device_probe(struct device *dev)
 	dev_dbg(dev, "probe\n");
 
 	acpi_dev_pm_attach(&client->dev, true);
-	status = driver->probe(client, i2c_match_id(driver->id_table, client));
+
+	/* When there are no more users of probe(), rename probe2 to probe. */
+	if (driver->probe2)
+		status = driver->probe2(client);
+	else if (driver->probe)
+		status = driver->probe(client,
+				       i2c_match_id(driver->id_table, client));
+	else
+		return -EINVAL;
 	if (status)
 		acpi_dev_pm_detach(&client->dev, true);
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 9d36f86..0a470ea 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -125,7 +125,8 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client,
  * struct i2c_driver - represent an I2C device driver
  * @class: What kind of i2c device we instantiate (for detect)
  * @attach_adapter: Callback for bus addition (deprecated)
- * @probe: Callback for device binding
+ * @probe: Callback for device binding - soon to be deprecated
+ * @probe2: New callback for device binding
  * @remove: Callback for device unbinding
  * @shutdown: Callback for device shutdown
  * @suspend: Callback for device suspend
@@ -170,6 +171,11 @@ struct i2c_driver {
 	int (*probe)(struct i2c_client *, const struct i2c_device_id *);
 	int (*remove)(struct i2c_client *);
 
+	/* New driver model interface to aid the seamless removal of the
+	 * current probe()'s, more commonly unused than used second parameter.
+	 */
+	int (*probe2)(struct i2c_client *);
+
 	/* driver model interfaces that don't relate to enumeration  */
 	void (*shutdown)(struct i2c_client *);
 	int (*suspend)(struct i2c_client *, pm_message_t mesg);
-- 
1.8.3.2

[PATCH 7/9] of/device: Allow I2C devices to OF match without supplying an OF node

[Lee Jones]

The I2C framework supplies a means for devices to be registered without
the requirement for platform_data, DT or ACPI.  The current solution is
that every single I2C device in the kernel is forced to supply a
normally empty/sparse I2C ID table so the I2C subsystem can match to.
In an effort to rid the kernel of these tables we need to provide some
temporary work arounds until we can straighten out the blocking factors.

This change is meant to be temporary and will be stripped out once
we've converted all I2C device drivers from of_match_device() over
to the new I2C generic i2c_of_match_device().

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/of/device.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index dafb973..3b1c8f9 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -1,6 +1,7 @@
 #include <linux/string.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
+#include <linux/i2c.h>
 #include <linux/of_device.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -21,9 +22,25 @@
 const struct of_device_id *of_match_device(const struct of_device_id *matches,
 					   const struct device *dev)
 {
+	const struct of_device_id *match;
+
 	if ((!matches) || (!dev->of_node))
 		return NULL;
-	return of_match_node(matches, dev->of_node);
+
+	match = of_match_node(matches, dev->of_node);
+	if (match)
+		return match;
+
+	/*
+	 * Low impact workaround, until we can convert all I2C compatible
+	 * strings over to the correctly specified <vendor>,<device> format.
+	 */
+	match = i2c_of_match_device_strip_vendor(matches, (struct device *)dev);
+	if (match)
+		dev_warn(dev,
+			 "WARNING: Vendor ID missing from compatible string\n");
+
+	return match;
 }
 EXPORT_SYMBOL(of_match_device);
 
-- 
1.8.3.2

[PATCH 8/9] mfd: 88pm860x: Move over to new I2C device .probe() call

[Lee Jones]

As part of an effort to rid the mostly unused second parameter for I2C
related .probe() functions and to conform to other existing frameworks
we're moving over to a temporary replacement .probe() call-back.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/88pm860x-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index bcfc9e8..d763f1f 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1127,8 +1127,7 @@ static int pm860x_dt_init(struct device_node *np,
 	return 0;
 }
 
-static int pm860x_probe(struct i2c_client *client,
-				  const struct i2c_device_id *id)
+static int pm860x_probe(struct i2c_client *client)
 {
 	struct pm860x_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct device_node *node = client->dev.of_node;
@@ -1255,7 +1254,7 @@ static struct i2c_driver pm860x_driver = {
 		.pm     = &pm860x_pm_ops,
 		.of_match_table	= pm860x_dt_ids,
 	},
-	.probe		= pm860x_probe,
+	.probe2		= pm860x_probe,
 	.remove		= pm860x_remove,
 	.id_table	= pm860x_id_table,
 };
-- 
1.8.3.2

[PATCH 9/9] mfd: as3722: Rid driver of superfluous I2C device ID structure

[Lee Jones]

Also remove unused second probe() parameter 'i2c_device_id'.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/as3722.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
index 39fa554..cb2fcf9 100644
--- a/drivers/mfd/as3722.c
+++ b/drivers/mfd/as3722.c
@@ -354,8 +354,7 @@ static int as3722_i2c_of_probe(struct i2c_client *i2c,
 	return 0;
 }
 
-static int as3722_i2c_probe(struct i2c_client *i2c,
-			const struct i2c_device_id *id)
+static int as3722_i2c_probe(struct i2c_client *i2c)
 {
 	struct as3722 *as3722;
 	unsigned long irq_flags;
@@ -428,21 +427,14 @@ static const struct of_device_id as3722_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, as3722_of_match);
 
-static const struct i2c_device_id as3722_i2c_id[] = {
-	{ "as3722", 0 },
-	{},
-};
-MODULE_DEVICE_TABLE(i2c, as3722_i2c_id);
-
 static struct i2c_driver as3722_i2c_driver = {
 	.driver = {
 		.name = "as3722",
 		.owner = THIS_MODULE,
 		.of_match_table = as3722_of_match,
 	},
-	.probe = as3722_i2c_probe,
+	.probe2 = as3722_i2c_probe,
 	.remove = as3722_i2c_remove,
-	.id_table = as3722_i2c_id,
 };
 
 module_i2c_driver(as3722_i2c_driver);
-- 
1.8.3.2

[PATCH 1/9] i2c: Add pointer dereference protection to i2c_match_id()

[Grant Likely]

On Fri,  6 Jun 2014 16:52:24 +0100, Lee Jones <lee.jones@linaro.org> wrote:
> Here we're providing dereference protection for i2c_match_id(), which
> saves us having to do it each time it's called.  We're also stripping
> out the (now) needless checks in i2c_device_match().  This patch paves
> the way for other, similar code trimming.
> 
> Acked-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/i2c/i2c-core.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 7c7f4b8..d3802dc 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -82,6 +82,9 @@ void i2c_transfer_trace_unreg(void)
>  static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
>  						const struct i2c_client *client)
>  {
> +	if (!(id && client))
> +		return NULL;
> +
>  	while (id->name[0]) {
>  		if (strcmp(client->name, id->name) == 0)
>  			return id;
> @@ -95,8 +98,6 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
>  	struct i2c_client	*client = i2c_verify_client(dev);
>  	struct i2c_driver	*driver;
>  
> -	if (!client)
> -		return 0;
>  
>  	/* Attempt an OF style match */
>  	if (of_driver_match_device(dev, drv))
> @@ -107,9 +108,10 @@ 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;
> +
> +	/* Finally an I2C match */
> +	if (i2c_match_id(driver->id_table, client))
> +		return 1;
>  
>  	return 0;

Nit: the above 4 lines can now simply be:

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

g.

[PATCH 3/9] i2c: Match using traditional OF methods, then by vendor-less compatible strings

[Grant Likely]

On Fri,  6 Jun 2014 16:52:26 +0100, Lee Jones <lee.jones@linaro.org> wrote:
> This function provides a single call for all I2C devices which need to
> match firstly using traditional OF means i.e by of_node, then if that
> fails we attempt to match using the supplied I2C client name with a
> list of supplied compatible strings with the '<vendor>,' string
> removed.  The latter is required due to the unruly naming conventions
> used currently by I2C devices.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/i2c/i2c-core.c | 13 +++++++++++++
>  include/linux/i2c.h    |  9 +++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index face8f9..80a2f2e 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1115,6 +1115,19 @@ const struct of_device_id
>  	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(i2c_of_match_device_strip_vendor);
> +
> +const struct of_device_id
> +*i2c_of_match_device(const struct of_device_id *matches, struct device *dev)

Should be:
const struct of_device_id *
i2c_of_match_device(const struct of_device_id *matches, struct i2c_client *dev)

Since it is only applicable to i2c_client. Don't allow random subsystems
to call into it with any struct device (even if there is little risk due
to the name and location of the function). Same comment goes for
i2c_of_match_device_strip_vendor().

In fact, i2c_of_match_device_strip_vendor() should be rolled directly
into i2c_of_match_device(). I know you're calling it in patch 7/9, but
that needs to be removed. I'll comment directly on that patch though.

g.

> +{
> +	const struct of_device_id *match;
> +
> +	match = of_match_device(matches, dev);
> +	if (match)
> +		return match;
> +
> +	return i2c_of_match_device_strip_vendor(matches, dev);
> +}
> +EXPORT_SYMBOL_GPL(i2c_of_match_device);
>  #else
>  static void of_i2c_register_devices(struct i2c_adapter *adap) { }
>  #endif /* CONFIG_OF */
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index f7ec75b..ab5866f 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -565,6 +565,9 @@ extern struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
>  extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
>  
>  extern const struct of_device_id
> +*i2c_of_match_device(const struct of_device_id *matches, struct device *dev);
> +
> +extern const struct of_device_id
>  *i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
>  				  struct device *dev);
>  #else
> @@ -580,6 +583,12 @@ static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node
>  }
>  
>  const struct of_device_id
> +*i2c_of_match_device(const struct of_device_id *matches, struct device *dev)
> +{
> +	return NULL;
> +}
> +
> +const struct of_device_id
>  *i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
>  				  struct device *dev)
>  {
> -- 
> 1.8.3.2
> 

[PATCH 7/9] of/device: Allow I2C devices to OF match without supplying an OF node

[Grant Likely]

On Fri,  6 Jun 2014 16:52:30 +0100, Lee Jones <lee.jones@linaro.org> wrote:
> The I2C framework supplies a means for devices to be registered without
> the requirement for platform_data, DT or ACPI.  The current solution is
> that every single I2C device in the kernel is forced to supply a
> normally empty/sparse I2C ID table so the I2C subsystem can match to.
> In an effort to rid the kernel of these tables we need to provide some
> temporary work arounds until we can straighten out the blocking factors.
> 
> This change is meant to be temporary and will be stripped out once
> we've converted all I2C device drivers from of_match_device() over
> to the new I2C generic i2c_of_match_device().

This change imposes the i2c workaround onto every single caller of
i2c_of_match_device(), regardless of subsystem, and regardless of the
desired behaviour. It also prevents an i2c driver from getting the
correct behavour when it wants it.

So, no, I'm not okay with hooking into the core code to work around the
subsystem problem. I2C drivers need to call the i2c specific version
directly. I know that adds a line of change to the driver patches, but I
think it is necessary.

g.

> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/of/device.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index dafb973..3b1c8f9 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -1,6 +1,7 @@
>  #include <linux/string.h>
>  #include <linux/kernel.h>
>  #include <linux/of.h>
> +#include <linux/i2c.h>
>  #include <linux/of_device.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
> @@ -21,9 +22,25 @@
>  const struct of_device_id *of_match_device(const struct of_device_id *matches,
>  					   const struct device *dev)
>  {
> +	const struct of_device_id *match;
> +
>  	if ((!matches) || (!dev->of_node))
>  		return NULL;
> -	return of_match_node(matches, dev->of_node);
> +
> +	match = of_match_node(matches, dev->of_node);
> +	if (match)
> +		return match;
> +
> +	/*
> +	 * Low impact workaround, until we can convert all I2C compatible
> +	 * strings over to the correctly specified <vendor>,<device> format.
> +	 */
> +	match = i2c_of_match_device_strip_vendor(matches, (struct device *)dev);
> +	if (match)
> +		dev_warn(dev,
> +			 "WARNING: Vendor ID missing from compatible string\n");
> +
> +	return match;
>  }
>  EXPORT_SYMBOL(of_match_device);
>  
> -- 
> 1.8.3.2
> 

[PATCH 0/9] i2c: Relax mandatory I2C ID table passing (+ some creep)

[Grant Likely]

On Fri,  6 Jun 2014 16:52:23 +0100, Lee Jones <lee.jones@linaro.org> wrote:
> Hi Wolfram,
>   
> As previously discussed I believe it should be okay for an I2C device
> driver _not_ supply an I2C ID table to match to.  The I2C subsystem
> should be able to match via other means, such as via OF tables.  The
> blocking factor during our previous conversation was to keep
> registering via sysfs up and running.  This set does that.
>   
> After thinking more deeply about the problem, it occurred to me that
> any I2C device driver which uses the sysfs method and issues an
> of_match_device() would also fail their probe().  Bolted on to this
> set is a new, more generic way for these devices to match against
> either of the I2C/OF tables.
> 
> I hope this ticks all of your boxes.
> 
> v2:
>   - Removal of ACPI support (this is really an OF issue).
>   - Add a new .probe2( with will seamlessly replace
>   - Supply a warning on devices matching via OF without a suitable compatible
>   - Remove unified match_device() - bad idea as it subverts type-safe behaviour
>   - Provide examples of the kind of clean-up possible after this set.
>     - I already have the full support from the maintainer of these drivers =;-) 

Aside from patch 7/9 and the comment I made about struct device vs.
struct i2c_client in the API, I think this series looks good. You can
add my acks to 1-6 and 8-9. Number 7 is nacked.

Acked-by: Grant Likely <grant.likely@linaro.org>

g.

> 
> Kind regards,
> Lee
> 
>  drivers/i2c/i2c-core.c      | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  drivers/mfd/88pm860x-core.c |  5 ++---
>  drivers/mfd/as3722.c        | 12 ++----------
>  drivers/of/device.c         | 19 ++++++++++++++++++-
>  include/linux/i2c.h         | 29 ++++++++++++++++++++++++++++-
>  5 files changed, 114 insertions(+), 24 deletions(-)
>