Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On 2018/10/19 16:42, Boris Brezillon wrote:
On Fri, 19 Oct 2018 16:29:40 +0800
Liang Yang wrote:
On 2018/10/19 4:50, Boris Brezillon wrote:
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
+static int meson_nfc_buffer_init(struct mtd_info *mtd)
+{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ struct meson_nfc *nfc = nand_get_controller_data(nand);
+ static int max_page_bytes, max_info_bytes;
+ int page_bytes, info_bytes;
+ int nsectors;
+
+ nsectors = mtd->writesize / nand->ecc.size;
+ page_bytes = mtd->writesize + mtd->oobsize;
+ info_bytes = nsectors * PER_INFO_BYTE;
+
+ if (nfc->data_buf && nfc->info_buf) {
+ if (max_page_bytes < page_bytes)
+ meson_nfc_free_buffer(nfc);
+ else
+ return 0;
+ }
+
+ max_page_bytes = max_t(int, max_page_bytes, page_bytes);
+ max_info_bytes = max_t(int, max_info_bytes, info_bytes);
+
+ nfc->data_buf = kmalloc(max_page_bytes, GFP_KERNEL);
Is there a good reason for not using chip->data_buf and allocating a
new buffer here?
when calling read_oob or write_oob, i need a mid-buffer which is used in
meson_nfc_prase_data_oob(). i.e. after reading a page data into
nfc->data_buf, I will format it and transfer to chip->data_buf.
+ if (!nfc->data_buf)
+ return -ENOMEM;
+
+ nfc->info_buf = kmalloc(max_info_bytes, GFP_KERNEL);
+ if (!nfc->info_buf) {
+ kfree(nfc->data_buf);
+ return -ENOMEM;
+ }
I'd recommend moving this info_buf in the priv chip struct, otherwise
you'll have to protect nfc->info_buf with a lock to prevent an already
register chip from using this pointer while you're reallocating the
buffer. Also, I think you have a memleak here.
ok, i will move it in the chip struct .
but how memleak happens even i have called meson_nfc_free_buffer before
and free data_buf if info_buf alloc faied.
You're right, I missed the call to meson_nfc_free_buffer() when
max_page_bytes < page_bytes. Still, this part is racy, just like the
nfc->data_buf replacement is if you have several NAND chips. I'd
recommend moving those bufs to the priv chip struct.
ok. i will move data_duf and info_buf to priv chip struct.
+
+ return 0;
+}
.
.
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On Fri, 19 Oct 2018 16:29:40 +0800
Liang Yang wrote:
> On 2018/10/19 4:50, Boris Brezillon wrote:
> > On Thu, 18 Oct 2018 13:09:05 +0800
> > Jianxin Pan wrote:
> >
> >> +static int meson_nfc_buffer_init(struct mtd_info *mtd)
> >> +{
> >> + struct nand_chip *nand = mtd_to_nand(mtd);
> >> + struct meson_nfc *nfc = nand_get_controller_data(nand);
> >> + static int max_page_bytes, max_info_bytes;
> >> + int page_bytes, info_bytes;
> >> + int nsectors;
> >> +
> >> + nsectors = mtd->writesize / nand->ecc.size;
> >> + page_bytes = mtd->writesize + mtd->oobsize;
> >> + info_bytes = nsectors * PER_INFO_BYTE;
> >> +
> >> + if (nfc->data_buf && nfc->info_buf) {
> >> + if (max_page_bytes < page_bytes)
> >> + meson_nfc_free_buffer(nfc);
> >> + else
> >> + return 0;
> >> + }
> >> +
> >> + max_page_bytes = max_t(int, max_page_bytes, page_bytes);
> >> + max_info_bytes = max_t(int, max_info_bytes, info_bytes);
> >> +
> >> + nfc->data_buf = kmalloc(max_page_bytes, GFP_KERNEL);
> >
> > Is there a good reason for not using chip->data_buf and allocating a
> > new buffer here?
> >
> when calling read_oob or write_oob, i need a mid-buffer which is used in
> meson_nfc_prase_data_oob(). i.e. after reading a page data into
> nfc->data_buf, I will format it and transfer to chip->data_buf.
>
>
> >> + if (!nfc->data_buf)
> >> + return -ENOMEM;
> >> +
> >> + nfc->info_buf = kmalloc(max_info_bytes, GFP_KERNEL);
> >> + if (!nfc->info_buf) {
> >> + kfree(nfc->data_buf);
> >> + return -ENOMEM;
> >> + }
> >
> > I'd recommend moving this info_buf in the priv chip struct, otherwise
> > you'll have to protect nfc->info_buf with a lock to prevent an already
> > register chip from using this pointer while you're reallocating the
> > buffer. Also, I think you have a memleak here.
> >
> ok, i will move it in the chip struct .
>
> but how memleak happens even i have called meson_nfc_free_buffer before
> and free data_buf if info_buf alloc faied.
You're right, I missed the call to meson_nfc_free_buffer() when
max_page_bytes < page_bytes. Still, this part is racy, just like the
nfc->data_buf replacement is if you have several NAND chips. I'd
recommend moving those bufs to the priv chip struct.
>
> >> +
> >> + return 0;
> >> +}
> >
> > .
> >
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On 2018/10/19 16:10, Boris Brezillon wrote:
On Fri, 19 Oct 2018 15:29:05 +0800
Liang Yang wrote:
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_COMPLETEBIT(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.
Absolutely.
+
+#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;
..
}
Why do you need to add a new field then? Just test
nand->options & NAND_NEED_SCRAMBLING directly or provide a helper
function that does that.
ok, i will fix it.
.
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On 2018/10/19 4:50, Boris Brezillon wrote:
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
+static int meson_nfc_buffer_init(struct mtd_info *mtd)
+{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ struct meson_nfc *nfc = nand_get_controller_data(nand);
+ static int max_page_bytes, max_info_bytes;
+ int page_bytes, info_bytes;
+ int nsectors;
+
+ nsectors = mtd->writesize / nand->ecc.size;
+ page_bytes = mtd->writesize + mtd->oobsize;
+ info_bytes = nsectors * PER_INFO_BYTE;
+
+ if (nfc->data_buf && nfc->info_buf) {
+ if (max_page_bytes < page_bytes)
+ meson_nfc_free_buffer(nfc);
+ else
+ return 0;
+ }
+
+ max_page_bytes = max_t(int, max_page_bytes, page_bytes);
+ max_info_bytes = max_t(int, max_info_bytes, info_bytes);
+
+ nfc->data_buf = kmalloc(max_page_bytes, GFP_KERNEL);
Is there a good reason for not using chip->data_buf and allocating a
new buffer here?
when calling read_oob or write_oob, i need a mid-buffer which is used in
meson_nfc_prase_data_oob(). i.e. after reading a page data into
nfc->data_buf, I will format it and transfer to chip->data_buf.
+ if (!nfc->data_buf)
+ return -ENOMEM;
+
+ nfc->info_buf = kmalloc(max_info_bytes, GFP_KERNEL);
+ if (!nfc->info_buf) {
+ kfree(nfc->data_buf);
+ return -ENOMEM;
+ }
I'd recommend moving this info_buf in the priv chip struct, otherwise
you'll have to protect nfc->info_buf with a lock to prevent an already
register chip from using this pointer while you're reallocating the
buffer. Also, I think you have a memleak here.
ok, i will move it in the chip struct .
but how memleak happens even i have called meson_nfc_free_buffer before
and free data_buf if info_buf alloc faied.
+
+ return 0;
+}
.
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On Fri, 19 Oct 2018 15:29:05 +0800
Liang Yang wrote:
> > 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_COMPLETEBIT(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.
Absolutely.
>
>
>
> >> +
> >> +#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;
> ..
> }
Why do you need to add a new field then? Just test
nand->options & NAND_NEED_SCRAMBLING directly or provide a helper
function that does that.
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On 2018/10/19 4:39, Boris Brezillon wrote:
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
+static int meson_nfc_calc_set_timing(struct meson_nfc *nfc,
+const struct nand_sdr_timings *timings)
+{
+ struct nand_timing *timing = &nfc->timing;
+ int div, bt_min, bt_max, bus_timing;
+ int ret;
+
+ div = DIV_ROUND_UP((timings->tRC_min / 1000), NFC_CLK_CYCLE);
+ ret = clk_set_rate(nfc->device_clk, 10 / div);
+ if (ret) {
+ dev_err(nfc->dev, "failed to set nand clock rate\n");
+ return ret;
+ }
+
+ timing->twb = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tWB_max),
+ div * NFC_CLK_CYCLE);
+ timing->tadl = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tADL_min),
+ div * NFC_CLK_CYCLE);
+ timing->twhr = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tWHR_min),
+ div * NFC_CLK_CYCLE);
+
+ bt_min = (timings->tREA_max + NFC_DEFAULT_DELAY) / div;
+ bt_max = (NFC_DEFAULT_DELAY + timings->tRHOH_min
+ + timings->tRC_min / 2) / div;
+
+ bt_min = DIV_ROUND_UP(bt_min, 1000);
+ bt_max = DIV_ROUND_UP(bt_max, 1000);
+
+ if (bt_max < bt_min)
+ return -EINVAL;
+
+ bus_timing = (bt_min + bt_max) / 2 + 1;
+
+ writel((1 << 21), nfc->reg_base + NFC_REG_CFG);
+ writel((NFC_CLK_CYCLE - 1) | (bus_timing << 5),
+ nfc->reg_base + NFC_REG_CFG);
+
+ writel((1 << 31), nfc->reg_base + NFC_REG_CMD);
+
+ return 0;
+}
+
+static int
+meson_nfc_setup_data_interface(struct nand_chip *nand, int csline,
+ const struct nand_data_interface *conf)
+{
+ struct meson_nfc *nfc = nand_get_controller_data(nand);
+ const struct nand_sdr_timings *timings;
+
+ timings = nand_get_sdr_timings(conf);
+ if (IS_ERR(timings))
+ return -ENOTSUPP;
+
+ if (csline == NAND_DATA_IFACE_CHECK_ONLY)
+ return 0;
Hm, before saying you supporting the requested timing, you should make
sure they are actually supported. I'd recommend splitting this in 2
steps:
1/ calc timings
2/ store the timings in the chip priv struct so that they can be
applied next time ->select_chip() is called.
ok, i will try to split.
+
+ meson_nfc_calc_set_timing(nfc, timings);
> You should not set the timing from ->setup_data_interface(), just
calculate them, make sure they are supported and store the state in the
private chip struct. Applying those timings should be done in
->select_chip(), so that you can support 2 chips with different timings.
em, i will fix it.
+ return 0;
+}
.
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On 2018/10/19 3:33, Boris Brezillon wrote: On Thu, 18 Oct 2018 13:09:05 +0800 Jianxin Pan wrote: From: Liang Yang Add initial support for the Amlogic NAND flash controller which found in the Meson-GXBB/GXL/AXG SoCs. Signed-off-by: Liang Yang Signed-off-by: Yixun Lan Signed-off-by: Jianxin Pan --- 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 000..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 + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define NFC_REG_CMD0x00 +#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_CFG0x04 +#define NFC_REG_DADR 0x08 +#define NFC_REG_IADR 0x0c +#define NFC_REG_BUF0x10 +#define NFC_REG_INFO 0x14 +#define NFC_REG_DC 0x18 +#define NFC_REG_ADR0x1c +#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_VER0x38 + +#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) & 0x)) +#define GENCMDDADDRH(adh, addr)((adh) | (((addr) >> 16) & 0x)) +#define GENCMDIADDRL(ail, addr)((ail) | ((addr) & 0x)) +#define GENCMDIADDRH(aih, addr)((aih) | (((addr) >> 16) & 0x)) + +#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 0x10 +#define CMD_FIFO_EMPTY_TIMEOUT 1000 + +#define
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On 2018/10/18 22:24, Boris Brezillon wrote:
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
+static int meson_nfc_exec_op(struct nand_chip *chip,
+const struct nand_operation *op, bool check_only)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct meson_nfc *nfc = nand_get_controller_data(chip);
+ const struct nand_op_instr *instr = NULL;
+ int ret = 0, cmd;
+ unsigned int op_id;
+ int i;
+
+ for (op_id = 0; op_id < op->ninstrs; op_id++) {
+ instr = &op->instrs[op_id];
+ switch (instr->type) {
+ case NAND_OP_CMD_INSTR:
+ if (instr->ctx.cmd.opcode == NAND_CMD_STATUS)
+ meson_nfc_cmd_idle(nfc, nfc->timing.twb);
Hm, I don't want drivers to base their decisions on the opcode value.
There's a ->delay_ns field in the instruction object, can't you use
that one instead? Also, I don't understand why this is only needed for
the STATUS command. It should normally be applied to all instructions.
em, it should be applied to all instructions.
i will fix it and use instr->delay_ns instead.
+ cmd = nfc->param.chip_select | NFC_CMD_CLE;
+ cmd |= instr->ctx.cmd.opcode & 0xff;
+ writel(cmd, nfc->reg_base + NFC_REG_CMD);
+ if (instr->ctx.cmd.opcode == NAND_CMD_STATUS)
+ meson_nfc_cmd_idle(nfc, nfc->timing.twhr);
+ break;
+
+ case NAND_OP_ADDR_INSTR:
+ for (i = 0; i < instr->ctx.addr.naddrs; i++) {
+ cmd = nfc->param.chip_select | NFC_CMD_ALE;
+ cmd |= instr->ctx.addr.addrs[i] & 0xff;
+ writel(cmd, nfc->reg_base + NFC_REG_CMD);
+ }
+ break;
+
+ case NAND_OP_DATA_IN_INSTR:
+ meson_nfc_read_buf(mtd, instr->ctx.data.buf.in,
+ instr->ctx.data.len);
+ break;
+
+ case NAND_OP_DATA_OUT_INSTR:
+ meson_nfc_write_buf(mtd, instr->ctx.data.buf.out,
+ instr->ctx.data.len);
+ break;
+
+ case NAND_OP_WAITRDY_INSTR:
+ meson_nfc_queue_rb(nfc, instr->ctx.waitrdy.timeout_ms);
+ break;
+ }
+ }
+ return ret;
+}
.
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
> +static int meson_nfc_buffer_init(struct mtd_info *mtd)
> +{
> + struct nand_chip *nand = mtd_to_nand(mtd);
> + struct meson_nfc *nfc = nand_get_controller_data(nand);
> + static int max_page_bytes, max_info_bytes;
> + int page_bytes, info_bytes;
> + int nsectors;
> +
> + nsectors = mtd->writesize / nand->ecc.size;
> + page_bytes = mtd->writesize + mtd->oobsize;
> + info_bytes = nsectors * PER_INFO_BYTE;
> +
> + if (nfc->data_buf && nfc->info_buf) {
> + if (max_page_bytes < page_bytes)
> + meson_nfc_free_buffer(nfc);
> + else
> + return 0;
> + }
> +
> + max_page_bytes = max_t(int, max_page_bytes, page_bytes);
> + max_info_bytes = max_t(int, max_info_bytes, info_bytes);
> +
> + nfc->data_buf = kmalloc(max_page_bytes, GFP_KERNEL);
Is there a good reason for not using chip->data_buf and allocating a
new buffer here?
> + if (!nfc->data_buf)
> + return -ENOMEM;
> +
> + nfc->info_buf = kmalloc(max_info_bytes, GFP_KERNEL);
> + if (!nfc->info_buf) {
> + kfree(nfc->data_buf);
> + return -ENOMEM;
> + }
I'd recommend moving this info_buf in the priv chip struct, otherwise
you'll have to protect nfc->info_buf with a lock to prevent an already
register chip from using this pointer while you're reallocating the
buffer. Also, I think you have a memleak here.
> +
> + return 0;
> +}
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
> +static int meson_nfc_calc_set_timing(struct meson_nfc *nfc,
> + const struct nand_sdr_timings *timings)
> +{
> + struct nand_timing *timing = &nfc->timing;
> + int div, bt_min, bt_max, bus_timing;
> + int ret;
> +
> + div = DIV_ROUND_UP((timings->tRC_min / 1000), NFC_CLK_CYCLE);
> + ret = clk_set_rate(nfc->device_clk, 10 / div);
> + if (ret) {
> + dev_err(nfc->dev, "failed to set nand clock rate\n");
> + return ret;
> + }
> +
> + timing->twb = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tWB_max),
> +div * NFC_CLK_CYCLE);
> + timing->tadl = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tADL_min),
> + div * NFC_CLK_CYCLE);
> + timing->twhr = DIV_ROUND_UP(PSEC_TO_NSEC(timings->tWHR_min),
> + div * NFC_CLK_CYCLE);
> +
> + bt_min = (timings->tREA_max + NFC_DEFAULT_DELAY) / div;
> + bt_max = (NFC_DEFAULT_DELAY + timings->tRHOH_min
> + + timings->tRC_min / 2) / div;
> +
> + bt_min = DIV_ROUND_UP(bt_min, 1000);
> + bt_max = DIV_ROUND_UP(bt_max, 1000);
> +
> + if (bt_max < bt_min)
> + return -EINVAL;
> +
> + bus_timing = (bt_min + bt_max) / 2 + 1;
> +
> + writel((1 << 21), nfc->reg_base + NFC_REG_CFG);
> + writel((NFC_CLK_CYCLE - 1) | (bus_timing << 5),
> +nfc->reg_base + NFC_REG_CFG);
> +
> + writel((1 << 31), nfc->reg_base + NFC_REG_CMD);
> +
> + return 0;
> +}
> +
> +static int
> +meson_nfc_setup_data_interface(struct nand_chip *nand, int csline,
> +const struct nand_data_interface *conf)
> +{
> + struct meson_nfc *nfc = nand_get_controller_data(nand);
> + const struct nand_sdr_timings *timings;
> +
> + timings = nand_get_sdr_timings(conf);
> + if (IS_ERR(timings))
> + return -ENOTSUPP;
> +
> + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> + return 0;
Hm, before saying you supporting the requested timing, you should make
sure they are actually supported. I'd recommend splitting this in 2
steps:
1/ calc timings
2/ store the timings in the chip priv struct so that they can be
applied next time ->select_chip() is called.
> +
> + meson_nfc_calc_set_timing(nfc, timings);
You should not set the timing from ->setup_data_interface(), just
calculate them, make sure they are supported and store the state in the
private chip struct. Applying those timings should be done in
->select_chip(), so that you can support 2 chips with different timings.
> + return 0;
> +}
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
> +static int meson_nand_bch_mode(struct nand_chip *nand)
> +{
> + struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> + struct meson_nand_ecc meson_ecc[] = {
> + MESON_ECC_DATA(NFC_ECC_BCH8_1K, 8),
> + MESON_ECC_DATA(NFC_ECC_BCH24_1K, 24),
> + MESON_ECC_DATA(NFC_ECC_BCH30_1K, 30),
> + MESON_ECC_DATA(NFC_ECC_BCH40_1K, 40),
> + MESON_ECC_DATA(NFC_ECC_BCH50_1K, 50),
> + MESON_ECC_DATA(NFC_ECC_BCH60_1K, 60),
> + };
> + int i, ret = 0;
> +
> + if (nand->ecc.strength > 60 || nand->ecc.strength < 8)
> + return -EINVAL;
> +
> + for (i = 0; i < sizeof(meson_ecc); i++) {
> + if (meson_ecc[i].strength == nand->ecc.strength) {
> + meson_chip->bch_mode = meson_ecc[i].bch;
> + break;
return 0;
> + }
> + }
> +
> + return ret;
return -EINVAL;
> +}
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On Thu, 18 Oct 2018 13:09:05 +0800 Jianxin Pan wrote: > From: Liang Yang > > Add initial support for the Amlogic NAND flash controller which found > in the Meson-GXBB/GXL/AXG SoCs. > > Signed-off-by: Liang Yang > Signed-off-by: Yixun Lan > Signed-off-by: Jianxin Pan > --- > 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 000..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 > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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_ENBIT(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) & 0x)) > +#define GENCMDDADDRH(adh, addr) ((adh) | (((addr) >> 16) & > 0x)) > +#define GENCMDIADDRL(ail, addr) ((ail) | ((addr) & 0x)) > +#define GENCMDIADDRH(aih, addr) ((aih) | (((addr) >> 16) & > 0x)) > + > +#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) >
Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
On Thu, 18 Oct 2018 13:09:05 +0800
Jianxin Pan wrote:
> +static int meson_nfc_exec_op(struct nand_chip *chip,
> + const struct nand_operation *op, bool check_only)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct meson_nfc *nfc = nand_get_controller_data(chip);
> + const struct nand_op_instr *instr = NULL;
> + int ret = 0, cmd;
> + unsigned int op_id;
> + int i;
> +
> + for (op_id = 0; op_id < op->ninstrs; op_id++) {
> + instr = &op->instrs[op_id];
> + switch (instr->type) {
> + case NAND_OP_CMD_INSTR:
> + if (instr->ctx.cmd.opcode == NAND_CMD_STATUS)
> + meson_nfc_cmd_idle(nfc, nfc->timing.twb);
Hm, I don't want drivers to base their decisions on the opcode value.
There's a ->delay_ns field in the instruction object, can't you use
that one instead? Also, I don't understand why this is only needed for
the STATUS command. It should normally be applied to all instructions.
> + cmd = nfc->param.chip_select | NFC_CMD_CLE;
> + cmd |= instr->ctx.cmd.opcode & 0xff;
> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
> + if (instr->ctx.cmd.opcode == NAND_CMD_STATUS)
> + meson_nfc_cmd_idle(nfc, nfc->timing.twhr);
> + break;
> +
> + case NAND_OP_ADDR_INSTR:
> + for (i = 0; i < instr->ctx.addr.naddrs; i++) {
> + cmd = nfc->param.chip_select | NFC_CMD_ALE;
> + cmd |= instr->ctx.addr.addrs[i] & 0xff;
> + writel(cmd, nfc->reg_base + NFC_REG_CMD);
> + }
> + break;
> +
> + case NAND_OP_DATA_IN_INSTR:
> + meson_nfc_read_buf(mtd, instr->ctx.data.buf.in,
> +instr->ctx.data.len);
> + break;
> +
> + case NAND_OP_DATA_OUT_INSTR:
> + meson_nfc_write_buf(mtd, instr->ctx.data.buf.out,
> + instr->ctx.data.len);
> + break;
> +
> + case NAND_OP_WAITRDY_INSTR:
> + meson_nfc_queue_rb(nfc, instr->ctx.waitrdy.timeout_ms);
> + break;
> + }
> + }
> + return ret;
> +}
[PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
From: Liang Yang Add initial support for the Amlogic NAND flash controller which found in the Meson-GXBB/GXL/AXG SoCs. Signed-off-by: Liang Yang Signed-off-by: Yixun Lan Signed-off-by: Jianxin Pan --- 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 000..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 + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define NFC_REG_CMD0x00 +#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_CFG0x04 +#define NFC_REG_DADR 0x08 +#define NFC_REG_IADR 0x0c +#define NFC_REG_BUF0x10 +#define NFC_REG_INFO 0x14 +#define NFC_REG_DC 0x18 +#define NFC_REG_ADR0x1c +#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_VER0x38 + +#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) & 0x)) +#define GENCMDDADDRH(adh, addr)((adh) | (((addr) >> 16) & 0x)) +#define GENCMDIADDRL(ail, addr)((ail) | ((addr) & 0x)) +#define GENCMDIADDRH(aih, addr)((aih) | (((addr) >> 16) & 0x)) + +#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 0x10 +#define CMD_FIFO_EMPTY_TIMEOUT 1000 + +#define MAX_CE_NUM 2 + +/* eMMC clock register, misc control */ +#define SD_EMMC_CLOCK 0x00 +#define CL
