All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
@ 2019-07-14 15:25 Ramon Fried
  2019-07-15 18:41 ` Joe Hershberger
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Ramon Fried @ 2019-07-14 15:25 UTC (permalink / raw)
  To: u-boot

Macb Ethernet controller requires a RX buffer of 128 bytes. It is
highly sub-optimal for Gigabit-capable GEM that is able to use
a bigger DMA buffer. Change this constant and associated macros
with data stored in the private structure.
RX DMA buffer size has to be multiple of 64 bytes as indicated in
DMA Configuration Register specification.

Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
---
v2: Fix multi-line comment style
 drivers/net/macb.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index c072f99d8f..3c8b9722b3 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -45,10 +45,17 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define MACB_RX_BUFFER_SIZE		4096
-#define MACB_RX_RING_SIZE		(MACB_RX_BUFFER_SIZE / 128)
+/*
+ * These buffer sizes must be power of 2 and divisible
+ * by RX_BUFFER_MULTIPLE
+ */
+#define MACB_RX_BUFFER_SIZE		128
+#define GEM_RX_BUFFER_SIZE		2048
 #define RX_BUFFER_MULTIPLE		64
+
+#define MACB_RX_RING_SIZE		32
 #define MACB_TX_RING_SIZE		16
+
 #define MACB_TX_TIMEOUT		1000
 #define MACB_AUTONEG_TIMEOUT	5000000
 
@@ -95,6 +102,7 @@ struct macb_device {
 	void			*tx_buffer;
 	struct macb_dma_desc	*rx_ring;
 	struct macb_dma_desc	*tx_ring;
+	size_t			rx_buffer_size;
 
 	unsigned long		rx_buffer_dma;
 	unsigned long		rx_ring_dma;
@@ -395,15 +403,16 @@ static int _macb_recv(struct macb_device *macb, uchar **packetp)
 		}
 
 		if (status & MACB_BIT(RX_EOF)) {
-			buffer = macb->rx_buffer + 128 * macb->rx_tail;
+			buffer = macb->rx_buffer +
+				macb->rx_buffer_size * macb->rx_tail;
 			length = status & RXBUF_FRMLEN_MASK;
 
 			macb_invalidate_rx_buffer(macb);
 			if (macb->wrapped) {
 				unsigned int headlen, taillen;
 
-				headlen = 128 * (MACB_RX_RING_SIZE
-						 - macb->rx_tail);
+				headlen = macb->rx_buffer_size *
+					(MACB_RX_RING_SIZE - macb->rx_tail);
 				taillen = length - headlen;
 				memcpy((void *)net_rx_packets[0],
 				       buffer, headlen);
@@ -701,7 +710,7 @@ static void gmac_configure_dma(struct macb_device *macb)
 	u32 buffer_size;
 	u32 dmacfg;
 
-	buffer_size = 128 / RX_BUFFER_MULTIPLE;
+	buffer_size = macb->rx_buffer_size / RX_BUFFER_MULTIPLE;
 	dmacfg = gem_readl(macb, DMACFG) & ~GEM_BF(RXBS, -1L);
 	dmacfg |= GEM_BF(RXBS, buffer_size);
 
@@ -746,7 +755,7 @@ static int _macb_init(struct macb_device *macb, const char *name)
 			paddr |= MACB_BIT(RX_WRAP);
 		macb->rx_ring[i].addr = paddr;
 		macb->rx_ring[i].ctrl = 0;
-		paddr += 128;
+		paddr += macb->rx_buffer_size;
 	}
 	macb_flush_ring_desc(macb, RX);
 	macb_flush_rx_buffer(macb);
@@ -957,8 +966,13 @@ static void _macb_eth_initialize(struct macb_device *macb)
 	int id = 0;	/* This is not used by functions we call */
 	u32 ncfgr;
 
+	if (macb_is_gem(macb))
+		macb->rx_buffer_size = GEM_RX_BUFFER_SIZE;
+	else
+		macb->rx_buffer_size = MACB_RX_BUFFER_SIZE;
+
 	/* TODO: we need check the rx/tx_ring_dma is dcache line aligned */
-	macb->rx_buffer = dma_alloc_coherent(MACB_RX_BUFFER_SIZE,
+	macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * MACB_RX_RING_SIZE,
 					     &macb->rx_buffer_dma);
 	macb->rx_ring = dma_alloc_coherent(MACB_RX_DMA_DESC_SIZE,
 					   &macb->rx_ring_dma);
-- 
2.22.0

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-07-14 15:25 [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM Ramon Fried
@ 2019-07-15 18:41 ` Joe Hershberger
  2019-07-15 23:25 ` Joe Hershberger
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2019-07-15 18:41 UTC (permalink / raw)
  To: u-boot

On Sun, Jul 14, 2019 at 10:25 AM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> Macb Ethernet controller requires a RX buffer of 128 bytes. It is
> highly sub-optimal for Gigabit-capable GEM that is able to use
> a bigger DMA buffer. Change this constant and associated macros
> with data stored in the private structure.
> RX DMA buffer size has to be multiple of 64 bytes as indicated in
> DMA Configuration Register specification.
>
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-07-14 15:25 [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM Ramon Fried
  2019-07-15 18:41 ` Joe Hershberger
@ 2019-07-15 23:25 ` Joe Hershberger
  2019-07-16  5:04   ` Ramon Fried
  2019-07-25 18:42 ` [U-Boot] " Joe Hershberger
  2019-08-22  9:38 ` [U-Boot] [PATCH v2] " Stefan Roese
  3 siblings, 1 reply; 10+ messages in thread
From: Joe Hershberger @ 2019-07-15 23:25 UTC (permalink / raw)
  To: u-boot

On Sun, Jul 14, 2019 at 10:25 AM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> Macb Ethernet controller requires a RX buffer of 128 bytes. It is
> highly sub-optimal for Gigabit-capable GEM that is able to use
> a bigger DMA buffer. Change this constant and associated macros
> with data stored in the private structure.
> RX DMA buffer size has to be multiple of 64 bytes as indicated in
> DMA Configuration Register specification.
>
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>

Is there a dependency here that you don't mention? This doesn't apply cleanly.

Thanks,
-Joe

> ---
> v2: Fix multi-line comment style
>  drivers/net/macb.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index c072f99d8f..3c8b9722b3 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -45,10 +45,17 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -#define MACB_RX_BUFFER_SIZE            4096
> -#define MACB_RX_RING_SIZE              (MACB_RX_BUFFER_SIZE / 128)
> +/*
> + * These buffer sizes must be power of 2 and divisible
> + * by RX_BUFFER_MULTIPLE
> + */
> +#define MACB_RX_BUFFER_SIZE            128
> +#define GEM_RX_BUFFER_SIZE             2048
>  #define RX_BUFFER_MULTIPLE             64
> +
> +#define MACB_RX_RING_SIZE              32
>  #define MACB_TX_RING_SIZE              16
> +
>  #define MACB_TX_TIMEOUT                1000
>  #define MACB_AUTONEG_TIMEOUT   5000000
>
> @@ -95,6 +102,7 @@ struct macb_device {
>         void                    *tx_buffer;
>         struct macb_dma_desc    *rx_ring;
>         struct macb_dma_desc    *tx_ring;
> +       size_t                  rx_buffer_size;
>
>         unsigned long           rx_buffer_dma;
>         unsigned long           rx_ring_dma;
> @@ -395,15 +403,16 @@ static int _macb_recv(struct macb_device *macb, uchar **packetp)
>                 }
>
>                 if (status & MACB_BIT(RX_EOF)) {
> -                       buffer = macb->rx_buffer + 128 * macb->rx_tail;
> +                       buffer = macb->rx_buffer +
> +                               macb->rx_buffer_size * macb->rx_tail;
>                         length = status & RXBUF_FRMLEN_MASK;
>
>                         macb_invalidate_rx_buffer(macb);
>                         if (macb->wrapped) {
>                                 unsigned int headlen, taillen;
>
> -                               headlen = 128 * (MACB_RX_RING_SIZE
> -                                                - macb->rx_tail);
> +                               headlen = macb->rx_buffer_size *
> +                                       (MACB_RX_RING_SIZE - macb->rx_tail);
>                                 taillen = length - headlen;
>                                 memcpy((void *)net_rx_packets[0],
>                                        buffer, headlen);
> @@ -701,7 +710,7 @@ static void gmac_configure_dma(struct macb_device *macb)
>         u32 buffer_size;
>         u32 dmacfg;
>
> -       buffer_size = 128 / RX_BUFFER_MULTIPLE;
> +       buffer_size = macb->rx_buffer_size / RX_BUFFER_MULTIPLE;
>         dmacfg = gem_readl(macb, DMACFG) & ~GEM_BF(RXBS, -1L);
>         dmacfg |= GEM_BF(RXBS, buffer_size);
>
> @@ -746,7 +755,7 @@ static int _macb_init(struct macb_device *macb, const char *name)
>                         paddr |= MACB_BIT(RX_WRAP);
>                 macb->rx_ring[i].addr = paddr;
>                 macb->rx_ring[i].ctrl = 0;
> -               paddr += 128;
> +               paddr += macb->rx_buffer_size;
>         }
>         macb_flush_ring_desc(macb, RX);
>         macb_flush_rx_buffer(macb);
> @@ -957,8 +966,13 @@ static void _macb_eth_initialize(struct macb_device *macb)
>         int id = 0;     /* This is not used by functions we call */
>         u32 ncfgr;
>
> +       if (macb_is_gem(macb))
> +               macb->rx_buffer_size = GEM_RX_BUFFER_SIZE;
> +       else
> +               macb->rx_buffer_size = MACB_RX_BUFFER_SIZE;
> +
>         /* TODO: we need check the rx/tx_ring_dma is dcache line aligned */
> -       macb->rx_buffer = dma_alloc_coherent(MACB_RX_BUFFER_SIZE,
> +       macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * MACB_RX_RING_SIZE,
>                                              &macb->rx_buffer_dma);
>         macb->rx_ring = dma_alloc_coherent(MACB_RX_DMA_DESC_SIZE,
>                                            &macb->rx_ring_dma);
> --
> 2.22.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-07-15 23:25 ` Joe Hershberger
@ 2019-07-16  5:04   ` Ramon Fried
  0 siblings, 0 replies; 10+ messages in thread
From: Ramon Fried @ 2019-07-16  5:04 UTC (permalink / raw)
  To: u-boot

On Tue, Jul 16, 2019 at 2:25 AM Joe Hershberger <joe.hershberger@ni.com> wrote:
>
> On Sun, Jul 14, 2019 at 10:25 AM Ramon Fried <rfried.dev@gmail.com> wrote:
> >
> > Macb Ethernet controller requires a RX buffer of 128 bytes. It is
> > highly sub-optimal for Gigabit-capable GEM that is able to use
> > a bigger DMA buffer. Change this constant and associated macros
> > with data stored in the private structure.
> > RX DMA buffer size has to be multiple of 64 bytes as indicated in
> > DMA Configuration Register specification.
> >
> > Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
>
> Is there a dependency here that you don't mention? This doesn't apply cleanly.
>
It should be applied on my earlier macb patches.
Thanks,
Ramon
> Thanks,
> -Joe
>

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

* [U-Boot] net/macb: increase RX buffer size for GEM
  2019-07-14 15:25 [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM Ramon Fried
  2019-07-15 18:41 ` Joe Hershberger
  2019-07-15 23:25 ` Joe Hershberger
@ 2019-07-25 18:42 ` Joe Hershberger
  2019-08-22  9:38 ` [U-Boot] [PATCH v2] " Stefan Roese
  3 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2019-07-25 18:42 UTC (permalink / raw)
  To: u-boot

Hi Ramon,

https://patchwork.ozlabs.org/patch/1131743/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-07-14 15:25 [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM Ramon Fried
                   ` (2 preceding siblings ...)
  2019-07-25 18:42 ` [U-Boot] " Joe Hershberger
@ 2019-08-22  9:38 ` Stefan Roese
  2019-08-22 11:15   ` Ramon Fried
  3 siblings, 1 reply; 10+ messages in thread
From: Stefan Roese @ 2019-08-22  9:38 UTC (permalink / raw)
  To: u-boot

Hi Ramon,

On 14.07.19 17:25, Ramon Fried wrote:
> Macb Ethernet controller requires a RX buffer of 128 bytes. It is
> highly sub-optimal for Gigabit-capable GEM that is able to use
> a bigger DMA buffer. Change this constant and associated macros
> with data stored in the private structure.
> RX DMA buffer size has to be multiple of 64 bytes as indicated in
> DMA Configuration Register specification.
> 
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>

This patch breaks ethernet on my AT91SAM9G25 based board. With this
patch now, tftp does not complete the xfer any more:

=> tftp 21000000 big
ethernet at f802c000: PHY present at 1
ethernet at f802c000: Starting autonegotiation...
ethernet at f802c000: Autonegotiation complete
ethernet at f802c000: link up, 100Mbps full-duplex (lpa: 0xc5e1)
Using ethernet at f802c000 device
TFTP from server 192.168.1.5; our IP address is 192.168.1.249
Filename 'big'.
Load address: 0x21000000
Loading: #T T

With this patch reverted (as well as with v2019.07), tftp works
just fine.

I did not look into the patch yet. Perhaps you have a quick idea
on why this breaks my platform.

BTW: When I disable the dcache (dcache off), tftp also works fine
with this patch. So its definitely something cache / dma related.

Thanks,
Stefan

> ---
> v2: Fix multi-line comment style
>   drivers/net/macb.c | 30 ++++++++++++++++++++++--------
>   1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index c072f99d8f..3c8b9722b3 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -45,10 +45,17 @@
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> -#define MACB_RX_BUFFER_SIZE		4096
> -#define MACB_RX_RING_SIZE		(MACB_RX_BUFFER_SIZE / 128)
> +/*
> + * These buffer sizes must be power of 2 and divisible
> + * by RX_BUFFER_MULTIPLE
> + */
> +#define MACB_RX_BUFFER_SIZE		128
> +#define GEM_RX_BUFFER_SIZE		2048
>   #define RX_BUFFER_MULTIPLE		64
> +
> +#define MACB_RX_RING_SIZE		32
>   #define MACB_TX_RING_SIZE		16
> +
>   #define MACB_TX_TIMEOUT		1000
>   #define MACB_AUTONEG_TIMEOUT	5000000
>   
> @@ -95,6 +102,7 @@ struct macb_device {
>   	void			*tx_buffer;
>   	struct macb_dma_desc	*rx_ring;
>   	struct macb_dma_desc	*tx_ring;
> +	size_t			rx_buffer_size;
>   
>   	unsigned long		rx_buffer_dma;
>   	unsigned long		rx_ring_dma;
> @@ -395,15 +403,16 @@ static int _macb_recv(struct macb_device *macb, uchar **packetp)
>   		}
>   
>   		if (status & MACB_BIT(RX_EOF)) {
> -			buffer = macb->rx_buffer + 128 * macb->rx_tail;
> +			buffer = macb->rx_buffer +
> +				macb->rx_buffer_size * macb->rx_tail;
>   			length = status & RXBUF_FRMLEN_MASK;
>   
>   			macb_invalidate_rx_buffer(macb);
>   			if (macb->wrapped) {
>   				unsigned int headlen, taillen;
>   
> -				headlen = 128 * (MACB_RX_RING_SIZE
> -						 - macb->rx_tail);
> +				headlen = macb->rx_buffer_size *
> +					(MACB_RX_RING_SIZE - macb->rx_tail);
>   				taillen = length - headlen;
>   				memcpy((void *)net_rx_packets[0],
>   				       buffer, headlen);
> @@ -701,7 +710,7 @@ static void gmac_configure_dma(struct macb_device *macb)
>   	u32 buffer_size;
>   	u32 dmacfg;
>   
> -	buffer_size = 128 / RX_BUFFER_MULTIPLE;
> +	buffer_size = macb->rx_buffer_size / RX_BUFFER_MULTIPLE;
>   	dmacfg = gem_readl(macb, DMACFG) & ~GEM_BF(RXBS, -1L);
>   	dmacfg |= GEM_BF(RXBS, buffer_size);
>   
> @@ -746,7 +755,7 @@ static int _macb_init(struct macb_device *macb, const char *name)
>   			paddr |= MACB_BIT(RX_WRAP);
>   		macb->rx_ring[i].addr = paddr;
>   		macb->rx_ring[i].ctrl = 0;
> -		paddr += 128;
> +		paddr += macb->rx_buffer_size;
>   	}
>   	macb_flush_ring_desc(macb, RX);
>   	macb_flush_rx_buffer(macb);
> @@ -957,8 +966,13 @@ static void _macb_eth_initialize(struct macb_device *macb)
>   	int id = 0;	/* This is not used by functions we call */
>   	u32 ncfgr;
>   
> +	if (macb_is_gem(macb))
> +		macb->rx_buffer_size = GEM_RX_BUFFER_SIZE;
> +	else
> +		macb->rx_buffer_size = MACB_RX_BUFFER_SIZE;
> +
>   	/* TODO: we need check the rx/tx_ring_dma is dcache line aligned */
> -	macb->rx_buffer = dma_alloc_coherent(MACB_RX_BUFFER_SIZE,
> +	macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * MACB_RX_RING_SIZE,
>   					     &macb->rx_buffer_dma);
>   	macb->rx_ring = dma_alloc_coherent(MACB_RX_DMA_DESC_SIZE,
>   					   &macb->rx_ring_dma);
> 

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-08-22  9:38 ` [U-Boot] [PATCH v2] " Stefan Roese
@ 2019-08-22 11:15   ` Ramon Fried
  2019-08-22 14:59     ` Stefan Roese
  0 siblings, 1 reply; 10+ messages in thread
From: Ramon Fried @ 2019-08-22 11:15 UTC (permalink / raw)
  To: u-boot



On August 22, 2019 12:38:08 PM GMT+03:00, Stefan Roese <stefan.roese@gmail.com> wrote:
>Hi Ramon,
>
>On 14.07.19 17:25, Ramon Fried wrote:
>> Macb Ethernet controller requires a RX buffer of 128 bytes. It is
>> highly sub-optimal for Gigabit-capable GEM that is able to use
>> a bigger DMA buffer. Change this constant and associated macros
>> with data stored in the private structure.
>> RX DMA buffer size has to be multiple of 64 bytes as indicated in
>> DMA Configuration Register specification.
>> 
>> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
>
>This patch breaks ethernet on my AT91SAM9G25 based board. With this
>patch now, tftp does not complete the xfer any more:
>
>=> tftp 21000000 big
>ethernet at f802c000: PHY present at 1
>ethernet at f802c000: Starting autonegotiation...
>ethernet at f802c000: Autonegotiation complete
>ethernet at f802c000: link up, 100Mbps full-duplex (lpa: 0xc5e1)
>Using ethernet at f802c000 device
>TFTP from server 192.168.1.5; our IP address is 192.168.1.249
>Filename 'big'.
>Load address: 0x21000000
>Loading: #T T
>
>With this patch reverted (as well as with v2019.07), tftp works
>just fine.
>
>I did not look into the patch yet. Perhaps you have a quick idea
>on why this breaks my platform.
>
>BTW: When I disable the dcache (dcache off), tftp also works fine
>with this patch. So its definitely something cache / dma related.
>
Thanks for letting me know, I'll be next to a PC tomorrow, I'll look into it. 
Thanks, 
Ramon 
>Thanks,
>Stefan
>
>> ---
>> v2: Fix multi-line comment style
>>   drivers/net/macb.c | 30 ++++++++++++++++++++++--------
>>   1 file changed, 22 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
>> index c072f99d8f..3c8b9722b3 100644
>> --- a/drivers/net/macb.c
>> +++ b/drivers/net/macb.c
>> @@ -45,10 +45,17 @@
>>   
>>   DECLARE_GLOBAL_DATA_PTR;
>>   
>> -#define MACB_RX_BUFFER_SIZE		4096
>> -#define MACB_RX_RING_SIZE		(MACB_RX_BUFFER_SIZE / 128)
>> +/*
>> + * These buffer sizes must be power of 2 and divisible
>> + * by RX_BUFFER_MULTIPLE
>> + */
>> +#define MACB_RX_BUFFER_SIZE		128
>> +#define GEM_RX_BUFFER_SIZE		2048
>>   #define RX_BUFFER_MULTIPLE		64
>> +
>> +#define MACB_RX_RING_SIZE		32
>>   #define MACB_TX_RING_SIZE		16
>> +
>>   #define MACB_TX_TIMEOUT		1000
>>   #define MACB_AUTONEG_TIMEOUT	5000000
>>   
>> @@ -95,6 +102,7 @@ struct macb_device {
>>   	void			*tx_buffer;
>>   	struct macb_dma_desc	*rx_ring;
>>   	struct macb_dma_desc	*tx_ring;
>> +	size_t			rx_buffer_size;
>>   
>>   	unsigned long		rx_buffer_dma;
>>   	unsigned long		rx_ring_dma;
>> @@ -395,15 +403,16 @@ static int _macb_recv(struct macb_device *macb,
>uchar **packetp)
>>   		}
>>   
>>   		if (status & MACB_BIT(RX_EOF)) {
>> -			buffer = macb->rx_buffer + 128 * macb->rx_tail;
>> +			buffer = macb->rx_buffer +
>> +				macb->rx_buffer_size * macb->rx_tail;
>>   			length = status & RXBUF_FRMLEN_MASK;
>>   
>>   			macb_invalidate_rx_buffer(macb);
>>   			if (macb->wrapped) {
>>   				unsigned int headlen, taillen;
>>   
>> -				headlen = 128 * (MACB_RX_RING_SIZE
>> -						 - macb->rx_tail);
>> +				headlen = macb->rx_buffer_size *
>> +					(MACB_RX_RING_SIZE - macb->rx_tail);
>>   				taillen = length - headlen;
>>   				memcpy((void *)net_rx_packets[0],
>>   				       buffer, headlen);
>> @@ -701,7 +710,7 @@ static void gmac_configure_dma(struct macb_device
>*macb)
>>   	u32 buffer_size;
>>   	u32 dmacfg;
>>   
>> -	buffer_size = 128 / RX_BUFFER_MULTIPLE;
>> +	buffer_size = macb->rx_buffer_size / RX_BUFFER_MULTIPLE;
>>   	dmacfg = gem_readl(macb, DMACFG) & ~GEM_BF(RXBS, -1L);
>>   	dmacfg |= GEM_BF(RXBS, buffer_size);
>>   
>> @@ -746,7 +755,7 @@ static int _macb_init(struct macb_device *macb,
>const char *name)
>>   			paddr |= MACB_BIT(RX_WRAP);
>>   		macb->rx_ring[i].addr = paddr;
>>   		macb->rx_ring[i].ctrl = 0;
>> -		paddr += 128;
>> +		paddr += macb->rx_buffer_size;
>>   	}
>>   	macb_flush_ring_desc(macb, RX);
>>   	macb_flush_rx_buffer(macb);
>> @@ -957,8 +966,13 @@ static void _macb_eth_initialize(struct
>macb_device *macb)
>>   	int id = 0;	/* This is not used by functions we call */
>>   	u32 ncfgr;
>>   
>> +	if (macb_is_gem(macb))
>> +		macb->rx_buffer_size = GEM_RX_BUFFER_SIZE;
>> +	else
>> +		macb->rx_buffer_size = MACB_RX_BUFFER_SIZE;
>> +
>>   	/* TODO: we need check the rx/tx_ring_dma is dcache line aligned
>*/
>> -	macb->rx_buffer = dma_alloc_coherent(MACB_RX_BUFFER_SIZE,
>> +	macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size *
>MACB_RX_RING_SIZE,
>>   					     &macb->rx_buffer_dma);
>>   	macb->rx_ring = dma_alloc_coherent(MACB_RX_DMA_DESC_SIZE,
>>   					   &macb->rx_ring_dma);
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-08-22 11:15   ` Ramon Fried
@ 2019-08-22 14:59     ` Stefan Roese
  2019-08-23  5:54       ` Ramon Fried
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Roese @ 2019-08-22 14:59 UTC (permalink / raw)
  To: u-boot

Hi Ramon,

On 22.08.19 13:15, Ramon Fried wrote:
> 
> 
> On August 22, 2019 12:38:08 PM GMT+03:00, Stefan Roese <stefan.roese@gmail.com> wrote:
>> Hi Ramon,
>>
>> On 14.07.19 17:25, Ramon Fried wrote:
>>> Macb Ethernet controller requires a RX buffer of 128 bytes. It is
>>> highly sub-optimal for Gigabit-capable GEM that is able to use
>>> a bigger DMA buffer. Change this constant and associated macros
>>> with data stored in the private structure.
>>> RX DMA buffer size has to be multiple of 64 bytes as indicated in
>>> DMA Configuration Register specification.
>>>
>>> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
>>
>> This patch breaks ethernet on my AT91SAM9G25 based board. With this
>> patch now, tftp does not complete the xfer any more:
>>
>> => tftp 21000000 big
>> ethernet at f802c000: PHY present at 1
>> ethernet at f802c000: Starting autonegotiation...
>> ethernet at f802c000: Autonegotiation complete
>> ethernet at f802c000: link up, 100Mbps full-duplex (lpa: 0xc5e1)
>> Using ethernet at f802c000 device
>> TFTP from server 192.168.1.5; our IP address is 192.168.1.249
>> Filename 'big'.
>> Load address: 0x21000000
>> Loading: #T T
>>
>> With this patch reverted (as well as with v2019.07), tftp works
>> just fine.
>>
>> I did not look into the patch yet. Perhaps you have a quick idea
>> on why this breaks my platform.
>>
>> BTW: When I disable the dcache (dcache off), tftp also works fine
>> with this patch. So its definitely something cache / dma related.
>>
> Thanks for letting me know, I'll be next to a PC tomorrow, I'll look
> into it.

I did look into this patch a bit and did not find any functional
change for the non-GEM part I'm using. Hopefully I'll find some more
time tomorrow to dig into this. If you spot something meanwhile,
then even better. ;)

Thanks,
Stefan

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-08-22 14:59     ` Stefan Roese
@ 2019-08-23  5:54       ` Ramon Fried
  2019-08-23  8:35         ` Stefan Roese
  0 siblings, 1 reply; 10+ messages in thread
From: Ramon Fried @ 2019-08-23  5:54 UTC (permalink / raw)
  To: u-boot



On August 22, 2019 5:59:11 PM GMT+03:00, Stefan Roese <sr@denx.de> wrote:
>Hi Ramon,
>
>On 22.08.19 13:15, Ramon Fried wrote:
>> 
>> 
>> On August 22, 2019 12:38:08 PM GMT+03:00, Stefan Roese
><stefan.roese@gmail.com> wrote:
>>> Hi Ramon,
>>>
>>> On 14.07.19 17:25, Ramon Fried wrote:
>>>> Macb Ethernet controller requires a RX buffer of 128 bytes. It is
>>>> highly sub-optimal for Gigabit-capable GEM that is able to use
>>>> a bigger DMA buffer. Change this constant and associated macros
>>>> with data stored in the private structure.
>>>> RX DMA buffer size has to be multiple of 64 bytes as indicated in
>>>> DMA Configuration Register specification.
>>>>
>>>> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
>>>
>>> This patch breaks ethernet on my AT91SAM9G25 based board. With this
>>> patch now, tftp does not complete the xfer any more:
>>>
>>> => tftp 21000000 big
>>> ethernet at f802c000: PHY present at 1
>>> ethernet at f802c000: Starting autonegotiation...
>>> ethernet at f802c000: Autonegotiation complete
>>> ethernet at f802c000: link up, 100Mbps full-duplex (lpa: 0xc5e1)
>>> Using ethernet at f802c000 device
>>> TFTP from server 192.168.1.5; our IP address is 192.168.1.249
>>> Filename 'big'.
>>> Load address: 0x21000000
>>> Loading: #T T
>>>
>>> With this patch reverted (as well as with v2019.07), tftp works
>>> just fine.
>>>
>>> I did not look into the patch yet. Perhaps you have a quick idea
>>> on why this breaks my platform.
>>>
>>> BTW: When I disable the dcache (dcache off), tftp also works fine
>>> with this patch. So its definitely something cache / dma related.
>>>
>> Thanks for letting me know, I'll be next to a PC tomorrow, I'll look
>> into it.
>
>I did look into this patch a bit and did not find any functional
>change for the non-GEM part I'm using. Hopefully I'll find some more
>time tomorrow to dig into this. If you spot something meanwhile,
>then even better. ;)
Are you sure this is the bad patch? 
try master branch and bisect. 
I would also make sure that your HW is doesn't identify as GEM. 
Thanks, Ramon 
>
>Thanks,
>Stefan

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM
  2019-08-23  5:54       ` Ramon Fried
@ 2019-08-23  8:35         ` Stefan Roese
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2019-08-23  8:35 UTC (permalink / raw)
  To: u-boot

On 23.08.19 07:54, Ramon Fried wrote:

<snip>

>>>> BTW: When I disable the dcache (dcache off), tftp also works fine
>>>> with this patch. So its definitely something cache / dma related.
>>>>
>>> Thanks for letting me know, I'll be next to a PC tomorrow, I'll look
>>> into it.
>>
>> I did look into this patch a bit and did not find any functional
>> change for the non-GEM part I'm using. Hopefully I'll find some more
>> time tomorrow to dig into this. If you spot something meanwhile,
>> then even better. ;)
> Are you sure this is the bad patch?
> try master branch and bisect.

I'm using master and this fails. When this patch in question is reverted,
everything works fine. So yes, I'm sure that its this patch.

> I would also make sure that your HW is doesn't identify as GEM.

Yes. macb->rx_buffer_size is set to 128 as without this patch.

I'll look a bit more deeper into this problem later today.

Thanks,
Stefan

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

end of thread, other threads:[~2019-08-23  8:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-14 15:25 [U-Boot] [PATCH v2] net/macb: increase RX buffer size for GEM Ramon Fried
2019-07-15 18:41 ` Joe Hershberger
2019-07-15 23:25 ` Joe Hershberger
2019-07-16  5:04   ` Ramon Fried
2019-07-25 18:42 ` [U-Boot] " Joe Hershberger
2019-08-22  9:38 ` [U-Boot] [PATCH v2] " Stefan Roese
2019-08-22 11:15   ` Ramon Fried
2019-08-22 14:59     ` Stefan Roese
2019-08-23  5:54       ` Ramon Fried
2019-08-23  8:35         ` Stefan Roese

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.