Message ID | 4F60ED8A.8090400@boundarydevices.com |
---|---|
State | Superseded |
Headers | show |
On Wednesday 14 March 2012 15:12:10 Eric Nelson wrote: > On 03/13/2012 10:41 PM, Mike Frysinger wrote: > > On Wednesday 14 March 2012 01:12:38 Eric Nelson wrote: > >> Most of the PPC devices seem to have values of 16 or 32 > >> for ARCH_DMA_MINALIGN, but PPC64BRIDGE and E500MC would > >> have a problem if their drivers don't implement a bounce > >> buffer because PKTALIGN< ARCH_DMA_MINALIGN. > >> > >> (see arch/powerpc/include/asm/cache.h) > >> > >> This condition is properly tested for in fec_mxc.c. > > > > so fix this in common code instead of hacking around it in individual > > drivers. seems to me that PKTALIGN should be defined to > > ARCH_DMA_MINALIGN and ultimately removed. > > I'm not in a position to test against MAKEALL, but it appears that all > architectures have cache.h and define ARCH_DMA_MINALIGN ARCH_DMA_MINALIGN is required. if an arch/board omits it, they are broken and you need not worry about it. we already have common code requiring int. > --- a/include/net.h > +++ b/include/net.h > > -#define PKTALIGN 32 > +#define PKTALIGN ARCH_DMA_MINALIGN looks fine to me -mike
On 03/14/2012 01:33 PM, Mike Frysinger wrote: > On Wednesday 14 March 2012 15:12:10 Eric Nelson wrote: >> On 03/13/2012 10:41 PM, Mike Frysinger wrote: >>> On Wednesday 14 March 2012 01:12:38 Eric Nelson wrote: >>>> Most of the PPC devices seem to have values of 16 or 32 >>>> for ARCH_DMA_MINALIGN, but PPC64BRIDGE and E500MC would >>>> have a problem if their drivers don't implement a bounce >>>> buffer because PKTALIGN< ARCH_DMA_MINALIGN. >>>> >>>> (see arch/powerpc/include/asm/cache.h) >>>> >>>> This condition is properly tested for in fec_mxc.c. >>> >>> so fix this in common code instead of hacking around it in individual >>> drivers. seems to me that PKTALIGN should be defined to >>> ARCH_DMA_MINALIGN and ultimately removed. >> >> I'm not in a position to test against MAKEALL, but it appears that all >> architectures have cache.h and define ARCH_DMA_MINALIGN > > ARCH_DMA_MINALIGN is required. if an arch/board omits it, they are broken and > you need not worry about it. we already have common code requiring int. > Sounds good. >> --- a/include/net.h >> +++ b/include/net.h >> >> -#define PKTALIGN 32 >> +#define PKTALIGN ARCH_DMA_MINALIGN > > looks fine to me > -mike You want I should send a formal patch? Should I consider "looks fine" to be an ack? If so, I'll also send an update (V5) to fec_mxc that removes the check on PKTALIGN. Please advise, Eric
On Wednesday 14 March 2012 17:04:32 Eric Nelson wrote: > You want I should send a formal patch? yes please > Should I consider "looks fine" to be an ack? i'll post an acked-by tag to that and then patchwork will do the right thing for people to track -mike
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;