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? > 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). > +#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; Matthias _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot