On Tue, Aug 01, 2023 at 06:03:18PM +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().
Have you used --patience when prepared the patch for sending?
...
> -const void *i2c_get_match_data(const struct i2c_client *client)
> +static const void *i2c_get_match_data_helper(const struct i2c_driver *driver,
> + const struct i2c_client *client)
> {
> - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
Does it make sense to remove and add an additional parameter? In one case it's
a copy, in another it probably the same, just hidden in the code.
> 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;
> const void *data;
> + if (!dev->driver)
> + return NULL;
When can this be true?
> + data = i2c_get_match_data_helper(to_i2c_driver(dev->driver), client);
> + if (data)
> + return data;
>
> - data = (const void *)match->driver_data;
> + if (dev->driver->of_match_table) {
> + const struct of_device_id *match;
> +
> + match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
> + (struct i2c_client *)client);
> + if (match)
> + data = match->data;
> }
>
> return data;
> }
...
> -static const struct of_device_id*
> +const struct of_device_id*
While here, add a missing space after of_device_id.
...
> +const struct of_device_id*
Ditto.
Or use below (weird) style in case it occurs more often than usual one.
> +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> + struct i2c_client *client);
> +
> const struct of_device_id
> *i2c_of_match_device(const struct of_device_id *matches,
> struct i2c_client *client);
...
> +static inline const struct of_device_id
> +*i2c_of_match_device(const struct of_device_id *matches,
As per above.
> + struct i2c_client *client)
> +{
> + return NULL;
> +}
--
With Best Regards,
Andy Shevchenko
