[PATCH 0/2] Don't set bus info for test drivers

[PATCH 0/2] Don't set bus info for test drivers

[Sakari Ailus]

Hi folks,

As it's no longer needed to set the bus_info field for querycap or MC
device in drivers, drop this from vimc and vivid drivers.

Sakari Ailus (2):
  media: vivid: Don't explicitly set bus_info
  media: vimc: Don't explicitly set bus_info

 drivers/media/test-drivers/vimc/vimc-capture.c | 2 --
 drivers/media/test-drivers/vimc/vimc-core.c    | 2 --
 drivers/media/test-drivers/vivid/vivid-core.c  | 4 ----
 3 files changed, 8 deletions(-)

-- 
2.39.2

[PATCH 1/2] media: vivid: Don't explicitly set bus_info

[Sakari Ailus]

The bus_info field in both struct media_device and struct v4l2_capability
is assigned by the framework for platform devices. Do not set it in the
driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/test-drivers/vivid/vivid-core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index 353f035fcd19..fdd070876a6d 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -239,8 +239,6 @@ static int vidioc_querycap(struct file *file, void  *priv,
 
 	strscpy(cap->driver, "vivid", sizeof(cap->driver));
 	strscpy(cap->card, "vivid", sizeof(cap->card));
-	snprintf(cap->bus_info, sizeof(cap->bus_info),
-		 "platform:%s-%03d", VIVID_MODULE_NAME, dev->inst);
 
 	cap->capabilities = dev->vid_cap_caps | dev->vid_out_caps |
 		dev->vbi_cap_caps | dev->vbi_out_caps |
@@ -1762,8 +1760,6 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 
 	/* Initialize media device */
 	strscpy(dev->mdev.model, VIVID_MODULE_NAME, sizeof(dev->mdev.model));
-	snprintf(dev->mdev.bus_info, sizeof(dev->mdev.bus_info),
-		 "platform:%s-%03d", VIVID_MODULE_NAME, inst);
 	dev->mdev.dev = &pdev->dev;
 	media_device_init(&dev->mdev);
 	dev->mdev.ops = &vivid_media_ops;
-- 
2.39.2

[PATCH 2/2] media: vimc: Don't explicitly set bus_info

[Sakari Ailus]

The platform device name is already set by V4L2 and MC frameworks for
media device bus_info and V4L2 querycap bus_info fields. Don't do it in
the driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/test-drivers/vimc/vimc-capture.c | 2 --
 drivers/media/test-drivers/vimc/vimc-core.c    | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/media/test-drivers/vimc/vimc-capture.c b/drivers/media/test-drivers/vimc/vimc-capture.c
index aa944270e716..1e7cc7c980f9 100644
--- a/drivers/media/test-drivers/vimc/vimc-capture.c
+++ b/drivers/media/test-drivers/vimc/vimc-capture.c
@@ -57,8 +57,6 @@ static int vimc_capture_querycap(struct file *file, void *priv,
 {
 	strscpy(cap->driver, VIMC_PDEV_NAME, sizeof(cap->driver));
 	strscpy(cap->card, KBUILD_MODNAME, sizeof(cap->card));
-	snprintf(cap->bus_info, sizeof(cap->bus_info),
-		 "platform:%s", VIMC_PDEV_NAME);
 
 	return 0;
 }
diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
index af127476e920..2a924abd0ebc 100644
--- a/drivers/media/test-drivers/vimc/vimc-core.c
+++ b/drivers/media/test-drivers/vimc/vimc-core.c
@@ -366,8 +366,6 @@ static int vimc_probe(struct platform_device *pdev)
 	/* Initialize media device */
 	strscpy(vimc->mdev.model, VIMC_MDEV_MODEL_NAME,
 		sizeof(vimc->mdev.model));
-	snprintf(vimc->mdev.bus_info, sizeof(vimc->mdev.bus_info),
-		 "platform:%s", VIMC_PDEV_NAME);
 	vimc->mdev.dev = &pdev->dev;
 	media_device_init(&vimc->mdev);
 
-- 
2.39.2

Re: [PATCH 1/2] media: vivid: Don't explicitly set bus_info

[Hans Verkuil]

On 29/11/2023 15:15, Sakari Ailus wrote:
> The bus_info field in both struct media_device and struct v4l2_capability
> is assigned by the framework for platform devices. Do not set it in the
> driver.

That doesn't work for vivid. The problem is that if you create multiple instances
(e.g. modprobe vivid n_devs=2), then each instance will report "platform:vivid.0"
as bus info, but they must give different bus_info names since that has to be unique.

It should be fine for vimc, though, since that's just a single instance.

So vivid is an exception to the rule. Rather than removing these snprintf lines,
adding a comment might be better.

For the record: I tried exactly the same thing myself some time ago, and then
discovered that that didn't fly.

Regards,

	Hans

> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/test-drivers/vivid/vivid-core.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
> index 353f035fcd19..fdd070876a6d 100644
> --- a/drivers/media/test-drivers/vivid/vivid-core.c
> +++ b/drivers/media/test-drivers/vivid/vivid-core.c
> @@ -239,8 +239,6 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>  	strscpy(cap->driver, "vivid", sizeof(cap->driver));
>  	strscpy(cap->card, "vivid", sizeof(cap->card));
> -	snprintf(cap->bus_info, sizeof(cap->bus_info),
> -		 "platform:%s-%03d", VIVID_MODULE_NAME, dev->inst);
>  
>  	cap->capabilities = dev->vid_cap_caps | dev->vid_out_caps |
>  		dev->vbi_cap_caps | dev->vbi_out_caps |
> @@ -1762,8 +1760,6 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
>  
>  	/* Initialize media device */
>  	strscpy(dev->mdev.model, VIVID_MODULE_NAME, sizeof(dev->mdev.model));
> -	snprintf(dev->mdev.bus_info, sizeof(dev->mdev.bus_info),
> -		 "platform:%s-%03d", VIVID_MODULE_NAME, inst);
>  	dev->mdev.dev = &pdev->dev;
>  	media_device_init(&dev->mdev);
>  	dev->mdev.ops = &vivid_media_ops;

Re: [PATCH 1/2] media: vivid: Don't explicitly set bus_info

[Sakari Ailus]

Hi Hans,

Thanks for the review.

On Wed, Nov 29, 2023 at 03:26:00PM +0100, Hans Verkuil wrote:
> On 29/11/2023 15:15, Sakari Ailus wrote:
> > The bus_info field in both struct media_device and struct v4l2_capability
> > is assigned by the framework for platform devices. Do not set it in the
> > driver.
> 
> That doesn't work for vivid. The problem is that if you create multiple instances
> (e.g. modprobe vivid n_devs=2), then each instance will report "platform:vivid.0"
> as bus info, but they must give different bus_info names since that has to be unique.
> 
> It should be fine for vimc, though, since that's just a single instance.
> 
> So vivid is an exception to the rule. Rather than removing these snprintf lines,
> adding a comment might be better.

Sounds reasonable. I'll do this in v2.

> 
> For the record: I tried exactly the same thing myself some time ago, and then
> discovered that that didn't fly.

-- 
Sakari Ailus

Re: [PATCH 2/2] media: vimc: Don't explicitly set bus_info

[Shuah Khan]

On 11/29/23 07:15, Sakari Ailus wrote:
> The platform device name is already set by V4L2 and MC frameworks for
> media device bus_info and V4L2 querycap bus_info fields. Don't do it in
> the driver.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>   drivers/media/test-drivers/vimc/vimc-capture.c | 2 --
>   drivers/media/test-drivers/vimc/vimc-core.c    | 2 --
>   2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/media/test-drivers/vimc/vimc-capture.c b/drivers/media/test-drivers/vimc/vimc-capture.c
> index aa944270e716..1e7cc7c980f9 100644
> --- a/drivers/media/test-drivers/vimc/vimc-capture.c
> +++ b/drivers/media/test-drivers/vimc/vimc-capture.c
> @@ -57,8 +57,6 @@ static int vimc_capture_querycap(struct file *file, void *priv,
>   {
>   	strscpy(cap->driver, VIMC_PDEV_NAME, sizeof(cap->driver));
>   	strscpy(cap->card, KBUILD_MODNAME, sizeof(cap->card));
> -	snprintf(cap->bus_info, sizeof(cap->bus_info),
> -		 "platform:%s", VIMC_PDEV_NAME);
>   
>   	return 0;
>   }
> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
> index af127476e920..2a924abd0ebc 100644
> --- a/drivers/media/test-drivers/vimc/vimc-core.c
> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
> @@ -366,8 +366,6 @@ static int vimc_probe(struct platform_device *pdev)
>   	/* Initialize media device */
>   	strscpy(vimc->mdev.model, VIMC_MDEV_MODEL_NAME,
>   		sizeof(vimc->mdev.model));
> -	snprintf(vimc->mdev.bus_info, sizeof(vimc->mdev.bus_info),
> -		 "platform:%s", VIMC_PDEV_NAME);
>   	vimc->mdev.dev = &pdev->dev;
>   	media_device_init(&vimc->mdev);
>   

Sorry this seems to have been lost in inbox.

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah