[PATCH][v2] mtd: rawnand: fsl_ifc: Fix nand waitfunc return value
Boris Brezillon
boris.brezillon at bootlin.com
Tue Mar 20 03:26:03 PDT 2018
On Tue, 20 Mar 2018 10:16:04 +0000
Jagdish Gediya <jagdish.gediya at nxp.com> wrote:
> Hi Boris,
>
> > -----Original Message-----
> > From: Boris Brezillon [mailto:boris.brezillon at bootlin.com]
> > Sent: Tuesday, March 20, 2018 1:51 PM
> > To: Jagdish Gediya <jagdish.gediya at nxp.com>
> > Cc: linux-mtd at lists.infradead.org; boris.brezillon at free-electrons.com;
> > computersforpeace at gmail.com; oss at buserror.net; Leo Li
> > <leoyang.li at nxp.com>; stable at vger.kernel.org
> > Subject: Re: [PATCH][v2] mtd: rawnand: fsl_ifc: Fix nand waitfunc return value
> >
> > On Tue, 20 Mar 2018 22:13:38 +0530
> > Jagdish Gediya <jagdish.gediya at nxp.com> wrote:
> >
> > > As per the IFC hardware manual, Most significant 2 bytes in nand_fsr
> > > register are the outcome of NAND READ STATUS command.
> > >
> > > So status value need to be shifted and aligned as per the nand
> > > framework requirement.
> > >
> > > Fixes: 82771882d960 ("NAND Machine support for Integrated Flash
> > > Controller")
> > > Cc: stable at vger.kernel.org # v3.18+
> > > Signed-off-by: Jagdish Gediya <jagdish.gediya at nxp.com>
> > > Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>
> > > ---
> > > Changes for v2: Incorporated comments from Boris Brezillon
> > > - Added fixes tag
> > >
> > > drivers/mtd/nand/fsl_ifc_nand.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/fsl_ifc_nand.c
> > > b/drivers/mtd/nand/fsl_ifc_nand.c index 9390cbd..0aa03ba 100644
> > > --- a/drivers/mtd/nand/fsl_ifc_nand.c
> > > +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> > > @@ -643,12 +643,13 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct
> > nand_chip *chip)
> > > fsl_ifc_run_command(mtd);
> > >
> > > nand_fsr = ifc_in32(&ifc->ifc_nand.nand_fsr);
> > > -
> > > + nand_fsr >>= 16;
> > > + nand_fsr = (nand_fsr >> 8) | (nand_fsr << 8);
> >
> > Why are you swapping the upper and lower bytes? If the NAND status is stored
> > in the upper byte of the fsr reg (bits 24 to 31), then shift the value by 24
> > directly. Note that (nand_fsr << 8) is useless here since you're masking
> > nand_fsr with 0xff before returning it.
> >
> nand_fsr register is,
> 31 0
> ---------------------------------------------
> | RS0 | RS1 | reserved | reserved |
> ---------------------------------------------
> I intended to make it,
> 31 0
> -----------------------------
> | 00 | 00 | RS1 | RS0 |
> -----------------------------
> RS0 gets the nand read status. I am not dropping RS1 value for possible future use.
Then store the nand status in a different var and keep nand_fsr
untouched:
status = nand_fsr >> 24;
Note that I don't see the point of keeping RS1 until you really need it.
> > > /*
> > > * The chip always seems to report that it is
> > > * write-protected, even when it is not.
> > > */
> > > - return nand_fsr | NAND_STATUS_WP;
> > > + return (nand_fsr & 0xff) | NAND_STATUS_WP;
> It should be 0xffff instead of 0xff.
No it should not. The ->waitfunc() semantic is enforced by the
framework and it expects a NAND status *byte*.
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
More information about the linux-mtd
mailing list