[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-media
Subject:    Re: [PATCH v2 01/13] of: property: add port base loop
From:       Kuninori Morimoto <kuninori.morimoto.gx () renesas ! com>
Date:       2024-01-31 23:25:20
Message-ID: 87r0hxnl0n.wl-kuninori.morimoto.gx () renesas ! com
[Download RAW message or body]


Hi Rob

Thank you for review

> > +/**
> > + * of_graph_get_next_port() - get next port node
> > + * @parent: pointer to the parent device node
> > + * @port: current port node, or NULL to get first
> > + *
> > + * Return: An 'port' node pointer with refcount incremented. Refcount
> > + * of the passed @prev node is decremented.
> > + */
> > +struct device_node *of_graph_get_next_port(const struct device_node *parent,
> > +					   struct device_node *port)
> > +{
> > +	if (!parent)
> > +		return NULL;
> > +
> > +	if (!port) {
> > +		struct device_node *node;
> > +
> > +		node = of_get_child_by_name(parent, "ports");
> > +		if (node) {
> > +			parent = node;
> > +			of_node_put(node);
> 
> The original code had this right, but here you have it wrong.
> 
> You are releasing ports here, but then using it...
> 
> > +		}
> > +
> > +		return of_get_child_by_name(parent, "port");
> 
> ...here. You have to get the child before you can put the parent.

You are reviewing v2, and it was already fixed in v3

Thank you for your help !!

Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto

[prev in list] [next in list] [prev in thread] [next in thread]