Hi Jagan,
On Sat, Jun 8, 2013 at 7:44 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Simon,
On Sat, Jun 8, 2013 at 8:02 PM, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On Sat, Jun 8, 2013 at 1:32 AM, Jagan Teki jagannadh.teki@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@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@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 statusregister\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