diff mbox

[U-Boot,V3] net: fec_mxc: allow use with cache enabled

Message ID 4F60ED8A.8090400@boundarydevices.com
State Superseded
Headers show

Commit Message

Eric Nelson March 14, 2012, 7:12 p.m. UTC 
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.
> -mike

Hi Mike,

I'm not in a position to test against MAKEALL, but it appears that all
architectures have cache.h and define ARCH_DMA_MINALIGN, so it should
be trivially easy to fix PKTALIGN to be at least ARCH_DMA_MINALIGN as
shown below.

PKTSIZE_ALIGN seems safe for all architectures at 1536.

Note that this will reduce the value to 16 for some PPC devices, but I
haven't found any place that this would break things.

Is this what you're after?

Are you in a position to run MAKEALL or can you advise about the
requirements?

Please advise,


Eric


~/u-boot-imx6$ git diff

Comments

Mike Frysinger March 14, 2012, 8:33 p.m. UTC | #1 
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
Eric Nelson March 14, 2012, 9:04 p.m. UTC | #2 
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
Mike Frysinger March 14, 2012, 9:15 p.m. UTC | #3 
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 mbox

Patch

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;