[U-Boot] [PATCH v3 17/17] sf: Add Flag status register polling support

[Simon Glass]

Hi Jagan,

On Thu, Jun 13, 2013 at 10:46 AM, Willis, Max <Max.Willis at spansion.com>wrote:

> Hi Jagan, Simon - Adding Bacem and Gernot from Spansion. Would you be able
> to add them to the other open threads as well? --Max
>
> -------------------------------------------------
> Max Willis, PhD
> Division Marketing Manager
> Software Marketing
> Spansion Inc.
> -------------------------------------------------
>
> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: Wednesday, June 12, 2013 10:32 PM
> To: Simon Glass
> Cc: Jagannadha Sutradharudu Teki; U-Boot Mailing List; Michal Simek; Tom
> Rini; Rajeshwari Shinde; Syed Hussain; Willis, Max; Todd Legler (tlegler);
> Jagannadha Sutradharudu Teki
> Subject: Re: [PATCH v3 17/17] sf: Add Flag status register polling support
>
> Hi Simon,
>
> On Thu, Jun 13, 2013 at 4:12 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:
> >>
> >> Flag status register polling is required for micron 512Mb flash
> >> devices onwards, for performing erase/program operations.
> >>
> >> Like polling for WIP(Write-In-Progress) bit in read status register,
> >> spi_flash_cmd_wait_ready will poll for PEC(Program-Erase-Control) bit
> >> in flag status register.
> >>
> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
> >>
> >> ---
> >> Changes for v3:
> >>         - define the flag status code on CONFIG_SPI_FLASH_STMICRO
> >> Changes for v2:
> >>         - none
> >>
> >>  drivers/mtd/spi/spi_flash.c          | 16 +++++++++++++---
> >>  drivers/mtd/spi/spi_flash_internal.h |  2 ++
> >>  2 files changed, 15 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi/spi_flash.c
> >> b/drivers/mtd/spi/spi_flash.c index bb37f54..4f97fab0 100644
> >> --- a/drivers/mtd/spi/spi_flash.c
> >> +++ b/drivers/mtd/spi/spi_flash.c
> >> @@ -200,12 +200,22 @@ int spi_flash_cmd_wait_ready(struct spi_flash
> >> *flash, unsigned long timeout)
> >>         unsigned long timebase;
> >>         int ret;
> >>         u8 status;
> >> +       u8 check_status = 0x0;
> >>         u8 poll_bit = STATUS_WIP;
> >>         u8 cmd = CMD_READ_STATUS;
> >>
> >> +#ifdef CONFIG_SPI_FLASH_STMICRO
> >> +       if (flash->size >= 0x4000000) {
> >> +               poll_bit = STATUS_PEC;
> >> +               check_status = poll_bit;
> >> +               cmd = CMD_FLAG_STATUS;
> >> +       }
> >> +#endif
> >> +
> >
> >
> > Similarly to the bank addressing I think this might be better done as
> > a 'poll method' rather than a hack for a particular flash type.
> >
>
> You mean to say read_status cmd and flag_status cmd need to find it on
> probe it self.?
> Can you please explain "better done as a 'poll method' '
>

What I mean is that spi_flash_cmd_wait_ready() is in generic code.

CONFIG_SPI_FLASH_STMICRO means that U-Boot will support that flash, but it
does not mean that it is the only flash. You have added a device-specific
thing into generic code. If someone compiles a U-Boot with ST and Winbond
flash, then both types of flash will use the code you added (the ST code
above).


>
> > Also I worry that in a system with more than one type of flash, this
> > code will fire for all flash types.
>
> Even if the system has multiple flashes of different size, we can probe
> once at a time, right?
>

> --
> Thanks,
> Jagan.
>


Regards,
Simon