Re: [U-Boot] [PATCH] spl: mmc: Add option to set eMMC HW boot partition
Marek Vasut writes: > On 9/3/19 4:16 PM, Lukasz Majewski wrote: >> From: Mans Rullgard >> >> This change allows setting pre-defined eMMC boot partition for SPL eMMC >> booting. It is necessary in the case when one wants to boot (through falcon >> boot) from eMMC after loading SPL from other memory (like SPI-NOR). >> >> Signed-off-by: Mans Rullgard >> [lukma: Edit the commit message] >> Signed-off-by: Lukasz Majewski >> Acked-by: Andreas Dannenberg > > Would it be better if this came from /chosen node in DT ? This patch was made for an imx28 based board. Fitting DT support into SPL on that chip won't be easy. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] U-Boot thumb interlink patches
Tom Rini <tr...@konsulko.com> writes: > And then: > https://patchwork.ozlabs.org/patch/909652/ > > has been reviewed and could be applied. Is there anything else I'm > missing? Thanks! I'd be more comfortable with this one if someone actually tested it. I can't since I don't have such hardware. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Beaglebone Black U-boot won't buid - THUMB instruction issue
John Babrick <johnbabr...@gmail.com> writes:
> Hello - I am trying to work through the book "Mastering Embedded Linux
> Programming" by Chris Simmonds, and trying to work through building u-boot
> after having built the cross toolchain. I am running into an error when I
> try to build u-boot, any ideas?
>
> johann@mars:~/uboot-work/u-boot$ make
> CROSS_COMPILE=arm-cortex_a8-linux-gnueabihf- am335x_evm_defconfig
> #
> # configuration written to .config
> #
> johann@mars:~/uboot-work/u-boot$ make
> CROSS_COMPILE=arm-cortex_a8-linux-gnueabihf-
> scripts/kconfig/conf --silentoldconfig Kconfig
> CHK include/config.h
> UPD include/config.h
> CFG u-boot.cfg
> GEN include/autoconf.mk
> GEN include/autoconf.mk.dep
> CFG spl/u-boot.cfg
> GEN spl/include/autoconf.mk
> CHK include/config/uboot.release
> CHK include/generated/version_autogenerated.h
> CHK include/generated/timestamp_autogenerated.h
> UPD include/generated/timestamp_autogenerated.h
> * CC lib/asm-offsets.s*
> *cc1: warning: target CPU does not support THUMB instructions*
> * CHK include/generated/generic-asm-offsets.h*
> * CC arch/arm/lib/asm-offsets.s*
> *cc1: warning: target CPU does not support THUMB instructions*
> CHK include/generated/asm-offsets.h
> HOSTCC scripts/dtc/dtc.o
> HOSTCC scripts/dtc/flattree.o
> HOSTCC scripts/dtc/fstree.o
> HOSTCC scripts/dtc/data.o
> HOSTCC scripts/dtc/livetree.o
> HOSTCC scripts/dtc/treesource.o
> HOSTCC scripts/dtc/srcpos.o
> HOSTCC scripts/dtc/checks.o
> HOSTCC scripts/dtc/util.o
> SHIPPED scripts/dtc/dtc-lexer.lex.c
> SHIPPED scripts/dtc/dtc-parser.tab.h
> HOSTCC scripts/dtc/dtc-lexer.lex.o
> SHIPPED scripts/dtc/dtc-parser.tab.c
> HOSTCC scripts/dtc/dtc-parser.tab.o
> HOSTLD scripts/dtc/dtc
> HOSTCC tools/mkenvimage.o
> HOSTCC tools/lib/crc32.o
> HOSTLD tools/mkenvimage
> HOSTCC tools/common/bootm.o
> HOSTCC tools/lib/fdtdec.o
> HOSTCC tools/fit_image.o
> HOSTCC tools/image-host.o
> HOSTCC tools/dumpimage.o
> HOSTLD tools/dumpimage
> HOSTCC tools/mkimage.o
> HOSTLD tools/mkimage
> CC arch/arm/cpu/armv7/cache_v7.o
> *cc1: warning: target CPU does not support THUMB instructions*
> *{standard input}: Assembler messages:*
> *{standard input}:42: Error: selected processor does not support `dsb sy'
> in ARM mode*
> *{standard input}:46: Error: selected processor does not support `isb sy'
> in ARM mode*
> *{standard input}:240: Error: selected processor does not support `dsb sy'
> in ARM mode*
> *{standard input}:244: Error: selected processor does not support `isb sy'
> in ARM mode*
> *{standard input}:368: Error: selected processor does not support `dsb sy'
> in ARM mode*
> *{standard input}:460: Error: selected processor does not support `dsb sy'
> in ARM mode*
> *{standard input}:464: Error: selected processor does not support `isb sy'
> in ARM mode*
> *{standard input}:594: Error: selected processor does not support `dsb sy'
> in ARM mode*
> *scripts/Makefile.build:278: recipe for target
> 'arch/arm/cpu/armv7/cache_v7.o' failed*
> make[1]: *** [arch/arm/cpu/armv7/cache_v7.o] Error 1
> Makefile:1363: recipe for target 'arch/arm/cpu/armv7' failed
I get a deluge of such errors with gcc-8. Use gcc-7 or earlier until
someone fixes it (or fix it yourself and send a patch).
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] GCC 7.x vs. C++ comments
Tom Rini <tr...@konsulko.com> writes: > - Shorter Please explain how // SPDX is significantly shorter than /* SPDX */ Having // comments next to /* ones is just hideously ugly. If some scanning tool is too stupid to handle standard C comments, it's the tool that needs to be fixed. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 2/2] sunxi: binman: Add U-Boot binary size check
Maxime Ripard <maxime.rip...@bootlin.com> writes:
> On Wed, May 02, 2018 at 03:24:50PM +0100, Måns Rullgård wrote:
>> Maxime Ripard <maxime.rip...@bootlin.com> writes:
>>
>> > 1;5201;0c
>> > On Wed, May 02, 2018 at 10:34:49AM +0100, Måns Rullgård wrote:
>> >> Siarhei Siamashka <siarhei.siamas...@gmail.com> writes:
>> >>
>> >> > On Tue, 01 May 2018 18:25:06 +0100
>> >> > Måns Rullgård <m...@mansr.com> wrote:
>> >> >
>> >> >> Maxime Ripard <maxime.rip...@free-electrons.com> writes:
>> >> >>
>> >> >> > The U-Boot binary may trip over its actual allocated size in the
>> >> >> > storage.
>> >> >> > In such a case, the environment will not be readable anymore (because
>> >> >> > corrupted when the new image was flashed), and any attempt at using
>> >> >> > saveenv
>> >> >> > to reconstruct the environment will result in a corrupted U-Boot
>> >> >> > binary.
>> >> >> >
>> >> >> > Reviewed-by: Andre Przywara <andre.przyw...@arm.com>
>> >> >> > Signed-off-by: Maxime Ripard <maxime.rip...@free-electrons.com>
>> >> >> > ---
>> >> >> > arch/arm/dts/sunxi-u-boot.dtsi | 12
>> >> >> > 1 file changed, 12 insertions(+)
>> >> >> >
>> >> >> > diff --git a/arch/arm/dts/sunxi-u-boot.dtsi
>> >> >> > b/arch/arm/dts/sunxi-u-boot.dtsi
>> >> >> > index 5adfd9bca2ec..72e95afd780e 100644
>> >> >> > --- a/arch/arm/dts/sunxi-u-boot.dtsi
>> >> >> > +++ b/arch/arm/dts/sunxi-u-boot.dtsi
>> >> >> > @@ -1,5 +1,14 @@
>> >> >> > #include
>> >> >> >
>> >> >> > +/*
>> >> >> > + * This is the maximum size the U-Boot binary can be, which is
>> >> >> > basically
>> >> >> > + * the start of the environment, minus the start of the U-Boot
>> >> >> > binary in
>> >> >> > + * the MMC. This makes the assumption that the MMC is using
>> >> >> > 512-bytes
>> >> >> > + * blocks, but devices using something other than that remains to be
>> >> >> > + * seen.
>> >> >> > + */
>> >> >> > +#define UBOOT_MMC_MAX_SIZE (CONFIG_ENV_OFFSET -
>> >> >> > (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512))
>> >> >> > +
>> >> >> > / {
>> >> >> > binman {
>> >> >> > filename = "u-boot-sunxi-with-spl.bin";
>> >> >> > @@ -8,6 +17,9 @@
>> >> >> > filename = "spl/sunxi-spl.bin";
>> >> >> > };
>> >> >> > u-boot-img {
>> >> >> > +#ifdef CONFIG_MMC
>> >> >> > +size = ;
>> >> >> > +#endif
>> >> >> > pos = ;
>> >> >> > };
>> >> >> > };
>> >> >> > --
>> >> >>
>> >> >> This is broken in (at least) two ways:
>> >> >>
>> >> >> 1. It is simply nonsensical if u-boot and env are on different devices
>> >> >>or not on mmc even if mmc support is enabled.
>> >> >>
>> >> >> 2. It causes u-boot-sunxi-with-spl.bin to be padded to the env offset.
>> >> >>At best, this is useless. If the env doesn't immediately follow
>> >> >>u-boot, it really breaks things.
>> >> >>
>> >> >> Please fix or revert, I don't really care which.
>> >> >
>> >> > The padding is not useless. It reserves space for future size expansions
>> >> > and makes it harder for the users to hurt themselves.
>> >> >
>> >> > For example, if the padded U-Boot size is 1024K while the actual size
>> >> > is only ~800K, then adventurous users might be tempted to fit some of
>> >> > their data into this gap. Yay, ~200K of storage space for free! Except
>> >> > that the next U-Boot release may grow up to 900K without any warning
>> >> &g
Re: [U-Boot] [PATCH 2/5] ARM: arm926ejs: fix lowlevel_init call
Chris Packham <judge.pack...@gmail.com> writes: > Hi Mans, Stefano, > > On Fri, Apr 27, 2018 at 9:00 PM Stefano Babic <sba...@denx.de> wrote: > >> On 21/04/2018 17:11, Mans Rullgard wrote: >> > The code attempts to preserve the value of LR by storing it in R12/IP >> > across the lowevel_init() call. However, this register is not saved >> > by the callee. Use a register that guaranteed to be preserved instead. >> > >> > Signed-off-by: Mans Rullgard <m...@mansr.com> >> > --- >> > arch/arm/cpu/arm926ejs/start.S | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/arch/arm/cpu/arm926ejs/start.S > b/arch/arm/cpu/arm926ejs/start.S >> > index 959d1ed86d8a..a6f0bdb70345 100644 >> > --- a/arch/arm/cpu/arm926ejs/start.S >> > +++ b/arch/arm/cpu/arm926ejs/start.S >> > @@ -105,9 +105,9 @@ flush_dcache: >> > /* >> >* Go setup Memory and board specific bits prior to relocation. >> >*/ >> > - mov ip, lr /* perserve link reg across call */ >> > + mov r4, lr /* perserve link reg across call */ >> > bl lowlevel_init /* go setup pll,mux,memory */ >> > - mov lr, ip /* restore link */ >> > + mov lr, r4 /* restore link */ >> > #endif >> > mov pc, lr /* back to my caller */ >> > #endif /* CONFIG_SKIP_LOWLEVEL_INIT */ >> > > >> Applied to u-boot-imx, thanks ! > > I think this might be causing me a problem on a Marvell Kirkwood board I'm > working on getting into upstream. It may also be problematic for orion5x > boards. Both of these use r4 in lowlevel_init. I've just sent an untested patch for orion5x. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 2/2] sunxi: binman: Add U-Boot binary size check
Maxime Ripard <maxime.rip...@bootlin.com> writes:
> 1;5201;0c
> On Wed, May 02, 2018 at 10:34:49AM +0100, Måns Rullgård wrote:
>> Siarhei Siamashka <siarhei.siamas...@gmail.com> writes:
>>
>> > On Tue, 01 May 2018 18:25:06 +0100
>> > Måns Rullgård <m...@mansr.com> wrote:
>> >
>> >> Maxime Ripard <maxime.rip...@free-electrons.com> writes:
>> >>
>> >> > The U-Boot binary may trip over its actual allocated size in the
>> >> > storage.
>> >> > In such a case, the environment will not be readable anymore (because
>> >> > corrupted when the new image was flashed), and any attempt at using
>> >> > saveenv
>> >> > to reconstruct the environment will result in a corrupted U-Boot binary.
>> >> >
>> >> > Reviewed-by: Andre Przywara <andre.przyw...@arm.com>
>> >> > Signed-off-by: Maxime Ripard <maxime.rip...@free-electrons.com>
>> >> > ---
>> >> > arch/arm/dts/sunxi-u-boot.dtsi | 12
>> >> > 1 file changed, 12 insertions(+)
>> >> >
>> >> > diff --git a/arch/arm/dts/sunxi-u-boot.dtsi
>> >> > b/arch/arm/dts/sunxi-u-boot.dtsi
>> >> > index 5adfd9bca2ec..72e95afd780e 100644
>> >> > --- a/arch/arm/dts/sunxi-u-boot.dtsi
>> >> > +++ b/arch/arm/dts/sunxi-u-boot.dtsi
>> >> > @@ -1,5 +1,14 @@
>> >> > #include
>> >> >
>> >> > +/*
>> >> > + * This is the maximum size the U-Boot binary can be, which is
>> >> > basically
>> >> > + * the start of the environment, minus the start of the U-Boot binary
>> >> > in
>> >> > + * the MMC. This makes the assumption that the MMC is using 512-bytes
>> >> > + * blocks, but devices using something other than that remains to be
>> >> > + * seen.
>> >> > + */
>> >> > +#define UBOOT_MMC_MAX_SIZE (CONFIG_ENV_OFFSET -
>> >> > (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512))
>> >> > +
>> >> > / {
>> >> > binman {
>> >> > filename = "u-boot-sunxi-with-spl.bin";
>> >> > @@ -8,6 +17,9 @@
>> >> > filename = "spl/sunxi-spl.bin";
>> >> > };
>> >> > u-boot-img {
>> >> > +#ifdef CONFIG_MMC
>> >> > + size = ;
>> >> > +#endif
>> >> > pos = ;
>> >> > };
>> >> > };
>> >> > --
>> >>
>> >> This is broken in (at least) two ways:
>> >>
>> >> 1. It is simply nonsensical if u-boot and env are on different devices
>> >>or not on mmc even if mmc support is enabled.
>> >>
>> >> 2. It causes u-boot-sunxi-with-spl.bin to be padded to the env offset.
>> >>At best, this is useless. If the env doesn't immediately follow
>> >>u-boot, it really breaks things.
>> >>
>> >> Please fix or revert, I don't really care which.
>> >
>> > The padding is not useless. It reserves space for future size expansions
>> > and makes it harder for the users to hurt themselves.
>> >
>> > For example, if the padded U-Boot size is 1024K while the actual size
>> > is only ~800K, then adventurous users might be tempted to fit some of
>> > their data into this gap. Yay, ~200K of storage space for free! Except
>> > that the next U-Boot release may grow up to 900K without any warning
>> > and if the users are not careful enough, then their data would be
>> > corrupted during upgrade.
>>
>> Do U-Boot users really need that level of hand-holding?
>
> Yes, and that's never a good argument to make, because...
>
>> > Could you please tell us what is your problem and why reverting this
>> > patch would fix it?
>>
>> See above. Best case, it just wastes space in the created file. If the
>> configuration is anything other than U-Boot immediately followed by env
>> on the same device, it _will_ break things horribly. A few examples:
>>
>> 1. U-Boot and env are on different devices, e.g. U-Boot on mmc and env
>> in SPI NOR flash. In this case, padding the file to the env offset
>> makes no sense. Writing the image will corrupt anyth
Re: [U-Boot] [PATCH v3 2/2] sunxi: binman: Add U-Boot binary size check
Siarhei Siamashka <siarhei.siamas...@gmail.com> writes:
> On Tue, 01 May 2018 18:25:06 +0100
> Måns Rullgård <m...@mansr.com> wrote:
>
>> Maxime Ripard <maxime.rip...@free-electrons.com> writes:
>>
>> > The U-Boot binary may trip over its actual allocated size in the storage.
>> > In such a case, the environment will not be readable anymore (because
>> > corrupted when the new image was flashed), and any attempt at using saveenv
>> > to reconstruct the environment will result in a corrupted U-Boot binary.
>> >
>> > Reviewed-by: Andre Przywara <andre.przyw...@arm.com>
>> > Signed-off-by: Maxime Ripard <maxime.rip...@free-electrons.com>
>> > ---
>> > arch/arm/dts/sunxi-u-boot.dtsi | 12
>> > 1 file changed, 12 insertions(+)
>> >
>> > diff --git a/arch/arm/dts/sunxi-u-boot.dtsi
>> > b/arch/arm/dts/sunxi-u-boot.dtsi
>> > index 5adfd9bca2ec..72e95afd780e 100644
>> > --- a/arch/arm/dts/sunxi-u-boot.dtsi
>> > +++ b/arch/arm/dts/sunxi-u-boot.dtsi
>> > @@ -1,5 +1,14 @@
>> > #include
>> >
>> > +/*
>> > + * This is the maximum size the U-Boot binary can be, which is basically
>> > + * the start of the environment, minus the start of the U-Boot binary in
>> > + * the MMC. This makes the assumption that the MMC is using 512-bytes
>> > + * blocks, but devices using something other than that remains to be
>> > + * seen.
>> > + */
>> > +#define UBOOT_MMC_MAX_SIZE(CONFIG_ENV_OFFSET -
>> > (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512))
>> > +
>> > / {
>> >binman {
>> >filename = "u-boot-sunxi-with-spl.bin";
>> > @@ -8,6 +17,9 @@
>> >filename = "spl/sunxi-spl.bin";
>> >};
>> >u-boot-img {
>> > +#ifdef CONFIG_MMC
>> > + size = ;
>> > +#endif
>> >pos = ;
>> >};
>> >};
>> > --
>>
>> This is broken in (at least) two ways:
>>
>> 1. It is simply nonsensical if u-boot and env are on different devices
>>or not on mmc even if mmc support is enabled.
>>
>> 2. It causes u-boot-sunxi-with-spl.bin to be padded to the env offset.
>>At best, this is useless. If the env doesn't immediately follow
>>u-boot, it really breaks things.
>>
>> Please fix or revert, I don't really care which.
>
> The padding is not useless. It reserves space for future size expansions
> and makes it harder for the users to hurt themselves.
>
> For example, if the padded U-Boot size is 1024K while the actual size
> is only ~800K, then adventurous users might be tempted to fit some of
> their data into this gap. Yay, ~200K of storage space for free! Except
> that the next U-Boot release may grow up to 900K without any warning
> and if the users are not careful enough, then their data would be
> corrupted during upgrade.
Do U-Boot users really need that level of hand-holding?
> Could you please tell us what is your problem and why reverting this
> patch would fix it?
See above. Best case, it just wastes space in the created file. If the
configuration is anything other than U-Boot immediately followed by env
on the same device, it _will_ break things horribly. A few examples:
1. U-Boot and env are on different devices, e.g. U-Boot on mmc and env
in SPI NOR flash. In this case, padding the file to the env offset
makes no sense. Writing the image will corrupt anything stored
after U-Boot at a smaller (but still safe) offset.
2. U-Boot at a non-zero offset, followed by env, but _not_ on mmc. If
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR, probably at its default
value, is smaller than the offset of U-Boot in its actual device,
the padding will be too large. Writing the image file will then
corrupt a stored env.
3. U-Boot at start of device, env at end as indicated by a negative
CONFIG_ENV_OFFSET. With this configuration, binman tries to pad the
image to (nearly) 2^64 bytes and promptly fills up your storage
device.
I keep running into variants of these, and I'm weary of it.
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 2/2] sunxi: binman: Add U-Boot binary size check
Maxime Ripard <maxime.rip...@free-electrons.com> writes:
> The U-Boot binary may trip over its actual allocated size in the storage.
> In such a case, the environment will not be readable anymore (because
> corrupted when the new image was flashed), and any attempt at using saveenv
> to reconstruct the environment will result in a corrupted U-Boot binary.
>
> Reviewed-by: Andre Przywara <andre.przyw...@arm.com>
> Signed-off-by: Maxime Ripard <maxime.rip...@free-electrons.com>
> ---
> arch/arm/dts/sunxi-u-boot.dtsi | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi
> index 5adfd9bca2ec..72e95afd780e 100644
> --- a/arch/arm/dts/sunxi-u-boot.dtsi
> +++ b/arch/arm/dts/sunxi-u-boot.dtsi
> @@ -1,5 +1,14 @@
> #include
>
> +/*
> + * This is the maximum size the U-Boot binary can be, which is basically
> + * the start of the environment, minus the start of the U-Boot binary in
> + * the MMC. This makes the assumption that the MMC is using 512-bytes
> + * blocks, but devices using something other than that remains to be
> + * seen.
> + */
> +#define UBOOT_MMC_MAX_SIZE (CONFIG_ENV_OFFSET -
> (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512))
> +
> / {
> binman {
> filename = "u-boot-sunxi-with-spl.bin";
> @@ -8,6 +17,9 @@
> filename = "spl/sunxi-spl.bin";
> };
> u-boot-img {
> +#ifdef CONFIG_MMC
> + size = ;
> +#endif
> pos = ;
> };
> };
> --
This is broken in (at least) two ways:
1. It is simply nonsensical if u-boot and env are on different devices
or not on mmc even if mmc support is enabled.
2. It causes u-boot-sunxi-with-spl.bin to be padded to the env offset.
At best, this is useless. If the env doesn't immediately follow
u-boot, it really breaks things.
Please fix or revert, I don't really care which.
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFH] Das U-Boot logo help
Tom Rini <tr...@konsulko.com> writes: > Hey all, > > So, as far as I can tell, U-Boot hasn't ever had an official logo. This > lead a while back to Alex using the following logo in a presentation > that I attended: > https://commons.wikimedia.org/wiki/File:Circle-icons-submarine.svg > > And I thought it was a pretty good fit too, and as a bonus it's already > GPLv2 or later licensed. The only thing missing is that it should > probably say U-Boot on it somewhere. Does anyone out there want to take > a pass at something like that? Thanks! I'm picturing something looking a bit ambiguously like either a submarine or a boot, but I don't have the artistic skills to draw it. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe.
Christoph Müllner <christoph.muell...@theobroma-systems.com> writes: >> On 21 Apr 2018, at 15:24, Måns Rullgård <m...@mansr.com> wrote: >> >> Klaus Goger <klaus.go...@theobroma-systems.com> writes: >> >>> The current arch implementation of memcpy cannot be called >>> from thumb code, because it does not use bx instructions on return. >>> This patch addresses that. Note, that this patch does not touch >>> the hot loop of memcpy, so performance is not affected. >>> >>> Tested on MXS (arm926ejs) with and without thumb-mode enabled. >>> >>> Signed-off-by: Klaus Goger <klaus.go...@theobroma-systems.com> >>> Signed-off-by: Christoph Muellner <christoph.muell...@theobroma-systems.com> >> >> There are many more instances of mov to pc that ought to be fixed too. >> Why not do them all at once rather than picking them off one by one as >> they happen to break things? > > I could not find exit points within memcpy other than those which we fixed. > The many other mov pc, $lr instructions are just branches within memcpy. > Am I overseeing anything here? I meant elsewhere, not just in memcpy. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/2] arm: Make arch specific memcpy thumb-safe.
Klaus Goger <klaus.go...@theobroma-systems.com> writes: > The current arch implementation of memcpy cannot be called > from thumb code, because it does not use bx instructions on return. > This patch addresses that. Note, that this patch does not touch > the hot loop of memcpy, so performance is not affected. > > Tested on MXS (arm926ejs) with and without thumb-mode enabled. > > Signed-off-by: Klaus Goger <klaus.go...@theobroma-systems.com> > Signed-off-by: Christoph Muellner <christoph.muell...@theobroma-systems.com> There are many more instances of mov to pc that ought to be fixed too. Why not do them all at once rather than picking them off one by one as they happen to break things? -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] arm: make arm926ejs startup code thumb compatible
Klaus Goger <klaus.go...@theobroma-systems.com> writes: > When building the mxs platform in thumb mode gcc generates code using > the intra procedure call scratch register (ip/r12) for the calling the > lowlevel_init function. This modifies the lr in flush_dcache which > causes u-boot proper to end in an endless loop. > > 40002334: e1a0c00emov ip, lr > 40002338: eb00df4cbl 4003a070 > <__lowlevel_init_from_arm> > 4000233c: e1a0e00cmov lr, ip > 40002340: e1a0f00emov pc, lr > [...] > 4003a070 <__lowlevel_init_from_arm>: > 4003a070: e59fc004ldr ip, [pc, #4]; 4003a07c > <__lowlevel_init_from_arm+0xc> > 4003a074: e08fc00cadd ip, pc, ip > 4003a078: e12fff1cbx ip > 4003a07c: fffc86cd.word 0xfffc86cd > > Instead of using the the ip/r12 register we use sl/r10 to preserve the > link register. > > According to "Procedure Call Standard for the ARM Architecture" by ARM > subroutines have to preserve the contents of register r4-r8, r10, r11 > and SP. So using r10 instead of r12 should be save. > > Signed-off-by: Klaus Goger <klaus.go...@theobroma-systems.com> > Signed-off-by: Christoph Muellner <christoph.muell...@theobroma-systems.com> This problem isn't specific to Thumb mode. An ARM build would also break if the lowlevel_init function happened to clobber r12, which it is permitted to do. It's just dumb luck that this hasn't happened yet. > --- > > Changes in v2: > - use bl instead of blx to call lowlevel_init > - remove mxs tag as it apply to all arm926ejs platforms > > arch/arm/cpu/arm926ejs/start.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > index 959d1ed86d..317df5c401 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -105,9 +105,9 @@ flush_dcache: > /* >* Go setup Memory and board specific bits prior to relocation. >*/ > - mov ip, lr /* perserve link reg across call */ > + mov sl, lr /* perserve link reg across call */ > bl lowlevel_init /* go setup pll,mux,memory */ > - mov lr, ip /* restore link */ > + mov lr, sl /* restore link */ I prefer to use plain register names (r10) rather than the aliases (sl) when not using them for the special functions indicated by the latter. > #endif > - mov pc, lr /* back to my caller */ > + bx lr /* back to my caller */ This change seems unrelated. Yes, bx is the preferred instruction, but using mov here isn't breaking anything. If it bothers you, feel free to make a separate patch fixing all the instances of mov to the pc register, not just this one. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] env: Fix operation of 'make environ'
Simon Glass <s...@chromium.org> writes: > This was broken by the recent environment refactoring. Specifically: > > $ make environ > scripts/Makefile.build:59: tools/environ/Makefile: No such file or directory > make[1]: *** No rule to make target 'tools/environ/Makefile'. Stop. > make: *** [Makefile:1469: environ] Error 2 > > Fix this by updating the Makefile and adjusting the #include filesnames in > two C files. > > Fixes: ec74f5f (Makefile: Rename 'env' target to 'environ') > Reported-by: Måns Rullgård <m...@mansr.com> > Signed-off-by: Simon Glass <s...@chromium.org> > --- > > Makefile | 2 +- > tools/env/env_attr.c | 2 +- > tools/env/env_flags.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) This still breaks any build scripts using the old target (yes, there are many). That's probably acceptable, but a mention of it in some release note would be nice. Come to think of it, if we're anyway going to break things, how about giving that target a more descriptive name than environ? I suggest envtools. > diff --git a/Makefile b/Makefile > index a0f3bfd2a52..dd9d5678118 100644 > --- a/Makefile > +++ b/Makefile > @@ -1466,7 +1466,7 @@ checkarmreloc: u-boot > fi > > environ: scripts_basic > - $(Q)$(MAKE) $(build)=tools/$@ > + $(Q)$(MAKE) $(build)=tools/env > > tools-only: scripts_basic $(version_h) $(timestamp_h) > $(Q)$(MAKE) $(build)=tools > diff --git a/tools/env/env_attr.c b/tools/env/env_attr.c > index 502d4c900bf..4d8536335c3 100644 > --- a/tools/env/env_attr.c > +++ b/tools/env/env_attr.c > @@ -1 +1 @@ > -#include "../../common/env_attr.c" > +#include "../../env/attr.c" > diff --git a/tools/env/env_flags.c b/tools/env/env_flags.c > index b261cb8e390..71e13e2021f 100644 > --- a/tools/env/env_flags.c > +++ b/tools/env/env_flags.c > @@ -1 +1 @@ > -#include "../../common/env_flags.c" > +#include "../../env/flags.c" > -- > 2.14.1.342.g6490525c54-goog > -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 01/30] Makefile: Rename 'env' target to 'environ'
Simon Glass <s...@chromium.org> writes:
> This target stops us using 'env' as a subdirectory. It is not mentioned in
> the help so seems to be an internal target. Rename it.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> Reviewed-by: Tom Rini <tr...@konsulko.com>
> ---
>
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 50a002e72f..04a22a6818 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1463,14 +1463,14 @@ checkarmreloc: u-boot
> false; \
> fi
>
> -env: scripts_basic
> +environ: scripts_basic
> $(Q)$(MAKE) $(build)=tools/$@
This broke things:
$ make environ
scripts/Makefile.build:59: tools/environ/Makefile: No such file or directory
make[1]: *** No rule to make target 'tools/environ/Makefile'. Stop.
make: *** [Makefile:1469: environ] Error 2
The 'env' target was used to build the fw_{print,set}env tools in the
tools/env directory. The make rule relies on the target having the same
name as the subdirectory.
> tools-only: scripts_basic $(version_h) $(timestamp_h)
> $(Q)$(MAKE) $(build)=tools
>
> tools-all: export HOST_TOOLS_ALL=y
> -tools-all: env tools ;
> +tools-all: environ tools ;
>
> cross_tools: export CROSS_BUILD_TOOLS=y
> cross_tools: tools ;
> --
> 2.14.0.rc1.383.gd1ce394fe2-goog
>
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Data Abort with gcc 7.1
Maxime Ripard <maxime.rip...@free-electrons.com> writes: > On Thu, Jul 13, 2017 at 11:20:34AM +0100, Peter Robinson wrote: >> >>> What hardware did this happen on? If it was on ARMv5, adding the packed >> >>> attribute is probably the correct fix. If it was ARMv6 or later, >> >>> something else is broken as well. >> >> >> >> It does not matter if this was ARMv6+ hardware or not. The current >> >> U-Boot code is wrong and we need to fix it. >> > >> > The question is how many errors there are. That's why I asked about the >> > hardware. >> >> I've seen it on a number of devices but they were all ARMv7+ >> (AllWinner, Rockchips etc) > > It was on an Allwinner SoCs with a Cortex-A7 CPU, so armv7. However, > as far as I know, the unaligned accesses are disable in u-boot. Yes, so it seems, although I can't fathom why. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Data Abort with gcc 7.1
Siarhei Siamashka <siarhei.siamas...@gmail.com> writes:
> On Thu, 13 Jul 2017 01:43:37 +0100
> Måns Rullgård <m...@mansr.com> wrote:
>
>> Maxime Ripard <maxime.rip...@free-electrons.com> writes:
>>
>> > Hi,
>> >
>> > I recently got a gcc 7.1 based toolchain, and it seems like it
>> > generates unaligned code, specifically in the net_set_ip_header
>> > function in my case.
>> >
>> > Whenever some packet is sent, this data abort is triggered:
>> >
>> > => setenv ipaddr 10.42.0.1; ping 10.42.0.254
>> > using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
>> > MAC de:ad:be:ef:00:01
>> > HOST MAC de:ad:be:af:00:00
>> > RNDIS ready
>> > musb-hdrc: peripheral reset irq lost!
>> > high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
>> > USB RNDIS network up!
>> > Using usb_ether device
>> > data abort
>> > pc : [<7ff9db10>] lr : [<7ff9f00c>]
>> > reloc pc : [<4a043b10>] lr : [<4a04500c>]
>> > sp : 7bf37cc8 ip : fp : 7ff6236c
>> > r10: 7ffed2b8 r9 : 7bf39ee8r8 : 7ffed2b8
>> > r7 : 0001 r6 : r5 : 002a r4 : 7ffed30e
>> > r3 : 1445 r2 : 01002a0ar1 : fe002a0a r0 : 7ffed30e
>> > Flags: nZCv IRQs off FIQs off Mode SVC_32
>> > Resetting CPU ...
>> >
>> > Running objdump on it gives us this:
>> >
>> > 4a043b04 :
>> >
>> >/*
>> > * Construct an IP header.
>> > */
>> >/* IP_HDR_SIZE / 4 (not including UDP) */
>> >ip->ip_hl_v = 0x45;
>> > 4a043b04: e59f3074ldr r3, [pc, #116] ; 4a043b80
>> > <net_set_ip_header+0x7c>
>> > {
>> > 4a043b08: e92d4013push{r0, r1, r4, lr}
>> > 4a043b0c: e1a04000mov r4, r0
>> >ip->ip_hl_v = 0x45;
>> > 4a043b10: e5803000str r3, [r0] < Abort
>> >ip->ip_tos = 0;
>> >ip->ip_len = htons(IP_HDR_SIZE);
>> >ip->ip_id= htons(net_ip_id++);
>> > 4a043b14: e59f3068ldr r3, [pc, #104] ; 4a043b84
>> > <net_set_ip_header+0x80>
>> >
>> > It seems like r0 is indeed set to an unaligned address (0x7ffed30e)
>> > for some reason.
>> >
>> > Using a Linaro 6.3 toolchain works on the same commit with the same
>> > config, so it really seems to be a compiler-related issue.
>> >
>> > It generates this code:
>> >
>> > 4a043ec4 :
>> >
>> >/*
>> > * Construct an IP header.
>> > */
>> >/* IP_HDR_SIZE / 4 (not including UDP) */
>> >ip->ip_hl_v = 0x45;
>> > 4a043ec4: e3a03045mov r3, #69 ; 0x45
>> > {
>> > 4a043ec8: e92d4013push{r0, r1, r4, lr}
>> > 4a043ecc: e1a04000mov r4, r0
>> >ip->ip_hl_v = 0x45;
>> > 4a043ed0: e5c03000strbr3, [r0]
>> >ip->ip_tos = 0;
>> >ip->ip_len = htons(IP_HDR_SIZE);
>> > 4a043ed4: e3a03b05mov r3, #5120 ; 0x1400
>> >ip->ip_tos = 0;
>> > 4a043ed8: e3a0mov r0, #0
>> >ip->ip_len = htons(IP_HDR_SIZE);
>> > 4a043edc: e1c430b2strhr3, [r4, #2]
>> >ip->ip_id= htons(net_ip_id++);
>> > 4a043ee0: e59f3064ldr r3, [pc, #100] ; 4a043f4c
>> > <net_set_ip_header+0x88>
>> >
>> > And it seems like it's using an strb instruction to avoid the
>> > unaligned access.
>> >
>> > As far as I know, we are passing --wno-unaligned-access, so the broken
>> > situation should not arise, and yet it does, so I'm a bit confused,
>> > and not really sure what to do from there.
>>
>> For reference, this is the relevant code:
>>
>> struct ip_udp_hdr {
>> u8 ip_hl_v;/* header length and version*/
>> u8 ip_tos; /* type of service */
>> u16 ip_len; /* total length */
>> u16 ip_id; /* identification */
>> u16 ip_off; /* fragment offset field*/
>> u8 ip_ttl; /* time to live */
>> u8 ip_p; /* protocol */
>> u16 ip_su
Re: [U-Boot] Data Abort with gcc 7.1
on't fragment */ ip->ip_ttl = 255; ip->ip_sum = 0; /* already in network byte order */ net_copy_ip((void *)>ip_src, ); /* already in network byte order */ net_copy_ip((void *)>ip_dst, ); } What's changed with gcc7 is that it now merges the writes to the first three fields (occupying 4 bytes) into a single 32-bit store. There is nothing wrong with this. Now struct ip_udp_hdr includes 32-bit fields, so it's natural alignment is 4 bytes. If the 'pkt' argument is not adequately aligned, the cast in the first line of the function becomes invalid. Judging by the register dump and disassembly, the pointer is indeed not thusly aligned, and this is an error. The misaligned pointer should still not cause a hardware abort, however, on ARMv6 or later which permits unaligned addresses with the STR instruction. That is unless some idiot has gone and enabled strict alignment checking again despite this being against all common sense. There was a lengthy argument about this a few years ago, ultimately resulting in the proper settings being put in place. What hardware did this happen on? If it was on ARMv5, adding the packed attribute is probably the correct fix. If it was ARMv6 or later, something else is broken as well. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] armv5te: make 'ret lr' produce iinterworking 'bx lr'
Tom Rini <tr...@konsulko.com> writes: > On Thu, Mar 02, 2017 at 02:27:03PM +0000, Måns Rullgård wrote: >> Tom Rini <tr...@konsulko.com> writes: >> >> > On Mon, Feb 27, 2017 at 08:19:07PM +0100, Albert ARIBAUD wrote: >> >> Current ARM assembler helper for the 'return to caller' pseudo-instruction >> >> turns 'ret lr' into 'mov pc, lr' for ARMv5TE. This causes the core to >> >> remain >> >> in its current ARM state even when the routine doing the 'ret' was called >> >> from Thumb-1 state, triggering an undefined instruction exception. >> >> >> >> This causes early run-time failures in all boards compiled using the >> >> Thumb-1 >> >> instruction set (for instance the Open-RD family). >> >> >> >> ARMv5TE supports 'bx lr' which properly implements interworking and thus >> >> correctly returns to Thumb-1 state from ARM state. >> >> >> >> This change makes 'ret lr' turn into 'bx lr' for ARMv5TE. >> >> >> >> Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net> >> >> --- >> >> Note: this patch supersedes patch "openrd: disable private arch memset, >> >> memcpy and libgcc" dated Sun, 26 Feb 2017 16:29:32 +0100. >> >> >> >> arch/arm/include/asm/assembler.h | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/arch/arm/include/asm/assembler.h >> >> b/arch/arm/include/asm/assembler.h >> >> index ae1e42fc06..c56daf2a1f 100644 >> >> --- a/arch/arm/include/asm/assembler.h >> >> +++ b/arch/arm/include/asm/assembler.h >> >> @@ -59,7 +59,7 @@ >> >> >> >> .irpc,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo >> >> .macro ret\c, reg >> >> -#if defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) >> >> +#if defined(__ARM_ARCH_5E__) >> >> mov\c pc, \reg >> >> #else >> >> .ifeqs "\reg", "lr" >> > >> > Emperical wins over theory, so I'll take this patch so we don't break >> > things. But looking at the kernel, the above is a test for >> > __LINUX_ARM_ARCH__ < 6 instead. But the kernel is hardly ever built and >> > run in Thumb mode rather than ARM mode, so they wouldn't be tickling >> > this particular issue. >> >> The kernel needs Thumb-2, so the requirements are a bit different there. > > Ah, right. > >> > I guess my big question now is, when can / should we not just use 'bx >> > lr' over 'mov pc, lr' ? >> >> All cores with Thumb support the bx instruction. The only time you want >> to use "mov pc" is with non-Thumb v4 and v5 cores. > > So, of ARM720T, ARM920T, ARM926EJS and ARM946ES, which fall into that > category? Or is it harder to say than that? All of those support Thumb and thus the bx instruction. The blx instruction, which simplifies interworking, is only available from v5T, but that's not relevant here. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] armv5te: make 'ret lr' produce iinterworking 'bx lr'
Tom Rini <tr...@konsulko.com> writes: > On Mon, Feb 27, 2017 at 08:19:07PM +0100, Albert ARIBAUD wrote: >> Current ARM assembler helper for the 'return to caller' pseudo-instruction >> turns 'ret lr' into 'mov pc, lr' for ARMv5TE. This causes the core to remain >> in its current ARM state even when the routine doing the 'ret' was called >> from Thumb-1 state, triggering an undefined instruction exception. >> >> This causes early run-time failures in all boards compiled using the Thumb-1 >> instruction set (for instance the Open-RD family). >> >> ARMv5TE supports 'bx lr' which properly implements interworking and thus >> correctly returns to Thumb-1 state from ARM state. >> >> This change makes 'ret lr' turn into 'bx lr' for ARMv5TE. >> >> Signed-off-by: Albert ARIBAUD <albert.u.b...@aribaud.net> >> --- >> Note: this patch supersedes patch "openrd: disable private arch memset, >> memcpy and libgcc" dated Sun, 26 Feb 2017 16:29:32 +0100. >> >> arch/arm/include/asm/assembler.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/include/asm/assembler.h >> b/arch/arm/include/asm/assembler.h >> index ae1e42fc06..c56daf2a1f 100644 >> --- a/arch/arm/include/asm/assembler.h >> +++ b/arch/arm/include/asm/assembler.h >> @@ -59,7 +59,7 @@ >> >> .irpc,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo >> .macro ret\c, reg >> -#if defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) >> +#if defined(__ARM_ARCH_5E__) >> mov\c pc, \reg >> #else >> .ifeqs "\reg", "lr" > > Emperical wins over theory, so I'll take this patch so we don't break > things. But looking at the kernel, the above is a test for > __LINUX_ARM_ARCH__ < 6 instead. But the kernel is hardly ever built and > run in Thumb mode rather than ARM mode, so they wouldn't be tickling > this particular issue. The kernel needs Thumb-2, so the requirements are a bit different there. > I guess my big question now is, when can / should we not just use 'bx > lr' over 'mov pc, lr' ? All cores with Thumb support the bx instruction. The only time you want to use "mov pc" is with non-Thumb v4 and v5 cores. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Makefile: Fix linking with modern binutils
Joel Stanley <j...@jms.id.au> writes: > Hello Tom, > > On Sat, Dec 10, 2016 at 12:53 AM, Tom Rini <tr...@konsulko.com> wrote: >>> -LDFLAGS_u-boot += $(LDFLAGS_FINAL) >>> +# Avoid Not enough room for program headers on binutils 2.28 onwards. >>> +# Flag was introduced in 2.26 >>> +LDFLAGS_u-boot += $(LDFLAGS_FINAL) \ >>> + $(call ld-ifversion, -ge, 2260, --no-dynamic-linker) >> >> This breaks on things like: >> $ arm-none-eabi-ld --version >> GNU ld (GNU Tools for ARM Embedded Processors) 2.24.0.20150921 > > The flag will only be added when the version is >= 2.26. Which part of > that version string will break the test? > >> so please use ld-option instead to add this when supported, thanks! > > If you'd just prefer ld-option over ld-ifversion then I can send a > different patch. Always test for the actual thing you need whenever possible. Version checks are fragile and should be a last resort. Consider what would happen if someone backported that functionality to an older binutils version. Going by version number alone, the flag would not be used and the build would fail. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] How to take value of an environmental variable of uboot in linux
Haris Papadopoulos <ha...@amberbox.com> writes: > Hi, > > I type fw_printenv variable_name to take the name of a uboot variable from > linux. > > I get variable_name=10 for example > > How can I use the value 10 in the continuation of my script? > > I have tried > > string = fw_printenv flag_boot_error > IFS="=" read name value <<< $string > > but it does not work "fw_printenv -n VAR" prints the value of variable VAR without the name and equals sign. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] static var mem alignment issue/question
Tim Harvey <thar...@gateworks.com> writes:
> On Tue, Nov 29, 2016 at 4:06 AM, Måns Rullgård <m...@mansr.com> wrote:
>> Tim Harvey <thar...@gateworks.com> writes:
>>
>>> Greetings,
>>>
>>> In debugging an issue with a rather old branch of U-Boot (2015-04) I
>>> found that the static assignment of a data buffer was not 32-bit
>>> aligned which caused data aborts. However I find that current U-boot
>>> master does not suffer this issue and no matter what I declare static
>>> before the particular variable its always 32-bit aligned. I don't see
>>> any code changes that would cause this and in both cases I'm building
>>> with the same gcc5.2.0 toolchain.
>>>
>>> The code in question is this this from cmd/fdt.c:
>>>
>>> } else if (argv[1][0] == 's') {
>>> char *pathp;/* path */
>>> char *prop; /* property */
>>> int nodeoffset;/* node offset from libfdt */
>>> static char data[SCRATCHPAD]; /* storage for the property
>>> */
>>> int len; /* new length of the property */
>>> int ret;
>>>
>>> What guarantee's that 'data' above is 32-bit aligned in master that is
>>> missing from the older U-Boot branch I'm using? Perhaps there is some
>>> arg in a Makefile that I'm missing?
>>
>> There are no such guarantees. Things often end up 4-byte aligned by
>> chance simply because most of them are a multiple of 4 bytes in size.
>> The code should be fixed, either by explicitly aligning the buffer or by
>> using the put_unaligned_b32() accessor in fdt_parse_prop(). Here's an
>> untested patch:
>
> Måns,
>
> My thoughts as well but what I found on a 2015.04 U-Boot adding a
> 'static char foo[1]' before the 'static char data[SCRATCHPAD]' and
> printing the addrs:
> foo:4ff9e0b2
> data:4ff9e0b3
> (what I would have expected... no alignment guarantees and clearly not
> quad-byte aligned)
>
> The same change on master:
> foo:4ff9fff4
> data:4ff9fbf0
> (not at all what I expected - quad byte aligned and more)
>
> Thus I figured there was something in a Makefile that was directing
> the compiler (same compiler in both cases)
>
> I find that merely including '' (with no code
> changes) changes the alignment and aligns propery???
Compilers are fickle creatures.
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] static var mem alignment issue/question
Tim Harvey <thar...@gateworks.com> writes:
> Greetings,
>
> In debugging an issue with a rather old branch of U-Boot (2015-04) I
> found that the static assignment of a data buffer was not 32-bit
> aligned which caused data aborts. However I find that current U-boot
> master does not suffer this issue and no matter what I declare static
> before the particular variable its always 32-bit aligned. I don't see
> any code changes that would cause this and in both cases I'm building
> with the same gcc5.2.0 toolchain.
>
> The code in question is this this from cmd/fdt.c:
>
> } else if (argv[1][0] == 's') {
> char *pathp;/* path */
> char *prop; /* property */
> int nodeoffset;/* node offset from libfdt */
> static char data[SCRATCHPAD]; /* storage for the property */
> int len; /* new length of the property */
> int ret;
>
> What guarantee's that 'data' above is 32-bit aligned in master that is
> missing from the older U-Boot branch I'm using? Perhaps there is some
> arg in a Makefile that I'm missing?
There are no such guarantees. Things often end up 4-byte aligned by
chance simply because most of them are a multiple of 4 bytes in size.
The code should be fixed, either by explicitly aligning the buffer or by
using the put_unaligned_b32() accessor in fdt_parse_prop(). Here's an
untested patch:
>From 714f6a16062cc43e9aadfba29b295365fd4926de Mon Sep 17 00:00:00 2001
From: Mans Rullgard <m...@mansr.com>
Date: Tue, 29 Nov 2016 11:59:37 +
Subject: [PATCH] cmd/fdt: fix unaligned memory access
The buffer passed to fdt_parse_prop() might be unaligned. Use the
proper access helper when writing 32-bit values.
Signed-off-by: Mans Rullgard <m...@mansr.com>
---
cmd/fdt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c
index b503357dc3a8..560b8cc9df61 100644
--- a/cmd/fdt.c
+++ b/cmd/fdt.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#define MAX_LEVEL 32 /* how deeply nested we will go */
#define SCRATCHPAD 1024/* bytes of scratchpad memory */
@@ -766,7 +767,7 @@ static int fdt_parse_prop(char * const *newval, int count,
char *data, int *len)
cp = newp;
tmp = simple_strtoul(cp, , 0);
- *(__be32 *)data = __cpu_to_be32(tmp);
+ put_unaligned_be32(tmp, data);
data += 4;
*len += 4;
--
2.10.2
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] "_ENTRY" is both a struct and a typedef?
<peter.ch...@data61.csiro.au> writes:
>>>>>> "Robert" == Robert P J Day <rpj...@crashcourse.ca> writes:
>
> Robert> from lib/hashtable.c:
>
> Robert> typedef struct _ENTRY { int used; ENTRY entry; } _ENTRY;
>
> Robert> ok, that's just kind of creepy ... defining a typedef over top
> Robert> of a struct of the same name. does anyone else find that
> Robert> strange?
>
> It's standard practice in some C styles. Personally I'd delete the
> struct tag, as the typedef should be used everywhere instead.
I'd rather drop the typedef. The struct/typedef is only referenced by
name in two places.
First in include/search.h:
/* Opaque type for internal use. */
struct _ENTRY;
[...]
/* Data type for reentrant functions. */
struct hsearch_data {
struct _ENTRY *table;
It is standard practice to use "struct foo" in such constructs.
The second use is further down in lib/hashtable.c:
htab->table = (_ENTRY *) calloc(htab->size + 1, sizeof(_ENTRY));
This line is terrible for two reasons:
1. Type-casting the return value of calloc is always wrong.
2. It is safer to use sizeof(*htab->table) instead of an explicit type.
If someone *really* wants to "fix" this, the proper thing is to drop the
typedef and rewrite the calloc line to not explicitly mention the type.
Then again, this is code borrowed from uclibc/glibc, so perhaps it's
best to simply leave it alone.
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] is minicom still discouraged as the serial comms program?
Tom Rini <tr...@konsulko.com> writes: > On Sat, Jul 16, 2016 at 06:52:38AM -0400, Robert P. J. Day wrote: > >> i know that minicom has been discouraged for quite some time WRT >> u-boot, is that still the case? and what was the flaw with minicom, >> anyway? and is there any problem with picocom, which seems to work for >> me just fine. > > As far as I know there's no real problem with minicom, just that it's a > pain to use in general, but that's just my opinion. It's an opinion shared by many. Minicom appears to be mainly intended for use with an actual modem, and when there isn't one involved, those features tend to get in the way. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] where *precisely* is u-boot's environment stored?
"Robert P. J. Day" <rpj...@crashcourse.ca> writes: > On Wed, 6 Jul 2016, Måns Rullgård wrote: > >> "Robert P. J. Day" <rpj...@crashcourse.ca> writes: > >> > p.s. how does the default environment get to the CONFIG_ENV_ADDR >> > defined in the board header file? is that done automatically when >> > u-boot starts to run and notices that there is no valid >> > environment info at that address, and therefore copies it for you? >> >> If the stored environment is invalid (e.g. uninitialised), the >> built-in default is used. Nothing is written until you issue a >> saveenv command. This saves a copy of the live environment to the >> configured location. > > ah, that's the final bit of the puzzle i was looking for ... the > first time a "saveenv" is done is when the configured location is > examined and, if uninitialized, the env is copied there and is used > from then on. saveenv always writes whatever is in memory to the permanent location, overwriting anything that was previously there. On startup, u-boot checks if the saved environment is valid and uses it if it is. If not, the defaults (as configured for the board) are used. Only the saveenv command writes anything to nonvolatile storage. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] where *precisely* is u-boot's environment stored?
"Robert P. J. Day" <rpj...@crashcourse.ca> writes:
> oh, wait, i think i just answered some of my questions based on this
> snippet from common/env_nvram.c:
>
> /*
> * Initialize Environment use
> *
> * We are still running from ROM, so data use is limited
> */
> int env_init(void)
> {
> #if defined(CONFIG_SYS_NVRAM_ACCESS_ROUTINE)
> ulong crc;
> uchar data[ENV_SIZE];
>
> nvram_read(, CONFIG_ENV_ADDR, sizeof(ulong));
> nvram_read(data, CONFIG_ENV_ADDR + sizeof(ulong), ENV_SIZE);
>
> if (crc32(0, data, ENV_SIZE) == crc) {
> gd->env_addr= (ulong)CONFIG_ENV_ADDR + sizeof(long);
> #else
> if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
> gd->env_addr= (ulong)_ptr->data;
> #endif
> gd->env_valid = 1;
> } else {
> gd->env_addr= (ulong)_environment[0];
> gd->env_valid = 0;
> }
>
> return 0;
> }
>
> so if there is a valid environment at the address specified by the
> board header file, it's used, otherwise fall back to
> default_environment[]. i had suspected it was something like that, i
> just hadn't found the code yet.
>
> is this written up somewhere?
Looks like you found your answer, at least in part. The environment is
stored in one of many possible locations, see the various env_*.c files.
Which one is used is determined by the board configuration.
> p.s. how does the default environment get to the CONFIG_ENV_ADDR
> defined in the board header file? is that done automatically when
> u-boot starts to run and notices that there is no valid environment
> info at that address, and therefore copies it for you?
If the stored environment is invalid (e.g. uninitialised), the built-in
default is used. Nothing is written until you issue a saveenv command.
This saves a copy of the live environment to the configured location.
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] malloc: Remove control-L in malloc.h
Marek Vasut <ma...@denx.de> writes: > On 05/30/2016 10:37 PM, Marek Vasut wrote: >> On 05/30/2016 10:15 PM, Wolfgang Denk wrote: >>> Dear Marek, >>> >>> In message <1464621732-7617-1-git-send-email-ma...@denx.de> you wrote: >>>> Remove what must've been a typo. >>>> >>>> Signed-off-by: Marek Vasut <ma...@denx.de> >>>> Cc: Tom Rini <tr...@konsulko.com> >>>> Cc: Simon Glass <s...@chromium.org> >>>> --- >>>> include/malloc.h | 1 - >>>> 1 file changed, 1 deletion(-) >>>> >>>> diff --git a/include/malloc.h b/include/malloc.h >>>> index 8175c75..e065473 100644 >>>> --- a/include/malloc.h >>>> +++ b/include/malloc.h >>>> @@ -215,7 +215,6 @@ >>>> >>>> */ >>>> >>>> - >>>> #ifndef __MALLOC_H__ >>>> #define __MALLOC_H__ >>> >>> This is most likely NOT a typo, but an intentional page break (yes, >>> there was a time when people used to _print_ source code on dead >>> wood). And I bet if comes from the original imported dlmalloc code, >>> so please check twice befoire introducing an unneeded difference. >> >> Urgh, ok, feel free to drop this patch, I don't really care. > > ...about this patch > > I didn't expect to meet control-L in any files, that's new (old). Kids these days... -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] malloc: Remove control-L in malloc.h
Marek Vasut <ma...@denx.de> writes: > Remove what must've been a typo. > > Signed-off-by: Marek Vasut <ma...@denx.de> > Cc: Tom Rini <tr...@konsulko.com> > Cc: Simon Glass <s...@chromium.org> > --- > include/malloc.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/include/malloc.h b/include/malloc.h > index 8175c75..e065473 100644 > --- a/include/malloc.h > +++ b/include/malloc.h > @@ -215,7 +215,6 @@ > > */ > > - > #ifndef __MALLOC_H__ > #define __MALLOC_H__ > > -- It was probably intentional. I've seen ^L (form feed) occasionally in old code. Perhaps the author added it to make the file paginate better if printed on paper. That said, I don't mind if you remove it. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] FPGA detection failure on Cyclone V soc development kit
Marek Vasut <ma...@denx.de> writes: > On 04/06/2016 05:29 PM, Dinh Nguyen wrote: >> On Wed, Apr 6, 2016 at 10:07 AM, Marek Vasut <marek.va...@gmail.com> wrote: >>> >>> I pushed some DDR fixes into u-boot-socfpga/ddr branch [1], which fixed >>> DDR calibration issue on a board I have in here. Can you try them ? Thanks >>> >>> [1] >>> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=shortlog;h=refs/heads/ddr >>> >> >> I'll do it first thing when I get back from ELC. > > Cool. I will do proper submission by then. I think Mans had a CV SoCDK > which didn't boot with the mainline SPL, so it'd be cool if he could try. I will when I get back from ELC. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] net: phy: micrel: fix divisor value for KSZ9031 phy skew
<dingu...@opensource.altera.com> writes:
> From: Dinh Nguyen <dingu...@opensource.altera.com>
>
> The picoseconds to register value divisor(ps_to_regval) should be 60 and not
> 200. Linux has KSZ9031_PS_TO_REG defined to be 60 as well. 60 is the correct
> divisor because the 4-bit skew values are defined from 0x(-420ps) to
> 0x(480ps), increments of 60.
>
> For example, a DTS skew value of 420, represents 0ps delay, which should be
> 0x7.
> With the previous divisor of 200, it would result in 0x2, which represents a
> -300ps delay.
>
> With this patch, ethernet on the SoCFPGA DE0 Atlas is now able to work with
> 1Gb ethernet.
Is this expected to make any difference on the Altera socdk? Both with
and without the patch, it takes a very long time (sometimes minutes) to
negotiate a link, but once it does it works fine.
> References:
> http://www.micrel.com/_PDF/Ethernet/datasheets/KSZ9031RNX.pdf -> page 26
>
> Signed-off-by: Dinh Nguyen <dingu...@opensource.altera.com>
> ---
> drivers/net/phy/micrel.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 19b6bc7..2530a5b 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -211,7 +211,7 @@ static int ksz90x1_of_config_group(struct phy_device
> *phydev,
> {
> struct udevice *dev = phydev->dev;
> struct phy_driver *drv = phydev->drv;
> - const int ps_to_regval = 200;
> + const int ps_to_regval = 60;
> int val[4];
> int i, changed = 0, offset, max;
> u16 regval = 0;
> --
> 2.6.2
--
Måns Rullgård
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] FPGA detection failure on Cyclone V soc development kit
Chin Liang See <cl...@altera.com> writes: > On Wed, 2016-01-27 at 13:46 +0000, Måns Rullgård wrote: >> Chin Liang See <cl...@altera.com> writes: >> >> > On Fri, 2016-01-22 at 10:35 -0600, Dinh Nguyen wrote: >> > > On 01/21/2016 10:31 AM, Marek Vasut wrote: >> > > > On Thursday, January 21, 2016 at 05:20:33 PM, Måns Rullgård >> > > > wrote: >> > > > > Tom Rini <tr...@konsulko.com> writes: >> > > > > > On Wed, Jan 20, 2016 at 08:31:30PM +, Måns Rullgård >> > > > > > wrote: >> > > > > > > I'm having a problem with u-boot 2016.01 failing to >> > > > > > > detect the FPGA on my Altera Cyclone V SoC Development >> > > > > > > Kit. On startup, it simply prints "FPGA: Not Altera chip >> > > > > > > ID" (the ID having been read as all -zero). No amount of >> > > > > > > messing with jumpers or switches makes a difference. The >> > > > > > > software on the SD card included in the box appears to >> > > > > > > work, so on a whim I took the SPL pre-loader from this >> > > > > > > card and combined it with the main 2016.01 u-boot. This >> > > > > > > makes the detection succeed, despite Marek baulking at >> > > > > > > this idea. The "good" SPL identifies as "U-Boot SPL >> > > > > > > 2013.01.01 (Dec 04 2014 - 08:59:41)" which is a different >> > > > > > > build date than the main u-boot on the same SD card, so >> > > > > > > which source code version it was built from is anyone's >> > > > > > > guess. >> > > > > > > >> > > > > > > What's interesting is that Marek's board works with u >> > > > > > > -boot 2016.01 while mine fails even with the very same >> > > > > > > binary. The boards are different revisions (his >> > > > > > > 100-0321003-C1, mine -E1), and the main Cyclone V chips >> > > > > > > are also different (his 5CSXFC6D6F31C8NES, mine >> > > > > > > 5CSXFC6D6F31C6N). >> > > > > > > >> > > > > > > Any suggestions for what to try next? >> > > > > > v2016.01 release or to of tree? If top of tree, try >> > > > > > http://patchwork.ozlabs.org/patch/570009/ >> > > > > Tried release, top of tree, and top of tree with that patch. >> > > > > Nothing works. >> > >> > Both part number is different in speed grade. This is first time I >> > heard about this issue. A quick suspect might due to clock. Can you >> > try to copy pll_config.h that is passing (from 2013.01.01) and >> > replace >> > the one in 2016? >> >> That doesn't work at all. Now it fails to detect the FPGA, then >> hangs after printing the amount of DRAM. > > Can you share with me the pll_config for 2013.01.01 that is working for > you? I don't know that it is. The only thing I've found to work is the unidentified SPL on the SD card that came with the dev kit. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] FPGA detection failure on Cyclone V soc development kit
Chin Liang See <cl...@altera.com> writes: > On Fri, 2016-01-22 at 10:35 -0600, Dinh Nguyen wrote: >> On 01/21/2016 10:31 AM, Marek Vasut wrote: >> > On Thursday, January 21, 2016 at 05:20:33 PM, Måns Rullgård wrote: >> > > Tom Rini <tr...@konsulko.com> writes: >> > > > On Wed, Jan 20, 2016 at 08:31:30PM +, Måns Rullgård wrote: >> > > > > I'm having a problem with u-boot 2016.01 failing to detect >> > > > > the FPGA on my Altera Cyclone V SoC Development Kit. On >> > > > > startup, it simply prints "FPGA: Not Altera chip ID" (the ID >> > > > > having been read as all -zero). No amount of messing with >> > > > > jumpers or switches makes a difference. The software on the >> > > > > SD card included in the box appears to work, so on a whim I >> > > > > took the SPL pre-loader from this card and combined it with >> > > > > the main 2016.01 u-boot. This makes the detection succeed, >> > > > > despite Marek baulking at this idea. The "good" SPL >> > > > > identifies as "U-Boot SPL 2013.01.01 (Dec 04 2014 - >> > > > > 08:59:41)" which is a different build date than the main >> > > > > u-boot on the same SD card, so which source code version it >> > > > > was built from is anyone's guess. >> > > > > >> > > > > What's interesting is that Marek's board works with u-boot >> > > > > 2016.01 while mine fails even with the very same binary. >> > > > > The boards are different revisions (his 100-0321003-C1, mine >> > > > > -E1), and the main Cyclone V chips are also different (his >> > > > > 5CSXFC6D6F31C8NES, mine 5CSXFC6D6F31C6N). >> > > > > >> > > > > Any suggestions for what to try next? >> > > > v2016.01 release or to of tree? If top of tree, try >> > > > http://patchwork.ozlabs.org/patch/570009/ >> > > Tried release, top of tree, and top of tree with that patch. >> > > Nothing works. > > Both part number is different in speed grade. This is first time I > heard about this issue. A quick suspect might due to clock. Can you > try to copy pll_config.h that is passing (from 2013.01.01) and replace > the one in 2016? That doesn't work at all. Now it fails to detect the FPGA, then hangs after printing the amount of DRAM. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 01/19] video: Add stb TrueType font renderer
Tom Rini <tr...@konsulko.com> writes: >> > style does not comply with U-Boot but I think it is best to leave alone to >> > permit the source to be synced later if needed. >> > >> > The only change is to fix a reference to fabs() which should route through >> > a macro to allow U-Boot to provide its own version. >> >> This seems to be using floating-point quite a bit. Unless I missed a >> recent change, that's not allowed in u-boot. > > You are generally speaking, correct. I am wondering if we don't need to > make exceptions, from time to time. For example, when we can easily > correct general math problems by using something from > that's one thing and should be done. > > On the other hand, we have this, which is adding a nice looking font for > the cases where our console is not a serial port but a screen and in > some cases a rather nice high DPI one too. So under the assumption that > no, we can't find a font we can borrow that doesn't also use floating > point, maybe we allow this, BUT with some caveats needing to be added > such as noting that hey, what happens if you 'go' some benchmark that > does FP stuff? Well, it better be save/restoring, yes? On some CPUs you also need to explicitly enable the FPU, and some rely on software completion of certain operations (usually involving subnormals). Of course we shouldn't let that prevent other systems having nice features. We just need to be a bit careful about when we enable them. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] FPGA detection failure on Cyclone V soc development kit
Tom Rini <tr...@konsulko.com> writes: > On Wed, Jan 20, 2016 at 08:31:30PM +0000, Måns Rullgård wrote: > >> I'm having a problem with u-boot 2016.01 failing to detect the FPGA on >> my Altera Cyclone V SoC Development Kit. On startup, it simply prints >> "FPGA: Not Altera chip ID" (the ID having been read as all-zero). No >> amount of messing with jumpers or switches makes a difference. The >> software on the SD card included in the box appears to work, so on a >> whim I took the SPL pre-loader from this card and combined it with the >> main 2016.01 u-boot. This makes the detection succeed, despite Marek >> baulking at this idea. The "good" SPL identifies as "U-Boot SPL >> 2013.01.01 (Dec 04 2014 - 08:59:41)" which is a different build date >> than the main u-boot on the same SD card, so which source code version >> it was built from is anyone's guess. >> >> What's interesting is that Marek's board works with u-boot 2016.01 while >> mine fails even with the very same binary. The boards are different >> revisions (his 100-0321003-C1, mine -E1), and the main Cyclone V chips >> are also different (his 5CSXFC6D6F31C8NES, mine 5CSXFC6D6F31C6N). >> >> Any suggestions for what to try next? > > v2016.01 release or to of tree? If top of tree, try > http://patchwork.ozlabs.org/patch/570009/ Tried release, top of tree, and top of tree with that patch. Nothing works. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] FPGA detection failure on Cyclone V soc development kit
I'm having a problem with u-boot 2016.01 failing to detect the FPGA on my Altera Cyclone V SoC Development Kit. On startup, it simply prints "FPGA: Not Altera chip ID" (the ID having been read as all-zero). No amount of messing with jumpers or switches makes a difference. The software on the SD card included in the box appears to work, so on a whim I took the SPL pre-loader from this card and combined it with the main 2016.01 u-boot. This makes the detection succeed, despite Marek baulking at this idea. The "good" SPL identifies as "U-Boot SPL 2013.01.01 (Dec 04 2014 - 08:59:41)" which is a different build date than the main u-boot on the same SD card, so which source code version it was built from is anyone's guess. What's interesting is that Marek's board works with u-boot 2016.01 while mine fails even with the very same binary. The boards are different revisions (his 100-0321003-C1, mine -E1), and the main Cyclone V chips are also different (his 5CSXFC6D6F31C8NES, mine 5CSXFC6D6F31C6N). Any suggestions for what to try next? -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 01/19] video: Add stb TrueType font renderer
Simon Glass <s...@chromium.org> writes: > This is a header file which provides a fairly light-weight TrueType > rendering implementation. It is pulled from http://nothings.org/. The code > style does not comply with U-Boot but I think it is best to leave alone to > permit the source to be synced later if needed. > > The only change is to fix a reference to fabs() which should route through > a macro to allow U-Boot to provide its own version. This seems to be using floating-point quite a bit. Unless I missed a recent change, that's not allowed in u-boot. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [ANN] U-Boot v2016.01-rc4 released
Peter Robinson <pbrobin...@gmail.com> writes: > On Tue, Jan 5, 2016 at 6:37 PM, Tom Rini <tr...@konsulko.com> wrote: >> On Tue, Jan 05, 2016 at 04:28:01PM -0200, Fabio Estevam wrote: >>> On Mon, Jan 4, 2016 at 8:23 PM, Tom Rini <tr...@konsulko.com> wrote: >>> > Hey all, >>> > >>> > I've tagged and uploaded 2016.01-rc4 now. I grabbed a few general >>> >>> Still can't see this tag. >> >> I just checked and it says everything up to date for me. But the tag >> isn't on the public side. Wolfgang? > > I still can't see the tag or the tar ball, it would be really nice to > close this gap so that everything is part of a single process so we > don't end up with the descrepency. Or at least delay the announcement until the thing actually exists publicly. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [ANN] U-Boot v2016.01-rc4 released
Tom Rini <tr...@konsulko.com> writes: > On Wed, Jan 06, 2016 at 09:56:53AM +0000, Måns Rullgård wrote: >> Peter Robinson <pbrobin...@gmail.com> writes: >> >> > On Tue, Jan 5, 2016 at 6:37 PM, Tom Rini <tr...@konsulko.com> wrote: >> >> On Tue, Jan 05, 2016 at 04:28:01PM -0200, Fabio Estevam wrote: >> >>> On Mon, Jan 4, 2016 at 8:23 PM, Tom Rini <tr...@konsulko.com> wrote: >> >>> > Hey all, >> >>> > >> >>> > I've tagged and uploaded 2016.01-rc4 now. I grabbed a few general >> >>> >> >>> Still can't see this tag. >> >> >> >> I just checked and it says everything up to date for me. But the tag >> >> isn't on the public side. Wolfgang? >> > >> > I still can't see the tag or the tar ball, it would be really nice to >> > close this gap so that everything is part of a single process so we >> > don't end up with the descrepency. >> >> Or at least delay the announcement until the thing actually exists >> publicly. > > Unfortunately that's not how the back-end stuff works. If you simply didn't send the email announcement until it was visible, nobody would notice. -- Måns Rullgård ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] net: fec_mxc: unregister mdio bus on probe error
Eric Nelson <e...@nelint.com> writes:
> Hi Mans,
>
> On 12/08/2015 08:38 AM, Mans Rullgard wrote:
>> If fecmxc_initialize_multi() fails, it frees but does not unregister
>> the mdio bus, causing subsequent uses of the "mii" command to crash.
>> Fix this by adding mdio_unregister() calls where needed.
>>
>> Signed-off-by: Mans Rullgard <m...@mansr.com>
>> ---
>> drivers/net/fec_mxc.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
>> index 1250d2a..6c5e80b 100644
>> --- a/drivers/net/fec_mxc.c
>> +++ b/drivers/net/fec_mxc.c
>> @@ -1109,6 +1109,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int
>> phy_id, uint32_t addr)
>> #ifdef CONFIG_PHYLIB
>> phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
>> if (!phydev) {
>> +mdio_unregister(bus);
>
> While you're in here, this should probably be mdio_free just
> to prevent somebody else from searching as I did.
>
>> free(bus);
>> return -ENOMEM;
>> }
>> @@ -1120,6 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int
>> phy_id, uint32_t addr)
>> #ifdef CONFIG_PHYLIB
>> free(phydev);
>> #endif
>> +mdio_unregister(bus);
>
> Ditto:
>> free(bus);
>> }
>> return ret;
>>
Agreed, but that's a separate issue.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] microblaze: Fix C99/gnu99 compatiblity for inline functions
it(int nr, volatile void
> *addr)
> return retval;
> }
>
> -extern inline int __test_and_set_bit(int nr, volatile void *addr)
> +static inline int __test_and_set_bit(int nr, volatile void *addr)
> {
> int mask, retval;
> volatile unsigned int *a = (volatile unsigned int *) addr;
> @@ -147,7 +126,7 @@ extern inline int __test_and_set_bit(int nr, volatile
> void *addr)
> return retval;
> }
>
> -extern inline int test_and_clear_bit(int nr, volatile void *addr)
> +static inline int test_and_clear_bit(int nr, volatile void *addr)
> {
> int mask, retval;
> volatile unsigned int *a = (volatile unsigned int *) addr;
> @@ -163,7 +142,7 @@ extern inline int test_and_clear_bit(int nr, volatile
> void *addr)
> return retval;
> }
>
> -extern inline int __test_and_clear_bit(int nr, volatile void *addr)
> +static inline int __test_and_clear_bit(int nr, volatile void *addr)
> {
> int mask, retval;
> volatile unsigned int *a = (volatile unsigned int *) addr;
> @@ -175,7 +154,7 @@ extern inline int __test_and_clear_bit(int nr, volatile
> void *addr)
> return retval;
> }
>
> -extern inline int test_and_change_bit(int nr, volatile void *addr)
> +static inline int test_and_change_bit(int nr, volatile void *addr)
> {
> int mask, retval;
> volatile unsigned int *a = (volatile unsigned int *) addr;
> @@ -191,7 +170,7 @@ extern inline int test_and_change_bit(int nr, volatile
> void *addr)
> return retval;
> }
>
> -extern inline int __test_and_change_bit(int nr, volatile void *addr)
> +static inline int __test_and_change_bit(int nr, volatile void *addr)
> {
> int mask, retval;
> volatile unsigned int *a = (volatile unsigned int *) addr;
> @@ -206,12 +185,12 @@ extern inline int __test_and_change_bit(int nr,
> volatile void *addr)
> /*
> * This routine doesn't need to be atomic.
> */
> -extern inline int __constant_test_bit(int nr, const volatile void *addr)
> +static inline int __constant_test_bit(int nr, const volatile void *addr)
> {
> return ((1UL << (nr & 31)) & (((const volatile unsigned int *) addr)[nr
> >> 5])) != 0;
> }
>
> -extern inline int __test_bit(int nr, volatile void *addr)
> +static inline int __test_bit(int nr, volatile void *addr)
> {
> int * a = (int *) addr;
> int mask;
> @@ -229,7 +208,7 @@ extern inline int __test_bit(int nr, volatile void *addr)
> #define find_first_zero_bit(addr, size) \
> find_next_zero_bit((addr), (size), 0)
>
> -extern inline int find_next_zero_bit(void *addr, int size, int offset)
> +static inline int find_next_zero_bit(void *addr, int size, int offset)
> {
> unsigned long *p = ((unsigned long *) addr) + (offset >> 5);
> unsigned long result = offset & ~31UL;
> @@ -275,7 +254,7 @@ found_middle:
> #define hweight8(x) generic_hweight8(x)
>
>
> -extern inline int ext2_set_bit(int nr, volatile void *addr)
> +static inline int ext2_set_bit(int nr, volatile void *addr)
> {
> int mask, retval;
> unsigned long flags;
> @@ -290,7 +269,7 @@ extern inline int ext2_set_bit(int nr, volatile void
> *addr)
> return retval;
> }
>
> -extern inline int ext2_clear_bit(int nr, volatile void *addr)
> +static inline int ext2_clear_bit(int nr, volatile void *addr)
> {
> int mask, retval;
> unsigned long flags;
> @@ -305,7 +284,7 @@ extern inline int ext2_clear_bit(int nr, volatile void
> *addr)
> return retval;
> }
>
> -extern inline int ext2_test_bit(int nr, const volatile void *addr)
> +static inline int ext2_test_bit(int nr, const volatile void *addr)
> {
> int mask;
> const volatile unsigned char*ADDR = (const unsigned char *) addr;
> diff --git a/arch/microblaze/include/asm/system.h
> b/arch/microblaze/include/asm/system.h
> index 3090835..3107748 100644
> --- a/arch/microblaze/include/asm/system.h
> +++ b/arch/microblaze/include/asm/system.h
> @@ -131,7 +131,7 @@ extern void *switch_thread (struct thread_struct *last,
>((__typeof__ (*(ptr)))__xchg ((unsigned long)(with), (ptr), sizeof
> (*(ptr
> #define tas(ptr) (xchg ((ptr), 1))
>
> -extern inline unsigned long __xchg(unsigned long with,
> +static inline unsigned long __xchg(unsigned long with,
> __volatile__ void *ptr, int size)
> {
> unsigned long tmp, flags;
> --
> 2.6.2
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Build error for PowerPPC using GCC 5.2 toolchain
Otavio Salvador <otavio.salva...@ossystems.com.br> writes: > Hello, > > At Yocto Project I sent the upgrade for the 2015.10 release, of > U-Boot, however it is failing badly: > > http://errors.yoctoproject.org/Errors/Details/21468/ > > Does someone has any idea how to fix it? The problem is the use of "extern inline" function definitions. GCC 5 defaults to the standard C99 semantics for this whereas earlier versions favoured the historical GNU semantics. The quick fix is to add the -fgnu89-inline compiler flag which requests the old behaviour. The proper fix is to replace all "extern inline" with "static inline" which is what they should have been all along. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] GCC 5.2 issue on imx28
Jörg Krause joerg.krause@embedded.rocks writes:
On Mi, 2015-08-05 at 20:23 +0100, Måns Rullgård wrote:
Jörg Krause joerg.krause@embedded.rocks writes:
Dear Måns Rullgård, Otavio Salvador,
On Di, 2015-07-28 at 14:39 +0100, Måns Rullgård wrote:
Otavio Salvador otavio.salva...@ossystems.com.br writes:
[snip]
There are two errors reports:
1. An undefined reference to the symbol lowlevel_init
2. A complaint about the .rel.plt section not being handled by
the
linker script.
The second error is probably caused by the first. A quick grep
turns
up
this snippet in arch/arm/cpu/arm926ejs/mxs/mxs.c:
/* Lowlevel init isn't used on i.MX28, so just have a dummy here
*/
inline void lowlevel_init(void) {}
The semantics for non-static functions declared inline have
changed
in
gcc5, causing the above (empty) function not to be emitted as an
external symbol.
Since that function is only referenced from start.S, it should
not be
declared inline at all. This patch should thus fix your problem:
diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c
b/arch/arm/cpu/arm926ejs/mxs/mxs.c
index ef130ae..b1d8721 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
@@ -24,7 +24,7 @@
DECLARE_GLOBAL_DATA_PTR;
/* Lowlevel init isn't used on i.MX28, so just have a dummy here
*/
-inline void lowlevel_init(void) {}
+void lowlevel_init(void) {}
void reset_cpu(ulong ignored) __attribute__((noreturn));
I stumbled over the same problem. Unfortunatly, I did not find this
patch before (only the error report from Otavia) and submitted a
similar patch [1] which keeps the inline keyword.
Best regards
Jörg Krause
[1] arm: mxs: make inline function compatible for GCC 5
https://patchwork.ozlabs.org/patch/504043/
Since the function is only referenced from outside the C file, any
use
of inline makes little sense to me. While your patch achieves the
result of creating a linkable instance of the function, it is more
complicated than it needs to be.
In my opinion it quite make sense to use inline for functions
referenced only from other files. The keyword is just an hint to the
compiler and why should it not make sense for other C files?
If using link-time optimisation, it could make sense, yes. If not,
there is no way a function can be inlined across translation units.
However, in this case it makes really no difference if lowlevel_init
is marked as inline, gcc will generate an BL lowlevel_init
instruction in both cases.
Correct. Here the only caller is in an assembly file, which is not
subject to link-time optimisations.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] GCC 5.2 issue on imx28
Jörg Krause joerg.krause@embedded.rocks writes:
Dear Måns Rullgård, Otavio Salvador,
On Di, 2015-07-28 at 14:39 +0100, Måns Rullgård wrote:
Otavio Salvador otavio.salva...@ossystems.com.br writes:
[snip]
There are two errors reports:
1. An undefined reference to the symbol lowlevel_init
2. A complaint about the .rel.plt section not being handled by the
linker script.
The second error is probably caused by the first. A quick grep turns
up
this snippet in arch/arm/cpu/arm926ejs/mxs/mxs.c:
/* Lowlevel init isn't used on i.MX28, so just have a dummy here */
inline void lowlevel_init(void) {}
The semantics for non-static functions declared inline have changed
in
gcc5, causing the above (empty) function not to be emitted as an
external symbol.
Since that function is only referenced from start.S, it should not be
declared inline at all. This patch should thus fix your problem:
diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c
b/arch/arm/cpu/arm926ejs/mxs/mxs.c
index ef130ae..b1d8721 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
@@ -24,7 +24,7 @@
DECLARE_GLOBAL_DATA_PTR;
/* Lowlevel init isn't used on i.MX28, so just have a dummy here */
-inline void lowlevel_init(void) {}
+void lowlevel_init(void) {}
void reset_cpu(ulong ignored) __attribute__((noreturn));
I stumbled over the same problem. Unfortunatly, I did not find this
patch before (only the error report from Otavia) and submitted a
similar patch [1] which keeps the inline keyword.
Best regards
Jörg Krause
[1] arm: mxs: make inline function compatible for GCC 5
https://patchwork.ozlabs.org/patch/504043/
Since the function is only referenced from outside the C file, any use
of inline makes little sense to me. While your patch achieves the
result of creating a linkable instance of the function, it is more
complicated than it needs to be.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] GCC 5.2 issue on imx28
Otavio Salvador otavio.salva...@ossystems.com.br writes:
Hello folks,
OE-Core is preparing for upgrade to GCC 5.2 as default compiler and
mx28 is failing[1] to build with it.
1. http://errors.yoctoproject.org/Errors/Details/13878/
I am not a linker guy so could someone shed any light on this?
There are two errors reports:
1. An undefined reference to the symbol lowlevel_init
2. A complaint about the .rel.plt section not being handled by the
linker script.
The second error is probably caused by the first. A quick grep turns up
this snippet in arch/arm/cpu/arm926ejs/mxs/mxs.c:
/* Lowlevel init isn't used on i.MX28, so just have a dummy here */
inline void lowlevel_init(void) {}
The semantics for non-static functions declared inline have changed in
gcc5, causing the above (empty) function not to be emitted as an
external symbol.
Since that function is only referenced from start.S, it should not be
declared inline at all. This patch should thus fix your problem:
diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c b/arch/arm/cpu/arm926ejs/mxs/mxs.c
index ef130ae..b1d8721 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
@@ -24,7 +24,7 @@
DECLARE_GLOBAL_DATA_PTR;
/* Lowlevel init isn't used on i.MX28, so just have a dummy here */
-inline void lowlevel_init(void) {}
+void lowlevel_init(void) {}
void reset_cpu(ulong ignored) __attribute__((noreturn));
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] x86: Fix a warning with gcc 4.4.4
Simon Glass s...@chromium.org writes:
This warning appears even though it seems that the compiler could work it
out. Fix it.
Signed-off-by: Simon Glass s...@chromium.org
---
arch/x86/cpu/cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index a4e639d..3583619 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -238,6 +238,7 @@ static void identify_cpu(struct cpu_device_id *cpu)
int i;
vendor_name[0] = '\0'; /* Unset */
+ cpu-device = 0; /* fix gcc 4.4.4 warning */
/* Find the id and vendor_name */
if (!has_cpuid()) {
--
Do other gcc versions warn as well? 4.4 is old and worse than both 4.3
and 4.5 in many ways, so there's very little reason to be using it. If
more sensible gcc versions do not warn, it's better leaving it as is to
avoid real errors creeping in unnoticed.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fdt_support: fix unaligned u64 accesses in fdt_fixup_memory_banks
Simon Glass s...@chromium.org writes:
Hi Rob,
On 24 October 2014 22:51, Rob Herring robherri...@gmail.com wrote:
On Sat, Oct 25, 2014 at 1:56 AM, Simon Glass s...@chromium.org wrote:
+Tom
Hi Rob,
On 15 October 2014 03:21, Rob Herring robherri...@gmail.com wrote:
From: Rob Herring r...@kernel.org
Commit f18295d3837c282f (fdt_support: fix an endian bug of
fdt_fixup_memory_banks) changed fdt_fixup_memory_banks cell writing from a
byte at a time to casting the buffer pointer to a 64-bit pointer. This can
result in unaligned accesses when there is a mixture of cell sizes of 1
and 2.
Should that be 739a01ed8e0?
Uhh, yes.
Cc: Masahiro Yamada yamad...@jp.panasonic.com
Signed-off-by: Rob Herring r...@kernel.org
---
common/fdt_support.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 3f64156..9c41f3a 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -388,19 +388,22 @@ static int fdt_pack_reg(const void *fdt, void *buf,
uint64_t *address,
int i;
int address_len = get_cells_len(fdt, #address-cells);
int size_len = get_cells_len(fdt, #size-cells);
+ u64 cell;
char *p = buf;
for (i = 0; i n; i++) {
if (address_len == 8)
- *(fdt64_t *)p = cpu_to_fdt64(address[i]);
+ cell = cpu_to_fdt64(address[i]);
else
- *(fdt32_t *)p = cpu_to_fdt32(address[i]);
+ cell = cpu_to_fdt32(address[i]);
+ memcpy(p, cell, address_len);
p += address_len;
if (size_len == 8)
- *(fdt64_t *)p = cpu_to_fdt64(size[i]);
+ cell = cpu_to_fdt64(size[i]);
else
- *(fdt32_t *)p = cpu_to_fdt32(size[i]);
+ cell = cpu_to_fdt32(size[i]);
+ memcpy(p, cell, size_len);
What will this do with 32-bit values? Aren't use assuming that the
first 32-bit word of 'cell' will contain the least significant 32
bits? Is that always true? I'm not quite sure.
We've already done the endian conversion, so we're just copying a
string of bytes only changing the alignment potentially.
Yes I think you are right. Then I suggest adding a comment here, as
memcpy() from a native type is unusual.
Better yet, use put_unaligned().
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings
Alexander Holler hol...@ahsoftware.de writes: Am 06.08.2014 08:43, schrieb Wolfgang Denk: Dear Alexander, In message 53dfdc99.2020...@ahsoftware.de you wrote: At least I've understood it such that nobody still has an objection=20 against the new feature for env import -t (-r). I object if it would be added without maintaining symmetry with env export. As explained I don't know how to add symmetry. It's impossible to export text concurrently in both formats. What is the problem with ignoring \r on input and not writing it on output? -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] Add option -r to env import to allow import of text files with CRLF as line endings
Alexander Holler hol...@ahsoftware.de writes: Am 03.08.2014 19:51, schrieb Wolfgang Denk: Dear Alexander, In message 53de658f.5010...@ahsoftware.de you wrote: Just to clarify: I see uEnv.txt (which only was possible through your env import implementation) as a read-only configuration file for u-boot, This is just one of the many possible usages. And I don't think all the necessary stuff to save a file in all the possible filesystems should end up in u-boot. Modifying filesystems is dangerous. Thius has nothing to do with exporting an environment. The export operation and the writing to the file system are two separate steps. If a file system driver contains write support or not depends on the file system code. For the environment it does not matter. If we have write support, we just use it. So from a u-boot point of view uEnv.txt is a read-only mechanism and I'm happy with it as such. As mentioned, this is but one usage. I think that env import / env export should be kept symmetric. Using a \r\n instead of \n when -r is used for env export should be something like 4 liner or such. But it would not be really symmetric. The -r for env import makes env import eat both formats, which means it can be used almost always, but using -r with env export would be a decision which always would be wrong for many people. Of course, adding the possibility to export the environment in a system-foreign format (Assuming nobody boots windows using u-boot) doesn't really make a harm, it just adds the danger that people will use -r for env export because it is used for env import too, which most likely would be wrong for most usage scenarios. Why not just make env import treat \r like any other whitespace? It would be a slight change from current behaviour, but the chance that someone actually relies on a trailing \r being part of the value is vanishingly small. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Avoid using gawk-specific strtonum()
Tom Rini tr...@ti.com writes:
On Wed, Jun 18, 2014 at 12:01:12PM +0200, Wolfgang Denk wrote:
Dear Simon,
In message 1403071815-32055-1-git-send-email-...@chromium.org you wrote:
We need to subtract two hex numbers. Avoid using strtonum() by doing the
subtraction in bc with a suitable input base.
Signed-off-by: Simon Glass s...@chromium.org
Reported-by: Vasili Galka vvv...@gmail.com
---
Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 24d9687..bb2f615 100644
--- a/Makefile
+++ b/Makefile
@@ -788,7 +788,8 @@ OBJCOPYFLAGS_u-boot.bin := -O binary
binary_size_check: u-boot.bin System.map FORCE
@file_size=`stat -c %s u-boot.bin` ; \
map_size=$(shell cat System.map | \
- awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end
= $$1} END {if (start != end != ) print strtonum(0x end) -
strtonum(0x start)}'); \
+ awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end
= $$1} END {if (start != end != ) print ibase=16; toupper(end)
- toupper(start)}' \
+ | bc); \
I think instead of introducing yet another tool dependency this could
be rewritten to use just bash:
awk '/_image_copy_start/ {start = $1}
/_image_binary_end/ {end = $1}
END { if (start != end != )
print printf %d $$((0x end - 0x start ))
}' |
bash
But now we're forcing bash.
Arithmetic expansion including hex constants is POSIX shell:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_04
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Licensing Question on ARM Semihosting Code
Darwin Rambo dra...@broadcom.com writes: Given the ARM header below, is this code possible to put into u-boot? For reference, I see this discussion below. http://lists.denx.de/pipermail/u-boot/2011-November/110884.html If this is not acceptable, presumably due to the All rights reserved and Redistribution in binary form... clauses below, we could try to find an alternative implementation that is GPL2.0+ licensed or rewrite the parts we need from scratch. Any suggestions you have would be most welcome. Thanks. Regards, Darwin Rambo /* * Copyright (c) 2013-2014, ARM Limited and Contributors. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: * * Redistributions of source code must retain the above copyright notice, this * list of conditions and the following disclaimer. * * Redistributions in binary form must reproduce the above copyright notice, * this list of conditions and the following disclaimer in the documentation * and/or other materials provided with the distribution. * * Neither the name of ARM nor the names of its contributors may be used * to endorse or promote products derived from this software without specific * prior written permission. * * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS IS * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE * POSSIBILITY OF SUCH DAMAGE. */ This is the standard 3-clause BSD licence, just without the usual explicit numbering. According to the FSF [1], it is compatible with the GPL. [1] http://www.gnu.org/licenses/license-list.html#ModifiedBSD -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Wolfgang Denk w...@denx.de writes: Dear Tom, In message 20140210222819.GE7049@bill-the-cat you wrote: I agree we shouldn't use __packed without a good reason. And I don't think we've got many no-reason uses right now but I'm all in favor of dropping them and keeping an eye on new users. The problem is that unless we do something, any of the valid and we aren't going to / can't change them __packed structs will be the next place things blow up when gcc optimizes things in a new (and valid based on what we've told it!) way. If I look around where __packed (and variants) is being used I could just cry. Just have a loog for example at include/ec_commands.h - it appears they always add __packed to all structs, including such where all entries are uint8_t - see for example struct ec_lpc_host_args :-( This is obviously insane and should be fixed. Deliberately miscompiling other code isn't going to help with that. Takes valid code? But would this original code run on an architecture where any unaligned access causes a machine check? My understanding is it doesn't? It would run because being __packed gcc would know to do the right thing on that architecture. We are not discussing application code here. What does that have to do with anything. The compiler works exactly the same whatever the code is for. We are dealing with low- level hardware related stuff. When I try to read from a 32 bit device register I must be absolutley sure that this will be a single 32 bit transfer. It is totally unacceptable if I have to fear that the compiler might break this up into 4 byte accesses behind my back. So because you're afraid of __packed abuse, you want to make _other_ completely valid code crash? Would it not be preferable to make the actually broken code fail? Then someone might notice and fix it. Furthermore, the scenario you seem worried about will still happen on ARMv5 and other architectures which do not support unaligned memory accesses. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Wolfgang Denk w...@denx.de writes: Dear Måns Rullgård, In message yw1xob2dakq4@unicorn.mansr.com you wrote: We are not discussing application code here. What does that have to do with anything. The compiler works exactly the same whatever the code is for. With application code you don't really care whether a variable is read/written in one piece or broken down into several smaller reads/writes - except when you notice that performance suffers. When accessing hardware, it often makes a fundamental difference whether you access a device register with it's correct size or not. Usually breaking down an access into smaller ones results in crashes or incorrect data or other errors. I'm well aware of this, but it has nothing to do with the issue at hand. We are dealing with low- level hardware related stuff. When I try to read from a 32 bit device register I must be absolutley sure that this will be a single 32 bit transfer. It is totally unacceptable if I have to fear that the compiler might break this up into 4 byte accesses behind my back. So because you're afraid of __packed abuse, you want to make _other_ completely valid code crash? Would it not be preferable to make the actually broken code fail? Then someone might notice and fix it. Furthermore, the scenario you seem worried about will still happen on ARMv5 and other architectures which do not support unaligned memory accesses. I wonder if you actually read Albert's arguments. I'll try to put it in simple words for you. No need to be condescending. No, this is not about __packed, at least not primarily. We are talking about code that performs unaligned accesses. There are architectures where the hardware supports this just fine; there are others where you pay for this with some penalty, but it still works; and there are yet others where it just does not work. Correct. And we cannot rely on the compiler to do the right thing because the compiler assumes the application model described above, while we need the device access model, i. e. something different. And we want all code (unless it is not inherently deeply architecture-specific) to be working on all architectures, whether these support unaligned accesses or not. Device registers are always aligned. In properly written code these are accessed using pointers the compiler knows to be aligned and thus does the right thing. If a device register is accessed in a manner that the compiler thinks it might be unaligned, this will result in the access being split on architectures like ARMv5 that do not support any unaligned accesses. As you point out, this will break at runtime. Any code doing this is thus broken and needs to be fixed. So I would like to adjust the default behaviour of the compiler to raise errors or at least warnings for all such unaligned accesses that he is capable of detecting, and I want clear runtime errors for the rest, on all architectures. Code that causes such errors needs to be reviewed and, normally, to be fixed. In cases where there are good reasons to perform unaligned accesses, these should normally be done explicitly, without automatic help from the compiler. Only if there is such good reasons for unaligned accesses AND there are good reasons not to touch the code AND we are sure it will not break on some architectures, then we should allow the compiler to use it's automatics. All of that makes sense. The problem is that the current settings do the exact opposite. By using -munaligned-access by default, you are asking the compiler to go ahead and do whatever it thinks is best, which is sometimes to perform an intentional unaligned access. Exactly when this will happen is largely impossible to predict. To get the behaviour you desire, the code should be compiled with -mno-unaligned-access. This tells the compiler to _never_ automatically perform an unaligned memory access. If it thinks an address might be unaligned, it will split the access. The *only* case where building with -munaligned-access might be required to make some code run correctly is if said code abuses the __packed attribute on pointers referring to device registers (which are always aligned and should never have this annotation), _and_ by sheer luck no actual unaligned accesses are introduced by the compiler. In this case, those build settings conspire to cover up a bug which will manifest itself when the code is built for a target that never allows unaligned accesses. Note also that adding -mno-unaligned-access results in exactly the same compiler behaviour as we always had prior to gcc 4.7, which presumably generated usable code. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, On Tue, 11 Feb 2014 15:33:09 +, Måns Rullgård m...@mansr.com wrote: The problem is that the current settings do the exact opposite. By using -munaligned-access by default, you are asking the compiler to go ahead and do whatever it thinks is best, which is sometimes to perform an intentional unaligned access. Exactly when this will happen is largely impossible to predict. The -munaligned-access option does *not* [ask] the compiler to go ahead and do whatever it thinks is best, it tells it to use direct native accesses when unaligned accesses are required, as opposed to splitting unaligned accesses into smaller but aligned aligned native accesses, which is what you get with -mno-unaligned-access. The flag does both of those things. It even gives the compiler permission to merge multiple adjacent accesses into a single wider one. To get the behaviour you desire, the code should be compiled with -mno-unaligned-access. This tells the compiler to _never_ automatically perform an unaligned memory access. If it thinks an address might be unaligned, it will split the access. This shows that you really have not read my argument, in which I *did* explain why we tell the compiler *not* to split unaligned accesses into smaller correctly aligned accesses, i.e., why -munaligned-access is used. I have read what you call your argument. Unfortunately for you, it is based on false premises, and as such any conclusions you arrive at are incorrect. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Wolfgang Denk w...@denx.de writes: Dear Måns Rullgård, In message yw1xfvnpaclm@unicorn.mansr.com you wrote: With application code you don't really care whether a variable is read/written in one piece or broken down into several smaller reads/writes - except when you notice that performance suffers. When accessing hardware, it often makes a fundamental difference whether you access a device register with it's correct size or not. Usually breaking down an access into smaller ones results in crashes or incorrect data or other errors. I'm well aware of this, but it has nothing to do with the issue at hand. This is by your definition, or based on which exact rationale? Device registers are always aligned. In properly written code these are accessed using pointers the compiler knows to be aligned and thus does the right thing. Maybe the device registers you have seen so far have always been perfectly aligned. Lucky you. Note that there are a number of designs around (that I do not hesitate to call broken) which have such properties. And when we include external hardware into the discussion like PCI attached devices or customer-designed FPGAs, it becomes even more interesting. Please explain how you would access an unaligned register from a CPU that doesn't allow unaligned accesses. Clearly such a combination of hardware can never work, so there is no use trying to cater for it in haphazard ways that end up breaking perfectly normal systems. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Tom, On Mon, 10 Feb 2014 08:21:39 -0500, Tom Rini tr...@ti.com wrote: On Mon, Feb 10, 2014 at 10:24:47AM +0100, Albert ARIBAUD wrote: Hi Tom, On Tue, 4 Feb 2014 12:05:33 -0500, Tom Rini tr...@ti.com wrote: When we tell the compiler to optimize for ARMv7 it assumes a default of unaligned accesses being supported at the hardware level and can make use of this to perform what it deems as an optimization in any case, including allowing for data to become unaligned. We explicitly disallow this hardware feature so we must tell the compiler. Cc: Albert ARIBAUD albert.u.b...@aribaud.net Cc: Mans Rullgard m...@mansr.com Signed-off-by: Tom Rini tr...@ti.com NAK -- the discrepancy between the compiler being told to allow native unaligned accesses while at the same time telling the hardware to trap them is conscious and voluntary. It was chosen to help detect unaligned accesses which are rarely necessary and can be explicitly performed by software on a case by case basis. If and when a specific file requires unaligned access which cannot be made by some other mean than enabling -mno-unaligned-access, then this file should have it added, not the whole of U-Boot. Right, I recall the discussion, and we chose wrong. I am quite prepared to discuss whether we chose wrong or right, and to change my mind when the conditions are right, but I'll need more than such a short and simple statement. :) I already gave you a detailed explanation some months ago. You refused to read it. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Måns Rullgård m...@mansr.com writes: Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Tom, On Mon, 10 Feb 2014 08:21:39 -0500, Tom Rini tr...@ti.com wrote: On Mon, Feb 10, 2014 at 10:24:47AM +0100, Albert ARIBAUD wrote: Hi Tom, On Tue, 4 Feb 2014 12:05:33 -0500, Tom Rini tr...@ti.com wrote: When we tell the compiler to optimize for ARMv7 it assumes a default of unaligned accesses being supported at the hardware level and can make use of this to perform what it deems as an optimization in any case, including allowing for data to become unaligned. We explicitly disallow this hardware feature so we must tell the compiler. Cc: Albert ARIBAUD albert.u.b...@aribaud.net Cc: Mans Rullgard m...@mansr.com Signed-off-by: Tom Rini tr...@ti.com NAK -- the discrepancy between the compiler being told to allow native unaligned accesses while at the same time telling the hardware to trap them is conscious and voluntary. It was chosen to help detect unaligned accesses which are rarely necessary and can be explicitly performed by software on a case by case basis. If and when a specific file requires unaligned access which cannot be made by some other mean than enabling -mno-unaligned-access, then this file should have it added, not the whole of U-Boot. Right, I recall the discussion, and we chose wrong. I am quite prepared to discuss whether we chose wrong or right, and to change my mind when the conditions are right, but I'll need more than such a short and simple statement. :) I already gave you a detailed explanation some months ago. You refused to read it. For reference: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/171876 -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, On Mon, 10 Feb 2014 15:14:49 +, Måns Rullgård m...@mansr.com wrote: Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Tom, On Mon, 10 Feb 2014 08:21:39 -0500, Tom Rini tr...@ti.com wrote: On Mon, Feb 10, 2014 at 10:24:47AM +0100, Albert ARIBAUD wrote: Hi Tom, On Tue, 4 Feb 2014 12:05:33 -0500, Tom Rini tr...@ti.com wrote: When we tell the compiler to optimize for ARMv7 it assumes a default of unaligned accesses being supported at the hardware level and can make use of this to perform what it deems as an optimization in any case, including allowing for data to become unaligned. We explicitly disallow this hardware feature so we must tell the compiler. Cc: Albert ARIBAUD albert.u.b...@aribaud.net Cc: Mans Rullgard m...@mansr.com Signed-off-by: Tom Rini tr...@ti.com NAK -- the discrepancy between the compiler being told to allow native unaligned accesses while at the same time telling the hardware to trap them is conscious and voluntary. It was chosen to help detect unaligned accesses which are rarely necessary and can be explicitly performed by software on a case by case basis. If and when a specific file requires unaligned access which cannot be made by some other mean than enabling -mno-unaligned-access, then this file should have it added, not the whole of U-Boot. Right, I recall the discussion, and we chose wrong. I am quite prepared to discuss whether we chose wrong or right, and to change my mind when the conditions are right, but I'll need more than such a short and simple statement. :) I already gave you a detailed explanation some months ago. You refused to read it. I can hardly have refused to read a message which I *answered*, laid out how I thought the issue should be solved... and got no answer after this. In your reply, you called the important parts of my explanation irrelevant. That's more or less the same thing. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] arm: Switch to -mno-unaligned-access when supported by the compiler
Tom Rini tr...@ti.com writes: When we tell the compiler to optimize for ARMv7 it assumes a default of unaligned accesses being supported at the hardware level and can make use of this to perform what it deems as an optimization in any case, including allowing for data to become unaligned. We explicitly disallow this hardware feature so we must tell the compiler. Cc: Albert ARIBAUD albert.u.b...@aribaud.net Cc: Mans Rullgard m...@mansr.com Signed-off-by: Tom Rini tr...@ti.com Acked-by: Mans Rullgard m...@mansr.com -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: Switch to -mno-unaligned-access when supported by the compiler
Tom Rini tr...@ti.com writes: When we tell the compiler to optimize for ARMv7 it assumes a default of unaligned accesses being supported at the hardware level and can make use of this to perform what it deems as an optimization in any case, including allowing for data to become unaligned. We explicitly disallow this hardware feature so we must tell the compiler. Cc: Albert ARIBAUD albert.u.b...@aribaud.net Cc: Mans Rullgard m...@mansr.com Signed-off-by: Tom Rini tr...@ti.com --- arch/arm/cpu/armv7/config.mk |6 +- arch/arm/cpu/armv8/config.mk |5 +- common/Makefile |4 -- doc/README.arm-unaligned-accesses | 122 - fs/ubifs/Makefile |3 - lib/Makefile |3 - 6 files changed, 5 insertions(+), 138 deletions(-) delete mode 100644 doc/README.arm-unaligned-accesses Awesome. You should also remove the reference to the (deleted) readme file from arch/arm/lib/interrupts.c -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] am3517: segmentation fault in Linux after upgrading from arago src to mainline u-boot
Yegor Yefremov yegorsli...@googlemail.com writes:
On Fri, Dec 6, 2013 at 8:28 AM, Yegor Yefremov
yegorsli...@googlemail.com wrote:
On Thu, Dec 5, 2013 at 6:28 PM, Måns Rullgård m...@mansr.com wrote:
Yegor Yefremov yegorsli...@googlemail.com writes:
On Thu, Nov 28, 2013 at 5:57 PM, Tom Rini tr...@ti.com wrote:
On Thu, Nov 28, 2013 at 05:39:09PM +0100, Yegor Yefremov wrote:
I'm having problems with the new u-boot (November 2013) and am3517
SoC. When I start Debian 7.0 armhf I get lots of Segmentation faults,
I can't even perform apt-get udpate. With the older u-boot (arago
sources from 2010) everything seems to work properly.
I've checked ATAG_MEM and both u-boot versions deliver same RAM start
address and size. I've compared EMIF4 settings
(arch/arm/cpu/armv7/omap3/emif4.c) and PINMUX and everything is the
same.
Where else can I look for differences?
Can you bisect the arago tree from what it's based on, to what the top
of tree is there? Some errata or another being patched up I guess..
I've compared the x-loader's init routine with the one from SPL. An
found this workaround in the mainline U-Boot
(arch/arm/cpu/armv7/omap3/board.c):
static void omap3_setup_aux_cr(void)
{
/* Workaround for Cortex-A8 errata: #454179 #430973
* Set IBE bit
* Set Disable Branch Size Mispredicts bit
* Workaround for erratum #621766
* Enable L1NEON bit
* ACR |= (IBE | DBSM | L1NEON) = ACR |= 0xE0
*/
omap3_update_aux_cr_secure(0xE0, 0);
}
After commenting this routine and I don't see any segmentation faults
during apt usage.
On an r1pX Cortex-A8 (as found in the 35xx chips) you absolutely want
those bits set, especially if you're running Thumb2 code. Otherwise you
will get random crashes.
Where can I find info, if am3517 is r1pX? I've looked through AM35x
ARM Microprocessor: Technical Reference Manual, but found nothing
similar.
This info was in the Errata.pdf:
AM3517AZCN i.e. silicon revision 1.1 (r1p7 ROM 15:58)
All of those errata are present in r1p7. The 454179 and 430973 also
need workarounds enabled in the kernel. Enabling the workarounds on a
CPU without the problems will not break anything, but it will cause a
minor performance drop.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] am3517: segmentation fault in Linux after upgrading from arago src to mainline u-boot
Yegor Yefremov yegorsli...@googlemail.com writes:
On Thu, Nov 28, 2013 at 5:57 PM, Tom Rini tr...@ti.com wrote:
On Thu, Nov 28, 2013 at 05:39:09PM +0100, Yegor Yefremov wrote:
I'm having problems with the new u-boot (November 2013) and am3517
SoC. When I start Debian 7.0 armhf I get lots of Segmentation faults,
I can't even perform apt-get udpate. With the older u-boot (arago
sources from 2010) everything seems to work properly.
I've checked ATAG_MEM and both u-boot versions deliver same RAM start
address and size. I've compared EMIF4 settings
(arch/arm/cpu/armv7/omap3/emif4.c) and PINMUX and everything is the
same.
Where else can I look for differences?
Can you bisect the arago tree from what it's based on, to what the top
of tree is there? Some errata or another being patched up I guess..
I've compared the x-loader's init routine with the one from SPL. An
found this workaround in the mainline U-Boot
(arch/arm/cpu/armv7/omap3/board.c):
static void omap3_setup_aux_cr(void)
{
/* Workaround for Cortex-A8 errata: #454179 #430973
* Set IBE bit
* Set Disable Branch Size Mispredicts bit
* Workaround for erratum #621766
* Enable L1NEON bit
* ACR |= (IBE | DBSM | L1NEON) = ACR |= 0xE0
*/
omap3_update_aux_cr_secure(0xE0, 0);
}
After commenting this routine and I don't see any segmentation faults
during apt usage.
On an r1pX Cortex-A8 (as found in the 35xx chips) you absolutely want
those bits set, especially if you're running Thumb2 code. Otherwise you
will get random crashes.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] toolchain problems when building iMX6 mx6qsabreauto (and another bootloader tool)
Robert Nelson robertcnel...@gmail.com writes: On Fri, Nov 29, 2013 at 4:28 PM, Wolfgang Denk w...@denx.de wrote: Dear Christian Gmeiner, In message cah9nwweb6s+3s4o25d7cifob9y3_7jmhosjo6o78fijquwz...@mail.gmail.com you wrote: I am running into the same problem. Me wonders which patch/change triggers this problem. I am using the same toolchain since I started to work with u-boot and it fails with the latest rc. Maybe Linaro started using a hard-float configuration only recently? For what it's worth, i just ran a git bisect on this issue today, as this compiler worked fine with v2013.10 toolchain: arm-linux-gnueabihf-gcc (crosstool-NG linaro-1.13.1-4.8-2013.10 - Linaro GCC 2013.10) 4.8.2 20131014 (prerelease) 762a88ccf8540948fbf8c31b40a29d1e0684a25b is the first bad commit commit 762a88ccf8540948fbf8c31b40a29d1e0684a25b Author: Pierre Aubert p.aub...@staubli.com Date: Thu Sep 19 17:48:59 2013 +0200 mx6: compute PLL PFD frequencies rather than using defines That commit introduces a 64-bit division without using the lldiv() function, which pulls in previously unused libgcc stuff. Try this patch: diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c index 873d9d0..752b3c8 100644 --- a/arch/arm/cpu/armv7/mx6/clock.c +++ b/arch/arm/cpu/armv7/mx6/clock.c @@ -5,6 +5,7 @@ */ #include common.h +#include div64.h #include asm/io.h #include asm/errno.h #include asm/arch/imx-regs.h @@ -123,8 +124,8 @@ static u32 mxc_get_pll_pfd(enum pll_clocks pll, int pfd_num) return 0; } - return (freq * 18) / ((div ANATOP_PFD_FRAC_MASK(pfd_num)) - ANATOP_PFD_FRAC_SHIFT(pfd_num)); + return lldiv(freq * 18, (div ANATOP_PFD_FRAC_MASK(pfd_num)) + ANATOP_PFD_FRAC_SHIFT(pfd_num)); } static u32 get_mcu_main_clk(void) -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] am3517: segmentation fault in Linux after upgrading from arago src to mainline u-boot
Tom Rini tr...@ti.com writes: On Thu, Nov 28, 2013 at 05:39:09PM +0100, Yegor Yefremov wrote: I'm having problems with the new u-boot (November 2013) and am3517 SoC. When I start Debian 7.0 armhf I get lots of Segmentation faults, I can't even perform apt-get udpate. With the older u-boot (arago sources from 2010) everything seems to work properly. I've checked ATAG_MEM and both u-boot versions deliver same RAM start address and size. I've compared EMIF4 settings (arch/arm/cpu/armv7/omap3/emif4.c) and PINMUX and everything is the same. Misconfigured memory would have been my first guess. Where else can I look for differences? Can you bisect the arago tree from what it's based on, to what the top of tree is there? Some errata or another being patched up I guess.. Which A8 revision does this chip use? Is it the same r1p3 as the 3530? -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] mx28: Error: unrecognized/unsupported processor variant (0x41069265).
Mårten Wikman marten.wik...@novia.fi writes: I can't see how that would end up in the wrong machine ID being passed to the kernel. Looks like it was a problem with my toolchain, changed to another and kernel started. I'm curious, which was the broken toolchain? -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] mx28: Error: unrecognized/unsupported processor variant (0x41069265).
Mårten Wikman marten.wik...@novia.fi writes: Still getting the CPU ID printed by the kernel? Then something between you setting the value and the kernel reading it is going wrong. Yes I still get the CPU ID printed by the kernel. What I understand you should not have to set machid manually if everything is done correctly? I've never had to set it manually, and either way it's strange for the CPU ID to show up there. The machine ID is passed to the kernel in r1 from the u-boot function boot_jump_linux(). I'd start by verifying that this function is indeed passing the proper value. To be able to boot u-boot on our custom board we have changed spl_mem_init.c so dram_vals matches our memory configuration and in include/configs/mx28evk.h where we changed PHYS_SDRAM_1_SIZE. The kernel image we're trying to load is compiled with mxs_defconfig without any changes and it loads without problems on the evk-board. Could it be that we somehow messed up with the memconfiguration? I can't see how that would end up in the wrong machine ID being passed to the kernel. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] mx28: Error: unrecognized/unsupported processor variant (0x41069265).
Mårten Wikman marten.wik...@novia.fi writes: Hello, I'm bringing up a custom board that includes a mx283 and 64MB DDR2 memory, now I have some troubles with getting u-boot to properly boot the kernel. I have enabled kernel_debugging and with earlyprintk I get the following output: ## Booting kernel from Legacy Image at 4200 ... Image Name: Linux-3.12.0-10554-g801a760 Created: 2013-11-19 20:56:47 UTC Image Type: ARM Linux Kernel Image (uncompressed) Data Size:3538288 Bytes = 3.4 MiB Load Address: 40008000 Entry Point: 40008000 Verifying Checksum ... OK ## Flattened Device Tree blob at 4100 Booting using the fdt blob at 0x4100 Loading Kernel Image ... OK Loading Device Tree to 43b21000, end 43b291ca ... OK Starting kernel ... Uncompressing Linux... done, booting the kernel. Error: unrecognized/unsupported processor variant (0x41069265). After I looked the error message up on the internet I understand that the machine-id u-boot is passing to the kernel is wrong. When I compare arch_number from bdinfo on my custom board and my mx28-evk they are the same and my mx28-evk boots the same kernel image without roblems. Can anybody give some advice how to solve this? 0x41069265 is the CP15 ID register value for ARM926. I'd look into how this value ended up where the machine ID should be. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] mx28: Error: unrecognized/unsupported processor variant (0x41069265).
Mårten Wikman marten.wik...@novia.fi writes: 0x41069265 is the CP15 ID register value for ARM926. I'd look into how this value ended up where the machine ID should be. I tried do set the correct machine ID with setenv machid but I get the same result even if u-boot says it's using the new machine ID. Still getting the CPU ID printed by the kernel? Then something between you setting the value and the kernel reading it is going wrong. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] cmd_test: fix a compile error on Blackfin
Masahiro Yamada yamad...@jp.panasonic.com writes: Before this commit, common/cmd_test.c defined _STDBOOL_H in order to avoid including stdbool.h. But this work-around is not a good idea. Blackfin header file arch/blackfin/include/asm/blackfin_local.h uses bool type here: extern bool bfin_os_log_check(void); This means Blackfin boards which define CONFIG_SYS_HUSH_PARSER always failed in compiling. This commit fixes this issue by undefining true and false macro. [...] +/* + * Undef true and false here to avoid macro expansion by stdbool.h + */ +#undef true +#undef false This won't work if stdbool.h defines true/false as an enum. Some versions of gcc did this. The correct fix is to change the code so it doesn't use any of the identifiers bool, true, or false for other purposes than those intended by stdbool.h. I agree with Wolfgang that using boolean types is generally a bad idea that only introduces problems. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] include: define bool type in a more portable way
Tom Rini tr...@ti.com writes: On Mon, Nov 18, 2013 at 05:28:56PM +0100, Wolfgang Denk wrote: Dear Masahiro Yamada, In message 1384770105-32364-1-git-send-email-yamad...@jp.panasonic.com you wrote: Currently U-boot defins bool type by including stdbool.h rather than defining directly. But it does not work for some cross compilers. Can you explain why this fails? AFAICT, stdbool.h is a compiler provided header file, which is mandatory by the C99 ISO standard. So if a compiler fails to work using it, it looks as if the compiler was broken. Indeed. And the very ancient but (as far as I know) still latest gcc for the SPARC port, which is 3.4.4, has it. So if there's a problem here with blackfin it feels strongly like a toolchain issue. FWIW, my Blackfin cross-gcc has a correct stdbool.h. Even gcc 2.95 has it. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Illegal use of FP ops in clock_ti814x.c
Wolfgang Denk w...@denx.de writes: Dear Måns Rullgård, In message yw1x8uxc28y9@unicorn.mansr.com you wrote: Something like this should be equivalent. That said, it looks suspiciously like it's meant to simply do a division and round up. If that is the case, +225 should be +249. It probably makes no difference for the values actually encountered. Umm... this is the part which I do not understand. The original code adds 90%; you add 90%, too. However, to round up, one usually adds only 50% ? Adding 50% would round to nearest. For integer division to round up, you must add one less than the divisor. Agreed. But do we want to round up? The original code used +90%, which is something else, too... That rounds fractions = 0.1 up while 0.1 is rounded down. It's an unusual thing to do, which is why I suspect it's not quite correct in the first place. Where are these 90% coming from? Are they in any way meaningful, or even critical? My guess is that it was someone's approximation of 249 / 250. I don't know the hardware, so it's conceivable that it really should be this way, although it seems unlikely. Are you able to test such a modificationon actual hardware? No. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Illegal use of FP ops in clock_ti814x.c
Wolfgang Denk w...@denx.de writes:
Dear Matt,
I hope you are the right person to address this to - if not, please
help to redirect to the current responsible developer.
Function pll_sigma_delta_val() in arch/arm/cpu/armv7/am33xx/clock_ti814x.c
incorrectly uses float data, which results in FP operations which
are not permitted in U-Boot.
The actual computation appears simple enough so a rewrite of the code
without using any floating point operations should be fairly easy, but
I don't understand the actual logic of this code, so I'd rather leave
this to someone who does.
Could you please help and clean up these three lines of code?
Something like this should be equivalent. That said, it looks
suspiciously like it's meant to simply do a division and round up. If
that is the case, +225 should be +249. It probably makes no difference
for the values actually encountered.
diff --git a/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
b/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
index ef14f47..9b5a47b 100644
--- a/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
+++ b/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
@@ -211,11 +211,8 @@ static u32 pll_dco_freq_sel(u32 clkout_dco)
static u32 pll_sigma_delta_val(u32 clkout_dco)
{
u32 sig_val = 0;
- float frac_div;
- frac_div = (float) clkout_dco / 250;
- frac_div = frac_div + 0.90;
- sig_val = (int)frac_div;
+ sig_val = (clkout_dco + 225) / 250;
sig_val = sig_val 24;
return sig_val;
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Illegal use of FP ops in clock_ti814x.c
Wolfgang Denk w...@denx.de writes:
Dear Måns Rullgård,
In message yw1xd2mp0yqu@unicorn.mansr.com you wrote:
Something like this should be equivalent. That said, it looks
suspiciously like it's meant to simply do a division and round up. If
that is the case, +225 should be +249. It probably makes no difference
for the values actually encountered.
Umm... this is the part which I do not understand.
The original code adds 90%; you add 90%, too. However, to round up,
one usually adds only 50% ?
Adding 50% would round to nearest. For integer division to round up,
you must add one less than the divisor.
diff --git a/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
b/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
index ef14f47..9b5a47b 100644
--- a/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
+++ b/arch/arm/cpu/armv7/am33xx/clock_ti814x.c
@@ -211,11 +211,8 @@ static u32 pll_dco_freq_sel(u32 clkout_dco)
static u32 pll_sigma_delta_val(u32 clkout_dco)
{
u32 sig_val = 0;
- float frac_div;
- frac_div = (float) clkout_dco / 250;
- frac_div = frac_div + 0.90;
- sig_val = (int)frac_div;
+ sig_val = (clkout_dco + 225) / 250;
sig_val = sig_val 24;
Where are these 90% coming from? Are they in any way meaningful, or
even critical?
My guess is that it was someone's approximation of 249 / 250. I don't
know the hardware, so it's conceivable that it really should be this
way, although it seems unlikely.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, On Tue, 15 Oct 2013 16:23:44 +0100, Måns Rullgård m...@mansr.com wrote: On the compiler side, gcc traditionally did not issue unaligned load/store instructions on ARM. Please be specific: gcc did not emit *native* unaligned accesses. Please explain what you mean by native unaligned access. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
Piotr Wilczek p.wilc...@samsung.com writes: -Original Message- From: Måns Rullgård [mailto:m...@mansr.com] Sent: Saturday, October 12, 2013 1:29 AM To: Piotr Wilczek Cc: u-boot@lists.denx.de; Tom Rini; Kyungmin Park Subject: Re: [PATCH] disk:efi: avoid unaligned access on efi partition Piotr Wilczek p.wilc...@samsung.com writes: In this patch static variable and memcpy instead of an assignment are used to avoid unaligned access exception on some ARM platforms. Signed-off-by: Piotr Wilczek p.wilc...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com CC: Tom Rini tr...@ti.com --- disk/part_efi.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/disk/part_efi.c b/disk/part_efi.c index b7524d6..303b8af 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -224,7 +224,8 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) p_mbr-signature = MSDOS_MBR_SIGNATURE; p_mbr-partition_record[0].sys_ind = EFI_PMBR_OSTYPE_EFI_GPT; p_mbr-partition_record[0].start_sect = 1; - p_mbr-partition_record[0].nr_sects = (u32) dev_desc-lba; + memcpy(p_mbr-partition_record[0].nr_sects, dev_desc-lba, + sizeof(dev_desc-lba)); Why is this assignment problematic? Note that the compiler may optimise the memcpy() call into a plain assignment including any alignment assumptions it was making in the original code. The correct fix is either to ensure that pointers are properly aligned or that things are annotated as potentially unaligned, whichever is more appropriate. Problem is that the legacy_mbr structure consists 'le16 unknown' field before 'partition_record' filed and the structure is packed. As a result the address of 'partition_record' filed is halfword aligned. The compiler uses str/ldr instructions (address must be 4-byte aligned) to copy u32 'lba' data thus causing unaligned access exception. If the struct has __attribute__((packed)), gcc should do the right thing already. Note that on ARMv6 and later ldr/str support unaligned addresses unless this is explicitly disabled in the system control register. If you do this, you _MUST_ compile with -mno-unaligned-access. Otherwise you will get problems. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, On Mon, 14 Oct 2013 11:50:42 +0100, Måns Rullgård m...@mansr.com wrote: Piotr Wilczek p.wilc...@samsung.com writes: -Original Message- From: Måns Rullgård [mailto:m...@mansr.com] Sent: Saturday, October 12, 2013 1:29 AM To: Piotr Wilczek Cc: u-boot@lists.denx.de; Tom Rini; Kyungmin Park Subject: Re: [PATCH] disk:efi: avoid unaligned access on efi partition Piotr Wilczek p.wilc...@samsung.com writes: In this patch static variable and memcpy instead of an assignment are used to avoid unaligned access exception on some ARM platforms. Signed-off-by: Piotr Wilczek p.wilc...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com CC: Tom Rini tr...@ti.com --- disk/part_efi.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/disk/part_efi.c b/disk/part_efi.c index b7524d6..303b8af 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -224,7 +224,8 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) p_mbr-signature = MSDOS_MBR_SIGNATURE; p_mbr-partition_record[0].sys_ind = EFI_PMBR_OSTYPE_EFI_GPT; p_mbr-partition_record[0].start_sect = 1; - p_mbr-partition_record[0].nr_sects = (u32) dev_desc-lba; + memcpy(p_mbr-partition_record[0].nr_sects, dev_desc-lba, + sizeof(dev_desc-lba)); Why is this assignment problematic? Note that the compiler may optimise the memcpy() call into a plain assignment including any alignment assumptions it was making in the original code. The correct fix is either to ensure that pointers are properly aligned or that things are annotated as potentially unaligned, whichever is more appropriate. Problem is that the legacy_mbr structure consists 'le16 unknown' field before 'partition_record' filed and the structure is packed. As a result the address of 'partition_record' filed is halfword aligned. The compiler uses str/ldr instructions (address must be 4-byte aligned) to copy u32 'lba' data thus causing unaligned access exception. If the struct has __attribute__((packed)), gcc should do the right thing already. Note that on ARMv6 and later ldr/str support unaligned addresses unless this is explicitly disabled in the system control register. If you do this, you _MUST_ compile with -mno-unaligned-access. Otherwise you will get problems. Please do not advise using native unaligned accesses on code that is not strictly used by ARMv6+ architectures: the present code, for instance, might be run on pre-ARMv6 or non-ARM platforms, and thus, should never assume ability to perform unaligned accesses natively. I'm advising no such thing. I said two things: 1. Declaring a struct with the 'packed' attribute makes gcc automatically generate correct code for all targets. _IF_ the selected target supports unaligned ldr/str, these might get used. 2. If your target is ARMv6 or later _AND_ you enable strict alignment checking in the system control register, you _MUST_ build with the -mno-unaligned-access flag. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
Albert ARIBAUD albert.u.b...@aribaud.net writes: Please do not advise using native unaligned accesses on code that is not strictly used by ARMv6+ architectures: the present code, for instance, might be run on pre-ARMv6 or non-ARM platforms, and thus, should never assume ability to perform unaligned accesses natively. I'm advising no such thing. I said two things: 1. Declaring a struct with the 'packed' attribute makes gcc automatically generate correct code for all targets. _IF_ the selected target supports unaligned ldr/str, these might get used. 2. If your target is ARMv6 or later _AND_ you enable strict alignment checking in the system control register, you _MUST_ build with the -mno-unaligned-access flag. Then I apologize; I had read Note that on ARMv6 and later ldr/str support unaligned addresses unless this is explicitly disabled in the system control register as a suggestion to use that capability. If building for ARMv6 or later, I do suggest allowing unaligned accesses. The moment you add -march=armv6 (or equivalent), you allow for a number of things not supported by older versions, so why not unaligned memory accesses? -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, On Mon, 14 Oct 2013 14:05:13 +0100, Måns Rullgård m...@mansr.com wrote: Albert ARIBAUD albert.u.b...@aribaud.net writes: Please do not advise using native unaligned accesses on code that is not strictly used by ARMv6+ architectures: the present code, for instance, might be run on pre-ARMv6 or non-ARM platforms, and thus, should never assume ability to perform unaligned accesses natively. I'm advising no such thing. I said two things: 1. Declaring a struct with the 'packed' attribute makes gcc automatically generate correct code for all targets. _IF_ the selected target supports unaligned ldr/str, these might get used. 2. If your target is ARMv6 or later _AND_ you enable strict alignment checking in the system control register, you _MUST_ build with the -mno-unaligned-access flag. Then I apologize; I had read Note that on ARMv6 and later ldr/str support unaligned addresses unless this is explicitly disabled in the system control register as a suggestion to use that capability. If building for ARMv6 or later, I do suggest allowing unaligned accesses. The moment you add -march=armv6 (or equivalent), you allow for a number of things not supported by older versions, so why not unaligned memory accesses? doc/README.arm-unaligned-accesses probably has the answer to your question, especially from line 21 onward. If not, I'll be happy to provide further clarification. That is about as backwards as it can get. By adding -munaligned-access you are telling gcc that unaligned accesses are supported and welcome. With this setting enabled, gcc can and will generate unaligned accesses just about anywhere. This setting is NOT compatible with the SCTRL.A bit being set (strict hardware alignment checking). You cannot enable strict alignment checking in hardware, tell the compiler unaligned accesses are OK, and then expect not to have problems. This is no more a valid combination than allowing floating-point instructions when the target has no FPU. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, On Mon, 14 Oct 2013 15:09:39 +0100, Måns Rullgård m...@mansr.com wrote: Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, On Mon, 14 Oct 2013 14:05:13 +0100, Måns Rullgård m...@mansr.com wrote: Albert ARIBAUD albert.u.b...@aribaud.net writes: Please do not advise using native unaligned accesses on code that is not strictly used by ARMv6+ architectures: the present code, for instance, might be run on pre-ARMv6 or non-ARM platforms, and thus, should never assume ability to perform unaligned accesses natively. I'm advising no such thing. I said two things: 1. Declaring a struct with the 'packed' attribute makes gcc automatically generate correct code for all targets. _IF_ the selected target supports unaligned ldr/str, these might get used. 2. If your target is ARMv6 or later _AND_ you enable strict alignment checking in the system control register, you _MUST_ build with the -mno-unaligned-access flag. Then I apologize; I had read Note that on ARMv6 and later ldr/str support unaligned addresses unless this is explicitly disabled in the system control register as a suggestion to use that capability. If building for ARMv6 or later, I do suggest allowing unaligned accesses. The moment you add -march=armv6 (or equivalent), you allow for a number of things not supported by older versions, so why not unaligned memory accesses? doc/README.arm-unaligned-accesses probably has the answer to your question, especially from line 21 onward. If not, I'll be happy to provide further clarification. That is about as backwards as it can get. By adding -munaligned-access you are telling gcc that unaligned accesses are supported and welcome. With this setting enabled, gcc can and will generate unaligned accesses just about anywhere. This setting is NOT compatible with the SCTRL.A bit being set (strict hardware alignment checking). You cannot enable strict alignment checking in hardware, tell the compiler unaligned accesses are OK, and then expect not to have problems. This is no more a valid combination than allowing floating-point instructions when the target has no FPU. I sense that you have not understood the reason why I want alignment checking enabled in ARM yet also want ARMv6+ builds to emit native unaligned accesses if they consider it needed. Your wishes are mutually exclusive. You cannot both allow hardware unaligned access AND at the same time trap them. The reason is, if we prevent ARMv6 builds from using native unaligned accesses, they would replace *all* such accesses with smaller, aligned, ones, which would not trigger a data abort; even those unaligned accesses cased by programming errors. If you disable unaligned accesses in hardware (as u-boot does), you have no option but doing them a byte at a time. Whereas if we allow ARMv6+ builds to use native unaligned accesses, yet enable alignment checks, then any native unaligned access will be caught as early as possible, and we'll find and cure the issue faster. This is, of course, assuming we don't voluntarily do native unaligned accesses, and in U-Boot, we indeed don't: in U-Boot, accesses are done on natural alignments. The hardware does not differentiate between intentional and unintentional unaligned accesses. Unlike some architectures, which have dedicated instructions for unaligned accesses, ARM uses the same instructions in both cases (with some limitations). Since I have set up this policy, experience (and it has been a while) shows that very few problems arise from alignment checks + native unaligned accesses. These roughly come from hardware- or standards- mandated unaligned fields, in which case they are worth explicitly accessing with unaligned macros, or from programming errors, which should be fixed. The problem is that when you tell gcc (using -munaligned-access) that hardware unaligned accesses are supported, you give it permission to compile even get/put_unaligned() calls (or otherwise annotated accesses) using regular LDR/STR instructions. If this code runs with strict checking enabled in hardware (SCTRL.A set), it will trap. What you probably want is to build with -mno-unaligned-access and enable strict hardware alignment checking. This ensures that any deliberate unaligned accesses (e.g. through get_unaligned) are split into multiple smaller accesses while trapping any (unintentional) unaligned word accesses. The most common alignment-related programming mistake is to dereference a pointer with insufficient alignment. It is far less common for pointers to be marked as unaligned when they do not need to be. Another benefit of it is, if the code builds and runs in mainline with alignment checks *and* native unaligned accesses enabled, then it builds and runs also if you disable either one; whereas code
Re: [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition
Piotr Wilczek p.wilc...@samsung.com writes: In this patch static variable and memcpy instead of an assignment are used to avoid unaligned access exception on some ARM platforms. Signed-off-by: Piotr Wilczek p.wilc...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com CC: Tom Rini tr...@ti.com --- disk/part_efi.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/disk/part_efi.c b/disk/part_efi.c index b7524d6..303b8af 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -224,7 +224,8 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc) p_mbr-signature = MSDOS_MBR_SIGNATURE; p_mbr-partition_record[0].sys_ind = EFI_PMBR_OSTYPE_EFI_GPT; p_mbr-partition_record[0].start_sect = 1; - p_mbr-partition_record[0].nr_sects = (u32) dev_desc-lba; + memcpy(p_mbr-partition_record[0].nr_sects, dev_desc-lba, +sizeof(dev_desc-lba)); Why is this assignment problematic? Note that the compiler may optimise the memcpy() call into a plain assignment including any alignment assumptions it was making in the original code. The correct fix is either to ensure that pointers are properly aligned or that things are annotated as potentially unaligned, whichever is more appropriate. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] merge arm64 to arm
Tom Rini tr...@ti.com writes: On 08/18/2013 11:46 PM, Sharma Bhupesh-B45370 wrote: [Re-posting as original msg was rejected due to HTML content..] FengHua feng...@phytium.com.cn writes: FengHua feng...@phytium.com.cn writes: hi tom, hi albert, yes, it's right. the u-boot could be more uniformly and maintainable if merging armv8 to arm architecture. I will try to migrate arm64 to armv8 subarchitecture of arm. do you have any other advice? Why? The architectures are vastly different, arm64 (aarch64) being only loosely inspired by the 32-bit arm. It is not like with x86/amd64 where a lot of code can be shared. Of course, with a seperated architecture the arm64 code will be clear and simple. when it merged with arm a few file should be duplicated with the name _v8 appended and many macro switch should be added. but most of the code will be in armv8 directory which paralleled with armv7. it seems that this implementation are more nice. ARMv8 defines both a 32-bit (aarch32) and a 64-bit (aarch64) instruction set. The naming you are suggesting would be misleading. aarch32 state is compatible with armv7. armv8 directory only provide aarch64 state support. as you said, it would be a little misleading. ARMv8 ARM (Architecture Reference Manual) mentions that the ARMv8 architecture has support for both AArch32 and AArch64 and the ARM can switch b/w the two instruction sets via exceptions. So, whether choosing a naming convention similar to linux (arch/arm64) would be more suitable is something to consider (even though some of the files might be a copy of what is available in arch/arm/cpu/armv7)? I think we'll see what happens with a single directory first. We aren't talking about a binary that has to work on all cases (right now...) A single binary can obviously never work. and we want to avoid massive duplication of all of the C code that really won't change. If there's a lot of code shared between these architectures, why is it in an architecture-specific directory in the first place? Maybe the proper solution is to move it out of arch/arm rather than moving code for an entirely different architecture in there. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] merge arm64 to arm
Tom Rini tr...@ti.com writes: On 08/19/2013 08:32 AM, Måns Rullgård wrote: Tom Rini tr...@ti.com writes: On 08/18/2013 11:46 PM, Sharma Bhupesh-B45370 wrote: [Re-posting as original msg was rejected due to HTML content..] FengHua feng...@phytium.com.cn writes: FengHua feng...@phytium.com.cn writes: hi tom, hi albert, yes, it's right. the u-boot could be more uniformly and maintainable if merging armv8 to arm architecture. I will try to migrate arm64 to armv8 subarchitecture of arm. do you have any other advice? Why? The architectures are vastly different, arm64 (aarch64) being only loosely inspired by the 32-bit arm. It is not like with x86/amd64 where a lot of code can be shared. Of course, with a seperated architecture the arm64 code will be clear and simple. when it merged with arm a few file should be duplicated with the name _v8 appended and many macro switch should be added. but most of the code will be in armv8 directory which paralleled with armv7. it seems that this implementation are more nice. ARMv8 defines both a 32-bit (aarch32) and a 64-bit (aarch64) instruction set. The naming you are suggesting would be misleading. aarch32 state is compatible with armv7. armv8 directory only provide aarch64 state support. as you said, it would be a little misleading. ARMv8 ARM (Architecture Reference Manual) mentions that the ARMv8 architecture has support for both AArch32 and AArch64 and the ARM can switch b/w the two instruction sets via exceptions. So, whether choosing a naming convention similar to linux (arch/arm64) would be more suitable is something to consider (even though some of the files might be a copy of what is available in arch/arm/cpu/armv7)? I think we'll see what happens with a single directory first. We aren't talking about a binary that has to work on all cases (right now...) A single binary can obviously never work. and we want to avoid massive duplication of all of the C code that really won't change. If there's a lot of code shared between these architectures, why is it in an architecture-specific directory in the first place? Maybe the proper solution is to move it out of arch/arm rather than moving code for an entirely different architecture in there. We are working in that direction (and one of the requests was to hook into that code, rather than duplicate things). Think of it as all ARM Ltd licensed cores not all 32bit-only ARM cores. Why does it matter which company designed it? By that reasoning, you'd put i960 (were it supported) under arch/x86 because it's from Intel. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] merge arm64 to arm
Scott Wood scottw...@freescale.com writes: On Mon, 2013-08-19 at 09:10 -0400, Tom Rini wrote: On 08/19/2013 09:01 AM, Måns Rullgård wrote: Tom Rini tr...@ti.com writes: On 08/19/2013 08:32 AM, Måns Rullgård wrote: If there's a lot of code shared between these architectures, why is it in an architecture-specific directory in the first place? Maybe the proper solution is to move it out of arch/arm rather than moving code for an entirely different architecture in there. We are working in that direction (and one of the requests was to hook into that code, rather than duplicate things). Think of it as all ARM Ltd licensed cores not all 32bit-only ARM cores. Why does it matter which company designed it? By that reasoning, you'd put i960 (were it supported) under arch/x86 because it's from Intel. Probably because I didn't get the it's a whole new unrelated to everything before world over there! memo. Probably because there is still quite a bit of similarity to older ARM. There's more to it than just the ISA, and even that isn't *that* much more different than x86 versus x86_64. It is quite a bit more different. 32-bit x86 is a subset of x86_64 where as arm64 is entirely new, if somewhat inspired by 32-bit arm. i960 is a bad analogy. It's often possible to turn arm32 asm into arm64 asm with some search and replace and minor manual fixups. Only if the original uses none of the distinguishing features of ARM like predicated instructions or variably shifted operands. Once you limit yourself to the remaining basic operations, every (RISC) architecture looks the same. Seriously tho, our directory structure is different from the kernel and it seems like things might look cleaner this way. If it doesn't, well, I'll admit to being wrong and we'll go back to a split arch directory. As I noted before, in Linux a bunch of other architectures started with a separate arch for 64-bit (x86, sparc, ppc...), and all of them eventually merged. In those cases, the 64-bitness is merely an extension to the 32-bit instruction set. Most of them don't even have a notion of 32-bit vs 64-bit mode of execution. AArch64 of course shares certain non-ISA aspects with AArch32. Page table formats and other architecturally defined system control features are the same, and code for managing these things should of course be shared. Some other features, e.g. exception handling, are different enough that sharing code is probably difficult. There is a tendency to see arm64/aarch64 as yet another 64-bit extension of a 32-bit architecture, which it is not. Assuming that software support will or can follow the model used by the others mentioned is thus a mistake. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] merge arm64 to arm
Tom Rini tr...@ti.com writes: On 08/19/2013 01:33 PM, Måns Rullgård wrote: Scott Wood scottw...@freescale.com writes: [snip] i960 is a bad analogy. It's often possible to turn arm32 asm into arm64 asm with some search and replace and minor manual fixups. Only if the original uses none of the distinguishing features of ARM like predicated instructions or variably shifted operands. Once you limit yourself to the remaining basic operations, every (RISC) architecture looks the same. [snip] AArch64 of course shares certain non-ISA aspects with AArch32. Page table formats and other architecturally defined system control features are the same, and code for managing these things should of course be shared. Some other features, e.g. exception handling, are different enough that sharing code is probably difficult. There is a tendency to see arm64/aarch64 as yet another 64-bit extension of a 32-bit architecture, which it is not. Assuming that software support will or can follow the model used by the others mentioned is thus a mistake. We don't have lots of hand-crafted assembly, and what we do, we largely have split out already into the cpu directories. I really think we just need to try this and see how it goes. Fine, let's see what it ends up looking like. That said, please consider naming things in a way that armv8 does not imply 64-bit. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] merge arm64 to arm
FengHua feng...@phytium.com.cn writes: hi tom, hi albert, yes, it's right. the u-boot could be more uniformly and maintainable if merging armv8 to arm architecture. I will try to migrate arm64 to armv8 subarchitecture of arm. do you have any other advice? Why? The architectures are vastly different, arm64 (aarch64) being only loosely inspired by the 32-bit arm. It is not like with x86/amd64 where a lot of code can be shared. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] merge arm64 to arm
FengHua feng...@phytium.com.cn writes: FengHua feng...@phytium.com.cn writes: hi tom, hi albert, yes, it's right. the u-boot could be more uniformly and maintainable if merging armv8 to arm architecture. I will try to migrate arm64 to armv8 subarchitecture of arm. do you have any other advice? Why? The architectures are vastly different, arm64 (aarch64) being only loosely inspired by the 32-bit arm. It is not like with x86/amd64 where a lot of code can be shared. Of course, with a seperated architecture the arm64 code will be clear and simple. when it merged with arm a few file should be duplicated with the name _v8 appended and many macro switch should be added. but most of the code will be in armv8 directory which paralleled with armv7. it seems that this implementation are more nice. ARMv8 defines both a 32-bit (aarch32) and a 64-bit (aarch64) instruction set. The naming you are suggesting would be misleading. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: byteorder: add optimized swab16 and swab32
Dirk Behme dirk.be...@gmail.com writes: Use the specialized ARM instructions for swapping 16 and 32 bit values instead of using the generic ones from include/linux/byteorder/swab.h. The x86 version in arch/x86/include/asm/byteorder.h was taken as an example for this. E.g. for the mx6qsabrelite target this results in ~4k less code. Which compiler are you using? GCC 4.5 and later recognises the byteswap pattern and emits these instructions automatically. This gives better code than the inline asm since it allows the compiler to do proper instruction scheduling, and in the 16-bit case it avoids extraneous masking. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] C99 and dynamic arrays
Tom Rini tom.r...@gmail.com writes: On Tue, Mar 12, 2013 at 7:22 PM, Simon Glass s...@google.com wrote: Hi, Given that we seem to allow C99 features in U-Boot I wonder if it would be OK to use dynamic arrays in SPL? I am trying to replace: ptr = malloc(size); with: char ptr[size]; to avoid use of malloc in SPL. Can I assume that is permitted? Without knowing the underlying mechanics of how that works, maybe. How it works depends on the compiler. Some compilers implement it by calling malloc(). GCC uses the stack. Regardless of how they are implemented, variable-length arrays should, in my opinion, never be used. There is simply no way they can be used safely since no mechanism for detecting failure is provided. If the requested size is too large, you will silently overflow the stack or end up with an invalid/null pointer. In an environment without full memory protection, errors resulting from this are very hard to track down. If the size is somehow limited to a safe value, it is more efficient to simply allocate this maximum size statically. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] C99 and dynamic arrays
Simon Glass s...@google.com writes: Hi Mans, On Wed, Mar 13, 2013 at 3:29 AM, Måns Rullgård m...@mansr.com wrote: Tom Rini tom.r...@gmail.com writes: On Tue, Mar 12, 2013 at 7:22 PM, Simon Glass s...@google.com wrote: Hi, Given that we seem to allow C99 features in U-Boot I wonder if it would be OK to use dynamic arrays in SPL? I am trying to replace: ptr = malloc(size); with: char ptr[size]; to avoid use of malloc in SPL. Can I assume that is permitted? Without knowing the underlying mechanics of how that works, maybe. How it works depends on the compiler. Some compilers implement it by calling malloc(). GCC uses the stack. Regardless of how they are implemented, variable-length arrays should, in my opinion, never be used. There is simply no way they can be used safely since no mechanism for detecting failure is provided. If the requested size is too large, you will silently overflow the stack or end up with an invalid/null pointer. In an environment without full memory protection, errors resulting from this are very hard to track down. I suppose we could check the available stack space. However I don't really see a clear stack bottom in U-Boot - I think it is set up to grow downwards as much as needed. I can certainly add sanity checks on the input values. There is no way to check stack usage from C. If the size is somehow limited to a safe value, it is more efficient to simply allocate this maximum size statically. Yes although this does waste BSS. Sorry, I meant a statically sized stack allocation. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] C99 and dynamic arrays
Simon Glass s...@chromium.org writes: [once more from correct address, sorry] Hi, On Wed, Mar 13, 2013 at 11:03 AM, Måns Rullgård m...@mansr.com wrote: Simon Glass s...@google.com writes: Hi Mans, On Wed, Mar 13, 2013 at 3:29 AM, Måns Rullgård m...@mansr.com wrote: Tom Rini tom.r...@gmail.com writes: On Tue, Mar 12, 2013 at 7:22 PM, Simon Glass s...@google.com wrote: Hi, Given that we seem to allow C99 features in U-Boot I wonder if it would be OK to use dynamic arrays in SPL? I am trying to replace: ptr = malloc(size); with: char ptr[size]; to avoid use of malloc in SPL. Can I assume that is permitted? Without knowing the underlying mechanics of how that works, maybe. How it works depends on the compiler. Some compilers implement it by calling malloc(). GCC uses the stack. Regardless of how they are implemented, variable-length arrays should, in my opinion, never be used. There is simply no way they can be used safely since no mechanism for detecting failure is provided. If the requested size is too large, you will silently overflow the stack or end up with an invalid/null pointer. In an environment without full memory protection, errors resulting from this are very hard to track down. I suppose we could check the available stack space. However I don't really see a clear stack bottom in U-Boot - I think it is set up to grow downwards as much as needed. I can certainly add sanity checks on the input values. There is no way to check stack usage from C. Well there is an architecture-specific way. A function can generally find its own stack pointer by taking the address of a local variable, so it is possible to write a function to check for stack overflow. Performing such checks without getting into undefined behaviours is tricky if not impossible, and modern compilers are quite effective at exploiting these, rendering such checks useless. Remember the deleted null checks in the kernel a while back? We could add this in U-Boot if it is a general problem. For my purposes the amount of stack I intend to allocate is fairly small (1-2KB perhaps). I'm sure what you _intend_ to allocate is safe, but what's to guarantee that the values are sane? If the size is somehow limited to a safe value, it is more efficient to simply allocate this maximum size statically. Yes although this does waste BSS. Sorry, I meant a statically sized stack allocation. OK, then I suppose this is not much different from dynamic arrays? It gives more efficient code, if nothing else. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] C99 and dynamic arrays
Stephen Warren swar...@wwwdotorg.org writes: On 03/13/2013 12:03 PM, Måns Rullgård wrote: Simon Glass s...@google.com writes: Hi Mans, On Wed, Mar 13, 2013 at 3:29 AM, Måns Rullgård m...@mansr.com wrote: Tom Rini tom.r...@gmail.com writes: On Tue, Mar 12, 2013 at 7:22 PM, Simon Glass s...@google.com wrote: Hi, Given that we seem to allow C99 features in U-Boot I wonder if it would be OK to use dynamic arrays in SPL? I am trying to replace: ptr = malloc(size); with: char ptr[size]; to avoid use of malloc in SPL. Can I assume that is permitted? Without knowing the underlying mechanics of how that works, maybe. How it works depends on the compiler. Some compilers implement it by calling malloc(). GCC uses the stack. Regardless of how they are implemented, variable-length arrays should, in my opinion, never be used. There is simply no way they can be used safely since no mechanism for detecting failure is provided. If the requested size is too large, you will silently overflow the stack or end up with an invalid/null pointer. In an environment without full memory protection, errors resulting from this are very hard to track down. I suppose we could check the available stack space. However I don't really see a clear stack bottom in U-Boot - I think it is set up to grow downwards as much as needed. I can certainly add sanity checks on the input values. There is no way to check stack usage from C. If the size is somehow limited to a safe value, it is more efficient to simply allocate this maximum size statically. Yes although this does waste BSS. Sorry, I meant a statically sized stack allocation. But, there's also no way to detect failure in that case either. No, but there is an obvious upper bound to the frame size. Absent recursion, a static analysis tool can find the maximum stack space required starting from a given point, but only if each function uses a fixed amount. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Introduce a global bool type
Albert ARIBAUD albert.u.b...@aribaud.net writes: Hi Måns, In other words, boolifying on use rather than on assignment is generally safer and usually at least as efficient. Except when assigning a C = A B where A and B happen to have no common bit set. Which is why I think 'boolifying' as soon as possible -- on assignment -- is way safer than on use. But that's not a boolean context. You should use A B. The thing is, when using a value, you know if you need a boolean, but not necessarily (easily) how the value was assigned. Conversely, when assigning a value, you do not know how it will be used. By always boolifying on use, you remove the need to keep track of which values are true booleans and which ones are arbitrary values (or even pointers) you happen to be using in a boolean fashion. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Introduce a global bool type
Scott Wood scottw...@freescale.com writes:
On 01/19/2013 03:30:30 AM, Albert ARIBAUD wrote:
/* what I favor */
clk_is_enabled = ((reg_val 9) 1) ? true: false;
ip_is_enabled = clk_is_enabled pwd_is_enabled;
if (clk_is_enabled) { ...
rather than assigning them 'zero/nonzero', or using bitwise ops on
booleans, or testing against boolean constants (although I concede
that the first line below wins over its counterpart above as far as
concision is concerned).
Conciseness can be improved with !!((reg_val 9) 1).
x 1 is already either zero or one. Any further operations are nothing
but obfuscation.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Introduce a global bool type
Scott Wood scottw...@freescale.com writes:
On 01/21/2013 04:36:42 PM, Måns Rullgård wrote:
Scott Wood scottw...@freescale.com writes:
On 01/19/2013 03:30:30 AM, Albert ARIBAUD wrote:
/* what I favor */
clk_is_enabled = ((reg_val 9) 1) ? true: false;
ip_is_enabled = clk_is_enabled pwd_is_enabled;
if (clk_is_enabled) { ...
rather than assigning them 'zero/nonzero', or using bitwise ops on
booleans, or testing against boolean constants (although I concede
that the first line below wins over its counterpart above as far as
concision is concerned).
Conciseness can be improved with !!((reg_val 9) 1).
x 1 is already either zero or one. Any further operations are
nothing
but obfuscation.
The point is to avoid depending on the actual integer values of the
boolean type,
Boolean expressions are defined to have a value of zero or one, and the
_Bool type (may it burn in hell) must be able to represent those values.
and make the code more robust against changes (e.g. someone later
comes along and says hmm, that 1 should be a 3 because we care about
that other register bit as well without noticing that it's being
assigned to a boolean.
If you stayed away from the silly _Bool type that wouldn't be a problem,
as long as any uses of the value treat all non-zero values equally, i.e.
only use it in a boolean context and not compare against explicit values
or perform arithmetic or bitwise logic operations on it.
In other words, boolifying on use rather than on assignment is generally
safer and usually at least as efficient.
--
Måns Rullgård
m...@mansr.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Introduce a global bool type
Wolfgang Denk w...@denx.de writes: Dear York Sun, In message 1357596628-27501-1-git-send-email-york...@freescale.com you wrote: 'bool' is defined in random places. This patch consolidates them into a single typedef. Has this been actually compile tested? ... --- a/include/linux/types.h +++ b/include/linux/types.h @@ -113,6 +113,8 @@ typedef __u64 u_int64_t; typedef __s64 int64_t; #endif +typedef _Bool bool; And what exactly would _Bool be? _Bool is a C99 type (though I fail to imagine why). If using this, one might as well use the C99 header stdbool.h providing macros for 'bool', 'true' and 'false' instead of this. Can we rather try and get rid of all this bool stuff instead? It's just obfuscating the code... Indeed. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] ARM: prevent misaligned array inits
Albert ARIBAUD albert.u.b...@aribaud.net writes: Under option -munaligned-access, gcc can perform local char or 16-bit array initializations using misaligned native accesses which will throw a data abort exception. Fix files where these array initializations were unneeded, and for files known to contain such initializations, enforce gcc option -mno-unaligned-access. Why don't you just stop setting the damn strict alignment flag on ARMv6 and later instead this endless hacking around? You really have only two sane choices here: 1. Keep strict alignment checking and build with -mno-unaligned-access. 2. Drop strict alignment checking and build with (implicit) -munaligned-access. Option 2 gives faster, smaller code, so the choice should be an easy one to make. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: armv7: add compile option -mno-unaligned-access if available
Tetsuyuki Kobayashi k...@kmckk.co.jp writes: Recent compiler generates unaligned memory access in armv7 default. But current U-Boot does not allow unaligned memory access, so it causes data abort exception. This patch add compile option -mno-unaligned-access if it is available. Why not allow unaligned accesses instead? -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: enable unaligned access on ARMv7
Albert ARIBAUD albert.u.b...@aribaud.net writes: I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there. If the access is in true RAM, then we can always realign the data; and I don't know of memory-mapped registers which would be unaligned wrt their width. If some USB controller is designed so, then the fix should only and explicitly affect that controller, because we don't know it it will always be used with an ARM implementation that can do unaligned accesses. The ARM architecture does not permit unaligned accesses to strongly ordered or device memory, so MMIO register accesses are always aligned. -- Måns Rullgård m...@mansr.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
