Skip to content

Commit

Permalink
mtd/spinand: rework detect procedure for different READ_ID operation
Browse files Browse the repository at this point in the history 
Currently there are 3 different variants of read_id implementation:
1. opcode only. Found in GD5FxGQ4xF.
2. opcode + 1 addr byte. Found in GD5GxGQ4xA/E
3. opcode + 1 dummy byte. Found in other currently supported chips.

Original implementation was for variant 1 and let detect function
of chips with variant 2 and 3 to ignore the first byte. This isn't
robust:

1. For chips of variant 2, if SPI master doesn't keep MOSI low
during read, chip will get a random id offset, and the entire id
buffer will shift by that offset, causing detect failure.

2. For chips of variant 1, if it happens to get a devid that equals
to manufacture id of variant 2 or 3 chips, it'll get incorrectly
detected.

This patch reworks detect procedure to address problems above. New
logic do detection for all variants separatedly, in 1-2-3 order.
Since all current detect methods do exactly the same id matching
procedure, unify them into core.c and remove detect method from
manufacture_ops.

This is a rework of Chuanhong Guo <[email protected]> patch
submitted to linux kernel

Signed-off-by: Mikhail Kshevetskiy <[email protected]>
Signed-off-by: Frieder Schrempf <[email protected]>
Link: https://lore.kernel.org/all/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
  • Loading branch information
@mkshevetskiy @passgat
mkshevetskiy authored and passgat committed Oct 13, 2023
1 parent 8670027 commit b20913e
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 208 deletions.
104 changes: 65 additions & 39 deletions drivers/mtd/nand/spi/core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <linux/mtd/spinand.h>
#include <linux/of.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/spi/spi.h>
#include <linux/spi/spi-mem.h>
#else
Expand Down Expand Up @@ -452,9 +453,11 @@ static int spinand_wait(struct spinand_device *spinand, u8 *s)
return status & STATUS_BUSY ? -ETIMEDOUT : 0;
}

static int spinand_read_id_op(struct spinand_device *spinand, u8 *buf)
static int spinand_read_id_op(struct spinand_device *spinand, u8 naddr,
u8 ndummy, u8 *buf)
{
struct spi_mem_op op = SPINAND_READID_OP(0, spinand->scratchbuf,
struct spi_mem_op op = SPINAND_READID_OP(naddr, ndummy,
spinand->scratchbuf,
SPINAND_MAX_ID_LEN);
int ret;

Expand Down Expand Up @@ -808,21 +811,6 @@ static int spinand_mtd_block_isreserved(struct mtd_info *mtd, loff_t offs)
return ret;
}

const struct spi_mem_op *
spinand_find_supported_op(struct spinand_device *spinand,
const struct spi_mem_op *ops,
unsigned int nops)
{
unsigned int i;

for (i = 0; i < nops; i++) {
if (spi_mem_supports_op(spinand->slave, &ops[i]))
return &ops[i];
}

return NULL;
}

static const struct nand_ops spinand_ops = {
.erase = spinand_erase,
.markbad = spinand_markbad,
Expand All @@ -837,24 +825,62 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = {
&winbond_spinand_manufacturer,
};

static int spinand_manufacturer_detect(struct spinand_device *spinand)
static int spinand_manufacturer_match(struct spinand_device *spinand,
enum spinand_readid_method rdid_method)
{
u8 *id = spinand->id.data;
unsigned int i;
int ret;

for (i = 0; i < ARRAY_SIZE(spinand_manufacturers); i++) {
ret = spinand_manufacturers[i]->ops->detect(spinand);
if (ret > 0) {
spinand->manufacturer = spinand_manufacturers[i];
return 0;
} else if (ret < 0) {
return ret;
}
}
const struct spinand_manufacturer *manufacturer =
spinand_manufacturers[i];

if (id[0] != manufacturer->id)
continue;

ret = spinand_match_and_init(spinand,
manufacturer->chips,
manufacturer->nchips,
rdid_method);
if (ret < 0)
continue;

spinand->manufacturer = manufacturer;
return 0;
}
return -ENOTSUPP;
}

static int spinand_id_detect(struct spinand_device *spinand)
{
u8 *id = spinand->id.data;
int ret;

ret = spinand_read_id_op(spinand, 0, 0, id);
if (ret)
return ret;
ret = spinand_manufacturer_match(spinand, SPINAND_READID_METHOD_OPCODE);
if (!ret)
return 0;

ret = spinand_read_id_op(spinand, 1, 0, id);
if (ret)
return ret;
ret = spinand_manufacturer_match(spinand,
SPINAND_READID_METHOD_OPCODE_ADDR);
if (!ret)
return 0;

ret = spinand_read_id_op(spinand, 0, 1, id);
if (ret)
return ret;
ret = spinand_manufacturer_match(spinand,
SPINAND_READID_METHOD_OPCODE_DUMMY);

return ret;
}

static int spinand_manufacturer_init(struct spinand_device *spinand)
{
if (spinand->manufacturer->ops->init)
Expand Down Expand Up @@ -910,9 +936,9 @@ spinand_select_op_variant(struct spinand_device *spinand,
* @spinand: SPI NAND object
* @table: SPI NAND device description table
* @table_size: size of the device description table
* @rdid_method: read id method to match
*
* Should be used by SPI NAND manufacturer drivers when they want to find a
* match between a device ID retrieved through the READ_ID command and an
* Match between a device ID retrieved through the READ_ID command and an
* entry in the SPI NAND description table. If a match is found, the spinand
* object will be initialized with information provided by the matching
* spinand_info entry.
Expand All @@ -921,22 +947,28 @@ spinand_select_op_variant(struct spinand_device *spinand,
*/
int spinand_match_and_init(struct spinand_device *spinand,
const struct spinand_info *table,
unsigned int table_size, u8 devid)
unsigned int table_size,
enum spinand_readid_method rdid_method)
{
u8 *id = spinand->id.data;
struct nand_device *nand = spinand_to_nand(spinand);
unsigned int i;

for (i = 0; i < table_size; i++) {
const struct spinand_info *info = &table[i];
const struct spi_mem_op *op;

if (devid != info->devid)
if (rdid_method != info->devid.method)
continue;

if (memcmp(id + 1, info->devid.id, info->devid.len))
continue;

nand->memorg = table[i].memorg;
nand->eccreq = table[i].eccreq;
spinand->eccinfo = table[i].eccinfo;
spinand->flags = table[i].flags;
spinand->id.len = 1 + table[i].devid.len;
spinand->select_target = table[i].select_target;

op = spinand_select_op_variant(spinand,
Expand Down Expand Up @@ -972,13 +1004,7 @@ static int spinand_detect(struct spinand_device *spinand)
if (ret)
return ret;

ret = spinand_read_id_op(spinand, spinand->id.data);
if (ret)
return ret;

spinand->id.len = SPINAND_MAX_ID_LEN;

ret = spinand_manufacturer_detect(spinand);
ret = spinand_id_detect(spinand);
if (ret) {
dev_err(spinand->slave->dev, "unknown raw ID %02x %02x %02x %02x\n",
spinand->id.data[0], spinand->id.data[1],
Expand Down Expand Up @@ -1083,11 +1109,11 @@ static int spinand_init(struct spinand_device *spinand)
for (i = 0; i < nand->memorg.ntargets; i++) {
ret = spinand_select_target(spinand, i);
if (ret)
goto err_free_bufs;
goto err_manuf_cleanup;

ret = spinand_lock_block(spinand, BL_ALL_UNLOCKED);
if (ret)
goto err_free_bufs;
goto err_manuf_cleanup;
}

ret = nanddev_init(nand, &spinand_ops, THIS_MODULE);
Expand Down
30 changes: 6 additions & 24 deletions drivers/mtd/nand/spi/gigadevice.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ static const struct mtd_ooblayout_ops gd5fxgqxxexxg_ooblayout = {
};

static const struct spinand_info gigadevice_spinand_table[] = {
SPINAND_INFO("GD5F1GQ4UExxG", 0xd1,
SPINAND_INFO("GD5F1GQ4UExxG",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xd1),
NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
NAND_ECCREQ(8, 512),
SPINAND_INFO_OP_VARIANTS(&gd5fxgq4_read_cache_variants,
Expand All @@ -167,7 +168,8 @@ static const struct spinand_info gigadevice_spinand_table[] = {
0,
SPINAND_ECCINFO(&gd5fxgqxxexxg_ooblayout,
gd5fxgq4xexxg_ecc_get_status)),
SPINAND_INFO("GD5F1GQ5UExxG", 0x51,
SPINAND_INFO("GD5F1GQ5UExxG",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x51),
NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
NAND_ECCREQ(4, 512),
SPINAND_INFO_OP_VARIANTS(&gd5f1gq5_read_cache_variants,
Expand All @@ -178,33 +180,13 @@ static const struct spinand_info gigadevice_spinand_table[] = {
gd5fxgq5xexxg_ecc_get_status)),
};

static int gigadevice_spinand_detect(struct spinand_device *spinand)
{
u8 *id = spinand->id.data;
int ret;

/*
* For GD NANDs, There is an address byte needed to shift in before IDs
* are read out, so the first byte in raw_id is dummy.
*/
if (id[1] != SPINAND_MFR_GIGADEVICE)
return 0;

ret = spinand_match_and_init(spinand, gigadevice_spinand_table,
ARRAY_SIZE(gigadevice_spinand_table),
id[2]);
if (ret)
return ret;

return 1;
}

static const struct spinand_manufacturer_ops gigadevice_spinand_manuf_ops = {
.detect = gigadevice_spinand_detect,
};

const struct spinand_manufacturer gigadevice_spinand_manufacturer = {
.id = SPINAND_MFR_GIGADEVICE,
.name = "GigaDevice",
.chips = gigadevice_spinand_table,
.nchips = ARRAY_SIZE(gigadevice_spinand_table),
.ops = &gigadevice_spinand_manuf_ops,
};
45 changes: 16 additions & 29 deletions drivers/mtd/nand/spi/macronix.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand,
}

static const struct spinand_info macronix_spinand_table[] = {
SPINAND_INFO("MX35LF1GE4AB", 0x12,
SPINAND_INFO("MX35LF1GE4AB",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x12),
NAND_MEMORG(1, 2048, 64, 64, 1024, 1, 1, 1),
NAND_ECCREQ(4, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
Expand All @@ -114,15 +115,17 @@ static const struct spinand_info macronix_spinand_table[] = {
SPINAND_HAS_QE_BIT,
SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
mx35lf1ge4ab_ecc_get_status)),
SPINAND_INFO("MX35LF2GE4AB", 0x22,
SPINAND_INFO("MX35LF2GE4AB",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x22),
NAND_MEMORG(1, 2048, 64, 64, 2048, 2, 1, 1),
NAND_ECCREQ(4, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
&write_cache_variants,
&update_cache_variants),
SPINAND_HAS_QE_BIT,
SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)),
SPINAND_INFO("MX35UF4GE4AD", 0xb7,
SPINAND_INFO("MX35UF4GE4AD",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xb7),
NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
NAND_ECCREQ(8, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
Expand All @@ -131,7 +134,8 @@ static const struct spinand_info macronix_spinand_table[] = {
SPINAND_HAS_QE_BIT,
SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
mx35lf1ge4ab_ecc_get_status)),
SPINAND_INFO("MX35UF2GE4AD", 0xa6,
SPINAND_INFO("MX35UF2GE4AD",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xa6),
NAND_MEMORG(1, 2048, 128, 64, 2048, 1, 1, 1),
NAND_ECCREQ(8, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
Expand All @@ -140,7 +144,8 @@ static const struct spinand_info macronix_spinand_table[] = {
SPINAND_HAS_QE_BIT,
SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
mx35lf1ge4ab_ecc_get_status)),
SPINAND_INFO("MX35UF2GE4AC", 0xa2,
SPINAND_INFO("MX35UF2GE4AC",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xa2),
NAND_MEMORG(1, 2048, 64, 64, 2048, 1, 1, 1),
NAND_ECCREQ(4, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
Expand All @@ -149,7 +154,8 @@ static const struct spinand_info macronix_spinand_table[] = {
SPINAND_HAS_QE_BIT,
SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
mx35lf1ge4ab_ecc_get_status)),
SPINAND_INFO("MX35UF1GE4AD", 0x96,
SPINAND_INFO("MX35UF1GE4AD",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x96),
NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
NAND_ECCREQ(8, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
Expand All @@ -158,7 +164,8 @@ static const struct spinand_info macronix_spinand_table[] = {
SPINAND_HAS_QE_BIT,
SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
mx35lf1ge4ab_ecc_get_status)),
SPINAND_INFO("MX35UF1GE4AC", 0x92,
SPINAND_INFO("MX35UF1GE4AC",
SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92),
NAND_MEMORG(1, 2048, 64, 64, 1024, 1, 1, 1),
NAND_ECCREQ(4, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
Expand All @@ -170,33 +177,13 @@ static const struct spinand_info macronix_spinand_table[] = {

};

static int macronix_spinand_detect(struct spinand_device *spinand)
{
u8 *id = spinand->id.data;
int ret;

/*
* Macronix SPI NAND read ID needs a dummy byte, so the first byte in
* raw_id is garbage.
*/
if (id[1] != SPINAND_MFR_MACRONIX)
return 0;

ret = spinand_match_and_init(spinand, macronix_spinand_table,
ARRAY_SIZE(macronix_spinand_table),
id[2]);
if (ret)
return ret;

return 1;
}

static const struct spinand_manufacturer_ops macronix_spinand_manuf_ops = {
.detect = macronix_spinand_detect,
};

const struct spinand_manufacturer macronix_spinand_manufacturer = {
.id = SPINAND_MFR_MACRONIX,
.name = "Macronix",
.chips = macronix_spinand_table,
.nchips = ARRAY_SIZE(macronix_spinand_table),
.ops = &macronix_spinand_manuf_ops,
};
Loading

0 comments on commit b20913e

Please sign in to comment.