| Message ID | 20230525091615.2324824-25-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Separate links and async sub-devices | expand |
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. > > if (err) > > - goto exit_v4l2_device_unregister; > > + goto exit_deactivate; > > > > return 0; > > -exit_v4l2_device_unregister: > > - v4l2_device_unregister(&pcdev->v4l2_dev); > > exit_deactivate: > > pxa_camera_deactivate(pcdev); > > tasklet_kill(&pcdev->task_eof); > > @@ -2420,6 +2419,8 @@ static int pxa_camera_probe(struct platform_device *pdev) > > dma_release_channel(pcdev->dma_chans[0]); > > exit_notifier_cleanup: > > v4l2_async_nf_cleanup(&pcdev->notifier); > > +exit_v4l2_device_unregister: > > + v4l2_device_unregister(&pcdev->v4l2_dev); > > return err; > > } > >
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.
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); if (err) - goto exit_v4l2_device_unregister; + goto exit_deactivate; return 0; -exit_v4l2_device_unregister: - v4l2_device_unregister(&pcdev->v4l2_dev); exit_deactivate: pxa_camera_deactivate(pcdev); tasklet_kill(&pcdev->task_eof); @@ -2420,6 +2419,8 @@ static int pxa_camera_probe(struct platform_device *pdev) dma_release_channel(pcdev->dma_chans[0]); exit_notifier_cleanup: v4l2_async_nf_cleanup(&pcdev->notifier); +exit_v4l2_device_unregister: + v4l2_device_unregister(&pcdev->v4l2_dev); return err; }
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(-)