All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write
@ 2017-02-17 17:46 Philipp Tomsich
  2017-02-17 17:46 ` [U-Boot] [PATCH v1 1/2] sun8i_emac: Set " Philipp Tomsich
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Philipp Tomsich @ 2017-02-17 17:46 UTC (permalink / raw)
  To: u-boot

To ensure compatibility with all PHYs, we need to keep the MDIO clock
(MDC) below 2.5MHz (the guaranteed operating limit from IEEE 802.3),
even if some PHYs will tolerate higher speeds.

This changeset also cleans up the MDIO read/write functions by
removing pointless bit-masking in a variable initialised to 0.




Philipp Tomsich (2):
  sun8i_emac: Set MDC divider for MDIO read/write
  sun8i_emac: remove unnecessary bit-masking for mdio_read/write

 drivers/net/sun8i_emac.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1 1/2] sun8i_emac: Set MDC divider for MDIO read/write
  2017-02-17 17:46 [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write Philipp Tomsich
@ 2017-02-17 17:46 ` Philipp Tomsich
  2017-02-17 17:46 ` [U-Boot] [PATCH v1 2/2] sun8i_emac: remove unnecessary bit-masking for mdio_read/write Philipp Tomsich
  2017-02-21 17:59 ` [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write Maxime Ripard
  2 siblings, 0 replies; 5+ messages in thread
From: Philipp Tomsich @ 2017-02-17 17:46 UTC (permalink / raw)
  To: u-boot

The IEEE 802.3 standard guarantees operation of the MDIO signals at up
to 2.5MHz (anything above this is a vendor-specific feature, although
most PHYs work at higher frequencies).  With the EMAC being fed by a
(typically) 300MHz clock (e.g. on the A64 this is AHB2, which should
be kept at 300MHz according to the CCU documentation), we need to use
the divide-by-128 setting to get us below 2.5MHz.

The ~2.34MHz clock signal (i.e. assuring that the MDC clock is indeed
derived from the AHB2 clock) has been verified using a A64-uQ7.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 drivers/net/sun8i_emac.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index b87210b..5ae17b7 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -1,92 +1,98 @@
 /*
  * (C) Copyright 2016
  * Author: Amit Singh Tomar, amittomer25 at gmail.com
  *
  * SPDX-License-Identifier:     GPL-2.0+
  *
  * Ethernet driver for H3/A64/A83T based SoC's
  *
  * It is derived from the work done by
  * LABBE Corentin & Chen-Yu Tsai for Linux, THANKS!
  *
 */
 
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/gpio.h>
 #include <common.h>
 #include <dm.h>
 #include <fdt_support.h>
 #include <linux/err.h>
 #include <malloc.h>
 #include <miiphy.h>
 #include <net.h>
 
 #define MDIO_CMD_MII_BUSY		BIT(0)
 #define MDIO_CMD_MII_WRITE		BIT(1)
 
 #define MDIO_CMD_MII_PHY_REG_ADDR_MASK	0x000001f0
 #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_MDC_DIV_RATIO_M_SHIFT  20
+#define MDIO_CMD_MDC_DIV_16             (0 << MDIO_CMD_MDC_DIV_RATIO_M_SHIFT)
+#define MDIO_CMD_MDC_DIV_32             (1 << MDIO_CMD_MDC_DIV_RATIO_M_SHIFT)
+#define MDIO_CMD_MDC_DIV_64             (2 << MDIO_CMD_MDC_DIV_RATIO_M_SHIFT)
+#define MDIO_CMD_MDC_DIV_128            (3 << MDIO_CMD_MDC_DIV_RATIO_M_SHIFT)
+
 #define CONFIG_TX_DESCR_NUM	32
 #define CONFIG_RX_DESCR_NUM	32
 #define CONFIG_ETH_BUFSIZE	2048 /* Note must be dma aligned */
 
 /*
  * The datasheet says that each descriptor can transfers up to 4096 bytes
  * But later, the register documentation reduces that value to 2048,
  * using 2048 cause strange behaviours and even BSP driver use 2047
  */
 #define CONFIG_ETH_RXSIZE	2044 /* Note must fit in ETH_BUFSIZE */
 
 #define TX_TOTAL_BUFSIZE	(CONFIG_ETH_BUFSIZE * CONFIG_TX_DESCR_NUM)
 #define RX_TOTAL_BUFSIZE	(CONFIG_ETH_BUFSIZE * CONFIG_RX_DESCR_NUM)
 
 #define H3_EPHY_DEFAULT_VALUE	0x58000
 #define H3_EPHY_DEFAULT_MASK	GENMASK(31, 15)
 #define H3_EPHY_ADDR_SHIFT	20
 #define REG_PHY_ADDR_MASK	GENMASK(4, 0)
 #define H3_EPHY_LED_POL		BIT(17)	/* 1: active low, 0: active high */
 #define H3_EPHY_SHUTDOWN	BIT(16)	/* 1: shutdown, 0: power up */
 #define H3_EPHY_SELECT		BIT(15) /* 1: internal PHY, 0: external PHY */
 
 #define SC_RMII_EN		BIT(13)
 #define SC_EPIT			BIT(2) /* 1: RGMII, 0: MII */
 #define SC_ETCS_MASK		GENMASK(1, 0)
 #define SC_ETCS_EXT_GMII	0x1
 #define SC_ETCS_INT_GMII	0x2
 
 #define CONFIG_MDIO_TIMEOUT	(3 * CONFIG_SYS_HZ)
 
 #define AHB_GATE_OFFSET_EPHY	0
 
 #if defined(CONFIG_MACH_SUN8I_H3)
 #define SUN8I_GPD8_GMAC		2
 #else
 #define SUN8I_GPD8_GMAC		4
 #endif
 
 /* H3/A64 EMAC Register's offset */
 #define EMAC_CTL0		0x00
 #define EMAC_CTL1		0x04
 #define EMAC_INT_STA		0x08
 #define EMAC_INT_EN		0x0c
 #define EMAC_TX_CTL0		0x10
 #define EMAC_TX_CTL1		0x14
 #define EMAC_TX_FLOW_CTL	0x1c
 #define EMAC_TX_DMA_DESC	0x20
 #define EMAC_RX_CTL0		0x24
 #define EMAC_RX_CTL1		0x28
 #define EMAC_RX_DMA_DESC	0x34
 #define EMAC_MII_CMD		0x48
 #define EMAC_MII_DATA		0x4c
 #define EMAC_ADDR0_HIGH		0x50
 #define EMAC_ADDR0_LOW		0x54
 #define EMAC_TX_DMA_STA		0xb0
 #define EMAC_TX_CUR_DESC	0xb4
 #define EMAC_TX_CUR_BUF		0xb8
 #define EMAC_RX_DMA_STA		0xc0
 #define EMAC_RX_CUR_DESC	0xc4
 
@@ -133,66 +139,74 @@ struct emac_eth_dev {
 static int sun8i_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
 {
 	struct emac_eth_dev *priv = bus->priv;
 	ulong start;
 	u32 miiaddr = 0;
 	int timeout = CONFIG_MDIO_TIMEOUT;
 
 	miiaddr &= ~MDIO_CMD_MII_WRITE;
 	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 
 	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
 
 	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
+	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
+	   to bring the MDC into the range permissible by the IEEE standard. */
+	miiaddr |= MDIO_CMD_MDC_DIV_128;
+
 	miiaddr |= MDIO_CMD_MII_BUSY;
 
 	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
 
 	start = get_timer(0);
 	while (get_timer(start) < timeout) {
 		if (!(readl(priv->mac_reg + EMAC_MII_CMD) & MDIO_CMD_MII_BUSY))
 			return readl(priv->mac_reg + EMAC_MII_DATA);
 		udelay(10);
 	};
 
 	return -1;
 }
 
 static int sun8i_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
 			    u16 val)
 {
 	struct emac_eth_dev *priv = bus->priv;
 	ulong start;
 	u32 miiaddr = 0;
 	int ret = -1, timeout = CONFIG_MDIO_TIMEOUT;
 
 	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 
 	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
 	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
 	miiaddr |= MDIO_CMD_MII_WRITE;
 	miiaddr |= MDIO_CMD_MII_BUSY;
 
+	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
+	   to bring the MDC into the range permissible by the IEEE standard. */
+	miiaddr |= MDIO_CMD_MDC_DIV_128;
+
 	writel(val, priv->mac_reg + EMAC_MII_DATA);
 	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
 
 	start = get_timer(0);
 	while (get_timer(start) < timeout) {
 		if (!(readl(priv->mac_reg + EMAC_MII_CMD) &
 					MDIO_CMD_MII_BUSY)) {
 			ret = 0;
 			break;
 		}
 		udelay(10);
 	};
 
 	return ret;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1 2/2] sun8i_emac: remove unnecessary bit-masking for mdio_read/write
  2017-02-17 17:46 [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write Philipp Tomsich
  2017-02-17 17:46 ` [U-Boot] [PATCH v1 1/2] sun8i_emac: Set " Philipp Tomsich
@ 2017-02-17 17:46 ` Philipp Tomsich
  2017-02-17 23:49   ` André Przywara
  2017-02-21 17:59 ` [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write Maxime Ripard
  2 siblings, 1 reply; 5+ messages in thread
From: Philipp Tomsich @ 2017-02-17 17:46 UTC (permalink / raw)
  To: u-boot

The MDIO read/write builds up the MII_CMD register from scratch (starting
with a value of 0). No need to mask out any fields before writing the new
values.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 drivers/net/sun8i_emac.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 5ae17b7..5094dd8 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -139,74 +139,66 @@ struct emac_eth_dev {
 static int sun8i_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
 {
 	struct emac_eth_dev *priv = bus->priv;
 	ulong start;
 	u32 miiaddr = 0;
 	int timeout = CONFIG_MDIO_TIMEOUT;
 
-	miiaddr &= ~MDIO_CMD_MII_WRITE;
-	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
-
-	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
-
 	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
 	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
 	   to bring the MDC into the range permissible by the IEEE standard. */
 	miiaddr |= MDIO_CMD_MDC_DIV_128;
 
 	miiaddr |= MDIO_CMD_MII_BUSY;
 
 	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
 
 	start = get_timer(0);
 	while (get_timer(start) < timeout) {
 		if (!(readl(priv->mac_reg + EMAC_MII_CMD) & MDIO_CMD_MII_BUSY))
 			return readl(priv->mac_reg + EMAC_MII_DATA);
 		udelay(10);
 	};
 
 	return -1;
 }
 
 static int sun8i_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
 			    u16 val)
 {
 	struct emac_eth_dev *priv = bus->priv;
 	ulong start;
 	u32 miiaddr = 0;
 	int ret = -1, timeout = CONFIG_MDIO_TIMEOUT;
 
-	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
 	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
-
-	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
 	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
 		MDIO_CMD_MII_PHY_ADDR_MASK;
 
 	miiaddr |= MDIO_CMD_MII_WRITE;
 	miiaddr |= MDIO_CMD_MII_BUSY;
 
 	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
 	   to bring the MDC into the range permissible by the IEEE standard. */
 	miiaddr |= MDIO_CMD_MDC_DIV_128;
 
 	writel(val, priv->mac_reg + EMAC_MII_DATA);
 	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
 
 	start = get_timer(0);
 	while (get_timer(start) < timeout) {
 		if (!(readl(priv->mac_reg + EMAC_MII_CMD) &
 					MDIO_CMD_MII_BUSY)) {
 			ret = 0;
 			break;
 		}
 		udelay(10);
 	};
 
 	return ret;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1 2/2] sun8i_emac: remove unnecessary bit-masking for mdio_read/write
  2017-02-17 17:46 ` [U-Boot] [PATCH v1 2/2] sun8i_emac: remove unnecessary bit-masking for mdio_read/write Philipp Tomsich
@ 2017-02-17 23:49   ` André Przywara
  0 siblings, 0 replies; 5+ messages in thread
From: André Przywara @ 2017-02-17 23:49 UTC (permalink / raw)
  To: u-boot

On 17/02/17 17:46, Philipp Tomsich wrote:
> The MDIO read/write builds up the MII_CMD register from scratch (starting
> with a value of 0). No need to mask out any fields before writing the new
> values.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

As a general comment: Can you configure your setup to use the standard
three lines of context? Reading over those big gaps is rather confusing
and may lead to changes being missed.
Thanks!

Cheers,
Andre.

> ---
>  drivers/net/sun8i_emac.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> index 5ae17b7..5094dd8 100644
> --- a/drivers/net/sun8i_emac.c
> +++ b/drivers/net/sun8i_emac.c
> @@ -139,74 +139,66 @@ struct emac_eth_dev {
>  static int sun8i_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
>  {
>  	struct emac_eth_dev *priv = bus->priv;
>  	ulong start;
>  	u32 miiaddr = 0;
>  	int timeout = CONFIG_MDIO_TIMEOUT;
>  
> -	miiaddr &= ~MDIO_CMD_MII_WRITE;
> -	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
>  	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> -
> -	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
> -
>  	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_ADDR_MASK;
>  
>  	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
>  	   to bring the MDC into the range permissible by the IEEE standard. */
>  	miiaddr |= MDIO_CMD_MDC_DIV_128;
>  
>  	miiaddr |= MDIO_CMD_MII_BUSY;
>  
>  	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
>  
>  	start = get_timer(0);
>  	while (get_timer(start) < timeout) {
>  		if (!(readl(priv->mac_reg + EMAC_MII_CMD) & MDIO_CMD_MII_BUSY))
>  			return readl(priv->mac_reg + EMAC_MII_DATA);
>  		udelay(10);
>  	};
>  
>  	return -1;
>  }
>  
>  static int sun8i_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
>  			    u16 val)
>  {
>  	struct emac_eth_dev *priv = bus->priv;
>  	ulong start;
>  	u32 miiaddr = 0;
>  	int ret = -1, timeout = CONFIG_MDIO_TIMEOUT;
>  
> -	miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
>  	miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> -
> -	miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
>  	miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
>  		MDIO_CMD_MII_PHY_ADDR_MASK;
>  
>  	miiaddr |= MDIO_CMD_MII_WRITE;
>  	miiaddr |= MDIO_CMD_MII_BUSY;
>  
>  	/* The MAC block is fed by a 300MHz clock, so we need to divide by 128
>  	   to bring the MDC into the range permissible by the IEEE standard. */
>  	miiaddr |= MDIO_CMD_MDC_DIV_128;
>  
>  	writel(val, priv->mac_reg + EMAC_MII_DATA);
>  	writel(miiaddr, priv->mac_reg + EMAC_MII_CMD);
>  
>  	start = get_timer(0);
>  	while (get_timer(start) < timeout) {
>  		if (!(readl(priv->mac_reg + EMAC_MII_CMD) &
>  					MDIO_CMD_MII_BUSY)) {
>  			ret = 0;
>  			break;
>  		}
>  		udelay(10);
>  	};
>  
>  	return ret;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write
  2017-02-17 17:46 [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write Philipp Tomsich
  2017-02-17 17:46 ` [U-Boot] [PATCH v1 1/2] sun8i_emac: Set " Philipp Tomsich
  2017-02-17 17:46 ` [U-Boot] [PATCH v1 2/2] sun8i_emac: remove unnecessary bit-masking for mdio_read/write Philipp Tomsich
@ 2017-02-21 17:59 ` Maxime Ripard
  2 siblings, 0 replies; 5+ messages in thread
From: Maxime Ripard @ 2017-02-21 17:59 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 17, 2017 at 06:46:51PM +0100, Philipp Tomsich wrote:
> To ensure compatibility with all PHYs, we need to keep the MDIO clock
> (MDC) below 2.5MHz (the guaranteed operating limit from IEEE 802.3),
> even if some PHYs will tolerate higher speeds.
> 
> This changeset also cleans up the MDIO read/write functions by
> removing pointless bit-masking in a variable initialised to 0.

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

For both.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170221/132d3e0e/attachment.sig>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-02-21 17:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-17 17:46 [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write Philipp Tomsich
2017-02-17 17:46 ` [U-Boot] [PATCH v1 1/2] sun8i_emac: Set " Philipp Tomsich
2017-02-17 17:46 ` [U-Boot] [PATCH v1 2/2] sun8i_emac: remove unnecessary bit-masking for mdio_read/write Philipp Tomsich
2017-02-17 23:49   ` André Przywara
2017-02-21 17:59 ` [U-Boot] [PATCH v1 0/2] sun8i_emac: set MDC divider for MDIO read/write Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.