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 ;).
> >> --- 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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
