Message ID | 1469635835-2063-1-git-send-email-hdegoede@redhat.com |
---|---|
State | Superseded |
Delegated to: | Hans de Goede |
Headers | show |
Hello, > index 7c088c3..877859c 100644 > --- a/drivers/net/sun8i_emac.c > +++ b/drivers/net/sun8i_emac.c > @@ -32,7 +32,8 @@ > > #define CONFIG_TX_DESCR_NUM 32 > #define CONFIG_RX_DESCR_NUM 32 > -#define CONFIG_ETH_BUFSIZE 2024 > +#define CONFIG_ETH_BUFSIZE 2048 > +#define CONFIG_ETH_RXSIZE 2024 /* Note most fit in ETH_BUFSIZE */ As per the following comment made in Linux driver. /* The datasheet said that each descriptor can transfers up to 4096bytes + * But latter, a register documentation reduce that value to 2048 + * Anyway using 2048 cause strange behaviours and even BSP driver use 2047 + */ Can we keep CONFIG_ETH_BUFSIZE to 2044 ? Thanks, Amit.
On Wed, 2016-07-27 at 18:10 +0200, Hans de Goede wrote: > This fixes the following CACHE warnings when using sun8i_emac: > > => dhcp > BOOTP broadcast 1 > BOOTP broadcast 2 > CACHE: Misaligned operation at range [7bf594a8, 7bf59628] > BOOTP broadcast 3 > CACHE: Misaligned operation at range [7bf59c90, 7bf59e10] > CACHE: Misaligned operation at range [7bf5a478, 7bf5a5f8] > DHCP client bound to address 10.42.43.80 (1009 ms) > > Cc: Chen-Yu Tsai <wens@csie.org> > Cc: Corentin LABBE <clabbe.montjoie@gmail.com> > Cc: Amit Singh Tomar <amittomer25@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/net/sun8i_emac.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c > index 7c088c3..877859c 100644 > --- a/drivers/net/sun8i_emac.c > +++ b/drivers/net/sun8i_emac.c > @@ -32,7 +32,8 @@ > > #define CONFIG_TX_DESCR_NUM 32 > #define CONFIG_RX_DESCR_NUM 32 > -#define CONFIG_ETH_BUFSIZE 2024 > +#define CONFIG_ETH_BUFSIZE 2048 > +#define CONFIG_ETH_RXSIZE 2024 /* Note most fit in ETH_BUFSIZE */ s/most/must/? A comment (perhaps in the commit message rather than the code) as to how/why RXSIZE and BUFSIZE interact to affect the alignment in the desired fasion would be useful, since it is non-obvious to me at least. I was about to speculate on the difference of 14 bytes relating to the Ethernet frame header, but then I realised it's 24 not 14 and deleted those paragraphs, which I think underscores the need for a comment ;-) Ian.
Hi, On 27-07-16 19:11, Amit Tomer wrote: > Hello, > >> index 7c088c3..877859c 100644 >> --- a/drivers/net/sun8i_emac.c >> +++ b/drivers/net/sun8i_emac.c >> @@ -32,7 +32,8 @@ >> >> #define CONFIG_TX_DESCR_NUM 32 >> #define CONFIG_RX_DESCR_NUM 32 >> -#define CONFIG_ETH_BUFSIZE 2024 >> +#define CONFIG_ETH_BUFSIZE 2048 >> +#define CONFIG_ETH_RXSIZE 2024 /* Note most fit in ETH_BUFSIZE */ > > As per the following comment made in Linux driver. > > /* The datasheet said that each descriptor can transfers up to 4096bytes > + * But latter, a register documentation reduce that value to 2048 > + * Anyway using 2048 cause strange behaviours and even BSP driver use 2047 > + */ > > Can we keep CONFIG_ETH_BUFSIZE to 2044 ? Note that my patch is introducing a CONFIG_ETH_RXSIZE and is using that where ever CONFIG_ETH_BUFSIZE is not used to actually allocate or cache-flush a buffer. And no we cannot use 2044 as then the 2nd buffer in the array still is not cache-aligned. But using 2048 is fine, as long as we only use it to allocate buffers, and not actually as the value passed to the hardware (which is what the new CONFIG_ETH_RXSIZE is for). I started with just using 2048 and that does indeed not work, but with the addition of CONFIG_ETH_RXSIZE things work fine. Regards, Hans
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 7c088c3..877859c 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -32,7 +32,8 @@ #define CONFIG_TX_DESCR_NUM 32 #define CONFIG_RX_DESCR_NUM 32 -#define CONFIG_ETH_BUFSIZE 2024 +#define CONFIG_ETH_BUFSIZE 2048 +#define CONFIG_ETH_RXSIZE 2024 /* Note most 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) @@ -324,7 +325,7 @@ static void rx_descs_init(struct emac_eth_dev *priv) desc_p->buf_addr = (uintptr_t)&rxbuffs[idx * CONFIG_ETH_BUFSIZE] ; desc_p->next = (uintptr_t)&desc_table_p[idx + 1]; - desc_p->st |= CONFIG_ETH_BUFSIZE; + desc_p->st |= CONFIG_ETH_RXSIZE; desc_p->status = BIT(31); } @@ -506,7 +507,7 @@ static int _sun8i_eth_recv(struct emac_eth_dev *priv, uchar **packetp) roundup(data_end, ARCH_DMA_MINALIGN)); if (good_packet) { - if (length > CONFIG_ETH_BUFSIZE) { + if (length > CONFIG_ETH_RXSIZE) { printf("Received packet is too big (len=%d)\n", length); return -EMSGSIZE;
This fixes the following CACHE warnings when using sun8i_emac: => dhcp BOOTP broadcast 1 BOOTP broadcast 2 CACHE: Misaligned operation at range [7bf594a8, 7bf59628] BOOTP broadcast 3 CACHE: Misaligned operation at range [7bf59c90, 7bf59e10] CACHE: Misaligned operation at range [7bf5a478, 7bf5a5f8] DHCP client bound to address 10.42.43.80 (1009 ms) Cc: Chen-Yu Tsai <wens@csie.org> Cc: Corentin LABBE <clabbe.montjoie@gmail.com> Cc: Amit Singh Tomar <amittomer25@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/net/sun8i_emac.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)