[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

Hi Wolfram,

Placing this firmly back on your plate.  I truly hope we don't miss
another merge-window.  This patch-set has the support of some pretty
senior kernel maintainers, so I hope acceptance shouldn't be too
difficult.

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.

v3:
  - Insist on passing 'struct i2c_client' instead of 'struct device'
  - Remove hook from of_match_device()

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

Lee Jones (8):
  i2c: Add pointer dereference protection to i2c_match_id()
  i2c: Add the ability to match device to compatible string without an
    of_node
  i2c: Match using traditional OF methods, then by vendor-less
    compatible strings
  i2c: Make I2C ID tables non-mandatory for DT'ed devices
  i2c: Export i2c_match_id() for direct use by device drivers
  i2c: Provide a temporary .probe2() call-back type
  mfd: 88pm860x: Move over to new I2C device .probe() call
  mfd: as3722: Rid driver of superfluous I2C device ID structure

 drivers/i2c/i2c-core.c      | 72 +++++++++++++++++++++++++++++++++++++++------
 drivers/mfd/88pm860x-core.c |  5 ++--
 drivers/mfd/as3722.c        | 12 ++------
 include/linux/i2c.h         | 22 +++++++++++++-
 4 files changed, 88 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

Hi Wolfram,

Placing this firmly back on your plate.  I truly hope we don't miss
another merge-window.  This patch-set has the support of some pretty
senior kernel maintainers, so I hope acceptance shouldn't be too
difficult.

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.

v3:
  - Insist on passing 'struct i2c_client' instead of 'struct device'
  - Remove hook from of_match_device()

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

Lee Jones (8):
  i2c: Add pointer dereference protection to i2c_match_id()
  i2c: Add the ability to match device to compatible string without an
    of_node
  i2c: Match using traditional OF methods, then by vendor-less
    compatible strings
  i2c: Make I2C ID tables non-mandatory for DT'ed devices
  i2c: Export i2c_match_id() for direct use by device drivers
  i2c: Provide a temporary .probe2() call-back type
  mfd: 88pm860x: Move over to new I2C device .probe() call
  mfd: as3722: Rid driver of superfluous I2C device ID structure

 drivers/i2c/i2c-core.c      | 72 +++++++++++++++++++++++++++++++++++++++------
 drivers/mfd/88pm860x-core.c |  5 ++--
 drivers/mfd/as3722.c        | 12 ++------
 include/linux/i2c.h         | 22 +++++++++++++-
 4 files changed, 88 insertions(+), 23 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

Hi Wolfram,

Placing this firmly back on your plate.  I truly hope we don't miss
another merge-window.  This patch-set has the support of some pretty
senior kernel maintainers, so I hope acceptance shouldn't be too
difficult.

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.

v3:
  - Insist on passing 'struct i2c_client' instead of 'struct device'
  - Remove hook from of_match_device()

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

Lee Jones (8):
  i2c: Add pointer dereference protection to i2c_match_id()
  i2c: Add the ability to match device to compatible string without an
    of_node
  i2c: Match using traditional OF methods, then by vendor-less
    compatible strings
  i2c: Make I2C ID tables non-mandatory for DT'ed devices
  i2c: Export i2c_match_id() for direct use by device drivers
  i2c: Provide a temporary .probe2() call-back type
  mfd: 88pm860x: Move over to new I2C device .probe() call
  mfd: as3722: Rid driver of superfluous I2C device ID structure

 drivers/i2c/i2c-core.c      | 72 +++++++++++++++++++++++++++++++++++++++------
 drivers/mfd/88pm860x-core.c |  5 ++--
 drivers/mfd/as3722.c        | 12 ++------
 include/linux/i2c.h         | 22 +++++++++++++-
 4 files changed, 88 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH 1/8] 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 632057a..d3c8e9f 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -83,6 +83,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;
@@ -96,8 +99,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))
@@ -108,9 +109,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.9.1

[PATCH 1/8] 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 632057a..d3c8e9f 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -83,6 +83,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;
@@ -96,8 +99,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))
@@ -108,9 +109,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.9.1

[PATCH 2/8] 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.

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

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d3c8e9f..eb46d15 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1095,6 +1095,27 @@ 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);
+
+static const struct of_device_id*
+i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
+				  struct i2c_client *client)
+{
+	const char *name;
+
+	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;
+}
+
 #else
 static void of_i2c_register_devices(struct i2c_adapter *adap) { }
 #endif /* CONFIG_OF */
-- 
1.9.1

[PATCH 2/8] 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.

Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d3c8e9f..eb46d15 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1095,6 +1095,27 @@ 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);
+
+static const struct of_device_id*
+i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
+				  struct i2c_client *client)
+{
+	const char *name;
+
+	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;
+}
+
 #else
 static void of_i2c_register_devices(struct i2c_adapter *adap) { }
 #endif /* CONFIG_OF */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/8] 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.

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

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d3c8e9f..eb46d15 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1095,6 +1095,27 @@ 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);
+
+static const struct of_device_id*
+i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
+				  struct i2c_client *client)
+{
+	const char *name;
+
+	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;
+}
+
 #else
 static void of_i2c_register_devices(struct i2c_adapter *adap) { }
 #endif /* CONFIG_OF */
-- 
1.9.1

[PATCH 3/8] 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.

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

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index eb46d15..5728b2e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1116,6 +1116,22 @@ i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
 	return NULL;
 }
 
+const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches,
+		     struct i2c_client *client)
+{
+	const struct of_device_id *match;
+
+	if (!(client && matches))
+		return NULL;
+
+	match = of_match_device(matches, &client->dev);
+	if (match)
+		return match;
+
+	return i2c_of_match_device_strip_vendor(matches, client);
+}
+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 a95efeb..a48eadd 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -564,6 +564,10 @@ 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(const struct of_device_id *matches,
+		     struct i2c_client *client);
+
 #else
 
 static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
@@ -575,6 +579,14 @@ 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(const struct of_device_id *matches,
+		     struct i2c_client *client)
+{
+	return NULL;
+}
+
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_ACPI
-- 
1.9.1

[PATCH 3/8] 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.

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

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index eb46d15..5728b2e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1116,6 +1116,22 @@ i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
 	return NULL;
 }
 
+const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches,
+		     struct i2c_client *client)
+{
+	const struct of_device_id *match;
+
+	if (!(client && matches))
+		return NULL;
+
+	match = of_match_device(matches, &client->dev);
+	if (match)
+		return match;
+
+	return i2c_of_match_device_strip_vendor(matches, client);
+}
+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 a95efeb..a48eadd 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -564,6 +564,10 @@ 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(const struct of_device_id *matches,
+		     struct i2c_client *client);
+
 #else
 
 static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
@@ -575,6 +579,14 @@ 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(const struct of_device_id *matches,
+		     struct i2c_client *client)
+{
+	return NULL;
+}
+
 #endif /* CONFIG_OF */
 
 #ifdef CONFIG_ACPI
-- 
1.9.1

[PATCH 4/8] 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 5728b2e..c6013cd 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -101,7 +101,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, client))
 		return 1;
 
 	/* Then ACPI style match */
@@ -269,7 +269,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, client))
 		return -ENODEV;
 
 	if (!device_can_wakeup(&client->dev))
-- 
1.9.1

[PATCH 4/8] 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 5728b2e..c6013cd 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -101,7 +101,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, client))
 		return 1;
 
 	/* Then ACPI style match */
@@ -269,7 +269,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, client))
 		return -ENODEV;
 
 	if (!device_can_wakeup(&client->dev))
-- 
1.9.1

[PATCH 5/8] 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.

Acked-by: Grant Likely <grant.likely@linaro.org>
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 c6013cd..7c21f49 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -80,7 +80,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))
@@ -93,6 +93,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 a48eadd..79b674d 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.9.1

[PATCH 5/8] 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.

Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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 c6013cd..7c21f49 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -80,7 +80,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))
@@ -93,6 +93,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 a48eadd..79b674d 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.9.1

[PATCH 5/8] 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.

Acked-by: Grant Likely <grant.likely@linaro.org>
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 c6013cd..7c21f49 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -80,7 +80,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))
@@ -93,6 +93,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 a48eadd..79b674d 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.9.1

[PATCH 6/8] 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 7c21f49..5c74bd0 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -270,8 +270,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
@@ -291,7 +289,15 @@ static int i2c_device_probe(struct device *dev)
 		return status;
 
 	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 79b674d..c8240e5 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.9.1

[PATCH 6/8] 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 7c21f49..5c74bd0 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -270,8 +270,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
@@ -291,7 +289,15 @@ static int i2c_device_probe(struct device *dev)
 		return status;
 
 	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 79b674d..c8240e5 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.9.1

[PATCH 7/8] 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.

Acked-by: Grant Likely <grant.likely@linaro.org>
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 3a26045..3372c47 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1134,8 +1134,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;
@@ -1262,7 +1261,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.9.1

[PATCH 7/8] 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.

Acked-by: Grant Likely <grant.likely@linaro.org>
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 3a26045..3372c47 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1134,8 +1134,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;
@@ -1262,7 +1261,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.9.1

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

[Lee Jones]

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

Acked-by: Grant Likely <grant.likely@linaro.org>
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.9.1

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

[Lee Jones]

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

Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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.9.1

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

[Lee Jones]

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

Acked-by: Grant Likely <grant.likely@linaro.org>
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.9.1

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

Hi Lee,

> Placing this firmly back on your plate.  I truly hope we don't miss
> another merge-window.

Nope, we won't. I'll still need a week or so due to other duties.

> This patch-set has the support of some pretty
> senior kernel maintainers, so I hope acceptance shouldn't be too
> difficult.

Cool, then they could ack it like Grant did? That surely helps.

> As previously discussed I believe it should be okay for an I2C device
> driver _not_ supply an I2C ID table to match to.

I agree...

> 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.

... yet it also should not cause regressions. If you fixed that, sounds
great!

> 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.

Even better :) I am generally positive with your patchset, but need to
review the implementation. For core stuff, this simply needs more
attention.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

Hi Lee,

> Placing this firmly back on your plate.  I truly hope we don't miss
> another merge-window.

Nope, we won't. I'll still need a week or so due to other duties.

> This patch-set has the support of some pretty
> senior kernel maintainers, so I hope acceptance shouldn't be too
> difficult.

Cool, then they could ack it like Grant did? That surely helps.

> As previously discussed I believe it should be okay for an I2C device
> driver _not_ supply an I2C ID table to match to.

I agree...

> 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.

... yet it also should not cause regressions. If you fixed that, sounds
great!

> 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.

Even better :) I am generally positive with your patchset, but need to
review the implementation. For core stuff, this simply needs more
attention.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Wolfram Sang]

Hi Lee,

> Placing this firmly back on your plate.  I truly hope we don't miss
> another merge-window.

Nope, we won't. I'll still need a week or so due to other duties.

> This patch-set has the support of some pretty
> senior kernel maintainers, so I hope acceptance shouldn't be too
> difficult.

Cool, then they could ack it like Grant did? That surely helps.

> As previously discussed I believe it should be okay for an I2C device
> driver _not_ supply an I2C ID table to match to.

I agree...

> 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.

... yet it also should not cause regressions. If you fixed that, sounds
great!

> 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.

Even better :) I am generally positive with your patchset, but need to
review the implementation. For core stuff, this simply needs more
attention.

Thanks,

   Wolfram

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140829/d14b349c/attachment.sig>

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

On Fri, 29 Aug 2014, Wolfram Sang wrote:
> > Placing this firmly back on your plate.  I truly hope we don't miss
> > another merge-window.
> 
> Nope, we won't. I'll still need a week or so due to other duties.

Perfectly reasonable.

> > This patch-set has the support of some pretty
> > senior kernel maintainers, so I hope acceptance shouldn't be too
> > difficult.
> 
> Cool, then they could ack it like Grant did? That surely helps.

I was talking about Grant (and Linus - I'll poke him seperately). ;)

> > As previously discussed I believe it should be okay for an I2C device
> > driver _not_ supply an I2C ID table to match to.
> 
> I agree...
> 
> > 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.
> 
> ... yet it also should not cause regressions. If you fixed that, sounds
> great!
>
> > 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.
> 
> Even better :) I am generally positive with your patchset, but need to
> review the implementation. For core stuff, this simply needs more
> attention.

Agree.

Thanks Wolfram.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

On Fri, 29 Aug 2014, Wolfram Sang wrote:
> > Placing this firmly back on your plate.  I truly hope we don't miss
> > another merge-window.
> 
> Nope, we won't. I'll still need a week or so due to other duties.

Perfectly reasonable.

> > This patch-set has the support of some pretty
> > senior kernel maintainers, so I hope acceptance shouldn't be too
> > difficult.
> 
> Cool, then they could ack it like Grant did? That surely helps.

I was talking about Grant (and Linus - I'll poke him seperately). ;)

> > As previously discussed I believe it should be okay for an I2C device
> > driver _not_ supply an I2C ID table to match to.
> 
> I agree...
> 
> > 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.
> 
> ... yet it also should not cause regressions. If you fixed that, sounds
> great!
>
> > 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.
> 
> Even better :) I am generally positive with your patchset, but need to
> review the implementation. For core stuff, this simply needs more
> attention.

Agree.

Thanks Wolfram.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

On Fri, 29 Aug 2014, Wolfram Sang wrote:
> > Placing this firmly back on your plate.  I truly hope we don't miss
> > another merge-window.
> 
> Nope, we won't. I'll still need a week or so due to other duties.

Perfectly reasonable.

> > This patch-set has the support of some pretty
> > senior kernel maintainers, so I hope acceptance shouldn't be too
> > difficult.
> 
> Cool, then they could ack it like Grant did? That surely helps.

I was talking about Grant (and Linus - I'll poke him seperately). ;)

> > As previously discussed I believe it should be okay for an I2C device
> > driver _not_ supply an I2C ID table to match to.
> 
> I agree...
> 
> > 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.
> 
> ... yet it also should not cause regressions. If you fixed that, sounds
> great!
>
> > 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.
> 
> Even better :) I am generally positive with your patchset, but need to
> review the implementation. For core stuff, this simply needs more
> attention.

Agree.

Thanks Wolfram.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

On Thu, Aug 28, 2014 at 03:35:30PM +0100, Lee Jones wrote:
> Hi Wolfram,
> 
> Placing this firmly back on your plate.  I truly hope we don't miss
> another merge-window.  This patch-set has the support of some pretty
> senior kernel maintainers, so I hope acceptance shouldn't be too
> difficult.
> 
> 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.

As mentioned in another thread, modaliases are one other possible side
effect. As Javier correctly mentions, the beaviour does not really
change with your patchset. Yet, if we remove i2c_device_id from drivers
too carelessly, they might not be bound anymore.

> 
> 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.
> 
> v3:
>   - Insist on passing 'struct i2c_client' instead of 'struct device'
>   - Remove hook from of_match_device()
> 
> 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
> 
> Lee Jones (8):
>   i2c: Add pointer dereference protection to i2c_match_id()
>   i2c: Add the ability to match device to compatible string without an
>     of_node
>   i2c: Match using traditional OF methods, then by vendor-less
>     compatible strings
>   i2c: Make I2C ID tables non-mandatory for DT'ed devices
>   i2c: Export i2c_match_id() for direct use by device drivers
>   i2c: Provide a temporary .probe2() call-back type
>   mfd: 88pm860x: Move over to new I2C device .probe() call
>   mfd: as3722: Rid driver of superfluous I2C device ID structure
> 
>  drivers/i2c/i2c-core.c      | 72 +++++++++++++++++++++++++++++++++++++++------
>  drivers/mfd/88pm860x-core.c |  5 ++--
>  drivers/mfd/as3722.c        | 12 ++------
>  include/linux/i2c.h         | 22 +++++++++++++-
>  4 files changed, 88 insertions(+), 23 deletions(-)
> 
> -- 
> 1.9.1
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

On Thu, Aug 28, 2014 at 03:35:30PM +0100, Lee Jones wrote:
> Hi Wolfram,
> 
> Placing this firmly back on your plate.  I truly hope we don't miss
> another merge-window.  This patch-set has the support of some pretty
> senior kernel maintainers, so I hope acceptance shouldn't be too
> difficult.
> 
> 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.

As mentioned in another thread, modaliases are one other possible side
effect. As Javier correctly mentions, the beaviour does not really
change with your patchset. Yet, if we remove i2c_device_id from drivers
too carelessly, they might not be bound anymore.

> 
> 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.
> 
> v3:
>   - Insist on passing 'struct i2c_client' instead of 'struct device'
>   - Remove hook from of_match_device()
> 
> 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
> 
> Lee Jones (8):
>   i2c: Add pointer dereference protection to i2c_match_id()
>   i2c: Add the ability to match device to compatible string without an
>     of_node
>   i2c: Match using traditional OF methods, then by vendor-less
>     compatible strings
>   i2c: Make I2C ID tables non-mandatory for DT'ed devices
>   i2c: Export i2c_match_id() for direct use by device drivers
>   i2c: Provide a temporary .probe2() call-back type
>   mfd: 88pm860x: Move over to new I2C device .probe() call
>   mfd: as3722: Rid driver of superfluous I2C device ID structure
> 
>  drivers/i2c/i2c-core.c      | 72 +++++++++++++++++++++++++++++++++++++++------
>  drivers/mfd/88pm860x-core.c |  5 ++--
>  drivers/mfd/as3722.c        | 12 ++------
>  include/linux/i2c.h         | 22 +++++++++++++-
>  4 files changed, 88 insertions(+), 23 deletions(-)
> 
> -- 
> 1.9.1
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Wolfram Sang]

On Thu, Aug 28, 2014 at 03:35:30PM +0100, Lee Jones wrote:
> Hi Wolfram,
> 
> Placing this firmly back on your plate.  I truly hope we don't miss
> another merge-window.  This patch-set has the support of some pretty
> senior kernel maintainers, so I hope acceptance shouldn't be too
> difficult.
> 
> 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.

As mentioned in another thread, modaliases are one other possible side
effect. As Javier correctly mentions, the beaviour does not really
change with your patchset. Yet, if we remove i2c_device_id from drivers
too carelessly, they might not be bound anymore.

> 
> 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.
> 
> v3:
>   - Insist on passing 'struct i2c_client' instead of 'struct device'
>   - Remove hook from of_match_device()
> 
> 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
> 
> Lee Jones (8):
>   i2c: Add pointer dereference protection to i2c_match_id()
>   i2c: Add the ability to match device to compatible string without an
>     of_node
>   i2c: Match using traditional OF methods, then by vendor-less
>     compatible strings
>   i2c: Make I2C ID tables non-mandatory for DT'ed devices
>   i2c: Export i2c_match_id() for direct use by device drivers
>   i2c: Provide a temporary .probe2() call-back type
>   mfd: 88pm860x: Move over to new I2C device .probe() call
>   mfd: as3722: Rid driver of superfluous I2C device ID structure
> 
>  drivers/i2c/i2c-core.c      | 72 +++++++++++++++++++++++++++++++++++++++------
>  drivers/mfd/88pm860x-core.c |  5 ++--
>  drivers/mfd/as3722.c        | 12 ++------
>  include/linux/i2c.h         | 22 +++++++++++++-
>  4 files changed, 88 insertions(+), 23 deletions(-)
> 
> -- 
> 1.9.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140912/33a1ade2/attachment.sig>

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2202 bytes --]

On Thu, Aug 28, 2014 at 03:35:32PM +0100, Lee Jones wrote:
> 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.

Is this really the plan? I mean, the old matching mechanism has been out
there for ages and I dunno how many already deployed DTs depend on it.
I'd think we need to keep this around forever.

> 
> Acked-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/i2c/i2c-core.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index d3c8e9f..eb46d15 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1095,6 +1095,27 @@ 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);
> +
> +static const struct of_device_id*
> +i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
> +				  struct i2c_client *client)
> +{
> +	const char *name;
> +
> +	for (; matches->compatible[0]; matches++) {
> +		name = strchr(matches->compatible, ',');
> +		if (!name)
> +			name = matches->compatible;
> +		else
> +			name++;
> +
> +		if (!strnicmp(client->name, name, strlen(client->name)))

Are compatible-properties case-independent? I though they were not.

> +			return matches;
> +	}
> +
> +	return NULL;
> +}
> +
>  #else
>  static void of_i2c_register_devices(struct i2c_adapter *adap) { }
>  #endif /* CONFIG_OF */
> -- 
> 1.9.1
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Wolfram Sang]

On Thu, Aug 28, 2014 at 03:35:32PM +0100, Lee Jones wrote:
> 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.

Is this really the plan? I mean, the old matching mechanism has been out
there for ages and I dunno how many already deployed DTs depend on it.
I'd think we need to keep this around forever.

> 
> Acked-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/i2c/i2c-core.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index d3c8e9f..eb46d15 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1095,6 +1095,27 @@ 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);
> +
> +static const struct of_device_id*
> +i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
> +				  struct i2c_client *client)
> +{
> +	const char *name;
> +
> +	for (; matches->compatible[0]; matches++) {
> +		name = strchr(matches->compatible, ',');
> +		if (!name)
> +			name = matches->compatible;
> +		else
> +			name++;
> +
> +		if (!strnicmp(client->name, name, strlen(client->name)))

Are compatible-properties case-independent? I though they were not.

> +			return matches;
> +	}
> +
> +	return NULL;
> +}
> +
>  #else
>  static void of_i2c_register_devices(struct i2c_adapter *adap) { }
>  #endif /* CONFIG_OF */
> -- 
> 1.9.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140912/b13801fa/attachment-0001.sig>

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]

> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 79b674d..c8240e5 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

I don't like the naming. What about 'simplified_probe' instead of
'probe2'? The old 'probe' would be deprecated immediately, I guess.

Also, this needs more information in the kernel docs. Come to think of
it, I'd like to see some more documentation in general. A document in
Documentation/i2c should explain the differences between the
probe-functions, the reason why they are there, the intended path to
migrate over and examples how to convert drivers and what to keep in
mind doing so.

>   * @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.

"largely unused second parameter"? Maybe more readable? At least, I
needed to read this sentence more than once to get it. BTW have you
measured how often it was used/unused?

> +	 */
> +	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);

What I like in general: After your series I2C does more like what other
subsystems (especially SPI) do.

What I dislike about this in general: Putting to the drivers that they
should find out about the matching. I mean the core already did the
matching, so why should a driver do that again? Especially since it can
get very cumbersome to do so with platform_data, DT and ACPI around
which need to be checked. This is why I stopped my implementation to
fix this issue. I felt the need for a helper function to assist the
drivers how they were matched. Ideally by just retrieving the
information which the subsystem cores already gathered. Making such a
thing generic was where I stopped working on that.

Has this been discussed somewhere already?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]

> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 79b674d..c8240e5 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

I don't like the naming. What about 'simplified_probe' instead of
'probe2'? The old 'probe' would be deprecated immediately, I guess.

Also, this needs more information in the kernel docs. Come to think of
it, I'd like to see some more documentation in general. A document in
Documentation/i2c should explain the differences between the
probe-functions, the reason why they are there, the intended path to
migrate over and examples how to convert drivers and what to keep in
mind doing so.

>   * @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.

"largely unused second parameter"? Maybe more readable? At least, I
needed to read this sentence more than once to get it. BTW have you
measured how often it was used/unused?

> +	 */
> +	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);

What I like in general: After your series I2C does more like what other
subsystems (especially SPI) do.

What I dislike about this in general: Putting to the drivers that they
should find out about the matching. I mean the core already did the
matching, so why should a driver do that again? Especially since it can
get very cumbersome to do so with platform_data, DT and ACPI around
which need to be checked. This is why I stopped my implementation to
fix this issue. I felt the need for a helper function to assist the
drivers how they were matched. Ideally by just retrieving the
information which the subsystem cores already gathered. Making such a
thing generic was where I stopped working on that.

Has this been discussed somewhere already?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Wolfram Sang]

> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 79b674d..c8240e5 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

I don't like the naming. What about 'simplified_probe' instead of
'probe2'? The old 'probe' would be deprecated immediately, I guess.

Also, this needs more information in the kernel docs. Come to think of
it, I'd like to see some more documentation in general. A document in
Documentation/i2c should explain the differences between the
probe-functions, the reason why they are there, the intended path to
migrate over and examples how to convert drivers and what to keep in
mind doing so.

>   * @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.

"largely unused second parameter"? Maybe more readable? At least, I
needed to read this sentence more than once to get it. BTW have you
measured how often it was used/unused?

> +	 */
> +	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);

What I like in general: After your series I2C does more like what other
subsystems (especially SPI) do.

What I dislike about this in general: Putting to the drivers that they
should find out about the matching. I mean the core already did the
matching, so why should a driver do that again? Especially since it can
get very cumbersome to do so with platform_data, DT and ACPI around
which need to be checked. This is why I stopped my implementation to
fix this issue. I felt the need for a helper function to assist the
drivers how they were matched. Ideally by just retrieving the
information which the subsystem cores already gathered. Making such a
thing generic was where I stopped working on that.

Has this been discussed somewhere already?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140912/84f9e089/attachment.sig>

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Javier Martinez Canillas]

[adding Sjoerd as cc who was the one that raised the module auto-loading issue]

Hello,

On Fri, Sep 12, 2014 at 3:46 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>
>> Placing this firmly back on your plate.  I truly hope we don't miss
>> another merge-window.  This patch-set has the support of some pretty
>> senior kernel maintainers, so I hope acceptance shouldn't be too
>> difficult.
>>
>> 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.
>
> As mentioned in another thread, modaliases are one other possible side
> effect. As Javier correctly mentions, the beaviour does not really
> change with your patchset. Yet, if we remove i2c_device_id from drivers
> too carelessly, they might not be bound anymore.
>

Right, removing the I2C ID table even from drivers that don't really
need it (e.g: IP blocks only present in DT platforms) will as you said
break module auto-loading. Probing will work since the OF table is
used to match the device in i2c_device_match() but is the I2C table
what is used to fill the valid module aliases with the current
behavior of i2c_device_uevent(), the aliases filled from the OF table
are never used.

So what I propose is to do it incrementally:

1) Merge Lee's series since that is definitely a step in the right
direction to not make an I2C table mandatory (still will be needed for
module auto loading though).

2) On a follow-up series, make sure that all I2C drivers have a proper
OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
fill the of:<foo> module aliases. That way the modules will have the
proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
(even when always i2c:<foo> is reported to user-space currently).

3) Apply the patch I posted [0] that changes the behavior of
i2c_device_uevent() to report the OF uevent env vars to user-space in
case of DT probing which after 2) should not regress any driver module
auto-loading since all drivers should fill the of:<foo> aliases.

After this, DT-only drivers will only need an OF table and legacy
drivers will only need an I2C table. Drivers that support both will
still need the two tables though which is a drawback of this approach
since unnecessary duplication will exist on these drivers and can
cause issues when both tables are not in sync as we saw on the issue
reported by Sjoerd on [1].

So an alternate approach could be to do the opposite, just remove the
OF tables entirely from the I2C drivers and only use the I2C table
even for DT-only drivers. This is possible since i2c_device_match()
will succeed even without an OF table because i2c_match_id() matches
for compatible strings and what is reported as uevent is what is in
the I2C table anyways. I believe that is what Sjoerd suggested but
I'll let him comment on that in case I misunderstood.

By the way, the SPI subsystem has the same behavior, I raised the issue on [2].

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/11/269
[1]: https://lkml.org/lkml/2014/9/9/100
[2]: https://lkml.org/lkml/2014/9/11/458

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Javier Martinez Canillas]

[adding Sjoerd as cc who was the one that raised the module auto-loading issue]

Hello,

On Fri, Sep 12, 2014 at 3:46 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>>
>> Placing this firmly back on your plate.  I truly hope we don't miss
>> another merge-window.  This patch-set has the support of some pretty
>> senior kernel maintainers, so I hope acceptance shouldn't be too
>> difficult.
>>
>> 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.
>
> As mentioned in another thread, modaliases are one other possible side
> effect. As Javier correctly mentions, the beaviour does not really
> change with your patchset. Yet, if we remove i2c_device_id from drivers
> too carelessly, they might not be bound anymore.
>

Right, removing the I2C ID table even from drivers that don't really
need it (e.g: IP blocks only present in DT platforms) will as you said
break module auto-loading. Probing will work since the OF table is
used to match the device in i2c_device_match() but is the I2C table
what is used to fill the valid module aliases with the current
behavior of i2c_device_uevent(), the aliases filled from the OF table
are never used.

So what I propose is to do it incrementally:

1) Merge Lee's series since that is definitely a step in the right
direction to not make an I2C table mandatory (still will be needed for
module auto loading though).

2) On a follow-up series, make sure that all I2C drivers have a proper
OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
fill the of:<foo> module aliases. That way the modules will have the
proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
(even when always i2c:<foo> is reported to user-space currently).

3) Apply the patch I posted [0] that changes the behavior of
i2c_device_uevent() to report the OF uevent env vars to user-space in
case of DT probing which after 2) should not regress any driver module
auto-loading since all drivers should fill the of:<foo> aliases.

After this, DT-only drivers will only need an OF table and legacy
drivers will only need an I2C table. Drivers that support both will
still need the two tables though which is a drawback of this approach
since unnecessary duplication will exist on these drivers and can
cause issues when both tables are not in sync as we saw on the issue
reported by Sjoerd on [1].

So an alternate approach could be to do the opposite, just remove the
OF tables entirely from the I2C drivers and only use the I2C table
even for DT-only drivers. This is possible since i2c_device_match()
will succeed even without an OF table because i2c_match_id() matches
for compatible strings and what is reported as uevent is what is in
the I2C table anyways. I believe that is what Sjoerd suggested but
I'll let him comment on that in case I misunderstood.

By the way, the SPI subsystem has the same behavior, I raised the issue on [2].

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/11/269
[1]: https://lkml.org/lkml/2014/9/9/100
[2]: https://lkml.org/lkml/2014/9/11/458

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Javier Martinez Canillas]

[adding Sjoerd as cc who was the one that raised the module auto-loading issue]

Hello,

On Fri, Sep 12, 2014 at 3:46 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>
>> Placing this firmly back on your plate.  I truly hope we don't miss
>> another merge-window.  This patch-set has the support of some pretty
>> senior kernel maintainers, so I hope acceptance shouldn't be too
>> difficult.
>>
>> 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.
>
> As mentioned in another thread, modaliases are one other possible side
> effect. As Javier correctly mentions, the beaviour does not really
> change with your patchset. Yet, if we remove i2c_device_id from drivers
> too carelessly, they might not be bound anymore.
>

Right, removing the I2C ID table even from drivers that don't really
need it (e.g: IP blocks only present in DT platforms) will as you said
break module auto-loading. Probing will work since the OF table is
used to match the device in i2c_device_match() but is the I2C table
what is used to fill the valid module aliases with the current
behavior of i2c_device_uevent(), the aliases filled from the OF table
are never used.

So what I propose is to do it incrementally:

1) Merge Lee's series since that is definitely a step in the right
direction to not make an I2C table mandatory (still will be needed for
module auto loading though).

2) On a follow-up series, make sure that all I2C drivers have a proper
OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
fill the of:<foo> module aliases. That way the modules will have the
proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
(even when always i2c:<foo> is reported to user-space currently).

3) Apply the patch I posted [0] that changes the behavior of
i2c_device_uevent() to report the OF uevent env vars to user-space in
case of DT probing which after 2) should not regress any driver module
auto-loading since all drivers should fill the of:<foo> aliases.

After this, DT-only drivers will only need an OF table and legacy
drivers will only need an I2C table. Drivers that support both will
still need the two tables though which is a drawback of this approach
since unnecessary duplication will exist on these drivers and can
cause issues when both tables are not in sync as we saw on the issue
reported by Sjoerd on [1].

So an alternate approach could be to do the opposite, just remove the
OF tables entirely from the I2C drivers and only use the I2C table
even for DT-only drivers. This is possible since i2c_device_match()
will succeed even without an OF table because i2c_match_id() matches
for compatible strings and what is reported as uevent is what is in
the I2C table anyways. I believe that is what Sjoerd suggested but
I'll let him comment on that in case I misunderstood.

By the way, the SPI subsystem has the same behavior, I raised the issue on [2].

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/11/269
[1]: https://lkml.org/lkml/2014/9/9/100
[2]: https://lkml.org/lkml/2014/9/11/458

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

On Fri, 12 Sep 2014, Javier Martinez Canillas wrote:

> [adding Sjoerd as cc who was the one that raised the module auto-loading issue]
> 
> Hello,
> 
> On Fri, Sep 12, 2014 at 3:46 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >>
> >> Placing this firmly back on your plate.  I truly hope we don't miss
> >> another merge-window.  This patch-set has the support of some pretty
> >> senior kernel maintainers, so I hope acceptance shouldn't be too
> >> difficult.
> >>
> >> 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.
> >
> > As mentioned in another thread, modaliases are one other possible side
> > effect. As Javier correctly mentions, the beaviour does not really
> > change with your patchset. Yet, if we remove i2c_device_id from drivers
> > too carelessly, they might not be bound anymore.
> >
> 
> Right, removing the I2C ID table even from drivers that don't really
> need it (e.g: IP blocks only present in DT platforms) will as you said
> break module auto-loading. Probing will work since the OF table is
> used to match the device in i2c_device_match() but is the I2C table
> what is used to fill the valid module aliases with the current
> behavior of i2c_device_uevent(), the aliases filled from the OF table
> are never used.
> 
> So what I propose is to do it incrementally:
> 
> 1) Merge Lee's series since that is definitely a step in the right
> direction to not make an I2C table mandatory (still will be needed for
> module auto loading though).
> 
> 2) On a follow-up series, make sure that all I2C drivers have a proper
> OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
> fill the of:<foo> module aliases. That way the modules will have the
> proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
> (even when always i2c:<foo> is reported to user-space currently).
> 
> 3) Apply the patch I posted [0] that changes the behavior of
> i2c_device_uevent() to report the OF uevent env vars to user-space in
> case of DT probing which after 2) should not regress any driver module
> auto-loading since all drivers should fill the of:<foo> aliases.

This sounds resonable.

> After this, DT-only drivers will only need an OF table and legacy
> drivers will only need an I2C table. Drivers that support both will
> still need the two tables though which is a drawback of this approach
> since unnecessary duplication will exist on these drivers and can
> cause issues when both tables are not in sync as we saw on the issue
> reported by Sjoerd on [1].
> 
> So an alternate approach could be to do the opposite, just remove the
> OF tables entirely from the I2C drivers and only use the I2C table
> even for DT-only drivers. This is possible since i2c_device_match()
> will succeed even without an OF table because i2c_match_id() matches
> for compatible strings and what is reported as uevent is what is in
> the I2C table anyways. I believe that is what Sjoerd suggested but
> I'll let him comment on that in case I misunderstood.

This would be really bad.  It would go completely against what I have
working to achieve and OF conventions.

> By the way, the SPI subsystem has the same behavior, I raised the issue on [2].
> 
> Best regards,
> Javier
> 
> [0]: https://lkml.org/lkml/2014/9/11/269
> [1]: https://lkml.org/lkml/2014/9/9/100
> [2]: https://lkml.org/lkml/2014/9/11/458

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

On Fri, 12 Sep 2014, Javier Martinez Canillas wrote:

> [adding Sjoerd as cc who was the one that raised the module auto-loading issue]
> 
> Hello,
> 
> On Fri, Sep 12, 2014 at 3:46 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >>
> >> Placing this firmly back on your plate.  I truly hope we don't miss
> >> another merge-window.  This patch-set has the support of some pretty
> >> senior kernel maintainers, so I hope acceptance shouldn't be too
> >> difficult.
> >>
> >> 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.
> >
> > As mentioned in another thread, modaliases are one other possible side
> > effect. As Javier correctly mentions, the beaviour does not really
> > change with your patchset. Yet, if we remove i2c_device_id from drivers
> > too carelessly, they might not be bound anymore.
> >
> 
> Right, removing the I2C ID table even from drivers that don't really
> need it (e.g: IP blocks only present in DT platforms) will as you said
> break module auto-loading. Probing will work since the OF table is
> used to match the device in i2c_device_match() but is the I2C table
> what is used to fill the valid module aliases with the current
> behavior of i2c_device_uevent(), the aliases filled from the OF table
> are never used.
> 
> So what I propose is to do it incrementally:
> 
> 1) Merge Lee's series since that is definitely a step in the right
> direction to not make an I2C table mandatory (still will be needed for
> module auto loading though).
> 
> 2) On a follow-up series, make sure that all I2C drivers have a proper
> OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
> fill the of:<foo> module aliases. That way the modules will have the
> proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
> (even when always i2c:<foo> is reported to user-space currently).
> 
> 3) Apply the patch I posted [0] that changes the behavior of
> i2c_device_uevent() to report the OF uevent env vars to user-space in
> case of DT probing which after 2) should not regress any driver module
> auto-loading since all drivers should fill the of:<foo> aliases.

This sounds resonable.

> After this, DT-only drivers will only need an OF table and legacy
> drivers will only need an I2C table. Drivers that support both will
> still need the two tables though which is a drawback of this approach
> since unnecessary duplication will exist on these drivers and can
> cause issues when both tables are not in sync as we saw on the issue
> reported by Sjoerd on [1].
> 
> So an alternate approach could be to do the opposite, just remove the
> OF tables entirely from the I2C drivers and only use the I2C table
> even for DT-only drivers. This is possible since i2c_device_match()
> will succeed even without an OF table because i2c_match_id() matches
> for compatible strings and what is reported as uevent is what is in
> the I2C table anyways. I believe that is what Sjoerd suggested but
> I'll let him comment on that in case I misunderstood.

This would be really bad.  It would go completely against what I have
working to achieve and OF conventions.

> By the way, the SPI subsystem has the same behavior, I raised the issue on [2].
> 
> Best regards,
> Javier
> 
> [0]: https://lkml.org/lkml/2014/9/11/269
> [1]: https://lkml.org/lkml/2014/9/9/100
> [2]: https://lkml.org/lkml/2014/9/11/458

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Lee Jones]

On Fri, 12 Sep 2014, Javier Martinez Canillas wrote:

> [adding Sjoerd as cc who was the one that raised the module auto-loading issue]
> 
> Hello,
> 
> On Fri, Sep 12, 2014 at 3:46 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >>
> >> Placing this firmly back on your plate.  I truly hope we don't miss
> >> another merge-window.  This patch-set has the support of some pretty
> >> senior kernel maintainers, so I hope acceptance shouldn't be too
> >> difficult.
> >>
> >> 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.
> >
> > As mentioned in another thread, modaliases are one other possible side
> > effect. As Javier correctly mentions, the beaviour does not really
> > change with your patchset. Yet, if we remove i2c_device_id from drivers
> > too carelessly, they might not be bound anymore.
> >
> 
> Right, removing the I2C ID table even from drivers that don't really
> need it (e.g: IP blocks only present in DT platforms) will as you said
> break module auto-loading. Probing will work since the OF table is
> used to match the device in i2c_device_match() but is the I2C table
> what is used to fill the valid module aliases with the current
> behavior of i2c_device_uevent(), the aliases filled from the OF table
> are never used.
> 
> So what I propose is to do it incrementally:
> 
> 1) Merge Lee's series since that is definitely a step in the right
> direction to not make an I2C table mandatory (still will be needed for
> module auto loading though).
> 
> 2) On a follow-up series, make sure that all I2C drivers have a proper
> OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
> fill the of:<foo> module aliases. That way the modules will have the
> proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
> (even when always i2c:<foo> is reported to user-space currently).
> 
> 3) Apply the patch I posted [0] that changes the behavior of
> i2c_device_uevent() to report the OF uevent env vars to user-space in
> case of DT probing which after 2) should not regress any driver module
> auto-loading since all drivers should fill the of:<foo> aliases.

This sounds resonable.

> After this, DT-only drivers will only need an OF table and legacy
> drivers will only need an I2C table. Drivers that support both will
> still need the two tables though which is a drawback of this approach
> since unnecessary duplication will exist on these drivers and can
> cause issues when both tables are not in sync as we saw on the issue
> reported by Sjoerd on [1].
> 
> So an alternate approach could be to do the opposite, just remove the
> OF tables entirely from the I2C drivers and only use the I2C table
> even for DT-only drivers. This is possible since i2c_device_match()
> will succeed even without an OF table because i2c_match_id() matches
> for compatible strings and what is reported as uevent is what is in
> the I2C table anyways. I believe that is what Sjoerd suggested but
> I'll let him comment on that in case I misunderstood.

This would be really bad.  It would go completely against what I have
working to achieve and OF conventions.

> By the way, the SPI subsystem has the same behavior, I raised the issue on [2].
> 
> Best regards,
> Javier
> 
> [0]: https://lkml.org/lkml/2014/9/11/269
> [1]: https://lkml.org/lkml/2014/9/9/100
> [2]: https://lkml.org/lkml/2014/9/11/458

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Javier Martinez Canillas]

Hello Lee,

On Tue, Sep 16, 2014 at 12:46 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 12 Sep 2014, Javier Martinez Canillas wrote:
>> So what I propose is to do it incrementally:
>>
>> 1) Merge Lee's series since that is definitely a step in the right
>> direction to not make an I2C table mandatory (still will be needed for
>> module auto loading though).
>>
>> 2) On a follow-up series, make sure that all I2C drivers have a proper
>> OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
>> fill the of:<foo> module aliases. That way the modules will have the
>> proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
>> (even when always i2c:<foo> is reported to user-space currently).
>>
>> 3) Apply the patch I posted [0] that changes the behavior of
>> i2c_device_uevent() to report the OF uevent env vars to user-space in
>> case of DT probing which after 2) should not regress any driver module
>> auto-loading since all drivers should fill the of:<foo> aliases.
>
> This sounds resonable.
>

Ok, sounds like a plan then.

>> After this, DT-only drivers will only need an OF table and legacy
>> drivers will only need an I2C table. Drivers that support both will
>> still need the two tables though which is a drawback of this approach
>> since unnecessary duplication will exist on these drivers and can
>> cause issues when both tables are not in sync as we saw on the issue
>> reported by Sjoerd on [1].
>>
>> So an alternate approach could be to do the opposite, just remove the
>> OF tables entirely from the I2C drivers and only use the I2C table
>> even for DT-only drivers. This is possible since i2c_device_match()
>> will succeed even without an OF table because i2c_match_id() matches
>> for compatible strings and what is reported as uevent is what is in
>> the I2C table anyways. I believe that is what Sjoerd suggested but
>> I'll let him comment on that in case I misunderstood.
>
> This would be really bad.  It would go completely against what I have
> working to achieve and OF conventions.
>

Yes, I was just mentioning the two possible approaches but Sjoerd did
a much better work on the SPI thread [0].

Another drawback of only using the I2C table is that the vendor prefix
of the compatible strings are not reported to user-space. So if there
are two devices with the same name (driven by two different drivers)
but different vendor, these are going to be probed correctly if
built-in but module auto-loading will fail since doesn't take into
account the vendor prefix. This is the current status btw and I guess
it was never noticed because there isn't two devices with the same
name today.

The drawback of using the OF based aliases is that many I2C drivers
probably rely on the current behavior and don't have a proper OF table
but IMHO that is a bug and has to be fixed anyways.

Of course that doesn't mean that we shouldn't make sure that all
drivers have a proper OF table (and using MODULE_DEVICE_TABLE) before
changing i2c_device_uevent(). Hence my proposal to do it in 3 steps as
explained before.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/15/70

Re: [PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Javier Martinez Canillas]

Hello Lee,

On Tue, Sep 16, 2014 at 12:46 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 12 Sep 2014, Javier Martinez Canillas wrote:
>> So what I propose is to do it incrementally:
>>
>> 1) Merge Lee's series since that is definitely a step in the right
>> direction to not make an I2C table mandatory (still will be needed for
>> module auto loading though).
>>
>> 2) On a follow-up series, make sure that all I2C drivers have a proper
>> OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
>> fill the of:<foo> module aliases. That way the modules will have the
>> proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
>> (even when always i2c:<foo> is reported to user-space currently).
>>
>> 3) Apply the patch I posted [0] that changes the behavior of
>> i2c_device_uevent() to report the OF uevent env vars to user-space in
>> case of DT probing which after 2) should not regress any driver module
>> auto-loading since all drivers should fill the of:<foo> aliases.
>
> This sounds resonable.
>

Ok, sounds like a plan then.

>> After this, DT-only drivers will only need an OF table and legacy
>> drivers will only need an I2C table. Drivers that support both will
>> still need the two tables though which is a drawback of this approach
>> since unnecessary duplication will exist on these drivers and can
>> cause issues when both tables are not in sync as we saw on the issue
>> reported by Sjoerd on [1].
>>
>> So an alternate approach could be to do the opposite, just remove the
>> OF tables entirely from the I2C drivers and only use the I2C table
>> even for DT-only drivers. This is possible since i2c_device_match()
>> will succeed even without an OF table because i2c_match_id() matches
>> for compatible strings and what is reported as uevent is what is in
>> the I2C table anyways. I believe that is what Sjoerd suggested but
>> I'll let him comment on that in case I misunderstood.
>
> This would be really bad.  It would go completely against what I have
> working to achieve and OF conventions.
>

Yes, I was just mentioning the two possible approaches but Sjoerd did
a much better work on the SPI thread [0].

Another drawback of only using the I2C table is that the vendor prefix
of the compatible strings are not reported to user-space. So if there
are two devices with the same name (driven by two different drivers)
but different vendor, these are going to be probed correctly if
built-in but module auto-loading will fail since doesn't take into
account the vendor prefix. This is the current status btw and I guess
it was never noticed because there isn't two devices with the same
name today.

The drawback of using the OF based aliases is that many I2C drivers
probably rely on the current behavior and don't have a proper OF table
but IMHO that is a bug and has to be fixed anyways.

Of course that doesn't mean that we shouldn't make sure that all
drivers have a proper OF table (and using MODULE_DEVICE_TABLE) before
changing i2c_device_uevent(). Hence my proposal to do it in 3 steps as
explained before.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/15/70

[PATCH RESEND 0/8] i2c: Relax mandatory I2C ID table passing

[Javier Martinez Canillas]

Hello Lee,

On Tue, Sep 16, 2014 at 12:46 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 12 Sep 2014, Javier Martinez Canillas wrote:
>> So what I propose is to do it incrementally:
>>
>> 1) Merge Lee's series since that is definitely a step in the right
>> direction to not make an I2C table mandatory (still will be needed for
>> module auto loading though).
>>
>> 2) On a follow-up series, make sure that all I2C drivers have a proper
>> OF table and that are using the MODULE_DEVICE_TABLE(of,...) macro to
>> fill the of:<foo> module aliases. That way the modules will have the
>> proper aliases of the form "of:<foo>" besides the "i2c:<foo>" one.
>> (even when always i2c:<foo> is reported to user-space currently).
>>
>> 3) Apply the patch I posted [0] that changes the behavior of
>> i2c_device_uevent() to report the OF uevent env vars to user-space in
>> case of DT probing which after 2) should not regress any driver module
>> auto-loading since all drivers should fill the of:<foo> aliases.
>
> This sounds resonable.
>

Ok, sounds like a plan then.

>> After this, DT-only drivers will only need an OF table and legacy
>> drivers will only need an I2C table. Drivers that support both will
>> still need the two tables though which is a drawback of this approach
>> since unnecessary duplication will exist on these drivers and can
>> cause issues when both tables are not in sync as we saw on the issue
>> reported by Sjoerd on [1].
>>
>> So an alternate approach could be to do the opposite, just remove the
>> OF tables entirely from the I2C drivers and only use the I2C table
>> even for DT-only drivers. This is possible since i2c_device_match()
>> will succeed even without an OF table because i2c_match_id() matches
>> for compatible strings and what is reported as uevent is what is in
>> the I2C table anyways. I believe that is what Sjoerd suggested but
>> I'll let him comment on that in case I misunderstood.
>
> This would be really bad.  It would go completely against what I have
> working to achieve and OF conventions.
>

Yes, I was just mentioning the two possible approaches but Sjoerd did
a much better work on the SPI thread [0].

Another drawback of only using the I2C table is that the vendor prefix
of the compatible strings are not reported to user-space. So if there
are two devices with the same name (driven by two different drivers)
but different vendor, these are going to be probed correctly if
built-in but module auto-loading will fail since doesn't take into
account the vendor prefix. This is the current status btw and I guess
it was never noticed because there isn't two devices with the same
name today.

The drawback of using the OF based aliases is that many I2C drivers
probably rely on the current behavior and don't have a proper OF table
but IMHO that is a bug and has to be fixed anyways.

Of course that doesn't mean that we shouldn't make sure that all
drivers have a proper OF table (and using MODULE_DEVICE_TABLE) before
changing i2c_device_uevent(). Hence my proposal to do it in 3 steps as
explained before.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/15/70