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

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.

NAND controller also supports in-built HW ECC engine.

NAND controller driver implements ->exec_op() to replace legacy hooks,
these specific call-back method to execute NAND operations.

Thanks Miquel, Boris, Andy, Arnd and Rob for the review comments and suggestions.
---
Resend-v15:
- Rebased to mtd/for-5.10
v15:
- Address Miquel review comments update
- add common helper function for status check for
ebu_nand_waitrdy()
v14:
- Address Andy's review comments
- align the headers and revome Duplicates
- replcace numerical const values by HZ_PER_MHZ and USEC_PER_SEC
defined macros
- add dev_err_probe() api instead of legacy err check
- add get_unaligned_le32() api instead of manual endiness
- remove redudent check
- split the lines logically in between and add require spaces
v13:
- Address Miquel Raynal review comments
- update the return type with variable 'ret'
- handle err check statement properly
- change the naming convention aligned with recently changed the naming
around the data interface
data structure and function names
- replace by div 8 instead of <<4 in ecc calculation better code readability
- handle check_only properly like existing drivers
v12-resend:
- No Change
v12:
- address Miquel Raynal review comments update
- add/modify the comments for better understanding
- handle the check_only variable
- update the ecc function based on the existing drivers
- add newline
- verify that mtd->name is set after nand_set_flash_node()
- add the check WARN_ON(ret);
v11-resend:
- Rebase to v5.8-rc1
v11:
- No Change
v10:
- No Change
v9:
- No change
v8:
- fix the kbuild bot warnings
- correct the typo's
v7:
- indentation issue is fixed
- add error check for retrieve the resource from dt
v6:
- update EBU_ADDR_SELx register base value build it from DT
- Add tabs in in Kconfig
v5:
- replace by 'HSNAND_CLE_OFFS | HSNAND_CS_OFFS' to NAND_WRITE_CMD and NAND_WRITE_ADDR
- remove the unused macros
- update EBU_ADDR_MASK(x) macro
- update the EBU_ADDR_SELx register values to be written
v4:
- add ebu_nand_cs structure for multiple-CS support
- mask/offset encoding for 0x51 value
- update macro HSNAND_CTL_ENABLE_ECC
- drop the op argument and un-used macros.
- updated the datatype and macros
- add function disable nand module
- remove ebu_host->dma_rx = NULL;
- rename MMIO address range variables to ebu and hsnand
- implement ->setup_data_interface()
- update label err_cleanup_nand and err_cleanup_dma
- add return value check in the nand_remove function
- add/remove tabs and spaces as per coding standard
- encoded CS ids by reg property
v3:
- Add depends on MACRO in Kconfig
- file name update in Makefile
- file name update to intel-nand-controller
- modification of MACRO divided like EBU, HSNAND and NAND
- add NAND_ALE_OFFS, NAND_CLE_OFFS and NAND_CS_OFFS
- rename lgm_ to ebu_ and _va suffix is removed in the whole file
- rename structure and varaibles as per review comments.
- remove lgm_read_byte(), lgm_dev_ready() and cmd_ctrl() un-used function
- update in exec_op() as per review comments
- rename function lgm_dma_exit() by lgm_dma_cleanup()
- hardcoded magic value for base and offset replaced by MACRO defined
- mtd_device_unregister() + nand_cleanup() instead of nand_release()
v2:
- implement the ->exec_op() to replaces the legacy hook-up.
- update the commit message
- add MIPS maintainers and xway_nand driver author in CC
v1:
- initial version


dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC
---
resend-v15:
- No change
v15:
- No change
v14:
- No change
v13:
- No change
v12-Resend:
- No Change
v12:
- No change
v11-resend:
- No change
v11:
- Fixed the compatible issue with example
10:
- fix bot errors
v9:
- Rob's review comments address
- dual licensed
- compatible change
- add reg-names
- drop clock-names and clock-cells
- correct typo's
v8:
No change
v7:
- Rob's review comments addressed
- dt-schema build issue fixed with upgraded dt-schema
v6:
- Rob's review comments addressed in YAML file
- add addr_sel0 and addr_sel1 reg-names in YAML example
v5:
- add the example in YAML file
v4:
- No change
v3:
- No change
v2:
YAML compatible string update to intel, lgm-nand-controller
v1:
- initial version

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

.../devicetree/bindings/mtd/intel,lgm-nand.yaml | 99 +++
drivers/mtd/nand/raw/Kconfig | 8 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/intel-nand-controller.c | 734 +++++++++++++++++++++
4 files changed, 842 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
create mode 100644 drivers/mtd/nand/raw/intel-nand-controller.c

--
2.11.0


Subject: [RESENDPATCH v15 1/2] dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC

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]>
Reviewed-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/mtd/intel,lgm-nand.yaml | 99 ++++++++++++++++++++++
1 file changed, 99 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..313daec4d783
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%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: 6
+
+ reg-names:
+ items:
+ - const: ebunand
+ - const: hsnand
+ - const: nand_cs0
+ - const: nand_cs1
+ - const: addr_sel0
+ - const: addr_sel1
+
+ clocks:
+ maxItems: 1
+
+ dmas:
+ maxItems: 2
+
+ dma-names:
+ items:
+ - const: tx
+ - const: rx
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^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
+ - reg-names
+ - clocks
+ - dmas
+ - dma-names
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ nand-controller@e0f00000 {
+ compatible = "intel,lgm-nand";
+ reg = <0xe0f00000 0x100>,
+ <0xe1000000 0x300>,
+ <0xe1400000 0x8000>,
+ <0xe1c00000 0x1000>,
+ <0x17400000 0x4>,
+ <0x17c00000 0x4>;
+ reg-names = "ebunand", "hsnand", "nand_cs0", "nand_cs1",
+ "addr_sel0", "addr_sel1";
+ clocks = <&cgu0 125>;
+ dmas = <&dma0 8>, <&dma0 9>;
+ dma-names = "tx", "rx";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+ reg = <0>;
+ nand-ecc-mode = "hw";
+ };
+ };
+
+...
--
2.11.0

Subject: [RESENDPATCH v15 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.

NAND controller driver implements ->exec_op() to replace legacy hooks,
these specific call-back method to execute NAND operations.

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

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 6c46f25b57e2..1b3690fd08dc 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -462,6 +462,14 @@ config MTD_NAND_ARASAN
Enables the driver for the Arasan NAND flash controller on
Zynq Ultrascale+ MPSoC.

+config MTD_NAND_INTEL_LGM
+ tristate "Support for NAND controller on Intel LGM SoC"
+ depends on OF || COMPILE_TEST
+ depends on HAS_IOMEM
+ help
+ Enables support for NAND Flash chips on Intel's LGM SoC.
+ NAND flash controller 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 2930f5b9015d..9e6037363fc6 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -58,6 +58,7 @@ 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_ARASAN) += arasan-nand-controller.o
+obj-$(CONFIG_MTD_NAND_INTEL_LGM) += intel-nand-controller.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-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
new file mode 100644
index 000000000000..0aefc441c7d5
--- /dev/null
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -0,0 +1,734 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (c) 2020 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/err.h>
+#include <linux/init.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand.h>
+
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <asm/unaligned.h>
+
+#define EBU_CLC 0x000
+#define EBU_CLC_RST 0x00000000u
+
+#define EBU_ADDR_SEL(n) (0x020 + (n) * 4)
+/* 5 bits 26:22 included for comparison in the ADDR_SELx */
+#define EBU_ADDR_MASK(x) ((x) << 4)
+#define EBU_ADDR_SEL_REGEN 0x1
+
+#define EBU_BUSCON(n) (0x060 + (n) * 4)
+#define EBU_BUSCON_CMULT_V4 0x1
+#define EBU_BUSCON_RECOVC(n) ((n) << 2)
+#define EBU_BUSCON_HOLDC(n) ((n) << 4)
+#define EBU_BUSCON_WAITRDC(n) ((n) << 6)
+#define EBU_BUSCON_WAITWRC(n) ((n) << 8)
+#define EBU_BUSCON_BCGEN_CS 0x0
+#define EBU_BUSCON_SETUP_EN BIT(22)
+#define EBU_BUSCON_ALEC 0xC000
+
+#define EBU_CON 0x0B0
+#define EBU_CON_NANDM_EN BIT(0)
+#define EBU_CON_NANDM_DIS 0x0
+#define EBU_CON_CSMUX_E_EN BIT(1)
+#define EBU_CON_ALE_P_LOW BIT(2)
+#define EBU_CON_CLE_P_LOW BIT(3)
+#define EBU_CON_CS_P_LOW BIT(4)
+#define EBU_CON_SE_P_LOW BIT(5)
+#define EBU_CON_WP_P_LOW BIT(6)
+#define EBU_CON_PRE_P_LOW BIT(7)
+#define EBU_CON_IN_CS_S(n) ((n) << 8)
+#define EBU_CON_OUT_CS_S(n) ((n) << 10)
+#define EBU_CON_LAT_EN_CS_P ((0x3D) << 18)
+
+#define EBU_WAIT 0x0B4
+#define EBU_WAIT_RDBY BIT(0)
+#define EBU_WAIT_WR_C BIT(3)
+
+#define HSNAND_CTL1 0x110
+#define HSNAND_CTL1_ADDR_SHIFT 24
+
+#define HSNAND_CTL2 0x114
+#define HSNAND_CTL2_ADDR_SHIFT 8
+#define HSNAND_CTL2_CYC_N_V5 (0x2 << 16)
+
+#define HSNAND_INT_MSK_CTL 0x124
+#define HSNAND_INT_MSK_CTL_WR_C BIT(4)
+
+#define HSNAND_INT_STA 0x128
+#define HSNAND_INT_STA_WR_C BIT(4)
+
+#define HSNAND_CTL 0x130
+#define HSNAND_CTL_ENABLE_ECC BIT(0)
+#define HSNAND_CTL_GO BIT(2)
+#define HSNAND_CTL_CE_SEL_CS(n) BIT(3 + (n))
+#define HSNAND_CTL_RW_READ 0x0
+#define HSNAND_CTL_RW_WRITE BIT(10)
+#define HSNAND_CTL_ECC_OFF_V8TH BIT(11)
+#define HSNAND_CTL_CKFF_EN 0x0
+#define HSNAND_CTL_MSG_EN BIT(17)
+
+#define HSNAND_PARA0 0x13c
+#define HSNAND_PARA0_PAGE_V8192 0x3
+#define HSNAND_PARA0_PIB_V256 (0x3 << 4)
+#define HSNAND_PARA0_BYP_EN_NP 0x0
+#define HSNAND_PARA0_BYP_DEC_NP 0x0
+#define HSNAND_PARA0_TYPE_ONFI BIT(18)
+#define HSNAND_PARA0_ADEP_EN BIT(21)
+
+#define HSNAND_CMSG_0 0x150
+#define HSNAND_CMSG_1 0x154
+
+#define HSNAND_ALE_OFFS BIT(2)
+#define HSNAND_CLE_OFFS BIT(3)
+#define HSNAND_CS_OFFS BIT(4)
+
+#define HSNAND_ECC_OFFSET 0x008
+
+#define NAND_DATA_IFACE_CHECK_ONLY -1
+
+#define MAX_CS 2
+
+#define HZ_PER_MHZ 1000000L
+#define USEC_PER_SEC 1000000L
+
+struct ebu_nand_cs {
+ void __iomem *chipaddr;
+ dma_addr_t nand_pa;
+ u32 addr_sel;
+};
+
+struct ebu_nand_controller {
+ struct nand_controller controller;
+ struct nand_chip chip;
+ struct device *dev;
+ void __iomem *ebu;
+ void __iomem *hsnand;
+ struct dma_chan *dma_tx;
+ struct dma_chan *dma_rx;
+ struct completion dma_access_complete;
+ unsigned long clk_rate;
+ struct clk *clk;
+ u32 nd_para0;
+ u8 cs_num;
+ struct ebu_nand_cs cs[MAX_CS];
+};
+
+static inline struct ebu_nand_controller *nand_to_ebu(struct nand_chip *chip)
+{
+ return container_of(chip, struct ebu_nand_controller, chip);
+}
+
+static int ebu_nand_waitrdy(struct nand_chip *chip, unsigned int time_out)
+{
+ struct ebu_nand_controller *ctrl = nand_to_ebu(chip);
+ u32 status;
+
+ return readl_poll_timeout(ctrl->ebu + EBU_WAIT, status,
+ (status & EBU_WAIT_RDBY) ||
+ (status & EBU_WAIT_WR_C), 20, time_out);
+}
+
+static u8 ebu_nand_readb(struct nand_chip *chip)
+{
+ struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
+ u8 cs_num = ebu_host->cs_num;
+ u8 val;
+
+ val = readb(ebu_host->cs[cs_num].chipaddr + HSNAND_CS_OFFS);
+ ebu_nand_waitrdy(chip, 1000);
+ return val;
+}
+
+static void ebu_nand_writeb(struct nand_chip *chip, u32 offset, u8 value)
+{
+ struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
+ u8 cs_num = ebu_host->cs_num;
+
+ writeb(value, ebu_host->cs[cs_num].chipaddr + offset);
+ ebu_nand_waitrdy(chip, 1000);
+}
+
+static void ebu_read_buf(struct nand_chip *chip, u_char *buf, unsigned int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++)
+ buf[i] = ebu_nand_readb(chip);
+}
+
+static void ebu_write_buf(struct nand_chip *chip, const u_char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++)
+ ebu_nand_writeb(chip, HSNAND_CS_OFFS, buf[i]);
+}
+
+static void ebu_nand_disable(struct nand_chip *chip)
+{
+ struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
+
+ writel(0, ebu_host->ebu + EBU_CON);
+}
+
+static void ebu_select_chip(struct nand_chip *chip)
+{
+ struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
+ void __iomem *nand_con = ebu_host->ebu + EBU_CON;
+ u32 cs = ebu_host->cs_num;
+
+ writel(EBU_CON_NANDM_EN | EBU_CON_CSMUX_E_EN | EBU_CON_CS_P_LOW |
+ EBU_CON_SE_P_LOW | EBU_CON_WP_P_LOW | EBU_CON_PRE_P_LOW |
+ EBU_CON_IN_CS_S(cs) | EBU_CON_OUT_CS_S(cs) |
+ EBU_CON_LAT_EN_CS_P, nand_con);
+}
+
+static void ebu_nand_setup_timing(struct ebu_nand_controller *ctrl,
+ const struct nand_sdr_timings *timings)
+{
+ unsigned int rate = clk_get_rate(ctrl->clk) / HZ_PER_MHZ;
+ unsigned int period = DIV_ROUND_UP(USEC_PER_SEC, rate);
+ u32 trecov, thold, twrwait, trdwait;
+ u32 reg = 0;
+
+ trecov = DIV_ROUND_UP(max(timings->tREA_max, timings->tREH_min),
+ period);
+ reg |= EBU_BUSCON_RECOVC(trecov);
+
+ thold = DIV_ROUND_UP(max(timings->tDH_min, timings->tDS_min), period);
+ reg |= EBU_BUSCON_HOLDC(thold);
+
+ trdwait = DIV_ROUND_UP(max(timings->tRC_min, timings->tREH_min),
+ period);
+ reg |= EBU_BUSCON_WAITRDC(trdwait);
+
+ twrwait = DIV_ROUND_UP(max(timings->tWC_min, timings->tWH_min), period);
+ reg |= EBU_BUSCON_WAITWRC(twrwait);
+
+ reg |= EBU_BUSCON_CMULT_V4 | EBU_BUSCON_BCGEN_CS | EBU_BUSCON_ALEC |
+ EBU_BUSCON_SETUP_EN;
+
+ writel(reg, ctrl->ebu + EBU_BUSCON(ctrl->cs_num));
+}
+
+static int ebu_nand_set_timings(struct nand_chip *chip, int csline,
+ const struct nand_interface_config *conf)
+{
+ struct ebu_nand_controller *ctrl = nand_to_ebu(chip);
+ const struct nand_sdr_timings *timings;
+
+ timings = nand_get_sdr_timings(conf);
+ if (IS_ERR(timings))
+ return PTR_ERR(timings);
+
+ if (csline == NAND_DATA_IFACE_CHECK_ONLY)
+ return 0;
+
+ ebu_nand_setup_timing(ctrl, timings);
+
+ return 0;
+}
+
+static int ebu_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 = HSNAND_ECC_OFFSET;
+ oobregion->length = chip->ecc.total;
+
+ return 0;
+}
+
+static int ebu_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 + HSNAND_ECC_OFFSET;
+ oobregion->length = mtd->oobsize - oobregion->offset;
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops ebu_nand_ooblayout_ops = {
+ .ecc = ebu_nand_ooblayout_ecc,
+ .free = ebu_nand_ooblayout_free,
+};
+
+static void ebu_dma_rx_callback(void *cookie)
+{
+ struct ebu_nand_controller *ebu_host = cookie;
+
+ dmaengine_terminate_async(ebu_host->dma_rx);
+
+ complete(&ebu_host->dma_access_complete);
+}
+
+static void ebu_dma_tx_callback(void *cookie)
+{
+ struct ebu_nand_controller *ebu_host = cookie;
+
+ dmaengine_terminate_async(ebu_host->dma_tx);
+
+ complete(&ebu_host->dma_access_complete);
+}
+
+static int ebu_dma_start(struct ebu_nand_controller *ebu_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 = ebu_host->dma_rx;
+ dma_completion = &ebu_host->dma_access_complete;
+ callback = ebu_dma_rx_callback;
+ } else {
+ chan = ebu_host->dma_tx;
+ dma_completion = &ebu_host->dma_access_complete;
+ callback = ebu_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(ebu_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 = ebu_host;
+ cookie = tx->tx_submit(tx);
+
+ ret = dma_submit_error(cookie);
+ if (ret) {
+ dev_err(ebu_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(ebu_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(ebu_host->dev, buf_dma, len, dir);
+
+ return ret;
+}
+
+static void ebu_nand_trigger(struct ebu_nand_controller *ebu_host,
+ int page, u32 cmd)
+{
+ unsigned int val;
+
+ val = cmd | (page & 0xFF) << HSNAND_CTL1_ADDR_SHIFT;
+ writel(val, ebu_host->hsnand + HSNAND_CTL1);
+ val = (page & 0xFFFF00) >> 8 | HSNAND_CTL2_CYC_N_V5;
+ writel(val, ebu_host->hsnand + HSNAND_CTL2);
+
+ writel(ebu_host->nd_para0, ebu_host->hsnand + HSNAND_PARA0);
+
+ /* clear first, will update later */
+ writel(0xFFFFFFFF, ebu_host->hsnand + HSNAND_CMSG_0);
+ writel(0xFFFFFFFF, ebu_host->hsnand + HSNAND_CMSG_1);
+
+ writel(HSNAND_INT_MSK_CTL_WR_C,
+ ebu_host->hsnand + HSNAND_INT_MSK_CTL);
+
+ if (!cmd)
+ val = HSNAND_CTL_RW_READ;
+ else
+ val = HSNAND_CTL_RW_WRITE;
+
+ writel(HSNAND_CTL_MSG_EN | HSNAND_CTL_CKFF_EN |
+ HSNAND_CTL_ECC_OFF_V8TH | HSNAND_CTL_CE_SEL_CS(ebu_host->cs_num) |
+ HSNAND_CTL_ENABLE_ECC | HSNAND_CTL_GO | val,
+ ebu_host->hsnand + HSNAND_CTL);
+}
+
+static int ebu_nand_read_page_hwecc(struct nand_chip *chip, u8 *buf,
+ int oob_required, int page)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
+ int ret, reg_data;
+
+ ebu_nand_trigger(ebu_host, page, NAND_CMD_READ0);
+
+ ret = ebu_dma_start(ebu_host, DMA_DEV_TO_MEM, buf, mtd->writesize);
+ if (ret)
+ return ret;
+
+ if (oob_required)
+ chip->ecc.read_oob(chip, page);
+
+ reg_data = readl(ebu_host->hsnand + HSNAND_CTL);
+ reg_data &= ~HSNAND_CTL_GO;
+ writel(reg_data, ebu_host->hsnand + HSNAND_CTL);
+
+ return 0;
+}
+
+static int ebu_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 ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
+ void __iomem *int_sta = ebu_host->hsnand + HSNAND_INT_STA;
+ int reg_data, ret, val;
+ u32 reg;
+
+ ebu_nand_trigger(ebu_host, page, NAND_CMD_SEQIN);
+
+ ret = ebu_dma_start(ebu_host, DMA_MEM_TO_DEV, buf, mtd->writesize);
+ if (ret)
+ return ret;
+
+ if (oob_required) {
+ reg = get_unaligned_le32(chip->oob_poi);
+ writel(reg, ebu_host->hsnand + HSNAND_CMSG_0);
+
+ reg = get_unaligned_le32(chip->oob_poi + 4);
+ writel(reg, ebu_host->hsnand + HSNAND_CMSG_1);
+ }
+
+ ret = readl_poll_timeout_atomic(int_sta, val, !(val & HSNAND_INT_STA_WR_C),
+ 10, 1000);
+ if (ret)
+ return ret;
+
+ reg_data = readl(ebu_host->hsnand + HSNAND_CTL);
+ reg_data &= ~HSNAND_CTL_GO;
+ writel(reg_data, ebu_host->hsnand + HSNAND_CTL);
+
+ return 0;
+}
+
+static const u8 ecc_strength[] = { 1, 1, 4, 8, 24, 32, 40, 60, };
+
+static int ebu_nand_attach_chip(struct nand_chip *chip)
+{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
+ u32 ecc_steps, ecc_bytes, ecc_total, pagesize, pg_per_blk;
+ u32 ecc_strength_ds = chip->ecc.strength;
+ u32 ecc_size = chip->ecc.size;
+ u32 writesize = mtd->writesize;
+ u32 blocksize = mtd->erasesize;
+ int bch_algo, start, val;
+
+ /* Default to an ECC size of 512 */
+ if (!chip->ecc.size)
+ chip->ecc.size = 512;
+
+ switch (ecc_size) {
+ case 512:
+ start = 1;
+ if (!ecc_strength_ds)
+ ecc_strength_ds = 4;
+ break;
+ case 1024:
+ start = 4;
+ if (!ecc_strength_ds)
+ ecc_strength_ds = 32;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* BCH ECC algorithm Settings for number of bits per 512B/1024B */
+ bch_algo = round_up(start + 1, 4);
+ for (val = start; val < bch_algo; val++) {
+ if (ecc_strength_ds == ecc_strength[val])
+ break;
+ }
+ if (val == bch_algo)
+ return -EINVAL;
+
+ if (ecc_strength_ds == 8)
+ ecc_bytes = 14;
+ else
+ ecc_bytes = DIV_ROUND_UP(ecc_strength_ds * fls(8 * ecc_size), 8);
+
+ ecc_steps = writesize / ecc_size;
+ ecc_total = ecc_steps * ecc_bytes;
+ if ((ecc_total + 8) > mtd->oobsize)
+ return -ERANGE;
+
+ chip->ecc.total = ecc_total;
+ pagesize = fls(writesize >> 11);
+ if (pagesize > HSNAND_PARA0_PAGE_V8192)
+ return -ERANGE;
+
+ pg_per_blk = fls((blocksize / writesize) >> 6) / 8;
+ if (pg_per_blk > HSNAND_PARA0_PIB_V256)
+ return -ERANGE;
+
+ ebu_host->nd_para0 = pagesize | pg_per_blk | HSNAND_PARA0_BYP_EN_NP |
+ HSNAND_PARA0_BYP_DEC_NP | HSNAND_PARA0_ADEP_EN |
+ HSNAND_PARA0_TYPE_ONFI | (val << 29);
+
+ mtd_set_ooblayout(mtd, &ebu_nand_ooblayout_ops);
+ chip->ecc.read_page = ebu_nand_read_page_hwecc;
+ chip->ecc.write_page = ebu_nand_write_page_hwecc;
+
+ return 0;
+}
+
+static int ebu_nand_exec_op(struct nand_chip *chip,
+ const struct nand_operation *op, bool check_only)
+{
+ const struct nand_op_instr *instr = NULL;
+ unsigned int op_id;
+ int i, time_out, ret = 0;
+
+ if (check_only)
+ return 0;
+
+ ebu_select_chip(chip);
+ for (op_id = 0; op_id < op->ninstrs; op_id++) {
+ instr = &op->instrs[op_id];
+
+ switch (instr->type) {
+ case NAND_OP_CMD_INSTR:
+ ebu_nand_writeb(chip, HSNAND_CLE_OFFS | HSNAND_CS_OFFS,
+ instr->ctx.cmd.opcode);
+ break;
+
+ case NAND_OP_ADDR_INSTR:
+ for (i = 0; i < instr->ctx.addr.naddrs; i++)
+ ebu_nand_writeb(chip,
+ HSNAND_ALE_OFFS | HSNAND_CS_OFFS,
+ instr->ctx.addr.addrs[i]);
+ break;
+
+ case NAND_OP_DATA_IN_INSTR:
+ ebu_read_buf(chip, instr->ctx.data.buf.in,
+ instr->ctx.data.len);
+ break;
+
+ case NAND_OP_DATA_OUT_INSTR:
+ ebu_write_buf(chip, instr->ctx.data.buf.out,
+ instr->ctx.data.len);
+ break;
+
+ case NAND_OP_WAITRDY_INSTR:
+ time_out = instr->ctx.waitrdy.timeout_ms * 1000;
+ ret = ebu_nand_waitrdy(chip, time_out);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static const struct nand_controller_ops ebu_nand_controller_ops = {
+ .attach_chip = ebu_nand_attach_chip,
+ .setup_interface = ebu_nand_set_timings,
+ .exec_op = ebu_nand_exec_op,
+};
+
+static void ebu_dma_cleanup(struct ebu_nand_controller *ebu_host)
+{
+ if (ebu_host->dma_rx)
+ dma_release_channel(ebu_host->dma_rx);
+
+ if (ebu_host->dma_tx)
+ dma_release_channel(ebu_host->dma_tx);
+}
+
+static int ebu_nand_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ebu_nand_controller *ebu_host;
+ struct nand_chip *nand;
+ struct mtd_info *mtd;
+ struct resource *res;
+ char *resname;
+ int ret, i;
+ u32 reg;
+
+ ebu_host = devm_kzalloc(dev, sizeof(*ebu_host), GFP_KERNEL);
+ if (!ebu_host)
+ return -ENOMEM;
+
+ ebu_host->dev = dev;
+ nand_controller_init(&ebu_host->controller);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
+ ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(ebu_host->ebu))
+ return PTR_ERR(ebu_host->ebu);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsnand");
+ ebu_host->hsnand = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(ebu_host->hsnand))
+ return PTR_ERR(ebu_host->hsnand);
+
+ ret = device_property_read_u32(dev, "nand,cs", &reg);
+ if (ret) {
+ dev_err(dev, "failed to get chip select: %d\n", ret);
+ return ret;
+ }
+ ebu_host->cs_num = reg;
+
+ for (i = 0; i < MAX_CS; i++) {
+ resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ resname);
+ ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);
+ ebu_host->cs[i].nand_pa = res->start;
+ if (IS_ERR(ebu_host->cs[i].chipaddr))
+ return PTR_ERR(ebu_host->cs[i].chipaddr);
+ }
+
+ ebu_host->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(ebu_host->clk))
+ return dev_err_probe(dev, PTR_ERR(ebu_host->clk),
+ "failed to get clock\n");
+
+ ret = clk_prepare_enable(ebu_host->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable clock: %d\n", ret);
+ return ret;
+ }
+ ebu_host->clk_rate = clk_get_rate(ebu_host->clk);
+
+ ebu_host->dma_tx = dma_request_chan(dev, "tx");
+ if (IS_ERR(ebu_host->dma_tx))
+ return dev_err_probe(dev, PTR_ERR(ebu_host->dma_tx),
+ "failed to request DMA tx chan!.\n");
+
+ ebu_host->dma_rx = dma_request_chan(dev, "rx");
+ if (IS_ERR(ebu_host->dma_rx))
+ return dev_err_probe(dev, PTR_ERR(ebu_host->dma_rx),
+ "failed to request DMA rx chan!.\n");
+
+ for (i = 0; i < MAX_CS; i++) {
+ resname = devm_kasprintf(dev, GFP_KERNEL, "addr_sel%d", i);
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ resname);
+ if (!res)
+ return -EINVAL;
+ ebu_host->cs[i].addr_sel = res->start;
+ writel(ebu_host->cs[i].addr_sel | EBU_ADDR_MASK(5) |
+ EBU_ADDR_SEL_REGEN, ebu_host->ebu + EBU_ADDR_SEL(i));
+ }
+
+ nand_set_flash_node(&ebu_host->chip, dev->of_node);
+ if (!mtd->name) {
+ dev_err(ebu_host->dev, "NAND label property is mandatory\n");
+ return -EINVAL;
+ }
+
+ mtd = nand_to_mtd(&ebu_host->chip);
+ mtd->dev.parent = dev;
+ ebu_host->dev = dev;
+
+ platform_set_drvdata(pdev, ebu_host);
+ nand_set_controller_data(&ebu_host->chip, ebu_host);
+
+ nand = &ebu_host->chip;
+ nand->controller = &ebu_host->controller;
+ nand->controller->ops = &ebu_nand_controller_ops;
+
+ /* Scan to find existence of the device */
+ ret = nand_scan(&ebu_host->chip, 1);
+ if (ret)
+ goto err_cleanup_dma;
+
+ ret = mtd_device_register(mtd, NULL, 0);
+ if (ret)
+ goto err_clean_nand;
+
+ return 0;
+
+err_clean_nand:
+ nand_cleanup(&ebu_host->chip);
+err_cleanup_dma:
+ ebu_dma_cleanup(ebu_host);
+ clk_disable_unprepare(ebu_host->clk);
+
+ return ret;
+}
+
+static int ebu_nand_remove(struct platform_device *pdev)
+{
+ struct ebu_nand_controller *ebu_host = platform_get_drvdata(pdev);
+ int ret;
+
+ ret = mtd_device_unregister(nand_to_mtd(&ebu_host->chip));
+ WARN_ON(ret);
+ nand_cleanup(&ebu_host->chip);
+ ebu_nand_disable(&ebu_host->chip);
+ ebu_dma_cleanup(ebu_host);
+ clk_disable_unprepare(ebu_host->clk);
+
+ return 0;
+}
+
+static const struct of_device_id ebu_nand_match[] = {
+ { .compatible = "intel,nand-controller", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ebu_nand_match);
+
+static struct platform_driver ebu_nand_driver = {
+ .probe = ebu_nand_probe,
+ .remove = ebu_nand_remove,
+ .driver = {
+ .name = "intel-nand-controller",
+ .of_match_table = ebu_nand_match,
+ },
+
+};
+module_platform_driver(ebu_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-10-28 21:42:47

by Miquel Raynal

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

Hello,

"Ramuthevar,Vadivel MuruganX"
<[email protected]> wrote on Mon, 26 Oct 2020
15:30:21 +0800:

> 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.
>
> NAND controller driver implements ->exec_op() to replace legacy hooks,
> these specific call-back method to execute NAND operations.

No need to mention legacy hooks here as they are not part of your
driver at all.

>
> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
> ---
> drivers/mtd/nand/raw/Kconfig | 8 +
> drivers/mtd/nand/raw/Makefile | 1 +
> drivers/mtd/nand/raw/intel-nand-controller.c | 734 +++++++++++++++++++++++++++
> 3 files changed, 743 insertions(+)
> create mode 100644 drivers/mtd/nand/raw/intel-nand-controller.c
>
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index 6c46f25b57e2..1b3690fd08dc 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -462,6 +462,14 @@ config MTD_NAND_ARASAN
> Enables the driver for the Arasan NAND flash controller on
> Zynq Ultrascale+ MPSoC.
>
> +config MTD_NAND_INTEL_LGM
> + tristate "Support for NAND controller on Intel LGM SoC"
> + depends on OF || COMPILE_TEST
> + depends on HAS_IOMEM
> + help
> + Enables support for NAND Flash chips on Intel's LGM SoC.
> + NAND flash controller 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 2930f5b9015d..9e6037363fc6 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -58,6 +58,7 @@ 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_ARASAN) += arasan-nand-controller.o
> +obj-$(CONFIG_MTD_NAND_INTEL_LGM) += intel-nand-controller.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-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
> new file mode 100644
> index 000000000000..0aefc441c7d5
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/intel-nand-controller.c
> @@ -0,0 +1,734 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (c) 2020 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/err.h>
> +#include <linux/init.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/rawnand.h>
> +#include <linux/mtd/nand_ecc.h>
> +#include <linux/mtd/nand.h>
> +
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <asm/unaligned.h>
> +
> +#define EBU_CLC 0x000
> +#define EBU_CLC_RST 0x00000000u
> +
> +#define EBU_ADDR_SEL(n) (0x020 + (n) * 4)
> +/* 5 bits 26:22 included for comparison in the ADDR_SELx */
> +#define EBU_ADDR_MASK(x) ((x) << 4)
> +#define EBU_ADDR_SEL_REGEN 0x1
> +
> +#define EBU_BUSCON(n) (0x060 + (n) * 4)
> +#define EBU_BUSCON_CMULT_V4 0x1
> +#define EBU_BUSCON_RECOVC(n) ((n) << 2)
> +#define EBU_BUSCON_HOLDC(n) ((n) << 4)
> +#define EBU_BUSCON_WAITRDC(n) ((n) << 6)
> +#define EBU_BUSCON_WAITWRC(n) ((n) << 8)
> +#define EBU_BUSCON_BCGEN_CS 0x0
> +#define EBU_BUSCON_SETUP_EN BIT(22)
> +#define EBU_BUSCON_ALEC 0xC000
> +
> +#define EBU_CON 0x0B0
> +#define EBU_CON_NANDM_EN BIT(0)
> +#define EBU_CON_NANDM_DIS 0x0
> +#define EBU_CON_CSMUX_E_EN BIT(1)
> +#define EBU_CON_ALE_P_LOW BIT(2)
> +#define EBU_CON_CLE_P_LOW BIT(3)
> +#define EBU_CON_CS_P_LOW BIT(4)
> +#define EBU_CON_SE_P_LOW BIT(5)
> +#define EBU_CON_WP_P_LOW BIT(6)
> +#define EBU_CON_PRE_P_LOW BIT(7)
> +#define EBU_CON_IN_CS_S(n) ((n) << 8)
> +#define EBU_CON_OUT_CS_S(n) ((n) << 10)
> +#define EBU_CON_LAT_EN_CS_P ((0x3D) << 18)
> +
> +#define EBU_WAIT 0x0B4
> +#define EBU_WAIT_RDBY BIT(0)
> +#define EBU_WAIT_WR_C BIT(3)
> +
> +#define HSNAND_CTL1 0x110
> +#define HSNAND_CTL1_ADDR_SHIFT 24
> +
> +#define HSNAND_CTL2 0x114
> +#define HSNAND_CTL2_ADDR_SHIFT 8
> +#define HSNAND_CTL2_CYC_N_V5 (0x2 << 16)
> +
> +#define HSNAND_INT_MSK_CTL 0x124
> +#define HSNAND_INT_MSK_CTL_WR_C BIT(4)
> +
> +#define HSNAND_INT_STA 0x128
> +#define HSNAND_INT_STA_WR_C BIT(4)
> +
> +#define HSNAND_CTL 0x130
> +#define HSNAND_CTL_ENABLE_ECC BIT(0)
> +#define HSNAND_CTL_GO BIT(2)
> +#define HSNAND_CTL_CE_SEL_CS(n) BIT(3 + (n))
> +#define HSNAND_CTL_RW_READ 0x0
> +#define HSNAND_CTL_RW_WRITE BIT(10)
> +#define HSNAND_CTL_ECC_OFF_V8TH BIT(11)
> +#define HSNAND_CTL_CKFF_EN 0x0
> +#define HSNAND_CTL_MSG_EN BIT(17)
> +
> +#define HSNAND_PARA0 0x13c
> +#define HSNAND_PARA0_PAGE_V8192 0x3
> +#define HSNAND_PARA0_PIB_V256 (0x3 << 4)
> +#define HSNAND_PARA0_BYP_EN_NP 0x0
> +#define HSNAND_PARA0_BYP_DEC_NP 0x0
> +#define HSNAND_PARA0_TYPE_ONFI BIT(18)
> +#define HSNAND_PARA0_ADEP_EN BIT(21)
> +
> +#define HSNAND_CMSG_0 0x150
> +#define HSNAND_CMSG_1 0x154
> +
> +#define HSNAND_ALE_OFFS BIT(2)
> +#define HSNAND_CLE_OFFS BIT(3)
> +#define HSNAND_CS_OFFS BIT(4)
> +
> +#define HSNAND_ECC_OFFSET 0x008
> +
> +#define NAND_DATA_IFACE_CHECK_ONLY -1
> +
> +#define MAX_CS 2
> +
> +#define HZ_PER_MHZ 1000000L
> +#define USEC_PER_SEC 1000000L
> +
> +struct ebu_nand_cs {
> + void __iomem *chipaddr;
> + dma_addr_t nand_pa;
> + u32 addr_sel;
> +};
> +
> +struct ebu_nand_controller {
> + struct nand_controller controller;
> + struct nand_chip chip;
> + struct device *dev;
> + void __iomem *ebu;
> + void __iomem *hsnand;
> + struct dma_chan *dma_tx;
> + struct dma_chan *dma_rx;
> + struct completion dma_access_complete;
> + unsigned long clk_rate;
> + struct clk *clk;
> + u32 nd_para0;
> + u8 cs_num;
> + struct ebu_nand_cs cs[MAX_CS];
> +};
> +
> +static inline struct ebu_nand_controller *nand_to_ebu(struct nand_chip *chip)
> +{
> + return container_of(chip, struct ebu_nand_controller, chip);
> +}
> +
> +static int ebu_nand_waitrdy(struct nand_chip *chip, unsigned int time_out)

Please mention the unit somewhere.

> +{
> + struct ebu_nand_controller *ctrl = nand_to_ebu(chip);
> + u32 status;
> +
> + return readl_poll_timeout(ctrl->ebu + EBU_WAIT, status,
> + (status & EBU_WAIT_RDBY) ||
> + (status & EBU_WAIT_WR_C), 20, time_out);
> +}
> +
> +static u8 ebu_nand_readb(struct nand_chip *chip)
> +{
> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
> + u8 cs_num = ebu_host->cs_num;
> + u8 val;
> +
> + val = readb(ebu_host->cs[cs_num].chipaddr + HSNAND_CS_OFFS);
> + ebu_nand_waitrdy(chip, 1000);
> + return val;
> +}
> +
> +static void ebu_nand_writeb(struct nand_chip *chip, u32 offset, u8 value)
> +{
> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
> + u8 cs_num = ebu_host->cs_num;
> +
> + writeb(value, ebu_host->cs[cs_num].chipaddr + offset);
> + ebu_nand_waitrdy(chip, 1000);
> +}
> +
> +static void ebu_read_buf(struct nand_chip *chip, u_char *buf, unsigned int len)
> +{
> + int i;
> +
> + for (i = 0; i < len; i++)
> + buf[i] = ebu_nand_readb(chip);
> +}
> +
> +static void ebu_write_buf(struct nand_chip *chip, const u_char *buf, int len)
> +{
> + int i;
> +
> + for (i = 0; i < len; i++)
> + ebu_nand_writeb(chip, HSNAND_CS_OFFS, buf[i]);
> +}
> +
> +static void ebu_nand_disable(struct nand_chip *chip)
> +{
> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
> +
> + writel(0, ebu_host->ebu + EBU_CON);
> +}
> +
> +static void ebu_select_chip(struct nand_chip *chip)
> +{
> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
> + void __iomem *nand_con = ebu_host->ebu + EBU_CON;
> + u32 cs = ebu_host->cs_num;
> +
> + writel(EBU_CON_NANDM_EN | EBU_CON_CSMUX_E_EN | EBU_CON_CS_P_LOW |
> + EBU_CON_SE_P_LOW | EBU_CON_WP_P_LOW | EBU_CON_PRE_P_LOW |
> + EBU_CON_IN_CS_S(cs) | EBU_CON_OUT_CS_S(cs) |
> + EBU_CON_LAT_EN_CS_P, nand_con);
> +}
> +
> +static void ebu_nand_setup_timing(struct ebu_nand_controller *ctrl,
> + const struct nand_sdr_timings *timings)
> +{
> + unsigned int rate = clk_get_rate(ctrl->clk) / HZ_PER_MHZ;
> + unsigned int period = DIV_ROUND_UP(USEC_PER_SEC, rate);
> + u32 trecov, thold, twrwait, trdwait;
> + u32 reg = 0;
> +
> + trecov = DIV_ROUND_UP(max(timings->tREA_max, timings->tREH_min),
> + period);
> + reg |= EBU_BUSCON_RECOVC(trecov);
> +
> + thold = DIV_ROUND_UP(max(timings->tDH_min, timings->tDS_min), period);
> + reg |= EBU_BUSCON_HOLDC(thold);
> +
> + trdwait = DIV_ROUND_UP(max(timings->tRC_min, timings->tREH_min),
> + period);
> + reg |= EBU_BUSCON_WAITRDC(trdwait);
> +
> + twrwait = DIV_ROUND_UP(max(timings->tWC_min, timings->tWH_min), period);
> + reg |= EBU_BUSCON_WAITWRC(twrwait);
> +
> + reg |= EBU_BUSCON_CMULT_V4 | EBU_BUSCON_BCGEN_CS | EBU_BUSCON_ALEC |
> + EBU_BUSCON_SETUP_EN;
> +
> + writel(reg, ctrl->ebu + EBU_BUSCON(ctrl->cs_num));
> +}
> +
> +static int ebu_nand_set_timings(struct nand_chip *chip, int csline,
> + const struct nand_interface_config *conf)
> +{
> + struct ebu_nand_controller *ctrl = nand_to_ebu(chip);
> + const struct nand_sdr_timings *timings;
> +
> + timings = nand_get_sdr_timings(conf);
> + if (IS_ERR(timings))
> + return PTR_ERR(timings);
> +
> + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> + return 0;
> +
> + ebu_nand_setup_timing(ctrl, timings);

I don't think adding this helper helps much. You could insert the code
from this function here directly?

> +
> + return 0;
> +}
> +
> +static int ebu_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 = HSNAND_ECC_OFFSET;
> + oobregion->length = chip->ecc.total;
> +
> + return 0;
> +}
> +
> +static int ebu_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 + HSNAND_ECC_OFFSET;
> + oobregion->length = mtd->oobsize - oobregion->offset;
> +
> + return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops ebu_nand_ooblayout_ops = {
> + .ecc = ebu_nand_ooblayout_ecc,
> + .free = ebu_nand_ooblayout_free,
> +};
> +
> +static void ebu_dma_rx_callback(void *cookie)
> +{
> + struct ebu_nand_controller *ebu_host = cookie;
> +
> + dmaengine_terminate_async(ebu_host->dma_rx);
> +
> + complete(&ebu_host->dma_access_complete);
> +}
> +
> +static void ebu_dma_tx_callback(void *cookie)
> +{
> + struct ebu_nand_controller *ebu_host = cookie;
> +
> + dmaengine_terminate_async(ebu_host->dma_tx);
> +
> + complete(&ebu_host->dma_access_complete);

Please check return codes when they are relevant, and return the
errors. Also treat them below.

> +}
> +
> +static int ebu_dma_start(struct ebu_nand_controller *ebu_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 = ebu_host->dma_rx;
> + dma_completion = &ebu_host->dma_access_complete;
> + callback = ebu_dma_rx_callback;
> + } else {
> + chan = ebu_host->dma_tx;
> + dma_completion = &ebu_host->dma_access_complete;
> + callback = ebu_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(ebu_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 = ebu_host;
> + cookie = tx->tx_submit(tx);
> +
> + ret = dma_submit_error(cookie);
> + if (ret) {
> + dev_err(ebu_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(ebu_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(ebu_host->dev, buf_dma, len, dir);
> +
> + return ret;
> +}
> +
> +static void ebu_nand_trigger(struct ebu_nand_controller *ebu_host,
> + int page, u32 cmd)
> +{
> + unsigned int val;
> +
> + val = cmd | (page & 0xFF) << HSNAND_CTL1_ADDR_SHIFT;
> + writel(val, ebu_host->hsnand + HSNAND_CTL1);
> + val = (page & 0xFFFF00) >> 8 | HSNAND_CTL2_CYC_N_V5;
> + writel(val, ebu_host->hsnand + HSNAND_CTL2);
> +
> + writel(ebu_host->nd_para0, ebu_host->hsnand + HSNAND_PARA0);
> +
> + /* clear first, will update later */
> + writel(0xFFFFFFFF, ebu_host->hsnand + HSNAND_CMSG_0);
> + writel(0xFFFFFFFF, ebu_host->hsnand + HSNAND_CMSG_1);
> +
> + writel(HSNAND_INT_MSK_CTL_WR_C,
> + ebu_host->hsnand + HSNAND_INT_MSK_CTL);
> +
> + if (!cmd)
> + val = HSNAND_CTL_RW_READ;
> + else
> + val = HSNAND_CTL_RW_WRITE;
> +
> + writel(HSNAND_CTL_MSG_EN | HSNAND_CTL_CKFF_EN |
> + HSNAND_CTL_ECC_OFF_V8TH | HSNAND_CTL_CE_SEL_CS(ebu_host->cs_num) |
> + HSNAND_CTL_ENABLE_ECC | HSNAND_CTL_GO | val,
> + ebu_host->hsnand + HSNAND_CTL);
> +}
> +
> +static int ebu_nand_read_page_hwecc(struct nand_chip *chip, u8 *buf,
> + int oob_required, int page)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
> + int ret, reg_data;
> +
> + ebu_nand_trigger(ebu_host, page, NAND_CMD_READ0);
> +
> + ret = ebu_dma_start(ebu_host, DMA_DEV_TO_MEM, buf, mtd->writesize);
> + if (ret)
> + return ret;
> +
> + if (oob_required)
> + chip->ecc.read_oob(chip, page);
> +
> + reg_data = readl(ebu_host->hsnand + HSNAND_CTL);
> + reg_data &= ~HSNAND_CTL_GO;
> + writel(reg_data, ebu_host->hsnand + HSNAND_CTL);
> +
> + return 0;
> +}
> +
> +static int ebu_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 ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
> + void __iomem *int_sta = ebu_host->hsnand + HSNAND_INT_STA;
> + int reg_data, ret, val;
> + u32 reg;
> +
> + ebu_nand_trigger(ebu_host, page, NAND_CMD_SEQIN);
> +
> + ret = ebu_dma_start(ebu_host, DMA_MEM_TO_DEV, buf, mtd->writesize);
> + if (ret)
> + return ret;
> +
> + if (oob_required) {
> + reg = get_unaligned_le32(chip->oob_poi);
> + writel(reg, ebu_host->hsnand + HSNAND_CMSG_0);
> +
> + reg = get_unaligned_le32(chip->oob_poi + 4);
> + writel(reg, ebu_host->hsnand + HSNAND_CMSG_1);
> + }
> +
> + ret = readl_poll_timeout_atomic(int_sta, val, !(val & HSNAND_INT_STA_WR_C),
> + 10, 1000);
> + if (ret)
> + return ret;
> +
> + reg_data = readl(ebu_host->hsnand + HSNAND_CTL);
> + reg_data &= ~HSNAND_CTL_GO;
> + writel(reg_data, ebu_host->hsnand + HSNAND_CTL);
> +
> + return 0;
> +}
> +
> +static const u8 ecc_strength[] = { 1, 1, 4, 8, 24, 32, 40, 60, };
> +
> +static int ebu_nand_attach_chip(struct nand_chip *chip)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
> + u32 ecc_steps, ecc_bytes, ecc_total, pagesize, pg_per_blk;
> + u32 ecc_strength_ds = chip->ecc.strength;
> + u32 ecc_size = chip->ecc.size;
> + u32 writesize = mtd->writesize;
> + u32 blocksize = mtd->erasesize;
> + int bch_algo, start, val;
> +
> + /* Default to an ECC size of 512 */
> + if (!chip->ecc.size)
> + chip->ecc.size = 512;
> +
> + switch (ecc_size) {
> + case 512:
> + start = 1;
> + if (!ecc_strength_ds)
> + ecc_strength_ds = 4;
> + break;
> + case 1024:
> + start = 4;
> + if (!ecc_strength_ds)
> + ecc_strength_ds = 32;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* BCH ECC algorithm Settings for number of bits per 512B/1024B */
> + bch_algo = round_up(start + 1, 4);
> + for (val = start; val < bch_algo; val++) {
> + if (ecc_strength_ds == ecc_strength[val])
> + break;
> + }
> + if (val == bch_algo)
> + return -EINVAL;
> +
> + if (ecc_strength_ds == 8)
> + ecc_bytes = 14;
> + else
> + ecc_bytes = DIV_ROUND_UP(ecc_strength_ds * fls(8 * ecc_size), 8);
> +
> + ecc_steps = writesize / ecc_size;
> + ecc_total = ecc_steps * ecc_bytes;
> + if ((ecc_total + 8) > mtd->oobsize)
> + return -ERANGE;
> +
> + chip->ecc.total = ecc_total;
> + pagesize = fls(writesize >> 11);
> + if (pagesize > HSNAND_PARA0_PAGE_V8192)
> + return -ERANGE;
> +
> + pg_per_blk = fls((blocksize / writesize) >> 6) / 8;
> + if (pg_per_blk > HSNAND_PARA0_PIB_V256)
> + return -ERANGE;
> +
> + ebu_host->nd_para0 = pagesize | pg_per_blk | HSNAND_PARA0_BYP_EN_NP |
> + HSNAND_PARA0_BYP_DEC_NP | HSNAND_PARA0_ADEP_EN |
> + HSNAND_PARA0_TYPE_ONFI | (val << 29);
> +
> + mtd_set_ooblayout(mtd, &ebu_nand_ooblayout_ops);
> + chip->ecc.read_page = ebu_nand_read_page_hwecc;
> + chip->ecc.write_page = ebu_nand_write_page_hwecc;
> +
> + return 0;
> +}
> +
> +static int ebu_nand_exec_op(struct nand_chip *chip,
> + const struct nand_operation *op, bool check_only)
> +{
> + const struct nand_op_instr *instr = NULL;
> + unsigned int op_id;
> + int i, time_out, ret = 0;
> +
> + if (check_only)
> + return 0;
> +
> + ebu_select_chip(chip);
> + for (op_id = 0; op_id < op->ninstrs; op_id++) {
> + instr = &op->instrs[op_id];
> +
> + switch (instr->type) {
> + case NAND_OP_CMD_INSTR:
> + ebu_nand_writeb(chip, HSNAND_CLE_OFFS | HSNAND_CS_OFFS,
> + instr->ctx.cmd.opcode);
> + break;
> +
> + case NAND_OP_ADDR_INSTR:
> + for (i = 0; i < instr->ctx.addr.naddrs; i++)
> + ebu_nand_writeb(chip,
> + HSNAND_ALE_OFFS | HSNAND_CS_OFFS,
> + instr->ctx.addr.addrs[i]);
> + break;
> +
> + case NAND_OP_DATA_IN_INSTR:
> + ebu_read_buf(chip, instr->ctx.data.buf.in,
> + instr->ctx.data.len);
> + break;
> +
> + case NAND_OP_DATA_OUT_INSTR:
> + ebu_write_buf(chip, instr->ctx.data.buf.out,
> + instr->ctx.data.len);
> + break;
> +
> + case NAND_OP_WAITRDY_INSTR:
> + time_out = instr->ctx.waitrdy.timeout_ms * 1000;
> + ret = ebu_nand_waitrdy(chip, time_out);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static const struct nand_controller_ops ebu_nand_controller_ops = {
> + .attach_chip = ebu_nand_attach_chip,
> + .setup_interface = ebu_nand_set_timings,
> + .exec_op = ebu_nand_exec_op,
> +};
> +
> +static void ebu_dma_cleanup(struct ebu_nand_controller *ebu_host)
> +{
> + if (ebu_host->dma_rx)
> + dma_release_channel(ebu_host->dma_rx);
> +
> + if (ebu_host->dma_tx)
> + dma_release_channel(ebu_host->dma_tx);
> +}
> +
> +static int ebu_nand_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ebu_nand_controller *ebu_host;
> + struct nand_chip *nand;
> + struct mtd_info *mtd;
> + struct resource *res;
> + char *resname;
> + int ret, i;
> + u32 reg;
> +
> + ebu_host = devm_kzalloc(dev, sizeof(*ebu_host), GFP_KERNEL);
> + if (!ebu_host)
> + return -ENOMEM;
> +
> + ebu_host->dev = dev;
> + nand_controller_init(&ebu_host->controller);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
> + ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(ebu_host->ebu))
> + return PTR_ERR(ebu_host->ebu);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsnand");
> + ebu_host->hsnand = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(ebu_host->hsnand))
> + return PTR_ERR(ebu_host->hsnand);
> +
> + ret = device_property_read_u32(dev, "nand,cs", &reg);

There is no nand,cs property. Use 'reg' instead.

> + if (ret) {
> + dev_err(dev, "failed to get chip select: %d\n", ret);
> + return ret;
> + }
> + ebu_host->cs_num = reg;

The following for loop is weird, above you can only store a single cs
number, while below you seem to reserve serveral memory areas. Please
clarify this code.

> +
> + for (i = 0; i < MAX_CS; i++) {
> + resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + resname);
> + ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);
> + ebu_host->cs[i].nand_pa = res->start;
> + if (IS_ERR(ebu_host->cs[i].chipaddr))
> + return PTR_ERR(ebu_host->cs[i].chipaddr);
> + }
> +
> + ebu_host->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(ebu_host->clk))
> + return dev_err_probe(dev, PTR_ERR(ebu_host->clk),
> + "failed to get clock\n");
> +
> + ret = clk_prepare_enable(ebu_host->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable clock: %d\n", ret);
> + return ret;
> + }
> + ebu_host->clk_rate = clk_get_rate(ebu_host->clk);
> +
> + ebu_host->dma_tx = dma_request_chan(dev, "tx");
> + if (IS_ERR(ebu_host->dma_tx))
> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_tx),
> + "failed to request DMA tx chan!.\n");
> +
> + ebu_host->dma_rx = dma_request_chan(dev, "rx");
> + if (IS_ERR(ebu_host->dma_rx))
> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_rx),
> + "failed to request DMA rx chan!.\n");
> +
> + for (i = 0; i < MAX_CS; i++) {
> + resname = devm_kasprintf(dev, GFP_KERNEL, "addr_sel%d", i);
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + resname);
> + if (!res)
> + return -EINVAL;
> + ebu_host->cs[i].addr_sel = res->start;
> + writel(ebu_host->cs[i].addr_sel | EBU_ADDR_MASK(5) |
> + EBU_ADDR_SEL_REGEN, ebu_host->ebu + EBU_ADDR_SEL(i));
> + }
> +
> + nand_set_flash_node(&ebu_host->chip, dev->of_node);
> + if (!mtd->name) {
> + dev_err(ebu_host->dev, "NAND label property is mandatory\n");
> + return -EINVAL;
> + }
> +
> + mtd = nand_to_mtd(&ebu_host->chip);
> + mtd->dev.parent = dev;
> + ebu_host->dev = dev;
> +
> + platform_set_drvdata(pdev, ebu_host);
> + nand_set_controller_data(&ebu_host->chip, ebu_host);
> +
> + nand = &ebu_host->chip;
> + nand->controller = &ebu_host->controller;
> + nand->controller->ops = &ebu_nand_controller_ops;
> +
> + /* Scan to find existence of the device */
> + ret = nand_scan(&ebu_host->chip, 1);
> + if (ret)
> + goto err_cleanup_dma;
> +
> + ret = mtd_device_register(mtd, NULL, 0);
> + if (ret)
> + goto err_clean_nand;
> +
> + return 0;
> +
> +err_clean_nand:
> + nand_cleanup(&ebu_host->chip);
> +err_cleanup_dma:
> + ebu_dma_cleanup(ebu_host);
> + clk_disable_unprepare(ebu_host->clk);
> +
> + return ret;
> +}
> +
> +static int ebu_nand_remove(struct platform_device *pdev)
> +{
> + struct ebu_nand_controller *ebu_host = platform_get_drvdata(pdev);
> + int ret;
> +
> + ret = mtd_device_unregister(nand_to_mtd(&ebu_host->chip));
> + WARN_ON(ret);
> + nand_cleanup(&ebu_host->chip);
> + ebu_nand_disable(&ebu_host->chip);
> + ebu_dma_cleanup(ebu_host);
> + clk_disable_unprepare(ebu_host->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ebu_nand_match[] = {
> + { .compatible = "intel,nand-controller", },

No version or soc in the compatible? (not mandatory).

> + {}
> +};
> +MODULE_DEVICE_TABLE(of, ebu_nand_match);
> +
> +static struct platform_driver ebu_nand_driver = {
> + .probe = ebu_nand_probe,
> + .remove = ebu_nand_remove,
> + .driver = {
> + .name = "intel-nand-controller",
> + .of_match_table = ebu_nand_match,
> + },
> +
> +};
> +module_platform_driver(ebu_nand_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Vadivel Murugan R <[email protected]>");
> +MODULE_DESCRIPTION("Intel's LGM External Bus NAND Controller driver");

Thanks,
Miquèl

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

Hi Miquel,

Thank you very much for the review comments...

On 28/10/2020 6:20 pm, Miquel Raynal wrote:
> Hello,
>
> "Ramuthevar,Vadivel MuruganX"
> <[email protected]> wrote on Mon, 26 Oct 2020
> 15:30:21 +0800:
>
>> 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.
>>
>> NAND controller driver implements ->exec_op() to replace legacy hooks,
>> these specific call-back method to execute NAND operations.
>
> No need to mention legacy hooks here as they are not part of your
> driver at all.
Ok, Noted.
>
>>
>> Signed-off-by: Ramuthevar Vadivel Murugan <[email protected]>
>> ---
>> drivers/mtd/nand/raw/Kconfig | 8 +
>> drivers/mtd/nand/raw/Makefile | 1 +
>> drivers/mtd/nand/raw/intel-nand-controller.c | 734 +++++++++++++++++++++++++++
>> 3 files changed, 743 insertions(+)
>> create mode 100644 drivers/mtd/nand/raw/intel-nand-controller.c
>>
>> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
>> index 6c46f25b57e2..1b3690fd08dc 100644
>> --- a/drivers/mtd/nand/raw/Kconfig
>> +++ b/drivers/mtd/nand/raw/Kconfig
>> @@ -462,6 +462,14 @@ config MTD_NAND_ARASAN
>> Enables the driver for the Arasan NAND flash controller on
>> Zynq Ultrascale+ MPSoC.
>>
>> +config MTD_NAND_INTEL_LGM
>> + tristate "Support for NAND controller on Intel LGM SoC"
>> + depends on OF || COMPILE_TEST
>> + depends on HAS_IOMEM
>> + help
>> + Enables support for NAND Flash chips on Intel's LGM SoC.
>> + NAND flash controller 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 2930f5b9015d..9e6037363fc6 100644
>> --- a/drivers/mtd/nand/raw/Makefile
>> +++ b/drivers/mtd/nand/raw/Makefile
>> @@ -58,6 +58,7 @@ 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_ARASAN) += arasan-nand-controller.o
>> +obj-$(CONFIG_MTD_NAND_INTEL_LGM) += intel-nand-controller.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-nand-controller.c b/drivers/mtd/nand/raw/intel-nand-controller.c
>> new file mode 100644
>> index 000000000000..0aefc441c7d5
>> --- /dev/null
>> +++ b/drivers/mtd/nand/raw/intel-nand-controller.c
>> @@ -0,0 +1,734 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/* Copyright (c) 2020 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/err.h>
>> +#include <linux/init.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/rawnand.h>
>> +#include <linux/mtd/nand_ecc.h>
>> +#include <linux/mtd/nand.h>
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +#include <asm/unaligned.h>
>> +
>> +#define EBU_CLC 0x000
>> +#define EBU_CLC_RST 0x00000000u
>> +
>> +#define EBU_ADDR_SEL(n) (0x020 + (n) * 4)
>> +/* 5 bits 26:22 included for comparison in the ADDR_SELx */
>> +#define EBU_ADDR_MASK(x) ((x) << 4)
>> +#define EBU_ADDR_SEL_REGEN 0x1
>> +
>> +#define EBU_BUSCON(n) (0x060 + (n) * 4)
>> +#define EBU_BUSCON_CMULT_V4 0x1
>> +#define EBU_BUSCON_RECOVC(n) ((n) << 2)
>> +#define EBU_BUSCON_HOLDC(n) ((n) << 4)
>> +#define EBU_BUSCON_WAITRDC(n) ((n) << 6)
>> +#define EBU_BUSCON_WAITWRC(n) ((n) << 8)
>> +#define EBU_BUSCON_BCGEN_CS 0x0
>> +#define EBU_BUSCON_SETUP_EN BIT(22)
>> +#define EBU_BUSCON_ALEC 0xC000
>> +
>> +#define EBU_CON 0x0B0
>> +#define EBU_CON_NANDM_EN BIT(0)
>> +#define EBU_CON_NANDM_DIS 0x0
>> +#define EBU_CON_CSMUX_E_EN BIT(1)
>> +#define EBU_CON_ALE_P_LOW BIT(2)
>> +#define EBU_CON_CLE_P_LOW BIT(3)
>> +#define EBU_CON_CS_P_LOW BIT(4)
>> +#define EBU_CON_SE_P_LOW BIT(5)
>> +#define EBU_CON_WP_P_LOW BIT(6)
>> +#define EBU_CON_PRE_P_LOW BIT(7)
>> +#define EBU_CON_IN_CS_S(n) ((n) << 8)
>> +#define EBU_CON_OUT_CS_S(n) ((n) << 10)
>> +#define EBU_CON_LAT_EN_CS_P ((0x3D) << 18)
>> +
>> +#define EBU_WAIT 0x0B4
>> +#define EBU_WAIT_RDBY BIT(0)
>> +#define EBU_WAIT_WR_C BIT(3)
>> +
>> +#define HSNAND_CTL1 0x110
>> +#define HSNAND_CTL1_ADDR_SHIFT 24
>> +
>> +#define HSNAND_CTL2 0x114
>> +#define HSNAND_CTL2_ADDR_SHIFT 8
>> +#define HSNAND_CTL2_CYC_N_V5 (0x2 << 16)
>> +
>> +#define HSNAND_INT_MSK_CTL 0x124
>> +#define HSNAND_INT_MSK_CTL_WR_C BIT(4)
>> +
>> +#define HSNAND_INT_STA 0x128
>> +#define HSNAND_INT_STA_WR_C BIT(4)
>> +
>> +#define HSNAND_CTL 0x130
>> +#define HSNAND_CTL_ENABLE_ECC BIT(0)
>> +#define HSNAND_CTL_GO BIT(2)
>> +#define HSNAND_CTL_CE_SEL_CS(n) BIT(3 + (n))
>> +#define HSNAND_CTL_RW_READ 0x0
>> +#define HSNAND_CTL_RW_WRITE BIT(10)
>> +#define HSNAND_CTL_ECC_OFF_V8TH BIT(11)
>> +#define HSNAND_CTL_CKFF_EN 0x0
>> +#define HSNAND_CTL_MSG_EN BIT(17)
>> +
>> +#define HSNAND_PARA0 0x13c
>> +#define HSNAND_PARA0_PAGE_V8192 0x3
>> +#define HSNAND_PARA0_PIB_V256 (0x3 << 4)
>> +#define HSNAND_PARA0_BYP_EN_NP 0x0
>> +#define HSNAND_PARA0_BYP_DEC_NP 0x0
>> +#define HSNAND_PARA0_TYPE_ONFI BIT(18)
>> +#define HSNAND_PARA0_ADEP_EN BIT(21)
>> +
>> +#define HSNAND_CMSG_0 0x150
>> +#define HSNAND_CMSG_1 0x154
>> +
>> +#define HSNAND_ALE_OFFS BIT(2)
>> +#define HSNAND_CLE_OFFS BIT(3)
>> +#define HSNAND_CS_OFFS BIT(4)
>> +
>> +#define HSNAND_ECC_OFFSET 0x008
>> +
>> +#define NAND_DATA_IFACE_CHECK_ONLY -1
>> +
>> +#define MAX_CS 2
>> +
>> +#define HZ_PER_MHZ 1000000L
>> +#define USEC_PER_SEC 1000000L
>> +
>> +struct ebu_nand_cs {
>> + void __iomem *chipaddr;
>> + dma_addr_t nand_pa;
>> + u32 addr_sel;
>> +};
>> +
>> +struct ebu_nand_controller {
>> + struct nand_controller controller;
>> + struct nand_chip chip;
>> + struct device *dev;
>> + void __iomem *ebu;
>> + void __iomem *hsnand;
>> + struct dma_chan *dma_tx;
>> + struct dma_chan *dma_rx;
>> + struct completion dma_access_complete;
>> + unsigned long clk_rate;
>> + struct clk *clk;
>> + u32 nd_para0;
>> + u8 cs_num;
>> + struct ebu_nand_cs cs[MAX_CS];
>> +};
>> +
>> +static inline struct ebu_nand_controller *nand_to_ebu(struct nand_chip *chip)
>> +{
>> + return container_of(chip, struct ebu_nand_controller, chip);
>> +}
>> +
>> +static int ebu_nand_waitrdy(struct nand_chip *chip, unsigned int time_out)
>
> Please mention the unit somewhere.Sure, will update
>
>> +{
>> + struct ebu_nand_controller *ctrl = nand_to_ebu(chip);
>> + u32 status;
>> +
>> + return readl_poll_timeout(ctrl->ebu + EBU_WAIT, status,
>> + (status & EBU_WAIT_RDBY) ||
>> + (status & EBU_WAIT_WR_C), 20, time_out);
>> +}
>> +
>> +static u8 ebu_nand_readb(struct nand_chip *chip)
>> +{
>> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
>> + u8 cs_num = ebu_host->cs_num;
>> + u8 val;
>> +
>> + val = readb(ebu_host->cs[cs_num].chipaddr + HSNAND_CS_OFFS);
>> + ebu_nand_waitrdy(chip, 1000);
>> + return val;
>> +}
>> +
>> +static void ebu_nand_writeb(struct nand_chip *chip, u32 offset, u8 value)
>> +{
>> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
>> + u8 cs_num = ebu_host->cs_num;
>> +
>> + writeb(value, ebu_host->cs[cs_num].chipaddr + offset);
>> + ebu_nand_waitrdy(chip, 1000);
>> +}
>> +
>> +static void ebu_read_buf(struct nand_chip *chip, u_char *buf, unsigned int len)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < len; i++)
>> + buf[i] = ebu_nand_readb(chip);
>> +}
>> +
>> +static void ebu_write_buf(struct nand_chip *chip, const u_char *buf, int len)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < len; i++)
>> + ebu_nand_writeb(chip, HSNAND_CS_OFFS, buf[i]);
>> +}
>> +
>> +static void ebu_nand_disable(struct nand_chip *chip)
>> +{
>> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
>> +
>> + writel(0, ebu_host->ebu + EBU_CON);
>> +}
>> +
>> +static void ebu_select_chip(struct nand_chip *chip)
>> +{
>> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
>> + void __iomem *nand_con = ebu_host->ebu + EBU_CON;
>> + u32 cs = ebu_host->cs_num;
>> +
>> + writel(EBU_CON_NANDM_EN | EBU_CON_CSMUX_E_EN | EBU_CON_CS_P_LOW |
>> + EBU_CON_SE_P_LOW | EBU_CON_WP_P_LOW | EBU_CON_PRE_P_LOW |
>> + EBU_CON_IN_CS_S(cs) | EBU_CON_OUT_CS_S(cs) |
>> + EBU_CON_LAT_EN_CS_P, nand_con);
>> +}
>> +
>> +static void ebu_nand_setup_timing(struct ebu_nand_controller *ctrl,
>> + const struct nand_sdr_timings *timings)
>> +{
>> + unsigned int rate = clk_get_rate(ctrl->clk) / HZ_PER_MHZ;
>> + unsigned int period = DIV_ROUND_UP(USEC_PER_SEC, rate);
>> + u32 trecov, thold, twrwait, trdwait;
>> + u32 reg = 0;
>> +
>> + trecov = DIV_ROUND_UP(max(timings->tREA_max, timings->tREH_min),
>> + period);
>> + reg |= EBU_BUSCON_RECOVC(trecov);
>> +
>> + thold = DIV_ROUND_UP(max(timings->tDH_min, timings->tDS_min), period);
>> + reg |= EBU_BUSCON_HOLDC(thold);
>> +
>> + trdwait = DIV_ROUND_UP(max(timings->tRC_min, timings->tREH_min),
>> + period);
>> + reg |= EBU_BUSCON_WAITRDC(trdwait);
>> +
>> + twrwait = DIV_ROUND_UP(max(timings->tWC_min, timings->tWH_min), period);
>> + reg |= EBU_BUSCON_WAITWRC(twrwait);
>> +
>> + reg |= EBU_BUSCON_CMULT_V4 | EBU_BUSCON_BCGEN_CS | EBU_BUSCON_ALEC |
>> + EBU_BUSCON_SETUP_EN;
>> +
>> + writel(reg, ctrl->ebu + EBU_BUSCON(ctrl->cs_num));
>> +}
>> +
>> +static int ebu_nand_set_timings(struct nand_chip *chip, int csline,
>> + const struct nand_interface_config *conf)
>> +{
>> + struct ebu_nand_controller *ctrl = nand_to_ebu(chip);
>> + const struct nand_sdr_timings *timings;
>> +
>> + timings = nand_get_sdr_timings(conf);
>> + if (IS_ERR(timings))
>> + return PTR_ERR(timings);
>> +
>> + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
>> + return 0;
>> +
>> + ebu_nand_setup_timing(ctrl, timings);
>
> I don't think adding this helper helps much. You could insert the code
> from this function here directly?
Yes, You're right, earlier we added directely, the during the code
adaptation exec_op() changed, will update it as per your suggestions
>
>> +
>> + return 0;
>> +}
>> +
>> +static int ebu_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 = HSNAND_ECC_OFFSET;
>> + oobregion->length = chip->ecc.total;
>> +
>> + return 0;
>> +}
>> +
>> +static int ebu_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 + HSNAND_ECC_OFFSET;
>> + oobregion->length = mtd->oobsize - oobregion->offset;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct mtd_ooblayout_ops ebu_nand_ooblayout_ops = {
>> + .ecc = ebu_nand_ooblayout_ecc,
>> + .free = ebu_nand_ooblayout_free,
>> +};
>> +
>> +static void ebu_dma_rx_callback(void *cookie)
>> +{
>> + struct ebu_nand_controller *ebu_host = cookie;
>> +
>> + dmaengine_terminate_async(ebu_host->dma_rx);
>> +
>> + complete(&ebu_host->dma_access_complete);
>> +}
>> +
>> +static void ebu_dma_tx_callback(void *cookie)
>> +{
>> + struct ebu_nand_controller *ebu_host = cookie;
>> +
>> + dmaengine_terminate_async(ebu_host->dma_tx);
>> +
>> + complete(&ebu_host->dma_access_complete);
>
> Please check return codes when they are relevant, and return the
> errors. Also treat them below.
Noted.
>
>> +}
>> +
>> +static int ebu_dma_start(struct ebu_nand_controller *ebu_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 = ebu_host->dma_rx;
>> + dma_completion = &ebu_host->dma_access_complete;
>> + callback = ebu_dma_rx_callback;
>> + } else {
>> + chan = ebu_host->dma_tx;
>> + dma_completion = &ebu_host->dma_access_complete;
>> + callback = ebu_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(ebu_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 = ebu_host;
>> + cookie = tx->tx_submit(tx);
>> +
>> + ret = dma_submit_error(cookie);
>> + if (ret) {
>> + dev_err(ebu_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(ebu_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(ebu_host->dev, buf_dma, len, dir);
>> +
>> + return ret;
>> +}
>> +
>> +static void ebu_nand_trigger(struct ebu_nand_controller *ebu_host,
>> + int page, u32 cmd)
>> +{
>> + unsigned int val;
>> +
>> + val = cmd | (page & 0xFF) << HSNAND_CTL1_ADDR_SHIFT;
>> + writel(val, ebu_host->hsnand + HSNAND_CTL1);
>> + val = (page & 0xFFFF00) >> 8 | HSNAND_CTL2_CYC_N_V5;
>> + writel(val, ebu_host->hsnand + HSNAND_CTL2);
>> +
>> + writel(ebu_host->nd_para0, ebu_host->hsnand + HSNAND_PARA0);
>> +
>> + /* clear first, will update later */
>> + writel(0xFFFFFFFF, ebu_host->hsnand + HSNAND_CMSG_0);
>> + writel(0xFFFFFFFF, ebu_host->hsnand + HSNAND_CMSG_1);
>> +
>> + writel(HSNAND_INT_MSK_CTL_WR_C,
>> + ebu_host->hsnand + HSNAND_INT_MSK_CTL);
>> +
>> + if (!cmd)
>> + val = HSNAND_CTL_RW_READ;
>> + else
>> + val = HSNAND_CTL_RW_WRITE;
>> +
>> + writel(HSNAND_CTL_MSG_EN | HSNAND_CTL_CKFF_EN |
>> + HSNAND_CTL_ECC_OFF_V8TH | HSNAND_CTL_CE_SEL_CS(ebu_host->cs_num) |
>> + HSNAND_CTL_ENABLE_ECC | HSNAND_CTL_GO | val,
>> + ebu_host->hsnand + HSNAND_CTL);
>> +}
>> +
>> +static int ebu_nand_read_page_hwecc(struct nand_chip *chip, u8 *buf,
>> + int oob_required, int page)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(chip);
>> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
>> + int ret, reg_data;
>> +
>> + ebu_nand_trigger(ebu_host, page, NAND_CMD_READ0);
>> +
>> + ret = ebu_dma_start(ebu_host, DMA_DEV_TO_MEM, buf, mtd->writesize);
>> + if (ret)
>> + return ret;
>> +
>> + if (oob_required)
>> + chip->ecc.read_oob(chip, page);
>> +
>> + reg_data = readl(ebu_host->hsnand + HSNAND_CTL);
>> + reg_data &= ~HSNAND_CTL_GO;
>> + writel(reg_data, ebu_host->hsnand + HSNAND_CTL);
>> +
>> + return 0;
>> +}
>> +
>> +static int ebu_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 ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
>> + void __iomem *int_sta = ebu_host->hsnand + HSNAND_INT_STA;
>> + int reg_data, ret, val;
>> + u32 reg;
>> +
>> + ebu_nand_trigger(ebu_host, page, NAND_CMD_SEQIN);
>> +
>> + ret = ebu_dma_start(ebu_host, DMA_MEM_TO_DEV, buf, mtd->writesize);
>> + if (ret)
>> + return ret;
>> +
>> + if (oob_required) {
>> + reg = get_unaligned_le32(chip->oob_poi);
>> + writel(reg, ebu_host->hsnand + HSNAND_CMSG_0);
>> +
>> + reg = get_unaligned_le32(chip->oob_poi + 4);
>> + writel(reg, ebu_host->hsnand + HSNAND_CMSG_1);
>> + }
>> +
>> + ret = readl_poll_timeout_atomic(int_sta, val, !(val & HSNAND_INT_STA_WR_C),
>> + 10, 1000);
>> + if (ret)
>> + return ret;
>> +
>> + reg_data = readl(ebu_host->hsnand + HSNAND_CTL);
>> + reg_data &= ~HSNAND_CTL_GO;
>> + writel(reg_data, ebu_host->hsnand + HSNAND_CTL);
>> +
>> + return 0;
>> +}
>> +
>> +static const u8 ecc_strength[] = { 1, 1, 4, 8, 24, 32, 40, 60, };
>> +
>> +static int ebu_nand_attach_chip(struct nand_chip *chip)
>> +{
>> + struct mtd_info *mtd = nand_to_mtd(chip);
>> + struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
>> + u32 ecc_steps, ecc_bytes, ecc_total, pagesize, pg_per_blk;
>> + u32 ecc_strength_ds = chip->ecc.strength;
>> + u32 ecc_size = chip->ecc.size;
>> + u32 writesize = mtd->writesize;
>> + u32 blocksize = mtd->erasesize;
>> + int bch_algo, start, val;
>> +
>> + /* Default to an ECC size of 512 */
>> + if (!chip->ecc.size)
>> + chip->ecc.size = 512;
>> +
>> + switch (ecc_size) {
>> + case 512:
>> + start = 1;
>> + if (!ecc_strength_ds)
>> + ecc_strength_ds = 4;
>> + break;
>> + case 1024:
>> + start = 4;
>> + if (!ecc_strength_ds)
>> + ecc_strength_ds = 32;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + /* BCH ECC algorithm Settings for number of bits per 512B/1024B */
>> + bch_algo = round_up(start + 1, 4);
>> + for (val = start; val < bch_algo; val++) {
>> + if (ecc_strength_ds == ecc_strength[val])
>> + break;
>> + }
>> + if (val == bch_algo)
>> + return -EINVAL;
>> +
>> + if (ecc_strength_ds == 8)
>> + ecc_bytes = 14;
>> + else
>> + ecc_bytes = DIV_ROUND_UP(ecc_strength_ds * fls(8 * ecc_size), 8);
>> +
>> + ecc_steps = writesize / ecc_size;
>> + ecc_total = ecc_steps * ecc_bytes;
>> + if ((ecc_total + 8) > mtd->oobsize)
>> + return -ERANGE;
>> +
>> + chip->ecc.total = ecc_total;
>> + pagesize = fls(writesize >> 11);
>> + if (pagesize > HSNAND_PARA0_PAGE_V8192)
>> + return -ERANGE;
>> +
>> + pg_per_blk = fls((blocksize / writesize) >> 6) / 8;
>> + if (pg_per_blk > HSNAND_PARA0_PIB_V256)
>> + return -ERANGE;
>> +
>> + ebu_host->nd_para0 = pagesize | pg_per_blk | HSNAND_PARA0_BYP_EN_NP |
>> + HSNAND_PARA0_BYP_DEC_NP | HSNAND_PARA0_ADEP_EN |
>> + HSNAND_PARA0_TYPE_ONFI | (val << 29);
>> +
>> + mtd_set_ooblayout(mtd, &ebu_nand_ooblayout_ops);
>> + chip->ecc.read_page = ebu_nand_read_page_hwecc;
>> + chip->ecc.write_page = ebu_nand_write_page_hwecc;
>> +
>> + return 0;
>> +}
>> +
>> +static int ebu_nand_exec_op(struct nand_chip *chip,
>> + const struct nand_operation *op, bool check_only)
>> +{
>> + const struct nand_op_instr *instr = NULL;
>> + unsigned int op_id;
>> + int i, time_out, ret = 0;
>> +
>> + if (check_only)
>> + return 0;
>> +
>> + ebu_select_chip(chip);
>> + for (op_id = 0; op_id < op->ninstrs; op_id++) {
>> + instr = &op->instrs[op_id];
>> +
>> + switch (instr->type) {
>> + case NAND_OP_CMD_INSTR:
>> + ebu_nand_writeb(chip, HSNAND_CLE_OFFS | HSNAND_CS_OFFS,
>> + instr->ctx.cmd.opcode);
>> + break;
>> +
>> + case NAND_OP_ADDR_INSTR:
>> + for (i = 0; i < instr->ctx.addr.naddrs; i++)
>> + ebu_nand_writeb(chip,
>> + HSNAND_ALE_OFFS | HSNAND_CS_OFFS,
>> + instr->ctx.addr.addrs[i]);
>> + break;
>> +
>> + case NAND_OP_DATA_IN_INSTR:
>> + ebu_read_buf(chip, instr->ctx.data.buf.in,
>> + instr->ctx.data.len);
>> + break;
>> +
>> + case NAND_OP_DATA_OUT_INSTR:
>> + ebu_write_buf(chip, instr->ctx.data.buf.out,
>> + instr->ctx.data.len);
>> + break;
>> +
>> + case NAND_OP_WAITRDY_INSTR:
>> + time_out = instr->ctx.waitrdy.timeout_ms * 1000;
>> + ret = ebu_nand_waitrdy(chip, time_out);
>> + break;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static const struct nand_controller_ops ebu_nand_controller_ops = {
>> + .attach_chip = ebu_nand_attach_chip,
>> + .setup_interface = ebu_nand_set_timings,
>> + .exec_op = ebu_nand_exec_op,
>> +};
>> +
>> +static void ebu_dma_cleanup(struct ebu_nand_controller *ebu_host)
>> +{
>> + if (ebu_host->dma_rx)
>> + dma_release_channel(ebu_host->dma_rx);
>> +
>> + if (ebu_host->dma_tx)
>> + dma_release_channel(ebu_host->dma_tx);
>> +}
>> +
>> +static int ebu_nand_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct ebu_nand_controller *ebu_host;
>> + struct nand_chip *nand;
>> + struct mtd_info *mtd;
>> + struct resource *res;
>> + char *resname;
>> + int ret, i;
>> + u32 reg;
>> +
>> + ebu_host = devm_kzalloc(dev, sizeof(*ebu_host), GFP_KERNEL);
>> + if (!ebu_host)
>> + return -ENOMEM;
>> +
>> + ebu_host->dev = dev;
>> + nand_controller_init(&ebu_host->controller);
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
>> + ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(ebu_host->ebu))
>> + return PTR_ERR(ebu_host->ebu);
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsnand");
>> + ebu_host->hsnand = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(ebu_host->hsnand))
>> + return PTR_ERR(ebu_host->hsnand);
>> +
>> + ret = device_property_read_u32(dev, "nand,cs", &reg);
>
> There is no nand,cs property. Use 'reg' instead.
Noted.
>
>> + if (ret) {
>> + dev_err(dev, "failed to get chip select: %d\n", ret);
>> + return ret;
>> + }
>> + ebu_host->cs_num = reg;
>
> The following for loop is weird, above you can only store a single cs
> number, while below you seem to reserve serveral memory areas. Please
> clarify this code.
This IP supports 2 chip select for 2 different memory regions so we used
the below for loop, as per reviewers comment updated.
EBU_CS0_BASE 0xE1C0_0000 (Memory-Mapped)
EBU_CS0_IO_BASE 0x1740_0000 (FPI I/O Mapped)

EBU_CS1_BASE 0xE140_0000 (Memory-Mapped)
EBU_CS1_IO_BASE 0x17C0_0000 (FPI I/O Mapped)

>
>> +
>> + for (i = 0; i < MAX_CS; i++) {
>> + resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> + resname);
>> + ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);
>> + ebu_host->cs[i].nand_pa = res->start;
>> + if (IS_ERR(ebu_host->cs[i].chipaddr))
>> + return PTR_ERR(ebu_host->cs[i].chipaddr);
>> + }
>> +
>> + ebu_host->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(ebu_host->clk))
>> + return dev_err_probe(dev, PTR_ERR(ebu_host->clk),
>> + "failed to get clock\n");
>> +
>> + ret = clk_prepare_enable(ebu_host->clk);
>> + if (ret) {
>> + dev_err(dev, "failed to enable clock: %d\n", ret);
>> + return ret;
>> + }
>> + ebu_host->clk_rate = clk_get_rate(ebu_host->clk);
>> +
>> + ebu_host->dma_tx = dma_request_chan(dev, "tx");
>> + if (IS_ERR(ebu_host->dma_tx))
>> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_tx),
>> + "failed to request DMA tx chan!.\n");
>> +
>> + ebu_host->dma_rx = dma_request_chan(dev, "rx");
>> + if (IS_ERR(ebu_host->dma_rx))
>> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_rx),
>> + "failed to request DMA rx chan!.\n");
>> +
>> + for (i = 0; i < MAX_CS; i++) {
>> + resname = devm_kasprintf(dev, GFP_KERNEL, "addr_sel%d", i);
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> + resname);
>> + if (!res)
>> + return -EINVAL;
>> + ebu_host->cs[i].addr_sel = res->start;
>> + writel(ebu_host->cs[i].addr_sel | EBU_ADDR_MASK(5) |
>> + EBU_ADDR_SEL_REGEN, ebu_host->ebu + EBU_ADDR_SEL(i));
>> + }
>> +
>> + nand_set_flash_node(&ebu_host->chip, dev->of_node);
>> + if (!mtd->name) {
>> + dev_err(ebu_host->dev, "NAND label property is mandatory\n");
>> + return -EINVAL;
>> + }
>> +
>> + mtd = nand_to_mtd(&ebu_host->chip);
>> + mtd->dev.parent = dev;
>> + ebu_host->dev = dev;
>> +
>> + platform_set_drvdata(pdev, ebu_host);
>> + nand_set_controller_data(&ebu_host->chip, ebu_host);
>> +
>> + nand = &ebu_host->chip;
>> + nand->controller = &ebu_host->controller;
>> + nand->controller->ops = &ebu_nand_controller_ops;
>> +
>> + /* Scan to find existence of the device */
>> + ret = nand_scan(&ebu_host->chip, 1);
>> + if (ret)
>> + goto err_cleanup_dma;
>> +
>> + ret = mtd_device_register(mtd, NULL, 0);
>> + if (ret)
>> + goto err_clean_nand;
>> +
>> + return 0;
>> +
>> +err_clean_nand:
>> + nand_cleanup(&ebu_host->chip);
>> +err_cleanup_dma:
>> + ebu_dma_cleanup(ebu_host);
>> + clk_disable_unprepare(ebu_host->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int ebu_nand_remove(struct platform_device *pdev)
>> +{
>> + struct ebu_nand_controller *ebu_host = platform_get_drvdata(pdev);
>> + int ret;
>> +
>> + ret = mtd_device_unregister(nand_to_mtd(&ebu_host->chip));
>> + WARN_ON(ret);
>> + nand_cleanup(&ebu_host->chip);
>> + ebu_nand_disable(&ebu_host->chip);
>> + ebu_dma_cleanup(ebu_host);
>> + clk_disable_unprepare(ebu_host->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id ebu_nand_match[] = {
>> + { .compatible = "intel,nand-controller", },
>
> No version or soc in the compatible? (not mandatory).
Yes, you're right, it should be "intel,lgm-ebunand", but this same
driver supports 2 dfferent SOC's , that's the reason kept as generic
"intel,nand-controller"

Best Regards
Vadivel
>
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, ebu_nand_match);
>> +
>> +static struct platform_driver ebu_nand_driver = {
>> + .probe = ebu_nand_probe,
>> + .remove = ebu_nand_remove,
>> + .driver = {
>> + .name = "intel-nand-controller",
>> + .of_match_table = ebu_nand_match,
>> + },
>> +
>> +};
>> +module_platform_driver(ebu_nand_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Vadivel Murugan R <[email protected]>");
>> +MODULE_DESCRIPTION("Intel's LGM External Bus NAND Controller driver");
>
> Thanks,
> Miquèl
>

2020-10-30 08:26:02

by Miquel Raynal

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

Hello,

> >> +static const struct nand_controller_ops ebu_nand_controller_ops = {
> >> + .attach_chip = ebu_nand_attach_chip,
> >> + .setup_interface = ebu_nand_set_timings,
> >> + .exec_op = ebu_nand_exec_op,
> >> +};
> >> +
> >> +static void ebu_dma_cleanup(struct ebu_nand_controller *ebu_host)
> >> +{
> >> + if (ebu_host->dma_rx)
> >> + dma_release_channel(ebu_host->dma_rx);
> >> +
> >> + if (ebu_host->dma_tx)
> >> + dma_release_channel(ebu_host->dma_tx);
> >> +}
> >> +
> >> +static int ebu_nand_probe(struct platform_device *pdev)
> >> +{
> >> + struct device *dev = &pdev->dev;
> >> + struct ebu_nand_controller *ebu_host;
> >> + struct nand_chip *nand;
> >> + struct mtd_info *mtd;
> >> + struct resource *res;
> >> + char *resname;
> >> + int ret, i;
> >> + u32 reg;
> >> +
> >> + ebu_host = devm_kzalloc(dev, sizeof(*ebu_host), GFP_KERNEL);
> >> + if (!ebu_host)
> >> + return -ENOMEM;
> >> +
> >> + ebu_host->dev = dev;
> >> + nand_controller_init(&ebu_host->controller);
> >> +
> >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
> >> + ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
> >> + if (IS_ERR(ebu_host->ebu))
> >> + return PTR_ERR(ebu_host->ebu);
> >> +
> >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsnand");
> >> + ebu_host->hsnand = devm_ioremap_resource(&pdev->dev, res);
> >> + if (IS_ERR(ebu_host->hsnand))
> >> + return PTR_ERR(ebu_host->hsnand);
> >> +
> >> + ret = device_property_read_u32(dev, "nand,cs", &reg);
> >
> > There is no nand,cs property. Use 'reg' instead.
> Noted.
> >
> >> + if (ret) {
> >> + dev_err(dev, "failed to get chip select: %d\n", ret);
> >> + return ret;
> >> + }
> >> + ebu_host->cs_num = reg;
> >
> > The following for loop is weird, above you can only store a single cs
> > number, while below you seem to reserve serveral memory areas. Please
> > clarify this code.
> This IP supports 2 chip select for 2 different memory regions so we used the below for loop, as per reviewers comment updated.
> EBU_CS0_BASE 0xE1C0_0000 (Memory-Mapped)
> EBU_CS0_IO_BASE 0x1740_0000 (FPI I/O Mapped)
>
> EBU_CS1_BASE 0xE140_0000 (Memory-Mapped)
> EBU_CS1_IO_BASE 0x17C0_0000 (FPI I/O Mapped)

Please make a difference between, "there are two CS, either can be
picked but we can use only one in this driver" or "there are two CS,
one or both can be used". You can start with supporting a single CS (no
matter which one is picked by the user with the reg property) but in
this case there is no such for loop because only 1 CS is used. Or you
can decide that both CS can be populated and in this case you must
handle this in ->select_chip().

>
> >
> >> +
> >> + for (i = 0; i < MAX_CS; i++) {
> >> + resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
> >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> >> + resname);
> >> + ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);
> >> + ebu_host->cs[i].nand_pa = res->start;
> >> + if (IS_ERR(ebu_host->cs[i].chipaddr))
> >> + return PTR_ERR(ebu_host->cs[i].chipaddr);
> >> + }
> >> +
> >> + ebu_host->clk = devm_clk_get(dev, NULL);
> >> + if (IS_ERR(ebu_host->clk))
> >> + return dev_err_probe(dev, PTR_ERR(ebu_host->clk),
> >> + "failed to get clock\n");
> >> +
> >> + ret = clk_prepare_enable(ebu_host->clk);
> >> + if (ret) {
> >> + dev_err(dev, "failed to enable clock: %d\n", ret);
> >> + return ret;
> >> + }
> >> + ebu_host->clk_rate = clk_get_rate(ebu_host->clk);
> >> +
> >> + ebu_host->dma_tx = dma_request_chan(dev, "tx");
> >> + if (IS_ERR(ebu_host->dma_tx))
> >> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_tx),
> >> + "failed to request DMA tx chan!.\n");
> >> +
> >> + ebu_host->dma_rx = dma_request_chan(dev, "rx");
> >> + if (IS_ERR(ebu_host->dma_rx))
> >> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_rx),
> >> + "failed to request DMA rx chan!.\n");
> >> +
> >> + for (i = 0; i < MAX_CS; i++) {
> >> + resname = devm_kasprintf(dev, GFP_KERNEL, "addr_sel%d", i);
> >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> >> + resname);
> >> + if (!res)
> >> + return -EINVAL;
> >> + ebu_host->cs[i].addr_sel = res->start;
> >> + writel(ebu_host->cs[i].addr_sel | EBU_ADDR_MASK(5) |
> >> + EBU_ADDR_SEL_REGEN, ebu_host->ebu + EBU_ADDR_SEL(i));
> >> + }
> >> +
> >> + nand_set_flash_node(&ebu_host->chip, dev->of_node);
> >> + if (!mtd->name) {
> >> + dev_err(ebu_host->dev, "NAND label property is mandatory\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + mtd = nand_to_mtd(&ebu_host->chip);
> >> + mtd->dev.parent = dev;
> >> + ebu_host->dev = dev;
> >> +
> >> + platform_set_drvdata(pdev, ebu_host);
> >> + nand_set_controller_data(&ebu_host->chip, ebu_host);
> >> +
> >> + nand = &ebu_host->chip;
> >> + nand->controller = &ebu_host->controller;
> >> + nand->controller->ops = &ebu_nand_controller_ops;
> >> +
> >> + /* Scan to find existence of the device */
> >> + ret = nand_scan(&ebu_host->chip, 1);
> >> + if (ret)
> >> + goto err_cleanup_dma;
> >> +
> >> + ret = mtd_device_register(mtd, NULL, 0);
> >> + if (ret)
> >> + goto err_clean_nand;
> >> +
> >> + return 0;
> >> +
> >> +err_clean_nand:
> >> + nand_cleanup(&ebu_host->chip);
> >> +err_cleanup_dma:
> >> + ebu_dma_cleanup(ebu_host);
> >> + clk_disable_unprepare(ebu_host->clk);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int ebu_nand_remove(struct platform_device *pdev)
> >> +{
> >> + struct ebu_nand_controller *ebu_host = platform_get_drvdata(pdev);
> >> + int ret;
> >> +
> >> + ret = mtd_device_unregister(nand_to_mtd(&ebu_host->chip));
> >> + WARN_ON(ret);
> >> + nand_cleanup(&ebu_host->chip);
> >> + ebu_nand_disable(&ebu_host->chip);
> >> + ebu_dma_cleanup(ebu_host);
> >> + clk_disable_unprepare(ebu_host->clk);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct of_device_id ebu_nand_match[] = {
> >> + { .compatible = "intel,nand-controller", },
> >
> > No version or soc in the compatible? (not mandatory).
> Yes, you're right, it should be "intel,lgm-ebunand", but this same driver supports 2 dfferent SOC's , that's the reason kept as generic
> "intel,nand-controller"

In this case I guess declaring two compatibles is the way to go.


Thanks,
Miquèl

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

Hi Miquel,

Thank you for the review comments...
On 30/10/2020 4:23 pm, Miquel Raynal wrote:
>>>> +
>>>> +static const struct of_device_id ebu_nand_match[] = {
>>>> + { .compatible = "intel,nand-controller", },
>>> No version or soc in the compatible? (not mandatory).
>> Yes, you're right, it should be "intel,lgm-ebunand", but this same driver supports 2 dfferent SOC's , that's the reason kept as generic
>> "intel,nand-controller"
> In this case I guess declaring two compatibles is the way to go.
Ok, Noted.

Best Regards
Vadivel

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

Hi Miquel,

On 30/10/2020 4:23 pm, Miquel Raynal wrote:
> Hello,
>
>>>> +static const struct nand_controller_ops ebu_nand_controller_ops = {
>>>> + .attach_chip = ebu_nand_attach_chip,
>>>> + .setup_interface = ebu_nand_set_timings,
>>>> + .exec_op = ebu_nand_exec_op,
>>>> +};
>>>> +
>>>> +static void ebu_dma_cleanup(struct ebu_nand_controller *ebu_host)
>>>> +{
>>>> + if (ebu_host->dma_rx)
>>>> + dma_release_channel(ebu_host->dma_rx);
>>>> +
>>>> + if (ebu_host->dma_tx)
>>>> + dma_release_channel(ebu_host->dma_tx);
>>>> +}
>>>> +
>>>> +static int ebu_nand_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct device *dev = &pdev->dev;
>>>> + struct ebu_nand_controller *ebu_host;
>>>> + struct nand_chip *nand;
>>>> + struct mtd_info *mtd;
>>>> + struct resource *res;
>>>> + char *resname;
>>>> + int ret, i;
>>>> + u32 reg;
>>>> +
>>>> + ebu_host = devm_kzalloc(dev, sizeof(*ebu_host), GFP_KERNEL);
>>>> + if (!ebu_host)
>>>> + return -ENOMEM;
>>>> +
>>>> + ebu_host->dev = dev;
>>>> + nand_controller_init(&ebu_host->controller);
>>>> +
>>>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
>>>> + ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
>>>> + if (IS_ERR(ebu_host->ebu))
>>>> + return PTR_ERR(ebu_host->ebu);
>>>> +
>>>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsnand");
>>>> + ebu_host->hsnand = devm_ioremap_resource(&pdev->dev, res);
>>>> + if (IS_ERR(ebu_host->hsnand))
>>>> + return PTR_ERR(ebu_host->hsnand);
>>>> +
>>>> + ret = device_property_read_u32(dev, "nand,cs", &reg);
>>>
>>> There is no nand,cs property. Use 'reg' instead.
>> Noted.
>>>
>>>> + if (ret) {
>>>> + dev_err(dev, "failed to get chip select: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> + ebu_host->cs_num = reg;
>>>
>>> The following for loop is weird, above you can only store a single cs
>>> number, while below you seem to reserve serveral memory areas. Please
>>> clarify this code.
>> This IP supports 2 chip select for 2 different memory regions so we used the below for loop, as per reviewers comment updated.
>> EBU_CS0_BASE 0xE1C0_0000 (Memory-Mapped)
>> EBU_CS0_IO_BASE 0x1740_0000 (FPI I/O Mapped)
>>
>> EBU_CS1_BASE 0xE140_0000 (Memory-Mapped)
>> EBU_CS1_IO_BASE 0x17C0_0000 (FPI I/O Mapped)
>
> Please make a difference between, "there are two CS, either can be
> picked but we can use only one in this driver" or "there are two CS,
> one or both can be used". You can start with supporting a single CS (no
> matter which one is picked by the user with the reg property) but in
> this case there is no such for loop because only 1 CS is used. Or you
> can decide that both CS can be populated and in this case you must
> handle this in ->select_chip().
Sure, I will hadle the same as per your suggestion, thanks a lot!

Regards
Vadivel
>
>>
>>>
>>>> +
>>>> + for (i = 0; i < MAX_CS; i++) {
>>>> + resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
>>>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>>>> + resname);
>>>> + ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);
>>>> + ebu_host->cs[i].nand_pa = res->start;
>>>> + if (IS_ERR(ebu_host->cs[i].chipaddr))
>>>> + return PTR_ERR(ebu_host->cs[i].chipaddr);
>>>> + }
>>>> +
>>>> + ebu_host->clk = devm_clk_get(dev, NULL);
>>>> + if (IS_ERR(ebu_host->clk))
>>>> + return dev_err_probe(dev, PTR_ERR(ebu_host->clk),
>>>> + "failed to get clock\n");
>>>> +
>>>> + ret = clk_prepare_enable(ebu_host->clk);
>>>> + if (ret) {
>>>> + dev_err(dev, "failed to enable clock: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> + ebu_host->clk_rate = clk_get_rate(ebu_host->clk);
>>>> +
>>>> + ebu_host->dma_tx = dma_request_chan(dev, "tx");
>>>> + if (IS_ERR(ebu_host->dma_tx))
>>>> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_tx),
>>>> + "failed to request DMA tx chan!.\n");
>>>> +
>>>> + ebu_host->dma_rx = dma_request_chan(dev, "rx");
>>>> + if (IS_ERR(ebu_host->dma_rx))
>>>> + return dev_err_probe(dev, PTR_ERR(ebu_host->dma_rx),
>>>> + "failed to request DMA rx chan!.\n");
>>>> +
>>>> + for (i = 0; i < MAX_CS; i++) {
>>>> + resname = devm_kasprintf(dev, GFP_KERNEL, "addr_sel%d", i);
>>>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>>>> + resname);
>>>> + if (!res)
>>>> + return -EINVAL;
>>>> + ebu_host->cs[i].addr_sel = res->start;
>>>> + writel(ebu_host->cs[i].addr_sel | EBU_ADDR_MASK(5) |
>>>> + EBU_ADDR_SEL_REGEN, ebu_host->ebu + EBU_ADDR_SEL(i));
>>>> + }
>>>> +
>>>> + nand_set_flash_node(&ebu_host->chip, dev->of_node);
>>>> + if (!mtd->name) {
>>>> + dev_err(ebu_host->dev, "NAND label property is mandatory\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + mtd = nand_to_mtd(&ebu_host->chip);
>>>> + mtd->dev.parent = dev;
>>>> + ebu_host->dev = dev;
>>>> +
>>>> + platform_set_drvdata(pdev, ebu_host);
>>>> + nand_set_controller_data(&ebu_host->chip, ebu_host);
>>>> +
>>>> + nand = &ebu_host->chip;
>>>> + nand->controller = &ebu_host->controller;
>>>> + nand->controller->ops = &ebu_nand_controller_ops;
>>>> +
>>>> + /* Scan to find existence of the device */
>>>> + ret = nand_scan(&ebu_host->chip, 1);
>>>> + if (ret)
>>>> + goto err_cleanup_dma;
>>>> +
>>>> + ret = mtd_device_register(mtd, NULL, 0);
>>>> + if (ret)
>>>> + goto err_clean_nand;
>>>> +
>>>> + return 0;
>>>> +
>>>> +err_clean_nand:
>>>> + nand_cleanup(&ebu_host->chip);
>>>> +err_cleanup_dma:
>>>> + ebu_dma_cleanup(ebu_host);
>>>> + clk_disable_unprepare(ebu_host->clk);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static int ebu_nand_remove(struct platform_device *pdev)
>>>> +{
>>>> + struct ebu_nand_controller *ebu_host = platform_get_drvdata(pdev);
>>>> + int ret;
>>>> +
>>>> + ret = mtd_device_unregister(nand_to_mtd(&ebu_host->chip));
>>>> + WARN_ON(ret);
>>>> + nand_cleanup(&ebu_host->chip);
>>>> + ebu_nand_disable(&ebu_host->chip);
>>>> + ebu_dma_cleanup(ebu_host);
>>>> + clk_disable_unprepare(ebu_host->clk);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id ebu_nand_match[] = {
>>>> + { .compatible = "intel,nand-controller", },
>>>
>>> No version or soc in the compatible? (not mandatory).
>> Yes, you're right, it should be "intel,lgm-ebunand", but this same driver supports 2 dfferent SOC's , that's the reason kept as generic
>> "intel,nand-controller"
>
> In this case I guess declaring two compatibles is the way to go.
>
>
> Thanks,
> Miquèl
>

2020-10-30 11:15:44

by kernel test robot

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

Hi "Ramuthevar,Vadivel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.10-rc1 next-20201030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Ramuthevar-Vadivel-MuruganX/mtd-rawnand-Add-NAND-controller-support-on-Intel-LGM-SoC/20201026-153225
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3650b228f83adda7e5ee532e2b90429c03f7b9ec
config: x86_64-randconfig-r023-20201030 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 772aaa602383cf82795572ebcd86b0e660f3579f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/8d6004cac3a7ca929c7c0a49a0d02dceaf8fd5d2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ramuthevar-Vadivel-MuruganX/mtd-rawnand-Add-NAND-controller-support-on-Intel-LGM-SoC/20201026-153225
git checkout 8d6004cac3a7ca929c7c0a49a0d02dceaf8fd5d2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/mtd/nand/raw/intel-nand-controller.c:664:7: warning: variable 'mtd' is uninitialized when used here [-Wuninitialized]
if (!mtd->name) {
^~~
drivers/mtd/nand/raw/intel-nand-controller.c:590:22: note: initialize the variable 'mtd' to silence this warning
struct mtd_info *mtd;
^
= NULL
1 warning generated.

vim +/mtd +664 drivers/mtd/nand/raw/intel-nand-controller.c

584
585 static int ebu_nand_probe(struct platform_device *pdev)
586 {
587 struct device *dev = &pdev->dev;
588 struct ebu_nand_controller *ebu_host;
589 struct nand_chip *nand;
590 struct mtd_info *mtd;
591 struct resource *res;
592 char *resname;
593 int ret, i;
594 u32 reg;
595
596 ebu_host = devm_kzalloc(dev, sizeof(*ebu_host), GFP_KERNEL);
597 if (!ebu_host)
598 return -ENOMEM;
599
600 ebu_host->dev = dev;
601 nand_controller_init(&ebu_host->controller);
602
603 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ebunand");
604 ebu_host->ebu = devm_ioremap_resource(&pdev->dev, res);
605 if (IS_ERR(ebu_host->ebu))
606 return PTR_ERR(ebu_host->ebu);
607
608 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsnand");
609 ebu_host->hsnand = devm_ioremap_resource(&pdev->dev, res);
610 if (IS_ERR(ebu_host->hsnand))
611 return PTR_ERR(ebu_host->hsnand);
612
613 ret = device_property_read_u32(dev, "nand,cs", &reg);
614 if (ret) {
615 dev_err(dev, "failed to get chip select: %d\n", ret);
616 return ret;
617 }
618 ebu_host->cs_num = reg;
619
620 for (i = 0; i < MAX_CS; i++) {
621 resname = devm_kasprintf(dev, GFP_KERNEL, "nand_cs%d", i);
622 res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
623 resname);
624 ebu_host->cs[i].chipaddr = devm_ioremap_resource(dev, res);
625 ebu_host->cs[i].nand_pa = res->start;
626 if (IS_ERR(ebu_host->cs[i].chipaddr))
627 return PTR_ERR(ebu_host->cs[i].chipaddr);
628 }
629
630 ebu_host->clk = devm_clk_get(dev, NULL);
631 if (IS_ERR(ebu_host->clk))
632 return dev_err_probe(dev, PTR_ERR(ebu_host->clk),
633 "failed to get clock\n");
634
635 ret = clk_prepare_enable(ebu_host->clk);
636 if (ret) {
637 dev_err(dev, "failed to enable clock: %d\n", ret);
638 return ret;
639 }
640 ebu_host->clk_rate = clk_get_rate(ebu_host->clk);
641
642 ebu_host->dma_tx = dma_request_chan(dev, "tx");
643 if (IS_ERR(ebu_host->dma_tx))
644 return dev_err_probe(dev, PTR_ERR(ebu_host->dma_tx),
645 "failed to request DMA tx chan!.\n");
646
647 ebu_host->dma_rx = dma_request_chan(dev, "rx");
648 if (IS_ERR(ebu_host->dma_rx))
649 return dev_err_probe(dev, PTR_ERR(ebu_host->dma_rx),
650 "failed to request DMA rx chan!.\n");
651
652 for (i = 0; i < MAX_CS; i++) {
653 resname = devm_kasprintf(dev, GFP_KERNEL, "addr_sel%d", i);
654 res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
655 resname);
656 if (!res)
657 return -EINVAL;
658 ebu_host->cs[i].addr_sel = res->start;
659 writel(ebu_host->cs[i].addr_sel | EBU_ADDR_MASK(5) |
660 EBU_ADDR_SEL_REGEN, ebu_host->ebu + EBU_ADDR_SEL(i));
661 }
662
663 nand_set_flash_node(&ebu_host->chip, dev->of_node);
> 664 if (!mtd->name) {
665 dev_err(ebu_host->dev, "NAND label property is mandatory\n");
666 return -EINVAL;
667 }
668
669 mtd = nand_to_mtd(&ebu_host->chip);
670 mtd->dev.parent = dev;
671 ebu_host->dev = dev;
672
673 platform_set_drvdata(pdev, ebu_host);
674 nand_set_controller_data(&ebu_host->chip, ebu_host);
675
676 nand = &ebu_host->chip;
677 nand->controller = &ebu_host->controller;
678 nand->controller->ops = &ebu_nand_controller_ops;
679
680 /* Scan to find existence of the device */
681 ret = nand_scan(&ebu_host->chip, 1);
682 if (ret)
683 goto err_cleanup_dma;
684
685 ret = mtd_device_register(mtd, NULL, 0);
686 if (ret)
687 goto err_clean_nand;
688
689 return 0;
690
691 err_clean_nand:
692 nand_cleanup(&ebu_host->chip);
693 err_cleanup_dma:
694 ebu_dma_cleanup(ebu_host);
695 clk_disable_unprepare(ebu_host->clk);
696
697 return ret;
698 }
699

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (6.36 kB)
.config.gz (41.94 kB)
Download all attachments