[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[Eric Nelson]

This will prevent the need for architectures whose DMA alignment
is greater than 32 to have bounce buffers.

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 include/net.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/include/net.h b/include/net.h
index e4d42c2..ff428d0 100644
--- a/include/net.h
+++ b/include/net.h
@@ -16,6 +16,7 @@
 #include <commproc.h>
 #endif	/* CONFIG_8xx */
 
+#include <asm/cache.h>
 #include <asm/byteorder.h>	/* for nton* / ntoh* stuff */
 
 
@@ -31,7 +32,7 @@
 # define PKTBUFSRX	4
 #endif
 
-#define PKTALIGN	32
+#define PKTALIGN	ARCH_DMA_MINALIGN
 
 /* IPv4 addresses are always 32 bits in size */
 typedef u32		IPaddr_t;
-- 
1.7.9

[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[Mike Frysinger]

Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120314/3fd690ec/attachment.pgp>

[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[Marek Vasut]

Dear Eric Nelson,

> This will prevent the need for architectures whose DMA alignment
> is greater than 32 to have bounce buffers.
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---
>  include/net.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net.h b/include/net.h
> index e4d42c2..ff428d0 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -16,6 +16,7 @@
>  #include <commproc.h>
>  #endif	/* CONFIG_8xx */
> 
> +#include <asm/cache.h>
>  #include <asm/byteorder.h>	/* for nton* / ntoh* stuff */
> 
> 
> @@ -31,7 +32,7 @@
>  # define PKTBUFSRX	4
>  #endif
> 
> -#define PKTALIGN	32
> +#define PKTALIGN	ARCH_DMA_MINALIGN
> 
>  /* IPv4 addresses are always 32 bits in size */
>  typedef u32		IPaddr_t;

Won't this break retarded hardware that has ARCH_DMA_MINALIGN smaller than 32 ?

Best regards,
Marek Vasut

[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[Mike Frysinger]

On Thursday 15 March 2012 22:50:08 Marek Vasut wrote:
> Dear Eric Nelson,
> > This will prevent the need for architectures whose DMA alignment
> > is greater than 32 to have bounce buffers.
> > 
> > --- a/include/net.h
> > +++ b/include/net.h
> > 
> > -#define PKTALIGN	32
> > +#define PKTALIGN	ARCH_DMA_MINALIGN
> 
> Won't this break retarded hardware that has ARCH_DMA_MINALIGN smaller than
> 32 ?

why would it ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120315/6f794f64/attachment.pgp>

[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[Eric Nelson]

On 03/15/2012 07:50 PM, Marek Vasut wrote:
> Dear Eric Nelson,
>
>> This will prevent the need for architectures whose DMA alignment
>> is greater than 32 to have bounce buffers.
>>
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>> ---
>>   include/net.h |    3 ++-
>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/net.h b/include/net.h
>> index e4d42c2..ff428d0 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -16,6 +16,7 @@
>>   #include<commproc.h>
>>   #endif	/* CONFIG_8xx */
>>
>> +#include<asm/cache.h>
>>   #include<asm/byteorder.h>	/* for nton* / ntoh* stuff */
>>
>>
>> @@ -31,7 +32,7 @@
>>   # define PKTBUFSRX	4
>>   #endif
>>
>> -#define PKTALIGN	32
>> +#define PKTALIGN	ARCH_DMA_MINALIGN
>>
>>   /* IPv4 addresses are always 32 bits in size */
>>   typedef u32		IPaddr_t;
>
> Won't this break retarded hardware that has ARCH_DMA_MINALIGN smaller than 32 ?
>

I can't see how it would.

This macro is only used for alignment and the PKTSIZE_ALIGN variable should be
safe on any architecture at 1536 (0x600) bytes.

I do see a potential problem with eth.c iff CONFIG_API is set, but I'm
unfamiliar with this API. It appears to me that this declaration
needs to be aligned, with or without the PKTALIGN change or it won't be
usable with DMA since PKTSIZE is unlikely to be cache line or DMA friendly
at 1518 (0x5EE) bytes.

	static struct {
		uchar data[PKTSIZE];
		int length;
	} eth_rcv_bufs[PKTBUFSRX];

It appears that the CONFIG_API macro is only defined for PMC440. Is this
supported for other machines?

Is there some test code available?

Please advise,


Eric

[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[Marek Vasut]

Dear Mike Frysinger,

> On Thursday 15 March 2012 22:50:08 Marek Vasut wrote:
> > Dear Eric Nelson,
> > 
> > > This will prevent the need for architectures whose DMA alignment
> > > is greater than 32 to have bounce buffers.
> > > 
> > > --- a/include/net.h
> > > +++ b/include/net.h
> > > 
> > > -#define PKTALIGN	32
> > > +#define PKTALIGN	ARCH_DMA_MINALIGN
> > 
> > Won't this break retarded hardware that has ARCH_DMA_MINALIGN smaller
> > than 32 ?
> 
> why would it ?

Nevermind.

Best regards,
Marek Vasut

[U-Boot] [PATCH] net: force PKTALIGN to ARCH_DMA_MINALIGN

[Mike Frysinger]

On Thursday 15 March 2012 23:36:34 Eric Nelson wrote:
> I do see a potential problem with eth.c iff CONFIG_API is set, but I'm
> unfamiliar with this API. It appears to me that this declaration
> needs to be aligned, with or without the PKTALIGN change or it won't be
> usable with DMA since PKTSIZE is unlikely to be cache line or DMA friendly
> at 1518 (0x5EE) bytes.
> 
> 	static struct {
> 		uchar data[PKTSIZE];
> 		int length;
> 	} eth_rcv_bufs[PKTBUFSRX];
> 
> It appears that the CONFIG_API macro is only defined for PMC440. Is this
> supported for other machines?

CONFIG_API is the new standalone api.  it uses a syscall-like approach to 
maintaining compatibility unlike the current standalone api which breaks 
easily.

however, this struct is used only internally.  the exposed function calls are 
unchanged.  so changing the size of the buffer is OK generally speaking.

there is a slight behavior change though.  if you look at the funcs, they only 
accept buffers that are smaller than PKTSIZE bytes.  i'm not sure we care about 
this detail though.  i think the max size you can send is arbitrary and was 
designed just so you could get a frame's worth of data at a time, so as long 
as that ceiling isn't violated, it's OK.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120316/d9268c5d/attachment.pgp>