Subject: [PATCH v1 0/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

This patch adds the new IP of NAND Flash Controller support on Intel's
Lightning Mountain(LGM) SoC.

DMA is used for burst data transfer operation, also DMA HW supports
aligned 32bit memory address and aligned data access by default.
DMA burst of 8 supported,Data register used to support the read/write
operation from/to device.

The NAND controller Subsystem consisting of LGM + ECC-NAND controller supports:
NAND Flash Features :
-16/8-bit data bus
Page + OOB Bytes
- 512 + (2 - 13) bytes per page page
- 2048 + (7 - 210) bytes per page
- 4096 + (13 - 420) bytes per page
- 8192 + (26 - 840) bytes per page

- Support 32/64/128/256/512/1024/2048/4096/8192/16384/32768 Mbytes flash device
- ECC calculation/generation and verification on-the-fly

Ramuthevar Vadivel Murugan (2):
dt-bindings: mtd: Add YAML for Nand Flash Controller support
mtd: rawnand: Add NAND controller support on Intel LGM SoC

.../devicetree/bindings/mtd/intel,lgm-nand.yaml | 61 ++
drivers/mtd/nand/raw/Kconfig | 7 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/intel_lgm_nand.c | 678 +++++++++++++++++++++
4 files changed, 747 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
create mode 100644 drivers/mtd/nand/raw/intel_lgm_nand.c

--
2.11.0


Subject: [PATCH v1 1/2] dt-bindings: mtd: Add YAML for Nand Flash Controller support

From: Ramuthevar Vadivel Murugan <[email protected]>

Add YAML file for dt-bindings to support NAND Flash Controller
on Intel's Lightning Mountain SoC.

Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
---
.../devicetree/bindings/mtd/intel,lgm-nand.yaml | 61 ++++++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
new file mode 100644
index 000000000000..361e5051c602
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/intel,lgm-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel LGM SoC NAND Controller Device Tree Bindings
+
+allOf:
+ - $ref: "nand-controller.yaml"
+
+maintainers:
+ - Ramuthevar Vadivel Murugan <[email protected]>
+
+properties:
+ compatible:
+ const: intel,lgm-nand
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ dmas:
+ maxItems: 2
+
+ dma-names:
+ enum:
+ - rx
+ - tx
+
+ pinctrl-names: true
+
+patternProperties:
+ "^pinctrl-[0-9]+$": true
+
+ "^nand@[a-f0-9]+$":
+ type: object
+ properties:
+ reg:
+ minimum: 0
+ maximum: 7
+
+ nand-ecc-mode: true
+
+ nand-ecc-algo:
+ const: hw
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - dmas
+
+additionalProperties: false
+
+...
--
2.11.0

Subject: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

From: Ramuthevar Vadivel Murugan <[email protected]>

This patch adds the new IP of Nand Flash Controller(NFC) support
on Intel's Lightning Mountain(LGM) SoC.

DMA is used for burst data transfer operation, also DMA HW supports
aligned 32bit memory address and aligned data access by default.
DMA burst of 8 supported. Data register used to support the read/write
operation from/to device.

Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
---
drivers/mtd/nand/raw/Kconfig | 7 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/intel_lgm_nand.c | 678 ++++++++++++++++++++++++++++++++++
3 files changed, 686 insertions(+)
create mode 100644 drivers/mtd/nand/raw/intel_lgm_nand.c

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index a80a46bb5b8b..9efc4bbaf4a3 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -457,6 +457,13 @@ config MTD_NAND_CADENCE
Enable the driver for NAND flash on platforms using a Cadence NAND
controller.

+config MTD_NAND_INTEL_LGM
+ tristate "Support for NAND controller on Intel LGM SoC"
+ depends on X86
+ help
+ Enables support for NAND Flash chips on Intel's LGM SoC.
+ NAND flash interfaced through the External Bus Unit.
+
comment "Misc"

config MTD_SM_COMMON
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 2d136b158fb7..49a301ae0c9d 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o
obj-$(CONFIG_MTD_NAND_STM32_FMC2) += stm32_fmc2_nand.o
obj-$(CONFIG_MTD_NAND_MESON) += meson_nand.o
obj-$(CONFIG_MTD_NAND_CADENCE) += cadence-nand-controller.o
+obj-$(CONFIG_MTD_NAND_INTEL_LGM) += intel_lgm_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/intel_lgm_nand.c b/drivers/mtd/nand/raw/intel_lgm_nand.c
new file mode 100644
index 000000000000..9c307c14c3eb
--- /dev/null
+++ b/drivers/mtd/nand/raw/intel_lgm_nand.c
@@ -0,0 +1,678 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (c) 2019 Intel Corporation. */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-direction.h>
+#include <linux/dma-mapping.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define LGM_CLC 0x000
+#define LGM_CLC_RST 0x00000000u
+
+#define LGM_NAND_ECC_OFFSET 0x008
+
+#define LGM_ADDR_SEL(n) (0x20 + (n) * 4)
+#define LGM_ADDR_MASK (5 << 4)
+#define LGM_ADDR_SEL_REGEN 0x1
+
+#define LGM_BUSCON(n) (0x60 + (n) * 4)
+#define LGM_BUSCON_CMULT_V4 0x1
+#define LGM_BUSCON_RECOVC(n) ((n) << 2)
+#define LGM_BUSCON_HOLDC(n) ((n) << 4)
+#define LGM_BUSCON_WAITRDC(n) ((n) << 6)
+#define LGM_BUSCON_WAITWRC(n) ((n) << 8)
+#define LGM_BUSCON_BCGEN_CS 0x0
+#define LGM_BUSCON_SETUP_EN BIT(22)
+#define LGM_BUSCON_ALEC 0xC000
+
+#define NAND_CON 0x0B0
+#define NAND_CON_NANDM_EN BIT(0)
+#define NAND_CON_NANDM_DIS 0x0
+#define NAND_CON_CSMUX_E_EN BIT(1)
+#define NAND_CON_ALE_P_LOW BIT(2)
+#define NAND_CON_CLE_P_LOW BIT(3)
+#define NAND_CON_CS_P_LOW BIT(4)
+#define NAND_CON_SE_P_LOW BIT(5)
+#define NAND_CON_WP_P_LOW BIT(6)
+#define NAND_CON_PRE_P_LOW BIT(7)
+#define NAND_CON_IN_CS_S(n) ((n) << 8)
+#define NAND_CON_OUT_CS_S(n) ((n) << 10)
+#define NAND_CON_LAT_EN_CS_P ((0x3D) << 18)
+
+#define NAND_WAIT 0x0B4
+#define NAND_WAIT_RDBY BIT(0)
+#define NAND_WAIT_WR_C BIT(3)
+
+#define NAND_CTL1 0x110
+#define NAND_CTL1_ADDR_3_SHIFT 24
+
+#define NAND_CTL2 0x114
+#define NAND_CTL2_ADDR_5_SHIFT 8
+#define NAND_CTL2_CYC_N_V5 (0x2 << 16)
+
+#define NAND_INT_MSK_CTL 0x124
+#define NAND_INT_MSK_CTL_WR_C BIT(4)
+
+#define NAND_INT_STA 0x128
+#define NAND_INT_STA_WR_C BIT(4)
+
+#define NAND_CTL 0x130
+#define NAND_CTL_MODE_ECC 0x1
+#define NAND_CTL_GO BIT(2)
+#define NAND_CTL_CE_SEL_CS(n) BIT(3 + (n))
+#define NAND_CTL_RW_READ 0x0
+#define NAND_CTL_RW_WRITE BIT(10)
+#define NAND_CTL_ECC_OFF_V8TH BIT(11)
+#define NAND_CTL_CKFF_EN 0x0
+#define NAND_CTL_MSG_EN BIT(17)
+
+#define NAND_PARA0 0x13c
+#define NAND_PARA0_PAGE_V8192 0x3
+#define NAND_PARA0_PIB_V256 (0x3 << 4)
+#define NAND_PARA0_BYP_EN_NP 0x0
+#define NAND_PARA0_BYP_DEC_NP 0x0
+#define NAND_PARA0_TYPE_ONFI BIT(18)
+#define NAND_PARA0_ADEP_EN BIT(21)
+
+#define NAND_CMSG_0 0x150
+#define NAND_CMSG_1 0x154
+
+#define NAND_WRITE_CMD (NAND_CON_CS_P_LOW | NAND_CON_CLE_P_LOW)
+#define NAND_WRITE_ADDR (NAND_CON_CS_P_LOW | NAND_CON_ALE_P_LOW)
+#define NAND_WRITE_DATA NAND_CON_CS_P_LOW
+#define NAND_READ_DATA NAND_CON_CS_P_LOW
+
+#define NAND_CHIP_NO_SELECTION -1
+#define NAND_CHIP_SELECTION 0x0
+
+struct lgm_nand_host {
+ struct nand_chip chip;
+ void __iomem *lgm_va;
+ void __iomem *hsnand_va;
+ void __iomem *nandaddr_va;
+ struct clk *clk;
+ unsigned long clk_rate;
+ u32 cs;
+ u32 nd_para0;
+ struct device *dev;
+ struct dma_chan *dma_tx;
+ struct dma_chan *dma_rx;
+ struct completion dma_access_complete;
+ const char *cs_name;
+};
+
+static u8 lgm_nand_readb(struct nand_chip *chip, int op)
+{
+ struct lgm_nand_host *lgm_host = nand_get_controller_data(chip);
+ void __iomem *nand_wait = lgm_host->lgm_va + NAND_WAIT;
+ u32 stat;
+ int ret;
+ u8 val;
+
+ val = readb(lgm_host->nandaddr_va + op);
+
+ ret = readl_poll_timeout(nand_wait, stat, stat & NAND_WAIT_WR_C,
+ 20, 1000);
+ if (ret)
+ dev_warn(lgm_host->dev,
+ "lgm nand write timeout. nand_wait(0x%p)=0x%x\n",
+ nand_wait, readl(nand_wait));
+
+ return val;
+}
+
+static void lgm_nand_writeb(struct nand_chip *chip, int op, u8 value)
+{
+ struct lgm_nand_host *lgm_host = nand_get_controller_data(chip);
+ void __iomem *nand_wait = lgm_host->lgm_va + NAND_WAIT;
+ u32 stat;
+ int ret;
+
+ writeb(value, lgm_host->nandaddr_va + op);
+
+ ret = readl_poll_timeout(nand_wait, stat, stat & NAND_WAIT_WR_C,
+ 20, 1000);
+ if (ret)
+ dev_warn(lgm_host->dev,
+ "lgm nand write timeout. nand_wait(0x%p)=0x%x\n",
+ nand_wait, readl(nand_wait));
+}
+
+static unsigned char lgm_read_byte(struct nand_chip *chip)
+{
+ return lgm_nand_readb(chip, NAND_READ_DATA);
+}
+
+static void lgm_read_buf(struct nand_chip *chip, u_char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++)
+ buf[i] = lgm_nand_readb(chip, NAND_WRITE_DATA);
+}
+
+static void lgm_write_buf(struct nand_chip *chip, const u_char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++)
+ lgm_nand_writeb(chip, NAND_WRITE_DATA, buf[i]);
+}
+
+static void lgm_select_chip(struct nand_chip *chip, int select)
+{
+ struct lgm_nand_host *lgm_host = nand_get_controller_data(chip);
+ void __iomem *nand_con = lgm_host->lgm_va + NAND_CON;
+ u32 cs = lgm_host->cs;
+ int val;
+
+ switch (select) {
+ case NAND_CHIP_NO_SELECTION:
+ val = readl(nand_con);
+ writel(val & ~NAND_CON_NANDM_EN, nand_con);
+ break;
+ case NAND_CHIP_SELECTION:
+ writel(NAND_CON_NANDM_EN | NAND_CON_CSMUX_E_EN |
+ NAND_CON_CS_P_LOW | NAND_CON_SE_P_LOW |
+ NAND_CON_WP_P_LOW | NAND_CON_PRE_P_LOW |
+ NAND_CON_IN_CS_S(cs) | NAND_CON_OUT_CS_S(cs) |
+ NAND_CON_LAT_EN_CS_P, nand_con);
+ break;
+ default:
+ break;
+ }
+}
+
+static int lgm_dev_ready(struct nand_chip *chip)
+{
+ struct lgm_nand_host *lgm_host = nand_get_controller_data(chip);
+
+ return readl(lgm_host->lgm_va + NAND_WAIT) & NAND_WAIT_RDBY;
+}
+
+static void lgm_cmd_ctrl(struct nand_chip *chip, int cmd, unsigned int ctrl)
+{
+ if (cmd == NAND_CMD_NONE)
+ return;
+
+ if (ctrl & NAND_CLE)
+ lgm_nand_writeb(chip, NAND_WRITE_CMD, cmd);
+ else if (ctrl & NAND_ALE)
+ lgm_nand_writeb(chip, NAND_WRITE_ADDR, cmd);
+}
+
+static int lgm_nand_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+
+ if (section)
+ return -ERANGE;
+
+ oobregion->offset = LGM_NAND_ECC_OFFSET;
+ oobregion->length = chip->ecc.total;
+
+ return 0;
+}
+
+static int lgm_nand_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+
+ if (section)
+ return -ERANGE;
+
+ oobregion->offset = chip->ecc.total + LGM_NAND_ECC_OFFSET;
+ oobregion->length = mtd->oobsize - oobregion->offset;
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops lgm_nand_ooblayout_ops = {
+ .ecc = lgm_nand_ooblayout_ecc,
+ .free = lgm_nand_ooblayout_free,
+};
+
+static void lgm_dma_rx_callback(void *cookie)
+{
+ struct lgm_nand_host *lgm_host = cookie;
+
+ dmaengine_terminate_async(lgm_host->dma_rx);
+
+ complete(&lgm_host->dma_access_complete);
+}
+
+static void lgm_dma_tx_callback(void *cookie)
+{
+ struct lgm_nand_host *lgm_host = cookie;
+
+ dmaengine_terminate_async(lgm_host->dma_tx);
+
+ complete(&lgm_host->dma_access_complete);
+}
+
+static int lgm_dma_start(struct lgm_nand_host *lgm_host, u32 dir,
+ const u8 *buf, u32 len)
+{
+ struct dma_async_tx_descriptor *tx;
+ struct completion *dma_completion;
+ dma_async_tx_callback callback;
+ struct dma_chan *chan;
+ dma_cookie_t cookie;
+ unsigned long flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
+ dma_addr_t buf_dma;
+ int ret;
+ u32 timeout;
+
+ if (dir == DMA_DEV_TO_MEM) {
+ chan = lgm_host->dma_rx;
+ dma_completion = &lgm_host->dma_access_complete;
+ callback = lgm_dma_rx_callback;
+ } else {
+ chan = lgm_host->dma_tx;
+ dma_completion = &lgm_host->dma_access_complete;
+ callback = lgm_dma_tx_callback;
+ }
+
+ buf_dma = dma_map_single(chan->device->dev, (void *)buf, len, dir);
+ if (dma_mapping_error(chan->device->dev, buf_dma)) {
+ dev_err(lgm_host->dev, "Failed to map DMA buffer\n");
+ ret = -EIO;
+ goto err_unmap;
+ }
+
+ tx = dmaengine_prep_slave_single(chan, buf_dma, len, dir, flags);
+ if (!tx)
+ return -ENXIO;
+
+ tx->callback = callback;
+ tx->callback_param = lgm_host;
+ cookie = tx->tx_submit(tx);
+
+ ret = dma_submit_error(cookie);
+ if (ret) {
+ dev_err(lgm_host->dev, "dma_submit_error %d\n", cookie);
+ ret = -EIO;
+ goto err_unmap;
+ }
+
+ init_completion(dma_completion);
+ dma_async_issue_pending(chan);
+
+ /* Wait DMA to finish the data transfer.*/
+ timeout =
+ wait_for_completion_timeout(dma_completion, msecs_to_jiffies(1000));
+ if (!timeout) {
+ dev_err(lgm_host->dev, "I/O Error in DMA RX (status %d)\n",
+ dmaengine_tx_status(chan, cookie, NULL));
+ dmaengine_terminate_sync(chan);
+ ret = -ETIMEDOUT;
+ goto err_unmap;
+ }
+
+ return 0;
+
+err_unmap:
+ dma_unmap_single(lgm_host->dev, buf_dma, len, dir);
+
+ return ret;
+}
+
+static void lgm_nand_trigger(struct lgm_nand_host *lgm_host, int page, u32 cmd)
+{
+ unsigned int val;
+
+ val = cmd | (page & 0xFF) << NAND_CTL1_ADDR_3_SHIFT;
+ writel(val, lgm_host->hsnand_va + NAND_CTL1);
+ val = (page & 0xFFFF00) >> 8 | NAND_CTL2_CYC_N_V5;
+ writel(val, lgm_host->hsnand_va + NAND_CTL2);
+
+ writel(lgm_host->nd_para0, lgm_host->hsnand_va + NAND_PARA0);
+
+ /* clear first, will update later */
+ writel(0xFFFFFFFF, lgm_host->hsnand_va + NAND_CMSG_0);
+ writel(0xFFFFFFFF, lgm_host->hsnand_va + NAND_CMSG_1);
+
+ writel(NAND_INT_MSK_CTL_WR_C, lgm_host->hsnand_va + NAND_INT_MSK_CTL);
+
+ val = cmd == NAND_CMD_READ0 ? NAND_CTL_RW_READ : NAND_CTL_RW_WRITE;
+
+ writel(NAND_CTL_MSG_EN | NAND_CTL_CKFF_EN | NAND_CTL_ECC_OFF_V8TH |
+ NAND_CTL_CE_SEL_CS(lgm_host->cs) | NAND_CTL_MODE_ECC |
+ NAND_CTL_GO | val, lgm_host->hsnand_va + NAND_CTL);
+}
+
+static int lgm_nand_read_page_hwecc(struct nand_chip *chip, u8 *buf,
+ int oob_required, int page)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct lgm_nand_host *lgm_host = nand_get_controller_data(chip);
+ int ret, x;
+
+ lgm_nand_trigger(lgm_host, page, NAND_CMD_READ0);
+
+ ret = lgm_dma_start(lgm_host, DMA_DEV_TO_MEM, buf, mtd->writesize);
+ if (ret)
+ return ret;
+
+ if (oob_required)
+ chip->ecc.read_oob(chip, page);
+
+ x = readl(lgm_host->hsnand_va + NAND_CTL);
+ x &= ~NAND_CTL_GO;
+ writel(x, lgm_host->hsnand_va + NAND_CTL);
+
+ return 0;
+}
+
+static int lgm_nand_write_page_hwecc(struct nand_chip *chip, const u8 *buf,
+ int oob_required, int page)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct lgm_nand_host *lgm_host = nand_get_controller_data(chip);
+ void __iomem *int_sta = lgm_host->hsnand_va + NAND_INT_STA;
+ int ret, val, x;
+ __be32 reg;
+
+ lgm_nand_trigger(lgm_host, page, NAND_CMD_SEQIN);
+
+ ret = lgm_dma_start(lgm_host, DMA_MEM_TO_DEV, buf, mtd->writesize);
+ if (ret)
+ return ret;
+
+ if (oob_required) {
+ const u8 *pdata;
+
+ pdata = chip->oob_poi;
+ reg = cpu_to_be32(*pdata++);
+ writel(reg, lgm_host->hsnand_va + NAND_CMSG_0);
+
+ reg = cpu_to_be32(*pdata);
+ writel(reg, lgm_host->hsnand_va + NAND_CMSG_1);
+ }
+
+ ret = readl_poll_timeout_atomic(int_sta, val,
+ !(val & NAND_INT_STA_WR_C), 10, 1000);
+ if (ret)
+ return -EIO;
+
+ x = readl(lgm_host->hsnand_va + NAND_CTL);
+ x &= ~NAND_CTL_GO;
+ writel(x, lgm_host->hsnand_va + NAND_CTL);
+
+ return 0;
+}
+
+static const u8 ecc_strength[] = { 1, 1, 4, 8, 24, 32, 40, 60, };
+
+static int lgm_nand_attach_chip(struct nand_chip *chip)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct lgm_nand_host *lgm_host = nand_get_controller_data(chip);
+ u32 eccsize, eccsteps, eccbytes, ecctotal, pagesize, pg_per_blk;
+ u32 eccstrength = chip->ecc.strength;
+ u32 writesize = mtd->writesize;
+ u32 blocksize = mtd->erasesize;
+ int start, val, i;
+
+ if (chip->ecc.mode != NAND_ECC_HW)
+ return 0;
+
+ /* Check whether eccsize is 0x0 or wrong. assign eccsize = 512 if YES */
+ if (!chip->ecc.size)
+ chip->ecc.size = 512;
+ eccsize = chip->ecc.size;
+
+ switch (eccsize) {
+ case 512:
+ start = 1;
+ if (!eccstrength)
+ eccstrength = 4;
+ break;
+ case 1024:
+ start = 4;
+ if (!eccstrength)
+ eccstrength = 32;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ i = round_up(start + 1, 4);
+ for (val = start; val < i; val++) {
+ if (eccstrength == ecc_strength[val])
+ break;
+ }
+ if (val == i)
+ return -EINVAL;
+
+ if (eccstrength == 8)
+ eccbytes = 14;
+ else
+ eccbytes = DIV_ROUND_UP(eccstrength * fls(8 * eccsize), 8);
+
+ eccsteps = writesize / eccsize;
+ ecctotal = eccsteps * eccbytes;
+ if ((ecctotal + 8) > mtd->oobsize)
+ return -ERANGE;
+
+ chip->ecc.total = ecctotal;
+ pagesize = fls(writesize >> 11);
+ if (pagesize > NAND_PARA0_PAGE_V8192)
+ return -ERANGE;
+
+ pg_per_blk = fls((blocksize / writesize) >> 6) << 4;
+ if (pg_per_blk > NAND_PARA0_PIB_V256)
+ return -ERANGE;
+
+ lgm_host->nd_para0 = pagesize | pg_per_blk | NAND_PARA0_BYP_EN_NP |
+ NAND_PARA0_BYP_DEC_NP | NAND_PARA0_ADEP_EN |
+ NAND_PARA0_TYPE_ONFI | (val << 29);
+
+ mtd_set_ooblayout(mtd, &lgm_nand_ooblayout_ops);
+ chip->ecc.read_page = lgm_nand_read_page_hwecc;
+ chip->ecc.write_page = lgm_nand_write_page_hwecc;
+
+ return 0;
+}
+
+static const struct nand_controller_ops lgm_nand_controller_ops = {
+ .attach_chip = lgm_nand_attach_chip,
+};
+
+static void lgm_dma_exit(struct lgm_nand_host *lgm_host)
+{
+ if (lgm_host->dma_rx) {
+ dma_release_channel(lgm_host->dma_rx);
+ lgm_host->dma_rx = NULL;
+ }
+
+ if (lgm_host->dma_tx) {
+ dma_release_channel(lgm_host->dma_tx);
+ lgm_host->dma_tx = NULL;
+ }
+}
+
+static int lgm_dma_init(struct device *dev, struct lgm_nand_host *lgm_host)
+{
+ int ret;
+
+ /* Prepare for TX DMA: */
+ lgm_host->dma_tx = dma_request_chan(dev, "tx");
+ if (IS_ERR(lgm_host->dma_tx)) {
+ ret = PTR_ERR(lgm_host->dma_tx);
+ dev_err(dev, "can't get the TX DMA channel, error %d!\n", ret);
+ goto err;
+ }
+
+ /* Prepare for RX: */
+ lgm_host->dma_rx = dma_request_chan(dev, "rx");
+ if (IS_ERR(lgm_host->dma_rx)) {
+ ret = PTR_ERR(lgm_host->dma_rx);
+ dev_err(dev, "can't get the RX DMA channel, error %d\n", ret);
+ goto err;
+ }
+
+ return 0;
+err:
+ return ret;
+}
+
+static int lgm_nand_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct lgm_nand_host *lgm_host;
+ phys_addr_t nandaddr_pa;
+ struct mtd_info *mtd;
+ struct resource *res;
+ int ret;
+ u32 cs;
+
+ lgm_host = devm_kzalloc(dev, sizeof(*lgm_host), GFP_KERNEL);
+ if (!lgm_host)
+ return -ENOMEM;
+
+ lgm_host->lgm_va = devm_platform_ioremap_resource_byname(pdev, "lgmnand");
+ if (IS_ERR(lgm_host->lgm_va))
+ return PTR_ERR(lgm_host->lgm_va);
+
+ lgm_host->hsnand_va = devm_platform_ioremap_resource_byname(pdev, "hsnand");
+ if (IS_ERR(lgm_host->hsnand_va))
+ return PTR_ERR(lgm_host->hsnand_va);
+
+ ret = device_property_read_u32(dev, "nand,cs", &cs);
+ if (ret) {
+ dev_err(dev, "failed to get chip select: %d\n", ret);
+ return ret;
+ }
+
+ lgm_host->cs = cs;
+
+ lgm_host->cs_name = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", cs);
+ if (IS_ERR(lgm_host->cs_name)) {
+ ret = PTR_ERR(lgm_host->cs_name);
+ dev_err(dev, "failed to get chip select name: %d\n", ret);
+ return ret;
+ }
+
+ res = devm_platform_ioremap_resource_byname(pdev, lgm_host->cs_name);
+ lgm_host->nandaddr_va = res;
+ nandaddr_pa = res->start;
+ if (IS_ERR(lgm_host->nandaddr_va))
+ return PTR_ERR(lgm_host->nandaddr_va);
+
+ lgm_host->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(lgm_host->clk)) {
+ ret = PTR_ERR(lgm_host->clk);
+ dev_err(dev, "failed to get clock: %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(lgm_host->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable clock: %d\n", ret);
+ return ret;
+ }
+ lgm_host->clk_rate = clk_get_rate(lgm_host->clk);
+
+ ret = lgm_dma_init(dev, lgm_host);
+ if (ret)
+ goto disable_clk;
+
+ writel(lower_32_bits(nandaddr_pa) | LGM_ADDR_SEL_REGEN | LGM_ADDR_MASK,
+ lgm_host->lgm_va + LGM_ADDR_SEL(cs));
+
+ writel(LGM_BUSCON_CMULT_V4 | LGM_BUSCON_RECOVC(1) |
+ LGM_BUSCON_HOLDC(1) | LGM_BUSCON_WAITRDC(2) |
+ LGM_BUSCON_WAITWRC(2) | LGM_BUSCON_BCGEN_CS | LGM_BUSCON_ALEC |
+ LGM_BUSCON_SETUP_EN, lgm_host->lgm_va + LGM_BUSCON(cs));
+
+ /*
+ * NAND physical address selection is based on the chip select
+ * and written to ADDR_SEL register to get Memory Region Base address.
+ * FPI Bus addresses are compared to this base address in conjunction
+ * with the mask control. Driver need to program this field!
+ * Address: 0x17400 if chip select is CS_0
+ * Address: 0x17C00 if chip select is CS_1
+ * Refer the Intel LGM SoC datasheet.
+ */
+ writel(0x17400051, lgm_host->lgm_va + LGM_ADDR_SEL(0));
+ writel(0x17C00051, lgm_host->lgm_va + LGM_ADDR_SEL(cs));
+ nand_set_flash_node(&lgm_host->chip, dev->of_node);
+ mtd = nand_to_mtd(&lgm_host->chip);
+ mtd->dev.parent = dev;
+ lgm_host->dev = dev;
+
+ platform_set_drvdata(pdev, lgm_host);
+ nand_set_controller_data(&lgm_host->chip, lgm_host);
+
+ lgm_host->chip.legacy.read_byte = lgm_read_byte;
+ lgm_host->chip.legacy.read_buf = lgm_read_buf;
+ lgm_host->chip.legacy.write_buf = lgm_write_buf;
+ lgm_host->chip.legacy.select_chip = lgm_select_chip;
+ lgm_host->chip.legacy.dev_ready = lgm_dev_ready;
+ lgm_host->chip.legacy.cmd_ctrl = lgm_cmd_ctrl;
+ lgm_host->chip.legacy.chip_delay = 30;
+ lgm_host->chip.legacy.dummy_controller.ops = &lgm_nand_controller_ops;
+
+ /* Scan to find existence of the device */
+ ret = nand_scan(&lgm_host->chip, 1);
+ if (ret)
+ goto exit_dma;
+
+ ret = mtd_device_register(mtd, NULL, 0);
+ if (ret)
+ goto clean_nand;
+
+ return 0;
+
+clean_nand:
+ nand_cleanup(&lgm_host->chip);
+exit_dma:
+ lgm_dma_exit(lgm_host);
+disable_clk:
+ clk_disable_unprepare(lgm_host->clk);
+
+ return ret;
+}
+
+static int lgm_nand_remove(struct platform_device *pdev)
+{
+ struct lgm_nand_host *lgm_host = platform_get_drvdata(pdev);
+
+ nand_release(&lgm_host->chip);
+ clk_disable_unprepare(lgm_host->clk);
+ lgm_dma_exit(lgm_host);
+
+ return 0;
+}
+
+static const struct of_device_id lgm_nand_match[] = {
+ { .compatible = "intel,lgm-nand", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, lgm_nand_match);
+
+static struct platform_driver lgm_nand_driver = {
+ .probe = lgm_nand_probe,
+ .remove = lgm_nand_remove,
+ .driver = {
+ .name = "intel-lgm-nand",
+ .of_match_table = lgm_nand_match,
+ },
+
+};
+module_platform_driver(lgm_nand_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Vadivel Murugan R <[email protected]>");
+MODULE_DESCRIPTION("Intel's LGM External Bus NAND Controller driver");
--
2.11.0

2020-04-15 10:41:37

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hello Ramuthevar,

On Tue, 14 Apr 2020 10:24:33 +0800
"Ramuthevar,Vadivel MuruganX"
<[email protected]> wrote:

> + lgm_host->chip.legacy.read_byte = lgm_read_byte;
> + lgm_host->chip.legacy.read_buf = lgm_read_buf;
> + lgm_host->chip.legacy.write_buf = lgm_write_buf;
> + lgm_host->chip.legacy.select_chip = lgm_select_chip;
> + lgm_host->chip.legacy.dev_ready = lgm_dev_ready;
> + lgm_host->chip.legacy.cmd_ctrl = lgm_cmd_ctrl;
> + lgm_host->chip.legacy.chip_delay = 30;
> + lgm_host->chip.legacy.dummy_controller.ops = &lgm_nand_controller_ops;
> +

Seriously, what's not clear in [1]? Okay, let's say you overlooked this
comment, isn't the name of the field explicit enough? We received a
few other drivers implementing the legacy interface in the last few
months so maybe there's something to improve on our end (update the
doc, move legacy drivers to a legacy sub-dir?).

Back to more constructive comment now: please implement ->exec_op() to
replace those legacy hooks.

Regards,

Boris

[1]https://elixir.bootlin.com/linux/v5.7-rc1/source/include/linux/mtd/rawnand.h#L987

2020-04-15 11:46:45

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: mtd: Add YAML for Nand Flash Controller support

On Tue, 14 Apr 2020 10:24:32 +0800
"Ramuthevar,Vadivel MuruganX"
<[email protected]> wrote:

> From: Ramuthevar Vadivel Murugan <[email protected]>
>
> Add YAML file for dt-bindings to support NAND Flash Controller
> on Intel's Lightning Mountain SoC.
>
> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
> ---
> .../devicetree/bindings/mtd/intel,lgm-nand.yaml | 61 ++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
>
> diff --git a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
> new file mode 100644
> index 000000000000..361e5051c602
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/intel,lgm-nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel LGM SoC NAND Controller Device Tree Bindings
> +
> +allOf:
> + - $ref: "nand-controller.yaml"
> +
> +maintainers:
> + - Ramuthevar Vadivel Murugan <[email protected]>
> +
> +properties:
> + compatible:
> + const: intel,lgm-nand

intel,lgm-nand-controller

> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + dmas:
> + maxItems: 2
> +
> + dma-names:
> + enum:
> + - rx
> + - tx
> +
> + pinctrl-names: true
> +
> +patternProperties:
> + "^pinctrl-[0-9]+$": true
> +
> + "^nand@[a-f0-9]+$":
> + type: object
> + properties:
> + reg:
> + minimum: 0
> + maximum: 7
> +
> + nand-ecc-mode: true
> +
> + nand-ecc-algo:
> + const: hw
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - dmas
> +
> +additionalProperties: false
> +
> +...

Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Boris,

    Thank you so much for review comments and your time...

On 14/4/2020 3:21 pm, Boris Brezillon wrote:
> Hello Ramuthevar,
>
> On Tue, 14 Apr 2020 10:24:33 +0800
> "Ramuthevar,Vadivel MuruganX"
> <[email protected]> wrote:
>
>> + lgm_host->chip.legacy.read_byte = lgm_read_byte;
>> + lgm_host->chip.legacy.read_buf = lgm_read_buf;
>> + lgm_host->chip.legacy.write_buf = lgm_write_buf;
>> + lgm_host->chip.legacy.select_chip = lgm_select_chip;
>> + lgm_host->chip.legacy.dev_ready = lgm_dev_ready;
>> + lgm_host->chip.legacy.cmd_ctrl = lgm_cmd_ctrl;
>> + lgm_host->chip.legacy.chip_delay = 30;
>> + lgm_host->chip.legacy.dummy_controller.ops = &lgm_nand_controller_ops;
>> +
> Seriously, what's not clear in [1]? Okay, let's say you overlooked this
> comment, isn't the name of the field explicit enough? We received a
> few other drivers implementing the legacy interface in the last few
> months so maybe there's something to improve on our end (update the
> doc, move legacy drivers to a legacy sub-dir?).
Understood the legacy to latest implementation based ->exec_op(), Thanks!
>
> Back to more constructive comment now: please implement ->exec_op() to
> replace those legacy hooks.

Agreed, will do the implementation of exec_op() hook and update the patches.

Regards
Vadivel
>
> Regards,
>
> Boris
>
> [1]https://elixir.bootlin.com/linux/v5.7-rc1/source/include/linux/mtd/rawnand.h#L987

Subject: Re: [PATCH v1 1/2] dt-bindings: mtd: Add YAML for Nand Flash Controller support

Hi Boris,

    Thank you so much for the review comments and your time...

On 1 4/4/2020 3:04 pm, Boris Brezillon wrote:
> On Tue, 14 Apr 2020 10:24:32 +0800
> "Ramuthevar,Vadivel MuruganX"
> <[email protected]> wrote:
>
>> From: Ramuthevar Vadivel Murugan <[email protected]>
>>
>> Add YAML file for dt-bindings to support NAND Flash Controller
>> on Intel's Lightning Mountain SoC.
>>
>> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
>> ---
>> .../devicetree/bindings/mtd/intel,lgm-nand.yaml | 61 ++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
>> new file mode 100644
>> index 000000000000..361e5051c602
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
>> @@ -0,0 +1,61 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mtd/intel,lgm-nand.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Intel LGM SoC NAND Controller Device Tree Bindings
>> +
>> +allOf:
>> + - $ref: "nand-controller.yaml"
>> +
>> +maintainers:
>> + - Ramuthevar Vadivel Murugan <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + const: intel,lgm-nand
> intel,lgm-nand-controller

Noted, will update in the next patch.

Regards
Vadivel
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + dmas:
>> + maxItems: 2
>> +
>> + dma-names:
>> + enum:
>> + - rx
>> + - tx
>> +
>> + pinctrl-names: true
>> +
>> +patternProperties:
>> + "^pinctrl-[0-9]+$": true
>> +
>> + "^nand@[a-f0-9]+$":
>> + type: object
>> + properties:
>> + reg:
>> + minimum: 0
>> + maximum: 7
>> +
>> + nand-ecc-mode: true
>> +
>> + nand-ecc-algo:
>> + const: hw
>> +
>> + additionalProperties: false
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> + - clock-names
>> + - dmas
>> +
>> +additionalProperties: false
>> +
>> +...

2020-04-16 01:11:51

by Martin Blumenstingl

[permalink] [raw]
Subject: RE: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi,

first of all: thank you for working on upstreaming this.
Especially since you are going to use the new exec_op style in v2 as
Boris suggested.

> From: Ramuthevar Vadivel Murugan <[email protected]>
>
> This patch adds the new IP of Nand Flash Controller(NFC) support
> on Intel's Lightning Mountain(LGM) SoC.
>
> DMA is used for burst data transfer operation, also DMA HW supports
> aligned 32bit memory address and aligned data access by default.
> DMA burst of 8 supported. Data register used to support the read/write
> operation from/to device.
I am wondering how this new hardware is different from the Lantiq NAND
controller IP - for which there is already a driver in mainline (it's
in drivers/mtd/nand/raw/xway_nand.c).
The CON and WAIT registers look suspiciously similar.

As far as I understand the "old" SoCs (VRX200 and earlier) don't have
a built-in ECC engine. This seems to have changed with ARX300 though
(again, AFAIK).

A bit of lineage on these SoCs (initially these were developed by
Infineon. Lantiq then started as an Infineon spin-off in 2009 and
was then acquired by Intel in 2015):
- Danube
- ARX100 from 2008/2009
- VRX200 from 2009/2010
- ARX300 from 2014
- GRX350 from 2015/2016
- GRX550 from 2017
- and now finally: LGM from 2020 (est.)

The existing xway_nand driver supports the Danube, ARX100 and VRX200
SoCs.


Best regards,
Martin

Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Martin,

    Thank you so much for review comments and your time...

On 16/4/2020 6:05 am, Martin Blumenstingl wrote:
> Hi,
>
> first of all: thank you for working on upstreaming this.
> Especially since you are going to use the new exec_op style in v2 as
> Boris suggested.
>
>> From: Ramuthevar Vadivel Murugan <[email protected]>
>>
>> This patch adds the new IP of Nand Flash Controller(NFC) support
>> on Intel's Lightning Mountain(LGM) SoC.
>>
>> DMA is used for burst data transfer operation, also DMA HW supports
>> aligned 32bit memory address and aligned data access by default.
>> DMA burst of 8 supported. Data register used to support the read/write
>> operation from/to device.
> I am wondering how this new hardware is different from the Lantiq NAND
> controller IP - for which there is already a driver in mainline (it's
> in drivers/mtd/nand/raw/xway_nand.c).
> The CON and WAIT registers look suspiciously similar.
>
> As far as I understand the "old" SoCs (VRX200 and earlier) don't have
> a built-in ECC engine. This seems to have changed with ARX300 though
> (again, AFAIK).
>
> A bit of lineage on these SoCs (initially these were developed by
> Infineon. Lantiq then started as an Infineon spin-off in 2009 and
> was then acquired by Intel in 2015):
> - Danube
> - ARX100 from 2008/2009
> - VRX200 from 2009/2010
> - ARX300 from 2014
> - GRX350 from 2015/2016
> - GRX550 from 2017
> - and now finally: LGM from 2020 (est.)
>
> The existing xway_nand driver supports the Danube, ARX100 and VRX200
> SoCs.
Lantiq upstreamed a driver for an older version of this IP core 8 years
ago, see here:
https://elixir.bootlin.com/linux/v5.5.6/source/drivers/mtd/nand/raw/xway_nand.c
It does not support DMA and ECC.
This upstream driver works with the xrx200, I do not know how well it
works with other SoCs.

Regards
Vadivel
>
>
> Best regards,
> Martin

2020-04-16 09:40:01

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, 16 Apr 2020 17:35:26 +0800
"Ramuthevar, Vadivel MuruganX"
<[email protected]> wrote:

> Hi Martin,
>
>     Thank you so much for review comments and your time...
>
> On 16/4/2020 6:05 am, Martin Blumenstingl wrote:
> > Hi,
> >
> > first of all: thank you for working on upstreaming this.
> > Especially since you are going to use the new exec_op style in v2 as
> > Boris suggested.
> >
> >> From: Ramuthevar Vadivel Murugan <[email protected]>
> >>
> >> This patch adds the new IP of Nand Flash Controller(NFC) support
> >> on Intel's Lightning Mountain(LGM) SoC.
> >>
> >> DMA is used for burst data transfer operation, also DMA HW supports
> >> aligned 32bit memory address and aligned data access by default.
> >> DMA burst of 8 supported. Data register used to support the read/write
> >> operation from/to device.
> > I am wondering how this new hardware is different from the Lantiq NAND
> > controller IP - for which there is already a driver in mainline (it's
> > in drivers/mtd/nand/raw/xway_nand.c).
> > The CON and WAIT registers look suspiciously similar.
> >
> > As far as I understand the "old" SoCs (VRX200 and earlier) don't have
> > a built-in ECC engine. This seems to have changed with ARX300 though
> > (again, AFAIK).
> >
> > A bit of lineage on these SoCs (initially these were developed by
> > Infineon. Lantiq then started as an Infineon spin-off in 2009 and
> > was then acquired by Intel in 2015):
> > - Danube
> > - ARX100 from 2008/2009
> > - VRX200 from 2009/2010
> > - ARX300 from 2014
> > - GRX350 from 2015/2016
> > - GRX550 from 2017
> > - and now finally: LGM from 2020 (est.)
> >
> > The existing xway_nand driver supports the Danube, ARX100 and VRX200
> > SoCs.
> Lantiq upstreamed a driver for an older version of this IP core 8 years
> ago, see here:
> https://elixir.bootlin.com/linux/v5.5.6/source/drivers/mtd/nand/raw/xway_nand.c
> It does not support DMA and ECC.

Then let's just extend this driver to support the new features. Plus,
we'll be happy to have one more of the existing driver converted to
->exec_op() ;-).

> This upstream driver works with the xrx200, I do not know how well it
> works with other SoCs.
>
> Regards
> Vadivel
> >
> >
> > Best regards,
> > Martin

Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Boris,

    Thank you for prompt reply...

On 16/4/2020 5:38 pm, Boris Brezillon wrote:
> On Thu, 16 Apr 2020 17:35:26 +0800
> "Ramuthevar, Vadivel MuruganX"
> <[email protected]> wrote:
>
>> Hi Martin,
>>
>>     Thank you so much for review comments and your time...
>>
>> On 16/4/2020 6:05 am, Martin Blumenstingl wrote:
>>> Hi,
>>>
>>> first of all: thank you for working on upstreaming this.
>>> Especially since you are going to use the new exec_op style in v2 as
>>> Boris suggested.
>>>
>>>> From: Ramuthevar Vadivel Murugan <[email protected]>
>>>>
>>>> This patch adds the new IP of Nand Flash Controller(NFC) support
>>>> on Intel's Lightning Mountain(LGM) SoC.
>>>>
>>>> DMA is used for burst data transfer operation, also DMA HW supports
>>>> aligned 32bit memory address and aligned data access by default.
>>>> DMA burst of 8 supported. Data register used to support the read/write
>>>> operation from/to device.
>>> I am wondering how this new hardware is different from the Lantiq NAND
>>> controller IP - for which there is already a driver in mainline (it's
>>> in drivers/mtd/nand/raw/xway_nand.c).
>>> The CON and WAIT registers look suspiciously similar.
>>>
>>> As far as I understand the "old" SoCs (VRX200 and earlier) don't have
>>> a built-in ECC engine. This seems to have changed with ARX300 though
>>> (again, AFAIK).
>>>
>>> A bit of lineage on these SoCs (initially these were developed by
>>> Infineon. Lantiq then started as an Infineon spin-off in 2009 and
>>> was then acquired by Intel in 2015):
>>> - Danube
>>> - ARX100 from 2008/2009
>>> - VRX200 from 2009/2010
>>> - ARX300 from 2014
>>> - GRX350 from 2015/2016
>>> - GRX550 from 2017
>>> - and now finally: LGM from 2020 (est.)
>>>
>>> The existing xway_nand driver supports the Danube, ARX100 and VRX200
>>> SoCs.
>> Lantiq upstreamed a driver for an older version of this IP core 8 years
>> ago, see here:
>> https://elixir.bootlin.com/linux/v5.5.6/source/drivers/mtd/nand/raw/xway_nand.c
>> It does not support DMA and ECC.
> Then let's just extend this driver to support the new features. Plus,
We do not have the platform to test also it's very old legacy driver .
> we'll be happy to have one more of the existing driver converted to
> ->exec_op() ;-).

I have completely adapted to ->exec_op() hook up to replace the legacy
call-back.

Regards
Vadivel
>
>> This upstream driver works with the xrx200, I do not know how well it
>> works with other SoCs.
>>
>> Regards
>> Vadivel
>>>
>>> Best regards,
>>> Martin

2020-04-16 10:29:34

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, 16 Apr 2020 17:45:49 +0800
"Ramuthevar, Vadivel MuruganX"
<[email protected]> wrote:

> >>>> From: Ramuthevar Vadivel Murugan <[email protected]>
> >>>>
> >>>> This patch adds the new IP of Nand Flash Controller(NFC) support
> >>>> on Intel's Lightning Mountain(LGM) SoC.
> >>>>
> >>>> DMA is used for burst data transfer operation, also DMA HW supports
> >>>> aligned 32bit memory address and aligned data access by default.
> >>>> DMA burst of 8 supported. Data register used to support the read/write
> >>>> operation from/to device.
> >>> I am wondering how this new hardware is different from the Lantiq NAND
> >>> controller IP - for which there is already a driver in mainline (it's
> >>> in drivers/mtd/nand/raw/xway_nand.c).
> >>> The CON and WAIT registers look suspiciously similar.
> >>>
> >>> As far as I understand the "old" SoCs (VRX200 and earlier) don't have
> >>> a built-in ECC engine. This seems to have changed with ARX300 though
> >>> (again, AFAIK).
> >>>
> >>> A bit of lineage on these SoCs (initially these were developed by
> >>> Infineon. Lantiq then started as an Infineon spin-off in 2009 and
> >>> was then acquired by Intel in 2015):
> >>> - Danube
> >>> - ARX100 from 2008/2009
> >>> - VRX200 from 2009/2010
> >>> - ARX300 from 2014
> >>> - GRX350 from 2015/2016
> >>> - GRX550 from 2017
> >>> - and now finally: LGM from 2020 (est.)
> >>>
> >>> The existing xway_nand driver supports the Danube, ARX100 and VRX200
> >>> SoCs.
> >> Lantiq upstreamed a driver for an older version of this IP core 8 years
> >> ago, see here:
> >> https://elixir.bootlin.com/linux/v5.5.6/source/drivers/mtd/nand/raw/xway_nand.c
> >> It does not support DMA and ECC.
> > Then let's just extend this driver to support the new features. Plus,
> We do not have the platform to test also it's very old legacy driver .

Well, if it's similar enough, we want to have one driver.

> > we'll be happy to have one more of the existing driver converted to
> > ->exec_op() ;-).
>
> I have completely adapted to ->exec_op() hook up to replace the legacy
> call-back.

I suspect porting what you've done to the xway driver shouldn't be too
complicated.

2020-04-16 11:25:05

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, 16 Apr 2020 18:40:53 +0800
"Ramuthevar, Vadivel MuruganX"
<[email protected]> wrote:

> >>> we'll be happy to have one more of the existing driver converted to
> >>> ->exec_op() ;-).
> >> I have completely adapted to ->exec_op() hook up to replace the legacy
> >> call-back.
> > I suspect porting what you've done to the xway driver shouldn't be too
> > complicated.
> Not ported from xway_nand.c driver , we have developed from the scratch
> to make it work on
> Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
> different and most of the registers won't match.
> if we port then it would be ugly and also what are the problem may occur
> we do not know.

Sorry but IMO they look similar enough to try to merge them.

2020-04-16 11:35:13

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, Apr 16, 2020 at 01:17:25PM +0200, Boris Brezillon wrote:
> On Thu, 16 Apr 2020 18:40:53 +0800
> "Ramuthevar, Vadivel MuruganX"
> <[email protected]> wrote:
>
> > >>> we'll be happy to have one more of the existing driver converted to
> > >>> ->exec_op() ;-).
> > >> I have completely adapted to ->exec_op() hook up to replace the legacy
> > >> call-back.
> > > I suspect porting what you've done to the xway driver shouldn't be too
> > > complicated.
> > Not ported from xway_nand.c driver , we have developed from the scratch
> > to make it work on
> > Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
> > different and most of the registers won't match.
> > if we port then it would be ugly and also what are the problem may occur
> > we do not know.
>
> Sorry but IMO they look similar enough to try to merge them.

I agree. I tried to convince them internally... but here we are.

--
With Best Regards,
Andy Shevchenko


Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On 16/4/2020 6:26 pm, Boris Brezillon wrote:
> On Thu, 16 Apr 2020 17:45:49 +0800
> "Ramuthevar, Vadivel MuruganX"
> <[email protected]> wrote:
>
>>>>>> From: Ramuthevar Vadivel Murugan <[email protected]>
>>>>>>
>>>>>> This patch adds the new IP of Nand Flash Controller(NFC) support
>>>>>> on Intel's Lightning Mountain(LGM) SoC.
>>>>>>
>>>>>> DMA is used for burst data transfer operation, also DMA HW supports
>>>>>> aligned 32bit memory address and aligned data access by default.
>>>>>> DMA burst of 8 supported. Data register used to support the read/write
>>>>>> operation from/to device.
>>>>> I am wondering how this new hardware is different from the Lantiq NAND
>>>>> controller IP - for which there is already a driver in mainline (it's
>>>>> in drivers/mtd/nand/raw/xway_nand.c).
>>>>> The CON and WAIT registers look suspiciously similar.
>>>>>
>>>>> As far as I understand the "old" SoCs (VRX200 and earlier) don't have
>>>>> a built-in ECC engine. This seems to have changed with ARX300 though
>>>>> (again, AFAIK).
>>>>>
>>>>> A bit of lineage on these SoCs (initially these were developed by
>>>>> Infineon. Lantiq then started as an Infineon spin-off in 2009 and
>>>>> was then acquired by Intel in 2015):
>>>>> - Danube
>>>>> - ARX100 from 2008/2009
>>>>> - VRX200 from 2009/2010
>>>>> - ARX300 from 2014
>>>>> - GRX350 from 2015/2016
>>>>> - GRX550 from 2017
>>>>> - and now finally: LGM from 2020 (est.)
>>>>>
>>>>> The existing xway_nand driver supports the Danube, ARX100 and VRX200
>>>>> SoCs.
>>>> Lantiq upstreamed a driver for an older version of this IP core 8 years
>>>> ago, see here:
>>>> https://elixir.bootlin.com/linux/v5.5.6/source/drivers/mtd/nand/raw/xway_nand.c
>>>> It does not support DMA and ECC.
>>> Then let's just extend this driver to support the new features. Plus,
>> We do not have the platform to test also it's very old legacy driver .
> Well, if it's similar enough, we want to have one driver.

Thank you very much for the time and suggestions...

This in-review driver is written for a new nand flash controller IP
which is used in LGM.

>>> we'll be happy to have one more of the existing driver converted to
>>> ->exec_op() ;-).
>> I have completely adapted to ->exec_op() hook up to replace the legacy
>> call-back.
> I suspect porting what you've done to the xway driver shouldn't be too
> complicated.
Not ported from xway_nand.c driver , we have developed from the scratch
to make it work on
Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
different and most of the registers won't match.
if we port then it would be ugly and also what are the problem may occur
we do not know.

Regards
Vadivel

2020-04-16 12:29:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, Apr 16, 2020 at 3:03 PM Boris Brezillon
<[email protected]> wrote:
> On Thu, 16 Apr 2020 19:38:03 +0800
> "Ramuthevar, Vadivel MuruganX"
> <[email protected]> wrote:
> > On 16/4/2020 7:17 pm, Boris Brezillon wrote:
> > > On Thu, 16 Apr 2020 18:40:53 +0800
> > > "Ramuthevar, Vadivel MuruganX"
> > > <[email protected]> wrote:

...

> > There are different features involved and lines of code is more, if we
> > add new driver patches over xway-nand driver
>
> How about retro-fitting the xway logic into your driver then? I mean,
> adding a 100 lines of code to your driver to get rid of the 500+ lines
> we have in xway_nand.c is still a win.
>
> >
> > is completely looks ugly and it may disturb the existing functionality
> > as well since we don't have platform to validate:'(.
>
> How ugly? Can you show us? Maybe we can come with a solution to make it
> less ugly.
>
> As for the testing part, there are 4 scenarios:
>
> 1/ Your changes work perfectly fine on older platforms. Yay \o/!
> 2/ You break the xway driver and existing users notice it before this
> series gets merged. Now you found someone to validate your changes.
> 3/ You break the xway driver and none of the existing users notice it
> before the driver is merged, but they notice it afterwards. Too bad
> this happened after we've merged the driver, but now you've found
> someone to help you fix the problem :P.
> 4/ You break things for old platforms but no one ever complains about
> it, either because there's no users left or because they never
> update their kernels. In any case, that's no longer your problem.
> Someone will remove those old platforms one day and get rid of the
> unneeded code in the NAND driver.
>
> What's more likely to happen is #3 or #4, and I think the NAND
> maintainer would be fine with both.
>
> Note that the NAND subsystem is full of unmaintained legacy drivers, so
> every time we see someone who could help us get rid or update one of
> them we have to take this opportunity.

Don't we rather insist to have a MAINTAINERS record for new code to
avoid (or delay at least) the fate of the legacy drivers?

--
With Best Regards,
Andy Shevchenko

2020-04-16 17:44:29

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, 16 Apr 2020 19:38:03 +0800
"Ramuthevar, Vadivel MuruganX"
<[email protected]> wrote:

> On 16/4/2020 7:17 pm, Boris Brezillon wrote:
> > On Thu, 16 Apr 2020 18:40:53 +0800
> > "Ramuthevar, Vadivel MuruganX"
> > <[email protected]> wrote:
> >
> >>>>> we'll be happy to have one more of the existing driver converted to
> >>>>> ->exec_op() ;-).
> >>>> I have completely adapted to ->exec_op() hook up to replace the legacy
> >>>> call-back.
> >>> I suspect porting what you've done to the xway driver shouldn't be too
> >>> complicated.
> >> Not ported from xway_nand.c driver , we have developed from the scratch
> >> to make it work on
> >> Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
> >> different and most of the registers won't match.
> >> if we port then it would be ugly and also what are the problem may occur
> >> we do not know.
> > Sorry but IMO they look similar enough to try to merge them.
>
> Thanks! Boris, need suggestion from you since you are maintainer and
> also expertise on mtd-subsystem.

I *was* the maintainer :).

>
> There are different features involved and lines of code is more, if we
> add new driver patches over xway-nand driver

How about retro-fitting the xway logic into your driver then? I mean,
adding a 100 lines of code to your driver to get rid of the 500+ lines
we have in xway_nand.c is still a win.

>
> is completely looks ugly and it may disturb the existing functionality
> as well since we don't have platform to validate:'(.

How ugly? Can you show us? Maybe we can come with a solution to make it
less ugly.

As for the testing part, there are 4 scenarios:

1/ Your changes work perfectly fine on older platforms. Yay \o/!
2/ You break the xway driver and existing users notice it before this
series gets merged. Now you found someone to validate your changes.
3/ You break the xway driver and none of the existing users notice it
before the driver is merged, but they notice it afterwards. Too bad
this happened after we've merged the driver, but now you've found
someone to help you fix the problem :P.
4/ You break things for old platforms but no one ever complains about
it, either because there's no users left or because they never
update their kernels. In any case, that's no longer your problem.
Someone will remove those old platforms one day and get rid of the
unneeded code in the NAND driver.

What's more likely to happen is #3 or #4, and I think the NAND
maintainer would be fine with both.

Note that the NAND subsystem is full of unmaintained legacy drivers, so
every time we see someone who could help us get rid or update one of
them we have to take this opportunity.

2020-04-16 17:48:48

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, 16 Apr 2020 15:26:51 +0300
Andy Shevchenko <[email protected]> wrote:

> On Thu, Apr 16, 2020 at 3:03 PM Boris Brezillon
> <[email protected]> wrote:
> > On Thu, 16 Apr 2020 19:38:03 +0800
> > "Ramuthevar, Vadivel MuruganX"
> > <[email protected]> wrote:
> > > On 16/4/2020 7:17 pm, Boris Brezillon wrote:
> > > > On Thu, 16 Apr 2020 18:40:53 +0800
> > > > "Ramuthevar, Vadivel MuruganX"
> > > > <[email protected]> wrote:
>
> ...
>
> > > There are different features involved and lines of code is more, if we
> > > add new driver patches over xway-nand driver
> >
> > How about retro-fitting the xway logic into your driver then? I mean,
> > adding a 100 lines of code to your driver to get rid of the 500+ lines
> > we have in xway_nand.c is still a win.
> >
> > >
> > > is completely looks ugly and it may disturb the existing functionality
> > > as well since we don't have platform to validate:'(.
> >
> > How ugly? Can you show us? Maybe we can come with a solution to make it
> > less ugly.
> >
> > As for the testing part, there are 4 scenarios:
> >
> > 1/ Your changes work perfectly fine on older platforms. Yay \o/!
> > 2/ You break the xway driver and existing users notice it before this
> > series gets merged. Now you found someone to validate your changes.
> > 3/ You break the xway driver and none of the existing users notice it
> > before the driver is merged, but they notice it afterwards. Too bad
> > this happened after we've merged the driver, but now you've found
> > someone to help you fix the problem :P.
> > 4/ You break things for old platforms but no one ever complains about
> > it, either because there's no users left or because they never
> > update their kernels. In any case, that's no longer your problem.
> > Someone will remove those old platforms one day and get rid of the
> > unneeded code in the NAND driver.
> >
> > What's more likely to happen is #3 or #4, and I think the NAND
> > maintainer would be fine with both.
> >
> > Note that the NAND subsystem is full of unmaintained legacy drivers, so
> > every time we see someone who could help us get rid or update one of
> > them we have to take this opportunity.
>
> Don't we rather insist to have a MAINTAINERS record for new code to
> avoid (or delay at least) the fate of the legacy drivers?
>

Well, that's what we do for new drivers, but the xway driver has been
added in 2012 and the policy was not enforced at that time. BTW, that
goes for most of the legacy drivers in have in the NAND subsystems
(some of them even predate the git era).

To be clear, I just checked and there's no official maintainer for this
driver. Best option would be to Cc the original author and contributors
who proposed functional changes to the code, as well as the MIPS
maintainers (Xway is a MIPS platform).

2020-04-16 17:49:11

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Thu, Apr 16, 2020 at 2:40 PM Boris Brezillon
<[email protected]> wrote:
> On Thu, 16 Apr 2020 15:26:51 +0300
> Andy Shevchenko <[email protected]> wrote:
> > On Thu, Apr 16, 2020 at 3:03 PM Boris Brezillon
> > <[email protected]> wrote:
> > > On Thu, 16 Apr 2020 19:38:03 +0800
> > > Note that the NAND subsystem is full of unmaintained legacy drivers, so
> > > every time we see someone who could help us get rid or update one of
> > > them we have to take this opportunity.
> >
> > Don't we rather insist to have a MAINTAINERS record for new code to
> > avoid (or delay at least) the fate of the legacy drivers?
> >
>
> Well, that's what we do for new drivers, but the xway driver has been
> added in 2012 and the policy was not enforced at that time. BTW, that
> goes for most of the legacy drivers in have in the NAND subsystems
> (some of them even predate the git era).
>
> To be clear, I just checked and there's no official maintainer for this
> driver. Best option would be to Cc the original author and contributors
> who proposed functional changes to the code, as well as the MIPS
> maintainers (Xway is a MIPS platform).

A lot of the pre-acquisition code for lantiq was contributed by Hauke
Mehrtens and John Crispin. There was an intermediate generation of
MIPS SoCs with patches posted for review by Intel in 2018 (presumably
by the same organizatiob), but those were never resubmitted after v2
and never merged:

https://lore.kernel.org/linux-mips/[email protected]/

Arnd

2020-04-16 20:34:38

by John Crispin

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On 16.04.20 15:20, Arnd Bergmann wrote:
> On Thu, Apr 16, 2020 at 2:40 PM Boris Brezillon
> <[email protected]> wrote:
>> On Thu, 16 Apr 2020 15:26:51 +0300
>> Andy Shevchenko <[email protected]> wrote:
>>> On Thu, Apr 16, 2020 at 3:03 PM Boris Brezillon
>>> <[email protected]> wrote:
>>>> On Thu, 16 Apr 2020 19:38:03 +0800
>>>> Note that the NAND subsystem is full of unmaintained legacy drivers, so
>>>> every time we see someone who could help us get rid or update one of
>>>> them we have to take this opportunity.
>>>
>>> Don't we rather insist to have a MAINTAINERS record for new code to
>>> avoid (or delay at least) the fate of the legacy drivers?
>>>
>>
>> Well, that's what we do for new drivers, but the xway driver has been
>> added in 2012 and the policy was not enforced at that time. BTW, that
>> goes for most of the legacy drivers in have in the NAND subsystems
>> (some of them even predate the git era).
>>
>> To be clear, I just checked and there's no official maintainer for this
>> driver. Best option would be to Cc the original author and contributors
>> who proposed functional changes to the code, as well as the MIPS
>> maintainers (Xway is a MIPS platform).
>
> A lot of the pre-acquisition code for lantiq was contributed by Hauke
> Mehrtens and John Crispin. There was an intermediate generation of
> MIPS SoCs with patches posted for review by Intel in 2018 (presumably
> by the same organizatiob), but those were never resubmitted after v2
> and never merged:
>
> https://lore.kernel.org/linux-mips/[email protected]/
>
> Arnd
>

Hi,

the legacy Mips SoC had a External Bus Unit (EBU), similar to an
Intel/Hitachi style bus. It was used back then for lots of things,
sometimes driving Leds via 74* latches, Arcadyan used it for external
reset lines and very rarely was it used for nand.

Looking at this series and comparing it with xway_nand.c we see that the
init sequence is near identical. Best guess is that the mountain lion
uses an internal block very similar to what the legacy mips silicon used
just in a newer generation and the new proposed driver is more feature
complete.

If this is the case ideally the xway_nand.c is dropped and that silicon
is made working with the newer driver. Chances are that we just need to
add a "support less features" style flag.

Unfortunately i no longer have the evalkit for the Mips SoCs.

John

2020-04-16 20:43:32

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Vadivel, Hi Boris,

On Thu, Apr 16, 2020 at 1:57 PM Boris Brezillon
<[email protected]> wrote:
[...]
> As for the testing part, there are 4 scenarios:
>
> 1/ Your changes work perfectly fine on older platforms. Yay \o/!
this would be awesome \o/

> 2/ You break the xway driver and existing users notice it before this
> series gets merged. Now you found someone to validate your changes.
The xway_nand driver is still used by 9 (or so) boards in OpenWrt: one
Danube, one ARX100 and the other 7 with VRX200
I can be the person to find out whether your changes break one of
these boards with VRX200 SoC and 128MB SLC NAND (and software ECC
since AFAIK this SoC doesn't have a hardware ECC engine).


Best regards,
Martin

Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Andy,

On 16/4/2020 7:32 pm, Andy Shevchenko wrote:
> On Thu, Apr 16, 2020 at 01:17:25PM +0200, Boris Brezillon wrote:
>> On Thu, 16 Apr 2020 18:40:53 +0800
>> "Ramuthevar, Vadivel MuruganX"
>> <[email protected]> wrote:
>>
>>>>>> we'll be happy to have one more of the existing driver converted to
>>>>>> ->exec_op() ;-).
>>>>> I have completely adapted to ->exec_op() hook up to replace the legacy
>>>>> call-back.
>>>> I suspect porting what you've done to the xway driver shouldn't be too
>>>> complicated.
>>> Not ported from xway_nand.c driver , we have developed from the scratch
>>> to make it work on
>>> Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
>>> different and most of the registers won't match.
>>> if we port then it would be ugly and also what are the problem may occur
>>> we do not know.
>> Sorry but IMO they look similar enough to try to merge them.
> I agree. I tried to convince them internally... but here we are.

Agreed,  will do the changes as Boris and Martin suggested, Thanks!

Regards
Vadivel

Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Boris,

On 16/4/2020 7:57 pm, Boris Brezillon wrote:
> On Thu, 16 Apr 2020 19:38:03 +0800
> "Ramuthevar, Vadivel MuruganX"
> <[email protected]> wrote:
>
>> On 16/4/2020 7:17 pm, Boris Brezillon wrote:
>>> On Thu, 16 Apr 2020 18:40:53 +0800
>>> "Ramuthevar, Vadivel MuruganX"
>>> <[email protected]> wrote:
>>>
>>>>>>> we'll be happy to have one more of the existing driver converted to
>>>>>>> ->exec_op() ;-).
>>>>>> I have completely adapted to ->exec_op() hook up to replace the legacy
>>>>>> call-back.
>>>>> I suspect porting what you've done to the xway driver shouldn't be too
>>>>> complicated.
>>>> Not ported from xway_nand.c driver , we have developed from the scratch
>>>> to make it work on
>>>> Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
>>>> different and most of the registers won't match.
>>>> if we port then it would be ugly and also what are the problem may occur
>>>> we do not know.
>>> Sorry but IMO they look similar enough to try to merge them.
>> Thanks! Boris, need suggestion from you since you are maintainer and
>> also expertise on mtd-subsystem.
> I *was* the maintainer :).
>
>> There are different features involved and lines of code is more, if we
>> add new driver patches over xway-nand driver
> How about retro-fitting the xway logic into your driver then? I mean,
> adding a 100 lines of code to your driver to get rid of the 500+ lines
> we have in xway_nand.c is still a win.
>
>> is completely looks ugly and it may disturb the existing functionality
>> as well since we don't have platform to validate:'(.
> How ugly? Can you show us? Maybe we can come with a solution to make it
> less ugly.
>
> As for the testing part, there are 4 scenarios:
>
> 1/ Your changes work perfectly fine on older platforms. Yay \o/!
> 2/ You break the xway driver and existing users notice it before this
> series gets merged. Now you found someone to validate your changes.
> 3/ You break the xway driver and none of the existing users notice it
> before the driver is merged, but they notice it afterwards. Too bad
> this happened after we've merged the driver, but now you've found
> someone to help you fix the problem :P.
> 4/ You break things for old platforms but no one ever complains about
> it, either because there's no users left or because they never
> update their kernels. In any case, that's no longer your problem.
> Someone will remove those old platforms one day and get rid of the
> unneeded code in the NAND driver.
>
> What's more likely to happen is #3 or #4, and I think the NAND
> maintainer would be fine with both.
>
> Note that the NAND subsystem is full of unmaintained legacy drivers, so
> every time we see someone who could help us get rid or update one of
> them we have to take this opportunity.
Agreed!, Thank you very much for the suggestions and clear inputs.
To proceed further, can you please share your inputs to dividing the tasks
and patches to be sent if possible.

Regards
Vadivel

2020-04-17 07:05:22

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

On Fri, 17 Apr 2020 13:21:39 +0800
"Ramuthevar, Vadivel MuruganX"
<[email protected]> wrote:

> Hi Boris,
>
> On 16/4/2020 7:57 pm, Boris Brezillon wrote:
> > On Thu, 16 Apr 2020 19:38:03 +0800
> > "Ramuthevar, Vadivel MuruganX"
> > <[email protected]> wrote:
> >
> >> On 16/4/2020 7:17 pm, Boris Brezillon wrote:
> >>> On Thu, 16 Apr 2020 18:40:53 +0800
> >>> "Ramuthevar, Vadivel MuruganX"
> >>> <[email protected]> wrote:
> >>>
> >>>>>>> we'll be happy to have one more of the existing driver converted to
> >>>>>>> ->exec_op() ;-).
> >>>>>> I have completely adapted to ->exec_op() hook up to replace the legacy
> >>>>>> call-back.
> >>>>> I suspect porting what you've done to the xway driver shouldn't be too
> >>>>> complicated.
> >>>> Not ported from xway_nand.c driver , we have developed from the scratch
> >>>> to make it work on
> >>>> Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
> >>>> different and most of the registers won't match.
> >>>> if we port then it would be ugly and also what are the problem may occur
> >>>> we do not know.
> >>> Sorry but IMO they look similar enough to try to merge them.
> >> Thanks! Boris, need suggestion from you since you are maintainer and
> >> also expertise on mtd-subsystem.
> > I *was* the maintainer :).
> >
> >> There are different features involved and lines of code is more, if we
> >> add new driver patches over xway-nand driver
> > How about retro-fitting the xway logic into your driver then? I mean,
> > adding a 100 lines of code to your driver to get rid of the 500+ lines
> > we have in xway_nand.c is still a win.
> >
> >> is completely looks ugly and it may disturb the existing functionality
> >> as well since we don't have platform to validate:'(.
> > How ugly? Can you show us? Maybe we can come with a solution to make it
> > less ugly.
> >
> > As for the testing part, there are 4 scenarios:
> >
> > 1/ Your changes work perfectly fine on older platforms. Yay \o/!
> > 2/ You break the xway driver and existing users notice it before this
> > series gets merged. Now you found someone to validate your changes.
> > 3/ You break the xway driver and none of the existing users notice it
> > before the driver is merged, but they notice it afterwards. Too bad
> > this happened after we've merged the driver, but now you've found
> > someone to help you fix the problem :P.
> > 4/ You break things for old platforms but no one ever complains about
> > it, either because there's no users left or because they never
> > update their kernels. In any case, that's no longer your problem.
> > Someone will remove those old platforms one day and get rid of the
> > unneeded code in the NAND driver.
> >
> > What's more likely to happen is #3 or #4, and I think the NAND
> > maintainer would be fine with both.
> >
> > Note that the NAND subsystem is full of unmaintained legacy drivers, so
> > every time we see someone who could help us get rid or update one of
> > them we have to take this opportunity.
> Agreed!, Thank you very much for the suggestions and clear inputs.
> To proceed further, can you please share your inputs to dividing the tasks
> and patches to be sent if possible.

Let's start with the new version you were about to post. We'll see how
we can merge both drivers based on that.

Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Boris,

On 17/4/2020 3:02 pm, Boris Brezillon wrote:
> On Fri, 17 Apr 2020 13:21:39 +0800
> "Ramuthevar, Vadivel MuruganX"
> <[email protected]> wrote:
>
>> Hi Boris,
>>
>> On 16/4/2020 7:57 pm, Boris Brezillon wrote:
>>> On Thu, 16 Apr 2020 19:38:03 +0800
>>> "Ramuthevar, Vadivel MuruganX"
>>> <[email protected]> wrote:
>>>
>>>> On 16/4/2020 7:17 pm, Boris Brezillon wrote:
>>>>> On Thu, 16 Apr 2020 18:40:53 +0800
>>>>> "Ramuthevar, Vadivel MuruganX"
>>>>> <[email protected]> wrote:
>>>>>
>>>>>>>>> we'll be happy to have one more of the existing driver converted to
>>>>>>>>> ->exec_op() ;-).
>>>>>>>> I have completely adapted to ->exec_op() hook up to replace the legacy
>>>>>>>> call-back.
>>>>>>> I suspect porting what you've done to the xway driver shouldn't be too
>>>>>>> complicated.
>>>>>> Not ported from xway_nand.c driver , we have developed from the scratch
>>>>>> to make it work on
>>>>>> Intel LGM SoC , it's new x86 ATOM based SoC, IP itself completely
>>>>>> different and most of the registers won't match.
>>>>>> if we port then it would be ugly and also what are the problem may occur
>>>>>> we do not know.
>>>>> Sorry but IMO they look similar enough to try to merge them.
>>>> Thanks! Boris, need suggestion from you since you are maintainer and
>>>> also expertise on mtd-subsystem.
>>> I *was* the maintainer :).
>>>
>>>> There are different features involved and lines of code is more, if we
>>>> add new driver patches over xway-nand driver
>>> How about retro-fitting the xway logic into your driver then? I mean,
>>> adding a 100 lines of code to your driver to get rid of the 500+ lines
>>> we have in xway_nand.c is still a win.
>>>
>>>> is completely looks ugly and it may disturb the existing functionality
>>>> as well since we don't have platform to validate:'(.
>>> How ugly? Can you show us? Maybe we can come with a solution to make it
>>> less ugly.
>>>
>>> As for the testing part, there are 4 scenarios:
>>>
>>> 1/ Your changes work perfectly fine on older platforms. Yay \o/!
>>> 2/ You break the xway driver and existing users notice it before this
>>> series gets merged. Now you found someone to validate your changes.
>>> 3/ You break the xway driver and none of the existing users notice it
>>> before the driver is merged, but they notice it afterwards. Too bad
>>> this happened after we've merged the driver, but now you've found
>>> someone to help you fix the problem :P.
>>> 4/ You break things for old platforms but no one ever complains about
>>> it, either because there's no users left or because they never
>>> update their kernels. In any case, that's no longer your problem.
>>> Someone will remove those old platforms one day and get rid of the
>>> unneeded code in the NAND driver.
>>>
>>> What's more likely to happen is #3 or #4, and I think the NAND
>>> maintainer would be fine with both.
>>>
>>> Note that the NAND subsystem is full of unmaintained legacy drivers, so
>>> every time we see someone who could help us get rid or update one of
>>> them we have to take this opportunity.
>> Agreed!, Thank you very much for the suggestions and clear inputs.
>> To proceed further, can you please share your inputs to dividing the tasks
>> and patches to be sent if possible.
> Let's start with the new version you were about to post. We'll see how
> we can merge both drivers based on that.

Thank you very much for the review comments and inputs , will post the
patches soon.

Regards
Vadivel

Subject: Re: [PATCH v1 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC

Hi Arnd,

On 16/4/2020 9:20 pm, Arnd Bergmann wrote:
> On Thu, Apr 16, 2020 at 2:40 PM Boris Brezillon
> <[email protected]> wrote:
>> On Thu, 16 Apr 2020 15:26:51 +0300
>> Andy Shevchenko <[email protected]> wrote:
>>> On Thu, Apr 16, 2020 at 3:03 PM Boris Brezillon
>>> <[email protected]> wrote:
>>>> On Thu, 16 Apr 2020 19:38:03 +0800
>>>> Note that the NAND subsystem is full of unmaintained legacy drivers, so
>>>> every time we see someone who could help us get rid or update one of
>>>> them we have to take this opportunity.
>>> Don't we rather insist to have a MAINTAINERS record for new code to
>>> avoid (or delay at least) the fate of the legacy drivers?
>>>
>> Well, that's what we do for new drivers, but the xway driver has been
>> added in 2012 and the policy was not enforced at that time. BTW, that
>> goes for most of the legacy drivers in have in the NAND subsystems
>> (some of them even predate the git era).
>>
>> To be clear, I just checked and there's no official maintainer for this
>> driver. Best option would be to Cc the original author and contributors
>> who proposed functional changes to the code, as well as the MIPS
>> maintainers (Xway is a MIPS platform).
> A lot of the pre-acquisition code for lantiq was contributed by Hauke
> Mehrtens and John Crispin. There was an intermediate generation of
> MIPS SoCs with patches posted for review by Intel in 2018 (presumably
> by the same organizatiob), but those were never resubmitted after v2
> and never merged:
>
> https://lore.kernel.org/linux-mips/[email protected]/
Thank you for reviewing our patches and your time...
The above patches for different SoC which is MIPS based, but whatever
the patch is sent by me is Intel X86 ATOM based LGM SoC.

Regards
Vadivel
>
> Arnd