Re: [PATCH v3 4/9] media: platform: microchip: use for_each_endpoint_of_node()
From: Dan Carpenter <hidden>
Date: 2024-05-30 06:37:40
Also in:
dri-devel, linux-arm-kernel, linux-fbdev, linux-media, linux-omap
On Thu, May 30, 2024 at 02:06:05AM +0000, Kuninori Morimoto wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c index 5ac149cf3647f..60b6d922d764e 100644 --- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c +++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c@@ -353,33 +353,29 @@ static const u32 isc_sama5d2_gamma_table[][GAMMA_ENTRIES] = { static int isc_parse_dt(struct device *dev, struct isc_device *isc) { struct device_node *np = dev->of_node; - struct device_node *epn = NULL; + struct device_node *epn; struct isc_subdev_entity *subdev_entity; unsigned int flags; - int ret; INIT_LIST_HEAD(&isc->subdev_entities); - while (1) { + for_each_endpoint_of_node(np, epn) { struct v4l2_fwnode_endpoint v4l2_epn = { .bus_type = 0 }; - - epn = of_graph_get_next_endpoint(np, epn); - if (!epn) - return 0; + int ret; ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(epn), &v4l2_epn); if (ret) { - ret = -EINVAL; + of_node_put(epn); dev_err(dev, "Could not parse the endpoint\n"); - break; + return -EINVAL; } subdev_entity = devm_kzalloc(dev, sizeof(*subdev_entity), GFP_KERNEL); if (!subdev_entity) { - ret = -ENOMEM; - break; + of_node_put(epn); + return -ENOMEM; } subdev_entity->epn = epn;
This code is an example of what Laurent was talking about. We're taking storing "subdev_entity->epn = epn" but then not incrementing the refcount. Perhaps it's not necessary? The difference between this and _scoped() would be if we stored epn and then returned. I feel like that's less common. We could detect that sort of thing using static analysis if it turned out to be an issue. regards, dan carpenter