Pull request: u-boot-dfu

Hi Marek,

Please find following code for u-boot-dfu repository.

The following changes since commit 76b2fad775ee3cb58788b11454655ba5a244ac56:

eth: asix88179: Add support for the driver model (2016-08-09 12:52:05 +0200)

are available in the git repository at:

ssh://gu-dfu at git.denx.de/u-boot-dfu e8091b356dd0f1e8e742a5da7a5e8256744b39b7

for you to fetch changes up to e8091b356dd0f1e8e742a5da7a5e8256744b39b7:

cmd: dfu: Add error handling for failed registration (2016-08-16 13:06:04 +0200)

----------------------------------------------------------------
B, Ravi (5):
spl: dfu: add dfu support in SPL
common: dfu: saperate the dfu common functionality
spl: dfu: adding dfu support functions for SPL-DFU
dra7x: boot: add dfu bootmode support
dra7x: configs: enable SPL-DFU support

Sanchayan Maity (1):
cmd: dfu: Add error handling for failed registration

 Kconfig                                      | 27 +++++++++++++++++++++++++++
 arch/arm/cpu/armv7/omap-common/boot-common.c |  5 +++++
 arch/arm/include/asm/arch-omap5/spl.h        |  2 +-
 cmd/dfu.c                                    | 61 ++-----------------------------------------------------------
 common/Makefile                              |  3 +++
 common/dfu.c                                 | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/spl/Makefile                          |  1 +
 common/spl/spl.c                             | 10 +++++++++-
 common/spl/spl_dfu.c                         | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/configs/dra7xx_evm.h                 | 20 +++++++++++++++++++-
 include/configs/ti_omap5_common.h            |  2 --
 include/g_dnl.h                              |  1 +
 include/spl.h                                |  8 ++++++++
 scripts/Makefile.spl                         |  4 ++++
 14 files changed, 229 insertions(+), 64 deletions(-)
 create mode 100644 common/dfu.c
 create mode 100644 common/spl/spl_dfu.c

Test HW: Exynos 4412 Odroid U3, Exynos5422 Odroid XU3

Tested-by Lukasz Majewski <[email protected]>

by HWT system

Quoted 1 line

Hi Marek,

Hi,

is that for master or next ?

Was this build tested ?

Quoted 58 lines

Please find following code for u-boot-dfu repository.

The following changes since commit
76b2fad775ee3cb58788b11454655ba5a244ac56:

eth: asix88179: Add support for the driver model (2016-08-09
12:52:05 +0200)

are available in the git repository at:

ssh://gu-dfu at git.denx.de/u-boot-dfu
e8091b356dd0f1e8e742a5da7a5e8256744b39b7

for you to fetch changes up to
e8091b356dd0f1e8e742a5da7a5e8256744b39b7:

cmd: dfu: Add error handling for failed registration (2016-08-16
13:06:04 +0200)

----------------------------------------------------------------
B, Ravi (5):
spl: dfu: add dfu support in SPL
common: dfu: saperate the dfu common functionality
spl: dfu: adding dfu support functions for SPL-DFU
dra7x: boot: add dfu bootmode support
dra7x: configs: enable SPL-DFU support

Sanchayan Maity (1):
cmd: dfu: Add error handling for failed registration

 Kconfig                                      | 27
+++++++++++++++++++++++++++
arch/arm/cpu/armv7/omap-common/boot-common.c |  5 +++++
arch/arm/include/asm/arch-omap5/spl.h        |  2 +-
cmd/dfu.c                                    | 61

++-----------------------------------------------------------
common/Makefile | 3 +++
common/dfu.c | 92
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

common/spl/Makefile                          |  1 +
common/spl/spl.c                             | 10 +++++++++-
common/spl/spl_dfu.c                         | 57
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/configs/dra7xx_evm.h                 | 20
+++++++++++++++++++- include/configs/ti_omap5_common.h
|  2 -- include/g_dnl.h                              |  1 +
include/spl.h                                |  8 ++++++++
scripts/Makefile.spl                         |  4 ++++ 14 files

changed, 229 insertions(+), 64 deletions(-) create mode 100644 common/dfu.c create mode 100644 common/spl/spl_dfu.c

Test HW: Exynos 4412 Odroid U3, Exynos5422 Odroid XU3

Tested-by Lukasz Majewski <[email protected]>

by HWT system

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +10 -0

+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

Ravi, to reproduce you need to fetch u-boot-dfu/test

and run buildman:

This is only a small subset of affected boards, so please test also "arm" (which might take long time).

./tools/buildman/buildman.py --branch=HEAD siemens --detail --verbose --show_errors --force-build --count=5 --output-dir=./BUILD/

Thanks Marek, for pointing out.

Quoted 10 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

Review

Quoted 9 lines

arm: + smartweb

(quoted diff) +8 -0

+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

Quoted 1 line

+make: *** [sub-make] Error 2

Quoted 9 lines

Ravi, to reproduce you need to fetch u-boot-dfu/test

and run buildman:

This is only a small subset of affected boards, so please test also "arm" (which might take long time).

./tools/buildman/buildman.py --branch=HEAD siemens --detail --verbose --show_errors --force-build --count=5 --output-dir=./BUILD/

Quoted 1 line

Thanks Marek, for pointing out.

I have test build for all ti-platforms, not checked for rest. My bad ..!
I could reproduce this error, for some platform CONFIG_SYS_CACHELINE_SIZE is not defined. (eg. configs/smartweb_defconfig). I will check with "arm" as well, let you know.

Hi Heiko

Quoted 21 lines

Quoted 12 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

Review

Quoted 8 lines

(quoted diff) +8 -0

+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

Quoted 1 line

Quoted 1 line

+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/..?)

Quoted 12 lines

Quoted 10 lines

Ravi, to reproduce you need to fetch u-boot-dfu/test

and run buildman:

This is only a small subset of affected boards, so please test also "arm" (which might take long time).

./tools/buildman/buildman.py --branch=HEAD siemens --detail --verbose --show_errors --force-build --count=5 --output-dir=./BUILD/

Quoted 5 lines

Quoted 1 line

Thanks Marek, for pointing out.

I have test build for all ti-platforms, not checked for rest. My bad ..!
I could reproduce this error, for some platform CONFIG_SYS_CACHELINE_SIZE is not defined. (eg. configs/smartweb_defconfig). I will check with "arm" as well, let you know.

Regards
Ravi

Quoted 6 lines

Quoted 6 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+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

Quoted 5 lines

Quoted 1 line

Quoted 1 line

+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.

Hello B, Ravi,

Am 17.08.2016 um 09:40 schrieb B, Ravi:

Quoted 24 lines

Hi Heiko

Quoted 22 lines

Quoted 18 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +3 -0

+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;

Review

Quoted 3 lines

(quoted diff) +3 -0

+         ^
+make[3]: *** [spl/common/dfu.o] Error 1
+make[2]: *** [spl/common] Error 2

+make[1]: *** [spl/u-boot-spl] Error 2

Quoted 6 lines

Quoted 1 line

Quoted 1 line

+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.

bye,
Heiko

Quoted 25 lines

Quoted 14 lines

Quoted 12 lines

Ravi, to reproduce you need to fetch u-boot-dfu/test

and run buildman:

This is only a small subset of affected boards, so please test also "arm" (which might take long time).

./tools/buildman/buildman.py --branch=HEAD siemens --detail --verbose --show_errors --force-build --count=5 --output-dir=./BUILD/

Quoted 7 lines

Quoted 1 line

Thanks Marek, for pointing out.

I have test build for all ti-platforms, not checked for rest. My bad ..! I could reproduce this error, for some platform CONFIG_SYS_CACHELINE_SIZE is not defined. (eg. configs/smartweb_defconfig). I will check with "arm" as well, let you know.

Regards
Ravi

Hi Heiko, Ravi,

Quoted 34 lines

Hello B, Ravi,

Am 17.08.2016 um 09:40 schrieb B, Ravi:

Quoted 24 lines

Hi Heiko

Quoted 22 lines

Quoted 18 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +3 -0

+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;

Review

Quoted 3 lines

(quoted diff) +3 -0

+         ^
+make[3]: *** [spl/common/dfu.o] Error 1
+make[2]: *** [spl/common] Error 2

+make[1]: *** [spl/u-boot-spl] Error 2

Quoted 6 lines

Quoted 1 line

Quoted 1 line

+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/..?)

Quoted 6 lines

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?

Quoted 48 lines

Hi Heiko, Ravi,

Quoted 34 lines

Hello B, Ravi,

Am 17.08.2016 um 09:40 schrieb B, Ravi:

Quoted 24 lines

Hi Heiko

Quoted 22 lines

Quoted 18 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +3 -0

+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;

Review

Quoted 3 lines

(quoted diff) +3 -0

+         ^
+make[3]: *** [spl/common/dfu.o] Error 1
+make[2]: *** [spl/common] Error 2

+make[1]: *** [spl/u-boot-spl] Error 2

Quoted 6 lines

Quoted 1 line

Quoted 1 line

+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/..?)

Quoted 6 lines

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]

Quoted 8 lines

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.

Hi Tom,

Quoted 66 lines · Lukasz Majewski

Quoted 48 lines

Hi Heiko, Ravi,

Quoted 34 lines

Hello B, Ravi,

Am 17.08.2016 um 09:40 schrieb B, Ravi:

Quoted 24 lines

Hi Heiko

Quoted 22 lines

Quoted 18 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +3 -0

+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;

Review

Quoted 3 lines

(quoted diff) +3 -0

+         ^
+make[3]: *** [spl/common/dfu.o] Error 1
+make[2]: *** [spl/common] Error 2

+make[1]: *** [spl/u-boot-spl] Error 2

Quoted 6 lines

Quoted 1 line

Quoted 1 line

+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/..?)

Quoted 6 lines

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]

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 82 lines

Hi Tom,

Quoted 67 lines · Lukasz Majewski

Quoted 49 lines

Hi Heiko, Ravi,

Quoted 35 lines

Hello B, Ravi,

Am 17.08.2016 um 09:40 schrieb B, Ravi:

Quoted 25 lines

Hi Heiko

Quoted 23 lines

Quoted 19 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +3 -0

+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;

Review

Quoted 3 lines

(quoted diff) +3 -0

+         ^
+make[3]: *** [spl/common/dfu.o] Error 1
+make[2]: *** [spl/common] Error 2

+make[1]: *** [spl/u-boot-spl] Error 2

Quoted 6 lines

Quoted 1 line

Quoted 1 line

+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/..?)

Quoted 6 lines

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]

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 1 line

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.

Hi Tom,

Quoted 83 lines · Lukasz Majewski

Quoted 82 lines

Hi Tom,

Quoted 67 lines · Lukasz Majewski

Quoted 49 lines

Hi Heiko, Ravi,

Quoted 35 lines

Hello B, Ravi,

Am 17.08.2016 um 09:40 schrieb B, Ravi:

Quoted 25 lines

Hi Heiko

Quoted 23 lines

Quoted 19 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +3 -0

+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;

Review

Quoted 3 lines

(quoted diff) +3 -0

+         ^
+make[3]: *** [spl/common/dfu.o] Error 1
+make[2]: *** [spl/common] Error 2

+make[1]: *** [spl/u-boot-spl] Error 2

Quoted 6 lines

Quoted 1 line

Quoted 1 line

+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/..?)

Quoted 6 lines

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]

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 8 lines

Quoted 1 line

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).

Quoted 96 lines

Hi Tom,

Quoted 93 lines · Lukasz Majewski

Quoted 79 lines

Hi Tom,

Quoted 68 lines

Quoted 49 lines · Lukasz Majewski

Hi Heiko, Ravi,

Quoted 35 lines

Hello B, Ravi,

Am 17.08.2016 um 09:40 schrieb B, Ravi:

Quoted 25 lines

Hi Heiko

Quoted 23 lines

Quoted 19 lines

Quoted 2 lines

is that for master or next ?

This patch _was_ supposed to go to "master"

Quoted 1 line

Was this build tested ?

Unfortunately, not so thoroughly as I thought.

Moving dfu code to SPL causes following error on some boards:

arm: + smartweb

(quoted diff) +3 -0

+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;

Review

Quoted 3 lines

(quoted diff) +3 -0

+         ^
+make[3]: *** [spl/common/dfu.o] Error 1
+make[2]: *** [spl/common] Error 2

+make[1]: *** [spl/u-boot-spl] Error 2

Quoted 6 lines

Quoted 1 line

Quoted 1 line

+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/..?)

Quoted 6 lines

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]

Quoted 8 lines

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 5 lines

+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.