linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Match data improvements for adv7180 driver
@ 2023-09-10 15:21 Biju Das
  2023-09-10 15:21 ` [PATCH 1/2] media: i2c: adv7180: Extend match support for OF tables Biju Das
  2023-09-10 15:22 ` [PATCH 2/2] media: i2c: adv7180: Drop CONFIG_OF ifdeffery Biju Das
  0 siblings, 2 replies; 5+ messages in thread
From: Biju Das @ 2023-09-10 15:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Biju Das, Lars-Peter Clausen, linux-media, linux-kernel,
	Biju Das, Andy Shevchenko

This patch series aims to add match data improvements for adv7180 driver.
This patch series is only compile tested.

Biju Das (2):
  media: i2c: adv7180: Extend match support for OF tables
  media: i2c: adv7180: Drop CONFIG_OF ifdeffery

 drivers/media/i2c/adv7180.c | 65 ++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 34 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 1/2] media: i2c: adv7180: Extend match support for OF tables
  2023-09-10 15:21 [PATCH 0/2] Match data improvements for adv7180 driver Biju Das
@ 2023-09-10 15:21 ` Biju Das
  2023-09-11  8:50   ` Jacopo Mondi
  2023-09-10 15:22 ` [PATCH 2/2] media: i2c: adv7180: Drop CONFIG_OF ifdeffery Biju Das
  1 sibling, 1 reply; 5+ messages in thread
From: Biju Das @ 2023-09-10 15:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Biju Das, Lars-Peter Clausen, linux-media, linux-kernel,
	Biju Das, Andy Shevchenko

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@bp.renesas.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] media: i2c: adv7180: Drop CONFIG_OF ifdeffery
  2023-09-10 15:21 [PATCH 0/2] Match data improvements for adv7180 driver Biju Das
  2023-09-10 15:21 ` [PATCH 1/2] media: i2c: adv7180: Extend match support for OF tables Biju Das
@ 2023-09-10 15:22 ` Biju Das
  2023-09-11  8:51   ` Jacopo Mondi
  1 sibling, 1 reply; 5+ messages in thread
From: Biju Das @ 2023-09-10 15:22 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Biju Das, Lars-Peter Clausen, linux-media, linux-kernel,
	Biju Das, Andy Shevchenko

Drop of_match_ptr() from adv7180_driver and get rid of ugly CONFIG_OF
if check. This slightly increases the size of adv7180_driver on non-OF
system and shouldn't be an issue.

Add mod_devicetable.h include.

It also allows, in case if needed, to enumerate this device via ACPI with
PRP0001 magic.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/media/i2c/adv7180.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index fc4f29e74e05..54134473186b 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2013 Cogent Embedded, Inc.
  * Copyright (C) 2013 Renesas Solutions Corp.
  */
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/errno.h>
@@ -1584,7 +1585,6 @@ static const struct i2c_device_id adv7180_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, adv7180_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id adv7180_of_id[] = {
 	{ .compatible = "adi,adv7180", &adv7180_info },
 	{ .compatible = "adi,adv7180cp", &adv7180_info },
@@ -1600,13 +1600,12 @@ static const struct of_device_id adv7180_of_id[] = {
 	{}
 };
 MODULE_DEVICE_TABLE(of, adv7180_of_id);
-#endif
 
 static struct i2c_driver adv7180_driver = {
 	.driver = {
 		   .name = KBUILD_MODNAME,
 		   .pm = ADV7180_PM_OPS,
-		   .of_match_table = of_match_ptr(adv7180_of_id),
+		   .of_match_table = adv7180_of_id,
 		   },
 	.probe = adv7180_probe,
 	.remove = adv7180_remove,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] media: i2c: adv7180: Extend match support for OF tables
  2023-09-10 15:21 ` [PATCH 1/2] media: i2c: adv7180: Extend match support for OF tables Biju Das
@ 2023-09-11  8:50   ` Jacopo Mondi
  0 siblings, 0 replies; 5+ messages in thread
From: Jacopo Mondi @ 2023-09-11  8:50 UTC (permalink / raw)
  To: Biju Das
  Cc: Mauro Carvalho Chehab, Lars-Peter Clausen, linux-media,
	linux-kernel, Biju Das, Andy Shevchenko

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@bp.renesas.com>

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

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] media: i2c: adv7180: Drop CONFIG_OF ifdeffery
  2023-09-10 15:22 ` [PATCH 2/2] media: i2c: adv7180: Drop CONFIG_OF ifdeffery Biju Das
@ 2023-09-11  8:51   ` Jacopo Mondi
  0 siblings, 0 replies; 5+ messages in thread
From: Jacopo Mondi @ 2023-09-11  8:51 UTC (permalink / raw)
  To: Biju Das
  Cc: Mauro Carvalho Chehab, Lars-Peter Clausen, linux-media,
	linux-kernel, Biju Das, Andy Shevchenko

Hi Biju

On Sun, Sep 10, 2023 at 04:22:00PM +0100, Biju Das wrote:
> Drop of_match_ptr() from adv7180_driver and get rid of ugly CONFIG_OF
> if check. This slightly increases the size of adv7180_driver on non-OF
> system and shouldn't be an issue.
>
> Add mod_devicetable.h include.
>
> It also allows, in case if needed, to enumerate this device via ACPI with
> PRP0001 magic.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

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

Thanks
  j

> ---
>  drivers/media/i2c/adv7180.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index fc4f29e74e05..54134473186b 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2013 Cogent Embedded, Inc.
>   * Copyright (C) 2013 Renesas Solutions Corp.
>   */
> +#include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/errno.h>
> @@ -1584,7 +1585,6 @@ static const struct i2c_device_id adv7180_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, adv7180_id);
>
> -#ifdef CONFIG_OF
>  static const struct of_device_id adv7180_of_id[] = {
>  	{ .compatible = "adi,adv7180", &adv7180_info },
>  	{ .compatible = "adi,adv7180cp", &adv7180_info },
> @@ -1600,13 +1600,12 @@ static const struct of_device_id adv7180_of_id[] = {
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, adv7180_of_id);
> -#endif
>
>  static struct i2c_driver adv7180_driver = {
>  	.driver = {
>  		   .name = KBUILD_MODNAME,
>  		   .pm = ADV7180_PM_OPS,
> -		   .of_match_table = of_match_ptr(adv7180_of_id),
> +		   .of_match_table = adv7180_of_id,
>  		   },
>  	.probe = adv7180_probe,
>  	.remove = adv7180_remove,
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-11 21:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-10 15:21 [PATCH 0/2] Match data improvements for adv7180 driver Biju Das
2023-09-10 15:21 ` [PATCH 1/2] media: i2c: adv7180: Extend match support for OF tables Biju Das
2023-09-11  8:50   ` Jacopo Mondi
2023-09-10 15:22 ` [PATCH 2/2] media: i2c: adv7180: Drop CONFIG_OF ifdeffery Biju Das
2023-09-11  8:51   ` Jacopo Mondi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).