On Fri, May 31, 2024 at 11:19:26AM +0300, Andy Shevchenko wrote: > On Fri, May 31, 2024 at 11:01 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Thu, May 30, 2024 at 04:36:59PM +0300, Andy Shevchenko wrote: > > > On Thu, May 30, 2024 at 2:52 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > > On Thu, May 30, 2024 at 11:26 AM Andy Shevchenko > > > > <andy.shevchenko@xxxxxxxxx> wrote: > > > > > Thu, May 30, 2024 at 11:19:29AM +0200, Geert Uytterhoeven kirjoitti: > > > > > > Use the scoped variant of for_each_child_of_node() to simplify the code. > > > > > > > > > > I do not see the point of this patch. This makes code actually more > > > > > complicated, and I'm not sure the code generation is the same and not worse. > > > > > > > > On arm32, a conversion to for_each_child_of_node_scoped() seems to > > > > cost ca. 48 bytes of additional code. > > > > > > > > BTW, the same is true for cases where the conversion does simplify > > > > cleanup. > > > > > > > > I checked "pinctrl: renesas: Use scope based of_node_put() cleanups", > > > > and all but the conversions in *_dt_node_to_map() cost 48 bytes each. > > > > > > Yeah. so for the cases where there are no returns from inside the loop > > > I prefer not to use _scoped. > > > > Eventually _scoped() loops will become the norm. Leaving some unscoped > > loops will be a fun surprise for the first person to introduce a return > > -EINVAL. > > It makes no sense when we have no return / goto semantics from inside > of the loop. I don't know why we should do worse binary code for no > benefit. The compiler ought to be able to determine when the cleanup function is not required and save those 48 bytes. That's why we have NULL checking in __free_device_node() instead of using the NULL check in of_node_put(). The compiler is already removing all the calls from the return statements where p is NULL. It seems like a small thing to one step further and delete the cleanup when it's not called on any path? regards, dan carpenter
