Hi Rob > > We already have of_graph_get_next_endpoint(), but it is not > > intuitive to use in some case. > > Can of_graph_get_next_endpoint() users be replaced with your new > helpers? I'd really like to get rid of the 3 remaining users. Hmm... of_graph_get_next_endpoint() will fetch "endpoint" beyond the "port", but new helper doesn't have such feature. Even though I try to replace it with new helper, I guess it will be almost same as current of_graph_get_next_endpoint() anyway. Alternative idea is... One of the big user of of_graph_get_next_endpoint() is for_each_endpoint_of_node() loop. So we can replace it to.. - for_each_endpoint_of_node(parent, endpoint) { + for_each_of_graph_port(parent, port) { + for_each_of_graph_port_endpoint(port, endpoint) { Above is possible, but it replaces single loop to multi loops. And, we still need to consider about of_fwnode_graph_get_next_endpoint() which is the last user of of_graph_get_next_endpoint() > > +struct device_node *of_graph_get_next_port_endpoint(const struct device_node *port, > > + struct device_node *prev) > > +{ > > + do { > > + prev = of_get_next_child(port, prev); > > + if (!prev) > > + break; > > + } while (!of_node_name_eq(prev, "endpoint")); > > Really, this check is validation as no other name is valid in a > port node. The kernel is not responsible for validation, but okay. > However, if we are going to keep this, might as well make it WARN(). OK, will do in v4 > > +/** > > + * for_each_of_graph_port_endpoint - iterate over every endpoint in a port node > > + * @parent: parent port node > > + * @child: loop variable pointing to the current endpoint node > > + * > > + * When breaking out of the loop, of_node_put(child) has to be called manually. > > No need for this requirement anymore. Use cleanup.h so this is > automatic. Do you mean it should include __free() inside this loop, like _scoped() ? #define for_each_child_of_node_scoped(parent, child) \ for (struct device_node *child __free(device_node) = \ of_get_next_child(parent, NULL); \ child != NULL; \ child = of_get_next_child(parent, child)) In such case, I wonder does it need to have _scoped() in loop name ? And in such case, I think we want to have non _scoped() loop too ? For example, when user want to use the param. for_each_of_graph_port_endpoint(port, endpoint) if (xxx == yyy) return endpoint; for_each_of_graph_port_endpoint_scoped(port, endpoint) if (xxx == yyy) return of_node_get(endpoint) Thank you for your help !! Best regards --- Kuninori Morimoto