Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v5 RESEND 2/4] i2c: Add i2c_device_get_match_data()
> callback
>
> On Thu, Aug 03, 2023 at 11:31:00AM +0100, Biju Das wrote:
> > Add i2c_device_get_match_data() callback to struct bus_type().
> >
> > While at it, introduced i2c_get_match_data_helper() to avoid code
> > duplication with i2c_get_match_data().
>
> It seems you are missing to Cc Andi for all these... (not your fault,
> rather unfortunately).
>
> Yes, while he is not directly involved into core changes the drivers are
> pretty much should consider this change.
Sure.
>
> ...
>
> > data = device_get_match_data(&client->dev);
> > - if (!data) {
> > - match = i2c_match_id(driver->id_table, client);
> > - if (!match)
> > - return NULL;
> > + if (data)
> > + return data;
> >
> > - data = (const void *)match->driver_data;
> > - }
> > -
> > - return data;
>
> Looking at this, it _might_ make sense to split another patch to prepare
> for better difference here.
OK.
>
> - if (!data) {
> - match = i2c_match_id(driver->id_table, client);
> - if (!match)
> - return NULL;
> + if (data)
> + return data;
> +
> + match = i2c_match_id(driver->id_table, client);
> + if (!match)
> + return NULL;
> +
> + return (const void *)match->driver_data;
>
> Just play with this idea.
Does these below 2 patches ok?
PATCH x:
-------
Subject: [PATCH 1/2] i2c: Enhance i2c_get_match_data()
Enhance i2c_get_match_data() for a faster path for device_get_
match_data().
While at it, add const to struct i2c_driver to prevent overriding
the driver pointer.
Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
---
drivers/i2c/i2c-core-base.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 60746652fd52..7005dfe64066 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -116,20 +116,19 @@ EXPORT_SYMBOL_GPL(i2c_match_id);
const void *i2c_get_match_data(const struct i2c_client *client)
{
- struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
+ const struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
const struct i2c_device_id *match;
const void *data;
data = device_get_match_data(&client->dev);
- if (!data) {
- match = i2c_match_id(driver->id_table, client);
- if (!match)
- return NULL;
+ if (data)
+ return data;
- data = (const void *)match->driver_data;
- }
+ match = i2c_match_id(driver->id_table, client);
+ if (!match)
+ return NULL;
- return data;
+ return (const void *)match->driver_data;
}
EXPORT_SYMBOL(i2c_get_match_data);
Patch x+1:
---------
Subject: [PATCH 2/2] i2c: Add i2c_device_get_match_data() callback
Add i2c_device_get_match_data() callback to struct bus_type().
While at it, introduced i2c_get_match_data_helper() to avoid code
duplication with i2c_get_match_data().
Suggested-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
---
drivers/i2c/i2c-core-base.c | 53 ++++++++++++++++++++++++++++++++-----
1 file changed, 47 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 7005dfe64066..d543460e47c2 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -114,15 +114,10 @@ const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
}
EXPORT_SYMBOL_GPL(i2c_match_id);
-const void *i2c_get_match_data(const struct i2c_client *client)
+static const void *i2c_get_match_data_helper(const struct i2c_client *client)
{
const struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
const struct i2c_device_id *match;
- const void *data;
-
- data = device_get_match_data(&client->dev);
- if (data)
- return data;
match = i2c_match_id(driver->id_table, client);
if (!match)
@@ -130,6 +125,51 @@ const void *i2c_get_match_data(const struct i2c_client *client)
return (const void *)match->driver_data;
}
+
+static const void *i2c_device_get_match_data(const struct device *dev)
+{
+ const struct device_driver *drv = dev->driver;
+ const struct i2c_client *client;
+ const void *data;
+
+ /*
+ * It is not guaranteed that the function is always called on a device
+ * bound to a driver (even though we normally expect this to be the
+ * case).
+ */
+ if (!drv)
+ return NULL;
+
+ /* TODO: use i2c_verify_client() when it accepts const pointer */
+ client = (dev->type == &i2c_client_type) ? to_i2c_client(dev) : NULL;
+ if (!client)
+ return NULL;
+
+ data = i2c_get_match_data_helper(client);
+ if (data)
+ return data;
+
+ if (drv->of_match_table) {
+ const struct of_device_id *match;
+
+ match = i2c_of_match_device_sysfs(drv->of_match_table, client);
+ if (match)
+ return match->data;
+ }
+
+ return NULL;
+}
+
+const void *i2c_get_match_data(const struct i2c_client *client)
+{
+ const void *data;
+
+ data = device_get_match_data(&client->dev);
+ if (data)
+ return data;
+
+ return i2c_get_match_data_helper(client);
+}
EXPORT_SYMBOL(i2c_get_match_data);
static int i2c_device_match(struct device *dev, struct device_driver *drv)
@@ -694,6 +734,7 @@ struct bus_type i2c_bus_type = {
.probe = i2c_device_probe,
.remove = i2c_device_remove,
.shutdown = i2c_device_shutdown,
+ .get_match_data = i2c_device_get_match_data,
};
EXPORT_SYMBOL_GPL(i2c_bus_type);
Cheers,
Biju
