Hi Boris, > -----Original Message----- > From: Boris Brezillon [mailto:boris.brezillon@xxxxxxxxxxx] > Sent: Tuesday, March 20, 2018 1:51 PM > To: Jagdish Gediya <jagdish.gediya@xxxxxxx> > Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx; boris.brezillon@xxxxxxxxxxxxxxxxxx; > computersforpeace@xxxxxxxxx; oss@xxxxxxxxxxxx; Leo Li > <leoyang.li@xxxxxxx>; stable@xxxxxxxxxxxxxxx > 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@xxxxxxx> 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@xxxxxxxxxxxxxxx # v3.18+ > > Signed-off-by: Jagdish Gediya <jagdish.gediya@xxxxxxx> > > Reviewed-by: Prabhakar Kushwaha <prabhakar.kushwaha@xxxxxxx> > > --- > > 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. > > /* > > * 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. > > } > > > > /* > > > Thanks, Jagdish