Re: [PATCH 3/3] mtd: spi-nor: sf: Add clear flag status register support

[Jagan Teki]

Hi Zhiqiang,

On 19 August 2015 at 17:42, Hou Zhiqiang <B48286@xxxxxxxxxxxxx> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jteki@xxxxxxxxxxxx]
>> Sent: 2015å8æ19æ 17:57
>> To: linux-mtd@xxxxxxxxxxxxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx; Jagan Teki; Hou Zhiqiang-B48286; Hu
>> Mingkai-B21284; David Woodhouse; Brian Norris
>> Subject: [PATCH 3/3] mtd: spi-nor: sf: Add clear flag status register
>> support
>>
>> The clear flag status register operation was required by Micron SPI-NOR
>> chips, which support FSR. And if an error bit of FSR have been set like
>> protection, voltage, erase, and program, it must be cleared by the clear
>> FSR operation.
>>
>> Signed-off-by: Jagan Teki <jteki@xxxxxxxxxxxx>
>> Cc: Hou Zhiqiang <B48286@xxxxxxxxxxxxx>
>> Cc: Mingkai.Hu <Mingkai.Hu@xxxxxxxxxxxxx>
>> Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
>> Cc: Brian Norris <computersforpeace@xxxxxxxxx>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 35 +++++++++++++++++++++++++++++++----
>>  include/linux/mtd/spi-nor.h   |  9 +++++++++
>>  2 files changed, 40 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-
>> nor.c index f954d03..c5c472d5 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -100,6 +100,28 @@ static int read_fsr(struct spi_nor *nor)  }
>>
>>  /*
>> + * Read the clear flag status register.
>> + * The clear flag status register operation was required by Micron
>> + * SPI-NOR chips, which support FSR. And if an error bit of FSR
>> + * have been set like protection, voltage, erase, and program,
>> + * it must be cleared by the clear FSR operation.
>> + * Returns zero for FSR bits cleared and negative if error occurred.
>> + */
>> +static int read_cfsr(struct spi_nor *nor) {
>> +     int ret;
>> +     u8 val;
>> +
>> +     ret = nor->read_reg(nor, SPINOR_OP_RDCFSR, &val, 1);
>
> There should be a write_reg instead of read_reg.
> There isnât a register named CFSR, and the command SPINOR_OP_RDCFSR
> is used to clear the FSR, another words reset FSR to default value.

Yes, SPINOR_OP_RDCFSR is clear flag status register, for clearing errors bits
on fsr we need to read cfsr once.

>
>> +     if (ret < 0) {
>> +             pr_err("error %d reading CFSR\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     return val;
>> +}
>> +
>> +/*
>>   * Read configuration register, returning its value in the
>>   * location. Return the configuration register value.
>>   * Returns negative if error occured.
>> @@ -209,10 +231,15 @@ static inline int spi_nor_sr_ready(struct spi_nor
>> *nor)  static inline int spi_nor_fsr_ready(struct spi_nor *nor)  {
>>       int fsr = read_fsr(nor);
>> -     if (fsr < 0)
>> -             return fsr;
>> -     else
>> -             return fsr & FSR_READY;
>> +     if (fsr & FSR_ERR_MASK) {
>> +             pr_err("flag status(0x%x) error occured\n", fsr);
>> +             int cfsr = read_cfsr(nor);
>> +             if (cfsr < 0)
>> +                     return cfsr;
>> +             return -1;
>> +     }
>> +
>> +     return fsr & FSR_READY;
>>  }
>>
>>  static int spi_nor_ready(struct spi_nor *nor) diff --git
>> a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index
>> c5a58c4..36c1681 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -35,6 +35,7 @@
>>  #define SPINOR_OP_RDID               0x9f    /* Read JEDEC ID */
>>  #define SPINOR_OP_RDCR               0x35    /* Read configuration register */
>>  #define SPINOR_OP_RDFSR              0x70    /* Read flag status register */
>> +#define SPINOR_OP_RDCFSR     0x50    /* Read clear flag status register */
>>
>>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes.
>> */
>>  #define SPINOR_OP_READ4              0x13    /* Read data bytes (low
>> frequency) */
>> @@ -74,6 +75,14 @@
>>  /* Enhanced Volatile Configuration Register bits */
>>  #define EVCR_QUAD_EN_MICRON    0x80    /* Micron Quad I/O */
>>
>> +/* Flag Status Register Error bits */
>> +#define FSR_ERR_PROT         0x2     /* Protection */
>> +#define FSR_ERR_VOLT         0x8     /* Voltage on Vpp */
>> +#define FSR_ERR_PROG         0x10    /* Program operation */
>> +#define FSR_ERR_ERASE                0x20    /* Erase operation */
>> +#define FSR_ERR_MASK         (FSR_ERR_PROT | FSR_ERR_VOLT | \
>> +                             FSR_ERR_PROG | FSR_ERR_ERASE)
>> +
>>  /* Flag Status Register bits */
>>  #define FSR_READY            0x80

thanks!
-- 
Jagan | openedev.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/