Messages in this thread | | | From | Marcin Wojtas <> | Date | Thu, 24 Jun 2021 00:10:28 +0200 | Subject | Re: [net-next: PATCH v3 2/6] net: mdiobus: Introduce fwnode_mdbiobus_register() |
| |
Hi,
śr., 23 cze 2021 o 22:22 Andrew Lunn <andrew@lunn.ch> napisał(a): > > On Mon, Jun 21, 2021 at 07:30:24PM +0200, Marcin Wojtas wrote: > > This patch introduces a new helper function that > > wraps acpi_/of_ mdiobus_register() and allows its > > usage via common fwnode_ interface. > > > > Fall back to raw mdiobus_register() in case CONFIG_FWNODE_MDIO > > is not enabled, in order to satisfy compatibility > > in all future user drivers. > > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > --- > > include/linux/fwnode_mdio.h | 12 +++++++++++ > > drivers/net/mdio/fwnode_mdio.c | 22 ++++++++++++++++++++ > > 2 files changed, 34 insertions(+) > > > > diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h > > index faf603c48c86..13d4ae8fee0a 100644 > > --- a/include/linux/fwnode_mdio.h > > +++ b/include/linux/fwnode_mdio.h > > @@ -16,6 +16,7 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, > > int fwnode_mdiobus_register_phy(struct mii_bus *bus, > > struct fwnode_handle *child, u32 addr); > > > > +int fwnode_mdiobus_register(struct mii_bus *bus, struct fwnode_handle *fwnode); > > #else /* CONFIG_FWNODE_MDIO */ > > int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, > > struct phy_device *phy, > > @@ -30,6 +31,17 @@ static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus, > > { > > return -EINVAL; > > } > > + > > +static inline int fwnode_mdiobus_register(struct mii_bus *bus, > > + struct fwnode_handle *fwnode) > > +{ > > + /* > > + * Fall back to mdiobus_register() function to register a bus. > > + * This way, we don't have to keep compat bits around in drivers. > > + */ > > + > > + return mdiobus_register(mdio); > > +} > > #endif > > I looked at this some more, and in the end i decided it was O.K. > > > +/** > > + * fwnode_mdiobus_register - bring up all the PHYs on a given MDIO bus and > > + * attach them to it. > > + * @bus: Target MDIO bus. > > + * @fwnode: Pointer to fwnode of the MDIO controller. > > + * > > + * Return values are determined accordingly to acpi_/of_ mdiobus_register() > > + * operation. > > + */ > > +int fwnode_mdiobus_register(struct mii_bus *bus, struct fwnode_handle *fwnode) > > +{ > > + if (is_acpi_node(fwnode)) > > + return acpi_mdiobus_register(bus, fwnode); > > + else if (is_of_node(fwnode)) > > + return of_mdiobus_register(bus, to_of_node(fwnode)); > > + else > > + return -EINVAL; > > I wounder if here you should call mdiobus_register(mdio), rather than > -EINVAL? > > I don't have a strong opinion.
Currently (and in foreseeable future) we support only DT/ACPI as a firmware description, reaching the last "else" means something really wrong. The case of lack of DT/ACPI and the fallback is handled on the include/linux/fwnode_mdio.h level.
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch> >
Thanks, Marcin
| |