Re: [U-Boot] [PATCH] net: phy: micrel: fix KSZ9031 clock skew for values greater 0ps

2019-01-07 Thread Andreas Pretzsch
Request for inclusion of below patch.
CC Joe Hershberger as listed maintainer for 'drivers/net/'.

Thanks,
  Andreas

On Thu, 2018-11-29 at 20:04 +0100, Andreas Pretzsch wrote:
> For KSZ9021, all skew register fields are 4-bit wide.
> For KSZ9031, the clock skew register fields are 5-bit wide.
> 
> The common code in ksz90x1_of_config_group calculating the combined
> register value checks if the requested value is above the maximum
> and uses this maximum if so. The calculation of this maximum uses
> the register width, but the check itself does not. It uses a
> hardcoded
> value of 0xf, which is too low in case of the 5-bit clock (0x1f).
> This detail was probably lost during driver unification.
> 
> Effect (only for KSZ9031 clock skews): For values greater 900 (==
> 0ps),
> this silently results in 1860 (== +960ps) instead of the requested
> one.
> 
> Fix the check by using the bit width instead of hardcoded value(s).
> 
> Signed-off-by: Andreas Pretzsch 
> ---
>  drivers/net/phy/micrel_ksz90x1.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel_ksz90x1.c
> b/drivers/net/phy/micrel_ksz90x1.c
> index 3951535bf1..63e7b0242b 100644
> --- a/drivers/net/phy/micrel_ksz90x1.c
> +++ b/drivers/net/phy/micrel_ksz90x1.c
> @@ -123,8 +123,8 @@ static int ksz90x1_of_config_group(struct
> phy_device *phydev,
>   } else {
>   changed = 1;/* Value was changed in OF */
>   /* Calculate the register value and fix corner
> cases */
> - if (val[i] > ps_to_regval * 0xf) {
> - max = (1 << ofcfg->grp[i].size) - 1;
> + max = (1 << ofcfg->grp[i].size) - 1;
> + if (val[i] > ps_to_regval * max) {
>   regval |= max << offset;
>   } else {
>       regval |= (val[i] / ps_to_regval) <<
> offset;

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas PretzschTel. +49-(0)7307-936088-1
Lange Strasse 28a   Fax: +49-(0)7307-936088-9
89250 Senden, Germany   email: a...@cn-eng.de

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


[U-Boot] [PATCH] net: phy: micrel: fix KSZ9031 clock skew for values greater 0ps

2018-11-29 Thread Andreas Pretzsch
For KSZ9021, all skew register fields are 4-bit wide.
For KSZ9031, the clock skew register fields are 5-bit wide.

The common code in ksz90x1_of_config_group calculating the combined
register value checks if the requested value is above the maximum
and uses this maximum if so. The calculation of this maximum uses
the register width, but the check itself does not. It uses a hardcoded
value of 0xf, which is too low in case of the 5-bit clock (0x1f).
This detail was probably lost during driver unification.

Effect (only for KSZ9031 clock skews): For values greater 900 (== 0ps),
this silently results in 1860 (== +960ps) instead of the requested one.

Fix the check by using the bit width instead of hardcoded value(s).

Signed-off-by: Andreas Pretzsch 
---
 drivers/net/phy/micrel_ksz90x1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/micrel_ksz90x1.c b/drivers/net/phy/micrel_ksz90x1.c
index 3951535bf1..63e7b0242b 100644
--- a/drivers/net/phy/micrel_ksz90x1.c
+++ b/drivers/net/phy/micrel_ksz90x1.c
@@ -123,8 +123,8 @@ static int ksz90x1_of_config_group(struct phy_device 
*phydev,
} else {
changed = 1;/* Value was changed in OF */
/* Calculate the register value and fix corner cases */
-   if (val[i] > ps_to_regval * 0xf) {
-   max = (1 << ofcfg->grp[i].size) - 1;
+   max = (1 << ofcfg->grp[i].size) - 1;
+   if (val[i] > ps_to_regval * max) {
regval |= max << offset;
} else {
regval |= (val[i] / ps_to_regval) << offset;
-- 
2.19.1

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


Re: [U-Boot] [PATCH] command "sspi": add write-only flag '.w' (discard read data)

2011-07-22 Thread Andreas Pretzsch
Am Donnerstag, den 21.07.2011, 11:01 +0200 schrieb Detlev Zundel:
> Hi Mike,
> 
> [...]
> 
> >> Following the common U-Boot way to do this, I'd suggest
> >>sspi [:][.]   [din_env_var]
> >> and either never print the read result to the console (my favorite) or
> >> only if no env variable to store is passed. To be defined, comments
> >> welcome.
> >
> > is this the common u-boot approach ?  seems like extending every random 
> > func 
> > to take a supplementary env var is the wrong way.  if the hush shell simply 
> > supported syntax like:
> > foo="`sspi ..`"
> > then every command would get this for free
> 
> Indeed - this would be the best solution and I would greatly appreciate
> such a feature as it would be a big step in what we can script
> (elegantly) in U-Boot.

Full ACK that. I'd love to have a (nearly) full-scale hush shell in
U-Boot. Esp. as IMHO we left the "small scale bootloader" area for quite
some time now. And adjusting to different hardware variants purely by
script instead of special code is a valuable target IMHO. A bit
inconvenient with U-Boot hush shell, but doable.

But I doubt that this will be there in the immediate future (sorry,
currently no time on my side for this). And it won't help with the old
parser.

So how to proceed with the sspi command ?
  - keep it as it is, ignoring the "read not scriptable" status
  - strip down the printf output to only the value (required for ``)
  - add the additional parameter

Personally, I don't care that much, as I don't need a SPI read in any of
my devices here and could live with the noisy SPI write. But as I opened
up this issue, I'd like to finalize it.

Wolfgang, Detlev, Mike, please decide.

In this context, I suppose the original "spi.w" patch is NAK ?

Also, given any rework of the sspi command, cmd_spi.c should probably be
checkpatch-massaged (9 errors, 11 warnings, incl. kernel-stuff) before.
The usual whitespace and line length issues.
I suppose this would go in despite merge window closure, but any other
change won't, so submit it as separate patch instead of a series ?

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [RFC] gpio command: return value on write, additional actions

2011-07-20 Thread Andreas Pretzsch
> Actually I would prefer to return an error status for all cases and
> return "valuable information" in the environment so we can easily use it
> subsequently.  As we do not have a back-tick operator in the shell, I
> believe that return codes are only useful as error codes.

True for now, beside one usage:
if gpio value  ; then ; else ; fi
Of course, the same could be achieved with
gpio value  ; if test $? -eq 1 ; then ; else ; fi


While we're at it, adding another optional parameter to store the value
read in an environment variable sounds sensible:
gpio   [env_var]


Another point: I'd like to quiet down the gpio command. Either in
default case or via some other way (like gpio.q or another parameter).
Call me picky, but I like my programs to be quiet unless there is an
error. Especially when running scripted.
Opinions/decisions ?


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [PATCH] command "sspi": add write-only flag '.w' (discard read data)

2011-07-20 Thread Andreas Pretzsch
Am Montag, den 18.07.2011, 13:49 -0400 schrieb Mike Frysinger:
> On Sat, Jul 16, 2011 at 13:32, Andreas Pretzsch wrote:
> > The sspi command writes the given data out on SPI and prints the data it
> > reads to the console. For write-only slaves (i.e. a SPI-connected latch
> > used as output expander), this is pointless and clutters the console.
> > When called as "sspi.w", this output is omitted.
> >
> > The flag is optional and backwards compatible, previous sspi revisions
> > would simply ignore the flag (checked back to 2011.03).
> 
> i think the flag is misleading.  "sspi.w" makes it sound like it'd
> call the SPI layer with a NULL read buffer and not simply omit the
> output.  what you describe is more like a "quiet" flag.

Correct, strictly speaking.
I chose it from a usage perspective, hence the "write", but ".q" for
quiet or similar would also have been possible. "write" just felt more
natural to me.

I also thought about passing NULL for the read buffer, but a quick
browse through the code showed that most, but not all SPI drivers are
prepared for that. And as there is already a static rx buffer in the spi
command code, I preferred to keep a few needless byte copies over fixing
all the spi drivers...

On the long run, the sspi command should be reworked anyway, as today
the read data is not usable in a script, it's just dumped to the
console.
Following the common U-Boot way to do this, I'd suggest
sspi [:][.]   [din_env_var]
and either never print the read result to the console (my favorite) or
only if no env variable to store is passed. To be defined, comments
welcome.

The issue here is that the current sspi command would barf due to excess
argument, hence "new" sspi usage in an env script won't work on older
U-Boot revisions, whereas this is fine with the ".w" approach.
Not really that of a problem, admittedly. Maybe I was a bit too deep in
the "don't break it if not really necessary" mode...


> along these lines, doesnt the general shell provide basic output
> redirection to support "silencing" all commands rather than having to
> add a "quiet" flag to them all ?  then your script could simply do
> "sspi ... >/dev/null" (or however u-boot does it).

Not that I know of. Just tried on hush parser, no effect. Also looks
like the whole redirect code is disabled by a #ifndef __U_BOOT__.
Maybe one could change env stdout before and after, might work, but I'd
call that grotesque...

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] [PATCH] command "sspi": add write-only flag '.w' (discard read data)

2011-07-16 Thread Andreas Pretzsch
The sspi command writes the given data out on SPI and prints the data it
reads to the console. For write-only slaves (i.e. a SPI-connected latch
used as output expander), this is pointless and clutters the console.
When called as "sspi.w", this output is omitted.

The flag is optional and backwards compatible, previous sspi revisions
would simply ignore the flag (checked back to 2011.03).

Signed-off-by: Andreas Pretzsch 
---
Data size (number of bits) are passed as separate parameter to sspi,
hence .w is "free" and not used as data size anyway.
---
 common/cmd_spi.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/common/cmd_spi.c b/common/cmd_spi.c
index 8c623c9..c56c191 100644
--- a/common/cmd_spi.c
+++ b/common/cmd_spi.c
@@ -72,12 +72,17 @@ int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
uchar tmp;
int   j;
int   rcode = 0;
+   int   write_only = 0;
 
/*
 * We use the last specified parameters, unless new ones are
 * entered.
 */
 
+   j = strlen(argv[0]);
+   if (j > 2 && argv[0][j-2] == '.' && argv[0][j-1] == 'w')
+   write_only = 1;
+
if ((flag & CMD_FLAG_REPEAT) == 0)
{
if (argc >= 2) {
@@ -131,10 +136,12 @@ int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
printf("Error during SPI transaction\n");
rcode = 1;
} else {
-   for(j = 0; j < ((bitlen + 7) / 8); j++) {
-   printf("%02X", din[j]);
+   if (!write_only) {
+   /* dump read values to console */
+   for (j = 0; j < ((bitlen + 7) / 8); j++)
+   printf("%02X", din[j]);
+   printf("\n");
}
-   printf("\n");
}
spi_release_bus(slave);
spi_free_slave(slave);
@@ -147,7 +154,8 @@ int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
 U_BOOT_CMD(
sspi,   5,  1,  do_spi,
"SPI utility command",
-   "[:][.]   - Send and receive bits\n"
+   "[.w] [:][.]   - Send and receive bits\n"
+   ".w- write only => don't print read result\n"
" - Identifies the SPI bus\n"
"  - Identifies the chip select\n"
"- Identifies the SPI mode to use\n"
-- 
1.7.5.4

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


[U-Boot] [PATCH v2 2/2] add command fitupd to run an update from a FIT image

2011-07-16 Thread Andreas Pretzsch
Command calls update_tftp() analogous to automatic update described
in doc/README.update.

Usage:
fitupd [addr]
- run update from FIT image at addr
  or from tftp 'updatefile'

Signed-off-by: Andreas Pretzsch 
---
Changes for v2:
   - void update_tftp() => int update_tftp(): return success/fail
---
 common/Makefile |1 +
 common/cmd_fitupd.c |   36 
 doc/README.update   |5 +
 3 files changed, 42 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_fitupd.c

diff --git a/common/Makefile b/common/Makefile
index 224b7cc..e92ee5b 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -94,6 +94,7 @@ COBJS-$(CONFIG_CMD_FAT) += cmd_fat.o
 COBJS-$(CONFIG_CMD_FDC)$(CONFIG_CMD_FDOS) += cmd_fdc.o
 COBJS-$(CONFIG_OF_LIBFDT) += cmd_fdt.o fdt_support.o
 COBJS-$(CONFIG_CMD_FDOS) += cmd_fdos.o
+COBJS-$(CONFIG_CMD_FITUPD) += cmd_fitupd.o
 COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
 ifdef CONFIG_FPGA
 COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
diff --git a/common/cmd_fitupd.c b/common/cmd_fitupd.c
new file mode 100644
index 000..4d1192b
--- /dev/null
+++ b/common/cmd_fitupd.c
@@ -0,0 +1,36 @@
+/*
+ * (C) Copyright 2011
+ * Andreas Pretzsch, carpe noctem engineering, a...@cn-eng.de
+ *
+ * This file is released under the terms of GPL v2 and any later version.
+ * See the file COPYING in the root directory of the source tree for details.
+ */
+
+#include 
+#include 
+
+#if !defined(CONFIG_UPDATE_TFTP)
+#error "CONFIG_UPDATE_TFTP required"
+#endif
+
+extern int update_tftp(ulong addr);
+
+static int do_fitupd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+   ulong addr = 0UL;
+
+   if (argc > 2)
+   return cmd_usage(cmdtp);
+
+   if (argc == 2)
+   addr = simple_strtoul(argv[1], NULL, 16);
+
+   return update_tftp(addr);
+}
+
+U_BOOT_CMD(fitupd, 2, 0, do_fitupd,
+   "update from FIT image",
+   "[addr]\n"
+   "\t- run update from FIT image at addr\n"
+   "\t  or from tftp 'updatefile'"
+);
diff --git a/doc/README.update b/doc/README.update
index 48f03b7..a7f4d9e 100644
--- a/doc/README.update
+++ b/doc/README.update
@@ -51,6 +51,11 @@ the mkimage tool. dtc tool with support for binary includes, 
e.g. in version
 to be prepared. Refer to the doc/uImage.FIT/ directory for more details on FIT
 images.
 
+This mechanism can be also triggered by the commmand "fitupd".
+If an optional, non-zero address is provided as argument, the TFTP transfer
+is skipped and the image at this address is used.
+The fitupd command is enabled by CONFIG_CMD_FITUPD.
+
 
 Example .its files
 --
-- 
1.7.5.4

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


[U-Boot] [PATCH v2 1/2] automatic update from FIT image: add optional address parameter

2011-07-16 Thread Andreas Pretzsch
Current update_tftp() flow:
  1.) fetch "updatefile" from defined TFTP server
  2.) check if FIT format
  3.) flash contained images

Add an address parameter to update_tftp(). If this address is non-zero,
skip the TFTP transfer and use the image at this address.
Also extend update_tftp() to return success/fail.

Signed-off-by: Andreas Pretzsch 
---
Changes for v2:
   - void update_tftp() => int update_tftp(): return success/fail
---
 common/main.c   |4 ++--
 common/update.c |   20 ++--
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/common/main.c b/common/main.c
index 2730c6f..0a310e0 100644
--- a/common/main.c
+++ b/common/main.c
@@ -51,7 +51,7 @@ void inline __show_boot_progress (int val) {}
 void show_boot_progress (int val) __attribute__((weak, 
alias("__show_boot_progress")));
 
 #if defined(CONFIG_UPDATE_TFTP)
-void update_tftp (void);
+int update_tftp (ulong addr);
 #endif /* CONFIG_UPDATE_TFTP */
 
 #define MAX_DELAY_STOP_STR 32
@@ -345,7 +345,7 @@ void main_loop (void)
 #endif /* CONFIG_PREBOOT */
 
 #if defined(CONFIG_UPDATE_TFTP)
-   update_tftp ();
+   update_tftp (0UL);
 #endif /* CONFIG_UPDATE_TFTP */
 
 #if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0)
diff --git a/common/update.c b/common/update.c
index 7528474..a19f136 100644
--- a/common/update.c
+++ b/common/update.c
@@ -238,13 +238,17 @@ static int update_fit_getparams(const void *fit, int 
noffset, ulong *addr,
return 0;
 }
 
-void update_tftp(void)
+int update_tftp(ulong addr)
 {
char *filename, *env_addr;
int images_noffset, ndepth, noffset;
ulong update_addr, update_fladdr, update_size;
-   ulong addr;
void *fit;
+   int ret = 0;
+
+   /* use already present image */
+   if (addr)
+   goto got_update_file;
 
printf("Auto-update from TFTP: ");
 
@@ -253,7 +257,7 @@ void update_tftp(void)
if (filename == NULL) {
printf("failed, env. variable '%s' not found\n",
UPDATE_FILE_ENV);
-   return;
+   return 1;
}
 
printf("trying update file '%s'\n", filename);
@@ -268,15 +272,16 @@ void update_tftp(void)
if (update_load(filename, CONFIG_UPDATE_TFTP_MSEC_MAX,
CONFIG_UPDATE_TFTP_CNT_MAX, addr)) {
printf("Can't load update file, aborting auto-update\n");
-   return;
+   return 1;
}
 
+got_update_file:
fit = (void *)addr;
 
if (!fit_check_format((void *)fit)) {
printf("Bad FIT format of the update file, aborting "
"auto-update\n");
-   return;
+   return 1;
}
 
/* process updates */
@@ -293,6 +298,7 @@ void update_tftp(void)
 
if (!fit_image_check_hashes(fit, noffset)) {
printf("Error: invalid update hash, aborting\n");
+   ret = 1;
goto next_node;
}
 
@@ -301,15 +307,17 @@ void update_tftp(void)
&update_fladdr, &update_size)) {
printf("Error: can't get update parameteres, "
"aborting\n");
+   ret = 1;
goto next_node;
}
if (update_flash(update_addr, update_fladdr, update_size)) {
printf("Error: can't flash update, aborting\n");
+   ret = 1;
goto next_node;
}
 next_node:
noffset = fdt_next_node(fit, noffset, &ndepth);
}
 
-   return;
+   return ret;
 }
-- 
1.7.5.4

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


[U-Boot] [PATCH v2 0/2] update from FIT image: add optional address, new command fitupd

2011-07-16 Thread Andreas Pretzsch
Create new command "fitupd", so an update from a FIT-image can also be
triggered from U-Boot shell, in addition to the automatic update after
bootup when 'updatefile' is set.
Also provide a way to use a FIT image in memory (e.g. loaded from MMC)
instead of always load from a TFTP server.
See doc/README.update for more information.

Adhere to present coding style => checkpatch warnings discarded.

Changes for v2:
   - void update_tftp() => int update_tftp(): return success/fail 

Andreas Pretzsch (2):
  automatic update from FIT image: add optional address parameter
  add command fitupd to run an update from a FIT image

 common/Makefile |1 +
 common/cmd_fitupd.c |   36 
 common/main.c   |4 ++--
 common/update.c |   20 ++--
 doc/README.update   |5 +
 5 files changed, 58 insertions(+), 8 deletions(-)
 create mode 100644 common/cmd_fitupd.c

-- 
1.7.5.4

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


Re: [U-Boot] [RFC] gpio command: return value on write, additional actions

2011-07-05 Thread Andreas Pretzsch
Am Dienstag, den 05.07.2011, 13:44 -0400 schrieb Mike Frysinger:
> On Tuesday, July 05, 2011 12:59:16 Andreas Pretzsch wrote:
> > As of today (2011.06), the generic gpio command (common/cmd_gpio.c)
> > gpio  
> > - input/set/clear/toggle the specified pin (e.g. PF10)
> > always returns the value read or set.
> > 
> > While this is sensible for read (input) and maybe (questionable) for
> > toggle, I don't see an usage for write (set/clear).
> 
> the trouble with toggle is that there's no way to get the value without 
> changing it to the input.  we'd have to add another field that has no 

True, at least on devices without hardware toggle support.
And you never know if you read back the real pin state, the output latch
or something invalid. And a "short" switch to input isn't always legal
wrt to the attached hardware. Great when setting 1 bit is worth 10 bit
of problems ;-)

Personally, I'd shift the gpio_toggle to the underlying hardware
specific code. And print an error if it's not supported.

> unintentional side effects like "gpio value".  then we could change all the 
> others to return 0/1 based on whether they succeeded, not based on the level 
> of the gpio pin.

Didn't quite get that. In terms of "gpio value" = "give me the current
(output latch) value without setting it to input if it's an output" ?

We can't change the return value of "gpio input", as it's out in the
wild and would break existing scripts.

I'd say clear/set/toggle are changeable, don't see any legit
return-value-usage here. For 100% backward compatibility, one could
leave them as they are and use 0|1 as new actions with return 0, as
proposed.

So these variants:
  gpio clear|0 => set to output, write 0, return success
  gpio set|1   => set to output, write 1, return success
  gpio toggle  => (set to output), toggle output, return success
  gpio input   => set to input, return pin value
  gpio value   => return current pin/latch/whatever value
or
  gpio clear   => set to output, write 0, return 0
  gpio set => set to output, write 1, return 1
  gpio 0   => set to output, write 0, return success
  gpio 1   => set to output, write 1, return success
  gpio toggle  => (set to output), toggle output, return new_val
  gpio input   => set to input, return pin value
  gpio value   => return current pin/latch/whatever value

Not perfectly symmetric, but the best way out I can think of.
Maybe "get" instead of "value", as it's more usual. OTOH, get (to some
people) implies "set to input", so value is more explicit.


> > Also, this leads to unexpected side effects with complex constructs,
> > e.g. consider this environment:
> > setA=gpio set PF1
> > setB=gpio clear PF2
> > setAB_separate=env run setA ; env run setB
> > setAB_concatenated=env run setA setB
> > setBA_concatenated=env run setB setA
> > 
> > While executing "setAB_separate" and "setBA_concatenated" work as
> > expected, "setAB_concatenated" will only run setA, but not setB, as setA
> > "failed" (ret=1). [1]
> > The same would apply to e.g. && constructs.
> 
> ive never used the shell in u-boot, but couldnt the first logic be:
>   setA=gpio set PF1 || :
>   setB=gpio clear PF2 || :

Not fully, you'd need "gpio set PF1 || true". Not that this makes it
less ugly...
BTW, nobody would mind that without notice. Stumbled over it by pure
accident myself.

> > As it works this way in the release and hence the behaviour is
> > essentially fixed, changing the return value is probably not an option.
> 
> not really.  poor behavior can be adjusted.
> -mike

Fine from my side, but I'm not the one to decide.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] [RFC] gpio command: return value on write, additional actions

2011-07-05 Thread Andreas Pretzsch
As of today (2011.06), the generic gpio command (common/cmd_gpio.c)
gpio  
- input/set/clear/toggle the specified pin (e.g. PF10)
always returns the value read or set.

While this is sensible for read (input) and maybe (questionable) for
toggle, I don't see an usage for write (set/clear).

Also, this leads to unexpected side effects with complex constructs,
e.g. consider this environment:
setA=gpio set PF1
setB=gpio clear PF2
setAB_separate=env run setA ; env run setB
setAB_concatenated=env run setA setB
setBA_concatenated=env run setB setA

While executing "setAB_separate" and "setBA_concatenated" work as
expected, "setAB_concatenated" will only run setA, but not setB, as setA
"failed" (ret=1). [1]
The same would apply to e.g. && constructs.

As it works this way in the release and hence the behaviour is
essentially fixed, changing the return value is probably not an option.
OTOH, the breakage here might be negligible, at least for set/clear.
But in any case, it should be documented.

Beside this, I'd like to extend the gpio command with the additional
actions "0" and "1", always returning 0 (given the write was
successful):
gpio  

As a nice benefit, it would strip down constructs like
if test ${LED_x} -eq 0;then gpio set PF1;else gpio clear PF1;fi
to a simple "gpio ${LED_x} PF1".


Any comments or objections before I submit a patch ?


[1] http://www.denx.de/wiki/DULG/CommandLineParsing "General rules"

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [STATUS] Getting ready for -rc1

2011-05-17 Thread Andreas Pretzsch
Am Montag, den 16.05.2011, 22:24 +0200 schrieb Wolfgang Denk:
> Hi everybody,
> 
> I would like to get our -rc1 within the next 2 days or so.
> 
> Please drop me (and the respective custodian) a note if you have any
> patches that are supposed to go in.

I'd like to request the inclusion of my patchset adding a command to
manually trigger an update from a FIT-image, see
  http://lists.denx.de/pipermail/u-boot/2011-April/090911.html
  http://lists.denx.de/pipermail/u-boot/2011-April/090912.html
  http://lists.denx.de/pipermail/u-boot/2011-April/090913.html
resp.
  http://patchwork.ozlabs.org/bundle/apr-cn-eng/fitupd/
  http://patchwork.ozlabs.org/patch/92055/
  http://patchwork.ozlabs.org/patch/92056/

I suppose Mr. Jerry Van Baren as the FDT custodian is the primary
contact, therefore I added him to CC. But maybe you could decide also.

Discussion leading to this feature was
  http://lists.denx.de/pipermail/u-boot/2011-April/090202.html
  http://lists.denx.de/pipermail/u-boot/2011-April/090210.html
  http://lists.denx.de/pipermail/u-boot/2011-April/090484.html
  http://lists.denx.de/pipermail/u-boot/2011-April/090914.html

Thanks in advance,
  Andreas

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] Policy for checkpatch usage?

2011-04-23 Thread Andreas Pretzsch
Am Donnerstag, den 21.04.2011, 17:46 +0200 schrieb Detlev Zundel:
> Thanks for the input.  Current base for discussion:
> 
>   Checkpatch is a tool that can help you find some style problems, but
>   is imperfect, and the things it complains about are of varying
>   importance.  So use common sense in interpreting the results.
> 
>   Warnings that clearly only make sense in the Linux kernel can be
>   ignored.  This includes "Use #include  instead of
>   " for example.
>   
>   If you encounter warnings for existing code, not modified by your
>   patch, consider submitting a separate, cosmetic-only patch -- clearly
>   described as such -- that *precedes* your substantive patch.

For minor modifications (e.g. changed arguments of a function call),
adhere to the present codingstyle of the module. Relating checkpatch
warnings can be ignored in this case. A respective note in the commit or
cover letter why they are ignored is desired.


> Anyone unhappy with that?  Will this help newcomers?

Sounds sensible to me. The extra paragraph above is to circumvent
intermixing codingstyle inside a file. For example in common/main.c,
there are "function (arg, arg)" all around. Not checkpatch compliant
(and ugly, IMHO). Now when I modify one of this calls, e.g add an
argument, I'd have to change this. Which leads to a mixture of styles in
the file, which is even worse than than adhering to the "broken" style.

In general, I'd suggest a - maybe informal - policy that while
codingstyle is important, there are more relevant aspects like elegant
code, sensible interfaces, proper use of static and const and so on.
I'm sure any developer is willing to debate about these issues, but most
volunteers (esp. company or freelance coders) will resign after the 3rd
round of whitespace ping-pong...

Not sure how to phrase that without contradicting the basic idea, maybe
with an additional paragraph like:
"New modules have to be as checkpatch compliant as possible. Violations
have to be justified in the commit or cover letter."


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] trigger automatic update (FIT image) from prompt instead of "updatefile" env variable

2011-04-19 Thread Andreas Pretzsch
Am Mittwoch, den 13.04.2011, 21:47 +0200 schrieb Andreas Pretzsch:
> Am Montag, den 11.04.2011, 23:11 +0200 schrieb Wolfgang Denk:
> > In message <1302554333.13241.158.ca...@ws-apr.office.loc> you wrote:
> > > Is there a way to manually trigger an automatic update using a FIT
> > > image, based on the way described in doc/README.update ?
> > 
> > Yes, there is.
> 
> I suppose you meant as soon as there is a new command ?
> 
> 
> > > I mean beside setting "updatefile" environment variable, saveenv and
> > > reboot, but trigger it from U-Boot prompt resp. script ?
> > > 
> > > As far as I can see, update_tftp() is only called once during the
> > > startup, but not referenced by any command.
> > 
> > It requires a two-step approach.  In the first step, implement such a
> > command - patches welcome :-)
> 
> Ok, let's see what I can squeeze in before the merge window closes.
> 
> Browsing the code, I see four ways to implement this, with #1 being my
> favorite. Straightforward and no risk of side effects.
> In all variants, I'm unsure about the command name and hope for
> suggestions.
> 
> 1.) new command calling update_tftp(), optionally with address
> parameter to omit the tftp load

Took this approach, as most easy and non-intrusive. Named the new
command "fitupd", sounds appropriate to me. Feel free to still request a
name change.
See patch series "update from FIT image: add optional address, new
command fitupd".


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] [PATCH 2/2] add command fitupd to run an update from a FIT image

2011-04-19 Thread Andreas Pretzsch
Command calls update_tftp() analogous to automatic update described
in doc/README.update.

Usage:
fitupd [addr]
- run update from FIT image at addr
  or from tftp 'updatefile'

Signed-off-by: Andreas Pretzsch 
---
 common/Makefile |1 +
 common/cmd_fitupd.c |   37 +
 doc/README.update   |5 +
 3 files changed, 43 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_fitupd.c

diff --git a/common/Makefile b/common/Makefile
index 4555716..4b81edc 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -94,6 +94,7 @@ COBJS-$(CONFIG_CMD_FAT) += cmd_fat.o
 COBJS-$(CONFIG_CMD_FDC)$(CONFIG_CMD_FDOS) += cmd_fdc.o
 COBJS-$(CONFIG_OF_LIBFDT) += cmd_fdt.o fdt_support.o
 COBJS-$(CONFIG_CMD_FDOS) += cmd_fdos.o
+COBJS-$(CONFIG_CMD_FITUPD) += cmd_fitupd.o
 COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
 ifdef CONFIG_FPGA
 COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
diff --git a/common/cmd_fitupd.c b/common/cmd_fitupd.c
new file mode 100644
index 000..7da3199
--- /dev/null
+++ b/common/cmd_fitupd.c
@@ -0,0 +1,37 @@
+/*
+ * (C) Copyright 2011
+ * Andreas Pretzsch, carpe noctem engineering, a...@cn-eng.de
+ *
+ * This file is released under the terms of GPL v2 and any later version.
+ * See the file COPYING in the root directory of the source tree for details.
+ */
+
+#include 
+#include 
+
+#if !defined(CONFIG_UPDATE_TFTP)
+#error "CONFIG_UPDATE_TFTP required"
+#endif
+
+extern void update_tftp(ulong addr);
+
+static int do_fitupd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+   ulong addr = 0UL;
+
+   if (argc > 2)
+   return cmd_usage(cmdtp);
+
+   if (argc == 2)
+   addr = simple_strtoul(argv[1], NULL, 16);
+
+   update_tftp(addr);
+   return 0;
+}
+
+U_BOOT_CMD(fitupd, 2, 0, do_fitupd,
+   "update from FIT image",
+   "[addr]\n"
+   "\t- run update from FIT image at addr\n"
+   "\t  or from tftp 'updatefile'"
+);
diff --git a/doc/README.update b/doc/README.update
index 48f03b7..a7f4d9e 100644
--- a/doc/README.update
+++ b/doc/README.update
@@ -51,6 +51,11 @@ the mkimage tool. dtc tool with support for binary includes, 
e.g. in version
 to be prepared. Refer to the doc/uImage.FIT/ directory for more details on FIT
 images.
 
+This mechanism can be also triggered by the commmand "fitupd".
+If an optional, non-zero address is provided as argument, the TFTP transfer
+is skipped and the image at this address is used.
+The fitupd command is enabled by CONFIG_CMD_FITUPD.
+
 
 Example .its files
 --
-- 
1.7.4.1

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


[U-Boot] [PATCH 1/2] automatic update from FIT image: add optional address parameter

2011-04-19 Thread Andreas Pretzsch
Current update_tftp() flow:
  1.) fetch "updatefile" from defined TFTP server
  2.) check if FIT format
  3.) flash contained images

Add an address parameter to update_tftp(). If this address is non-zero,
skip the TFTP transfer and use the image at this address.

Signed-off-by: Andreas Pretzsch 
---
 common/main.c   |4 ++--
 common/update.c |8 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/common/main.c b/common/main.c
index dcbacc9..9b86934 100644
--- a/common/main.c
+++ b/common/main.c
@@ -51,7 +51,7 @@ void inline __show_boot_progress (int val) {}
 void show_boot_progress (int val) __attribute__((weak, 
alias("__show_boot_progress")));
 
 #if defined(CONFIG_UPDATE_TFTP)
-void update_tftp (void);
+void update_tftp (ulong addr);
 #endif /* CONFIG_UPDATE_TFTP */
 
 #define MAX_DELAY_STOP_STR 32
@@ -356,7 +356,7 @@ void main_loop (void)
 #endif /* CONFIG_PREBOOT */
 
 #if defined(CONFIG_UPDATE_TFTP)
-   update_tftp ();
+   update_tftp (0UL);
 #endif /* CONFIG_UPDATE_TFTP */
 
 #if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0)
diff --git a/common/update.c b/common/update.c
index 7528474..531c7d6 100644
--- a/common/update.c
+++ b/common/update.c
@@ -238,14 +238,17 @@ static int update_fit_getparams(const void *fit, int 
noffset, ulong *addr,
return 0;
 }
 
-void update_tftp(void)
+void update_tftp(ulong addr)
 {
char *filename, *env_addr;
int images_noffset, ndepth, noffset;
ulong update_addr, update_fladdr, update_size;
-   ulong addr;
void *fit;
 
+   /* use already present image */
+   if (addr)
+   goto got_update_file;
+
printf("Auto-update from TFTP: ");
 
/* get the file name of the update file */
@@ -271,6 +274,7 @@ void update_tftp(void)
return;
}
 
+got_update_file:
fit = (void *)addr;
 
if (!fit_check_format((void *)fit)) {
-- 
1.7.4.1

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


[U-Boot] [PATCH 0/2] update from FIT image: add optional address, new command fitupd

2011-04-19 Thread Andreas Pretzsch
Create new command "fitupd", so an update from a FIT-image can also be
triggered from U-Boot shell, in addition to the automatic update after
bootup when 'updatefile' is set.
Also provide a way to use a FIT image in memory (e.g. loaded from MMC)
instead of always load from a TFTP server.
See doc/README.update for more information.

Adhere to present coding style => checkpatch warnings discarded.

Andreas Pretzsch (2):
  automatic update from FIT image: add optional address parameter
  add command fitupd to run an update from a FIT image

 common/Makefile |1 +
 common/cmd_fitupd.c |   37 +
 common/main.c   |4 ++--
 common/update.c |8 ++--
 doc/README.update   |5 +
 5 files changed, 51 insertions(+), 4 deletions(-)
 create mode 100644 common/cmd_fitupd.c

-- 
1.7.4.1

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


Re: [U-Boot] [RFC] skip area in flash/memory commands (cp, cmp, ...)

2011-04-13 Thread Andreas Pretzsch
Am Dienstag, den 12.04.2011, 16:15 +0200 schrieb Wolfgang Denk:
> Dear Andreas Pretzsch,
> 
> In message <1302614926.27200.12.ca...@ws-apr.office.loc> you wrote:
> >
> > True, but it would pollute the env with transient variables like
> > fileaddr, filesize and serveraddr. Nothing serious, of course.
> 
> Do you really care to delete these before each and every saveenv?
> Or is your environment so small that you have to worry about 20 or 30
> additional bytes?

Neither. As stated, pure, irrelevant cosmetics.


> > But in case of bootlimit enabled, it would also save "bootcount", which
> > might be a bad idea...
> 
> Why?  It does not matter at all.

ACK. Looked at the code, bootcount is initialized via board-specific
bootcount_load(), not from env. So again, pure cosmetics. Sorry for the
noise.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] trigger automatic update (FIT image) from prompt instead of "updatefile" env variable

2011-04-13 Thread Andreas Pretzsch
Am Montag, den 11.04.2011, 23:11 +0200 schrieb Wolfgang Denk:
> In message <1302554333.13241.158.ca...@ws-apr.office.loc> you wrote:
> > Is there a way to manually trigger an automatic update using a FIT
> > image, based on the way described in doc/README.update ?
> 
> Yes, there is.

I suppose you meant as soon as there is a new command ?


> > I mean beside setting "updatefile" environment variable, saveenv and
> > reboot, but trigger it from U-Boot prompt resp. script ?
> > 
> > As far as I can see, update_tftp() is only called once during the
> > startup, but not referenced by any command.
> 
> It requires a two-step approach.  In the first step, implement such a
> command - patches welcome :-)

Ok, let's see what I can squeeze in before the merge window closes.

Browsing the code, I see four ways to implement this, with #1 being my
favorite. Straightforward and no risk of side effects.
In all variants, I'm unsure about the command name and hope for
suggestions.

1.) new command calling update_tftp(), optionally with address
parameter to omit the tftp load

2.) additional fdt subcommand, like in
tftp fit_image ; fdt addr ${loadaddr} ; fdt flash

3.) "imxtract ${loadaddr} flash" or similar

4.) extend "source" command


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [RFC] skip area in flash/memory commands (cp, cmp, ...)

2011-04-12 Thread Andreas Pretzsch
Am Montag, den 11.04.2011, 23:08 +0200 schrieb Wolfgang Denk:
> > > But if you really want to keep the existing embedded environment when 
> > > flashing a new U-Boot, then after you have loaded the new U-Boot in RAM 
> > > as usual, all you need is to overwrite its environment with the one 
> > > already in Flash with a single, standard, cp.b instruction. After that 
> > > you can erase Flash and copy from RAM to Flash as usual.
> > 
> > ACK. The most obvious and simple approach for this scenario. And
> 
> No. It may be obvious, but there are far easier ways.
> 
> The most simple way is a sequence of tftp, protect off, erase, cp,
> and finally saveenv.

True, but it would pollute the env with transient variables like
fileaddr, filesize and serveraddr. Nothing serious, of course.
But in case of bootlimit enabled, it would also save "bootcount", which
might be a bad idea...

BTW, not sure if it's worth the effort (or a good idea at all), but one
might think about a concept of transient variables (in standard parser,
not checked hush parser).

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [PATCH] gpio: check request result

2011-04-12 Thread Andreas Pretzsch
Am Dienstag, den 12.04.2011, 03:03 -0400 schrieb Mike Frysinger:
> Make sure the pin request passed before attempting to use it later on.
> 
> Signed-off-by: Mike Frysinger 
Tested-by: Andreas Pretzsch 

Verified on Blackfin BF561 with full port range.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [PATCH] gpio: generalize for all generic gpio providers

2011-04-12 Thread Andreas Pretzsch
Am Montag, den 11.04.2011, 23:14 -0400 schrieb Mike Frysinger:
> On Monday, April 11, 2011 15:34:17 Andreas Pretzsch wrote:
> > Am Sonntag, den 03.04.2011, 04:43 -0400 schrieb Mike Frysinger:
> > > + return port_base + simple_strtoul(name, NULL, 10);
> > 
> > Remark: Leads to an oom access when exceeding the processor number of
> > GPIOs, e.g. PF48 on a BF561. IMHO, no problem but only a cosmetic issue,
> > not worth adding an additional per-cpu check.
> > In the end, no difference to other user errors like an invalid memory
> > address.
> 
> err, oom ?  i guess you mean oob ?

bfin> gpio input PF0
gpio: pin PF0 (gpio 0) value is 1
bfin> gpio input pf47
gpio: pin pf47 (gpio 47) value is 1
bfin> gpio input pf48
DCPLB exception outside of memory map at 0x3300
[...]
PANIC: Blackfin internal error


> simple to fix by having the return of gpio_request() get checked ...

Works, thanks. With "[U-Boot] [PATCH] gpio: check request result":

bfin> gpio input PF0
gpio: pin PF0 (gpio 0) value is 1
bfin> gpio input pf47
gpio: pin pf47 (gpio 47) value is 1
bfin> gpio input pf48
gpio: requesting pin 48 failed
bfin> 


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] trigger automatic update (FIT image) from prompt instead of "updatefile" env variable

2011-04-11 Thread Andreas Pretzsch
Is there a way to manually trigger an automatic update using a FIT
image, based on the way described in doc/README.update ?

I mean beside setting "updatefile" environment variable, saveenv and
reboot, but trigger it from U-Boot prompt resp. script ?

As far as I can see, update_tftp() is only called once during the
startup, but not referenced by any command.

Use case: Disaster recovery update via altbootcmd.

Thanks in advance,
  A. Pretzsch

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [RFC] skip area in flash/memory commands (cp, cmp, ...)

2011-04-11 Thread Andreas Pretzsch
Am Montag, den 11.04.2011, 21:31 +0200 schrieb Albert ARIBAUD:
> Le 11/04/2011 20:52, Andreas Pretzsch a écrit :
> > Objective: Skip an area in memory commands
> >
> > Example use case: Skip over embedded environment when updating U-Boot
> >
> > In case of an embedded environment, a complete U-Boot binary consists of
> > two code sections and an environment block in between. In an update flow
> > tftp/erase/cp/cmp conveniently using the complete image, the environment
> > is also overwritten. As the env might contain device specific data, this
> > is not always desirable.
> > Of course, one could overcome this with several approaches, e.g.
> >- provide two distinct U-Boot parts on the tftp server
> >- load the complete image and erase/cp/cmp only the desired parts,
> >  either via hardcoded addresses and sizes or via some setexpr magic
> >- other ways I simply missed
> >
> > While this is sufficent, I'd find it handy to be able to skip an area in
> > the above commands.
> > Possible approaches:
> >- explicit via additional parameters to cp, cmp, etc.
> >- implicit for the embedded environment (confusing and less generic)
> >- based on e.g. some skip_addr, skip_len environment variables or
> >  even sets (addr1,addr2 + len1,len2)
> >
> > There might be other use cases I didn't thought of by now. Also, the
> > command list is probably not complete.
> >
> > What do you think, worth the effort, acceptable in mainline or
> > over-engineering ?
> 
> Over-engineering IMO.
> 
> There is a reason why a board defines its environment as embedded, and 
> it is because it wants it applied when flashing a new U-Boot.

Or simply not to waste flash when having a few small sectors.

> But if you really want to keep the existing embedded environment when 
> flashing a new U-Boot, then after you have loaded the new U-Boot in RAM 
> as usual, all you need is to overwrite its environment with the one 
> already in Flash with a single, standard, cp.b instruction. After that 
> you can erase Flash and copy from RAM to Flash as usual.

ACK. The most obvious and simple approach for this scenario. And
rewriting the environment during an U-Boot update doesn't hurt that much
either, the possible window of data loss is reasonably small.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [PATCH] Blackfin: cmd_gpio: allow port pins over 15

2011-04-11 Thread Andreas Pretzsch
Am Freitag, den 08.04.2011, 16:24 -0400 schrieb Mike Frysinger:
> this is obsoleted by my recent patch:
>   [U-Boot] [PATCH] gpio: generalize for all generic gpio providers
> -mike

ACK. Thanks very much for your work.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [PATCH] gpio: generalize for all generic gpio providers

2011-04-11 Thread Andreas Pretzsch
Am Sonntag, den 03.04.2011, 04:43 -0400 schrieb Mike Frysinger:
> The Blackfin gpio command isn't terribly Blackfin-specific.  So generalize
> the few pieces into two new optional helpers:
>   name_to_gpio() - turn a string name into a GPIO #
>   gpio_status() - display current pin bindings (think /proc/gpio)
> 
> Once these pieces are pulled out, we can relocate the cmd_gpio.c into the
> common directory.
> 
> Signed-off-by: Mike Frysinger 
Tested-by: Andreas Pretzsch 

Verified on Blackfin BF561 with full port range.


> --- a/arch/blackfin/include/asm/gpio.h
> +++ b/arch/blackfin/include/asm/gpio.h
> @@ -196,6 +196,59 @@ static inline int gpio_is_valid(int number)
>   return number >= 0 && number < MAX_BLACKFIN_GPIOS;
>  }
>  
> +#include 
> +
> +static inline int name_to_gpio(const char *name)
> +{
> + int port_base;
> +
> + if (tolower(*name) == 'p') {
> + ++name;
> +
> + switch (tolower(*name)) {
> +#ifdef GPIO_PA0
> + case 'a': port_base = GPIO_PA0; break;
> +#endif
> +#ifdef GPIO_PB0
> + case 'b': port_base = GPIO_PB0; break;
> +#endif
> +#ifdef GPIO_PC0
> + case 'c': port_base = GPIO_PC0; break;
> +#endif
> +#ifdef GPIO_PD0
> + case 'd': port_base = GPIO_PD0; break;
> +#endif
> +#ifdef GPIO_PE0
> + case 'e': port_base = GPIO_PE0; break;
> +#endif
> +#ifdef GPIO_PF0
> + case 'f': port_base = GPIO_PF0; break;
> +#endif
> +#ifdef GPIO_PG0
> + case 'g': port_base = GPIO_PG0; break;
> +#endif
> +#ifdef GPIO_PH0
> + case 'h': port_base = GPIO_PH0; break;
> +#endif
> +#ifdef GPIO_PI0
> + case 'i': port_base = GPIO_PI0; break;
> +#endif
> +#ifdef GPIO_PJ
> + case 'j': port_base = GPIO_PJ0; break;
> +#endif
> + default:  return -1;
> + }
> +
> + ++name;
> + } else
> + port_base = 0;
> +
> + return port_base + simple_strtoul(name, NULL, 10);

Remark: Leads to an oom access when exceeding the processor number of
GPIOs, e.g. PF48 on a BF561. IMHO, no problem but only a cosmetic issue,
not worth adding an additional per-cpu check.
In the end, no difference to other user errors like an invalid memory
address.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] [RFC] skip area in flash/memory commands (cp, cmp, ...)

2011-04-11 Thread Andreas Pretzsch
Objective: Skip an area in memory commands

Example use case: Skip over embedded environment when updating U-Boot

In case of an embedded environment, a complete U-Boot binary consists of
two code sections and an environment block in between. In an update flow
tftp/erase/cp/cmp conveniently using the complete image, the environment
is also overwritten. As the env might contain device specific data, this
is not always desirable.
Of course, one could overcome this with several approaches, e.g.
  - provide two distinct U-Boot parts on the tftp server
  - load the complete image and erase/cp/cmp only the desired parts,
either via hardcoded addresses and sizes or via some setexpr magic
  - other ways I simply missed

While this is sufficent, I'd find it handy to be able to skip an area in
the above commands.
Possible approaches:
  - explicit via additional parameters to cp, cmp, etc.
  - implicit for the embedded environment (confusing and less generic)
  - based on e.g. some skip_addr, skip_len environment variables or
even sets (addr1,addr2 + len1,len2)

There might be other use cases I didn't thought of by now. Also, the
command list is probably not complete.

What do you think, worth the effort, acceptable in mainline or
over-engineering ?


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] MTD partitions not mounted by the kernel

2011-02-10 Thread Andreas Pretzsch
Am Donnerstag, den 10.02.2011, 17:04 +0100 schrieb Alexandre Gambier:
> Dear Wolfgang,
> 
> I tried to put some printk in the MTD driver and it seems that the 
> parse_mtd_partitions function is never called...

parse_mtd_partitions() is called from the mapping drivers. See e.g.
linux/drivers/mtd/maps/ and linux/drivers/mtd/nand/.

The mtd-id provided in kernel-cmdline has to match the name of the
mapping driver, e.g. "physmap-flash" in case of
drivers/mtd/maps/physmap.c.

See linux/drivers/mtd/cmdlinepart.c for the format. Your spec looks
fine, presumably beside "NOR" and "NAND" names.

> I will try to find what's wrong with my kernel configuration.

You'll need CONFIG_MTD_PARTITIONS and CONFIG_MTD_CMDLINE_PARTS.

Also check /proc/cmdline that it's really passed and not overwritten by
hardcoded kernel commandline (CONFIG_CMDLINE_BOOL not set).

> 
> alex
> 
> On 02/10/2011 03:59 PM, Wolfgang Denk wrote:
> > Dear Alexandre Gambier,
> >
> > In message<4d53f9fa.2070...@ftemaximal.fr>  you wrote:
> >> mtdids  : nor0=NOR,nand0=NAND
> > ...
> >> mtdparts=NOR:512k(U-Boot),128k(Environment),4M(Kernel),-(FreeNOR);NAND:32M(FS),-(FreeNAND)
> > ...
> >> The problem is that once my system is running the MTD devices in /dev
> >> are not created and the file /proc/mtd is empty.
> >>
> >> Is my command line wrong ?
> > I think so. Most probably your kernel uses different identifiers
> > instead of "NOR" and "NAND".  Check the kernel boot messages!
> >
> > Best regards,
> >
> > Wolfgang Denk
> >
> _______
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] Blackfin: cmd_gpio port/pin naming

2011-01-10 Thread Andreas Pretzsch
Am Montag, den 10.01.2011, 11:28 -0500 schrieb Mike Frysinger:
> On Mon, Jan 10, 2011 at 10:59 AM, Andreas Pretzsch wrote:
> > As your code already uses the Linux GPIO conventions and naming (without
> > the gpio_chip stuff, which is not necessary for a bootloader IMHO), it'd
> > be a solid base for that.
> >
> > Mr. Denk, are there any plans for a generic GPIO layer in U-Boot ?
> 
> if we arent supporting gpio_chips, then there really isnt much need
> for common GPIO code.  there is already a generic GPIO layer in U-Boot
> just like in Linux -- include asm/gpio.h and use the normal gpio_xxx
> set of functions.

Well, partly. True for Blackfin and a few ARM/AVR32/NIOS2, albeit the
latter have a mixture of specific names like at91_get_gpio_value(),
pin_to_controller(), kw_gpio_get_value(), etc.

Therefore unifying them to common names and syntax would be the first
step. Of course, the gpio_chip approach of Linux is the cleaner way,
adding only a few extra bytes of code/ram. I'm fine with both.
As time permits, I'm willing to help here.

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] [PATCH] Blackfin: cmd_gpio: allow port pins over 15

2011-01-10 Thread Andreas Pretzsch
The pin portion of the specified GPIO was limited to 0..15.
While this is correct for Blackfins with different bank names (e.g.
BF537 with PF, PG, PH with 16 pins each), it's not sufficient for
Blackfins with linear naming like the BF561 (PF0..PF47).
Therefore check only for a valid GPIO number.

Attn.: Passing a # too high for the port will wrap over to the next
port(s), e.g. PA16 => PB0. Not a problem for correct scripts, though.

Signed-off-by: Andreas Pretzsch 
---
 arch/blackfin/cpu/cmd_gpio.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/blackfin/cpu/cmd_gpio.c b/arch/blackfin/cpu/cmd_gpio.c
index e96413b..9fd5a19 100644
--- a/arch/blackfin/cpu/cmd_gpio.c
+++ b/arch/blackfin/cpu/cmd_gpio.c
@@ -83,7 +83,11 @@ int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
 
/* grab the <#> portion */
ulong pin = simple_strtoul(str_pin + 1, NULL, 10);
-   if (pin > 15)
+   /* Attn.: passing a # too high for the port will wrap over to
+* the next port(s), e.g. PA16 => PB0.
+* Check relaxed due to linear ports (e.g. PF0..PF47 @ BF561).
+*/
+   if (!gpio_is_valid(pin))
goto show_usage;
 
/* grab the pin before we tweak it */
-- 
1.7.2.3

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


Re: [U-Boot] Blackfin: cmd_gpio port/pin naming

2011-01-10 Thread Andreas Pretzsch
Am Freitag, den 07.01.2011, 17:38 -0500 schrieb Mike Frysinger: 
> On Friday, January 07, 2011 15:50:30 Andreas Pretzsch wrote:
> > the Blackfin U-Boot GPIO command (see "arch/blackfin/cpu/cmd_gpio.c")
> > specifies the port/pin naming in the form "[p][port]<#>", e.g. PF11.
> > The pin portion of the specified GPIO is limited to 0..15.
> > While this is correct for Blackfins with different bank names (e.g.
> > BF537 with PF, PG, PH), it's not sufficient for Blackfins with linear
> > naming like the BF561 (PF0..PF47).
> 
> if we cut out the friendly port naming, the rest of the code is no longer 
> Blackfin specific.  so what i had been thinking of doing at some point was 
> dropping that as a requirement and making it a "nice" feature so it could be 
> moved into common/cmd_gpio.c for everyone to use.

Sounds like a good idea to me.
As your code already uses the Linux GPIO conventions and naming (without
the gpio_chip stuff, which is not necessary for a bootloader IMHO), it'd
be a solid base for that.

Mr. Denk, are there any plans for a generic GPIO layer in U-Boot ?


> so if we rip out that part and just make it something like:
> ...
> #ifndef name_to_gpio
> #define name_to_gpio(name) simple_strtoul(name, NULL, 10)
> #endif

Personally, I'd go with arch specific functions, following the Linux
codebase, something like:
#ifdef CONFIG_GENERIC_GPIO
#include 
#else
static inline int name_to_gpio(char *name)
{
return simple_strtoul(name, NULL, 10);
}
...
#endif


> ...
> ulong pin = name_to_gpio(argv[2]);
> if (!gpio_is_valid(pin))
>   goto usage;
> ...
> 
> although perhaps in your case, we can just change "if (pin > 15)" to "if 
> (!gpio_is_valid(pin))" and forget about the people who do PF34 on parts that 
> only have PF0..PF15 (they'll instead get like PH4 or whatever).

Would ACK that. Won't break any (correct) scripts out in the field and
solves the BF561 gpio issue with minimal effort.
I'll send a patch the next couple of minutes.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de


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


[U-Boot] Blackfin: cmd_gpio port/pin naming

2011-01-07 Thread Andreas Pretzsch
Dear Mr. Frysinger,

the Blackfin U-Boot GPIO command (see "arch/blackfin/cpu/cmd_gpio.c")
specifies the port/pin naming in the form "[p][port]<#>", e.g. PF11.
The pin portion of the specified GPIO is limited to 0..15.
While this is correct for Blackfins with different bank names (e.g.
BF537 with PF, PG, PH), it's not sufficient for Blackfins with linear
naming like the BF561 (PF0..PF47).

Based on the port name and the pin number, it's transformed into linear
GPIO numbering, which is passed to the GPIO routines (e.g. PG1 @ BF537
=> 16+1).

Therefore simply changing the limit from 15 to MAX_BLACKFIN_GPIOS resp.
MAX_RESOURCES is not an option (for all Blackfins).

I see three possible solutions:
  - Introduce a new define MAX_BLACKFIN_GPIO_PORT_NUMBER or similar
in all arch/blackfin/include/asm/mach-bfxxx/gpio.h and use that
for the check. Name suggestions are welcome.
  - #ifdef the check based on BFxxx_FAMILY resp. CONFIG_BFxxx.
Not sure about the best defines here. And it's ugly.
  - Add another port naming specification to cmd_gpio like "P42".

I'd opt for the first approach, but before submitting a patch, I'd like
to hear your opinion first.

N.B.: Didn't have a closer look at the portmux stuff.

Best regards,
  A. Pretzsch

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] Blackfin: cdef register accessor defines: missing 16-bit variants for SPORT RX/TX

2011-01-04 Thread Andreas Pretzsch
The Blackfin SPORT RX/TX registers have to be accessed as 16-bit or
32-bit only, depending on the configured SPORT data word length.
Everything else leads to an exception.

In the various cdef headers
arch/blackfin/include/asm/mach-bfXXX/BFXXX_cdef.h
there are only 32-bit variants.

The manual states e.g. for SPORTx_TX register:
DAB/PAB writes must match their size to the data word length.
For word length up to and including 16 bits, use a 16-bit write. Use a
32-bit write for word length greater than 16 bits.

Of course, one could use a simple "bfin_write16(SPORT1_TX, val)" instead
of the macros, but I would suggest extending the cdef macros.
The current Linux headers provide them like this:
#define bfin_write_SPORT1_TX(val)  bfin_write32(SPORT1_TX,val)
#define bfin_write_SPORT1_TX32(val)bfin_write32(SPORT1_TX,val)
#define bfin_write_SPORT1_TX16(val)bfin_write16(SPORT1_TX,val)

The U-Boot Blackfin cdef headers seem to be generated externally:
/* DO NOT EDIT THIS FILE
 * Automatically generated by generate-cdef-headers.xsl
 * DO NOT EDIT THIS FILE
 */
Hence I did not prepare a patch.
Mr. Frysinger, I suppose the generator is under your control ?
May I ask you to update it accordingly ?

No idea about the other registers, only came across the SPORT RX/TX.

BTW, as a minor (somewhat cosmetic) issue, there are also read accessors
for the TX register (and vice versa for RX), which is illegal according
to the documentation. Both in Linux and in U-Boot. Admittedly, not a
real issue.

Thanks in advance,
  A. Pretzsch

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


Re: [U-Boot] [RFC] utilize flash small block sizes to reduce flash footprint

2010-09-27 Thread Andreas Pretzsch
Dear Albert,

first, thanks for reading through my RFC. I agree that there are a
couple of simplifications in there, e.g. reset vectors. Written after
about 12 hours of hacking...

I see you're struggling with a comparable, yet a bit more complicated
issue. I have to admit that I only skimmed over the threads, but it
looks to me that most of it could be done also with linkers help.

Personally, I'm fine with a tuned linker file. Preparations are already
there in U-Boot and the real life block allocation is pretty static,
too. So following the KISS principle, I'd go with that approach.

For the specific case of mixed-sector-size flashes with linear
allocation (which is all the RFC was about in the first place), Wolfgang
pointed to embedded environment resp. linker adaption. Which solves the
issue perfectly. Therefore I see "my" case as closed and the RFC as
redundant.

As time permits, I'll have a look at your points again later. But
honestly, the stack on my desk piles up a bit too much right now...
Sorry.

Best regards,
  Andreas


Am Montag, den 27.09.2010, 08:37 +0200 schrieb Albert ARIBAUD:
> Hi Andreas,
> 
> Le 27/09/2010 07:15, Andreas Pretzsch a écrit :
> > In a nit-picking moment trying to save some flash storage, I looked at
> > the current typical flash layout of U-Boot and came up with below RFC.
> >
> > Before I start to implement anything, I'd like to hear your comments
> > about this, especially from the architecture maintainers. Given that you
> > have some spare time to look into this, of course.
> > Sorry in case this was discussed before, did only a coarse list archive
> > search.
> >
> > I'd volunteer to provide an implementation for the architectures I have
> > close at hand today (Blackfin, ARM11), as time permits. For other
> > architectures, I'd need some assistance.
> 
> I am willing to provide some assistance, as I recently posted (then 
> withdrew in order to take Heiko's reloc patches into account) a patch 
> that would maximize use of the topmost flash block for systems that boot 
> at a high address yet fat from the topmost address.
> 
> Find my comments interspersed with your RFC.
> 
> > Abstract
> > 
> >
> > Approach to utilize flashes with different block sizes to reduce U-Boot
> > flash footprint.
> > In general only relevant for boards with environment in flash.
> >
> >
> > Prerequisites
> > -
> >
> > Quite a few NOR flashes are divided into different block sizes.
> > For example, the widespread Intel/Numonyx/Micron P30 series is divided
> > into 4 blocks of 32 kByte, followed by n blocks of 128 kByte. Depending
> > on variant, the smaller blocks are the first blocks (bottom) or the last
> > blocks (top). The bottom variant is more common, as it allows flexible
> > total sizes without a change in the location of the small blocks.
> 
> This is considering about a single case of FLASH block layout. On my 
> systems, I have two cases, a 29LV400 with 1x16, 2x8, 1x32, 7x64 KB 
> layout; and a uniform Spansion s29gl with 64 KB sectors -- and even the 
> uniform case could handle some optimizing.
> 
> > Given a not uncommon U-Boot size of around 128kB (+/- a few 10kB),
> > accompanied by a separate flash block for a configurable environment, it
> > would make sense to make use of this layout.
> >
> > All below statements are focused on NOR flashes like above, with
> > environment in flash. Also things like standalone applications,
> > out-of-binary graphics data, etc. are not taken into account.
> > Hence this RFC, as I probably miss some requirements and/or problems.
> >
> >
> > Flash layouts
> > -
> >
> > Current best-case layout:
> >   32kB U-Boot
> >   32kB U-Boot
> >   32kB U-Boot
> >   32kB U-Boot
> > 128kB U-Boot environment
> > =>  256kB flash used
> >
> > Current worst-case layout:
> >   32kB U-Boot
> >   32kB U-Boot
> >   32kB U-Boot
> >   32kB U-Boot
> > 128kB U-Boot (given U-Boot is>128kB)
> > 128kB U-Boot environment
> > 128kB optional: VPD
> > =>  384kB to 512kB flash used
> >
> > Suggested layout:
> >   32kB U-Boot environment
> >   32kB U-Boot or optional VPD
> >   32kB U-Boot
> >   32kB U-Boot
> > 128kB U-Boot
> > =>  256kB flash used with 192kB/224kB room for U-Boot
> 
> This is actualy what is done by Lacie when using the 29LV400: they put 
> the environment in one of the 8K sectors.
> 
> > VPD: Vital Product Data (board type, MAC addresses, serial number,
> > licenses, certificates, etc.), which are

Re: [U-Boot] [RFC] utilize flash small block sizes to reduce flash footprint

2010-09-27 Thread Andreas Pretzsch
Am Montag, den 27.09.2010, 10:59 +0200 schrieb Wolfgang Denk:
> You design a big and somewhat complicate solution for a problem which

ACK. Had the linker approach also in mind, but didn't want to touch each
and every lds. But definitively the more elegant solution.

> does not exist, because it has already been solved more than a decase
> ago, i. e. right with the very first versions of U-Boot (or PPCBoot, as
> it was called by then).
> 
> We call this feature "embedded environment", and all what it takes to
> use it oin a machine is a somehwat hand-crafted version of the linker
> script which aligns the location of the environmentin the right
> (small) flash sectors.
> 
> This works like a charm, and I recommend you have a look into this.

Sigh. Big brown paper bag for me.

Solves exactly the "issue" I described and works perfectly. And is
documented very well in the README. As usual, things are already solved
in U-Boot. Thanks for the pointer.

Really had blinkers on, no idea why I didn't see it. Respectively, saw
it but somehow mixed it up with hardcoded, read-only environment.
Lession learned: Get some sleep before scribbling senseless RFCs.

Sorry for the noise, case closed.


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de

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


[U-Boot] [RFC] utilize flash small block sizes to reduce flash footprint

2010-09-26 Thread Andreas Pretzsch
In a nit-picking moment trying to save some flash storage, I looked at
the current typical flash layout of U-Boot and came up with below RFC.

Before I start to implement anything, I'd like to hear your comments
about this, especially from the architecture maintainers. Given that you
have some spare time to look into this, of course.
Sorry in case this was discussed before, did only a coarse list archive
search.

I'd volunteer to provide an implementation for the architectures I have
close at hand today (Blackfin, ARM11), as time permits. For other
architectures, I'd need some assistance.


Abstract


Approach to utilize flashes with different block sizes to reduce U-Boot
flash footprint.
In general only relevant for boards with environment in flash.


Prerequisites
-

Quite a few NOR flashes are divided into different block sizes.
For example, the widespread Intel/Numonyx/Micron P30 series is divided
into 4 blocks of 32 kByte, followed by n blocks of 128 kByte. Depending
on variant, the smaller blocks are the first blocks (bottom) or the last
blocks (top). The bottom variant is more common, as it allows flexible
total sizes without a change in the location of the small blocks.

Given a not uncommon U-Boot size of around 128kB (+/- a few 10kB),
accompanied by a separate flash block for a configurable environment, it
would make sense to make use of this layout.

All below statements are focused on NOR flashes like above, with
environment in flash. Also things like standalone applications,
out-of-binary graphics data, etc. are not taken into account.
Hence this RFC, as I probably miss some requirements and/or problems.


Flash layouts
-

Current best-case layout:
 32kB U-Boot
 32kB U-Boot
 32kB U-Boot
 32kB U-Boot
128kB U-Boot environment
=> 256kB flash used

Current worst-case layout:
 32kB U-Boot
 32kB U-Boot
 32kB U-Boot
 32kB U-Boot
128kB U-Boot (given U-Boot is >128kB)
128kB U-Boot environment
128kB optional: VPD
=> 384kB to 512kB flash used

Suggested layout:
 32kB U-Boot environment
 32kB U-Boot or optional VPD
 32kB U-Boot
 32kB U-Boot
128kB U-Boot
=> 256kB flash used with 192kB/224kB room for U-Boot

VPD: Vital Product Data (board type, MAC addresses, serial number,
licenses, certificates, etc.), which are not to be (solely) saved in
U-Boot environment, for whatever reason.


Current status
--

A quick grep indicates that most, if not all boards locate U-Boot at the
start of the flash.

The U-Boot environment is written to the start of the configured flash
block. Prior to that, this block is erased, its content is lost.


Possible approach
-

Most CPUs start execution at flash address 0 or somewhere in this region
at the reset ISR vector.

In case U-Boot is not located there but some flash sectors later, at
this reset vector there has to be some architecture specific code block,
which could be a
  - simple jump to U-Boot entry
  - dummy ISR table with reset vector pointing to U-Boot
  - specific trampoline code
  - ...

In other words, a few bytes (up to e.g. 1kB) of (static) binary data,
generated at compile time.

This has to be put before the environment, either implicit or included
at the start of "struct environment_s".
The latter would of course break backward compatibility, at least
without some additional code.

The variant of including this code into a specific block would again
waste a flash block resp. mandate the presence of a VPD or similar.


Things to define / check


  - U-Boot entry point on non-PIC builds
(U-Boot start != CONFIG_SYS_FLASH_BASE)
  - U-Boot on architectures not capable of a single jump forward
by e.g. 32kB or 64kB
  - redundant environment
  - mixed flash/external environment
  - ...


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch  Tel. +49-(0)731-5521572
Hahnengasse 3 Fax: +49-(0)731-5521573
89073 Ulm, Germanyemail: a...@cn-eng.de


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


Re: [U-Boot] smc911x: problem with if there is ethernet traffic

2009-10-26 Thread Andreas Pretzsch
Am Monday 26 October 2009 12:35:14 schrieb Raffaele Recalcati:
 > I've customized the commit f67066b6b0740b826ed862615c5ab022aaf4779a
 > for my pxa255/smx911x (lan9118) board.
 > tftp works nice, also through a switch, but only if I'm not
 > connected to the LAN.

On rare occasions, I observe sporadic tx problems (collisions, etc.) 
with a 9221 over here. No idea if it's the smc driver or (more likely) 
some other problem with the board. Found by accident that fiddling with 
speed and/or half-/full-duplex settings of the tftp server expelled 
them typically. Had no time to look into this by now, just throwing in 
another bit of confusion ;-)


-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch Tel. 0731/98588-800
Marlene-Dietrich-Strasse 5   Fax: 0731/98588-801
89231 Neu-Ulm, Germany   email: a...@cn-eng.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] Blackfin: UART on SPORT

2009-09-23 Thread Andreas Pretzsch
In Linux, there is support for a UART on a SPORT, see 
linux-2.6.x/drivers/serial/bfin_sport_uart.[ch].
It can also be used as console.

In U-Boot, I cannot find similar code, therefore I'd like to ask if 
anybody ported/implemented this for U-Boot already ?

If not, is there any objection not to port this code to U-Boot, given 
that someone (which might be me) has some spare time for this ?


Thanks in advance,
  A. Pretzsch

-- 

carpe noctem engineering
Ingenieurbuero fuer Hard- & Software-Entwicklung Andreas Pretzsch
Dipl.-Ing. (FH) Andreas Pretzsch Tel. 0731/98588-800
Marlene-Dietrich-Strasse 5   Fax: 0731/98588-801
89231 Neu-Ulm, Germany   email: a...@cn-eng.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] smc911x: add support for LAN9221

2009-07-09 Thread Andreas Pretzsch
Signed-off-by: Andreas Pretzsch 
---
 drivers/net/smc911x.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h
index 80d2ce0..c14003c 100644
--- a/drivers/net/smc911x.h
+++ b/drivers/net/smc911x.h
@@ -382,6 +382,7 @@ static inline void smc911x_reg_write(u32 addr, u32 val)
 #define CHIP_9216  0x116a
 #define CHIP_9217  0x117a
 #define CHIP_9218  0x118a
+#define CHIP_9221  0x9221
 
 struct chip_id {
u16 id;
@@ -398,6 +399,7 @@ static const struct chip_id chip_ids[] =  {
{ CHIP_9216, "LAN9216" },
{ CHIP_9217, "LAN9217" },
{ CHIP_9218, "LAN9218" },
+   { CHIP_9221, "LAN9221" },
{ 0, NULL },
 };
 
-- 
1.6.3.3

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