Skip to contentSkip to conversation

Add support for nuvoton ma35 nand controller

PATCH Superseded Hui-Ping Chen 3 participants 7 messages Sep 20, 2024 – Sep 24, 2024

Subsystems (4)

v5 of 11 Superseded Latest is v11 →

lore
Share & tools
lore
#0 lore raw

v5 cover Add support for nuvoton ma35 nand controller

This patch series adds the mtd nand driver for the nuvoton ma35 ARMv8 SoC. It includes DT binding documentation and the ma35 mtd nand driver.

v5:
- Update ma35d1 mtd nand driver
- Remove unnecessary definitions and comments.
- Modified DMA API call sequence.
- Move the ECC check out of the interrupt handler. Check it after reading a page.

v4:
- Update nuvoton,ma35d1-nand.yaml
- rename 'nuvoton,ma35d1-nand' to 'nuvoton,ma35d1-nand-controller'.
- Update ma35d1 mtd nand driver
- Rewrite the NAND driver using the exec_op API.

v3:
- Update ma35d1 mtd nand driver
- Release IRQ handler.
- Remove unused functions.
- Remove '.owner'.

v2:
- Update nuvoton,ma35d1-nand.yaml
- Adjust the order and remove any unnecessary items.
- Add 'nand-ecc-step-size' and 'nand-ecc-strength' to the required list.
- Update ma35d1 mtd nand driver
- Fix coding style.
- Use 'devm_clk_get' instead of 'of_clk_get'.
- Use 'dev_err_probe' instead of 'dev_err'.
- Remove 'pr_info' and 'of_match_ptr'.
- Remove 'module_init' and 'module_exit'.

Hui-Ping Chen (2):
dt-bindings: mtd: nuvoton,ma35d1-nand: add new bindings
mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

 .../bindings/mtd/nuvoton,ma35d1-nand.yaml     |  93 ++
 drivers/mtd/nand/raw/Kconfig                  |   8 +
 drivers/mtd/nand/raw/Makefile                 |   1 +
 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c    | 886 ++++++++++++++++++
 4 files changed, 988 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml
 create mode 100644 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c
Signature · 1 line
2.25.1
#1 lore raw

PATCH v5 1/2 dt-bindings: mtd: nuvoton,ma35d1-nand: add new bindings

Add dt-bindings for the Nuvoton MA35 SoC NAND Controller.

---
 .../bindings/mtd/nuvoton,ma35d1-nand.yaml     | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml
Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml new file +93 -0
new file mode 100644
index 000000000000..a8a549644c98
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/nuvoton,ma35d1-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 NAND Flash Interface (NFI) Controller
+
+maintainers:
+  - Hui-Ping Chen <[email protected]>
+
+allOf:
+  - $ref: nand-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,ma35d1-nand-controller
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+patternProperties:
+  "^nand@[a-f0-9]$":
+    type: object
+    $ref: raw-nand-chip.yaml
+    properties:
+      nand-ecc-step-size:
+        enum: [512, 1024]
+
+      nand-ecc-strength:
+        enum: [8, 12, 24]
+
+    required:
+      - nand-ecc-step-size
+      - nand-ecc-strength
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        nand-controller@401A0000 {
+            compatible = "nuvoton,ma35d1-nand-controller";
+            reg = <0x0 0x401A0000 0x0 0x1000>;
+            interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clk NAND_GATE>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            nand@0 {
+                reg = <0>;
+
+                nand-on-flash-bbt;
+                nand-ecc-step-size = <512>;
+                nand-ecc-strength = <8>;
+
+                partitions {
+                    compatible = "fixed-partitions";
+                    #address-cells = <1>;
+                    #size-cells = <1>;
+
+                    uboot@0 {
+                        label = "nand-uboot";
+                        read-only;
+                        reg = <0x0 0x300000>;
+                    };
+                };
+            };
+        };
+    };
+
+...
Signature · 1 line
2.25.1
#2 lore raw

PATCH v5 2/2 mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

In reply to Hui-Ping Chen

Nuvoton MA35 SoCs NAND Flash Interface Controller supports 2kiB, 4kiB and 8kiB page size, and up to 8-bit, 12-bit, and 24-bit hardware ECC calculation circuit to protect data.

---
 drivers/mtd/nand/raw/Kconfig               |   8 +
 drivers/mtd/nand/raw/Makefile              |   1 +
 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c | 886 +++++++++++++++++++++
 3 files changed, 895 insertions(+)
 create mode 100644 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c
drivers/mtd/nand/raw/Kconfig +8 -0
index 614257308516..a95d91e61c42 100644
@@ -448,6 +448,14 @@ config MTD_NAND_RENESAS
 	  Enables support for the NAND controller found on Renesas R-Car
 	  Gen3 and RZ/N1 SoC families.
 
+config MTD_NAND_NUVOTON_MA35
+	tristate "Nuvoton MA35 SoC NAND controller"
+	depends on ARCH_MA35 || COMPILE_TEST
+	depends on OF
+	help
+	  Enables support for the NAND controller found on
+	  the Nuvoton MA35 series SoCs.
+
 comment "Misc"
 
 config MTD_SM_COMMON
drivers/mtd/nand/raw/Makefile +1 -0
index 25120a4afada..b8e1b3af6942 100644
@@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_INTEL_LGM)	+= intel-nand-controller.o
 obj-$(CONFIG_MTD_NAND_ROCKCHIP)		+= rockchip-nand-controller.o
 obj-$(CONFIG_MTD_NAND_PL35X)		+= pl35x-nand-controller.o
 obj-$(CONFIG_MTD_NAND_RENESAS)		+= renesas-nand-controller.o
+obj-$(CONFIG_MTD_NAND_NUVOTON_MA35)	+= nuvoton_ma35d1_nand.o
 
 nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o
 nand-objs += nand_onfi.o
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +886 -0

886 lines not on this page · raw message

Signature · 1 line
2.25.1
#3 lore raw

PATCH v5 2/2 mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

Hi,

Quoted 63 lines

Nuvoton MA35 SoCs NAND Flash Interface Controller supports 2kiB, 4kiB and 8kiB page size, and up to 8-bit, 12-bit, and 24-bit hardware ECC calculation circuit to protect data.

---
   drivers/mtd/nand/raw/Kconfig               |   8 +
   drivers/mtd/nand/raw/Makefile              |   1 +
   drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c | 886 +++++++++++++++++++++
   3 files changed, 895 insertions(+)

create mode 100644 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c

drivers/mtd/nand/raw/Kconfig +8 -0
index 614257308516..a95d91e61c42 100644
@@ -448,6 +448,14 @@ config MTD_NAND_RENESAS
   	  Enables support for the NAND controller found on Renesas R-Car
   	  Gen3 and RZ/N1 SoC families.
 
+config MTD_NAND_NUVOTON_MA35
+	tristate "Nuvoton MA35 SoC NAND controller"
+	depends on ARCH_MA35 || COMPILE_TEST
+	depends on OF
+	help
+	  Enables support for the NAND controller found on
+	  the Nuvoton MA35 series SoCs.
+
   comment "Misc"
 
   config MTD_SM_COMMON
drivers/mtd/nand/raw/Makefile +1 -0
index 25120a4afada..b8e1b3af6942 100644
@@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_INTEL_LGM)	+= intel-nand-controller.o
   obj-$(CONFIG_MTD_NAND_ROCKCHIP)		+= rockchip-nand-controller.o
   obj-$(CONFIG_MTD_NAND_PL35X)		+= pl35x-nand-controller.o
   obj-$(CONFIG_MTD_NAND_RENESAS)		+= renesas-nand-controller.o
+obj-$(CONFIG_MTD_NAND_NUVOTON_MA35)	+= nuvoton_ma35d1_nand.o
 
   nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o
   nand-objs += nand_onfi.o
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +12 -0
new file mode 100644
index 000000000000..5b53b7f0b9cb
@@ -0,0 +1,886 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Nuvoton Technology Corp.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 22

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+

nit: Unnecessary new line.

Quoted 44 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +44 -0
+/* NFI Registers */
+#define MA35_NFI_REG_DMACTL		0x400
+#define   DMA_EN				BIT(0)
+#define   DMA_RST				BIT(1)
+#define   DMA_BUSY				BIT(9)
+
+#define MA35_NFI_REG_DMASA		0x408
+#define MA35_NFI_REG_GCTL		0x800
+#define   NAND_EN				BIT(3)
+
+#define MA35_NFI_REG_NANDCTL		0x8A0
+#define   SWRST				BIT(0)
+#define   DMA_R_EN				BIT(1)
+#define   DMA_W_EN				BIT(2)
+#define   ECC_CHK				BIT(7)
+#define   PROT3BEN				BIT(8)
+#define   PSIZE_2K				BIT(16)
+#define   PSIZE_4K				BIT(17)
+#define   PSIZE_8K				GENMASK(17, 16)
+#define   PSIZE_MASK				GENMASK(17, 16)
+#define   BCH_T24				BIT(18)
+#define   BCH_T8				BIT(20)
+#define   BCH_T12				BIT(21)
+#define   BCH_NONE				(0x0)
+#define   BCH_MASK				GENMASK(22, 18)
+#define   ECC_EN				BIT(23)
+#define   DISABLE_CS0				BIT(25)
+
+#define MA35_NFI_REG_NANDINTEN	0x8A8
+#define MA35_NFI_REG_NANDINTSTS	0x8AC
+#define   INT_DMA				BIT(0)
+#define   INT_ECC				BIT(2)
+#define   INT_RB0				BIT(10)
+#define   INT_RB0_STS				BIT(18)
+
+#define MA35_NFI_REG_NANDCMD		0x8B0
+#define MA35_NFI_REG_NANDADDR		0x8B4
+#define   ENDADDR				BIT(31)
+
+#define MA35_NFI_REG_NANDDATA		0x8B8
+#define MA35_NFI_REG_NANDRACTL	0x8BC
+#define MA35_NFI_REG_NANDECTL		0x8C0
+#define   ENABLE_WP				0x0
+#define   DISABLE_WP				BIT(0)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 76

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+
+#define MA35_NFI_REG_NANDECCES0	0x8D0
+#define   ECC_STATUS_MASK			GENMASK(1, 0)
+#define   ECC_ERR_CNT_MASK			GENMASK(4, 0)
+
+#define MA35_NFI_REG_NANDECCEA0	0x900
+#define MA35_NFI_REG_NANDECCED0	0x960
+#define MA35_NFI_REG_NANDRA0		0xA00
+
+

nit: Unnecessary new line.

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 87

Quoted 11 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +11 -0
+/* Define for the BCH hardware ECC engine */
+/* define the total padding bytes for 512/1024 data segment */
+#define MA35_BCH_PADDING_512	32
+#define MA35_BCH_PADDING_1024	64
+/* define the BCH parity code length for 512 bytes data pattern */
+#define MA35_PARITY_BCH8	15
+#define MA35_PARITY_BCH12	23
+/* define the BCH parity code length for 1024 bytes data pattern */
+#define MA35_PARITY_BCH24	45
+
+

nit: Unnecessary new line.

Quoted 132 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +132 -0
+struct ma35_nand_info {
+	struct nand_controller controller;
+	struct nand_chip chip;
+	struct device *dev;
+	void __iomem *regs;
+	int irq;
+	struct clk *clk;
+	struct completion complete;
+
+	u32 bch;
+	u32 bitflips;
+	u8 *ecc_buf;
+};
+
+static int ma35_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->length = chip->ecc.total;
+	oobregion->offset = mtd->oobsize - oobregion->length;
+
+	return 0;
+}
+
+static int ma35_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->length = mtd->oobsize - chip->ecc.total - 2;
+	oobregion->offset = 2;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops ma35_ooblayout_ops = {
+	.free = ma35_ooblayout_free,
+	.ecc = ma35_ooblayout_ecc,
+};
+
+static inline void ma35_clear_spare(struct nand_chip *chip, int size)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	int i;
+
+	for (i = 0; i < size/4; i++)
+		writel(0xff, nand->regs + MA35_NFI_REG_NANDRA0);
+}
+
+static inline void read_remaining_bytes(struct ma35_nand_info *nand, u32 *buf,
+						u32 offset, int size)
+{
+	u32 value = readl(nand->regs + MA35_NFI_REG_NANDRA0 + offset);
+	u8 *ptr = (u8 *)buf;
+	int i;
+
+	for (i = 0; i < size; i++)
+		ptr[i] = (value >> (i * 8)) & 0xff;
+}
+
+
+static inline void ma35_read_spare(struct nand_chip *chip, int size, u32 *buf, u32 offset)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	int i, j;
+
+	if ((offset % 4) == 0) {
+		for (i = 0, j = 0; i < size / 4; i++, j += 4)
+			*buf++ = readl(nand->regs + MA35_NFI_REG_NANDRA0 + offset + j);
+
+		read_remaining_bytes(nand, buf, offset + j, size % 4);
+	} else {
+		read_remaining_bytes(nand, buf, offset, 4 - (offset % 4));
+		offset += 4;
+		size -= (4 - (offset % 4));
+
+		for (i = 0, j = 0; i < size / 4; i++, j += 4)
+			*buf++ = readl(nand->regs + MA35_NFI_REG_NANDRA0 + offset + j);
+
+		read_remaining_bytes(nand, buf, offset + j, size % 4);
+	}
+}
+
+static inline void ma35_write_spare(struct nand_chip *chip, int size, u32 *buf)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	u32 value;
+	int i, j;
+	u8 *ptr;
+
+	for (i = 0, j = 0; i < size / 4; i++, j += 4)
+		writel(*buf++, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+
+	ptr = (u8 *)buf;
+	switch (size % 4) {
+	case 1:
+		writel(*ptr, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+		break;
+	case 2:
+		value = *ptr | (*(ptr+1) << 8);
+		writel(value, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+		break;
+	case 3:
+		value = *ptr | (*(ptr+1) << 8) | (*(ptr+2) << 16);
+		writel(value, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+		break;
+	default:
+		break;
+	}
+}
+
+static inline void ma35_nand_target_enable(struct ma35_nand_info *nand)
+{
+	writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),
+		nand->regs+MA35_NFI_REG_NANDCTL);
+}
+
+static inline void ma35_nand_target_disable(struct ma35_nand_info *nand)
+{
+	writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,
+		nand->regs + MA35_NFI_REG_NANDCTL);
+}
+
+
+static void ma35_nand_hwecc_init(struct ma35_nand_info *nand)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 229

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+{
+	struct mtd_info *mtd = nand_to_mtd(&nand->chip);
+	u32 reg;
+
+	/* resets the internal state machine and counters */
+	reg = readl(nand->regs + MA35_NFI_REG_NANDCTL);
+	reg |= SWRST;
+	writel(reg, nand->regs + MA35_NFI_REG_NANDCTL);
+	while (readl(nand->regs + MA35_NFI_REG_NANDCTL) & SWRST)
+		;

Shouldn't there be a timeout?

Quoted 15 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +15 -0
+
+	/* Redundant area size */
+	writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
+	/* Protect redundant 3 bytes */
+	reg = readl(nand->regs + MA35_NFI_REG_NANDCTL);
+	reg |= (PROT3BEN | ECC_CHK);
+	writel(reg, nand->regs + MA35_NFI_REG_NANDCTL);
+
+	if (nand->bch == BCH_NONE) {
+		/* Disable H/W ECC, ECC parity check enable bit during read page */
+		writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~ECC_EN),
+			nand->regs + MA35_NFI_REG_NANDCTL);
+	} else {
+		/* Set BCH algorithm */

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 254

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+		writel((readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~BCH_MASK)) |
+			nand->bch, nand->regs + MA35_NFI_REG_NANDCTL);
+
+		/* Enable H/W ECC, ECC parity check enable bit during read page */
+		writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | ECC_EN,
+			nand->regs + MA35_NFI_REG_NANDCTL);
+	}
+}
+
+

nit: Unnecessary new line.

Quoted 182 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +182 -0
+/* Correct data by BCH alrogithm */
+static void ma35_nfi_correct(struct ma35_nand_info *nand, u8 index,
+				 u8 err_cnt, u8 *addr)
+{
+	u32 temp_data[24], temp_addr[24];
+	u32 padding_len, parity_len;
+	u32 value, offset, remain;
+	u32 err_data[6];
+	u8  i, j;
+
+	/* configurations */
+	switch (nand->bch) {
+	case BCH_T24:
+		parity_len = MA35_PARITY_BCH24;
+		padding_len = MA35_BCH_PADDING_1024;
+		break;
+	case BCH_T12:
+		parity_len = MA35_PARITY_BCH12;
+		padding_len = MA35_BCH_PADDING_512;
+		break;
+	case BCH_T8:
+		parity_len = MA35_PARITY_BCH8;
+		padding_len = MA35_BCH_PADDING_512;
+		break;
+	default:
+		dev_warn(nand->dev, "NAND ERROR: invalid SMCR_BCH_TSEL = 0x%08X\n",
+			(u32)(readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK));
+		return;
+	}
+
+	/* got valid BCH_ECC_DATAx and parse them to temp_data[]
+	 * got the valid register number of BCH_ECC_DATAx since
+	 * one register include 4 error bytes
+	 */
+	j = (err_cnt + 3) / 4;
+	j = (j > 6) ? 6 : j;
+	for (i = 0; i < j; i++)
+		err_data[i] = readl(nand->regs + MA35_NFI_REG_NANDECCED0 + i * 4);
+
+	for (i = 0; i < j; i++) {
+		temp_data[i*4+0] = err_data[i] & 0xff;
+		temp_data[i*4+1] = (err_data[i] >> 8) & 0xff;
+		temp_data[i*4+2] = (err_data[i] >> 16) & 0xff;
+		temp_data[i*4+3] = (err_data[i] >> 24) & 0xff;
+	}
+
+	/* got valid REG_BCH_ECC_ADDRx and parse them to temp_addr[]
+	 * got the valid register number of REG_BCH_ECC_ADDRx since
+	 * one register include 2 error addresses
+	 */
+	j = (err_cnt + 1) / 2;
+	j = (j > 12) ? 12 : j;
+	for (i = 0; i < j; i++) {
+		temp_addr[i*2+0] = readl(nand->regs + MA35_NFI_REG_NANDECCEA0 + i * 4)
+					& 0x07ff;
+		temp_addr[i*2+1] = (readl(nand->regs + MA35_NFI_REG_NANDECCEA0 + i * 4)
+					>> 16) & 0x07ff;
+	}
+
+	/* pointer to begin address of field that with data error */
+	addr += index * nand->chip.ecc.steps;
+
+	/* correct each error bytes */
+	for (i = 0; i < err_cnt; i++) {
+		u32 corrected_index = temp_addr[i];
+
+		/* for wrong data in field */
+		if (corrected_index < nand->chip.ecc.steps)
+			*(addr + corrected_index) ^= temp_data[i];
+
+		/* for wrong first-3-bytes in redundancy area */
+		else if (corrected_index < (nand->chip.ecc.steps + 3)) {
+			corrected_index -= nand->chip.ecc.steps;
+			temp_addr[i] += (parity_len * index);	/* field offset */
+
+			value = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+			value ^= temp_data[i] << (8 * corrected_index);
+			writel(value, nand->regs + MA35_NFI_REG_NANDRA0);
+		}
+		/* for wrong parity code in redundancy area
+		 * BCH_ERR_ADDRx = [data in field] + [3 bytes] + [xx] + [parity code]
+		 *                                   |<--     padding bytes      -->|
+		 * The BCH_ERR_ADDRx for last parity code always = field size + padding size.
+		 * So, the first parity code = field size + padding size - parity code length.
+		 * For example, for BCH T12, the first parity code = 512 + 32 - 23 = 521.
+		 * That is, error byte address offset within field is
+		 */
+		else {
+			corrected_index -= (nand->chip.ecc.steps + padding_len - parity_len);
+
+			/* final address = first parity code of first field +
+			 *                 offset of fields +
+			 *                 offset within field
+			 */
+			offset = (readl(nand->regs + MA35_NFI_REG_NANDRACTL) & 0x1ff) -
+				(parity_len * nand->chip.ecc.steps) +
+				(parity_len * index) + corrected_index;
+
+			remain = offset % 4;
+			value = readl(nand->regs + MA35_NFI_REG_NANDRA0 + offset - remain);
+			value ^= temp_data[i] << (8 * remain);
+			writel(value, nand->regs + MA35_NFI_REG_NANDRA0 + offset - remain);
+		}
+	}
+}
+
+static int ma35_nfi_ecc_check(struct nand_chip *chip, u8 *addr)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int i, j, nchunks = 0;
+	int report_err = 0;
+	int err_cnt = 0;
+	u32 status;
+
+	nchunks = mtd->writesize / chip->ecc.steps;
+	if (nchunks < 4)
+		nchunks = 1;
+	else
+		nchunks /= 4;
+
+	for (j = 0; j < nchunks; j++) {
+		status = readl(nand->regs + MA35_NFI_REG_NANDECCES0 + j * 4);
+		if (!status)
+			continue;
+
+		for (i = 0; i < 4; i++) {
+			if (!(status & ECC_STATUS_MASK)) {
+				/* No error */
+				status >>= 8;
+				continue;
+
+			} else if ((status & ECC_STATUS_MASK) == 0x01) {
+				/* Correctable error */
+				err_cnt = (status >> 2) & ECC_ERR_CNT_MASK;
+				ma35_nfi_correct(nand, j*4+i, err_cnt, addr);
+				report_err += err_cnt;
+
+			} else {
+				/* uncorrectable error */
+				dev_warn(nand->dev, "uncorrectable error! 0x%4x\n", status);
+				return -1;
+			}
+			status >>= 8;
+		}
+	}
+	return report_err;
+}
+
+
+static void ma35_nand_dmac_init(struct ma35_nand_info *nand)
+{
+	/* DMAC reset and enable */
+	writel(DMA_RST | DMA_EN, nand->regs + MA35_NFI_REG_DMACTL);
+	writel(DMA_EN, nand->regs + MA35_NFI_REG_DMACTL);
+
+	/* Clear DMA finished flag */
+	writel(INT_DMA | INT_ECC, nand->regs + MA35_NFI_REG_NANDINTSTS);
+
+	init_completion(&nand->complete);
+}
+
+
+static int ma35_nand_do_write(struct nand_chip *chip, const u8 *addr, u32 len)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	dma_addr_t dma_addr;
+	int ret = 0, i;
+	u32 reg;
+
+	if (len != mtd->writesize) {
+		for (i = 0; i < len; i++)
+			writel(addr[i], nand->regs + MA35_NFI_REG_NANDDATA);
+		return 0;
+	}
+
+	ma35_nand_dmac_init(nand);
+
+	writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
+	writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTEN);

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 446

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+	/* To mark this page as dirty. */
+	reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+	if (reg & 0xffff0000)
+		writel(reg & 0xffff, nand->regs + MA35_NFI_REG_NANDRA0);
+
+	dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_TO_DEVICE);
+	ret = dma_mapping_error(nand->dev, dma_addr);
+	if (ret) {
+		dev_err(nand->dev, "dma mapping error\n");
+		return -EINVAL;

Shouldn't this return -ENOMEM or simply ret when there's an error? Also, should we consider unmapping the page with dma_unmap_single in that case?

Quoted 31 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +31 -0
+	}
+	dma_sync_single_for_device(nand->dev, dma_addr, len, DMA_TO_DEVICE);
+
+	writel((unsigned long)dma_addr, nand->regs + MA35_NFI_REG_DMASA);
+	writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DMA_W_EN,
+		nand->regs + MA35_NFI_REG_NANDCTL);
+	ret = wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000));
+	if (!ret) {
+		dev_err(nand->dev, "write timeout\n");
+		ret = -ETIMEDOUT;
+	}
+
+	dma_unmap_single(nand->dev, dma_addr, len, DMA_TO_DEVICE);
+
+	return ret;
+}
+
+static int ma35_nand_do_read(struct nand_chip *chip, u8 *addr, u32 len)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int ret = 0, cnt = 0, i;
+	dma_addr_t dma_addr;
+	u32 reg;
+
+	if (len != mtd->writesize) {
+		for (i = 0; i < len; i++)
+			*(addr+i) = (u8)readl(nand->regs + MA35_NFI_REG_NANDDATA);
+		return 0;
+	}
+

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 487

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+	ma35_nand_dmac_init(nand);
+
+	writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
+	/* setup and start DMA using dma_addr */
+	dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_FROM_DEVICE);
+	ret = dma_mapping_error(nand->dev, dma_addr);
+	if (ret) {
+		dev_err(nand->dev, "dma mapping error\n");
+		return -EINVAL;

Same as above.

Quoted 63 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +63 -0
+	}
+
+	writel((unsigned long)dma_addr, nand->regs + MA35_NFI_REG_DMASA);
+	writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DMA_R_EN,
+		nand->regs + MA35_NFI_REG_NANDCTL);
+	ret = wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000));
+	if (!ret) {
+		dev_err(nand->dev, "read timeout\n");
+		ret = -ETIMEDOUT;
+	}
+
+	dma_unmap_single(nand->dev, dma_addr, len, DMA_FROM_DEVICE);
+
+	reg = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
+	if (reg & INT_ECC) {
+		cnt = ma35_nfi_ecc_check(&nand->chip, addr);
+		if (cnt < 0) {
+			mtd->ecc_stats.failed++;
+			writel(DMA_RST | DMA_EN, nand->regs + MA35_NFI_REG_DMACTL);
+			writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | SWRST,
+				nand->regs + MA35_NFI_REG_NANDCTL);
+		} else {
+			mtd->ecc_stats.corrected += cnt;
+			nand->bitflips = cnt;
+		}
+		writel(INT_ECC, nand->regs + MA35_NFI_REG_NANDINTSTS);
+	}
+
+	return ret;
+}
+
+
+static int ma35_nand_write_page_hwecc(struct nand_chip *chip, const u8 *buf,
+				      int oob_required, int page)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	void *ecc_calc = chip->ecc.calc_buf;
+
+	ma35_clear_spare(chip, mtd->oobsize);
+	ma35_write_spare(chip, mtd->oobsize - chip->ecc.total,
+			(u32 *)chip->oob_poi);
+
+	nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize);
+	nand_prog_page_end_op(chip);
+
+	/* Copy parity code in NANDRA to calc */
+	ma35_read_spare(chip, chip->ecc.total, (u32 *)ecc_calc,
+			mtd->oobsize - chip->ecc.total);
+
+	/* Copy parity code in calc to oob_poi */
+	memcpy(chip->oob_poi + (mtd->oobsize - chip->ecc.total),
+		ecc_calc, chip->ecc.total);
+
+	return 0;
+}
+
+static int ma35_nand_read_page_hwecc(struct nand_chip *chip, u8 *buf,
+					int oob_required, int page)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	u32 reg;
+

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 560

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+	/* read the OOB area  */
+	nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
+	nand->bitflips = 0;
+
+	/* copy OOB data to NANDRA for page read */
+	ma35_write_spare(chip, mtd->oobsize, (u32 *)chip->oob_poi);
+
+	reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+	if (reg & 0xffff0000)
+		memset((void *)buf, 0xff, mtd->writesize);

If only one branch of a conditional statement contains a single statement, you should use braces in both branches.

if (condition) {
         do_this();
         do_that();
} else {
         otherwise();
}
Quoted 71 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +71 -0
+	else {
+		/* read data from nand */
+		nand_read_page_op(chip, page, 0, buf, mtd->writesize);
+
+		/* restore OOB data from SMRA */
+		ma35_read_spare(chip, mtd->oobsize, (u32 *)chip->oob_poi, 0);
+	}
+
+	return nand->bitflips;
+}
+
+
+static int ma35_nand_read_oob_hwecc(struct nand_chip *chip, int page)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	u32 reg;
+
+	nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
+
+	/* copy OOB data to NANDRA for page read */
+	ma35_write_spare(chip, mtd->oobsize, (u32 *)chip->oob_poi);
+
+	reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+	if (reg & 0xffff0000)
+		memset((void *)chip->oob_poi, 0xff, mtd->oobsize);
+
+	return 0;
+}
+
+static irqreturn_t ma35_nand_irq(int irq, void *id)
+{
+	struct ma35_nand_info *nand = (struct ma35_nand_info *)id;
+	u32 isr;
+
+	isr = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
+	if (isr & INT_DMA) {
+		writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTSTS);
+		complete(&nand->complete);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int ma35_nand_attach_chip(struct nand_chip *chip)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	unsigned int reg;
+
+	if (chip->options & NAND_BUSWIDTH_16) {
+		dev_err(nand->dev, "16 bits bus width not supported");
+		return -EINVAL;
+	}
+
+	/* support only ecc hw mode */
+	if (chip->ecc.engine_type != NAND_ECC_ENGINE_TYPE_ON_HOST) {
+		dev_err(nand->dev, "ecc.engine_type not supported\n");
+		return -EINVAL;
+	}
+
+	nand->ecc_buf = devm_kzalloc(nand->dev, mtd->writesize + mtd->oobsize,
+					GFP_KERNEL);
+	if (!nand->ecc_buf)
+		return  -ENOMEM;
+	chip->ecc.calc_buf = nand->ecc_buf;
+
+	/* Set PSize */
+	reg = readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~PSIZE_MASK);
+	if (mtd->writesize == 2048)
+		writel(reg | PSIZE_2K, nand->regs + MA35_NFI_REG_NANDCTL);

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 641

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+	else if (mtd->writesize == 4096)
+		writel(reg | PSIZE_4K, nand->regs + MA35_NFI_REG_NANDCTL);
+	else if (mtd->writesize == 8192)
+		writel(reg | PSIZE_8K, nand->regs + MA35_NFI_REG_NANDCTL);
+
+	chip->ecc.steps = mtd->writesize / chip->ecc.size;
+	if (chip->ecc.strength == 0) {
+		nand->bch = BCH_NONE; /* No ECC */
+		chip->ecc.total = 0;
+

nit: New line is not really needed here.

Quoted 44 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +44 -0
+	} else if (chip->ecc.strength <= 8) {
+		nand->bch = BCH_T8; /* T8 */
+		chip->ecc.total = chip->ecc.steps * MA35_PARITY_BCH8;
+
+	} else if (chip->ecc.strength <= 12) {
+		nand->bch = BCH_T12; /* T12 */
+		chip->ecc.total = chip->ecc.steps * MA35_PARITY_BCH12;
+
+	} else if (chip->ecc.strength <= 24) {
+		nand->bch = BCH_T24; /* T24 */
+		chip->ecc.total = chip->ecc.steps * MA35_PARITY_BCH24;
+
+	} else {
+		dev_warn(nand->dev, "NAND Controller is not support this flash. (%d, %d)\n",
+			mtd->writesize, mtd->oobsize);
+	}
+
+	chip->ecc.bytes = chip->ecc.total / chip->ecc.steps;
+	mtd_set_ooblayout(mtd, &ma35_ooblayout_ops);
+
+	/* add mtd-id. The string should same as uboot definition */
+	mtd->name = "nand0";
+
+	ma35_nand_hwecc_init(nand);
+
+	writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL);
+
+	return 0;
+}
+
+
+static int ma35_nfc_exec_instr(struct nand_chip *chip,
+			      const struct nand_op_instr *instr)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	unsigned int i;
+	u32 status;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		writel(instr->ctx.cmd.opcode, nand->regs + MA35_NFI_REG_NANDCMD);
+		return 0;
+
+	case NAND_OP_ADDR_INSTR:

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 695

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+		for (i = 0; i < instr->ctx.addr.naddrs; i++) {
+			if (i == (instr->ctx.addr.naddrs - 1))
+				writel(instr->ctx.addr.addrs[i] | ENDADDR,
+					nand->regs + MA35_NFI_REG_NANDADDR);
+			else
+				writel(instr->ctx.addr.addrs[i],
+					nand->regs + MA35_NFI_REG_NANDADDR);
+		}
+		return 0;
+

nit: New line is not really needed after return statement.

Quoted 195 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +191 -0
+	case NAND_OP_DATA_IN_INSTR:
+		ma35_nand_do_read(chip, instr->ctx.data.buf.in, instr->ctx.data.len);
+		return 0;
+
+	case NAND_OP_DATA_OUT_INSTR:
+		ma35_nand_do_write(chip, instr->ctx.data.buf.out, instr->ctx.data.len);
+		return 0;
+
+	case NAND_OP_WAITRDY_INSTR:
+		return readl_poll_timeout(nand->regs + MA35_NFI_REG_NANDINTSTS, status,
+					  status & INT_RB0, 20,
+					  instr->ctx.waitrdy.timeout_ms * 1000);
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+
+static int ma35_nfc_exec_op(struct nand_chip *chip,
+			  const struct nand_operation *op,
+			  bool check_only)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	u32 i, reg;
+	int ret = 0;
+
+	if (check_only)
+		return 0;
+
+	ma35_nand_target_enable(nand);
+
+	reg = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
+	reg |= INT_RB0;
+	writel(reg, nand->regs + MA35_NFI_REG_NANDINTSTS);
+
+	for (i = 0; i < op->ninstrs; i++) {
+		ret = ma35_nfc_exec_instr(chip, &op->instrs[i]);
+		if (ret)
+			break;
+	}
+
+	ma35_nand_target_disable(nand);
+
+	return ret;
+}
+
+
+static const struct nand_controller_ops ma35_nfc_ops = {
+	.attach_chip = ma35_nand_attach_chip,
+	.exec_op = ma35_nfc_exec_op,
+};
+
+static int ma35_nand_probe(struct platform_device *pdev)
+{
+	struct ma35_nand_info *nand;
+	struct nand_chip *chip;
+	struct mtd_info *mtd;
+	int ret = 0;
+
+	nand = devm_kzalloc(&pdev->dev, sizeof(*nand), GFP_KERNEL);
+	if (!nand)
+		return -ENOMEM;
+
+	nand_controller_init(&nand->controller);
+	nand->controller.ops = &ma35_nfc_ops;
+
+	nand->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(nand->regs))
+		return PTR_ERR(nand->regs);
+
+	nand->dev = &pdev->dev;
+	chip = &nand->chip;
+	nand_set_controller_data(chip, nand);
+	nand_set_flash_node(chip, pdev->dev.of_node);
+
+	nand->clk = devm_clk_get_enabled(&pdev->dev, "nand_gate");
+	if (IS_ERR(nand->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(nand->clk),
+				     "failed to find nand clock\n");
+
+	nand->irq = platform_get_irq(pdev, 0);
+	if (nand->irq < 0)
+		return dev_err_probe(&pdev->dev, nand->irq,
+				     "failed to get platform irq\n");
+
+	ret = devm_request_irq(&pdev->dev, nand->irq, ma35_nand_irq,
+				  IRQF_TRIGGER_HIGH, "ma35d1-nand", nand);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request NAND irq\n");
+		return -ENXIO;
+	}
+
+	nand->chip.controller = &nand->controller;
+	platform_set_drvdata(pdev, nand);
+
+	chip->options |= NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA;
+
+	chip->ecc.engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
+	chip->ecc.write_page = ma35_nand_write_page_hwecc;
+	chip->ecc.read_page  = ma35_nand_read_page_hwecc;
+	chip->ecc.read_oob   = ma35_nand_read_oob_hwecc;
+
+	mtd = nand_to_mtd(chip);
+	mtd->priv = chip;
+	mtd->owner = THIS_MODULE;
+	mtd->dev.parent = &pdev->dev;
+
+	writel(NAND_EN, nand->regs + MA35_NFI_REG_GCTL);
+
+	ret = nand_scan(chip, 1);
+	if (ret)
+		return ret;
+
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret) {
+		nand_cleanup(chip);
+		return ret;
+	}
+
+	return ret;
+}
+
+static void ma35_nand_remove(struct platform_device *pdev)
+{
+	struct ma35_nand_info *nand = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = mtd_device_unregister(nand_to_mtd(&nand->chip));
+	WARN_ON(ret);
+	nand_cleanup(&nand->chip);
+}
+
+/* PM Support */
+#ifdef CONFIG_PM
+static int ma35_nand_suspend(struct platform_device *pdev, pm_message_t pm)
+{
+	struct ma35_nand_info *nand = platform_get_drvdata(pdev);
+	int ret = 0;
+	u32 val;
+
+	/* wait DMAC to ready */
+	ret = readl_poll_timeout(nand->regs + MA35_NFI_REG_DMACTL, val,
+				 !(val & DMA_BUSY), 50, HZ/2);
+	if (ret)
+		dev_warn(&pdev->dev, "dma busy\n");
+
+	clk_disable(nand->clk);
+
+	return ret;
+}
+
+static int ma35_nand_resume(struct platform_device *pdev)
+{
+	struct ma35_nand_info *nand = platform_get_drvdata(pdev);
+
+	clk_enable(nand->clk);
+	ma35_nand_hwecc_init(nand);
+	ma35_nand_dmac_init(nand);
+
+	return 0;
+}
+
+#else
+#define ma35_nand_suspend NULL
+#define ma35_nand_resume NULL
+#endif
+
+static const struct of_device_id ma35_nfi_of_match[] = {
+	{ .compatible = "nuvoton,ma35d1-nand" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ma35_nfi_of_match);
+
+static struct platform_driver ma35_nand_driver = {
+	.driver = {
+		.name = "ma35d1-nand",
+		.of_match_table = ma35_nfi_of_match,
+	},
+	.probe = ma35_nand_probe,
+	.remove = ma35_nand_remove,
+	.suspend = ma35_nand_suspend,
+	.resume = ma35_nand_resume,
+};
+
+module_platform_driver(ma35_nand_driver);
+
+MODULE_DESCRIPTION("Nuvoton ma35 NAND driver");
+MODULE_AUTHOR("Hui-Ping Chen <[email protected]>");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.25.1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

#4 lore raw

Re: [PATCH v5 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

Hi Amit,

Quoted 25 lines · Amit Singh Tomar

Hi,

Quoted 23 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +23 -0
+static int ma35_nand_do_write(struct nand_chip *chip, const u8 *addr, u32 len)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	dma_addr_t dma_addr;
+	int ret = 0, i;
+	u32 reg;
+
+	if (len != mtd->writesize) {
+		for (i = 0; i < len; i++)
+			writel(addr[i], nand->regs + MA35_NFI_REG_NANDDATA);
+		return 0;
+	}
+
+	ma35_nand_dmac_init(nand);
+
+	writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
+	writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTEN);
+	/* To mark this page as dirty. */
+	reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+	if (reg & 0xffff0000)
+		writel(reg & 0xffff, nand->regs + MA35_NFI_REG_NANDRA0);
Quoted 8 lines

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 446

Quoted 6 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +6 -0
+
+	dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_TO_DEVICE);
+	ret = dma_mapping_error(nand->dev, dma_addr);
+	if (ret) {
+		dev_err(nand->dev, "dma mapping error\n");
+		return -EINVAL;

Shouldn't this return -ENOMEM or simply ret when there's an error? Also, should we consider unmapping the page with dma_unmap_single in that case?

When dma_map_single() returns an error the pages are not mapped, no need to unmap then.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#5 lore raw

PATCH v5 2/2 mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

Quoted 38 lines

Hi Amit,

Quoted 25 lines · Amit Singh Tomar

Hi,

Quoted 23 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +23 -0
+static int ma35_nand_do_write(struct nand_chip *chip, const u8 *addr, u32 len)
+{
+	struct ma35_nand_info *nand = nand_get_controller_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	dma_addr_t dma_addr;
+	int ret = 0, i;
+	u32 reg;
+
+	if (len != mtd->writesize) {
+		for (i = 0; i < len; i++)
+			writel(addr[i], nand->regs + MA35_NFI_REG_NANDDATA);
+		return 0;
+	}
+
+	ma35_nand_dmac_init(nand);
+
+	writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
+	writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTEN);
+	/* To mark this page as dirty. */
+	reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+	if (reg & 0xffff0000)
+		writel(reg & 0xffff, nand->regs + MA35_NFI_REG_NANDRA0);
Quoted 8 lines

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 446

Quoted 6 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +6 -0
+
+	dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_TO_DEVICE);
+	ret = dma_mapping_error(nand->dev, dma_addr);
+	if (ret) {
+		dev_err(nand->dev, "dma mapping error\n");
+		return -EINVAL;

Shouldn't this return -ENOMEM or simply ret when there's an error? Also, should we consider unmapping the page with dma_unmap_single in that case?

Quoted 3 lines

When dma_map_single() returns an error the pages are not mapped, no need to unmap then.

Thanks for the clarification.
-Amit

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

#6 lore raw

Re: [PATCH v5 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

In reply to Amit Singh Tomar

Hi Amit,

Thank you for your reply.

Quoted 67 lines · Amit Singh Tomar

Hi,

Quoted 65 lines

Nuvoton MA35 SoCs NAND Flash Interface Controller supports 2kiB, 4kiB and 8kiB page size, and up to 8-bit, 12-bit, and 24-bit hardware ECC calculation circuit to protect data.

---
   drivers/mtd/nand/raw/Kconfig               |   8 + drivers/mtd/nand/raw/Makefile              |   1 + drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c | 886
+++++++++++++++++++++

   3 files changed, 895 insertions(+)

   create mode 100644 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c

drivers/mtd/nand/raw/Kconfig +0 -0
index 614257308516..a95d91e61c42 100644
@@ -448,6 +448,14 @@ config MTD_NAND_RENESAS

         Enables support for the NAND controller found on Renesas R-Car Gen3 and RZ/N1 SoC families.
   +config MTD_NAND_NUVOTON_MA35
+    tristate "Nuvoton MA35 SoC NAND controller"
+    depends on ARCH_MA35 || COMPILE_TEST
+    depends on OF
+    help
+      Enables support for the NAND controller found on
+      the Nuvoton MA35 series SoCs.
+
   comment "Misc"
      config MTD_SM_COMMON

drivers/mtd/nand/raw/Makefile +0 -0
index 25120a4afada..b8e1b3af6942 100644
@@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_INTEL_LGM)    += 

intel-nand-controller.o
   obj-$(CONFIG_MTD_NAND_ROCKCHIP)        += rockchip-nand-controller.o obj-$(CONFIG_MTD_NAND_PL35X)        += pl35x-nand-controller.o obj-$(CONFIG_MTD_NAND_RENESAS)        += renesas-nand-controller.o +obj-$(CONFIG_MTD_NAND_NUVOTON_MA35)    += nuvoton_ma35d1_nand.o nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o nand-objs += nand_onfi.o

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +15 -0
new file mode 100644
index 000000000000..5b53b7f0b9cb
@@ -0,0 +1,886 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Nuvoton Technology Corp.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
Quoted 8 lines

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 22

Quoted 7 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +7 -0
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+

nit: Unnecessary new line.

I will check and remove it.

Quoted 47 lines
Quoted 47 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +47 -0
+/* NFI Registers */
+#define MA35_NFI_REG_DMACTL        0x400
+#define   DMA_EN                BIT(0)
+#define   DMA_RST                BIT(1)
+#define   DMA_BUSY                BIT(9)
+
+#define MA35_NFI_REG_DMASA        0x408
+#define MA35_NFI_REG_GCTL        0x800
+#define   NAND_EN                BIT(3)
+
+#define MA35_NFI_REG_NANDCTL        0x8A0
+#define   SWRST                BIT(0)
+#define   DMA_R_EN                BIT(1)
+#define   DMA_W_EN                BIT(2)
+#define   ECC_CHK                BIT(7)
+#define   PROT3BEN                BIT(8)
+#define   PSIZE_2K                BIT(16)
+#define   PSIZE_4K                BIT(17)
+#define   PSIZE_8K                GENMASK(17, 16)
+#define   PSIZE_MASK                GENMASK(17, 16)
+#define   BCH_T24                BIT(18)
+#define   BCH_T8                BIT(20)
+#define   BCH_T12                BIT(21)
+#define   BCH_NONE                (0x0)
+#define   BCH_MASK                GENMASK(22, 18)
+#define   ECC_EN                BIT(23)
+#define   DISABLE_CS0                BIT(25)
+
+#define MA35_NFI_REG_NANDINTEN    0x8A8
+#define MA35_NFI_REG_NANDINTSTS    0x8AC
+#define   INT_DMA                BIT(0)
+#define   INT_ECC                BIT(2)
+#define   INT_RB0                BIT(10)
+#define   INT_RB0_STS                BIT(18)
+
+#define MA35_NFI_REG_NANDCMD        0x8B0
+#define MA35_NFI_REG_NANDADDR        0x8B4
+#define   ENDADDR                BIT(31)
+
+#define MA35_NFI_REG_NANDDATA        0x8B8
+#define MA35_NFI_REG_NANDRACTL    0x8BC
+#define MA35_NFI_REG_NANDECTL        0x8C0
+#define   ENABLE_WP                0x0
+#define   DISABLE_WP                BIT(0)
+
+#define MA35_NFI_REG_NANDECCES0    0x8D0
+#define   ECC_STATUS_MASK            GENMASK(1, 0)
Quoted 8 lines

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 76

Quoted 7 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +7 -0
+#define   ECC_ERR_CNT_MASK            GENMASK(4, 0)
+
+#define MA35_NFI_REG_NANDECCEA0    0x900
+#define MA35_NFI_REG_NANDECCED0    0x960
+#define MA35_NFI_REG_NANDRA0        0xA00
+
+

nit: Unnecessary new line.

I will check and remove it.

Quoted 4 lines
Quoted 4 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +4 -0
+/* Define for the BCH hardware ECC engine */
+/* define the total padding bytes for 512/1024 data segment */
+#define MA35_BCH_PADDING_512    32
+#define MA35_BCH_PADDING_1024    64
Quoted 8 lines

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 87

Quoted 7 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +7 -0
+/* define the BCH parity code length for 512 bytes data pattern */
+#define MA35_PARITY_BCH8    15
+#define MA35_PARITY_BCH12    23
+/* define the BCH parity code length for 1024 bytes data pattern */
+#define MA35_PARITY_BCH24    45
+
+

nit: Unnecessary new line.

I will check and remove it.

Quoted 141 lines
Quoted 141 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +57 -0
+struct ma35_nand_info {
+    struct nand_controller controller;
+    struct nand_chip chip;
+    struct device *dev;
+    void __iomem *regs;
+    int irq;
+    struct clk *clk;
+    struct completion complete;
+
+    u32 bch;
+    u32 bitflips;
+    u8 *ecc_buf;
+};
+
+static int ma35_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->length = chip->ecc.total;
+    oobregion->offset = mtd->oobsize - oobregion->length;
+
+    return 0;
+}
+
+static int ma35_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->length = mtd->oobsize - chip->ecc.total - 2;
+    oobregion->offset = 2;
+
+    return 0;
+}
+
+static const struct mtd_ooblayout_ops ma35_ooblayout_ops = {
+    .free = ma35_ooblayout_free,
+    .ecc = ma35_ooblayout_ecc,
+};
+
+static inline void ma35_clear_spare(struct nand_chip *chip, int size)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    int i;
+
+    for (i = 0; i < size/4; i++)
+        writel(0xff, nand->regs + MA35_NFI_REG_NANDRA0);
+}
+
+static inline void read_remaining_bytes(struct ma35_nand_info *nand, 

u32 *buf,
+                        u32 offset, int size)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +11 -0
+{
+    u32 value = readl(nand->regs + MA35_NFI_REG_NANDRA0 + offset);
+    u8 *ptr = (u8 *)buf;
+    int i;
+
+    for (i = 0; i < size; i++)
+        ptr[i] = (value >> (i * 8)) & 0xff;
+}
+
+
+static inline void ma35_read_spare(struct nand_chip *chip, int size, 

u32 *buf, u32 offset)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +7 -0
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    int i, j;
+
+    if ((offset % 4) == 0) {
+        for (i = 0, j = 0; i < size / 4; i++, j += 4)
+            *buf++ = readl(nand->regs + MA35_NFI_REG_NANDRA0 + 

offset + j);

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +9 -0
+
+        read_remaining_bytes(nand, buf, offset + j, size % 4);
+    } else {
+        read_remaining_bytes(nand, buf, offset, 4 - (offset % 4));
+        offset += 4;
+        size -= (4 - (offset % 4));
+
+        for (i = 0, j = 0; i < size / 4; i++, j += 4)
+            *buf++ = readl(nand->regs + MA35_NFI_REG_NANDRA0 + 

offset + j);

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +6 -0
+
+        read_remaining_bytes(nand, buf, offset + j, size % 4);
+    }
+}
+
+static inline void ma35_write_spare(struct nand_chip *chip, int 

size, u32 *buf)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +34 -0
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    u32 value;
+    int i, j;
+    u8 *ptr;
+
+    for (i = 0, j = 0; i < size / 4; i++, j += 4)
+        writel(*buf++, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+
+    ptr = (u8 *)buf;
+    switch (size % 4) {
+    case 1:
+        writel(*ptr, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+        break;
+    case 2:
+        value = *ptr | (*(ptr+1) << 8);
+        writel(value, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+        break;
+    case 3:
+        value = *ptr | (*(ptr+1) << 8) | (*(ptr+2) << 16);
+        writel(value, nand->regs + MA35_NFI_REG_NANDRA0 + j);
+        break;
+    default:
+        break;
+    }
+}
+
+static inline void ma35_nand_target_enable(struct ma35_nand_info *nand)
+{
+    writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),
+        nand->regs+MA35_NFI_REG_NANDCTL);
+}
+
+static inline void ma35_nand_target_disable(struct ma35_nand_info 

*nand)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+{
+    writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,
+        nand->regs + MA35_NFI_REG_NANDCTL);
+}
+
+
+static void ma35_nand_hwecc_init(struct ma35_nand_info *nand)
+{
+    struct mtd_info *mtd = nand_to_mtd(&nand->chip);
+    u32 reg;
Quoted 8 lines
Quoted 7 lines

+
+    /* resets the internal state machine and counters */ +    reg = readl(nand->regs + MA35_NFI_REG_NANDCTL); +    reg |= SWRST;
+    writel(reg, nand->regs + MA35_NFI_REG_NANDCTL); +    while (readl(nand->regs + MA35_NFI_REG_NANDCTL) & SWRST) +        ;

Shouldn't there be a timeout?

This bit is the reset state machine.

The hardware will automatically clear this bit after a few clock cycles,

so there is no timeout added.

Quoted 21 lines
Quoted 21 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +11 -0
+
+    /* Redundant area size */
+    writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
+    /* Protect redundant 3 bytes */
+    reg = readl(nand->regs + MA35_NFI_REG_NANDCTL);
+    reg |= (PROT3BEN | ECC_CHK);
+    writel(reg, nand->regs + MA35_NFI_REG_NANDCTL);
+
+    if (nand->bch == BCH_NONE) {
+        /* Disable H/W ECC, ECC parity check enable bit during read 

page */
+        writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~ECC_EN), +            nand->regs + MA35_NFI_REG_NANDCTL);
+    } else {
+        /* Set BCH algorithm */
+        writel((readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~BCH_MASK)) |
+            nand->bch, nand->regs + MA35_NFI_REG_NANDCTL); +
+        /* Enable H/W ECC, ECC parity check enable bit during read

Quoted 8 lines

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 120

Quoted 7 lines

page */
+        writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | ECC_EN, +            nand->regs + MA35_NFI_REG_NANDCTL);
+    }

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +3 -0
+}
+
+

nit: Unnecessary new line.

I will check and remove it.

Quoted 202 lines
Quoted 202 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +26 -0
+/* Correct data by BCH alrogithm */
+static void ma35_nfi_correct(struct ma35_nand_info *nand, u8 index,
+                 u8 err_cnt, u8 *addr)
+{
+    u32 temp_data[24], temp_addr[24];
+    u32 padding_len, parity_len;
+    u32 value, offset, remain;
+    u32 err_data[6];
+    u8  i, j;
+
+    /* configurations */
+    switch (nand->bch) {
+    case BCH_T24:
+        parity_len = MA35_PARITY_BCH24;
+        padding_len = MA35_BCH_PADDING_1024;
+        break;
+    case BCH_T12:
+        parity_len = MA35_PARITY_BCH12;
+        padding_len = MA35_BCH_PADDING_512;
+        break;
+    case BCH_T8:
+        parity_len = MA35_PARITY_BCH8;
+        padding_len = MA35_BCH_PADDING_512;
+        break;
+    default:
+        dev_warn(nand->dev, "NAND ERROR: invalid SMCR_BCH_TSEL = 

0x%08X\n",
+            (u32)(readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK));
+        return;
+    }
+
+    /* got valid BCH_ECC_DATAx and parse them to temp_data[] +     * got the valid register number of BCH_ECC_DATAx since +     * one register include 4 error bytes
+     */
+    j = (err_cnt + 3) / 4;
+    j = (j > 6) ? 6 : j;
+    for (i = 0; i < j; i++)
+        err_data[i] = readl(nand->regs + MA35_NFI_REG_NANDECCED0 + i
* 4);

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +16 -0
+
+    for (i = 0; i < j; i++) {
+        temp_data[i*4+0] = err_data[i] & 0xff;
+        temp_data[i*4+1] = (err_data[i] >> 8) & 0xff;
+        temp_data[i*4+2] = (err_data[i] >> 16) & 0xff;
+        temp_data[i*4+3] = (err_data[i] >> 24) & 0xff;
+    }
+
+    /* got valid REG_BCH_ECC_ADDRx and parse them to temp_addr[]
+     * got the valid register number of REG_BCH_ECC_ADDRx since
+     * one register include 2 error addresses
+     */
+    j = (err_cnt + 1) / 2;
+    j = (j > 12) ? 12 : j;
+    for (i = 0; i < j; i++) {
+        temp_addr[i*2+0] = readl(nand->regs + 

MA35_NFI_REG_NANDECCEA0 + i * 4)
+                    & 0x07ff;
+        temp_addr[i*2+1] = (readl(nand->regs +
MA35_NFI_REG_NANDECCEA0 + i * 4)
+                    >> 16) & 0x07ff;
+    }

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +23 -0
+
+    /* pointer to begin address of field that with data error */
+    addr += index * nand->chip.ecc.steps;
+
+    /* correct each error bytes */
+    for (i = 0; i < err_cnt; i++) {
+        u32 corrected_index = temp_addr[i];
+
+        /* for wrong data in field */
+        if (corrected_index < nand->chip.ecc.steps)
+            *(addr + corrected_index) ^= temp_data[i];
+
+        /* for wrong first-3-bytes in redundancy area */
+        else if (corrected_index < (nand->chip.ecc.steps + 3)) {
+            corrected_index -= nand->chip.ecc.steps;
+            temp_addr[i] += (parity_len * index);    /* field offset */
+
+            value = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+            value ^= temp_data[i] << (8 * corrected_index);
+            writel(value, nand->regs + MA35_NFI_REG_NANDRA0);
+        }
+        /* for wrong parity code in redundancy area
+         * BCH_ERR_ADDRx = [data in field] + [3 bytes] + [xx] + 

[parity code]
+         *                                   |<-- padding bytes -->|
+         * The BCH_ERR_ADDRx for last parity code always = field size + padding size.
+         * So, the first parity code = field size + padding size - parity code length.
+         * For example, for BCH T12, the first parity code = 512 + 32 - 23 = 521.
+         * That is, error byte address offset within field is +         */
+        else {
+            corrected_index -= (nand->chip.ecc.steps + padding_len - parity_len);
+
+            /* final address = first parity code of first field + +             *                 offset of fields +
+             *                 offset within field
+             */
+            offset = (readl(nand->regs + MA35_NFI_REG_NANDRACTL) & 0x1ff) -
+                (parity_len * nand->chip.ecc.steps) +
+                (parity_len * index) + corrected_index;

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +44 -2
+
+            remain = offset % 4;
+            value = readl(nand->regs + MA35_NFI_REG_NANDRA0 + offset 
- remain);
+            value ^= temp_data[i] << (8 * remain);
+            writel(value, nand->regs + MA35_NFI_REG_NANDRA0 + offset 
- remain);
+        }
+    }
+}
+
+static int ma35_nfi_ecc_check(struct nand_chip *chip, u8 *addr)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    struct mtd_info *mtd = nand_to_mtd(chip);
+    int i, j, nchunks = 0;
+    int report_err = 0;
+    int err_cnt = 0;
+    u32 status;
+
+    nchunks = mtd->writesize / chip->ecc.steps;
+    if (nchunks < 4)
+        nchunks = 1;
+    else
+        nchunks /= 4;
+
+    for (j = 0; j < nchunks; j++) {
+        status = readl(nand->regs + MA35_NFI_REG_NANDECCES0 + j * 4);
+        if (!status)
+            continue;
+
+        for (i = 0; i < 4; i++) {
+            if (!(status & ECC_STATUS_MASK)) {
+                /* No error */
+                status >>= 8;
+                continue;
+
+            } else if ((status & ECC_STATUS_MASK) == 0x01) {
+                /* Correctable error */
+                err_cnt = (status >> 2) & ECC_ERR_CNT_MASK;
+                ma35_nfi_correct(nand, j*4+i, err_cnt, addr);
+                report_err += err_cnt;
+
+            } else {
+                /* uncorrectable error */
+                dev_warn(nand->dev, "uncorrectable error! 0x%4x\n", 

status);
+                return -1;
+            }
+            status >>= 8;
+        }
+    }
+    return report_err;

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +17 -0
+}
+
+
+static void ma35_nand_dmac_init(struct ma35_nand_info *nand)
+{
+    /* DMAC reset and enable */
+    writel(DMA_RST | DMA_EN, nand->regs + MA35_NFI_REG_DMACTL);
+    writel(DMA_EN, nand->regs + MA35_NFI_REG_DMACTL);
+
+    /* Clear DMA finished flag */
+    writel(INT_DMA | INT_ECC, nand->regs + MA35_NFI_REG_NANDINTSTS);
+
+    init_completion(&nand->complete);
+}
+
+
+static int ma35_nand_do_write(struct nand_chip *chip, const u8 

*addr, u32 len)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +22 -0
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    struct mtd_info *mtd = nand_to_mtd(chip);
+    dma_addr_t dma_addr;
+    int ret = 0, i;
+    u32 reg;
+
+    if (len != mtd->writesize) {
+        for (i = 0; i < len; i++)
+            writel(addr[i], nand->regs + MA35_NFI_REG_NANDDATA);
+        return 0;
+    }
+
+    ma35_nand_dmac_init(nand);
+
+    writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
+    writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTEN);
+    /* To mark this page as dirty. */
+    reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+    if (reg & 0xffff0000)
+        writel(reg & 0xffff, nand->regs + MA35_NFI_REG_NANDRA0);
Quoted 8 lines
Quoted 7 lines

+
+    dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_TO_DEVICE);
+    ret = dma_mapping_error(nand->dev, dma_addr); +    if (ret) {
+        dev_err(nand->dev, "dma mapping error\n"); +        return -EINVAL;

Shouldn't this return -ENOMEM or simply ret when there's an error? Also, should we consider unmapping the page with dma_unmap_single in that case?

Sascha has explained this issue.

Thanks Sascha.

Quoted 37 lines
Quoted 37 lines

+    }
+    dma_sync_single_for_device(nand->dev, dma_addr, len, DMA_TO_DEVICE);
+
+    writel((unsigned long)dma_addr, nand->regs + MA35_NFI_REG_DMASA); +    writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DMA_W_EN, +        nand->regs + MA35_NFI_REG_NANDCTL);
+    ret = wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000));
+    if (!ret) {
+        dev_err(nand->dev, "write timeout\n");
+        ret = -ETIMEDOUT;
+    }

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +24 -0
+
+    dma_unmap_single(nand->dev, dma_addr, len, DMA_TO_DEVICE);
+
+    return ret;
+}
+
+static int ma35_nand_do_read(struct nand_chip *chip, u8 *addr, u32 len)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    struct mtd_info *mtd = nand_to_mtd(chip);
+    int ret = 0, cnt = 0, i;
+    dma_addr_t dma_addr;
+    u32 reg;
+
+    if (len != mtd->writesize) {
+        for (i = 0; i < len; i++)
+            *(addr+i) = (u8)readl(nand->regs + MA35_NFI_REG_NANDDATA);
+        return 0;
+    }
+
+    ma35_nand_dmac_init(nand);
+
+    writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
+
Quoted 8 lines
Quoted 7 lines

+    /* setup and start DMA using dma_addr */
+    dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_FROM_DEVICE);
+    ret = dma_mapping_error(nand->dev, dma_addr); +    if (ret) {
+        dev_err(nand->dev, "dma mapping error\n"); +        return -EINVAL;

Same as above.

As above.

Quoted 77 lines
Quoted 65 lines

+    }
+
+    writel((unsigned long)dma_addr, nand->regs + MA35_NFI_REG_DMASA); +    writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DMA_R_EN, +        nand->regs + MA35_NFI_REG_NANDCTL);
+    ret = wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000));
+    if (!ret) {
+        dev_err(nand->dev, "read timeout\n");
+        ret = -ETIMEDOUT;
+    }

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +23 -0
+
+    dma_unmap_single(nand->dev, dma_addr, len, DMA_FROM_DEVICE);
+
+    reg = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
+    if (reg & INT_ECC) {
+        cnt = ma35_nfi_ecc_check(&nand->chip, addr);
+        if (cnt < 0) {
+            mtd->ecc_stats.failed++;
+            writel(DMA_RST | DMA_EN, nand->regs + MA35_NFI_REG_DMACTL);
+            writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | SWRST,
+                nand->regs + MA35_NFI_REG_NANDCTL);
+        } else {
+            mtd->ecc_stats.corrected += cnt;
+            nand->bitflips = cnt;
+        }
+        writel(INT_ECC, nand->regs + MA35_NFI_REG_NANDINTSTS);
+    }
+
+    return ret;
+}
+
+
+static int ma35_nand_write_page_hwecc(struct nand_chip *chip, const 

u8 *buf,
+                      int oob_required, int page)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +29 -0
+{
+    struct mtd_info *mtd = nand_to_mtd(chip);
+    void *ecc_calc = chip->ecc.calc_buf;
+
+    ma35_clear_spare(chip, mtd->oobsize);
+    ma35_write_spare(chip, mtd->oobsize - chip->ecc.total,
+            (u32 *)chip->oob_poi);
+
+    nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize);
+    nand_prog_page_end_op(chip);
+
+    /* Copy parity code in NANDRA to calc */
+    ma35_read_spare(chip, chip->ecc.total, (u32 *)ecc_calc,
+            mtd->oobsize - chip->ecc.total);
+
+    /* Copy parity code in calc to oob_poi */
+    memcpy(chip->oob_poi + (mtd->oobsize - chip->ecc.total),
+        ecc_calc, chip->ecc.total);
+
+    return 0;
+}
+
+static int ma35_nand_read_page_hwecc(struct nand_chip *chip, u8 *buf,
+                    int oob_required, int page)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    struct mtd_info *mtd = nand_to_mtd(chip);
+    u32 reg;
+

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 560

Quoted 10 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +10 -0
+    /* read the OOB area  */
+    nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
+    nand->bitflips = 0;
+
+    /* copy OOB data to NANDRA for page read */
+    ma35_write_spare(chip, mtd->oobsize, (u32 *)chip->oob_poi);
+
+    reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+    if (reg & 0xffff0000)
+        memset((void *)buf, 0xff, mtd->writesize);

If only one branch of a conditional statement contains a single statement, you should use braces in both branches.

Quoted 7 lines

if (condition) {
         do_this();
         do_that();
} else {
         otherwise();
}

I will fix it.

Quoted 75 lines
Quoted 75 lines

+    else {
+        /* read data from nand */
+        nand_read_page_op(chip, page, 0, buf, mtd->writesize);

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +59 -0
+
+        /* restore OOB data from SMRA */
+        ma35_read_spare(chip, mtd->oobsize, (u32 *)chip->oob_poi, 0);
+    }
+
+    return nand->bitflips;
+}
+
+
+static int ma35_nand_read_oob_hwecc(struct nand_chip *chip, int page)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    struct mtd_info *mtd = nand_to_mtd(chip);
+    u32 reg;
+
+    nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
+
+    /* copy OOB data to NANDRA for page read */
+    ma35_write_spare(chip, mtd->oobsize, (u32 *)chip->oob_poi);
+
+    reg = readl(nand->regs + MA35_NFI_REG_NANDRA0);
+    if (reg & 0xffff0000)
+        memset((void *)chip->oob_poi, 0xff, mtd->oobsize);
+
+    return 0;
+}
+
+static irqreturn_t ma35_nand_irq(int irq, void *id)
+{
+    struct ma35_nand_info *nand = (struct ma35_nand_info *)id;
+    u32 isr;
+
+    isr = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
+    if (isr & INT_DMA) {
+        writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTSTS);
+        complete(&nand->complete);
+    }
+
+    return IRQ_HANDLED;
+}
+
+static int ma35_nand_attach_chip(struct nand_chip *chip)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    struct mtd_info *mtd = nand_to_mtd(chip);
+    unsigned int reg;
+
+    if (chip->options & NAND_BUSWIDTH_16) {
+        dev_err(nand->dev, "16 bits bus width not supported");
+        return -EINVAL;
+    }
+
+    /* support only ecc hw mode */
+    if (chip->ecc.engine_type != NAND_ECC_ENGINE_TYPE_ON_HOST) {
+        dev_err(nand->dev, "ecc.engine_type not supported\n");
+        return -EINVAL;
+    }
+
+    nand->ecc_buf = devm_kzalloc(nand->dev, mtd->writesize + 

mtd->oobsize,
+                    GFP_KERNEL);
+    if (!nand->ecc_buf)
+        return  -ENOMEM;
+    chip->ecc.calc_buf = nand->ecc_buf;
+
+    /* Set PSize */
+    reg = readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~PSIZE_MASK); +    if (mtd->writesize == 2048)
+        writel(reg | PSIZE_2K, nand->regs + MA35_NFI_REG_NANDCTL); +    else if (mtd->writesize == 4096)
+        writel(reg | PSIZE_4K, nand->regs + MA35_NFI_REG_NANDCTL); +    else if (mtd->writesize == 8192)

Quoted 8 lines

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c line 641

Quoted 7 lines

+        writel(reg | PSIZE_8K, nand->regs + MA35_NFI_REG_NANDCTL);

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +6 -0
+
+    chip->ecc.steps = mtd->writesize / chip->ecc.size;
+    if (chip->ecc.strength == 0) {
+        nand->bch = BCH_NONE; /* No ECC */
+        chip->ecc.total = 0;
+

nit: New line is not really needed here.

I will check and remove it.

Quoted 49 lines
Quoted 49 lines

+    } else if (chip->ecc.strength <= 8) {
+        nand->bch = BCH_T8; /* T8 */
+        chip->ecc.total = chip->ecc.steps * MA35_PARITY_BCH8;

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +11 -0
+
+    } else if (chip->ecc.strength <= 12) {
+        nand->bch = BCH_T12; /* T12 */
+        chip->ecc.total = chip->ecc.steps * MA35_PARITY_BCH12;
+
+    } else if (chip->ecc.strength <= 24) {
+        nand->bch = BCH_T24; /* T24 */
+        chip->ecc.total = chip->ecc.steps * MA35_PARITY_BCH24;
+
+    } else {
+        dev_warn(nand->dev, "NAND Controller is not support this 

flash. (%d, %d)\n",
+            mtd->writesize, mtd->oobsize);
+    }

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +25 -0
+
+    chip->ecc.bytes = chip->ecc.total / chip->ecc.steps;
+    mtd_set_ooblayout(mtd, &ma35_ooblayout_ops);
+
+    /* add mtd-id. The string should same as uboot definition */
+    mtd->name = "nand0";
+
+    ma35_nand_hwecc_init(nand);
+
+    writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL);
+
+    return 0;
+}
+
+
+static int ma35_nfc_exec_instr(struct nand_chip *chip,
+                  const struct nand_op_instr *instr)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    unsigned int i;
+    u32 status;
+
+    switch (instr->type) {
+    case NAND_OP_CMD_INSTR:
+        writel(instr->ctx.cmd.opcode, nand->regs + 

MA35_NFI_REG_NANDCMD);
+        return 0;
+
+    case NAND_OP_ADDR_INSTR:
+        for (i = 0; i < instr->ctx.addr.naddrs; i++) { +            if (i == (instr->ctx.addr.naddrs - 1)) +                writel(instr->ctx.addr.addrs[i] | ENDADDR,

Quoted 8 lines
Quoted 7 lines

+                    nand->regs + MA35_NFI_REG_NANDADDR); +            else
+                writel(instr->ctx.addr.addrs[i], +                    nand->regs + MA35_NFI_REG_NANDADDR); +        }
+        return 0;
+

nit: New line is not really needed after return statement.

I will check and remove it.

Quoted 191 lines
Quoted 191 lines

+    case NAND_OP_DATA_IN_INSTR:
+        ma35_nand_do_read(chip, instr->ctx.data.buf.in, instr->ctx.data.len);
+        return 0;
+
+    case NAND_OP_DATA_OUT_INSTR:
+        ma35_nand_do_write(chip, instr->ctx.data.buf.out, instr->ctx.data.len);
+        return 0;
+
+    case NAND_OP_WAITRDY_INSTR:
+        return readl_poll_timeout(nand->regs + MA35_NFI_REG_NANDINTSTS, status,
+                      status & INT_RB0, 20,
+                      instr->ctx.waitrdy.timeout_ms * 1000); +    default:
+        break;
+    }

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +122 -0
+
+    return -EINVAL;
+}
+
+
+static int ma35_nfc_exec_op(struct nand_chip *chip,
+              const struct nand_operation *op,
+              bool check_only)
+{
+    struct ma35_nand_info *nand = nand_get_controller_data(chip);
+    u32 i, reg;
+    int ret = 0;
+
+    if (check_only)
+        return 0;
+
+    ma35_nand_target_enable(nand);
+
+    reg = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
+    reg |= INT_RB0;
+    writel(reg, nand->regs + MA35_NFI_REG_NANDINTSTS);
+
+    for (i = 0; i < op->ninstrs; i++) {
+        ret = ma35_nfc_exec_instr(chip, &op->instrs[i]);
+        if (ret)
+            break;
+    }
+
+    ma35_nand_target_disable(nand);
+
+    return ret;
+}
+
+
+static const struct nand_controller_ops ma35_nfc_ops = {
+    .attach_chip = ma35_nand_attach_chip,
+    .exec_op = ma35_nfc_exec_op,
+};
+
+static int ma35_nand_probe(struct platform_device *pdev)
+{
+    struct ma35_nand_info *nand;
+    struct nand_chip *chip;
+    struct mtd_info *mtd;
+    int ret = 0;
+
+    nand = devm_kzalloc(&pdev->dev, sizeof(*nand), GFP_KERNEL);
+    if (!nand)
+        return -ENOMEM;
+
+    nand_controller_init(&nand->controller);
+    nand->controller.ops = &ma35_nfc_ops;
+
+    nand->regs = devm_platform_ioremap_resource(pdev, 0);
+    if (IS_ERR(nand->regs))
+        return PTR_ERR(nand->regs);
+
+    nand->dev = &pdev->dev;
+    chip = &nand->chip;
+    nand_set_controller_data(chip, nand);
+    nand_set_flash_node(chip, pdev->dev.of_node);
+
+    nand->clk = devm_clk_get_enabled(&pdev->dev, "nand_gate");
+    if (IS_ERR(nand->clk))
+        return dev_err_probe(&pdev->dev, PTR_ERR(nand->clk),
+                     "failed to find nand clock\n");
+
+    nand->irq = platform_get_irq(pdev, 0);
+    if (nand->irq < 0)
+        return dev_err_probe(&pdev->dev, nand->irq,
+                     "failed to get platform irq\n");
+
+    ret = devm_request_irq(&pdev->dev, nand->irq, ma35_nand_irq,
+                  IRQF_TRIGGER_HIGH, "ma35d1-nand", nand);
+    if (ret) {
+        dev_err(&pdev->dev, "failed to request NAND irq\n");
+        return -ENXIO;
+    }
+
+    nand->chip.controller = &nand->controller;
+    platform_set_drvdata(pdev, nand);
+
+    chip->options |= NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA;
+
+    chip->ecc.engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
+    chip->ecc.write_page = ma35_nand_write_page_hwecc;
+    chip->ecc.read_page  = ma35_nand_read_page_hwecc;
+    chip->ecc.read_oob   = ma35_nand_read_oob_hwecc;
+
+    mtd = nand_to_mtd(chip);
+    mtd->priv = chip;
+    mtd->owner = THIS_MODULE;
+    mtd->dev.parent = &pdev->dev;
+
+    writel(NAND_EN, nand->regs + MA35_NFI_REG_GCTL);
+
+    ret = nand_scan(chip, 1);
+    if (ret)
+        return ret;
+
+    ret = mtd_device_register(mtd, NULL, 0);
+    if (ret) {
+        nand_cleanup(chip);
+        return ret;
+    }
+
+    return ret;
+}
+
+static void ma35_nand_remove(struct platform_device *pdev)
+{
+    struct ma35_nand_info *nand = platform_get_drvdata(pdev);
+    int ret;
+
+    ret = mtd_device_unregister(nand_to_mtd(&nand->chip));
+    WARN_ON(ret);
+    nand_cleanup(&nand->chip);
+}
+
+/* PM Support */
+#ifdef CONFIG_PM
+static int ma35_nand_suspend(struct platform_device *pdev, 

pm_message_t pm)

drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +50 -0
+{
+    struct ma35_nand_info *nand = platform_get_drvdata(pdev);
+    int ret = 0;
+    u32 val;
+
+    /* wait DMAC to ready */
+    ret = readl_poll_timeout(nand->regs + MA35_NFI_REG_DMACTL, val,
+                 !(val & DMA_BUSY), 50, HZ/2);
+    if (ret)
+        dev_warn(&pdev->dev, "dma busy\n");
+
+    clk_disable(nand->clk);
+
+    return ret;
+}
+
+static int ma35_nand_resume(struct platform_device *pdev)
+{
+    struct ma35_nand_info *nand = platform_get_drvdata(pdev);
+
+    clk_enable(nand->clk);
+    ma35_nand_hwecc_init(nand);
+    ma35_nand_dmac_init(nand);
+
+    return 0;
+}
+
+#else
+#define ma35_nand_suspend NULL
+#define ma35_nand_resume NULL
+#endif
+
+static const struct of_device_id ma35_nfi_of_match[] = {
+    { .compatible = "nuvoton,ma35d1-nand" },
+    {},
+};
+MODULE_DEVICE_TABLE(of, ma35_nfi_of_match);
+
+static struct platform_driver ma35_nand_driver = {
+    .driver = {
+        .name = "ma35d1-nand",
+        .of_match_table = ma35_nfi_of_match,
+    },
+    .probe = ma35_nand_probe,
+    .remove = ma35_nand_remove,
+    .suspend = ma35_nand_suspend,
+    .resume = ma35_nand_resume,
+};
+
+module_platform_driver(ma35_nand_driver);
Quoted 9 lines
Quoted 8 lines
drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file +4 -0
+
+MODULE_DESCRIPTION("Nuvoton ma35 NAND driver");
+MODULE_AUTHOR("Hui-Ping Chen <[email protected]>");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.25.1

Best regards,

Hui-Ping Chen