diff mbox series

[v4,35/38] media: davinci: Init async notifier after registering V4L2 device

Message ID 20230622114028.908825-36-sakari.ailus@linux.intel.com
State New
Headers show
Series Separate links and async sub-devices | expand

Commit Message

Sakari Ailus June 22, 2023, 11:40 a.m. UTC
Initialise the V4L2 async notifier after registering the V4L2 device, just
before parsing DT for async sub-devices. This way the device can be made
available to the V4L2 async framework from the notifier init time onwards.
A subsequent patch will add struct v4l2_device as an argument to
v4l2_async_nf_init().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de> # imx6qp
Tested_by: Niklas Söderlund <niklas.soderlund@ragnatech.se> # rcar + adv746x
Tested-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> # Apalis i.MX6Q with TC358743
---
 .../media/platform/ti/davinci/vpif_capture.c    | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Lad, Prabhakar June 26, 2023, 3:36 p.m. UTC | #1
Hi Sakari,

Thank you for the patch.

On Thu, Jun 22, 2023 at 12:41 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Initialise the V4L2 async notifier after registering the V4L2 device, just
> before parsing DT for async sub-devices. This way the device can be made
> available to the V4L2 async framework from the notifier init time onwards.
> A subsequent patch will add struct v4l2_device as an argument to
> v4l2_async_nf_init().
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de> # imx6qp
> Tested_by: Niklas Söderlund <niklas.soderlund@ragnatech.se> # rcar + adv746x
> Tested-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> # Apalis i.MX6Q with TC358743
> ---
>  .../media/platform/ti/davinci/vpif_capture.c    | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/ti/davinci/vpif_capture.c b/drivers/media/platform/ti/davinci/vpif_capture.c
> index a63c9e51dac41..9b97e26be0892 100644
> --- a/drivers/media/platform/ti/davinci/vpif_capture.c
> +++ b/drivers/media/platform/ti/davinci/vpif_capture.c
> @@ -1608,18 +1608,12 @@ static __init int vpif_probe(struct platform_device *pdev)
>         int res_idx = 0;
>         int i, err;
>
> -       pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
> -       if (!pdev->dev.platform_data) {
> -               dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> -               return -EINVAL;
> -       }
> -
>         vpif_dev = &pdev->dev;
>
>         err = initialize_vpif();
>         if (err) {
>                 v4l2_err(vpif_dev->driver, "Error initializing vpif\n");
> -               goto cleanup;
> +               return err;
>         }
>
>         err = v4l2_device_register(vpif_dev, &vpif_obj.v4l2_dev);
> @@ -1655,6 +1649,12 @@ static __init int vpif_probe(struct platform_device *pdev)
>                 goto vpif_unregister;
>         }
>
> +       pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
Just a couple of lines above we reference pdev->dev.platform_data
while assigning it to vpif_obj.config, so this has to be moved prior
to assigning vpif_obj.config.

> +       if (!pdev->dev.platform_data) {
> +               dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
unrelated to this patch maybe we can drop the extra space after data.  Giving..

Cheers,
Prabhakar
Sakari Ailus June 29, 2023, 8:44 a.m. UTC | #2
Hi Prabhakar,

On Mon, Jun 26, 2023 at 04:36:12PM +0100, Lad, Prabhakar wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thu, Jun 22, 2023 at 12:41 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Initialise the V4L2 async notifier after registering the V4L2 device, just
> > before parsing DT for async sub-devices. This way the device can be made
> > available to the V4L2 async framework from the notifier init time onwards.
> > A subsequent patch will add struct v4l2_device as an argument to
> > v4l2_async_nf_init().
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Tested-by: Philipp Zabel <p.zabel@pengutronix.de> # imx6qp
> > Tested_by: Niklas Söderlund <niklas.soderlund@ragnatech.se> # rcar + adv746x
> > Tested-by: Aishwarya Kothari <aishwarya.kothari@toradex.com> # Apalis i.MX6Q with TC358743
> > ---
> >  .../media/platform/ti/davinci/vpif_capture.c    | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/media/platform/ti/davinci/vpif_capture.c b/drivers/media/platform/ti/davinci/vpif_capture.c
> > index a63c9e51dac41..9b97e26be0892 100644
> > --- a/drivers/media/platform/ti/davinci/vpif_capture.c
> > +++ b/drivers/media/platform/ti/davinci/vpif_capture.c
> > @@ -1608,18 +1608,12 @@ static __init int vpif_probe(struct platform_device *pdev)
> >         int res_idx = 0;
> >         int i, err;
> >
> > -       pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
> > -       if (!pdev->dev.platform_data) {
> > -               dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> > -               return -EINVAL;
> > -       }
> > -
> >         vpif_dev = &pdev->dev;
> >
> >         err = initialize_vpif();
> >         if (err) {
> >                 v4l2_err(vpif_dev->driver, "Error initializing vpif\n");
> > -               goto cleanup;
> > +               return err;
> >         }
> >
> >         err = v4l2_device_register(vpif_dev, &vpif_obj.v4l2_dev);
> > @@ -1655,6 +1649,12 @@ static __init int vpif_probe(struct platform_device *pdev)
> >                 goto vpif_unregister;
> >         }
> >
> > +       pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
> Just a couple of lines above we reference pdev->dev.platform_data
> while assigning it to vpif_obj.config, so this has to be moved prior
> to assigning vpif_obj.config.

Thanks. I had missed this --- very seldom this is used for anything else in
probe than setting up the notifier.

> 
> > +       if (!pdev->dev.platform_data) {
> > +               dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> unrelated to this patch maybe we can drop the extra space after data.  Giving..

I'll address this for v5.
diff mbox series

Patch

diff --git a/drivers/media/platform/ti/davinci/vpif_capture.c b/drivers/media/platform/ti/davinci/vpif_capture.c
index a63c9e51dac41..9b97e26be0892 100644
--- a/drivers/media/platform/ti/davinci/vpif_capture.c
+++ b/drivers/media/platform/ti/davinci/vpif_capture.c
@@ -1608,18 +1608,12 @@  static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
-	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
-	if (!pdev->dev.platform_data) {
-		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
-		return -EINVAL;
-	}
-
 	vpif_dev = &pdev->dev;
 
 	err = initialize_vpif();
 	if (err) {
 		v4l2_err(vpif_dev->driver, "Error initializing vpif\n");
-		goto cleanup;
+		return err;
 	}
 
 	err = v4l2_device_register(vpif_dev, &vpif_obj.v4l2_dev);
@@ -1655,6 +1649,12 @@  static __init int vpif_probe(struct platform_device *pdev)
 		goto vpif_unregister;
 	}
 
+	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		goto probe_subdev_out;
+	}
+
 	if (!vpif_obj.config->asd_sizes[0]) {
 		int i2c_id = vpif_obj.config->i2c_adapter_id;
 
@@ -1695,14 +1695,13 @@  static __init int vpif_probe(struct platform_device *pdev)
 	return 0;
 
 probe_subdev_out:
+	v4l2_async_nf_cleanup(&vpif_obj.notifier);
 	/* free sub devices memory */
 	kfree(vpif_obj.sd);
 vpif_unregister:
 	v4l2_device_unregister(&vpif_obj.v4l2_dev);
 vpif_free:
 	free_vpif_objs();
-cleanup:
-	v4l2_async_nf_cleanup(&vpif_obj.notifier);
 
 	return err;
 }