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
