[PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()

[PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()

[Frank.Li]

Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
simplify media code.

Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
rkisp1-dev.c only silience improvement.

Anyways, *_for_each_*_scoped() already use widely and make code clean.

Build test only.

Sakari Ailus:
	when I try to improve the patch
"Add common helper library for 1-to-1 subdev registration", I found need
camss.c pattern, so I create this small improvement firstly.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v2:
- colllect review by tags
- fix typo and indent.
- see each patch's change log.
- Link to v1: https://patch.msgid.link/20260622-fw_scoped-v1-0-a37d0aac0a68@nxp.com

---
Frank Li (4):
      device property: Introduce fwnode_graph_for_each_endpoint_scoped()
      media: mc: use fwnode_graph_for_each_endpoint_scoped() to simpilfy code
      media: rkisp1: use fwnode_graph_for_each_endpoint_scoped() to simplify code
      media: qcom: camss: use fwnode_graph_for_each_endpoint_scoped() to simplify code

 drivers/media/platform/qcom/camss/camss.c           | 17 +++++------------
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c |  4 +---
 drivers/media/v4l2-core/v4l2-mc.c                   |  5 +----
 include/linux/property.h                            |  5 +++++
 4 files changed, 12 insertions(+), 19 deletions(-)
---
base-commit: 3ce97bd3c4f18608335e709c24d6a40e7036cab8
change-id: 20260620-fw_scoped-5dab644510a1

Best regards,
--  
Frank Li <Frank.Li@nxp.com>

[PATCH v2 1/4] device property: Introduce fwnode_graph_for_each_endpoint_scoped()

[Frank.Li]

From: Frank Li <Frank.Li@nxp.com>

Similar to recently propose for_each_child_of_node_scoped() this new
version of the loop macro instantiates a new local struct fwnode_handle *
that uses the __free(fwnode_handle) auto cleanup handling so that if a
reference to a node is held on early exit from the loop the reference will
be released. If the loop runs to completion, the child pointer will be NULL
and no action will be taken.

The reason this is useful is that it removes the need for
fwnode_handle_put() on early loop exits. If there is a need to retain the
reference, then return_ptr(child) or no_free_ptr(child) may be used to
safely disable the auto cleanup.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v2
- collect Andy and Guoniu's reviewed-by tags
- fix indention
- remove extra space in commit message
---
 include/linux/property.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 14c304db46648..d51824c13d2cc 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -545,6 +545,11 @@ unsigned int fwnode_graph_get_endpoint_count(const struct fwnode_handle *fwnode,
 	for (child = fwnode_graph_get_next_endpoint(fwnode, NULL); child;	\
 	     child = fwnode_graph_get_next_endpoint(fwnode, child))
 
+#define fwnode_graph_for_each_endpoint_scoped(fwnode, child)			\
+	for (struct fwnode_handle *child __free(fwnode_handle) =		\
+		fwnode_graph_get_next_endpoint(fwnode, NULL);		\
+	     child; child = fwnode_graph_get_next_endpoint(fwnode, child))
+
 int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 				struct fwnode_endpoint *endpoint);
 

-- 
2.43.0

Re: [PATCH v2 1/4] device property: Introduce fwnode_graph_for_each_endpoint_scoped()

[Laurent Pinchart]

Hi Frank,

Thank you for the patch.

On Wed, Jun 24, 2026 at 01:00:09PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Similar to recently propose for_each_child_of_node_scoped() this new
> version of the loop macro instantiates a new local struct fwnode_handle *
> that uses the __free(fwnode_handle) auto cleanup handling so that if a
> reference to a node is held on early exit from the loop the reference will
> be released. If the loop runs to completion, the child pointer will be NULL
> and no action will be taken.
> 
> The reason this is useful is that it removes the need for
> fwnode_handle_put() on early loop exits. If there is a need to retain the
> reference, then return_ptr(child) or no_free_ptr(child) may be used to
> safely disable the auto cleanup.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Nice idea.

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

> ---
> change in v2
> - collect Andy and Guoniu's reviewed-by tags
> - fix indention
> - remove extra space in commit message
> ---
>  include/linux/property.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 14c304db46648..d51824c13d2cc 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -545,6 +545,11 @@ unsigned int fwnode_graph_get_endpoint_count(const struct fwnode_handle *fwnode,
>  	for (child = fwnode_graph_get_next_endpoint(fwnode, NULL); child;	\
>  	     child = fwnode_graph_get_next_endpoint(fwnode, child))
>  
> +#define fwnode_graph_for_each_endpoint_scoped(fwnode, child)			\
> +	for (struct fwnode_handle *child __free(fwnode_handle) =		\
> +		fwnode_graph_get_next_endpoint(fwnode, NULL);		\
> +	     child; child = fwnode_graph_get_next_endpoint(fwnode, child))
> +
>  int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
>  				struct fwnode_endpoint *endpoint);
>  

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/4] media: mc: use fwnode_graph_for_each_endpoint_scoped() to simpilfy code

[Frank.Li]

From: Frank Li <Frank.Li@nxp.com>

Use cleanup helper fwnode_graph_for_each_endpoint_scoped() to simpilfy
code.

Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/media/v4l2-core/v4l2-mc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
index 937d358697e19..5d7fcd67dc42e 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -324,12 +324,10 @@ EXPORT_SYMBOL_GPL(v4l_vb2q_enable_media_source);
 int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd,
 				    struct media_pad *sink, u32 flags)
 {
-	struct fwnode_handle *endpoint;
-
 	if (!(sink->flags & MEDIA_PAD_FL_SINK))
 		return -EINVAL;
 
-	fwnode_graph_for_each_endpoint(src_sd->fwnode, endpoint) {
+	fwnode_graph_for_each_endpoint_scoped(src_sd->fwnode, endpoint) {
 		struct fwnode_handle *remote_ep;
 		int src_idx, sink_idx, ret;
 		struct media_pad *src;
@@ -397,7 +395,6 @@ int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd,
 				src_sd->entity.name, src_idx,
 				sink->entity->name, sink_idx, ret);
 
-			fwnode_handle_put(endpoint);
 			return ret;
 		}
 	}

-- 
2.43.0

Re: [PATCH v2 2/4] media: mc: use fwnode_graph_for_each_endpoint_scoped() to simpilfy code

[Laurent Pinchart]

On Wed, Jun 24, 2026 at 01:00:10PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Use cleanup helper fwnode_graph_for_each_endpoint_scoped() to simpilfy
> code.
> 
> Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

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

> ---
>  drivers/media/v4l2-core/v4l2-mc.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
> index 937d358697e19..5d7fcd67dc42e 100644
> --- a/drivers/media/v4l2-core/v4l2-mc.c
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -324,12 +324,10 @@ EXPORT_SYMBOL_GPL(v4l_vb2q_enable_media_source);
>  int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd,
>  				    struct media_pad *sink, u32 flags)
>  {
> -	struct fwnode_handle *endpoint;
> -
>  	if (!(sink->flags & MEDIA_PAD_FL_SINK))
>  		return -EINVAL;
>  
> -	fwnode_graph_for_each_endpoint(src_sd->fwnode, endpoint) {
> +	fwnode_graph_for_each_endpoint_scoped(src_sd->fwnode, endpoint) {
>  		struct fwnode_handle *remote_ep;
>  		int src_idx, sink_idx, ret;
>  		struct media_pad *src;
> @@ -397,7 +395,6 @@ int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd,
>  				src_sd->entity.name, src_idx,
>  				sink->entity->name, sink_idx, ret);
>  
> -			fwnode_handle_put(endpoint);
>  			return ret;
>  		}
>  	}

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/4] media: rkisp1: use fwnode_graph_for_each_endpoint_scoped() to simplify code

[Frank.Li]

From: Frank Li <Frank.Li@nxp.com>

Use fwnode_graph_for_each_endpoint_scoped() to simplify code.

No functional changes.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Andy: Keep original code because too much break. and I am working on more
generally solution, so the whole function can be replaced.
---
 drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index 1791c02a40ae1..f90e01301943c 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -187,7 +187,6 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 {
 	struct v4l2_async_notifier *ntf = &rkisp1->notifier;
 	struct fwnode_handle *fwnode = dev_fwnode(rkisp1->dev);
-	struct fwnode_handle *ep;
 	unsigned int index = 0;
 	int ret = 0;
 
@@ -195,7 +194,7 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 
 	ntf->ops = &rkisp1_subdev_notifier_ops;
 
-	fwnode_graph_for_each_endpoint(fwnode, ep) {
+	fwnode_graph_for_each_endpoint_scoped(fwnode, ep) {
 		struct fwnode_handle *port;
 		struct v4l2_fwnode_endpoint vep = { };
 		struct rkisp1_sensor_async *rk_asd;
@@ -286,7 +285,6 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
 	}
 
 	if (ret) {
-		fwnode_handle_put(ep);
 		v4l2_async_nf_cleanup(ntf);
 		return ret;
 	}

-- 
2.43.0

Re: [PATCH v2 3/4] media: rkisp1: use fwnode_graph_for_each_endpoint_scoped() to simplify code

[Laurent Pinchart]

On Wed, Jun 24, 2026 at 01:00:11PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Use fwnode_graph_for_each_endpoint_scoped() to simplify code.
> 
> No functional changes.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

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

> ---
> Andy: Keep original code because too much break. and I am working on more
> generally solution, so the whole function can be replaced.
> ---
>  drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index 1791c02a40ae1..f90e01301943c 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -187,7 +187,6 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
>  {
>  	struct v4l2_async_notifier *ntf = &rkisp1->notifier;
>  	struct fwnode_handle *fwnode = dev_fwnode(rkisp1->dev);
> -	struct fwnode_handle *ep;
>  	unsigned int index = 0;
>  	int ret = 0;
>  
> @@ -195,7 +194,7 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
>  
>  	ntf->ops = &rkisp1_subdev_notifier_ops;
>  
> -	fwnode_graph_for_each_endpoint(fwnode, ep) {
> +	fwnode_graph_for_each_endpoint_scoped(fwnode, ep) {
>  		struct fwnode_handle *port;
>  		struct v4l2_fwnode_endpoint vep = { };
>  		struct rkisp1_sensor_async *rk_asd;
> @@ -286,7 +285,6 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
>  	}
>  
>  	if (ret) {
> -		fwnode_handle_put(ep);
>  		v4l2_async_nf_cleanup(ntf);
>  		return ret;
>  	}

-- 
Regards,

Laurent Pinchart

[PATCH v2 4/4] media: qcom: camss: use fwnode_graph_for_each_endpoint_scoped() to simplify code

[Frank.Li]

From: Frank Li <Frank.Li@nxp.com>

Use fwnode_graph_for_each_endpoint_scoped() to simplify code.

No functional changes.

Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v2
- fix typo simplify
- collect andy, gouniou and loic's review tags
---
 drivers/media/platform/qcom/camss/camss.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 2123f6388e3d7..23f3cc30a15a5 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -4793,30 +4793,23 @@ static int camss_parse_endpoint_node(struct device *dev,
 static int camss_parse_ports(struct camss *camss)
 {
 	struct device *dev = camss->dev;
-	struct fwnode_handle *fwnode = dev_fwnode(dev), *ep;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
 	int ret;
 
-	fwnode_graph_for_each_endpoint(fwnode, ep) {
+	fwnode_graph_for_each_endpoint_scoped(fwnode, ep) {
 		struct camss_async_subdev *csd;
 
 		csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
 						      typeof(*csd));
-		if (IS_ERR(csd)) {
-			ret = PTR_ERR(csd);
-			goto err_cleanup;
-		}
+		if (IS_ERR(csd))
+			return PTR_ERR(csd);
 
 		ret = camss_parse_endpoint_node(dev, ep, csd);
 		if (ret < 0)
-			goto err_cleanup;
+			return ret;
 	}
 
 	return 0;
-
-err_cleanup:
-	fwnode_handle_put(ep);
-
-	return ret;
 }
 
 /*

-- 
2.43.0

Re: [PATCH v2 4/4] media: qcom: camss: use fwnode_graph_for_each_endpoint_scoped() to simplify code

[Laurent Pinchart]

On Wed, Jun 24, 2026 at 01:00:12PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Use fwnode_graph_for_each_endpoint_scoped() to simplify code.
> 
> No functional changes.
> 
> Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

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

> ---
> change in v2
> - fix typo simplify
> - collect andy, gouniou and loic's review tags
> ---
>  drivers/media/platform/qcom/camss/camss.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 2123f6388e3d7..23f3cc30a15a5 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4793,30 +4793,23 @@ static int camss_parse_endpoint_node(struct device *dev,
>  static int camss_parse_ports(struct camss *camss)
>  {
>  	struct device *dev = camss->dev;
> -	struct fwnode_handle *fwnode = dev_fwnode(dev), *ep;
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
>  	int ret;
>  
> -	fwnode_graph_for_each_endpoint(fwnode, ep) {
> +	fwnode_graph_for_each_endpoint_scoped(fwnode, ep) {
>  		struct camss_async_subdev *csd;
>  
>  		csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
>  						      typeof(*csd));
> -		if (IS_ERR(csd)) {
> -			ret = PTR_ERR(csd);
> -			goto err_cleanup;
> -		}
> +		if (IS_ERR(csd))
> +			return PTR_ERR(csd);
>  
>  		ret = camss_parse_endpoint_node(dev, ep, csd);
>  		if (ret < 0)
> -			goto err_cleanup;
> +			return ret;
>  	}
>  
>  	return 0;
> -
> -err_cleanup:
> -	fwnode_handle_put(ep);
> -
> -	return ret;
>  }
>  
>  /*

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()

[Laurent Pinchart]

Hi Frank,

On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> simplify media code.
> 
> Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> rkisp1-dev.c only silience improvement.
> 
> Anyways, *_for_each_*_scoped() already use widely and make code clean.
> 
> Build test only.
> 
> Sakari Ailus:
> 	when I try to improve the patch
> "Add common helper library for 1-to-1 subdev registration", I found need
> camss.c pattern, so I create this small improvement firstly.

Those are nice cleanups, thank you.

After applying this series, the only left users of the
fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
They can all be trivially replaced with the scoped variant. Should we
add a patch to use fwnode_graph_for_each_endpoint_scoped() everywhere,
and drop fwnode_graph_for_each_endpoint() ?

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Changes in v2:
> - colllect review by tags
> - fix typo and indent.
> - see each patch's change log.
> - Link to v1: https://patch.msgid.link/20260622-fw_scoped-v1-0-a37d0aac0a68@nxp.com
> 
> ---
> Frank Li (4):
>       device property: Introduce fwnode_graph_for_each_endpoint_scoped()
>       media: mc: use fwnode_graph_for_each_endpoint_scoped() to simpilfy code
>       media: rkisp1: use fwnode_graph_for_each_endpoint_scoped() to simplify code
>       media: qcom: camss: use fwnode_graph_for_each_endpoint_scoped() to simplify code
> 
>  drivers/media/platform/qcom/camss/camss.c           | 17 +++++------------
>  drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c |  4 +---
>  drivers/media/v4l2-core/v4l2-mc.c                   |  5 +----
>  include/linux/property.h                            |  5 +++++
>  4 files changed, 12 insertions(+), 19 deletions(-)
> ---
> base-commit: 3ce97bd3c4f18608335e709c24d6a40e7036cab8
> change-id: 20260620-fw_scoped-5dab644510a1

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()

[Frank Li]

On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > simplify media code.
> >
> > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > rkisp1-dev.c only silience improvement.
> >
> > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> >
> > Build test only.
> >
> > Sakari Ailus:
> > 	when I try to improve the patch
> > "Add common helper library for 1-to-1 subdev registration", I found need
> > camss.c pattern, so I create this small improvement firstly.
>
> Those are nice cleanups, thank you.
>
> After applying this series, the only left users of the
> fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.

I already checked previously, two place use it.

fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
ep is NULL, which totally equial to scoped() version.

another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
caller to call put().

if use scoped() version, need use no_free_ptr() at return, which make think
a little bit complex.

It'd better leave these as it.

Frank

Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()

[Laurent Pinchart]

On Wed, Jun 24, 2026 at 02:35:14PM -0500, Frank Li wrote:
> On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> > On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > > simplify media code.
> > >
> > > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > > rkisp1-dev.c only silience improvement.
> > >
> > > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> > >
> > > Build test only.
> > >
> > > Sakari Ailus:
> > > 	when I try to improve the patch
> > > "Add common helper library for 1-to-1 subdev registration", I found need
> > > camss.c pattern, so I create this small improvement firstly.
> >
> > Those are nice cleanups, thank you.
> >
> > After applying this series, the only left users of the
> > fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
> 
> I already checked previously, two place use it.
> 
> fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
> ep is NULL, which totally equial to scoped() version.
> 
> another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
> caller to call put().
> 
> if use scoped() version, need use no_free_ptr() at return, which make think
> a little bit complex.

It would introduce a tiny bit of extra complexity there, but the
advantage (in my opinion) is that we'll be able to remove the less safe
fwnode_graph_for_each_endpoint() macro.

Now one may argue that the risk of
fwnode_graph_for_each_endpoint_scoped() is returning the iterator
without using no_free_ptr(). I wonder if that would be easier to catch
in static analysis tools than the current pattern that leaks a reference
when exiting the loop early.

> It'd better leave these as it.

-- 
Regards,

Laurent Pinchart