[PATCHv2] drivers: mtd: spinand: Add generic spinand frameowrk.

Florian Fainelli florian at openwrt.org
Wed Jul 3 13:17:51 EDT 2013


Hello,

2013/7/3 Sourav Poddar <sourav.poddar at ti.com>:
> From: Mona Anonuevo <manonuevo at micron.com>
>
> This patch adds support for a generic spinand framework(spinand_mtd.c).
> This frameowrk can be used for other spi based flash devices. The idea
> is to have a common model under drivers/mtd, as also present for other non spi
> devices(there is a generic framework and device part simply attaches itself to it.)

Resending my comments since your previous submissino

>
> Signed-off-by: Mona Anonuevo <manonuevo at micron.com>
> Signed-off-by: Tuan Nguyen <tqnguyen at micron.com>
> Signed-off-by: Sourav Poddar <sourav.poddar at ti.com>
> ----
>

[snip]

> +if MTD_SPINAND
> +
> +config MTD_SPINAND_ONDIEECC
> +       bool "Use SPINAND internal ECC"
> +       help
> +        Internel ECC
> +
> +config MTD_SPINAND_SWECC
> +       bool "Use software ECC"
> +       depends on MTD_NAND
> +       help
> +        software ECC

Can this somehow be made a runtime thing?

[snip]

> +               if (count < oob_num && ops->oobbuf && chip->oobbuf) {
> +                       int size;
> +                       int offset, len, temp;
> +
> +                       /* repack spare to oob */
> +                       memset(chip->oobbuf, 0, info->ecclayout->oobavail);
> +
> +                       temp = 0;
> +                       offset = info->ecclayout->oobfree[0].offset;
> +                       len = info->ecclayout->oobfree[0].length;
> +                       memcpy(chip->oobbuf + temp,
> +                               chip->buf + info->page_main_size + offset, len);

Sounds like a for look might be useful here

> +
> +                       temp += len;
> +                       offset = info->ecclayout->oobfree[1].offset;
> +                       len = info->ecclayout->oobfree[1].length;
> +                       memcpy(chip->oobbuf + temp,
> +                               chip->buf + info->page_main_size + offset, len);
> +
> +                       temp += len;
> +                       offset = info->ecclayout->oobfree[2].offset;
> +                       len = info->ecclayout->oobfree[2].length;
> +                       memcpy(chip->oobbuf + temp,
> +                               chip->buf + info->page_main_size + offset, len);
> +
> +                       temp += len;
> +                       offset = info->ecclayout->oobfree[3].offset;
> +                       len = info->ecclayout->oobfree[3].length;
> +                       memcpy(chip->oobbuf + temp,
> +                               chip->buf + info->page_main_size + offset, len);
> +

[snip]

> +                       /* repack oob to spare */
> +                       temp = 0;
> +                       offset = info->ecclayout->oobfree[0].offset;
> +                       len = info->ecclayout->oobfree[0].length;
> +                       memcpy(chip->buf + info->page_main_size + offset,
> +                                       chip->oobbuf + temp, len);

And here too.

> +
> +                       temp += len;
> +                       offset = info->ecclayout->oobfree[1].offset;
> +                       len = info->ecclayout->oobfree[1].length;
> +                       memcpy(chip->buf + info->page_main_size + offset,
> +                                       chip->oobbuf + temp, len);
> +
> +                       temp += len;
> +                       offset = info->ecclayout->oobfree[2].offset;
> +                       len = info->ecclayout->oobfree[2].length;
> +                       memcpy(chip->buf + info->page_main_size + offset,
> +                                       chip->oobbuf + temp, len);
> +
> +                       temp += len;
> +                       offset = info->ecclayout->oobfree[3].offset;
> +                       len = info->ecclayout->oobfree[3].length;
> +                       memcpy(chip->buf + info->page_main_size + offset,
> +                                       chip->oobbuf + temp, len);
> +               }

[snip]

> +++ b/include/linux/mtd/spinand.h
> @@ -0,0 +1,155 @@
> +/*
> + *  linux/include/linux/mtd/spinand.h
> + *  Copyright (c) 2009-2010 Micron Technology, Inc.
> + *  This software is licensed under the terms of the GNU General Public
> + *  License version 2, as published by the Free Software Foundation, and
> + *  may be copied, distributed, and modified under those terms.
> +
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> +/bin/bash: 4: command not found
> + *
> + *  based on nand.h
> + */
> +#ifndef __LINUX_MTD_SPI_NAND_H
> +#define __LINUX_MTD_SPI_NAND_H
> +
> +#include <linux/wait.h>
> +#include <linux/spinlock.h>
> +#include <linux/mtd/mtd.h>
> +
> +/* cmd */
> +#define CMD_READ                               0x13
> +#define CMD_READ_RDM                   0x03
> +#define CMD_PROG_PAGE_CLRCACHE 0x02
> +#define CMD_PROG_PAGE                  0x84
> +#define CMD_PROG_PAGE_EXC              0x10
> +#define CMD_ERASE_BLK                  0xd8
> +#define CMD_WR_ENABLE                  0x06
> +#define CMD_WR_DISABLE                 0x04
> +#define CMD_READ_ID                    0x9f
> +#define CMD_RESET                              0xff
> +#define CMD_READ_REG                   0x0f
> +#define CMD_WRITE_REG                  0x1f

Can we somehow namespace these defines so they are not so generic?
Something like SPI_NAND_CMD_READ would be just fine I guess.

> +
> +/* feature/ status reg */
> +#define REG_BLOCK_LOCK         0xa0
> +#define REG_OTP                                0xb0
> +#define REG_STATUS                     0xc0/* timing */
> +
> +/* status */
> +#define STATUS_OIP_MASK                0x01
> +#define STATUS_READY           (0 << 0)
> +#define STATUS_BUSY                    (1 << 0)
> +
> +#define STATUS_E_FAIL_MASK     0x04
> +#define STATUS_E_FAIL          (1 << 2)
> +
> +#define STATUS_P_FAIL_MASK     0x08
> +#define STATUS_P_FAIL          (1 << 3)
> +
> +#define STATUS_ECC_MASK                0x30
> +#define STATUS_ECC_1BIT_CORRECTED      (1 << 4)
> +#define STATUS_ECC_ERROR                       (2 << 4)
> +#define STATUS_ECC_RESERVED                    (3 << 4)
> +
> +
> +/*ECC enable defines*/
> +#define OTP_ECC_MASK           0x10
> +#define OTP_ECC_OFF                    0
> +#define OTP_ECC_ON                     1
> +
> +#define ECC_DISABLED
> +#define ECC_IN_NAND
> +#define ECC_SOFT
> +
> +/* block lock */
> +#define BL_ALL_LOCKED      0x38
> +#define BL_1_2_LOCKED      0x30
> +#define BL_1_4_LOCKED      0x28
> +#define BL_1_8_LOCKED      0x20
> +#define BL_1_16_LOCKED     0x18
> +#define BL_1_32_LOCKED     0x10
> +#define BL_1_64_LOCKED     0x08
> +#define BL_ALL_UNLOCKED    0

--
Florian



More information about the linux-mtd mailing list