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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sat, 2 May
2020 10:12:15 +0200:

> On Wed, 29 Apr 2020 17:55:36 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> 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@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_onfi.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> > index ee0f2c2549c1..19e1be94c7e3 100644
> > --- a/drivers/mtd/nand/raw/nand_onfi.c
> > +++ b/drivers/mtd/nand/raw/nand_onfi.c
> > @@ -160,14 +160,14 @@ int nand_onfi_detect(struct nand_chip *chip)
> >  	if (!pbuf)
> >  		return -ENOMEM;
> >  
> > -	ret = nand_read_param_page_op(chip, 0, NULL, 0);
> > -	if (ret) {
> > -		ret = 0;
> > -		goto free_onfi_param_page;
> > -	}
> > -
> >  	for (i = 0; i < ONFI_PARAM_PAGES; i++) {
> > -		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
> > +		if (!i)
> > +			ret = nand_read_param_page_op(chip, 0, &pbuf[i],
> > +						      sizeof(*pbuf));
> > +		else
> > +			ret = nand_change_read_column_op(chip, sizeof(*pbuf) * i,
> > +							 &pbuf[i], sizeof(*pbuf),
> > +							 true);  
> 
> Oops! Looks like this this change will break at least 3 drivers which
> support NAND_CMD_PARAM but don't support NAND_CMD_RNDOUT:
> fsl_ifc_nand, mxc_nand and qcom_nandc.


That's right, here is the new implementation that I am going to share:

-> if (!i) -> READ PARAM PAGE
-> if (use_datain) -> DATA IN
-> else -> RNDOUT.

The use_datain boolean is set to true in the following cases:
- the driver is compliant with exec-op and supports simple reads
- the driver is not compliant with exec-op (we keep the behavior
  unchanged for these drivers).

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