| Message ID | 20160816154125.7124307a@amdc2363 |
|---|---|
| State | Accepted |
| Delegated to: | Marek Vasut |
| Headers | show |
On 08/16/2016 03:41 PM, Lukasz Majewski wrote: > Hi Marek, Hi, is that for master or next ? Was this build tested ? > 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@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 <l.majewski@samsung.com> > > by HWT system >
Hi Marek, > On 08/16/2016 03:41 PM, Lukasz Majewski wrote: > > Hi Marek, > > Hi, > > 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 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. > > > 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@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 <l.majewski@samsung.com> > > > > by HWT system > > > >
Hi Lukasz, Marek >> >> 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 >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. 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 >>> >>> 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/..?) >>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. >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
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. bye, Heiko > >>> 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. > >> 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 >
>>>> +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 spl-dfu patch series include usb header files in SPL for DFU, since cache line is not defined which is causing the build failure for smartweb platform. >The RM for the at91sam9260 says: 8 words cache line size >So 32 would be the correct value. Ok thanks. Regards Ravi
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? > > bye, > Heiko > > > >>> 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. > > > >> 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 > > >
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.
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. >
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.
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). >
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.
Hi Tom, > 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. > Ok, now it is clear :-) Thanks for providing solution.
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@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 <l.majewski@samsung.com> by HWT system