[U-Boot] [PATCH] [FIX] cmd_nand: fix help of nand erase subcommand

2011-05-19 Thread Daniel Hobi
Since commit 30486322 (nand erase: .spread, .part, .chip subcommands)
the arguments off and size are no longer optional.

Signed-off-by: Daniel Hobi 
Cc: Scott Wood 
---
 common/cmd_nand.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 7bd37de..44c4d1f 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -693,7 +693,7 @@ U_BOOT_CMD(
"write 'size' bytes starting at offset 'off' with yaffs format\n"
"from memory address 'addr', skipping bad blocks.\n"
 #endif
-   "nand erase[.spread] [clean] [off [size]] - erase 'size' bytes "
+   "nand erase[.spread] [clean] off size - erase 'size' bytes "
"from offset 'off'\n"
"With '.spread', erase enough for given file size, otherwise,\n"
"'size' includes skipped bad blocks.\n"
-- 
1.7.5.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] [FIX] env_nand: zero-initialize variable nand_erase_options

2011-05-18 Thread Daniel Hobi
Commit 30486322 (nand erase: .spread, .part, .chip subcommands)
added a new field to struct nand_erase_options, but forgot to
update common/env_nand.c.

Depending on the stack state and bad block distribution, saveenv()
can thus erase more than CONFIG_ENV_RANGE bytes which may corrupt
the following NAND sectors/partitions.

Signed-off-by: Daniel Hobi 
---
 common/env_nand.c |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/common/env_nand.c b/common/env_nand.c
index 980425a..14446a6 100644
--- a/common/env_nand.c
+++ b/common/env_nand.c
@@ -193,10 +193,8 @@ int saveenv(void)
int ret = 0;
nand_erase_options_t nand_erase_options;
 
+   memset(&nand_erase_options, 0, sizeof(nand_erase_options));
nand_erase_options.length = CONFIG_ENV_RANGE;
-   nand_erase_options.quiet = 0;
-   nand_erase_options.jffs2 = 0;
-   nand_erase_options.scrub = 0;
 
if (CONFIG_ENV_RANGE < CONFIG_ENV_SIZE)
return 1;
@@ -249,10 +247,8 @@ int saveenv(void)
char*res;
nand_erase_options_t nand_erase_options;
 
+   memset(&nand_erase_options, 0, sizeof(nand_erase_options));
nand_erase_options.length = CONFIG_ENV_RANGE;
-   nand_erase_options.quiet = 0;
-   nand_erase_options.jffs2 = 0;
-   nand_erase_options.scrub = 0;
nand_erase_options.offset = CONFIG_ENV_OFFSET;
 
if (CONFIG_ENV_RANGE < CONFIG_ENV_SIZE)
-- 
1.7.3.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH V4 1/2] arm926ejs: fix linker file for newer ld support

2010-11-15 Thread Daniel Hobi
Hi Eric,

On 10.11.2010 00:43, Eric Cooper wrote:
> I have been periodically rebasing my patches for the Seagate DockStar
> on master.  But ever since the elf_reloc changes were merged, I have
> been unable to get a working build.  First I saw symptoms similar to
> what Alexander Holler reported (failing during NAND initialization due
> to incorrect BSS relocation), but the latest arm926ejs ld script
> patches did not fix it.  The system would hang here:
> 
> U-Boot 2010.12-rc1 (Nov 09 2010 - 17:52:38)
> Seagate FreeAgent DockStar
> 
> SoC:   Kirkwood 88F6281_A0
> DRAM:  128 MiB
> NAND:  
> 
> This was built with the CodeSourcery 2010q1 toolchain.  I also
> included Alexander's patch to double-check the relocation, and no
> error message gets printed.

Did you locally fix arch/arm/cpu/arm926ejs/kirkwood/timer.c to not
access uninitialized data (variables timestamp and lastdec) before
relocation?

Otherwise timer_init() will write into the relocation table which may
lead to your problem. If you change unrelated code, timer_init() will
overwrite some other relocation table entries and U-Boot seems to work.

@Albert: If your patch is applied without fixing the accesses to BSS
before relocation, some ARM ports will be broken even when compiling
with ELDK 4.2.

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

2010-11-10 Thread Daniel Hobi
Hi Albert and Heiko,

On 10.11.2010 17:46, Heiko Schocher wrote:
> Daniel Hobi wrote:
>> Why not add alignment to start.S?
>>
>>  /* Set stackpointer in internal RAM to call board_init_f */
>>  call_board_init_f:
>>  ldr sp, =(CONFIG_SYS_INIT_SP_ADDR)
>> +bic sp, sp, #7
> 
> Hmm.. because we do in board_init_f:
> 
>  gd = (gd_t *) (CONFIG_SYS_INIT_SP_ADDR);

gd (struct global_data) does not require the same alignment as the stack
pointer. If CONFIG_SYS_INIT_SP_ADDR is 4-byte aligned, but not 8-byte
aligned, 4 bytes between these two are unused. If you care about that,
just 8-byte align gd in the assignment above. It makes sense to at least
4-byte align gd anyway.

> Hmm.. maybe we should call board_init_f with the info, where to
> find the stack, but this would be a change for all architectures,
> not only arm ...

We would tell board_init_f where to find the global_data, not the stack.
If other architectures also use the same computation for stack (in
start.S) and global_data (in board_init_f), this change should be
considered.

On 10.11.2010 17:29, Albert ARIBAUD wrote:
> Bad solution IMO: if the symbol is used elsewhere, one would have to 
> always remember that it should be aligned down to a multiple of
> eight. We may not need to make it a generated constant, but we
> definitely need to make sure it is correctly aligned wherever it is
> used, e.g. by defining it as
> 
> #define CONFIG_SYS_INIT_SP_ADDR
>   (0xC8012004-GENERATED_GBL_DATA_SIZE)&~7

I think it's much less error-prone to align CONFIG_SYS_INIT_SP_ADDR
wherever it is used. It should be used only at two places anyway: to
initialize the early stack pointer and the global_data pointer.

As a reminder: it was extremely difficult the find the misaligned stack
pointer (resulting in the patch from 2009-10-06) given the symptoms. And
since you cannot expect board maintainers to spend an equal amount of
time to solve this problem over and over again, we definitely need to
align CONFIG_SYS_INIT_SP_ADDR where it is used, and not where it is defined.

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

2010-11-10 Thread Daniel Hobi
Hi Heiko,

On 10.11.2010 16:40, Heiko Schocher wrote:
> Daniel Hobi wrote:
>> But you also added assembly code to setup the initial stack pointer in
>> arch/arm/cpu/*/start.S (ie commit ab86f72c for arm926ejf) which reads:
>>
>> /* Set stackpointer in internal RAM to call board_init_f */
>> call_board_init_f:
>>  ldr sp, =(CONFIG_SYS_INIT_SP_ADDR)
>>
>> CONFIG_SYS_INIT_SP_ADDR may not be aligned properly, especially with
>> your latest patch to km_arm.h:
>>
>> #define CONFIG_SYS_INIT_SP_ADDR  (0xC8012000 - GENERATED_GBL_DATA_SIZE)
> 
> Ah, good catch.
> 
> Then we should add this alignment into the generation of
> GENERATED_GBL_DATA_SIZE.

Hm? The stack pointer needs alignment, not GENERATED_GBL_DATA_SIZE. What
happens if I define:

#define CONFIG_SYS_INIT_SP_ADDR (0xC8012004 - GENERATED_GBL_DATA_SIZE)

Why not add alignment to start.S?

 /* Set stackpointer in internal RAM to call board_init_f */
 call_board_init_f:
ldr sp, =(CONFIG_SYS_INIT_SP_ADDR)
+   bic sp, sp, #7

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

2010-11-10 Thread Daniel Hobi
Hi Heiko,

On 10.11.2010 16:02, Heiko Schocher wrote:
> Daniel Hobi wrote:
>> Reading your patch, I noticed that we don't align the early stack
>> pointer to an 8-byte boundary which may lead to the problem described here:
>>
>> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/69342
> 
> This article is from 2009-10-06 06:44:22 GMT (1 year, 5 weeks, 2 hours and 15 
> minutes ago)
> 
>> Since you seem to be the author of the relevant assembly code, could you
>> please prepare a fix (adding "bic sp, sp, #7")?
> 
> We do this, since relocation is introduced, now here:
>
> [link to arch/arm/lib/board.c]

Yes, I know.

But you also added assembly code to setup the initial stack pointer in
arch/arm/cpu/*/start.S (ie commit ab86f72c for arm926ejf) which reads:

/* Set stackpointer in internal RAM to call board_init_f */
call_board_init_f:
ldr sp, =(CONFIG_SYS_INIT_SP_ADDR)

CONFIG_SYS_INIT_SP_ADDR may not be aligned properly, especially with
your latest patch to km_arm.h:

#define CONFIG_SYS_INIT_SP_ADDR (0xC8012000 - GENERATED_GBL_DATA_SIZE)

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

2010-11-10 Thread Daniel Hobi
Hi Heiko,

On 10.11.2010 15:15, Heiko Schocher wrote:
> Daniel Hobi wrote:
>> @Heiko: include/configs/km_arm.h may have the same problem.
> 
> I made a patch for this, see:
> 
> http://lists.denx.de/pipermail/u-boot/2010-November/081275.html

Reading your patch, I noticed that we don't align the early stack
pointer to an 8-byte boundary which may lead to the problem described here:

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/69342

Since you seem to be the author of the relevant assembly code, could you
please prepare a fix (adding "bic sp, sp, #7")?

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] kirkwood: CONFIG_SYS_INIT_SP_ADDR wrong?

2010-11-10 Thread Daniel Hobi
Hi Prafulla,

In commit 0b20ed76 (Kirkwood: Changes specific to ARM relocation
support), you set CONFIG_SYS_INIT_SP_ADDR to 0xC8012000 which is
supposed to lie within the internal Security SRAM.

However, the Kirkwood Functional Specification (chapter 2.13 Default
Address Map) and arch/arm/include/asm/arch-kirkwood/cpu.h suggest that
the Security SRAM is mapped to 0xC801. Given the size of 2 KiB, the
upper end would be 0xC8010800.

So I am wondering whether the value 0xC8012000 works at all.

In addition, "CONFIG_SYS_INIT_SP_ADDR should point to RAM with enough
space for global data above and enough stack space below", as Reinhard
Meyer points out in:

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/87490

Since you use quite a large print buffer (CONFIG_SYS_PBSIZE > 1 KiB), I
assume something like 0xC8010700 might work.

@Heiko: include/configs/km_arm.h may have the same problem.

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH V4 1/2] arm926ejs: fix linker file for newer ld support

2010-11-10 Thread Daniel Hobi
On 10.11.2010 13:48, Albert ARIBAUD wrote:
> Le 10/11/2010 13:31, Daniel Hobi a écrit :
>>>> But shouldn't this change be applied to all ARM linker scripts, ie
>>>> arch/arm/cpu/*/u-boot.lds?
>>>
>>> Yes, it should. :)
>>
>> Can you please provide such a patch?
> 
> I could, but I tend to provide patches only for boards that I can test, 
> which basically covers only arm926ejs, or that I can get tested; I'd 
> prefer not to provide patches for HW that I cannot test, and thus I 
> would prefer that patches for other cpus be handled by people who 
> actually own boards with these cpus and can test their patching. After 
> all, this very bugfix is due to ELF relocations having been tested with 
> too poor a range of toolchains.

I prefer a single patch to solve one problem in all places. And since
you probably have most experience with the new ARM relocation, that
patch should come from you.

The ARM architecture and board maintainers will test your patch during
the current release cycle.

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] tools/env: cleanup host build flags

2010-11-10 Thread Daniel Hobi
This patch makes tools/env/Makefile more similar to tools/imls:
- define HOSTSRCS and HOSTCPPFLAGS, so that .depend generation works.
- include U-Boot headers using -idirafter to prevent picking up
  u-boot/include/errno.h.
- use HOSTCFLAGS_NOPED (fw_env.c does not conform to -pedantic).

In order to cross-compile tools/env, override the HOSTCC variable
as in this example:

  make tools env HOSTCC=bfin-uclinux-gcc

Signed-off-by: Daniel Hobi 
---
 tools/env/Makefile |   15 ++-
 1 files changed, 10 insertions(+), 5 deletions(-)

Let's fix these issues now and cleanup the CC vs HOSTCC discrepancy
later.

diff --git a/tools/env/Makefile b/tools/env/Makefile
index f893040..04dfe9c 100644
--- a/tools/env/Makefile
+++ b/tools/env/Makefile
@@ -23,19 +23,24 @@
 
 include $(TOPDIR)/config.mk
 
-SRCS   := $(obj)crc32.c  fw_env.c  fw_env_main.c
+HOSTSRCS := $(obj)crc32.c  fw_env.c  fw_env_main.c
 HEADERS:= fw_env.h
 
-HOSTCFLAGS += -Wall -DUSE_HOSTCC -I$(SRCTREE)/include
+# Compile for a hosted environment on the target
+HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
+-idirafter $(OBJTREE)/include2 \
+-idirafter $(OBJTREE)/include \
+-DUSE_HOSTCC
 
 ifeq ($(MTD_VERSION),old)
-HOSTCFLAGS += -DMTD_OLD
+HOSTCPPFLAGS += -DMTD_OLD
 endif
 
 all:   $(obj)fw_printenv
 
-$(obj)fw_printenv: $(SRCS) $(HEADERS)
-   $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $(SRCS)
+# Some files complain if compiled with -pedantic, use HOSTCFLAGS_NOPED
+$(obj)fw_printenv: $(HOSTSRCS) $(HEADERS)
+   $(HOSTCC) $(HOSTCFLAGS_NOPED) $(HOSTLDFLAGS) -o $@ $(HOSTSRCS)
 
 clean:
rm -f $(obj)fw_printenv $(obj)crc32.c
-- 
1.7.3.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [WIP] tools/env: cleanup host build flags

2010-11-10 Thread Daniel Hobi
Hi Mike,

On 10.11.2010 10:23, Mike Frysinger wrote:
> On Monday, October 11, 2010 12:06:46 Daniel Hobi wrote:
>> - use the cross compiler again (fw_printenv is intended for a
>>   hosted environment on the target).
> 
> the cross-compiler used to create u-boot has no guarantee that it'll produce 
> executables useful for the target OS.  often this isnt the case.  HOSTCC 
> however will produce useful userspace applications for whatever host the user 
> has selection.

But in many cases the default for CC is sufficient to build such
executables, whereas the default for HOSTCC is almost never (except when
HOST==TARGET).

Usually HOSTCC refers to the system where the toolchain is running. So
maybe we should introduce TARGETCC to build executables running on the
system the toolchain is generating executables for. TARGETCC would
default to CC and could be overriden in the way you demonstrated:

> this works perfectly fine for me:
>   make tools env HOSTCC=bfin-uclinux-gcc

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH V4 1/2] arm926ejs: fix linker file for newer ld support

2010-11-10 Thread Daniel Hobi
Hi Albert,

On 09.11.2010 19:47, Albert ARIBAUD wrote:
> Le 09/11/2010 19:24, Daniel Hobi a écrit :
>> Thank you. This patch is required to get Kirkwood-based boards working
>> again when using the CodeSourcery 2009q3 toolchain.
> 
> (can't find the 2010q3 Lite toolchain on CodeSourcery's site, latest is 
> 2010q1 apparently... Can you tell me which gcc and which ld version is 
> used in 2010q3?)

Andreas is correct: I'm still using Sourcery G++ Lite 2009q3-67.

> I think this V4 of my patchset could be now committed to 
> u-boot-arm/master, and if possible even on u-boot-arm for the december 
> release of u-boot, as it is a bugfix.

Since all ARM boards are broken when using a recent toolchain, the patch
should go in as fast as possible.

>> But shouldn't this change be applied to all ARM linker scripts, ie
>> arch/arm/cpu/*/u-boot.lds?
> 
> Yes, it should. :)

Can you please provide such a patch?

>> And on many ARM platforms (including Kirkwood), the timer implementation
>> is still accessing BSS variables before relocation.
>>
>> Is someone working on this? Candidates are:
>>
>> $ git grep "static ulong timestamp"
>> arch/arm/cpu/arm1136/mx31/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm1136/omap24xx/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm1176/tnetv107x/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm720t/interrupts.c:static ulong timestamp;
>> arch/arm/cpu/arm920t/a320/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm920t/at91rm9200/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm920t/s3c24x0/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/davinci/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/kirkwood/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/mx25/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/mx27/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/omap/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/orion5x/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/spear/timer.c:static ulong timestamp;
>> arch/arm/cpu/arm926ejs/versatile/timer.c:static ulong timestamp;
>> arch/arm/cpu/armv7/mx5/timer.c:static ulong timestamp;
>> arch/arm/cpu/armv7/omap-common/timer.c:static ulong timestamp;
>> arch/arm/cpu/lh7a40x/timer.c:static ulong timestamp;
>> arch/arm/cpu/s3c44b0/timer.c:static ulong timestamp;
> 
> Normally, the board maintainers should handle this during the window 
> after next version... Dunno if that is practical, but OTOH it would 
> easily show which boards are still maintained and which are not. :)

Since accessing BSS variables before relocation is a severe bug, we
should handle this now for all SoCs. But right now, there are two
approaches:

 - Perform initialization of static variables after relocation, and
   thus forbid using functions which access such variables before
   relocation (reset_timer*, get_timer*, set_timer).

   You patched arch/arm/cpu/arm926ejs/orion5x/timer.c to use this
   approach.

 - Move static variables to struct global_data, so they can be used
   before relocation. Used by AT91 timers and proposed for A320 and
   S3C64xx in:

   http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88095
   http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88160

I hope we can solve this problem in the same way for all ARM timers. And
if we use the second approach, we probably can generalize the AT91 data
in struct global_data as proposed by Andreas.

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH V4 1/2] arm926ejs: fix linker file for newer ld support

2010-11-09 Thread Daniel Hobi
Hi Albert,

On 04.11.2010 23:22, Albert Aribaud wrote:
> older ld emitted all ELF relocations in input sections named
> .rel.dyn, whereas newer ld uses names of the form .rel*. The
> linker script only collected .rel.dyn input sections. Rewrite
> to collect all .rel* input sections and overlay with .bss.

Tested-by: Daniel Hobi 

Thank you. This patch is required to get Kirkwood-based boards working
again when using the CodeSourcery 2009q3 toolchain.

But shouldn't this change be applied to all ARM linker scripts, ie
arch/arm/cpu/*/u-boot.lds?

And on many ARM platforms (including Kirkwood), the timer implementation
is still accessing BSS variables before relocation.

Is someone working on this? Candidates are:

$ git grep "static ulong timestamp"
arch/arm/cpu/arm1136/mx31/timer.c:static ulong timestamp;
arch/arm/cpu/arm1136/omap24xx/timer.c:static ulong timestamp;
arch/arm/cpu/arm1176/tnetv107x/timer.c:static ulong timestamp;
arch/arm/cpu/arm720t/interrupts.c:static ulong timestamp;
arch/arm/cpu/arm920t/a320/timer.c:static ulong timestamp;
arch/arm/cpu/arm920t/at91rm9200/timer.c:static ulong timestamp;
arch/arm/cpu/arm920t/s3c24x0/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/davinci/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/kirkwood/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/mx25/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/mx27/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/omap/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/orion5x/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/spear/timer.c:static ulong timestamp;
arch/arm/cpu/arm926ejs/versatile/timer.c:static ulong timestamp;
arch/arm/cpu/armv7/mx5/timer.c:static ulong timestamp;
arch/arm/cpu/armv7/omap-common/timer.c:static ulong timestamp;
arch/arm/cpu/lh7a40x/timer.c:static ulong timestamp;
arch/arm/cpu/s3c44b0/timer.c:static ulong timestamp;

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 4/4] tools/env: use host build flags

2010-11-09 Thread Daniel Hobi
Hi Steve,

On 09.11.2010 01:01, Steve Sakoman wrote:
> On Mon, Nov 8, 2010 at 3:15 PM, Wolfgang Denk  wrote:
>> In message  you wrote:
>>> It just occurred to me that this will no longer cross compile the fw_env
>>> tool - effectively rendering the tool useless for me.
>>
>> Argh... Indeed, that's broken.
> 
> I ran into this issue this week too.
> 
> Reverting is one option. I also tried Daniel Hobi's patch "tools/env:
> cleanup host build flags" and that worked for me.

You are refering to this WIP patch:

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/86236

It makes the fw_env tool Makefile similar to the imls Makefile and
solves the errno inclusion problem reported by Detlev.

I still think we need a third set of CC, CFLAGS, SRCS, etc defines for
compiling tools running on the target. However, we probably can default
to the combination CC+HOSTCFLAGS for most platforms.

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [WIP] tools/env: cleanup host build flags

2010-10-11 Thread Daniel Hobi
This patch makes tools/env/Makefile more similar to tools/imls:
- define HOSTSRCS and HOSTCPPFLAGS, so that .depend generation works.
- include U-Boot headers using -idirafter to prevent picking up
  u-boot/include/errno.h.
- use HOSTCFLAGS_NOPED (fw_env.c does not conform to -pedantic).
- use the cross compiler again (fw_printenv is intended for a
  hosted environment on the target).

Signed-off-by: Daniel Hobi 
Cc: Mike Frysinger 
Cc: Wolfgang Denk 

---
 tools/env/Makefile |   15 ++-
 1 files changed, 10 insertions(+), 5 deletions(-)


Hi Scott,

In commit d984fed0 (makefiles: fixes for building build tools),
you suggest that using $(CC) with host flags (HOSTCFLAGS, etc)
is the correct way to use the cross compiler to generate binaries
for a hosted environment on the target.

On the other hand, you use $(HOSTCC) to generate the .depend file
in rules.mk which leads to wrong dependencies.

I think we need to differentiate three cases:
 - (free-standing) U-Boot: use CC and CFLAGS
 - native tools (mkimage, etc): use HOSTCC and HOSTCFLAGS
 - Linux environment on the target (imls, fw_printenv):

   Can we use CC and HOSTCFLAGS, or do we need a third set of
   variables for flags?

   If reusing HOSTCFLAGS: how do we fix dependency generation?

Best regards,
Daniel

diff --git a/tools/env/Makefile b/tools/env/Makefile
index f893040..a7bed87 100644
--- a/tools/env/Makefile
+++ b/tools/env/Makefile
@@ -23,19 +23,24 @@
 
 include $(TOPDIR)/config.mk
 
-SRCS   := $(obj)crc32.c  fw_env.c  fw_env_main.c
+HOSTSRCS := $(obj)crc32.c  fw_env.c  fw_env_main.c
 HEADERS:= fw_env.h
 
-HOSTCFLAGS += -Wall -DUSE_HOSTCC -I$(SRCTREE)/include
+# Compile for a hosted environment on the target
+HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
+-idirafter $(OBJTREE)/include2 \
+-idirafter $(OBJTREE)/include \
+-DUSE_HOSTCC
 
 ifeq ($(MTD_VERSION),old)
-HOSTCFLAGS += -DMTD_OLD
+HOSTCPPFLAGS += -DMTD_OLD
 endif
 
 all:   $(obj)fw_printenv
 
-$(obj)fw_printenv: $(SRCS) $(HEADERS)
-   $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $(SRCS)
+# Some files complain if compiled with -pedantic, use HOSTCFLAGS_NOPED
+$(obj)fw_printenv: $(HOSTSRCS) $(HEADERS)
+   $(CC) $(HOSTCFLAGS_NOPED) $(HOSTLDFLAGS) -o $@ $(HOSTSRCS)
 
 clean:
rm -f $(obj)fw_printenv $(obj)crc32.c
-- 
1.7.2.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] tools/imls: fix comment in Makefile

2010-10-11 Thread Daniel Hobi
Commit d984fed0 (makefiles: fixes for building build tools)
changed the variable name FIT_CFLAGS to HOSTCFLAGS_NOPED
but forgot to update to corresponding comment.

Signed-off-by: Daniel Hobi 
Cc: Scott Wood 
Cc: Wolfgang Denk 
---
 tools/imls/Makefile |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/imls/Makefile b/tools/imls/Makefile
index 8407277..0caa397 100644
--- a/tools/imls/Makefile
+++ b/tools/imls/Makefile
@@ -67,7 +67,7 @@ $(obj)imls:   $(obj)imls.o $(obj)crc32.o $(obj)image.o 
$(obj)md5.o \
$(CC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
$(STRIP) $@
 
-# Some files complain if compiled with -pedantic, use FIT_CFLAGS
+# Some files complain if compiled with -pedantic, use HOSTCFLAGS_NOPED
 $(obj)image.o: $(SRCTREE)/common/image.c
$(CC) -g $(HOSTCFLAGS_NOPED) -c -o $@ $<
 
-- 
1.7.2.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 2/2 v2] tools/env: fail on invalid options

2010-09-16 Thread Daniel Hobi

Signed-off-by: Daniel Hobi 
---
v2:
 - print a hint to --help before returning

 tools/env/fw_env_main.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

Hi Wolfgang,

On 15.09.2010 21:18, Wolfgang Denk wrote:
> In message <1284572787-9842-2-git-send-email-daniel.h...@schmid-telecom.ch> 
> you wrote:
>> +default: /* '?' */
>> +return EXIT_FAILURE;
> 
> This should print an error message before returning; for example, run
> usage() as in the 'h' case - just with different return code.

getopt_long(3) already prints a suitable error message (also see [PATCH 1/2]):

$ fw_printenv -a
fw_printenv: invalid option -- 'a'

v2 of this patch additionally prints a hint to the user, in the same way
as some Linux core utilities (Debian coreutils: cat, date and 94 others).

I prefer this solution to calling usage() which would hide the real error
message by printing 20 additional lines.

Best regards,
Daniel

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index baf3a4d..c654057 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -105,6 +105,10 @@ main(int argc, char *argv[])
case 'h':
usage();
return EXIT_SUCCESS;
+   default: /* '?' */
+   fprintf(stderr, "Try `%s --help' for more information."
+   "\n", cmdname);
+   return EXIT_FAILURE;
}
}
 
-- 
1.7.2.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 1/2] tools/env: allow option "-n" for fw_printenv

2010-09-15 Thread Daniel Hobi
In commit bd7b26f8 (Tools: set multiple variable with fw_setenv utility),
the option parsing was changed to getopt_long(3), but option "-n"
of fw_printenv was not included.

This leads to an error message "invalid option -- 'n'" on stderr,
although the output on stdout is correct.

Signed-off-by: Daniel Hobi 
---
 tools/env/fw_env_main.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 82116b4..baf3a4d 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -59,7 +59,7 @@ void usage(void)
 
fprintf(stderr, "fw_printenv/fw_setenv, "
"a command line interface to U-Boot environment\n\n"
-   "usage:\tfw_printenv\n"
+   "usage:\tfw_printenv [-n] [variable name]\n"
"\tfw_setenv [variable name] [variable value]\n"
"\tfw_setenv -s [ file ]\n"
"\tfw_setenv -s - < [ file ]\n\n"
@@ -93,9 +93,12 @@ main(int argc, char *argv[])
cmdname = p + 1;
}
 
-   while ((c = getopt_long (argc, argv, "s:h",
+   while ((c = getopt_long (argc, argv, "ns:h",
long_options, NULL)) != EOF) {
switch (c) {
+   case 'n':
+   /* handled in fw_printenv */
+   break;
case 's':
script_file = optarg;
break;
-- 
1.7.2.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 2/2] tools/env: fail on invalid options

2010-09-15 Thread Daniel Hobi

Signed-off-by: Daniel Hobi 
---
 tools/env/fw_env_main.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index baf3a4d..381ed14 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -105,6 +105,8 @@ main(int argc, char *argv[])
case 'h':
usage();
return EXIT_SUCCESS;
+   default: /* '?' */
+   return EXIT_FAILURE;
}
}
 
-- 
1.7.2.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v3] Makefile: fix parallel build

2010-01-18 Thread Daniel Hobi
During parallel build, the top Makefile spawns multiple sub-makes for
targets in cpu/$(CPU) and $(dir $(LDSCRIPT)). If the .depend files are
not present in these directories, the sub-makes may end up generating
these files simultaneously which leads to corrupted content.

A typical error message is:

.depend:39: *** multiple target patterns.  Stop.

This patch serializes the creation of .depend in cpu/$(CPU) and
$(dir $(LDSCRIPT)) by adding these directories to the depend target
in the top Makefile.

Other directories in $(LIBS) are not affected since they contain only
one Make target and thus only one sub-make is spawned per directory.

Signed-off-by: Daniel Hobi 
---
v3:
 - Add a comment why make _depend is required for these two subdirs
v2:
 - Also build target depend in $(dir $(LDSCRIPT)) (suggested by Mike)
 - Break overlong line

 Makefile |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 793fcec..1ab197b 100644
--- a/Makefile
+++ b/Makefile
@@ -398,8 +398,11 @@ updater:
 env:
$(MAKE) -C tools/env all MTD_VERSION=${MTD_VERSION} || exit 1
 
+# Explicitly make _depend in subdirs containing multiple targets to prevent
+# parallel sub-makes creating .depend files simultaneously.
 depend dep:$(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
-   for dir in $(SUBDIRS) ; do $(MAKE) -C $$dir _depend ; done
+   for dir in $(SUBDIRS) cpu/$(CPU) $(dir $(LDSCRIPT)) ; do \
+   $(MAKE) -C $$dir _depend ; done
 
 TAG_SUBDIRS = $(SUBDIRS)
 TAG_SUBDIRS += $(dir $(__LIBS))
-- 
1.6.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2] Makefile: fix parallel build

2009-12-15 Thread Daniel Hobi
During parallel build, the top Makefile spawns multiple sub-makes for
targets in cpu/$(CPU) and $(dir $(LDSCRIPT)). If the .depend files are
not present in these directories, the sub-makes may end up generating
these files simultaneously which leads to corrupted content.

A typical error message is:

.depend:39: *** multiple target patterns.  Stop.

This patch serializes the creation of .depend in cpu/$(CPU) and
$(dir $(LDSCRIPT)) by adding these directories to the depend target
in the top Makefile.

Other directories in $(LIBS) are not affected since they contain only
one Make target and thus only one sub-make is spawned per directory.

Signed-off-by: Daniel Hobi 
---
v2:
 - Break overlong line
 - Add comment about other directories to commit message
 - Also build target _depend in $(dir $(LDSCRIPT)) (as suggested by Mike)
   This requires the Makefile in $(dir $(LDSCRIPT)) to provide a
   _depend target, which is the case for all LDSCRIPT values in the
   current tree

diff --git a/Makefile b/Makefile
index 19b5ac0..e9be7a5 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,8 @@ env:
$(MAKE) -C tools/env all MTD_VERSION=${MTD_VERSION} || exit 1
 
 depend dep:$(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
-   for dir in $(SUBDIRS) ; do $(MAKE) -C $$dir _depend ; done
+   for dir in $(SUBDIRS) cpu/$(CPU) $(dir $(LDSCRIPT)) ; do \
+   $(MAKE) -C $$dir _depend ; done
 
 TAG_SUBDIRS += include
 TAG_SUBDIRS += lib_generic board/$(BOARDDIR)
-- 
1.6.5.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Makefile: fix parallel build

2009-12-15 Thread Daniel Hobi
On 11.12.2009 20:25, Mike Frysinger wrote:
> On Thursday 10 December 2009 08:41:07 Daniel Hobi wrote:
>> During parallel build, the top Makefile spawns multiple sub-makes
>> for targets in cpu/$(CPU). If cpu/$(CPU)/.depend is not present, the
>> sub-makes may end up generating this file simultaneously which leads
>> to corrupted content.
>>
>> A typical error message is:
>>
>> .depend:39: *** multiple target patterns.  Stop.
>>
>> This patch serializes the creation of cpu/$(CPU)/.depend by adding
>> cpu/$(CPU) to the depend target in the top Makefile.
> 
> seems to happen in lib_$(ARCH)/ too, but in reviewing my logs from the last 
> few months, most parallel .depend failures have indeed been in cpu/$(CPU)/.  
> maybe this is just coincidence though ... perhaps the depend target should 
> walk all subdirs instead of a just random few ones (use LIBS).

The problem with cpu/$(CPU)/ is that the top Makefile builds multiple
targets within this directory in parallel - at least start.o and
lib$(CPU).a. For all other directories, parallel build should work,
since there is only one target per directory.

Can you provide any commit ID where building lib_$(ARCH)/ failed?

Thus, adding all LIBS to the depend target should not make any
difference. In order to properly track such dependencies we should
switch to non-recursive Makefiles (as seen in Linux) since Recursive
Make [is] Considered Harmful[1].

[1] http://miller.emu.id.au/pmiller/books/rmch/

Best regards,
Daniel

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] Makefile: fix parallel build

2009-12-10 Thread Daniel Hobi
During parallel build, the top Makefile spawns multiple sub-makes
for targets in cpu/$(CPU). If cpu/$(CPU)/.depend is not present, the
sub-makes may end up generating this file simultaneously which leads
to corrupted content.

A typical error message is:

.depend:39: *** multiple target patterns.  Stop.

This patch serializes the creation of cpu/$(CPU)/.depend by adding
cpu/$(CPU) to the depend target in the top Makefile.

Signed-off-by: Daniel Hobi 
---
 Makefile |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 19b5ac0..2fd22c7 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,7 @@ env:
$(MAKE) -C tools/env all MTD_VERSION=${MTD_VERSION} || exit 1
 
 depend dep:$(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
-   for dir in $(SUBDIRS) ; do $(MAKE) -C $$dir _depend ; done
+   for dir in $(SUBDIRS) cpu/$(CPU); do $(MAKE) -C $$dir _depend ; 
done
 
 TAG_SUBDIRS += include
 TAG_SUBDIRS += lib_generic board/$(BOARDDIR)
-- 
1.6.5.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] Fix computation in nand_util.c:get_len_incl_bad

2009-12-01 Thread Daniel Hobi
Depending on offset, flash size and the number of bad blocks,
get_len_incl_bad may return a too small value which may lead to:

1) If there are no bad blocks, nand_{read,write}_skip_bad chooses the
bad block aware read/write code. This may hurt performance, but does
not have any adverse effects.

2) If there are bad blocks, the nand_{read,write}_skip_bad may choose
the bad block unaware read/write code (if len_incl_bad == *length)
which leads to corrupted data.

Signed-off-by: Daniel Hobi 
---
@Scott: please review
@Wolfgang: please consider for 2009.11

diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index bec9277..7085d42 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -452,7 +452,7 @@ static size_t get_len_incl_bad (nand_info_t *nand, loff_t 
offset,
len_incl_bad += block_len;
offset   += block_len;
 
-   if ((offset + len_incl_bad) >= nand->size)
+   if (offset >= nand->size)
break;
}
 
-- 
1.5.6.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot