Re: [PATCH 6/8] mtd: nand: mxc: Add own write_page

[Miquel Raynal <miquel.raynal@free-electrons.com>]

Hi Sascha,

On Tue,  9 Jan 2018 11:11:46 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Now that we have our own read_page function add a write_page function
> for consistency aswell. This can be a lot easier than the generic
> function since we do not have to iterate over subpages but can write
> the whole page at once. Also add write_page_raw and write_oob for
> proper raw and oob write support.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/mxc_nand.c | 56
> +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56
> insertions(+)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index 83bb01ab7079..6fbf98851a96 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info
> *mtd, struct nand_chip *chip, page);
>  }
>  
> +static int __mxc_nand_write_page(struct mtd_info *mtd, struct
> nand_chip *chip,
> +				 const uint8_t *buf, int
> oob_required, int page) +{
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	memcpy32_toio(host->main_area0, buf, mtd->writesize);
> +	copy_spare(mtd, false, chip->oob_poi);

If I remember correctly, there was a "spare only" mechanism to avoid
doing full page read/write when unnecessary. What about using it here
when (buf == NULL && oob_required)?

> +
> +	host->devtype_data->send_page(mtd, NFC_INPUT);
> +	host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	return 0;
> +}
> +
> +static int mxc_nand_write_page(struct mtd_info *mtd, struct
> nand_chip *chip,
> +			       const uint8_t *buf, int oob_required,
> int page) +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->enable_hwecc(nand_chip, true);
> +
> +	return __mxc_nand_write_page(mtd, chip, buf, oob_required,
> page); +}
> +
> +static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct
> nand_chip *chip,
> +				   const uint8_t *buf, int
> oob_required, int page) +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->enable_hwecc(nand_chip, false);
> +
> +	return __mxc_nand_write_page(mtd, chip, buf, oob_required,
> page); +}
> +
> +static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip
> *chip,
> +			      int page)
> +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	memset(host->data_buf, 0xff, mtd->writesize);

If the above solution works, this won't be needed anymore.

> +
> +	host->devtype_data->enable_hwecc(nand_chip, false);
> +
> +	return __mxc_nand_write_page(mtd, chip, host->data_buf, 1,
> page);

And this could be:
    return __mxc_nand_write_page(mtd, chip, NULL, 1, page);

> +}
> +
>  static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char
> *dat, u_char *ecc_code)
>  {
> @@ -1907,6 +1960,9 @@ static int mxcnd_probe(struct platform_device
> *pdev) this->ecc.read_page = mxc_nand_read_page;
>  		this->ecc.read_page_raw = mxc_nand_read_page_raw;
>  		this->ecc.read_oob = mxc_nand_read_oob;
> +		this->ecc.write_page = mxc_nand_write_page;
> +		this->ecc.write_page_raw = mxc_nand_write_page_raw;
> +		this->ecc.write_oob = mxc_nand_write_oob;
>  		this->ecc.calculate = mxc_nand_calculate_ecc;
>  		this->ecc.hwctl = mxc_nand_enable_hwecc;
>  		this->ecc.correct = host->devtype_data->correct_data;


Thanks,
Miquèl