Re: [PATCH v1 1/5] mtd: rawnand: meson: fix NAND access for read/write
From: Liang Yang
Date: Thu Apr 13 2023 - 01:58:33 EST
Hi Arseniy,
On 2023/4/13 13:10, Arseniy Krasnov wrote:
[ EXTERNAL EMAIL ]
On 12.04.2023 16:30, Liang Yang wrote:
Hi,
On 2023/4/12 20:03, Arseniy Krasnov wrote:
[ EXTERNAL EMAIL ]
On 12.04.2023 13:24, Arseniy Krasnov wrote:
On 12.04.2023 12:37, Liang Yang wrote:
Hi Arseniy,
Thanks for pointing out this problem. also comment inline.
On 2023/4/12 14:16, Arseniy Krasnov wrote:
[ EXTERNAL EMAIL ]
This fixes read/write functionality. New command sequences were ported
from old vendor's driver. Without this patch driver works unstable. This
change is tested with 'nanddump'/'nandwrite' utilities and mounting
JFFS2 filesystem on AXG family (A113X SoC).
Fixes: 8fae856c5350 ("mtd: rawnand: meson: add support for Amlogic NAND flash controller")
Signed-off-by: Arseniy Krasnov <AVKrasnov@xxxxxxxxxxxxxx>
---
drivers/mtd/nand/raw/meson_nand.c | 116 ++++++++++++++++++++++++++----
1 file changed, 101 insertions(+), 15 deletions(-)
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index 074e14225c06..256c37c76526 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -26,6 +26,7 @@
#define NFC_CMD_IDLE (0xc << 14)
#define NFC_CMD_CLE (0x5 << 14)
#define NFC_CMD_ALE (0x6 << 14)
+#define NFC_CMD_DRD (0x8 << 14)
#define NFC_CMD_ADL ((0 << 16) | (3 << 20))
#define NFC_CMD_ADH ((1 << 16) | (3 << 20))
#define NFC_CMD_AIL ((2 << 16) | (3 << 20))
@@ -84,6 +85,7 @@
#define DMA_BUSY_TIMEOUT 0x100000
#define CMD_FIFO_EMPTY_TIMEOUT 1000
+#define DEVICE_READY_TIMEOUT 1000
#define MAX_CE_NUM 2
@@ -255,8 +257,26 @@ static void meson_nfc_select_chip(struct nand_chip *nand, int chip)
}
}
+static int meson_nfc_wait_cmd_finish(struct meson_nfc *nfc,
+ unsigned int timeout_ms)
+{
+ u32 cmd_size = 0;
+ int ret;
+
+ /* wait cmd fifo is empty */
+ ret = readl_relaxed_poll_timeout(nfc->reg_base + NFC_REG_CMD, cmd_size,
+ !NFC_CMD_GET_SIZE(cmd_size),
+ 10, timeout_ms * 1000);
+ if (ret)
+ dev_err(nfc->dev, "wait for empty CMD FIFO timed out\n");
+
+ return ret;
+}
+
static void meson_nfc_cmd_idle(struct meson_nfc *nfc, u32 time)
{
+ meson_nfc_wait_cmd_finish(nfc, 0);
+
writel(nfc->param.chip_select | NFC_CMD_IDLE | (time & 0x3ff),
nfc->reg_base + NFC_REG_CMD);
}
@@ -308,23 +328,9 @@ static void meson_nfc_drain_cmd(struct meson_nfc *nfc)
*/
meson_nfc_cmd_idle(nfc, 0);
meson_nfc_cmd_idle(nfc, 0);
+ meson_nfc_wait_cmd_finish(nfc, 1000);
}
-static int meson_nfc_wait_cmd_finish(struct meson_nfc *nfc,
- unsigned int timeout_ms)
-{
- u32 cmd_size = 0;
- int ret;
-
- /* wait cmd fifo is empty */
- ret = readl_relaxed_poll_timeout(nfc->reg_base + NFC_REG_CMD, cmd_size,
- !NFC_CMD_GET_SIZE(cmd_size),
- 10, timeout_ms * 1000);
- if (ret)
- dev_err(nfc->dev, "wait for empty CMD FIFO time out\n");
-
- return ret;
-}
static int meson_nfc_wait_dma_finish(struct meson_nfc *nfc)
{
@@ -631,6 +637,48 @@ static int meson_nfc_rw_cmd_prepare_and_execute(struct nand_chip *nand,
return 0;
}
+static uint8_t meson_nfc_read_byte(struct nand_chip *nand)
+{
+ struct meson_nfc *nfc = nand_get_controller_data(nand);
+
+ writel(NFC_CMD_DRD, nfc->reg_base + NFC_REG_CMD);
+ meson_nfc_cmd_idle(nfc, nfc->timing.twb);
+ meson_nfc_drain_cmd(nfc);
+
+ return readl(nfc->reg_base + NFC_REG_BUF);
+}
+
+static int meson_nfc_wait_dev_ready(struct nand_chip *nand)
+{
+ struct meson_nfc *nfc = nand_get_controller_data(nand);
+ u32 cs = nfc->param.chip_select;
+ unsigned long cnt = 0;
+
+ meson_nfc_drain_cmd(nfc);
+
+ writel(cs | NFC_CMD_CLE | NAND_CMD_STATUS, nfc->reg_base + NFC_REG_CMD);
+
+ /* 10 ms. */
+ while (cnt < DEVICE_READY_TIMEOUT) {
+ uint8_t status;
+
+ status = meson_nfc_read_byte(nand);
+
+ if (status & NAND_STATUS_READY)
+ break;
+
+ usleep_range(10, 11);
+ cnt++;
+ }
+
+ if (cnt == DEVICE_READY_TIMEOUT) {
+ dev_err(nfc->dev, "device ready timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
static int meson_nfc_write_page_sub(struct nand_chip *nand,
int page, int raw)
{
@@ -643,6 +691,10 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
u32 cmd;
int ret;
+ ret = meson_nfc_wait_dev_ready(nand);
+ if (ret)
+ return ret;
+
meson_nfc_select_chip(nand, nand->cur_cs);
data_len = mtd->writesize + mtd->oobsize;
@@ -667,12 +719,20 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
NFC_CMD_SCRAMBLER_DISABLE);
}
+ ret = meson_nfc_wait_dma_finish(nfc);
+ if (ret)
+ return ret;
+
cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
writel(cmd, nfc->reg_base + NFC_REG_CMD);
meson_nfc_queue_rb(nfc, PSEC_TO_MSEC(sdr->tPROG_max));
meson_nfc_dma_buffer_release(nand, data_len, info_len, DMA_TO_DEVICE);
+ ret = meson_nfc_wait_dev_ready(nand);
+ if (ret)
+ return ret;
+
return ret;
}
@@ -720,6 +780,21 @@ static void meson_nfc_check_ecc_pages_valid(struct meson_nfc *nfc,
} while (!ret);
}
+static inline int meson_nfc_send_read(struct nand_chip *nand)
+{
+ struct meson_nfc *nfc = nand_get_controller_data(nand);
+ u32 cs = nfc->param.chip_select;
+ int ret;
+
+ ret = meson_nfc_wait_dev_ready(nand);
+ if (ret)
+ return ret;
+
+ writel(cs | NFC_CMD_CLE | NAND_CMD_READ0, nfc->reg_base + NFC_REG_CMD);
+
+ return 0;
+}
+
it already calls meson_nfc_queue_rb() in meson_nfc_rw_cmd_prepare_and_execute(). Could you implements this in meson_nfc_queue_rb()? and we can use the irq method.
also without Ready/Busy pin, the meson_nfc_queue_rb() should change like below:
......
#define NFC_CMD_RB_INT ((0xb << 10) | BIT(18))
Sorry, I can see this define as (and it is used in the driver):
#define NFC_CMD_RB_INT BIT(14)
in drivers/mtd/nand/raw/meson_nand.c
Which one is correct ?
we need to modify the define 'NFC_CMD_RB_INT' as ((0xb << 10) | BIT(18)).
Ok, NFC_CMD_RB_INT - it is "Ready/Busy_Interrupt" ? You suppose that currently it is
defined incorrectly, so irq waiting does not work?
Previous defined BIT(14) is for having the external Ready/Busy pin of
the NAND device connected to the controller. the new define is for
reading status by sending read status(70H) command and read status from
the data bus(checking the IO6). the both can work on AXG soc.
when the controller RB command is sent, the controller automatically
checks the level of external Ready/Busy pin or the data bus(IO6)
periodicity. and generate the irq signal if status is ready.
Thanks, Arseniy
Thanks, Arseniy
meson_nfc_cmd_idle(nfc, 0);
cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_STATUS;
writel(cmd, nfc->reg_base + NFC_REG_CMD);
meson_nfc_cmd_idle(nfc, 5);
cmd = NFC_CMD_RB | NFC_CMD_RB_INT | nfc->timing.tbers_max;
writel(cmd, nfc->reg_base + NFC_REG_CMD);
ret = wait_for_completion_timeout(&nfc->completion,
msecs_to_jiffies(timeout_ms));
if (ret == 0)
ret = -1;
writel(cs | NFC_CMD_CLE | NAND_CMD_READ0, nfc->reg_base + NFC_REG_CMD);
......
Thanks for reply! I'll try this code! One more question about OOB processing in this
driver (as You are author of it):
OOB size is 64 bytes, but for example if I have 1K ECC, 2 bytes user bytes and 14
bytes for ECC code for each 1K. In this case I have access to only 32 bytes of OOB:
2 x (2 user bytes + 14 ECC bytes). Correct me if i'm wrong, but rest of OOB (next
32 bytes) become unavailable (in both raw and ECC modes) ?
Thanks, Arseniy
static int meson_nfc_read_page_sub(struct nand_chip *nand,
int page, int raw)
{
@@ -734,10 +809,18 @@ static int meson_nfc_read_page_sub(struct nand_chip *nand,
data_len = mtd->writesize + mtd->oobsize;
info_len = nand->ecc.steps * PER_INFO_BYTE;
+ ret = meson_nfc_wait_dev_ready(nand);
+ if (ret)
+ return ret;
+
ret = meson_nfc_rw_cmd_prepare_and_execute(nand, page, DIRREAD);
if (ret)
return ret;
+ ret = meson_nfc_send_read(nand);
+ if (ret)
+ return ret;
+
ret = meson_nfc_dma_buffer_setup(nand, meson_chip->data_buf,
data_len, meson_chip->info_buf,
info_len, DMA_FROM_DEVICE);
@@ -754,6 +837,9 @@ static int meson_nfc_read_page_sub(struct nand_chip *nand,
}
ret = meson_nfc_wait_dma_finish(nfc);
+ if (ret)
+ return ret;
+
meson_nfc_check_ecc_pages_valid(nfc, nand, raw);
meson_nfc_dma_buffer_release(nand, data_len, info_len, DMA_FROM_DEVICE);
--
Thanks,
Liang