Re: [PATCH v2 06/19] mtd: rawnand: cafe: Demistify register fields

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Lubomir Rintel <lkundrak@v3.sk>
Cc: Richard Weinberger <richard@nod.at>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	linux-mtd@lists.infradead.org,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH v2 06/19] mtd: rawnand: cafe: Demistify register fields
Date: Wed, 6 May 2020 22:53:22 +0200	[thread overview]
Message-ID: <20200506225322.1e4fb230@collabora.com> (raw)
In-Reply-To: <20200506204638.GB207924@furthur.local>

On Wed, 6 May 2020 22:46:38 +0200
Lubomir Rintel <lkundrak@v3.sk> wrote:

> On Tue, May 05, 2020 at 12:13:40PM +0200, Boris Brezillon wrote:
> > The driver has a bunch of magic values. Let's define proper register
> > fields based on the spec [1] and use them.
> > 
> > [1]http://wiki.laptop.org/images/5/5c/88ALP01_Datasheet_July_2007.pdf
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > Reviewed-by: Lubomir Rintel <lkundrak@v3.sk>
> > Tested-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > Changes in v2:
> > * Add R-b/T-b
> > * Get rid of unrelated select_chip() change
> > * Fix CE selection logic
> > * s/CAFE_NAND_CTRL2_CMD2/CAFE_NAND_CTRL2_HAS_CMD2/ (Reported by Lubomir)
> > ---
> >  drivers/mtd/nand/raw/cafe_nand.c | 349 ++++++++++++++++++++++++-------
> >  1 file changed, 268 insertions(+), 81 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
> > index 156a308b530b..7fb5b21aeb00 100644
> > --- a/drivers/mtd/nand/raw/cafe_nand.c
> > +++ b/drivers/mtd/nand/raw/cafe_nand.c  
> ...
> > @@ -604,9 +722,9 @@ static int cafe_nand_attach_chip(struct nand_chip *chip)
> >  	/* Restore the DMA flag */
> >  	cafe->usedma = usedma;
> >  
> > -	cafe->ctl2 = BIT(27); /* Reed-Solomon ECC */
> > -	if (mtd->writesize == 2048)
> > -		cafe->ctl2 |= BIT(29); /* 2KiB page size */
> > +	cafe->ctl2 = CAFE_NAND_CTRL2_ECC_ALG_RS |
> > +		     CAFE_FIELD_PREP(NAND_CTRL2, PAGE_SIZE,
> > +				     mtd->writesize / 512);
> >  
> >  	/* Set up ECC according to the type of chip we found */
> >  	mtd_set_ooblayout(mtd, &cafe_ooblayout_ops);  
> 
> One more thing here. With page size of 2048 B, the old code would set bits
> 29,28 to 2 (10 binary), whereas 2048/512 tries to set 4 (00 binary, MSB
> overflows).
> 
> For 512 B pages, it the page size bits would previously remain zero,
> while with the patch they'd be set to one.
> 
> The old behavior was more like this:
> 
> -       cafe->ctl2 = CAFE_NAND_CTRL2_ECC_ALG_RS |
> -                    CAFE_FIELD_PREP(NAND_CTRL2, PAGE_SIZE,
> -                                    mtd->writesize / 512);

Oops, it was supposed to be 'mtd->writesize / 1024' here. I'll fix that.

> +       cafe->ctl2 = CAFE_NAND_CTRL2_ECC_ALG_RS;
> +       if (mtd->writesize == 2048)
> +               cafe->ctl2 |= CAFE_FIELD_PREP(NAND_CTRL2, PAGE_SIZE, 2);
> 
> That said, I have no idea what difference it makes. I'm still able to
> mount a JFFS2 filesystem from the flash (mine has 2K pages) without any
> trouble.
> 
> Lubo

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/