Re: [PATCH v3 14/24] of: property: add of_graph_get_next_endpoint()
Hi Luca Thank you for your review > > To handle endpoint more intuitive, create of_graph_get_next_endpoint() > > > > of_graph_get_next_endpoint(port1, NULL); // A1 > > of_graph_get_next_endpoint(port1, A1); // A2 > > of_graph_get_next_endpoint(port1, A2); // NULL > > The idea looks good. My only concern is about reusing the > of_graph_get_next_endpoint() name after having removed the old, different > function having the same name. This can be confusing in the first > place to who is used to the old function, and also to anybody rebasing > their patches on top of a new kernel to find their code behaving > differently. > > Also, as now we'd have two similar variants of this function, it would > be good if each of them were having a name that clearly identifies in > which way they differ from the other. > > So a better name for this function would probably be > of_graph_get_next_port_endpoint() I guess, to clearly differentiate from > of_graph_get_next_device_endpoint(). Yes, Indeed, Thank you for pointing it ! I will update it on v4 Thank you for your help !! Best regards --- Renesas Electronics Ph.D. Kuninori Morimoto
Re: [PATCH v3 14/24] of: property: add of_graph_get_next_endpoint()
Hello Kuninori Morimoto, On Wed, 31 Jan 2024 05:06:36 + Kuninori Morimoto wrote: > To handle endpoint more intuitive, create of_graph_get_next_endpoint() > > of_graph_get_next_endpoint(port1, NULL); // A1 > of_graph_get_next_endpoint(port1, A1); // A2 > of_graph_get_next_endpoint(port1, A2); // NULL The idea looks good. My only concern is about reusing the of_graph_get_next_endpoint() name after having removed the old, different function having the same name. This can be confusing in the first place to who is used to the old function, and also to anybody rebasing their patches on top of a new kernel to find their code behaving differently. Also, as now we'd have two similar variants of this function, it would be good if each of them were having a name that clearly identifies in which way they differ from the other. So a better name for this function would probably be of_graph_get_next_port_endpoint() I guess, to clearly differentiate from of_graph_get_next_device_endpoint(). Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
