Boris, Am 12.05.2016 um 14:36 schrieb Boris Brezillon: > Hi Richard, > > On Thu, 12 May 2016 13:32:45 +0200 > Richard Weinberger <[email protected]> wrote: > >> Sometimes all you need is a NAND with a given page, erase and chip size >> to load and inspect a certain image. >> OOB, ECC sizes and other metrics don't matter much then. >> In such a situation I find myself often fiddling around with NAND IDs >> to get what I need, especially when the given NAND ID from the datasheet >> decode to something else than the NAND advertises via ONFI. >> Using the new nandsim.approx module parameter it is possible to specify >> the page size, number of pages per block and the total number of blocks. >> Nandsim will then emulate a SLC NAND with 128 bytes OOB and the given >> sizes. Nandsim achieves this by providing ONFI parameters to NAND core. > > The current nandsim implementation is indeed incomplete and does not > allow for the full NAND chain emulation. > I first considered dispatching the emulation bit in the different > components (the NAND controller, the NAND chip and the generic/standard > code), but that means introduction a new infrastructure and I'm now > unsure that this is appropriate.
Yep. But a facelifting for nandsim would not harm. ;) > The other question we should ask ourselves is whether this NAND > emulation layer belongs here. I mean, we could emulate a bunch of NAND > chips and NAND controller directly in Qemu instead of trying to teach > nandsim how to emulate advanced chips. I like nandsim a lot because I can use it without visualization. Especially for stress testing UBI/UBIFS nandsim on bare metal is very useful. >> >> Signed-off-by: Richard Weinberger <[email protected]> >> --- >> drivers/mtd/nand/nand_base.c | 12 +++++---- >> drivers/mtd/nand/nandsim.c | 62 >> +++++++++++++++++++++++++++++++++++++++----- >> include/linux/mtd/nand.h | 3 +++ >> 3 files changed, 66 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index 557b846..25432c2 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -3208,7 +3208,7 @@ static void sanitize_string(uint8_t *s, size_t len) >> strim(s); >> } >> >> -static u16 onfi_crc16(u16 crc, u8 const *p, size_t len) >> +u16 nand_onfi_crc16(u16 crc, u8 const *p, size_t len) >> { >> int i; >> while (len--) { >> @@ -3246,7 +3246,7 @@ static int nand_flash_detect_ext_param_page(struct >> mtd_info *mtd, >> >> /* Read out the Extended Parameter Page. */ >> chip->read_buf(mtd, (uint8_t *)ep, len); >> - if ((onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2) >> + if ((nand_onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2) >> != le16_to_cpu(ep->crc))) { >> pr_debug("fail in the CRC.\n"); >> goto ext_out; >> @@ -3328,14 +3328,16 @@ static int nand_flash_detect_onfi(struct mtd_info >> *mtd, struct nand_chip *chip, >> /* Try ONFI for unknown chip or LP */ >> chip->cmdfunc(mtd, NAND_CMD_READID, 0x20, -1); >> if (chip->read_byte(mtd) != 'O' || chip->read_byte(mtd) != 'N' || >> - chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I') >> + chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I') { >> + >> return 0; >> + } > > Hm, this change seems unnecessary. Whops, there was a debug printk. >> >> chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); >> for (i = 0; i < 3; i++) { >> for (j = 0; j < sizeof(*p); j++) >> ((uint8_t *)p)[j] = chip->read_byte(mtd); >> - if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == >> + if (nand_onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == >> le16_to_cpu(p->crc)) { >> break; >> } >> @@ -3442,7 +3444,7 @@ static int nand_flash_detect_jedec(struct mtd_info >> *mtd, struct nand_chip *chip, >> for (j = 0; j < sizeof(*p); j++) >> ((uint8_t *)p)[j] = chip->read_byte(mtd); >> >> - if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 510) == >> + if (nand_onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 510) == >> le16_to_cpu(p->crc)) >> break; >> } >> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c >> index a58169a2..b5be38a 100644 >> --- a/drivers/mtd/nand/nandsim.c >> +++ b/drivers/mtd/nand/nandsim.c >> @@ -114,6 +114,15 @@ static u_char id_bytes[8] = { >> [3] = CONFIG_NANDSIM_FOURTH_ID_BYTE, >> [4 ... 7] = 0xFF, >> }; >> +static unsigned int approx[3]; >> + >> +static struct nand_onfi_params id_onfi = { >> + .sig = {'O', 'N', 'F', 'I'}, >> + .manufacturer = "nandsim ", >> + .model = "nandsim ", >> +}; >> + >> +static unsigned char *id_p; >> >> module_param_array(id_bytes, byte, NULL, 0400); >> module_param_named(first_id_byte, id_bytes[0], byte, 0400); >> @@ -139,6 +148,7 @@ module_param(overridesize, uint, 0400); >> module_param(cache_file, charp, 0400); >> module_param(bbt, uint, 0400); >> module_param(bch, uint, 0400); >> +module_param_array(approx, uint, NULL, 0400); >> >> MODULE_PARM_DESC(id_bytes, "The ID bytes returned by NAND Flash 'read >> ID' command"); >> MODULE_PARM_DESC(first_id_byte, "The first byte returned by NAND Flash >> 'read ID' command (manufacturer ID) (obsolete)"); >> @@ -174,6 +184,10 @@ MODULE_PARM_DESC(cache_file, "File to use to cache >> nand pages instead of mem >> MODULE_PARM_DESC(bbt, "0 OOB, 1 BBT with marker in OOB, 2 >> BBT with marker in data area"); >> MODULE_PARM_DESC(bch, "Enable BCH ecc and set how many bits >> should " >> "be correctable in 512-byte blocks"); >> +MODULE_PARM_DESC(approx, "Approximation mode, simulate a good enough >> NAND that satisfies" >> + " page size, block size and number of blocks." >> + " e.g. 4096,128,8192 will give you a NAND with >> 4KiB page size, 128 pages per" >> + " block and 8192 blocks in total."); >> >> /* The largest possible page size */ >> #define NS_LARGEST_PAGE_SIZE 4096 >> @@ -229,6 +243,7 @@ MODULE_PARM_DESC(bch, "Enable BCH ecc and >> set how many bits should " >> #define STATE_CMD_RESET 0x0000000C /* reset */ >> #define STATE_CMD_RNDOUT 0x0000000D /* random output command */ >> #define STATE_CMD_RNDOUTSTART 0x0000000E /* random output start command */ >> +#define STATE_CMD_PARAM 0x0000000F /* param output start command */ >> #define STATE_CMD_MASK 0x0000000F /* command states mask */ >> >> /* After an address is input, the simulator goes to one of these states */ >> @@ -245,7 +260,8 @@ MODULE_PARM_DESC(bch, "Enable BCH ecc and >> set how many bits should " >> #define STATE_DATAOUT 0x00001000 /* waiting for page data output */ >> #define STATE_DATAOUT_ID 0x00002000 /* waiting for ID bytes output */ >> #define STATE_DATAOUT_STATUS 0x00003000 /* waiting for status output */ >> -#define STATE_DATAOUT_MASK 0x00007000 /* data output states mask */ >> +#define STATE_DATAOUT_ONFI 0x00007000 /* waiting for ONFI output */ >> +#define STATE_DATAOUT_MASK 0x0000F000 /* data output states mask */ >> >> /* Previous operation is done, ready to accept new requests */ >> #define STATE_READY 0x00000000 >> @@ -307,7 +323,6 @@ struct nandsim { >> unsigned int nbparts; >> >> uint busw; /* flash chip bus width (8 or 16) */ >> - u_char ids[8]; /* chip's ID bytes */ >> uint32_t options; /* chip's characteristic bits */ >> uint32_t state; /* current chip state */ >> uint32_t nxstate; /* next expected state */ >> @@ -408,6 +423,8 @@ static struct nandsim_operations { >> {OPT_ANY, {STATE_CMD_STATUS, STATE_DATAOUT_STATUS, STATE_READY}}, >> /* Read ID */ >> {OPT_ANY, {STATE_CMD_READID, STATE_ADDR_ZERO, STATE_DATAOUT_ID, >> STATE_READY}}, >> + /* Read param */ >> + {OPT_ANY, {STATE_CMD_PARAM, STATE_ADDR_ZERO, STATE_DATAOUT_ONFI, >> STATE_READY}}, >> /* Large page devices read page */ >> {OPT_LARGEPAGE, {STATE_CMD_READ0, STATE_ADDR_PAGE, STATE_CMD_READSTART >> | ACTION_CPY, >> STATE_DATAOUT, STATE_READY}}, >> @@ -1077,6 +1094,8 @@ static char *get_state_name(uint32_t state) >> return "STATE_CMD_RNDOUT"; >> case STATE_CMD_RNDOUTSTART: >> return "STATE_CMD_RNDOUTSTART"; >> + case STATE_CMD_PARAM: >> + return "STATE_CMD_PARAM"; >> case STATE_ADDR_PAGE: >> return "STATE_ADDR_PAGE"; >> case STATE_ADDR_SEC: >> @@ -1125,6 +1144,7 @@ static int check_command(int cmd) >> case NAND_CMD_RESET: >> case NAND_CMD_RNDOUT: >> case NAND_CMD_RNDOUTSTART: >> + case NAND_CMD_PARAM: >> return 0; >> >> default: >> @@ -1164,6 +1184,8 @@ static uint32_t get_state_by_command(unsigned command) >> return STATE_CMD_RNDOUT; >> case NAND_CMD_RNDOUTSTART: >> return STATE_CMD_RNDOUTSTART; >> + case NAND_CMD_PARAM: >> + return STATE_CMD_PARAM; >> } >> >> NS_ERR("get_state_by_command: unknown command, BUG\n"); >> @@ -1856,9 +1878,17 @@ static void switch_state(struct nandsim *ns) >> break; >> >> case STATE_DATAOUT_ID: >> - ns->regs.num = ns->geom.idbytes; >> + if (ns->regs.row == 0x20) { >> + ns->regs.num = sizeof(id_onfi.sig); >> + id_p = (unsigned char *)id_onfi.sig; >> + } else { >> + ns->regs.num = ns->geom.idbytes; >> + id_p = (unsigned char *)id_bytes; >> + } >> + break; >> + case STATE_DATAOUT_ONFI: >> + ns->regs.num = sizeof(id_onfi); >> break; >> - Another whitespace damage^^. >> case STATE_DATAOUT_STATUS: >> ns->regs.count = ns->regs.num = 0; >> break; >> @@ -1951,7 +1981,11 @@ static u_char ns_nand_read_byte(struct mtd_info *mtd) >> break; >> case STATE_DATAOUT_ID: >> NS_DBG("read_byte: read ID byte %d, total = %d\n", >> ns->regs.count, ns->regs.num); >> - outb = ns->ids[ns->regs.count]; >> + outb = id_p[ns->regs.count]; >> + ns->regs.count += 1; >> + break; >> + case STATE_DATAOUT_ONFI: >> + outb = ((unsigned char *)&id_onfi)[ns->regs.count]; >> ns->regs.count += 1; >> break; >> default: >> @@ -2289,10 +2323,26 @@ static int __init ns_init_module(void) >> nand->geom.idbytes = 4; >> else >> nand->geom.idbytes = 2; >> + >> + if (approx[0] && approx[1] && approx[2]) { >> + id_onfi.byte_per_page = cpu_to_le32(approx[0]); >> + id_onfi.pages_per_block = cpu_to_le32(approx[1]); >> + id_onfi.blocks_per_lun = cpu_to_le32(approx[2]); >> + >> + id_onfi.spare_bytes_per_page = cpu_to_le32(128); >> + id_onfi.lun_count = cpu_to_le32(1); >> + id_onfi.bits_per_cell = cpu_to_le32(1); >> + id_onfi.revision = cpu_to_le32(1 << 5); >> + id_onfi.crc = cpu_to_le16(nand_onfi_crc16(ONFI_CRC_BASE, >> (uint8_t *)&id_onfi, >> + sizeof(id_onfi) - >> sizeof(id_onfi.crc))); >> + >> + nand->geom.idbytes = 2; >> + id_bytes[0] = id_bytes[1] = 0xFF; >> + } >> + >> nand->regs.status = NS_STATUS_OK(nand); >> nand->nxstate = STATE_UNKNOWN; >> nand->options |= OPT_PAGE512; /* temporary value */ >> - memcpy(nand->ids, id_bytes, sizeof(nand->ids)); >> if (bus_width == 16) { >> nand->busw = 16; >> chip->options |= NAND_BUSWIDTH_16; >> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h >> index 56574ba..82f6db2 100644 >> --- a/include/linux/mtd/nand.h >> +++ b/include/linux/mtd/nand.h >> @@ -50,6 +50,9 @@ extern int nand_lock(struct mtd_info *mtd, loff_t ofs, >> uint64_t len); >> /* unlocks specified locked blocks */ >> extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len); >> >> +/* calculate ONFI CRC */ >> +extern u16 nand_onfi_crc16(u16 crc, u8 const *p, size_t len); >> + >> /* The maximum number of NAND chips in an array */ >> #define NAND_MAX_CHIPS 8 >> > > Just a general feedback on the whole approach. If what you really want > is a solution to skip the chip detection and manually set the chip > info, then you shouldn't call nand_scan_ident() and manually fill > the mtd and nand_chip fields instead. Okay, that would work too. I was not sure whether I'm allowed to do so. Thanks, //richard
