Re: [PATCH v2 08/11] mtd: rawnand: jedec: Adapt the parameter page read to constraint controllers

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Sun, 3 May 2020 21:06:23 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Wed, 29 Apr
> 2020 18:04:05 +0200:
> 
> > On Wed, 29 Apr 2020 17:55:37 +0200
> > Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> >   
> > > We already know that there are controllers not able to read the three
> > > copies of the parameter page in one go. The workaround was to first
> > > request the controller to assert command and address cycles on the
> > > NAND bus to trigger a parameter page read, and then do a read
> > > operation for each page.
> > > 
> > > But there are also controllers which are not able to split the
> > > parameter page read between the command/address cycles and the actual
> > > data operation.
> > > 
> > > All controllers are expected to be able to change the read column
> > > though. So let's use a regular parameter page read operation for the
> > > first iteration and use a change read column operation for the
> > > following copies.
> > > 
> > > The extra command and address cycles sent over the NAND bus are
> > > negligible compared to the amount of data that is being transferred
> > > anyway.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > > ---
> > >  drivers/mtd/nand/raw/nand_jedec.c | 28 +++++++++++++++-------------
> > >  1 file changed, 15 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> > > index 15937e02c64f..b2be9056759a 100644
> > > --- a/drivers/mtd/nand/raw/nand_jedec.c
> > > +++ b/drivers/mtd/nand/raw/nand_jedec.c
> > > @@ -25,7 +25,7 @@ int nand_jedec_detect(struct nand_chip *chip)
> > >  {
> > >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > >  	struct nand_memory_organization *memorg;
> > > -	struct nand_jedec_params *p;
> > > +	struct nand_jedec_params *p = NULL, *pbuf;    
> > 
> > Looks like you've merged 2 different commits here. I remember you had a
> > separate commit adding pbuf to avoid an extra memcpy().  
> 
> Actually this was only fixed in the onfi detection routine. The jedec
> equivalent does not copy the page (the copy came from the 3-way merge
> that has only been added to onfi detection).
> 
> Here pbuf represents the buffer containing the three pages while p now
> only points to the actual page that is correct.

Oh, I see. Looks like the p = pbuf assignment is wrong BTW (should be
p = &pbuf[i]).

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