Re: [PATCH v2 resend 3/8] media: platform: microchip: use for_each_endpoint_of_node()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hello Morimoto-san,

Thank you for the patch.

On Tue, May 28, 2024 at 11:55:37PM +0000, Kuninori Morimoto wrote:
> We already have for_each_endpoint_of_node(), don't use
> of_graph_get_next_endpoint() directly. Replace it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../microchip/microchip-sama5d2-isc.c         | 19 +++++++------------
>  .../microchip/microchip-sama7g5-isc.c         | 19 +++++++------------
>  2 files changed, 14 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
> index 5ac149cf3647f..d9298771f5097 100644
> --- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
> +++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
> @@ -356,30 +356,26 @@ static int isc_parse_dt(struct device *dev, struct isc_device *isc)
>  	struct device_node *epn = NULL;

There's no need anymore to initialize epn to NULL. Same in
microchip-sama7g5-isc.c. With this addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

>  	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;
>  
> @@ -400,9 +396,8 @@ static int isc_parse_dt(struct device *dev, struct isc_device *isc)
>  
>  		list_add_tail(&subdev_entity->list, &isc->subdev_entities);
>  	}
> -	of_node_put(epn);
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static int microchip_isc_probe(struct platform_device *pdev)
> diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> index 73445f33d26ba..36204fee10aa2 100644
> --- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> +++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> @@ -339,33 +339,29 @@ static int xisc_parse_dt(struct device *dev, struct isc_device *isc)
>  	struct device_node *epn = NULL;
>  	struct isc_subdev_entity *subdev_entity;
>  	unsigned int flags;
> -	int ret;
>  	bool mipi_mode;
>  
>  	INIT_LIST_HEAD(&isc->subdev_entities);
>  
>  	mipi_mode = of_property_read_bool(np, "microchip,mipi-mode");
>  
> -	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;
>  
> @@ -389,9 +385,8 @@ static int xisc_parse_dt(struct device *dev, struct isc_device *isc)
>  
>  		list_add_tail(&subdev_entity->list, &isc->subdev_entities);
>  	}
> -	of_node_put(epn);
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static int microchip_xisc_probe(struct platform_device *pdev)

-- 
Regards,

Laurent Pinchart