diff mbox series

[14/15] net: sun8i-emac: Lower MDIO frequency

Message ID 20200706004046.20842-15-andre.przywara@arm.com
State Accepted
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series net: sun8i-emac fixes and cleanups | expand

Commit Message

André Przywara July 6, 2020, 12:40 a.m. UTC
When sending a command via the MDIO bus, the Designware MAC expects some
bits in the CMD register to describe the clock divider value between
the main clock and the MDIO clock.
So far we were omitting these bits, resulting in setting "00", which
means "/ 16", so ending up with an MDIO frequency of either 18.75 or
12.5 MHz.
All the internal PHYs in the H3/H5/H6 SoCs as well as the Gbit Realtek
PHYs seem to be fine with that - although it looks like to be severly
overclocked (the MDIO spec limits the frequency to 2.5 MHz).
However the external 100Mbit PHY on the Pine64 (non-plus) board is
not happy with that, Ethernet was actually never working there, as the
PHY didn't probe.

As we set the EMAC clock (via AHB2) to 300 MHz in ATF (on the 64-bit
SoCs), and use 200 MHz on the H3, we need the highest divider of 128
to let the MDIO clock end up below the required 2.5 MHz.

This enables Ethernet on the Pine64(non-plus).

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/net/sun8i_emac.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Jagan Teki July 11, 2020, 9:27 a.m. UTC | #1
On Mon, Jul 6, 2020 at 6:12 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> When sending a command via the MDIO bus, the Designware MAC expects some
> bits in the CMD register to describe the clock divider value between
> the main clock and the MDIO clock.
> So far we were omitting these bits, resulting in setting "00", which
> means "/ 16", so ending up with an MDIO frequency of either 18.75 or
> 12.5 MHz.
> All the internal PHYs in the H3/H5/H6 SoCs as well as the Gbit Realtek
> PHYs seem to be fine with that - although it looks like to be severly
> overclocked (the MDIO spec limits the frequency to 2.5 MHz).
> However the external 100Mbit PHY on the Pine64 (non-plus) board is
> not happy with that, Ethernet was actually never working there, as the
> PHY didn't probe.

How come the existing divider cannot work with 100Mbit external
PHY(assuming external regulator pin as properly enabled) since it
works with 1Gbit already?

Jagan.
André Przywara July 11, 2020, 11:53 p.m. UTC | #2
On 11/07/2020 10:27, Jagan Teki wrote:

Hi,

> On Mon, Jul 6, 2020 at 6:12 AM Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> When sending a command via the MDIO bus, the Designware MAC expects some
>> bits in the CMD register to describe the clock divider value between
>> the main clock and the MDIO clock.
>> So far we were omitting these bits, resulting in setting "00", which
>> means "/ 16", so ending up with an MDIO frequency of either 18.75 or
>> 12.5 MHz.
>> All the internal PHYs in the H3/H5/H6 SoCs as well as the Gbit Realtek
>> PHYs seem to be fine with that - although it looks like to be severly
>> overclocked (the MDIO spec limits the frequency to 2.5 MHz).
>> However the external 100Mbit PHY on the Pine64 (non-plus) board is
>> not happy with that, Ethernet was actually never working there, as the
>> PHY didn't probe.
> 
> How come the existing divider cannot work with 100Mbit external
> PHY(assuming external regulator pin as properly enabled) since it
> works with 1Gbit already?

Because it's far too high to be MDIO spec compliant. My guess is that
some PHYs (for instance the RTL8211 used on most boards with GBit
Ethernet) can cope with that, but apparently that's too much for the
RTL8201 on the Pine64. I would say that most boards have either an
external GBit PHY or use the internal PHY for 100MBit, so there are not
many boards using the sun8i EMAC with an 8201 PHY, that's why nobody
complained so far.

At least this is my best guess, based on the observation that this patch
makes the difference between Ethernet working or not on the Pine64
(non-plus).

Cheers,
Andre.
diff mbox series

Patch

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index dc716f94f3..b2007b4c1d 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -41,6 +41,11 @@ 
 #define MDIO_CMD_MII_PHY_REG_ADDR_SHIFT	4
 #define MDIO_CMD_MII_PHY_ADDR_MASK	0x0001f000
 #define MDIO_CMD_MII_PHY_ADDR_SHIFT	12
+#define MDIO_CMD_MII_CLK_CSR_DIV_16	0x0
+#define MDIO_CMD_MII_CLK_CSR_DIV_32	0x1
+#define MDIO_CMD_MII_CLK_CSR_DIV_64	0x2
+#define MDIO_CMD_MII_CLK_CSR_DIV_128	0x3
+#define MDIO_CMD_MII_CLK_CSR_SHIFT	20
 
 #define CONFIG_TX_DESCR_NUM	32
 #define CONFIG_RX_DESCR_NUM	32
@@ -199,6 +204,12 @@  static int sun8i_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
 	mii_cmd |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
+	/*
+	 * The EMAC clock is either 200 or 300 MHz, so we need a divider
+	 * of 128 to get the MDIO frequency below the required 2.5 MHz.
+	 */
+	mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 << MDIO_CMD_MII_CLK_CSR_SHIFT;
+
 	mii_cmd |= MDIO_CMD_MII_BUSY;
 
 	writel(mii_cmd, priv->mac_reg + EMAC_MII_CMD);
@@ -224,6 +235,12 @@  static int sun8i_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
 	mii_cmd |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
+	/*
+	 * The EMAC clock is either 200 or 300 MHz, so we need a divider
+	 * of 128 to get the MDIO frequency below the required 2.5 MHz.
+	 */
+	mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 << MDIO_CMD_MII_CLK_CSR_SHIFT;
+
 	mii_cmd |= MDIO_CMD_MII_WRITE;
 	mii_cmd |= MDIO_CMD_MII_BUSY;