Re: [PATCH v1 1/5] mtd: rawnand: meson: fix NAND access for read/write
From: Arseniy Krasnov
Date: Wed Apr 12 2023 - 06:28:17 EST
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))
>
> 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);
>