[PATCH 0/3] Convert fsmc_nand driver to ->exec_op() to retrieve a unique ID

[PATCH 0/3] Convert fsmc_nand driver to ->exec_op() to retrieve a unique ID

[Miquel Raynal]

Hi,

A previous work is migrating the NAND framework from ->cmdfunc() and
->cmd_ctrl() to ->exec_op(), a different way to interract between NAND
core and NAND controller drivers.

Retrieving a unique ID from some ONFI NAND chips needs to send a
particular command to the chip. As there is no feedback from
->cmdfunc(), it may or it may not work depending on the controller
driver in use, but the core will not be notified. Using ->exec_op(),
instead, allows the core to know if the command can be handled by the
controller driver. For this reason, only drivers converted to
->exec_op() will benefit from the nand_read_unique_id() function
introduced in this series, as the first patch does it with the
fsmc_nand driver.

About exposing the unique ID in the mtd_info structure, the commit
message will give more details but long story short: it was hard to do
better without doing a deep, time-consuming rework. So I am really open
to suggestions.

Thank you,
Miquèl


Miquel Raynal (3):
  mtd: nand: fsmc: use ->exec_op()
  mtd: nand: add the infrastructure to retrieve the ONFI unique ID
  mtd: nand: add sysfs entry for NAND chip unique ID

 drivers/mtd/mtdcore.c        |  33 +++++-
 drivers/mtd/nand/fsmc_nand.c | 234 ++++++++++++++++++++++++-------------------
 drivers/mtd/nand/nand_base.c | 103 ++++++++++++++++++-
 include/linux/mtd/mtd.h      |   1 +
 include/linux/mtd/rawnand.h  |  19 +++-
 5 files changed, 283 insertions(+), 107 deletions(-)

-- 
2.11.0

[PATCH 1/3] mtd: nand: fsmc: use ->exec_op()

[Miquel Raynal]

Remove the decrecated ->cmd_ctrl() implementation to use ->exec_op() in
the fsmc_nand driver.

Implement the ->select_chip() hook to avoid having to support the hack
from the core that send a NAND_CMD_NONE with NAND_NCE to signal a
deassertion of nCE.

Also remove the use of IO_ADDR_[R|W] in this driver and use a pointer to
the control registers to avoid doing several arithmetic operations
(including a multiplication) each time a control register is read or
written.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/mtd/nand/fsmc_nand.c | 234 ++++++++++++++++++++++++-------------------
 1 file changed, 132 insertions(+), 102 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index b44e5c6545e0..76246d904c28 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -103,10 +103,6 @@
 #define ECC3			0x1C
 #define FSMC_NAND_BANK_SZ	0x20
 
-#define FSMC_NAND_REG(base, bank, reg)		(base + FSMC_NOR_REG_SIZE + \
-						(FSMC_NAND_BANK_SZ * (bank)) + \
-						reg)
-
 #define FSMC_BUSY_WAIT_TIMEOUT	(1 * HZ)
 
 struct fsmc_nand_timings {
@@ -144,6 +140,7 @@ enum access_mode {
  * @cmd_va:		NAND port for Command.
  * @addr_va:		NAND port for Address.
  * @regs_va:		FSMC regs base address.
+ * @regs:		Registers base address for a given bank.
  */
 struct fsmc_nand_data {
 	u32			pid;
@@ -166,6 +163,7 @@ struct fsmc_nand_data {
 	void __iomem		*cmd_va;
 	void __iomem		*addr_va;
 	void __iomem		*regs_va;
+	void __iomem		*regs;
 };
 
 static int fsmc_ecc1_ooblayout_ecc(struct mtd_info *mtd, int section,
@@ -258,45 +256,6 @@ static inline struct fsmc_nand_data *mtd_to_fsmc(struct mtd_info *mtd)
 }
 
 /*
- * fsmc_cmd_ctrl - For facilitaing Hardware access
- * This routine allows hardware specific access to control-lines(ALE,CLE)
- */
-static void fsmc_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
-{
-	struct nand_chip *this = mtd_to_nand(mtd);
-	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
-	void __iomem *regs = host->regs_va;
-	unsigned int bank = host->bank;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		u32 pc;
-
-		if (ctrl & NAND_CLE) {
-			this->IO_ADDR_R = host->cmd_va;
-			this->IO_ADDR_W = host->cmd_va;
-		} else if (ctrl & NAND_ALE) {
-			this->IO_ADDR_R = host->addr_va;
-			this->IO_ADDR_W = host->addr_va;
-		} else {
-			this->IO_ADDR_R = host->data_va;
-			this->IO_ADDR_W = host->data_va;
-		}
-
-		pc = readl(FSMC_NAND_REG(regs, bank, PC));
-		if (ctrl & NAND_NCE)
-			pc |= FSMC_ENABLE;
-		else
-			pc &= ~FSMC_ENABLE;
-		writel_relaxed(pc, FSMC_NAND_REG(regs, bank, PC));
-	}
-
-	mb();
-
-	if (cmd != NAND_CMD_NONE)
-		writeb_relaxed(cmd, this->IO_ADDR_W);
-}
-
-/*
  * fsmc_nand_setup - FSMC (Flexible Static Memory Controller) init routine
  *
  * This routine initializes timing parameters related to NAND memory access in
@@ -307,8 +266,6 @@ static void fsmc_nand_setup(struct fsmc_nand_data *host,
 {
 	uint32_t value = FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON;
 	uint32_t tclr, tar, thiz, thold, twait, tset;
-	unsigned int bank = host->bank;
-	void __iomem *regs = host->regs_va;
 
 	tclr = (tims->tclr & FSMC_TCLR_MASK) << FSMC_TCLR_SHIFT;
 	tar = (tims->tar & FSMC_TAR_MASK) << FSMC_TAR_SHIFT;
@@ -318,18 +275,13 @@ static void fsmc_nand_setup(struct fsmc_nand_data *host,
 	tset = (tims->tset & FSMC_TSET_MASK) << FSMC_TSET_SHIFT;
 
 	if (host->nand.options & NAND_BUSWIDTH_16)
-		writel_relaxed(value | FSMC_DEVWID_16,
-				FSMC_NAND_REG(regs, bank, PC));
+		writel_relaxed(value | FSMC_DEVWID_16, host->regs + PC);
 	else
-		writel_relaxed(value | FSMC_DEVWID_8,
-				FSMC_NAND_REG(regs, bank, PC));
-
-	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) | tclr | tar,
-			FSMC_NAND_REG(regs, bank, PC));
-	writel_relaxed(thiz | thold | twait | tset,
-			FSMC_NAND_REG(regs, bank, COMM));
-	writel_relaxed(thiz | thold | twait | tset,
-			FSMC_NAND_REG(regs, bank, ATTRIB));
+		writel_relaxed(value | FSMC_DEVWID_8, host->regs + PC);
+
+	writel_relaxed(readl(host->regs + PC) | tclr | tar, host->regs + PC);
+	writel_relaxed(thiz | thold | twait | tset, host->regs + COMM);
+	writel_relaxed(thiz | thold | twait | tset, host->regs + ATTRIB);
 }
 
 static int fsmc_calc_timings(struct fsmc_nand_data *host,
@@ -419,15 +371,11 @@ static int fsmc_setup_data_interface(struct mtd_info *mtd, int csline,
 static void fsmc_enable_hwecc(struct mtd_info *mtd, int mode)
 {
 	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
-	void __iomem *regs = host->regs_va;
-	uint32_t bank = host->bank;
-
-	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) & ~FSMC_ECCPLEN_256,
-			FSMC_NAND_REG(regs, bank, PC));
-	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) & ~FSMC_ECCEN,
-			FSMC_NAND_REG(regs, bank, PC));
-	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) | FSMC_ECCEN,
-			FSMC_NAND_REG(regs, bank, PC));
+
+	writel_relaxed(readl(host->regs + PC) & ~FSMC_ECCPLEN_256,
+		       host->regs + PC);
+	writel_relaxed(readl(host->regs + PC) & ~FSMC_ECCEN, host->regs + PC);
+	writel_relaxed(readl(host->regs + PC) | FSMC_ECCEN, host->regs + PC);
 }
 
 /*
@@ -439,13 +387,11 @@ static int fsmc_read_hwecc_ecc4(struct mtd_info *mtd, const uint8_t *data,
 				uint8_t *ecc)
 {
 	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
-	void __iomem *regs = host->regs_va;
-	uint32_t bank = host->bank;
 	uint32_t ecc_tmp;
 	unsigned long deadline = jiffies + FSMC_BUSY_WAIT_TIMEOUT;
 
 	do {
-		if (readl_relaxed(FSMC_NAND_REG(regs, bank, STS)) & FSMC_CODE_RDY)
+		if (readl_relaxed(host->regs + STS) & FSMC_CODE_RDY)
 			break;
 		else
 			cond_resched();
@@ -456,25 +402,25 @@ static int fsmc_read_hwecc_ecc4(struct mtd_info *mtd, const uint8_t *data,
 		return -ETIMEDOUT;
 	}
 
-	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC1));
+	ecc_tmp = readl_relaxed(host->regs + ECC1);
 	ecc[0] = (uint8_t) (ecc_tmp >> 0);
 	ecc[1] = (uint8_t) (ecc_tmp >> 8);
 	ecc[2] = (uint8_t) (ecc_tmp >> 16);
 	ecc[3] = (uint8_t) (ecc_tmp >> 24);
 
-	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC2));
+	ecc_tmp = readl_relaxed(host->regs + ECC2);
 	ecc[4] = (uint8_t) (ecc_tmp >> 0);
 	ecc[5] = (uint8_t) (ecc_tmp >> 8);
 	ecc[6] = (uint8_t) (ecc_tmp >> 16);
 	ecc[7] = (uint8_t) (ecc_tmp >> 24);
 
-	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC3));
+	ecc_tmp = readl_relaxed(host->regs + ECC3);
 	ecc[8] = (uint8_t) (ecc_tmp >> 0);
 	ecc[9] = (uint8_t) (ecc_tmp >> 8);
 	ecc[10] = (uint8_t) (ecc_tmp >> 16);
 	ecc[11] = (uint8_t) (ecc_tmp >> 24);
 
-	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, STS));
+	ecc_tmp = readl_relaxed(host->regs + STS);
 	ecc[12] = (uint8_t) (ecc_tmp >> 16);
 
 	return 0;
@@ -489,11 +435,9 @@ static int fsmc_read_hwecc_ecc1(struct mtd_info *mtd, const uint8_t *data,
 				uint8_t *ecc)
 {
 	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
-	void __iomem *regs = host->regs_va;
-	uint32_t bank = host->bank;
 	uint32_t ecc_tmp;
 
-	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC1));
+	ecc_tmp = readl_relaxed(host->regs + ECC1);
 	ecc[0] = (uint8_t) (ecc_tmp >> 0);
 	ecc[1] = (uint8_t) (ecc_tmp >> 8);
 	ecc[2] = (uint8_t) (ecc_tmp >> 16);
@@ -598,18 +542,18 @@ static int dma_xfer(struct fsmc_nand_data *host, void *buffer, int len,
  */
 static void fsmc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 {
+	struct fsmc_nand_data *host  = mtd_to_fsmc(mtd);
 	int i;
-	struct nand_chip *chip = mtd_to_nand(mtd);
 
 	if (IS_ALIGNED((uint32_t)buf, sizeof(uint32_t)) &&
 			IS_ALIGNED(len, sizeof(uint32_t))) {
 		uint32_t *p = (uint32_t *)buf;
 		len = len >> 2;
 		for (i = 0; i < len; i++)
-			writel_relaxed(p[i], chip->IO_ADDR_W);
+			writel_relaxed(p[i], host->data_va);
 	} else {
 		for (i = 0; i < len; i++)
-			writeb_relaxed(buf[i], chip->IO_ADDR_W);
+			writeb_relaxed(buf[i], host->data_va);
 	}
 }
 
@@ -621,18 +565,18 @@ static void fsmc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
  */
 static void fsmc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 {
+	struct fsmc_nand_data *host  = mtd_to_fsmc(mtd);
 	int i;
-	struct nand_chip *chip = mtd_to_nand(mtd);
 
 	if (IS_ALIGNED((uint32_t)buf, sizeof(uint32_t)) &&
 			IS_ALIGNED(len, sizeof(uint32_t))) {
 		uint32_t *p = (uint32_t *)buf;
 		len = len >> 2;
 		for (i = 0; i < len; i++)
-			p[i] = readl_relaxed(chip->IO_ADDR_R);
+			p[i] = readl_relaxed(host->data_va);
 	} else {
 		for (i = 0; i < len; i++)
-			buf[i] = readb_relaxed(chip->IO_ADDR_R);
+			buf[i] = readb_relaxed(host->data_va);
 	}
 }
 
@@ -663,6 +607,102 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
 	dma_xfer(host, (void *)buf, len, DMA_TO_DEVICE);
 }
 
+/* fsmc_select_chip - assert or deassert nCE */
+static void fsmc_select_chip(struct mtd_info *mtd, int chipnr)
+{
+	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
+	u32 pc;
+
+	/* Support only one CS */
+	if (chipnr > 0)
+		return;
+
+	pc = readl(host->regs + PC);
+	if (chipnr < 0)
+		writel_relaxed(pc & ~FSMC_ENABLE, host->regs + PC);
+	else
+		writel_relaxed(pc | FSMC_ENABLE, host->regs + PC);
+
+	/* nCE line must be asserted before starting any operation */
+	mb();
+}
+
+/*
+ * fsmc_exec_op - hook called by the core to execute NAND operations
+ *
+ * This controller is simple enough and thus does not need to use the parser
+ * provided by the core, instead, handle every situation here.
+ */
+static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op,
+			bool check_only)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
+	const struct nand_op_instr *instr = NULL;
+	int ret = 0;
+	unsigned int op_id;
+	int i;
+
+	pr_debug("Executing operation [%d instructions]:\n", op->ninstrs);
+	for (op_id = 0; op_id < op->ninstrs; op_id++) {
+		instr = &op->instrs[op_id];
+
+		switch (instr->type) {
+		case NAND_OP_CMD_INSTR:
+			pr_debug("  ->CMD      [0x%02x]\n",
+				 instr->ctx.cmd.opcode);
+
+			writeb_relaxed(instr->ctx.cmd.opcode, host->cmd_va);
+			break;
+
+		case NAND_OP_ADDR_INSTR:
+			pr_debug("  ->ADDR     [%d cyc]",
+				 instr->ctx.addr.naddrs);
+
+			for (i = 0; i < instr->ctx.addr.naddrs; i++)
+				writeb_relaxed(instr->ctx.addr.addrs[i],
+					       host->addr_va);
+			break;
+
+		case NAND_OP_DATA_IN_INSTR:
+			pr_debug("  ->DATA_IN  [%d B%s]\n", instr->ctx.data.len,
+				 instr->ctx.data.force_8bit ?
+				 ", force 8-bit" : "");
+
+			if (host->mode == USE_DMA_ACCESS)
+				fsmc_read_buf_dma(mtd, instr->ctx.data.buf.in,
+						  instr->ctx.data.len);
+			else
+				fsmc_read_buf(mtd, instr->ctx.data.buf.in,
+					      instr->ctx.data.len);
+			break;
+
+		case NAND_OP_DATA_OUT_INSTR:
+			pr_debug("  ->DATA_OUT [%d B%s]\n", instr->ctx.data.len,
+				 instr->ctx.data.force_8bit ?
+				 ", force 8-bit" : "");
+
+			if (host->mode == USE_DMA_ACCESS)
+				fsmc_write_buf_dma(mtd, instr->ctx.data.buf.out,
+						   instr->ctx.data.len);
+			else
+				fsmc_write_buf(mtd, instr->ctx.data.buf.out,
+					       instr->ctx.data.len);
+			break;
+
+		case NAND_OP_WAITRDY_INSTR:
+			pr_debug("  ->WAITRDY  [max %d ms]\n",
+				 instr->ctx.waitrdy.timeout_ms);
+
+			ret = nand_soft_waitrdy(chip,
+						instr->ctx.waitrdy.timeout_ms);
+			break;
+		}
+	}
+
+	return ret;
+}
+
 /*
  * fsmc_read_page_hwecc
  * @mtd:	mtd info structure
@@ -754,13 +794,11 @@ static int fsmc_bch8_correct_data(struct mtd_info *mtd, uint8_t *dat,
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
-	void __iomem *regs = host->regs_va;
-	unsigned int bank = host->bank;
 	uint32_t err_idx[8];
 	uint32_t num_err, i;
 	uint32_t ecc1, ecc2, ecc3, ecc4;
 
-	num_err = (readl_relaxed(FSMC_NAND_REG(regs, bank, STS)) >> 10) & 0xF;
+	num_err = (readl_relaxed(host->regs + STS) >> 10) & 0xF;
 
 	/* no bit flipping */
 	if (likely(num_err == 0))
@@ -803,10 +841,10 @@ static int fsmc_bch8_correct_data(struct mtd_info *mtd, uint8_t *dat,
 	 * uint64_t array and error offset indexes are populated in err_idx
 	 * array
 	 */
-	ecc1 = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC1));
-	ecc2 = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC2));
-	ecc3 = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC3));
-	ecc4 = readl_relaxed(FSMC_NAND_REG(regs, bank, STS));
+	ecc1 = readl_relaxed(host->regs + ECC1);
+	ecc2 = readl_relaxed(host->regs + ECC2);
+	ecc3 = readl_relaxed(host->regs + ECC3);
+	ecc4 = readl_relaxed(host->regs + STS);
 
 	err_idx[0] = (ecc1 >> 0) & 0x1FFF;
 	err_idx[1] = (ecc1 >> 13) & 0x1FFF;
@@ -933,6 +971,9 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 		return PTR_ERR(host->clk);
 	}
 
+	host->regs = host->regs_va + FSMC_NOR_REG_SIZE +
+		     (host->bank * FSMC_NAND_BANK_SZ);
+
 	ret = clk_prepare_enable(host->clk);
 	if (ret)
 		return ret;
@@ -960,9 +1001,8 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand_set_flash_node(nand, pdev->dev.of_node);
 
 	mtd->dev.parent = &pdev->dev;
-	nand->IO_ADDR_R = host->data_va;
-	nand->IO_ADDR_W = host->data_va;
-	nand->cmd_ctrl = fsmc_cmd_ctrl;
+	nand->exec_op = fsmc_exec_op;
+	nand->select_chip = fsmc_select_chip;
 	nand->chip_delay = 30;
 
 	/*
@@ -974,8 +1014,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	nand->ecc.size = 512;
 	nand->badblockbits = 7;
 
-	switch (host->mode) {
-	case USE_DMA_ACCESS:
+	if (host->mode == USE_DMA_ACCESS) {
 		dma_cap_zero(mask);
 		dma_cap_set(DMA_MEMCPY, mask);
 		host->read_dma_chan = dma_request_channel(mask, filter, NULL);
@@ -988,15 +1027,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "Unable to get write dma channel\n");
 			goto err_req_write_chnl;
 		}
-		nand->read_buf = fsmc_read_buf_dma;
-		nand->write_buf = fsmc_write_buf_dma;
-		break;
-
-	default:
-	case USE_WORD_ACCESS:
-		nand->read_buf = fsmc_read_buf;
-		nand->write_buf = fsmc_write_buf;
-		break;
 	}
 
 	if (host->dev_timings)
-- 
2.11.0

Re: [PATCH 1/3] mtd: nand: fsmc: use ->exec_op()

[Boris Brezillon]

Hi Miquel,

On Tue, 14 Nov 2017 16:46:20 +0100
Miquel Raynal <miquel.raynal@free-electrons.com> wrote:

> Remove the decrecated ->cmd_ctrl() implementation to use ->exec_op() in
> the fsmc_nand driver.
> 
> Implement the ->select_chip() hook to avoid having to support the hack
> from the core that send a NAND_CMD_NONE with NAND_NCE to signal a
> deassertion of nCE.
> 
> Also remove the use of IO_ADDR_[R|W] in this driver and use a pointer to
> the control registers to avoid doing several arithmetic operations
> (including a multiplication) each time a control register is read or
> written.

Can you split that in 2 patches (one that gets rid of IO_ADDR_[R|W] and
a second one switching from ->cmd_ctrl() to ->exec_op()
+ ->select_chip())?

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> ---
>  drivers/mtd/nand/fsmc_nand.c | 234 ++++++++++++++++++++++++-------------------
>  1 file changed, 132 insertions(+), 102 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
> index b44e5c6545e0..76246d904c28 100644
> --- a/drivers/mtd/nand/fsmc_nand.c
> +++ b/drivers/mtd/nand/fsmc_nand.c
> @@ -103,10 +103,6 @@
>  #define ECC3			0x1C
>  #define FSMC_NAND_BANK_SZ	0x20
>  
> -#define FSMC_NAND_REG(base, bank, reg)		(base + FSMC_NOR_REG_SIZE + \
> -						(FSMC_NAND_BANK_SZ * (bank)) + \
> -						reg)
> -
>  #define FSMC_BUSY_WAIT_TIMEOUT	(1 * HZ)
>  
>  struct fsmc_nand_timings {
> @@ -144,6 +140,7 @@ enum access_mode {
>   * @cmd_va:		NAND port for Command.
>   * @addr_va:		NAND port for Address.
>   * @regs_va:		FSMC regs base address.
> + * @regs:		Registers base address for a given bank.
>   */
>  struct fsmc_nand_data {
>  	u32			pid;
> @@ -166,6 +163,7 @@ struct fsmc_nand_data {
>  	void __iomem		*cmd_va;
>  	void __iomem		*addr_va;
>  	void __iomem		*regs_va;
> +	void __iomem		*regs;

I'm almost sure you don't need an extra field here. Just re-use reg_va
and make it point to the appropriate bank registers instead of FSMC base.

>  };
>  
>  static int fsmc_ecc1_ooblayout_ecc(struct mtd_info *mtd, int section,
> @@ -258,45 +256,6 @@ static inline struct fsmc_nand_data *mtd_to_fsmc(struct mtd_info *mtd)
>  }
>  
>  /*
> - * fsmc_cmd_ctrl - For facilitaing Hardware access
> - * This routine allows hardware specific access to control-lines(ALE,CLE)
> - */
> -static void fsmc_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
> -{
> -	struct nand_chip *this = mtd_to_nand(mtd);
> -	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
> -	void __iomem *regs = host->regs_va;
> -	unsigned int bank = host->bank;
> -
> -	if (ctrl & NAND_CTRL_CHANGE) {
> -		u32 pc;
> -
> -		if (ctrl & NAND_CLE) {
> -			this->IO_ADDR_R = host->cmd_va;
> -			this->IO_ADDR_W = host->cmd_va;
> -		} else if (ctrl & NAND_ALE) {
> -			this->IO_ADDR_R = host->addr_va;
> -			this->IO_ADDR_W = host->addr_va;
> -		} else {
> -			this->IO_ADDR_R = host->data_va;
> -			this->IO_ADDR_W = host->data_va;
> -		}
> -
> -		pc = readl(FSMC_NAND_REG(regs, bank, PC));
> -		if (ctrl & NAND_NCE)
> -			pc |= FSMC_ENABLE;
> -		else
> -			pc &= ~FSMC_ENABLE;
> -		writel_relaxed(pc, FSMC_NAND_REG(regs, bank, PC));
> -	}
> -
> -	mb();
> -
> -	if (cmd != NAND_CMD_NONE)
> -		writeb_relaxed(cmd, this->IO_ADDR_W);
> -}
> -
> -/*
>   * fsmc_nand_setup - FSMC (Flexible Static Memory Controller) init routine
>   *
>   * This routine initializes timing parameters related to NAND memory access in
> @@ -307,8 +266,6 @@ static void fsmc_nand_setup(struct fsmc_nand_data *host,
>  {
>  	uint32_t value = FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON;
>  	uint32_t tclr, tar, thiz, thold, twait, tset;
> -	unsigned int bank = host->bank;
> -	void __iomem *regs = host->regs_va;
>  
>  	tclr = (tims->tclr & FSMC_TCLR_MASK) << FSMC_TCLR_SHIFT;
>  	tar = (tims->tar & FSMC_TAR_MASK) << FSMC_TAR_SHIFT;
> @@ -318,18 +275,13 @@ static void fsmc_nand_setup(struct fsmc_nand_data *host,
>  	tset = (tims->tset & FSMC_TSET_MASK) << FSMC_TSET_SHIFT;
>  
>  	if (host->nand.options & NAND_BUSWIDTH_16)
> -		writel_relaxed(value | FSMC_DEVWID_16,
> -				FSMC_NAND_REG(regs, bank, PC));
> +		writel_relaxed(value | FSMC_DEVWID_16, host->regs + PC);
>  	else
> -		writel_relaxed(value | FSMC_DEVWID_8,
> -				FSMC_NAND_REG(regs, bank, PC));
> -
> -	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) | tclr | tar,
> -			FSMC_NAND_REG(regs, bank, PC));
> -	writel_relaxed(thiz | thold | twait | tset,
> -			FSMC_NAND_REG(regs, bank, COMM));
> -	writel_relaxed(thiz | thold | twait | tset,
> -			FSMC_NAND_REG(regs, bank, ATTRIB));
> +		writel_relaxed(value | FSMC_DEVWID_8, host->regs + PC);
> +
> +	writel_relaxed(readl(host->regs + PC) | tclr | tar, host->regs + PC);
> +	writel_relaxed(thiz | thold | twait | tset, host->regs + COMM);
> +	writel_relaxed(thiz | thold | twait | tset, host->regs + ATTRIB);
>  }
>  
>  static int fsmc_calc_timings(struct fsmc_nand_data *host,
> @@ -419,15 +371,11 @@ static int fsmc_setup_data_interface(struct mtd_info *mtd, int csline,
>  static void fsmc_enable_hwecc(struct mtd_info *mtd, int mode)
>  {
>  	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
> -	void __iomem *regs = host->regs_va;
> -	uint32_t bank = host->bank;
> -
> -	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) & ~FSMC_ECCPLEN_256,
> -			FSMC_NAND_REG(regs, bank, PC));
> -	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) & ~FSMC_ECCEN,
> -			FSMC_NAND_REG(regs, bank, PC));
> -	writel_relaxed(readl(FSMC_NAND_REG(regs, bank, PC)) | FSMC_ECCEN,
> -			FSMC_NAND_REG(regs, bank, PC));
> +
> +	writel_relaxed(readl(host->regs + PC) & ~FSMC_ECCPLEN_256,
> +		       host->regs + PC);
> +	writel_relaxed(readl(host->regs + PC) & ~FSMC_ECCEN, host->regs + PC);
> +	writel_relaxed(readl(host->regs + PC) | FSMC_ECCEN, host->regs + PC);
>  }
>  
>  /*
> @@ -439,13 +387,11 @@ static int fsmc_read_hwecc_ecc4(struct mtd_info *mtd, const uint8_t *data,
>  				uint8_t *ecc)
>  {
>  	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
> -	void __iomem *regs = host->regs_va;
> -	uint32_t bank = host->bank;
>  	uint32_t ecc_tmp;
>  	unsigned long deadline = jiffies + FSMC_BUSY_WAIT_TIMEOUT;
>  
>  	do {
> -		if (readl_relaxed(FSMC_NAND_REG(regs, bank, STS)) & FSMC_CODE_RDY)
> +		if (readl_relaxed(host->regs + STS) & FSMC_CODE_RDY)
>  			break;
>  		else
>  			cond_resched();
> @@ -456,25 +402,25 @@ static int fsmc_read_hwecc_ecc4(struct mtd_info *mtd, const uint8_t *data,
>  		return -ETIMEDOUT;
>  	}
>  
> -	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC1));
> +	ecc_tmp = readl_relaxed(host->regs + ECC1);
>  	ecc[0] = (uint8_t) (ecc_tmp >> 0);
>  	ecc[1] = (uint8_t) (ecc_tmp >> 8);
>  	ecc[2] = (uint8_t) (ecc_tmp >> 16);
>  	ecc[3] = (uint8_t) (ecc_tmp >> 24);
>  
> -	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC2));
> +	ecc_tmp = readl_relaxed(host->regs + ECC2);
>  	ecc[4] = (uint8_t) (ecc_tmp >> 0);
>  	ecc[5] = (uint8_t) (ecc_tmp >> 8);
>  	ecc[6] = (uint8_t) (ecc_tmp >> 16);
>  	ecc[7] = (uint8_t) (ecc_tmp >> 24);
>  
> -	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC3));
> +	ecc_tmp = readl_relaxed(host->regs + ECC3);
>  	ecc[8] = (uint8_t) (ecc_tmp >> 0);
>  	ecc[9] = (uint8_t) (ecc_tmp >> 8);
>  	ecc[10] = (uint8_t) (ecc_tmp >> 16);
>  	ecc[11] = (uint8_t) (ecc_tmp >> 24);
>  
> -	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, STS));
> +	ecc_tmp = readl_relaxed(host->regs + STS);
>  	ecc[12] = (uint8_t) (ecc_tmp >> 16);
>  
>  	return 0;
> @@ -489,11 +435,9 @@ static int fsmc_read_hwecc_ecc1(struct mtd_info *mtd, const uint8_t *data,
>  				uint8_t *ecc)
>  {
>  	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
> -	void __iomem *regs = host->regs_va;
> -	uint32_t bank = host->bank;
>  	uint32_t ecc_tmp;
>  
> -	ecc_tmp = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC1));
> +	ecc_tmp = readl_relaxed(host->regs + ECC1);
>  	ecc[0] = (uint8_t) (ecc_tmp >> 0);
>  	ecc[1] = (uint8_t) (ecc_tmp >> 8);
>  	ecc[2] = (uint8_t) (ecc_tmp >> 16);
> @@ -598,18 +542,18 @@ static int dma_xfer(struct fsmc_nand_data *host, void *buffer, int len,
>   */
>  static void fsmc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
>  {
> +	struct fsmc_nand_data *host  = mtd_to_fsmc(mtd);
>  	int i;
> -	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	if (IS_ALIGNED((uint32_t)buf, sizeof(uint32_t)) &&
>  			IS_ALIGNED(len, sizeof(uint32_t))) {
>  		uint32_t *p = (uint32_t *)buf;
>  		len = len >> 2;
>  		for (i = 0; i < len; i++)
> -			writel_relaxed(p[i], chip->IO_ADDR_W);
> +			writel_relaxed(p[i], host->data_va);
>  	} else {
>  		for (i = 0; i < len; i++)
> -			writeb_relaxed(buf[i], chip->IO_ADDR_W);
> +			writeb_relaxed(buf[i], host->data_va);
>  	}
>  }
>  
> @@ -621,18 +565,18 @@ static void fsmc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
>   */
>  static void fsmc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>  {
> +	struct fsmc_nand_data *host  = mtd_to_fsmc(mtd);
>  	int i;
> -	struct nand_chip *chip = mtd_to_nand(mtd);
>  
>  	if (IS_ALIGNED((uint32_t)buf, sizeof(uint32_t)) &&
>  			IS_ALIGNED(len, sizeof(uint32_t))) {
>  		uint32_t *p = (uint32_t *)buf;
>  		len = len >> 2;
>  		for (i = 0; i < len; i++)
> -			p[i] = readl_relaxed(chip->IO_ADDR_R);
> +			p[i] = readl_relaxed(host->data_va);
>  	} else {
>  		for (i = 0; i < len; i++)
> -			buf[i] = readb_relaxed(chip->IO_ADDR_R);
> +			buf[i] = readb_relaxed(host->data_va);
>  	}
>  }
>  
> @@ -663,6 +607,102 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
>  	dma_xfer(host, (void *)buf, len, DMA_TO_DEVICE);
>  }
>  
> +/* fsmc_select_chip - assert or deassert nCE */
> +static void fsmc_select_chip(struct mtd_info *mtd, int chipnr)
> +{
> +	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
> +	u32 pc;
> +
> +	/* Support only one CS */
> +	if (chipnr > 0)
> +		return;
> +
> +	pc = readl(host->regs + PC);
> +	if (chipnr < 0)
> +		writel_relaxed(pc & ~FSMC_ENABLE, host->regs + PC);
> +	else
> +		writel_relaxed(pc | FSMC_ENABLE, host->regs + PC);
> +
> +	/* nCE line must be asserted before starting any operation */
> +	mb();
> +}
> +
> +/*
> + * fsmc_exec_op - hook called by the core to execute NAND operations
> + *
> + * This controller is simple enough and thus does not need to use the parser
> + * provided by the core, instead, handle every situation here.
> + */
> +static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op,
> +			bool check_only)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
> +	const struct nand_op_instr *instr = NULL;
> +	int ret = 0;
> +	unsigned int op_id;
> +	int i;
> +
> +	pr_debug("Executing operation [%d instructions]:\n", op->ninstrs);
> +	for (op_id = 0; op_id < op->ninstrs; op_id++) {
> +		instr = &op->instrs[op_id];
> +
> +		switch (instr->type) {
> +		case NAND_OP_CMD_INSTR:
> +			pr_debug("  ->CMD      [0x%02x]\n",
> +				 instr->ctx.cmd.opcode);
> +
> +			writeb_relaxed(instr->ctx.cmd.opcode, host->cmd_va);
> +			break;
> +
> +		case NAND_OP_ADDR_INSTR:
> +			pr_debug("  ->ADDR     [%d cyc]",
> +				 instr->ctx.addr.naddrs);
> +
> +			for (i = 0; i < instr->ctx.addr.naddrs; i++)
> +				writeb_relaxed(instr->ctx.addr.addrs[i],
> +					       host->addr_va);
> +			break;
> +
> +		case NAND_OP_DATA_IN_INSTR:
> +			pr_debug("  ->DATA_IN  [%d B%s]\n", instr->ctx.data.len,
> +				 instr->ctx.data.force_8bit ?
> +				 ", force 8-bit" : "");
> +
> +			if (host->mode == USE_DMA_ACCESS)
> +				fsmc_read_buf_dma(mtd, instr->ctx.data.buf.in,
> +						  instr->ctx.data.len);
> +			else
> +				fsmc_read_buf(mtd, instr->ctx.data.buf.in,
> +					      instr->ctx.data.len);
> +			break;
> +
> +		case NAND_OP_DATA_OUT_INSTR:
> +			pr_debug("  ->DATA_OUT [%d B%s]\n", instr->ctx.data.len,
> +				 instr->ctx.data.force_8bit ?
> +				 ", force 8-bit" : "");
> +
> +			if (host->mode == USE_DMA_ACCESS)
> +				fsmc_write_buf_dma(mtd, instr->ctx.data.buf.out,
> +						   instr->ctx.data.len);
> +			else
> +				fsmc_write_buf(mtd, instr->ctx.data.buf.out,
> +					       instr->ctx.data.len);
> +			break;
> +
> +		case NAND_OP_WAITRDY_INSTR:
> +			pr_debug("  ->WAITRDY  [max %d ms]\n",
> +				 instr->ctx.waitrdy.timeout_ms);
> +
> +			ret = nand_soft_waitrdy(chip,
> +						instr->ctx.waitrdy.timeout_ms);

I guess nand_soft_waitrdy() will be part of the next version of your
->exec_op() series, right?

> +			break;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  /*
>   * fsmc_read_page_hwecc
>   * @mtd:	mtd info structure
> @@ -754,13 +794,11 @@ static int fsmc_bch8_correct_data(struct mtd_info *mtd, uint8_t *dat,
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	struct fsmc_nand_data *host = mtd_to_fsmc(mtd);
> -	void __iomem *regs = host->regs_va;
> -	unsigned int bank = host->bank;
>  	uint32_t err_idx[8];
>  	uint32_t num_err, i;
>  	uint32_t ecc1, ecc2, ecc3, ecc4;
>  
> -	num_err = (readl_relaxed(FSMC_NAND_REG(regs, bank, STS)) >> 10) & 0xF;
> +	num_err = (readl_relaxed(host->regs + STS) >> 10) & 0xF;
>  
>  	/* no bit flipping */
>  	if (likely(num_err == 0))
> @@ -803,10 +841,10 @@ static int fsmc_bch8_correct_data(struct mtd_info *mtd, uint8_t *dat,
>  	 * uint64_t array and error offset indexes are populated in err_idx
>  	 * array
>  	 */
> -	ecc1 = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC1));
> -	ecc2 = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC2));
> -	ecc3 = readl_relaxed(FSMC_NAND_REG(regs, bank, ECC3));
> -	ecc4 = readl_relaxed(FSMC_NAND_REG(regs, bank, STS));
> +	ecc1 = readl_relaxed(host->regs + ECC1);
> +	ecc2 = readl_relaxed(host->regs + ECC2);
> +	ecc3 = readl_relaxed(host->regs + ECC3);
> +	ecc4 = readl_relaxed(host->regs + STS);
>  
>  	err_idx[0] = (ecc1 >> 0) & 0x1FFF;
>  	err_idx[1] = (ecc1 >> 13) & 0x1FFF;
> @@ -933,6 +971,9 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  		return PTR_ERR(host->clk);
>  	}
>  
> +	host->regs = host->regs_va + FSMC_NOR_REG_SIZE +
> +		     (host->bank * FSMC_NAND_BANK_SZ);
> +
>  	ret = clk_prepare_enable(host->clk);
>  	if (ret)
>  		return ret;
> @@ -960,9 +1001,8 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  	nand_set_flash_node(nand, pdev->dev.of_node);
>  
>  	mtd->dev.parent = &pdev->dev;
> -	nand->IO_ADDR_R = host->data_va;
> -	nand->IO_ADDR_W = host->data_va;
> -	nand->cmd_ctrl = fsmc_cmd_ctrl;
> +	nand->exec_op = fsmc_exec_op;
> +	nand->select_chip = fsmc_select_chip;
>  	nand->chip_delay = 30;
>  
>  	/*
> @@ -974,8 +1014,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  	nand->ecc.size = 512;
>  	nand->badblockbits = 7;
>  
> -	switch (host->mode) {
> -	case USE_DMA_ACCESS:
> +	if (host->mode == USE_DMA_ACCESS) {
>  		dma_cap_zero(mask);
>  		dma_cap_set(DMA_MEMCPY, mask);
>  		host->read_dma_chan = dma_request_channel(mask, filter, NULL);
> @@ -988,15 +1027,6 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  			dev_err(&pdev->dev, "Unable to get write dma channel\n");
>  			goto err_req_write_chnl;
>  		}
> -		nand->read_buf = fsmc_read_buf_dma;
> -		nand->write_buf = fsmc_write_buf_dma;
> -		break;
> -
> -	default:
> -	case USE_WORD_ACCESS:
> -		nand->read_buf = fsmc_read_buf;
> -		nand->write_buf = fsmc_write_buf;
> -		break;
>  	}
>  
>  	if (host->dev_timings)

[PATCH 2/3] mtd: nand: add the infrastructure to retrieve the ONFI unique ID

[Miquel Raynal]

Add the infrastructure in the NAND core to retrieve the ONFI unique ID.
The controller driver must support ->exec_op(), the NAND chip must be
ONFI and support the feature.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/rawnand.h  | 19 +++++++++-
 2 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 44574a226980..0bbc1fc04e01 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -5418,6 +5418,85 @@ static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type)
 }
 
 /*
+ * Read the ONFI unique ID and write it to the given *dest pointer.
+ * The ID is 16 bytes wide, repeated with its XORed counterpart, 16 times.
+ *
+ * This function does not select/unselect the CS line.
+ *
+ * Returns 0 if the ID was read a written to dest, an error otherwise.
+ */
+static int nand_read_unique_id(struct nand_chip *chip, char *dest)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 id[ONFI_UNIQUEID_LEN * 2];
+	int string_len = ONFI_FULL_UNIQUEID_STRING_LEN;
+	int ret, i, j, pos;
+
+	/* ->exec_op related definitions */
+	const struct nand_sdr_timings *sdr =
+		nand_get_sdr_timings(&chip->data_interface);
+	u8 addr = 0;
+	struct nand_op_instr instrs[] = {
+		NAND_OP_CMD(NAND_CMD_READ_UNIQUEID, 0),
+		NAND_OP_ADDR(1, &addr, PSEC_TO_NSEC(sdr->tWB_max)),
+		NAND_OP_WAIT_RDY(PSEC_TO_MSEC(sdr->tR_max),
+				 PSEC_TO_NSEC(sdr->tRR_min)),
+	};
+	struct nand_operation op = NAND_OPERATION(instrs);
+
+	if (!chip->exec_op)
+		return -ENOTSUPP;
+
+	if (!dest)
+		return -EINVAL;
+
+	if (!(onfi_opt_cmd(chip) & ONFI_OPT_CMD_READ_UNIQUEID))
+		return -ENOTSUPP;
+
+	ret = nand_exec_op(chip, &op);
+	if (ret)
+		return ret;
+
+	/* Pattern is repeated 16 times */
+	for (i = 0; i < ONFI_UNIQUEID_REPETITIONS; i++) {
+		/* Each pattern is 32B wide (the ID + the ID XORed) */
+		if (chip->exec_op) {
+			struct nand_op_instr instrs[] = {
+				NAND_OP_8BIT_DATA_IN(sizeof(id), id, 0),
+			};
+			struct nand_operation op = NAND_OPERATION(instrs);
+
+			ret = nand_exec_op(chip, &op);
+			if (ret)
+				return ret;
+		} else {
+			chip->read_buf(mtd, id, sizeof(id));
+		}
+
+		/* The ID (16B) must be checked with its XORed counterpart */
+		for (j = 0; j < ONFI_UNIQUEID_LEN; j++)
+			if ((id[j] ^ id[j + ONFI_UNIQUEID_LEN]) != 0xFF)
+				break;
+
+		/* ID is correct if the inner 'for' loop went until the end */
+		if (j == ONFI_UNIQUEID_LEN)
+			break;
+	}
+
+	/* A successful read would have break'ed the outer 'for' loop */
+	if (i == ONFI_UNIQUEID_REPETITIONS)
+		return -EINVAL;
+
+	pos = snprintf(dest, string_len, "%02x-", chip->id.data[0]);
+	for (i = 0; i < ONFI_UNIQUEID_LEN; i++)
+		pos += snprintf(dest + pos, string_len - pos, "%02x", id[i]);
+
+	pr_info("chip has unique ID: %s\n", dest);
+
+	return 0;
+}
+
+/*
  * Set the bad block marker/indicator (BBM/BBI) patterns according to some
  * heuristic patterns using various detected parameters (e.g., manufacturer,
  * page size, cell-type information).
@@ -6289,6 +6368,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	struct nand_buffers *nbuf = NULL;
+	char unique_id[ONFI_FULL_UNIQUEID_STRING_LEN];
 	int ret, i;
 
 	/* New bad blocks should be marked in OOB, flash-based BBT, or both */
@@ -6585,6 +6665,11 @@ int nand_scan_tail(struct mtd_info *mtd)
 	mtd->_max_bad_blocks = nand_max_bad_blocks;
 	mtd->writebufsize = mtd->writesize;
 
+	/* Read the unique ID from the first die if available */
+	chip->select_chip(mtd, 0);
+	nand_read_unique_id(chip, unique_id);
+	chip->select_chip(mtd, -1);
+
 	/*
 	 * Initialize bitflip_threshold to its default prior scan_bbt() call.
 	 * scan_bbt() might invoke mtd_read(), thus bitflip_threshold must be
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index a5d73919cddb..9e584c95bc3e 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -85,6 +85,7 @@ void nand_wait_ready(struct mtd_info *mtd);
 #define NAND_CMD_READSTART	0x30
 #define NAND_CMD_RNDOUTSTART	0xE0
 #define NAND_CMD_CACHEDPROG	0x15
+#define NAND_CMD_READ_UNIQUEID	0xED
 
 #define NAND_CMD_NONE		-1
 
@@ -251,8 +252,18 @@ struct nand_chip;
 /* ONFI subfeature parameters length */
 #define ONFI_SUBFEATURE_PARAM_LEN	4
 
-/* ONFI optional commands SET/GET FEATURES supported? */
+/* ONFI optional commands supported */
 #define ONFI_OPT_CMD_SET_GET_FEATURES	(1 << 2)
+#define ONFI_OPT_CMD_READ_UNIQUEID	(1 << 5)
+
+/*
+ * ONFI unique ID length and number of repetitions. The full unique ID is the
+ * manufacturer ID (1B) plus the unique device ID (16B). Also count the '-'
+ * between both IDs and the '\0' at the end in the 'STRING_LEN'.
+ */
+#define ONFI_UNIQUEID_LEN		16
+#define ONFI_UNIQUEID_REPETITIONS	16
+#define ONFI_FULL_UNIQUEID_STRING_LEN	((1 + ONFI_UNIQUEID_LEN) * 2 + 2)
 
 struct nand_onfi_params {
 	/* rev info and features block */
@@ -1556,6 +1567,12 @@ static inline int onfi_feature(struct nand_chip *chip)
 	return chip->onfi_version ? le16_to_cpu(chip->onfi_params.features) : 0;
 }
 
+/* return the supported optional commands */
+static inline int onfi_opt_cmd(struct nand_chip *chip)
+{
+	return chip->onfi_version ? le16_to_cpu(chip->onfi_params.opt_cmd) : 0;
+}
+
 /* return the supported asynchronous timing mode. */
 static inline int onfi_get_async_timing_mode(struct nand_chip *chip)
 {
-- 
2.11.0

Re: [PATCH 2/3] mtd: nand: add the infrastructure to retrieve the ONFI unique ID

[Miquel RAYNAL]

Hello,

In the cover letter I forgot to give a link to the series this work is
based on:
http://patchwork.ozlabs.org/project/linux-mtd/list/?series=12308

I also spotted a mistake there:

> +static int nand_read_unique_id(struct nand_chip *chip, char *dest)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	u8 id[ONFI_UNIQUEID_LEN * 2];
> +	int string_len = ONFI_FULL_UNIQUEID_STRING_LEN;
> +	int ret, i, j, pos;
> +
> +	/* ->exec_op related definitions */
> +	const struct nand_sdr_timings *sdr =
> +		nand_get_sdr_timings(&chip->data_interface);
> +	u8 addr = 0;
> +	struct nand_op_instr instrs[] = {
> +		NAND_OP_CMD(NAND_CMD_READ_UNIQUEID, 0),
> +		NAND_OP_ADDR(1, &addr, PSEC_TO_NSEC(sdr->tWB_max)),
> +		NAND_OP_WAIT_RDY(PSEC_TO_MSEC(sdr->tR_max),
> +				 PSEC_TO_NSEC(sdr->tRR_min)),
> +	};
> +	struct nand_operation op = NAND_OPERATION(instrs);
> +
> +	if (!chip->exec_op)
> +		return -ENOTSUPP;
> +
> +	if (!dest)
> +		return -EINVAL;
> +
> +	if (!(onfi_opt_cmd(chip) & ONFI_OPT_CMD_READ_UNIQUEID))
> +		return -ENOTSUPP;
> +
> +	ret = nand_exec_op(chip, &op);
> +	if (ret)
> +		return ret;
> +
> +	/* Pattern is repeated 16 times */
> +	for (i = 0; i < ONFI_UNIQUEID_REPETITIONS; i++) {
> +		/* Each pattern is 32B wide (the ID + the ID XORed)
> */
> +		if (chip->exec_op) {
> +			struct nand_op_instr instrs[] = {
> +				NAND_OP_8BIT_DATA_IN(sizeof(id), id,
> 0),
> +			};
> +			struct nand_operation op =
> NAND_OPERATION(instrs); +
> +			ret = nand_exec_op(chip, &op);
> +			if (ret)
> +				return ret;
> +		} else {
> +			chip->read_buf(mtd, id, sizeof(id));
> +		}

No need for that 'else' statement as this function will not run without
->exec_op() support.


Sorry about that, I will correct it in a later version.
Miquèl

[PATCH 3/3] mtd: nand: add sysfs entry for NAND chip unique ID

[Miquel Raynal]

ONFI NAND chips may support the unique ID feature which is, once
concatenated with the NAND manufacturer ID, a real unique ID.

It has been tried to make the NAND framework declare a "nand" bus type,
with NAND chips being the bus devices and having their particular
unique ID. But in this configuration, the chips could not be attached
to the MTD class anymore.

No easy solution was doable in a reasonable amount of time, so instead
it has been chosen to use the mtd_info structure to store the ID and
turn it visible only if filled by the NAND framework.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
---
 drivers/mtd/mtdcore.c        | 33 ++++++++++++++++++++++++++++++++-
 drivers/mtd/nand/nand_base.c | 26 +++++++++++++++++++-------
 include/linux/mtd/mtd.h      |  1 +
 3 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index e7ea842ba3db..d43090348295 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -319,6 +319,15 @@ static ssize_t mtd_bbtblocks_show(struct device *dev,
 }
 static DEVICE_ATTR(bbt_blocks, S_IRUGO, mtd_bbtblocks_show, NULL);
 
+static ssize_t nand_unique_id_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%s\n", mtd->nand_unique_id);
+}
+static DEVICE_ATTR_RO(nand_unique_id);
+
 static struct attribute *mtd_attrs[] = {
 	&dev_attr_type.attr,
 	&dev_attr_flags.attr,
@@ -336,9 +345,31 @@ static struct attribute *mtd_attrs[] = {
 	&dev_attr_bad_blocks.attr,
 	&dev_attr_bbt_blocks.attr,
 	&dev_attr_bitflip_threshold.attr,
+	&dev_attr_nand_unique_id.attr,
+	NULL,
+};
+
+static umode_t mtd_attr_is_visible(struct kobject *kobj,
+				   struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+
+	if (attr == &dev_attr_nand_unique_id.attr && !mtd->nand_unique_id)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group mtd_group = {
+	.attrs		= mtd_attrs,
+	.is_visible	= mtd_attr_is_visible,
+};
+
+static const struct attribute_group *mtd_groups[] = {
+	&mtd_group,
 	NULL,
 };
-ATTRIBUTE_GROUPS(mtd);
 
 static const struct device_type mtd_devtype = {
 	.name		= "mtd",
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 0bbc1fc04e01..e5ea3b159551 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -6368,7 +6368,6 @@ int nand_scan_tail(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	struct nand_buffers *nbuf = NULL;
-	char unique_id[ONFI_FULL_UNIQUEID_STRING_LEN];
 	int ret, i;
 
 	/* New bad blocks should be marked in OOB, flash-based BBT, or both */
@@ -6666,9 +6665,15 @@ int nand_scan_tail(struct mtd_info *mtd)
 	mtd->writebufsize = mtd->writesize;
 
 	/* Read the unique ID from the first die if available */
-	chip->select_chip(mtd, 0);
-	nand_read_unique_id(chip, unique_id);
-	chip->select_chip(mtd, -1);
+	mtd->nand_unique_id = kmalloc(ONFI_FULL_UNIQUEID_STRING_LEN,
+				      GFP_KERNEL);
+	if (mtd->nand_unique_id) {
+		chip->select_chip(mtd, 0);
+		ret = nand_read_unique_id(chip, mtd->nand_unique_id);
+		chip->select_chip(mtd, -1);
+		if (ret)
+			kfree(mtd->nand_unique_id);
+	}
 
 	/*
 	 * Initialize bitflip_threshold to its default prior scan_bbt() call.
@@ -6681,7 +6686,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 	/* Initialize the ->data_interface field. */
 	ret = nand_init_data_interface(chip);
 	if (ret)
-		goto err_nand_manuf_cleanup;
+		goto err_free_unique_id;
 
 	/* Enter fastest possible mode on all dies. */
 	for (i = 0; i < chip->numchips; i++) {
@@ -6690,7 +6695,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 		chip->select_chip(mtd, -1);
 
 		if (ret)
-			goto err_nand_manuf_cleanup;
+			goto err_free_unique_id;
 	}
 
 	/* Check, if we should skip the bad block table scan */
@@ -6700,10 +6705,12 @@ int nand_scan_tail(struct mtd_info *mtd)
 	/* Build bad block table */
 	ret = chip->scan_bbt(mtd);
 	if (ret)
-		goto err_nand_manuf_cleanup;
+		goto err_free_unique_id;
 
 	return 0;
 
+err_free_unique_id:
+	kfree(mtd->nand_unique_id);
 
 err_nand_manuf_cleanup:
 	nand_manufacturer_cleanup(chip);
@@ -6758,10 +6765,15 @@ EXPORT_SYMBOL(nand_scan);
  */
 void nand_cleanup(struct nand_chip *chip)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
 	if (chip->ecc.mode == NAND_ECC_SOFT &&
 	    chip->ecc.algo == NAND_ECC_BCH)
 		nand_bch_free((struct nand_bch_control *)chip->ecc.priv);
 
+	/* Free unique ID if existing */
+	kfree(mtd->nand_unique_id);
+
 	/* Free bad block table memory */
 	kfree(chip->bbt);
 	if (!(chip->options & NAND_OWN_BUFFERS) && chip->buffers) {
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 6cd0f6b7658b..817eecd08fb5 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -269,6 +269,7 @@ struct mtd_info {
 
 	// Kernel-only stuff starts here.
 	const char *name;
+	u8 *nand_unique_id;
 	int index;
 
 	/* OOB layout description */
-- 
2.11.0

Re: [PATCH 3/3] mtd: nand: add sysfs entry for NAND chip unique ID

[Boris Brezillon]

On Tue, 14 Nov 2017 16:46:22 +0100
Miquel Raynal <miquel.raynal@free-electrons.com> wrote:

> ONFI NAND chips may support the unique ID feature which is, once
> concatenated with the NAND manufacturer ID, a real unique ID.
> 
> It has been tried to make the NAND framework declare a "nand" bus type,
> with NAND chips being the bus devices and having their particular
> unique ID. But in this configuration, the chips could not be attached
> to the MTD class anymore.
> 
> No easy solution was doable in a reasonable amount of time, so instead
> it has been chosen to use the mtd_info structure to store the ID and
> turn it visible only if filled by the NAND framework.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> ---
>  drivers/mtd/mtdcore.c        | 33 ++++++++++++++++++++++++++++++++-
>  drivers/mtd/nand/nand_base.c | 26 +++++++++++++++++++-------
>  include/linux/mtd/mtd.h      |  1 +
>  3 files changed, 52 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index e7ea842ba3db..d43090348295 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -319,6 +319,15 @@ static ssize_t mtd_bbtblocks_show(struct device *dev,
>  }
>  static DEVICE_ATTR(bbt_blocks, S_IRUGO, mtd_bbtblocks_show, NULL);
>  
> +static ssize_t nand_unique_id_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct mtd_info *mtd = dev_get_drvdata(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%s\n", mtd->nand_unique_id);
> +}
> +static DEVICE_ATTR_RO(nand_unique_id);
> +
>  static struct attribute *mtd_attrs[] = {
>  	&dev_attr_type.attr,
>  	&dev_attr_flags.attr,
> @@ -336,9 +345,31 @@ static struct attribute *mtd_attrs[] = {
>  	&dev_attr_bad_blocks.attr,
>  	&dev_attr_bbt_blocks.attr,
>  	&dev_attr_bitflip_threshold.attr,
> +	&dev_attr_nand_unique_id.attr,
> +	NULL,
> +};
> +
> +static umode_t mtd_attr_is_visible(struct kobject *kobj,
> +				   struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct mtd_info *mtd = dev_get_drvdata(dev);
> +
> +	if (attr == &dev_attr_nand_unique_id.attr && !mtd->nand_unique_id)
> +		return 0;
> +
> +	return attr->mode;
> +}
> +
> +static const struct attribute_group mtd_group = {
> +	.attrs		= mtd_attrs,
> +	.is_visible	= mtd_attr_is_visible,
> +};
> +
> +static const struct attribute_group *mtd_groups[] = {
> +	&mtd_group,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(mtd);
>  
>  static const struct device_type mtd_devtype = {
>  	.name		= "mtd",
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 0bbc1fc04e01..e5ea3b159551 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -6368,7 +6368,6 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	struct nand_buffers *nbuf = NULL;
> -	char unique_id[ONFI_FULL_UNIQUEID_STRING_LEN];
>  	int ret, i;
>  
>  	/* New bad blocks should be marked in OOB, flash-based BBT, or both */
> @@ -6666,9 +6665,15 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	mtd->writebufsize = mtd->writesize;
>  
>  	/* Read the unique ID from the first die if available */
> -	chip->select_chip(mtd, 0);
> -	nand_read_unique_id(chip, unique_id);
> -	chip->select_chip(mtd, -1);
> +	mtd->nand_unique_id = kmalloc(ONFI_FULL_UNIQUEID_STRING_LEN,
> +				      GFP_KERNEL);
> +	if (mtd->nand_unique_id) {
> +		chip->select_chip(mtd, 0);
> +		ret = nand_read_unique_id(chip, mtd->nand_unique_id);
> +		chip->select_chip(mtd, -1);
> +		if (ret)
> +			kfree(mtd->nand_unique_id);
> +	}
>  
>  	/*
>  	 * Initialize bitflip_threshold to its default prior scan_bbt() call.
> @@ -6681,7 +6686,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	/* Initialize the ->data_interface field. */
>  	ret = nand_init_data_interface(chip);
>  	if (ret)
> -		goto err_nand_manuf_cleanup;
> +		goto err_free_unique_id;
>  
>  	/* Enter fastest possible mode on all dies. */
>  	for (i = 0; i < chip->numchips; i++) {
> @@ -6690,7 +6695,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>  		chip->select_chip(mtd, -1);
>  
>  		if (ret)
> -			goto err_nand_manuf_cleanup;
> +			goto err_free_unique_id;
>  	}
>  
>  	/* Check, if we should skip the bad block table scan */
> @@ -6700,10 +6705,12 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	/* Build bad block table */
>  	ret = chip->scan_bbt(mtd);
>  	if (ret)
> -		goto err_nand_manuf_cleanup;
> +		goto err_free_unique_id;
>  
>  	return 0;
>  
> +err_free_unique_id:
> +	kfree(mtd->nand_unique_id);
>  
>  err_nand_manuf_cleanup:
>  	nand_manufacturer_cleanup(chip);
> @@ -6758,10 +6765,15 @@ EXPORT_SYMBOL(nand_scan);
>   */
>  void nand_cleanup(struct nand_chip *chip)
>  {
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +
>  	if (chip->ecc.mode == NAND_ECC_SOFT &&
>  	    chip->ecc.algo == NAND_ECC_BCH)
>  		nand_bch_free((struct nand_bch_control *)chip->ecc.priv);
>  
> +	/* Free unique ID if existing */
> +	kfree(mtd->nand_unique_id);
> +
>  	/* Free bad block table memory */
>  	kfree(chip->bbt);
>  	if (!(chip->options & NAND_OWN_BUFFERS) && chip->buffers) {
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 6cd0f6b7658b..817eecd08fb5 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -269,6 +269,7 @@ struct mtd_info {
>  
>  	// Kernel-only stuff starts here.
>  	const char *name;
> +	u8 *nand_unique_id;

Should be "const char *" since this is a printable string that is not
supposed to be modified.

MTD folks (Richard, Marek, Brian, Cyrille), I'd like a feedback on this
patch.

I personally don't like the idea of putting NAND-specific (in this case
it's even more specific than NAND because it's only applicable to
ONFI-NANDs that support the READ_UNIQUE_ID command) stuff into mtd_info,
but there's currently no easy way to create NAND-specific sysfs entry.

We could add a nand_register() wrapper that would call
mtd_device_register() and add sysfs/debugfs entries after that, but
that implies patching all NAND drivers (again).

Any idea how we could make it less invasive?

Also, is there any other flash device that expose some sort of
unique-id, and if there is, what's the format?

Regards,

Boris