diff mbox series

[11/18] media: marvell: cafe: Register V4L2 device earlier

Message ID 20230330115853.1628216-12-sakari.ailus@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Separate links and async sub-devices | expand

Commit Message

Sakari Ailus March 30, 2023, 11:58 a.m. UTC 
Register V4L2 device before the async notifier so the struct device will
be available for the notifier.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/marvell/cafe-driver.c | 12 ++++++++++--
 drivers/media/platform/marvell/mcam-core.c   |  6 ------
 2 files changed, 10 insertions(+), 8 deletions(-)

Comments

Laurent Pinchart April 25, 2023, 12:27 a.m. UTC | #1 
Hi Sakari,

Thank you for the patch.

On Thu, Mar 30, 2023 at 02:58:46PM +0300, Sakari Ailus wrote:
> Register V4L2 device before the async notifier so the struct device will
> be available for the notifier.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/platform/marvell/cafe-driver.c | 12 ++++++++++--
>  drivers/media/platform/marvell/mcam-core.c   |  6 ------
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/marvell/cafe-driver.c b/drivers/media/platform/marvell/cafe-driver.c
> index dd1bba70bd79..4d8843623255 100644
> --- a/drivers/media/platform/marvell/cafe-driver.c
> +++ b/drivers/media/platform/marvell/cafe-driver.c
> @@ -536,6 +536,11 @@ static int cafe_pci_probe(struct pci_dev *pdev,
>  	if (ret)
>  		goto out_pdown;
>  
> +	ret = v4l2_device_register(mcam->dev, &mcam->v4l2_dev);
> +	if (ret)
> +		goto out_smbus_shutdown;
> +
> +
>  	v4l2_async_nf_init(&mcam->notifier);
>  
>  	asd = v4l2_async_nf_add_i2c(&mcam->notifier,
> @@ -544,12 +549,12 @@ static int cafe_pci_probe(struct pci_dev *pdev,
>  				    struct v4l2_async_connection);
>  	if (IS_ERR(asd)) {
>  		ret = PTR_ERR(asd);
> -		goto out_smbus_shutdown;
> +		goto out_v4l2_device_unregister;
>  	}
>  
>  	ret = mccic_register(mcam);
>  	if (ret)
> -		goto out_smbus_shutdown;
> +		goto out_v4l2_device_unregister;
>  
>  	clkdev_create(mcam->mclk, "xclk", "%d-%04x",
>  		i2c_adapter_id(cam->i2c_adapter), ov7670_info.addr);
> @@ -565,6 +570,8 @@ static int cafe_pci_probe(struct pci_dev *pdev,
>  
>  out_mccic_shutdown:
>  	mccic_shutdown(mcam);
> +out_v4l2_device_unregister:
> +	v4l2_device_unregister(&mcam->v4l2_dev);
>  out_smbus_shutdown:
>  	cafe_smbus_shutdown(cam);
>  out_pdown:
> @@ -587,6 +594,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,
>  static void cafe_shutdown(struct cafe_camera *cam)
>  {
>  	mccic_shutdown(&cam->mcam);
> +	v4l2_device_unregister(&cam->mcam.v4l2_dev);
>  	cafe_smbus_shutdown(cam);
>  	free_irq(cam->pdev->irq, cam);
>  	pci_iounmap(cam->pdev, cam->mcam.regs);
> diff --git a/drivers/media/platform/marvell/mcam-core.c b/drivers/media/platform/marvell/mcam-core.c
> index b74a362ec075..2ecdcbcb37fd 100644
> --- a/drivers/media/platform/marvell/mcam-core.c
> +++ b/drivers/media/platform/marvell/mcam-core.c
> @@ -1866,10 +1866,6 @@ int mccic_register(struct mcam_camera *cam)
>  	/*
>  	 * Register with V4L
>  	 */
> -	ret = v4l2_device_register(cam->dev, &cam->v4l2_dev);
> -	if (ret)
> -		goto out;
> -
>  	mutex_init(&cam->s_mutex);
>  	cam->state = S_NOTREADY;
>  	mcam_set_config_needed(cam, 1);
> @@ -1915,7 +1911,6 @@ int mccic_register(struct mcam_camera *cam)
>  
>  out:
>  	v4l2_async_nf_unregister(&cam->notifier);
> -	v4l2_device_unregister(&cam->v4l2_dev);
>  	v4l2_async_nf_cleanup(&cam->notifier);

Wouldn't the v4l2_async_nf_* calls be better placed in cafe-driver.c,
given that v4l2_async_nf_init() is called there too ? Same below.

>  	return ret;
>  }
> @@ -1937,7 +1932,6 @@ void mccic_shutdown(struct mcam_camera *cam)
>  		mcam_free_dma_bufs(cam);
>  	v4l2_ctrl_handler_free(&cam->ctrl_handler);
>  	v4l2_async_nf_unregister(&cam->notifier);
> -	v4l2_device_unregister(&cam->v4l2_dev);
>  	v4l2_async_nf_cleanup(&cam->notifier);
>  }
>  EXPORT_SYMBOL_GPL(mccic_shutdown);
Sakari Ailus April 28, 2023, 11:22 a.m. UTC | #2 
Hi Laurent,

On Tue, Apr 25, 2023 at 03:27:11AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thu, Mar 30, 2023 at 02:58:46PM +0300, Sakari Ailus wrote:
> > Register V4L2 device before the async notifier so the struct device will
> > be available for the notifier.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/platform/marvell/cafe-driver.c | 12 ++++++++++--
> >  drivers/media/platform/marvell/mcam-core.c   |  6 ------
> >  2 files changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/media/platform/marvell/cafe-driver.c b/drivers/media/platform/marvell/cafe-driver.c
> > index dd1bba70bd79..4d8843623255 100644
> > --- a/drivers/media/platform/marvell/cafe-driver.c
> > +++ b/drivers/media/platform/marvell/cafe-driver.c
> > @@ -536,6 +536,11 @@ static int cafe_pci_probe(struct pci_dev *pdev,
> >  	if (ret)
> >  		goto out_pdown;
> >  
> > +	ret = v4l2_device_register(mcam->dev, &mcam->v4l2_dev);
> > +	if (ret)
> > +		goto out_smbus_shutdown;
> > +
> > +
> >  	v4l2_async_nf_init(&mcam->notifier);
> >  
> >  	asd = v4l2_async_nf_add_i2c(&mcam->notifier,
> > @@ -544,12 +549,12 @@ static int cafe_pci_probe(struct pci_dev *pdev,
> >  				    struct v4l2_async_connection);
> >  	if (IS_ERR(asd)) {
> >  		ret = PTR_ERR(asd);
> > -		goto out_smbus_shutdown;
> > +		goto out_v4l2_device_unregister;
> >  	}
> >  
> >  	ret = mccic_register(mcam);
> >  	if (ret)
> > -		goto out_smbus_shutdown;
> > +		goto out_v4l2_device_unregister;
> >  
> >  	clkdev_create(mcam->mclk, "xclk", "%d-%04x",
> >  		i2c_adapter_id(cam->i2c_adapter), ov7670_info.addr);
> > @@ -565,6 +570,8 @@ static int cafe_pci_probe(struct pci_dev *pdev,
> >  
> >  out_mccic_shutdown:
> >  	mccic_shutdown(mcam);
> > +out_v4l2_device_unregister:
> > +	v4l2_device_unregister(&mcam->v4l2_dev);
> >  out_smbus_shutdown:
> >  	cafe_smbus_shutdown(cam);
> >  out_pdown:
> > @@ -587,6 +594,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,
> >  static void cafe_shutdown(struct cafe_camera *cam)
> >  {
> >  	mccic_shutdown(&cam->mcam);
> > +	v4l2_device_unregister(&cam->mcam.v4l2_dev);
> >  	cafe_smbus_shutdown(cam);
> >  	free_irq(cam->pdev->irq, cam);
> >  	pci_iounmap(cam->pdev, cam->mcam.regs);
> > diff --git a/drivers/media/platform/marvell/mcam-core.c b/drivers/media/platform/marvell/mcam-core.c
> > index b74a362ec075..2ecdcbcb37fd 100644
> > --- a/drivers/media/platform/marvell/mcam-core.c
> > +++ b/drivers/media/platform/marvell/mcam-core.c
> > @@ -1866,10 +1866,6 @@ int mccic_register(struct mcam_camera *cam)
> >  	/*
> >  	 * Register with V4L
> >  	 */
> > -	ret = v4l2_device_register(cam->dev, &cam->v4l2_dev);
> > -	if (ret)
> > -		goto out;
> > -
> >  	mutex_init(&cam->s_mutex);
> >  	cam->state = S_NOTREADY;
> >  	mcam_set_config_needed(cam, 1);
> > @@ -1915,7 +1911,6 @@ int mccic_register(struct mcam_camera *cam)
> >  
> >  out:
> >  	v4l2_async_nf_unregister(&cam->notifier);
> > -	v4l2_device_unregister(&cam->v4l2_dev);
> >  	v4l2_async_nf_cleanup(&cam->notifier);
> 
> Wouldn't the v4l2_async_nf_* calls be better placed in cafe-driver.c,
> given that v4l2_async_nf_init() is called there too ? Same below.

Probably yes, but I'd like to avoid making unnecessary changes to drivers I
can't test.

> 
> >  	return ret;
> >  }
> > @@ -1937,7 +1932,6 @@ void mccic_shutdown(struct mcam_camera *cam)
> >  		mcam_free_dma_bufs(cam);
> >  	v4l2_ctrl_handler_free(&cam->ctrl_handler);
> >  	v4l2_async_nf_unregister(&cam->notifier);
> > -	v4l2_device_unregister(&cam->v4l2_dev);
> >  	v4l2_async_nf_cleanup(&cam->notifier);
> >  }
> >  EXPORT_SYMBOL_GPL(mccic_shutdown);
>
diff mbox series

Patch

diff --git a/drivers/media/platform/marvell/cafe-driver.c b/drivers/media/platform/marvell/cafe-driver.c
index dd1bba70bd79..4d8843623255 100644
--- a/drivers/media/platform/marvell/cafe-driver.c
+++ b/drivers/media/platform/marvell/cafe-driver.c
@@ -536,6 +536,11 @@  static int cafe_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto out_pdown;
 
+	ret = v4l2_device_register(mcam->dev, &mcam->v4l2_dev);
+	if (ret)
+		goto out_smbus_shutdown;
+
+
 	v4l2_async_nf_init(&mcam->notifier);
 
 	asd = v4l2_async_nf_add_i2c(&mcam->notifier,
@@ -544,12 +549,12 @@  static int cafe_pci_probe(struct pci_dev *pdev,
 				    struct v4l2_async_connection);
 	if (IS_ERR(asd)) {
 		ret = PTR_ERR(asd);
-		goto out_smbus_shutdown;
+		goto out_v4l2_device_unregister;
 	}
 
 	ret = mccic_register(mcam);
 	if (ret)
-		goto out_smbus_shutdown;
+		goto out_v4l2_device_unregister;
 
 	clkdev_create(mcam->mclk, "xclk", "%d-%04x",
 		i2c_adapter_id(cam->i2c_adapter), ov7670_info.addr);
@@ -565,6 +570,8 @@  static int cafe_pci_probe(struct pci_dev *pdev,
 
 out_mccic_shutdown:
 	mccic_shutdown(mcam);
+out_v4l2_device_unregister:
+	v4l2_device_unregister(&mcam->v4l2_dev);
 out_smbus_shutdown:
 	cafe_smbus_shutdown(cam);
 out_pdown:
@@ -587,6 +594,7 @@  static int cafe_pci_probe(struct pci_dev *pdev,
 static void cafe_shutdown(struct cafe_camera *cam)
 {
 	mccic_shutdown(&cam->mcam);
+	v4l2_device_unregister(&cam->mcam.v4l2_dev);
 	cafe_smbus_shutdown(cam);
 	free_irq(cam->pdev->irq, cam);
 	pci_iounmap(cam->pdev, cam->mcam.regs);
diff --git a/drivers/media/platform/marvell/mcam-core.c b/drivers/media/platform/marvell/mcam-core.c
index b74a362ec075..2ecdcbcb37fd 100644
--- a/drivers/media/platform/marvell/mcam-core.c
+++ b/drivers/media/platform/marvell/mcam-core.c
@@ -1866,10 +1866,6 @@  int mccic_register(struct mcam_camera *cam)
 	/*
 	 * Register with V4L
 	 */
-	ret = v4l2_device_register(cam->dev, &cam->v4l2_dev);
-	if (ret)
-		goto out;
-
 	mutex_init(&cam->s_mutex);
 	cam->state = S_NOTREADY;
 	mcam_set_config_needed(cam, 1);
@@ -1915,7 +1911,6 @@  int mccic_register(struct mcam_camera *cam)
 
 out:
 	v4l2_async_nf_unregister(&cam->notifier);
-	v4l2_device_unregister(&cam->v4l2_dev);
 	v4l2_async_nf_cleanup(&cam->notifier);
 	return ret;
 }
@@ -1937,7 +1932,6 @@  void mccic_shutdown(struct mcam_camera *cam)
 		mcam_free_dma_bufs(cam);
 	v4l2_ctrl_handler_free(&cam->ctrl_handler);
 	v4l2_async_nf_unregister(&cam->notifier);
-	v4l2_device_unregister(&cam->v4l2_dev);
 	v4l2_async_nf_cleanup(&cam->notifier);
 }
 EXPORT_SYMBOL_GPL(mccic_shutdown);