[PATCH 0/3] Uniformly assign sub-device names

[PATCH 0/3] Uniformly assign sub-device names

[Sakari Ailus]

Hi all,

Recently we noticed the sub-device drivers used a few different naming
schemes, as well as a few more different implementations of that naming.
This set adds a framework function to call to name sub-devices for cases
where a sub-device driver exposes more than one sub-device.

This set brings no functional change.

Sakari Ailus (3):
  v4l: subdev: Add a function to set an I²C sub-device's name
  smiapp: Use v4l2_i2c_subdev_set_name
  v4l: sr030pc30: Remove redundant setting of sub-device name

 drivers/media/i2c/smiapp/smiapp-core.c | 10 ++++------
 drivers/media/i2c/sr030pc30.c          |  1 -
 drivers/media/v4l2-core/v4l2-common.c  | 18 ++++++++++++++----
 include/media/v4l2-common.h            | 12 ++++++++++++
 4 files changed, 30 insertions(+), 11 deletions(-)

-- 
2.11.0

[PATCH 1/3] v4l: subdev: Add a function to set an I²C sub-device's name

[Sakari Ailus]

v4l2_i2c_subdev_set_name() can be used to assign a name to a sub-device.
This way uniform names can be formed easily without having to resort to
things such as snprintf in drivers.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 18 ++++++++++++++----
 include/media/v4l2-common.h           | 12 ++++++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index b518b92d6d96..9c48b90b4ae8 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -109,6 +109,19 @@ EXPORT_SYMBOL(v4l2_ctrl_query_fill);
 
 #if IS_ENABLED(CONFIG_I2C)
 
+void v4l2_i2c_subdev_set_name(struct v4l2_subdev *sd, struct i2c_client *client,
+			      const char *devname, const char *postfix)
+{
+	if (!devname)
+		devname = client->dev.driver->name;
+	if (!postfix)
+		postfix = "";
+
+	snprintf(sd->name, sizeof(sd->name), "%s%s %d-%04x", devname, postfix,
+		 i2c_adapter_id(client->adapter), client->addr);
+}
+EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_set_name);
+
 void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
 		const struct v4l2_subdev_ops *ops)
 {
@@ -120,10 +133,7 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
 	/* i2c_client and v4l2_subdev point to one another */
 	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);
+	v4l2_i2c_subdev_set_name(sd, client, NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
 
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index cdc87ec61e54..bd880a909ecf 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -155,6 +155,18 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
 		const unsigned short *probe_addrs);
 
 /**
+ * v4l2_i2c_subdev_set_name - Set name for an I²C sub-device
+ *
+ * @sd: pointer to &struct v4l2_subdev
+ * @client: pointer to struct i2c_client
+ * @devname: the name of the device; if NULL, the I²C device's name will be used
+ * @postfix: sub-device specific string to put right after the I²C device name;
+ *	     may be NULL
+ */
+void v4l2_i2c_subdev_set_name(struct v4l2_subdev *sd, struct i2c_client *client,
+			      const char *devname, const char *postfix);
+
+/**
  * v4l2_i2c_subdev_init - Initializes a &struct v4l2_subdev with data from
  *	an i2c_client struct.
  *
-- 
2.11.0

[PATCH 2/3] smiapp: Use v4l2_i2c_subdev_set_name

[Sakari Ailus]

Use v4l2_i2c_subdev_set_name() to set the name of the smiapp driver's
sub-devices. There is no functional change.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 1236683da8f7..99f3b295ae3c 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2617,9 +2617,7 @@ static void smiapp_create_subdev(struct smiapp_sensor *sensor,
 	ssd->npads = num_pads;
 	ssd->source_pad = num_pads - 1;
 
-	snprintf(ssd->sd.name,
-		 sizeof(ssd->sd.name), "%s %s %d-%4.4x", sensor->minfo.name,
-		 name, i2c_adapter_id(client->adapter), client->addr);
+	v4l2_i2c_subdev_set_name(&ssd->sd, client, sensor->minfo.name, name);
 
 	smiapp_get_native_size(ssd, &ssd->sink_fmt);
 
@@ -3064,9 +3062,9 @@ static int smiapp_probe(struct i2c_client *client,
 	if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
 		sensor->pll.flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS;
 
-	smiapp_create_subdev(sensor, sensor->scaler, "scaler", 2);
-	smiapp_create_subdev(sensor, sensor->binner, "binner", 2);
-	smiapp_create_subdev(sensor, sensor->pixel_array, "pixel_array", 1);
+	smiapp_create_subdev(sensor, sensor->scaler, " scaler", 2);
+	smiapp_create_subdev(sensor, sensor->binner, " binner", 2);
+	smiapp_create_subdev(sensor, sensor->pixel_array, " pixel_array", 1);
 
 	dev_dbg(&client->dev, "profile %d\n", sensor->minfo.smiapp_profile);
 
-- 
2.11.0

[PATCH 3/3] sr030pc30: Remove redundant setting of sub-device name

[Sakari Ailus]

The sub-device name is set right after in v4l2_i2c_subdev_init(). Remove
the redundant strcpy() call.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/sr030pc30.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
index 2a4882cddc51..3d3fb1cda28c 100644
--- a/drivers/media/i2c/sr030pc30.c
+++ b/drivers/media/i2c/sr030pc30.c
@@ -703,7 +703,6 @@ static int sr030pc30_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	sd = &info->sd;
-	strcpy(sd->name, MODULE_NAME);
 	info->pdata = client->dev.platform_data;
 
 	v4l2_i2c_subdev_init(sd, client, &sr030pc30_ops);
-- 
2.11.0

Re: [PATCH 1/3] v4l: subdev: Add a function to set an I²C sub-device's name

[Hans Verkuil]

On 08/29/2018 12:52 PM, Sakari Ailus wrote:
> v4l2_i2c_subdev_set_name() can be used to assign a name to a sub-device.
> This way uniform names can be formed easily without having to resort to
> things such as snprintf in drivers.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans

> ---
>  drivers/media/v4l2-core/v4l2-common.c | 18 ++++++++++++++----
>  include/media/v4l2-common.h           | 12 ++++++++++++
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index b518b92d6d96..9c48b90b4ae8 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -109,6 +109,19 @@ EXPORT_SYMBOL(v4l2_ctrl_query_fill);
>  
>  #if IS_ENABLED(CONFIG_I2C)
>  
> +void v4l2_i2c_subdev_set_name(struct v4l2_subdev *sd, struct i2c_client *client,
> +			      const char *devname, const char *postfix)
> +{
> +	if (!devname)
> +		devname = client->dev.driver->name;
> +	if (!postfix)
> +		postfix = "";
> +
> +	snprintf(sd->name, sizeof(sd->name), "%s%s %d-%04x", devname, postfix,
> +		 i2c_adapter_id(client->adapter), client->addr);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_set_name);
> +
>  void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
>  		const struct v4l2_subdev_ops *ops)
>  {
> @@ -120,10 +133,7 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
>  	/* i2c_client and v4l2_subdev point to one another */
>  	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);
> +	v4l2_i2c_subdev_set_name(sd, client, NULL, NULL);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
>  
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index cdc87ec61e54..bd880a909ecf 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -155,6 +155,18 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
>  		const unsigned short *probe_addrs);
>  
>  /**
> + * v4l2_i2c_subdev_set_name - Set name for an I²C sub-device
> + *
> + * @sd: pointer to &struct v4l2_subdev
> + * @client: pointer to struct i2c_client
> + * @devname: the name of the device; if NULL, the I²C device's name will be used
> + * @postfix: sub-device specific string to put right after the I²C device name;
> + *	     may be NULL
> + */
> +void v4l2_i2c_subdev_set_name(struct v4l2_subdev *sd, struct i2c_client *client,
> +			      const char *devname, const char *postfix);
> +
> +/**
>   * v4l2_i2c_subdev_init - Initializes a &struct v4l2_subdev with data from
>   *	an i2c_client struct.
>   *
> 

Re: [PATCH 2/3] smiapp: Use v4l2_i2c_subdev_set_name

[Hans Verkuil]

On 08/29/2018 12:52 PM, Sakari Ailus wrote:
> Use v4l2_i2c_subdev_set_name() to set the name of the smiapp driver's
> sub-devices. There is no functional change.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>


Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans

> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index 1236683da8f7..99f3b295ae3c 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2617,9 +2617,7 @@ static void smiapp_create_subdev(struct smiapp_sensor *sensor,
>  	ssd->npads = num_pads;
>  	ssd->source_pad = num_pads - 1;
>  
> -	snprintf(ssd->sd.name,
> -		 sizeof(ssd->sd.name), "%s %s %d-%4.4x", sensor->minfo.name,
> -		 name, i2c_adapter_id(client->adapter), client->addr);
> +	v4l2_i2c_subdev_set_name(&ssd->sd, client, sensor->minfo.name, name);
>  
>  	smiapp_get_native_size(ssd, &ssd->sink_fmt);
>  
> @@ -3064,9 +3062,9 @@ static int smiapp_probe(struct i2c_client *client,
>  	if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
>  		sensor->pll.flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS;
>  
> -	smiapp_create_subdev(sensor, sensor->scaler, "scaler", 2);
> -	smiapp_create_subdev(sensor, sensor->binner, "binner", 2);
> -	smiapp_create_subdev(sensor, sensor->pixel_array, "pixel_array", 1);
> +	smiapp_create_subdev(sensor, sensor->scaler, " scaler", 2);
> +	smiapp_create_subdev(sensor, sensor->binner, " binner", 2);
> +	smiapp_create_subdev(sensor, sensor->pixel_array, " pixel_array", 1);
>  
>  	dev_dbg(&client->dev, "profile %d\n", sensor->minfo.smiapp_profile);
>  
> 

Re: [PATCH 3/3] sr030pc30: Remove redundant setting of sub-device name

[Hans Verkuil]

On 08/29/2018 12:52 PM, Sakari Ailus wrote:
> The sub-device name is set right after in v4l2_i2c_subdev_init(). Remove
> the redundant strcpy() call.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>


Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans

> ---
>  drivers/media/i2c/sr030pc30.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
> index 2a4882cddc51..3d3fb1cda28c 100644
> --- a/drivers/media/i2c/sr030pc30.c
> +++ b/drivers/media/i2c/sr030pc30.c
> @@ -703,7 +703,6 @@ static int sr030pc30_probe(struct i2c_client *client,
>  		return -ENOMEM;
>  
>  	sd = &info->sd;
> -	strcpy(sd->name, MODULE_NAME);
>  	info->pdata = client->dev.platform_data;
>  
>  	v4l2_i2c_subdev_init(sd, client, &sr030pc30_ops);
>