On Thu, May 30, 2024 at 02:06:05AM +0000, Kuninori Morimoto wrote:
> 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
