[U-Boot] [PATCH v2 16/16] sf: Add Flag status register polling support
Simon Glass
sjg at chromium.org
Sun Jun 9 16:13:36 CEST 2013
Hi Jagan,
On Sat, Jun 8, 2013 at 7:44 AM, Jagan Teki <jagannadh.teki at gmail.com> wrote:
> Hi Simon,
>
> On Sat, Jun 8, 2013 at 8:02 PM, Simon Glass <sjg at chromium.org> wrote:
> > Hi Jagan,
> >
> > On Sat, Jun 8, 2013 at 1:32 AM, Jagan Teki <jagannadh.teki at gmail.com>
> wrote:
> >>
> >> Hi Simon,
> >>
> >> Please let know your comments.
> >>
> >> I have changed the logic, but removed spi_flash_cmd_poll_bit() use
> >> poll code on spi_flash_cmd_wait_ready()
> >> as no other call for spi_flash_cmd_poll_bit() this.
> >>
> >> And also for read_status the check_status i assigned as 0,earlier it
> >> has direct 0 (w/o check_status variable).
> >>
> >> To add the support for flag status on the same code, i define this
> >> check_status.
> >> I don't see any coding functionality change for now, compared to before.
> >
> >
> > This is not the right patch, but in one of them you remove
> > spi_flash_cmd_poll_bit(), so that it no longer works the same way. You
> will
> > get lots of individual SPI transactions on the bus instead of a single
> one
> > that reads the status byte continuously.
>
> I couldn't understand you clearly.
>
> Earlier all erase/write they were calling spi_flash_cmd_wait_ready
> with flash, timeout values.
> spi_flash_cmd_wait_ready() is intern call to spi_flash_cmd_poll_bit()
> with CMD_READ_STATUS and STATUS_WIP.
>
> In my changes I have removed spi_flash_cmd_poll_bit() as there is no
> other caller except spi_flash_cmd_wait_ready()
> so all polling stuff on spi_flash_cmd_wait_ready() means, still
> erase/program can call spi_flash_cmd_wait_ready() with
> flash, timeout and CMD_READ_STATUS and STAUS_WIP were use directly
> used on spi_flash_cmd_wait_ready() instead
> of passing through spi_flash_cmd_poll_bit().
>
> Any wrong here, please comment.
>
You have changed the implementation of spi_flash_cmd_wait_ready() so that
instead of polling for the bit within a single SPI transaction (CS staying
asserted) you are now doing multiple transactions.
The old code for spi_flash_cmd_wait_ready() ended up with an algorithm like
this, when you look inside the nested functions:
spi_claim_bus(spi);
ret = spi_xfer(spi, cmd_len * 8, cmd, NULL, SPI_XFER_BEGIN);
do {
ret = spi_xfer(spi, 8, NULL, &status, 0);
...
}
spi_xfer(spi, 0, NULL, NULL, SPI_XFER_END);
spi_release_bus(spi);
The new code is:
do {
spi_claim_bus(spi);
ret = spi_xfer(spi, cmd_len * 8, cmd, NULL, SPI_XFER_BEGIN);
ret = spi_xfer(spi, data_len * 8, data_out, data_in, SPI_XFER_END);
spi_release_bus(spi);
...
} while (...)
Can you see the difference? The bus clain and SPI_XFER_BEGIN/END is now
inside the loop.
Is there a reason why this change is needed? Does updating the bank address
not work with the old code?
It is find to move the spi_flash_cmd_poll_bit() function into
spi_flash_cmd_wait_ready() - I am not worried about that. I am only worried
about your change of algorithm as above.
Regards,
Simon
>
> Thanks,
> Jagan.
>
> >
> > Do we need to change this?
> >
> >
> >>
> >>
> >> --
> >> Thanks,
> >> Jagan.
> >>
> >> On Fri, May 31, 2013 at 6:22 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 v2:
> >> > - none
> >> >
> >> > drivers/mtd/spi/spi_flash.c | 15 ++++++++++++---
> >> > drivers/mtd/spi/spi_flash_internal.h | 3 +++
> >> > 2 files changed, 15 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> >> > index 527423d..8cd2988 100644
> >> > --- a/drivers/mtd/spi/spi_flash.c
> >> > +++ b/drivers/mtd/spi/spi_flash.c
> >> > @@ -195,25 +195,34 @@ 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;
> >> >
> >> > + if ((flash->idcode0 == 0x20) &&
> >> > + (flash->size >= SPI_FLASH_512MB_STMIC)) {
> >> > + poll_bit = STATUS_PEC;
> >> > + check_status = poll_bit;
> >> > + cmd = CMD_FLAG_STATUS;
> >> > + }
> >> > +
> >> > timebase = get_timer(0);
> >> > do {
> >> > WATCHDOG_RESET();
> >> >
> >> > ret = spi_flash_read_common(flash, &cmd, 1, &status,
> 1);
> >> > if (ret < 0) {
> >> > - debug("SF: fail to read read status
> >> > register\n");
> >> > + debug("SF: fail to read %s status register\n",
> >> > + cmd == CMD_READ_STATUS ? "read" :
> >> > "flag");
> >> > return ret;
> >> > }
> >> >
> >> > - if ((status & poll_bit) == 0)
> >> > + if ((status & poll_bit) == check_status)
> >> > break;
> >> >
> >> > } while (get_timer(timebase) < timeout);
> >> >
> >> > - if ((status & poll_bit) == 0)
> >> > + if ((status & poll_bit) == check_status)
> >> > return 0;
> >> >
> >> > /* Timed out */
> >> > diff --git a/drivers/mtd/spi/spi_flash_internal.h
> >> > b/drivers/mtd/spi/spi_flash_internal.h
> >> > index ac4530f..cb7a505 100644
> >> > --- a/drivers/mtd/spi/spi_flash_internal.h
> >> > +++ b/drivers/mtd/spi/spi_flash_internal.h
> >> > @@ -13,6 +13,7 @@
> >> > #define SPI_FLASH_SECTOR_ERASE_TIMEOUT (10 * CONFIG_SYS_HZ)
> >> >
> >> > #define SPI_FLASH_16MB_BOUN 0x1000000
> >> > +#define SPI_FLASH_512MB_STMIC 0x4000000
> >> >
> >> > /* Common commands */
> >> > #define CMD_READ_ID 0x9f
> >> > @@ -24,6 +25,7 @@
> >> > #define CMD_PAGE_PROGRAM 0x02
> >> > #define CMD_WRITE_DISABLE 0x04
> >> > #define CMD_READ_STATUS 0x05
> >> > +#define CMD_FLAG_STATUS 0x70
> >> > #define CMD_WRITE_ENABLE 0x06
> >> > #define CMD_ERASE_4K 0x20
> >> > #define CMD_ERASE_32K 0x52
> >> > @@ -38,6 +40,7 @@
> >> >
> >> > /* Common status */
> >> > #define STATUS_WIP 0x01
> >> > +#define STATUS_PEC 0x80
> >> >
> >> > /* Send a single-byte command to the device and read the response */
> >> > int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response,
> size_t
> >> > len);
> >> > --
> >> > 1.8.3
> >>
>
More information about the U-Boot
mailing list