Message ID | 20201215164315.3666-9-calvin.johnson@oss.nxp.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | ACPI support for dpaa2 driver | expand |
Context | Check | Description |
---|---|---|
netdev/cover_letter | success | Link |
netdev/fixes_present | success | Link |
netdev/patch_count | success | Link |
netdev/tree_selection | success | Clearly marked for net-next |
netdev/subject_prefix | success | Link |
netdev/source_inline | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Link |
netdev/module_param | success | Was 0 now: 0 |
netdev/build_32bit | fail | Errors and warnings before: 620 this patch: 625 |
netdev/kdoc | success | Errors and warnings before: 1 this patch: 1 |
netdev/verify_fixes | success | Link |
netdev/checkpatch | warning | WARNING: line length of 81 exceeds 80 columns |
netdev/build_allmodconfig_warn | fail | Errors and warnings before: 536 this patch: 541 |
netdev/header_inline | success | Link |
netdev/stable | success | Stable not CCed |
On Tue, Dec 15, 2020 at 6:44 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > > Introduce fwnode_mdiobus_register() to register PHYs on the mdiobus. > If the fwnode is DT node, then call of_mdiobus_register(). > If it is an ACPI node, then: > - disable auto probing of mdiobus > - register mdiobus > - save fwnode to mdio structure > - loop over child nodes & register a phy_device for each PHY ... > +/** > + * fwnode_mdiobus_register - Register mii_bus and create PHYs from fwnode > + * @mdio: pointer to mii_bus structure > + * @fwnode: pointer to fwnode of MDIO bus. > + * > + * This function registers the mii_bus structure and registers a phy_device > + * for each child node of @fwnode. > + */ > +int fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode) > +{ > + struct fwnode_handle *child; > + unsigned long long addr; > + acpi_status status; > + int ret; > + > + if (is_of_node(fwnode)) { > + return of_mdiobus_register(mdio, to_of_node(fwnode)); > + } else if (is_acpi_node(fwnode)) { I would rather see this as simple as if (is_of_node(fwnode)) return of_mdiobus_register(mdio, to_of_node(fwnode)); if (is_acpi_node(fwnode)) return acpi_mdiobus_register(mdio, fwnode); where the latter one is defined somewhere in drivers/acpi/. > + /* Mask out all PHYs from auto probing. */ > + mdio->phy_mask = ~0; > + ret = mdiobus_register(mdio); > + if (ret) > + return ret; > + > + mdio->dev.fwnode = fwnode; > + /* Loop over the child nodes and register a phy_device for each PHY */ > + fwnode_for_each_child_node(fwnode, child) { > + status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(child), > + "_ADR", NULL, &addr); > + if (ACPI_FAILURE(status)) { Isn't it fwnode_get_id() now? > + pr_debug("_ADR returned %d\n", status); > + continue; > + } > + if (addr < 0 || addr >= PHY_MAX_ADDR) > + continue; addr can't be less than 0. > + ret = fwnode_mdiobus_register_phy(mdio, child, addr); > + if (ret == -ENODEV) > + dev_err(&mdio->dev, > + "MDIO device at address %lld is missing.\n", > + addr); > + } > + return 0; > + } > + return -EINVAL; > +}
On Tue, Dec 15, 2020 at 07:53:26PM +0200, Andy Shevchenko wrote: > On Tue, Dec 15, 2020 at 6:44 PM Calvin Johnson > <calvin.johnson@oss.nxp.com> wrote: > > > > Introduce fwnode_mdiobus_register() to register PHYs on the mdiobus. > > If the fwnode is DT node, then call of_mdiobus_register(). > > If it is an ACPI node, then: > > - disable auto probing of mdiobus > > - register mdiobus > > - save fwnode to mdio structure > > - loop over child nodes & register a phy_device for each PHY > > ... > > > +/** > > + * fwnode_mdiobus_register - Register mii_bus and create PHYs from fwnode > > + * @mdio: pointer to mii_bus structure > > + * @fwnode: pointer to fwnode of MDIO bus. > > + * > > + * This function registers the mii_bus structure and registers a phy_device > > + * for each child node of @fwnode. > > + */ > > +int fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode) > > +{ > > + struct fwnode_handle *child; > > + unsigned long long addr; > > + acpi_status status; > > + int ret; > > + > > + if (is_of_node(fwnode)) { > > + return of_mdiobus_register(mdio, to_of_node(fwnode)); > > + } else if (is_acpi_node(fwnode)) { > > I would rather see this as simple as > > if (is_of_node(fwnode)) > return of_mdiobus_register(mdio, to_of_node(fwnode)); > if (is_acpi_node(fwnode)) > return acpi_mdiobus_register(mdio, fwnode); > > where the latter one is defined somewhere in drivers/acpi/. Makes sense. I'll do it. But I think it will be better to place acpi_mdiobus_register() here itself in the network subsystem, maybe /drivers/net/mdio/acpi_mdio.c. > > > + /* Mask out all PHYs from auto probing. */ > > + mdio->phy_mask = ~0; > > + ret = mdiobus_register(mdio); > > + if (ret) > > + return ret; > > + > > + mdio->dev.fwnode = fwnode; > > + /* Loop over the child nodes and register a phy_device for each PHY */ > > + fwnode_for_each_child_node(fwnode, child) { > > > + status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(child), > > + "_ADR", NULL, &addr); > > + if (ACPI_FAILURE(status)) { > > Isn't it fwnode_get_id() now? Yes. Will change it. > > > + pr_debug("_ADR returned %d\n", status); > > + continue; > > + } > > > + if (addr < 0 || addr >= PHY_MAX_ADDR) > > + continue; > > addr can't be less than 0. Yes. will update in v3. > > > + ret = fwnode_mdiobus_register_phy(mdio, child, addr); > > + if (ret == -ENODEV) > > + dev_err(&mdio->dev, > > + "MDIO device at address %lld is missing.\n", > > + addr); > > + } > > + return 0; > > + } > > + return -EINVAL; > > +} > > -- > With Best Regards, > Andy Shevchenko
On Fri, Dec 18, 2020 at 7:40 AM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > On Tue, Dec 15, 2020 at 07:53:26PM +0200, Andy Shevchenko wrote: > > On Tue, Dec 15, 2020 at 6:44 PM Calvin Johnson > > <calvin.johnson@oss.nxp.com> wrote: ... > > I would rather see this as simple as > > > > if (is_of_node(fwnode)) > > return of_mdiobus_register(mdio, to_of_node(fwnode)); > > if (is_acpi_node(fwnode)) > > return acpi_mdiobus_register(mdio, fwnode); > > > > where the latter one is defined somewhere in drivers/acpi/. > Makes sense. I'll do it. But I think it will be better to place > acpi_mdiobus_register() here itself in the network subsystem, maybe > /drivers/net/mdio/acpi_mdio.c. Even better, thanks!
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 3361a1a86e97..e7ad34908936 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -8,6 +8,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/acpi.h> #include <linux/delay.h> #include <linux/device.h> #include <linux/errno.h> @@ -567,6 +568,55 @@ static int mdiobus_create_device(struct mii_bus *bus, return ret; } +/** + * fwnode_mdiobus_register - Register mii_bus and create PHYs from fwnode + * @mdio: pointer to mii_bus structure + * @fwnode: pointer to fwnode of MDIO bus. + * + * This function registers the mii_bus structure and registers a phy_device + * for each child node of @fwnode. + */ +int fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode) +{ + struct fwnode_handle *child; + unsigned long long addr; + acpi_status status; + int ret; + + if (is_of_node(fwnode)) { + return of_mdiobus_register(mdio, to_of_node(fwnode)); + } else if (is_acpi_node(fwnode)) { + /* Mask out all PHYs from auto probing. */ + mdio->phy_mask = ~0; + ret = mdiobus_register(mdio); + if (ret) + return ret; + + mdio->dev.fwnode = fwnode; + /* Loop over the child nodes and register a phy_device for each PHY */ + fwnode_for_each_child_node(fwnode, child) { + status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(child), + "_ADR", NULL, &addr); + if (ACPI_FAILURE(status)) { + pr_debug("_ADR returned %d\n", status); + continue; + } + + if (addr < 0 || addr >= PHY_MAX_ADDR) + continue; + + ret = fwnode_mdiobus_register_phy(mdio, child, addr); + if (ret == -ENODEV) + dev_err(&mdio->dev, + "MDIO device at address %lld is missing.\n", + addr); + } + return 0; + } + return -EINVAL; +} +EXPORT_SYMBOL(fwnode_mdiobus_register); + /** * __mdiobus_register - bring up all the PHYs on a given bus and attach them to bus * @bus: target mii_bus diff --git a/include/linux/phy.h b/include/linux/phy.h index 10a66b65a008..67ea4ca6f76f 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -383,6 +383,7 @@ static inline struct mii_bus *mdiobus_alloc(void) return mdiobus_alloc_size(0); } +int fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode); int __mdiobus_register(struct mii_bus *bus, struct module *owner); int __devm_mdiobus_register(struct device *dev, struct mii_bus *bus, struct module *owner);
Introduce fwnode_mdiobus_register() to register PHYs on the mdiobus. If the fwnode is DT node, then call of_mdiobus_register(). If it is an ACPI node, then: - disable auto probing of mdiobus - register mdiobus - save fwnode to mdio structure - loop over child nodes & register a phy_device for each PHY Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> --- Changes in v2: None drivers/net/phy/mdio_bus.c | 50 ++++++++++++++++++++++++++++++++++++++ include/linux/phy.h | 1 + 2 files changed, 51 insertions(+)