net: fec_mxc: configure MDIO hold time

PATCH New Mans Rullgard 3 participants 7 messages Dec 8, 2015 – Jan 3, 2016 Network drivers

Share & tools

Mans Rullgard

author

2015-12-08 15:38 UTC #0 lore raw

PATCH 1/2 net: fec_mxc: configure MDIO hold time

If the host clock frequency is higher than 100 MHz, the MDIO hold time needs to be increased from its current setting of one cycle in order to meet the specified minium of 10 ns. Writing an appropriate value to the HOLDTIME field of the MII_SPEED register achieves this.

Comment copied from Linux kernel.

Signed-off-by Mans Rullgard <[email protected]>

---
 drivers/net/fec_mxc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

drivers/net/fec_mxc.c +15 -3

index 79f6737..1250d2a 100644
@@ -131,13 +131,25 @@ static void fec_mii_setspeed(struct ethernet_regs *eth)
 	/*
 	 * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
 	 * and do not drop the Preamble.
+	 *
+	 * The i.MX28 and i.MX6 types have another filed in the MSCR (aka
+	 * MII_SPEED) register that defines the MDIO output hold time. Earlier
+	 * versions are RAZ there, so just ignore the difference and write the
+	 * register always.
+	 * The minimal hold time according to IEE802.3 (clause 22) is 10 ns.
+	 * HOLDTIME + 1 is the number of clk cycles the fec is holding the
+	 * output.
+	 * The HOLDTIME bitfield takes values between 0 and 7 (inclusive).
+	 * Given that ceil(clkrate / 5000000) <= 64, the calculation for
+	 * holdtime cannot result in a value greater than 3.
 	 */
-	register u32 speed = DIV_ROUND_UP(imx_get_fecclk(), 5000000);
+	u32 pclk = imx_get_fecclk();
+	u32 speed = DIV_ROUND_UP(pclk, 5000000);
+	u32 hold = DIV_ROUND_UP(pclk, 100000000) - 1;
 #ifdef FEC_QUIRK_ENET_MAC
 	speed--;
 #endif
-	speed <<= 1;
-	writel(speed, &eth->mii_speed);
+	writel(speed << 1 | hold << 8, &eth->mii_speed);
 	debug("%s: mii_speed %08x\n", __func__, readl(&eth->mii_speed));
 }

Signature · 1 line

2.6.3

Mans Rullgard

author

2015-12-08 15:38 UTC #1 lore raw

PATCH 2/2 net: fec_mxc: unregister mdio bus on probe error

If fecmxc_initialize_multi() fails, it frees but does not unregister the mdio bus, causing subsequent uses of the "mii" command to crash. Fix this by adding mdio_unregister() calls where needed.

Signed-off-by Mans Rullgard <[email protected]>

---
 drivers/net/fec_mxc.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/net/fec_mxc.c +2 -0

index 1250d2a..6c5e80b 100644
@@ -1109,6 +1109,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 	phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
 	if (!phydev) {
+		mdio_unregister(bus);
 		free(bus);
 		return -ENOMEM;
 	}

@@ -1120,6 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 		free(phydev);
 #endif
+		mdio_unregister(bus);
 		free(bus);
 	}
 	return ret;

Signature · 1 line

2.6.3

Eric Nelson

2015-12-08 16:17 UTC #2 lore raw

PATCH 2/2 net: fec_mxc: unregister mdio bus on probe error

Hi Mans,

Quoted 14 lines · Mans Rullgard

If fecmxc_initialize_multi() fails, it frees but does not unregister the mdio bus, causing subsequent uses of the "mii" command to crash. Fix this by adding mdio_unregister() calls where needed.
Signed-off-by Mans Rullgard <[email protected]>
---
 drivers/net/fec_mxc.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/fec_mxc.c +0 -0
index 1250d2a..6c5e80b 100644

drivers/net/fec_mxc.c int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr) · line 1112

Quoted 4 lines

drivers/net/fec_mxc.c +1 -0

@@ -1109,6 +1109,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 	phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
 	if (!phydev) {
+		mdio_unregister(bus);

While you're in here, this should probably be mdio_free just to prevent somebody else from searching as I did.

drivers/net/fec_mxc.c int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr) · line 1124

Quoted 8 lines

drivers/net/fec_mxc.c +1 -0

 		free(bus);
 		return -ENOMEM;
 	}

@@ -1120,6 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 		free(phydev);
 #endif
+		mdio_unregister(bus);

Ditto:

drivers/net/fec_mxc.c int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr) · line 1127

Quoted 4 lines

drivers/net/fec_mxc.c +0 -0
 		free(bus);
 	}
 	return ret;

Otherwise,

Reviewed-by Eric Nelson <[email protected]>

Måns Rullgård

author

2015-12-08 16:21 UTC #3 lore raw

PATCH 2/2 net: fec_mxc: unregister mdio bus on probe error

Quoted 25 lines · Eric Nelson

Hi Mans,
Quoted 14 lines · Mans Rullgard
If fecmxc_initialize_multi() fails, it frees but does not unregister the mdio bus, causing subsequent uses of the "mii" command to crash. Fix this by adding mdio_unregister() calls where needed.
Signed-off-by Mans Rullgard <[email protected]>
---
 drivers/net/fec_mxc.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/fec_mxc.c +0 -0
index 1250d2a..6c5e80b 100644
drivers/net/fec_mxc.c int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr) · line 1112
Quoted 4 lines
drivers/net/fec_mxc.c +1 -0
@@ -1109,6 +1109,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 	phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
 	if (!phydev) {
+		mdio_unregister(bus);
While you're in here, this should probably be mdio_free just to prevent somebody else from searching as I did.

Quoted 14 lines

drivers/net/fec_mxc.c int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr) · line 1124
Quoted 8 lines
drivers/net/fec_mxc.c +1 -0
 		free(bus);
 		return -ENOMEM;
 	}
@@ -1120,6 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 #ifdef CONFIG_PHYLIB
 		free(phydev);
 #endif
+		mdio_unregister(bus);
Ditto:
Quoted 4 lines
drivers/net/fec_mxc.c +0 -0
 		free(bus);
 	}
 	return ret;

Agreed, but that's a separate issue.

Signature · 2 lines

M?ns Rullg?rd
mans at mansr.com

Stefano Babic

2016-01-03 14:28 UTC #4 lore raw

PATCH 2/2 net: fec_mxc: unregister mdio bus on probe error

In reply to Mans Rullgard

Quoted 6 lines · Mans Rullgard

If fecmxc_initialize_multi() fails, it frees but does not unregister the mdio bus, causing subsequent uses of the "mii" command to crash. Fix this by adding mdio_unregister() calls where needed.
Signed-off-by Mans Rullgard <[email protected]>
---

Applied to u-boot-imx, thanks!

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Eric Nelson

2015-12-08 16:08 UTC #5 lore raw

PATCH 1/2 net: fec_mxc: configure MDIO hold time

In reply to Mans Rullgard

Hi Mans,

Quoted 17 lines · Mans Rullgard

If the host clock frequency is higher than 100 MHz, the MDIO hold time needs to be increased from its current setting of one cycle in order to meet the specified minium of 10 ns. Writing an appropriate value to the HOLDTIME field of the MII_SPEED register achieves this.
Comment copied from Linux kernel.
Signed-off-by Mans Rullgard <[email protected]>
---
 drivers/net/fec_mxc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
drivers/net/fec_mxc.c +0 -0
index 79f6737..1250d2a 100644

drivers/net/fec_mxc.c static void fec_mii_setspeed(struct ethernet_regs *eth) · line 134

Quoted 4 lines

drivers/net/fec_mxc.c +1 -0

@@ -131,13 +131,25 @@ static void fec_mii_setspeed(struct ethernet_regs *eth)
 	/*
 	 * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
 	 * and do not drop the Preamble.
+	 *

s/filed/field/

Quoted 15 lines

drivers/net/fec_mxc.c +12 -1

+	 * The i.MX28 and i.MX6 types have another filed in the MSCR (aka
+	 * MII_SPEED) register that defines the MDIO output hold time. Earlier
+	 * versions are RAZ there, so just ignore the difference and write the
+	 * register always.
+	 * The minimal hold time according to IEE802.3 (clause 22) is 10 ns.
+	 * HOLDTIME + 1 is the number of clk cycles the fec is holding the
+	 * output.
+	 * The HOLDTIME bitfield takes values between 0 and 7 (inclusive).
+	 * Given that ceil(clkrate / 5000000) <= 64, the calculation for
+	 * holdtime cannot result in a value greater than 3.
 	 */
-	register u32 speed = DIV_ROUND_UP(imx_get_fecclk(), 5000000);
+	u32 pclk = imx_get_fecclk();
+	u32 speed = DIV_ROUND_UP(pclk, 5000000);

drivers/net/fec_mxc.c static void fec_mii_setspeed(struct ethernet_regs *eth) · line 155

Quoted 10 lines

drivers/net/fec_mxc.c +2 -2

+	u32 hold = DIV_ROUND_UP(pclk, 100000000) - 1;
 #ifdef FEC_QUIRK_ENET_MAC
 	speed--;
 #endif
-	speed <<= 1;
-	writel(speed, &eth->mii_speed);
+	writel(speed << 1 | hold << 8, &eth->mii_speed);
 	debug("%s: mii_speed %08x\n", __func__, readl(&eth->mii_speed));
 }

Otherwise,

Reviewed-by Eric Nelson <[email protected]>

Stefano Babic

2016-01-03 14:27 UTC #6 lore raw

PATCH 1/2 net: fec_mxc: configure MDIO hold time

Quoted 15 lines · Eric Nelson

Hi Mans,
Quoted 12 lines · Mans Rullgard
If the host clock frequency is higher than 100 MHz, the MDIO hold time needs to be increased from its current setting of one cycle in order to meet the specified minium of 10 ns. Writing an appropriate value to the HOLDTIME field of the MII_SPEED register achieves this.
Comment copied from Linux kernel.
Signed-off-by Mans Rullgard <[email protected]>
---
 drivers/net/fec_mxc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Quoted 11 lines

drivers/net/fec_mxc.c static void fec_mii_setspeed(struct ethernet_regs *eth) · line 134
Quoted 9 lines
drivers/net/fec_mxc.c +1 -0
index 79f6737..1250d2a 100644
@@ -131,13 +131,25 @@ static void fec_mii_setspeed(struct ethernet_regs *eth)
 	/*
 	 * Set MII_SPEED = (1/(mii_speed * 2)) * System Clock
 	 * and do not drop the Preamble.
+	 *
s/filed/field/

Fixed by applying.

Applied to u-boot-imx, thanks!

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================