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

Jagan Teki
2015-08-26T10:37:56Z

Hi Zhiqiang,

On 25 August 2015 at 07:52, Hou Zhiqiang <B48286@freescale.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jteki@openedev.com]
>> Sent: 2015年8月21日 15:12
>> To: Hou Zhiqiang-B48286
>> Cc: linux-mtd@lists.infradead.org; Hu Mingkai-B21284; Brian Norris; David
>> Woodhouse; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH 3/3] mtd: spi-nor: sf: Add clear flag status register
>> support
>>
>> Hi Zhiqiang,
>>
>> On 20 August 2015 at 08:36, Hou Zhiqiang <B48286@freescale.com> wrote:
>> > Hello Jagan,
>> >
>> >> -----Original Message-----
>> >> From: Jagan Teki [mailto:jteki@openedev.com]
>> >> Sent: 2015年8月20日 1:49
>> >> To: Hou Zhiqiang-B48286
>> >> Cc: linux-mtd@lists.infradead.org; Hu Mingkai-B21284; Brian Norris;
>> >> David Woodhouse; linux-kernel@vger.kernel.org
>> >> Subject: Re: [PATCH 3/3] mtd: spi-nor: sf: Add clear flag status
>> >> register support
>> >>
>> >> Hi Zhiqiang,
>> >>
>> >> On 19 August 2015 at 17:42, Hou Zhiqiang <B48286@freescale.com> wrote:
>> >> > Hi Jagan,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Jagan Teki [mailto:jteki@openedev.com]
>> >> >> Sent: 2015年8月19日 17:57
>> >> >> To: linux-mtd@lists.infradead.org
>> >> >> Cc: linux-kernel@vger.kernel.org; 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@openedev.com>
>> >> >> Cc: Hou Zhiqiang <B48286@freescale.com>
>> >> >> Cc: Mingkai.Hu <Mingkai.Hu@freescale.com>
>> >> >> Cc: David Woodhouse <dwmw2@infradead.org>
>> >> >> Cc: Brian Norris <computersforpeace@gmail.com>
>> >> >> ---
>> >> >> 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.
>> >>
>> >
>> > Sorry, I'm not clear for this operation. Please correct me if I'm wrong.
>> > As far as I understand, this command is used to reset the FSR. Does a
>> > value Will be read back? And there is not the register CFSR, so I
>> > don't know which register will be read by SPINOR_OP_RDCFSR?
>>
>> Sorry for the confusion in previous email.
>>
>> If there is any error bits set during FSR operation, those will reset
>> back to original values by reading CFSR ie means the bits on flag status
>> register revert back to original state, so-that the flag status register
>> is ready for next FSR operation.
>>
>> I have defined this, SPINOR_OP_RDCFSR on the patch
>>
>> 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 */
>>
>> For more information pls- refer flash datasheet[1]
>>
>> Page, 29: about fsr
>> Page, 41: about cfsr
>
> Upon page 41, I think, it is a CMD which is used to clear the FSR instead of
> a CFSR Register. This section is to expound the "CLEAR FLAG STATUS REGISTER
> Command", and it says execute this CMD to clear the error bits, rather than
> read the CFSR to clear those bits.

Agreed that it could be command (clear flag status register - 0x50)
write operation with
NULL data, will prepare next version and send the same.

thanks!
--
Jagan | openedev.