* [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name
@ 2018-08-01 21:20 Philippe De Muyter
2018-08-01 21:20 ` [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation Philippe De Muyter
2018-09-05 7:53 ` [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name Hans Verkuil
0 siblings, 2 replies; 7+ messages in thread
From: Philippe De Muyter @ 2018-08-01 21:20 UTC (permalink / raw)
To: linux-media, hans.verkuil; +Cc: Philippe De Muyter
While v4l2_i2c_subdev_init does give a unique name to the subdev, matching
the one appearing in dmesg for messages generated by dev_info and friends
(e.g. imx185 30-0010), v4l2_spi_subdev_init does a poor job, copying only
the driver name, but not the dev_name(), yielding e.g. "imx185", but
missing the "spi1.1" part, and not generating a unique name.
Fix that.
Signed-off-by: Philippe De Muyter <phdm@macqel.be>
---
drivers/media/v4l2-core/v4l2-common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index b518b92..5471c6d 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -255,7 +255,9 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
v4l2_set_subdevdata(sd, spi);
spi_set_drvdata(spi, sd);
/* initialize name */
- strlcpy(sd->name, spi->dev.driver->name, sizeof(sd->name));
+ snprintf(sd->name, sizeof(sd->name), "%s %s",
+ spi->dev.driver->name, dev_name(&spi->dev));
+
}
EXPORT_SYMBOL_GPL(v4l2_spi_subdev_init);
--
1.8.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
2018-08-01 21:20 [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name Philippe De Muyter
@ 2018-08-01 21:20 ` Philippe De Muyter
2018-08-03 12:43 ` Sakari Ailus
2018-09-05 7:53 ` [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name Hans Verkuil
1 sibling, 1 reply; 7+ messages in thread
From: Philippe De Muyter @ 2018-08-01 21:20 UTC (permalink / raw)
To: linux-media, hans.verkuil; +Cc: Philippe De Muyter
When v4l2_i2c_subdev_init is called, dev_name(&client->dev) has already
been set. Use it to generate subdev's name instead of recreating it
with "%d-%04x". This improves the similarity in subdev's name creation
between v4l2_i2c_subdev_init and v4l2_spi_subdev_init.
Signed-off-by: Philippe De Muyter <phdm@macqel.be>
---
drivers/media/v4l2-core/v4l2-common.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 5471c6d..b062111 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
v4l2_set_subdevdata(sd, client);
i2c_set_clientdata(client, sd);
/* initialize name */
- snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
- client->dev.driver->name, i2c_adapter_id(client->adapter),
- client->addr);
+ snprintf(sd->name, sizeof(sd->name), "%s %s",
+ client->dev.driver->name, dev_name(&client->dev));
}
EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
--
1.8.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
2018-08-01 21:20 ` [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation Philippe De Muyter
@ 2018-08-03 12:43 ` Sakari Ailus
2018-08-03 13:46 ` Philippe De Muyter
2018-08-08 8:43 ` Philippe De Muyter
0 siblings, 2 replies; 7+ messages in thread
From: Sakari Ailus @ 2018-08-03 12:43 UTC (permalink / raw)
To: Philippe De Muyter; +Cc: linux-media, hans.verkuil
Hi Philippe,
On Wed, Aug 01, 2018 at 11:20:57PM +0200, Philippe De Muyter wrote:
> When v4l2_i2c_subdev_init is called, dev_name(&client->dev) has already
> been set. Use it to generate subdev's name instead of recreating it
> with "%d-%04x". This improves the similarity in subdev's name creation
> between v4l2_i2c_subdev_init and v4l2_spi_subdev_init.
>
> Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> ---
> drivers/media/v4l2-core/v4l2-common.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 5471c6d..b062111 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> v4l2_set_subdevdata(sd, client);
> i2c_set_clientdata(client, sd);
> /* initialize name */
> - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> - client->dev.driver->name, i2c_adapter_id(client->adapter),
> - client->addr);
> + snprintf(sd->name, sizeof(sd->name), "%s %s",
> + client->dev.driver->name, dev_name(&client->dev));
> }
> EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
>
I like the patch in principle. But what's the effect of this on the actual
sub-device (and entity) names? Looking at i2c_dev_set_name(), this will be
different. We can't change the existing entity naming in drivers, this will
break applications that expect them to be named in a certain way.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
2018-08-03 12:43 ` Sakari Ailus
@ 2018-08-03 13:46 ` Philippe De Muyter
2018-08-03 14:11 ` Sakari Ailus
2018-08-08 8:43 ` Philippe De Muyter
1 sibling, 1 reply; 7+ messages in thread
From: Philippe De Muyter @ 2018-08-03 13:46 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, hans.verkuil
Hi Sakari,
On Fri, Aug 03, 2018 at 03:43:15PM +0300, Sakari Ailus wrote:
> Hi Philippe,
>
> On Wed, Aug 01, 2018 at 11:20:57PM +0200, Philippe De Muyter wrote:
> > When v4l2_i2c_subdev_init is called, dev_name(&client->dev) has already
> > been set. Use it to generate subdev's name instead of recreating it
> > with "%d-%04x". This improves the similarity in subdev's name creation
> > between v4l2_i2c_subdev_init and v4l2_spi_subdev_init.
> >
> > Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> > ---
> > drivers/media/v4l2-core/v4l2-common.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 5471c6d..b062111 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> > v4l2_set_subdevdata(sd, client);
> > i2c_set_clientdata(client, sd);
> > /* initialize name */
> > - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> > - client->dev.driver->name, i2c_adapter_id(client->adapter),
> > - client->addr);
> > + snprintf(sd->name, sizeof(sd->name), "%s %s",
> > + client->dev.driver->name, dev_name(&client->dev));
> > }
> > EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
> >
>
> I like the patch in principle. But what's the effect of this on the actual
> sub-device (and entity) names? Looking at i2c_dev_set_name(), this will be
> different. We can't change the existing entity naming in drivers, this will
> break applications that expect them to be named in a certain way.
Yeah. I am 10 years too late.
Maybe adding for a long transition period a kernel message giving the new
name and the old one if they are different ?
Thank you
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
2018-08-03 13:46 ` Philippe De Muyter
@ 2018-08-03 14:11 ` Sakari Ailus
0 siblings, 0 replies; 7+ messages in thread
From: Sakari Ailus @ 2018-08-03 14:11 UTC (permalink / raw)
To: Philippe De Muyter; +Cc: linux-media, hans.verkuil
On Fri, Aug 03, 2018 at 03:46:32PM +0200, Philippe De Muyter wrote:
> Hi Sakari,
>
> On Fri, Aug 03, 2018 at 03:43:15PM +0300, Sakari Ailus wrote:
> > Hi Philippe,
> >
> > On Wed, Aug 01, 2018 at 11:20:57PM +0200, Philippe De Muyter wrote:
> > > When v4l2_i2c_subdev_init is called, dev_name(&client->dev) has already
> > > been set. Use it to generate subdev's name instead of recreating it
> > > with "%d-%04x". This improves the similarity in subdev's name creation
> > > between v4l2_i2c_subdev_init and v4l2_spi_subdev_init.
> > >
> > > Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> > > ---
> > > drivers/media/v4l2-core/v4l2-common.c | 5 ++---
> > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > > index 5471c6d..b062111 100644
> > > --- a/drivers/media/v4l2-core/v4l2-common.c
> > > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > > @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> > > v4l2_set_subdevdata(sd, client);
> > > i2c_set_clientdata(client, sd);
> > > /* initialize name */
> > > - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> > > - client->dev.driver->name, i2c_adapter_id(client->adapter),
> > > - client->addr);
> > > + snprintf(sd->name, sizeof(sd->name), "%s %s",
> > > + client->dev.driver->name, dev_name(&client->dev));
> > > }
> > > EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
> > >
> >
> > I like the patch in principle. But what's the effect of this on the actual
> > sub-device (and entity) names? Looking at i2c_dev_set_name(), this will be
> > different. We can't change the existing entity naming in drivers, this will
> > break applications that expect them to be named in a certain way.
>
> Yeah. I am 10 years too late.
>
> Maybe adding for a long transition period a kernel message giving the new
> name and the old one if they are different ?
I think this information would be usable for the property API if we ever get
around defining it precisely and implementing it. That's long term stuff in
any case.
<URL:https://spinics.net/lists/linux-media/msg90160.html>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
2018-08-03 12:43 ` Sakari Ailus
2018-08-03 13:46 ` Philippe De Muyter
@ 2018-08-08 8:43 ` Philippe De Muyter
1 sibling, 0 replies; 7+ messages in thread
From: Philippe De Muyter @ 2018-08-08 8:43 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, hans.verkuil
On Fri, Aug 03, 2018 at 03:43:15PM +0300, Sakari Ailus wrote:
> Hi Philippe,
>
> On Wed, Aug 01, 2018 at 11:20:57PM +0200, Philippe De Muyter wrote:
> > When v4l2_i2c_subdev_init is called, dev_name(&client->dev) has already
> > been set. Use it to generate subdev's name instead of recreating it
> > with "%d-%04x". This improves the similarity in subdev's name creation
> > between v4l2_i2c_subdev_init and v4l2_spi_subdev_init.
> >
> > Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> > ---
> > drivers/media/v4l2-core/v4l2-common.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 5471c6d..b062111 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> > v4l2_set_subdevdata(sd, client);
> > i2c_set_clientdata(client, sd);
> > /* initialize name */
> > - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> > - client->dev.driver->name, i2c_adapter_id(client->adapter),
> > - client->addr);
> > + snprintf(sd->name, sizeof(sd->name), "%s %s",
> > + client->dev.driver->name, dev_name(&client->dev));
> > }
> > EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
> >
>
> I like the patch in principle. But what's the effect of this on the actual
> sub-device (and entity) names? Looking at i2c_dev_set_name(), this will be
> different. We can't change the existing entity naming in drivers, this will
> break applications that expect them to be named in a certain way.
Does your comment also prevent patch 1/2
"media: v4l2-common: v4l2_spi_subdev_init : generate unique name"
to be accepted ?
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name
2018-08-01 21:20 [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name Philippe De Muyter
2018-08-01 21:20 ` [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation Philippe De Muyter
@ 2018-09-05 7:53 ` Hans Verkuil
1 sibling, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2018-09-05 7:53 UTC (permalink / raw)
To: Philippe De Muyter, linux-media, hans.verkuil
On 08/01/18 23:20, Philippe De Muyter wrote:
> While v4l2_i2c_subdev_init does give a unique name to the subdev, matching
> the one appearing in dmesg for messages generated by dev_info and friends
> (e.g. imx185 30-0010), v4l2_spi_subdev_init does a poor job, copying only
> the driver name, but not the dev_name(), yielding e.g. "imx185", but
> missing the "spi1.1" part, and not generating a unique name.
>
> Fix that.
>
> Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Regards,
Hans
> ---
> drivers/media/v4l2-core/v4l2-common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index b518b92..5471c6d 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -255,7 +255,9 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
> v4l2_set_subdevdata(sd, spi);
> spi_set_drvdata(spi, sd);
> /* initialize name */
> - strlcpy(sd->name, spi->dev.driver->name, sizeof(sd->name));
> + snprintf(sd->name, sizeof(sd->name), "%s %s",
> + spi->dev.driver->name, dev_name(&spi->dev));
> +
> }
> EXPORT_SYMBOL_GPL(v4l2_spi_subdev_init);
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-09-05 12:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-01 21:20 [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name Philippe De Muyter
2018-08-01 21:20 ` [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation Philippe De Muyter
2018-08-03 12:43 ` Sakari Ailus
2018-08-03 13:46 ` Philippe De Muyter
2018-08-03 14:11 ` Sakari Ailus
2018-08-08 8:43 ` Philippe De Muyter
2018-09-05 7:53 ` [PATCH 1/2] media: v4l2-common: v4l2_spi_subdev_init : generate unique name Hans Verkuil
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).