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

[Kieran Bingham]

From: Lee Jones <lee....@linaro.org>

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....@linaro.org>
Signed-off-by: Lee Jones <lee....@linaro.org>
[Kieran: fix rebase conflicts and adapt for dev_pm_domain_{attach,detach}]
Signed-off-by: Kieran Bingham <kie...@bingham.xyz>
Tested-by: Kieran Bingham <kie...@bingham.xyz>
---
Changes since v4 [Kieran]
- Rename .probe2 to probe_new
- Checkpatch warnings fixed
---
drivers/i2c/i2c-core.c | 15 ++++++++++++---
include/linux/i2c.h | 8 +++++++-
2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 64d543b041b1..268fec3b6931 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -697,8 +697,6 @@ static int i2c_device_probe(struct device *dev)
}

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
@@ -740,7 +738,18 @@ static int i2c_device_probe(struct device *dev)
if (status == -EPROBE_DEFER)
goto err_clear_wakeup_irq;

- status = driver->probe(client, i2c_match_id(driver->id_table, client));
+ /*
+ * When there are no more users of probe(),
+ * rename probe_new to probe.
+ */
+ if (driver->probe_new)
+ status = driver->probe_new(client);
+ else if (driver->probe)
+ status = driver->probe(client,
+ i2c_match_id(driver->id_table, client));
+ else
+ status = -EINVAL;
+
if (status)
goto err_detach_pm_domain;

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e3aa3f5e59a8..c8f73b9f51fe 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -130,7 +130,8 @@ i2c_smbus_read_i2c_block_data_or_emulated(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
+ * @probe_new: New callback for device binding
* @remove: Callback for device unbinding
* @shutdown: Callback for device shutdown
* @alert: Alert callback, for example for the SMBus alert protocol
@@ -173,6 +174,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 (*probe_new)(struct i2c_client *);
+
/* driver model interfaces that don't relate to enumeration */
void (*shutdown)(struct i2c_client *);

--
2.5.0

[Kieran Bingham]

This patch set finally pops up again, after a long time stuck somewhere in the
midst of my stack.

As it stood last year, the requirements were to rename probe2 to probe_new, and
ensure that it was correctly tested. The rename was the easy bit, but the
testing took me more time to get things set up properly. And other commitments
then got in the way of things. Of course this patch set has also been rebased
as well, but there wasn't any major pain there.

Testing
-------

To try to establish testing, I have used a beagle-bone-black, and a DS1307 RTC
connected to the BBB SCL and SDA lines. The main reason for these choices is
accesibility. i.e. I have them, and the BBB readily boots a kernel for me to
test and iterate with.

I've tested the device with i2cdetect, and then worked through testing the
sysfs interface, device tree, and module autoloading, each time ensuring that
the RTC enumerates and operates

* new_device (built-in, and external module)
echo ds1307 0x68 > /sys/bus/i2c/devices/i2c-2/new_device
cat /sys/class/rtc/rtc0/date

- Both of those worked fine.

* Device Tree
I tested that the device would still register by adding a node in the device
tree for the board, and testing with a built-in module.

- This worked fine.

* Module Autoloading
With the device tree node in the board dts file, it wouldn't automatically
load from the external module. This was due to the rtc-ds1307 module not
exporting an of_match table, and not yet having Javier's "report OF style
modalias when probing using DT" [0] patch applied

- With the module updated, and Javiers patch applied, the module autoloads

Finally, I feel I can safely add this tag to the patch set:
Tested-by: Kieran Bingham <kie...@bingham.xyz>

Please let me know if there is any other specific use case missing here that
needs to be tested.

[0] https://patchwork.ozlabs.org/patch/502201/

Patches
-------
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 .probe_new() 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 | 75 +++++++++++++++++++++++++++++++++++++++------
drivers/mfd/88pm860x-core.c | 5 ++-
drivers/mfd/as3722.c | 12 ++------
include/linux/i2c.h | 22 ++++++++++++-
4 files changed, 91 insertions(+), 23 deletions(-)

--
2.5.0

[Kieran Bingham]

From: Lee Jones <lee....@linaro.org>

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....@linaro.org>
Signed-off-by: Lee Jones <lee....@linaro.org>

Signed-off-by: Kieran Bingham <kie...@bingham.xyz>
Tested-by: Kieran Bingham <kie...@bingham.xyz>
---

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 e584d88ee337..e980c8257676 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -493,6 +493,9 @@ static inline int acpi_i2c_install_space_handler(struct i2c_adapter *adapter)
static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
const struct i2c_client *client)
{
+ if (!(id && client))
+ return NULL;
+
while (id->name[0]) {
if (strcmp(client->name, id->name) == 0)
return id;
@@ -506,8 +509,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))
@@ -518,9 +519,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;
}
--
2.5.0

[Kieran Bingham]

From: Lee Jones <lee....@linaro.org>

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....@linaro.org>
Signed-off-by: Lee Jones <lee....@linaro.org>

[Kieran: Fix static inline usage on !CONFIG_OF]

Signed-off-by: Kieran Bingham <kie...@bingham.xyz>
Tested-by: Kieran Bingham <kie...@bingham.xyz>
---

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 f75e78f6f9c1..88211009d282 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1515,6 +1515,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 200cf13b00f6..547d213d2a33 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -689,6 +689,10 @@ extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
/* must call i2c_put_adapter() when done with returned i2c_adapter device */
struct i2c_adapter *of_get_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)
@@ -705,6 +709,14 @@ static inline struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node
{
return NULL;
}
+
+static inline const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches,
+ struct i2c_client *client)
+{
+ return NULL;
+}
+
#endif /* CONFIG_OF */

#endif /* _LINUX_I2C_H */
--
2.5.0

[Kieran Bingham]

From: Lee Jones <lee....@linaro.org>

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....@linaro.org>
Signed-off-by: Lee Jones <lee....@linaro.org>

Signed-off-by: Kieran Bingham <kie...@bingham.xyz>

---
Changes since v4

- Rename .probe2 to probe_new

---
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 25e1aafae60c..227b99018657 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1132,8 +1132,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;
@@ -1259,7 +1258,7 @@ static struct i2c_driver pm860x_driver = {
.pm = &pm860x_pm_ops,
.of_match_table = pm860x_dt_ids,
},
- .probe = pm860x_probe,
+ .probe_new = pm860x_probe,
.remove = pm860x_remove,
.id_table = pm860x_id_table,
};
--
2.5.0

[Kieran Bingham]

From: Lee Jones <lee....@linaro.org>

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

Acked-by: Grant Likely <grant....@linaro.org>
Signed-off-by: Lee Jones <lee....@linaro.org>
Signed-off-by: Kieran Bingham <kie...@bingham.xyz>

---
Changes since v4
- Rename .probe2 to probe_new
---

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 e1f597f97f86..28fc62af6861 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;
@@ -453,12 +452,6 @@ 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 const struct dev_pm_ops as3722_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(as3722_i2c_suspend, as3722_i2c_resume)
};
@@ -469,9 +462,8 @@ static struct i2c_driver as3722_i2c_driver = {
.of_match_table = as3722_of_match,
.pm = &as3722_pm_ops,
},
- .probe = as3722_i2c_probe,
+ .probe_new = as3722_i2c_probe,
.remove = as3722_i2c_remove,
- .id_table = as3722_i2c_id,
};

module_i2c_driver(as3722_i2c_driver);
--
2.5.0

[Lee Jones]

Great work Kieran.

Wolfram, Javier,

Looks like Kieran has ticked each of your boxes.

I guess there is nothing stopping this set from being applied now,
right?

> Please let me know if there is any other specific use case missing here that
> needs to be tested.
>
> [0] https://patchwork.ozlabs.org/patch/502201/
>
> Patches
> -------
> 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 .probe_new() 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 | 75 +++++++++++++++++++++++++++++++++++++++------
> drivers/mfd/88pm860x-core.c | 5 ++-
> drivers/mfd/as3722.c | 12 ++------
> include/linux/i2c.h | 22 ++++++++++++-
> 4 files changed, 91 insertions(+), 23 deletions(-)
>

--

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

[Javier Martinez Canillas]

Hello Lee,

On 05/09/2016 05:14 AM, Lee Jones wrote:
> On Wed, 04 May 2016, Kieran Bingham wrote:
>

[snip].

>>
>> Finally, I feel I can safely add this tag to the patch set:
>> Tested-by: Kieran Bingham <kie...@bingham.xyz>
>
> Great work Kieran.
>
> Wolfram, Javier,
>
> Looks like Kieran has ticked each of your boxes.
>
> I guess there is nothing stopping this set from being applied now,
> right?
>

That's correct, on a quick looks it seems that this set is ready to be
merged and it would be great if that is sooner rather than later. I'll
do a detailed review and testing today.

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

[Javier Martinez Canillas]

On 05/04/2016 11:14 AM, Kieran Bingham wrote:
> From: Lee Jones <lee....@linaro.org>
>
> 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....@linaro.org>
> Signed-off-by: Lee Jones <lee....@linaro.org>
> Signed-off-by: Kieran Bingham <kie...@bingham.xyz>
> Tested-by: Kieran Bingham <kie...@bingham.xyz>
> ---

Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>
Tested-by: Javier Martinez Canillas <jav...@osg.samsung.com>

[Javier Martinez Canillas]

On 05/04/2016 11:14 AM, Kieran Bingham wrote:

> From: Lee Jones <lee....@linaro.org>
>
> 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....@linaro.org>
> Signed-off-by: Lee Jones <lee....@linaro.org>
> [Kieran: Fix static inline usage on !CONFIG_OF]
> Signed-off-by: Kieran Bingham <kie...@bingham.xyz>
> Tested-by: Kieran Bingham <kie...@bingham.xyz>
> ---

[Javier Martinez Canillas]

On 05/04/2016 11:14 AM, Kieran Bingham wrote:

> From: Lee Jones <lee....@linaro.org>
>
> 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....@linaro.org>
> Signed-off-by: Lee Jones <lee....@linaro.org>
> [Kieran: fix rebase conflicts and adapt for dev_pm_domain_{attach,detach}]
> Signed-off-by: Kieran Bingham <kie...@bingham.xyz>
> Tested-by: Kieran Bingham <kie...@bingham.xyz>
> ---

[Javier Martinez Canillas]

On 05/04/2016 11:14 AM, Kieran Bingham wrote:

> From: Lee Jones <lee....@linaro.org>
>
> 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....@linaro.org>
> Signed-off-by: Lee Jones <lee....@linaro.org>
> Signed-off-by: Kieran Bingham <kie...@bingham.xyz>
>

Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>

[Javier Martinez Canillas]

On 05/04/2016 11:14 AM, Kieran Bingham wrote:

Just a note that this can only be made because the driver's Kconfig symbol
is bool and not tristate. Since for drivers that can be built as a module,
the I2C core always reports a MODALIAS of the form "i2c:as3722" and so the
i2c_device_id array and the MODULE_DEVICE_TABLE() are needed even when not
used by the driver.

As mentioned the change is correct for this driver but I just wanted to
point out in case other authors try to do the same change for drivers that
can be built as a module and so breaking module auto-loading.

[Javier Martinez Canillas]

Hello Kieran,

On 05/04/2016 11:14 AM, Kieran Bingham wrote:

Same here, I've tested this series using an Exynos5800 Peach Pi Chromebook
that has a I2C touchpad device (Atmel mXT540S). So I used this series and
removed the i2c_device_id table from the device driver.

The driver could match correctly using the of_device_id table and also the
module was auto-loaded when using my mentioned RFC patch to report OF style
module aliases instead of always using the legacy one.

I've also reviewed the patches and the changes looks good to me. I hope the
patches can finally land since have been in the list for almost 2 years [0].

[0]: https://lkml.org/lkml/2014/8/28/283

[Lee Jones]

Sounds like a subsequent patch might be required to fix that use-case
too. I'll add it to my TODO. :)

> Reviewed-by: Javier Martinez Canillas <jav...@osg.samsung.com>
>
> Best regards,

--

[Kieran Bingham]

Hi Javier,

Looks like the original submission [1] is even closer to 2 years ago,
at 2nd June 2014!

> [0]: https://lkml.org/lkml/2014/8/28/283
[1]: https://lkml.org/lkml/2014/6/2/274

Thanks for taking the time to test as well. Lets aim to complete the
conversion in less than 2 years :)

(Note to self when referencing in the future, "Hello 2018")

--
Regards

Kieran Bingham

[Javier Martinez Canillas]

Hello Lee,

On 05/10/2016 03:33 AM, Lee Jones wrote:
> On Tue, 10 May 2016, Javier Martinez Canillas wrote:

[snip]

>>>
>>> -static const struct i2c_device_id as3722_i2c_id[] = {
>>> - { "as3722", 0 },
>>> - {},
>>> -};
>>> -MODULE_DEVICE_TABLE(i2c, as3722_i2c_id);
>>> -
>>
>> Just a note that this can only be made because the driver's Kconfig symbol
>> is bool and not tristate. Since for drivers that can be built as a module,
>> the I2C core always reports a MODALIAS of the form "i2c:as3722" and so the
>> i2c_device_id array and the MODULE_DEVICE_TABLE() are needed even when not
>> used by the driver.
>>
>> As mentioned the change is correct for this driver but I just wanted to
>> point out in case other authors try to do the same change for drivers that
>> can be built as a module and so breaking module auto-loading.
>
> Sounds like a subsequent patch might be required to fix that use-case
> too. I'll add it to my TODO. :)
>

Well, the fix is really trivial and I've posted it as an RFC patch a [0] a
long time ago. That is what Kieran and my used to test module autoload with
this patch series.

The problem is that a lot of I2C drivers are relying on how the subsystem
behave (always match using i2c_device_id table and report i2c: modalias)
and so OF drivers don't have an of_device_id table since was not necessary.

So if the RFC patch lands [0], that will break a lot of drivers since after
that, the I2C devices registered via OF will report a of: modalias but will
not have a OF aliases in their modules.

We need a flag day to change the I2C subsystem behaviour and that can only
happen after all the in-tree I2C drivers have proper exported of_device_id.

I posted a patch series almost a year ago [1] trying to fix the I2C drivers
that I could find using a script but then found that removing the I2C table
was not possible also due how the subsystem did the matching. Fortunately
your patch series fixed this :)

So after your series land, I plan to do the same investigation again and
post patches to fix all the remaining I2C drivers so the modalias patch can
finally land and the I2C subsystem report modalias like other subsystems do.

[0]: https://patchwork.ozlabs.org/patch/502201/
[1]: https://lkml.org/lkml/2015/7/30/519

[Lee Jones]

Sounds perfect. Thanks for the explanation.

Wolfram,
Things are looking up for the subsystem, please do your thing, so we
can ensure awesomeness. :)

> [0]: https://patchwork.ozlabs.org/patch/502201/
> [1]: https://lkml.org/lkml/2015/7/30/519

[Kieran Bingham]

This script will then add the MODULE_DEVICE_TABLE(of, ...) to correctly export
the device tree table

Usage: spatch --sp-file scripts/coccinelle/i2c/i2c_table_missing_export.cocci . --in-place
---

Javier,

I've been working on scripts to automate some of the conversion processes
involved in this DT_I2C project.

This one adds missing exports for i2c device tables.

Feel free to use it if it helps.

I have another one which searches for drivers which have both an I2C device table, and an OF device table,
and a probe function which does not use the second parameter.

It then performs the conversion to the probe_new() and removes the I2C device table.

Should help with the bulk of the conversion

--
Kieran

.../coccinelle/i2c/of_table_missing_export.cocci | 34 ++++++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 scripts/coccinelle/i2c/of_table_missing_export.cocci

diff --git a/scripts/coccinelle/i2c/of_table_missing_export.cocci b/scripts/coccinelle/i2c/of_table_missing_export.cocci
new file mode 100644
index 000000000000..5d1950fdf8a3
--- /dev/null
+++ b/scripts/coccinelle/i2c/of_table_missing_export.cocci
@@ -0,0 +1,34 @@
+// Look for I2C drivers without an exported of_device_id table
+//
+
+// C1 : Identify the i2c_device_id array
+
+@ i2c_dev_id @
+identifier arr;
+@@
+struct i2c_device_id arr[] = { ... };
+
+// C2 : For now, we only want to match on I2C drivers
+
+@ dev_id depends on i2c_dev_id @
+identifier arr;
+@@
+struct of_device_id arr[] = { ... };
+
+// C2 : Check if we already export the MODULE_DEVICE_TABLE
+
+@ of_dev_table depends on dev_id @
+declarer name MODULE_DEVICE_TABLE;
+identifier of;
+identifier dev_id.arr;
+@@
+ MODULE_DEVICE_TABLE(of, arr);
+
+
+// A1: Export it!
+
+@ add_mod_dev_table depends on !of_dev_table @
+identifier dev_id.arr;
+@@
+struct of_device_id arr[] = { ... };
++ MODULE_DEVICE_TABLE(of, arr);
--
2.5.0

[Kieran Bingham]

Adds MODULE_DEVICE_TABLE(i2c, ...) to correctly export tables in i2c drivers
---
Ahem, My appologies, I wrongly sent the patch I was working on for OF tables,

Of course this is the correct patch for adding the i2c_device_id exports,
which I think is what you are currently looking at
--
Kieran

.../coccinelle/i2c/i2c_table_missing_export.cocci | 30 ++++++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 scripts/coccinelle/i2c/i2c_table_missing_export.cocci

diff --git a/scripts/coccinelle/i2c/i2c_table_missing_export.cocci b/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
new file mode 100644
index 000000000000..58c06856e4d4
--- /dev/null
+++ b/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
@@ -0,0 +1,30 @@
+// Look for I2C drivers without an exported i2c_device_id table,
+// and export it using the MODULE_DEVICE_TABLE();
+//
+// Usage:
+// spatch --sp-file scripts/coccinelle/i2c/i2c_table_missing_export.cocci . --in-place

+
+// C1 : Identify the i2c_device_id array
+

+@ dev_id @

+identifier arr;
+@@
+struct i2c_device_id arr[] = { ... };
+

+// C2 : Check if we already export the MODULE_DEVICE_TABLE
+

+@ i2c_dev_table depends on dev_id @
+declarer name MODULE_DEVICE_TABLE;
+identifier i2c;
+identifier dev_id.arr;
+@@
+ MODULE_DEVICE_TABLE(i2c, arr);

+
+
+// A1: Export it!
+

+@ add_mod_dev_table depends on !i2c_dev_table @
+identifier dev_id.arr;
+@@
+struct i2c_device_id arr[] = { ... };
++ MODULE_DEVICE_TABLE(i2c, arr);
--
2.5.0

[Javier Martinez Canillas]

Hello Kieran,

Thanks for the patch, it would be great to automate this to avoid new
drivers to introduce new issues!

On 05/10/2016 11:07 AM, Kieran Bingham wrote:
> Adds MODULE_DEVICE_TABLE(i2c, ...) to correctly export tables in i2c drivers
> ---
> Ahem, My appologies, I wrongly sent the patch I was working on for OF tables,
>

No worries and in fact we need to identify both (I2C && OF tables not exported).

There are currently 3 issues that have to be fixed on I2C drivers w.r.t to device
and driver match and module auto-loading:

1) Drivers with I2C table but not exported (match will work but autoload no).

2) Drivers with OF table but not exported (match will work but autoload no).

3) Drivers with an I2C table but with no OF table even when the I2C devices
are registered via OF. Match and autoload works because the core always
reports a MODALIAS=i2c:* for both but once we change it to reports the
corret modalias, things will break since now will be MODALIAS=of:N*fT*C
and the module won't have a modalias of that form.

1) and 2) are somehow easy and your semantic patches can be used to find
and fix them.

Now 3) is more tricky since is hard to say if a DT is relying on the fact
that the I2C core strips the manufacture from the compatible string and
just matches using the model part.

I think the best we can do is to check if there are such a users in the
in-tree DTS or documented compatible in the DT bindings documents. But if
someone is using with a DT that's not in mainline, then things will break.

So if you can think of a way to automatize 3) using coccinelle, that will
be great. My coccinelle knowledge is near to non-existent but one issue I
see is that coccinelle context seems to be restricted to just the current
file so it can't check in external files like DTS or DT bindings docs.

> Of course this is the correct patch for adding the i2c_device_id exports,
> which I think is what you are currently looking at
> --
> Kieran
>

[snip]

> +
> +// A1: Export it!
> +
> +@ add_mod_dev_table depends on !i2c_dev_table @
> +identifier dev_id.arr;
> +@@
> +struct i2c_device_id arr[] = { ... };
> ++ MODULE_DEVICE_TABLE(i2c, arr);
>

A problem with this semantic patch is that is going to give a lot of
false positives. Not always a MODULE_DEVICE_TABLE(i2c,...) is needed
after a i2c_device_id array, drivers that can't be build as a module
(i.e: whose Kconfig is not tristate) or board files are two examples.

One way I think we can minimize this is by checking if the driver is
including the module.h header file or not. This still will give some
false positives, since many drivers that can't be build as module
wrongly include module.h but in any case that's something that needs
to be fixed in those drivers.

So I think you need something like the following change to squash with
your patch:

diff --git a/scripts/coccinelle/i2c/i2c_table_missing_export.cocci b/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
index 58c06856e4d4..df98279e326d 100644
--- a/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
+++ b/scripts/coccinelle/i2c/i2c_table_missing_export.cocci
@@ -4,9 +4,16 @@
// Usage:

// spatch --sp-file scripts/coccinelle/i2c/i2c_table_missing_export.cocci . --in-place

+// C0 : Check if module.h is included
+
+@ includes_module @
+@@
+
+# include <linux/module.h>
+

// C1 : Identify the i2c_device_id array

-@ dev_id @
+@ dev_id depends on includes_module @
identifier arr;
@@
struct i2c_device_id arr[] = { ... };

[Lee Jones]

Wolfram,

Any reason this still has not been accepted?

On Wed, 04 May 2016, Kieran Bingham wrote:

[Wolfram Sang]

Hi Kieran,

> * Device Tree
> I tested that the device would still register by adding a node in the device
> tree for the board, and testing with a built-in module.
>
> - This worked fine.
>
> * Module Autoloading
> With the device tree node in the board dts file, it wouldn't automatically
> load from the external module. This was due to the rtc-ds1307 module not
> exporting an of_match table, and not yet having Javier's "report OF style
> modalias when probing using DT" [0] patch applied

What I didn't get here: did your version of the RTC driver use probe()
or probe_new() without i2c_device_id table or did you try both? I assume
module autoloading only fails with probe_new(), otherwise we would be in
serious trouble. But I'd wonder then that userspace instantiation works.

Thanks to you and Javier for the testing. I pushed the patches to a
local branch for now and will merge once this question is clear.

Regards,

Wolfram

[Javier Martinez Canillas]

Hello Wolfram,

On 06/09/2016 03:15 PM, Wolfram Sang wrote:
> Hi Kieran,
>
>> * Device Tree
>> I tested that the device would still register by adding a node in the device
>> tree for the board, and testing with a built-in module.
>>
>> - This worked fine.
>>
>> * Module Autoloading
>> With the device tree node in the board dts file, it wouldn't automatically
>> load from the external module. This was due to the rtc-ds1307 module not
>> exporting an of_match table, and not yet having Javier's "report OF style
>> modalias when probing using DT" [0] patch applied
>
> What I didn't get here: did your version of the RTC driver use probe()
> or probe_new() without i2c_device_id table or did you try both? I assume
> module autoloading only fails with probe_new(), otherwise we would be in
> serious trouble. But I'd wonder then that userspace instantiation works.
>

I can't answer for Kieran but you trimmed this last sentence from him:

> - With the module updated, and Javiers patch applied, the module autoloads
>

So my understanding is that by updated he meant a patched rtc-ds1307 driver
using a .probe_new, whose i2c_device_id table was removed and of_device_id
table added (that's not present in the mainline driver).

And that's why he needed my RFC patch to report a MODALIAS=of:N*T*Cfoo,bar
and match what's exported to the module using the of_device_id table.

Because drivers that only use .probe and have an i2c_device_id table will
continue to match and report MODALIAS=i2c:foo as before after this series.

> Thanks to you and Javier for the testing. I pushed the patches to a
> local branch for now and will merge once this question is clear.
>
> Regards,
>
> Wolfram
>

[Wolfram Sang]

On Thu, Jun 09, 2016 at 03:45:52PM -0400, Javier Martinez Canillas wrote:
> Hello Wolfram,
>
> On 06/09/2016 03:15 PM, Wolfram Sang wrote:
> > Hi Kieran,
> >
> >> * Device Tree
> >> I tested that the device would still register by adding a node in the device
> >> tree for the board, and testing with a built-in module.
> >>
> >> - This worked fine.
> >>
> >> * Module Autoloading
> >> With the device tree node in the board dts file, it wouldn't automatically
> >> load from the external module. This was due to the rtc-ds1307 module not
> >> exporting an of_match table, and not yet having Javier's "report OF style
> >> modalias when probing using DT" [0] patch applied

Let's call this a)

> >
> > What I didn't get here: did your version of the RTC driver use probe()
> > or probe_new() without i2c_device_id table or did you try both? I assume
> > module autoloading only fails with probe_new(), otherwise we would be in
> > serious trouble. But I'd wonder then that userspace instantiation works.
> >
>
> I can't answer for Kieran but you trimmed this last sentence from him:
>
> > - With the module updated, and Javiers patch applied, the module autoloads

Let's call this b)

> >
>
> So my understanding is that by updated he meant a patched rtc-ds1307 driver
> using a .probe_new, whose i2c_device_id table was removed and of_device_id
> table added (that's not present in the mainline driver).
>
> And that's why he needed my RFC patch to report a MODALIAS=of:N*T*Cfoo,bar
> and match what's exported to the module using the of_device_id table.
>
> Because drivers that only use .probe and have an i2c_device_id table will
> continue to match and report MODALIAS=i2c:foo as before after this series.

Yes, this is my understanding and expectation, too. However, he wrote
that module loading fails on a) where he never wrote anything about an
updated module before. That comes only later with b). So what I would
have expected:

1) update the module (which is "b)" above)
2) autoloading fails (which is "a)" above)
3) applying your patch
4) everything works

which implies

0) nothing done, everything works

But I don't see this in the text, so I better ask. This also raises the
question open how userspace instantiation was tested. With or without
updating (ideally both).

Thanks,

Wolfram

[Kieran Bingham]

Hi Wolfram,

Yes, Javier's interpretation is correct here.

> Yes, this is my understanding and expectation, too. However, he wrote
> that module loading fails on a) where he never wrote anything about an
> updated module before. That comes only later with b). So what I would
> have expected:
>
> 1) update the module (which is "b)" above)
> 2) autoloading fails (which is "a)" above)
> 3) applying your patch
> 4) everything works
>
> which implies
>
> 0) nothing done, everything works
>
> But I don't see this in the text, so I better ask. This also raises the
> question open how userspace instantiation was tested. With or without
> updating (ideally both).

Well it's a month later, and I can't remember - so I've retested.

Kernel built with this patchset - and *without* Javiers patch for 0)
position test.

RTC_DS1307 driver *unmodified*

root@arm:~# uname -a
Linux arm 4.7.0-rc2-00027-g72baec98c9f0 #27 SMP Fri Jun 10 10:42:09 BST
2016 armv7l GNU/Linux

root@arm:~# lsmod
Module Size Used by Not tainted
omap_rng 3927 0
rng_core 6410 1 omap_rng
rtc_ds1307 12121 0

root@arm:~# cat /sys/class/rtc/rtc0/device/modalias
i2c:ds1307

root@arm:~# cat /sys/class/rtc/rtc0/date
2016-06-10


* Module autoload successful,
* i2c read successful.


Code tested at
repository https://github.com/kbingham/linux.git
tag i2c-dt/v4.7-rc2-relax-conversion-zero-test

Is this what you were looking for?


> Thanks,
>
> Wolfram
>

--
Regards

Kieran Bingham

[Wolfram Sang]

> Is this what you were looking for?

Mostly, thanks. This verifies that the old way still works. Good!

The new way (no i2c_device_ids, just compatibles) will need Javier's
patch to work with module auto loading, I know. But what about userspace
instantiation with built-in driver? I didn't understand if this was
tested using the new way. And do you need then the full-compatible or
the vendor-stripped string?

Thanks,

Wolfram

[Kieran Bingham]

Hi Wolfram,

When I reported :

> * new_device (built-in, and external module)
> echo ds1307 0x68 > /sys/bus/i2c/devices/i2c-2/new_device
> cat /sys/class/rtc/rtc0/date
>
> - Both of those worked fine.

That was *without* Javier's patch, but hopefully obviously *with* Lee's
patchset.

Do you need this testing *with* Javiers patch as well?
I have not tested this combination.

I've already switched my dev board environment back to the Salvator-X
for $DAYJOB and switching back now will have to wait to the weekend.

[Wolfram Sang]

> When I reported :
>
> > * new_device (built-in, and external module)
> > echo ds1307 0x68 > /sys/bus/i2c/devices/i2c-2/new_device
> > cat /sys/class/rtc/rtc0/date
> >
> > - Both of those worked fine.
>
> That was *without* Javier's patch, but hopefully obviously *with* Lee's
> patchset.
>
> Do you need this testing *with* Javiers patch as well?

Without Javiers patch. But with a modified rtc driver which will only
use proper compatibles, no i2c_device_ids, and probe_new. And this one
should be able to instantiate via userspace with the driver builtin.

We have documented ways of instantiating. All I ask for is to make sure
the old style and new style work with them. Check the attached sketch
for an example (only compile-tested and conversion to probe_new is
missing). Can you instantiate the "maxim,ds1307" (with the additional
printout) and "dallas,ds1307" via userspace?

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 821d9c089cdb48..10a4e5bc923e07 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -31,6 +31,7 @@
*/
enum ds_type {
ds_1307,
+ maxim_1307,
ds_1337,
ds_1338,
ds_1339,
@@ -144,6 +145,10 @@ static struct chip_desc chips[last_ds_type] = {
.nvram_offset = 8,
.nvram_size = 56,
},
+ [maxim_1307] = {
+ .nvram_offset = 8,
+ .nvram_size = 56,
+ },
[ds_1337] = {
.alarm = 1,
},
@@ -173,23 +178,6 @@ static struct chip_desc chips[last_ds_type] = {
},
};

-static const struct i2c_device_id ds1307_id[] = {
- { "ds1307", ds_1307 },
- { "ds1337", ds_1337 },
- { "ds1338", ds_1338 },
- { "ds1339", ds_1339 },
- { "ds1388", ds_1388 },
- { "ds1340", ds_1340 },
- { "ds3231", ds_3231 },
- { "m41t00", m41t00 },
- { "mcp7940x", mcp794xx },
- { "mcp7941x", mcp794xx },
- { "pt7c4338", ds_1307 },
- { "rx8025", rx_8025 },
- { }
-};
-MODULE_DEVICE_TABLE(i2c, ds1307_id);
-
/*----------------------------------------------------------------------*/

#define BLOCK_DATA_MAX_TRIES 10
@@ -1435,6 +1423,9 @@ read_rtc:
*/
tmp = ds1307->regs[DS1307_REG_SECS];
switch (ds1307->type) {
+ case maxim_1307:
+ dev_info(&client->dev, "I'm a Maxim\n");
+ /* fallthrough */
case ds_1307:
case m41t00:
/* clock halted? turn it on, so clock can tick. */
@@ -1610,13 +1601,22 @@ static int ds1307_remove(struct i2c_client *client)
return 0;
}

+#ifdef CONFIG_OF
+static const struct of_device_id ds1307_dt_ids[] = {
+ { .compatible = "dallas,ds1307", .data = (void *)ds_1307 },
+ { .compatible = "maxim,ds1307", .data = (void *)maxim_1307 },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ds1302_dt_ids);
+#endif
+
static struct i2c_driver ds1307_driver = {
.driver = {
.name = "rtc-ds1307",
+ .of_match_table = of_match_ptr(ds1307_dt_ids),
},
.probe = ds1307_probe,
.remove = ds1307_remove,
- .id_table = ds1307_id,
};

module_i2c_driver(ds1307_driver);

[Kieran Bingham]

Just for testing, specify a ds9999 device to identify the code path used when
instantiating the driver from userspace.

As we match on only the device, not the manufacturer, I've changed your sketch
so that the test maxim line is on a compatible with name ds9999 to make it
unique. Otherwise, we would match to the dallas,ds1307 id type.

If you would prefer that we support separate manufacturers, I can update the
match function so that it attempts a full match first, followed by a stripped
manufacturer match. I'm not certain if we have a need for that at the moment
though, as the current drivers simply match on the device name:

This patch also demonstrates a method to obtain the device ID from the new
match system for drivers which would normally have expected this information to
be passed in.


Testing:
=======-

root@arm:~# echo ds9999 0x68 > /sys/bus/i2c/devices/i2c-2/new_device
[ 43.262432] rtc-ds1307 2-0068: I'm a Maxim ...
[ 43.268707] rtc-ds1307 2-0068: rtc core: registered ds9999 as rtc0
[ 43.275276] rtc-ds1307 2-0068: 56 bytes nvram
[ 43.279920] i2c i2c-2: new_device: Instantiated device ds9999 at 0x68

root@arm:~# cat /sys/class/rtc/rtc0/date
2016-06-12

root@arm:~# cat /sys/class/rtc/rtc0/name
ds9999


---
drivers/rtc/rtc-ds1307.c | 60 ++++++++++++++++++++++++++++++------------------
1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 821d9c089cdb..d97e8adb866b 100644

--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -31,6 +31,7 @@
*/
enum ds_type {
ds_1307,
+ maxim_1307,
ds_1337,
ds_1338,
ds_1339,
@@ -144,6 +145,10 @@ static struct chip_desc chips[last_ds_type] = {
.nvram_offset = 8,
.nvram_size = 56,
},
+ [maxim_1307] = {
+ .nvram_offset = 8,
+ .nvram_size = 56,
+ },
[ds_1337] = {
.alarm = 1,
},

@@ -173,22 +178,6 @@ static struct chip_desc chips[last_ds_type] = {

},
};

-static const struct i2c_device_id ds1307_id[] = {
- { "ds1307", ds_1307 },
- { "ds1337", ds_1337 },
- { "ds1338", ds_1338 },
- { "ds1339", ds_1339 },
- { "ds1388", ds_1388 },
- { "ds1340", ds_1340 },
- { "ds3231", ds_3231 },
- { "m41t00", m41t00 },
- { "mcp7940x", mcp794xx },
- { "mcp7941x", mcp794xx },
- { "pt7c4338", ds_1307 },
- { "rx8025", rx_8025 },
- { }
-};
-MODULE_DEVICE_TABLE(i2c, ds1307_id);

/*----------------------------------------------------------------------*/

@@ -1226,13 +1215,27 @@ static void ds1307_clks_register(struct ds1307 *ds1307)

#endif /* CONFIG_COMMON_CLK */

-static int ds1307_probe(struct i2c_client *client,

- const struct i2c_device_id *id)

+static const struct of_device_id ds1307_dt_ids[] = {

+ /* We are only matching on the device name, *NOT* the manufacturer name
+ * I.e. dallas, maxim, are dropped in the search when someone tries to load a
+ * 'ds1307', and hence first match wins.
+ *
+ * We could extend this to do a full match first, followed by a fallback match
+ * to just the device name.
+ */

+ { .compatible = "dallas,ds1307", .data = (void *)ds_1307 },

+ { .compatible = "maxim,ds9999", .data = (void *)maxim_1307 },

+ { /* sentinel */ }
+};

+MODULE_DEVICE_TABLE(of, ds1307_dt_ids);
+
+static int ds1307_probe(struct i2c_client *client)
{
struct ds1307 *ds1307;
int err = -ENODEV;
int tmp;
- struct chip_desc *chip = &chips[id->driver_data];
+ const struct of_device_id *idof;
+ struct chip_desc *chip;
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
bool want_irq = false;
bool ds1307_can_wakeup_device = false;
@@ -1255,10 +1258,20 @@ static int ds1307_probe(struct i2c_client *client,
if (!ds1307)
return -ENOMEM;

+ /* If we've got this far, this shouldn't be able to fail - but check anyway for now */
+ idof = i2c_of_match_device(ds1307_dt_ids, client);
+ if (!idof) {
+ dev_err(&client->dev, "Probe failed to find an id entry\n");
+ return -ENODEV;
+ }
+
+ /* Now we can set our chip entry */
+ chip = &chips[(int)idof->data];
+
i2c_set_clientdata(client, ds1307);

ds1307->client = client;
- ds1307->type = id->driver_data;
+ ds1307->type = (int) idof->data;

if (!pdata && client->dev.of_node)
ds1307_trickle_of_init(client, chip);
@@ -1435,6 +1448,9 @@ read_rtc:

*/
tmp = ds1307->regs[DS1307_REG_SECS];
switch (ds1307->type) {
+ case maxim_1307:

+ dev_info(&client->dev, "I'm a Maxim ... \n");

+ /* fallthrough */
case ds_1307:
case m41t00:
/* clock halted? turn it on, so clock can tick. */

@@ -1613,10 +1629,10 @@ static int ds1307_remove(struct i2c_client *client)

static struct i2c_driver ds1307_driver = {
.driver = {
.name = "rtc-ds1307",
+ .of_match_table = of_match_ptr(ds1307_dt_ids),
},

- .probe = ds1307_probe,
+ .probe_new = ds1307_probe,

.remove = ds1307_remove,
- .id_table = ds1307_id,
};

module_i2c_driver(ds1307_driver);

--
2.7.4

[Wolfram Sang]

> As we match on only the device, not the manufacturer, I've changed your sketch
> so that the test maxim line is on a compatible with name ds9999 to make it
> unique. Otherwise, we would match to the dallas,ds1307 id type.

OK, so I assumed correctly that userspace instantiation wouldn't have
worked fully.

> If you would prefer that we support separate manufacturers, I can update the
> match function so that it attempts a full match first, followed by a stripped
> manufacturer match. I'm not certain if we have a need for that at the moment
> though, as the current drivers simply match on the device name:

I think we *need* that. We can't instantiate my previous example via
userspace otherwise. Also, stripping vendor names is what we want to get
rid of with this series, no?

> This patch also demonstrates a method to obtain the device ID from the new
> match system for drivers which would normally have expected this information to
> be passed in.

Thanks for the testing!

Wolfram

[Kieran Bingham]

Hi Wolfram,

On 13/06/16 18:13, Wolfram Sang wrote:
>
>> As we match on only the device, not the manufacturer, I've changed your sketch
>> so that the test maxim line is on a compatible with name ds9999 to make it
>> unique. Otherwise, we would match to the dallas,ds1307 id type.
>
> OK, so I assumed correctly that userspace instantiation wouldn't have
> worked fully.
>
>> If you would prefer that we support separate manufacturers, I can update the
>> match function so that it attempts a full match first, followed by a stripped
>> manufacturer match. I'm not certain if we have a need for that at the moment
>> though, as the current drivers simply match on the device name:
>
> I think we *need* that. We can't instantiate my previous example via
> userspace otherwise. Also, stripping vendor names is what we want to get
> rid of with this series, no?

Awww <multiple expletives removed>

I just re-read this - Has this series been blocked on me without me
realising?

/me cowers in a corner in shame...

I think the aim of this series was to simplify the code structures.

Making the vendor names usable, makes sense IMO, but I don't think that
was a goal of the original series.

I think the series was trying to make an improvement *without* changing
functionality / usage.

Re-introducing vendor names into the i2c matching would be a change in
scope from my view, but if it's required to get this series moving let
me know.

>> This patch also demonstrates a method to obtain the device ID from the new
>> match system for drivers which would normally have expected this information to
>> be passed in.
>
> Thanks for the testing!
>
> Wolfram
>

--
Regards

Kieran Bingham