Re: [PATCH v2] regulator: max20086: Make similar OF and ID table

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Biju,

Thank you for the patch.

On Sun, Sep 03, 2023 at 04:42:57PM +0100, Biju Das wrote:
> Make similar OF and ID table to extend support for ID match using
> i2c_match_data(). Currently it works only for OF match tables as the
> driver_data is wrong for ID match.

If it's currently broken for the I2C ID table, let's drop it instead of
fixing it. We can always add it back when this device will appear on
non-OF systems, if it ever does. The max20086_dt_ids table would then
not need to be modified to extract the max20086_chip_info instances.

> While at it, drop blank lines before MODULE_DEVICE_TABLE* and remove
> trailing comma in the terminator entry for OF/ID table.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> Note:
>  This patch is only compile tested.
> 
> v1->v2:
>  * Removed trailing comma in the terminator entry for OF/ID table.
> ---
>  drivers/regulator/max20086-regulator.c | 65 ++++++++++++--------------
>  1 file changed, 31 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/regulator/max20086-regulator.c b/drivers/regulator/max20086-regulator.c
> index 32f47b896fd1..59eb23d467ec 100644
> --- a/drivers/regulator/max20086-regulator.c
> +++ b/drivers/regulator/max20086-regulator.c
> @@ -223,7 +223,7 @@ static int max20086_i2c_probe(struct i2c_client *i2c)
>  		return -ENOMEM;
>  
>  	chip->dev = &i2c->dev;
> -	chip->info = device_get_match_data(chip->dev);
> +	chip->info = i2c_get_match_data(i2c);
>  
>  	i2c_set_clientdata(i2c, chip);
>  
> @@ -275,45 +275,42 @@ static int max20086_i2c_probe(struct i2c_client *i2c)
>  	return 0;
>  }
>  
> -static const struct i2c_device_id max20086_i2c_id[] = {
> -	{ "max20086" },
> -	{ "max20087" },
> -	{ "max20088" },
> -	{ "max20089" },
> -	{ /* Sentinel */ },
> +static const struct max20086_chip_info max20086_chip_info = {
> +	.id = MAX20086_DEVICE_ID_MAX20086,
> +	.num_outputs = 4,
> +};
> +
> +static const struct max20086_chip_info max20087_chip_info = {
> +	.id = MAX20086_DEVICE_ID_MAX20087,
> +	.num_outputs = 4,
> +};
> +
> +static const struct max20086_chip_info max20088_chip_info = {
> +	.id = MAX20086_DEVICE_ID_MAX20088,
> +	.num_outputs = 2,
> +};
> +
> +static const struct max20086_chip_info max20089_chip_info = {
> +	.id = MAX20086_DEVICE_ID_MAX20089,
> +	.num_outputs = 2,
>  };
>  
> +static const struct i2c_device_id max20086_i2c_id[] = {
> +	{ "max20086", (kernel_ulong_t)&max20086_chip_info },
> +	{ "max20087", (kernel_ulong_t)&max20087_chip_info },
> +	{ "max20088", (kernel_ulong_t)&max20088_chip_info },
> +	{ "max20089", (kernel_ulong_t)&max20089_chip_info },
> +	{ /* Sentinel */ }
> +};
>  MODULE_DEVICE_TABLE(i2c, max20086_i2c_id);
>  
>  static const struct of_device_id max20086_dt_ids[] __maybe_unused = {
> -	{
> -		.compatible = "maxim,max20086",
> -		.data = &(const struct max20086_chip_info) {
> -			.id = MAX20086_DEVICE_ID_MAX20086,
> -			.num_outputs = 4,
> -		}
> -	}, {
> -		.compatible = "maxim,max20087",
> -		.data = &(const struct max20086_chip_info) {
> -			.id = MAX20086_DEVICE_ID_MAX20087,
> -			.num_outputs = 4,
> -		}
> -	}, {
> -		.compatible = "maxim,max20088",
> -		.data = &(const struct max20086_chip_info) {
> -			.id = MAX20086_DEVICE_ID_MAX20088,
> -			.num_outputs = 2,
> -		}
> -	}, {
> -		.compatible = "maxim,max20089",
> -		.data = &(const struct max20086_chip_info) {
> -			.id = MAX20086_DEVICE_ID_MAX20089,
> -			.num_outputs = 2,
> -		}
> -	},
> -	{ /* Sentinel */ },
> +	{ .compatible = "maxim,max20086", .data = &max20086_chip_info },
> +	{ .compatible = "maxim,max20087", .data = &max20087_chip_info },
> +	{ .compatible = "maxim,max20088", .data = &max20088_chip_info },
> +	{ .compatible = "maxim,max20089", .data = &max20089_chip_info },
> +	{ /* Sentinel */ }
>  };
> -
>  MODULE_DEVICE_TABLE(of, max20086_dt_ids);
>  
>  static struct i2c_driver max20086_regulator_driver = {

-- 
Regards,

Laurent Pinchart