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

[Hou Zhiqiang]

Hi Jagan,

> -----Original Message-----
> From: Jagan Teki [mailto:jteki@xxxxxxxxxxxx]
> Sent: 2015å8æ21æ 15:12
> To: Hou Zhiqiang-B48286
> Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx; Hu Mingkai-B21284; Brian Norris; David
> Woodhouse; linux-kernel@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxx> wrote:
> > Hello Jagan,
> >
> >> -----Original Message-----
> >> From: Jagan Teki [mailto:jteki@xxxxxxxxxxxx]
> >> Sent: 2015å8æ20æ 1:49
> >> To: Hou Zhiqiang-B48286
> >> Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx; Hu Mingkai-B21284; Brian Norris;
> >> David Woodhouse; linux-kernel@xxxxxxxxxxxxxxx
> >> 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@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.
> >>
> >
> > 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.

> 
> [1]
> https://www.google.co.in/url?sa=t&rct=j&q=&esrc=s&source=web&cd=2&cad=rja
> &uact=8&ved=0CCIQFjABahUKEwjTwY3zzbnHAhXVj44KHdiWDVU&url=https%3A%2F%2Fww
> w.micron.com%2F~%2Fmedia%2Fdocuments%2Fproducts%2Fdata-sheet%2Fnor-
> flash%2Fserial-
> nor%2Fn25q%2Fn25q_512mb_1ce_3v_65nm.pdf&ei=Uc3WVZPVCNWfugTYrbaoBQ&usg=AFQ
> jCNEN4dLie4U9OTSbNxN-h9jyN9InDQ&sig2=vRQTvlkbgsgJw1Fy9z1K8A
> 
Thanks!
Zhiqiang
N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå