| Message ID | 20240610100530.1107771-20-sakari.ailus@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Media device lifetime management | expand |
On 10/06/2024 12:05, Sakari Ailus wrote: > Release all the resources when the media device is released, moving away > from the struct v4l2_device used for that purpose. This is done to > exemplify the use of the media device's release callback. > > Switch to container_of_const(), too, while we're changing the code anyway. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/media/test-drivers/vimc/vimc-core.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c > index af127476e920..3e59f8c256c7 100644 > --- a/drivers/media/test-drivers/vimc/vimc-core.c > +++ b/drivers/media/test-drivers/vimc/vimc-core.c > @@ -264,13 +264,12 @@ static int vimc_add_subdevs(struct vimc_device *vimc) > return 0; > } > > -static void vimc_v4l2_dev_release(struct v4l2_device *v4l2_dev) > +static void vimc_mdev_release(struct media_device *mdev) > { > struct vimc_device *vimc = > - container_of(v4l2_dev, struct vimc_device, v4l2_dev); > + container_of_const(mdev, struct vimc_device, mdev); Please don't mix this in. It makes no sense here since vimc is never const. Such a change doesn't belong in this series. So just leave it at container_of and update the commit log. With that change you can add my: Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Regards, Hans > > vimc_release_subdevs(vimc); > - media_device_cleanup(&vimc->mdev); > kfree(vimc->ent_devs); > kfree(vimc); > } > @@ -336,6 +335,10 @@ static int vimc_register_devices(struct vimc_device *vimc) > return ret; > } > > +static const struct media_device_ops vimc_mdev_ops = { > + .release = vimc_mdev_release, > +}; > + > static int vimc_probe(struct platform_device *pdev) > { > const struct font_desc *font = find_font("VGA8x16"); > @@ -369,12 +372,12 @@ static int vimc_probe(struct platform_device *pdev) > snprintf(vimc->mdev.bus_info, sizeof(vimc->mdev.bus_info), > "platform:%s", VIMC_PDEV_NAME); > vimc->mdev.dev = &pdev->dev; > + vimc->mdev.ops = &vimc_mdev_ops; > media_device_init(&vimc->mdev); > > ret = vimc_register_devices(vimc); > if (ret) { > - media_device_cleanup(&vimc->mdev); > - kfree(vimc); > + media_device_put(&vimc->mdev); > return ret; > } > /* > @@ -382,7 +385,6 @@ static int vimc_probe(struct platform_device *pdev) > * if the registration fails, we release directly from probe > */ > > - vimc->v4l2_dev.release = vimc_v4l2_dev_release; > platform_set_drvdata(pdev, vimc); > return 0; > } > @@ -397,6 +399,7 @@ static void vimc_remove(struct platform_device *pdev) > media_device_unregister(&vimc->mdev); > v4l2_device_unregister(&vimc->v4l2_dev); > v4l2_device_put(&vimc->v4l2_dev); > + media_device_put(&vimc->mdev); > } > > static void vimc_dev_release(struct device *dev)
Hi Hans, Thank you for the review. On Mon, Jun 17, 2024 at 11:49:54AM +0200, Hans Verkuil wrote: > On 10/06/2024 12:05, Sakari Ailus wrote: > > Release all the resources when the media device is released, moving away > > from the struct v4l2_device used for that purpose. This is done to > > exemplify the use of the media device's release callback. > > > > Switch to container_of_const(), too, while we're changing the code anyway. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > drivers/media/test-drivers/vimc/vimc-core.c | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c > > index af127476e920..3e59f8c256c7 100644 > > --- a/drivers/media/test-drivers/vimc/vimc-core.c > > +++ b/drivers/media/test-drivers/vimc/vimc-core.c > > @@ -264,13 +264,12 @@ static int vimc_add_subdevs(struct vimc_device *vimc) > > return 0; > > } > > > > -static void vimc_v4l2_dev_release(struct v4l2_device *v4l2_dev) > > +static void vimc_mdev_release(struct media_device *mdev) > > { > > struct vimc_device *vimc = > > - container_of(v4l2_dev, struct vimc_device, v4l2_dev); > > + container_of_const(mdev, struct vimc_device, mdev); > > Please don't mix this in. It makes no sense here since vimc is never > const. Such a change doesn't belong in this series. So just leave it > at container_of and update the commit log. Ok, I can remove it. I also posted a patch to address the matter in container_of() documentation. Let's see. > > With that change you can add my: > > Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Thank you!
diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c index af127476e920..3e59f8c256c7 100644 --- a/drivers/media/test-drivers/vimc/vimc-core.c +++ b/drivers/media/test-drivers/vimc/vimc-core.c @@ -264,13 +264,12 @@ static int vimc_add_subdevs(struct vimc_device *vimc) return 0; } -static void vimc_v4l2_dev_release(struct v4l2_device *v4l2_dev) +static void vimc_mdev_release(struct media_device *mdev) { struct vimc_device *vimc = - container_of(v4l2_dev, struct vimc_device, v4l2_dev); + container_of_const(mdev, struct vimc_device, mdev); vimc_release_subdevs(vimc); - media_device_cleanup(&vimc->mdev); kfree(vimc->ent_devs); kfree(vimc); } @@ -336,6 +335,10 @@ static int vimc_register_devices(struct vimc_device *vimc) return ret; } +static const struct media_device_ops vimc_mdev_ops = { + .release = vimc_mdev_release, +}; + static int vimc_probe(struct platform_device *pdev) { const struct font_desc *font = find_font("VGA8x16"); @@ -369,12 +372,12 @@ static int vimc_probe(struct platform_device *pdev) snprintf(vimc->mdev.bus_info, sizeof(vimc->mdev.bus_info), "platform:%s", VIMC_PDEV_NAME); vimc->mdev.dev = &pdev->dev; + vimc->mdev.ops = &vimc_mdev_ops; media_device_init(&vimc->mdev); ret = vimc_register_devices(vimc); if (ret) { - media_device_cleanup(&vimc->mdev); - kfree(vimc); + media_device_put(&vimc->mdev); return ret; } /* @@ -382,7 +385,6 @@ static int vimc_probe(struct platform_device *pdev) * if the registration fails, we release directly from probe */ - vimc->v4l2_dev.release = vimc_v4l2_dev_release; platform_set_drvdata(pdev, vimc); return 0; } @@ -397,6 +399,7 @@ static void vimc_remove(struct platform_device *pdev) media_device_unregister(&vimc->mdev); v4l2_device_unregister(&vimc->v4l2_dev); v4l2_device_put(&vimc->v4l2_dev); + media_device_put(&vimc->mdev); } static void vimc_dev_release(struct device *dev)
Release all the resources when the media device is released, moving away from the struct v4l2_device used for that purpose. This is done to exemplify the use of the media device's release callback. Switch to container_of_const(), too, while we're changing the code anyway. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/test-drivers/vimc/vimc-core.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)