| Message ID | 20230802112317.252745-3-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Extend device_get_match_data() to struct bus_type | expand |
On Wed, Aug 02, 2023 at 12:23:16PM +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(). ... > +static const void *i2c_get_match_data_helper(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; > + > + match = i2c_match_id(driver->id_table, client); > + if (!match) > + return NULL; > + > + return (const void *)match->driver_data; > +} Yes, perfect! ... > +static const void *i2c_device_get_match_data(const struct device *dev) > +{ > + /* TODO: use i2c_verify_client() when it accepts const pointer */ > + const struct i2c_client *client = (dev->type == &i2c_client_type) ? > + to_i2c_client(dev) : NULL; > + > + if (!client || !dev->driver) > + return NULL; > + > + return i2c_get_match_data_helper(client); I believe below looks better from readability and maintenance perspectives. const struct i2c_client *client; /* ...comment as in Dmitry's reply in v3 thread on why we need this check... */ if (!dev->driver) 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; > + return i2c_get_match_data_helper(client); > +}
Hi Andy Shevchenko, Thanks for the feedback. > Subject: Re: [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() > callback > > On Wed, Aug 02, 2023 at 12:23:16PM +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(). > > ... > > > +static const void *i2c_get_match_data_helper(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; > > + > > + match = i2c_match_id(driver->id_table, client); > > + if (!match) > > + return NULL; > > + > > + return (const void *)match->driver_data; } > > Yes, perfect! > > ... > > > +static const void *i2c_device_get_match_data(const struct device > > +*dev) { > > + /* TODO: use i2c_verify_client() when it accepts const pointer */ > > + const struct i2c_client *client = (dev->type == > &i2c_client_type) ? > > + to_i2c_client(dev) : NULL; > > + > > + if (!client || !dev->driver) > > + return NULL; > > + > > + return i2c_get_match_data_helper(client); > > > I believe below looks better from readability and maintenance > perspectives. Agreed. Will Incorporate latest comment from v4 as well. Cheers, Biju > > const struct i2c_client *client; > > /* ...comment as in Dmitry's reply in v3 thread on why we need > this check... */ > if (!dev->driver) > 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; > > > + return i2c_get_match_data_helper(client); > > +} > > -- > With Best Regards, > Andy Shevchenko >
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 60746652fd52..2436f23e63af 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -114,22 +114,39 @@ 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) { - 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; + + match = i2c_match_id(driver->id_table, client); + if (!match) + return NULL; + + return (const void *)match->driver_data; +} + +static const void *i2c_device_get_match_data(const struct device *dev) +{ + /* TODO: use i2c_verify_client() when it accepts const pointer */ + const struct i2c_client *client = (dev->type == &i2c_client_type) ? + to_i2c_client(dev) : NULL; + + if (!client || !dev->driver) + return NULL; + + return i2c_get_match_data_helper(client); +} + +const void *i2c_get_match_data(const struct i2c_client *client) +{ 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; - } - - return data; + return i2c_get_match_data_helper(client); } EXPORT_SYMBOL(i2c_get_match_data); @@ -695,6 +712,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);
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@gmail.com> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v3->v4: * Dropped struct i2c_driver parameter from i2c_get_match_data_helper() * Split I2C sysfs handling in seperate patch. v2->v3: * Extended to support i2c_of_match_device() as suggested by Andy. * Changed i2c_of_match_device_sysfs() as non-static function as it is needed for i2c_device_get_match_data(). * Added a TODO comment to use i2c_verify_client() when it accepts const pointer. * Added multiple returns to make code path for device_get_match_data() faster in i2c_get_match_data(). RFC v1->v2: * Replaced "Signed-off-by"->"Suggested-by" tag for Dmitry. * Fixed build warnings reported by kernel test robot <lkp@intel.com> * Added const qualifier to return type and parameter struct i2c_driver in i2c_get_match_data_helper(). * Added const qualifier to struct i2c_driver in i2c_get_match_data() * Dropped driver variable from i2c_device_get_match_data() * Replaced to_i2c_client with logic for assigning verify_client as it returns non const pointer. --- drivers/i2c/i2c-core-base.c | 38 +++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-)