On Tue, Jan 30, 2024 at 09:37:42AM +0200, Tomi Valkeinen wrote: > On 30/01/2024 09:31, Sakari Ailus wrote: > > Hi Morimoto-san, > > > > On Tue, Jan 30, 2024 at 12:34:55AM +0000, Kuninori Morimoto wrote: > > > > > > Hi Laurent, Sakari > > > > > > Thank you for your review > > > > > > > > The strategy sounds good to me. However, I'm wondering if you shouldn't > > > > > take one more step in the core, and implement these as fwnode > > > > > operations. Or is there a reason why OF is special, and iterating over > > > > > ports would be useful for drivers on OF systems but not on other types > > > > > of systems ? > > > > > > > > I'd prefer that, too. > > > > > > It is very easy reason, because I'm not fwnode user ;P > > > I'm not familiar with fwnode, but in my quick check, it seems it is easy > > > to expand fwnode side functions if of_graph side function exist ? > > > > That would be one way to do that, yes, but I suggested using the existing > > endpoint iterators as that would keep the firmware specific implementation > > more simple. The (slight) drawback is that for each node returned, you'd > > need to check its parent (i.e. port node) is the same as the port you're > > interested in. The alternative may involve reworking the struct > > fwnode_operations interface somewhat, including swnode, DT and ACPI > > implementations. > > > > But we still need the of_* versions, don't we, for patches 4 to 13? Yes, my comment was indeed about the fwnode property API only. -- Sakari Ailus