From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eric Nelson <eric.nelson@boundarydevices.com>
Date: Fri, 27 Jan 2012 06:39:06 -0700
Subject: [U-Boot] [PATCH V3 4/6] sf command: allow default chip select
 through CONFIG_SPI_FLASH_CS
In-Reply-To: <201201262150.28632.vapier@gentoo.org>
References: <1327421904-21487-1-git-send-email-eric.nelson@boundarydevices.com>
	<201201241308.07644.vapier@gentoo.org>
	<4F21FC4E.1090400@boundarydevices.com>
	<201201262150.28632.vapier@gentoo.org>
Message-ID: <4F22A8FA.5030304@boundarydevices.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 01/26/2012 07:50 PM, Mike Frysinger wrote:
> On Thursday 26 January 2012 20:22:22 Eric Nelson wrote:
>> 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.
>
> all new CONFIG_xxx defines should be documented in the top level README.
> granted, the existin SF ones have slipped in without being documented, but
> that's bad for them ;).
>

Ok. I'll re-send with README updates.

>>>> --- a/common/cmd_sf.c
>>>> +++ b/common/cmd_sf.c
>>>>
>>>> -	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):
>
> those boards are dumb -- that define isn't used anywhere, so that's just dead
> code.  cmd_sf has a standard already, so new defines specific to cmd_sf should
> follow that.
> -mike

Ok. I'll let their maintainers update appropriately.