[U-Boot] [PATCH v3 01/17] sf: Add bank address register writing support

[Tom Rini]

On Thu, Jun 13, 2013 at 08:23:13AM +0530, Jagan Teki wrote:
> Hi Simon,
> 
> On Thu, Jun 13, 2013 at 3:48 AM, Simon Glass <sjg at chromium.org> wrote:
> > Hi Jagan,
> >
> > On Tue, Jun 11, 2013 at 12:23 PM, Jagannadha Sutradharudu Teki
> > <jagannadha.sutradharudu-teki at xilinx.com> wrote:
> >>
> >> This patch provides support to program a flash bank address
> >> register.
> >>
> >> extended/bank address register contains an information to access
> >> the 4th byte addressing in 3-byte address mode.
> >>
> >> Currently added an bank address register writing support for
> >> spansion flashes.
> >>
> >> reff' the spec for more details about bank addr register
> >> in Page-63, Table 8.16
> >> http://www.spansion.com/Support/Datasheets/S25FL128S_256S_00.pdf
> >>
> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
> >
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> > See comment below, perhaps a follow-on patch so you can get this in first?
> >
> >>
> >> ---
> >> Changes for v3:
> >>         - none
> >> Changes for v2:
> >>         - none
> >>
> >>  drivers/mtd/spi/spi_flash.c          | 37
> >> ++++++++++++++++++++++++++++++++++++
> >>  drivers/mtd/spi/spi_flash_internal.h |  6 ++++++
> >>  include/spi_flash.h                  |  2 ++
> >>  3 files changed, 45 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> >> index 0e38f59..7aba520 100644
> >> --- a/drivers/mtd/spi/spi_flash.c
> >> +++ b/drivers/mtd/spi/spi_flash.c
> >> @@ -278,6 +278,40 @@ int spi_flash_cmd_write_status(struct spi_flash
> >> *flash, u8 sr)
> >>         return 0;
> >>  }
> >>
> >> +int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8 bank_sel)
> >> +{
> >> +       u8 cmd, idcode0;
> >> +       int ret;
> >> +
> >> +       idcode0 = flash->idcode0;
> >> +       if (idcode0 == 0x01) {
> >
> >
> > I wonder why you need this to be hard-coded here? As a follow-on patch, I
> > suggest you have something in the probe function which decides which bank
> > addressing algorithm to use (e.g. winbond, ST, none). and then this code can
> > be:
> >
> > switch (flash->bank_addr_algo) {
> > case SF_ALGO_WINBOND:
> >    ...
> > case SF_ALGO_ST:
> >   ...
> > }
> 
> Means in probe time we can assign particular bank commands something like
> 
> switch (*idp) {
> case SF_WINBOND_IDCODE0:
>      flash->bank_wr_cmd = ;
>      flash->bank_rd_cmd = ;
> case SF_SPAN_IDCODE0:
>     flash->bank_wr_cmd = ;
>     flash->bank_rd_cmd = ;
> }
> 
> Please let me know your comments.

Correct, and similar to how some of the filesystem code works.  Looks
good to me.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130613/4f608164/attachment.pgp>