[PATCH 04/11] nand: spi: Add read function support
Boris Brezillon
boris.brezillon at free-electrons.com
Tue Feb 21 01:51:53 PST 2017
On Tue, 21 Feb 2017 16:00:03 +0800
Peter Pan <peterpandong at micron.com> wrote:
> Add read/readoob support for SPI NAND. Will be
> registered under struct mtd_info later.
>
> Signed-off-by: Peter Pan <peterpandong at micron.com>
> ---
> drivers/mtd/nand/spi/spi-nand-base.c | 321 +++++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nand.h | 3 +
> 2 files changed, 324 insertions(+)
>
> diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
> index b75e5cd..ee94eb8 100644
> --- a/drivers/mtd/nand/spi/spi-nand-base.c
> +++ b/drivers/mtd/nand/spi/spi-nand-base.c
> @@ -18,9 +18,62 @@
>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/sched.h>
> #include <linux/delay.h>
> #include <linux/mtd/spi-nand.h>
>
> +/**
> + * spi_nand_get_device - [GENERIC] Get chip for selected access
> + * @mtd: MTD device structure
> + * @new_state: the state which is requested
> + *
> + * Get the device and lock it for exclusive access
> + */
> +static int spi_nand_get_device(struct mtd_info *mtd, int new_state)
> +{
> + struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
> + DECLARE_WAITQUEUE(wait, current);
> +
> + /*
> + * Grab the lock and see if the device is available
> + */
> + while (1) {
> + spin_lock(&this->chip_lock);
> + if (this->state == FL_READY) {
> + this->state = new_state;
> + spin_unlock(&this->chip_lock);
> + break;
> + }
> + if (new_state == FL_PM_SUSPENDED) {
> + spin_unlock(&this->chip_lock);
> + return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
> + }
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + add_wait_queue(&this->wq, &wait);
> + spin_unlock(&this->chip_lock);
> + schedule();
> + remove_wait_queue(&this->wq, &wait);
> + }
Do we really need this fl_state dance? It could be replaced with a
simple mutex that is taken for the whole flash operation. This leaves
the suspended case which could be handled with:
static void spinand_suspend(struct mtd_info *mtd)
{
struct spi_nand_chip *this = mtd_to_spinand(mtd);
if (!mutex_trylock(&this->lock))
return -EAGAIN;
return 0;
}
For other operations (read, write, lock, unlock, ...), you just have to
take the lock before accessing the NAND and release it when you're done.
Again, I'm saying that because I don't want to end up with something
that is overly complex for no real reason, but maybe I'm missing
something.
> + return 0;
> +}
> +
> +/**
> + * spi_nand_release_device - [GENERIC] release chip
> + * @mtd: MTD device structure
> + *
> + * Deselect, release chip lock and wake up anyone waiting on the device
> + */
> +static void spi_nand_release_device(struct mtd_info *mtd)
> +{
> + struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
> +
> + /* Release the chip */
> + spin_lock(&this->chip_lock);
> + this->state = FL_READY;
> + wake_up(&this->wq);
> + spin_unlock(&this->chip_lock);
> +}
> +
> static inline int spi_nand_issue_cmd(struct spi_nand_chip *chip,
> struct spi_nand_cmd *cmd)
> {
> @@ -313,6 +366,36 @@ static int spi_nand_erase_block(struct spi_nand_chip *chip,
> }
>
> /**
> + * spi_nand_wait - wait until the command is done
> + * @chip: SPI-NAND device structure
> + * @s: buffer to store status register(can be NULL)
> + */
> +static int spi_nand_wait(struct spi_nand_chip *chip, u8 *s)
> +{
> + unsigned long timeo = jiffies;
> + u8 status;
> + int ret = -ETIMEDOUT;
> + int count = 0;
> +
> + #define MIN_TRY_COUNT 3
Please, no macro definitions inside functions.
> + timeo += msecs_to_jiffies(20);
> +
> + while (time_before(jiffies, timeo) || count < MIN_TRY_COUNT) {
> + spi_nand_read_status(chip, &status);
> + if ((status & STATUS_OIP_MASK) == STATUS_READY) {
> + ret = 0;
> + goto out;
> + }
> + count++;
> + }
Hm, if your SPI controller is not able to read 3 times the status reg
in 20ms it means it's really slow.
I'd recommend doing something like:
do {
spi_nand_read_status(chip, &status);
if ((status & STATUS_OIP_MASK) == STATUS_READY)
goto out;
} while (time_before(jiffies, timeo));
/*
* Extra read, just in case the STATUS_READY bit has changed
* since out last check
*/
spi_nand_read_status(chip, &status);
> +out:
> + if (s)
> + *s = status;
> +
> + return ret;
return (status & STATUS_OIP_MASK) == STATUS_READY ? 0 : -ETIMEDOUT;
> +}
> +
> +/**
> * spi_nand_read_id - send 9Fh command to get ID
> * @chip: SPI-NAND device structure
> * @buf: buffer to store id
> @@ -363,6 +446,244 @@ static int spi_nand_lock_block(struct spi_nand_chip *chip, u8 lock)
> return spi_nand_write_reg(chip, REG_BLOCK_LOCK, &lock);
> }
>
> +/**
> + * spi_nand_do_read_page - read page from flash to buffer
> + * @mtd: MTD device structure
> + * @page_addr: page address/raw address
> + * @column: column address
> + * @ecc_off: without ecc or not
> + * @corrected: how many bit error corrected
> + * @buf: data buffer
> + * @len: data length to read
> + */
> +static int spi_nand_do_read_page(struct mtd_info *mtd, u32 page_addr,
> + bool ecc_off, int *corrected, bool oob_only)
> +{
> + struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
> + struct nand_device *nand = mtd_to_nand(mtd);
> + int ret, ecc_error = 0;
> + u8 status;
> +
> + spi_nand_read_page_to_cache(chip, page_addr);
> + ret = spi_nand_wait(chip, &status);
> + if (ret < 0) {
> + pr_err("error %d waiting page 0x%x to cache\n",
> + ret, page_addr);
> + return ret;
> + }
> + if (!oob_only)
> + spi_nand_read_from_cache(chip, page_addr, 0,
> + nand_page_size(nand) + nand_per_page_oobsize(nand), chip->buf);
> + else
> + spi_nand_read_from_cache(chip, page_addr, nand_page_size(nand),
> + nand_per_page_oobsize(nand), chip->oobbuf);
> + if (!ecc_off) {
> + chip->get_ecc_status(chip, status, corrected, &ecc_error);
> + /*
> + * If there's an ECC error, print a message and notify MTD
> + * about it. Then complete the read, to load actual data on
> + * the buffer (instead of the status result).
> + */
> + if (ecc_error) {
> + pr_err("internal ECC error reading page 0x%x\n",
> + page_addr);
> + mtd->ecc_stats.failed++;
> + } else if (*corrected) {
> + mtd->ecc_stats.corrected += *corrected;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * spi_nand_transfer_oob - transfer oob to client buffer
> + * @chip: SPI-NAND device structure
> + * @oob: oob destination address
> + * @ops: oob ops structure
> + * @len: size of oob to transfer
> + */
> +static void spi_nand_transfer_oob(struct spi_nand_chip *chip, u8 *oob,
> + struct mtd_oob_ops *ops, size_t len)
> +{
> + struct mtd_info *mtd = spi_nand_to_mtd(chip);
> + int ret;
> +
> + switch (ops->mode) {
> +
> + case MTD_OPS_PLACE_OOB:
> + case MTD_OPS_RAW:
> + memcpy(oob, chip->oobbuf + ops->ooboffs, len);
> + return;
> +
> + case MTD_OPS_AUTO_OOB:
> + ret = mtd_ooblayout_get_databytes(mtd, oob, chip->oobbuf,
> + ops->ooboffs, len);
> + BUG_ON(ret);
> + return;
> +
> + default:
> + BUG();
Let's try to avoid these BUG_ON() calls. If you want transfer_oob() to
report an error, change the function prototype and check the ret code
in the caller.
> + }
> +}
> +
> +/**
> + * spi_nand_read_pages - read data from flash to buffer
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob operations description structure
> + * @max_bitflips: maximum bitflip count
> + * Description:
> + * Normal read function, read one page to buffer before issue
> + * another.
> + */
> +static int spi_nand_read_pages(struct mtd_info *mtd, loff_t from,
> + struct mtd_oob_ops *ops, unsigned int *max_bitflips)
> +{
> + struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
> + struct nand_device *nand = mtd_to_nand(mtd);
> + int page_addr, page_offset, size, ret;
> + unsigned int corrected = 0;
> + int readlen = ops->len;
> + int oobreadlen = ops->ooblen;
> + bool ecc_off = ops->mode == MTD_OPS_RAW;
> + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
> + mtd->oobavail : mtd->oobsize;
> + bool oob_only = ops->datbuf == NULL;
> +
> + page_addr = nand_offs_to_page(nand, from);
> + page_offset = from & (nand_page_size(nand) - 1);
> + ops->retlen = 0;
> + *max_bitflips = 0;
> +
Comment for future reworks: we're doing the same thing in the // NAND
framework, maybe we should let the generic NAND layer do this
transformation for us (1 mtd request -> X page read/write requests).
> + while (1) {
> + ret = spi_nand_do_read_page(mtd, page_addr, ecc_off,
> + &corrected, oob_only);
> + if (ret)
> + break;
> + *max_bitflips = max(*max_bitflips, corrected);
> + if (ops->datbuf) {
> + size = min_t(int, readlen, nand_page_size(nand) - page_offset);
> + memcpy(ops->datbuf + ops->retlen,
> + chip->buf + page_offset, size);
> + ops->retlen += size;
> + readlen -= size;
> + page_offset = 0;
> + }
> + if (ops->oobbuf) {
> + size = min(oobreadlen, ooblen);
> + spi_nand_transfer_oob(chip,
> + ops->oobbuf + ops->oobretlen, ops, size);
> + ops->oobretlen += size;
> + oobreadlen -= size;
> + }
> + if (!readlen && !oobreadlen)
> + break;
> + page_addr++;
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * spi_nand_do_read_ops - read data from flash to buffer
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob ops structure
> + * Description:
> + * Disable internal ECC before reading when MTD_OPS_RAW set.
> + */
> +static int spi_nand_do_read_ops(struct mtd_info *mtd, loff_t from,
> + struct mtd_oob_ops *ops)
> +{
> + struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
> + int ret;
> + struct mtd_ecc_stats stats;
> + unsigned int max_bitflips = 0;
> + int oobreadlen = ops->ooblen;
> + bool ecc_off = ops->mode == MTD_OPS_RAW;
> + int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
> + mtd->oobavail : mtd->oobsize;
> +
> + if (oobreadlen > 0) {
> + ooblen -= ops->ooboffs;
> + ops->oobretlen = 0;
> + }
> + stats = mtd->ecc_stats;
> + if (ecc_off)
> + spi_nand_disable_ecc(chip);
> + ret = spi_nand_read_pages(mtd, from, ops, &max_bitflips);
> + if (ecc_off)
> + spi_nand_enable_ecc(chip);
> + if (ret)
> + return ret;
> +
> + if (mtd->ecc_stats.failed - stats.failed)
> + return -EBADMSG;
> +
> + return max_bitflips;
> +}
> +
> +/**
> + * spi_nand_read - [MTD Interface] SPI-NAND read
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @len: number of bytes to read
> + * @retlen: pointer to variable to store the number of read bytes
> + * @buf: the databuffer to put data
> + */
> +static int spi_nand_read(struct mtd_info *mtd, loff_t from, size_t len,
> + size_t *retlen, u8 *buf)
> +{
> + struct mtd_oob_ops ops;
> + int ret;
> +
> + spi_nand_get_device(mtd, FL_READING);
> +
> + memset(&ops, 0, sizeof(ops));
> + ops.len = len;
> + ops.datbuf = buf;
> + ops.mode = MTD_OPS_PLACE_OOB;
> + ret = spi_nand_do_read_ops(mtd, from, &ops);
> +
> + *retlen = ops.retlen;
> +
> + spi_nand_release_device(mtd);
> +
> + return ret;
> +}
> +
> +/**
> + * spi_nand_read_oob - [MTD Interface] read data and/or out-of-band
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob operation description structure
> + */
> +static int spi_nand_read_oob(struct mtd_info *mtd, loff_t from,
> + struct mtd_oob_ops *ops)
> +{
> + int ret = -ENOTSUPP;
> +
> + ops->retlen = 0;
> + spi_nand_get_device(mtd, FL_READING);
> +
> + switch (ops->mode) {
> + case MTD_OPS_PLACE_OOB:
> + case MTD_OPS_AUTO_OOB:
> + case MTD_OPS_RAW:
> + break;
> +
> + default:
> + goto out;
> + }
> +
> + ret = spi_nand_do_read_ops(mtd, from, ops);
> +
> +out:
> + spi_nand_release_device(mtd);
> +
> + return ret;
> +}
> MODULE_DESCRIPTION("SPI NAND framework");
> MODULE_AUTHOR("Peter Pan<peterpandong at micron.com>");
> MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
> index 1fcbad7..b61045b 100644
> --- a/include/linux/mtd/spi-nand.h
> +++ b/include/linux/mtd/spi-nand.h
> @@ -132,6 +132,7 @@
> * @read_cache_op: [REPLACEABLE] Opcode of read from cache
> * @write_cache_op: [REPLACEABLE] Opcode of program load
> * @command_fn: [BOARDSPECIFIC] function to handle command transfer
> + * @get_ecc_status: [REPLACEABLE] get ecc and bitflip status
> * @buf: [INTERN] buffer for read/write data
> * @oobbuf: [INTERN] buffer for read/write oob
> * @controller_caps: [INTERN] capacities of SPI NAND controller
> @@ -153,6 +154,8 @@ struct spi_nand_chip {
> u8 write_cache_op;
> int (*command_fn)(struct spi_nand_chip *this,
> struct spi_nand_cmd *cmd);
> + void (*get_ecc_status)(struct spi_nand_chip *this, unsigned int status,
> + unsigned int *corrected, unsigned int *ecc_errors);
As for the ->command_fn(), put that in a different structure (not sure
it is NAND controller related, so maybe a spinand_ecc_engine and
spinand_ecc_engine_ops structures).
> u8 *buf;
> u8 *oobbuf;
> u32 controller_caps;
More information about the linux-mtd
mailing list