On Fri, Oct 02, 2020 at 12:05:14PM +0100, Grant Likely wrote: > > > On 30/09/2020 17:04, Calvin Johnson wrote: > > Extract phy_id from compatible string. This will be used by > > fwnode_mdiobus_register_phy() to create phy device using the > > phy_id. > > > > Signed-off-by: Calvin Johnson <calvin.johnson@xxxxxxxxxxx> > > --- > > > > drivers/net/phy/phy_device.c | 32 +++++++++++++++++++++++++++++++- > > include/linux/phy.h | 5 +++++ > > 2 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > > index c4aec56d0a95..162abde6223d 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -9,6 +9,7 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <linux/acpi.h> > > #include <linux/bitmap.h> > > #include <linux/delay.h> > > #include <linux/errno.h> > > @@ -845,6 +846,27 @@ static int get_phy_c22_id(struct mii_bus *bus, int addr, u32 *phy_id) > > return 0; > > } > > +/* Extract the phy ID from the compatible string of the form > > + * ethernet-phy-idAAAA.BBBB. > > + */ > > +int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id) > > +{ > > + unsigned int upper, lower; > > + const char *cp; > > + int ret; > > + > > + ret = fwnode_property_read_string(fwnode, "compatible", &cp); > > + if (ret) > > + return ret; > > + > > + if (sscanf(cp, "ethernet-phy-id%4x.%4x", &upper, &lower) == 2) { > > + *phy_id = ((upper & 0xFFFF) << 16) | (lower & 0xFFFF); > > + return 0; > > + } > > + return -EINVAL; > > +} > > +EXPORT_SYMBOL(fwnode_get_phy_id); > > This block, and the changes in patch 4 duplicate functions from > drivers/of/of_mdio.c, but it doesn't refactor anything in > drivers/of/of_mdio.c to use the new path. Is your intent to bring all of the > parsing in these functions of "compatible" into the ACPI code path? > > If so, then the existing code path needs to be refactored to work with > fwnode_handle instead of device_node. > > If not, then the DT path in these functions should call out to of_mdio, > while the ACPI path only does what is necessary. I'll work on refactoring as Florian and Rob are also suggesting the same. Thanks Calvin