Re: [PATCH 1/2] media: tvp541x: Extend match support for OF tables

[Jacopo Mondi]

Hi Biju

On Sun, Sep 10, 2023 at 06:40:50PM +0100, Biju Das wrote:
> The driver has an OF match table, still, it uses an ID lookup table for
> retrieving match data. Currently, the driver is working on the
> assumption that an I2C device registered via OF will always match a
> legacy I2C device ID. The correct approach is to have an OF device ID
> table using i2c_get_match_data() if the devices are registered via OF/ID.
>
> Unify the OF/ID table by using tvp514x_reg as match data for both
> these tables and replace the ID lookup table for the match data by
> i2c_get_match_data() and simplifly probe().
>
> While at it, remove the trailing comma in the terminator entry for the OF
> table making code robust against (theoretical) misrebases or other similar
> things where the new entry goes _after_ the termination without the
> compiler noticing.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>

Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@xxxxxxxxxxxxxxxx>

Thanks
  j

> ---
>  drivers/media/i2c/tvp514x.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
> index aa6d4b67b6d5..4d0ffaa312c5 100644
> --- a/drivers/media/i2c/tvp514x.c
> +++ b/drivers/media/i2c/tvp514x.c
> @@ -118,7 +118,7 @@ struct tvp514x_decoder {
>  	struct media_pad pad;
>  	struct v4l2_mbus_framefmt format;
>
> -	struct tvp514x_reg *int_seq;
> +	const struct tvp514x_reg *int_seq;
>  };
>
>  /* TVP514x default register values */
> @@ -1024,7 +1024,6 @@ tvp514x_get_pdata(struct i2c_client *client)
>  static int
>  tvp514x_probe(struct i2c_client *client)
>  {
> -	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	struct tvp514x_platform_data *pdata = tvp514x_get_pdata(client);
>  	struct tvp514x_decoder *decoder;
>  	struct v4l2_subdev *sd;
> @@ -1049,7 +1048,7 @@ tvp514x_probe(struct i2c_client *client)
>  	memcpy(decoder->tvp514x_regs, tvp514x_reg_list_default,
>  			sizeof(tvp514x_reg_list_default));
>
> -	decoder->int_seq = (struct tvp514x_reg *)id->driver_data;
> +	decoder->int_seq = i2c_get_match_data(client);
>
>  	/* Copy board specific information here */
>  	decoder->pdata = pdata;
> @@ -1183,22 +1182,21 @@ static const struct tvp514x_reg tvp514xm_init_reg_seq[] = {
>   * driver_data - Driver data
>   */
>  static const struct i2c_device_id tvp514x_id[] = {
> -	{"tvp5146", (unsigned long)tvp5146_init_reg_seq},
> -	{"tvp5146m2", (unsigned long)tvp514xm_init_reg_seq},
> -	{"tvp5147", (unsigned long)tvp5147_init_reg_seq},
> -	{"tvp5147m1", (unsigned long)tvp514xm_init_reg_seq},
> -	{},
> +	{"tvp5146", (kernel_ulong_t)tvp5146_init_reg_seq },
> +	{"tvp5146m2", (kernel_ulong_t)tvp514xm_init_reg_seq },
> +	{"tvp5147", (kernel_ulong_t)tvp5147_init_reg_seq },
> +	{"tvp5147m1", (kernel_ulong_t)tvp514xm_init_reg_seq },
> +	{ /* sentinel */ }
>  };
> -
>  MODULE_DEVICE_TABLE(i2c, tvp514x_id);
>
>  #if IS_ENABLED(CONFIG_OF)
>  static const struct of_device_id tvp514x_of_match[] = {
> -	{ .compatible = "ti,tvp5146", },
> -	{ .compatible = "ti,tvp5146m2", },
> -	{ .compatible = "ti,tvp5147", },
> -	{ .compatible = "ti,tvp5147m1", },
> -	{ /* sentinel */ },
> +	{ .compatible = "ti,tvp5146", .data = tvp5146_init_reg_seq },
> +	{ .compatible = "ti,tvp5146m2", .data = tvp514xm_init_reg_seq },
> +	{ .compatible = "ti,tvp5147", .data = tvp5147_init_reg_seq },
> +	{ .compatible = "ti,tvp5147m1", .data = tvp514xm_init_reg_seq },
> +	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, tvp514x_of_match);
>  #endif
> --
> 2.25.1
>