Messages in this thread | | | Date | Thu, 13 Apr 2023 08:10:31 +0300 | Subject | Re: [PATCH v1 1/5] mtd: rawnand: meson: fix NAND access for read/write | From | Arseniy Krasnov <> |
| |
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@sberdevices.ru> >>>>> --- >>>>> 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?
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); >>>> >> >
| |