On Thu, Mar 11, 2021 at 02:14:37PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 11, 2021 at 8:22 AM Calvin Johnson
> <calvin.johnson@xxxxxxxxxxx> wrote:
> >
> > Define acpi_mdiobus_register() to Register mii_bus and create PHYs for
> > each ACPI child node.
> >
> > Signed-off-by: Calvin Johnson <calvin.johnson@xxxxxxxxxxx>
> > ---
> >
> > Changes in v7:
> > - Include headers directly used in acpi_mdio.c
> >
> > Changes in v6:
> > - use GENMASK() and ACPI_COMPANION_SET()
> > - some cleanup
> > - remove unwanted header inclusion
> >
> > Changes in v5:
> > - add missing MODULE_LICENSE()
> > - replace fwnode_get_id() with OF and ACPI function calls
> >
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> >
> > MAINTAINERS | 1 +
> > drivers/net/mdio/Kconfig | 7 +++++
> > drivers/net/mdio/Makefile | 1 +
> > drivers/net/mdio/acpi_mdio.c | 56 ++++++++++++++++++++++++++++++++++++
> > include/linux/acpi_mdio.h | 25 ++++++++++++++++
> > 5 files changed, 90 insertions(+)
> > create mode 100644 drivers/net/mdio/acpi_mdio.c
> > create mode 100644 include/linux/acpi_mdio.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 146de41d2656..051377b7fa94 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6680,6 +6680,7 @@ F: Documentation/devicetree/bindings/net/mdio*
> > F: Documentation/devicetree/bindings/net/qca,ar803x.yaml
> > F: Documentation/networking/phy.rst
> > F: drivers/net/mdio/
> > +F: drivers/net/mdio/acpi_mdio.c
> > F: drivers/net/mdio/fwnode_mdio.c
> > F: drivers/net/mdio/of_mdio.c
> > F: drivers/net/pcs/
> > diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
> > index 2d5bf5ccffb5..fc8c787b448f 100644
> > --- a/drivers/net/mdio/Kconfig
> > +++ b/drivers/net/mdio/Kconfig
> > @@ -36,6 +36,13 @@ config OF_MDIO
> > help
> > OpenFirmware MDIO bus (Ethernet PHY) accessors
> >
> > +config ACPI_MDIO
> > + def_tristate PHYLIB
>
> > + depends on ACPI
> > + depends on PHYLIB
>
> Same issue, they are no-ops.
>
> I guess you have to surround OF and ACPI and FWNODE variants by
>
> if PHYLIB
> ...
> endif
>
> This will be an equivalent to depends on PHYLIB
>
> and put this into Makefile
>
> ifneq ($(CONFIG_ACPI),)
> obj-$(CONFIG_PHYLIB) += acpi_mdio.o
> endif
>
> This will give you the rest, i.e. default PHYLIB + depends on ACPI
>
> Similar for OF
>
I checked the effect of y/n/m choice of PHYLIB on FWNODE_MDIO.
As expected with def_tristate, whenever PHYLIB changes to y/n/m corresponding
change is reflected on FWNODE_MDIO, also considering the state of other
depending CONFIGS like OF and ACPI.
Symbol: FWNODE_MDIO [=n]
│
│ Type : tristate
│
│ Defined at drivers/net/mdio/Kconfig:22
│
│ Depends on: NETDEVICES [=y] && MDIO_DEVICE [=y] && ACPI [=y] && OF [=y] &&
PHYLIB [=n] │
│ Selects: FIXED_PHY [=n]
Effect is similar for ACPI_MDIO and OF_MDIO.
So instead of above proposed method, I think what you proposed in your earlier
email, i.e, "depends on (ACPI || OF) || COMPILE_TEST" seems to be better for
FWNODE_MDIO.
Shall we go ahead with this?
Regards
Calvin
> > + help
> > + ACPI MDIO bus (Ethernet PHY) accessors
> > +
> > if MDIO_BUS
> >
> > config MDIO_DEVRES
> > diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
> > index ea5390e2ef84..e8b739a3df1c 100644
> > --- a/drivers/net/mdio/Makefile
> > +++ b/drivers/net/mdio/Makefile
> > @@ -1,6 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0
> > # Makefile for Linux MDIO bus drivers
> >
> > +obj-$(CONFIG_ACPI_MDIO) += acpi_mdio.o
> > obj-$(CONFIG_FWNODE_MDIO) += fwnode_mdio.o
> > obj-$(CONFIG_OF_MDIO) += of_mdio.o
> >
> > diff --git a/drivers/net/mdio/acpi_mdio.c b/drivers/net/mdio/acpi_mdio.c
> > new file mode 100644
> > index 000000000000..60a86e3fc246
> > --- /dev/null
> > +++ b/drivers/net/mdio/acpi_mdio.c
> > @@ -0,0 +1,56 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ACPI helpers for the MDIO (Ethernet PHY) API
> > + *
> > + * This file provides helper functions for extracting PHY device information
> > + * out of the ACPI ASL and using it to populate an mii_bus.
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/acpi_mdio.h>
> > +#include <linux/bits.h>
> > +#include <linux/dev_printk.h>
> > +#include <linux/fwnode_mdio.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +
> > +MODULE_AUTHOR("Calvin Johnson <calvin.johnson@xxxxxxxxxxx>");
> > +MODULE_LICENSE("GPL");
> > +
> > +/**
> > + * acpi_mdiobus_register - Register mii_bus and create PHYs from the ACPI ASL.
> > + * @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 acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode)
> > +{
> > + struct fwnode_handle *child;
> > + u32 addr;
> > + int ret;
> > +
> > + /* Mask out all PHYs from auto probing. */
> > + mdio->phy_mask = GENMASK(31, 0);
> > + ret = mdiobus_register(mdio);
> > + if (ret)
> > + return ret;
> > +
> > + ACPI_COMPANION_SET(&mdio->dev, to_acpi_device_node(fwnode));
> > +
> > + /* Loop over the child nodes and register a phy_device for each PHY */
> > + fwnode_for_each_child_node(fwnode, child) {
> > + ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > + if (ret || addr >= PHY_MAX_ADDR)
> > + continue;
> > +
> > + ret = fwnode_mdiobus_register_phy(mdio, child, addr);
> > + if (ret == -ENODEV)
> > + dev_err(&mdio->dev,
> > + "MDIO device at address %d is missing.\n",
> > + addr);
> > + }
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(acpi_mdiobus_register);
> > diff --git a/include/linux/acpi_mdio.h b/include/linux/acpi_mdio.h
> > new file mode 100644
> > index 000000000000..748d261fe2f9
> > --- /dev/null
> > +++ b/include/linux/acpi_mdio.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * ACPI helper for the MDIO (Ethernet PHY) API
> > + */
> > +
> > +#ifndef __LINUX_ACPI_MDIO_H
> > +#define __LINUX_ACPI_MDIO_H
> > +
> > +#include <linux/phy.h>
> > +
> > +#if IS_ENABLED(CONFIG_ACPI_MDIO)
> > +int acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode);
> > +#else /* CONFIG_ACPI_MDIO */
> > +static inline int acpi_mdiobus_register(struct mii_bus *mdio, 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
> > +
> > +#endif /* __LINUX_ACPI_MDIO_H */
> > --
> > 2.17.1
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
