public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Eric Nelson <eric.nelson@boundarydevices.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V3 4/6] sf command: allow default chip select through CONFIG_SPI_FLASH_CS
Date: Thu, 26 Jan 2012 18:22:22 -0700	[thread overview]
Message-ID: <4F21FC4E.1090400@boundarydevices.com> (raw)
In-Reply-To: <201201241308.07644.vapier@gentoo.org>

On 01/24/2012 11:08 AM, Mike Frysinger wrote:
> On Tuesday 24 January 2012 11:18:22 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
>
> NAK (to this version of the patch): missing README update, and other issues
> below
>

Which README? The only references I find to serial flash support
are in board-specific README files.

~/u-boot$ find . -iname \*readme\* | xargs grep -w sf
./doc/README.p2041rdb:	=> sf erase 0 100000
./doc/README.p2041rdb:	=> sf write 1000000 0 $filesize
./doc/README.p2041rdb:	=> sf erase 110000 10000
./doc/README.p2041rdb:	=> sf write 1000000 110000 $filesize
./doc/README.sh7757lcr:   => sf probe 0
./doc/README.sh7757lcr:   => sf erase 0 80000
./doc/README.sh7757lcr:   => sf write 0x89000000 0 80000

I can start one of those for the SabreLite board, but that's un-related
to this patch.

>> --- a/common/cmd_sf.c
>> +++ b/common/cmd_sf.c
>>
>> +#ifndef CONFIG_SPI_FLASH_CS
>> +	if (argc<  2) {
>> +		printf("%s: missing arguments\n", __func__);
>>   		return -1;
>
> 	return cmd_usage(cmdtp);
>
>> -	if (*endp == ':') {
>> -		if (endp[1] == 0)
>> -			return -1;
>> +	}
>> +#else
>> +	cs = CONFIG_SPI_FLASH_CS ;
>> +#endif
>
> you're setting the default CS, not locking it in.  so a better config knob name
> would be something like:
> 	CONFIG_SF_DEFAULT_CS
> this matches the existing CONFIG_SF_XXX defines
>
> also, you have a spurious space before the semicolon there
>
Thanks Mike,

FWIW, I chose this name on purpose to make life easier on a couple of
other boards immediately (efika and vision2):

~/u-boot$ grep CONFIG_SPI_FLASH_CS include/configs/*
include/configs/efikamx.h:#define CONFIG_SPI_FLASH_CS		(1 | 121 << 8)
include/configs/m28evk.h:#define	CONFIG_SPI_FLASH_CS		2
include/configs/mx6qsabrelite.h.rej: 	#define CONFIG_SPI_FLASH_CS	1
include/configs/vision2.h:#define CONFIG_SPI_FLASH_CS	(1 | (121 << 8))

>>   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"
>
> this is ugly.  i'd rather just omit it and not worry about the syntax being
> perfect.

Works for me.

  reply	other threads:[~2012-01-27  1:22 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-24 16:18 [U-Boot] [PATCH V3 0/6] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
2012-01-24 16:18 ` [U-Boot] [PATCH V3 1/6] mxc_spi: move machine specifics into CPU headers Eric Nelson
2012-01-24 16:18 ` [U-Boot] [PATCH V3 2/6] mx6q: Add support for ECSPI through mxc_spi driver Eric Nelson
2012-01-24 16:18 ` [U-Boot] [PATCH V3 3/6] mx6q: mx6qsabrelite: Add ECSPI support to the Sabrelite platform Eric Nelson
2012-01-24 16:18 ` [U-Boot] [PATCH V3 4/6] sf command: allow default chip select through CONFIG_SPI_FLASH_CS Eric Nelson
2012-01-24 16:27   ` Fabio Estevam
2012-01-24 18:08   ` Mike Frysinger
2012-01-27  1:22     ` Eric Nelson [this message]
2012-01-27  2:50       ` Mike Frysinger
2012-01-27 13:39         ` Eric Nelson
2012-01-25 15:10   ` Matthias Fuchs
2012-01-25 19:10     ` Mike Frysinger
2012-01-27  1:36     ` Eric Nelson
2012-01-27  2:52       ` Mike Frysinger
2012-01-24 16:18 ` [U-Boot] [PATCH V3 5/6] mx6q: mx6qsabrelite: Provide default chip-select for serial flash Eric Nelson
2012-01-24 16:18 ` [U-Boot] [PATCH V3 6/6] mx6q: mx6qsabrelite: Conditionally define macros for environment in " Eric Nelson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F21FC4E.1090400@boundarydevices.com \
    --to=eric.nelson@boundarydevices.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox