[10/18] media: pxa_camera: Register V4L2 device early, fix probe error handling
Commit Message
Register V4L2 device before initialising the notifier. This way the device
is available to the notifier from the beginning.
Also fix error handling in probe.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/platform/intel/pxa_camera.c | 30 +++++++++++++----------
1 file changed, 17 insertions(+), 13 deletions(-)
Comments
Hi Sakari,
Thank you for the patch.
On Thu, Mar 30, 2023 at 02:58:45PM +0300, Sakari Ailus wrote:
> Register V4L2 device before initialising the notifier. This way the device
> is available to the notifier from the beginning.
Could you please explain in the commit message why this is needed ? Same
comment for subsequent patches in this series.
> Also fix error handling in probe.
Splitting this in two patches, with the fix first, would make it easier
to review.
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/platform/intel/pxa_camera.c | 30 +++++++++++++----------
> 1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/intel/pxa_camera.c b/drivers/media/platform/intel/pxa_camera.c
> index b848a2a9032f..31ae220ee4f3 100644
> --- a/drivers/media/platform/intel/pxa_camera.c
> +++ b/drivers/media/platform/intel/pxa_camera.c
> @@ -2289,6 +2289,10 @@ static int pxa_camera_probe(struct platform_device *pdev)
> if (IS_ERR(pcdev->clk))
> return PTR_ERR(pcdev->clk);
>
> + 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;
> @@ -2306,10 +2310,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_notifier_cleanup;
>
> if (!(pcdev->platform_flags & (PXA_CAMERA_DATAWIDTH_8 |
> PXA_CAMERA_DATAWIDTH_9 | PXA_CAMERA_DATAWIDTH_10))) {
> @@ -2342,8 +2346,10 @@ static int pxa_camera_probe(struct platform_device *pdev)
> * Request the regions.
> */
> base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> + if (IS_ERR(base)) {
> + err = PTR_ERR(base);
> + goto exit_notifier_cleanup;
> + }
>
> pcdev->irq = irq;
> pcdev->base = base;
> @@ -2352,7 +2358,8 @@ static int pxa_camera_probe(struct platform_device *pdev)
> pcdev->dma_chans[0] = dma_request_chan(&pdev->dev, "CI_Y");
> if (IS_ERR(pcdev->dma_chans[0])) {
> dev_err(&pdev->dev, "Can't request DMA for Y\n");
> - return PTR_ERR(pcdev->dma_chans[0]);
> + err = PTR_ERR(pcdev->dma_chans[0]);
> + goto exit_notifier_cleanup;
> }
>
> pcdev->dma_chans[1] = dma_request_chan(&pdev->dev, "CI_U");
> @@ -2392,23 +2399,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_notifier_cleanup;
> + goto exit_deactivate;
>
> pcdev->notifier.ops = &pxa_camera_sensor_ops;
> err = v4l2_async_nf_register(&pcdev->v4l2_dev, &pcdev->notifier);
> if (err)
> - goto exit_notifier_cleanup;
> + goto exit_deactivate;
>
> return 0;
> -exit_notifier_cleanup:
> - v4l2_async_nf_cleanup(&pcdev->notifier);
> - v4l2_device_unregister(&pcdev->v4l2_dev);
> exit_deactivate:
> pxa_camera_deactivate(pcdev);
> tasklet_kill(&pcdev->task_eof);
> @@ -2418,6 +2419,9 @@ static int pxa_camera_probe(struct platform_device *pdev)
> dma_release_channel(pcdev->dma_chans[1]);
> exit_free_dma_y:
> dma_release_channel(pcdev->dma_chans[0]);
> +exit_notifier_cleanup:
> + v4l2_async_nf_cleanup(&pcdev->notifier);
> + v4l2_device_unregister(&pcdev->v4l2_dev);
> return err;
> }
>
Hi Laurent,
On Tue, Apr 25, 2023 at 03:25:06AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Thu, Mar 30, 2023 at 02:58:45PM +0300, Sakari Ailus wrote:
> > Register V4L2 device before initialising the notifier. This way the device
> > is available to the notifier from the beginning.
>
> Could you please explain in the commit message why this is needed ? Same
> comment for subsequent patches in this series.
Yes.
>
> > Also fix error handling in probe.
>
> Splitting this in two patches, with the fix first, would make it easier
> to review.
I'll address these in v2.
@@ -2289,6 +2289,10 @@ static int pxa_camera_probe(struct platform_device *pdev)
if (IS_ERR(pcdev->clk))
return PTR_ERR(pcdev->clk);
+ 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;
@@ -2306,10 +2310,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_notifier_cleanup;
if (!(pcdev->platform_flags & (PXA_CAMERA_DATAWIDTH_8 |
PXA_CAMERA_DATAWIDTH_9 | PXA_CAMERA_DATAWIDTH_10))) {
@@ -2342,8 +2346,10 @@ static int pxa_camera_probe(struct platform_device *pdev)
* Request the regions.
*/
base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ if (IS_ERR(base)) {
+ err = PTR_ERR(base);
+ goto exit_notifier_cleanup;
+ }
pcdev->irq = irq;
pcdev->base = base;
@@ -2352,7 +2358,8 @@ static int pxa_camera_probe(struct platform_device *pdev)
pcdev->dma_chans[0] = dma_request_chan(&pdev->dev, "CI_Y");
if (IS_ERR(pcdev->dma_chans[0])) {
dev_err(&pdev->dev, "Can't request DMA for Y\n");
- return PTR_ERR(pcdev->dma_chans[0]);
+ err = PTR_ERR(pcdev->dma_chans[0]);
+ goto exit_notifier_cleanup;
}
pcdev->dma_chans[1] = dma_request_chan(&pdev->dev, "CI_U");
@@ -2392,23 +2399,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_notifier_cleanup;
+ goto exit_deactivate;
pcdev->notifier.ops = &pxa_camera_sensor_ops;
err = v4l2_async_nf_register(&pcdev->v4l2_dev, &pcdev->notifier);
if (err)
- goto exit_notifier_cleanup;
+ goto exit_deactivate;
return 0;
-exit_notifier_cleanup:
- v4l2_async_nf_cleanup(&pcdev->notifier);
- v4l2_device_unregister(&pcdev->v4l2_dev);
exit_deactivate:
pxa_camera_deactivate(pcdev);
tasklet_kill(&pcdev->task_eof);
@@ -2418,6 +2419,9 @@ static int pxa_camera_probe(struct platform_device *pdev)
dma_release_channel(pcdev->dma_chans[1]);
exit_free_dma_y:
dma_release_channel(pcdev->dma_chans[0]);
+exit_notifier_cleanup:
+ v4l2_async_nf_cleanup(&pcdev->notifier);
+ v4l2_device_unregister(&pcdev->v4l2_dev);
return err;
}