Re: [PATCH v3 11/13] mtd: rawnand: Expose monolithic read/write_page_raw() helpers

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Mon,  4 May 2020 10:24:12 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> The current nand_read/write_page_raw() helpers are already widely used
> but do not fit the purpose of "constrained" controllers which cannot,
> for instance, separate command/address cycles with data cycles.
> 
> Workaround this issue by proposing alternative helpers that cannot be

								^can

> used by controller drivers instead.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 79 ++++++++++++++++++++++++++++++++
>  include/linux/mtd/rawnand.h      |  8 +++-
>  2 files changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 413974df8a5e..6ec24eee355b 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -2638,6 +2638,48 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
>  }
>  EXPORT_SYMBOL(nand_read_page_raw);
>  
> +/**
> + * nand_monolithic_read_page_raw - Monolithic page read in raw mode
> + * @chip: NAND chip info structure
> + * @buf: buffer to store read data
> + * @oob_required: caller requires OOB data read to chip->oob_poi
> + * @page: page number to read
> + *
> + * This is a raw page read, ie. without any error detection/correction.
> + * Monolithic means we are requesting all the relevant data (main plus
> + * eventually OOB) to be loaded in the NAND cache and sent over the
> + * bus (from the NAND chip to the NAND controller) in a single
> + * operation. This is an alternative to nand_read_page_raw(), which
> + * first reads the main data, and if the OOB data is requested too,
> + * then reads more data on the bus. Indeed, this approach is not
> + * well supported by all controller drivers.

I'd drop the last sentence. The rest looks good.

> + */
> +int nand_monolithic_read_page_raw(struct nand_chip *chip, u8 *buf,
> +				  int oob_required, int page)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	unsigned int size = mtd->writesize;
> +	u8 *read_buf = buf;
> +	int ret;
> +
> +	if (oob_required) {
> +		size += mtd->oobsize;
> +
> +		if (buf != chip->data_buf)
> +			read_buf = nand_get_data_buf(chip);
> +	}
> +
> +	ret = nand_read_page_op(chip, page, 0, read_buf, size);
> +	if (ret)
> +		return ret;
> +
> +	if (buf != chip->data_buf)
> +		memcpy(buf, read_buf, mtd->writesize);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(nand_monolithic_read_page_raw);
> +
>  /**
>   * nand_read_page_raw_syndrome - [INTERN] read raw page data without ecc
>   * @chip: nand chip info structure
> @@ -3646,6 +3688,43 @@ int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
>  }
>  EXPORT_SYMBOL(nand_write_page_raw);
>  
> +/**
> + * nand_monolithic_write_page_raw - Monolithic page write in raw mode
> + * @chip: NAND chip info structure
> + * @buf: data buffer to write
> + * @oob_required: must write chip->oob_poi to OOB
> + * @page: page number to write
> + *
> + * This is a raw page write, ie. without any error detection/correction.
> + * Monolithic means we are requesting all the relevant data (main plus
> + * eventually OOB) to be sent over the bus and effectively programmed
> + * into the NAND chip arrays in a single operation. This is an
> + * alternative to nand_write_page_raw(), which first sends the main
> + * data, then eventually send the OOB data by latching more data
> + * cycles on the NAND bus, and finally sends the program command to
> + * synchronyze the NAND chip cache. Indeed, this approach is not well
> + * supported by all controller drivers.

Ditto.

With this addressed:

Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>

> + */
> +int nand_monolithic_write_page_raw(struct nand_chip *chip, const u8 *buf,
> +				   int oob_required, int page)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	unsigned int size = mtd->writesize;
> +	u8 *write_buf = (u8 *)buf;
> +
> +	if (oob_required) {
> +		size += mtd->oobsize;
> +
> +		if (buf != chip->data_buf) {
> +			write_buf = nand_get_data_buf(chip);
> +			memcpy(write_buf, buf, mtd->writesize);
> +		}
> +	}
> +
> +	return nand_prog_page_op(chip, page, 0, write_buf, size);
> +}
> +EXPORT_SYMBOL(nand_monolithic_write_page_raw);
> +
>  /**
>   * nand_write_page_raw_syndrome - [INTERN] raw page write function
>   * @chip: nand chip info structure
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 70380c91731c..830e05dd1e1d 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1328,13 +1328,17 @@ int nand_read_oob_std(struct nand_chip *chip, int page);
>  int nand_get_set_features_notsupp(struct nand_chip *chip, int addr,
>  				  u8 *subfeature_param);
>  
> -/* Default read_page_raw implementation */
> +/* Default read_page_raw implementations */
>  int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
>  		       int page);
> +int nand_monolithic_read_page_raw(struct nand_chip *chip, uint8_t *buf,
> +				  int oob_required, int page);
>  
> -/* Default write_page_raw implementation */
> +/* Default write_page_raw implementations */
>  int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
>  			int oob_required, int page);
> +int nand_monolithic_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
> +				   int oob_required, int page);
>  
>  /* Reset and initialize a NAND device */
>  int nand_reset(struct nand_chip *chip, int chipnr);


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