Re: [PATCH] mtd: spinand: Add support for all Toshiba Memory products

From: Schrempf Frieder
Date: Wed Jan 16 2019 - 07:55:45 EST


On 16.01.19 11:42, Frieder Schrempf wrote:
> Hi Yoshio,
>
> On 16.01.19 06:53, Yoshio Furuyama wrote:
>> Add device table for Toshiba Memory products.
>> Also, generalize OOB layout structure and function names.
>>
>> Signed-off-by: Yoshio Furuyama <tmcmc-mb-yfuruyama7@xxxxxxxxxxxxxxxx>
>>
>> ---
>> Â drivers/mtd/nand/spi/toshiba.c |ÂÂ 79
>> +++++++++++++++++++++++++++++++++-------
>> Â 1 file changed, 65 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/spi/toshiba.c
>> b/drivers/mtd/nand/spi/toshiba.c
>> index 0812655..0916962 100644
>> --- a/drivers/mtd/nand/spi/toshiba.c
>> +++ b/drivers/mtd/nand/spi/toshiba.c
>> @@ -25,19 +25,19 @@ static SPINAND_OP_VARIANTS(write_cache_variants,
>> Â static SPINAND_OP_VARIANTS(update_cache_variants,
>> ÂÂÂÂÂÂÂÂÂ SPINAND_PROG_LOAD(false, 0, NULL, 0));
>> -static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
>> +static int tc58cxgxsx_ooblayout_ecc(struct mtd_info *mtd, int section,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct mtd_oob_region *region)
>> Â {
>> -ÂÂÂ if (section > 7)
>> +ÂÂÂ if (section > 0)
>> ÂÂÂÂÂÂÂÂÂ return -ERANGE;
>> -ÂÂÂ region->offset = 128 + 16 * section;
>> -ÂÂÂ region->length = 16;
>> +ÂÂÂ region->offset = mtd->oobsize / 2;
>> +ÂÂÂ region->length = mtd->oobsize / 2;
>> ÂÂÂÂÂ return 0;
>> Â }
>> -static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
>> +static int tc58cxgxsx_ooblayout_free(struct mtd_info *mtd, int section,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct mtd_oob_region *region)
>> Â {
>> ÂÂÂÂÂ if (section > 0)
>> @@ -45,17 +45,17 @@ static int tc58cvg2s0h_ooblayout_free(struct
>> mtd_info *mtd, int section,
>> ÂÂÂÂÂ /* 2 bytes reserved for BBM */
>> ÂÂÂÂÂ region->offset = 2;
>> -ÂÂÂ region->length = 126;
>> +ÂÂÂ region->length = mtd->oobsize / 2 - 2;
>> ÂÂÂÂÂ return 0;
>> Â }
>> -static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
>> -ÂÂÂ .ecc = tc58cvg2s0h_ooblayout_ecc,
>> -ÂÂÂ .free = tc58cvg2s0h_ooblayout_free,
>> +static const struct mtd_ooblayout_ops tc58cxgxsx_ooblayout = {
>> +ÂÂÂ .ecc = tc58cxgxsx_ooblayout_ecc,
>> +ÂÂÂ .free = tc58cxgxsx_ooblayout_free,
>> Â };
>> -static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
>> +static int tc58cxgxsx_ecc_get_status(struct spinand_device *spinand,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ u8 status)
>> Â {
>> ÂÂÂÂÂ struct nand_device *nand = spinand_to_nand(spinand);
>> @@ -94,15 +94,66 @@ static int tc58cvg2s0h_ecc_get_status(struct
>> spinand_device *spinand,
>> Â }
>> Â static const struct spinand_info toshiba_spinand_table[] = {
>> -ÂÂÂ SPINAND_INFO("TC58CVG2S0H", 0xCD,
>> +ÂÂÂ /* 3.3V 1Gb */
>> +ÂÂÂ SPINAND_INFO("TC58CVG0S3", 0xC2,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_ECCREQ(8, 512),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &write_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &update_cache_variants),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ 0,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_ECCINFO(&tc58cxgxsx_ooblayout,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tc58cxgxsx_ecc_get_status)),
>> +ÂÂÂ /* 3.3V 2Gb */
>> +ÂÂÂ SPINAND_INFO("TC58CVG1S3", 0xCB,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_MEMORG(1, 2048, 128, 64, 2048, 1, 1, 1),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_ECCREQ(8, 512),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &write_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &update_cache_variants),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ 0,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_ECCINFO(&tc58cxgxsx_ooblayout,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tc58cxgxsx_ecc_get_status)),
>> +ÂÂÂ /* 3.3V 4Gb */
>> +ÂÂÂ SPINAND_INFO("TC58CVG2S0", 0xCD,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_ECCREQ(8, 512),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &write_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &update_cache_variants),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ 0,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_ECCINFO(&tc58cxgxsx_ooblayout,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tc58cxgxsx_ecc_get_status)),
>> +ÂÂÂ /* 1.8V 1Gb */
>> +ÂÂÂ SPINAND_INFO("TC58CYG0S3", 0xB2,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_ECCREQ(8, 512),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &write_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &update_cache_variants),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ 0,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_ECCINFO(&tc58cxgxsx_ooblayout,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tc58cxgxsx_ecc_get_status)),
>> +ÂÂÂ /* 1.8V 2Gb */
>> +ÂÂÂ SPINAND_INFO("TC58CYG1S3", 0xBB,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_MEMORG(1, 2048, 128, 64, 2048, 1, 1, 1),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ NAND_ECCREQ(8, 512),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &write_cache_variants,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &update_cache_variants),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ 0,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_ECCINFO(&tc58cxgxsx_ooblayout,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tc58cxgxsx_ecc_get_status)),
>> +ÂÂÂ /* 1.8V 4Gb */
>> +ÂÂÂ SPINAND_INFO("TC58CYG2S0", 0xBD,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ NAND_ECCREQ(8, 512),
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &write_cache_variants,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &update_cache_variants),
>> -ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_HAS_QE_BIT,
>
> You're removing the SPINAND_HAS_QE_BIT. I just double checked one of the
> datasheets and it seems like there is indeed no QE bit in the
> configuration register. I wonder where I got that information from.
> Maybe I was looking at the wrong datasheet!?
>
> Anyway, I will try to do a quick test on my device with TC58CVG2S0 and
> let you now about the results.

As expected, it seems to work fine without setting the QE bit. The bit
is marked as reserved in the datasheet, so it probably does not hurt
much that the driver queued for 5.0 sets it.

>
> Thanks,
> Frieder
>
>> -ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
>> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tc58cvg2s0h_ecc_get_status)),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ 0,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂ SPINAND_ECCINFO(&tc58cxgxsx_ooblayout,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tc58cxgxsx_ecc_get_status)),
>> Â };
>> Â static int toshiba_spinand_detect(struct spinand_device *spinand)
>>