From: Ioana Ciornei <[email protected]>
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_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 v8:
- fixed some checkpatch warnings/checks
- included linux/fwnode_mdio.h in fwnode_mdio.c (fixed the build warnings)
- added fwnode_find_mii_timestamper() and
fwnode_mdiobus_phy_device_register() in order to get rid of the cycle
dependency.
- change to 'depends on (ACPI || OF) || COMPILE_TEST (for FWNODE_MDIO)
- remove the fwnode_mdiobus_register from fwnode_mdio.c since it
introduces a cycle of dependencies.
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 (15):
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/fsl: Use [acpi|of]_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 | 88 ++++++-----
.../net/ethernet/freescale/dpaa2/dpaa2-mac.h | 2 +-
drivers/net/ethernet/freescale/xgmac_mdio.c | 30 ++--
drivers/net/mdio/Kconfig | 14 ++
drivers/net/mdio/Makefile | 4 +-
drivers/net/mdio/acpi_mdio.c | 56 +++++++
drivers/net/mdio/fwnode_mdio.c | 144 ++++++++++++++++++
drivers/net/mdio/of_mdio.c | 138 ++---------------
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 | 26 ++++
include/linux/fwnode_mdio.h | 35 +++++
include/linux/phy.h | 32 ++++
include/linux/phylink.h | 3 +
19 files changed, 691 insertions(+), 190 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
--
2.31.1
From: Calvin Johnson <[email protected]>
Introduce ACPI mechanism to get PHYs registered on a MDIO bus and
provide them to be connected to MAC.
Describe properties "phy-handle" and "phy-mode".
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7: None
Changes in v6:
- Minor cleanup
Changes in v5:
- More cleanup
Changes in v4:
- More cleanup
Changes in v3: None
Changes in v2:
- Updated with more description in document
Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++++
1 file changed, 133 insertions(+)
create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
diff --git a/Documentation/firmware-guide/acpi/dsd/phy.rst b/Documentation/firmware-guide/acpi/dsd/phy.rst
new file mode 100644
index 000000000000..7d01ae8b3cc6
--- /dev/null
+++ b/Documentation/firmware-guide/acpi/dsd/phy.rst
@@ -0,0 +1,133 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+MDIO bus and PHYs in ACPI
+=========================
+
+The PHYs on an MDIO bus [1] are probed and registered using
+fwnode_mdiobus_register_phy().
+
+Later, for connecting these PHYs to their respective MACs, the PHYs registered
+on the MDIO bus have to be referenced.
+
+This document introduces two _DSD properties that are to be used
+for connecting PHYs on the MDIO bus [3] to the MAC layer.
+
+These properties are defined in accordance with the "Device
+Properties UUID For _DSD" [2] document and the
+daffd814-6eba-4d8c-8a91-bc9bbf4aa301 UUID must be used in the Device
+Data Descriptors containing them.
+
+phy-handle
+----------
+For each MAC node, a device property "phy-handle" is used to reference
+the PHY that is registered on an MDIO bus. This is mandatory for
+network interfaces that have PHYs connected to MAC via MDIO bus.
+
+During the MDIO bus driver initialization, PHYs on this bus are probed
+using the _ADR object as shown below and are registered on the MDIO bus.
+
+::
+ Scope(\_SB.MDI0)
+ {
+ Device(PHY1) {
+ Name (_ADR, 0x1)
+ } // end of PHY1
+
+ Device(PHY2) {
+ Name (_ADR, 0x2)
+ } // end of PHY2
+ }
+
+Later, during the MAC driver initialization, the registered PHY devices
+have to be retrieved from the MDIO bus. For this, the MAC driver needs
+references to the previously registered PHYs which are provided
+as device object references (e.g. \_SB.MDI0.PHY1).
+
+phy-mode
+--------
+The "phy-mode" _DSD property is used to describe the connection to
+the PHY. The valid values for "phy-mode" are defined in [4].
+
+The following ASL example illustrates the usage of these properties.
+
+DSDT entry for MDIO node
+------------------------
+
+The MDIO bus has an SoC component (MDIO controller) and a platform
+component (PHYs on the MDIO bus).
+
+a) Silicon Component
+This node describes the MDIO controller, MDI0
+---------------------------------------------
+::
+ Scope(_SB)
+ {
+ Device(MDI0) {
+ Name(_HID, "NXP0006")
+ Name(_CCA, 1)
+ Name(_UID, 0)
+ Name(_CRS, ResourceTemplate() {
+ Memory32Fixed(ReadWrite, MDI0_BASE, MDI_LEN)
+ Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
+ {
+ MDI0_IT
+ }
+ }) // end of _CRS for MDI0
+ } // end of MDI0
+ }
+
+b) Platform Component
+The PHY1 and PHY2 nodes represent the PHYs connected to MDIO bus MDI0
+---------------------------------------------------------------------
+::
+ Scope(\_SB.MDI0)
+ {
+ Device(PHY1) {
+ Name (_ADR, 0x1)
+ } // end of PHY1
+
+ Device(PHY2) {
+ Name (_ADR, 0x2)
+ } // end of PHY2
+ }
+
+DSDT entries representing MAC nodes
+-----------------------------------
+
+Below are the MAC nodes where PHY nodes are referenced.
+phy-mode and phy-handle are used as explained earlier.
+------------------------------------------------------
+::
+ Scope(\_SB.MCE0.PR17)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package (2) {"phy-mode", "rgmii-id"},
+ Package (2) {"phy-handle", \_SB.MDI0.PHY1}
+ }
+ })
+ }
+
+ Scope(\_SB.MCE0.PR18)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package (2) {"phy-mode", "rgmii-id"},
+ Package (2) {"phy-handle", \_SB.MDI0.PHY2}}
+ }
+ })
+ }
+
+References
+==========
+
+[1] Documentation/networking/phy.rst
+
+[2] https://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
+
+[3] Documentation/firmware-guide/acpi/DSD-properties-rules.rst
+
+[4] Documentation/devicetree/bindings/net/ethernet-controller.yaml
--
2.31.1
From: Calvin Johnson <[email protected]>
Refactor of_phy_find_device() to use fwnode_phy_find_device().
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/net/mdio/of_mdio.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 6ef8b6e40189..0ba1158796d9 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -360,18 +360,7 @@ EXPORT_SYMBOL(of_mdio_find_device);
*/
struct phy_device *of_phy_find_device(struct device_node *phy_np)
{
- struct mdio_device *mdiodev;
-
- mdiodev = of_mdio_find_device(phy_np);
- if (!mdiodev)
- return NULL;
-
- if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
- return to_phy_device(&mdiodev->dev);
-
- put_device(&mdiodev->dev);
-
- return NULL;
+ return fwnode_phy_find_device(of_fwnode_handle(phy_np));
}
EXPORT_SYMBOL(of_phy_find_device);
--
2.31.1
From: Calvin Johnson <[email protected]>
With the introduction of fwnode_get_phy_id(), refactor of_get_phy_id()
to use fwnode equivalent.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/net/mdio/of_mdio.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 0ba1158796d9..29f121cba314 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -29,17 +29,7 @@ MODULE_LICENSE("GPL");
* ethernet-phy-idAAAA.BBBB */
static int of_get_phy_id(struct device_node *device, u32 *phy_id)
{
- struct property *prop;
- const char *cp;
- unsigned int upper, lower;
-
- of_property_for_each_string(device, "compatible", prop, cp) {
- if (sscanf(cp, "ethernet-phy-id%4x.%4x", &upper, &lower) == 2) {
- *phy_id = ((upper & 0xFFFF) << 16) | (lower & 0xFFFF);
- return 0;
- }
- }
- return -EINVAL;
+ return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
}
static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
--
2.31.1
From: Calvin Johnson <[email protected]>
Define fwnode_mdio_find_device() to get a pointer to the
mdio_device from fwnode passed to the function.
Refactor of_mdio_find_device() to use fwnode_mdio_find_device().
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7:
- correct fwnode_mdio_find_device() description
Changes in v6:
- fix warning for function parameter of fwnode_mdio_find_device()
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/net/mdio/of_mdio.c | 11 +----------
drivers/net/phy/phy_device.c | 23 +++++++++++++++++++++++
include/linux/phy.h | 7 +++++++
3 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 8e97d5b825f5..6ef8b6e40189 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -347,16 +347,7 @@ EXPORT_SYMBOL(of_mdiobus_register);
*/
struct mdio_device *of_mdio_find_device(struct device_node *np)
{
- struct device *d;
-
- if (!np)
- return NULL;
-
- d = bus_find_device_by_of_node(&mdio_bus_type, np);
- if (!d)
- return NULL;
-
- return to_mdio_device(d);
+ return fwnode_mdio_find_device(of_fwnode_handle(np));
}
EXPORT_SYMBOL(of_mdio_find_device);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1539ea021ac0..363cc70d00ca 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2863,6 +2863,29 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
return phydrv->config_intr && phydrv->handle_interrupt;
}
+/**
+ * fwnode_mdio_find_device - Given a fwnode, find the mdio_device
+ * @fwnode: pointer to the mdio_device's fwnode
+ *
+ * If successful, returns a pointer to the mdio_device with the embedded
+ * struct device refcount incremented by one, or NULL on failure.
+ * The caller should call put_device() on the mdio_device after its use.
+ */
+struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode)
+{
+ struct device *d;
+
+ if (!fwnode)
+ return NULL;
+
+ d = bus_find_device_by_fwnode(&mdio_bus_type, fwnode);
+ if (!d)
+ return NULL;
+
+ return to_mdio_device(d);
+}
+EXPORT_SYMBOL(fwnode_mdio_find_device);
+
/**
* phy_probe - probe and init a PHY device
* @dev: device to probe and init
diff --git a/include/linux/phy.h b/include/linux/phy.h
index ed332ac92e25..7aa97f4e5387 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1377,10 +1377,17 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
bool is_c45,
struct phy_c45_device_ids *c45_ids);
#if IS_ENABLED(CONFIG_PHYLIB)
+struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode);
struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45);
int phy_device_register(struct phy_device *phy);
void phy_device_free(struct phy_device *phydev);
#else
+static inline
+struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode)
+{
+ return 0;
+}
+
static inline
struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
{
--
2.31.1
From: Calvin Johnson <[email protected]>
Define phylink_fwnode_phy_connect() to connect phy specified by
a fwnode to a phylink instance.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7: None
Changes in v6:
- remove OF check for fixed-link
Changes in v5: None
Changes in v4:
- call phy_device_free() before returning
Changes in v3: None
Changes in v2: None
drivers/net/phy/phylink.c | 54 +++++++++++++++++++++++++++++++++++++++
include/linux/phylink.h | 3 +++
2 files changed, 57 insertions(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 96d8e88b4e46..9cc0f69faafe 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -5,6 +5,7 @@
*
* Copyright (C) 2015 Russell King
*/
+#include <linux/acpi.h>
#include <linux/ethtool.h>
#include <linux/export.h>
#include <linux/gpio/consumer.h>
@@ -1125,6 +1126,59 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn,
}
EXPORT_SYMBOL_GPL(phylink_of_phy_connect);
+/**
+ * phylink_fwnode_phy_connect() - connect the PHY specified in the fwnode.
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @fwnode: a pointer to a &struct fwnode_handle.
+ * @flags: PHY-specific flags to communicate to the PHY device driver
+ *
+ * Connect the phy specified @fwnode to the phylink instance specified
+ * by @pl.
+ *
+ * Returns 0 on success or a negative errno.
+ */
+int phylink_fwnode_phy_connect(struct phylink *pl,
+ struct fwnode_handle *fwnode,
+ u32 flags)
+{
+ struct fwnode_handle *phy_fwnode;
+ struct phy_device *phy_dev;
+ int ret;
+
+ /* Fixed links and 802.3z are handled without needing a PHY */
+ if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
+ (pl->cfg_link_an_mode == MLO_AN_INBAND &&
+ phy_interface_mode_is_8023z(pl->link_interface)))
+ return 0;
+
+ phy_fwnode = fwnode_get_phy_node(fwnode);
+ if (IS_ERR(phy_fwnode)) {
+ if (pl->cfg_link_an_mode == MLO_AN_PHY)
+ return -ENODEV;
+ return 0;
+ }
+
+ phy_dev = fwnode_phy_find_device(phy_fwnode);
+ /* We're done with the phy_node handle */
+ fwnode_handle_put(phy_fwnode);
+ if (!phy_dev)
+ return -ENODEV;
+
+ ret = phy_attach_direct(pl->netdev, phy_dev, flags,
+ pl->link_interface);
+ if (ret) {
+ phy_device_free(phy_dev);
+ return ret;
+ }
+
+ ret = phylink_bringup_phy(pl, phy_dev, pl->link_config.interface);
+ if (ret)
+ phy_detach(phy_dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phylink_fwnode_phy_connect);
+
/**
* phylink_disconnect_phy() - disconnect any PHY attached to the phylink
* instance.
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index fd2acfd9b597..afb3ded0b691 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -441,6 +441,9 @@ void phylink_destroy(struct phylink *);
int phylink_connect_phy(struct phylink *, struct phy_device *);
int phylink_of_phy_connect(struct phylink *, struct device_node *, u32 flags);
+int phylink_fwnode_phy_connect(struct phylink *pl,
+ struct fwnode_handle *fwnode,
+ u32 flags);
void phylink_disconnect_phy(struct phylink *);
void phylink_mac_change(struct phylink *, bool up);
--
2.31.1
From: Calvin Johnson <[email protected]>
Depending on the device node type, call the specific OF or ACPI
mdiobus_register function.
Note: For both ACPI and DT cases, endianness of MDIO controller
need to be specified using "little-endian" property.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8:
- Directly call the OF or ACPI variants of registering the MDIO bus.
This is needed because the fwnode_mdio.c module should only implement
features which can be achieved without going back to the OF/ACPI
variants. Without this restrictions we directly end up in a dependency
cycle: of_mdio -> fwnode_mdio -> of_mdio.
- Changed the commit title since the fwnode_mdiobus_register() is no
longer available
Changes in v7:
- Include fwnode_mdio.h
- Alphabetically sort header inclusions
Changes in v6: None
Changes in v5: None
Changes in v4:
- Cleanup xgmac_mdio_probe()
Changes in v3:
- Avoid unnecessary line removal
- Remove unused inclusion of acpi.h
Changes in v2: None
drivers/net/ethernet/freescale/xgmac_mdio.c | 30 ++++++++++++++-------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index bfa2826c5545..0b68852379da 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -2,6 +2,7 @@
* QorIQ 10G MDIO Controller
*
* Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2021 NXP
*
* Authors: Andy Fleming <[email protected]>
* Timur Tabi <[email protected]>
@@ -11,15 +12,17 @@
* kind, whether express or implied.
*/
-#include <linux/kernel.h>
-#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/acpi_mdio.h>
#include <linux/interrupt.h>
-#include <linux/module.h>
-#include <linux/phy.h>
+#include <linux/kernel.h>
#include <linux/mdio.h>
+#include <linux/module.h>
#include <linux/of_address.h>
-#include <linux/of_platform.h>
#include <linux/of_mdio.h>
+#include <linux/of_platform.h>
+#include <linux/phy.h>
+#include <linux/slab.h>
/* Number of microseconds to wait for a register to respond */
#define TIMEOUT 1000
@@ -243,10 +246,10 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
static int xgmac_mdio_probe(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
- struct mii_bus *bus;
- struct resource *res;
+ struct fwnode_handle *fwnode;
struct mdio_fsl_priv *priv;
+ struct resource *res;
+ struct mii_bus *bus;
int ret;
/* In DPAA-1, MDIO is one of the many FMan sub-devices. The FMan
@@ -279,13 +282,22 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
goto err_ioremap;
}
+ /* For both ACPI and DT cases, endianness of MDIO controller
+ * needs to be specified using "little-endian" property.
+ */
priv->is_little_endian = device_property_read_bool(&pdev->dev,
"little-endian");
priv->has_a011043 = device_property_read_bool(&pdev->dev,
"fsl,erratum-a011043");
- ret = of_mdiobus_register(bus, np);
+ fwnode = pdev->dev.fwnode;
+ if (is_of_node(fwnode))
+ ret = of_mdiobus_register(bus, to_of_node(fwnode));
+ else if (is_acpi_node(fwnode))
+ ret = acpi_mdiobus_register(bus, fwnode);
+ else
+ ret = -EINVAL;
if (ret) {
dev_err(&pdev->dev, "cannot register MDIO bus\n");
goto err_registration;
--
2.31.1
From: Calvin Johnson <[email protected]>
Introduce a wrapper around the _ADR evaluation.
Signed-off-by: Calvin Johnson <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
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
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 3b54b8fd7396..e7ddd281afff 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 c60745f657e9..6ace3a0f1415 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -710,6 +710,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
@@ -965,6 +967,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.31.1
From: Calvin Johnson <[email protected]>
Introduce fwnode_mdiobus_register_phy() to register PHYs on the
mdiobus. From the compatible string, identify whether the PHY is
c45 and based on this create a PHY device instance which is
registered on the mdiobus.
Along with fwnode_mdiobus_register_phy() also introduce
fwnode_find_mii_timestamper() and fwnode_mdiobus_phy_device_register()
since they are needed.
While at it, also use the newly introduced fwnode operation in
of_mdiobus_phy_device_register() and remove of_find_mii_timestamper()
since it's not needed anymore.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8:
- fixed some checkpatch warnings/checks
- included linux/fwnode_mdio.h in fwnode_mdio.c (fixed the build warnings)
- added fwnode_find_mii_timestamper() and fwnode_mdiobus_phy_device_register()
in order to get rid of the cycle dependency.
- change to 'depends on (ACPI || OF) || COMPILE_TEST
Changes in v7:
- Call unregister_mii_timestamper() without NULL check
- Create fwnode_mdio.c and move fwnode_mdiobus_register_phy()
Changes in v6:
- Initialize mii_ts to NULL
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
MAINTAINERS | 1 +
drivers/net/mdio/Kconfig | 7 ++
drivers/net/mdio/Makefile | 3 +-
drivers/net/mdio/fwnode_mdio.c | 144 +++++++++++++++++++++++++++++++++
drivers/net/mdio/of_mdio.c | 61 +-------------
include/linux/fwnode_mdio.h | 35 ++++++++
6 files changed, 193 insertions(+), 58 deletions(-)
create mode 100644 drivers/net/mdio/fwnode_mdio.c
create mode 100644 include/linux/fwnode_mdio.h
diff --git a/MAINTAINERS b/MAINTAINERS
index e69c1991ec3b..e8f8b6c33a51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6811,6 +6811,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/fwnode_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 d06e06f5e31a..422e9e042a3c 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -19,6 +19,13 @@ config MDIO_BUS
reflects whether the mdio_bus/mdio_device code is built as a
loadable module or built-in.
+config FWNODE_MDIO
+ def_tristate PHYLIB
+ depends on (ACPI || OF) || COMPILE_TEST
+ select FIXED_PHY
+ help
+ FWNODE MDIO bus (Ethernet PHY) accessors
+
config OF_MDIO
def_tristate PHYLIB
depends on OF
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index c3ec0ef989df..2e6813c709eb 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for Linux MDIO bus drivers
-obj-$(CONFIG_OF_MDIO) += of_mdio.o
+obj-$(CONFIG_FWNODE_MDIO) += fwnode_mdio.o
+obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
new file mode 100644
index 000000000000..e6985fcf7921
--- /dev/null
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * fwnode helpers for the MDIO (Ethernet PHY) API
+ *
+ * This file provides helper functions for extracting PHY device information
+ * out of the fwnode and using it to populate an mii_bus.
+ */
+
+#include <linux/acpi.h>
+#include <linux/fwnode_mdio.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+
+MODULE_AUTHOR("Calvin Johnson <[email protected]>");
+MODULE_LICENSE("GPL");
+
+static struct mii_timestamper *
+fwnode_find_mii_timestamper(struct fwnode_handle *fwnode)
+{
+ struct of_phandle_args arg;
+ int err;
+
+ if (is_acpi_node(fwnode))
+ return NULL;
+
+ err = of_parse_phandle_with_fixed_args(to_of_node(fwnode), "timestamper", 1, 0, &arg);
+
+ if (err == -ENOENT)
+ return NULL;
+ else if (err)
+ return ERR_PTR(err);
+
+ if (arg.args_count != 1)
+ return ERR_PTR(-EINVAL);
+
+ return register_mii_timestamper(arg.np, arg.args[0]);
+}
+
+int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
+ struct phy_device *phy,
+ struct fwnode_handle *child, u32 addr)
+{
+ int rc;
+
+ rc = fwnode_irq_get(child, 0);
+ if (rc == -EPROBE_DEFER)
+ return rc;
+
+ if (rc > 0) {
+ phy->irq = rc;
+ mdio->irq[addr] = rc;
+ } else {
+ phy->irq = mdio->irq[addr];
+ }
+
+ if (fwnode_property_read_bool(child, "broken-turn-around"))
+ mdio->phy_ignore_ta_mask |= 1 << addr;
+
+ fwnode_property_read_u32(child, "reset-assert-us",
+ &phy->mdio.reset_assert_delay);
+ fwnode_property_read_u32(child, "reset-deassert-us",
+ &phy->mdio.reset_deassert_delay);
+
+ /* Associate the fwnode with the device structure so it
+ * can be looked up later
+ */
+ fwnode_handle_get(child);
+ phy->mdio.dev.fwnode = child;
+
+ /* All data is now stored in the phy struct;
+ * register it
+ */
+ rc = phy_device_register(phy);
+ if (rc) {
+ fwnode_handle_put(child);
+ return rc;
+ }
+
+ dev_dbg(&mdio->dev, "registered phy %p fwnode at address %i\n",
+ child, addr);
+ return 0;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_phy_device_register);
+
+int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+ struct fwnode_handle *child, u32 addr)
+{
+ struct mii_timestamper *mii_ts = NULL;
+ struct phy_device *phy;
+ bool is_c45 = false;
+ u32 phy_id;
+ int rc;
+
+ mii_ts = fwnode_find_mii_timestamper(child);
+ if (IS_ERR(mii_ts))
+ return PTR_ERR(mii_ts);
+
+ rc = fwnode_property_match_string(child, "compatible",
+ "ethernet-phy-ieee802.3-c45");
+ if (rc >= 0)
+ is_c45 = true;
+
+ if (is_c45 || fwnode_get_phy_id(child, &phy_id))
+ phy = get_phy_device(bus, addr, is_c45);
+ else
+ phy = phy_device_create(bus, addr, phy_id, 0, NULL);
+ if (IS_ERR(phy)) {
+ unregister_mii_timestamper(mii_ts);
+ return PTR_ERR(phy);
+ }
+
+ if (is_acpi_node(child)) {
+ phy->irq = bus->irq[addr];
+
+ /* Associate the fwnode with the device structure so it
+ * can be looked up later.
+ */
+ phy->mdio.dev.fwnode = child;
+
+ /* All data is now stored in the phy struct, so register it */
+ rc = phy_device_register(phy);
+ if (rc) {
+ phy_device_free(phy);
+ fwnode_handle_put(phy->mdio.dev.fwnode);
+ return rc;
+ }
+ } else if (is_of_node(child)) {
+ rc = fwnode_mdiobus_phy_device_register(bus, phy, child, addr);
+ if (rc) {
+ unregister_mii_timestamper(mii_ts);
+ phy_device_free(phy);
+ return rc;
+ }
+ }
+
+ /* phy->mii_ts may already be defined by the PHY driver. A
+ * mii_timestamper probed via the device tree will still have
+ * precedence.
+ */
+ if (mii_ts)
+ phy->mii_ts = mii_ts;
+ return 0;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index d73c0570f19c..9b1cadfd465d 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -32,65 +32,12 @@ static int of_get_phy_id(struct device_node *device, u32 *phy_id)
return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
}
-static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
-{
- struct of_phandle_args arg;
- int err;
-
- err = of_parse_phandle_with_fixed_args(node, "timestamper", 1, 0, &arg);
-
- if (err == -ENOENT)
- return NULL;
- else if (err)
- return ERR_PTR(err);
-
- if (arg.args_count != 1)
- return ERR_PTR(-EINVAL);
-
- return register_mii_timestamper(arg.np, arg.args[0]);
-}
-
int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
- struct device_node *child, u32 addr)
+ struct device_node *child, u32 addr)
{
- int rc;
-
- rc = of_irq_get(child, 0);
- if (rc == -EPROBE_DEFER)
- return rc;
-
- if (rc > 0) {
- phy->irq = rc;
- mdio->irq[addr] = rc;
- } else {
- phy->irq = mdio->irq[addr];
- }
-
- if (of_property_read_bool(child, "broken-turn-around"))
- mdio->phy_ignore_ta_mask |= 1 << addr;
-
- of_property_read_u32(child, "reset-assert-us",
- &phy->mdio.reset_assert_delay);
- of_property_read_u32(child, "reset-deassert-us",
- &phy->mdio.reset_deassert_delay);
-
- /* Associate the OF node with the device structure so it
- * can be looked up later */
- of_node_get(child);
- phy->mdio.dev.of_node = child;
- phy->mdio.dev.fwnode = of_fwnode_handle(child);
-
- /* All data is now stored in the phy struct;
- * register it */
- rc = phy_device_register(phy);
- if (rc) {
- of_node_put(child);
- return rc;
- }
-
- dev_dbg(&mdio->dev, "registered phy %pOFn at address %i\n",
- child, addr);
- return 0;
+ return fwnode_mdiobus_phy_device_register(mdio, phy,
+ of_fwnode_handle(child),
+ addr);
}
EXPORT_SYMBOL(of_mdiobus_phy_device_register);
diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
new file mode 100644
index 000000000000..faf603c48c86
--- /dev/null
+++ b/include/linux/fwnode_mdio.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * FWNODE helper for the MDIO (Ethernet PHY) API
+ */
+
+#ifndef __LINUX_FWNODE_MDIO_H
+#define __LINUX_FWNODE_MDIO_H
+
+#include <linux/phy.h>
+
+#if IS_ENABLED(CONFIG_FWNODE_MDIO)
+int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
+ struct phy_device *phy,
+ struct fwnode_handle *child, u32 addr);
+
+int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+ struct fwnode_handle *child, u32 addr);
+
+#else /* CONFIG_FWNODE_MDIO */
+int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
+ struct phy_device *phy,
+ struct fwnode_handle *child, u32 addr)
+{
+ return -EINVAL;
+}
+
+static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+ struct fwnode_handle *child,
+ u32 addr)
+{
+ return -EINVAL;
+}
+#endif
+
+#endif /* __LINUX_FWNODE_MDIO_H */
--
2.31.1
From: Calvin Johnson <[email protected]>
Define fwnode_phy_find_device() to iterate an mdiobus and find the
phy device of the provided phy fwnode. Additionally define
device_phy_find_device() to find phy device of provided device.
Define fwnode_get_phy_node() to get phy_node using named reference.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
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()
Changes in v2:
- use reverse christmas tree ordering for local variables
drivers/net/phy/phy_device.c | 62 ++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 20 ++++++++++++
2 files changed, 82 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 363cc70d00ca..f651c4feb49f 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -9,6 +9,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h>
#include <linux/bitmap.h>
#include <linux/delay.h>
#include <linux/errno.h>
@@ -2886,6 +2887,67 @@ struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode)
}
EXPORT_SYMBOL(fwnode_mdio_find_device);
+/**
+ * fwnode_phy_find_device - For provided phy_fwnode, find phy_device.
+ *
+ * @phy_fwnode: Pointer to the phy's fwnode.
+ *
+ * If successful, returns a pointer to the phy_device with the embedded
+ * struct device refcount incremented by one, or NULL on failure.
+ */
+struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode)
+{
+ struct mdio_device *mdiodev;
+
+ mdiodev = fwnode_mdio_find_device(phy_fwnode);
+ if (!mdiodev)
+ return NULL;
+
+ if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
+ return to_phy_device(&mdiodev->dev);
+
+ put_device(&mdiodev->dev);
+
+ return NULL;
+}
+EXPORT_SYMBOL(fwnode_phy_find_device);
+
+/**
+ * device_phy_find_device - For the given device, get the phy_device
+ * @dev: Pointer to the given device
+ *
+ * Refer return conditions of fwnode_phy_find_device().
+ */
+struct phy_device *device_phy_find_device(struct device *dev)
+{
+ return fwnode_phy_find_device(dev_fwnode(dev));
+}
+EXPORT_SYMBOL_GPL(device_phy_find_device);
+
+/**
+ * fwnode_get_phy_node - Get the phy_node using the named reference.
+ * @fwnode: Pointer to fwnode from which phy_node has to be obtained.
+ *
+ * Refer return conditions of fwnode_find_reference().
+ * For ACPI, only "phy-handle" is supported. Legacy DT properties "phy"
+ * and "phy-device" are not supported in ACPI. DT supports all the three
+ * named references to the phy node.
+ */
+struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *phy_node;
+
+ /* Only phy-handle is used for ACPI */
+ phy_node = fwnode_find_reference(fwnode, "phy-handle", 0);
+ if (is_acpi_node(fwnode) || !IS_ERR(phy_node))
+ return phy_node;
+ phy_node = fwnode_find_reference(fwnode, "phy", 0);
+ if (IS_ERR(phy_node))
+ phy_node = fwnode_find_reference(fwnode, "phy-device", 0);
+ return phy_node;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_phy_node);
+
/**
* phy_probe - probe and init a PHY device
* @dev: device to probe and init
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7aa97f4e5387..f9b5fb099fa6 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1378,6 +1378,9 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
struct phy_c45_device_ids *c45_ids);
#if IS_ENABLED(CONFIG_PHYLIB)
struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode);
+struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode);
+struct phy_device *device_phy_find_device(struct device *dev);
+struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode);
struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45);
int phy_device_register(struct phy_device *phy);
void phy_device_free(struct phy_device *phydev);
@@ -1388,6 +1391,23 @@ struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode)
return 0;
}
+static inline
+struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode)
+{
+ return NULL;
+}
+
+static inline struct phy_device *device_phy_find_device(struct device *dev)
+{
+ return NULL;
+}
+
+static inline
+struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode)
+{
+ return NULL;
+}
+
static inline
struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
{
--
2.31.1
From: Calvin Johnson <[email protected]>
Refactor of_mdiobus_register_phy() to use fwnode_mdiobus_register_phy().
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7:
- include fwnode_mdio.h
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/net/mdio/of_mdio.c | 39 ++------------------------------------
1 file changed, 2 insertions(+), 37 deletions(-)
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 9b1cadfd465d..8744b1e1c2b1 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -10,6 +10,7 @@
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/fwnode_mdio.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/netdevice.h>
@@ -44,43 +45,7 @@ EXPORT_SYMBOL(of_mdiobus_phy_device_register);
static int of_mdiobus_register_phy(struct mii_bus *mdio,
struct device_node *child, u32 addr)
{
- struct mii_timestamper *mii_ts;
- struct phy_device *phy;
- bool is_c45;
- int rc;
- u32 phy_id;
-
- mii_ts = of_find_mii_timestamper(child);
- if (IS_ERR(mii_ts))
- return PTR_ERR(mii_ts);
-
- is_c45 = of_device_is_compatible(child,
- "ethernet-phy-ieee802.3-c45");
-
- if (!is_c45 && !of_get_phy_id(child, &phy_id))
- phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
- else
- phy = get_phy_device(mdio, addr, is_c45);
- if (IS_ERR(phy)) {
- unregister_mii_timestamper(mii_ts);
- return PTR_ERR(phy);
- }
-
- rc = of_mdiobus_phy_device_register(mdio, phy, child, addr);
- if (rc) {
- unregister_mii_timestamper(mii_ts);
- phy_device_free(phy);
- return rc;
- }
-
- /* phy->mii_ts may already be defined by the PHY driver. A
- * mii_timestamper probed via the device tree will still have
- * precedence.
- */
- if (mii_ts)
- phy->mii_ts = mii_ts;
-
- return 0;
+ return fwnode_mdiobus_register_phy(mdio, of_fwnode_handle(child), addr);
}
static int of_mdiobus_register_device(struct mii_bus *mdio,
--
2.31.1
From: Calvin Johnson <[email protected]>
Callers of unregister_mii_timestamper() currently check for NULL
value of mii_ts before calling it.
Place the NULL check inside unregister_mii_timestamper() and update
the callers accordingly.
Signed-off-by: Calvin Johnson <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7:
- check NULL in unregister_mii_timestamper()
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/net/mdio/of_mdio.c | 6 ++----
drivers/net/phy/mii_timestamper.c | 3 +++
drivers/net/phy/phy_device.c | 3 +--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 29f121cba314..d73c0570f19c 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -115,15 +115,13 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
else
phy = get_phy_device(mdio, addr, is_c45);
if (IS_ERR(phy)) {
- if (mii_ts)
- unregister_mii_timestamper(mii_ts);
+ unregister_mii_timestamper(mii_ts);
return PTR_ERR(phy);
}
rc = of_mdiobus_phy_device_register(mdio, phy, child, addr);
if (rc) {
- if (mii_ts)
- unregister_mii_timestamper(mii_ts);
+ unregister_mii_timestamper(mii_ts);
phy_device_free(phy);
return rc;
}
diff --git a/drivers/net/phy/mii_timestamper.c b/drivers/net/phy/mii_timestamper.c
index b71b7456462d..51ae0593a04f 100644
--- a/drivers/net/phy/mii_timestamper.c
+++ b/drivers/net/phy/mii_timestamper.c
@@ -111,6 +111,9 @@ void unregister_mii_timestamper(struct mii_timestamper *mii_ts)
struct mii_timestamping_desc *desc;
struct list_head *this;
+ if (!mii_ts)
+ return;
+
/* mii_timestamper statically registered by the PHY driver won't use the
* register_mii_timestamper() and thus don't have ->device set. Don't
* try to unregister these.
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0ce5c7274930..e4b935b0b71b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -945,8 +945,7 @@ EXPORT_SYMBOL(phy_device_register);
*/
void phy_device_remove(struct phy_device *phydev)
{
- if (phydev->mii_ts)
- unregister_mii_timestamper(phydev->mii_ts);
+ unregister_mii_timestamper(phydev->mii_ts);
device_del(&phydev->mdio.dev);
--
2.31.1
From: Calvin Johnson <[email protected]>
Define acpi_mdiobus_register() to Register mii_bus and create PHYs for
each ACPI child node.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
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 | 26 +++++++++++++++++
5 files changed, 91 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 e8f8b6c33a51..2172f594be8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6811,6 +6811,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 422e9e042a3c..99a6c13a11af 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -34,6 +34,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 2e6813c709eb..15f8dc4042ce 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 <[email protected]>");
+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..0a24ab7cb66f
--- /dev/null
+++ b/include/linux/acpi_mdio.h
@@ -0,0 +1,26 @@
+/* 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.31.1
From: Calvin Johnson <[email protected]>
Extract phy_id from compatible string. This will be used by
fwnode_mdiobus_register_phy() to create phy device using the
phy_id.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- Use traditional comparison pattern
- Use GENMASK
Changes in v2: None
drivers/net/phy/phy_device.c | 21 +++++++++++++++++++++
include/linux/phy.h | 5 +++++
2 files changed, 26 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index f651c4feb49f..0ce5c7274930 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -834,6 +834,27 @@ static int get_phy_c22_id(struct mii_bus *bus, int addr, u32 *phy_id)
return 0;
}
+/* Extract the phy ID from the compatible string of the form
+ * ethernet-phy-idAAAA.BBBB.
+ */
+int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id)
+{
+ unsigned int upper, lower;
+ const char *cp;
+ int ret;
+
+ ret = fwnode_property_read_string(fwnode, "compatible", &cp);
+ if (ret)
+ return ret;
+
+ if (sscanf(cp, "ethernet-phy-id%4x.%4x", &upper, &lower) != 2)
+ return -EINVAL;
+
+ *phy_id = ((upper & GENMASK(15, 0)) << 16) | (lower & GENMASK(15, 0));
+ return 0;
+}
+EXPORT_SYMBOL(fwnode_get_phy_id);
+
/**
* get_phy_device - reads the specified PHY device and returns its @phy_device
* struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f9b5fb099fa6..b60694734b07 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1377,6 +1377,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
bool is_c45,
struct phy_c45_device_ids *c45_ids);
#if IS_ENABLED(CONFIG_PHYLIB)
+int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id);
struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode);
struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode);
struct phy_device *device_phy_find_device(struct device *dev);
@@ -1385,6 +1386,10 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45);
int phy_device_register(struct phy_device *phy);
void phy_device_free(struct phy_device *phydev);
#else
+static inline int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id)
+{
+ return 0;
+}
static inline
struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode)
{
--
2.31.1
From: Calvin Johnson <[email protected]>
Modify dpaa2_mac_get_node() to get the dpmac fwnode from either
DT or ACPI.
Modify dpaa2_mac_get_if_mode() to get interface mode from dpmac_node
which is a fwnode.
Modify dpaa2_pcs_create() to create pcs from dpmac_node fwnode.
Modify dpaa2_mac_connect() to support ACPI along with DT.
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8:
- adjust code over latest changes applied on the driver
Changes in v7:
- remove unnecassary checks
Changes in v6:
- use dev_fwnode()
- remove useless else
- replace of_device_is_available() to fwnode_device_is_available()
Changes in v5:
- replace fwnode_get_id() with OF and ACPI function calls
Changes in v4: None
Changes in v3: None
Changes in v2:
- Refactor OF functions to use fwnode functions
.../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 88 +++++++++++--------
.../net/ethernet/freescale/dpaa2/dpaa2-mac.h | 2 +-
2 files changed, 53 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 4dfadf2b70d6..ae6d382d8735 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -1,6 +1,9 @@
// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
/* Copyright 2019 NXP */
+#include <linux/acpi.h>
+#include <linux/property.h>
+
#include "dpaa2-eth.h"
#include "dpaa2-mac.h"
@@ -34,39 +37,51 @@ static int phy_mode(enum dpmac_eth_if eth_if, phy_interface_t *if_mode)
return 0;
}
-/* Caller must call of_node_put on the returned value */
-static struct device_node *dpaa2_mac_get_node(u16 dpmac_id)
+static struct fwnode_handle *dpaa2_mac_get_node(struct device *dev,
+ u16 dpmac_id)
{
- struct device_node *dpmacs, *dpmac = NULL;
- u32 id;
+ struct fwnode_handle *fwnode, *parent, *child = NULL;
+ struct device_node *dpmacs = NULL;
int err;
+ u32 id;
- dpmacs = of_find_node_by_name(NULL, "dpmacs");
- if (!dpmacs)
- return NULL;
+ fwnode = dev_fwnode(dev->parent);
+ if (is_of_node(fwnode)) {
+ dpmacs = of_find_node_by_name(NULL, "dpmacs");
+ if (!dpmacs)
+ return NULL;
+ parent = of_fwnode_handle(dpmacs);
+ } else if (is_acpi_node(fwnode)) {
+ parent = fwnode;
+ }
- while ((dpmac = of_get_next_child(dpmacs, dpmac)) != NULL) {
- err = of_property_read_u32(dpmac, "reg", &id);
+ fwnode_for_each_child_node(parent, child) {
+ err = -EINVAL;
+ if (is_acpi_device_node(child))
+ err = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &id);
+ else if (is_of_node(child))
+ err = of_property_read_u32(to_of_node(child), "reg", &id);
if (err)
continue;
- if (id == dpmac_id)
- break;
- }
+ if (id == dpmac_id) {
+ of_node_put(dpmacs);
+ return child;
+ }
+ }
of_node_put(dpmacs);
-
- return dpmac;
+ return NULL;
}
-static int dpaa2_mac_get_if_mode(struct device_node *node,
+static int dpaa2_mac_get_if_mode(struct fwnode_handle *dpmac_node,
struct dpmac_attr attr)
{
phy_interface_t if_mode;
int err;
- err = of_get_phy_mode(node, &if_mode);
- if (!err)
- return if_mode;
+ err = fwnode_get_phy_mode(dpmac_node);
+ if (err > 0)
+ return err;
err = phy_mode(attr.eth_if, &if_mode);
if (!err)
@@ -235,26 +250,27 @@ static const struct phylink_mac_ops dpaa2_mac_phylink_ops = {
};
static int dpaa2_pcs_create(struct dpaa2_mac *mac,
- struct device_node *dpmac_node, int id)
+ struct fwnode_handle *dpmac_node,
+ int id)
{
struct mdio_device *mdiodev;
- struct device_node *node;
+ struct fwnode_handle *node;
- node = of_parse_phandle(dpmac_node, "pcs-handle", 0);
- if (!node) {
+ node = fwnode_find_reference(dpmac_node, "pcs-handle", 0);
+ if (IS_ERR(node)) {
/* do not error out on old DTS files */
netdev_warn(mac->net_dev, "pcs-handle node not found\n");
return 0;
}
- if (!of_device_is_available(node)) {
+ if (!fwnode_device_is_available(node)) {
netdev_err(mac->net_dev, "pcs-handle node not available\n");
- of_node_put(node);
+ fwnode_handle_put(node);
return -ENODEV;
}
- mdiodev = of_mdio_find_device(node);
- of_node_put(node);
+ mdiodev = fwnode_mdio_find_device(node);
+ fwnode_handle_put(node);
if (!mdiodev)
return -EPROBE_DEFER;
@@ -283,13 +299,13 @@ static void dpaa2_pcs_destroy(struct dpaa2_mac *mac)
int dpaa2_mac_connect(struct dpaa2_mac *mac)
{
struct net_device *net_dev = mac->net_dev;
- struct device_node *dpmac_node;
+ struct fwnode_handle *dpmac_node;
struct phylink *phylink;
int err;
mac->if_link_type = mac->attr.link_type;
- dpmac_node = mac->of_node;
+ dpmac_node = mac->fw_node;
if (!dpmac_node) {
netdev_err(net_dev, "No dpmac@%d node found.\n", mac->attr.id);
return -ENODEV;
@@ -304,7 +320,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
* error out if the interface mode requests them and there is no PHY
* to act upon them
*/
- if (of_phy_is_fixed_link(dpmac_node) &&
+ if (of_phy_is_fixed_link(to_of_node(dpmac_node)) &&
(mac->if_mode == PHY_INTERFACE_MODE_RGMII_ID ||
mac->if_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
mac->if_mode == PHY_INTERFACE_MODE_RGMII_TXID)) {
@@ -324,7 +340,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
mac->phylink_config.type = PHYLINK_NETDEV;
phylink = phylink_create(&mac->phylink_config,
- of_fwnode_handle(dpmac_node), mac->if_mode,
+ dpmac_node, mac->if_mode,
&dpaa2_mac_phylink_ops);
if (IS_ERR(phylink)) {
err = PTR_ERR(phylink);
@@ -335,9 +351,9 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
if (mac->pcs)
phylink_set_pcs(mac->phylink, &mac->pcs->pcs);
- err = phylink_of_phy_connect(mac->phylink, dpmac_node, 0);
+ err = phylink_fwnode_phy_connect(mac->phylink, dpmac_node, 0);
if (err) {
- netdev_err(net_dev, "phylink_of_phy_connect() = %d\n", err);
+ netdev_err(net_dev, "phylink_fwnode_phy_connect() = %d\n", err);
goto err_phylink_destroy;
}
@@ -384,8 +400,8 @@ int dpaa2_mac_open(struct dpaa2_mac *mac)
/* Find the device node representing the MAC device and link the device
* behind the associated netdev to it.
*/
- mac->of_node = dpaa2_mac_get_node(mac->attr.id);
- net_dev->dev.of_node = mac->of_node;
+ mac->fw_node = dpaa2_mac_get_node(&mac->mc_dev->dev, mac->attr.id);
+ net_dev->dev.of_node = to_of_node(mac->fw_node);
return 0;
@@ -399,8 +415,8 @@ void dpaa2_mac_close(struct dpaa2_mac *mac)
struct fsl_mc_device *dpmac_dev = mac->mc_dev;
dpmac_close(mac->mc_io, 0, dpmac_dev->mc_handle);
- if (mac->of_node)
- of_node_put(mac->of_node);
+ if (mac->fw_node)
+ fwnode_handle_put(mac->fw_node);
}
static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = {
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
index 8ebcb3420d02..7842cbb2207a 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
@@ -24,7 +24,7 @@ struct dpaa2_mac {
phy_interface_t if_mode;
enum dpmac_link_type if_link_type;
struct lynx_pcs *pcs;
- struct device_node *of_node;
+ struct fwnode_handle *fw_node;
};
bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
--
2.31.1
From: Calvin Johnson <[email protected]>
Refactor phylink_of_phy_connect() to use phylink_fwnode_phy_connect().
Signed-off-by: Calvin Johnson <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/net/phy/phylink.c | 39 +--------------------------------------
1 file changed, 1 insertion(+), 38 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 9cc0f69faafe..bb9eeb74f70a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1085,44 +1085,7 @@ EXPORT_SYMBOL_GPL(phylink_connect_phy);
int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn,
u32 flags)
{
- struct device_node *phy_node;
- struct phy_device *phy_dev;
- int ret;
-
- /* Fixed links and 802.3z are handled without needing a PHY */
- if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
- (pl->cfg_link_an_mode == MLO_AN_INBAND &&
- phy_interface_mode_is_8023z(pl->link_interface)))
- return 0;
-
- phy_node = of_parse_phandle(dn, "phy-handle", 0);
- if (!phy_node)
- phy_node = of_parse_phandle(dn, "phy", 0);
- if (!phy_node)
- phy_node = of_parse_phandle(dn, "phy-device", 0);
-
- if (!phy_node) {
- if (pl->cfg_link_an_mode == MLO_AN_PHY)
- return -ENODEV;
- return 0;
- }
-
- phy_dev = of_phy_find_device(phy_node);
- /* We're done with the phy_node handle */
- of_node_put(phy_node);
- if (!phy_dev)
- return -ENODEV;
-
- ret = phy_attach_direct(pl->netdev, phy_dev, flags,
- pl->link_interface);
- if (ret)
- return ret;
-
- ret = phylink_bringup_phy(pl, phy_dev, pl->link_config.interface);
- if (ret)
- phy_detach(phy_dev);
-
- return ret;
+ return phylink_fwnode_phy_connect(pl, of_fwnode_handle(dn), flags);
}
EXPORT_SYMBOL_GPL(phylink_of_phy_connect);
--
2.31.1
On Thu, Jun 10, 2021 at 07:39:02PM +0300, Ioana Ciornei wrote:
> From: Ioana Ciornei <[email protected]>
>
> This patch set provides ACPI support to DPAA2 network drivers.
Just to be clear and avoid confusion, there is a standing NACK against
this patchset. Please see the discussion here:
https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/#23518385
So far, i've not seen any indication the issues raised there have been
resolved. I don't see any Acked-by from an ACPI maintainer. So this
code remains NACKed.
Andrew
On Thu, Jun 10, 2021 at 6:56 PM Andrew Lunn <[email protected]> wrote:
>
> On Thu, Jun 10, 2021 at 07:39:02PM +0300, Ioana Ciornei wrote:
> > From: Ioana Ciornei <[email protected]>
> >
> > This patch set provides ACPI support to DPAA2 network drivers.
>
> Just to be clear and avoid confusion, there is a standing NACK against
> this patchset. Please see the discussion here:
>
> https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/#23518385
>
> So far, i've not seen any indication the issues raised there have been
> resolved. I don't see any Acked-by from an ACPI maintainer. So this
> code remains NACKed.
Andrew,
The ACPI maintainers did bless the use of the ACPI standards followed
in this patchset, and their only abstinence from ACK'ing the patchset
was whether the code was used in production systems. Well currently,
not only have we, SolidRun, been using this patchset and the associated
ACPI tables in our SystemsReady certified firmware for the HoneyComb,
but we also have customers using this same patchset and firmware on
their systems rolled out to customers.
Additionally we have an entire new product line based on Marvell's
Armada CN913x series, which also needs this patchset to be fully
functional.
I am quite certain this is more than enough production systems using
this ACPI description method for networking to progress this patchset
forward.
-Jon
On Thu, Jun 10, 2021 at 6:40 PM Ioana Ciornei <[email protected]> wrote:
>
> From: Calvin Johnson <[email protected]>
>
> Introduce ACPI mechanism to get PHYs registered on a MDIO bus and
> provide them to be connected to MAC.
This is not an "ACPI mechanism", because it is not part of the ACPI
specification or support documentation thereof.
I would call it "a mechanism based on generic ACPI _DSD device
properties definition []1]". And provide a reference to the _DSD
properties definition document.
With that changed, you can add
Acked-by: Rafael J. Wysocki <[email protected]>
to this patch.
Note, however, that within the traditional ACPI framework, the _DSD
properties are consumed by the driver that binds to the device
represented by the ACPI device object containing the _DSD in question
in its scope, while in this case IIUC the properties are expected to
be consumed by the general networking code in the kernel. That is not
wrong in principle, but it means that operating systems other than
Linux are not likely to be using them.
> Describe properties "phy-handle" and "phy-mode".
>
> Signed-off-by: Calvin Johnson <[email protected]>
> Signed-off-by: Ioana Ciornei <[email protected]>
> ---
>
> Changes in v8: None
> Changes in v7: None
> Changes in v6:
> - Minor cleanup
>
> Changes in v5:
> - More cleanup
>
> Changes in v4:
> - More cleanup
>
> Changes in v3: None
> Changes in v2:
> - Updated with more description in document
>
> Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++++
> 1 file changed, 133 insertions(+)
> create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
>
> diff --git a/Documentation/firmware-guide/acpi/dsd/phy.rst b/Documentation/firmware-guide/acpi/dsd/phy.rst
> new file mode 100644
> index 000000000000..7d01ae8b3cc6
> --- /dev/null
> +++ b/Documentation/firmware-guide/acpi/dsd/phy.rst
> @@ -0,0 +1,133 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=========================
> +MDIO bus and PHYs in ACPI
> +=========================
> +
> +The PHYs on an MDIO bus [1] are probed and registered using
> +fwnode_mdiobus_register_phy().
> +
> +Later, for connecting these PHYs to their respective MACs, the PHYs registered
> +on the MDIO bus have to be referenced.
> +
> +This document introduces two _DSD properties that are to be used
> +for connecting PHYs on the MDIO bus [3] to the MAC layer.
> +
> +These properties are defined in accordance with the "Device
> +Properties UUID For _DSD" [2] document and the
> +daffd814-6eba-4d8c-8a91-bc9bbf4aa301 UUID must be used in the Device
> +Data Descriptors containing them.
> +
> +phy-handle
> +----------
> +For each MAC node, a device property "phy-handle" is used to reference
> +the PHY that is registered on an MDIO bus. This is mandatory for
> +network interfaces that have PHYs connected to MAC via MDIO bus.
> +
> +During the MDIO bus driver initialization, PHYs on this bus are probed
> +using the _ADR object as shown below and are registered on the MDIO bus.
> +
> +::
> + Scope(\_SB.MDI0)
> + {
> + Device(PHY1) {
> + Name (_ADR, 0x1)
> + } // end of PHY1
> +
> + Device(PHY2) {
> + Name (_ADR, 0x2)
> + } // end of PHY2
> + }
> +
> +Later, during the MAC driver initialization, the registered PHY devices
> +have to be retrieved from the MDIO bus. For this, the MAC driver needs
> +references to the previously registered PHYs which are provided
> +as device object references (e.g. \_SB.MDI0.PHY1).
> +
> +phy-mode
> +--------
> +The "phy-mode" _DSD property is used to describe the connection to
> +the PHY. The valid values for "phy-mode" are defined in [4].
> +
> +The following ASL example illustrates the usage of these properties.
> +
> +DSDT entry for MDIO node
> +------------------------
> +
> +The MDIO bus has an SoC component (MDIO controller) and a platform
> +component (PHYs on the MDIO bus).
> +
> +a) Silicon Component
> +This node describes the MDIO controller, MDI0
> +---------------------------------------------
> +::
> + Scope(_SB)
> + {
> + Device(MDI0) {
> + Name(_HID, "NXP0006")
> + Name(_CCA, 1)
> + Name(_UID, 0)
> + Name(_CRS, ResourceTemplate() {
> + Memory32Fixed(ReadWrite, MDI0_BASE, MDI_LEN)
> + Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
> + {
> + MDI0_IT
> + }
> + }) // end of _CRS for MDI0
> + } // end of MDI0
> + }
> +
> +b) Platform Component
> +The PHY1 and PHY2 nodes represent the PHYs connected to MDIO bus MDI0
> +---------------------------------------------------------------------
> +::
> + Scope(\_SB.MDI0)
> + {
> + Device(PHY1) {
> + Name (_ADR, 0x1)
> + } // end of PHY1
> +
> + Device(PHY2) {
> + Name (_ADR, 0x2)
> + } // end of PHY2
> + }
> +
> +DSDT entries representing MAC nodes
> +-----------------------------------
> +
> +Below are the MAC nodes where PHY nodes are referenced.
> +phy-mode and phy-handle are used as explained earlier.
> +------------------------------------------------------
> +::
> + Scope(\_SB.MCE0.PR17)
> + {
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package (2) {"phy-mode", "rgmii-id"},
> + Package (2) {"phy-handle", \_SB.MDI0.PHY1}
> + }
> + })
> + }
> +
> + Scope(\_SB.MCE0.PR18)
> + {
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package (2) {"phy-mode", "rgmii-id"},
> + Package (2) {"phy-handle", \_SB.MDI0.PHY2}}
> + }
> + })
> + }
> +
> +References
> +==========
> +
> +[1] Documentation/networking/phy.rst
> +
> +[2] https://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> +
> +[3] Documentation/firmware-guide/acpi/DSD-properties-rules.rst
> +
> +[4] Documentation/devicetree/bindings/net/ethernet-controller.yaml
> --
> 2.31.1
>
On Thu, Jun 10, 2021 at 6:40 PM Ioana Ciornei <[email protected]> wrote:
>
> From: Calvin Johnson <[email protected]>
>
> Introduce a wrapper around the _ADR evaluation.
>
> Signed-off-by: Calvin Johnson <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Ioana Ciornei <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
>
> Changes in v8: None
> 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
>
> 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 3b54b8fd7396..e7ddd281afff 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 c60745f657e9..6ace3a0f1415 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -710,6 +710,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
> @@ -965,6 +967,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.31.1
>
On 10/06/2021 17:39, Ioana Ciornei wrote:
> From: Calvin Johnson <[email protected]>
>
> Introduce ACPI mechanism to get PHYs registered on a MDIO bus and
> provide them to be connected to MAC.
>
> Describe properties "phy-handle" and "phy-mode".
>
> Signed-off-by: Calvin Johnson <[email protected]>
> Signed-off-by: Ioana Ciornei <[email protected]>
Looks reasonable to me. I'm not a kernel maintainer any more, so my
Acked-by: may not be very valuable, but here it is anyway:
Acked-by: Grant Likely <[email protected]>
> --- >
> Changes in v8: None
> Changes in v7: None
> Changes in v6:
> - Minor cleanup
>
> Changes in v5:
> - More cleanup
>
> Changes in v4:
> - More cleanup
>
> Changes in v3: None
> Changes in v2:
> - Updated with more description in document
>
> Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++++
> 1 file changed, 133 insertions(+)
> create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
>
> diff --git a/Documentation/firmware-guide/acpi/dsd/phy.rst b/Documentation/firmware-guide/acpi/dsd/phy.rst
> new file mode 100644
> index 000000000000..7d01ae8b3cc6
> --- /dev/null
> +++ b/Documentation/firmware-guide/acpi/dsd/phy.rst
> @@ -0,0 +1,133 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=========================
> +MDIO bus and PHYs in ACPI
> +=========================
> +
> +The PHYs on an MDIO bus [1] are probed and registered using
> +fwnode_mdiobus_register_phy().
> +
> +Later, for connecting these PHYs to their respective MACs, the PHYs registered
> +on the MDIO bus have to be referenced.
> +
> +This document introduces two _DSD properties that are to be used
> +for connecting PHYs on the MDIO bus [3] to the MAC layer.
> +
> +These properties are defined in accordance with the "Device
> +Properties UUID For _DSD" [2] document and the
> +daffd814-6eba-4d8c-8a91-bc9bbf4aa301 UUID must be used in the Device
> +Data Descriptors containing them.
> +
> +phy-handle
> +----------
> +For each MAC node, a device property "phy-handle" is used to reference
> +the PHY that is registered on an MDIO bus. This is mandatory for
> +network interfaces that have PHYs connected to MAC via MDIO bus.
> +
> +During the MDIO bus driver initialization, PHYs on this bus are probed
> +using the _ADR object as shown below and are registered on the MDIO bus.
> +
> +::
> + Scope(\_SB.MDI0)
> + {
> + Device(PHY1) {
> + Name (_ADR, 0x1)
> + } // end of PHY1
> +
> + Device(PHY2) {
> + Name (_ADR, 0x2)
> + } // end of PHY2
> + }
> +
> +Later, during the MAC driver initialization, the registered PHY devices
> +have to be retrieved from the MDIO bus. For this, the MAC driver needs
> +references to the previously registered PHYs which are provided
> +as device object references (e.g. \_SB.MDI0.PHY1).
> +
> +phy-mode
> +--------
> +The "phy-mode" _DSD property is used to describe the connection to
> +the PHY. The valid values for "phy-mode" are defined in [4].
> +
> +The following ASL example illustrates the usage of these properties.
> +
> +DSDT entry for MDIO node
> +------------------------
> +
> +The MDIO bus has an SoC component (MDIO controller) and a platform
> +component (PHYs on the MDIO bus).
> +
> +a) Silicon Component
> +This node describes the MDIO controller, MDI0
> +---------------------------------------------
> +::
> + Scope(_SB)
> + {
> + Device(MDI0) {
> + Name(_HID, "NXP0006")
> + Name(_CCA, 1)
> + Name(_UID, 0)
> + Name(_CRS, ResourceTemplate() {
> + Memory32Fixed(ReadWrite, MDI0_BASE, MDI_LEN)
> + Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
> + {
> + MDI0_IT
> + }
> + }) // end of _CRS for MDI0
> + } // end of MDI0
> + }
> +
> +b) Platform Component
> +The PHY1 and PHY2 nodes represent the PHYs connected to MDIO bus MDI0
> +---------------------------------------------------------------------
> +::
> + Scope(\_SB.MDI0)
> + {
> + Device(PHY1) {
> + Name (_ADR, 0x1)
> + } // end of PHY1
> +
> + Device(PHY2) {
> + Name (_ADR, 0x2)
> + } // end of PHY2
> + }
> +
> +DSDT entries representing MAC nodes
> +-----------------------------------
> +
> +Below are the MAC nodes where PHY nodes are referenced.
> +phy-mode and phy-handle are used as explained earlier.
> +------------------------------------------------------
> +::
> + Scope(\_SB.MCE0.PR17)
> + {
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package (2) {"phy-mode", "rgmii-id"},
> + Package (2) {"phy-handle", \_SB.MDI0.PHY1}
> + }
> + })
> + }
> +
> + Scope(\_SB.MCE0.PR18)
> + {
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package (2) {"phy-mode", "rgmii-id"},
> + Package (2) {"phy-handle", \_SB.MDI0.PHY2}}
> + }
> + })
> + }
> +
> +References
> +==========
> +
> +[1] Documentation/networking/phy.rst
> +
> +[2] https://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> +
> +[3] Documentation/firmware-guide/acpi/DSD-properties-rules.rst
> +
> +[4] Documentation/devicetree/bindings/net/ethernet-controller.yaml
>
On 10/06/2021 17:39, Ioana Ciornei wrote:
> From: Ioana Ciornei <[email protected]>
>
> 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_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.
In terms of design, this whole series looks right to me. The way data is
encoded in ACPI is entirely appropriate. As long as Rob Herring is okay
with the DT code changes I support this series being merged.
For the whole series:
Acked-by: Grant Likely <[email protected]>
>
> Tested-on: LX2160ARDB
>
> Changes in v8:
> - fixed some checkpatch warnings/checks
> - included linux/fwnode_mdio.h in fwnode_mdio.c (fixed the build warnings)
> - added fwnode_find_mii_timestamper() and
> fwnode_mdiobus_phy_device_register() in order to get rid of the cycle
> dependency.
> - change to 'depends on (ACPI || OF) || COMPILE_TEST (for FWNODE_MDIO)
> - remove the fwnode_mdiobus_register from fwnode_mdio.c since it
> introduces a cycle of dependencies.
>
> 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 (15):
> 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/fsl: Use [acpi|of]_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 | 88 ++++++-----
> .../net/ethernet/freescale/dpaa2/dpaa2-mac.h | 2 +-
> drivers/net/ethernet/freescale/xgmac_mdio.c | 30 ++--
> drivers/net/mdio/Kconfig | 14 ++
> drivers/net/mdio/Makefile | 4 +-
> drivers/net/mdio/acpi_mdio.c | 56 +++++++
> drivers/net/mdio/fwnode_mdio.c | 144 ++++++++++++++++++
> drivers/net/mdio/of_mdio.c | 138 ++---------------
> 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 | 26 ++++
> include/linux/fwnode_mdio.h | 35 +++++
> include/linux/phy.h | 32 ++++
> include/linux/phylink.h | 3 +
> 19 files changed, 691 insertions(+), 190 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
>
On Thu, Jun 10, 2021 at 6:40 PM Ioana Ciornei <[email protected]> wrote:
>
> From: Calvin Johnson <[email protected]>
>
> Define acpi_mdiobus_register() to Register mii_bus and create PHYs for
> each ACPI child node.
>
> Signed-off-by: Calvin Johnson <[email protected]>
> Signed-off-by: Ioana Ciornei <[email protected]>
> ---
>
> Changes in v8: None
> 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 | 26 +++++++++++++++++
> 5 files changed, 91 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 e8f8b6c33a51..2172f594be8f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6811,6 +6811,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 422e9e042a3c..99a6c13a11af 100644
> --- a/drivers/net/mdio/Kconfig
> +++ b/drivers/net/mdio/Kconfig
> @@ -34,6 +34,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 2e6813c709eb..15f8dc4042ce 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 <[email protected]>");
> +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.
It would be good to mention that @fwnode is expected to represent an
ACPI device object corresponding to mdiio and its children are
expected to correspond to the PHY devices on that bus (if my
understanding of this is correct, that is).
With that addressed, please feel free to add
Acked-by: Rafael J. Wysocki <[email protected]>
to this patch.
> + */
> +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..0a24ab7cb66f
> --- /dev/null
> +++ b/include/linux/acpi_mdio.h
> @@ -0,0 +1,26 @@
> +/* 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.31.1
>
On Thu, Jun 10, 2021 at 6:40 PM Ioana Ciornei <[email protected]> wrote:
>
> From: Calvin Johnson <[email protected]>
>
> Modify dpaa2_mac_get_node() to get the dpmac fwnode from either
> DT or ACPI.
>
> Modify dpaa2_mac_get_if_mode() to get interface mode from dpmac_node
> which is a fwnode.
>
> Modify dpaa2_pcs_create() to create pcs from dpmac_node fwnode.
>
> Modify dpaa2_mac_connect() to support ACPI along with DT.
>
> Signed-off-by: Calvin Johnson <[email protected]>
> Signed-off-by: Ioana Ciornei <[email protected]>
From the ACPI side
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
>
> Changes in v8:
> - adjust code over latest changes applied on the driver
>
> Changes in v7:
> - remove unnecassary checks
>
> Changes in v6:
> - use dev_fwnode()
> - remove useless else
> - replace of_device_is_available() to fwnode_device_is_available()
>
> Changes in v5:
> - replace fwnode_get_id() with OF and ACPI function calls
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - Refactor OF functions to use fwnode functions
>
> .../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 88 +++++++++++--------
> .../net/ethernet/freescale/dpaa2/dpaa2-mac.h | 2 +-
> 2 files changed, 53 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> index 4dfadf2b70d6..ae6d382d8735 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> @@ -1,6 +1,9 @@
> // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> /* Copyright 2019 NXP */
>
> +#include <linux/acpi.h>
> +#include <linux/property.h>
> +
> #include "dpaa2-eth.h"
> #include "dpaa2-mac.h"
>
> @@ -34,39 +37,51 @@ static int phy_mode(enum dpmac_eth_if eth_if, phy_interface_t *if_mode)
> return 0;
> }
>
> -/* Caller must call of_node_put on the returned value */
> -static struct device_node *dpaa2_mac_get_node(u16 dpmac_id)
> +static struct fwnode_handle *dpaa2_mac_get_node(struct device *dev,
> + u16 dpmac_id)
> {
> - struct device_node *dpmacs, *dpmac = NULL;
> - u32 id;
> + struct fwnode_handle *fwnode, *parent, *child = NULL;
> + struct device_node *dpmacs = NULL;
> int err;
> + u32 id;
>
> - dpmacs = of_find_node_by_name(NULL, "dpmacs");
> - if (!dpmacs)
> - return NULL;
> + fwnode = dev_fwnode(dev->parent);
> + if (is_of_node(fwnode)) {
> + dpmacs = of_find_node_by_name(NULL, "dpmacs");
> + if (!dpmacs)
> + return NULL;
> + parent = of_fwnode_handle(dpmacs);
> + } else if (is_acpi_node(fwnode)) {
> + parent = fwnode;
> + }
>
> - while ((dpmac = of_get_next_child(dpmacs, dpmac)) != NULL) {
> - err = of_property_read_u32(dpmac, "reg", &id);
> + fwnode_for_each_child_node(parent, child) {
> + err = -EINVAL;
> + if (is_acpi_device_node(child))
> + err = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &id);
> + else if (is_of_node(child))
> + err = of_property_read_u32(to_of_node(child), "reg", &id);
> if (err)
> continue;
> - if (id == dpmac_id)
> - break;
> - }
>
> + if (id == dpmac_id) {
> + of_node_put(dpmacs);
> + return child;
> + }
> + }
> of_node_put(dpmacs);
> -
> - return dpmac;
> + return NULL;
> }
>
> -static int dpaa2_mac_get_if_mode(struct device_node *node,
> +static int dpaa2_mac_get_if_mode(struct fwnode_handle *dpmac_node,
> struct dpmac_attr attr)
> {
> phy_interface_t if_mode;
> int err;
>
> - err = of_get_phy_mode(node, &if_mode);
> - if (!err)
> - return if_mode;
> + err = fwnode_get_phy_mode(dpmac_node);
> + if (err > 0)
> + return err;
>
> err = phy_mode(attr.eth_if, &if_mode);
> if (!err)
> @@ -235,26 +250,27 @@ static const struct phylink_mac_ops dpaa2_mac_phylink_ops = {
> };
>
> static int dpaa2_pcs_create(struct dpaa2_mac *mac,
> - struct device_node *dpmac_node, int id)
> + struct fwnode_handle *dpmac_node,
> + int id)
> {
> struct mdio_device *mdiodev;
> - struct device_node *node;
> + struct fwnode_handle *node;
>
> - node = of_parse_phandle(dpmac_node, "pcs-handle", 0);
> - if (!node) {
> + node = fwnode_find_reference(dpmac_node, "pcs-handle", 0);
> + if (IS_ERR(node)) {
> /* do not error out on old DTS files */
> netdev_warn(mac->net_dev, "pcs-handle node not found\n");
> return 0;
> }
>
> - if (!of_device_is_available(node)) {
> + if (!fwnode_device_is_available(node)) {
> netdev_err(mac->net_dev, "pcs-handle node not available\n");
> - of_node_put(node);
> + fwnode_handle_put(node);
> return -ENODEV;
> }
>
> - mdiodev = of_mdio_find_device(node);
> - of_node_put(node);
> + mdiodev = fwnode_mdio_find_device(node);
> + fwnode_handle_put(node);
> if (!mdiodev)
> return -EPROBE_DEFER;
>
> @@ -283,13 +299,13 @@ static void dpaa2_pcs_destroy(struct dpaa2_mac *mac)
> int dpaa2_mac_connect(struct dpaa2_mac *mac)
> {
> struct net_device *net_dev = mac->net_dev;
> - struct device_node *dpmac_node;
> + struct fwnode_handle *dpmac_node;
> struct phylink *phylink;
> int err;
>
> mac->if_link_type = mac->attr.link_type;
>
> - dpmac_node = mac->of_node;
> + dpmac_node = mac->fw_node;
> if (!dpmac_node) {
> netdev_err(net_dev, "No dpmac@%d node found.\n", mac->attr.id);
> return -ENODEV;
> @@ -304,7 +320,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
> * error out if the interface mode requests them and there is no PHY
> * to act upon them
> */
> - if (of_phy_is_fixed_link(dpmac_node) &&
> + if (of_phy_is_fixed_link(to_of_node(dpmac_node)) &&
> (mac->if_mode == PHY_INTERFACE_MODE_RGMII_ID ||
> mac->if_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
> mac->if_mode == PHY_INTERFACE_MODE_RGMII_TXID)) {
> @@ -324,7 +340,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
> mac->phylink_config.type = PHYLINK_NETDEV;
>
> phylink = phylink_create(&mac->phylink_config,
> - of_fwnode_handle(dpmac_node), mac->if_mode,
> + dpmac_node, mac->if_mode,
> &dpaa2_mac_phylink_ops);
> if (IS_ERR(phylink)) {
> err = PTR_ERR(phylink);
> @@ -335,9 +351,9 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
> if (mac->pcs)
> phylink_set_pcs(mac->phylink, &mac->pcs->pcs);
>
> - err = phylink_of_phy_connect(mac->phylink, dpmac_node, 0);
> + err = phylink_fwnode_phy_connect(mac->phylink, dpmac_node, 0);
> if (err) {
> - netdev_err(net_dev, "phylink_of_phy_connect() = %d\n", err);
> + netdev_err(net_dev, "phylink_fwnode_phy_connect() = %d\n", err);
> goto err_phylink_destroy;
> }
>
> @@ -384,8 +400,8 @@ int dpaa2_mac_open(struct dpaa2_mac *mac)
> /* Find the device node representing the MAC device and link the device
> * behind the associated netdev to it.
> */
> - mac->of_node = dpaa2_mac_get_node(mac->attr.id);
> - net_dev->dev.of_node = mac->of_node;
> + mac->fw_node = dpaa2_mac_get_node(&mac->mc_dev->dev, mac->attr.id);
> + net_dev->dev.of_node = to_of_node(mac->fw_node);
>
> return 0;
>
> @@ -399,8 +415,8 @@ void dpaa2_mac_close(struct dpaa2_mac *mac)
> struct fsl_mc_device *dpmac_dev = mac->mc_dev;
>
> dpmac_close(mac->mc_io, 0, dpmac_dev->mc_handle);
> - if (mac->of_node)
> - of_node_put(mac->of_node);
> + if (mac->fw_node)
> + fwnode_handle_put(mac->fw_node);
> }
>
> static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = {
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> index 8ebcb3420d02..7842cbb2207a 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> @@ -24,7 +24,7 @@ struct dpaa2_mac {
> phy_interface_t if_mode;
> enum dpmac_link_type if_link_type;
> struct lynx_pcs *pcs;
> - struct device_node *of_node;
> + struct fwnode_handle *fw_node;
> };
>
> bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
> --
> 2.31.1
>
On 10/06/2021 19:05, Rafael J. Wysocki wrote:
> On Thu, Jun 10, 2021 at 6:40 PM Ioana Ciornei <[email protected]> wrote:
>>
>> From: Calvin Johnson <[email protected]>
>>
>> Introduce ACPI mechanism to get PHYs registered on a MDIO bus and
>> provide them to be connected to MAC.
>
> This is not an "ACPI mechanism", because it is not part of the ACPI
> specification or support documentation thereof.
>
> I would call it "a mechanism based on generic ACPI _DSD device
> properties definition []1]". And provide a reference to the _DSD
> properties definition document.
>
> With that changed, you can add
>
> Acked-by: Rafael J. Wysocki <[email protected]>
>
> to this patch.
>
> Note, however, that within the traditional ACPI framework, the _DSD
> properties are consumed by the driver that binds to the device
> represented by the ACPI device object containing the _DSD in question
> in its scope, while in this case IIUC the properties are expected to
> be consumed by the general networking code in the kernel. That is not
> wrong in principle, but it means that operating systems other than
> Linux are not likely to be using them.
>
Doesn't this land at the level of device drivers though? None of this
data needs to be consumed by the OS generic ACPI parsing code, but the
network device driver can use it to parse the MDIO and MAC configuraiton
and set itself up appropriately.
The only difference in the Linux case is that the code is implemented in
a way that can be leveraged by other network drivers instead of being
entirely contained within the dpaa driver.
g.
>> Describe properties "phy-handle" and "phy-mode".
>>
>> Signed-off-by: Calvin Johnson <[email protected]>
>> Signed-off-by: Ioana Ciornei <[email protected]>
>> ---
>>
>> Changes in v8: None
>> Changes in v7: None
>> Changes in v6:
>> - Minor cleanup
>>
>> Changes in v5:
>> - More cleanup
>>
>> Changes in v4:
>> - More cleanup
>>
>> Changes in v3: None
>> Changes in v2:
>> - Updated with more description in document
>>
>> Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++++
>> 1 file changed, 133 insertions(+)
>> create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
>>
>> diff --git a/Documentation/firmware-guide/acpi/dsd/phy.rst b/Documentation/firmware-guide/acpi/dsd/phy.rst
>> new file mode 100644
>> index 000000000000..7d01ae8b3cc6
>> --- /dev/null
>> +++ b/Documentation/firmware-guide/acpi/dsd/phy.rst
>> @@ -0,0 +1,133 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=========================
>> +MDIO bus and PHYs in ACPI
>> +=========================
>> +
>> +The PHYs on an MDIO bus [1] are probed and registered using
>> +fwnode_mdiobus_register_phy().
>> +
>> +Later, for connecting these PHYs to their respective MACs, the PHYs registered
>> +on the MDIO bus have to be referenced.
>> +
>> +This document introduces two _DSD properties that are to be used
>> +for connecting PHYs on the MDIO bus [3] to the MAC layer.
>> +
>> +These properties are defined in accordance with the "Device
>> +Properties UUID For _DSD" [2] document and the
>> +daffd814-6eba-4d8c-8a91-bc9bbf4aa301 UUID must be used in the Device
>> +Data Descriptors containing them.
>> +
>> +phy-handle
>> +----------
>> +For each MAC node, a device property "phy-handle" is used to reference
>> +the PHY that is registered on an MDIO bus. This is mandatory for
>> +network interfaces that have PHYs connected to MAC via MDIO bus.
>> +
>> +During the MDIO bus driver initialization, PHYs on this bus are probed
>> +using the _ADR object as shown below and are registered on the MDIO bus.
>> +
>> +::
>> + Scope(\_SB.MDI0)
>> + {
>> + Device(PHY1) {
>> + Name (_ADR, 0x1)
>> + } // end of PHY1
>> +
>> + Device(PHY2) {
>> + Name (_ADR, 0x2)
>> + } // end of PHY2
>> + }
>> +
>> +Later, during the MAC driver initialization, the registered PHY devices
>> +have to be retrieved from the MDIO bus. For this, the MAC driver needs
>> +references to the previously registered PHYs which are provided
>> +as device object references (e.g. \_SB.MDI0.PHY1).
>> +
>> +phy-mode
>> +--------
>> +The "phy-mode" _DSD property is used to describe the connection to
>> +the PHY. The valid values for "phy-mode" are defined in [4].
>> +
>> +The following ASL example illustrates the usage of these properties.
>> +
>> +DSDT entry for MDIO node
>> +------------------------
>> +
>> +The MDIO bus has an SoC component (MDIO controller) and a platform
>> +component (PHYs on the MDIO bus).
>> +
>> +a) Silicon Component
>> +This node describes the MDIO controller, MDI0
>> +---------------------------------------------
>> +::
>> + Scope(_SB)
>> + {
>> + Device(MDI0) {
>> + Name(_HID, "NXP0006")
>> + Name(_CCA, 1)
>> + Name(_UID, 0)
>> + Name(_CRS, ResourceTemplate() {
>> + Memory32Fixed(ReadWrite, MDI0_BASE, MDI_LEN)
>> + Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
>> + {
>> + MDI0_IT
>> + }
>> + }) // end of _CRS for MDI0
>> + } // end of MDI0
>> + }
>> +
>> +b) Platform Component
>> +The PHY1 and PHY2 nodes represent the PHYs connected to MDIO bus MDI0
>> +---------------------------------------------------------------------
>> +::
>> + Scope(\_SB.MDI0)
>> + {
>> + Device(PHY1) {
>> + Name (_ADR, 0x1)
>> + } // end of PHY1
>> +
>> + Device(PHY2) {
>> + Name (_ADR, 0x2)
>> + } // end of PHY2
>> + }
>> +
>> +DSDT entries representing MAC nodes
>> +-----------------------------------
>> +
>> +Below are the MAC nodes where PHY nodes are referenced.
>> +phy-mode and phy-handle are used as explained earlier.
>> +------------------------------------------------------
>> +::
>> + Scope(\_SB.MCE0.PR17)
>> + {
>> + Name (_DSD, Package () {
>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>> + Package () {
>> + Package (2) {"phy-mode", "rgmii-id"},
>> + Package (2) {"phy-handle", \_SB.MDI0.PHY1}
>> + }
>> + })
>> + }
>> +
>> + Scope(\_SB.MCE0.PR18)
>> + {
>> + Name (_DSD, Package () {
>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>> + Package () {
>> + Package (2) {"phy-mode", "rgmii-id"},
>> + Package (2) {"phy-handle", \_SB.MDI0.PHY2}}
>> + }
>> + })
>> + }
>> +
>> +References
>> +==========
>> +
>> +[1] Documentation/networking/phy.rst
>> +
>> +[2] https://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>> +
>> +[3] Documentation/firmware-guide/acpi/DSD-properties-rules.rst
>> +
>> +[4] Documentation/devicetree/bindings/net/ethernet-controller.yaml
>> --
>> 2.31.1
>>
On Thu, Jun 10, 2021 at 7:51 PM Jon Nettleton <[email protected]> wrote:
>
> On Thu, Jun 10, 2021 at 6:56 PM Andrew Lunn <[email protected]> wrote:
> >
> > On Thu, Jun 10, 2021 at 07:39:02PM +0300, Ioana Ciornei wrote:
> > > From: Ioana Ciornei <[email protected]>
> > >
> > > This patch set provides ACPI support to DPAA2 network drivers.
> >
> > Just to be clear and avoid confusion, there is a standing NACK against
> > this patchset. Please see the discussion here:
> >
> > https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/#23518385
> >
> > So far, i've not seen any indication the issues raised there have been
> > resolved. I don't see any Acked-by from an ACPI maintainer. So this
> > code remains NACKed.
>
> Andrew,
>
> The ACPI maintainers did bless the use of the ACPI standards followed
> in this patchset, and their only abstinence from ACK'ing the patchset
> was whether the code was used in production systems. Well currently,
> not only have we, SolidRun, been using this patchset and the associated
> ACPI tables in our SystemsReady certified firmware for the HoneyComb,
> but we also have customers using this same patchset and firmware on
> their systems rolled out to customers.
>
> Additionally we have an entire new product line based on Marvell's
> Armada CN913x series, which also needs this patchset to be fully
> functional.
>
> I am quite certain this is more than enough production systems using
> this ACPI description method for networking to progress this patchset
> forward.
And I believe that you have all of the requisite ACKs from the ACPI
side now, so it is up to the networking guys to decide what to do
next.
Thanks,
Rafael
On 10/06/2021 19:25, Rafael J. Wysocki wrote:
> On Thu, Jun 10, 2021 at 7:51 PM Jon Nettleton <[email protected]> wrote:
>>
>> On Thu, Jun 10, 2021 at 6:56 PM Andrew Lunn <[email protected]> wrote:
>>>
>>> On Thu, Jun 10, 2021 at 07:39:02PM +0300, Ioana Ciornei wrote:
>>>> From: Ioana Ciornei <[email protected]>
>>>>
>>>> This patch set provides ACPI support to DPAA2 network drivers.
>>>
>>> Just to be clear and avoid confusion, there is a standing NACK against
>>> this patchset. Please see the discussion here:
>>>
>>> https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/#23518385
>>>
>>> So far, i've not seen any indication the issues raised there have been
>>> resolved. I don't see any Acked-by from an ACPI maintainer. So this
>>> code remains NACKed.
>>
>> Andrew,
>>
>> The ACPI maintainers did bless the use of the ACPI standards followed
>> in this patchset, and their only abstinence from ACK'ing the patchset
>> was whether the code was used in production systems. Well currently,
>> not only have we, SolidRun, been using this patchset and the associated
>> ACPI tables in our SystemsReady certified firmware for the HoneyComb,
>> but we also have customers using this same patchset and firmware on
>> their systems rolled out to customers.
>>
>> Additionally we have an entire new product line based on Marvell's
>> Armada CN913x series, which also needs this patchset to be fully
>> functional.
>>
>> I am quite certain this is more than enough production systems using
>> this ACPI description method for networking to progress this patchset
>> forward.
>
> And I believe that you have all of the requisite ACKs from the ACPI
> side now, so it is up to the networking guys to decide what to do
> next.
You've also my ACK as emeritus Devicetree maintainer. It is well past
time for this series to get merged.
g.
On Thu, Jun 10, 2021 at 8:23 PM Grant Likely <[email protected]> wrote:
>
> On 10/06/2021 19:05, Rafael J. Wysocki wrote:
> > On Thu, Jun 10, 2021 at 6:40 PM Ioana Ciornei <[email protected]> wrote:
> >>
> >> From: Calvin Johnson <[email protected]>
> >>
> >> Introduce ACPI mechanism to get PHYs registered on a MDIO bus and
> >> provide them to be connected to MAC.
> >
> > This is not an "ACPI mechanism", because it is not part of the ACPI
> > specification or support documentation thereof.
> >
> > I would call it "a mechanism based on generic ACPI _DSD device
> > properties definition []1]". And provide a reference to the _DSD
> > properties definition document.
> >
> > With that changed, you can add
> >
> > Acked-by: Rafael J. Wysocki <[email protected]>
> >
> > to this patch.
> >
> > Note, however, that within the traditional ACPI framework, the _DSD
> > properties are consumed by the driver that binds to the device
> > represented by the ACPI device object containing the _DSD in question
> > in its scope, while in this case IIUC the properties are expected to
> > be consumed by the general networking code in the kernel. That is not
> > wrong in principle, but it means that operating systems other than
> > Linux are not likely to be using them.
> >
>
> Doesn't this land at the level of device drivers though? None of this
> data needs to be consumed by the OS generic ACPI parsing code, but the
> network device driver can use it to parse the MDIO and MAC configuraiton
> and set itself up appropriately.
That's right in general, which is why I said that doing it this way
wasn't wrong.
> And I believe that you have all of the requisite ACKs from the ACPI
> side now, so it is up to the networking guys to decide what to do
> next.
Thanks for the Acked-by's.
Since they were missing, the networking guys have deliberately been
ignoring this code. Now they have been given, we will start the review
work.
Thanks
Andrew
On Thu, Jun 10, 2021 at 7:40 PM Ioana Ciornei <[email protected]> wrote:
>
> From: Calvin Johnson <[email protected]>
>
> Depending on the device node type, call the specific OF or ACPI
> mdiobus_register function.
>
> Note: For both ACPI and DT cases, endianness of MDIO controller
controllers
> need to be specified using "little-endian" property.
using the
...
> Changes in v8:
> - Directly call the OF or ACPI variants of registering the MDIO bus.
> This is needed because the fwnode_mdio.c module should only implement
> features which can be achieved without going back to the OF/ACPI
> variants. Without this restrictions we directly end up in a dependency
> cycle: of_mdio -> fwnode_mdio -> of_mdio.
Shouldn't be simple fwnode_mdio.h.
The idea of fwnode APIs that they provide a common ground for all
resource providers.
> - Changed the commit title since the fwnode_mdiobus_register() is no
> longer available
>
> Changes in v7:
> - Include fwnode_mdio.h
> - Alphabetically sort header inclusions
I suppose this should be a separate change.
--
With Best Regards,
Andy Shevchenko
On Thu, Jun 10, 2021 at 7:40 PM Ioana Ciornei <[email protected]> wrote:
>
> From: Ioana Ciornei <[email protected]>
>
> 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_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.
In general it looks fine to me. What really worries me is the calls like
of_foo -> fwnode_bar -> of_baz.
As I have commented in one patch the idea of fwnode APIs is to have a
common ground for all resource providers. So, at the end it shouldn't
be a chain of calls like above mentioned. Either fix the name (so, the
first one will be in fwnode or device namespace) or fix the API that
it will be fwnode/device API.
--
With Best Regards,
Andy Shevchenko
On Fri, Jun 11, 2021 at 12:00:02PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 10, 2021 at 7:40 PM Ioana Ciornei <[email protected]> wrote:
> >
> > From: Ioana Ciornei <[email protected]>
> >
> > 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_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.
>
> In general it looks fine to me. What really worries me is the calls like
>
> of_foo -> fwnode_bar -> of_baz.
>
> As I have commented in one patch the idea of fwnode APIs is to have a
> common ground for all resource providers. So, at the end it shouldn't
> be a chain of calls like above mentioned. Either fix the name (so, the
> first one will be in fwnode or device namespace) or fix the API that
> it will be fwnode/device API.
These types of cyclic calls do not exist anymore.
The fwnode_mdio does not call back into of_mdio but instead it directly
implements any necessary operations using the fwnode_handle.
The only calls happening are 'of_mdio -> fwnode_mdio' so that we
leverage the common fwnode handling and do not duplicate code.
Ioana