[U-Boot] [PATCH v3 07/13] sf: Add FSR support to spi_flash_cmd_wait_ready

[Jagan Teki]

On 2 September 2015 at 13:30, Marek Vasut <marex at denx.de> wrote:
> On Wednesday, September 02, 2015 at 08:09:51 AM, Jagan Teki wrote:
>> This patch adds flag status register reading support to
>> spi_flash_cmd_wait_ready.
>>
>> 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>
>> Cc: Hou Zhiqiang <B48286 at freescale.com>
>> Tested-by: Jagan Teki <jteki at openedev.com>
>> ---
>> Changes for v3:
>>       - none
>> Changes for v2:
>>       - Optime code for fsr
>>
>>  drivers/mtd/spi/sf_internal.h |  1 +
>>  drivers/mtd/spi/sf_ops.c      | 60
>> ++++++++++++++++++++++++++++++++++++++----- drivers/mtd/spi/sf_probe.c
>> |  4 +--
>>  include/spi_flash.h           |  2 --
>>  4 files changed, 56 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index 53998fc..8a3e5ec 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -53,6 +53,7 @@ enum {
>>
>>  enum spi_nor_option_flags {
>>       SNOR_F_SST_WR           = (1 << 0),
>> +     SNOR_F_USE_FSR          = (1 << 1),
>>  };
>>
>>  #define SPI_FLASH_3B_ADDR_LEN                3
>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> index c1cdde7..85e15b9 100644
>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -41,6 +41,21 @@ int spi_flash_cmd_read_status(struct spi_flash *flash,
>> u8 *rs) return 0;
>>  }
>>
>> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
>> +{
>> +     int ret;
>> +     u8 cmd;
>> +
>> +     cmd = CMD_FLAG_STATUS;
>
> You can use const u8 cmd = CMD_...
>
>> +     ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
>> +     if (ret < 0) {
>> +             debug("SF: fail to read flag status register\n");
>> +             return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
>>  {
>>       u8 cmd;
>> @@ -139,20 +154,53 @@ static void spi_flash_dual_flash(struct spi_flash
>> *flash, u32 *addr) }
>>  #endif
>>
>> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
>
> Drop the bogus inline keyword, compiler will handle this better.

Does this means it requires -finline-small-functions optimization flag?

Based on the function-definition-size it will optimize using this flag
w/o need of 'inline' keyword, is this the case here?

>
>> +{
>> +     u8 sr;
>> +     int ret;
>> +
>> +     ret = spi_flash_cmd_read_status(flash, &sr);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     return !(sr & STATUS_WIP);
>> +}
>> +
>> +static inline int spi_flash_fsr_ready(struct spi_flash *flash)
>> +{
>> +     u8 fsr;
>> +     int ret;
>> +
>> +     ret = read_fsr(flash, &fsr);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     return fsr & STATUS_PEC;
>> +}
>> +
>> +static int spi_flash_ready(struct spi_flash *flash)
>> +{
>> +     int sr, fsr;
>> +     sr = spi_flash_sr_ready(flash);
>> +     if (sr < 0)
>> +             return sr;
>> +     fsr = flash->flags & SNOR_F_USE_FSR ? spi_flash_fsr_ready(flash) : 1;
>
> The ternary operator is just making the code more confusing, don't use it.
>
>> +     if (fsr < 0)
>> +             return fsr;
>> +     return sr && fsr;
>> +}
>> +
>>  int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
>> timeout) {
>> -     int timebase, ret, sr;
>> +     int timebase, ret;
>>
>>       timebase = get_timer(0);
>>
>>       while (get_timer(timebase) < timeout) {
>> -             ret = spi_flash_cmd_read_status(flash, &sr);
>> +             ret = spi_flash_ready(flash);
>>               if (ret < 0)
>>                       return ret;
>> -
>> -             if (sr < 0)
>> -                     break;
>> -             else if (!(sr & STATUS_WIP))
>> +             if (ret)
>>                       return 0;
>>       }
>>
>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
>> index 85390d4..758347b 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -255,11 +255,9 @@ static int spi_flash_validate_params(struct spi_slave
>> *spi, u8 *idcode, flash->dummy_byte = 1;
>>       }
>>
>> -     /* Poll cmd selection */
>> -     flash->poll_cmd = CMD_READ_STATUS;
>>  #ifdef CONFIG_SPI_FLASH_STMICRO
>>       if (params->flags & E_FSR)
>> -             flash->poll_cmd = CMD_FLAG_STATUS;
>> +             flash->flags |= SNOR_F_USE_FSR;
>>  #endif
>>
>>       /* Configure the BAR - discover bank cmds and read current bank */
>> diff --git a/include/spi_flash.h b/include/spi_flash.h
>> index 8d85468..4312d3d 100644
>> --- a/include/spi_flash.h
>> +++ b/include/spi_flash.h
>> @@ -49,7 +49,6 @@ struct spi_slave;
>>   * @bank_read_cmd:   Bank read cmd
>>   * @bank_write_cmd:  Bank write cmd
>>   * @bank_curr:               Current flash bank
>> - * @poll_cmd:                Poll cmd - for flash erase/program
>>   * @erase_cmd:               Erase cmd 4K, 32K, 64K
>>   * @read_cmd:                Read cmd - Array Fast, Extn read and quad read.
>>   * @write_cmd:               Write cmd - page and quad program.
>> @@ -82,7 +81,6 @@ struct spi_flash {
>>       u8 bank_write_cmd;
>>       u8 bank_curr;
>>  #endif
>> -     u8 poll_cmd;
>>       u8 erase_cmd;
>>       u8 read_cmd;
>>       u8 write_cmd;


--  Jagan.