Hi Biju
On Sun, Sep 10, 2023 at 04:21:59PM +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 struct adv7180_chip_info as match data for
> both these tables and replace the ID lookup table for the match data by
> i2c_get_match_data().
>
> 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/adv7180.c | 60 ++++++++++++++++++-------------------
> 1 file changed, 29 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 99ba925e8ec8..fc4f29e74e05 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -1395,7 +1395,6 @@ static int init_device(struct adv7180_state *state)
>
> static int adv7180_probe(struct i2c_client *client)
> {
> - const struct i2c_device_id *id = i2c_client_get_device_id(client);
> struct device_node *np = client->dev.of_node;
> struct adv7180_state *state;
> struct v4l2_subdev *sd;
> @@ -1411,7 +1410,7 @@ static int adv7180_probe(struct i2c_client *client)
>
> state->client = client;
> state->field = V4L2_FIELD_ALTERNATE;
> - state->chip_info = (struct adv7180_chip_info *)id->driver_data;
> + state->chip_info = i2c_get_match_data(client);
>
> state->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
> GPIOD_OUT_HIGH);
> @@ -1536,22 +1535,6 @@ static void adv7180_remove(struct i2c_client *client)
> mutex_destroy(&state->mutex);
> }
>
> -static const struct i2c_device_id adv7180_id[] = {
> - { "adv7180", (kernel_ulong_t)&adv7180_info },
> - { "adv7180cp", (kernel_ulong_t)&adv7180_info },
> - { "adv7180st", (kernel_ulong_t)&adv7180_info },
> - { "adv7182", (kernel_ulong_t)&adv7182_info },
> - { "adv7280", (kernel_ulong_t)&adv7280_info },
> - { "adv7280-m", (kernel_ulong_t)&adv7280_m_info },
> - { "adv7281", (kernel_ulong_t)&adv7281_info },
> - { "adv7281-m", (kernel_ulong_t)&adv7281_m_info },
> - { "adv7281-ma", (kernel_ulong_t)&adv7281_ma_info },
> - { "adv7282", (kernel_ulong_t)&adv7282_info },
> - { "adv7282-m", (kernel_ulong_t)&adv7282_m_info },
> - {},
> -};
> -MODULE_DEVICE_TABLE(i2c, adv7180_id);
> -
> #ifdef CONFIG_PM_SLEEP
> static int adv7180_suspend(struct device *dev)
> {
> @@ -1585,22 +1568,37 @@ static SIMPLE_DEV_PM_OPS(adv7180_pm_ops, adv7180_suspend, adv7180_resume);
> #define ADV7180_PM_OPS NULL
> #endif
>
> +static const struct i2c_device_id adv7180_id[] = {
> + { "adv7180", (kernel_ulong_t)&adv7180_info },
> + { "adv7180cp", (kernel_ulong_t)&adv7180_info },
> + { "adv7180st", (kernel_ulong_t)&adv7180_info },
> + { "adv7182", (kernel_ulong_t)&adv7182_info },
> + { "adv7280", (kernel_ulong_t)&adv7280_info },
> + { "adv7280-m", (kernel_ulong_t)&adv7280_m_info },
> + { "adv7281", (kernel_ulong_t)&adv7281_info },
> + { "adv7281-m", (kernel_ulong_t)&adv7281_m_info },
> + { "adv7281-ma", (kernel_ulong_t)&adv7281_ma_info },
> + { "adv7282", (kernel_ulong_t)&adv7282_info },
> + { "adv7282-m", (kernel_ulong_t)&adv7282_m_info },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, adv7180_id);
> +
> #ifdef CONFIG_OF
> static const struct of_device_id adv7180_of_id[] = {
> - { .compatible = "adi,adv7180", },
> - { .compatible = "adi,adv7180cp", },
> - { .compatible = "adi,adv7180st", },
> - { .compatible = "adi,adv7182", },
> - { .compatible = "adi,adv7280", },
> - { .compatible = "adi,adv7280-m", },
> - { .compatible = "adi,adv7281", },
> - { .compatible = "adi,adv7281-m", },
> - { .compatible = "adi,adv7281-ma", },
> - { .compatible = "adi,adv7282", },
> - { .compatible = "adi,adv7282-m", },
> - { },
> + { .compatible = "adi,adv7180", &adv7180_info },
> + { .compatible = "adi,adv7180cp", &adv7180_info },
> + { .compatible = "adi,adv7180st", &adv7180_info },
> + { .compatible = "adi,adv7182", &adv7182_info },
> + { .compatible = "adi,adv7280", &adv7280_info },
> + { .compatible = "adi,adv7280-m", &adv7280_m_info },
> + { .compatible = "adi,adv7281", &adv7281_info },
> + { .compatible = "adi,adv7281-m", &adv7281_m_info },
> + { .compatible = "adi,adv7281-ma", &adv7281_ma_info },
> + { .compatible = "adi,adv7282", &adv7282_info },
> + { .compatible = "adi,adv7282-m", &adv7282_m_info },
> + {}
> };
> -
> MODULE_DEVICE_TABLE(of, adv7180_of_id);
> #endif
>
> --
> 2.25.1
>
