| Message ID | 20180222202918.8708-2-stefan@agner.ch |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | mtd: nand: vf610_nfc: make use of ->exec_op() | expand |
On Thu, 22 Feb 2018 21:29:17 +0100 Stefan Agner <stefan@agner.ch> wrote: > This reworks the driver to make use of ->exec_op() callback. The > command sequencer of the VF610 NFC aligns well with the new ops > interface. > > The ops are translated to a NFC command code while filling the > necessary registers. Instead of using the special status and > read id command codes (which require the status/id form special > registers) the driver now uses the main data buffer for all > commands. This simplifies the driver as no special casing is > needed. > > For control data (status byte, id bytes and parameter page) the > driver needs to reverse byte order for little endian CPUs since > the controller seems to store the bytes in big endian order in > the data buffer. > > The current state seems to pass MTD tests on a Colibri VF61. > > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > drivers/mtd/nand/raw/vf610_nfc.c | 439 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 425 insertions(+), 14 deletions(-) > > diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c > index 5d7a1f8f580f..9baf80766307 100644 > --- a/drivers/mtd/nand/raw/vf610_nfc.c > +++ b/drivers/mtd/nand/raw/vf610_nfc.c > @@ -74,6 +74,22 @@ > #define RESET_CMD_CODE 0x4040 > #define STATUS_READ_CMD_CODE 0x4068 > > +/* NFC_CMD2[CODE] controller cycle bit masks */ > +#define COMMAND_CMD_BYTE1 BIT(14) > +#define COMMAND_CAR_BYTE1 BIT(13) > +#define COMMAND_CAR_BYTE2 BIT(12) > +#define COMMAND_RAR_BYTE1 BIT(11) > +#define COMMAND_RAR_BYTE2 BIT(10) > +#define COMMAND_RAR_BYTE3 BIT(9) > +#define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) - 1) > +#define COMMAND_WRITE_DATA BIT(8) > +#define COMMAND_CMD_BYTE2 BIT(7) > +#define COMMAND_RB_HANDSHAKE BIT(6) > +#define COMMAND_READ_DATA BIT(5) > +#define COMMAND_CMD_BYTE3 BIT(4) > +#define COMMAND_READ_STATUS BIT(3) > +#define COMMAND_READ_ID BIT(2) > + > /* NFC ECC mode define */ > #define ECC_BYPASS 0 > #define ECC_45_BYTE 6 > @@ -97,10 +113,14 @@ > /* NFC_COL_ADDR Field */ > #define COL_ADDR_MASK 0x0000FFFF > #define COL_ADDR_SHIFT 0 > +#define COL_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) > + > > /* NFC_ROW_ADDR Field */ > #define ROW_ADDR_MASK 0x00FFFFFF > #define ROW_ADDR_SHIFT 0 > +#define ROW_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) > + > #define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000 > #define ROW_ADDR_CHIP_SEL_RB_SHIFT 28 > #define ROW_ADDR_CHIP_SEL_MASK 0x0F000000 > @@ -165,6 +185,7 @@ struct vf610_nfc { > enum vf610_nfc_variant variant; > struct clk *clk; > bool use_hw_ecc; > + bool page_access; This field deserves a comment IMO, even if you describe in details what it does in the rd/wr_from/to_sram() funcs. > u32 ecc_mode; > }; > > @@ -173,6 +194,11 @@ static inline struct vf610_nfc *mtd_to_nfc(struct mtd_info *mtd) > return container_of(mtd_to_nand(mtd), struct vf610_nfc, chip); > } > > +static inline struct vf610_nfc *chip_to_nfc(struct nand_chip *chip) > +{ > + return container_of(chip, struct vf610_nfc, chip); > +} > + > static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg) > { > return readl(nfc->regs + reg); > @@ -214,6 +240,86 @@ static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src, > memcpy(dst, src, n); > } > > +static inline bool vf610_nfc_is_little_endian(void) It's not really the NFC that is LE, is it? I mean, you could have the kernel configured in BIG ENDIAN for this platform, and then you wouldn't have to do the byte-swapping. > +{ > +#ifdef __LITTLE_ENDIAN > + return true; > +#else > + return false; > +#endif > +} > + > +/** > + * Read accessor for internal SRAM buffer > + * @dst: destination address in regular memory > + * @src: source address in SRAM buffer > + * @len: bytes to copy > + * @fix_endian: Fix endianness if required > + * > + * Use this accessor for the internal SRAM buffers. On the ARM > + * Freescale Vybrid SoC it's known that the driver can treat > + * the SRAM buffer as if it's memory. Other platform might need > + * to treat the buffers differently. > + * > + * The controller stores bytes from the NAND chip internally in big > + * endianness. On little endian platforms such as Vybrid this leads > + * to reversed byte order. > + * For performance reason (and earlier probably due to unanawareness) ^ unawareness > + * the driver avoids correcting endianness where it has control over > + * write and read side (e.g. page wise data access). > + * In case endianness matters len should be a multiple of 4. > + */ > +static inline void vf610_nfc_rd_from_sram(void *dst, const void __iomem *src, > + size_t len, bool fix_endian) > +{ > + if (vf610_nfc_is_little_endian() && fix_endian) { > + unsigned int i; > + > + for (i = 0; i < len; i += 4) { > + u32 val = be32_to_cpu(__raw_readl(src + i)); > + memcpy(dst + i, &val, min(sizeof(val), len - i)); > + } > + } else { > + memcpy_fromio(dst, src, len); > + } > +} > + > +/** > + * Write accessor for internal SRAM buffer > + * @dst: destination address in SRAM buffer > + * @src: source address in regular memory > + * @len: bytes to copy > + * @fix_endian: Fix endianness if required > + * > + * Use this accessor for the internal SRAM buffers. On the ARM > + * Freescale Vybrid SoC it's known that the driver can treat > + * the SRAM buffer as if it's memory. Other platform might need > + * to treat the buffers differently. > + * > + * The controller stores bytes from the NAND chip internally in big > + * endianness. On little endian platforms such as Vybrid this leads > + * to reversed byte order. > + * For performance reason (and earlier probably due to unanawareness) ^ unawareness > + * the driver avoids correcting endianness where it has control over > + * write and read side (e.g. page wise data access). > + * In case endianness matters len should be a multiple of 4. That's the opposite actually. If fix_endian is set, we don't have any requirement on len alignment, because the cpu_to_be32(val) will save us. But we have a problem when fix_endian is false an len is not aligned on 4, because, AFAIR memcpy_toio() does 32-bit accesses when things are properly aligned on 32 bits, and when that's not the case it falls back to 16 or 8 bit accesses, which in our case may lead to data loss on LE platforms. > + */ > +static inline void vf610_nfc_wr_to_sram(void __iomem *dst, const void *src, > + size_t len, bool fix_endian) > +{ > + if (vf610_nfc_is_little_endian() && fix_endian) { > + unsigned int i; > + > + for (i = 0; i < len; i += 4) { > + u32 val; > + memcpy(&val, src + i, min(sizeof(val), len - i)); > + __raw_writel(cpu_to_be32(val), dst + i); > + } > + } else { > + memcpy_toio(dst, src, len); > + } > +} > + > /* Clear flags for upcoming command */ > static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc) > { > @@ -489,6 +595,170 @@ static int vf610_nfc_dev_ready(struct mtd_info *mtd) > return 1; > } > > +static inline void vf610_nfc_run(struct vf610_nfc *nfc, u32 col, u32 row, u32 cmd1, u32 cmd2, u32 trfr_sz) > +{ > + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK, > + COL_ADDR_SHIFT, col); > + > + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK, > + ROW_ADDR_SHIFT, row); > + > + vf610_nfc_write(nfc, NFC_SECTOR_SIZE, trfr_sz); > + vf610_nfc_write(nfc, NFC_FLASH_CMD1, cmd1); > + vf610_nfc_write(nfc, NFC_FLASH_CMD2, cmd2); > + > + dev_dbg(nfc->dev, "col 0x%08x, row 0x%08x, cmd1 0x%08x, cmd2 0x%08x, trfr_sz %d\n", > + col, row, cmd1, cmd2, trfr_sz); > + > + vf610_nfc_done(nfc); > +} > + > +static inline const struct nand_op_instr *vf610_get_next_instr( > + const struct nand_subop *subop, int *op_id) > +{ > + if (*op_id + 1 >= subop->ninstrs) > + return NULL; > + > + (*op_id)++; > + > + return &subop->instrs[*op_id]; > +} > + > +static int vf610_nfc_cmd(struct nand_chip *chip, > + const struct nand_subop *subop) > +{ > + const struct nand_op_instr *instr; > + struct vf610_nfc *nfc = chip_to_nfc(chip); > + int op_id = -1, trfr_sz = 0, offset; > + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; > + bool force8bit = false; > + > + /* > + * Some ops are optional, but the hardware requires the operations > + * to be in this exact order. > + * The op parser enforces the order and makes sure that there isn't > + * a read and write element in a single operation. > + */ > + instr = vf610_get_next_instr(subop, &op_id); > + if (!instr) > + return -EINVAL; > + > + if (instr && instr->type == NAND_OP_CMD_INSTR) { > + dev_dbg(nfc->dev, "OP_CMD: code 0x%02x\n", instr->ctx.cmd.opcode); > + cmd2 |= instr->ctx.cmd.opcode << CMD_BYTE1_SHIFT; > + code |= COMMAND_CMD_BYTE1; > + > + instr = vf610_get_next_instr(subop, &op_id); > + } > + > + if (instr && instr->type == NAND_OP_ADDR_INSTR) { > + int naddrs = nand_subop_get_num_addr_cyc(subop, op_id); > + int i = nand_subop_get_addr_start_off(subop, op_id); > + > + for (; i < naddrs; i++) { > + u8 val = instr->ctx.addr.addrs[i]; > + if (i < 2) > + col |= COL_ADDR(i, val); > + else > + row |= ROW_ADDR(i - 2, val); > + } > + code |= COMMAND_NADDR_BYTES(naddrs); > + > + dev_dbg(nfc->dev, "OP_ADDR: col %d, row %d\n", col, row); > + > + instr = vf610_get_next_instr(subop, &op_id); > + } > + > + if (instr && instr->type == NAND_OP_DATA_OUT_INSTR) { > + trfr_sz = nand_subop_get_data_len(subop, op_id); > + offset = nand_subop_get_data_start_off(subop, op_id); > + force8bit = instr->ctx.data.force_8bit; > + > + dev_dbg(nfc->dev, "OP_DATA_OUT: len %d, offset %d\n", > + trfr_sz, offset); > + > + /* We don't care about endianness when writing a NAND page */ > + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0) + offset, > + instr->ctx.data.buf.in + offset, > + trfr_sz, !nfc->page_access); > + code |= COMMAND_WRITE_DATA; > + > + instr = vf610_get_next_instr(subop, &op_id); > + } > + > + if (instr && instr->type == NAND_OP_CMD_INSTR) { > + cmd1 |= instr->ctx.cmd.opcode << CMD_BYTE2_SHIFT; > + code |= COMMAND_CMD_BYTE2; > + > + instr = vf610_get_next_instr(subop, &op_id); You seem to have debug traces almost everywhere except here and... > + } > + > + if (instr && instr->type == NAND_OP_WAITRDY_INSTR) { > + code |= COMMAND_RB_HANDSHAKE; > + > + instr = vf610_get_next_instr(subop, &op_id); ... here. Can we please make that consistent. Either drop all of them or add the missing ones. IMHO it's more interesting to have a dev_dbg() giving the col, row, cmd1, cmd2 and trfr_sz values in vf610_nfc_run() rather than those you add here, because they kind of duplicate the information given in nand_op_parser_trace(). > + } > + > + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { > + trfr_sz = nand_subop_get_data_len(subop, op_id); > + offset = nand_subop_get_data_start_off(subop, op_id); > + force8bit = instr->ctx.data.force_8bit; > + > + dev_dbg(nfc->dev, "OP_DATA_IN: len %d, offset %d\n", > + trfr_sz, offset); > + > + code |= COMMAND_READ_DATA; > + } Still not a big fan of the if (instr && instr->type == NAND_OP_XXX_INSTR) { ... instr = vf610_get_next_instr(subop, &op_id); } approach, but I'm ready to make a concession on that :-). > + > + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) > + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); > + > + cmd2 |= code << CMD_CODE_SHIFT; > + > + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); > + > + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { > + /* We don't care about endianness when reading a NAND page */ > + vf610_nfc_rd_from_sram(instr->ctx.data.buf.in + offset, > + nfc->regs + NFC_MAIN_AREA(0) + offset, > + trfr_sz, !nfc->page_access); > + } > + > + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) > + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); > + > + return 0; > +} > + > +static const struct nand_op_parser vf610_nfc_op_parser = NAND_OP_PARSER( > + NAND_OP_PARSER_PATTERN( > + vf610_nfc_cmd, > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), > + NAND_OP_PARSER_PAT_DATA_OUT_ELEM(true, PAGE_2K + OOB_MAX), > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true)), > + NAND_OP_PARSER_PATTERN( > + vf610_nfc_cmd, > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true), > + NAND_OP_PARSER_PAT_DATA_IN_ELEM(true, PAGE_2K + OOB_MAX)), > + ); > + > +static int vf610_nfc_exec_op(struct nand_chip *chip, > + const struct nand_operation *op, > + bool check_only) > +{ > + struct vf610_nfc *nfc = chip_to_nfc(chip); > + > + dev_dbg(nfc->dev, "exec_op, opcode 0x%02x\n", op->instrs[0].ctx.cmd.opcode); Bad idea: according to the pattern list you described above the first instruction is not necessarily a CMD instruction. I think you should drop this dev_dbg(). > + > + return nand_op_parser_exec_op(chip, &vf610_nfc_op_parser, op, check_only); > +} > + > + > /* > * This function supports Vybrid only (MPC5125 would have full RB and four CS) > */ > @@ -526,9 +796,14 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, > if (!(ecc_status & ECC_STATUS_MASK)) > return ecc_count; > > - /* Read OOB without ECC unit enabled */ > - vf610_nfc_command(mtd, NAND_CMD_READOOB, 0, page); > - vf610_nfc_read_buf(mtd, oob, mtd->oobsize); > + /* > + * Read OOB without ECC unit enabled. We temporarily set ->page_access > + * to true to make sure vf610_nfc_cmd() does not swap bytes when > + * reading data from the internal SRAM. > + */ > + nfc->page_access = true; > + nand_read_oob_op(&nfc->chip, page, 0, oob, mtd->oobsize); > + nfc->page_access = false; > > /* > * On an erased page, bit count (including OOB) should be zero or > @@ -542,12 +817,46 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, > static int vf610_nfc_read_page(struct mtd_info *mtd, struct nand_chip *chip, > uint8_t *buf, int oob_required, int page) > { > - int eccsize = chip->ecc.size; > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int trfr_sz = mtd->writesize + mtd->oobsize; > + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; > int stat; > > - nand_read_page_op(chip, page, 0, buf, eccsize); > + cmd2 |= NAND_CMD_READ0 << CMD_BYTE1_SHIFT; > + code |= COMMAND_CMD_BYTE1; > + > + code |= COMMAND_CAR_BYTE1; > + code |= COMMAND_CAR_BYTE2; > + > + row = ROW_ADDR(0, page & 0xff); > + code |= COMMAND_RAR_BYTE1; > + row |= ROW_ADDR(1, page >> 8); > + code |= COMMAND_RAR_BYTE2; > + > + if (chip->options & NAND_ROW_ADDR_3) { > + row |= ROW_ADDR(2, page >> 16); > + code |= COMMAND_RAR_BYTE3; > + } > + > + cmd1 |= NAND_CMD_READSTART << CMD_BYTE2_SHIFT; > + code |= COMMAND_CMD_BYTE2; > + > + code |= COMMAND_RB_HANDSHAKE; > + code |= COMMAND_READ_DATA; > + cmd2 |= code << CMD_CODE_SHIFT; > + > + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); > + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); > + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); > + > + /* We don't care about endianness when reading a NAND page */ > + vf610_nfc_rd_from_sram(buf, nfc->regs + NFC_MAIN_AREA(0), > + mtd->writesize, false); > if (oob_required) > - vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); > + vf610_nfc_rd_from_sram(chip->oob_poi, > + nfc->regs + NFC_MAIN_AREA(0) + > + mtd->writesize, > + mtd->oobsize, false); > > stat = vf610_nfc_correct_data(mtd, buf, chip->oob_poi, page); > > @@ -564,16 +873,113 @@ static int vf610_nfc_write_page(struct mtd_info *mtd, struct nand_chip *chip, > const uint8_t *buf, int oob_required, int page) > { > struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int trfr_sz = mtd->writesize + mtd->oobsize; > + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; > + int ret = 0; > + > + cmd2 |= NAND_CMD_SEQIN << CMD_BYTE1_SHIFT; > + code |= COMMAND_CMD_BYTE1; > + > + code |= COMMAND_CAR_BYTE1; > + code |= COMMAND_CAR_BYTE2; > + > + row = ROW_ADDR(0, page & 0xff); > + code |= COMMAND_RAR_BYTE1; > + row |= ROW_ADDR(1, page >> 8); > + code |= COMMAND_RAR_BYTE2; > + if (chip->options & NAND_ROW_ADDR_3) { > + row |= ROW_ADDR(2, page >> 16); > + code |= COMMAND_RAR_BYTE3; > + } > > - nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize); > - if (oob_required) > - vf610_nfc_write_buf(mtd, chip->oob_poi, mtd->oobsize); > + cmd1 |= NAND_CMD_PAGEPROG << CMD_BYTE2_SHIFT; > + code |= COMMAND_CMD_BYTE2; > + > + code |= COMMAND_WRITE_DATA; > + > + /* We don't care about endianness when writing a NAND page */ > + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0), buf, > + mtd->writesize, false); > > - /* Always write whole page including OOB due to HW ECC */ > - nfc->use_hw_ecc = true; > - nfc->write_sz = mtd->writesize + mtd->oobsize; > + code |= COMMAND_RB_HANDSHAKE; > + cmd2 |= code << CMD_CODE_SHIFT; > > - return nand_prog_page_end_op(chip); > + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); > + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); > + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); > + > + return ret; > +} > + > +static int vf610_nfc_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, > + uint8_t *buf, int oob_required, int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data > + * from the internal SRAM. > + */ > + nfc->page_access = true; > + ret = nand_read_page_raw(mtd, chip, buf, oob_required, page); > + nfc->page_access = false; > + > + return ret; > +} > + > +static int vf610_nfc_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, > + const uint8_t *buf, int oob_required, int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data ^ writing data to > + * from the internal SRAM. > + */ > + nfc->page_access = true; > + ret = nand_write_page_raw(mtd, chip, buf, oob_required, page); > + nfc->page_access = false; > + > + return ret; As you discovered while debugging the new implementation, this doesn't work, because then the STATUS byte is read without the proper byte-swapping enabled. So the solution would be to replace the above code by: nfc->page_access = true; ret = nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize); if (!ret && oob_required) ret = nand_write_data_op(chip, chip->oob_poi, mtd->oobsize, false); nfc->page_access = false; if (ret) return ret; return nand_prog_page_end_op(chip); > +} > + > +static int vf610_nfc_read_oob(struct mtd_info *mtd, struct nand_chip *chip, > + int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data > + * from the internal SRAM. > + */ > + nfc->page_access = true; > + ret = nand_read_oob_std(mtd, chip, page); > + nfc->page_access = false; > + > + return ret; > +} > +static int vf610_nfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, > + int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data > + * from the internal SRAM. > + */ > + nfc->page_access = true; > + ret = nand_write_oob_std(mtd, chip, page); > + nfc->page_access = false; > + > + return ret; Same problem here. The above logic should be replaced by: nfc->page_access = true; ret = nand_prog_page_begin_op(chip, page, mtd->writesize, chip->oob_poi, mtd->oobsize); nfc->page_access = false; if (ret) return ret; return nand_prog_page_end_op(chip); > } > > static const struct of_device_id vf610_nfc_dt_ids[] = { > @@ -590,6 +996,7 @@ static void vf610_nfc_preinit_controller(struct vf610_nfc *nfc) > vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT); > vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT); > vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT); > + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); > > /* Disable virtual pages, only one elementary transfer unit */ > vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK, > @@ -686,6 +1093,7 @@ static int vf610_nfc_probe(struct platform_device *pdev) > chip->read_word = vf610_nfc_read_word; > chip->read_buf = vf610_nfc_read_buf; > chip->write_buf = vf610_nfc_write_buf; > + chip->exec_op = vf610_nfc_exec_op; > chip->select_chip = vf610_nfc_select_chip; > chip->onfi_set_features = nand_onfi_get_set_features_notsupp; > chip->onfi_get_features = nand_onfi_get_set_features_notsupp; > @@ -755,7 +1163,10 @@ static int vf610_nfc_probe(struct platform_device *pdev) > > chip->ecc.read_page = vf610_nfc_read_page; > chip->ecc.write_page = vf610_nfc_write_page; > - > + chip->ecc.read_page_raw = vf610_nfc_read_page_raw; > + chip->ecc.write_page_raw = vf610_nfc_write_page_raw; > + chip->ecc.read_oob = vf610_nfc_read_oob; > + chip->ecc.write_oob = vf610_nfc_write_oob; > chip->ecc.size = PAGE_2K; > } >
Hi Stefan, Thanks for the work. Just a few comments below, nothing big. On Thu, 22 Feb 2018 21:29:17 +0100, Stefan Agner <stefan@agner.ch> wrote: > This reworks the driver to make use of ->exec_op() callback. The > command sequencer of the VF610 NFC aligns well with the new ops > interface. > > The ops are translated to a NFC command code while filling the I am not a fan of contractions in plain English, mostly when "operations" is an actual noun for something that is clear in the NAND framework. > necessary registers. Instead of using the special status and > read id command codes (which require the status/id form special s/id/ID? ^ ^ from? ^ The sentence is not very clear, maybe something like "Instead of using the special status and read ID command codes (which require a special configuration in the controller registers)." ? > registers) the driver now uses the main data buffer for all > commands. This simplifies the driver as no special casing is > needed. > > For control data (status byte, id bytes and parameter page) the > driver needs to reverse byte order for little endian CPUs since > the controller seems to store the bytes in big endian order in > the data buffer. > > The current state seems to pass MTD tests on a Colibri VF61. > > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > drivers/mtd/nand/raw/vf610_nfc.c | 439 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 425 insertions(+), 14 deletions(-) > > diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c > index 5d7a1f8f580f..9baf80766307 100644 > --- a/drivers/mtd/nand/raw/vf610_nfc.c > +++ b/drivers/mtd/nand/raw/vf610_nfc.c > @@ -74,6 +74,22 @@ > #define RESET_CMD_CODE 0x4040 > #define STATUS_READ_CMD_CODE 0x4068 > > +/* NFC_CMD2[CODE] controller cycle bit masks */ > +#define COMMAND_CMD_BYTE1 BIT(14) > +#define COMMAND_CAR_BYTE1 BIT(13) > +#define COMMAND_CAR_BYTE2 BIT(12) > +#define COMMAND_RAR_BYTE1 BIT(11) > +#define COMMAND_RAR_BYTE2 BIT(10) > +#define COMMAND_RAR_BYTE3 BIT(9) > +#define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) - 1) > +#define COMMAND_WRITE_DATA BIT(8) > +#define COMMAND_CMD_BYTE2 BIT(7) > +#define COMMAND_RB_HANDSHAKE BIT(6) > +#define COMMAND_READ_DATA BIT(5) > +#define COMMAND_CMD_BYTE3 BIT(4) > +#define COMMAND_READ_STATUS BIT(3) > +#define COMMAND_READ_ID BIT(2) > + > /* NFC ECC mode define */ > #define ECC_BYPASS 0 > #define ECC_45_BYTE 6 > @@ -97,10 +113,14 @@ > /* NFC_COL_ADDR Field */ > #define COL_ADDR_MASK 0x0000FFFF > #define COL_ADDR_SHIFT 0 > +#define COL_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) > + Extra space > > /* NFC_ROW_ADDR Field */ > #define ROW_ADDR_MASK 0x00FFFFFF > #define ROW_ADDR_SHIFT 0 > +#define ROW_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) > + > #define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000 > #define ROW_ADDR_CHIP_SEL_RB_SHIFT 28 > #define ROW_ADDR_CHIP_SEL_MASK 0x0F000000 > @@ -165,6 +185,7 @@ struct vf610_nfc { > enum vf610_nfc_variant variant; > struct clk *clk; > bool use_hw_ecc; > + bool page_access; > u32 ecc_mode; > }; > > @@ -173,6 +194,11 @@ static inline struct vf610_nfc *mtd_to_nfc(struct mtd_info *mtd) > return container_of(mtd_to_nand(mtd), struct vf610_nfc, chip); > } > > +static inline struct vf610_nfc *chip_to_nfc(struct nand_chip *chip) > +{ > + return container_of(chip, struct vf610_nfc, chip); > +} > + > static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg) > { > return readl(nfc->regs + reg); > @@ -214,6 +240,86 @@ static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src, > memcpy(dst, src, n); > } > > +static inline bool vf610_nfc_is_little_endian(void) > +{ > +#ifdef __LITTLE_ENDIAN > + return true; > +#else > + return false; > +#endif > +} > + > +/** > + * Read accessor for internal SRAM buffer > + * @dst: destination address in regular memory > + * @src: source address in SRAM buffer > + * @len: bytes to copy > + * @fix_endian: Fix endianness if required > + * > + * Use this accessor for the internal SRAM buffers. On the ARM > + * Freescale Vybrid SoC it's known that the driver can treat > + * the SRAM buffer as if it's memory. Other platform might need > + * to treat the buffers differently. > + * > + * The controller stores bytes from the NAND chip internally in big > + * endianness. On little endian platforms such as Vybrid this leads > + * to reversed byte order. > + * For performance reason (and earlier probably due to unanawareness) > + * the driver avoids correcting endianness where it has control over > + * write and read side (e.g. page wise data access). > + * In case endianness matters len should be a multiple of 4. > + */ > +static inline void vf610_nfc_rd_from_sram(void *dst, const void __iomem *src, > + size_t len, bool fix_endian) > +{ > + if (vf610_nfc_is_little_endian() && fix_endian) { > + unsigned int i; > + > + for (i = 0; i < len; i += 4) { > + u32 val = be32_to_cpu(__raw_readl(src + i)); > + memcpy(dst + i, &val, min(sizeof(val), len - i)); > + } > + } else { > + memcpy_fromio(dst, src, len); > + } > +} > + > +/** > + * Write accessor for internal SRAM buffer > + * @dst: destination address in SRAM buffer > + * @src: source address in regular memory > + * @len: bytes to copy > + * @fix_endian: Fix endianness if required > + * > + * Use this accessor for the internal SRAM buffers. On the ARM > + * Freescale Vybrid SoC it's known that the driver can treat > + * the SRAM buffer as if it's memory. Other platform might need > + * to treat the buffers differently. > + * > + * The controller stores bytes from the NAND chip internally in big > + * endianness. On little endian platforms such as Vybrid this leads > + * to reversed byte order. > + * For performance reason (and earlier probably due to unanawareness) > + * the driver avoids correcting endianness where it has control over > + * write and read side (e.g. page wise data access). > + * In case endianness matters len should be a multiple of 4. > + */ > +static inline void vf610_nfc_wr_to_sram(void __iomem *dst, const void *src, > + size_t len, bool fix_endian) > +{ > + if (vf610_nfc_is_little_endian() && fix_endian) { > + unsigned int i; > + > + for (i = 0; i < len; i += 4) { > + u32 val; > + memcpy(&val, src + i, min(sizeof(val), len - i)); > + __raw_writel(cpu_to_be32(val), dst + i); > + } > + } else { > + memcpy_toio(dst, src, len); > + } > +} > + > /* Clear flags for upcoming command */ > static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc) > { > @@ -489,6 +595,170 @@ static int vf610_nfc_dev_ready(struct mtd_info *mtd) > return 1; > } > > +static inline void vf610_nfc_run(struct vf610_nfc *nfc, u32 col, u32 row, u32 cmd1, u32 cmd2, u32 trfr_sz) > +{ > + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK, > + COL_ADDR_SHIFT, col); > + > + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK, > + ROW_ADDR_SHIFT, row); > + > + vf610_nfc_write(nfc, NFC_SECTOR_SIZE, trfr_sz); > + vf610_nfc_write(nfc, NFC_FLASH_CMD1, cmd1); > + vf610_nfc_write(nfc, NFC_FLASH_CMD2, cmd2); > + > + dev_dbg(nfc->dev, "col 0x%08x, row 0x%08x, cmd1 0x%08x, cmd2 0x%08x, trfr_sz %d\n", Do you really need 8 digits? Commands are bytes, col is two bytes. "trfr_sz" is not very explicit for a debug message. > + col, row, cmd1, cmd2, trfr_sz); > + > + vf610_nfc_done(nfc); > +} > + > +static inline const struct nand_op_instr *vf610_get_next_instr( > + const struct nand_subop *subop, int *op_id) > +{ > + if (*op_id + 1 >= subop->ninstrs) > + return NULL; > + > + (*op_id)++; > + > + return &subop->instrs[*op_id]; > +} > + > +static int vf610_nfc_cmd(struct nand_chip *chip, > + const struct nand_subop *subop) > +{ > + const struct nand_op_instr *instr; > + struct vf610_nfc *nfc = chip_to_nfc(chip); > + int op_id = -1, trfr_sz = 0, offset; > + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; > + bool force8bit = false; > + > + /* > + * Some ops are optional, but the hardware requires the operations > + * to be in this exact order. > + * The op parser enforces the order and makes sure that there isn't > + * a read and write element in a single operation. > + */ > + instr = vf610_get_next_instr(subop, &op_id); > + if (!instr) > + return -EINVAL; > + > + if (instr && instr->type == NAND_OP_CMD_INSTR) { > + dev_dbg(nfc->dev, "OP_CMD: code 0x%02x\n", instr->ctx.cmd.opcode); > + cmd2 |= instr->ctx.cmd.opcode << CMD_BYTE1_SHIFT; > + code |= COMMAND_CMD_BYTE1; > + > + instr = vf610_get_next_instr(subop, &op_id); > + } I still don't like the syntax, but if Boris is okay, then let's forget about it. > + > + if (instr && instr->type == NAND_OP_ADDR_INSTR) { > + int naddrs = nand_subop_get_num_addr_cyc(subop, op_id); > + int i = nand_subop_get_addr_start_off(subop, op_id); > + > + for (; i < naddrs; i++) { > + u8 val = instr->ctx.addr.addrs[i]; > + if (i < 2) > + col |= COL_ADDR(i, val); > + else > + row |= ROW_ADDR(i - 2, val); > + } > + code |= COMMAND_NADDR_BYTES(naddrs); > + > + dev_dbg(nfc->dev, "OP_ADDR: col %d, row %d\n", col, row); > + > + instr = vf610_get_next_instr(subop, &op_id); > + } > + > + if (instr && instr->type == NAND_OP_DATA_OUT_INSTR) { > + trfr_sz = nand_subop_get_data_len(subop, op_id); > + offset = nand_subop_get_data_start_off(subop, op_id); > + force8bit = instr->ctx.data.force_8bit; > + > + dev_dbg(nfc->dev, "OP_DATA_OUT: len %d, offset %d\n", > + trfr_sz, offset); > + > + /* We don't care about endianness when writing a NAND page */ > + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0) + offset, > + instr->ctx.data.buf.in + offset, > + trfr_sz, !nfc->page_access); > + code |= COMMAND_WRITE_DATA; > + > + instr = vf610_get_next_instr(subop, &op_id); > + } > + > + if (instr && instr->type == NAND_OP_CMD_INSTR) { > + cmd1 |= instr->ctx.cmd.opcode << CMD_BYTE2_SHIFT; > + code |= COMMAND_CMD_BYTE2; > + > + instr = vf610_get_next_instr(subop, &op_id); > + } > + > + if (instr && instr->type == NAND_OP_WAITRDY_INSTR) { > + code |= COMMAND_RB_HANDSHAKE; > + > + instr = vf610_get_next_instr(subop, &op_id); > + } > + > + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { > + trfr_sz = nand_subop_get_data_len(subop, op_id); > + offset = nand_subop_get_data_start_off(subop, op_id); > + force8bit = instr->ctx.data.force_8bit; > + > + dev_dbg(nfc->dev, "OP_DATA_IN: len %d, offset %d\n", > + trfr_sz, offset); > + > + code |= COMMAND_READ_DATA; > + } > + > + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) > + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); > + > + cmd2 |= code << CMD_CODE_SHIFT; > + > + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); > + > + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { > + /* We don't care about endianness when reading a NAND page */ > + vf610_nfc_rd_from_sram(instr->ctx.data.buf.in + offset, > + nfc->regs + NFC_MAIN_AREA(0) + offset, > + trfr_sz, !nfc->page_access); > + } > + > + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) > + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); > + > + return 0; > +} > + > +static const struct nand_op_parser vf610_nfc_op_parser = NAND_OP_PARSER( > + NAND_OP_PARSER_PATTERN( > + vf610_nfc_cmd, > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), > + NAND_OP_PARSER_PAT_DATA_OUT_ELEM(true, PAGE_2K + OOB_MAX), > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true)), > + NAND_OP_PARSER_PATTERN( > + vf610_nfc_cmd, > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true), > + NAND_OP_PARSER_PAT_DATA_IN_ELEM(true, PAGE_2K + OOB_MAX)), > + ); > + > +static int vf610_nfc_exec_op(struct nand_chip *chip, > + const struct nand_operation *op, > + bool check_only) > +{ > + struct vf610_nfc *nfc = chip_to_nfc(chip); > + > + dev_dbg(nfc->dev, "exec_op, opcode 0x%02x\n", op->instrs[0].ctx.cmd.opcode); > + > + return nand_op_parser_exec_op(chip, &vf610_nfc_op_parser, op, check_only); > +} > + > + Extra space. > /* > * This function supports Vybrid only (MPC5125 would have full RB and four CS) > */ > @@ -526,9 +796,14 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, > if (!(ecc_status & ECC_STATUS_MASK)) > return ecc_count; > > - /* Read OOB without ECC unit enabled */ > - vf610_nfc_command(mtd, NAND_CMD_READOOB, 0, page); > - vf610_nfc_read_buf(mtd, oob, mtd->oobsize); > + /* > + * Read OOB without ECC unit enabled. We temporarily set ->page_access > + * to true to make sure vf610_nfc_cmd() does not swap bytes when > + * reading data from the internal SRAM. > + */ > + nfc->page_access = true; > + nand_read_oob_op(&nfc->chip, page, 0, oob, mtd->oobsize); > + nfc->page_access = false; > > /* > * On an erased page, bit count (including OOB) should be zero or > @@ -542,12 +817,46 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, > static int vf610_nfc_read_page(struct mtd_info *mtd, struct nand_chip *chip, > uint8_t *buf, int oob_required, int page) > { > - int eccsize = chip->ecc.size; > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int trfr_sz = mtd->writesize + mtd->oobsize; > + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; > int stat; > > - nand_read_page_op(chip, page, 0, buf, eccsize); > + cmd2 |= NAND_CMD_READ0 << CMD_BYTE1_SHIFT; > + code |= COMMAND_CMD_BYTE1; > + > + code |= COMMAND_CAR_BYTE1; > + code |= COMMAND_CAR_BYTE2; > + > + row = ROW_ADDR(0, page & 0xff); > + code |= COMMAND_RAR_BYTE1; > + row |= ROW_ADDR(1, page >> 8); > + code |= COMMAND_RAR_BYTE2; > + > + if (chip->options & NAND_ROW_ADDR_3) { > + row |= ROW_ADDR(2, page >> 16); > + code |= COMMAND_RAR_BYTE3; > + } > + > + cmd1 |= NAND_CMD_READSTART << CMD_BYTE2_SHIFT; > + code |= COMMAND_CMD_BYTE2; > + > + code |= COMMAND_RB_HANDSHAKE; > + code |= COMMAND_READ_DATA; > + cmd2 |= code << CMD_CODE_SHIFT; > + > + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); > + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); > + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); > + > + /* We don't care about endianness when reading a NAND page */ Well, if I understand correctly, this comment is not very accurate. It is more like we don't care because this is how it was handled in the passed and we cannot change it. Also, endianness is picked by the user and it will work as long as read and write operations use the same convention. > + vf610_nfc_rd_from_sram(buf, nfc->regs + NFC_MAIN_AREA(0), > + mtd->writesize, false); > if (oob_required) > - vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); > + vf610_nfc_rd_from_sram(chip->oob_poi, > + nfc->regs + NFC_MAIN_AREA(0) + > + mtd->writesize, > + mtd->oobsize, false); > > stat = vf610_nfc_correct_data(mtd, buf, chip->oob_poi, page); > > @@ -564,16 +873,113 @@ static int vf610_nfc_write_page(struct mtd_info *mtd, struct nand_chip *chip, > const uint8_t *buf, int oob_required, int page) > { > struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int trfr_sz = mtd->writesize + mtd->oobsize; > + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; > + int ret = 0; > + > + cmd2 |= NAND_CMD_SEQIN << CMD_BYTE1_SHIFT; > + code |= COMMAND_CMD_BYTE1; > + > + code |= COMMAND_CAR_BYTE1; > + code |= COMMAND_CAR_BYTE2; > + > + row = ROW_ADDR(0, page & 0xff); > + code |= COMMAND_RAR_BYTE1; > + row |= ROW_ADDR(1, page >> 8); > + code |= COMMAND_RAR_BYTE2; > + if (chip->options & NAND_ROW_ADDR_3) { > + row |= ROW_ADDR(2, page >> 16); > + code |= COMMAND_RAR_BYTE3; > + } > > - nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize); > - if (oob_required) > - vf610_nfc_write_buf(mtd, chip->oob_poi, mtd->oobsize); > + cmd1 |= NAND_CMD_PAGEPROG << CMD_BYTE2_SHIFT; > + code |= COMMAND_CMD_BYTE2; > + > + code |= COMMAND_WRITE_DATA; > + > + /* We don't care about endianness when writing a NAND page */ Same. > + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0), buf, > + mtd->writesize, false); > > - /* Always write whole page including OOB due to HW ECC */ > - nfc->use_hw_ecc = true; > - nfc->write_sz = mtd->writesize + mtd->oobsize; > + code |= COMMAND_RB_HANDSHAKE; > + cmd2 |= code << CMD_CODE_SHIFT; > > - return nand_prog_page_end_op(chip); > + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); > + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); > + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); > + > + return ret; > +} > + > +static int vf610_nfc_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, > + uint8_t *buf, int oob_required, int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data > + * from the internal SRAM. > + */ > + nfc->page_access = true; > + ret = nand_read_page_raw(mtd, chip, buf, oob_required, page); > + nfc->page_access = false; > + > + return ret; > +} > + > +static int vf610_nfc_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, > + const uint8_t *buf, int oob_required, int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data > + * from the internal SRAM. > + */ > + nfc->page_access = true; > + ret = nand_write_page_raw(mtd, chip, buf, oob_required, page); > + nfc->page_access = false; > + > + return ret; > +} > + > +static int vf610_nfc_read_oob(struct mtd_info *mtd, struct nand_chip *chip, > + int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data > + * from the internal SRAM. > + */ > + nfc->page_access = true; > + ret = nand_read_oob_std(mtd, chip, page); > + nfc->page_access = false; > + > + return ret; > +} > +static int vf610_nfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, > + int page) > +{ > + struct vf610_nfc *nfc = mtd_to_nfc(mtd); > + int ret; > + > + /* > + * We temporarily set ->page_access to true to make sure > + * vf610_nfc_cmd() does not swap bytes when reading data > + * from the internal SRAM. > + */ > + nfc->page_access = true; It looks like most of the time you use ->page_access it is for something else than just "configuring page access for actual page access". Maybe you should rename this variable? Also, I don't know what Boris prefers but maybe we should move this comment to the variable declaration and here just mention what you do (prevent byte swapping) instead of how you do it. > + ret = nand_write_oob_std(mtd, chip, page); > + nfc->page_access = false; > + > + return ret; > } > > static const struct of_device_id vf610_nfc_dt_ids[] = { > @@ -590,6 +996,7 @@ static void vf610_nfc_preinit_controller(struct vf610_nfc *nfc) > vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT); > vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT); > vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT); > + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); > > /* Disable virtual pages, only one elementary transfer unit */ > vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK, > @@ -686,6 +1093,7 @@ static int vf610_nfc_probe(struct platform_device *pdev) > chip->read_word = vf610_nfc_read_word; > chip->read_buf = vf610_nfc_read_buf; > chip->write_buf = vf610_nfc_write_buf; > + chip->exec_op = vf610_nfc_exec_op; > chip->select_chip = vf610_nfc_select_chip; > chip->onfi_set_features = nand_onfi_get_set_features_notsupp; > chip->onfi_get_features = nand_onfi_get_set_features_notsupp; > @@ -755,7 +1163,10 @@ static int vf610_nfc_probe(struct platform_device *pdev) > > chip->ecc.read_page = vf610_nfc_read_page; > chip->ecc.write_page = vf610_nfc_write_page; > - > + chip->ecc.read_page_raw = vf610_nfc_read_page_raw; > + chip->ecc.write_page_raw = vf610_nfc_write_page_raw; > + chip->ecc.read_oob = vf610_nfc_read_oob; > + chip->ecc.write_oob = vf610_nfc_write_oob; > chip->ecc.size = PAGE_2K; > } > Best regards, Miquèl
On 22.02.2018 23:06, Boris Brezillon wrote: > On Thu, 22 Feb 2018 21:29:17 +0100 > Stefan Agner <stefan@agner.ch> wrote: > >> This reworks the driver to make use of ->exec_op() callback. The >> command sequencer of the VF610 NFC aligns well with the new ops >> interface. >> >> The ops are translated to a NFC command code while filling the >> necessary registers. Instead of using the special status and >> read id command codes (which require the status/id form special >> registers) the driver now uses the main data buffer for all >> commands. This simplifies the driver as no special casing is >> needed. >> >> For control data (status byte, id bytes and parameter page) the >> driver needs to reverse byte order for little endian CPUs since >> the controller seems to store the bytes in big endian order in >> the data buffer. >> >> The current state seems to pass MTD tests on a Colibri VF61. >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> --- >> drivers/mtd/nand/raw/vf610_nfc.c | 439 +++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 425 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c >> index 5d7a1f8f580f..9baf80766307 100644 >> --- a/drivers/mtd/nand/raw/vf610_nfc.c >> +++ b/drivers/mtd/nand/raw/vf610_nfc.c >> @@ -74,6 +74,22 @@ >> #define RESET_CMD_CODE 0x4040 >> #define STATUS_READ_CMD_CODE 0x4068 >> >> +/* NFC_CMD2[CODE] controller cycle bit masks */ >> +#define COMMAND_CMD_BYTE1 BIT(14) >> +#define COMMAND_CAR_BYTE1 BIT(13) >> +#define COMMAND_CAR_BYTE2 BIT(12) >> +#define COMMAND_RAR_BYTE1 BIT(11) >> +#define COMMAND_RAR_BYTE2 BIT(10) >> +#define COMMAND_RAR_BYTE3 BIT(9) >> +#define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) - 1) >> +#define COMMAND_WRITE_DATA BIT(8) >> +#define COMMAND_CMD_BYTE2 BIT(7) >> +#define COMMAND_RB_HANDSHAKE BIT(6) >> +#define COMMAND_READ_DATA BIT(5) >> +#define COMMAND_CMD_BYTE3 BIT(4) >> +#define COMMAND_READ_STATUS BIT(3) >> +#define COMMAND_READ_ID BIT(2) >> + >> /* NFC ECC mode define */ >> #define ECC_BYPASS 0 >> #define ECC_45_BYTE 6 >> @@ -97,10 +113,14 @@ >> /* NFC_COL_ADDR Field */ >> #define COL_ADDR_MASK 0x0000FFFF >> #define COL_ADDR_SHIFT 0 >> +#define COL_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) >> + >> >> /* NFC_ROW_ADDR Field */ >> #define ROW_ADDR_MASK 0x00FFFFFF >> #define ROW_ADDR_SHIFT 0 >> +#define ROW_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) >> + >> #define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000 >> #define ROW_ADDR_CHIP_SEL_RB_SHIFT 28 >> #define ROW_ADDR_CHIP_SEL_MASK 0x0F000000 >> @@ -165,6 +185,7 @@ struct vf610_nfc { >> enum vf610_nfc_variant variant; >> struct clk *clk; >> bool use_hw_ecc; >> + bool page_access; > > This field deserves a comment IMO, even if you describe in details what > it does in the rd/wr_from/to_sram() funcs. > >> u32 ecc_mode; >> }; >> >> @@ -173,6 +194,11 @@ static inline struct vf610_nfc *mtd_to_nfc(struct mtd_info *mtd) >> return container_of(mtd_to_nand(mtd), struct vf610_nfc, chip); >> } >> >> +static inline struct vf610_nfc *chip_to_nfc(struct nand_chip *chip) >> +{ >> + return container_of(chip, struct vf610_nfc, chip); >> +} >> + >> static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg) >> { >> return readl(nfc->regs + reg); >> @@ -214,6 +240,86 @@ static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src, >> memcpy(dst, src, n); >> } >> >> +static inline bool vf610_nfc_is_little_endian(void) > > It's not really the NFC that is LE, is it? I mean, you could have the > kernel configured in BIG ENDIAN for this platform, and then you wouldn't > have to do the byte-swapping. > >> +{ >> +#ifdef __LITTLE_ENDIAN >> + return true; >> +#else >> + return false; >> +#endif >> +} >> + >> +/** >> + * Read accessor for internal SRAM buffer >> + * @dst: destination address in regular memory >> + * @src: source address in SRAM buffer >> + * @len: bytes to copy >> + * @fix_endian: Fix endianness if required >> + * >> + * Use this accessor for the internal SRAM buffers. On the ARM >> + * Freescale Vybrid SoC it's known that the driver can treat >> + * the SRAM buffer as if it's memory. Other platform might need >> + * to treat the buffers differently. >> + * >> + * The controller stores bytes from the NAND chip internally in big >> + * endianness. On little endian platforms such as Vybrid this leads >> + * to reversed byte order. >> + * For performance reason (and earlier probably due to unanawareness) > > ^ unawareness > >> + * the driver avoids correcting endianness where it has control over >> + * write and read side (e.g. page wise data access). >> + * In case endianness matters len should be a multiple of 4. >> + */ >> +static inline void vf610_nfc_rd_from_sram(void *dst, const void __iomem *src, >> + size_t len, bool fix_endian) >> +{ >> + if (vf610_nfc_is_little_endian() && fix_endian) { >> + unsigned int i; >> + >> + for (i = 0; i < len; i += 4) { >> + u32 val = be32_to_cpu(__raw_readl(src + i)); >> + memcpy(dst + i, &val, min(sizeof(val), len - i)); >> + } >> + } else { >> + memcpy_fromio(dst, src, len); >> + } >> +} >> + >> +/** >> + * Write accessor for internal SRAM buffer >> + * @dst: destination address in SRAM buffer >> + * @src: source address in regular memory >> + * @len: bytes to copy >> + * @fix_endian: Fix endianness if required >> + * >> + * Use this accessor for the internal SRAM buffers. On the ARM >> + * Freescale Vybrid SoC it's known that the driver can treat >> + * the SRAM buffer as if it's memory. Other platform might need >> + * to treat the buffers differently. >> + * >> + * The controller stores bytes from the NAND chip internally in big >> + * endianness. On little endian platforms such as Vybrid this leads >> + * to reversed byte order. >> + * For performance reason (and earlier probably due to unanawareness) > > ^ unawareness > >> + * the driver avoids correcting endianness where it has control over >> + * write and read side (e.g. page wise data access). >> + * In case endianness matters len should be a multiple of 4. > > That's the opposite actually. If fix_endian is set, we don't have any > requirement on len alignment, because the cpu_to_be32(val) will save > us. But we have a problem when fix_endian is false an len is not > aligned on 4, because, AFAIR memcpy_toio() does 32-bit accesses when > things are properly aligned on 32 bits, and when that's not the case it > falls back to 16 or 8 bit accesses, which in our case may lead to data > loss on LE platforms. > >> + */ >> +static inline void vf610_nfc_wr_to_sram(void __iomem *dst, const void *src, >> + size_t len, bool fix_endian) >> +{ >> + if (vf610_nfc_is_little_endian() && fix_endian) { >> + unsigned int i; >> + >> + for (i = 0; i < len; i += 4) { >> + u32 val; >> + memcpy(&val, src + i, min(sizeof(val), len - i)); >> + __raw_writel(cpu_to_be32(val), dst + i); >> + } >> + } else { >> + memcpy_toio(dst, src, len); >> + } >> +} >> + >> /* Clear flags for upcoming command */ >> static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc) >> { >> @@ -489,6 +595,170 @@ static int vf610_nfc_dev_ready(struct mtd_info *mtd) >> return 1; >> } >> >> +static inline void vf610_nfc_run(struct vf610_nfc *nfc, u32 col, u32 row, u32 cmd1, u32 cmd2, u32 trfr_sz) >> +{ >> + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK, >> + COL_ADDR_SHIFT, col); >> + >> + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK, >> + ROW_ADDR_SHIFT, row); >> + >> + vf610_nfc_write(nfc, NFC_SECTOR_SIZE, trfr_sz); >> + vf610_nfc_write(nfc, NFC_FLASH_CMD1, cmd1); >> + vf610_nfc_write(nfc, NFC_FLASH_CMD2, cmd2); >> + >> + dev_dbg(nfc->dev, "col 0x%08x, row 0x%08x, cmd1 0x%08x, cmd2 0x%08x, trfr_sz %d\n", >> + col, row, cmd1, cmd2, trfr_sz); >> + >> + vf610_nfc_done(nfc); >> +} >> + >> +static inline const struct nand_op_instr *vf610_get_next_instr( >> + const struct nand_subop *subop, int *op_id) >> +{ >> + if (*op_id + 1 >= subop->ninstrs) >> + return NULL; >> + >> + (*op_id)++; >> + >> + return &subop->instrs[*op_id]; >> +} >> + >> +static int vf610_nfc_cmd(struct nand_chip *chip, >> + const struct nand_subop *subop) >> +{ >> + const struct nand_op_instr *instr; >> + struct vf610_nfc *nfc = chip_to_nfc(chip); >> + int op_id = -1, trfr_sz = 0, offset; >> + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; >> + bool force8bit = false; >> + >> + /* >> + * Some ops are optional, but the hardware requires the operations >> + * to be in this exact order. >> + * The op parser enforces the order and makes sure that there isn't >> + * a read and write element in a single operation. >> + */ >> + instr = vf610_get_next_instr(subop, &op_id); >> + if (!instr) >> + return -EINVAL; >> + >> + if (instr && instr->type == NAND_OP_CMD_INSTR) { >> + dev_dbg(nfc->dev, "OP_CMD: code 0x%02x\n", instr->ctx.cmd.opcode); >> + cmd2 |= instr->ctx.cmd.opcode << CMD_BYTE1_SHIFT; >> + code |= COMMAND_CMD_BYTE1; >> + >> + instr = vf610_get_next_instr(subop, &op_id); >> + } >> + >> + if (instr && instr->type == NAND_OP_ADDR_INSTR) { >> + int naddrs = nand_subop_get_num_addr_cyc(subop, op_id); >> + int i = nand_subop_get_addr_start_off(subop, op_id); >> + >> + for (; i < naddrs; i++) { >> + u8 val = instr->ctx.addr.addrs[i]; >> + if (i < 2) >> + col |= COL_ADDR(i, val); >> + else >> + row |= ROW_ADDR(i - 2, val); >> + } >> + code |= COMMAND_NADDR_BYTES(naddrs); >> + >> + dev_dbg(nfc->dev, "OP_ADDR: col %d, row %d\n", col, row); >> + >> + instr = vf610_get_next_instr(subop, &op_id); >> + } >> + >> + if (instr && instr->type == NAND_OP_DATA_OUT_INSTR) { >> + trfr_sz = nand_subop_get_data_len(subop, op_id); >> + offset = nand_subop_get_data_start_off(subop, op_id); >> + force8bit = instr->ctx.data.force_8bit; >> + >> + dev_dbg(nfc->dev, "OP_DATA_OUT: len %d, offset %d\n", >> + trfr_sz, offset); >> + >> + /* We don't care about endianness when writing a NAND page */ >> + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0) + offset, >> + instr->ctx.data.buf.in + offset, >> + trfr_sz, !nfc->page_access); >> + code |= COMMAND_WRITE_DATA; >> + >> + instr = vf610_get_next_instr(subop, &op_id); >> + } >> + >> + if (instr && instr->type == NAND_OP_CMD_INSTR) { >> + cmd1 |= instr->ctx.cmd.opcode << CMD_BYTE2_SHIFT; >> + code |= COMMAND_CMD_BYTE2; >> + >> + instr = vf610_get_next_instr(subop, &op_id); > > You seem to have debug traces almost everywhere except here and... > >> + } >> + >> + if (instr && instr->type == NAND_OP_WAITRDY_INSTR) { >> + code |= COMMAND_RB_HANDSHAKE; >> + >> + instr = vf610_get_next_instr(subop, &op_id); > > ... here. Can we please make that consistent. Either drop all of them > or add the missing ones. > > IMHO it's more interesting to have a dev_dbg() giving the col, row, > cmd1, cmd2 and trfr_sz values in vf610_nfc_run() rather than those you > add here, because they kind of duplicate the information given in > nand_op_parser_trace(). > >> + } >> + >> + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { >> + trfr_sz = nand_subop_get_data_len(subop, op_id); >> + offset = nand_subop_get_data_start_off(subop, op_id); >> + force8bit = instr->ctx.data.force_8bit; >> + >> + dev_dbg(nfc->dev, "OP_DATA_IN: len %d, offset %d\n", >> + trfr_sz, offset); >> + >> + code |= COMMAND_READ_DATA; >> + } > > Still not a big fan of the > > if (instr && instr->type == NAND_OP_XXX_INSTR) { > ... > instr = vf610_get_next_instr(subop, &op_id); > } > > approach, but I'm ready to make a concession on that :-). > >> + >> + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) >> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); >> + >> + cmd2 |= code << CMD_CODE_SHIFT; >> + >> + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); >> + >> + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { >> + /* We don't care about endianness when reading a NAND page */ >> + vf610_nfc_rd_from_sram(instr->ctx.data.buf.in + offset, >> + nfc->regs + NFC_MAIN_AREA(0) + offset, >> + trfr_sz, !nfc->page_access); >> + } >> + >> + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) >> + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); >> + >> + return 0; >> +} >> + >> +static const struct nand_op_parser vf610_nfc_op_parser = NAND_OP_PARSER( >> + NAND_OP_PARSER_PATTERN( >> + vf610_nfc_cmd, >> + NAND_OP_PARSER_PAT_CMD_ELEM(true), >> + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), >> + NAND_OP_PARSER_PAT_DATA_OUT_ELEM(true, PAGE_2K + OOB_MAX), >> + NAND_OP_PARSER_PAT_CMD_ELEM(true), >> + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true)), >> + NAND_OP_PARSER_PATTERN( >> + vf610_nfc_cmd, >> + NAND_OP_PARSER_PAT_CMD_ELEM(true), >> + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), >> + NAND_OP_PARSER_PAT_CMD_ELEM(true), >> + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true), >> + NAND_OP_PARSER_PAT_DATA_IN_ELEM(true, PAGE_2K + OOB_MAX)), >> + ); >> + >> +static int vf610_nfc_exec_op(struct nand_chip *chip, >> + const struct nand_operation *op, >> + bool check_only) >> +{ >> + struct vf610_nfc *nfc = chip_to_nfc(chip); >> + >> + dev_dbg(nfc->dev, "exec_op, opcode 0x%02x\n", op->instrs[0].ctx.cmd.opcode); > > Bad idea: according to the pattern list you described above the first > instruction is not necessarily a CMD instruction. I think you should > drop this dev_dbg(). > >> + >> + return nand_op_parser_exec_op(chip, &vf610_nfc_op_parser, op, check_only); >> +} >> + >> + >> /* >> * This function supports Vybrid only (MPC5125 would have full RB and four CS) >> */ >> @@ -526,9 +796,14 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, >> if (!(ecc_status & ECC_STATUS_MASK)) >> return ecc_count; >> >> - /* Read OOB without ECC unit enabled */ >> - vf610_nfc_command(mtd, NAND_CMD_READOOB, 0, page); >> - vf610_nfc_read_buf(mtd, oob, mtd->oobsize); >> + /* >> + * Read OOB without ECC unit enabled. We temporarily set ->page_access >> + * to true to make sure vf610_nfc_cmd() does not swap bytes when >> + * reading data from the internal SRAM. >> + */ >> + nfc->page_access = true; >> + nand_read_oob_op(&nfc->chip, page, 0, oob, mtd->oobsize); >> + nfc->page_access = false; >> >> /* >> * On an erased page, bit count (including OOB) should be zero or >> @@ -542,12 +817,46 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, >> static int vf610_nfc_read_page(struct mtd_info *mtd, struct nand_chip *chip, >> uint8_t *buf, int oob_required, int page) >> { >> - int eccsize = chip->ecc.size; >> + struct vf610_nfc *nfc = mtd_to_nfc(mtd); >> + int trfr_sz = mtd->writesize + mtd->oobsize; >> + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; >> int stat; >> >> - nand_read_page_op(chip, page, 0, buf, eccsize); >> + cmd2 |= NAND_CMD_READ0 << CMD_BYTE1_SHIFT; >> + code |= COMMAND_CMD_BYTE1; >> + >> + code |= COMMAND_CAR_BYTE1; >> + code |= COMMAND_CAR_BYTE2; >> + >> + row = ROW_ADDR(0, page & 0xff); >> + code |= COMMAND_RAR_BYTE1; >> + row |= ROW_ADDR(1, page >> 8); >> + code |= COMMAND_RAR_BYTE2; >> + >> + if (chip->options & NAND_ROW_ADDR_3) { >> + row |= ROW_ADDR(2, page >> 16); >> + code |= COMMAND_RAR_BYTE3; >> + } >> + >> + cmd1 |= NAND_CMD_READSTART << CMD_BYTE2_SHIFT; >> + code |= COMMAND_CMD_BYTE2; >> + >> + code |= COMMAND_RB_HANDSHAKE; >> + code |= COMMAND_READ_DATA; >> + cmd2 |= code << CMD_CODE_SHIFT; >> + >> + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); >> + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); >> + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); >> + >> + /* We don't care about endianness when reading a NAND page */ >> + vf610_nfc_rd_from_sram(buf, nfc->regs + NFC_MAIN_AREA(0), >> + mtd->writesize, false); >> if (oob_required) >> - vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); >> + vf610_nfc_rd_from_sram(chip->oob_poi, >> + nfc->regs + NFC_MAIN_AREA(0) + >> + mtd->writesize, >> + mtd->oobsize, false); >> >> stat = vf610_nfc_correct_data(mtd, buf, chip->oob_poi, page); >> >> @@ -564,16 +873,113 @@ static int vf610_nfc_write_page(struct mtd_info *mtd, struct nand_chip *chip, >> const uint8_t *buf, int oob_required, int page) >> { >> struct vf610_nfc *nfc = mtd_to_nfc(mtd); >> + int trfr_sz = mtd->writesize + mtd->oobsize; >> + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; >> + int ret = 0; >> + >> + cmd2 |= NAND_CMD_SEQIN << CMD_BYTE1_SHIFT; >> + code |= COMMAND_CMD_BYTE1; >> + >> + code |= COMMAND_CAR_BYTE1; >> + code |= COMMAND_CAR_BYTE2; >> + >> + row = ROW_ADDR(0, page & 0xff); >> + code |= COMMAND_RAR_BYTE1; >> + row |= ROW_ADDR(1, page >> 8); >> + code |= COMMAND_RAR_BYTE2; >> + if (chip->options & NAND_ROW_ADDR_3) { >> + row |= ROW_ADDR(2, page >> 16); >> + code |= COMMAND_RAR_BYTE3; >> + } >> >> - nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize); >> - if (oob_required) >> - vf610_nfc_write_buf(mtd, chip->oob_poi, mtd->oobsize); >> + cmd1 |= NAND_CMD_PAGEPROG << CMD_BYTE2_SHIFT; >> + code |= COMMAND_CMD_BYTE2; >> + >> + code |= COMMAND_WRITE_DATA; >> + >> + /* We don't care about endianness when writing a NAND page */ >> + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0), buf, >> + mtd->writesize, false); >> >> - /* Always write whole page including OOB due to HW ECC */ >> - nfc->use_hw_ecc = true; >> - nfc->write_sz = mtd->writesize + mtd->oobsize; >> + code |= COMMAND_RB_HANDSHAKE; >> + cmd2 |= code << CMD_CODE_SHIFT; >> >> - return nand_prog_page_end_op(chip); >> + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); >> + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); >> + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); >> + >> + return ret; >> +} >> + >> +static int vf610_nfc_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, >> + uint8_t *buf, int oob_required, int page) >> +{ >> + struct vf610_nfc *nfc = mtd_to_nfc(mtd); >> + int ret; >> + >> + /* >> + * We temporarily set ->page_access to true to make sure >> + * vf610_nfc_cmd() does not swap bytes when reading data >> + * from the internal SRAM. >> + */ >> + nfc->page_access = true; >> + ret = nand_read_page_raw(mtd, chip, buf, oob_required, page); >> + nfc->page_access = false; >> + >> + return ret; >> +} >> + >> +static int vf610_nfc_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, >> + const uint8_t *buf, int oob_required, int page) >> +{ >> + struct vf610_nfc *nfc = mtd_to_nfc(mtd); >> + int ret; >> + >> + /* >> + * We temporarily set ->page_access to true to make sure >> + * vf610_nfc_cmd() does not swap bytes when reading data > > ^ writing data to > >> + * from the internal SRAM. >> + */ >> + nfc->page_access = true; >> + ret = nand_write_page_raw(mtd, chip, buf, oob_required, page); >> + nfc->page_access = false; >> + >> + return ret; > > As you discovered while debugging the new implementation, this doesn't > work, because then the STATUS byte is read without the proper > byte-swapping enabled. So the solution would be to replace the above > code by: > > nfc->page_access = true; > ret = nand_prog_page_begin_op(chip, page, 0, buf, > mtd->writesize); > if (!ret && oob_required) > ret = nand_write_data_op(chip, chip->oob_poi, > mtd->oobsize, false); > nfc->page_access = false; > > if (ret) > return ret; > > return nand_prog_page_end_op(chip); > >> +} >> + >> +static int vf610_nfc_read_oob(struct mtd_info *mtd, struct nand_chip *chip, >> + int page) >> +{ >> + struct vf610_nfc *nfc = mtd_to_nfc(mtd); >> + int ret; >> + >> + /* >> + * We temporarily set ->page_access to true to make sure >> + * vf610_nfc_cmd() does not swap bytes when reading data >> + * from the internal SRAM. >> + */ >> + nfc->page_access = true; >> + ret = nand_read_oob_std(mtd, chip, page); >> + nfc->page_access = false; >> + >> + return ret; >> +} >> +static int vf610_nfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, >> + int page) >> +{ >> + struct vf610_nfc *nfc = mtd_to_nfc(mtd); >> + int ret; >> + >> + /* >> + * We temporarily set ->page_access to true to make sure >> + * vf610_nfc_cmd() does not swap bytes when reading data >> + * from the internal SRAM. >> + */ >> + nfc->page_access = true; >> + ret = nand_write_oob_std(mtd, chip, page); >> + nfc->page_access = false; >> + >> + return ret; > > Same problem here. The above logic should be replaced by: > > nfc->page_access = true; > ret = nand_prog_page_begin_op(chip, page, mtd->writesize, > chip->oob_poi, mtd->oobsize); > > nfc->page_access = false; > > if (ret) > return ret; > > return nand_prog_page_end_op(chip); > Read/write seems to work, but there seems to be an issue with data: root@colibri-vf:~# insmod mtd_oobtest.ko dev=3 [ 69.609120] [ 69.610664] ================================================= [ 69.616734] mtd_oobtest: MTD device: 3 [ 69.631659] mtd_oobtest: MTD device size 16777216, eraseblock size 131072, page size 2048, count of eraseblocks 128, pages per eraseblock 64, OOB size 64 [ 69.647814] mtd_test: scanning for bad eraseblocks [ 69.654166] mtd_test: scanned 128 eraseblocks, 0 are bad [ 69.659507] mtd_oobtest: test 1 of 5 [ 69.736812] mtd_oobtest: writing OOBs of whole device [ 69.755753] mtd_oobtest: written up to eraseblock 0 [ 71.500646] mtd_oobtest: written 128 eraseblocks [ 71.505397] mtd_oobtest: verifying all eraseblocks [ 71.510336] mtd_oobtest: error @addr[0x0:0x0] 0xff -> 0xe diff 0xf1 [ 71.516727] mtd_oobtest: error @addr[0x0:0x1] 0xff -> 0x10 diff 0xef [ 71.523164] mtd_oobtest: error: verify failed at 0x0 [ 71.530372] mtd_oobtest: error @addr[0x800:0x0] 0xff -> 0x82 diff 0x7d [ 71.537083] mtd_oobtest: error @addr[0x800:0x1] 0xff -> 0x10 diff 0xef [ 71.543709] mtd_oobtest: error: verify failed at 0x800 Need to check what is exactly going wrong. -- Stefan >> } >> >> static const struct of_device_id vf610_nfc_dt_ids[] = { >> @@ -590,6 +996,7 @@ static void vf610_nfc_preinit_controller(struct vf610_nfc *nfc) >> vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT); >> vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT); >> vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT); >> + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); >> >> /* Disable virtual pages, only one elementary transfer unit */ >> vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK, >> @@ -686,6 +1093,7 @@ static int vf610_nfc_probe(struct platform_device *pdev) >> chip->read_word = vf610_nfc_read_word; >> chip->read_buf = vf610_nfc_read_buf; >> chip->write_buf = vf610_nfc_write_buf; >> + chip->exec_op = vf610_nfc_exec_op; >> chip->select_chip = vf610_nfc_select_chip; >> chip->onfi_set_features = nand_onfi_get_set_features_notsupp; >> chip->onfi_get_features = nand_onfi_get_set_features_notsupp; >> @@ -755,7 +1163,10 @@ static int vf610_nfc_probe(struct platform_device *pdev) >> >> chip->ecc.read_page = vf610_nfc_read_page; >> chip->ecc.write_page = vf610_nfc_write_page; >> - >> + chip->ecc.read_page_raw = vf610_nfc_read_page_raw; >> + chip->ecc.write_page_raw = vf610_nfc_write_page_raw; >> + chip->ecc.read_oob = vf610_nfc_read_oob; >> + chip->ecc.write_oob = vf610_nfc_write_oob; >> chip->ecc.size = PAGE_2K; >> } >>
On 26.02.2018 08:48, Stefan Agner wrote: > On 22.02.2018 23:06, Boris Brezillon wrote: >> On Thu, 22 Feb 2018 21:29:17 +0100 >> Stefan Agner <stefan@agner.ch> wrote: >> >>> This reworks the driver to make use of ->exec_op() callback. The >>> command sequencer of the VF610 NFC aligns well with the new ops >>> interface. >>> >>> The ops are translated to a NFC command code while filling the >>> necessary registers. Instead of using the special status and >>> read id command codes (which require the status/id form special >>> registers) the driver now uses the main data buffer for all >>> commands. This simplifies the driver as no special casing is >>> needed. >>> >>> For control data (status byte, id bytes and parameter page) the >>> driver needs to reverse byte order for little endian CPUs since >>> the controller seems to store the bytes in big endian order in >>> the data buffer. >>> >>> The current state seems to pass MTD tests on a Colibri VF61. >>> >>> Signed-off-by: Stefan Agner <stefan@agner.ch> >>> --- >>> drivers/mtd/nand/raw/vf610_nfc.c | 439 +++++++++++++++++++++++++++++++++++++-- >>> 1 file changed, 425 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c >>> index 5d7a1f8f580f..9baf80766307 100644 >>> --- a/drivers/mtd/nand/raw/vf610_nfc.c >>> +++ b/drivers/mtd/nand/raw/vf610_nfc.c >>> @@ -74,6 +74,22 @@ >>> #define RESET_CMD_CODE 0x4040 >>> #define STATUS_READ_CMD_CODE 0x4068 >>> >>> +/* NFC_CMD2[CODE] controller cycle bit masks */ >>> +#define COMMAND_CMD_BYTE1 BIT(14) >>> +#define COMMAND_CAR_BYTE1 BIT(13) >>> +#define COMMAND_CAR_BYTE2 BIT(12) >>> +#define COMMAND_RAR_BYTE1 BIT(11) >>> +#define COMMAND_RAR_BYTE2 BIT(10) >>> +#define COMMAND_RAR_BYTE3 BIT(9) >>> +#define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) - 1) >>> +#define COMMAND_WRITE_DATA BIT(8) >>> +#define COMMAND_CMD_BYTE2 BIT(7) >>> +#define COMMAND_RB_HANDSHAKE BIT(6) >>> +#define COMMAND_READ_DATA BIT(5) >>> +#define COMMAND_CMD_BYTE3 BIT(4) >>> +#define COMMAND_READ_STATUS BIT(3) >>> +#define COMMAND_READ_ID BIT(2) >>> + >>> /* NFC ECC mode define */ >>> #define ECC_BYPASS 0 >>> #define ECC_45_BYTE 6 >>> @@ -97,10 +113,14 @@ >>> /* NFC_COL_ADDR Field */ >>> #define COL_ADDR_MASK 0x0000FFFF >>> #define COL_ADDR_SHIFT 0 >>> +#define COL_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) >>> + >>> >>> /* NFC_ROW_ADDR Field */ >>> #define ROW_ADDR_MASK 0x00FFFFFF >>> #define ROW_ADDR_SHIFT 0 >>> +#define ROW_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) >>> + >>> #define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000 >>> #define ROW_ADDR_CHIP_SEL_RB_SHIFT 28 >>> #define ROW_ADDR_CHIP_SEL_MASK 0x0F000000 >>> @@ -165,6 +185,7 @@ struct vf610_nfc { >>> enum vf610_nfc_variant variant; >>> struct clk *clk; >>> bool use_hw_ecc; >>> + bool page_access; >> >> This field deserves a comment IMO, even if you describe in details what >> it does in the rd/wr_from/to_sram() funcs. >> >>> u32 ecc_mode; >>> }; >>> >>> @@ -173,6 +194,11 @@ static inline struct vf610_nfc *mtd_to_nfc(struct mtd_info *mtd) >>> return container_of(mtd_to_nand(mtd), struct vf610_nfc, chip); >>> } >>> >>> +static inline struct vf610_nfc *chip_to_nfc(struct nand_chip *chip) >>> +{ >>> + return container_of(chip, struct vf610_nfc, chip); >>> +} >>> + >>> static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg) >>> { >>> return readl(nfc->regs + reg); >>> @@ -214,6 +240,86 @@ static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src, >>> memcpy(dst, src, n); >>> } >>> >>> +static inline bool vf610_nfc_is_little_endian(void) >> >> It's not really the NFC that is LE, is it? I mean, you could have the >> kernel configured in BIG ENDIAN for this platform, and then you wouldn't >> have to do the byte-swapping. >> >>> +{ >>> +#ifdef __LITTLE_ENDIAN >>> + return true; >>> +#else >>> + return false; >>> +#endif >>> +} >>> + >>> +/** >>> + * Read accessor for internal SRAM buffer >>> + * @dst: destination address in regular memory >>> + * @src: source address in SRAM buffer >>> + * @len: bytes to copy >>> + * @fix_endian: Fix endianness if required >>> + * >>> + * Use this accessor for the internal SRAM buffers. On the ARM >>> + * Freescale Vybrid SoC it's known that the driver can treat >>> + * the SRAM buffer as if it's memory. Other platform might need >>> + * to treat the buffers differently. >>> + * >>> + * The controller stores bytes from the NAND chip internally in big >>> + * endianness. On little endian platforms such as Vybrid this leads >>> + * to reversed byte order. >>> + * For performance reason (and earlier probably due to unanawareness) >> >> ^ unawareness >> >>> + * the driver avoids correcting endianness where it has control over >>> + * write and read side (e.g. page wise data access). >>> + * In case endianness matters len should be a multiple of 4. >>> + */ >>> +static inline void vf610_nfc_rd_from_sram(void *dst, const void __iomem *src, >>> + size_t len, bool fix_endian) >>> +{ >>> + if (vf610_nfc_is_little_endian() && fix_endian) { >>> + unsigned int i; >>> + >>> + for (i = 0; i < len; i += 4) { >>> + u32 val = be32_to_cpu(__raw_readl(src + i)); >>> + memcpy(dst + i, &val, min(sizeof(val), len - i)); >>> + } >>> + } else { >>> + memcpy_fromio(dst, src, len); >>> + } >>> +} >>> + >>> +/** >>> + * Write accessor for internal SRAM buffer >>> + * @dst: destination address in SRAM buffer >>> + * @src: source address in regular memory >>> + * @len: bytes to copy >>> + * @fix_endian: Fix endianness if required >>> + * >>> + * Use this accessor for the internal SRAM buffers. On the ARM >>> + * Freescale Vybrid SoC it's known that the driver can treat >>> + * the SRAM buffer as if it's memory. Other platform might need >>> + * to treat the buffers differently. >>> + * >>> + * The controller stores bytes from the NAND chip internally in big >>> + * endianness. On little endian platforms such as Vybrid this leads >>> + * to reversed byte order. >>> + * For performance reason (and earlier probably due to unanawareness) >> >> ^ unawareness >> >>> + * the driver avoids correcting endianness where it has control over >>> + * write and read side (e.g. page wise data access). >>> + * In case endianness matters len should be a multiple of 4. >> >> That's the opposite actually. If fix_endian is set, we don't have any >> requirement on len alignment, because the cpu_to_be32(val) will save >> us. But we have a problem when fix_endian is false an len is not >> aligned on 4, because, AFAIR memcpy_toio() does 32-bit accesses when >> things are properly aligned on 32 bits, and when that's not the case it >> falls back to 16 or 8 bit accesses, which in our case may lead to data >> loss on LE platforms. >> I guess that also applies to read? Wouldn't that mean that my status command is broken? The stack usually does a 1 byte read there.... <snip> >> Same problem here. The above logic should be replaced by: >> >> nfc->page_access = true; >> ret = nand_prog_page_begin_op(chip, page, mtd->writesize, >> chip->oob_poi, mtd->oobsize); >> >> nfc->page_access = false; >> >> if (ret) >> return ret; >> >> return nand_prog_page_end_op(chip); >> > > Read/write seems to work, but there seems to be an issue with data: > > root@colibri-vf:~# insmod mtd_oobtest.ko dev=3 > [ 69.609120] > [ 69.610664] ================================================= > [ 69.616734] mtd_oobtest: MTD device: 3 > [ 69.631659] mtd_oobtest: MTD device size 16777216, eraseblock size > 131072, page size 2048, count of eraseblocks 128, pages per eraseblock > 64, OOB size 64 > [ 69.647814] mtd_test: scanning for bad eraseblocks > [ 69.654166] mtd_test: scanned 128 eraseblocks, 0 are bad > [ 69.659507] mtd_oobtest: test 1 of 5 > [ 69.736812] mtd_oobtest: writing OOBs of whole device > [ 69.755753] mtd_oobtest: written up to eraseblock 0 > [ 71.500646] mtd_oobtest: written 128 eraseblocks > [ 71.505397] mtd_oobtest: verifying all eraseblocks > [ 71.510336] mtd_oobtest: error @addr[0x0:0x0] 0xff -> 0xe diff 0xf1 > [ 71.516727] mtd_oobtest: error @addr[0x0:0x1] 0xff -> 0x10 diff 0xef > [ 71.523164] mtd_oobtest: error: verify failed at 0x0 > [ 71.530372] mtd_oobtest: error @addr[0x800:0x0] 0xff -> 0x82 diff > 0x7d > [ 71.537083] mtd_oobtest: error @addr[0x800:0x1] 0xff -> 0x10 diff > 0xef > [ 71.543709] mtd_oobtest: error: verify failed at 0x800 > > Need to check what is exactly going wrong. I found the issue there, the COMMAND_NADDR_BYTES macro should look like this: #define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) + 1) -- Stefan
On Mon, 26 Feb 2018 21:05:35 +0100 Stefan Agner <stefan@agner.ch> wrote: > On 26.02.2018 08:48, Stefan Agner wrote: > > On 22.02.2018 23:06, Boris Brezillon wrote: > >> On Thu, 22 Feb 2018 21:29:17 +0100 > >> Stefan Agner <stefan@agner.ch> wrote: > >> > >>> This reworks the driver to make use of ->exec_op() callback. The > >>> command sequencer of the VF610 NFC aligns well with the new ops > >>> interface. > >>> > >>> The ops are translated to a NFC command code while filling the > >>> necessary registers. Instead of using the special status and > >>> read id command codes (which require the status/id form special > >>> registers) the driver now uses the main data buffer for all > >>> commands. This simplifies the driver as no special casing is > >>> needed. > >>> > >>> For control data (status byte, id bytes and parameter page) the > >>> driver needs to reverse byte order for little endian CPUs since > >>> the controller seems to store the bytes in big endian order in > >>> the data buffer. > >>> > >>> The current state seems to pass MTD tests on a Colibri VF61. > >>> > >>> Signed-off-by: Stefan Agner <stefan@agner.ch> > >>> --- > >>> drivers/mtd/nand/raw/vf610_nfc.c | 439 +++++++++++++++++++++++++++++++++++++-- > >>> 1 file changed, 425 insertions(+), 14 deletions(-) > >>> > >>> diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c > >>> index 5d7a1f8f580f..9baf80766307 100644 > >>> --- a/drivers/mtd/nand/raw/vf610_nfc.c > >>> +++ b/drivers/mtd/nand/raw/vf610_nfc.c > >>> @@ -74,6 +74,22 @@ > >>> #define RESET_CMD_CODE 0x4040 > >>> #define STATUS_READ_CMD_CODE 0x4068 > >>> > >>> +/* NFC_CMD2[CODE] controller cycle bit masks */ > >>> +#define COMMAND_CMD_BYTE1 BIT(14) > >>> +#define COMMAND_CAR_BYTE1 BIT(13) > >>> +#define COMMAND_CAR_BYTE2 BIT(12) > >>> +#define COMMAND_RAR_BYTE1 BIT(11) > >>> +#define COMMAND_RAR_BYTE2 BIT(10) > >>> +#define COMMAND_RAR_BYTE3 BIT(9) > >>> +#define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) - 1) > >>> +#define COMMAND_WRITE_DATA BIT(8) > >>> +#define COMMAND_CMD_BYTE2 BIT(7) > >>> +#define COMMAND_RB_HANDSHAKE BIT(6) > >>> +#define COMMAND_READ_DATA BIT(5) > >>> +#define COMMAND_CMD_BYTE3 BIT(4) > >>> +#define COMMAND_READ_STATUS BIT(3) > >>> +#define COMMAND_READ_ID BIT(2) > >>> + > >>> /* NFC ECC mode define */ > >>> #define ECC_BYPASS 0 > >>> #define ECC_45_BYTE 6 > >>> @@ -97,10 +113,14 @@ > >>> /* NFC_COL_ADDR Field */ > >>> #define COL_ADDR_MASK 0x0000FFFF > >>> #define COL_ADDR_SHIFT 0 > >>> +#define COL_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) > >>> + > >>> > >>> /* NFC_ROW_ADDR Field */ > >>> #define ROW_ADDR_MASK 0x00FFFFFF > >>> #define ROW_ADDR_SHIFT 0 > >>> +#define ROW_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) > >>> + > >>> #define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000 > >>> #define ROW_ADDR_CHIP_SEL_RB_SHIFT 28 > >>> #define ROW_ADDR_CHIP_SEL_MASK 0x0F000000 > >>> @@ -165,6 +185,7 @@ struct vf610_nfc { > >>> enum vf610_nfc_variant variant; > >>> struct clk *clk; > >>> bool use_hw_ecc; > >>> + bool page_access; > >> > >> This field deserves a comment IMO, even if you describe in details what > >> it does in the rd/wr_from/to_sram() funcs. > >> > >>> u32 ecc_mode; > >>> }; > >>> > >>> @@ -173,6 +194,11 @@ static inline struct vf610_nfc *mtd_to_nfc(struct mtd_info *mtd) > >>> return container_of(mtd_to_nand(mtd), struct vf610_nfc, chip); > >>> } > >>> > >>> +static inline struct vf610_nfc *chip_to_nfc(struct nand_chip *chip) > >>> +{ > >>> + return container_of(chip, struct vf610_nfc, chip); > >>> +} > >>> + > >>> static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg) > >>> { > >>> return readl(nfc->regs + reg); > >>> @@ -214,6 +240,86 @@ static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src, > >>> memcpy(dst, src, n); > >>> } > >>> > >>> +static inline bool vf610_nfc_is_little_endian(void) > >> > >> It's not really the NFC that is LE, is it? I mean, you could have the > >> kernel configured in BIG ENDIAN for this platform, and then you wouldn't > >> have to do the byte-swapping. > >> > >>> +{ > >>> +#ifdef __LITTLE_ENDIAN > >>> + return true; > >>> +#else > >>> + return false; > >>> +#endif > >>> +} > >>> + > >>> +/** > >>> + * Read accessor for internal SRAM buffer > >>> + * @dst: destination address in regular memory > >>> + * @src: source address in SRAM buffer > >>> + * @len: bytes to copy > >>> + * @fix_endian: Fix endianness if required > >>> + * > >>> + * Use this accessor for the internal SRAM buffers. On the ARM > >>> + * Freescale Vybrid SoC it's known that the driver can treat > >>> + * the SRAM buffer as if it's memory. Other platform might need > >>> + * to treat the buffers differently. > >>> + * > >>> + * The controller stores bytes from the NAND chip internally in big > >>> + * endianness. On little endian platforms such as Vybrid this leads > >>> + * to reversed byte order. > >>> + * For performance reason (and earlier probably due to unanawareness) > >> > >> ^ unawareness > >> > >>> + * the driver avoids correcting endianness where it has control over > >>> + * write and read side (e.g. page wise data access). > >>> + * In case endianness matters len should be a multiple of 4. > >>> + */ > >>> +static inline void vf610_nfc_rd_from_sram(void *dst, const void __iomem *src, > >>> + size_t len, bool fix_endian) > >>> +{ > >>> + if (vf610_nfc_is_little_endian() && fix_endian) { > >>> + unsigned int i; > >>> + > >>> + for (i = 0; i < len; i += 4) { > >>> + u32 val = be32_to_cpu(__raw_readl(src + i)); > >>> + memcpy(dst + i, &val, min(sizeof(val), len - i)); > >>> + } > >>> + } else { > >>> + memcpy_fromio(dst, src, len); > >>> + } > >>> +} > >>> + > >>> +/** > >>> + * Write accessor for internal SRAM buffer > >>> + * @dst: destination address in SRAM buffer > >>> + * @src: source address in regular memory > >>> + * @len: bytes to copy > >>> + * @fix_endian: Fix endianness if required > >>> + * > >>> + * Use this accessor for the internal SRAM buffers. On the ARM > >>> + * Freescale Vybrid SoC it's known that the driver can treat > >>> + * the SRAM buffer as if it's memory. Other platform might need > >>> + * to treat the buffers differently. > >>> + * > >>> + * The controller stores bytes from the NAND chip internally in big > >>> + * endianness. On little endian platforms such as Vybrid this leads > >>> + * to reversed byte order. > >>> + * For performance reason (and earlier probably due to unanawareness) > >> > >> ^ unawareness > >> > >>> + * the driver avoids correcting endianness where it has control over > >>> + * write and read side (e.g. page wise data access). > >>> + * In case endianness matters len should be a multiple of 4. > >> > >> That's the opposite actually. If fix_endian is set, we don't have any > >> requirement on len alignment, because the cpu_to_be32(val) will save > >> us. But we have a problem when fix_endian is false an len is not > >> aligned on 4, because, AFAIR memcpy_toio() does 32-bit accesses when > >> things are properly aligned on 32 bits, and when that's not the case it > >> falls back to 16 or 8 bit accesses, which in our case may lead to data > >> loss on LE platforms. > >> > > I guess that also applies to read? Yes it does. > > Wouldn't that mean that my status command is broken? The stack usually > does a 1 byte read there.... No, because endianness it taken into account in this case. That's the very reason you previously had an issue when the STATUS command was executed with ->page_access set to true (in this case endianness is ignored), and this problem disappeared when you moved the call that was executing a STATUS op outside of the ->page_access=true section. > > <snip> > > >> Same problem here. The above logic should be replaced by: > >> > >> nfc->page_access = true; > >> ret = nand_prog_page_begin_op(chip, page, mtd->writesize, > >> chip->oob_poi, mtd->oobsize); > >> > >> nfc->page_access = false; > >> > >> if (ret) > >> return ret; > >> > >> return nand_prog_page_end_op(chip); > >> > > > > Read/write seems to work, but there seems to be an issue with data: > > > > root@colibri-vf:~# insmod mtd_oobtest.ko dev=3 > > [ 69.609120] > > [ 69.610664] ================================================= > > [ 69.616734] mtd_oobtest: MTD device: 3 > > [ 69.631659] mtd_oobtest: MTD device size 16777216, eraseblock size > > 131072, page size 2048, count of eraseblocks 128, pages per eraseblock > > 64, OOB size 64 > > [ 69.647814] mtd_test: scanning for bad eraseblocks > > [ 69.654166] mtd_test: scanned 128 eraseblocks, 0 are bad > > [ 69.659507] mtd_oobtest: test 1 of 5 > > [ 69.736812] mtd_oobtest: writing OOBs of whole device > > [ 69.755753] mtd_oobtest: written up to eraseblock 0 > > [ 71.500646] mtd_oobtest: written 128 eraseblocks > > [ 71.505397] mtd_oobtest: verifying all eraseblocks > > [ 71.510336] mtd_oobtest: error @addr[0x0:0x0] 0xff -> 0xe diff 0xf1 > > [ 71.516727] mtd_oobtest: error @addr[0x0:0x1] 0xff -> 0x10 diff 0xef > > [ 71.523164] mtd_oobtest: error: verify failed at 0x0 > > [ 71.530372] mtd_oobtest: error @addr[0x800:0x0] 0xff -> 0x82 diff > > 0x7d > > [ 71.537083] mtd_oobtest: error @addr[0x800:0x1] 0xff -> 0x10 diff > > 0xef > > [ 71.543709] mtd_oobtest: error: verify failed at 0x800 > > > > Need to check what is exactly going wrong. > > I found the issue there, the COMMAND_NADDR_BYTES macro should look like > this: > > #define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) + 1) I don't remember what I initially suggested, but this one is good ;-).
diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c index 5d7a1f8f580f..9baf80766307 100644 --- a/drivers/mtd/nand/raw/vf610_nfc.c +++ b/drivers/mtd/nand/raw/vf610_nfc.c @@ -74,6 +74,22 @@ #define RESET_CMD_CODE 0x4040 #define STATUS_READ_CMD_CODE 0x4068 +/* NFC_CMD2[CODE] controller cycle bit masks */ +#define COMMAND_CMD_BYTE1 BIT(14) +#define COMMAND_CAR_BYTE1 BIT(13) +#define COMMAND_CAR_BYTE2 BIT(12) +#define COMMAND_RAR_BYTE1 BIT(11) +#define COMMAND_RAR_BYTE2 BIT(10) +#define COMMAND_RAR_BYTE3 BIT(9) +#define COMMAND_NADDR_BYTES(x) GENMASK(13, 13 - (x) - 1) +#define COMMAND_WRITE_DATA BIT(8) +#define COMMAND_CMD_BYTE2 BIT(7) +#define COMMAND_RB_HANDSHAKE BIT(6) +#define COMMAND_READ_DATA BIT(5) +#define COMMAND_CMD_BYTE3 BIT(4) +#define COMMAND_READ_STATUS BIT(3) +#define COMMAND_READ_ID BIT(2) + /* NFC ECC mode define */ #define ECC_BYPASS 0 #define ECC_45_BYTE 6 @@ -97,10 +113,14 @@ /* NFC_COL_ADDR Field */ #define COL_ADDR_MASK 0x0000FFFF #define COL_ADDR_SHIFT 0 +#define COL_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) + /* NFC_ROW_ADDR Field */ #define ROW_ADDR_MASK 0x00FFFFFF #define ROW_ADDR_SHIFT 0 +#define ROW_ADDR(pos, val) ((val & 0xFF) << (8 * (pos))) + #define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000 #define ROW_ADDR_CHIP_SEL_RB_SHIFT 28 #define ROW_ADDR_CHIP_SEL_MASK 0x0F000000 @@ -165,6 +185,7 @@ struct vf610_nfc { enum vf610_nfc_variant variant; struct clk *clk; bool use_hw_ecc; + bool page_access; u32 ecc_mode; }; @@ -173,6 +194,11 @@ static inline struct vf610_nfc *mtd_to_nfc(struct mtd_info *mtd) return container_of(mtd_to_nand(mtd), struct vf610_nfc, chip); } +static inline struct vf610_nfc *chip_to_nfc(struct nand_chip *chip) +{ + return container_of(chip, struct vf610_nfc, chip); +} + static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg) { return readl(nfc->regs + reg); @@ -214,6 +240,86 @@ static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src, memcpy(dst, src, n); } +static inline bool vf610_nfc_is_little_endian(void) +{ +#ifdef __LITTLE_ENDIAN + return true; +#else + return false; +#endif +} + +/** + * Read accessor for internal SRAM buffer + * @dst: destination address in regular memory + * @src: source address in SRAM buffer + * @len: bytes to copy + * @fix_endian: Fix endianness if required + * + * Use this accessor for the internal SRAM buffers. On the ARM + * Freescale Vybrid SoC it's known that the driver can treat + * the SRAM buffer as if it's memory. Other platform might need + * to treat the buffers differently. + * + * The controller stores bytes from the NAND chip internally in big + * endianness. On little endian platforms such as Vybrid this leads + * to reversed byte order. + * For performance reason (and earlier probably due to unanawareness) + * the driver avoids correcting endianness where it has control over + * write and read side (e.g. page wise data access). + * In case endianness matters len should be a multiple of 4. + */ +static inline void vf610_nfc_rd_from_sram(void *dst, const void __iomem *src, + size_t len, bool fix_endian) +{ + if (vf610_nfc_is_little_endian() && fix_endian) { + unsigned int i; + + for (i = 0; i < len; i += 4) { + u32 val = be32_to_cpu(__raw_readl(src + i)); + memcpy(dst + i, &val, min(sizeof(val), len - i)); + } + } else { + memcpy_fromio(dst, src, len); + } +} + +/** + * Write accessor for internal SRAM buffer + * @dst: destination address in SRAM buffer + * @src: source address in regular memory + * @len: bytes to copy + * @fix_endian: Fix endianness if required + * + * Use this accessor for the internal SRAM buffers. On the ARM + * Freescale Vybrid SoC it's known that the driver can treat + * the SRAM buffer as if it's memory. Other platform might need + * to treat the buffers differently. + * + * The controller stores bytes from the NAND chip internally in big + * endianness. On little endian platforms such as Vybrid this leads + * to reversed byte order. + * For performance reason (and earlier probably due to unanawareness) + * the driver avoids correcting endianness where it has control over + * write and read side (e.g. page wise data access). + * In case endianness matters len should be a multiple of 4. + */ +static inline void vf610_nfc_wr_to_sram(void __iomem *dst, const void *src, + size_t len, bool fix_endian) +{ + if (vf610_nfc_is_little_endian() && fix_endian) { + unsigned int i; + + for (i = 0; i < len; i += 4) { + u32 val; + memcpy(&val, src + i, min(sizeof(val), len - i)); + __raw_writel(cpu_to_be32(val), dst + i); + } + } else { + memcpy_toio(dst, src, len); + } +} + /* Clear flags for upcoming command */ static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc) { @@ -489,6 +595,170 @@ static int vf610_nfc_dev_ready(struct mtd_info *mtd) return 1; } +static inline void vf610_nfc_run(struct vf610_nfc *nfc, u32 col, u32 row, u32 cmd1, u32 cmd2, u32 trfr_sz) +{ + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK, + COL_ADDR_SHIFT, col); + + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK, + ROW_ADDR_SHIFT, row); + + vf610_nfc_write(nfc, NFC_SECTOR_SIZE, trfr_sz); + vf610_nfc_write(nfc, NFC_FLASH_CMD1, cmd1); + vf610_nfc_write(nfc, NFC_FLASH_CMD2, cmd2); + + dev_dbg(nfc->dev, "col 0x%08x, row 0x%08x, cmd1 0x%08x, cmd2 0x%08x, trfr_sz %d\n", + col, row, cmd1, cmd2, trfr_sz); + + vf610_nfc_done(nfc); +} + +static inline const struct nand_op_instr *vf610_get_next_instr( + const struct nand_subop *subop, int *op_id) +{ + if (*op_id + 1 >= subop->ninstrs) + return NULL; + + (*op_id)++; + + return &subop->instrs[*op_id]; +} + +static int vf610_nfc_cmd(struct nand_chip *chip, + const struct nand_subop *subop) +{ + const struct nand_op_instr *instr; + struct vf610_nfc *nfc = chip_to_nfc(chip); + int op_id = -1, trfr_sz = 0, offset; + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; + bool force8bit = false; + + /* + * Some ops are optional, but the hardware requires the operations + * to be in this exact order. + * The op parser enforces the order and makes sure that there isn't + * a read and write element in a single operation. + */ + instr = vf610_get_next_instr(subop, &op_id); + if (!instr) + return -EINVAL; + + if (instr && instr->type == NAND_OP_CMD_INSTR) { + dev_dbg(nfc->dev, "OP_CMD: code 0x%02x\n", instr->ctx.cmd.opcode); + cmd2 |= instr->ctx.cmd.opcode << CMD_BYTE1_SHIFT; + code |= COMMAND_CMD_BYTE1; + + instr = vf610_get_next_instr(subop, &op_id); + } + + if (instr && instr->type == NAND_OP_ADDR_INSTR) { + int naddrs = nand_subop_get_num_addr_cyc(subop, op_id); + int i = nand_subop_get_addr_start_off(subop, op_id); + + for (; i < naddrs; i++) { + u8 val = instr->ctx.addr.addrs[i]; + if (i < 2) + col |= COL_ADDR(i, val); + else + row |= ROW_ADDR(i - 2, val); + } + code |= COMMAND_NADDR_BYTES(naddrs); + + dev_dbg(nfc->dev, "OP_ADDR: col %d, row %d\n", col, row); + + instr = vf610_get_next_instr(subop, &op_id); + } + + if (instr && instr->type == NAND_OP_DATA_OUT_INSTR) { + trfr_sz = nand_subop_get_data_len(subop, op_id); + offset = nand_subop_get_data_start_off(subop, op_id); + force8bit = instr->ctx.data.force_8bit; + + dev_dbg(nfc->dev, "OP_DATA_OUT: len %d, offset %d\n", + trfr_sz, offset); + + /* We don't care about endianness when writing a NAND page */ + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0) + offset, + instr->ctx.data.buf.in + offset, + trfr_sz, !nfc->page_access); + code |= COMMAND_WRITE_DATA; + + instr = vf610_get_next_instr(subop, &op_id); + } + + if (instr && instr->type == NAND_OP_CMD_INSTR) { + cmd1 |= instr->ctx.cmd.opcode << CMD_BYTE2_SHIFT; + code |= COMMAND_CMD_BYTE2; + + instr = vf610_get_next_instr(subop, &op_id); + } + + if (instr && instr->type == NAND_OP_WAITRDY_INSTR) { + code |= COMMAND_RB_HANDSHAKE; + + instr = vf610_get_next_instr(subop, &op_id); + } + + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { + trfr_sz = nand_subop_get_data_len(subop, op_id); + offset = nand_subop_get_data_start_off(subop, op_id); + force8bit = instr->ctx.data.force_8bit; + + dev_dbg(nfc->dev, "OP_DATA_IN: len %d, offset %d\n", + trfr_sz, offset); + + code |= COMMAND_READ_DATA; + } + + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); + + cmd2 |= code << CMD_CODE_SHIFT; + + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); + + if (instr && instr->type == NAND_OP_DATA_IN_INSTR) { + /* We don't care about endianness when reading a NAND page */ + vf610_nfc_rd_from_sram(instr->ctx.data.buf.in + offset, + nfc->regs + NFC_MAIN_AREA(0) + offset, + trfr_sz, !nfc->page_access); + } + + if (force8bit && (chip->options & NAND_BUSWIDTH_16)) + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT); + + return 0; +} + +static const struct nand_op_parser vf610_nfc_op_parser = NAND_OP_PARSER( + NAND_OP_PARSER_PATTERN( + vf610_nfc_cmd, + NAND_OP_PARSER_PAT_CMD_ELEM(true), + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), + NAND_OP_PARSER_PAT_DATA_OUT_ELEM(true, PAGE_2K + OOB_MAX), + NAND_OP_PARSER_PAT_CMD_ELEM(true), + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true)), + NAND_OP_PARSER_PATTERN( + vf610_nfc_cmd, + NAND_OP_PARSER_PAT_CMD_ELEM(true), + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 5), + NAND_OP_PARSER_PAT_CMD_ELEM(true), + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true), + NAND_OP_PARSER_PAT_DATA_IN_ELEM(true, PAGE_2K + OOB_MAX)), + ); + +static int vf610_nfc_exec_op(struct nand_chip *chip, + const struct nand_operation *op, + bool check_only) +{ + struct vf610_nfc *nfc = chip_to_nfc(chip); + + dev_dbg(nfc->dev, "exec_op, opcode 0x%02x\n", op->instrs[0].ctx.cmd.opcode); + + return nand_op_parser_exec_op(chip, &vf610_nfc_op_parser, op, check_only); +} + + /* * This function supports Vybrid only (MPC5125 would have full RB and four CS) */ @@ -526,9 +796,14 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, if (!(ecc_status & ECC_STATUS_MASK)) return ecc_count; - /* Read OOB without ECC unit enabled */ - vf610_nfc_command(mtd, NAND_CMD_READOOB, 0, page); - vf610_nfc_read_buf(mtd, oob, mtd->oobsize); + /* + * Read OOB without ECC unit enabled. We temporarily set ->page_access + * to true to make sure vf610_nfc_cmd() does not swap bytes when + * reading data from the internal SRAM. + */ + nfc->page_access = true; + nand_read_oob_op(&nfc->chip, page, 0, oob, mtd->oobsize); + nfc->page_access = false; /* * On an erased page, bit count (including OOB) should be zero or @@ -542,12 +817,46 @@ static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat, static int vf610_nfc_read_page(struct mtd_info *mtd, struct nand_chip *chip, uint8_t *buf, int oob_required, int page) { - int eccsize = chip->ecc.size; + struct vf610_nfc *nfc = mtd_to_nfc(mtd); + int trfr_sz = mtd->writesize + mtd->oobsize; + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; int stat; - nand_read_page_op(chip, page, 0, buf, eccsize); + cmd2 |= NAND_CMD_READ0 << CMD_BYTE1_SHIFT; + code |= COMMAND_CMD_BYTE1; + + code |= COMMAND_CAR_BYTE1; + code |= COMMAND_CAR_BYTE2; + + row = ROW_ADDR(0, page & 0xff); + code |= COMMAND_RAR_BYTE1; + row |= ROW_ADDR(1, page >> 8); + code |= COMMAND_RAR_BYTE2; + + if (chip->options & NAND_ROW_ADDR_3) { + row |= ROW_ADDR(2, page >> 16); + code |= COMMAND_RAR_BYTE3; + } + + cmd1 |= NAND_CMD_READSTART << CMD_BYTE2_SHIFT; + code |= COMMAND_CMD_BYTE2; + + code |= COMMAND_RB_HANDSHAKE; + code |= COMMAND_READ_DATA; + cmd2 |= code << CMD_CODE_SHIFT; + + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); + + /* We don't care about endianness when reading a NAND page */ + vf610_nfc_rd_from_sram(buf, nfc->regs + NFC_MAIN_AREA(0), + mtd->writesize, false); if (oob_required) - vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); + vf610_nfc_rd_from_sram(chip->oob_poi, + nfc->regs + NFC_MAIN_AREA(0) + + mtd->writesize, + mtd->oobsize, false); stat = vf610_nfc_correct_data(mtd, buf, chip->oob_poi, page); @@ -564,16 +873,113 @@ static int vf610_nfc_write_page(struct mtd_info *mtd, struct nand_chip *chip, const uint8_t *buf, int oob_required, int page) { struct vf610_nfc *nfc = mtd_to_nfc(mtd); + int trfr_sz = mtd->writesize + mtd->oobsize; + u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0; + int ret = 0; + + cmd2 |= NAND_CMD_SEQIN << CMD_BYTE1_SHIFT; + code |= COMMAND_CMD_BYTE1; + + code |= COMMAND_CAR_BYTE1; + code |= COMMAND_CAR_BYTE2; + + row = ROW_ADDR(0, page & 0xff); + code |= COMMAND_RAR_BYTE1; + row |= ROW_ADDR(1, page >> 8); + code |= COMMAND_RAR_BYTE2; + if (chip->options & NAND_ROW_ADDR_3) { + row |= ROW_ADDR(2, page >> 16); + code |= COMMAND_RAR_BYTE3; + } - nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize); - if (oob_required) - vf610_nfc_write_buf(mtd, chip->oob_poi, mtd->oobsize); + cmd1 |= NAND_CMD_PAGEPROG << CMD_BYTE2_SHIFT; + code |= COMMAND_CMD_BYTE2; + + code |= COMMAND_WRITE_DATA; + + /* We don't care about endianness when writing a NAND page */ + vf610_nfc_wr_to_sram(nfc->regs + NFC_MAIN_AREA(0), buf, + mtd->writesize, false); - /* Always write whole page including OOB due to HW ECC */ - nfc->use_hw_ecc = true; - nfc->write_sz = mtd->writesize + mtd->oobsize; + code |= COMMAND_RB_HANDSHAKE; + cmd2 |= code << CMD_CODE_SHIFT; - return nand_prog_page_end_op(chip); + vf610_nfc_ecc_mode(nfc, nfc->ecc_mode); + vf610_nfc_run(nfc, col, row, cmd1, cmd2, trfr_sz); + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); + + return ret; +} + +static int vf610_nfc_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, + uint8_t *buf, int oob_required, int page) +{ + struct vf610_nfc *nfc = mtd_to_nfc(mtd); + int ret; + + /* + * We temporarily set ->page_access to true to make sure + * vf610_nfc_cmd() does not swap bytes when reading data + * from the internal SRAM. + */ + nfc->page_access = true; + ret = nand_read_page_raw(mtd, chip, buf, oob_required, page); + nfc->page_access = false; + + return ret; +} + +static int vf610_nfc_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, + const uint8_t *buf, int oob_required, int page) +{ + struct vf610_nfc *nfc = mtd_to_nfc(mtd); + int ret; + + /* + * We temporarily set ->page_access to true to make sure + * vf610_nfc_cmd() does not swap bytes when reading data + * from the internal SRAM. + */ + nfc->page_access = true; + ret = nand_write_page_raw(mtd, chip, buf, oob_required, page); + nfc->page_access = false; + + return ret; +} + +static int vf610_nfc_read_oob(struct mtd_info *mtd, struct nand_chip *chip, + int page) +{ + struct vf610_nfc *nfc = mtd_to_nfc(mtd); + int ret; + + /* + * We temporarily set ->page_access to true to make sure + * vf610_nfc_cmd() does not swap bytes when reading data + * from the internal SRAM. + */ + nfc->page_access = true; + ret = nand_read_oob_std(mtd, chip, page); + nfc->page_access = false; + + return ret; +} +static int vf610_nfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip, + int page) +{ + struct vf610_nfc *nfc = mtd_to_nfc(mtd); + int ret; + + /* + * We temporarily set ->page_access to true to make sure + * vf610_nfc_cmd() does not swap bytes when reading data + * from the internal SRAM. + */ + nfc->page_access = true; + ret = nand_write_oob_std(mtd, chip, page); + nfc->page_access = false; + + return ret; } static const struct of_device_id vf610_nfc_dt_ids[] = { @@ -590,6 +996,7 @@ static void vf610_nfc_preinit_controller(struct vf610_nfc *nfc) vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT); vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT); vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT); + vf610_nfc_ecc_mode(nfc, ECC_BYPASS); /* Disable virtual pages, only one elementary transfer unit */ vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK, @@ -686,6 +1093,7 @@ static int vf610_nfc_probe(struct platform_device *pdev) chip->read_word = vf610_nfc_read_word; chip->read_buf = vf610_nfc_read_buf; chip->write_buf = vf610_nfc_write_buf; + chip->exec_op = vf610_nfc_exec_op; chip->select_chip = vf610_nfc_select_chip; chip->onfi_set_features = nand_onfi_get_set_features_notsupp; chip->onfi_get_features = nand_onfi_get_set_features_notsupp; @@ -755,7 +1163,10 @@ static int vf610_nfc_probe(struct platform_device *pdev) chip->ecc.read_page = vf610_nfc_read_page; chip->ecc.write_page = vf610_nfc_write_page; - + chip->ecc.read_page_raw = vf610_nfc_read_page_raw; + chip->ecc.write_page_raw = vf610_nfc_write_page_raw; + chip->ecc.read_oob = vf610_nfc_read_oob; + chip->ecc.write_oob = vf610_nfc_write_oob; chip->ecc.size = PAGE_2K; }
This reworks the driver to make use of ->exec_op() callback. The command sequencer of the VF610 NFC aligns well with the new ops interface. The ops are translated to a NFC command code while filling the necessary registers. Instead of using the special status and read id command codes (which require the status/id form special registers) the driver now uses the main data buffer for all commands. This simplifies the driver as no special casing is needed. For control data (status byte, id bytes and parameter page) the driver needs to reverse byte order for little endian CPUs since the controller seems to store the bytes in big endian order in the data buffer. The current state seems to pass MTD tests on a Colibri VF61. Signed-off-by: Stefan Agner <stefan@agner.ch> --- drivers/mtd/nand/raw/vf610_nfc.c | 439 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 425 insertions(+), 14 deletions(-)