[U-Boot] [PATCH 1/8] net: zynq: Allocate BD_SPACE in connection to RX_BUF

BD_SEPRN_SPACE should not have hard coded value and it will be calculated based on the number of buffer descriptors that we would like to use.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 0e741dd605a6..5aa21b9f24a7 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -149,7 +149,7 @@ struct emac_bd { */ #define BD_SPACE 0x100000 /* BD separation space */ -#define BD_SEPRN_SPACE 64 +#define BD_SEPRN_SPACE (RX_BUF * sizeof(struct emac_bd))
/* Initialized, rxbd_current, rx_first_buf must be 0 after init */ struct zynq_gem_priv {

Fix incorrect sequence in BD handling.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 5aa21b9f24a7..f13a244c7756 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -432,9 +432,6 @@ static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) struct zynq_gem_priv *priv = dev->priv; struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
- /* setup BD */ - writel((u32)priv->tx_bd, ®s->txqbase); - /* Setup Tx BD */ memset(priv->tx_bd, 0, sizeof(struct emac_bd));
@@ -443,6 +440,9 @@ static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) ZYNQ_GEM_TXBUF_LAST_MASK | ZYNQ_GEM_TXBUF_WRAP_MASK;
+ /* setup BD */ + writel((u32)priv->tx_bd, ®s->txqbase); + addr = (u32) ptr; addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN);

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Fix incorrect sequence in BD handling.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index f13a244c7756..7084488b25ec 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -458,8 +458,6 @@ static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) setbits_le32(®s->nwctrl, ZYNQ_GEM_NWCTRL_STARTTX_MASK);
/* Read TX BD status */ - if (priv->tx_bd->status & ZYNQ_GEM_TXBUF_UNDERRUN) - printf("TX underrun\n"); if (priv->tx_bd->status & ZYNQ_GEM_TXBUF_EXHAUSTED) printf("TX buffers exhausted in mid frame\n");

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote: Don't you need some body here to appease checkpatch.pl?
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Target is duplicating packets. IP prefetches another BD and process it when the first one is sent. Adding one dummy BD to the chain fix the problem with packet duplication.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 7084488b25ec..e49aa86127d1 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -46,6 +46,7 @@ /* Wrap bit, last descriptor */ #define ZYNQ_GEM_TXBUF_WRAP_MASK 0x40000000 #define ZYNQ_GEM_TXBUF_LAST_MASK 0x00008000 /* Last buffer */ +#define ZYNQ_GEM_TXBUF_USED_MASK 0x80000000 /* Used by Hw */
#define ZYNQ_GEM_NWCTRL_TXEN_MASK 0x00000008 /* Enable transmit */ #define ZYNQ_GEM_NWCTRL_RXEN_MASK 0x00000004 /* Enable receive */ @@ -431,14 +432,19 @@ static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) u32 addr, size; struct zynq_gem_priv *priv = dev->priv; struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase; + struct emac_bd *current_bd = &priv->tx_bd[1];
/* Setup Tx BD */ memset(priv->tx_bd, 0, sizeof(struct emac_bd));
priv->tx_bd->addr = (u32)ptr; priv->tx_bd->status = (len & ZYNQ_GEM_TXBUF_FRMLEN_MASK) | - ZYNQ_GEM_TXBUF_LAST_MASK | - ZYNQ_GEM_TXBUF_WRAP_MASK; + ZYNQ_GEM_TXBUF_LAST_MASK; + /* Dummy descriptor to mark it as the last in descriptor chain */ + current_bd->addr = 0x0; + current_bd->status = ZYNQ_GEM_TXBUF_WRAP_MASK | + ZYNQ_GEM_TXBUF_LAST_MASK| + ZYNQ_GEM_TXBUF_USED_MASK;
/* setup BD */ writel((u32)priv->tx_bd, ®s->txqbase);

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Target is duplicating packets. IP prefetches another BD and process it when the first one is sent. Adding one dummy BD to the chain fix the problem with packet duplication.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Wait till BD is process to ensure that packet was sent successfully.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index e49aa86127d1..c56e02132ae9 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -23,6 +23,7 @@ #include <asm/system.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> +#include <asm-generic/errno.h>
#if !defined(CONFIG_PHYLIB) # error XILINX_GEM_ETHERNET requires PHYLIB @@ -86,6 +87,8 @@ ZYNQ_GEM_DMACR_TXSIZE | \ ZYNQ_GEM_DMACR_RXBUF)
+#define ZYNQ_GEM_TSR_DONE 0x00000020 /* Tx done mask */ + /* Use MII register 1 (MII status register) to detect PHY */ #define PHY_DETECT_REG 1
@@ -427,6 +430,33 @@ static int zynq_gem_init(struct eth_device *dev, bd_t * bis) return 0; }
+static inline int wait_for_bit(const char *func, u32 *reg, const u32 mask, + bool set, unsigned int timeout) +{ + u32 val; + unsigned long start = get_timer(0); + + while (1) { + val = readl(reg); + + if (!set) + val = ~val; + + if ((val & mask) == mask) + return 0; + + if (get_timer(start) > timeout) + break; + + udelay(1); + } + + debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n", + func, reg, mask, set); + + return -ETIMEDOUT; +} + static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) { u32 addr, size; @@ -467,7 +497,8 @@ static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) if (priv->tx_bd->status & ZYNQ_GEM_TXBUF_EXHAUSTED) printf("TX buffers exhausted in mid frame\n");
- return 0; + return wait_for_bit(__func__, ®s->txsr, ZYNQ_GEM_TSR_DONE, + true, 20000); }
/* Do not check frame_recd flag in rx_status register 0x20 - just poll BD */

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Wait till BD is process to ensure that packet was sent successfully.
process -> processed
Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/net/zynq_gem.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index e49aa86127d1..c56e02132ae9 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -23,6 +23,7 @@ #include <asm/system.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> +#include <asm-generic/errno.h>
#if !defined(CONFIG_PHYLIB) # error XILINX_GEM_ETHERNET requires PHYLIB @@ -86,6 +87,8 @@ ZYNQ_GEM_DMACR_TXSIZE | \ ZYNQ_GEM_DMACR_RXBUF)
+#define ZYNQ_GEM_TSR_DONE 0x00000020 /* Tx done mask */
/* Use MII register 1 (MII status register) to detect PHY */ #define PHY_DETECT_REG 1
@@ -427,6 +430,33 @@ static int zynq_gem_init(struct eth_device *dev, bd_t * bis) return 0; }
+static inline int wait_for_bit(const char *func, u32 *reg, const u32 mask,
bool set, unsigned int timeout)
There is no need to specify "inline" here.
+{
u32 val;
unsigned long start = get_timer(0);
while (1) {
val = readl(reg);
if (!set)
val = ~val;
if ((val & mask) == mask)
return 0;
if (get_timer(start) > timeout)
break;
udelay(1);
}
debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n",
func, reg, mask, set);
return -ETIMEDOUT;
+}
static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) { u32 addr, size; @@ -467,7 +497,8 @@ static int zynq_gem_send(struct eth_device *dev, void *ptr, int len) if (priv->tx_bd->status & ZYNQ_GEM_TXBUF_EXHAUSTED) printf("TX buffers exhausted in mid frame\n");
return 0;
return wait_for_bit(__func__, ®s->txsr, ZYNQ_GEM_TSR_DONE,
true, 20000);
}
/* Do not check frame_recd flag in rx_status register 0x20 - just poll BD */
2.5.0
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On 11/02/2015 10:40 PM, Joe Hershberger wrote:
On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Wait till BD is process to ensure that packet was sent successfully.
process -> processed
Fixed in v2.
Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/net/zynq_gem.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index e49aa86127d1..c56e02132ae9 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -23,6 +23,7 @@ #include <asm/system.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> +#include <asm-generic/errno.h>
#if !defined(CONFIG_PHYLIB) # error XILINX_GEM_ETHERNET requires PHYLIB @@ -86,6 +87,8 @@ ZYNQ_GEM_DMACR_TXSIZE | \ ZYNQ_GEM_DMACR_RXBUF)
+#define ZYNQ_GEM_TSR_DONE 0x00000020 /* Tx done mask */
/* Use MII register 1 (MII status register) to detect PHY */ #define PHY_DETECT_REG 1
@@ -427,6 +430,33 @@ static int zynq_gem_init(struct eth_device *dev, bd_t * bis) return 0; }
+static inline int wait_for_bit(const char *func, u32 *reg, const u32 mask,
bool set, unsigned int timeout)
There is no need to specify "inline" here.
Fixed in v2.
Thanks, Michal

Using set and clear macro is incorrect because it is not overwritting origin mdc clock division setup. For example origin setup is 8(0b001) and new setup is 64(0b100) which means 0b101 is setup which is 96 divider. Using writel to rewrite all setting like for 1000Mbit/s case.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index c56e02132ae9..a9384ce73144 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -410,8 +410,8 @@ static int zynq_gem_init(struct eth_device *dev, bd_t * bis) clk_rate = ZYNQ_GEM_FREQUENCY_1000; break; case SPEED_100: - clrsetbits_le32(®s->nwcfg, ZYNQ_GEM_NWCFG_SPEED1000, - ZYNQ_GEM_NWCFG_INIT | ZYNQ_GEM_NWCFG_SPEED100); + writel(ZYNQ_GEM_NWCFG_INIT | ZYNQ_GEM_NWCFG_SPEED100, + ®s->nwcfg); clk_rate = ZYNQ_GEM_FREQUENCY_100; break; case SPEED_10:

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Using set and clear macro is incorrect because it is not overwritting origin mdc clock division setup. For example origin setup is 8(0b001) and new setup is 64(0b100) which means 0b101 is setup which is 96 divider. Using writel to rewrite all setting like for 1000Mbit/s case.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Driver cleanup.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index a9384ce73144..3962c83f7ac6 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -59,7 +59,6 @@ #define ZYNQ_GEM_NWCFG_FDEN 0x000000002 /* Full Duplex mode */ #define ZYNQ_GEM_NWCFG_FSREM 0x000020000 /* FCS removal */ #define ZYNQ_GEM_NWCFG_MDCCLKDIV 0x000080000 /* Div pclk by 32, 80MHz */ -#define ZYNQ_GEM_NWCFG_MDCCLKDIV2 0x0000c0000 /* Div pclk by 48, 120MHz */
#ifdef CONFIG_ARM64 # define ZYNQ_GEM_DBUS_WIDTH (1 << 21) /* 64 bit bus */

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Driver cleanup.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Based on spec: "MDC must not exceed 2.5 MHz (MDC is only active during MDIO read and write operations)" Zynq is running on 111MHz. Current setting is 32 which is 111/32=3.47 which is above of 2.5MHz. Using 48 divider will give us correct setting according spec (111/48=2.31).
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 3962c83f7ac6..045954a7b059 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -58,7 +58,7 @@ #define ZYNQ_GEM_NWCFG_SPEED1000 0x000000400 /* 1Gbps operation */ #define ZYNQ_GEM_NWCFG_FDEN 0x000000002 /* Full Duplex mode */ #define ZYNQ_GEM_NWCFG_FSREM 0x000020000 /* FCS removal */ -#define ZYNQ_GEM_NWCFG_MDCCLKDIV 0x000080000 /* Div pclk by 32, 80MHz */ +#define ZYNQ_GEM_NWCFG_MDCCLKDIV 0x0000c0000 /* Div pclk by 48, max 120MHz */
#ifdef CONFIG_ARM64 # define ZYNQ_GEM_DBUS_WIDTH (1 << 21) /* 64 bit bus */

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Based on spec: "MDC must not exceed 2.5 MHz (MDC is only active during MDIO read and write operations)" Zynq is running on 111MHz. Current setting is 32 which is 111/32=3.47
Isn't it dependent on which board and what the clock setup is? Should this be specified by the board?
which is above of 2.5MHz. Using 48 divider will give us correct setting according spec (111/48=2.31).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

On 11/02/2015 10:39 PM, Joe Hershberger wrote:
On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Based on spec: "MDC must not exceed 2.5 MHz (MDC is only active during MDIO read and write operations)" Zynq is running on 111MHz. Current setting is 32 which is 111/32=3.47
Isn't it dependent on which board and what the clock setup is? Should this be specified by the board?
I am not aware about it. It is related to internal chip clock. I expect that 111MHz is software default value is is used by most of the chip. The clock range based on mainline Linux driver is 80MHz - 120MHz that's why I expect all non standard configuration will fit to it. For lower values as we used till now none reported any functional issue too.
which is above of 2.5MHz. Using 48 divider will give us correct setting according spec (111/48=2.31).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks, Michal

On Wed, Nov 4, 2015 at 7:40 AM, Michal Simek michal.simek@xilinx.com wrote:
On 11/02/2015 10:39 PM, Joe Hershberger wrote:
On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Based on spec: "MDC must not exceed 2.5 MHz (MDC is only active during MDIO read and write operations)" Zynq is running on 111MHz. Current setting is 32 which is 111/32=3.47
Isn't it dependent on which board and what the clock setup is? Should this be specified by the board?
I am not aware about it. It is related to internal chip clock. I expect that 111MHz is software default value is is used by most of the chip. The clock range based on mainline Linux driver is 80MHz - 120MHz that's why I expect all non standard configuration will fit to it. For lower values as we used till now none reported any functional issue too.
OK, sounds good.
which is above of 2.5MHz. Using 48 divider will give us correct setting according spec (111/48=2.31).
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks, Michal

On Tue, Oct 27, 2015 at 10:17 AM, Michal Simek michal.simek@xilinx.com wrote:
BD_SEPRN_SPACE should not have hard coded value and it will be calculated based on the number of buffer descriptors that we would like to use.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
participants (2)
-
Joe Hershberger
-
Michal Simek