| Message ID | 20210118015258.3993-5-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | V4L2 Async notifier API cleanup | expand |
Hi Ezequiel, On 1/17/21 10:52 PM, Ezequiel Garcia wrote: > The use of v4l2_async_notifier_add_subdev will be discouraged. > Drivers are instead encouraged to use a helper such as > v4l2_async_notifier_add_fwnode_remote_subdev. > > This fixes a misuse of the API, as v4l2_async_notifier_add_subdev > should get a kmalloc'ed struct v4l2_async_subdev, > removing some boilerplate code while at it. > > Use the appropriate helper v4l2_async_notifier_add_fwnode_remote_subdev, > which handles the needed setup, instead of open-coding it. > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > drivers/media/platform/exynos4-is/media-dev.c | 25 +++++++++++-------- > drivers/media/platform/exynos4-is/media-dev.h | 2 +- > 2 files changed, 15 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c > index e636c33e847b..de26ecdcfe81 100644 > --- a/drivers/media/platform/exynos4-is/media-dev.c > +++ b/drivers/media/platform/exynos4-is/media-dev.c > @@ -401,6 +401,7 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, > int index = fmd->num_sensors; > struct fimc_source_info *pd = &fmd->sensor[index].pdata; > struct device_node *rem, *np; > + struct v4l2_async_subdev *asd; > struct v4l2_fwnode_endpoint endpoint = { .bus_type = 0 }; > int ret; > > @@ -418,10 +419,11 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, > pd->mux_id = (endpoint.base.port - 1) & 0x1; > > rem = of_graph_get_remote_port_parent(ep); > - of_node_put(ep); > if (rem == NULL) { > v4l2_info(&fmd->v4l2_dev, "Remote device at %pOF not found\n", > ep); > + of_node_put(rem); rem is NULL here. Regards, Helen > + of_node_put(ep); > return 0; > } > > @@ -450,6 +452,7 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, > * checking parent's node name. > */ > np = of_get_parent(rem); > + of_node_put(rem); > > if (of_node_name_eq(np, "i2c-isp")) > pd->fimc_bus_type = FIMC_BUS_TYPE_ISP_WRITEBACK; > @@ -458,20 +461,19 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, > of_node_put(np); > > if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor))) { > - of_node_put(rem); > + of_node_put(ep); > return -EINVAL; > } > > - fmd->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_FWNODE; > - fmd->sensor[index].asd.match.fwnode = of_fwnode_handle(rem); > + asd = v4l2_async_notifier_add_fwnode_remote_subdev( > + &fmd->subdev_notifier, of_fwnode_handle(ep), sizeof(*asd)); > > - ret = v4l2_async_notifier_add_subdev(&fmd->subdev_notifier, > - &fmd->sensor[index].asd); > - if (ret) { > - of_node_put(rem); > - return ret; > - } > + of_node_put(ep); > + > + if (IS_ERR(asd)) > + return PTR_ERR(asd); > > + fmd->sensor[index].asd = asd; > fmd->num_sensors++; > > return 0; > @@ -1381,7 +1383,8 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier, > > /* Find platform data for this sensor subdev */ > for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) > - if (fmd->sensor[i].asd.match.fwnode == > + if (fmd->sensor[i].asd && > + fmd->sensor[i].asd->match.fwnode == > of_fwnode_handle(subdev->dev->of_node)) > si = &fmd->sensor[i]; > > diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h > index 9447fafe23c6..a3876d668ea6 100644 > --- a/drivers/media/platform/exynos4-is/media-dev.h > +++ b/drivers/media/platform/exynos4-is/media-dev.h > @@ -83,7 +83,7 @@ struct fimc_camclk_info { > */ > struct fimc_sensor_info { > struct fimc_source_info pdata; > - struct v4l2_async_subdev asd; > + struct v4l2_async_subdev *asd; > struct v4l2_subdev *subdev; > struct fimc_dev *host; > }; >
On Tue, 2021-01-19 at 15:52 -0300, Helen Koike wrote: > Hi Ezequiel, > > On 1/17/21 10:52 PM, Ezequiel Garcia wrote: > > The use of v4l2_async_notifier_add_subdev will be discouraged. > > Drivers are instead encouraged to use a helper such as > > v4l2_async_notifier_add_fwnode_remote_subdev. > > > > This fixes a misuse of the API, as v4l2_async_notifier_add_subdev > > should get a kmalloc'ed struct v4l2_async_subdev, > > removing some boilerplate code while at it. > > > > Use the appropriate helper v4l2_async_notifier_add_fwnode_remote_subdev, > > which handles the needed setup, instead of open-coding it. > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > drivers/media/platform/exynos4-is/media-dev.c | 25 +++++++++++-------- > > drivers/media/platform/exynos4-is/media-dev.h | 2 +- > > 2 files changed, 15 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c > > index e636c33e847b..de26ecdcfe81 100644 > > --- a/drivers/media/platform/exynos4-is/media-dev.c > > +++ b/drivers/media/platform/exynos4-is/media-dev.c > > @@ -401,6 +401,7 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, > > int index = fmd->num_sensors; > > struct fimc_source_info *pd = &fmd->sensor[index].pdata; > > struct device_node *rem, *np; > > + struct v4l2_async_subdev *asd; > > struct v4l2_fwnode_endpoint endpoint = { .bus_type = 0 }; > > int ret; > > > > @@ -418,10 +419,11 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, > > pd->mux_id = (endpoint.base.port - 1) & 0x1; > > > > rem = of_graph_get_remote_port_parent(ep); > > - of_node_put(ep); > > if (rem == NULL) { > > v4l2_info(&fmd->v4l2_dev, "Remote device at %pOF not found\n", > > ep); > > + of_node_put(rem); > > rem is NULL here. > Ouch, crap. Thanks for spotting this! Ezequiel
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index e636c33e847b..de26ecdcfe81 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -401,6 +401,7 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, int index = fmd->num_sensors; struct fimc_source_info *pd = &fmd->sensor[index].pdata; struct device_node *rem, *np; + struct v4l2_async_subdev *asd; struct v4l2_fwnode_endpoint endpoint = { .bus_type = 0 }; int ret; @@ -418,10 +419,11 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, pd->mux_id = (endpoint.base.port - 1) & 0x1; rem = of_graph_get_remote_port_parent(ep); - of_node_put(ep); if (rem == NULL) { v4l2_info(&fmd->v4l2_dev, "Remote device at %pOF not found\n", ep); + of_node_put(rem); + of_node_put(ep); return 0; } @@ -450,6 +452,7 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, * checking parent's node name. */ np = of_get_parent(rem); + of_node_put(rem); if (of_node_name_eq(np, "i2c-isp")) pd->fimc_bus_type = FIMC_BUS_TYPE_ISP_WRITEBACK; @@ -458,20 +461,19 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, of_node_put(np); if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor))) { - of_node_put(rem); + of_node_put(ep); return -EINVAL; } - fmd->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_FWNODE; - fmd->sensor[index].asd.match.fwnode = of_fwnode_handle(rem); + asd = v4l2_async_notifier_add_fwnode_remote_subdev( + &fmd->subdev_notifier, of_fwnode_handle(ep), sizeof(*asd)); - ret = v4l2_async_notifier_add_subdev(&fmd->subdev_notifier, - &fmd->sensor[index].asd); - if (ret) { - of_node_put(rem); - return ret; - } + of_node_put(ep); + + if (IS_ERR(asd)) + return PTR_ERR(asd); + fmd->sensor[index].asd = asd; fmd->num_sensors++; return 0; @@ -1381,7 +1383,8 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier, /* Find platform data for this sensor subdev */ for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) - if (fmd->sensor[i].asd.match.fwnode == + if (fmd->sensor[i].asd && + fmd->sensor[i].asd->match.fwnode == of_fwnode_handle(subdev->dev->of_node)) si = &fmd->sensor[i]; diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h index 9447fafe23c6..a3876d668ea6 100644 --- a/drivers/media/platform/exynos4-is/media-dev.h +++ b/drivers/media/platform/exynos4-is/media-dev.h @@ -83,7 +83,7 @@ struct fimc_camclk_info { */ struct fimc_sensor_info { struct fimc_source_info pdata; - struct v4l2_async_subdev asd; + struct v4l2_async_subdev *asd; struct v4l2_subdev *subdev; struct fimc_dev *host; };