On Wed, Aug 02, 2023 at 12:23:17PM +0100, Biju Das wrote:
> Extend i2c_device_get_match_data() to i2c sysfs interface by changing
> i2c_of_match_device_sysfs() to non-static and call this function as a
> fallback for i2c_get_match_data_helper().
...
> /* 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;
Oh, yeah, definitely, please follow advice for previous patch I have given.
> if (!client || !dev->driver)
> return NULL;
>
> - return i2c_get_match_data_helper(client);
> + data = i2c_get_match_data_helper(client);
> + if (data)
> + return data;
> +
> + if (dev->driver->of_match_table) {
> + const struct of_device_id *match;
> +
> + match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
> + client);
So, with this it makes sense to have
strucct device_driver *drv = dev->driver;
...
if (!drv)
return NULL;
The above for the first patch even. And here
match = i2c_of_match_device_sysfs(drv->of_match_table, client);
> + if (match)
> + return match->data;
> + }
> + return data;
This actually should be
return NULL;
to avoid (potentially) stale pointers.
> }
...
> -static const struct of_device_id*
> +const struct of_device_id *
> i2c_of_match_device_sysfs(const struct of_device_id *matches,
> - struct i2c_client *client)
> + const struct i2c_client *client)
> {
> const char *name;
>
> @@ -141,6 +141,7 @@ i2c_of_match_device_sysfs(const struct of_device_id *matches,
>
> return NULL;
> }
> +EXPORT_SYMBOL_GPL(i2c_of_match_device_sysfs);
>
> const struct of_device_id
> *i2c_of_match_device(const struct of_device_id *matches,
> diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
> index 1247e6e6e975..e4d397b67989 100644
> --- a/drivers/i2c/i2c-core.h
> +++ b/drivers/i2c/i2c-core.h
> @@ -82,8 +82,17 @@ static inline void i2c_acpi_remove_space_handler(struct i2c_adapter *adapter) {
>
> #ifdef CONFIG_OF
> void of_i2c_register_devices(struct i2c_adapter *adap);
> +const struct of_device_id *
> +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> + const struct i2c_client *client);
> #else
> static inline void of_i2c_register_devices(struct i2c_adapter *adap) { }
> +static inline const struct of_device_id *
> +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> + const struct i2c_client *client)
> +{
> + return NULL;
> +}
> #endif
> extern struct notifier_block i2c_of_notifier;
That's what above I meant to see as a separate change.
--
With Best Regards,
Andy Shevchenko
