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

[Tom Rini]

On Wed, Aug 17, 2016 at 06:17:44PM +0200, Lukasz Majewski wrote:
> Hi Tom,
> 
> > 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.
> 
> +1
> 
> If I understood you correctly, for armv7 boards with undefined
> CONFIG_SYS_CACHELINE_SIZE we should use the largest cache line (IIRC
> 64B).

I'm saying we should just mirror the kernel and universally use 64B on
CPU_V7 (and note, not V7M) and 32B otherwise, wrt ARM at least.

-- 
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/6ecc6310/attachment.sig>