Re: [PATCH 04/13] media: exynos4-is: Use v4l2_async_notifier_add_fwnode_remote_subdev
From: Jacopo Mondi <jacopo@jmondi.org>
Date: 2021-01-16 17:39:47
Hi Ezequiel On Tue, Jan 12, 2021 at 10:23:30AM -0300, Ezequiel Garcia wrote:
quoted hunk ↗ jump to hunk
The use of v4l2_async_notifier_add_subdev is 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 <redacted> --- drivers/media/platform/exynos4-is/media-dev.c | 25 +++++++++---------- drivers/media/platform/exynos4-is/media-dev.h | 2 +- 2 files changed, 13 insertions(+), 14 deletions(-)diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index e636c33e847b..196166a9a4e5 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,7 +419,6 @@ 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 you remove it from here, don't forget to put it in the here below error path
if (rem == NULL) {quoted hunk ↗ jump to hunk
v4l2_info(&fmd->v4l2_dev, "Remote device at %pOF not found\n", ep);@@ -450,6 +450,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);
unrelated but good
quoted hunk ↗ jump to hunk
if (of_node_name_eq(np, "i2c-isp")) pd->fimc_bus_type = FIMC_BUS_TYPE_ISP_WRITEBACK;@@ -457,21 +458,18 @@ static int fimc_md_parse_one_endpoint(struct fimc_md *fmd, pd->fimc_bus_type = pd->sensor_bus_type; of_node_put(np); - if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor))) { - of_node_put(rem);
I think if you need to keep 'ep' around until the call to v4l2_async_notifier_add_fwnode_remote_subdev() below, it should be put here as you remove the above of_node_put(ep). I wonder if registering the async subdev before parsing the endpoint would make things simpler. Not required for this patch though.
quoted hunk ↗ jump to hunk
+ if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor))) 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 +1379,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 &&
Is this needed ? If the subdev has bound the async subdev has been allocated correctly, doesn't it ? With the ep ref counting clarified Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Thanks j
quoted hunk ↗ jump to hunk
+ 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; }; --2.29.2