Re: [RESEND PATCH v3 24/32] media: pxa_camera: Register V4L2 device early

From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org,
	Philipp Zabel <p.zabel@pengutronix.de>,
	hverkuil@xs4all.nl, Francesco Dolcini <francesco@dolcini.it>,
	aishwarya.kothari@toradex.com, Robert Foss <rfoss@kernel.org>,
	Todor Tomov <todor.too@gmail.com>,
	Hyun Kwon <hyun.kwon@xilinx.com>,
	bingbu.cao@intel.com, niklas.soderlund@ragnatech.se,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Benjamin Mugnier <benjamin.mugnier@foss.st.com>,
	Sylvain Petinot <sylvain.petinot@foss.st.com>,
	Eugen Hristev <eugen.hristev@collabora.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Maxime Ripard <mripard@kernel.org>,
	Rui Miguel Silva <rmfrfs@gmail.com>,
	Fabio Estevam <festevam@gmail.com>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Dafna Hirschfeld <dafna@fastmail.com>,
	Hugues Fruchet <hugues.fruchet@foss.st.com>,
	Yong Deng <yong.deng@magewell.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	"Lad, Prabhakar" <prabhakar.csengg@gmail.com>,
	Benoit Parrot <bparrot@ti.com>,
	Steve Longerbeam <slongerbeam@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Sowjanya Komatineni <skomatineni@nvidia.com>,
	Marco Felsch <m.felsch@pengutronix.de>
Subject: Re: [RESEND PATCH v3 24/32] media: pxa_camera: Register V4L2 device early
Date: Tue, 13 Jun 2023 15:08:08 +0000	[thread overview]
Message-ID: <ZIiGWE5EowXaRuWW@kekkonen.localdomain> (raw)
In-Reply-To: <20230530045608.GP21633@pendragon.ideasonboard.com>

Hi Laurent,

On Tue, May 30, 2023 at 07:56:08AM +0300, Laurent Pinchart wrote:
> On Tue, May 30, 2023 at 07:54:46AM +0300, Laurent Pinchart wrote:
> > Hi Sakari,
> > 
> > Thank you for the patch.
> > 
> > On Thu, May 25, 2023 at 12:16:07PM +0300, Sakari Ailus wrote:
> > > Register V4L2 device before initialising the notifier. This way the device
> > > is available to the notifier from the beginning which makes it possible to
> > > use it for debug prints.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >  drivers/media/platform/intel/pxa_camera.c | 19 ++++++++++---------
> > >  1 file changed, 10 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/intel/pxa_camera.c b/drivers/media/platform/intel/pxa_camera.c
> > > index dad5e8d97683e..5df93fd4ff04b 100644
> > > --- a/drivers/media/platform/intel/pxa_camera.c
> > > +++ b/drivers/media/platform/intel/pxa_camera.c
> > > @@ -2307,6 +2307,10 @@ static int pxa_camera_probe(struct platform_device *pdev)
> > >  		return err;
> > >  	}
> > >  
> > > +	err = v4l2_device_register(&pdev->dev, &pcdev->v4l2_dev);
> > > +	if (err)
> > > +		return err;
> > > +
> > >  	v4l2_async_nf_init(&pcdev->notifier);
> > >  	pcdev->res = res;
> > >  	pcdev->pdata = pdev->dev.platform_data;
> > > @@ -2324,10 +2328,10 @@ static int pxa_camera_probe(struct platform_device *pdev)
> > >  	} else if (pdev->dev.of_node) {
> > >  		err = pxa_camera_pdata_from_dt(&pdev->dev, pcdev);
> > >  	} else {
> > > -		return -ENODEV;
> > > +		err = -ENODEV;
> > >  	}
> > >  	if (err < 0)
> > > -		return err;
> > > +		goto exit_v4l2_device_unregister;
> > >  
> > >  	if (!(pcdev->platform_flags & (PXA_CAMERA_DATAWIDTH_8 |
> > >  			PXA_CAMERA_DATAWIDTH_9 | PXA_CAMERA_DATAWIDTH_10))) {
> > > @@ -2393,22 +2397,17 @@ static int pxa_camera_probe(struct platform_device *pdev)
> > >  	pxa_camera_activate(pcdev);
> > >  
> > >  	platform_set_drvdata(pdev, pcdev);
> > > -	err = v4l2_device_register(&pdev->dev, &pcdev->v4l2_dev);
> > > -	if (err)
> > > -		goto exit_deactivate;
> > >  
> > >  	err = pxa_camera_init_videobuf2(pcdev);
> > >  	if (err)
> > > -		goto exit_v4l2_device_unregister;
> > > +		goto exit_deactivate;
> > >  
> > >  	pcdev->notifier.ops = &pxa_camera_sensor_ops;
> > >  	err = v4l2_async_nf_register(&pcdev->v4l2_dev, &pcdev->notifier);
> > 
> > The v4l2_device isn't made available to the notifier before this call,
> > so why is it necessary to register it earlier ?
> 
> Ah, it's because of patch 31/32. Please record this in the commit
> message.

It's already in the commit message, as you requested in an earlier review.

-- 
Sakari Ailus