Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
From: Liang Yang
Date: Fri Oct 19 2018 - 03:29:28 EST
On 2018/10/19 3:33, Boris Brezillon wrote:
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan <jianxin.pan@xxxxxxxxxxx> wrote:
From: Liang Yang <liang.yang@xxxxxxxxxxx>
Add initial support for the Amlogic NAND flash controller which found
in the Meson-GXBB/GXL/AXG SoCs.
Signed-off-by: Liang Yang <liang.yang@xxxxxxxxxxx>
Signed-off-by: Yixun Lan <yixun.lan@xxxxxxxxxxx>
Signed-off-by: Jianxin Pan <jianxin.pan@xxxxxxxxxxx>
---
drivers/mtd/nand/raw/Kconfig | 10 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/meson_nand.c | 1370 +++++++++++++++++++++++++++++++++++++
3 files changed, 1381 insertions(+)
create mode 100644 drivers/mtd/nand/raw/meson_nand.c
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index c7efc31..223b041 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -541,4 +541,14 @@ config MTD_NAND_TEGRA
is supported. Extra OOB bytes when using HW ECC are currently
not supported.
+config MTD_NAND_MESON
+ tristate "Support for NAND controller on Amlogic's Meson SoCs"
+ depends on ARCH_MESON || COMPILE_TEST
+ depends on COMMON_CLK_AMLOGIC
+ select COMMON_CLK_REGMAP_MESON
+ select MFD_SYSCON
+ help
+ Enables support for NAND controller on Amlogic's Meson SoCs.
+ This controller is found on Meson GXBB, GXL, AXG SoCs.
+
endif # MTD_NAND
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 57159b3..a2cc2fe 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/
obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o
obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o
obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o
+obj-$(CONFIG_MTD_NAND_MESON) += meson_nand.o
nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o
nand-objs += nand_onfi.o
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
new file mode 100644
index 0000000..bcaac53
--- /dev/null
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -0,0 +1,1370 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Amlogic Meson Nand Flash Controller Driver
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Liang Yang <liang.yang@xxxxxxxxxxx>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/clk.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/iopoll.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#define NFC_REG_CMD 0x00
+#define NFC_CMD_DRD (0x8 << 14)
+#define NFC_CMD_IDLE (0xc << 14)
+#define NFC_CMD_DWR (0x4 << 14)
+#define NFC_CMD_CLE (0x5 << 14)
+#define NFC_CMD_ALE (0x6 << 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))
+#define NFC_CMD_AIH ((3 << 16) | (3 << 20))
+#define NFC_CMD_SEED ((8 << 16) | (3 << 20))
+#define NFC_CMD_M2N ((0 << 17) | (2 << 20))
+#define NFC_CMD_N2M ((1 << 17) | (2 << 20))
+#define NFC_CMD_RB BIT(20)
+#define NFC_CMD_IO6 ((0xb << 10) | (1 << 18))
+
+#define NFC_REG_CFG 0x04
+#define NFC_REG_DADR 0x08
+#define NFC_REG_IADR 0x0c
+#define NFC_REG_BUF 0x10
+#define NFC_REG_INFO 0x14
+#define NFC_REG_DC 0x18
+#define NFC_REG_ADR 0x1c
+#define NFC_REG_DL 0x20
+#define NFC_REG_DH 0x24
+#define NFC_REG_CADR 0x28
+#define NFC_REG_SADR 0x2c
+#define NFC_REG_PINS 0x30
+#define NFC_REG_VER 0x38
+
+#define NFC_RB_IRQ_EN BIT(21)
+#define NFC_INT_MASK (3 << 20)
+
+#define CMDRWGEN(cmd_dir, ran, bch, short_mode, page_size, pages) \
+ ( \
+ (cmd_dir) | \
+ ((ran) << 19) | \
+ ((bch) << 14) | \
+ ((short_mode) << 13) | \
+ (((page_size) & 0x7f) << 6) | \
+ ((pages) & 0x3f) \
+ )
+
+#define GENCMDDADDRL(adl, addr) ((adl) | ((addr) & 0xffff))
+#define GENCMDDADDRH(adh, addr) ((adh) | (((addr) >> 16) & 0xffff))
+#define GENCMDIADDRL(ail, addr) ((ail) | ((addr) & 0xffff))
+#define GENCMDIADDRH(aih, addr) ((aih) | (((addr) >> 16) & 0xffff))
+
+#define RB_STA(x) (1 << (26 + (x)))
+#define DMA_DIR(dir) ((dir) ? NFC_CMD_N2M : NFC_CMD_M2N)
+
+#define ECC_CHECK_RETURN_FF (-1)
+
+#define NAND_CE0 (0xe << 10)
+#define NAND_CE1 (0xd << 10)
+
+#define DMA_BUSY_TIMEOUT 0x100000
+#define CMD_FIFO_EMPTY_TIMEOUT 1000
+
+#define MAX_CE_NUM 2
+
+/* eMMC clock register, misc control */
+#define SD_EMMC_CLOCK 0x00
+#define CLK_ALWAYS_ON BIT(28)
+#define CLK_SELECT_NAND BIT(31)
+#define CLK_DIV_MASK GENMASK(5, 0)
+
+#define NFC_CLK_CYCLE 6
+
+/* nand flash controller delay 3 ns */
+#define NFC_DEFAULT_DELAY 3000
+
+#define MAX_ECC_INDEX 10
+
+#define MUX_CLK_NUM_PARENTS 2
+
+#define ROW_ADDER(page, index) (((page) >> (8 * (index))) & 0xff)
+#define MAX_CYCLE_ROW_ADDRS 3
+#define MAX_CYCLE_COLUMN_ADDRS 2
+#define DIRREAD 1
+#define DIRWRITE 0
+
+#define ECC_PARITY_BCH8_512B 14
+
+struct meson_nfc_info_format {
+ u16 info_bytes;
+
+ /* bit[5:0] are valid */
+ u8 zero_cnt;
+ struct ecc_sta {
+ u8 eccerr_cnt : 6;
+ u8 notused : 1;
+ u8 completed : 1;
+ } ecc;
It's usually a bad idea to use bitfields for things like HW
regs/descriptors fields because it's hard to tell where the bits are
actually placed (not even sure the C standard defines how this should be
done).
ok.
+ u32 reserved;
+};
How about defining that the HW returns an array of __le64 instead and then
define the following macros which you can use after converting in the
CPU endianness
#define ECC_GET_PROTECTED_OOB_BYTE(x, y) (((x) >> (8 * (1 + y)) & GENMASK(7, 0))
#define ECC_COMPLETE BIT(31)
#define ECC_ERR_CNT(x) (((x) >> 24) & GENMASK(5, 0))
(I'm not entirely sure the field positions are correct, but I'll let you
check that).
ok. i think it should be:
#define ECC_GET_PROTECTED_OOB_BYTE(x, y) (((x) >> (8 * y) &
GENMASK(7, 0))
if x represents the u64 and y represents the index of the u64.
+
+#define PER_INFO_BYTE (sizeof(struct meson_nfc_info_format))
+
+struct meson_nfc_nand_chip {
+ struct list_head node;
+ struct nand_chip nand;
+ bool is_scramble;
I think I already mentioned the NAND_NEED_SCRAMBLING flag []. Please
drop this field and test (chip->flags & NAND_NEED_SCRAMBLING) instead.
em, i use NAND_NEED_SCRAMBLING and is_scramble is set:
static int meson_nand_attach_chip(struct nand_chip *nand)
{
......
meson_chip->is_scramble =
(nand->options & NAND_NEED_SCRAMBLING) ? 1 : 0;
......
}
+ int bch_mode;
+ int nsels;
+ u8 sels[0];
+};
+
+struct meson_nand_ecc {
+ int bch;
+ int strength;
+};
+
+struct meson_nfc_data {
+ const struct nand_ecc_caps *ecc_caps;
+};
+
+struct meson_nfc_param {
+ int chip_select;
+ int rb_select;
+};
+
+struct nand_rw_cmd {
+ int cmd0;
+ int col[MAX_CYCLE_COLUMN_ADDRS];
+ int row[MAX_CYCLE_ROW_ADDRS];
+ int cmd1;
+};
+
+struct nand_timing {
+ int twb;
+ int tadl;
+ int twhr;
+};
+
+struct meson_nfc {
+ struct nand_controller controller;
+ struct clk *core_clk;
+ struct clk *device_clk;
+ struct clk *phase_tx;
+ struct clk *phase_rx;
+
+ struct device *dev;
+ void __iomem *reg_base;
+ struct regmap *reg_clk;
+ struct completion completion;
+ struct list_head chips;
+ const struct meson_nfc_data *data;
+ struct meson_nfc_param param;
+ struct nand_timing timing;
+ union {
+ int cmd[32];
+ struct nand_rw_cmd rw;
+ } cmdfifo;
+
+ dma_addr_t data_dma;
+ dma_addr_t info_dma;
+
+ unsigned long assigned_cs;
+
+ u8 *data_buf;
+ u8 *info_buf;
+};
+
+enum {
+ NFC_ECC_BCH8_1K = 2,
+ NFC_ECC_BCH24_1K,
+ NFC_ECC_BCH30_1K,
+ NFC_ECC_BCH40_1K,
+ NFC_ECC_BCH50_1K,
+ NFC_ECC_BCH60_1K,
+};
+
+#define MESON_ECC_DATA(b, s) { .bch = (b), .strength = (s)}
+
+static int meson_nand_calc_ecc_bytes(int step_size, int strength)
+{
+ int ecc_bytes;
+
+ if (step_size == 512 && strength == 8)
+ return ECC_PARITY_BCH8_512B;
+
+ ecc_bytes = DIV_ROUND_UP(strength * fls(step_size * 8), 8);
+ if (ecc_bytes % 2)
+ ecc_bytes++;
Just a tiny detail, but this can be replaced by ALIGN(ecc_bytes, 2).
ok
+
+ return ecc_bytes;
+}
+
+NAND_ECC_CAPS_SINGLE(meson_gxl_ecc_caps,
+ meson_nand_calc_ecc_bytes, 1024, 8, 24, 30, 40, 50, 60);
+NAND_ECC_CAPS_SINGLE(meson_axg_ecc_caps,
+ meson_nand_calc_ecc_bytes, 1024, 8);
+
+static inline
+struct meson_nfc_nand_chip *to_meson_nand(struct nand_chip *nand)
+{
+ return container_of(nand, struct meson_nfc_nand_chip, nand);
+}
+
+static void meson_nfc_select_chip(struct nand_chip *nand, int chip)
+{
+ struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
+ struct meson_nfc *nfc = nand_get_controller_data(nand);
+
+ if (chip < 0 || chip > MAX_CE_NUM)
You can add a WARN_ON_ONCE() around chip > MAX_CE_NUM, because this should
never happen.
ok
+ return;
+
+ nfc->param.chip_select = meson_chip->sels[chip] ? NAND_CE1 : NAND_CE0;
+ nfc->param.rb_select = nfc->param.chip_select;
+}
+
+static inline void meson_nfc_cmd_idle(struct meson_nfc *nfc, u32 time)
+{
+ writel(nfc->param.chip_select | NFC_CMD_IDLE | (time & 0x3ff),
+ nfc->reg_base + NFC_REG_CMD);
+}
+
+static inline void meson_nfc_cmd_seed(struct meson_nfc *nfc, u32 seed)
+{
+ writel(NFC_CMD_SEED | (0xc2 + (seed & 0x7fff)),
+ nfc->reg_base + NFC_REG_CMD);
+}
+
+static void meson_nfc_cmd_access(struct meson_nfc *nfc,
+ struct mtd_info *mtd, int raw, bool dir)
Just pass a nand_chip or a meson_nfc_nand_chip object here, nfc and mtd
can be extracted from there.
ok
+{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
+ u32 cmd, pagesize, pages;
+ int bch = meson_chip->bch_mode;
+ int len = mtd->writesize;
+ int scramble = meson_chip->is_scramble ? 1 : 0;
+
+ pagesize = nand->ecc.size;
+
+ if (raw) {
+ bch = NAND_ECC_NONE;
You set bch to NAND_ECC_NONE but never use the variable afterwards, is
that normal?
ok, i will fix it. it is not used.
+ len = mtd->writesize + mtd->oobsize;
+ cmd = (len & 0x3fff) | (scramble << 19) | DMA_DIR(dir);
Please use a macro to describe bit 19:
#define NFC_CMD_SCRAMBLER_ENABLE BIT(19)
ok
+ writel(cmd, nfc->reg_base + NFC_REG_CMD);
+ return;
+ }
+
+ pages = len / nand->ecc.size;
+
+ cmd = CMDRWGEN(DMA_DIR(dir), scramble, bch, 0, pagesize, pages);
+
+ writel(cmd, nfc->reg_base + NFC_REG_CMD);
+}
+
+static inline void meson_nfc_drain_cmd(struct meson_nfc *nfc)
+{
+ /*
+ * Insert two commands to make sure all valid commands are finished.
+ *
+ * The Nand flash controller is designed as two stages pipleline -
+ * a) fetch and b) excute.
+ * There might be cases when the driver see command queue is empty,
+ * but the Nand flash controller still has two commands buffered,
+ * one is fetched into NFC request queue (ready to run), and another
+ * is actively executing. So pushing 2 "IDLE" commands guarantees that
+ * the pipeline is emptied.
+ */
+ meson_nfc_cmd_idle(nfc, 0);
+ meson_nfc_cmd_idle(nfc, 0);
+}
+
+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_poll_timeout(nfc->reg_base + NFC_REG_CMD, cmd_size,
+ !((cmd_size >> 22) & 0x1f),
Define a macro to extract the cmd_size:
#define NFC_CMD_GET_SIZE(x) (((x) >> 22) & GENMASK(4, 0))
ok
+ 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)
+{
+ meson_nfc_drain_cmd(nfc);
+
+ return meson_nfc_wait_cmd_finish(nfc, DMA_BUSY_TIMEOUT);
+}
+
+static inline
+struct meson_nfc_info_format *nfc_info_ptr(struct meson_nfc *nfc,
+ int index)
+{
+ return (struct meson_nfc_info_format *)&nfc->info_buf[index * 8];
As said previously, I think ->info_buf should be an array of __le64, and
all you should do here is
return le64_to_cpu(nfc->info_buf[index]);
Then you can use the macros defined above to extract the results you
need.
ok
+}
+
+static u8 *meson_nfc_oob_ptr(struct meson_nfc *nfc,
+ struct mtd_info *mtd, int i)
+{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ int len;
+
+ len = nand->ecc.size * (i + 1) + (nand->ecc.bytes + 2) * i;
+
+ return nfc->data_buf + len;
+}
+
+static u8 *meson_nfc_data_ptr(struct meson_nfc *nfc,
+ struct mtd_info *mtd, int i)
+{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ int len;
+ int temp;
+
+ temp = nand->ecc.size + nand->ecc.bytes;
+ len = (temp + 2) * i;
+
+ return nfc->data_buf + len;
+}
+
+static void meson_nfc_prase_data_oob(struct meson_nfc *nfc,
What does prase mean? Maybe _set_ and _get_ would be better that _prase_
and _format_.
+ struct mtd_info *mtd, u8 *buf, u8 *oobbuf)
As said previously, please stop passing mtd objects around. Same goes
for nfc if you already have to pass a nand_chip object.
ok
+{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ int i, oob_len = 0;
+ u8 *dsrc, *osrc;
+
+ for (i = 0; i < mtd->writesize / nand->ecc.size; i++) {
You have ecc->steps for that, no need to do the division everytime.
ok, it will fix it.
+ if (buf) {
+ dsrc = meson_nfc_data_ptr(nfc, mtd, i);
+ memcpy(buf, dsrc, nand->ecc.size);
+ buf += nand->ecc.size;
+ }
+ oob_len = nand->ecc.bytes + 2;
Looks like oob_len does not change, so you can move the oob_len
assignment outside of this loop.
em.i will fix it.
+ osrc = meson_nfc_oob_ptr(nfc, mtd, i);
+ memcpy(oobbuf, osrc, oob_len);
+ oobbuf += oob_len;
+ }
+}
+
+static void meson_nfc_format_data_oob(struct meson_nfc *nfc,
+ struct mtd_info *mtd,
+ const u8 *buf, u8 *oobbuf)
+{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ int i, oob_len = 0;
+ u8 *dsrc, *osrc;
+
+ for (i = 0; i < mtd->writesize / nand->ecc.size; i++) {
+ if (buf) {
+ dsrc = meson_nfc_data_ptr(nfc, mtd, i);
+ memcpy(dsrc, buf, nand->ecc.size);
+ buf += nand->ecc.size;
+ }
+ oob_len = nand->ecc.bytes + 2;
+ osrc = meson_nfc_oob_ptr(nfc, mtd, i);
+ memcpy(osrc, oobbuf, oob_len);
+ oobbuf += oob_len;
+ }
+}
+
+static int meson_nfc_queue_rb(struct meson_nfc *nfc, int timeout_ms)
+{
+ u32 cmd, cfg;
+ int ret = 0;
+
+ meson_nfc_cmd_idle(nfc, nfc->timing.twb);
+ meson_nfc_drain_cmd(nfc);
+ meson_nfc_wait_cmd_finish(nfc, CMD_FIFO_EMPTY_TIMEOUT);
+
+ cfg = readl(nfc->reg_base + NFC_REG_CFG);
+ cfg |= (1 << 21);
+ writel(cfg, nfc->reg_base + NFC_REG_CFG);
+
+ init_completion(&nfc->completion);
+
+ cmd = NFC_CMD_RB | (0x1 << 14) | nfc->param.chip_select | 0x18;
Please define macros for all those magic values (0x18 and 1 << 14)
ok
+ writel(cmd, nfc->reg_base + NFC_REG_CMD);
+
+ ret = wait_for_completion_timeout(&nfc->completion,
+ msecs_to_jiffies(timeout_ms));
+ if (ret == 0) {
+ dev_err(nfc->dev, "wait nand irq timeout\n");
+ ret = -1;
-ETIMEDOUT;
+ }
+ return ret;
+}
+
+static void meson_nfc_set_user_byte(struct mtd_info *mtd,
+ struct nand_chip *chip, u8 *oob_buf)
+{
+ struct meson_nfc *nfc = nand_get_controller_data(chip);
+ struct meson_nfc_info_format *info;
+ int i, count;
+
+ for (i = 0, count = 0; i < chip->ecc.steps; i++, count += 2) {
+ info = nfc_info_ptr(nfc, i);
+ info->info_bytes =
+ oob_buf[count] | (oob_buf[count + 1] << 8);
Use a macro to set/get protected OOB bytes.
ok
+ }
+}
+
.