| Message ID | 20210218052654.28995-11-calvin.johnson@oss.nxp.com (mailing list archive) |
|---|---|
| State | Deferred |
| 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/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 91 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Thu, Feb 18, 2021 at 7:28 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. > +#include <linux/acpi.h> > +#include <linux/acpi_mdio.h> Perhaps it's better to provide the headers that this file is direct user of, i.e. bits.h dev_printk.h module.h types.h The rest seems fine because they are guaranteed to be included by acpi.h (IIUC about fwnode API and acpi_mdio includes MDIO PHY APIs).
On Thu, Feb 18, 2021 at 05:08:05PM +0200, Andy Shevchenko wrote: > On Thu, Feb 18, 2021 at 7:28 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. > > > +#include <linux/acpi.h> > > +#include <linux/acpi_mdio.h> > > Perhaps it's better to provide the headers that this file is direct > user of, i.e. > bits.h > dev_printk.h Looks like device.h needs to be used instead of dev_printk.h. Please let me know if you've a different opinion. > module.h > types.h > > The rest seems fine because they are guaranteed to be included by > acpi.h (IIUC about fwnode API and acpi_mdio includes MDIO PHY APIs). > Thanks Calvin
On Mon, Mar 8, 2021 at 4:11 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > On Thu, Feb 18, 2021 at 05:08:05PM +0200, Andy Shevchenko wrote: > > On Thu, Feb 18, 2021 at 7:28 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. > > > > > +#include <linux/acpi.h> > > > +#include <linux/acpi_mdio.h> > > > > Perhaps it's better to provide the headers that this file is direct > > user of, i.e. > > bits.h > > dev_printk.h > > Looks like device.h needs to be used instead of dev_printk.h. Please > let me know if you've a different opinion. I don't see the user of device.h. dev_printk.h is definitely in use here... Do you see a user for device.h? Which line in your code requires it? It might be that I don't see something quite obvious... > > module.h > > types.h > > > > The rest seems fine because they are guaranteed to be included by > > acpi.h (IIUC about fwnode API and acpi_mdio includes MDIO PHY APIs).
On Mon, Mar 08, 2021 at 04:57:35PM +0200, Andy Shevchenko wrote: > On Mon, Mar 8, 2021 at 4:11 PM Calvin Johnson > <calvin.johnson@oss.nxp.com> wrote: > > On Thu, Feb 18, 2021 at 05:08:05PM +0200, Andy Shevchenko wrote: > > > On Thu, Feb 18, 2021 at 7:28 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. > > > > > > > +#include <linux/acpi.h> > > > > +#include <linux/acpi_mdio.h> > > > > > > Perhaps it's better to provide the headers that this file is direct > > > user of, i.e. > > > bits.h > > > dev_printk.h > > > > Looks like device.h needs to be used instead of dev_printk.h. Please > > let me know if you've a different opinion. > > I don't see the user of device.h. dev_printk.h is definitely in use here... > Do you see a user for device.h? Which line in your code requires it? > > It might be that I don't see something quite obvious... I thought of including device.h instead of dev_printk.h because, it is the only file that includes dev_printk.h and device.h is widely used. Of course, it will mean that dev_printk.h is indirectly called. Regards Calvin
On Mon, Mar 8, 2021 at 6:28 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > On Mon, Mar 08, 2021 at 04:57:35PM +0200, Andy Shevchenko wrote: .... > I thought of including device.h instead of dev_printk.h because, it is the > only file that includes dev_printk.h and device.h is widely used. Of course, > it will mean that dev_printk.h is indirectly called. The split happened recently, not every developer knows about it and definitely most of the contributors are too lazy to properly write the inclusion block in their code.
diff --git a/MAINTAINERS b/MAINTAINERS index 1a219335efe0..41d16d77b6cf 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/of_mdio.c F: drivers/net/pcs/ F: drivers/net/phy/ diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig index a10cc460d7cf..df6bb7837d6a 100644 --- a/drivers/net/mdio/Kconfig +++ b/drivers/net/mdio/Kconfig @@ -27,6 +27,13 @@ config OF_MDIO help OpenFirmware MDIO bus (Ethernet PHY) accessors +config ACPI_MDIO + def_tristate PHYLIB + depends on ACPI + depends on PHYLIB + 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 5c498dde463f..2373ade8af13 100644 --- a/drivers/net/mdio/Makefile +++ b/drivers/net/mdio/Makefile @@ -2,6 +2,7 @@ # Makefile for Linux MDIO bus drivers obj-$(CONFIG_OF_MDIO) += of_mdio.o +obj-$(CONFIG_ACPI_MDIO) += acpi_mdio.o obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o diff --git a/drivers/net/mdio/acpi_mdio.c b/drivers/net/mdio/acpi_mdio.c new file mode 100644 index 000000000000..091c8272e596 --- /dev/null +++ b/drivers/net/mdio/acpi_mdio.c @@ -0,0 +1,51 @@ +// 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> + +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 */
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 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 | 51 ++++++++++++++++++++++++++++++++++++ include/linux/acpi_mdio.h | 25 ++++++++++++++++++ 5 files changed, 85 insertions(+) create mode 100644 drivers/net/mdio/acpi_mdio.c create mode 100644 include/linux/acpi_mdio.h