| Message ID | 20210311062011.8054-1-calvin.johnson@oss.nxp.com |
|---|---|
| Headers | show |
| Series | ACPI support for dpaa2 driver | expand |
On Thu, Mar 11, 2021 at 8:22 AM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > > Introduce a wrapper around the _ADR evaluation. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> > --- > > Changes in v7: None > Changes in v6: None > Changes in v5: > - Replace fwnode_get_id() with acpi_get_local_address() > > Changes in v4: > - Improve code structure to handle all cases > > Changes in v3: > - Modified to retrieve reg property value for ACPI as well > - Resolved compilation issue with CONFIG_ACPI = n > - Added more info into documentation > > Changes in v2: None > > drivers/acpi/utils.c | 14 ++++++++++++++ > include/linux/acpi.h | 7 +++++++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c > index 682edd913b3b..41fe380a09a7 100644 > --- a/drivers/acpi/utils.c > +++ b/drivers/acpi/utils.c > @@ -277,6 +277,20 @@ acpi_evaluate_integer(acpi_handle handle, > > EXPORT_SYMBOL(acpi_evaluate_integer); > > +int acpi_get_local_address(acpi_handle handle, u32 *addr) > +{ > + unsigned long long adr; > + acpi_status status; > + > + status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &adr); > + if (ACPI_FAILURE(status)) > + return -ENODATA; > + > + *addr = (u32)adr; > + return 0; > +} > +EXPORT_SYMBOL(acpi_get_local_address); > + > acpi_status > acpi_evaluate_reference(acpi_handle handle, > acpi_string pathname, > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index fcdaab723916..700f9fc303ab 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -706,6 +706,8 @@ static inline u64 acpi_arch_get_root_pointer(void) > } > #endif > > +int acpi_get_local_address(acpi_handle handle, u32 *addr); > + > #else /* !CONFIG_ACPI */ > > #define acpi_disabled 1 > @@ -953,6 +955,11 @@ static inline struct acpi_device *acpi_resource_consumer(struct resource *res) > return NULL; > } > > +static inline int acpi_get_local_address(acpi_handle handle, u32 *addr) > +{ > + return -ENODEV; > +} > + > #endif /* !CONFIG_ACPI */ > > #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC > -- > 2.17.1 >
On Thu, Mar 11, 2021 at 8:00 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > On Thu, Mar 11, 2021 at 02:09:37PM +0200, Andy Shevchenko wrote: > > On Thu, Mar 11, 2021 at 8:21 AM Calvin Johnson > > <calvin.johnson@oss.nxp.com> wrote: ... > > > +config FWNODE_MDIO > > > + def_tristate PHYLIB > > > > (Seems "selectable only" item) > > What do you mean by "selectable only" item here? Can you please point to some > other example? The Kconfig sections without descriptions are not user-visible. No user can run menuconfig and check a box with "I want this to be compiled". tristate // selectable-only tristate "bla bla bla" // user visible and selectable > > > + depends on ACPI > > > + depends on OF > > > > Wouldn't be better to have > > depends on (ACPI || OF) || COMPILE_TEST > > > > And honestly I don't understand it in either (AND or OR) variant. Why > > do you need a dependency like this for fwnode API? > > Here, fwnode_mdiobus_register_phy() uses objects from both ACPI and OF. APIs? Calls? What really fails if we have !ACPI and / or !OF? > > Moreover dependencies don't work for "selectable only" items. > > > > > + depends on PHYLIB > > > + select FIXED_PHY -- With Best Regards, Andy Shevchenko
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@oss.nxp.com> wrote: > > > > Define acpi_mdiobus_register() to Register mii_bus and create PHYs for > > each ACPI child node. > > > > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> > > --- > > > > 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@oss.nxp.com>"); > > +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
This patch set provides ACPI support to DPAA2 network drivers. It also introduces new fwnode based APIs to support phylink and phy layers Following functions are defined: phylink_fwnode_phy_connect() fwnode_mdiobus_register_phy() fwnode_mdiobus_register() fwnode_get_phy_id() fwnode_phy_find_device() device_phy_find_device() fwnode_get_phy_node() fwnode_mdio_find_device() acpi_get_local_address() First one helps in connecting phy to phylink instance. Next three helps in getting phy_id and registering phy to mdiobus Next two help in finding a phy on a mdiobus. Next one helps in getting phy_node from a fwnode. Last one is used to get local address from _ADR object. Corresponding OF functions are refactored. Tested-on: LX2160ARDB Changes in v7: - correct fwnode_mdio_find_device() description - check NULL in unregister_mii_timestamper() - Call unregister_mii_timestamper() without NULL check - Create fwnode_mdio.c and move fwnode_mdiobus_register_phy() - include fwnode_mdio.h - Include headers directly used in acpi_mdio.c - Move fwnode_mdiobus_register() to fwnode_mdio.c - Include fwnode_mdio.h - Alphabetically sort header inclusions - remove unnecassary checks Changes in v6: - Minor cleanup - fix warning for function parameter of fwnode_mdio_find_device() - Initialize mii_ts to NULL - use GENMASK() and ACPI_COMPANION_SET() - some cleanup - remove unwanted header inclusion - remove OF check for fixed-link - use dev_fwnode() - remove useless else - replace of_device_is_available() to fwnode_device_is_available() Changes in v5: - More cleanup - Replace fwnode_get_id() with acpi_get_local_address() - add missing MODULE_LICENSE() - replace fwnode_get_id() with OF and ACPI function calls - replace fwnode_get_id() with OF and ACPI function calls Changes in v4: - More cleanup - Improve code structure to handle all cases - Remove redundant else from fwnode_mdiobus_register() - Cleanup xgmac_mdio_probe() - call phy_device_free() before returning Changes in v3: - Add more info on legacy DT properties "phy" and "phy-device" - Redefine fwnode_phy_find_device() to follow of_phy_find_device() - Use traditional comparison pattern - Use GENMASK - Modified to retrieve reg property value for ACPI as well - Resolved compilation issue with CONFIG_ACPI = n - Added more info into documentation - Use acpi_mdiobus_register() - Avoid unnecessary line removal - Remove unused inclusion of acpi.h Changes in v2: - Updated with more description in document - use reverse christmas tree ordering for local variables - Refactor OF functions to use fwnode functions Calvin Johnson (16): Documentation: ACPI: DSD: Document MDIO PHY net: phy: Introduce fwnode_mdio_find_device() net: phy: Introduce phy related fwnode functions of: mdio: Refactor of_phy_find_device() net: phy: Introduce fwnode_get_phy_id() of: mdio: Refactor of_get_phy_id() net: mii_timestamper: check NULL in unregister_mii_timestamper() net: mdiobus: Introduce fwnode_mdiobus_register_phy() of: mdio: Refactor of_mdiobus_register_phy() ACPI: utils: Introduce acpi_get_local_address() net: mdio: Add ACPI support code for mdio net: mdiobus: Introduce fwnode_mdiobus_register() net/fsl: Use fwnode_mdiobus_register() net: phylink: introduce phylink_fwnode_phy_connect() net: phylink: Refactor phylink_of_phy_connect() net: dpaa2-mac: Add ACPI support for DPAA2 MAC driver Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++++ MAINTAINERS | 2 + drivers/acpi/utils.c | 14 ++ .../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 84 ++++++----- drivers/net/ethernet/freescale/xgmac_mdio.c | 22 +-- drivers/net/mdio/Kconfig | 16 +++ drivers/net/mdio/Makefile | 4 +- drivers/net/mdio/acpi_mdio.c | 56 ++++++++ drivers/net/mdio/fwnode_mdio.c | 98 +++++++++++++ drivers/net/mdio/of_mdio.c | 80 +---------- drivers/net/phy/mii_timestamper.c | 3 + drivers/net/phy/phy_device.c | 109 +++++++++++++- drivers/net/phy/phylink.c | 41 ++++-- include/linux/acpi.h | 7 + include/linux/acpi_mdio.h | 25 ++++ include/linux/fwnode_mdio.h | 29 ++++ include/linux/of_mdio.h | 6 +- include/linux/phy.h | 31 ++++ include/linux/phylink.h | 3 + 19 files changed, 631 insertions(+), 132 deletions(-) create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst create mode 100644 drivers/net/mdio/acpi_mdio.c create mode 100644 drivers/net/mdio/fwnode_mdio.c create mode 100644 include/linux/acpi_mdio.h create mode 100644 include/linux/fwnode_mdio.h