On Wed, Jul 27, 2022 at 08:43:15AM +0200, Marcin Wojtas wrote: > fixed-link PHYs API is used by DSA and a number of drivers > and was depending on of_. Switch to fwnode_ so to make it > hardware description agnostic and allow to be used in ACPI > world as well. ... > + /* Old binding */ > + len = fwnode_property_count_u32(fwnode, "fixed-link"); > + if (len == 5) > + return true; > + > + return false; Can be return len == 5; or return fwnode_...(...) == 5; Original also good, so up to you, ... > + if (fwnode_property_read_u32(fixed_link_node, "speed", > + &status.speed)) { > + fwnode_handle_put(fixed_link_node); > + return -EINVAL; > + } Why shadowing actual error code? Either ret = fwnode_...(...); if (ret) { ... return ret; } or add a comment explaining the above magic transformations. ... > + /* Old binding */ > + if (fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop, > + ARRAY_SIZE(fixed_link_prop)) == 0) { > + status.link = 1; > + status.duplex = fixed_link_prop[1]; > + status.speed = fixed_link_prop[2]; > + status.pause = fixed_link_prop[3]; > + status.asym_pause = fixed_link_prop[4]; > + goto register_phy; > + } > + > + return -ENODEV; Ditto. -- With Best Regards, Andy Shevchenko