Re: [PATCH 1/2] cmd: exit: Fix return value propagation out of environment scripts

[Hector Palacios]

Hi Marek

On 18/12/22 21:46, Marek Vasut wrote:

Make sure the 'exit' command as well as 'exit $val' command exits
from environment scripts immediately and propagates return value
out of those scripts fully. That means the following behavior is
expected:

"
=> setenv foo 'echo bar ; exit 1' ; run foo ; echo $?
bar
1
=> setenv foo 'echo bar ; exit 0' ; run foo ; echo $?
bar
0
=> setenv foo 'echo bar ; exit -2' ; run foo ; echo $?
bar
0
"

As well as the followin behavior:

"
=> setenv foo 'echo bar ; exit 3 ; echo fail'; run foo; echo $?
bar
3
=> setenv foo 'echo bar ; exit 1 ; echo fail'; run foo; echo $?
bar
1
=> setenv foo 'echo bar ; exit 0 ; echo fail'; run foo; echo $?
bar
0
=> setenv foo 'echo bar ; exit -1 ; echo fail'; run foo; echo $?
bar
0
=> setenv foo 'echo bar ; exit -2 ; echo fail'; run foo; echo $?
bar
0
=> setenv foo 'echo bar ; exit ; echo fail'; run foo; echo $?
bar
0
"

Fixes: 8c4e3b79bd0 ("cmd: exit: Fix return value")
Signed-off-by: Marek Vasut 
---
Cc: Adrian Vovk 
Cc: Hector Palacios 
Cc: Pantelis Antoniou 
Cc: Simon Glass 
Cc: Tom Rini 
---
  cmd/exit.c|  7 +--
  common/cli.c  |  7 ---
  common/cli_hush.c | 21 +++--
  3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/cmd/exit.c b/cmd/exit.c
index 2c7132693ad..7bf241ec732 100644
--- a/cmd/exit.c
+++ b/cmd/exit.c
@@ -10,10 +10,13 @@
  static int do_exit(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
  {
+   int r;
+
+   r = 0;
 if (argc > 1)
-   return dectoul(argv[1], NULL);
+   r = simple_strtoul(argv[1], NULL, 10);

-   return 0;
+   return -r - 2;
  }

  U_BOOT_CMD(
diff --git a/common/cli.c b/common/cli.c
index a47d6a3f2b4..ba45dad2db5 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -146,7 +146,7 @@ int run_commandf(const char *fmt, ...)
  #if defined(CONFIG_CMD_RUN)
  int do_run(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
  {
-   int i;
+   int i, ret;

 if (argc < 2)
 return CMD_RET_USAGE;
@@ -160,8 +160,9 @@ int do_run(struct cmd_tbl *cmdtp, int flag, int argc, char 
*const argv[])
 return 1;
 }

-   if (run_command(arg, flag | CMD_FLAG_ENV) != 0)
-   return 1;
+   ret = run_command(arg, flag | CMD_FLAG_ENV);
+   if (ret)
+   return ret;
 }
 return 0;
  }
diff --git a/common/cli_hush.c b/common/cli_hush.c
index 1467ff81b35..b8940b19735 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -1902,7 +1902,7 @@ static int run_list_real(struct pipe *pi)
 last_return_code = -rcode - 2;
 return -2;  /* exit */
 }
-   last_return_code=(rcode == 0) ? 0 : 1;
+   last_return_code = rcode;
  #endif
  #ifndef __U_BOOT__
 pi->num_progs = save_num_progs; /* restore number of programs 
*/
@@ -3212,7 +3212,15 @@ static int parse_stream_outer(struct in_str *inp, int 
flag)
 printf("exit not allowed from main input 
shell.\n");
 continue;
 }
-   break;
+   /*
+* DANGER
+* Return code -2 is special in this context,
+* it indicates exit from inner pipe instead
+* of return code itself, the return code is
+* stored in 'last_return_code' variable!
+* DANGER
+*/
+   return -2;
 }
 if (code == -1)
 flag_repeat = 0;
@@ -3249,9 +3257,9 @@ int parse_string_outer(const char *s, int flag)
  #endif /* __U_BOOT__ */
  {
 struct in_str input;
+   int rcode;
  #ifdef __U_BOOT__
 char *p = NULL;
-   int rcode;
 if (!s)
 return 1;
 if (!*s)
@@ -3263,11 +3271,12 @@ int parse_string_outer(const char *s, int flag)
 setup_string_in_str(, p);
 rcode = parse_stream_outer(, flag);
 free(p);
-   return rcode;
+   return rcode == -2 ? last_return_code : rcode;
 } else {
  #endif
 setup_string_in_str(, s);
-   return parse_stream_outer(, flag);
+   rcode = parse_stream_outer(, flag);
+   return rcode == -2 ? last_return_code : rcode;
  #ifdef __U_BOOT__
 }
  #endif
@@ -3287,7 +3296,7 @@ int parse_file_outer(void)
 setup_file_in_str();
  #endif
 rcode = parse_stream_outer(, FLAG_PARSE_SEMICOLON);
-   return

Re: cmd: exit: Exit functionality broken

[Hector Palacios]

Hi Max,

On 12/13/22 13:24, Max van den Biggelaar wrote:

Hi,

I have a question regarding the U-Boot exit command. We are currently using 
mainline U-Boot 2022.04 version to provide our embedded systems with a 
bootloader image. To start our firmware via U-Boot environment, we use a 
bootscript to start our firmware. However, when we tried to exit a bootscript 
with the exit command, the bootscript was never exited.

After debugging to investigate the problem, we found this commit 
(https://github.com/u-boot/u-boot/commit/8c4e3b79bd0bb76eea16869e9666e19047c0d005)
 in mainline U-Boot:
cmd: exit: Fix return value


In case exit is called in a script without parameter, the command
returns -2 ; in case exit is called with a numerical parameter,
the command returns -2 and lower. This leads to the following problem:
=> setenv foo 'echo bar ; exit 1' ; run foo ; echo $?
bar
0
=> setenv foo 'echo bar ; exit 0' ; run foo ; echo $?
bar
0
=> setenv foo 'echo bar ; exit -2' ; run foo ; echo $?
bar
0
That is, no matter what the 'exit' command argument is, the return
value is always 0 and so it is not possible to use script return
value in subsequent tests.

Fix this and simplify the exit command such that if exit is called with
no argument, the command returns 0, just like 'true' in cmd/test.c. In
case the command is called with any argument that is positive integer,
the argument is set as return value.
=> setenv foo 'echo bar ; exit 1' ; run foo ; echo $?
bar
1
=> setenv foo 'echo bar ; exit 0' ; run foo ; echo $?
bar
0
=> setenv foo 'echo bar ; exit -2' ; run foo ; echo $?
bar
0

Note that this does change ABI established in 2004 , although it is
unclear whether that ABI was originally OK or not.

Fixes: 
c26e454
Signed-off-by: Marek Vasut 
Cc: Pantelis Antoniou 
Cc: Tom Rini 

This commit does solve the problem of returning the correct value given to the 
exit command, but this breaks the following source code in common/cli_hush.c:
https://github.com/u-boot/u-boot/blob/master/common/cli_hush.c#L3207

In the previous versions of U-Boot, such as 2020.04, the exit command returned 
-2, which was expected of the exit command API. However, after the patch above 
to fix the return value, -2 was never returned and the functionality to exit a 
bootscript or mainline U-Boot shell is not supported anymore. Thus, by the 
patch above the return value is fixed, but the functionality of the exit 
command is broken.

My question is if the functionality of this patch is fully tested/qualified to 
push in mainline U-Boot source code? And if so, is the functionality of the 
exit command also going to be fixed so that in future U-Boot source code 
releases bootscripts can be exited with this command?


I believe Marek's commit must be reverted as having 'exit' return a code 
other than 0 (success) or 1 (error) was never part of U-Boot. I don't 
know if reverting may break newer scripts, but now many scripts are 
broken because 'exit' does not currently work.


Anyway, Marek wanted to give it a second thought. See 
https://www.mail-archive.com/u-boot%40lists.denx.de/msg456830.html


Regards
--
Héctor Palacios

Re: [PATCH] cli_hush: fix 'exit' cmd that was not exiting scripts

[Hector Palacios]

On 11/21/22 09:55, Hector Palacios wrote:

Hi Marek,

On 11/19/22 15:12, Marek Vasut wrote:

On 11/18/22 12:19, Hector Palacios wrote:

Commit 8c4e3b79bd0bb76eea16869e9666e19047c0d005 supposedly
passed one-level up the argument passed to 'exit' but it also
broke 'exit' purpose of stopping a script.

In reality, even if 'do_exit()' is capable of returning any
integer, the cli only admits '1' or '0' as return values.

This commit respects the current implementation to allow 'exit'
to at least return '1' for future processing, but returns
when the command being run is 'exit'.

Before this:

  => setenv foo 'echo bar ; exit 3 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit 1 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit 0 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit -1 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit -2 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit ; echo should not see this'; run 
foo; echo $?

  bar
  should not see this
  0

After this:

 => setenv foo 'echo bar ; exit 3 ; echo should not see 
this'; run foo; echo $?

 bar
 1
 => setenv foo 'echo bar ; exit 1 ; echo should not see 
this'; run foo; echo $?

 bar
 1
 => setenv foo 'echo bar ; exit 0 ; echo should not see 
this'; run foo; echo $?

 bar
 0
 => setenv foo 'echo bar ; exit -1 ; echo should not see 
this'; run foo; echo $?

 bar
 0
 => setenv foo 'echo bar ; exit -2 ; echo should not see 
this'; run foo; echo $?

 bar
 0
 => setenv foo 'echo bar ; exit ; echo should not see this'; 
run foo; echo $?

 bar
 0

Reported-by: Adrian Vovk 
Signed-off-by: Hector Palacios 
---
  common/cli_hush.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 1467ff81b35b..9fe8b87e02d7 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -1902,6 +1902,10 @@ static int run_list_real(struct pipe *pi)
  last_return_code = -rcode - 2;
  return -2;  /* exit */
  }
+ if (!strcmp(pi->progs->argv[0], "exit")) {
+ last_return_code = rcode;
+ return rcode;   /* exit */
+ }
  last_return_code=(rcode == 0) ? 0 : 1;
  #endif
  #ifndef __U_BOOT__


Looking at the code just above this change 'if (rcode < -1)
last_return_code = -rcode - 2', that explains the odd 'return -r - 2' in
cmd/exit.c I think.


That's what I thought, too. The cli captures a -2 as the number to exit 
a  script, and with -rcode -2 was exiting and returning a 0.
Instead of capturing a magic number, I'm suggesting to capture 'exit' 
command.




I wonder, can we somehow fix the return code handling in cmd/exit.c
instead, so that it would cover both this behavior listed in this patch,
and 8c4e3b79bd0 ("cmd: exit: Fix return value") ? The cmd/exit.c seems
like the right place to fix it.


I didn't revert or touched 8c4e3b79bd0 but if what you wanted to do with 
that commit is to return any positive integer to the upper layers, I 
must say that just doesn't work because the cli_hush only processes 1 
(failure) or 0 (success), so there's no way for something such as 'exit 
3' to produce a $? of 3.
I think the 'exit' command should only be used with this old U-Boot 
standard of considering 1 a failure and 0 a success.


I could remove the 'if (rcode < -1)  last_return_code = -rcode - 2', 
which doesn't add much value now, but other than that I'm unsure of what 
you have in mind as to fix cmd/exit.c.


I just saw my patch causes a data abort on if conditionals, when 
accessing argv[0].


Maybe we'd rather simply revert 8c4e3b79bd0 ("cmd: exit: Fix return 
value") and let the exit command return 0 in all cases, as it is 
documented, at least until we find a proper solution.

--
Héctor Palacios

Re: [PATCH] cli_hush: fix 'exit' cmd that was not exiting scripts

[Hector Palacios]

Hi Marek,

On 11/19/22 15:12, Marek Vasut wrote:

On 11/18/22 12:19, Hector Palacios wrote:

Commit 8c4e3b79bd0bb76eea16869e9666e19047c0d005 supposedly
passed one-level up the argument passed to 'exit' but it also
broke 'exit' purpose of stopping a script.

In reality, even if 'do_exit()' is capable of returning any
integer, the cli only admits '1' or '0' as return values.

This commit respects the current implementation to allow 'exit'
to at least return '1' for future processing, but returns
when the command being run is 'exit'.

Before this:

  => setenv foo 'echo bar ; exit 3 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit 1 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit 0 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit -1 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit -2 ; echo should not see this'; 
run foo; echo $?

  bar
  should not see this
  0
  => setenv foo 'echo bar ; exit ; echo should not see this'; run 
foo; echo $?

  bar
  should not see this
  0

After this:

 => setenv foo 'echo bar ; exit 3 ; echo should not see this'; 
run foo; echo $?

 bar
 1
 => setenv foo 'echo bar ; exit 1 ; echo should not see this'; 
run foo; echo $?

 bar
 1
 => setenv foo 'echo bar ; exit 0 ; echo should not see this'; 
run foo; echo $?

 bar
 0
 => setenv foo 'echo bar ; exit -1 ; echo should not see 
this'; run foo; echo $?

 bar
 0
 => setenv foo 'echo bar ; exit -2 ; echo should not see 
this'; run foo; echo $?

 bar
 0
 => setenv foo 'echo bar ; exit ; echo should not see this'; 
run foo; echo $?

 bar
 0

Reported-by: Adrian Vovk 
Signed-off-by: Hector Palacios 
---
  common/cli_hush.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 1467ff81b35b..9fe8b87e02d7 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -1902,6 +1902,10 @@ static int run_list_real(struct pipe *pi)
  last_return_code = -rcode - 2;
  return -2;  /* exit */
  }
+ if (!strcmp(pi->progs->argv[0], "exit")) {
+ last_return_code = rcode;
+ return rcode;   /* exit */
+ }
  last_return_code=(rcode == 0) ? 0 : 1;
  #endif
  #ifndef __U_BOOT__


Looking at the code just above this change 'if (rcode < -1)
last_return_code = -rcode - 2', that explains the odd 'return -r - 2' in
cmd/exit.c I think.


That's what I thought, too. The cli captures a -2 as the number to exit 
a  script, and with -rcode -2 was exiting and returning a 0.
Instead of capturing a magic number, I'm suggesting to capture 'exit' 
command.




I wonder, can we somehow fix the return code handling in cmd/exit.c
instead, so that it would cover both this behavior listed in this patch,
and 8c4e3b79bd0 ("cmd: exit: Fix return value") ? The cmd/exit.c seems
like the right place to fix it.


I didn't revert or touched 8c4e3b79bd0 but if what you wanted to do with 
that commit is to return any positive integer to the upper layers, I 
must say that just doesn't work because the cli_hush only processes 1 
(failure) or 0 (success), so there's no way for something such as 'exit 
3' to produce a $? of 3.
I think the 'exit' command should only be used with this old U-Boot 
standard of considering 1 a failure and 0 a success.


I could remove the 'if (rcode < -1)  last_return_code = -rcode - 2', 
which doesn't add much value now, but other than that I'm unsure of what 
you have in mind as to fix cmd/exit.c.





btw. it would be good to write a unit test for this, since it is
becoming messy.


Regards
--
Héctor Palacios

[PATCH] cli_hush: fix 'exit' cmd that was not exiting scripts

[Hector Palacios]

Commit 8c4e3b79bd0bb76eea16869e9666e19047c0d005 supposedly
passed one-level up the argument passed to 'exit' but it also
broke 'exit' purpose of stopping a script.

In reality, even if 'do_exit()' is capable of returning any
integer, the cli only admits '1' or '0' as return values.

This commit respects the current implementation to allow 'exit'
to at least return '1' for future processing, but returns
when the command being run is 'exit'.

Before this:

=> setenv foo 'echo bar ; exit 3 ; echo should not see this'; run foo; 
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit 1 ; echo should not see this'; run foo; 
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit 0 ; echo should not see this'; run foo; 
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit -1 ; echo should not see this'; run foo; 
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit -2 ; echo should not see this'; run foo; 
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit ; echo should not see this'; run foo; 
echo $?
bar
should not see this
0

After this:

=> setenv foo 'echo bar ; exit 3 ; echo should not see this'; run foo; 
echo $?
bar
1
=> setenv foo 'echo bar ; exit 1 ; echo should not see this'; run foo; 
echo $?
bar
1
=> setenv foo 'echo bar ; exit 0 ; echo should not see this'; run foo; 
echo $?
bar
0
=> setenv foo 'echo bar ; exit -1 ; echo should not see this'; run foo; 
echo $?
bar
0
=> setenv foo 'echo bar ; exit -2 ; echo should not see this'; run foo; 
echo $?
bar
0
=> setenv foo 'echo bar ; exit ; echo should not see this'; run foo; 
echo $?
bar
0

Reported-by: Adrian Vovk 
Signed-off-by: Hector Palacios 
---
 common/cli_hush.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 1467ff81b35b..9fe8b87e02d7 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -1902,6 +1902,10 @@ static int run_list_real(struct pipe *pi)
last_return_code = -rcode - 2;
return -2;  /* exit */
}
+   if (!strcmp(pi->progs->argv[0], "exit")) {
+   last_return_code = rcode;
+   return rcode;   /* exit */
+   }
last_return_code=(rcode == 0) ? 0 : 1;
 #endif
 #ifndef __U_BOOT__

[U-Boot] [PATCH] cmd: mii: don't check address for 'device' subcommand

[Hector Palacios]

All mii operations require a valid PHY address except the 'device'
command, which expects the PHY name rather than the address.

Signed-off-by: Hector Palacios 
---
 cmd/mii.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/mii.c b/cmd/mii.c
index ce7b393eeaae..c0c42a851f90 100644
--- a/cmd/mii.c
+++ b/cmd/mii.c
@@ -313,7 +313,7 @@ static int do_mii(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
mask = simple_strtoul(argv[5], NULL, 16);
}
 
-   if (addrhi > 31) {
+   if (addrhi > 31 && strncmp(op, "de", 2)) {
printf("Incorrect PHY address. Range should be 0-31\n");
return CMD_RET_USAGE;
}
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] Very slow mtest on i.MX6UL over dual-die DDR3 (two chip selects)

[Hector Palacios]

Hi Fabio,

On 05/17/2017 11:08 PM, Fabio Estevam wrote:
> Hi Hector,
> 
> On Wed, May 17, 2017 at 5:50 AM, Palacios, Hector
> <hector.palac...@digi.com> wrote:
> 
>> The code is on Github [1] (well, not the dual-die DDR3 yet) but there isn't 
>> much to see for this issue other than:
> 
> I looked at your code and I see:
> 
> int dram_init(void)
> {
> gd->ram_size = ((ulong)CONFIG_DDR_MB * SZ_1M);
> 
> return 0;
> }
> 
> which may be worth investigating.
> 
> Take a look in this same function at
> board/freescale/mx53loco/mx53loco.c and also in
> include/configs/mx53loco.h (hint: we pass #define CONFIG_NR_DRAM_BANKS
> 2).

In case I wasn't clear, I don't have any memory mapping problems. I have one 
single
DDR chip, which internally uses two dies and two chip selects. Setting
CONFIG_NR_DRAM_BANKS to 1 (and full size) or 2 (and half size) in U-Boot 
doesn't make
any difference in terms of performance.

Additional investigation showed the following:

NXP DDR stress test takes the same time to complete (successfully) in both 
variants
(single-die with one chip select and 1GB density per CS, and dual-die with two 
chip
select and 512MB per CS)

The slow memory access is global in U-Boot, not limited to 'mtest' command. A 
memory
copy command for 256M (time cp.l 9000 8000 400) takes:
- 1.304s on the single-die DDR3
- 15.866 seconds on the dual-die DDR3

Note that both data/instuction cache are ON on both devices, and that I'm only
exercising the lower memory (only CS0 on the dual-die chip) to avoid potential 
issues
or delays between changing from CS0 to CS1.

I also verified that configuring the MMDC0 for using only one CS on the 
dual-die DDR3
chip (and only half the size), does not help. This DDR is still performing 
slowly in
U-Boot, but I can't find the reason why.

Thanks for your help, anyway.
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/1] mtd: nand-base: fix bug writing 1 byte less than page size

[Hector Palacios]

On 07/18/2016 09:18 AM, Hector Palacios wrote:
> nand_do_write_ops() determines if it is writing a partial page with the
> formula:
>   if (column || (writelen < mtd->writesize - 1))
> 
> When 'writelen' is exactly 1 byte less than the NAND page size the formula
> equates to zero, so the code doesn't process it as a partial write, although
> it should.
> As a consequence the function remains in the while(1) loop with 'writelen'
> becoming 0x and iterating until the watchdog timeout triggers.
> 
> To reproduce the issue on a NAND with 2K page (0x800):
>   => nand erase.part 
>   => nand write $loadaddr  7ff
> 
> Signed-off-by: Hector Palacios <hector.palac...@digi.com>
> 
> https://jira.digi.com/browse/DUB-619
> ---
>  drivers/mtd/nand/nand_base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index c0e381ad2d15..cb15bb21e2f0 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2524,7 +2524,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, 
> loff_t to,
>  
>   WATCHDOG_RESET();
>   /* Partial page write? */
> - if (unlikely(column || writelen < (mtd->writesize - 1))) {
> + if (unlikely(column || writelen < mtd->writesize)) {
>   cached = 0;
>   bytes = min_t(int, bytes - column, (int) writelen);
>       chip->pagebuf = -1;
> 

Please disregard. This patch was generated over U-Boot v2015.04.
I sent a corrected v2 patch.

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

[U-Boot] [PATCH v2 1/1] mtd: nand: fix bug writing 1 byte less than page size

[Hector Palacios]

nand_do_write_ops() determines if it is writing a partial page with the
formula:
part_pagewr = (column || writelen < (mtd->writesize - 1))

When 'writelen' is exactly 1 byte less than the NAND page size the formula
equates to zero, so the code doesn't process it as a partial write, although
it should.
As a consequence the function remains in the while(1) loop with 'writelen'
becoming 0x and iterating until the watchdog timeout triggers.

To reproduce the issue on a NAND with 2K page (0x800):
=> nand erase.part 
=> nand write $loadaddr  7ff

Signed-off-by: Hector Palacios <hector.palac...@digi.com>
---
Changes in v2
- v1 was wronly generated over U-Boot v2015.04


 drivers/mtd/nand/nand_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 689716753ae6..8280288a5ea1 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2411,7 +2411,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t 
to,
int cached = writelen > bytes && page != blockmask;
uint8_t *wbuf = buf;
int use_bufpoi;
-   int part_pagewr = (column || writelen < (mtd->writesize - 1));
+   int part_pagewr = (column || writelen < mtd->writesize);
 
if (part_pagewr)
use_bufpoi = 1;
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/1] mtd: nand-base: fix bug writing 1 byte less than page size

[Hector Palacios]

nand_do_write_ops() determines if it is writing a partial page with the
formula:
if (column || (writelen < mtd->writesize - 1))

When 'writelen' is exactly 1 byte less than the NAND page size the formula
equates to zero, so the code doesn't process it as a partial write, although
it should.
As a consequence the function remains in the while(1) loop with 'writelen'
becoming 0x and iterating until the watchdog timeout triggers.

To reproduce the issue on a NAND with 2K page (0x800):
=> nand erase.part 
=> nand write $loadaddr  7ff

Signed-off-by: Hector Palacios <hector.palac...@digi.com>

https://jira.digi.com/browse/DUB-619
---
 drivers/mtd/nand/nand_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c0e381ad2d15..cb15bb21e2f0 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2524,7 +2524,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t 
to,
 
WATCHDOG_RESET();
/* Partial page write? */
-   if (unlikely(column || writelen < (mtd->writesize - 1))) {
+   if (unlikely(column || writelen < mtd->writesize)) {
cached = 0;
bytes = min_t(int, bytes - column, (int) writelen);
chip->pagebuf = -1;
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 1/1] mtd: nand: fix bug writing 1 byte less than page size

[Hector Palacios]

On 07/15/2016 08:49 PM, Scott Wood wrote:
> On Fri, 2016-07-15 at 09:45 +0200, Hector Palacios wrote:
>> nand_do_write_ops() determines if it is writing a partial page with the
>> formula:
>>  part_pagewr = (column || writelen < (mtd->writesize - 1))
>>
>> When 'writelen' is exactly 1 byte less than the NAND page size the formula
>> equates to zero, so the code doesn't process it as a partial write, although
>> it should.
>> As a consequence the function remains in the while(1) loop with 'writelen'
>> becoming 0x and iterating until the watchdog timeout triggers.
>>
>> To reproduce the issue on a NAND with 2K page (0x800):
>>      => nand erase.part 
>>  => nand write $loadaddr  7ff
>>
>> Signed-off-by: Hector Palacios <hector.palac...@digi.com>
>> ---
>>  drivers/mtd/nand/nand_base.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 689716753ae6..c8be74849e56 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -877,7 +877,7 @@ static int nand_wait(struct mtd_info *mtd, struct
>> nand_chip *chip)
>>  
>>  u32 timer = (CONFIG_SYS_HZ * timeo) / 1000;
>>  u32 time_start;
>> - 
>> +
>>  time_start = get_timer(0);
>>  while (get_timer(time_start) < timer) {
>>  if (chip->dev_ready) {
> 
> This change is unrelated.

Ok. Removed.

>> @@ -2411,7 +2411,7 @@ static int nand_do_write_ops(struct mtd_info *mtd,
>> loff_t to,
>>  int cached = writelen > bytes && page != blockmask;
>>  uint8_t *wbuf = buf;
>>  int use_bufpoi;
>> -int part_pagewr = (column || writelen < (mtd->writesize -
>> 1));
>> +int part_pagewr = (column || writelen < mtd->writesize);
>>  
>>  if (part_pagewr)
>>  use_bufpoi = 1;
>>
> 
> Could you send a non-RFC patch?  This also needs to get fixed in Linux.

Done. I will prepare a patch for Linux, but do you know how to reproduce it in 
Linux?
'nandwrite' checks that the file size is a multiple of the page size or else it 
will
ask you to force padding, and 'dd' over the mtdblock device is also rounding up 
the
write size to a page size multiple. Maybe that's the reason why it went 
undetected for
so long.

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

[U-Boot] [RFC PATCH 1/1] mtd: nand: fix bug writing 1 byte less than page size

[Hector Palacios]

nand_do_write_ops() determines if it is writing a partial page with the
formula:
part_pagewr = (column || writelen < (mtd->writesize - 1))

When 'writelen' is exactly 1 byte less than the NAND page size the formula
equates to zero, so the code doesn't process it as a partial write, although
it should.
As a consequence the function remains in the while(1) loop with 'writelen'
becoming 0x and iterating until the watchdog timeout triggers.

To reproduce the issue on a NAND with 2K page (0x800):
=> nand erase.part 
=> nand write $loadaddr  7ff

Signed-off-by: Hector Palacios <hector.palac...@digi.com>
---
 drivers/mtd/nand/nand_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 689716753ae6..c8be74849e56 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -877,7 +877,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip 
*chip)
 
u32 timer = (CONFIG_SYS_HZ * timeo) / 1000;
u32 time_start;
- 
+
time_start = get_timer(0);
while (get_timer(time_start) < timer) {
if (chip->dev_ready) {
@@ -2411,7 +2411,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t 
to,
int cached = writelen > bytes && page != blockmask;
uint8_t *wbuf = buf;
int use_bufpoi;
-   int part_pagewr = (column || writelen < (mtd->writesize - 1));
+   int part_pagewr = (column || writelen < mtd->writesize);
 
if (part_pagewr)
use_bufpoi = 1;
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC PATCH 0/1] bug writing to NAND 1 byte less than page size

[Hector Palacios]

Hi,

I found this bug on v2015.04 but I guess it happens also in v2016.07
as I believe it has been there since 2007.
It is so strange that nobody caught this before that I fear I'm doing
something wrong or not taking something into account on the suggested
fix, so I'd like to know if others can reproduce it and if the fix looks
correct.

I reproduced it on an i.MX6UL platform and on an i.MX28 platform but
it is a bug on the NAND core driver, so it should be reproducible with
any architecture. In fact, it is reproducible with any size, as long
as it is a multiple of the page size, less one byte.

Thanks in advance for your comments.

Hector Palacios (1):
  mtd: nand: fix bug writing 1 byte less than page size

 drivers/mtd/nand/nand_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

[U-Boot] [PATCH v2] bootm: fixup silent Linux out of BOOTM_STATE_LOADOS state

[Hector Palacios]

The function fixup_silent_linux() is called in status BOOTM_STATE_LOADOS
to silence Linux if variable 'silent' is set.
Currently only the 'bootm' command state machine contains
BOOTM_STATE_LOADOS, but others like 'booti' or 'bootz' commands do not.
This means silent Linux does not work with these commands.

This patch moves the fixup_silent_linux() call out of the
BOOTM_STATE_LOADOS state and into BOOTM_STATE_OS_PREP, to silence Linux
independently of the used command (booti, bootm or bootz).

Signed-off-by: Hector Palacios <hector.palac...@digi.com>
Reviewed-by: Simon Glass <s...@chromium.org>
---
Changes in v2:
 - Removed empty line.
---
 common/bootm.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index 2431019b3f40..f7f909f86aec 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -635,10 +635,6 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[],
goto err;
else if (ret == BOOTM_ERR_OVERLAP)
ret = 0;
-#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
-   if (images->os.os == IH_OS_LINUX)
-   fixup_silent_linux();
-#endif
}
 
/* Relocate the ramdisk */
@@ -683,8 +679,13 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[],
ret = boot_fn(BOOTM_STATE_OS_CMDLINE, argc, argv, images);
if (!ret && (states & BOOTM_STATE_OS_BD_T))
ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images);
-   if (!ret && (states & BOOTM_STATE_OS_PREP))
+   if (!ret && (states & BOOTM_STATE_OS_PREP)) {
+#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
+   if (images->os.os == IH_OS_LINUX)
+   fixup_silent_linux();
+#endif
ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images);
+   }
 
 #ifdef CONFIG_TRACE
/* Pretend to run the OS, then run a user command */
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] bootm: fixup silent Linux out of BOOTM_STATE_LOADOS state

[Hector Palacios]

The function fixup_silent_linux() is called in status BOOTM_STATE_LOADOS
to silence Linux if variable 'silent' is set.
Currently only the 'bootm' command state machine contains
BOOTM_STATE_LOADOS, but others like 'booti' or 'bootz' commands do not.
This means silent Linux does not work with these commands.

This patch moves the fixup_silent_linux() call out of the
BOOTM_STATE_LOADOS state and into BOOTM_STATE_OS_PREP, to silence Linux
independently of the used command (booti, bootm or bootz).

Signed-off-by: Hector Palacios <hector.palac...@digi.com>
---
 common/bootm.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index 81a522eec348..521468c1ecd0 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -668,10 +668,6 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[],
goto err;
else if (ret == BOOTM_ERR_OVERLAP)
ret = 0;
-#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
-   if (images->os.os == IH_OS_LINUX)
-   fixup_silent_linux();
-#endif
}
 
/* Relocate the ramdisk */
@@ -711,13 +707,19 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[],
return 1;
}
 
+
/* Call various other states that are not generally used */
if (!ret && (states & BOOTM_STATE_OS_CMDLINE))
ret = boot_fn(BOOTM_STATE_OS_CMDLINE, argc, argv, images);
if (!ret && (states & BOOTM_STATE_OS_BD_T))
ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images);
-   if (!ret && (states & BOOTM_STATE_OS_PREP))
+   if (!ret && (states & BOOTM_STATE_OS_PREP)) {
+#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
+   if (images->os.os == IH_OS_LINUX)
+   fixup_silent_linux();
+#endif
ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images);
+   }
 
 #ifdef CONFIG_TRACE
/* Pretend to run the OS, then run a user command */
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] mx6: support i.MX6UL speed grading reading from OTP bits

[Hector Palacios]

i.MX6UL defines speed grading in OCOTP register 0x440[17:16]
as follows:
00  reserved
01  528MHz
10  700MHz
11  reserved

This commit removes the constants (which had the speed hardcoded
in their names) and uses values instead.

Signed-off-by: Hector Palacios <hector.palac...@digi.com>
---
 arch/arm/cpu/armv7/mx6/soc.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
index d4b22ad7f315..49b5773b1b77 100644
--- a/arch/arm/cpu/armv7/mx6/soc.c
+++ b/arch/arm/cpu/armv7/mx6/soc.c
@@ -103,10 +103,6 @@ u32 get_cpu_rev(void)
  * defines a 2-bit SPEED_GRADING
  */
 #define OCOTP_CFG3_SPEED_SHIFT 16
-#define OCOTP_CFG3_SPEED_800MHZ0
-#define OCOTP_CFG3_SPEED_850MHZ1
-#define OCOTP_CFG3_SPEED_1GHZ  2
-#define OCOTP_CFG3_SPEED_1P2GHZ3
 
 u32 get_cpu_speed_grade_hz(void)
 {
@@ -122,18 +118,23 @@ u32 get_cpu_speed_grade_hz(void)
 
switch (val) {
/* Valid for IMX6DQ */
-   case OCOTP_CFG3_SPEED_1P2GHZ:
+   case 3:
if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
return 12;
-   /* Valid for IMX6SX/IMX6SDL/IMX6DQ */
-   case OCOTP_CFG3_SPEED_1GHZ:
-   return 99600;
-   /* Valid for IMX6DQ */
-   case OCOTP_CFG3_SPEED_850MHZ:
+   /* Valid for IMX6SX/IMX6SDL/IMX6DQ/IMX6UL */
+   case 2:
+   if (is_cpu_type(MXC_CPU_MX6UL))
+   return 7;
+   else
+   return 99600;
+   /* Valid for IMX6DQ/IMX6UL */
+   case 1:
if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
return 85200;
+   else if (is_cpu_type(MXC_CPU_MX6UL))
+   return 52800;
/* Valid for IMX6SX/IMX6SDL/IMX6DQ */
-   case OCOTP_CFG3_SPEED_800MHZ:
+   case 0:
return 79200;
}
return 0;
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH V2] ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

[Hector Palacios]

On 12/04/2015 08:32 PM, Eric Nelson wrote:
> The low four bits of the SYSCTL register are reserved on the USDHC
> controller on i.MX6 and i.MX7 processors, but are used for clocking
> operations on earlier models.
> 
> Guard against their usage by hiding the bit mask macros on those
> processors.
> 
> These bits are used to prevent glitches when changing clocks on
> i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
> 
> From the i.MX6DQ RM:
>   To prevent possible glitch on the card clock, clear the
>   FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS
>   or DVS in System Control Register) or setting RSTA bit.
> 
> Signed-off-by: Eric Nelson <e...@nelint.com>

Reviewed-by: Hector Palacios <hector.palac...@digi.com>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mmc: update MMC_ERASE argument to match Linux kernel.

[Hector Palacios]

Hi Eric and Fabio,

On 12/07/2015 03:50 PM, Eric Nelson wrote:
> Table 41 of the JEDEC standard for eMMC says that bit 31 of
> the command argument is obsolete when issuing the ERASE
> command (CMD38) on page 115 of this document:
>   http://www.jedec.org/sites/default/files/docs/jesd84-B45.pdf
> 
> The SD Card Association Physical Layer Simplified Specification also
> makes no mention of the use of bit 31.
>   https://www.sdcard.org/downloads/pls/part1_410.pdf
> 
> The Linux kernel distinguishes between secure (bit 31 set) and
> non-secure erase, and this patch copies the macro names from
> include/linux/mmc/core.h.
> 
> Tested-by: Fabio Estevam <fabio.este...@freescale.com>
> Signed-off-by: Eric Nelson <e...@nelint.com>
> ---
>  drivers/mmc/mmc_write.c | 2 +-
>  include/mmc.h   | 7 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> index 7aea7e9..221bf30 100644
> --- a/drivers/mmc/mmc_write.c
> +++ b/drivers/mmc/mmc_write.c
> @@ -51,7 +51,7 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, 
> lbaint_t blkcnt)
>   goto err_out;
>  
>   cmd.cmdidx = MMC_CMD_ERASE;
> - cmd.cmdarg = SECURE_ERASE;
> + cmd.cmdarg = MMC_ERASE_ARG;
>   cmd.resp_type = MMC_RSP_R1b;
>  
>   err = mmc_send_cmd(mmc, , NULL);
> diff --git a/include/mmc.h b/include/mmc.h
> index cda9a19..b89962a 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -121,7 +121,12 @@
>  #define OCR_VOLTAGE_MASK 0x007FFF80
>  #define OCR_ACCESS_MODE  0x6000
>  
> -#define SECURE_ERASE 0x8000
> +#define MMC_ERASE_ARG0x
> +#define MMC_SECURE_ERASE_ARG 0x8000
> +#define MMC_TRIM_ARG 0x0001
> +#define MMC_DISCARD_ARG  0x0003
> +#define MMC_SECURE_TRIM1_ARG 0x8001
> +#define MMC_SECURE_TRIM2_ARG 0x80008000
>  
>  #define MMC_STATUS_MASK  (~0x0206BF7F)
>  #define MMC_STATUS_SWITCH_ERROR  (1 << 7)
> 

This fixes the issue on eMMC. Very good job! Thank you.

Tested-by: Hector Palacios <hector.palac...@digi.com>

On the uSD card, it occasionally works (when it didn't before), but it still 
fails
many times, after erasing a random number of blocks (at random times).
I guess this must be a different issue, though.
I could reproduce it on my board (4 data lines) and on the SabreSD (8 data 
lines) with
v2015.04. Here is an output from the SabreSD:

=> time mmc erase 20 100

MMC erase: dev # 1, block # 2097152, count 256 ... 256 blocks erased: OK

time: 1.866 seconds
=> time mmc erase 20 1000

MMC erase: dev # 1, block # 2097152, count 4096 ... Timeout waiting for DAT0 to 
go high!
mmc erase failed
1409 blocks erased: ERROR

time: 11.263 seconds
=> time mmc erase 20 1

MMC erase: dev # 1, block # 2097152, count 65536 ... Timeout waiting for DAT0 
to go high!
mmc erase failed
8192 blocks erased: ERROR

time: 59.139 seconds


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

Re: [U-Boot] [PATCH] ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

[Hector Palacios]

Hi Eric,

On 12/04/2015 06:43 PM, Eric Nelson wrote:
> On 12/04/2015 10:38 AM, Eric Nelson wrote:
>> On 12/04/2015 10:32 AM, Eric Nelson wrote:
>>> The low four bits of the SYSCTL register are reserved on the USDHC
>>> controller on i.MX6 and i.MX7 processors, but are used for clocking
>>> operations on earlier models.
>>>
>>> Guard against their usage by hiding the bit mask macros on those
>>> processors.
>>>
>>> These bits are used to prevent glitches when changing clocks on
>>> i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
>>>
>>> From the i.MX6DQ RM:
>>> To prevent possible glitch on the card clock, clear the
>>> FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS
>>> or DVS in System Control Register) or setting RSTA bit.
>>>
>>> Signed-off-by: Eric Nelson <e...@nelint.com>
>>
>> I forgot to add an in-reply-to header.
>>
>> http://lists.denx.de/pipermail/u-boot/2015-December/thread.html#236651
>>
>>
> 
> Fabio, I haven't been able to reproduce the "mmc erase/ENGcm03648"
> issue (with or without a code change) for a couple of hours now.
> 
> Can you give this a spin?
> 
> It seems unlikely to address the issue unless what we're seeing is a
> side effect of a glitch while switching clocks.

As Fabio, I can reproduce this 100% of the times.
The patch does not fix it, though.

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

Re: [U-Boot] [PATCH] ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

[Hector Palacios]

Hi Eric,

On 12/04/2015 07:33 PM, Eric Nelson wrote:
> Hi Fabio and Hector,
> 
> On 12/04/2015 10:43 AM, Eric Nelson wrote:
>> On 12/04/2015 10:38 AM, Eric Nelson wrote:
>>> On 12/04/2015 10:32 AM, Eric Nelson wrote:
 The low four bits of the SYSCTL register are reserved on the USDHC
 controller on i.MX6 and i.MX7 processors, but are used for clocking
 operations on earlier models.

 Guard against their usage by hiding the bit mask macros on those
 processors.

 These bits are used to prevent glitches when changing clocks on
 i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

 From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the
FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS
or DVS in System Control Register) or setting RSTA bit.

 Signed-off-by: Eric Nelson 
>>>
>>> I forgot to add an in-reply-to header.
>>>
>>> http://lists.denx.de/pipermail/u-boot/2015-December/thread.html#236651
>>>
>>>
>>
>> Fabio, I haven't been able to reproduce the "mmc erase/ENGcm03648"
>> issue (with or without a code change) for a couple of hours now.
>>
>> Can you give this a spin?
>>
>> It seems unlikely to address the issue unless what we're seeing is a
>> side effect of a glitch while switching clocks.
>>
>>
> 
> I switched back to a v2014.10 release and am able to reproduce the
> issue at will.
> 
> The sysctl patch had no effect, but adding an #ifndef around the
> ENGcm03648 block allows things to proceed.
> 
> +#ifndef CONFIG_FSL_USDHC
> /* Workaround for ESDHC errata ENGcm03648 */
> if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
> ...
> 
> Since the need to poll the DAT0 line is only documented in the
> errata for the i.MX35, I wonder if this isn't the right thing to do.
> 
> The CC bit of irqstat does indicate that the command completed
> and without error (I'm seeing values of 1 in irqstat).
> 
> From what I can tell, the linux kernel doesn't do this test and
> doesn't appear to have any trouble.
> 
> What code base are you running against (u-boot-imx/master)?

I'm running v2015.04 on a non-upstream platform.

> What do you see if you do the same?

The command takes a while and it is erasing all blocks.
I still get a timeout error at the end and a zero number of sectors, though.

=> mmc erase 441000 1

MMC erase: dev # 0, block # 4460544, count 65536 ... Timeout waiting card ready
0 blocks erased: ERROR


but all blocks were erased correctly.

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

Re: [U-Boot] [PATCH] ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

[Hector Palacios]

Hi,

On 12/04/2015 06:32 PM, Eric Nelson wrote:
> The low four bits of the SYSCTL register are reserved on the USDHC
> controller on i.MX6 and i.MX7 processors, but are used for clocking
> operations on earlier models.
> 
> Guard against their usage by hiding the bit mask macros on those
> processors.
> 
> These bits are used to prevent glitches when changing clocks on
> i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
> 
> From the i.MX6DQ RM:
>   To prevent possible glitch on the card clock, clear the
>   FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS
>   or DVS in System Control Register) or setting RSTA bit.
> 
> Signed-off-by: Eric Nelson <e...@nelint.com>
> ---
>  drivers/mmc/fsl_esdhc.c | 15 +--
>  include/fsl_esdhc.h |  2 ++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index c5054d6..1ccc576 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
>  
>   clk = (pre_div << 8) | (div << 4);
>  
> +#ifdef CONFIG_FSL_USDHC
> + esdhc_setbits32(>sysctl, SYSCTL_RSTA);
> +#else
>   esdhc_clrbits32(>sysctl, SYSCTL_CKEN);
> +#endif
>  
>   esdhc_clrsetbits32(>sysctl, SYSCTL_CLOCK_MASK, clk);
>  
>   udelay(1);
>  
> - clk = SYSCTL_PEREN | SYSCTL_CKEN;
> +#ifdef CONFIG_FSL_USDHC
> + esdhc_clrbits32(>sysctl, SYSCTL_RSTA);
> +#else
> + esdhc_setbits32(>sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
> +#endif
>  
> - esdhc_setbits32(>sysctl, clk);
>  }
>  
>  #ifdef CONFIG_FSL_ESDHC_USE_PERIPHERAL_CLK
> @@ -585,7 +592,9 @@ static int esdhc_init(struct mmc *mmc)
>   esdhc_write32(>scr, 0x0040);
>  #endif
>  
> +#ifndef CONFIG_FSL_USDHC
>   esdhc_setbits32(>sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
> +#endif
>  
>   /* Set the initial clock speed */
>   mmc_set_clock(mmc, 40);
> @@ -657,8 +666,10 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg 
> *cfg)
>   /* First reset the eSDHC controller */
>   esdhc_reset(regs);
>  
> +#ifndef CONFIG_FSL_USDHC
>   esdhc_setbits32(>sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
>   | SYSCTL_IPGEN | SYSCTL_CKEN);
> +#endif
>  
>   writel(SDHCI_IRQ_EN_BITS, >irqstaten);
>   memset(>cfg, 0, sizeof(cfg->cfg));
> diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
> index aa1b4cf..a4b87ce 100644
> --- a/include/fsl_esdhc.h
> +++ b/include/fsl_esdhc.h
> @@ -25,10 +25,12 @@
>  #define SYSCTL_INITA 0x0800
>  #define SYSCTL_TIMEOUT_MASK  0x000f
>  #define SYSCTL_CLOCK_MASK0xfff0
> +#if !defined(CONFIG_MX6)

Per your commit message should this be
#if (!defined(CONFIG_MX6) && !defined(CONFIG_MX7))

>  #define SYSCTL_CKEN  0x0008
>  #define SYSCTL_PEREN 0x0004
>  #define SYSCTL_HCKEN 0x0002
>  #define SYSCTL_IPGEN 0x0001
> +#endif
>  #define SYSCTL_RSTA  0x0100
>  #define SYSCTL_RSTC  0x0200
>  #define SYSCTL_RSTD  0x0400
> 


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

Re: [U-Boot] mmc erase fails from U-Boot command line

[Hector Palacios]

Dear Cliff,

On 10/16/2015 01:46 PM, Cliff Brust wrote:
> I have the need to erase our eMMC from U-Boot on our custom board due to a 
> hard wired
> boot up configuration. Our design is based on the Freescale i.MX6Q SabreSD 
> Board
> reference design. The bottom line is the U-Boot command "mmc erase" is 
> failing with
> the error "Timeout waiting for DAT0 to go high!".  Here's a list of the U-Boot
> commands issued and the result of each so you can see what is going on.
> 
> => mmc list
> FSL_SDHC: 0
> FSL_SDHC: 1 (SD)
> FSL_SDHC: 2 (eMMC)
> 
> => mmc dev 2
> switch to partitions #0, OK
> mmc2(part 0) is current device
> 
> => mmc info
> Device: FSL_SDHC
> Manufacturer ID: 45
> OEM: 100
> Name: SEM08
> Tran Speed: 5200
> Rd Block Len: 512
> MMC version 4.4.1
> High Capacity: Yes
> Capacity: 7.4 GiB
> Bus Width: 8-bit
> Erase Group Size: 512 KiB
> HC WP Group Size: 16 MiB
> User Capacity: 7.4 GiB WRREL
> Boot Capacity: 2 MiB ENH
> RPMB Capacity: 128 KiB ENH
> 
> => mmc erase 0 0x400
> MMC erase: dev # 2, block # 0, count 1024 ...
> Timeout waiting for DAT0 to go high!
> mmc erase failed
> 0 blocks erased: ERROR
> 
> 
> Any insight on this issue is greatly appreciated.
> Thanks,  Cliff

This issue is reproducible on Freescale's SABRESD on both SD card and eMMC with
v2015.04. The issue has been there always, I believe.
Apparently the command erases the first block, but the operation returns an 
error, so
it aborts and it doesn't continue erasing futher blocks.

I opened a similar thread a while ago:
http://lists.denx.de/pipermail/u-boot/2015-June/215912.html

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

Re: [U-Boot] [PATCH v2] ubi: enable error reporting in initialization

[Hector Palacios]

Hi Andrew,

On 11/05/2014 08:31 PM, Andrew Ruder wrote:
 The UBI layer will disable much of its error reporting when it is
 compiled into the linux kernel to avoid stopping boot.  We want this
 error reporting in U-Boot since we don't initialize the UBI layer until
 it is used and want the error reporting.
 
 We force this by telling the UBI layer we are building as a module.
 
 Signed-off-by: Andrew Ruder andrew.ru...@elecsyscorp.com
 Cc: Wolfgang Denk w...@denx.de
 Cc: Heiko Schocher h...@denx.de
 Cc: Kyungmin Park kmp...@infradead.org
 ---
  include/ubi_uboot.h | 8 
  1 file changed, 8 insertions(+)
 
 diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
 index 1fd15f4..324fe72 100644
 --- a/include/ubi_uboot.h
 +++ b/include/ubi_uboot.h
 @@ -51,6 +51,14 @@
  
  #undef CONFIG_MTD_UBI_BLOCK
  
 +/* ubi_init() disables returning error codes when built into the Linux
 + * kernel so that it doesn't hang the Linux kernel boot process.  Since
 + * the U-Boot driver code depends on getting valid error codes from this
 + * function we just tell the UBI layer that we are building as a module
 + * (which only enables the additional error reporting).
 + */
 +#define CONFIG_MTD_UBI_MODULE
 +
  #if !defined(CONFIG_MTD_UBI_BEB_LIMIT)
  #define CONFIG_MTD_UBI_BEB_LIMIT 20
  #endif
 

I applied this patch but apparently I'm suffering a memory leak in a certain
condition. I wonder if you can reproduce it:

1. Assume you have an empty partition called MyPart.
2. Write it with some dummy data:
= mw.l $loadaddr deadbeed 1000
= nand write $loadaddr MyPart 1000
3. Set it as the UBI part:
= ubi part MyPart
This (which returned 0 before, despite failing during the attach procedure now 
should
return an error).
4. Run the command again several times:
= ubi part MyPart
= ubi part MyPart

In my case, after calling this three times, the target hangs. I think the exit 
path in
ubi_init() does not properly free every allocated memory. I was not able to 
find the
root cause, though.

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

Re: [U-Boot] [PATCH v2] ubi: enable error reporting in initialization

[Hector Palacios]

Hi Andrew,

On 11/05/2014 08:31 PM, Andrew Ruder wrote:
 The UBI layer will disable much of its error reporting when it is
 compiled into the linux kernel to avoid stopping boot.  We want this
 error reporting in U-Boot since we don't initialize the UBI layer until
 it is used and want the error reporting.
 
 We force this by telling the UBI layer we are building as a module.
 
 Signed-off-by: Andrew Ruder andrew.ru...@elecsyscorp.com
 Cc: Wolfgang Denk w...@denx.de
 Cc: Heiko Schocher h...@denx.de
 Cc: Kyungmin Park kmp...@infradead.org
 ---
  include/ubi_uboot.h | 8 
  1 file changed, 8 insertions(+)
 
 diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
 index 1fd15f4..324fe72 100644
 --- a/include/ubi_uboot.h
 +++ b/include/ubi_uboot.h
 @@ -51,6 +51,14 @@
  
  #undef CONFIG_MTD_UBI_BLOCK
  
 +/* ubi_init() disables returning error codes when built into the Linux
 + * kernel so that it doesn't hang the Linux kernel boot process.  Since
 + * the U-Boot driver code depends on getting valid error codes from this
 + * function we just tell the UBI layer that we are building as a module
 + * (which only enables the additional error reporting).
 + */
 +#define CONFIG_MTD_UBI_MODULE
 +
  #if !defined(CONFIG_MTD_UBI_BEB_LIMIT)
  #define CONFIG_MTD_UBI_BEB_LIMIT 20
  #endif
 

I applied this patch but apparently I'm suffering a memory leak in a certain
condition. I wonder if you can reproduce it:

1. Assume you have an empty partition called MyPart.
2. Write it with some dummy data:
= mw.l $loadaddr deadbeed 1000
= nand write $loadaddr MyPart 1000
3. Set it as the UBI part:
= ubi part MyPart
This (which returned 0 before, despite failing during the attach procedure now 
should
return an error).
4. Run the command again several times:
= ubi part MyPart
= ubi part MyPart

In my case, after calling this three times, the target hangs. I think the exit 
path in
ubi_init() does not properly free every allocated memory. I was not able to 
find the
root cause, though.

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

[U-Boot] [PATCH 3/3] mxs_ocotp: clear the error flag before initiating write operation

[Hector Palacios]

A previous operation may have set the error flag, which must be cleared
before a new write operation can be issued.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 drivers/misc/mxs_ocotp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c
index 1659ee6a5eec..6f0a1d3e6da8 100644
--- a/drivers/misc/mxs_ocotp.c
+++ b/drivers/misc/mxs_ocotp.c
@@ -187,6 +187,8 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t 
mask)
uint32_t hclk_val, vddio_val;
int ret;
 
+   mxs_ocotp_clear_error();
+
/* Make sure the banks are closed for reading. */
ret = mxs_ocotp_read_bank_open(0);
if (ret) {
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/3] mxs_ocotp: check for errors from the OTP controller after writing

[Hector Palacios]

The write operation may fail when trying to write to a locked area. In
this case the ERROR bit is set in the CTRL register. Check for that
condition and return an error.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 drivers/misc/mxs_ocotp.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c
index 09002814f2f0..1659ee6a5eec 100644
--- a/drivers/misc/mxs_ocotp.c
+++ b/drivers/misc/mxs_ocotp.c
@@ -221,6 +221,13 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t 
mask)
goto fail;
}
 
+   /* Check for errors */
+   if (readl(ocotp_regs-hw_ocotp_ctrl)  OCOTP_CTRL_ERROR) {
+   puts(Failed writing fuses!\n);
+   ret = -EPERM;
+   goto fail;
+   }
+
 fail:
mxs_ocotp_scale_vddio(0, vddio_val);
if (mxs_ocotp_scale_hclk(0, hclk_val))
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/3] mxs_ocotp: prevent error path from returning success

[Hector Palacios]

The code may goto 'fail' upon error with 'ret' variable set to an error
code, but this variable was being overwritten by a final preparation
function to restore the HCLK, so success was (in general) returned even
after an error was hit previously.

With this change, the function may now return success even if the final
preparation function fails, but it's probably enough to print a message
because (if successful) the real programming of the fuses has already
completed.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 drivers/misc/mxs_ocotp.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c
index 545d3ebf520e..09002814f2f0 100644
--- a/drivers/misc/mxs_ocotp.c
+++ b/drivers/misc/mxs_ocotp.c
@@ -223,11 +223,8 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t 
mask)
 
 fail:
mxs_ocotp_scale_vddio(0, vddio_val);
-   ret = mxs_ocotp_scale_hclk(0, hclk_val);
-   if (ret) {
+   if (mxs_ocotp_scale_hclk(0, hclk_val))
puts(Failed scaling up the HCLK!\n);
-   return ret;
-   }
 
return ret;
 }
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] mxs_ocotp: check for errors from the OTP controller after writing

[Hector Palacios]

Hi Fabio,

On 11/21/2014 06:10 PM, Fabio Estevam wrote:
 Hi Hector,
 
 On Fri, Nov 21, 2014 at 2:54 PM, Hector Palacios
 hector.palac...@digi.com wrote:
 The write operation may fail when trying to write to a locked area. In
 this case the ERROR bit is set in the CTRL register. Check for that
 condition and return an error.

 Signed-off-by: Hector Palacios hector.palac...@digi.com
 ---
  drivers/misc/mxs_ocotp.c | 7 +++
  1 file changed, 7 insertions(+)

 diff --git a/drivers/misc/mxs_ocotp.c b/drivers/misc/mxs_ocotp.c
 index 09002814f2f0..1659ee6a5eec 100644
 --- a/drivers/misc/mxs_ocotp.c
 +++ b/drivers/misc/mxs_ocotp.c
 @@ -221,6 +221,13 @@ static int mxs_ocotp_write_fuse(uint32_t addr, uint32_t 
 mask)
 goto fail;
 }

 +   /* Check for errors */
 +   if (readl(ocotp_regs-hw_ocotp_ctrl)  OCOTP_CTRL_ERROR) {
 +   puts(Failed writing fuses!\n);
 +   ret = -EPERM;
 +   goto fail;
 +   }
 +
  fail:
 
 What about doing this instead?
 
/* Check for errors */
ret = readl(ocotp_regs-hw_ocotp_ctrl)  OCOTP_CTRL_ERROR);
if (ret) {
printfs(Failed writing the fuses: %d\n, ret);
goto fail;
}
 

Although the code looks nicer you are not returning a meaningful error code, 
just a
mask value (and yes, I know the error code does not get anywhere, but still).
--
Hector Palacios

PS. Sorry about the resending of the message but the mailing list server kept
rejecting it.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2] cmd_fuse: return CMD_RET_FAILURE on error

[Hector Palacios]

Fuse drivers, like the mxs_ocotp.c, may return negative error codes but
the commands are only allowed to return CMD_RET_* enum values to the
shell, otherwise the following error appears:

exit not allowed from main input shell.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---

Changes for v2:
Reword commit message.

 common/cmd_fuse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/cmd_fuse.c b/common/cmd_fuse.c
index abab9789b0df..d4bc0f6c94a1 100644
--- a/common/cmd_fuse.c
+++ b/common/cmd_fuse.c
@@ -128,7 +128,7 @@ static int do_fuse(cmd_tbl_t *cmdtp, int flag, int argc, 
char *const argv[])
 
 err:
puts(ERROR\n);
-   return ret;
+   return CMD_RET_FAILURE;
 }
 
 U_BOOT_CMD(
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] cmd_fuse: return CMD_RET_FAILURE on error

[Hector Palacios]

Fuse drivers, like the mxs_ocotp.c, may return negative error codes but
the commands are not allowed to return negative error codes to the shell,
otherwise the following error appears:

exit not allowed from main input shell.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 common/cmd_fuse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/cmd_fuse.c b/common/cmd_fuse.c
index abab9789b0df..d4bc0f6c94a1 100644
--- a/common/cmd_fuse.c
+++ b/common/cmd_fuse.c
@@ -128,7 +128,7 @@ static int do_fuse(cmd_tbl_t *cmdtp, int flag, int argc, 
char *const argv[])
 
 err:
puts(ERROR\n);
-   return ret;
+   return CMD_RET_FAILURE;
 }
 
 U_BOOT_CMD(
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Protected variable 'ethaddr' can be reset to default value (or deleted)

[Hector Palacios]

Hello,

I have enabled environment flags and have protected ethaddr variable in write-once 
mode: ethaddr:mo.


As expected, once set, I cannot overwrite this variable with standard setenv 
command.
Also as expected, I can overwrite it at any time by passing the -f (forced) option to 
setenv:

setenv -f ethaddr XX:XX:XX:XX:XX:XX

The 'env default' command is supposed to reset the environment to the default values. 
Its help is:


env default [-f] -a - [forcibly] reset default environment
env default [-f] var [...] - [forcibly] reset variable(s) to their default 
values

So I expect that 'env default ethaddr' does not change the value of 'ethaddr' (which 
actually occurs) because the variable is protected, but that adding the -f option does 
change the value to the default (which doesn't occur). In other words:


  env default ethaddr   Does not change 'ethaddr' (OK)
  env default -f ethaddrDoes not change 'ethaddr' (Problem #1)

Similarly, I was also expecting that 'env default -a', which resets the whole 
environment to its default values, does not modify protected 'ethaddr' (like it 
happens when you specify the variable) and that 'env default -f -a' resets it. 
However, both commands do reset the protected variables. In other words:


  env default -aChanges/deletes 'ethaddr' (Problem #2)
  env default -f -a Changes/deletes 'ethaddr' (OK)

Actually the '-f' option, despite being in the help text is not taken into 
consideration at the 'default' subcommand. This can easily be checked in function 
do_env_default() in common/cmd_nvedit.c, where the local variable 'flag' is not 
propagated anywhere in the function.


I can easily fix Problem #1 by propagating local variable 'flag' in do_env_default() 
to set_default_vars().


Problem #2 is more difficult because U-Boot is creating a new env table from scratch, 
disregarding previous existing entries, and calling env_flags_validate() in 
'env_op_create' mode, which only checks for ENV_FLAGS_VARACCESS_PREVENT_CREATE permission.

Any ideas on how to properly fix this?

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCHv5 09/14] mmc: support the DDR mode for eMMC

[Hector Palacios]

On 05/21/2014 04:20 AM, Jaehoon Chung wrote:

Hi, Hector.

On 05/21/2014 01:37 AM, Hector Palacios wrote:

Hi,

On 05/16/2014 06:59 AM, Jaehoon Chung wrote:

Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
Tested-by: Lukasz Majewski l.majew...@samsung.com
Acked-by: Lukasz Majewski l.majew...@samsung.com


What platforms did you test DDR mode on?


I have tested DDR mode with exynos board.


I tried this on an Freescale i.MX6 based platform but the driver returned error 
code COM_ERR (-18) when trying to switch to any of the DDR modes. I guess the 
fsl_esdhc.c driver needs some adaptation for DDR to work.


I didn't know how work DDR mode at fsl_esdhc.c.(If you share the host 
controller TRM, it's helpful to me.)
Host controller also need to change the DDR mode.

I wonder your board didn't work not to enable the DDR mode?


Thank you, yes the fsl_esdhc.c driver does not yet support enabling of DDR mode.
I think it should be easy to implement, I believe it is just a question of changing 
the clock frequency.


Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCHv5 09/14] mmc: support the DDR mode for eMMC

[Hector Palacios]

Hi,

On 05/16/2014 06:59 AM, Jaehoon Chung wrote:

Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
Tested-by: Lukasz Majewski l.majew...@samsung.com
Acked-by: Lukasz Majewski l.majew...@samsung.com


What platforms did you test DDR mode on?
I tried this on an Freescale i.MX6 based platform but the driver returned error code 
COM_ERR (-18) when trying to switch to any of the DDR modes. I guess the fsl_esdhc.c 
driver needs some adaptation for DDR to work.


Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fs: fat: Fix cache align error message in fatwrite to use USB media

[Hector Palacios]

Hello,

On 04/03/2014 06:44 AM, Nobuhiro Iwamatsu wrote:

Use of malloc of do_fat_write() from USB media causes cache error on


I would remove 'from USB media' from the commit log as it doesn't really matter where 
you read from.



ARM v7 platforms. Perhaps, the same problem will occur at any other CPUs.
This replaces malloc with memalign to fix cache buffer alignment.

Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu...@renesas.com
Signed-off-by: Yoshiyuki Ito yoshiyuki.ito...@renesas.com
---
  fs/fat/fat_write.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 9f5e911..cef138e 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -952,7 +952,7 @@ static int do_fat_write(const char *filename, void *buffer,
}

mydata-fatbufnum = -1;
-   mydata-fatbuf = malloc(FATBUFSIZE);
+   mydata-fatbuf = memalign(ARCH_DMA_MINALIGN, FATBUFSIZE);
if (mydata-fatbuf == NULL) {
debug(Error: allocating memory\n);
return -1;



Tested on an i.MX6 custom platform.

Tested-by: Hector Palacios hector.palac...@digi.com

Thanks,
--
Héctor Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation

[Hector Palacios]

Hi,

On 03/28/2014 03:36 PM, Eric Nelson wrote:

Hi Hector,

On 03/28/2014 06:49 AM, Fabio Estevam wrote:

On Fri, Mar 28, 2014 at 7:15 AM, Hector Palacios
hector.palac...@digi.com wrote:

Cache was invalidated on the read operation, but it should
also be flushed otherwise.

Signed-off-by: Hector Palacios hector.palac...@digi.com


After further testing it looks like I misinterpreted the results:
First, please disregard the patch as it does not fix anything.
Second, 'mmc part' command seems to be returning cached data after I use 'gpt' command 
to partition the uSD card. I can reproduce it as follows (consider mmc 1 is my uSD card):


1. Write random data to corrupt the partition table
= mmc dev 1
= mmc write $loadaddr 0 30
2. Check partition table is corrupt
= mmc part  (shows error invalid GPT)
3. Soft reset the target
4. Write a correct partition table
= mmc dev 1
= gpt write mmc 1 ...
5. Read back partition table
= mmc part

At this point 'mmc part' returns again an incorrect partition table. However, if after 
a while I do an 'mmc rescan' or a soft reset and rerun the 'mmc part' command, it will 
show the correct partition table was written.


The partition table is read during mmc_init():

int test_part_efi(block_dev_desc_t * dev_desc)
{
ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, legacymbr, 1, dev_desc-blksz);

/* Read legacy MBR from block 0 and validate it */
if ((dev_desc-block_read(dev_desc-dev, 0, 1, (ulong *)legacymbr) != 1)
|| (is_pmbr_valid(legacymbr) != 1)) {
return -1;
}
return 0;
}

Could it be that the read partition table is cached so that after writing it with 
'gpt', reading it again returns cached data instead of physical data, just written?


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

[U-Boot] [PATCH RFC] fsl_esdhc: flush cache after non-read operation

[Hector Palacios]

Cache was invalidated on the read operation, but it should
also be flushed otherwise.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---

Notes:
After enabling L2 cache on i.MX6 I found out that many times
when running the 'gpt' command to partition a uSD card, the
data was not written at all, or was badly written to the media.

This patch seems to solve it but I'm not sure if that's the
right place to flush the cache. Could someone please comment?

Thank you.

 drivers/mmc/fsl_esdhc.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index e945c0a470ca..5ef575eb0272 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -253,6 +253,16 @@ static int esdhc_setup_data(struct mmc *mmc, struct 
mmc_data *data)
return 0;
 }
 
+static void check_and_flush_dcache_range
+   (struct mmc_cmd *cmd,
+struct mmc_data *data) {
+   unsigned start = (unsigned)data-dest ;
+   unsigned size = roundup(ARCH_DMA_MINALIGN,
+   data-blocks*data-blocksize);
+   unsigned end = start+size ;
+   flush_dcache_range(start, end);
+}
+
 static void check_and_invalidate_dcache_range
(struct mmc_cmd *cmd,
 struct mmc_data *data) {
@@ -401,6 +411,8 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct 
mmc_data *data)
 #endif
if (data-flags  MMC_DATA_READ)
check_and_invalidate_dcache_range(cmd, data);
+   else
+   check_and_flush_dcache_range(cmd, data);
}
 
esdhc_write32(regs-irqstat, -1);
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH V2] disk:efi: avoid unaligned access on efi partition

[Hector Palacios]

On 01/28/2014 01:46 PM, Piotr Wilczek wrote:

This patch fixes part_efi code 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
CC: Albert ARIBAUD albert.u.b...@aribaud.net
---
Chnages for V2:
  - used put_unaligned to copy value;
  - use __aligned to align local array;

  disk/part_efi.c |7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 9c33ae7..eb2cd57 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -225,7 +225,7 @@ 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;
+   put_unaligned(dev_desc-lba, p_mbr-partition_record[0].nr_sects);

/* Write MBR sector to the MMC device */
if (dev_desc-block_write(dev_desc-dev, 0, 1, p_mbr) != 1) {
@@ -361,6 +361,8 @@ int gpt_fill_pte(gpt_header *gpt_h, gpt_entry *gpt_e,
  #ifdef CONFIG_PARTITION_UUIDS
char *str_uuid;
  #endif
+   static efi_guid_t basic_guid __aligned(0x04) =
+   PARTITION_BASIC_DATA_GUID;

for (i = 0; i  parts; i++) {
/* partition starting lba */
@@ -388,8 +390,7 @@ int gpt_fill_pte(gpt_header *gpt_h, gpt_entry *gpt_e,
gpt_e[i].ending_lba = cpu_to_le64(offset - 1);

/* partition type GUID */
-   memcpy(gpt_e[i].partition_type_guid.b,
-   PARTITION_BASIC_DATA_GUID, 16);
+   gpt_e[i].partition_type_guid = basic_guid;

  #ifdef CONFIG_PARTITION_UUIDS
str_uuid = partitions[i].uuid;



Sorry, I sent my tested-by on a different thread:
http://patchwork.ozlabs.org/patch/319649/

Tested on i.MX6 (armv7) custom board and it is working fine without
the -mno-unaligned-access flag.

Tested-by: Hector Palacios hector.palac...@digi.com

Replying to Tom's question in that other thread, regarding the issue:

 Can you give me some steps on how to hit this bug?  I believe it's a bug
 and I believe we need to fix it, I just want to investigate a few things
 while we've got a trigger case right now.  Thanks!

GPT partition table needs to write a protective MBR to sector 0.
The MBR structure has four partition entries (each occupying 16 bytes) at unaligned 
offsets +1BEh, +1CEh, +1DEh, +1EEh (see [1]). The structure of each partition entry is 
defined at include/part_efi.h


struct partition {
u8 boot_ind;/* 0x80 - active */
u8 head;/* starting head */
u8 sector;  /* starting sector */
u8 cyl; /* starting cylinder */
u8 sys_ind; /* What partition type */
u8 end_head;/* end head */
u8 end_sector;  /* end sector */
u8 end_cyl; /* end cylinder */
__le32 start_sect;  /* starting sector counting from 0 */
__le32 nr_sects;/* nr of sectors in partition */
} __packed;

showing eight 1-byte fields and two 4-byte fields. Since the offsets for each 
partition entry are unaligned, the last two fields (which are 32bit) are unaligned as 
well. But it's not an error, it's just the specification of the MBR requires these 
fields to be at those exact offsets. So the usage of unaligned macros for accessing 
those fields is justified.


[1] http://en.wikipedia.org/wiki/Master_boot_record#Sector_layout

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation

[Hector Palacios]

The calloc() call was allocating space for the sizeof the struct
pointer rather than for the struct contents.
Besides, since this buffer is passed to mmc for writing and some
platforms may use cache, the legacy_mbr struct should be cache-aligned.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---

Notes:
Changes since V1:
- Cache-align declaration of p_mbr pointer
- Use *p_mbr in sizeof() to match kernel CodingStyle

 disk/part_efi.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 5dfaf490c89a..42936e04fb67 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -229,10 +229,10 @@ int test_part_efi(block_dev_desc_t * dev_desc)
  */
 static int set_protective_mbr(block_dev_desc_t *dev_desc)
 {
-   legacy_mbr *p_mbr;
-
/* Setup the Protective MBR */
-   p_mbr = calloc(1, sizeof(p_mbr));
+   ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, p_mbr, 1);
+   memset(p_mbr, 0, sizeof(*p_mbr));
+
if (p_mbr == NULL) {
printf(%s: calloc failed!\n, __func__);
return -1;
@@ -247,11 +247,9 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc)
if (dev_desc-block_write(dev_desc-dev, 0, 1, p_mbr) != 1) {
printf(** Can't write to device %d **\n,
dev_desc-dev);
-   free(p_mbr);
return -1;
}
 
-   free(p_mbr);
return 0;
 }
 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] part_efi: fix protective_mbr struct allocation

[Hector Palacios]

The calloc() call was allocating space for the sizeof the struct
pointer rather than for the struct contents.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 disk/part_efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 5dfaf490c89a..7fabec059d7a 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -232,7 +232,7 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc)
legacy_mbr *p_mbr;
 
/* Setup the Protective MBR */
-   p_mbr = calloc(1, sizeof(p_mbr));
+   p_mbr = calloc(1, sizeof(legacy_mbr));
if (p_mbr == NULL) {
printf(%s: calloc failed!\n, __func__);
return -1;
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 1/2] env_mmc: make board configurable the partition for the environment

[Hector Palacios]

This complements commit 9404a5fc7cb58 env_mmc: allow environment to be
in an eMMC partition by allowing boards to accommodate the partition
to use for the environment in different scenarios (similarly to what is
done with the mmc dev number). Depending on the detected boot media,
boards may decide to store the environment in a different partition.

The __weak function also allows to remove some ifdefs from the code.
If CONFIG_SYS_MMC_ENV_PART is not defined, partition 0 is assumed
(default value for U-Boot when a partition is not provided).

Signed-off-by: Hector Palacios hector.palac...@digi.com
Reviewed-by: Stephen Warren swar...@nvidia.com
---

Notes:
Changes since v2:
- Move CONFIG_SYS_MMC_ENV_PART to config_fallbacks
Changes since v1:
- Use default define if not set

 common/env_mmc.c   | 25 +
 include/config_fallbacks.h |  6 ++
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/common/env_mmc.c b/common/env_mmc.c
index 78c2bc7a1f08..8dbeb057a3c4 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -66,6 +66,11 @@ __weak int mmc_get_env_devno(void)
return CONFIG_SYS_MMC_ENV_DEV;
 }
 
+__weak int mmc_get_env_partno(void)
+{
+   return CONFIG_SYS_MMC_ENV_PART;
+}
+
 int env_init(void)
 {
/* use default */
@@ -77,6 +82,9 @@ int env_init(void)
 
 static int init_mmc_for_env(struct mmc *mmc)
 {
+   int mmc_env_devno = mmc_get_env_devno();
+   int mmc_env_partno = mmc_get_env_partno();
+
if (!mmc) {
puts(No MMC card found\n);
return -1;
@@ -87,30 +95,23 @@ static int init_mmc_for_env(struct mmc *mmc)
return -1;
}
 
-#ifdef CONFIG_SYS_MMC_ENV_PART
-   if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num) {
-   int mmc_env_devno = mmc_get_env_devno();
-
-   if (mmc_switch_part(mmc_env_devno,
-   CONFIG_SYS_MMC_ENV_PART)) {
+   if (mmc_env_partno != mmc-part_num) {
+   if (mmc_switch_part(mmc_env_devno, mmc_env_partno)) {
puts(MMC partition switch failed\n);
return -1;
}
}
-#endif
 
return 0;
 }
 
 static void fini_mmc_for_env(struct mmc *mmc)
 {
-#ifdef CONFIG_SYS_MMC_ENV_PART
int mmc_env_devno = mmc_get_env_devno();
+   int mmc_env_partno = mmc_get_env_partno();
 
-   if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num)
-   mmc_switch_part(mmc_env_devno,
-   mmc-part_num);
-#endif
+   if (mmc_env_partno != mmc-part_num)
+   mmc_switch_part(mmc_env_devno, mmc-part_num);
 }
 
 #ifdef CONFIG_CMD_SAVEENV
diff --git a/include/config_fallbacks.h b/include/config_fallbacks.h
index e59ee963f71f..7a6f55feb12e 100644
--- a/include/config_fallbacks.h
+++ b/include/config_fallbacks.h
@@ -53,4 +53,10 @@
 #define HAVE_BLOCK_DEVICE
 #endif
 
+#if defined(CONFIG_MMC)
+#if !defined(CONFIG_SYS_MMC_ENV_PART)
+#define CONFIG_SYS_MMC_ENV_PART 0
+#endif
+#endif
+
 #endif /* __CONFIG_FALLBACKS_H */
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 2/2] config_fallbacks: default CONFIG_SYS_MMC_ENV_DEV to 0

[Hector Palacios]

Since function mmc_get_env_devno is __weak and can be overridden by
board code, boards do not need to mandatory define
CONFIG_SYS_MMC_ENV_DEV.
If the constant is not defined, define it to 0 by default.

Signed-off-by: Hector Palacios hector.palac...@digi.com
Reviewed-by: Stephen Warren swar...@nvidia.com
---

Notes:
Changes since v2:
- Move CONFIG_SYS_MMC_ENV_DEV to config_fallbacks
Changes since v1:
- Use default define if not set

 include/config_fallbacks.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/config_fallbacks.h b/include/config_fallbacks.h
index 7a6f55feb12e..b17fb28d90b9 100644
--- a/include/config_fallbacks.h
+++ b/include/config_fallbacks.h
@@ -54,6 +54,9 @@
 #endif
 
 #if defined(CONFIG_MMC)
+#if !defined(CONFIG_SYS_MMC_ENV_DEV)
+#define CONFIG_SYS_MMC_ENV_DEV 0
+#endif
 #if !defined(CONFIG_SYS_MMC_ENV_PART)
 #define CONFIG_SYS_MMC_ENV_PART 0
 #endif
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/2] env_mmc: default to 0 if CONFIG_SYS_MMC_ENV_DEV not defined

[Hector Palacios]

Since function mmc_get_env_devno is __weak and can be overridden by
board code, boards do not need to mandatory define
CONFIG_SYS_MMC_ENV_DEV.
If the constant is not defined, define it to 0 by default.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---

Notes:
Changes since v1:
- Use default define if not set

 common/env_mmc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/common/env_mmc.c b/common/env_mmc.c
index 570caf63aeae..c9df4c49e2b8 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -61,9 +61,14 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 
*env_addr)
return 0;
 }
 
+#if !defined(CONFIG_SYS_MMC_ENV_DEV)
+#define CONFIG_SYS_MMC_ENV_DEV 0
+#endif
+
 __weak int mmc_get_env_devno(void)
 {
return CONFIG_SYS_MMC_ENV_DEV;
+}
 
 #if !defined(CONFIG_SYS_MMC_ENV_PART)
 #define CONFIG_SYS_MMC_ENV_PART 0
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/2] env_mmc: make board configurable the partition for the environment

[Hector Palacios]

This complements commit 9404a5fc7cb58 env_mmc: allow environment to be
in an eMMC partition by allowing boards to accommodate the partition
to use for the environment in different scenarios (similarly to what is
done with the mmc dev number). Depending on the detected boot media,
boards may decide to store the environment in a different partition.

The __weak function also allows to remove some ifdefs from the code.
If CONFIG_SYS_MMC_ENV_PART is not defined, partition 0 is assumed
(default value for U-Boot when a partition is not provided).

Signed-off-by: Hector Palacios hector.palac...@digi.com
---

Notes:
Changes since v1:
- Use default define if not set

 common/env_mmc.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/common/env_mmc.c b/common/env_mmc.c
index 78c2bc7a1f08..570caf63aeae 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -64,6 +64,14 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 
*env_addr)
 __weak int mmc_get_env_devno(void)
 {
return CONFIG_SYS_MMC_ENV_DEV;
+
+#if !defined(CONFIG_SYS_MMC_ENV_PART)
+#define CONFIG_SYS_MMC_ENV_PART 0
+#endif
+
+__weak int mmc_get_env_partno(void)
+{
+   return CONFIG_SYS_MMC_ENV_PART;
 }
 
 int env_init(void)
@@ -77,6 +85,9 @@ int env_init(void)
 
 static int init_mmc_for_env(struct mmc *mmc)
 {
+   int mmc_env_devno = mmc_get_env_devno();
+   int mmc_env_partno = mmc_get_env_partno();
+
if (!mmc) {
puts(No MMC card found\n);
return -1;
@@ -87,30 +98,23 @@ static int init_mmc_for_env(struct mmc *mmc)
return -1;
}
 
-#ifdef CONFIG_SYS_MMC_ENV_PART
-   if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num) {
-   int mmc_env_devno = mmc_get_env_devno();
-
-   if (mmc_switch_part(mmc_env_devno,
-   CONFIG_SYS_MMC_ENV_PART)) {
+   if (mmc_env_partno != mmc-part_num) {
+   if (mmc_switch_part(mmc_env_devno, mmc_env_partno)) {
puts(MMC partition switch failed\n);
return -1;
}
}
-#endif
 
return 0;
 }
 
 static void fini_mmc_for_env(struct mmc *mmc)
 {
-#ifdef CONFIG_SYS_MMC_ENV_PART
int mmc_env_devno = mmc_get_env_devno();
+   int mmc_env_partno = mmc_get_env_partno();
 
-   if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num)
-   mmc_switch_part(mmc_env_devno,
-   mmc-part_num);
-#endif
+   if (mmc_env_partno != mmc-part_num)
+   mmc_switch_part(mmc_env_devno, mmc-part_num);
 }
 
 #ifdef CONFIG_CMD_SAVEENV
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] env_mmc: default to 0 if CONFIG_SYS_MMC_ENV_DEV not defined

[Hector Palacios]

Hi Stephen,

On 01/15/2014 06:40 PM, Stephen Warren wrote:

On 01/15/2014 03:53 AM, Hector Palacios wrote:

Since function mmc_get_env_devno is __weak and can be overridden by
board code, boards do not necessarily need to define
CONFIG_SYS_MMC_ENV_DEV. If the constant is not defined, return 0 by
default.


Ah, I see that's a better justification for allowing the define not to
be set.



diff --git a/common/env_mmc.c b/common/env_mmc.c



  __weak int mmc_get_env_devno(void)
  {
+#ifdef CONFIG_SYS_MMC_ENV_DEV
return CONFIG_SYS_MMC_ENV_DEV;
+#endif


Same comment here; need a #else here, and #endif below.

+   return 0;
+}


Re: Hector's comments: Perhaps the following in
include/config_fallbacks.h would be appropriate?

#ifndef CONFIG_SYS_MMC_ENV_DEV
#define CONFIG_SYS_MMC_ENV_DEV 0
#endif


This define is only used in common/env_mmc.c so I think the default definition should 
stay on that file, rather than in include/config_fallbacks.h.


Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/2] env_mmc: make board configurable the partition for the environment

[Hector Palacios]

This complements commit 9404a5fc7cb58 env_mmc: allow environment to be
in an eMMC partition by allowing boards to accommodate the partition
to use for the environment in different scenarios (similarly to what is
done with the mmc dev number). Depending on the detected boot media,
boards may decide to store the environment in a different partition.

The __weak function also allows to remove some ifdefs from the code.
If CONFIG_SYS_MMC_ENV_PART is not defined, partition 0 is assumed
(default value for U-Boot when a partition is not provided).

Signed-off-by: Hector Palacios hector.palac...@digi.com
CC: Stephen Warren swar...@nvidia.com
CC: Andy Fleming aflem...@freescale.com
---
 common/env_mmc.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/common/env_mmc.c b/common/env_mmc.c
index 78c2bc7a1f08..d569b070e005 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -64,6 +64,13 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 
*env_addr)
 __weak int mmc_get_env_devno(void)
 {
return CONFIG_SYS_MMC_ENV_DEV;
+
+__weak int mmc_get_env_partno(void)
+{
+#ifdef CONFIG_SYS_MMC_ENV_PART
+   return CONFIG_SYS_MMC_ENV_PART;
+#endif
+   return 0;
 }
 
 int env_init(void)
@@ -77,6 +84,9 @@ int env_init(void)
 
 static int init_mmc_for_env(struct mmc *mmc)
 {
+   int mmc_env_devno = mmc_get_env_devno();
+   int mmc_env_partno = mmc_get_env_partno();
+
if (!mmc) {
puts(No MMC card found\n);
return -1;
@@ -87,30 +97,23 @@ static int init_mmc_for_env(struct mmc *mmc)
return -1;
}
 
-#ifdef CONFIG_SYS_MMC_ENV_PART
-   if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num) {
-   int mmc_env_devno = mmc_get_env_devno();
-
-   if (mmc_switch_part(mmc_env_devno,
-   CONFIG_SYS_MMC_ENV_PART)) {
+   if (mmc_env_partno != mmc-part_num) {
+   if (mmc_switch_part(mmc_env_devno, mmc_env_partno)) {
puts(MMC partition switch failed\n);
return -1;
}
}
-#endif
 
return 0;
 }
 
 static void fini_mmc_for_env(struct mmc *mmc)
 {
-#ifdef CONFIG_SYS_MMC_ENV_PART
int mmc_env_devno = mmc_get_env_devno();
+   int mmc_env_partno = mmc_get_env_partno();
 
-   if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num)
-   mmc_switch_part(mmc_env_devno,
-   mmc-part_num);
-#endif
+   if (mmc_env_partno != mmc-part_num)
+   mmc_switch_part(mmc_env_devno, mmc-part_num);
 }
 
 #ifdef CONFIG_CMD_SAVEENV
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/2] env_mmc: default to 0 if CONFIG_SYS_MMC_ENV_DEV not defined

[Hector Palacios]

Since function mmc_get_env_devno is __weak and can be overridden by
board code, boards do not necessarily need to define
CONFIG_SYS_MMC_ENV_DEV. If the constant is not defined, return 0 by
default.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 common/env_mmc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/common/env_mmc.c b/common/env_mmc.c
index d569b070e005..bd3bc1d50b15 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -63,7 +63,11 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 
*env_addr)
 
 __weak int mmc_get_env_devno(void)
 {
+#ifdef CONFIG_SYS_MMC_ENV_DEV
return CONFIG_SYS_MMC_ENV_DEV;
+#endif
+   return 0;
+}
 
 __weak int mmc_get_env_partno(void)
 {
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] i.MX6DQ/DLS: rename and fix pad MX6_PAD_RGMII_TX_CTL__ENET_REF_CLK

[Hector Palacios]

Fix incorrect name for iomux 0x7 of MX6_PAD_RGMII_TX_CTL which really
has function ENET_REF_CLK and not ANATOP_REF_OUT.
On imx6q also fix select input value to 1.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 arch/arm/include/asm/arch-mx6/mx6dl_pins.h | 2 +-
 arch/arm/include/asm/arch-mx6/mx6q_pins.h  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h 
b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
index 3dd3bfe36a4d..1880c0957ae1 100644
--- a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
+++ b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
@@ -1430,7 +1430,7 @@ enum {
MX6_PAD_RGMII_TX_CTL__RGMII_TX_CTL = 
IOMUX_PAD(0x06BC, 0x02D4, 1, 0x, 0, 0),
MX6_PAD_RGMII_TX_CTL__GPIO_6_26= 
IOMUX_PAD(0x06BC, 0x02D4, 5, 0x, 0, 0),
MX6_PAD_RGMII_TX_CTL__MIPI_CORE_DPHY_TEST_IN_7 = 
IOMUX_PAD(0x06BC, 0x02D4, 6, 0x, 0, 0),
-   MX6_PAD_RGMII_TX_CTL__ENET_ANATOP_ETHERNET_REF_OUT = 
IOMUX_PAD(0x06BC, 0x02D4, 7, 0x080C, 1, 0),
+   MX6_PAD_RGMII_TX_CTL__ENET_REF_CLK = 
IOMUX_PAD(0x06BC, 0x02D4, 7, 0x080C, 1, 0),
MX6_PAD_RGMII_TXC__USBOH3_H2_DATA  = 
IOMUX_PAD(0x06C0, 0x02D8, 16, 0x, 0, 0),
MX6_PAD_RGMII_TXC__ENET_RGMII_TXC  = 
IOMUX_PAD(0x06C0, 0x02D8, 1, 0x, 0, 0),
MX6_PAD_RGMII_TXC__SPDIF_SPDIF_EXTCLK  = 
IOMUX_PAD(0x06C0, 0x02D8, 2, 0x08F4, 1, 0),
diff --git a/arch/arm/include/asm/arch-mx6/mx6q_pins.h 
b/arch/arm/include/asm/arch-mx6/mx6q_pins.h
index 02a40d4f5c6c..9714dc69fe88 100644
--- a/arch/arm/include/asm/arch-mx6/mx6q_pins.h
+++ b/arch/arm/include/asm/arch-mx6/mx6q_pins.h
@@ -84,7 +84,7 @@ enum {
MX6_PAD_RGMII_TX_CTL__RGMII_TX_CTL  = IOMUX_PAD(0x0388, 0x0074, 1, 
0x, 0, 0),
MX6_PAD_RGMII_TX_CTL__GPIO_6_26 = IOMUX_PAD(0x0388, 0x0074, 5, 0x, 
0, 0),
MX6_PAD_RGMII_TX_CTL__CORE_DPHY_IN_7= IOMUX_PAD(0x0388, 0x0074, 6, 
0x, 0, 0),
-   MX6_PAD_RGMII_TX_CTL__ANATOP_REF_OUT= IOMUX_PAD(0x0388, 0x0074, 7, 
0x083C, 0, 0),
+   MX6_PAD_RGMII_TX_CTL__ENET_REF_CLK  = IOMUX_PAD(0x0388, 0x0074, 7, 
0x083C, 1, 0),
MX6_PAD_RGMII_RD1__MIPI_HSI_CTRL_TX_FL = IOMUX_PAD(0x038C, 0x0078, 0, 
0x, 0, 0),
MX6_PAD_RGMII_RD1__ENET_RGMII_RD1   = IOMUX_PAD(0x038C, 0x0078, 1, 
0x084C, 0, 0),
MX6_PAD_RGMII_RD1__GPIO_6_27= IOMUX_PAD(0x038C, 0x0078, 5, 
0x, 0, 0),
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] env_mmc: make board configurable the partition for the environment

[Hector Palacios]

Hello Otavio,

On 01/15/2014 12:09 PM, Otavio Salvador wrote:

Hello Hector,

On Wed, Jan 15, 2014 at 8:53 AM, Hector Palacios hector.palac...@digi.com
mailto:hector.palac...@digi.com wrote:

This complements commit 9404a5fc7cb58 env_mmc: allow environment to be
in an eMMC partition by allowing boards to accommodate the partition
to use for the environment in different scenarios (similarly to what is
done with the mmc dev number). Depending on the detected boot media,
boards may decide to store the environment in a different partition.

The __weak function also allows to remove some ifdefs from the code.
If CONFIG_SYS_MMC_ENV_PART is not defined, partition 0 is assumed
(default value for U-Boot when a partition is not provided).

Signed-off-by: Hector Palacios hector.palac...@digi.com
mailto:hector.palac...@digi.com
CC: Stephen Warren swar...@nvidia.com mailto:swar...@nvidia.com
CC: Andy Fleming aflem...@freescale.com mailto:aflem...@freescale.com
---
  common/env_mmc.c | 27 +++
  1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/common/env_mmc.c b/common/env_mmc.c
index 78c2bc7a1f08..d569b070e005 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -64,6 +64,13 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, 
u32
*env_addr)
  __weak int mmc_get_env_devno(void)
  {
 return CONFIG_SYS_MMC_ENV_DEV;
+
+__weak int mmc_get_env_partno(void)
+{
+#ifdef CONFIG_SYS_MMC_ENV_PART
+   return CONFIG_SYS_MMC_ENV_PART;
+#endif
+   return 0;


Maybe:

#ifndef CONFIG_SYS_MMC_ENV_PART
#define CONFIG_SYS_MMC_ENV_PART 0
#endif

__weak int mmc_get_env_partno(void)
{
 return CONFIG_SYS_MMC_ENV_PART;
}


Much better. I'll do this for both patches. Thanks.

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Are mmc open/close subcommands needed?

[Hector Palacios]

Dear Otavio,

On 01/03/2014 06:35 PM, Otavio Salvador wrote:

On Thu, Jan 2, 2014 at 9:36 PM, Marek Vasut ma...@denx.de wrote:

On Thursday, January 02, 2014 at 05:53:00 PM, Hector Palacios wrote:

Hi,

I saw commit 2a91c9134675140853577b565210458b5774e6cf that introduces mmc
subcommands 'open' and 'close' to access eMMC boot partitions and was
wondering if they are really needed. Can't the same be achieved with
already existing 'mmc dev [dev] [part]' command?

mmc open dev boot_partition
   is the same as
mmc dev dev part
   where part is the boot partition

mmc close dev boot_partition
   is the same as
mmc dev dev 0
   as a 0 will switch to partition 0 (user data).

Best regards,
--
Hector Palacios


+CC Panto


No; this is for different use.

The open and close are to open the Boot partitions part of eMMC 4; by
default those eMMC will be using the user partitions, not the boot
area.

The nice, and confusing thing, is that those boot partitions also
start in address 0 as a 'virtual disk'.


I don't think they are different commands that do the same by coincidence. The 'mmc 
dev' command, when passed a fourth argument for the partition is calling the function 
'mmc_switch_part()'.
This function executes the CMD6 command to write the Extended CSD register 
PARTITION_CONFIG field that gives you access to any MMC partition (user data area, 
boot partitions 1 and 2, RPMB, General Purpose 1-4 partitions).


It doesn't look like specific eMMC commands are needed to access the boot partitions, 
but maybe I'm misinterpreting the code.


Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Are mmc open/close subcommands needed?

[Hector Palacios]

Hi,

I saw commit 2a91c9134675140853577b565210458b5774e6cf that introduces mmc subcommands 
'open' and 'close' to access eMMC boot partitions and was wondering if they are really 
needed. Can't the same be achieved with already existing 'mmc dev [dev] [part]' command?


mmc open dev boot_partition
 is the same as
mmc dev dev part
 where part is the boot partition

mmc close dev boot_partition
 is the same as
mmc dev dev 0
 as a 0 will switch to partition 0 (user data).

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] fatls shows duplicated entries with long and short names

[Hector Palacios]

Dear Jagan,

On 10/14/2013 06:57 PM, Jagan Teki wrote:

On Mon, Oct 14, 2013 at 9:37 PM, Tom Rini tr...@ti.com wrote:

On Mon, Oct 14, 2013 at 06:00:20PM +0200, Hector Palacios wrote:

Dear Marek,

I noticed that 'fatls' displays duplicated filenames (short and
long) for every file in the media:

# fatls mmc 0
   2083460   uimage-myplatform
   2083460   uimage~1
  1520   rootfs-dummy.jffs2
  1520   rootfs~1.jff
   3294952   uimage
   3294952   uimage

The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by
you, who moved the chksum calculation out of an if() and now the
code never enters this:

#ifdef CONFIG_SUPPORT_VFAT
   else if (dols == LS_ROOT  csum == prevcksum) {
   prevcksum = 0x;
   dentptr++;
   continue;
   }
#endif

Could you please check?


Can you please provide more details about your platform and what U-Boot
rev you see this on exactly?  I haven't seen anything like this on
Beaglebone Black recently, for example.



This kind of issue we faced(by Michal)
http://u-boot.10912.n7.nabble.com/FAT-problem-with-new-mkcksum-implementation-td145817.html

Where the issue got resolved with the change from Marek
vfat: Fix mkcksum argument sizes
(sha: 6ad77d88e57f6ab815ec7e85c5ac329054318c73)


I was testing on v2013.01. This patch fixes it.
Thank you.

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Hector Palacios]

Dear Marek,

On 10/13/2013 05:20 PM, Marek Vasut wrote:

Do not reconfigure the FEC clock during board_eth_init(), otherwise
the FEC might have stability issues, refuse to autonegotiate link
entirely or even corrupt packets while indicating correct checksum
on them. Instead, move the FEC clock init to board_early_init_f(),
where all the other upstream clock are initialized and also make
sure there is proper stabilization delay.


Do you have any means to reproduce the problem?
I have seldom seen times when the Ethernet did not work, but it was so infrequent that 
it was impossible to know where it came from or how to reproduce it.



[...]


diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c
index 33d38cf..5065ee8 100644
--- a/board/denx/m28evk/m28evk.c
+++ b/board/denx/m28evk/m28evk.c
@@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
   */
  int board_early_init_f(void)
  {
+   struct mxs_clkctrl_regs *clkctrl_regs =
+   (struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
+


You may want to wrap these within #ifdef CONFIG_CMD_NET to avoid a warning if 
not defined.



diff --git a/board/schulercontrol/sc_sps_1/sc_sps_1.c 
b/board/schulercontrol/sc_sps_1/sc_sps_1.c
index 7f0b591..9d3c970 100644
--- a/board/schulercontrol/sc_sps_1/sc_sps_1.c
+++ b/board/schulercontrol/sc_sps_1/sc_sps_1.c
@@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
   */
  int board_early_init_f(void)
  {
+   struct mxs_clkctrl_regs *clkctrl_regs =
+   (struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
+


And the same here.

It works ok, but I can't say if it fixes exactly those seldom initialization problems 
because I don't have a reliable way to reproduce them.


Tested-by: Hector Palacios hector.palac...@digi.com

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Hector Palacios]

Dear Marek,

On 10/13/2013 05:20 PM, Marek Vasut wrote:

Do not reconfigure the FEC clock during board_eth_init(), otherwise
the FEC might have stability issues, refuse to autonegotiate link
entirely or even corrupt packets while indicating correct checksum
on them. Instead, move the FEC clock init to board_early_init_f(),
where all the other upstream clock are initialized and also make
sure there is proper stabilization delay.

Signed-off-by: Marek Vasut ma...@denx.de
Cc: Fabio Estevam fabio.este...@freescale.com
Cc: Hector Palacios hector.palac...@digi.com
Cc: Oliver Metz oli...@freetz.org
Cc: Otavio Salvador ota...@ossystems.com.br
Cc: Stefano Babic sba...@denx.de
Cc: Tom Rini tr...@ti.com
---
  board/bluegiga/apx4devkit/apx4devkit.c   | 10 --
  board/denx/m28evk/m28evk.c   | 21 +++--
  board/freescale/mx28evk/mx28evk.c|  9 +
  board/schulercontrol/sc_sps_1/sc_sps_1.c | 19 +++
  4 files changed, 31 insertions(+), 28 deletions(-)


[...]


diff --git a/board/freescale/mx28evk/mx28evk.c 
b/board/freescale/mx28evk/mx28evk.c
index 5005fe2..2c93c44 100644
--- a/board/freescale/mx28evk/mx28evk.c
+++ b/board/freescale/mx28evk/mx28evk.c
@@ -41,6 +41,11 @@ int board_early_init_f(void)
/* SSP2 clock at 160MHz */
mxs_set_sspclk(MXC_SSPCLK2, 16, 0);

+#ifdef CONFIG_CMD_NET
+   cpu_eth_init(NULL);
+   udelay(1);
+#endif
+
  #ifdefCONFIG_CMD_USB
mxs_iomux_setup_pad(MX28_PAD_SSP2_SS1__USB1_OVERCURRENT);
mxs_iomux_setup_pad(MX28_PAD_AUART2_RX__GPIO_3_8 |
@@ -102,10 +107,6 @@ int board_eth_init(bd_t *bis)
struct eth_device *dev;
int ret;

-   ret = cpu_eth_init(bis);
-   if (ret)
-   return ret;
-
/* MX28EVK uses ENET_CLK PAD to drive FEC clock */
writel(CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN,
   clkctrl_regs-hw_clkctrl_enet);


Why didn't you move the clk enable instruction on mx28evk, as you did with the rest of 
platforms?



Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] fatls shows duplicated entries with long and short names

[Hector Palacios]

Dear Marek,

I noticed that 'fatls' displays duplicated filenames (short and long) for every file 
in the media:


# fatls mmc 0
  2083460   uimage-myplatform
  2083460   uimage~1
 1520   rootfs-dummy.jffs2
 1520   rootfs~1.jff
  3294952   uimage
  3294952   uimage

The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the 
chksum calculation out of an if() and now the code never enters this:


#ifdef CONFIG_SUPPORT_VFAT
else if (dols == LS_ROOT  csum == prevcksum) {
prevcksum = 0x;
dentptr++;
continue;
}
#endif

Could you please check?

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] gpmi-nand driver and jffs2 support

[Hector Palacios]

 unnecessary 
platform_set_drvdata()
63d5c5167c38967ded910c74b2190c368957c4ee mtd: nand: fsmc_nand: remove unnecessary 
platform_set_drvdata()
0c45084c9b2ab66af282193d43f736d02f5dcb1e mtd: nand: docg4: remove unnecessary 
platform_set_drvdata()
385f9c1ef89efaf4bd4d4f3236e81c755b8caddf mtd: nand: bf5xx_nand: remove unnecessary 
platform_set_drvdata()
65a561c4335f598a2e33239d25f4d1a1a4e0fcf1 mtd: nand: atmel_nand: remove unnecessary 
platform_set_drvdata()
c27ed70c52292fc322050ddf80e84aa0de4f mtd: nand: ams-delta: remove unnecessary 
platform_set_drvdata()
49649863a7dd1f3ee0c46320ff78856974e9772a mtd: nand: pxa3xx: Add support for Read 
parameter page command
caa75d26a05fa5155855a86b58acb74e916c2328 mtd: nand: pxa3xx: Add address support for 
READID command
a456431d85b37365bb324c48300781ec940fff99 mtd: nand: pxa3xx: Fix MODULE_DEVICE_TABLE 
declaration

a6ab4f906eb96cfbaa8823e6668bb840020c3252 mtd: nand: pxa3xx: Use 
of_machine_is_compatible()
c6bf90f74720c76ddededb035d335827b55e9c87 mtd: nand: pxa3xx: Set info-use_dma 
properly
98dde6f6ec74e855a206904c6e4f82baa041ca9a mtd: atmel_nand: add a new dt binding item 
for nand dma support
4df160fe6b080116a37ac90d4619983b71a3d21d mtd: atmel_nand: replace cpu_is_at32ap7000() 
with a nand platform data
40aeefd2d7b15037141a36bb99ee8ac0fc192d77 mtd: gpmi-nand: fix error return from 
gpmi_get_clks()

359a1df828b36e9a9994a23418544bc88c28074f mtd: nand: davinci: use 
devm_ioremap_resource()
6e0efb855af64ab8322a9eb523027fc8f6bc43f6 mtd: nand_base: Only use GET/SET FEATURES 
command on chips that support them.

9ab677c6bb897ef2108292996e66530cce66c2b9 mtd: nand: fsmc: update of OF support
ff59928a191e881df9e267b93506ed016ac15a1a mtd: nand: pxa3xx: Move buffer release code 
to its own function
0c0412495b9b3481220ca2b550254749efcca2b9 mtd: nand: pxa3xx: Check for 
clk_prepare_enable() return value
9ecbf288af72b90c141dcfdd41a1159a41ca2992 mtd: nand: pxa3xx: Use clk_prepare_enable and 
clk_disable_unprepare

62f822b04664f2bb5259013dc033da773e6e47b8 mtd: nand: pxa3xx: Use devm_clk_get
9b948fb5b989ac8faf387968dd9e1de4b4fb2cb1 mtd: nand: pxa3xx: Use 
devm_ioremap_resource
7e4dd5c37fa010dd55c243e58e9c22a03d73687d mtd: nand: pxa3xx: Use devm_kzalloc
57646f659a8a9cdc52f1ad561efbe8df653ccba6 mtd: nand_base: Use io{read, write}*_rep 
functions for transfer
b426958cc7b80a7bc82e2d93e2ad9324876fe982 mtd: nand-gpio: Add missing owner field in 
platform_driver struct
9822c3457e6664f6b7476fc0ccee242204b95272 mtd: nand-gpio: Rename internal variables to 
match functionality
c9906128d056e743e0f207147e0f880b2242c047 mtd: nand-gpio: Unneeded dependency on ARM 
removed
38af907581d9397d41ee8237f3e67a7d1319a8ce mtd: nand-gpio: Use default nand_base 
{read/write}_buf functions
dd6ca7acd8bebf280dbdb1d675732fa7f9cac8bd mtd: nand-gpio: Do not override GPIOs if 
driver uses platform_data but OF is enabled in
45078431a6ea64791ad8a72ac5860c0671ac53d4 mtd: nand-gpio: Use default dev_ready 
function if RDY is missing in configuration
055deba61df0a1e34d7682701babb62678f9ab9e mtd: nand-gpio: Convert driver to using 
resource-managed functions

b6eec0dada833875d85a3c0917a394ad57dfb54b mtd: fsl_ifc_nand: set 
NAND_NO_SUBPAGE_WRITE
892bccd8efae6175dc9d8628627e347c526564d2 mtd: fsmc_nand: add CONFIG_PM_SLEEP to 
suspend/resume functions
18c40318889629ad2f767603b0f4b51ebb71e4de mtd: r852: add CONFIG_PM_SLEEP to 
suspend/resume functions

42def8ab25ecd8a81f1bd4b452445ed3636d159f mtd: fsl_ifc_nand: remove incorrect 
kfree()
d855bd95ad35568b109f1b4587b68f0c950011f5 mtd: omap2: allow bulding as a module



Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] gpmi-nand driver and jffs2 support

[Hector Palacios]

On 09/19/2013 06:07 PM, Marek Vasut wrote:

Dear Huang Shijie,

Sorry, I think I got lost in the discussion. I will pull out.

btw Hector, I managed to verify JFFS2 mounting doesn't work on 3.10 , now I'm
trying to backport patches from -next and apply the GPMI NAND patchset, without
much luck yet.


Oops, sorry, did you mean you were trying to backport them into U-Boot? Then please 
disregards my previous message.



Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec_mxc: Fix timeouts during tftp transfer

[Hector Palacios]

On 09/16/2013 03:10 AM, Fabio Estevam wrote:

From: Fabio Estevam fabio.este...@freescale.com

Performing tftp transfers on mx28 results in random timeouts.

Hector Palacios and Robert Hodaszi analyzed the root cause being related to the
alignment of the 'buff' buffer inside fec_recv().

GCC versions such as 4.4/4.5 are more likely to exhibit such problem.

Use ALLOC_CACHE_ALIGN_BUFFER() for making the proper alignment of buffer.

Reported-by: Hector Palacios hector.palac...@digi.com
Tested-by: Oliver Metz oli...@freetz.org
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
  drivers/net/fec_mxc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 690e572..b423ff6 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -794,7 +794,7 @@ static int fec_recv(struct eth_device *dev)
uint16_t bd_status;
uint32_t addr, size, end;
int i;
-   uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+   ALLOC_CACHE_ALIGN_BUFFER(uchar, buff, FEC_MAX_PKT_SIZE);

/*
 * Check if any critical events have happened



Tested-by: Hector Palacios hector.palac...@digi.com

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Hector Palacios]

Hello,

Going back to this old thread I have some news regarding the problem with TFTP 
transmissions blocking (timed out) after 10 seconds on the FEC of the MX28.

See below:

On 07/17/2013 05:55 PM, Hector Palacios wrote:

Dear Marek,

On 07/16/2013 06:44 AM, Marek Vasut wrote:

Dear Fabio Estevam,


On Tue, Jul 16, 2013 at 12:51 AM, Fabio Estevam feste...@gmail.com wrote:

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

hector.palac...@digi.com wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M'
in your EVK?


Yes, this is what I have been running since the beginning.


If it doesn't fail, could you try running it again after playing with
the environment (setting/printing some variables).


I can't reproduce the problem here.


As I said, this issue appeared with different TFTP servers and is
independent of whether the dcache is or not enabled.


Can you transfer from a PC to another PC via TFTP?


Yes I can.


Another thing of interest would be a 'tcpdump' pcap capture of that connection.


I was initially filtering out only TFTP packets of my wireshark trace and all 
looked
correct. After taking a second look to the full trace I see now a hint.
Around 7 seconds after starting the TFTP transfer the server is sending an ARP 
to the
target asking for the owner of the target's IP. The target is receiving this 
ARP and
apparently responding (at least this is what my debug code shows as it gets into
arp.c:ArpReceive(), case ARPOP_REQUEST and sending a packet), but this ARP 
reply from
the target is not reaching the network. My sniffer does not capture this reply.

The server resends the ARP request twice more (seconds 8 and 9) to the target 
and
since it doesn't get a reply then sends a broadcast ARP (seconds 10) asking who 
has
that IP. Since nobody responds it stops sending data.

The times that it works (and I don't know the magic behind using a numeric 
address
versus using ${loadaddr} when they have the same value), the ARP replies do 
reach the
network and the server continues the transmission normally.

Using a v2009 U-Boot, the behaviour is exactly the same, but the target's ARP 
replies
always reach the network, and the transfers always succeed.

Since Fabio cannot reproduce it I guess it must be a local ghost. :o(


We tracked down the issue to an ARP request from the server that was never answered by 
the target. We later noticed that the problem did not happen anymore when building 
U-Boot with a different toolchain and that the issue seemed to be in the alignment of 
the RX buffer in the stack, which old GCC compilers seem to do wrong.


Here is a patch:

From: Robert Hodaszi robert.hoda...@digi.com
Date: Fri, 6 Sep 2013 09:50:52 +0200
Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because
 of GCC bug

Older GCC versions don't handle well alignment on stack variables.
The temporary RX buffer is a local variable, so it is on the stack.
Because the FEC driver is using DMA for transmission, receive and
transmit buffers should be aligned on 64 byte. The transmit buffers are
not allocated in the driver internally, it sends the packets directly
as it gets them. So these packets should be aligned.
When the ARP layer wants to reply to an ARP request, it uses the FEC
driver's temporary RX buffer (used to pass data to the ARP layer) to
store the new packet, and pass it back to the FEC driver's send function.
Because of a GCC bug, this buffer is not aligned well, and when the
driver tries to send it, it first rounds the address down to the
alignment boundary. That causes invalid data.

To fix it, don't put the temporary onto the stack.

Signed-off-by: Robert Hodaszi robert.hoda...@digi.com
Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 drivers/net/fec_mxc.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index f4f72b7..315017e 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -828,7 +828,10 @@ static int fec_recv(struct eth_device *dev)
uint16_t bd_status;
uint32_t addr, size, end;
int i;
-   uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+   /* Don't place this variable on the stack, because older GCC version
+* doesn't handle alignement on stack well.
+*/
+   static uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);

/*
 * Check if any critical events have happened


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

Re: [U-Boot] gpmi-nand driver and jffs2 support

[Hector Palacios]

Dear Marek,

On 09/04/2013 04:38 PM, Marek Vasut wrote:

Dear Huang Shijie,


On Wed, Sep 04, 2013 at 04:00:36PM +0200, Marek Vasut wrote:

Dear Huang Shijie,
How come hector was then able to write his JFFS2 partition ?


If he uses the gpmi, he should not write the JFFS2, since the gpmi
does not support the jffs2. He will get the failure in the end.


Hector, can you comment on this?


I don't think I'm following these comments. The facts are:
1) A JFFS2 filesystem image written with nandwrite (mtd-utils v1.5.0)
a) does not mount on kernel v3.10
b) mounts OK on linux-next kernel (v3.12) with the patchset [1] from 
Huang
	(actually I didn't use linux-next but instead a v3.10 where I merged all the commits 
done to MTD in linux-next, which are a lot).


2) A JFFS2 filesystem image written with U-Boot v2013.01
a) mounts OK on old FSL kernel 2.6.35
b) does not mount on kernel v3.10 (neither on v3.8, I believe).
c) does not mount on linux-next with the patchset [1]

[1] http://lists.infradead.org/pipermail/linux-mtd/2013-August/048360.html

Marek, could you please confirm 2b on your side, just in case I'm doing anything wrong 
in my custom U-Boot?




So the jffs2 support is compatiable all the time.


Is the old Freescale 2.6.35 GPMI NAND format compatible with the one
after applying this patchset?


Not compatible.

This patch set is still underreview.


So this patch breaks compatiblity with FSL kernel release? This needs fixing,
otherwise it's impossible to do a drop-in replacement for the ancient FSL
kernel.


that I could mount with Linux 3.7 and earlier?


I think the mount can be succeeded.


Ok, does that mean that we need this patchset in U-Boot in order to
properly write JFFS2 onto GPMI NAND there? Is that the message you
wanted to relay to us?


Besides this patchset, the u-boot needs more patches to sync with the
kernel mtd code. Such as the full-id features.


Can you elaborate on this more? This is very vague, I would like to know what
exactly is missing.


Yes, please, we need more details. This seems to be related with how the MTD drivers 
(in Linux and in U-Boot) access the OOB area to store the JFFS2 cleanmarkers, right?


The error I'm receiving from the kernel is at fs/jffs2/wbuf.c

if (!oinfo || oinfo-oobavail == 0) {
pr_err(inconsistent device description\n);
return -EINVAL;
}

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] gpmi-nand driver and jffs2 support

[Hector Palacios]

Dear Huang,

On 09/02/2013 10:50 AM, Huang Shijie wrote:

于 2013年09月02日 16:42, Hector Palacios 写道:

I am writing the JFFS2 partition from my custom U-Boot. Do you mean
that they way it writes it could not be compatible with what the new
driver expects? That sounds really bad.


The mtd code(as well as the gpmi driver) has merge many patch to the kernel,
BUT, the relative patches are not submitted to uboot maillist. So the
code is not aligned between the
uboot and the kernel.


Ok, I just rewrote the JFFS2 partition using the mtd-utils and now it mounts correctly 
and without any error.
So does this mean that U-Boot is now unable to properly write a JFFS2 partition for it 
to be understood by the linux-next kernel? What is exactly the difference? Does it 
only affect Freescale NAND controllers?


I'm including the U-Boot mailing list in CC.
The complete thread for reference: http://patchwork.ozlabs.org/patch/271322/

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] i.MX28 AUTO_RESTART, watchdog, battery and poweroff

[Hector Palacios]

Greetings,

I'm not sure I understand the function mxs_power_clear_auto_restart() in 
spl_power_init.c

The name of the function seems to indicate that its purpose is to *clear* the 
AUTO_RESTART bit, but it is actually setting it:


setbits_le32(rtc_regs-hw_rtc_persistent0,
RTC_PERSISTENT0_AUTO_RESTART);

I believe though, that setting this bit is OK because it guarantees that the target 
can be reset from a reset button or a watchdog reset even when powered from the battery.


Am I missing something? Otherwise I think this function should be renamed to 
mxs_power_set_auto_restart().


Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] ARM: mxs: rename function that sets AUTO_RESTART flag

[Hector Palacios]

The AUTO_RESTART flag of HW_RTC_PERSISTENT0 register will
power up the chip automatically 180ms after power down.
This bit must be enabled by the boot loader to ensure the
target can start upon hardware reset or watchdog reset
even when powered from a battery.

Currently the function named 'mxs_power_clear_auto_restart()'
is setting this flag although the 'clear' in its name suggest
the opposite.

This patch renames the function to 'mxs_power_set_auto_restart()'
and removes the comment about EVK revision A which was confusing
because the function indeed was setting the flag.

Signed-off-by: Hector Palacios hector.palac...@digi.com
---
 arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c 
b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
index 5ee2d88..285ac79 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
@@ -52,7 +52,7 @@ static void mxs_power_clock2pll(void)
CLKCTRL_CLKSEQ_BYPASS_CPU);
 }
 
-static void mxs_power_clear_auto_restart(void)
+static void mxs_power_set_auto_restart(void)
 {
struct mxs_rtc_regs *rtc_regs =
(struct mxs_rtc_regs *)MXS_RTC_BASE;
@@ -65,10 +65,7 @@ static void mxs_power_clear_auto_restart(void)
while (readl(rtc_regs-hw_rtc_ctrl)  RTC_CTRL_CLKGATE)
;
 
-   /*
-* Due to the hardware design bug of mx28 EVK-A
-* we need to set the AUTO_RESTART bit.
-*/
+   /* Do nothing if flag already set */
if (readl(rtc_regs-hw_rtc_persistent0)  RTC_PERSISTENT0_AUTO_RESTART)
return;
 
@@ -903,7 +900,7 @@ void mxs_power_init(void)
(struct mxs_power_regs *)MXS_POWER_BASE;
 
mxs_power_clock2xtal();
-   mxs_power_clear_auto_restart();
+   mxs_power_set_auto_restart();
mxs_power_set_linreg();
mxs_power_setup_5v_detect();
 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] ARM: mxs: rename function that sets AUTO_RESTART flag

[Hector Palacios]

Dear Marek,

On 07/24/2013 05:54 PM, Marek Vasut wrote:

Dear Hector Palacios,


The AUTO_RESTART flag of HW_RTC_PERSISTENT0 register will
power up the chip automatically 180ms after power down.
This bit must be enabled by the boot loader to ensure the
target can start upon hardware reset or watchdog reset
even when powered from a battery.

Currently the function named 'mxs_power_clear_auto_restart()'
is setting this flag although the 'clear' in its name suggest
the opposite.

This patch renames the function to 'mxs_power_set_auto_restart()'
and removes the comment about EVK revision A which was confusing
because the function indeed was setting the flag.


Why? The comment seems fully valid to me.


The comment is confusing for a number of reasons:
- It says for revision A of the EVK the flag must be enabled but the code is common 
for all platforms and there is no distinction about the platform where it runs. The 
flag must be enabled for any platform.
- The flag is set by the function in any case (except if it is already set) so the 
comment superfluous.
- The comment doesn't even say what the problem is on EVK_A or why the setting of the 
bit helps.


Unless someone from Freescale elaborates the comment, I would remove it.
At minimum I would move it somewhere else, like to the top of the function, not where 
it is now. Maybe Fabio can help.



Signed-off-by: Hector Palacios hector.palac...@digi.com
---
  arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 9 +++--
  1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 5ee2d88..285ac79
100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
@@ -52,7 +52,7 @@ static void mxs_power_clock2pll(void)
CLKCTRL_CLKSEQ_BYPASS_CPU);
  }

-static void mxs_power_clear_auto_restart(void)
+static void mxs_power_set_auto_restart(void)
  {
struct mxs_rtc_regs *rtc_regs =
(struct mxs_rtc_regs *)MXS_RTC_BASE;
@@ -65,10 +65,7 @@ static void mxs_power_clear_auto_restart(void)
while (readl(rtc_regs-hw_rtc_ctrl)  RTC_CTRL_CLKGATE)
;

-   /*
-* Due to the hardware design bug of mx28 EVK-A
-* we need to set the AUTO_RESTART bit.
-*/
+   /* Do nothing if flag already set */


You're changing the behavior here and it's not documented. I see no point in
this change while at it.


I'm not changing the behavior. I just renamed the function to reflect what the 
function does, which is set the flag, not clear it. Apart from the renaming, I didn't 
touch a line of real code.



if (readl(rtc_regs-hw_rtc_persistent0)  RTC_PERSISTENT0_AUTO_RESTART)
return;

@@ -903,7 +900,7 @@ void mxs_power_init(void)
(struct mxs_power_regs *)MXS_POWER_BASE;

mxs_power_clock2xtal();
-   mxs_power_clear_auto_restart();
+   mxs_power_set_auto_restart();
mxs_power_set_linreg();
mxs_power_setup_5v_detect();


Best regards,
Marek Vasut




Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Hector Palacios]

Dear Marek,

On 07/16/2013 06:44 AM, Marek Vasut wrote:

Dear Fabio Estevam,


On Tue, Jul 16, 2013 at 12:51 AM, Fabio Estevam feste...@gmail.com wrote:

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

hector.palac...@digi.com wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M'
in your EVK?


Yes, this is what I have been running since the beginning.


If it doesn't fail, could you try running it again after playing with
the environment (setting/printing some variables).


I can't reproduce the problem here.


As I said, this issue appeared with different TFTP servers and is
independent of whether the dcache is or not enabled.


Can you transfer from a PC to another PC via TFTP?


Yes I can.


Another thing of interest would be a 'tcpdump' pcap capture of that connection.


I was initially filtering out only TFTP packets of my wireshark trace and all looked 
correct. After taking a second look to the full trace I see now a hint.
Around 7 seconds after starting the TFTP transfer the server is sending an ARP to the 
target asking for the owner of the target's IP. The target is receiving this ARP and 
apparently responding (at least this is what my debug code shows as it gets into 
arp.c:ArpReceive(), case ARPOP_REQUEST and sending a packet), but this ARP reply from 
the target is not reaching the network. My sniffer does not capture this reply.


The server resends the ARP request twice more (seconds 8 and 9) to the target and 
since it doesn't get a reply then sends a broadcast ARP (seconds 10) asking who has 
that IP. Since nobody responds it stops sending data.


The times that it works (and I don't know the magic behind using a numeric address 
versus using ${loadaddr} when they have the same value), the ARP replies do reach the 
network and the server continues the transmission normally.


Using a v2009 U-Boot, the behaviour is exactly the same, but the target's ARP replies 
always reach the network, and the transfers always succeed.


Since Fabio cannot reproduce it I guess it must be a local ghost. :o(

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Hector Palacios]

Hi Marek,

On 07/12/2013 06:48 PM, Marek Vasut wrote:

 [...]



but I found something:
It is very strange that the timeouts appear always after transferring
between 20 and 24 MiB. So I thought maybe it was not an issue with the
size of the file or the number of packets received, but instead a timed
issue (an issue that happens after some period of time). I checked, and in
fact the timeouts occur exactly 10 seconds after running the tftp command.
I verified that this is what is happening by adding a udelay(10) at
fec_send(). In this case, the timeout also occurs after 10 seconds, but
due to the delay, I have transferred only a few Kbytes.


Holy moly!


I tried to change different timeout related constants at tftp.c but still
the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something.
Really odd. Does it tell you anything?


LAN8720 phy, right? Try implementing something like [1], by clearing the
EDPWRDOWN bit , the PHY will never enter low-power mode. It's just a simple PHY
register RMW which you can stick somewhere into the PHY net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine/+/b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0


No, my PHY is a Micrel KSZ8031RNLI.

The hint about the PHY possibly going to power down mode is interesting but I checked 
the PHY registers and EDPD mode (Energy Detect Power Down) is off, at least before 
running the tftp command. Power Down mode is off too, so unless these are somehow 
enabled during the TFTP process, this is not what's happening.


The sniffer shows that the TFTP server simply stops sending data packets. I can see 
however the target sending several times the ACK packet to the last received data 
packet. This would point to the TFTP server (as Albert suggested), but the fact is the 
problem occurs with different TFTP servers (I tried three different servers) and it 
does not happen with an old v2009 U-Boot using the same target.


Many times, though not always, after the timeout occurs, I cancel with CTRL+C and run 
the tftp command again, and then the file downloads completely.

The PHY is my usual suspect...

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Hector Palacios]

Hi Marek,

On 07/15/2013 02:30 PM, Marek Vasut wrote:

Dear Hector Palacios,


Hi Marek,

On 07/12/2013 06:48 PM, Marek Vasut wrote:

  [...]

but I found something:
It is very strange that the timeouts appear always after transferring
between 20 and 24 MiB. So I thought maybe it was not an issue with the
size of the file or the number of packets received, but instead a timed
issue (an issue that happens after some period of time). I checked, and
in fact the timeouts occur exactly 10 seconds after running the tftp
command. I verified that this is what is happening by adding a
udelay(10) at fec_send(). In this case, the timeout also occurs
after 10 seconds, but due to the delay, I have transferred only a few
Kbytes.


Holy moly!


I tried to change different timeout related constants at tftp.c but
still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or
something. Really odd. Does it tell you anything?


LAN8720 phy, right? Try implementing something like [1], by clearing the
EDPWRDOWN bit , the PHY will never enter low-power mode. It's just a
simple PHY register RMW which you can stick somewhere into the PHY
net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine/+
/b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0


No, my PHY is a Micrel KSZ8031RNLI.

The hint about the PHY possibly going to power down mode is interesting but
I checked the PHY registers and EDPD mode (Energy Detect Power Down) is
off, at least before running the tftp command. Power Down mode is off too,
so unless these are somehow enabled during the TFTP process, this is not
what's happening.


OK, makes sense.


The sniffer shows that the TFTP server simply stops sending data packets. I
can see however the target sending several times the ACK packet to the
last received data packet. This would point to the TFTP server (as Albert
suggested), but the fact is the problem occurs with different TFTP servers
(I tried three different servers) and it does not happen with an old v2009
U-Boot using the same target.


Can you try running dcache off command before running the TFTP transfer? Does
it still behave like this?

You might need to define #define CONFIG_CMD_CACHE for this to work.


Sourcery!
It's not that it works with dcache off, I found something even more strange:
The way I reproduce this issue is by setting the 'bootcmd' to 'dboot ${loadaddr} 
file100M'. When you set the 'bootcmd' like this:


setenv bootcmd tftp ${loadaddr} file100M

this eventually expands to

bootcmd=tftp 0x4200 file100M

So this is the command that runs automatically after the bootdelay.
I just discovered that if instead of letting the auto boot run, I press a key to stop 
the auto boot and run the command by hand (either running 'boot' or typing the command 
'tftp 0x4200 file100M'), the tftp transfer works perfectly.
On the other hand, if I do the same but use the environment variable ${loadaddr}, i.e. 
'tftp ${loadaddr} file100M'. It will stop after 10 seconds.


And to make things funnier I just reproduced this issue on the MX28EVK using the imx 
U-Boot custodian tree at commit a3f170cdbf7ae0bd24c94c2f46725699bbd69f05. That 
discards being a platform issue.

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your 
EVK?
If it doesn't fail, could you try running it again after playing with the environment 
(setting/printing some variables).
As I said, this issue appeared with different TFTP servers and is independent of 
whether the dcache is or not enabled.


Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Hector Palacios]

On 07/15/2013 05:12 PM, Marek Vasut wrote:

Hi Hector,


Hi Marek,

On 07/15/2013 02:30 PM, Marek Vasut wrote:

Dear Hector Palacios,


Hi Marek,

On 07/12/2013 06:48 PM, Marek Vasut wrote:

   [...]

but I found something:
It is very strange that the timeouts appear always after transferring
between 20 and 24 MiB. So I thought maybe it was not an issue with the
size of the file or the number of packets received, but instead a
timed issue (an issue that happens after some period of time). I
checked, and in fact the timeouts occur exactly 10 seconds after
running the tftp command. I verified that this is what is happening
by adding a udelay(10) at fec_send(). In this case, the timeout
also occurs after 10 seconds, but due to the delay, I have
transferred only a few Kbytes.


Holy moly!


I tried to change different timeout related constants at tftp.c but
still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or
something. Really odd. Does it tell you anything?


LAN8720 phy, right? Try implementing something like [1], by clearing
the EDPWRDOWN bit , the PHY will never enter low-power mode. It's just
a simple PHY register RMW which you can stick somewhere into the PHY
net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine
/+ /b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0


No, my PHY is a Micrel KSZ8031RNLI.

The hint about the PHY possibly going to power down mode is interesting
but I checked the PHY registers and EDPD mode (Energy Detect Power
Down) is off, at least before running the tftp command. Power Down mode
is off too, so unless these are somehow enabled during the TFTP
process, this is not what's happening.


OK, makes sense.


The sniffer shows that the TFTP server simply stops sending data
packets. I can see however the target sending several times the ACK
packet to the last received data packet. This would point to the TFTP
server (as Albert suggested), but the fact is the problem occurs with
different TFTP servers (I tried three different servers) and it does
not happen with an old v2009 U-Boot using the same target.


Can you try running dcache off command before running the TFTP
transfer? Does it still behave like this?

You might need to define #define CONFIG_CMD_CACHE for this to work.


Sourcery!
It's not that it works with dcache off, I found something even more
strange: The way I reproduce this issue is by setting the 'bootcmd' to
'dboot ${loadaddr} file100M'. When you set the 'bootcmd' like this:

setenv bootcmd tftp ${loadaddr} file100M

this eventually expands to

bootcmd=tftp 0x4200 file100M

So this is the command that runs automatically after the bootdelay.
I just discovered that if instead of letting the auto boot run, I press a
key to stop the auto boot and run the command by hand (either running
'boot' or typing the command 'tftp 0x4200 file100M'), the tftp
transfer works perfectly.
On the other hand, if I do the same but use the environment variable
${loadaddr}, i.e. 'tftp ${loadaddr} file100M'. It will stop after 10
seconds.


Count it be your hardware needs some more delay to stabilize?


It's not a problem of running the command later. If I run the command later, manually, 
but use ${loadaddr} instead of the hardcoded value 0x4200, then it will fail. It's 
like if accessing the environment for grabbing the value of ${loadaddr} or for 
grabbing the value of ${bootcmd}, made the problem appear, while if I run a manual 
command that does not require accesing the environment, it works.



And to make things funnier I just reproduced this issue on the MX28EVK
using the imx U-Boot custodian tree at commit
a3f170cdbf7ae0bd24c94c2f46725699bbd69f05. That discards being a platform
issue.


It still might be a PHY issue, no?


I guess it might, but the MX28EVK uses a different PHY. An SMSC, I believe.


@Fabio: could you manually run the command 'tftp ${loadaddr} file100M'


I guess that'd be a 100MB file?


Yes. The size is not that important as far as it takes more than 10 seconds to 
download. The keypoint here is using the environment variable, rather than an hex value.


Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Hector Palacios]

Dear Marek,

On 07/12/2013 05:51 AM, Marek Vasut wrote:

Hi,


On Thu, Jul 11, 2013 at 8:18 PM, Fabio Estevam feste...@gmail.com wrote:

On Thu, Jul 11, 2013 at 8:03 PM, Marek Vasut ma...@denx.de wrote:

The MX28 multi-layer AHB bus can be too slow and trigger the
FEC DMA too early, before all the data hit the DRAM. This patch
ensures the data are written in the RAM before the DMA starts.
Please see the comment in the patch for full details.

This patch was produced with an amazing help from Albert Aribaud,
who pointed out it can possibly be such a bus synchronisation
issue.

Signed-off-by: Marek Vasut ma...@denx.de
Cc: Albert ARIBAUD albert.u.b...@aribaud.net
Cc: Fabio Estevam fabio.este...@freescale.com
Cc: Stefano Babic sba...@denx.de


Excellent, managed to transfer 90MB via TFTP on mx28evk without a
single timeout.

Tested-by: Fabio Estevam fabio.este...@freescale.com


It's working here too.

Tested-by: Alexandre Pereira da Silva aletes@gmail.com


Nice to hear, thank Albert for finding this.


Thanks for sharing.

Unfortunately I'm still seeing non-recoverable timeouts when doing tftp 
transfers.
Nevertheless, with this patch sometimes I'm able to transfer big files (100MiB) 
without problems (I was never able before). So this is a big improvement.
I applied this patch over a v2013.01, does it need any additional patch? I think I saw 
one email about flush dcache...


Considering the other guys seem to work without problems I guess this scenario is 
specific to my board. I'm using a Micrel KSZ8031RNLI at 50MHz. I always suspect from 
the PHY.
I'm disconcerted because usually the timeouts occur after having successfully 
downloaded 22 or 24 MiB. Other times it just downloads completely.


In any case, good job!

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Hector Palacios]

Hi Marek,

On 07/12/2013 02:01 PM, Marek Vasut wrote:

Hi Hector,


Dear Marek,

On 07/12/2013 05:51 AM, Marek Vasut wrote:

Hi,


On Thu, Jul 11, 2013 at 8:18 PM, Fabio Estevam feste...@gmail.com wrote:

On Thu, Jul 11, 2013 at 8:03 PM, Marek Vasut ma...@denx.de wrote:

The MX28 multi-layer AHB bus can be too slow and trigger the
FEC DMA too early, before all the data hit the DRAM. This patch
ensures the data are written in the RAM before the DMA starts.
Please see the comment in the patch for full details.

This patch was produced with an amazing help from Albert Aribaud,
who pointed out it can possibly be such a bus synchronisation
issue.

Signed-off-by: Marek Vasut ma...@denx.de
Cc: Albert ARIBAUD albert.u.b...@aribaud.net
Cc: Fabio Estevam fabio.este...@freescale.com
Cc: Stefano Babic sba...@denx.de


Excellent, managed to transfer 90MB via TFTP on mx28evk without a
single timeout.

Tested-by: Fabio Estevam fabio.este...@freescale.com


It's working here too.

Tested-by: Alexandre Pereira da Silva aletes@gmail.com


Nice to hear, thank Albert for finding this.


Thanks for sharing.

Unfortunately I'm still seeing non-recoverable timeouts when doing tftp
transfers. Nevertheless, with this patch sometimes I'm able to transfer
big files (100MiB) without problems (I was never able before). So this is
a big improvement. I applied this patch over a v2013.01, does it need any
additional patch? I think I saw one email about flush dcache...


Try Stefano's tree as Fabio suggested. I think it's already pushed and includes
the fixes.


I just tried, but it didn't help.


Considering the other guys seem to work without problems I guess this
scenario is specific to my board. I'm using a Micrel KSZ8031RNLI at 50MHz.
I always suspect from the PHY.


You can try using the PHYLIB (CONFIG_PHYLIB and CONFIG_PHY_SMSC as in sc_sps_1.h
) . Also, can you check which of the two ret = -EINVAL is triggered in
fec_send() ? You can add simple printf() alongside both of them.


fec_send() *does not* ever fail, but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 
24 MiB. So I thought maybe it was not an issue with the size of the file or the number 
of packets received, but instead a timed issue (an issue that happens after some 
period of time). I checked, and in fact the timeouts occur exactly 10 seconds after 
running the tftp command.
I verified that this is what is happening by adding a udelay(10) at fec_send(). In 
this case, the timeout also occurs after 10 seconds, but due to the delay, I have 
transferred only a few Kbytes.
I tried to change different timeout related constants at tftp.c but still the issue 
happens after 10s.

It's like if, after these 10 seconds, the PHY lost the link or something. 
Really odd.
Does it tell you anything?

Best regards,
--
Hector Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] TFTP pause and timeout on MX28 platforms

[Hector Palacios]

Dear Marek Vasut,

On 03/13/2013 07:10 PM, Marek Vasut wrote:

Dear Hector Palacios,


Dear Marek Vasut,

On 03/13/2013 05:52 PM, Marek Vasut wrote:

Dear Hector Palacios,


Dear Marek Vasut,

On 03/13/2013 05:19 PM, Marek Vasut wrote:

Dear Hector Palacios,


Dear Marek Vasut,

On 03/13/2013 11:58 AM, Marek Vasut wrote:

Dear Hector Palacios,


Hello,

When doing large TFTP downloads (files bigger than 10MiB) on my MX28
platform I'm sometimes experiencing pauses and timeouts. U-Boot will
eventually restart the transmission and sometimes may successfully
complete the download, but other times it won't.
On my platform it was somewhat mitigated (but not resolved) by
reverting this patch from Marek:

commit 67449098a86be18cbdb27345bebe8da57e5d8899
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:50 2012 +

 FEC: Rework the TX wait mechanism

The problem is reproducible in Freescale's MX28EVK.
The larger the file, the more chances to see the problem. Please try
with 50MiB files or larger. Has anybody else seen it?

On an old U-Boot v2009 TFTP transmission was twice slower than with
v2013, but didn't suffer from this.


What version of u-boot do you use ? It was resolved in recent
versions.


I'm using v2013.01.
Could you please point me to the commit(s) that solve it?
The commits I see on drivers/net/fec_mxc.c after v2013.01 are from
Troy Kisky and don't seem to address this.


commit c0b5a3bbb0cd40a6b23b7b07e2182a5bcdc8c31c
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:51 2012 +

   FEC: Replace magic contants

and the three before this one. Is your PHY operating correctly? This
fixed the hangs on multiple MX28 platforms for me.


v2013.01 contains all these patches but I was able to reproduce this
both on my platform and on the mx28evk.

My problem is not a hang, but TFTP pauses (1 to 3 seconds) and timeouts:
#
#T T T T T T

At first I thought it was my platform's PHY (Micrel KSZ8031) but I also
see it on mx28evk.


Do you use PHYLIB (see configs/sc_sps_1.h for example)? Also check if
your transceiver mode is not misconfigured. Moreover, see
board/denx/m28evk/spl_boot.c fecmxc_mii_postcall(), maybe you also need
to program some registers of the PHY (V11 and V10 prototypes used
KSZ8051)?


No I don't use PHYLIB. My platform is based on Freescale's mx28evk (not
DENX's m28evk). I saw fecmxc_mii_postcall() and I do a similar thing
because of my phy model KSZ8031, but as I said the pauses and timeouts
occur in both platforms (mx28evk uses a different PHY: SMSC LAN8720a).

Are you able to download a 50MiB file via TFTP several times without any
problem on m28evk?


Yes.


Interesting. What PHY does m28evk platform have? I was thinking on a problem of the 
fec driver given that it can be reproduced in the mx28evk and in my platform, with 
different PHYs, but if it is not reproducible in m28evk I'll need to recheck the 
platform code.



btw. the original TFTP had size limit of 32MB, aren't you hitting this?


No it's not a server problem. I can sometimes transfer bigger files without problems. 
Other times it just pauses and/or timeouts after 3 or 4MiB.


Thank you
--
Héctor Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] TFTP pause and timeout on MX28 platforms

[Hector Palacios]

Hello,

When doing large TFTP downloads (files bigger than 10MiB) on my MX28 platform I'm 
sometimes experiencing pauses and timeouts. U-Boot will eventually restart the 
transmission and sometimes may successfully complete the download, but other times it 
won't.
On my platform it was somewhat mitigated (but not resolved) by reverting this patch 
from Marek:


commit 67449098a86be18cbdb27345bebe8da57e5d8899
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:50 2012 +

FEC: Rework the TX wait mechanism

The problem is reproducible in Freescale's MX28EVK.
The larger the file, the more chances to see the problem. Please try with 50MiB files 
or larger. Has anybody else seen it?


On an old U-Boot v2009 TFTP transmission was twice slower than with v2013, but didn't 
suffer from this.

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

Re: [U-Boot] TFTP pause and timeout on MX28 platforms

[Hector Palacios]

Dear Marek Vasut,

On 03/13/2013 11:58 AM, Marek Vasut wrote:

Dear Hector Palacios,


Hello,

When doing large TFTP downloads (files bigger than 10MiB) on my MX28
platform I'm sometimes experiencing pauses and timeouts. U-Boot will
eventually restart the transmission and sometimes may successfully
complete the download, but other times it won't.
On my platform it was somewhat mitigated (but not resolved) by reverting
this patch from Marek:

commit 67449098a86be18cbdb27345bebe8da57e5d8899
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:50 2012 +

  FEC: Rework the TX wait mechanism

The problem is reproducible in Freescale's MX28EVK.
The larger the file, the more chances to see the problem. Please try with
50MiB files or larger. Has anybody else seen it?

On an old U-Boot v2009 TFTP transmission was twice slower than with v2013,
but didn't suffer from this.


What version of u-boot do you use ? It was resolved in recent versions.


I'm using v2013.01.
Could you please point me to the commit(s) that solve it?
The commits I see on drivers/net/fec_mxc.c after v2013.01 are from Troy Kisky and 
don't seem to address this.

Thank you,

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

Re: [U-Boot] TFTP pause and timeout on MX28 platforms

[Hector Palacios]

Dear Marek Vasut,

On 03/13/2013 05:19 PM, Marek Vasut wrote:

Dear Hector Palacios,


Dear Marek Vasut,

On 03/13/2013 11:58 AM, Marek Vasut wrote:

Dear Hector Palacios,


Hello,

When doing large TFTP downloads (files bigger than 10MiB) on my MX28
platform I'm sometimes experiencing pauses and timeouts. U-Boot will
eventually restart the transmission and sometimes may successfully
complete the download, but other times it won't.
On my platform it was somewhat mitigated (but not resolved) by reverting
this patch from Marek:

commit 67449098a86be18cbdb27345bebe8da57e5d8899
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:50 2012 +

   FEC: Rework the TX wait mechanism

The problem is reproducible in Freescale's MX28EVK.
The larger the file, the more chances to see the problem. Please try
with 50MiB files or larger. Has anybody else seen it?

On an old U-Boot v2009 TFTP transmission was twice slower than with
v2013, but didn't suffer from this.


What version of u-boot do you use ? It was resolved in recent versions.


I'm using v2013.01.
Could you please point me to the commit(s) that solve it?
The commits I see on drivers/net/fec_mxc.c after v2013.01 are from Troy
Kisky and don't seem to address this.


commit c0b5a3bbb0cd40a6b23b7b07e2182a5bcdc8c31c
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:51 2012 +

 FEC: Replace magic contants

and the three before this one. Is your PHY operating correctly? This fixed the
hangs on multiple MX28 platforms for me.


v2013.01 contains all these patches but I was able to reproduce this both on my 
platform and on the mx28evk.

My problem is not a hang, but TFTP pauses (1 to 3 seconds) and timeouts:
 #
 #T T T T T T

At first I thought it was my platform's PHY (Micrel KSZ8031) but I also see it 
on mx28evk.

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

Re: [U-Boot] TFTP pause and timeout on MX28 platforms

[Hector Palacios]

Dear Marek Vasut,

On 03/13/2013 05:52 PM, Marek Vasut wrote:

Dear Hector Palacios,


Dear Marek Vasut,

On 03/13/2013 05:19 PM, Marek Vasut wrote:

Dear Hector Palacios,


Dear Marek Vasut,

On 03/13/2013 11:58 AM, Marek Vasut wrote:

Dear Hector Palacios,


Hello,

When doing large TFTP downloads (files bigger than 10MiB) on my MX28
platform I'm sometimes experiencing pauses and timeouts. U-Boot will
eventually restart the transmission and sometimes may successfully
complete the download, but other times it won't.
On my platform it was somewhat mitigated (but not resolved) by
reverting this patch from Marek:

commit 67449098a86be18cbdb27345bebe8da57e5d8899
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:50 2012 +

FEC: Rework the TX wait mechanism

The problem is reproducible in Freescale's MX28EVK.
The larger the file, the more chances to see the problem. Please try
with 50MiB files or larger. Has anybody else seen it?

On an old U-Boot v2009 TFTP transmission was twice slower than with
v2013, but didn't suffer from this.


What version of u-boot do you use ? It was resolved in recent versions.


I'm using v2013.01.
Could you please point me to the commit(s) that solve it?
The commits I see on drivers/net/fec_mxc.c after v2013.01 are from Troy
Kisky and don't seem to address this.


commit c0b5a3bbb0cd40a6b23b7b07e2182a5bcdc8c31c
Author: Marek Vasut ma...@denx.de
Date:   Wed Aug 29 03:49:51 2012 +

  FEC: Replace magic contants

and the three before this one. Is your PHY operating correctly? This
fixed the hangs on multiple MX28 platforms for me.


v2013.01 contains all these patches but I was able to reproduce this both
on my platform and on the mx28evk.
My problem is not a hang, but TFTP pauses (1 to 3 seconds) and timeouts:
   #
   #T T T T T T

At first I thought it was my platform's PHY (Micrel KSZ8031) but I also see
it on mx28evk.


Do you use PHYLIB (see configs/sc_sps_1.h for example)? Also check if your
transceiver mode is not misconfigured. Moreover, see
board/denx/m28evk/spl_boot.c fecmxc_mii_postcall(), maybe you also need to
program some registers of the PHY (V11 and V10 prototypes used KSZ8051)?


No I don't use PHYLIB. My platform is based on Freescale's mx28evk (not DENX's 
m28evk). I saw fecmxc_mii_postcall() and I do a similar thing because of my phy model 
KSZ8031, but as I said the pauses and timeouts occur in both platforms (mx28evk uses a 
different PHY: SMSC LAN8720a).


Are you able to download a 50MiB file via TFTP several times without any problem on 
m28evk?


Best regards,
--
Héctor Palacios
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] problem to boot i.MX28 custom platform

[Hector Palacios]

On 02/26/2013 01:19 PM, Hector Palacios wrote:

Greetings,

I'm porting a custom platform based on mx28evk to latest U-Boot.
I modified the iomux.c for my platform (different DUART pins) and made some 
changes to
mxs_adjust_memory_params() to accommodate to my DRAM chip.

I built the u-boot.sb. The platform however does not boot either from USB or 
from MMC.
When loaded through USB, sb_loader complains with the messages at [1].
I fear the CPU resets before even executing the SPL code, but how could I tell? 
Is
there a way to check if the CPU reaches this code at all?
I don't see any ROM error codes either and the CPU seems to reset (when the USB
recovery mode fails, the target defaults to boot from flash my old v2009 
U-Boot).
Is there any other init stuff (linker scripts or something) I should take care 
of
that's preventing sb_loader to fully load the image?

Thank you.


[1] sb_loader output

'Freescale,Inc.' 'ROM Recovery' device at USB path 0002:0034:00
Downloading 415504 bytes...
hid_write report id 01 len 0x1f

hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
ERROR: hid_write() returned -1 (error (null))



Apparently the problem was due to my platform suffering a hardware specific VDDD 
brownout during the initialization.


Just in case it helps anybody, the reason why I was not able to see anything on the 
DUART port during SPL initialization, despite having enabled CONFIG_SPL_SERIAL_SUPPORT 
and having serial_puts() on the SPL code, was that my platform uses alternate pins for 
the DUART than the default used by the BOOT ROM and the EVK, so apart from configuring 
the correct IOMUX, I needed to unconfigure the pins used by BOOT ROM for the DUART (on 
board/vendor/iomux.c):


@@ -33,8 +33,12 @@

 const iomux_cfg_t iomux_setup[] = {
/* DUART */
-   MX28_PAD_PWM0__DUART_RX,
-   MX28_PAD_PWM1__DUART_TX,
+   /* Unconfigure BOOT ROM default DUART */
+   MX28_PAD_PWM0__GPIO_3_16,
+   MX28_PAD_PWM1__GPIO_3_17,
+   /* Configure DUART on alternate pins */
+   MX28_PAD_I2C0_SCL__DUART_RX,
+   MX28_PAD_I2C0_SDA__DUART_TX,

/* MMC0 */
MX28_PAD_SSP0_DATA0__SSP0_D0 | MUX_CONFIG_SSP0,
@@ -159,9 +163,6 @@ const iomux_cfg_t iomux_setup[] = {
MX28_PAD_SSP2_MISO__SSP2_D0 | MUX_CONFIG_SSP2,
MX28_PAD_SSP2_SS0__SSP2_D3 |
(MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP),
-   /* I2C */
-   MX28_PAD_I2C0_SCL__I2C0_SCL,
-   MX28_PAD_I2C0_SDA__I2C0_SDA,
 };

 #define HW_DRAM_CTL29  (0x74  2)

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

[U-Boot] problem to boot i.MX28 custom platform

[Hector Palacios]

Greetings,

I'm porting a custom platform based on mx28evk to latest U-Boot.
I modified the iomux.c for my platform (different DUART pins) and made some changes to 
mxs_adjust_memory_params() to accommodate to my DRAM chip.


I built the u-boot.sb. The platform however does not boot either from USB or 
from MMC.
When loaded through USB, sb_loader complains with the messages at [1].
I fear the CPU resets before even executing the SPL code, but how could I tell? Is 
there a way to check if the CPU reaches this code at all?
I don't see any ROM error codes either and the CPU seems to reset (when the USB 
recovery mode fails, the target defaults to boot from flash my old v2009 U-Boot).
Is there any other init stuff (linker scripts or something) I should take care of 
that's preventing sb_loader to fully load the image?


Thank you.


[1] sb_loader output

'Freescale,Inc.' 'ROM Recovery' device at USB path 0002:0034:00
Downloading 415504 bytes...
hid_write report id 01 len 0x1f

hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
.
hid_write report id 02 len 0x400
ERROR: hid_write() returned -1 (error (null))

--
Héctor Palacios

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