Re: [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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);
> +}

-- 
With Best Regards,
Andy Shevchenko