[U-Boot] [GIT] Pull request: u-boot-dfu

[Tom Rini <trini@konsulko.com>]

On Wed, Aug 17, 2016 at 04:55:52PM +0200, Lukasz Majewski wrote:
> Hi Tom,
> 
> > On Wed, Aug 17, 2016 at 11:56:59AM +0200, Lukasz Majewski wrote:
> > > Hi Heiko, Ravi,
> > > 
> > > > Hello B, Ravi,
> > > > 
> > > > Am 17.08.2016 um 09:40 schrieb B, Ravi:
> > > > > Hi Heiko
> > > > >
> > > > >>>>
> > > > >>>> is that for master or next ?
> > > > >
> > > > >>> This patch _was_ supposed to go to "master"
> > > > >
> > > > >>>> Was this build tested ?
> > > > >
> > > > >>> Unfortunately, not so thoroughly as I thought.
> > > > >
> > > > >>> Moving dfu code to SPL causes following error on some boards:
> > > > >
> > > > >>>        arm:  +   smartweb
> > > > >>> +In file included from ../include/dfu.h:18:0,
> > > > >>> +                 from ../common/dfu.c:16:
> > > > >>> +../include/linux/usb/composite.h:331:9: error: requested
> > > > >>> alignment is +not an integer constant
> > > > >>> +  struct usb_device_descriptor
> > > > >>> __aligned(CONFIG_SYS_CACHELINE_SIZE) desc;
> > > > >>> +         ^
> > > > >>> +make[3]: *** [spl/common/dfu.o] Error 1
> > > > >>> +make[2]: *** [spl/common] Error 2
> > > > >> +make[1]: *** [spl/u-boot-spl] Error 2
> > > > >>> +make: *** [sub-make] Error 2
> > > > >
> > > > > The CONFIG_SYS_CACHELINE_SIZE is not defined for smartweb
> > > > > platform which is causing build error. By defining this error
> > > > > goes away. What would be value for cache line size for smartweb
> > > > > platform? (32/64/..?)
> > > > 
> > > > Hups? Which patch introduced this? I wonder if other at91
> > > > based boards does not show the same error?
> > > > 
> > > > The RM for the at91sam9260 says:  8 words cache line size
> > > > 
> > > > So 32 would be the correct value.
> > > 
> > > Heiko, thanks for your support.
> > > 
> > > Ravi, Marek Vasut before accepting PR tests following archs with
> > > buildman: arm, aarch64, mips, powerpc, nios2 [1]
> > > 
> > > Regarding the CONFIG_SYS_CACHELINE_SIZE, some older boards do not
> > > define it. Hence we have several problems (also with other patches
> > > http://patchwork.ozlabs.org/patch/657216/).
> > > 
> > > IMHO, it would be best to check [1] and then add this define if
> > > required.
> > > 
> > > Or does anybody have any better idea?
> > 
> > This is something we can get right in Kconfig, I believe.  Looking at
> > the Linux Kernel, the relevant bits I think are in
> > arch/arm/mm/Kconfig: config ARM_L1_CACHE_SHIFT_6
> >         bool
> >         default y if CPU_V7
> >         help
> >           Setting ARM L1 cache line size to 64 Bytes.
> > 
> > config ARM_L1_CACHE_SHIFT
> >         int
> >         default 6 if ARM_L1_CACHE_SHIFT_6
> >         default 5
> > 
> > I only hesitate a little as there's not a 1:1 match-up between those
> > values and all ARMv7 platforms we have today for example.
> 
> True.
> 
> I do recall that some Cortex-A9 had 32B L1 cache and Cortex-A8 had 64B
> L1 cache line size.
> 
> And both were ARMv7 cores.

I had a quick conversation with some folks and the answer is that it's
OK to round up (as it appears that our uses of CONFIG_SYS_CACHELINE_SIZE
are) so long as the mechanics of the flushes work at the smallest
implemented increment, which is what the Linux Kernel does and we're
using that code.  So we should be safe to mirror the kernel logic here.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160817/99f4ed79/attachment.sig>