[U-Boot] [PATCH 1/3] net: davinci_emac: Remove useless dcache ops on descriptors
ATM the rx and tx descriptors are handled as cached memory while they lie in a dedicated RAM of the SoCs, which is an uncached area. Removing the said dcache ops, while optimizing the logic and clarifying the code, also gets rid of most of the check_cache_range() incurred warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com --- drivers/net/davinci_emac.c | 28 ---------------------------- 1 file changed, 28 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index b030498..947bfab 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -108,26 +108,6 @@ static u_int8_t num_phy;
phy_t phy[CONFIG_SYS_DAVINCI_EMAC_PHY_COUNT];
-static inline void davinci_flush_rx_descs(void) -{ - /* flush the whole RX descs area */ - flush_dcache_range(EMAC_WRAPPER_RAM_ADDR + EMAC_RX_DESC_BASE, - EMAC_WRAPPER_RAM_ADDR + EMAC_TX_DESC_BASE); -} - -static inline void davinci_invalidate_rx_descs(void) -{ - /* invalidate the whole RX descs area */ - invalidate_dcache_range(EMAC_WRAPPER_RAM_ADDR + EMAC_RX_DESC_BASE, - EMAC_WRAPPER_RAM_ADDR + EMAC_TX_DESC_BASE); -} - -static inline void davinci_flush_desc(emac_desc *desc) -{ - flush_dcache_range((unsigned long)desc, - (unsigned long)desc + sizeof(*desc)); -} - static int davinci_eth_set_mac_addr(struct eth_device *dev) { unsigned long mac_hi; @@ -491,8 +471,6 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis) emac_rx_active_tail = rx_desc; emac_rx_queue_active = 1;
- davinci_flush_rx_descs(); - /* Enable TX/RX */ writel(EMAC_MAX_ETHERNET_PKT_SIZE, &adap_emac->RXMAXLEN); writel(0, &adap_emac->RXBUFFEROFFSET); @@ -655,7 +633,6 @@ static int davinci_eth_send_packet (struct eth_device *dev,
flush_dcache_range((unsigned long)packet, (unsigned long)packet + length); - davinci_flush_desc(emac_tx_desc);
/* Send the packet */ writel(BD_TO_HW((unsigned long)emac_tx_desc), &adap_emac->TX0HDP); @@ -689,8 +666,6 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) volatile emac_desc *tail_desc; int status, ret = -1;
- davinci_invalidate_rx_descs(); - rx_curr_desc = emac_rx_active_head; if (!rx_curr_desc) return 0; @@ -729,7 +704,6 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) rx_curr_desc->buff_off_len = EMAC_MAX_ETHERNET_PKT_SIZE; rx_curr_desc->pkt_flag_len = EMAC_CPPI_OWNERSHIP_BIT; rx_curr_desc->next = 0; - davinci_flush_desc(rx_curr_desc);
if (emac_rx_active_head == 0) { printf ("INFO: emac_rcv_pkt: active queue head = 0\n"); @@ -747,13 +721,11 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) tail_desc->next = BD_TO_HW((ulong) curr_desc); status = tail_desc->pkt_flag_len; if (status & EMAC_CPPI_EOQ_BIT) { - davinci_flush_desc(tail_desc); writel(BD_TO_HW((ulong)curr_desc), &adap_emac->RX0HDP); status &= ~EMAC_CPPI_EOQ_BIT; tail_desc->pkt_flag_len = status; } - davinci_flush_desc(tail_desc); } return (ret); }
check_cache_range() warns that the top boundaries are not properly aligned while flushing and invalidating the buffers and make these operations to fail. ATM the RX bottom boundaries are aligned by design with EMAC_RXBUF_SIZE, properly aligned with ARCH_DMA_MINALIGN, however the top ones are not.
This gets rid of the warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com --- drivers/net/davinci_emac.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 947bfab..55461b0 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -632,7 +632,8 @@ static int davinci_eth_send_packet (struct eth_device *dev, EMAC_CPPI_EOP_BIT);
flush_dcache_range((unsigned long)packet, - (unsigned long)packet + length); + round_up((unsigned long)packet + length, + ARCH_DMA_MINALIGN));
/* Send the packet */ writel(BD_TO_HW((unsigned long)emac_tx_desc), &adap_emac->TX0HDP); @@ -677,7 +678,8 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) } else { unsigned long tmp = (unsigned long)rx_curr_desc->buffer;
- invalidate_dcache_range(tmp, tmp + EMAC_RXBUF_SIZE); + invalidate_dcache_range(tmp, round_up(tmp + EMAC_RXBUF_SIZE, + ARCH_DMA_MINALIGN)); net_process_received_packet( rx_curr_desc->buffer, rx_curr_desc->buff_off_len & 0xffff);
On Sun, Aug 14, 2016 at 03:03:16PM +0000, Karl Beldan wrote:
check_cache_range() warns that the top boundaries are not properly aligned while flushing and invalidating the buffers and make these operations to fail. ATM the RX bottom boundaries are aligned by design with EMAC_RXBUF_SIZE, properly aligned with ARCH_DMA_MINALIGN, however the top ones are not.
This gets rid of the warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
drivers/net/davinci_emac.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 947bfab..55461b0 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -632,7 +632,8 @@ static int davinci_eth_send_packet (struct eth_device *dev, EMAC_CPPI_EOP_BIT);
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
round_up((unsigned long)packet + length,ARCH_DMA_MINALIGN));
It's preferred to use: (unsigned long)packet + ALIGN(length, PKTALIGN)); here instead of ARCH_DMA_MINALIGN.
On Sun, Aug 14, 2016 at 11:47:25AM -0400, Tom Rini wrote:
On Sun, Aug 14, 2016 at 03:03:16PM +0000, Karl Beldan wrote:
check_cache_range() warns that the top boundaries are not properly aligned while flushing and invalidating the buffers and make these operations to fail. ATM the RX bottom boundaries are aligned by design with EMAC_RXBUF_SIZE, properly aligned with ARCH_DMA_MINALIGN, however the top ones are not.
This gets rid of the warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
drivers/net/davinci_emac.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 947bfab..55461b0 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -632,7 +632,8 @@ static int davinci_eth_send_packet (struct eth_device *dev, EMAC_CPPI_EOP_BIT);
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
round_up((unsigned long)packet + length,ARCH_DMA_MINALIGN));It's preferred to use: (unsigned long)packet + ALIGN(length, PKTALIGN)); here instead of ARCH_DMA_MINALIGN.
Hmm, I think your suggestion is buggy. The cache primitives act on [laddr, haddr[, i.e. haddr is excluded, IOW you are missing the tail of the packet (that's why I rounded up).
Conceptually I still prefer ARCH_DMA_MINALIGN, also all other code in the base does so.
Rgds, Karl
On Sun, Aug 14, 2016 at 07:43:56PM +0000, Karl Beldan wrote:
On Sun, Aug 14, 2016 at 11:47:25AM -0400, Tom Rini wrote:
On Sun, Aug 14, 2016 at 03:03:16PM +0000, Karl Beldan wrote:
check_cache_range() warns that the top boundaries are not properly aligned while flushing and invalidating the buffers and make these operations to fail. ATM the RX bottom boundaries are aligned by design with EMAC_RXBUF_SIZE, properly aligned with ARCH_DMA_MINALIGN, however the top ones are not.
This gets rid of the warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
drivers/net/davinci_emac.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 947bfab..55461b0 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -632,7 +632,8 @@ static int davinci_eth_send_packet (struct eth_device *dev, EMAC_CPPI_EOP_BIT);
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
round_up((unsigned long)packet + length,ARCH_DMA_MINALIGN));It's preferred to use: (unsigned long)packet + ALIGN(length, PKTALIGN)); here instead of ARCH_DMA_MINALIGN.
Hmm, I think your suggestion is buggy. The cache primitives act on [laddr, haddr[, i.e. haddr is excluded, IOW you are missing the tail of the packet (that's why I rounded up).
Joe, we've got some instances using ALIGN(length, PKTALIGN) and others using roundup. And I can't imagine that we shouldn't be using the same thing everywhere here. So, what do you say to Karl's comment? Thanks!
Conceptually I still prefer ARCH_DMA_MINALIGN, also all other code in the base does so.
Being network code, we should use PKTALIGN here and elsewhere.
On Sun, Aug 14, 2016 at 07:43:56PM +0000, Karl Beldan wrote:
On Sun, Aug 14, 2016 at 11:47:25AM -0400, Tom Rini wrote:
On Sun, Aug 14, 2016 at 03:03:16PM +0000, Karl Beldan wrote:
check_cache_range() warns that the top boundaries are not properly aligned while flushing and invalidating the buffers and make these operations to fail. ATM the RX bottom boundaries are aligned by design with EMAC_RXBUF_SIZE, properly aligned with ARCH_DMA_MINALIGN, however the top ones are not.
This gets rid of the warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
drivers/net/davinci_emac.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 947bfab..55461b0 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -632,7 +632,8 @@ static int davinci_eth_send_packet (struct eth_device *dev, EMAC_CPPI_EOP_BIT);
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
round_up((unsigned long)packet + length,ARCH_DMA_MINALIGN));It's preferred to use: (unsigned long)packet + ALIGN(length, PKTALIGN)); here instead of ARCH_DMA_MINALIGN.
Hmm, I think your suggestion is buggy. The cache primitives act on [laddr, haddr[, i.e. haddr is excluded, IOW you are missing the tail of the packet (that's why I rounded up).
Just checked, your ALIGN macro also rounds up so your suggestion is not buggy, my bad.
Conceptually I still prefer ARCH_DMA_MINALIGN, also all other code in the base does so.
Your suggestion is seconded by Joe, I'll send v2.
Rgds, Karl
On Sun, Aug 14, 2016 at 10:47 AM, Tom Rini trini@konsulko.com wrote:
On Sun, Aug 14, 2016 at 03:03:16PM +0000, Karl Beldan wrote:
check_cache_range() warns that the top boundaries are not properly aligned while flushing and invalidating the buffers and make these operations to fail. ATM the RX bottom boundaries are aligned by design with EMAC_RXBUF_SIZE, properly aligned with ARCH_DMA_MINALIGN, however the top ones are not.
This gets rid of the warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
drivers/net/davinci_emac.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 947bfab..55461b0 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -632,7 +632,8 @@ static int davinci_eth_send_packet (struct eth_device *dev, EMAC_CPPI_EOP_BIT);
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
round_up((unsigned long)packet + length,ARCH_DMA_MINALIGN));It's preferred to use: (unsigned long)packet + ALIGN(length, PKTALIGN)); here instead of ARCH_DMA_MINALIGN.
Correct, please resend.
Thanks, -Joe
ATM when receiving a packet the whole buffer is invalidated, this change optimizes this behaviour.
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com --- drivers/net/davinci_emac.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 55461b0..e26e727 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -677,13 +677,13 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) printf ("WARN: emac_rcv_pkt: Error in packet\n"); } else { unsigned long tmp = (unsigned long)rx_curr_desc->buffer; + unsigned short len = + rx_curr_desc->buff_off_len & 0xffff;
- invalidate_dcache_range(tmp, round_up(tmp + EMAC_RXBUF_SIZE, - ARCH_DMA_MINALIGN)); - net_process_received_packet( - rx_curr_desc->buffer, - rx_curr_desc->buff_off_len & 0xffff); - ret = rx_curr_desc->buff_off_len & 0xffff; + invalidate_dcache_range(tmp, round_up(tmp + len, + ARCH_DMA_MINALIGN)); + net_process_received_packet(rx_curr_desc->buffer, len); + ret = len; }
/* Ack received packet descriptor */
On Sun, Aug 14, 2016 at 10:03 AM, Karl Beldan karl.beldan@gmail.com wrote:
ATM when receiving a packet the whole buffer is invalidated, this change optimizes this behaviour.
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
drivers/net/davinci_emac.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 55461b0..e26e727 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -677,13 +677,13 @@ static int davinci_eth_rcv_packet (struct eth_device *dev) printf ("WARN: emac_rcv_pkt: Error in packet\n"); } else { unsigned long tmp = (unsigned long)rx_curr_desc->buffer;
unsigned short len =rx_curr_desc->buff_off_len & 0xffff;
invalidate_dcache_range(tmp, round_up(tmp + EMAC_RXBUF_SIZE,ARCH_DMA_MINALIGN));net_process_received_packet(rx_curr_desc->buffer,rx_curr_desc->buff_off_len & 0xffff);ret = rx_curr_desc->buff_off_len & 0xffff;
invalidate_dcache_range(tmp, round_up(tmp + len,ARCH_DMA_MINALIGN));
Here again, please use tmp + ALIGN(len, PKTALIGN)
net_process_received_packet(rx_curr_desc->buffer, len);ret = len; }
Thanks, -Joe
On Sun, Aug 14, 2016 at 03:03:15PM +0000, Karl Beldan wrote:
ATM the rx and tx descriptors are handled as cached memory while they lie in a dedicated RAM of the SoCs, which is an uncached area. Removing the said dcache ops, while optimizing the logic and clarifying the code, also gets rid of most of the check_cache_range() incurred warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
On Sun, Aug 14, 2016 at 10:03 AM, Karl Beldan karl.beldan@gmail.com wrote:
ATM the rx and tx descriptors are handled as cached memory while they lie in a dedicated RAM of the SoCs, which is an uncached area. Removing the said dcache ops, while optimizing the logic and clarifying the code, also gets rid of most of the check_cache_range() incurred warnings: CACHE: Misaligned operation at range
Signed-off-by: Karl Beldan karl.beldan+oss@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
participants (5)
-
Joe Hershberger -
Joe Hershberger -
Karl Beldan -
Karl Beldan -
Tom Rini