[U-Boot] [PATCH v2 6/7] sf: Update status reg check in spi_flash_cmd_wait_ready

[Jagan Teki]

On 19 August 2015 at 03:32, Marek Vasut <marex at denx.de> wrote:
> On Monday, August 17, 2015 at 12:32:53 PM, Jagan Teki wrote:
>> Current flash wait_ready logic is not modular to add new
>> register status check, hence few of the logic is used from
>> Linux spi-nor framework.
>>
>> Below are the sf speed runs with 'sf update' on whole flash, 16MiB.
>>
>> => sf update 0x100 0x0 0x1000000
>> device 0 whole chip
>> 16777216 bytes written, 0 bytes skipped in 59.564s, speed 289262 B/s
>>
>> => sf update 0x100 0x0 0x1000000
>> device 0 whole chip
>> 16777216 bytes written, 0 bytes skipped in 62.549s, speed 275036 B/s
>>
>> => sf update 0x100 0x0 0x1000000
>> device 0 whole chip
>> 16777216 bytes written, 0 bytes skipped in 61.276s, speed 284359 B/s
>>
>> Signed-off-by: Jagan Teki <jteki at openedev.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Marek Vasut <marex at denx.de>
>> Cc: Michal Simek <michal.simek at xilinx.com>
>> Cc: Siva Durga Prasad Paladugu <sivadur at xilinx.com>
>> Cc: Stefan Roese <sr at denx.de>
>> Cc: Tom Warren <twarren at nvidia.com>
>> Cc: Bin Meng <bmeng.cn at gmail.com>
>> Cc: Tom Rini <trini at konsulko.com>
>> Tested-by: Jagan Teki <jteki at openedev.com>
>> ---
>>  drivers/mtd/spi/sf_ops.c | 76
>> ++++++++++++------------------------------------ 1 file changed, 18
>> insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> index 0f34450..7d7c264 100644
>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -138,72 +138,32 @@ static void spi_flash_dual_flash(struct spi_flash
>> *flash, u32 *addr) }
>>  #endif
>>
>> -static int spi_flash_poll_status(struct spi_slave *spi, unsigned long
>> timeout, -                             u8 cmd, u8 poll_bit)
>> +/*
>> + * Service routine to read status register until ready, or timeout occurs.
>> + * Returns non-zero if error.
>
> If you're adding this sort of description, please make it a kerneldoc.

Probably I will remove these comments, there are no comments before.

>
>> + */
>> +int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
>> deadline) {
>
> Please rename $deadline to $timeout and note in the kerneldoc that the
> timeout is in mSec.

Yes, will revert back to timeout

>
>> -     unsigned long timebase;
>> -     unsigned long flags = SPI_XFER_BEGIN;
>> -     int ret;
>> -     u8 status;
>> -     u8 check_status = 0x0;
>> +     int timeout, ret, sr;
>>
>> -     if (cmd == CMD_FLAG_STATUS)
>> -             check_status = poll_bit;
>> +     timeout = get_timer(0);
>
> This should be $start or something.
>
>> -#ifdef CONFIG_SF_DUAL_FLASH
>> -     if (spi->flags & SPI_XFER_U_PAGE)
>> -             flags |= SPI_XFER_U_PAGE;
>> -#endif
>> -     ret = spi_xfer(spi, 8, &cmd, NULL, flags);
>> -     if (ret) {
>> -             debug("SF: fail to read %s status register\n",
>> -                   cmd == CMD_READ_STATUS ? "read" : "flag");
>> -             return ret;
>> -     }
>> -
>> -     timebase = get_timer(0);
>> -     do {
>> -             WATCHDOG_RESET();
>> -
>> -             ret = spi_xfer(spi, 8, NULL, &status, 0);
>> -             if (ret)
>> -                     return -1;
>> +     while (get_timer(timeout) < deadline) {
>> +             ret = spi_flash_cmd_read_status(flash, &sr);
>> +             if (ret < 0)
>> +                     return ret;
>>
>> -             if ((status & poll_bit) == check_status)
>> +             if (sr < 0)
>>                       break;
>> +             else if (!(sr & STATUS_WIP))
>> +                     return 0;
>>
>> -     } while (get_timer(timebase) < timeout);
>> -
>> -     spi_xfer(spi, 0, NULL, NULL, SPI_XFER_END);
>> -
>> -     if ((status & poll_bit) == check_status)
>> -             return 0;
>> -
>> -     /* Timed out */
>> -     debug("SF: time out!\n");
>> -     return -1;
>> -}
>> -
>> -int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
>> timeout) -{
>> -     struct spi_slave *spi = flash->spi;
>> -     int ret;
>> -     u8 poll_bit = STATUS_WIP;
>> -     u8 cmd = CMD_READ_STATUS;
>> -
>> -     ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
>> -     if (ret < 0)
>> -             return ret;
>> -
>> -     if (flash->poll_cmd == CMD_FLAG_STATUS) {
>> -             poll_bit = STATUS_PEC;
>> -             cmd = CMD_FLAG_STATUS;
>> -             ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
>> -             if (ret < 0)
>> -                     return ret;
>> +             cond_resched();
>
> Huh ? Why do you need cond_resched() here at all ? Drop it please.

Yes will do.

>
>>       }
>>
>> -     return 0;
>> +     printf("SF: Timeout!\n");
>> +
>> +     return -ETIMEDOUT;
>>  }

thanks!
-- 
Jagan | openedev.