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

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

Hi Sakari,

Am Freitag, 22. Juli 2022, 08:28:55 CEST schrieb Sakari Ailus:
> Hi Alexander,
> 
> On Wed, Jul 20, 2022 at 09:54:40AM +0200, Alexander Stein wrote:
> > 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.
> Where would that information come from?

I don't have experience with ACPI. So you essentially are saying ACPI can't 
provide this kind of information?

> > 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?
> 
> How would you "fix" it? It currently uses the driver name unless the driver
> set the name for the device. This works reasonably well in all cases.

If ACPI can't provide the actual device model name, then you can't fix it, 
agreed.

Best regards,
Alexander