diff mbox series

[v3,6/7] media: i2c: ov9282: Set v4l2 subdev name according to sensor model

Message ID 20220722131947.2456988-7-alexander.stein@ew.tq-group.com (mailing list archive)
State Changes Requested
Delegated to: Sakari Ailus
Headers
Series OV9281 support |

Commit Message

Alexander Stein July 22, 2022, 1:19 p.m. UTC 
  To distinguish ov9281 & ov9282 the name has to be explicitly set.
i2c_client already has the name parsed from the compatible.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/media/i2c/ov9282.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Sakari Ailus July 22, 2022, 1:50 p.m. UTC | #1 
Hi Alexander,

On Fri, Jul 22, 2022 at 03:19:46PM +0200, Alexander Stein wrote:
> To distinguish ov9281 & ov9282 the name has to be explicitly set.
> i2c_client already has the name parsed from the compatible.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  drivers/media/i2c/ov9282.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index 352dbe21a902..dbc0a4cd060f 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -1047,6 +1047,7 @@ static int ov9282_probe(struct i2c_client *client)
>  
>  	/* Initialize subdev */
>  	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);
> +	v4l2_i2c_subdev_set_name(&ov9282->sd, client, client->name, NULL);
>  

Could you instead do this based on the compatible string in the driver,
using device_get_match_data()? The approach works on non-OF systems, too.

>  	ret = ov9282_parse_hw_config(ov9282);
>  	if (ret) {
Alexander Stein July 25, 2022, 6:38 a.m. UTC | #2 
Hi Sakari,

Am Freitag, 22. Juli 2022, 15:50:30 CEST schrieb Sakari Ailus:
> Hi Alexander,
> 
> On Fri, Jul 22, 2022 at 03:19:46PM +0200, Alexander Stein wrote:
> > To distinguish ov9281 & ov9282 the name has to be explicitly set.
> > i2c_client already has the name parsed from the compatible.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > 
> >  drivers/media/i2c/ov9282.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 352dbe21a902..dbc0a4cd060f 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -1047,6 +1047,7 @@ static int ov9282_probe(struct i2c_client *client)
> > 
> >  	/* Initialize subdev */
> >  	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);
> > 
> > +	v4l2_i2c_subdev_set_name(&ov9282->sd, client, client->name, NULL);
> 
> Could you instead do this based on the compatible string in the driver,
> using device_get_match_data()? The approach works on non-OF systems, too.

I actually don't like doing the same as of_modalias_node() is doing.
Until non-OF support is added (if ever), I don't see any benefit in doing so 
right now.

Best regards,
Alexander

> >  	ret = ov9282_parse_hw_config(ov9282);
> >  	if (ret) {
Sakari Ailus July 28, 2022, 11:47 a.m. UTC | #3 
Hi Alexander,

On Mon, Jul 25, 2022 at 08:38:57AM +0200, Alexander Stein wrote:
> Hi Sakari,
> 
> Am Freitag, 22. Juli 2022, 15:50:30 CEST schrieb Sakari Ailus:
> > Hi Alexander,
> > 
> > On Fri, Jul 22, 2022 at 03:19:46PM +0200, Alexander Stein wrote:
> > > To distinguish ov9281 & ov9282 the name has to be explicitly set.
> > > i2c_client already has the name parsed from the compatible.
> > > 
> > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > ---
> > > 
> > >  drivers/media/i2c/ov9282.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > > index 352dbe21a902..dbc0a4cd060f 100644
> > > --- a/drivers/media/i2c/ov9282.c
> > > +++ b/drivers/media/i2c/ov9282.c
> > > @@ -1047,6 +1047,7 @@ static int ov9282_probe(struct i2c_client *client)
> > > 
> > >  	/* Initialize subdev */
> > >  	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);
> > > 
> > > +	v4l2_i2c_subdev_set_name(&ov9282->sd, client, client->name, NULL);
> > 
> > Could you instead do this based on the compatible string in the driver,
> > using device_get_match_data()? The approach works on non-OF systems, too.
> 
> I actually don't like doing the same as of_modalias_node() is doing.
> Until non-OF support is added (if ever), I don't see any benefit in doing so 
> right now.

client->name will be wrong on un-OF; putting this string to device match
data is a better option. It'll be about the same number of lines, too.
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 352dbe21a902..dbc0a4cd060f 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -1047,6 +1047,7 @@  static int ov9282_probe(struct i2c_client *client)
 
 	/* Initialize subdev */
 	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);
+	v4l2_i2c_subdev_set_name(&ov9282->sd, client, client->name, NULL);
 
 	ret = ov9282_parse_hw_config(ov9282);
 	if (ret) {