RE: [net-next,PATCH 2/2] enetc: Make eMDIO MDC rate configurable from DT
From: Wei Fang
Date: Tue Oct 20 2026 - 03:44:42 EST
> >>> For ENETC device, we already have a 'ref' clock in the enetc node, so
> >>> devm_clk_get(dev, NULL) may get the 'ref' clock rather than the system
> >>> clock of NETC.
> >> Shouldn't the clock be described in the MDIO DT node ?
> >
> > The enetc_mdio_read_* and enetc_mdio_write_* interfaces are not only
> > used by EMDIO devices, the ENETCs and NETC switches also use them.
> > NETC switches and ENETCs can access external PHYs or internal PCS/Serdes
> > through their own port MDIO registers, instead of through EMDIO devices.
> >
> > Your first patch only modified the EMDIO binding-doc, not the ENETC and
> > switch binding-doc.
> So I need to patch also
> Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml ? Any other
> binding document ?
>
fsl,enetc.yaml and nxp,netc-switch.yaml.
However, once again, I don't highly recommend adding a clocks property to
the DTS to obtain the MDC source clock frequency. My reasons are as follows:
1. The MDC source clock is the NETC system clock, which is a fixed configuration
with a fixed frequency for each SoC, hardcode the rate in driver is simpler.
2. ENETC, EMDIO, and the NETC switch may all use the MDIO bus. Whether to
add the clocks property to these nodes depends on the requirements. This clocks
property should not be added to the shared .dtsi file; it needs to be added to
the .dts file. This means that adding this property might be necessary for each
new .dts file, which is cumbersome.
3. This clocks property is not a required property. If the node doesn't have this
property, your approach is to set the MDC to a low frequency. As Andrew's
point out, some PHYs need to download firmware via the MDIO bus. The lower
the MDC frequency, the longer the download time. Therefore, it's recommended
to set the MDC clock to the maximum value of 2.5MHz specified by 802.3 by
default. However, without the clocks property in the node, we cannot obtain
the NETC system clock. And then we cannot set the MDC rate to 2.5Mhz.
Therefore, I prefer to hardcode the system clock frequency for each version
of NETC in the driver, and obtain the corresponding clock frequency through
hardware revision during driver loading. I have already completed a patch,
as follows:
---------------------------------------------------------------------------------------------------
diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
index bef19d7d8f16..0b24d4bf3e19 100644
--- a/drivers/net/dsa/netc/netc_main.c
+++ b/drivers/net/dsa/netc/netc_main.c
@@ -214,6 +214,11 @@ static int netc_port_create_emdio_bus(struct netc_port *np, int port,
snprintf(bus->id, MII_BUS_ID_SIZE, "%s-p%d-emdio",
dev_name(dev), port);
+ err = enetc_mdio_clock_div_set(bus, priv->revision,
+ of_fwnode_handle(node));
+ if (err)
+ goto free_mdiobus;
+
err = of_mdiobus_register(bus, node);
if (err) {
dev_err_probe(dev, err, "Cannot register EMDIO bus\n");
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 8387dd208adb..46414f30469f 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1010,6 +1010,12 @@ static int vsc9959_mdio_bus_alloc(struct ocelot *ocelot)
mdio_priv->mdio_base = 0;
snprintf(bus->id, MII_BUS_ID_SIZE, "%s-imdio", dev_name(dev));
+ rc = enetc_mdio_clock_div_set(bus, ENETC_MDIO_REV(1, 0), NULL);
+ if (rc) {
+ mdiobus_free(bus);
+ return rc;
+ }
+
/* Needed in order to initialize the bus mutex lock */
rc = mdiobus_register(bus);
if (rc < 0) {
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index a90a04a47bef..1ca6dafc2b8d 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -14,7 +14,7 @@
#define ENETC_MDIO_DATA 0x8 /* MDIO data */
#define ENETC_MDIO_ADDR 0xc /* MDIO address */
-#define MDIO_CFG_CLKDIV(x) ((((x) >> 1) & 0xff) << 8)
+#define MDIO_CFG_CLKDIV GENMASK(15, 7)
#define MDIO_CFG_BSY BIT(0)
#define MDIO_CFG_RD_ER BIT(1)
#define MDIO_CFG_HOLD(x) (((x) << 2) & GENMASK(4, 2))
@@ -22,15 +22,18 @@
/* external MDIO only - driven on neg MDC edge */
#define MDIO_CFG_NEG BIT(23)
-#define ENETC_EMDIO_CFG \
+#define ENETC_EMDIO_CFG(div) \
(MDIO_CFG_HOLD(2) | \
- MDIO_CFG_CLKDIV(66) | \
+ FIELD_PREP(MDIO_CFG_CLKDIV, div) | \
MDIO_CFG_NEG)
#define MDIO_CTL_DEV_ADDR(x) ((x) & 0x1f)
#define MDIO_CTL_PORT_ADDR(x) (((x) & 0x1f) << 5)
#define MDIO_CTL_READ BIT(15)
+#define ENETC_MDC_SRC_400M 400000000U
+#define ENETC_MDC_SRC_333M 333333333U
+
static inline u32 enetc_mdio_rd(struct enetc_mdio_priv *mdio_priv, int off)
{
return enetc_port_rd_mdio(mdio_priv->hw, mdio_priv->mdio_base + off);
@@ -55,6 +58,49 @@ static int enetc_mdio_wait_complete(struct enetc_mdio_priv *mdio_priv)
is_busy, !is_busy, 10, 10 * 1000);
}
+static u32 enetc_mdio_source_clock_rate(u16 revision)
+{
+ switch (revision) {
+ case ENETC_MDIO_REV(1, 0):
+ return ENETC_MDC_SRC_400M;
+ case ENETC_MDIO_REV(4, 1):
+ case ENETC_MDIO_REV(4, 3):
+ return ENETC_MDC_SRC_333M;
+ default:
+ return 0;
+ }
+}
+
+int enetc_mdio_clock_div_set(struct mii_bus *bus, u16 revision,
+ struct fwnode_handle *fwnode)
+{
+ u32 src_clk_rate = enetc_mdio_source_clock_rate(revision);
+ struct enetc_mdio_priv *mdio_priv = bus->priv;
+ struct device *dev = bus->parent;
+ u32 div, mdc_freq = 2500000;
+
+ if (!src_clk_rate)
+ return dev_err_probe(dev, -EINVAL,
+ "The MDC source clock rate is 0\n");
+
+ fwnode_property_read_u32(fwnode, "clock-frequency", &mdc_freq);
+ if (!mdc_freq || mdc_freq > src_clk_rate / 2)
+ return dev_err_probe(dev, -EINVAL,
+ "The MDC rate %u is out of range\n",
+ mdc_freq);
+
+ div = DIV_ROUND_UP(src_clk_rate, 2 * mdc_freq) - 1;
+ if (div < 5 || div > 0x1ff)
+ return dev_err_probe(dev, -EINVAL,
+ "MDIO_CLK_DIV %u is out of range\n",
+ div);
+
+ mdio_priv->mdc_div = div;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(enetc_mdio_clock_div_set);
+
int enetc_mdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
u16 value)
{
@@ -63,7 +109,7 @@ int enetc_mdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
u16 dev_addr;
int ret;
- mdio_cfg = ENETC_EMDIO_CFG;
+ mdio_cfg = ENETC_EMDIO_CFG(mdio_priv->mdc_div);
dev_addr = regnum & 0x1f;
mdio_cfg &= ~MDIO_CFG_ENC45;
@@ -95,7 +141,7 @@ int enetc_mdio_write_c45(struct mii_bus *bus, int phy_id, int dev_addr,
u32 mdio_ctl, mdio_cfg;
int ret;
- mdio_cfg = ENETC_EMDIO_CFG;
+ mdio_cfg = ENETC_EMDIO_CFG(mdio_priv->mdc_div);
mdio_cfg |= MDIO_CFG_ENC45;
enetc_mdio_wr(mdio_priv, ENETC_MDIO_CFG, mdio_cfg);
@@ -133,7 +179,7 @@ int enetc_mdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
u16 dev_addr, value;
int ret;
- mdio_cfg = ENETC_EMDIO_CFG;
+ mdio_cfg = ENETC_EMDIO_CFG(mdio_priv->mdc_div);
dev_addr = regnum & 0x1f;
mdio_cfg &= ~MDIO_CFG_ENC45;
@@ -176,7 +222,7 @@ int enetc_mdio_read_c45(struct mii_bus *bus, int phy_id, int dev_addr,
u16 value;
int ret;
- mdio_cfg = ENETC_EMDIO_CFG;
+ mdio_cfg = ENETC_EMDIO_CFG(mdio_priv->mdc_div);
mdio_cfg |= MDIO_CFG_ENC45;
enetc_mdio_wr(mdio_priv, ENETC_MDIO_CFG, mdio_cfg);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
index e108cac8288d..bf3ca885c86f 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
@@ -11,6 +11,14 @@
#define ENETC_MDIO_BUS_NAME ENETC_MDIO_DEV_NAME " Bus"
#define ENETC_MDIO_DRV_NAME ENETC_MDIO_DEV_NAME " driver"
+/* The EMDIO PCI function of both NETC v1 (LS1028A) and NETC v4 has a
+ * global register block at offset 0x10000 of BAR0, which is always
+ * covered by the BAR0 size (128 KB).
+ */
+#define EMDIO_GLOBAL_OFFSET 0x10000
+#define EMDIO_GLOBAL_IPBRR0 0xbf8
+#define IPBRR0_IP_REV GENMASK(15, 0)
+
DEFINE_STATIC_KEY_FALSE(enetc_has_err050089);
EXPORT_SYMBOL_GPL(enetc_has_err050089);
@@ -33,6 +41,16 @@ static void enetc_emdio_disable_err050089(struct pci_dev *pdev)
}
}
+static u16 enetc_emdio_get_revision(struct enetc_hw *hw)
+{
+ int offset = EMDIO_GLOBAL_OFFSET + EMDIO_GLOBAL_IPBRR0;
+ u32 val;
+
+ val = enetc_port_rd_mdio(hw, offset);
+
+ return FIELD_GET(IPBRR0_IP_REV, val);
+}
+
static int enetc_pci_mdio_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
@@ -41,6 +59,7 @@ static int enetc_pci_mdio_probe(struct pci_dev *pdev,
void __iomem *port_regs;
struct enetc_hw *hw;
struct mii_bus *bus;
+ u16 rev;
int err;
port_regs = pci_iomap(pdev, 0, 0);
@@ -87,6 +106,10 @@ static int enetc_pci_mdio_probe(struct pci_dev *pdev,
}
enetc_emdio_enable_err050089(pdev);
+ rev = enetc_emdio_get_revision(hw);
+ err = enetc_mdio_clock_div_set(bus, rev, dev_fwnode(dev));
+ if (err)
+ goto err_mdc_div_set;
err = of_mdiobus_register(bus, dev->of_node);
if (err)
@@ -97,6 +120,7 @@ static int enetc_pci_mdio_probe(struct pci_dev *pdev,
return 0;
err_mdiobus_reg:
+err_mdc_div_set:
enetc_emdio_disable_err050089(pdev);
pci_release_region(pdev, 0);
err_pci_mem_reg:
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
index 8206884294a4..308b7ce2a70a 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
@@ -308,6 +308,11 @@ static int enetc_mdio_probe(struct enetc_pf *pf, struct device_node *np)
snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+ err = enetc_mdio_clock_div_set(bus, pf->si->revision,
+ of_fwnode_handle(np));
+ if (err)
+ return err;
+
err = of_mdiobus_register(bus, np);
if (err)
return dev_err_probe(dev, err, "cannot register MDIO bus\n");
@@ -358,6 +363,10 @@ static int enetc_imdio_create(struct enetc_pf *pf)
snprintf(bus->id, MII_BUS_ID_SIZE, "%s-imdio", dev_name(dev));
+ err = enetc_mdio_clock_div_set(bus, pf->si->revision, NULL);
+ if (err)
+ goto free_mdio_bus;
+
err = mdiobus_register(bus);
if (err) {
dev_err(dev, "cannot register internal MDIO bus (%d)\n", err);
diff --git a/include/linux/fsl/enetc_mdio.h b/include/linux/fsl/enetc_mdio.h
index 623ccfcbf39c..edc0a9730e63 100644
--- a/include/linux/fsl/enetc_mdio.h
+++ b/include/linux/fsl/enetc_mdio.h
@@ -4,6 +4,7 @@
#ifndef _FSL_ENETC_MDIO_H_
#define _FSL_ENETC_MDIO_H_
+#include <linux/bitfield.h>
#include <linux/phy.h>
/* PCS registers */
@@ -17,6 +18,11 @@
#define ENETC_PCS_IF_MODE_SGMII_SPEED(x) (((x) << 2) & GENMASK(3, 2))
#define ENETC_PCS_IF_MODE_DUPLEX_HALF BIT(3)
+#define ENETC_MDIO_REV_MAJOR GENMASK(15, 8)
+#define ENETC_MDIO_REV_MINOR GENMASK(7, 0)
+#define ENETC_MDIO_REV(m, n) (FIELD_PREP_CONST(ENETC_MDIO_REV_MAJOR, m) | \
+ FIELD_PREP_CONST(ENETC_MDIO_REV_MINOR, n))
+
/* Not a mistake, the SerDes PLL needs to be set at 3.125 GHz by Reset
* Configuration Word (RCW, outside Linux control) for 2.5G SGMII mode. The PCS
* still thinks it's at gigabit.
@@ -33,6 +39,7 @@ struct enetc_hw;
struct enetc_mdio_priv {
struct enetc_hw *hw;
int mdio_base;
+ u32 mdc_div;
};
#if IS_REACHABLE(CONFIG_FSL_ENETC_MDIO)
@@ -44,6 +51,8 @@ int enetc_mdio_read_c45(struct mii_bus *bus, int phy_id, int devad, int regnum);
int enetc_mdio_write_c45(struct mii_bus *bus, int phy_id, int devad, int regnum,
u16 value);
struct enetc_hw *enetc_hw_alloc(struct device *dev, void __iomem *port_regs);
+int enetc_mdio_clock_div_set(struct mii_bus *bus, u16 revision,
+ struct fwnode_handle *fwnode);
#else
@@ -63,6 +72,10 @@ static inline struct enetc_hw *enetc_hw_alloc(struct device *dev,
void __iomem *port_regs)
{ return ERR_PTR(-EINVAL); }
+static inline int enetc_mdio_clock_div_set(struct mii_bus *bus, u16 revision,
+ struct fwnode_handle *fwnode)
+{ return -EINVAL; }
+
#endif
#endif