Re: Re: [PATCH 1/1] media: v4l2-core: v4l2-i2c: Fix default device name

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi Sakari,

thanks for the feedback.

Am Montag, 18. Juli 2022, 20:39:27 CEST schrieb Sakari Ailus:
> Hi Alexander,
> 
> Thanks for the patch.
> 
> On Mon, Jul 18, 2022 at 02:01:51PM +0200, Alexander Stein wrote:
> > Documentation to v4l2_i2c_subdev_set_name says I²C device name if
> > devname parameter is NULL. But instead the I²C driver name is assigned.
> > Fix this by actually using the i2c_client->name for the subdev name.
> > 
> > Fixes: 0658293012af ("media: v4l: subdev: Add a function to set an I²C
> > sub-device's name") Signed-off-by: Alexander Stein
> > <alexander.stein@ew.tq-group.com>
> > ---
> > Having a sensor node in DT with compatible = "ovti,ov9281" the subdev
> > name is "ov9282" without this patch. With this patch applied it is
> > "ov9281". The i2c_client->name is "ov9281" in every case.
> > 
> >  drivers/media/v4l2-core/v4l2-i2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-i2c.c
> > b/drivers/media/v4l2-core/v4l2-i2c.c index b4acca75644b..90c3c799317a
> > 100644
> > --- a/drivers/media/v4l2-core/v4l2-i2c.c
> > +++ b/drivers/media/v4l2-core/v4l2-i2c.c
> > @@ -33,7 +33,7 @@ void v4l2_i2c_subdev_set_name(struct v4l2_subdev *sd,
> > 
> >  			      const char *devname, const char 
*postfix)
> >  
> >  {
> >  
> >  	if (!devname)
> > 
> > -		devname = client->dev.driver->name;
> > +		devname = client->name;
> 
> This would work OF somehow but not on ACPI. At the moment, at least, I think
> this information needs to come from the driver itself.

I've not much experience regarding ACPI, nor can I test anything. Which 
information from ACPI should be used as devname? AFAICS i2c_acpi_fill_info 
need to be modified to set info->type as well. This should fix the ACPI case.

Nevertheless currently either the documentation or the implementation needs a 
fix, they don't match. You can also opt to driver name in case client->name is 
empty. Is this acceptable?

Best regards,
Alexander