Re: [PATCH v2] mtd: spinand: micron: add support for MT29F2G01AAAED
On Sun, 23 Aug 2020 19:14:10 +0800
Thirumalesha Narasimhappa wrote:
> The MT29F2G01AAAED is a single die, 2Gb Micron SPI NAND Flash with 4-bit
> ECC
>
> Signed-off-by: Thirumalesha Narasimhappa
> ---
> v2: removed SPINAND_SELECT_TARGET as per the comments & fixed typo errors
>
> drivers/mtd/nand/spi/micron.c | 78 +++
> 1 file changed, 78 insertions(+)
>
> diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
> index 5d370cfcdaaa..c21ca395d657 100644
> --- a/drivers/mtd/nand/spi/micron.c
> +++ b/drivers/mtd/nand/spi/micron.c
> @@ -18,6 +18,9 @@
> #define MICRON_STATUS_ECC_4TO6_BITFLIPS (3 << 4)
> #define MICRON_STATUS_ECC_7TO8_BITFLIPS (5 << 4)
>
> +/* For Micron MT29F2G01AAAED Device */
> +#define MICRON_STATUS_ECC_1TO4_BITFLIPS (1 << 4)
> +
You shouldn't need that new definition (see below).
> #define MICRON_CFG_CRBIT(0)
>
> /*
> @@ -44,6 +47,19 @@ static SPINAND_OP_VARIANTS(update_cache_variants,
> SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> SPINAND_PROG_LOAD(false, 0, NULL, 0));
>
> +/* Micron MT29F2G01AAAED Device */
> +static SPINAND_OP_VARIANTS(read_cache_variants_mt29f2g01aaaed,
> + SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants_mt29f2g01aaaed,
> + SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants_mt29f2g01aaaed,
> + SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
Okay, I'd suggest picking more generic names. How about renaming the
existing variants into quadio_read_cache_variants,
x4_write_cache_variants and x4_update_cache_variants and naming the new
ones x4_read_cache_variants, x1_write_cache_variants and
x1_update_cache_variants.
> static int micron_8_ooblayout_ecc(struct mtd_info *mtd, int section,
> struct mtd_oob_region *region)
> {
> @@ -69,11 +85,41 @@ static int micron_8_ooblayout_free(struct mtd_info *mtd,
> int section,
> return 0;
> }
>
> +static int mt29f2g01aaaed_ooblayout_ecc(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *region)
> +{
> + if (section >= 4)
> + return -ERANGE;
Hm, I'd try to deduce the max section based on the
pagesize/ecc-step-size so we can re-use the same ooblayout def for different
pagesize/ecc-step-size combinations.
> +
> + region->offset = (section * 16) + 8;
> + region->length = 8;
> +
> + return 0;
> +}
> +
> +static int mt29f2g01aaaed_ooblayout_free(struct mtd_info *mtd, int section,
> + struct mtd_oob_region *region)
> +{
> + if (section >= 4)
> + return -ERANGE;
> +
> + /* Reserve 2 bytes for the BBM. */
> + region->offset = (section * 16) + 2;
You should probably only reserve those 2 bytes in section 0 (where the
BBM is).
> + region->length = 6;
> +
> + return 0;
> +}
> +
> static const struct mtd_ooblayout_ops micron_8_ooblayout = {
> .ecc = micron_8_ooblayout_ecc,
> .free = micron_8_ooblayout_free,
> };
>
> +static const struct mtd_ooblayout_ops mt29f2g01aaaed_ooblayout = {
Maybe name that one micron_interleaved_ooblayout, and rename
micron_8_ooblayout into micron_grouped_ooblayout.
> + .ecc = mt29f2g01aaaed_ooblayout_ecc,
> + .free = mt29f2g01aaaed_ooblayout_free,
> +};
> +
> static int micron_select_target(struct spinand_device *spinand,
> unsigned int target)
> {
> @@ -114,6 +160,27 @@ static int micron_8_ecc_get_status(struct spinand_device
> *spinand,
> return -EINVAL;
> }
>
> +static int mt29f2g01aaaed_ecc_get_status(struct spinand_device *spinand,
> + u8 status)
> +{
> + switch (status & MICRON_STATUS_ECC_MASK) {
> + case STATUS_ECC_NO_BITFLIPS:
> + return 0;
> +
> + case STATUS_ECC_UNCOR_ERROR:
> + return -EBADMSG;
> +
> + /* 1 to 4-bit error detected and corrected */
> + case MICRON_STATUS_ECC_1TO4_BITFLIPS:
> + return 4;
> +
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
Looks like you're duplicating spinand_check_ecc_status(). Just leave the
get_status hook to NULL, that should do trick.
Re: [PATCH v2] mtd: spinand: micron: add support for MT29F2G01AAAED
Hi Thirumalesha,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.9-rc1 next-20200821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Thirumalesha-Narasimhappa/mtd-spinand-micron-add-support-for-MT29F2G01AAAED/20200823-191310
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
c3d8f220d01220a5b253e422be407d068dc65511
config: openrisc-randconfig-m031-20200823 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=openrisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/mtd/nand/spi/micron.c:323: error: unterminated argument list
>> invoking macro "SPINAND_INFO"
323 | };
|
>> drivers/mtd/nand/spi/micron.c:288:2: error: 'SPINAND_INFO' undeclared here
>> (not in a function)
288 | SPINAND_INFO("MT29F2G01AAAED",
| ^~~~
>> drivers/mtd/nand/spi/micron.c:323: error: expected '}' at end of input
323 | };
|
drivers/mtd/nand/spi/micron.c:184:59: note: to match this '{'
184 | static const struct spinand_info micron_spinand_table[] = {
| ^
drivers/mtd/nand/spi/micron.c:184:34: warning: 'micron_spinand_table'
defined but not used [-Wunused-const-variable=]
184 | static const struct spinand_info micron_spinand_table[] = {
| ^~~~
drivers/mtd/nand/spi/micron.c:163:12: warning:
'mt29f2g01aaaed_ecc_get_status' defined but not used [-Wunused-function]
163 | static int mt29f2g01aaaed_ecc_get_status(struct spinand_device
*spinand,
|^
drivers/mtd/nand/spi/micron.c:118:39: warning: 'mt29f2g01aaaed_ooblayout'
defined but not used [-Wunused-const-variable=]
118 | static const struct mtd_ooblayout_ops mt29f2g01aaaed_ooblayout = {
| ^~~~
In file included from drivers/mtd/nand/spi/micron.c:11:
drivers/mtd/nand/spi/micron.c:60:28: warning:
'update_cache_variants_mt29f2g01aaaed' defined but not used
[-Wunused-const-variable=]
60 | static SPINAND_OP_VARIANTS(update_cache_variants_mt29f2g01aaaed,
|^~~~
include/linux/mtd/spinand.h:265:35: note: in definition of macro
'SPINAND_OP_VARIANTS'
265 | const struct spinand_op_variants name = { \
| ^~~~
drivers/mtd/nand/spi/micron.c:57:28: warning:
'write_cache_variants_mt29f2g01aaaed' defined but not used
[-Wunused-const-variable=]
57 | static SPINAND_OP_VARIANTS(write_cache_variants_mt29f2g01aaaed,
|^~~
include/linux/mtd/spinand.h:265:35: note: in definition of macro
'SPINAND_OP_VARIANTS'
265 | const struct spinand_op_variants name = { \
| ^~~~
drivers/mtd/nand/spi/micron.c:51:28: warning:
'read_cache_variants_mt29f2g01aaaed' defined but not used
[-Wunused-const-variable=]
51 | static SPINAND_OP_VARIANTS(read_cache_variants_mt29f2g01aaaed,
|^~
include/linux/mtd/spinand.h:265:35: note: in definition of macro
'SPINAND_OP_VARIANTS'
265 | const struct spinand_op_variants name = { \
| ^~~~
#
https://github.com/0day-ci/linux/commit/8dc175bd1853ebc961fea42976cffc290b5fbf22
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Thirumalesha-Narasimhappa/mtd-spinand-micron-add-support-for-MT29F2G01AAAED/20200823-191310
git checkout 8dc175bd1853ebc961fea42976cffc290b5fbf22
vim +/SPINAND_INFO +323 drivers/mtd/nand/spi/micron.c
8dc175bd1853eb Thirumalesha Narasimhappa 2020-08-23 183
a508e8875e135d Peter Pan 2018-06-22 184 static const struct
spinand_info micron_spinand_table[] = {
8511a3a9937e30 Shivamurthy Shastri 2020-03-11 185/* M79A 2Gb
3.3V */
f1541773af49ec Chuanhong Guo 2020-02-08 186
SPINAND_INFO("MT29F2G01ABAGD",
f1541773af49ec Chuanhong Guo 2020-02-08 187
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x24),
377e517b5fa535 Boris Brezillon
[PATCH v2] mtd: spinand: micron: add support for MT29F2G01AAAED
The MT29F2G01AAAED is a single die, 2Gb Micron SPI NAND Flash with 4-bit
ECC
Signed-off-by: Thirumalesha Narasimhappa
---
v2: removed SPINAND_SELECT_TARGET as per the comments & fixed typo errors
drivers/mtd/nand/spi/micron.c | 78 +++
1 file changed, 78 insertions(+)
diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
index 5d370cfcdaaa..c21ca395d657 100644
--- a/drivers/mtd/nand/spi/micron.c
+++ b/drivers/mtd/nand/spi/micron.c
@@ -18,6 +18,9 @@
#define MICRON_STATUS_ECC_4TO6_BITFLIPS(3 << 4)
#define MICRON_STATUS_ECC_7TO8_BITFLIPS(5 << 4)
+/* For Micron MT29F2G01AAAED Device */
+#define MICRON_STATUS_ECC_1TO4_BITFLIPS(1 << 4)
+
#define MICRON_CFG_CR BIT(0)
/*
@@ -44,6 +47,19 @@ static SPINAND_OP_VARIANTS(update_cache_variants,
SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
SPINAND_PROG_LOAD(false, 0, NULL, 0));
+/* Micron MT29F2G01AAAED Device */
+static SPINAND_OP_VARIANTS(read_cache_variants_mt29f2g01aaaed,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
+
+static SPINAND_OP_VARIANTS(write_cache_variants_mt29f2g01aaaed,
+ SPINAND_PROG_LOAD(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants_mt29f2g01aaaed,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
static int micron_8_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *region)
{
@@ -69,11 +85,41 @@ static int micron_8_ooblayout_free(struct mtd_info *mtd,
int section,
return 0;
}
+static int mt29f2g01aaaed_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section >= 4)
+ return -ERANGE;
+
+ region->offset = (section * 16) + 8;
+ region->length = 8;
+
+ return 0;
+}
+
+static int mt29f2g01aaaed_ooblayout_free(struct mtd_info *mtd, int section,
+struct mtd_oob_region *region)
+{
+ if (section >= 4)
+ return -ERANGE;
+
+ /* Reserve 2 bytes for the BBM. */
+ region->offset = (section * 16) + 2;
+ region->length = 6;
+
+ return 0;
+}
+
static const struct mtd_ooblayout_ops micron_8_ooblayout = {
.ecc = micron_8_ooblayout_ecc,
.free = micron_8_ooblayout_free,
};
+static const struct mtd_ooblayout_ops mt29f2g01aaaed_ooblayout = {
+ .ecc = mt29f2g01aaaed_ooblayout_ecc,
+ .free = mt29f2g01aaaed_ooblayout_free,
+};
+
static int micron_select_target(struct spinand_device *spinand,
unsigned int target)
{
@@ -114,6 +160,27 @@ static int micron_8_ecc_get_status(struct spinand_device
*spinand,
return -EINVAL;
}
+static int mt29f2g01aaaed_ecc_get_status(struct spinand_device *spinand,
+u8 status)
+{
+ switch (status & MICRON_STATUS_ECC_MASK) {
+ case STATUS_ECC_NO_BITFLIPS:
+ return 0;
+
+ case STATUS_ECC_UNCOR_ERROR:
+ return -EBADMSG;
+
+ /* 1 to 4-bit error detected and corrected */
+ case MICRON_STATUS_ECC_1TO4_BITFLIPS:
+ return 4;
+
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
static const struct spinand_info micron_spinand_table[] = {
/* M79A 2Gb 3.3V */
SPINAND_INFO("MT29F2G01ABAGD",
@@ -217,6 +284,17 @@ static const struct spinand_info micron_spinand_table[] = {
SPINAND_ECCINFO(_8_ooblayout,
micron_8_ecc_get_status),
SPINAND_SELECT_TARGET(micron_select_target)),
+ /* M69A 2Gb 3.3V */
+ SPINAND_INFO("MT29F2G01AAAED",
+SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x9F),
+NAND_MEMORG(1, 2048, 64, 64, 2048, 80, 2, 1, 1),
+NAND_ECCREQ(4, 512),
+
SPINAND_INFO_OP_VARIANTS(_cache_variants_mt29f2g01aaaed,
+
_cache_variants_mt29f2g01aaaed,
+
_cache_variants_mt29f2g01aaaed),
+0,
+SPINAND_ECCINFO(_ooblayout,
+mt29f2g01aaaed_ecc_get_status),
};
static int micron_spinand_init(struct spinand_device *spinand)
--
2.17.1
