[PATCH 10/18] mtd: rawnand: factor nand_command_lp() into nand_command()
Boris Brezillon
boris.brezillon at bootlin.com
Fri Apr 20 13:34:54 PDT 2018
Hi Sam, Arnout,
On Fri, 20 Apr 2018 10:19:38 +0200
Sam Lefebvre <sam.lefebvre at essensium.com> wrote:
> From: "Arnout Vandecappelle (Essensium/Mind)" <arnout at mind.be>
>
> The nand_command() and nand_command_lp() functions are very similar.
> Factor them into a single function, and use conditions on writesize to
> identify the differences.
>
> is_lp checks are added everywhere to make sure the behaviour is exactly
> the same as before. Most likely, the checks in CACHEDPROG, RNDIN and
> RNDOUT are not needed since these commands are only valid for LP
> devices. But since I'm not sure of that, I'm leaving it as is.
>
> The only side effect of this patch is that the large-page behaviour is
> activated a little bit earlier: as soon as writesize is set. However,
> only SEQIN, READOOB, READ0, CACHEDPROG, RNDIN and RNDOUT behave
> differently between small and large page. Of these, only RNDOUT is used
> in nand_detect(). RNDOUT is used by nand_change_read_column_op() which
> is called by nand_flash_detect_ext_param_page(). Before this patch, the
> switch to nand_command_lp was already made just before calling that
> function so the behaviour doesn't change.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout at mind.be>
> ---
> Note that I don't have access to a small-page device, so only tested on
> large-page devices. Also only tested on i.MX6Q (gpmi-nand).
>
> I only verified the lack of change in behaviour during nand_detect by
> reading the code, so it's possible that I missed something. Testing on
> various devices (ONFI, JEDEC, non-ONFI/JEDEC) is needed to be really
> sure that nothing breaks.
>
> Note that this patch can be removed from the series without affecting
> the rest.
Hm, I don't want to risk any regression, so I'm gonna pass on this
patch, especially since we're trying to get rid of ->cmdfunc() in favor
or ->exec_op().
The same goes for patch 9, sorry.
Regards,
Boris
> ---
> drivers/mtd/nand/raw/nand_base.c | 236 ++++++++++++---------------------------
> 1 file changed, 70 insertions(+), 166 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index bcc0344b1f27..320efbe41bd6 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -747,121 +747,6 @@ int nand_soft_waitrdy(struct nand_chip *chip, unsigned long timeout_ms)
> };
> EXPORT_SYMBOL_GPL(nand_soft_waitrdy);
>
> -/**
> - * nand_command - [DEFAULT] Send command to NAND device
> - * @mtd: MTD device structure
> - * @command: the command to be sent
> - * @column: the column address for this command, -1 if none
> - * @page_addr: the page address for this command, -1 if none
> - *
> - * Send command to NAND device. This function is used for small page devices
> - * (512 Bytes per page).
> - */
> -static void nand_command(struct mtd_info *mtd, unsigned int command,
> - int column, int page_addr)
> -{
> - register struct nand_chip *chip = mtd_to_nand(mtd);
> - int ctrl = NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE;
> -
> - /* Write out the command to the device */
> - if (command == NAND_CMD_SEQIN) {
> - int readcmd;
> -
> - if (column >= mtd->writesize) {
> - /* OOB area */
> - column -= mtd->writesize;
> - readcmd = NAND_CMD_READOOB;
> - } else if (column < 256) {
> - /* First 256 bytes --> READ0 */
> - readcmd = NAND_CMD_READ0;
> - } else {
> - column -= 256;
> - readcmd = NAND_CMD_READ1;
> - }
> - chip->cmd_ctrl(mtd, readcmd, ctrl);
> - ctrl &= ~NAND_CTRL_CHANGE;
> - }
> - if (command != NAND_CMD_NONE)
> - chip->cmd_ctrl(mtd, command, ctrl);
> -
> - /* Address cycle, when necessary */
> - ctrl = NAND_NCE | NAND_ALE | NAND_CTRL_CHANGE;
> - /* Serially input address */
> - if (column != -1) {
> - /* Adjust columns for 16 bit buswidth */
> - if (chip->options & NAND_BUSWIDTH_16 &&
> - !nand_opcode_8bits(command))
> - column >>= 1;
> - chip->cmd_ctrl(mtd, column, ctrl);
> - ctrl &= ~NAND_CTRL_CHANGE;
> - }
> - if (page_addr != -1) {
> - chip->cmd_ctrl(mtd, page_addr, ctrl);
> - ctrl &= ~NAND_CTRL_CHANGE;
> - chip->cmd_ctrl(mtd, page_addr >> 8, ctrl);
> - if (chip->options & NAND_ROW_ADDR_3)
> - chip->cmd_ctrl(mtd, page_addr >> 16, ctrl);
> - }
> - chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> -
> - /*
> - * Program and erase have their own busy handlers status and sequential
> - * in needs no delay
> - */
> - switch (command) {
> -
> - case NAND_CMD_NONE:
> - case NAND_CMD_PAGEPROG:
> - case NAND_CMD_ERASE1:
> - case NAND_CMD_ERASE2:
> - case NAND_CMD_SEQIN:
> - case NAND_CMD_STATUS:
> - case NAND_CMD_READID:
> - case NAND_CMD_SET_FEATURES:
> - return;
> -
> - case NAND_CMD_RESET:
> - if (chip->dev_ready)
> - break;
> - udelay(chip->chip_delay);
> - chip->cmd_ctrl(mtd, NAND_CMD_STATUS,
> - NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> - chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> - NAND_NCE | NAND_CTRL_CHANGE);
> - /* EZ-NAND can take upto 250ms as per ONFi v4.0 */
> - nand_wait_status_ready(mtd, 250);
> - return;
> -
> - /* This applies to read commands */
> - case NAND_CMD_READ0:
> - /*
> - * READ0 is sometimes used to exit GET STATUS mode. When this
> - * is the case no address cycles are requested, and we can use
> - * this information to detect that we should not wait for the
> - * device to be ready.
> - */
> - if (column == -1 && page_addr == -1)
> - return;
> -
> - default:
> - /*
> - * If we don't have access to the busy pin, we apply the given
> - * command delay
> - */
> - if (!chip->dev_ready) {
> - udelay(chip->chip_delay);
> - return;
> - }
> - }
> - /*
> - * Apply this short delay always to ensure that we do wait tWB in
> - * any case on any machine.
> - */
> - ndelay(100);
> -
> - nand_wait_ready(mtd);
> -}
> -
> static void nand_ccs_delay(struct nand_chip *chip)
> {
> /*
> @@ -882,26 +767,48 @@ static void nand_ccs_delay(struct nand_chip *chip)
> }
>
> /**
> - * nand_command_lp - [DEFAULT] Send command to NAND large page device
> + * nand_command - [DEFAULT] Send command to NAND device
> * @mtd: MTD device structure
> * @command: the command to be sent
> * @column: the column address for this command, -1 if none
> * @page_addr: the page address for this command, -1 if none
> *
> - * Send command to NAND device. This is the version for the new large page
> - * devices. We don't have the separate regions as we have in the small page
> - * devices. We must emulate NAND_CMD_READOOB to keep the code compatible.
> + * Send command to NAND device.
> */
> -static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> +static void nand_command(struct mtd_info *mtd, unsigned int command,
> int column, int page_addr)
> {
> register struct nand_chip *chip = mtd_to_nand(mtd);
> int ctrl = NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE;
> + /* Large page devices (> 512 bytes) behave slightly differently. */
> + bool is_lp = mtd->writesize > 512;
>
> - /* Emulate NAND_CMD_READOOB */
> - if (command == NAND_CMD_READOOB) {
> - column += mtd->writesize;
> - command = NAND_CMD_READ0;
> + if (is_lp) {
> + /* Large page devices don't have the separate regions as we
> + * have in the small page devices. We must emulate
> + * NAND_CMD_READOOB to keep the code compatible.
> + */
> + if (command == NAND_CMD_READOOB) {
> + column += mtd->writesize;
> + command = NAND_CMD_READ0;
> + }
> + } else if (command == NAND_CMD_SEQIN) {
> + /* Write out the command to the device */
> + int readcmd;
> +
> + if (column >= mtd->writesize) {
> + /* OOB area */
> + column -= mtd->writesize;
> + readcmd = NAND_CMD_READOOB;
> + } else if (column < 256) {
> + /* First 256 bytes --> READ0 */
> + readcmd = NAND_CMD_READ0;
> + } else {
> + column -= 256;
> + readcmd = NAND_CMD_READ1;
> + }
> + chip->cmd_ctrl(mtd, readcmd, ctrl);
> + ctrl &= ~NAND_CTRL_CHANGE;
> }
>
> /* Command latch cycle */
> @@ -920,7 +827,7 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> ctrl &= ~NAND_CTRL_CHANGE;
>
> /* Only output a single addr cycle for 8bits opcodes. */
> - if (!nand_opcode_8bits(command))
> + if (is_lp && !nand_opcode_8bits(command))
> chip->cmd_ctrl(mtd, column >> 8, ctrl);
> }
> if (page_addr != -1) {
> @@ -939,7 +846,6 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> switch (command) {
>
> case NAND_CMD_NONE:
> - case NAND_CMD_CACHEDPROG:
> case NAND_CMD_PAGEPROG:
> case NAND_CMD_ERASE1:
> case NAND_CMD_ERASE2:
> @@ -949,9 +855,17 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> case NAND_CMD_SET_FEATURES:
> return;
>
> + case NAND_CMD_CACHEDPROG:
> + if (is_lp)
> + return;
> + break;
> +
> case NAND_CMD_RNDIN:
> - nand_ccs_delay(chip);
> - return;
> + if (is_lp) {
> + nand_ccs_delay(chip);
> + return;
> + }
> + break;
>
> case NAND_CMD_RESET:
> if (chip->dev_ready)
> @@ -966,40 +880,44 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> return;
>
> case NAND_CMD_RNDOUT:
> - /* No ready / busy check necessary */
> - chip->cmd_ctrl(mtd, NAND_CMD_RNDOUTSTART,
> - NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> - chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> - NAND_NCE | NAND_CTRL_CHANGE);
> -
> - nand_ccs_delay(chip);
> - return;
> + if (is_lp) {
> + /* No ready / busy check necessary */
> + chip->cmd_ctrl(mtd, NAND_CMD_RNDOUTSTART,
> + NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> + chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> + NAND_NCE | NAND_CTRL_CHANGE);
> +
> + nand_ccs_delay(chip);
> + return;
> + }
> + break;
>
> case NAND_CMD_READ0:
> /*
> * READ0 is sometimes used to exit GET STATUS mode. When this
> * is the case no address cycles are requested, and we can use
> - * this information to detect that READSTART should not be
> - * issued.
> + * this information to detect that that we should not wait for
> + * the device to be ready and READSTART should not be issued.
> */
> if (column == -1 && page_addr == -1)
> return;
>
> - chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> - NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> - chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> - NAND_NCE | NAND_CTRL_CHANGE);
> -
> - /* This applies to read commands */
> - default:
> - /*
> - * If we don't have access to the busy pin, we apply the given
> - * command delay.
> - */
> - if (!chip->dev_ready) {
> - udelay(chip->chip_delay);
> - return;
> + if (is_lp) {
> + chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> + NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> + chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> + NAND_NCE | NAND_CTRL_CHANGE);
> }
> + /* Read commands must wait */
> + break;
> + }
> + /*
> + * If we don't have access to the busy pin, we apply the given command
> + * delay.
> + */
> + if (!chip->dev_ready) {
> + udelay(chip->chip_delay);
> + return;
> }
>
> /*
> @@ -5180,16 +5098,6 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
> chip->ecc_step_ds = 512;
> } else if (chip->parameters.onfi.version >= 21 &&
> (le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
> -
> - /*
> - * The nand_flash_detect_ext_param_page() uses the
> - * Change Read Column command which maybe not supported
> - * by the chip->cmdfunc. So try to update the chip->cmdfunc
> - * now. We do not replace user supplied command function.
> - */
> - if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
> - chip->cmdfunc = nand_command_lp;
> -
> /* The Extended Parameter Page is supported since ONFI 2.1. */
> if (nand_flash_detect_ext_param_page(chip, p))
> pr_warn("Failed to detect ONFI extended param page\n");
> @@ -5686,10 +5594,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> chip->badblockbits = 8;
> chip->erase = single_erase;
>
> - /* Do not replace user supplied command function! */
> - if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
> - chip->cmdfunc = nand_command_lp;
> -
> pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
> maf_id, dev_id);
> pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
More information about the linux-mtd
mailing list