On 01/25/2012 08:10 AM, Matthias Fuchs wrote:
Hi Eric,
please see my comments below.
On 24.01.2012 17:18, Eric Nelson wrote:
This patch allows a board configuration file to provide a default
chip-select for serial flash so that first argument to the 'sf' command
is optional.
On boards that use the mxc_spi driver and a GPIO for chip select, this allows
a much simpler command line:
U-Boot> sf probe
instead of
U-Boot> sf probe 0x5300
Signed-off-by: Eric Nelson<eric.nel...@boundarydevices.com>
Acked-by: Dirk Behme<dirk.be...@de.bosch.com>
Acked-by: Stefano Babic<sba...@denx.de>
---
common/cmd_sf.c | 34 +++++++++++++++++++++++-----------
1 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 7225656..4b32171 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -70,20 +70,28 @@ static int do_spi_flash_probe(int argc, char * const argv[])
char *endp;
struct spi_flash *new;
- if (argc< 2)
- return -1;
-
- cs = simple_strtoul(argv[1],&endp, 0);
- if (*argv[1] == 0 || (*endp != 0&& *endp != ':'))
+#ifndef CONFIG_SPI_FLASH_CS
+ if (argc< 2) {
+ printf("%s: missing arguments\n", __func__);
I think this format for the error message is a little bit untypical for
u-boot. We do not show up the internal C function name. Better would be
to show the command usage, right?
Looking at this area of the code in more detail, there are quite
a few cases where improper usage silently return -1.
I'm inclined to either follow that lead or toss them together
with a "goto usage" as done in do_spi_flash().
Any preferences?
return -1;
- if (*endp == ':') {
- if (endp[1] == 0)
- return -1;
+ }
+#else
+ cs = CONFIG_SPI_FLASH_CS ;
Other options for the spi flash subsystem are called
CONFIG_SF_DEFAULT_MODE|SPEED. It think it make sense to call
this CONFIG_SF_DEFAULT_CS and CONFIG_SF_DEFAULT_BUS (see below).
See include/configs/efikamx.h (reference to unused symbol CONFIG_SPI_FLASH_CS).
+#endif
- bus = cs;
- cs = simple_strtoul(endp + 1,&endp, 0);
- if (*endp != 0)
+ if (argc>= 2) {
+ cs = simple_strtoul(argv[1],&endp, 0);
+ if (*argv[1] == 0 || (*endp != 0&& *endp != ':'))
return -1;
+ if (*endp == ':') {
+ if (endp[1] == 0)
+ return -1;
+
+ bus = cs;
+ cs = simple_strtoul(endp + 1,&endp, 0);
+ if (*endp != 0)
+ return -1;
+ }
}
if (argc>= 3) {
@@ -299,7 +307,11 @@ usage:
U_BOOT_CMD(
sf, 5, 1, do_spi_flash,
"SPI flash sub-system",
+#ifndef CONFIG_SPI_FLASH_CS
"probe [bus:]cs [hz] [mode] - init flash device on given SPI
bus\n"
+#else
+ "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI
bus\n"
+#endif
" and chip select\n"
"sf read addr offset len - read `len' bytes starting at\n"
" `offset' to memory at `addr'\n"
Can you also add a config option for the SPI bus number? I think these
two need to handled in the same patch.
So you could add this stuff:
diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 9e97c8e..fa4312a 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -63,7 +63,11 @@ static int sf_parse_len_arg(char *arg, ulong *len)
static int do_spi_flash_probe(int argc, char * const argv[])
{
+#ifdef CONFIG_SF_DEFAULT_BUS
+ unsigned int bus = CONFIG_SF_DEFAULT_BUS;
+#else
unsigned int bus = 0;
+#endif
unsigned int cs;
unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
unsigned int mode = CONFIG_SF_DEFAULT_MODE;
Can do.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
