[Linaro-mm-sig] Re: [PATCH v3 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC

Hi Hui-Ping,

hpchen0nvt@gmail.com wrote on Wed, 21 Aug 2024 07:11:32 +0000:

Nuvoton MA35 SoCs NAND Flash Interface Controller supports 2KB, 4KB and 8KB page size, and up to 8-bit,

Suffix is: kiB

12-bit, and 24-bit hardware ECC calculation circuit to protect data communication.

It's not the communication, it's the data itself.

Signed-off-by: Hui-Ping Chen hpchen0nvt@gmail.com

drivers/mtd/nand/raw/Kconfig | 8 + drivers/mtd/nand/raw/Makefile | 1 + drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c | 1068 ++++++++++++++++++++ 3 files changed, 1077 insertions(+) create mode 100644 drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index 614257308516..932bf2215470 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -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_NVT_MA35

Is NVT so common or is it just one opportunity to save 4 chars in a Kconfig file?? I'd prefer something more easy to understand.

• 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 diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile index 25120a4afada..cdfdfee3f5f3 100644 --- a/drivers/mtd/nand/raw/Makefile +++ b/drivers/mtd/nand/raw/Makefile @@ -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_NVT_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 diff --git a/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c b/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c new file mode 100644 index 000000000000..b4586d7a7a45 --- /dev/null +++ b/drivers/mtd/nand/raw/nuvoton_ma35d1_nand.c @@ -0,0 +1,1068 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Copyright (C) 2024 Nuvoton Technology Corp.
• */

+#include <linux/slab.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/platform_device.h> +#include <linux/delay.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/of.h>

Not sure why you didn't sort the below includes with the ones above?

+#include <linux/mtd/mtd.h> +#include <linux/mtd/partitions.h> +#include <linux/mtd/rawnand.h> +#include <linux/dma-mapping.h> +#include <linux/dmaengine.h>

+/* NFI DMA Registers */ +#define MA35_NFI_REG_BUFFER0 (0x000)

You don't need all these parentheses

+#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_DMABCNT (0x40C) +#define MA35_NFI_REG_DMAINTEN (0x410) +#define MA35_NFI_REG_DMAINTSTS (0x414)

+/* NFI Global Registers */ +#define MA35_NFI_REG_GCTL (0x800) +#define NAND_EN BIT(3) +#define MA35_NFI_REG_GINTEN (0x804) +#define MA35_NFI_REG_GINTSTS (0x808)

+/* NAND-type Flash Registers */ +#define MA35_NFI_REG_NANDCTL (0x8A0) +#define SWRST BIT(0) +#define DMA_W_EN BIT(1) +#define DMA_R_EN BIT(2) +#define ECC_CHK BIT(7) +#define PROT3BEN BIT(8) +#define PSIZE_2K (1 << 16) +#define PSIZE_4K (2 << 16) +#define PSIZE_8K (3 << 16) +#define PSIZE_MASK (3 << 16) +#define BCH_T24 BIT(18) +#define BCH_T8 BIT(20) +#define BCH_T12 BIT(21) +#define BCH_NONE (0x0) +#define BCH_MASK (0x1f << 18) +#define ECC_EN BIT(23) +#define DISABLE_CS0 BIT(25)

+#define MA35_NFI_REG_NANDTMCTL (0x8A4) +#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 (0x3) +#define ECC_ERR_CNT_MASK (0x1f)

+#define MA35_NFI_REG_NANDECCES1 (0x8D4) +#define MA35_NFI_REG_NANDECCES2 (0x8D8) +#define MA35_NFI_REG_NANDECCES3 (0x8DC)

+/* NAND-type Flash BCH Error Address Registers */ +#define MA35_NFI_REG_NANDECCEA0 (0x900) +#define MA35_NFI_REG_NANDECCEA1 (0x904) +#define MA35_NFI_REG_NANDECCEA2 (0x908) +#define MA35_NFI_REG_NANDECCEA3 (0x90C) +#define MA35_NFI_REG_NANDECCEA4 (0x910) +#define MA35_NFI_REG_NANDECCEA5 (0x914) +#define MA35_NFI_REG_NANDECCEA6 (0x918) +#define MA35_NFI_REG_NANDECCEA7 (0x91C) +#define MA35_NFI_REG_NANDECCEA8 (0x920) +#define MA35_NFI_REG_NANDECCEA9 (0x924) +#define MA35_NFI_REG_NANDECCEA10 (0x928) +#define MA35_NFI_REG_NANDECCEA11 (0x92C)

+/* NAND-type Flash BCH Error Data Registers */ +#define MA35_NFI_REG_NANDECCED0 (0x960) +#define MA35_NFI_REG_NANDECCED1 (0x964) +#define MA35_NFI_REG_NANDECCED2 (0x968) +#define MA35_NFI_REG_NANDECCED3 (0x96C) +#define MA35_NFI_REG_NANDECCED4 (0x970) +#define MA35_NFI_REG_NANDECCED5 (0x974)

+/* NAND-type Flash Redundant Area Registers */ +#define MA35_NFI_REG_NANDRA0 (0xA00) +#define MA35_NFI_REG_NANDRA1 (0xA04)

+#define SKIP_SPARE_BYTES 4

+/* BCH algorithm related constants and variables */ +static const int ma35_parity[3][4] = {

• {0, 60, 92, 90}, /* for 2K */
• {0, 120, 184, 180}, /* for 4K */
• {0, 240, 368, 360}, /* for 8K */

Can you please create definitions for the matrix rows? (using an enum seems appropriate)

And maybe an array of three structures would be best because I believe you're defining offsets for something which is not clear to the reader.

+};

+struct ma35_nand_info {

• struct nand_controller controller;
• struct device *dev;
• void __iomem *regs;
• int irq;
• struct clk *clk;
• struct completion complete;
• struct mtd_info mtd;

Please have a look at nand_to_mtd()

• struct nand_chip chip;

Is there a single CS supported? Is there a single RB supported?

• struct mtd_partition *parts;

No, this has nothing to do here.

• struct nand_ecclayout_user nand_oob;

Deprecated structure

• int nr_parts;
• u32 bch;
• u8 *dma_buf;
• spinlock_t dma_lock;
• dma_addr_t dma_addr;

+};

+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,

+};

+/*

• • Initialize hardware ECC
• */

+static void ma35_nand_hwecc_init(struct ma35_nand_info *nand) +{

• struct mtd_info *mtd = nand_to_mtd(&nand->chip);
• /* reset nand controller */

Reset NAND

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | SWRST,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

I believe it's fine to do it on several lines and probably clearer.

u32 reg = readl(); reg |= SOMETHING; writel();

No wait after the reset?

• /* Redundant area size */
• writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
• /* Protect redundant 3 bytes */

What does that mean?

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | PROT3BEN,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• /* Write the ECC parity codes automatically to NAND Flash */
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | ECC_CHK,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

No, by default you should disabled the ECC engine. Then when you need it you enable/use/disable it.

• 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 page */
  

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | ECC_EN,
  

• 	nand->regs + MA35_NFI_REG_NANDCTL);
  

• }
• spin_lock_init(&nand->dma_lock);

+}

+static void ma35_nand_initialize(struct ma35_nand_info *nand) +{

• writel(NAND_EN, nand->regs + MA35_NFI_REG_GCTL);

+}

+/* Define some constants for BCH */

for the BCH hardware ECC engine

+/* define the total padding bytes for 512/1024 data segment */ +#define BCH_PADDING_LEN_512 32 +#define BCH_PADDING_LEN_1024 64 +/* define the BCH parity code length for 512 bytes data pattern */ +#define BCH_PARITY_LEN_T8 15 +#define BCH_PARITY_LEN_T12 23 +/* define the BCH parity code length for 1024 bytes data pattern */ +#define BCH_PARITY_LEN_T24 45

Is T the strength? Can we name it strength instead?

Please move the definitions at the top

+/* Correct data by BCH alrogithm */ +static void ma35_nfi_correctdata(struct ma35_nand_info *nand, u8 index,

• 		 u8 err_cnt, u8 *addr)
  

correctdata vs correct, the naming needs to be improved

+{

• u8 *ptr = (u8 *)((long)nand->regs + MA35_NFI_REG_NANDRA0);

Haha, no, never.

Please compile with C=1 and see how this explodes.

Also, you can enable W=1

• u32 field_len, padding_len, parity_len;
• u32 temp_data[24], temp_addr[24];
• u32 total_field_num, page;
• u32 err_data[6];
• u8 *smra_index;
• u8 i, j;
• /* assign parameters for different BCH and page size */

configurations

• switch (readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK) {
• case BCH_T24:

• field_len = 1024;
  

• parity_len = BCH_PARITY_LEN_T24;
  

• padding_len = BCH_PADDING_LEN_1024;
  

• break;
  

• case BCH_T12:

• field_len = 512;
  

• parity_len = BCH_PARITY_LEN_T12;
  

• padding_len = BCH_PADDING_LEN_512;
  

• break;
  

• case BCH_T8:

• field_len = 512;
  

• parity_len = BCH_PARITY_LEN_T8;
  

• padding_len = BCH_PADDING_LEN_512;
  

• break;
  

• default:

• pr_warn("NAND ERROR: invalid SMCR_BCH_TSEL = 0x%08X\n",
  

• 	(u32)(readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK));
  

• return;
  

• }
• page = readl(nand->regs + MA35_NFI_REG_NANDCTL) & PSIZE_MASK;
• switch (page) {
• case PSIZE_8K:

• total_field_num = 8192 / field_len; break;
  

• case PSIZE_4K:

• total_field_num = 4096 / field_len; break;
  

• case PSIZE_2K:

• total_field_num = 2048 / field_len; break;
  

Break on a new line

• default:

• pr_warn("NAND ERROR: invalid SMCR_PSIZE = 0x%08X\n", page);
  

• 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 / 4;
• j++;
• if (j > 6)

• j = 6;	/* there are 6 BCH_ECC_DATAx registers to support BCH T24 */
  

• 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 / 2;
• j++;
• if (j > 12)

• j = 12; /* there are 12 REG_BCH_ECC_ADDRx registers to support BCH T24 */
  

• for (i = 0; i < j; i++) {

• /* 11 bits for error address */
  

• 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-1) * field_len;
• /* correct each error bytes */
• for (i = 0; i < err_cnt; i++) {

• /* for wrong data in field */
  

• if (temp_addr[i] < field_len)
  

• 	*(addr+temp_addr[i]) ^= temp_data[i];
  

• /* for wrong first-3-bytes in redundancy area */
  

• else if (temp_addr[i] < (field_len+3)) {
  

• 	temp_addr[i] -= field_len;
  

• 	temp_addr[i] += (parity_len * (index-1));	/* field offset */
  

• 	*(ptr + temp_addr[i]) ^= temp_data[i];
  

• }
  

• /* 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 {
  

• 	temp_addr[i] = temp_addr[i] - (field_len + padding_len - parity_len);
  

• 	/* smra_index point to the first parity code of
  

• 	 * first field in register SMRA0~n
  

• 	 */
  

• 	smra_index = (u8 *)(ptr +
  

• 		(readl(nand->regs+MA35_NFI_REG_NANDRACTL) & 0x1ff) -
  

• 		(parity_len * total_field_num));
  

• 	/* final address = first parity code of first field + */
  

• 	/*                 offset of fields +                 */
  

• 	/*                 offset within field                */
  

Coding style

• 	*((u8 *)smra_index + (parity_len * (index - 1)) + temp_addr[i])
  

• 		^= temp_data[i];
  

-ENOPARSE

• }
  

• } /* end of for (i < err_cnt) */

Useless comment

+}

+static int ma35_nfi_correct(struct nand_chip *chip, unsigned long addr) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• struct mtd_info *mtd = nand_to_mtd(chip);
• int status, i, j, field = 0;
• int report_err = 0;
• int err_cnt = 0;
• if ((readl(nand->regs + MA35_NFI_REG_NANDCTL) & BCH_MASK) == BCH_T24)

• field = mtd->writesize / 1024;
  

Can we call this a nchunks? Also, you're supposed to expect some DT properties (based on your bindings) and you're not using their values, it's strange.

• else

• field = mtd->writesize / 512;
  

• if (field < 4)

• field = 1;
  

• else

• field /= 4;
  

• for (j = 0; j < field; j++) {

• status = readl(nand->regs + MA35_NFI_REG_NANDECCES0 + j*4);
  

• if (!status)
  

• 	continue;
  

Is this case relevant? Isn't it treated below?

• for (i = 1; i < 5; i++) {
  

From 1 to 5? That's strange.

• 	if (!(status & ECC_STATUS_MASK)) {
  

• 		/* No error */
  

• 		status >>= 8;
  

You can't do that 5 times on a int.

• 		continue;
  

• 	} else if ((status & ECC_STATUS_MASK) == 0x01) {
  

• 		/* Correctable error */
  

• 		err_cnt = (status >> 2) & ECC_ERR_CNT_MASK;
  

Please use FIELD_GET()

• 		pr_warn("Field (%d, %d) have %d error!\n", j, i, err_cnt);
  

dev_warn()

• 		ma35_nfi_correctdata(nand, j*4+i, err_cnt, (u8 *)addr);
  

• 		report_err += err_cnt;
  

• 	} else {
  

• 		/* uncorrectable error */
  

• 		pr_warn("uncorrectable error! 0x%4x\n", status);
  

• 		return -1;
  

• 	}
  

• 	status >>= 8;
  

• }
  

• }
• return report_err;

+}

+static int ma35_nand_correct_data(struct nand_chip *chip, u_char *dat,

• 		  u_char *read_ecc, u_char *calc_ecc)
  

+{

• return 0;

?

+}

+static void ma35_nand_enable_hwecc(struct nand_chip *chip, int mode) +{

?

+}

+/*

• • Initial dma controller

Initialize DMA

• */

+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);

+}

+/*

• • read a byte from NAND controller

Read (same below)

• */

+static u8 ma35_nand_read_byte(struct nand_chip *chip) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• u8 ret;
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),

• nand->regs+MA35_NFI_REG_NANDCTL);
  

• ret = (u8)readl(nand->regs + MA35_NFI_REG_NANDDATA);

readb? maybe you need to handle endianness?

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• return ret;

+}

+/*

• • read data from NAND controller
• */

+static void ma35_nand_read_buf(struct nand_chip *chip, u8 *buf, int len) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• int i;
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• for (i = 0; i < len; i++)

• buf[i] = (u8)readl(nand->regs + MA35_NFI_REG_NANDDATA);
  

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

+} +/*

• • write data to NAND controller
• */

+static void ma35_nand_write_buf(struct nand_chip *chip, const u8 *buf, int len) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• int i;
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• for (i = 0; i < len; i++)

• writel(buf[i], nand->regs + MA35_NFI_REG_NANDDATA);
  

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

+}

+/*

• • configure and start dma transfer
• */

+static inline int ma35_nand_dma_transfer(struct nand_chip *chip,

• 			  const u_char *addr, u32 len, int is_write)
  

+{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• struct mtd_info *mtd = nand_to_mtd(chip);
• unsigned long timeo = jiffies + HZ/2;
• dma_addr_t dma_addr;
• int ret;
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),

• nand->regs+MA35_NFI_REG_NANDCTL);
  

• /* For save, wait DMAC to ready */

Please rework this comment

• while (1) {

• if ((readl(nand->regs + MA35_NFI_REG_DMACTL) & DMA_BUSY) == 0)
  

• 	break;
  

• if (time_after(jiffies, timeo))
  

• 	return -ETIMEDOUT;
  

readl_poll_timeout()

• }
• /* Reinitial dmac */
• ma35_nand_dmac_init(nand);
• writel(mtd->oobsize, nand->regs + MA35_NFI_REG_NANDRACTL);
• /* setup and start DMA using dma_addr */
• if (is_write) {

• u8 *ptr = (u8 *)((long)nand->regs + MA35_NFI_REG_NANDRA0);
  

• writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTEN);
  

• /* To mark this page as dirty. */
  

• if (ptr[3] == 0xFF)
  

• 	ptr[3] = 0;
  

• if (ptr[2] == 0xFF)
  

• 	ptr[2] = 0;
  

• /* Fill dma_addr */
  

• dma_addr = dma_map_single(nand->dev, (void *)addr, len, DMA_TO_DEVICE);
  

• dma_sync_single_for_device(nand->dev, dma_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;
  

• }
  

• 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);
  

• wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000));
  

• dma_unmap_single(nand->dev, dma_addr, len, DMA_TO_DEVICE);
  

• } else {

• writel(INT_DMA | INT_ECC, nand->regs + MA35_NFI_REG_NANDINTEN);
  

• /* Fill 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;
  

• }
  

• nand->dma_buf = (u8 *) addr;
  

the cast should not be neede

• nand->dma_addr = dma_addr;
  

• 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);
  

• wait_for_completion_timeout(&nand->complete, msecs_to_jiffies(1000));
  

Always check return values.

• dma_sync_single_for_cpu(nand->dev, dma_addr, len, DMA_FROM_DEVICE);
  

• dma_unmap_single(nand->dev, dma_addr, len, DMA_FROM_DEVICE);
  

• }
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• return 0;

+}

+static void ma35_read_buf_dma(struct nand_chip *chip, u_char *buf, int len) +{

• struct mtd_info *mtd = nand_to_mtd(chip);
• if (len == mtd->writesize)

• ma35_nand_dma_transfer(chip, buf, len, 0x0);
  

• else

• ma35_nand_read_buf(chip, buf, len);
  

+}

+static void ma35_write_buf_dma(struct nand_chip *chip, const u_char *buf, int len) +{

• struct mtd_info *mtd = nand_to_mtd(chip);
• if (len == mtd->writesize)

• ma35_nand_dma_transfer(chip, (u_char *)buf, len, 0x1);
  

• else

• ma35_nand_write_buf(chip, buf, len);
  

+}

This...

+static int ma35_nand_devready(struct nand_chip *chip) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• unsigned int val;
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),

• nand->regs+MA35_NFI_REG_NANDCTL);
  

• val = (readl(nand->regs + MA35_NFI_REG_NANDINTSTS) & INT_RB0_STS) ? 1 : 0;
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,

• nand->regs+MA35_NFI_REG_NANDCTL);
  

• return val;

+}

+static int ma35_waitfunc(struct nand_chip *chip) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• unsigned long timeo = jiffies;
• int status = -1;
• timeo += msecs_to_jiffies(400);
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• while (time_before(jiffies, timeo)) {

• status = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
  

• if (status & INT_RB0) {
  

• 	writel(INT_RB0, nand->regs + MA35_NFI_REG_NANDINTSTS);
  

• 	status = 0;
  

• 	break;
  

• }
  

• cond_resched();
  

• }
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• return status;

+}

+static void ma35_nand_command(struct nand_chip *chip, u32 command, int column, int page_addr) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• struct mtd_info *mtd = nand_to_mtd(chip);
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),

• nand->regs + MA35_NFI_REG_NANDCTL);
  

• writel(INT_RB0, nand->regs + MA35_NFI_REG_NANDINTSTS);
• if (command == NAND_CMD_READOOB) {

• command = NAND_CMD_READ0;
  

• column += mtd->writesize;
  

• }
• switch (command) {
• case NAND_CMD_RESET:

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• break;
  

• case NAND_CMD_READID:

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• writel(ENDADDR|column, nand->regs + MA35_NFI_REG_NANDADDR);
  

• break;
  

• case NAND_CMD_PARAM:

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• writel(ENDADDR|column, nand->regs + MA35_NFI_REG_NANDADDR);
  

• ma35_waitfunc(chip);
  

• break;
  

• case NAND_CMD_READ0:

• writel(ENABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL);
  

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• if (column != -1) {
  

• 	writel(column & 0xff, nand->regs + MA35_NFI_REG_NANDADDR);
  

• 	writel((column >> 8) & 0xff, nand->regs + MA35_NFI_REG_NANDADDR);
  

• }
  

• if (page_addr != -1) {
  

• 	writel(page_addr & 0xff, nand->regs + MA35_NFI_REG_NANDADDR);
  

• 	if (chip->options & NAND_ROW_ADDR_3) {
  

• 		writel((page_addr >> 8) & 0xff,
  

• 			nand->regs + MA35_NFI_REG_NANDADDR);
  

• 		writel(((page_addr >> 16) & 0xff) | ENDADDR,
  

• 			nand->regs + MA35_NFI_REG_NANDADDR);
  

• 	} else {
  

• 		writel(((page_addr >> 8) & 0xff) | ENDADDR,
  

• 			nand->regs + MA35_NFI_REG_NANDADDR);
  

• 	}
  

• }
  

• writel(NAND_CMD_READSTART, nand->regs + MA35_NFI_REG_NANDCMD);
  

• ma35_waitfunc(chip);
  

• break;
  

• case NAND_CMD_ERASE1:

• writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL);
  

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• writel(page_addr & 0xff, nand->regs + MA35_NFI_REG_NANDADDR);
  

• if (chip->options & NAND_ROW_ADDR_3) {
  

• 	writel((page_addr >> 8) & 0xff,
  

• 		nand->regs + MA35_NFI_REG_NANDADDR);
  

• 	writel(((page_addr >> 16) & 0xff) | ENDADDR,
  

• 		nand->regs + MA35_NFI_REG_NANDADDR);
  

• } else {
  

• 	writel(((page_addr >> 8) & 0xff) | ENDADDR,
  

• 		nand->regs + MA35_NFI_REG_NANDADDR);
  

• }
  

• break;
  

• case NAND_CMD_SEQIN:

• writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL);
  

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• writel(column & 0xff, nand->regs + MA35_NFI_REG_NANDADDR);
  

• writel(column >> 8, nand->regs + MA35_NFI_REG_NANDADDR);
  

• writel(page_addr & 0xff, nand->regs + MA35_NFI_REG_NANDADDR);
  

• if (chip->options & NAND_ROW_ADDR_3) {
  

• 	writel((page_addr >> 8) & 0xff,
  

• 		nand->regs + MA35_NFI_REG_NANDADDR);
  

• 	writel(((page_addr >> 16) & 0xff) | ENDADDR,
  

• 		nand->regs + MA35_NFI_REG_NANDADDR);
  

• } else {
  

• 	writel(((page_addr >> 8) & 0xff) | ENDADDR,
  

• 		nand->regs + MA35_NFI_REG_NANDADDR);
  

• }
  

• break;
  

• case NAND_CMD_STATUS:

• writel(DISABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL);
  

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• break;
  

• default:

• writel(command, nand->regs + MA35_NFI_REG_NANDCMD);
  

• }
• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,

• nand->regs + MA35_NFI_REG_NANDCTL);
  

+}

... is totally legacy. ->exec_op() is the API to implement instead.

+/* select chip */ +static void ma35_nand_select_chip(struct nand_chip *chip, int cs) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• if (cs == 0)

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) & (~DISABLE_CS0),
  

• 	nand->regs + MA35_NFI_REG_NANDCTL);
  

• else

• writel(readl(nand->regs + MA35_NFI_REG_NANDCTL) | DISABLE_CS0,
  

• 	nand->regs + MA35_NFI_REG_NANDCTL);
  

+}

+static int ma35_nand_calculate_ecc(struct nand_chip *chip, const u_char *dat,

• 		   u_char *ecc_code)
  

+{

• return 0;

+}

+static int ma35_nand_write_page_hwecc(struct nand_chip *chip, const u8 *buf,

• 		      int oob_required, int page)
  

+{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• u8 *ptr = (u8 *)((long)nand->regs + MA35_NFI_REG_NANDRA0);
• struct mtd_info *mtd = nand_to_mtd(chip);
• u8 *ecc_calc = chip->ecc.calc_buf;
• memset((void *)ptr, 0xFF, mtd->oobsize);
• memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize - chip->ecc.total);
• ma35_nand_command(chip, NAND_CMD_SEQIN, 0, page);
• ma35_nand_dma_transfer(chip, buf, mtd->writesize, 0x1);
• ma35_nand_command(chip, NAND_CMD_PAGEPROG, -1, -1);
• ma35_waitfunc(chip);
• /* Copy parity code in NANDRA to calc */
• memcpy((void *)ecc_calc,

• (void *)(ptr + (mtd->oobsize - chip->ecc.total)),
  

• chip->ecc.total);
  

• /* Copy parity code in calc to oob_poi */
• memcpy((void *)(chip->oob_poi + (mtd->oobsize - chip->ecc.total)),

• (void *)ecc_calc, chip->ecc.total);
  

• return 0;

+}

+static int ma35_nand_read_page_hwecc_oob_first(struct nand_chip *chip, u8 *buf,

• 				int oob_required, int page)
  

+{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• char *ptr = (char *)((long)nand->regs + MA35_NFI_REG_NANDRA0);
• struct mtd_info *mtd = nand_to_mtd(chip);
• /* 1. read the OOB area */
• ma35_nand_command(chip, NAND_CMD_READOOB, 0, page);
• ma35_nand_read_buf(chip, chip->oob_poi, mtd->oobsize);
• /* 2. copy OOB data to NANDRA for page read */
• memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize);
• if ((*(ptr+2) != 0) && (*(ptr+3) != 0))

• memset((void *)buf, 0xff, mtd->writesize);
  

• else {

• /* 3. read data from nand */
  

• ma35_nand_command(chip, NAND_CMD_READ0, 0, page);
  

• ma35_nand_dma_transfer(chip, buf, mtd->writesize, 0x0);
  

• /* 4. restore OOB data from SMRA */
  

• memcpy((void *)chip->oob_poi, (void *)ptr, mtd->oobsize);
  

• }
• return 0;

+}

Not sure why you need these two implementations? Usually there is only one per controller and fully depends on your controller capabilities/limitations. If you have no limitation, just support the simple case.

+static void ma35_layout_oob_table(struct nand_ecclayout_user *oobtable,

• 		  int oobsize, int eccbytes)
  

+{

• oobtable->eccbytes = eccbytes;
• oobtable->oobavail = oobsize - SKIP_SPARE_BYTES - eccbytes;
• oobtable->oobfree[0].offset = SKIP_SPARE_BYTES;
• oobtable->oobfree[0].length = oobsize - eccbytes - oobtable->oobfree[0].offset;
• oobtable->oobfree[1].offset = 0;
• oobtable->oobfree[1].length = 0;

+}

I'm not sure why you would need that.

+static int ma35_nand_read_oob_hwecc(struct nand_chip *chip, int page) +{

• struct ma35_nand_info *nand = nand_get_controller_data(chip);
• char *ptr = (char *)((long)nand->regs + MA35_NFI_REG_NANDRA0);
• struct mtd_info *mtd = nand_to_mtd(chip);
• ma35_nand_command(chip, NAND_CMD_READOOB, 0, page);
• ma35_nand_read_buf(chip, chip->oob_poi, mtd->oobsize);
• /* copy OOB data to NANDRA for page read */
• memcpy((void *)ptr, (void *)chip->oob_poi, mtd->oobsize);
• if ((*(ptr+2) != 0) && (*(ptr+3) != 0))

• 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;
• struct mtd_info *mtd = nand_to_mtd(&nand->chip);
• int stat = 0;
• u32 isr;
• spin_lock(&nand->dma_lock);
• isr = readl(nand->regs + MA35_NFI_REG_NANDINTSTS);
• if (isr & INT_ECC) {

• dma_sync_single_for_cpu(nand->dev, nand->dma_addr, mtd->writesize,
  

• 			DMA_FROM_DEVICE);
  

• stat = ma35_nfi_correct(&nand->chip, (unsigned long)nand->dma_buf);
  

• if (stat < 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 if (stat > 0) {
  

• 	mtd->ecc_stats.corrected += stat;   /* Add corrected bit count */
  

• }
  

• writel(INT_ECC, nand->regs + MA35_NFI_REG_NANDINTSTS);
  

• }
• if (isr & INT_DMA) {

• writel(INT_DMA, nand->regs + MA35_NFI_REG_NANDINTSTS);
  

• complete(&nand->complete);
  

• }
• spin_unlock(&nand->dma_lock);
• 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;

Please have a look at other controller drivers. You need to allow:

- no correction - sw correction - hw BCH correction

• /* 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)

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

• if (chip->ecc.strength == 0) {

• nand->bch = BCH_NONE; /* No ECC */
  

• ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize,
  

• 			ma35_parity[mtd->writesize>>12][0]);
  

• } else if (chip->ecc.strength <= 8) {

• nand->bch = BCH_T8; /* T8 */
  

• ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize,
  

• 			ma35_parity[mtd->writesize>>12][1]);
  

• } else if (chip->ecc.strength <= 12) {

• nand->bch = BCH_T12; /* T12 */
  

• ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize,
  

• 			ma35_parity[mtd->writesize>>12][2]);
  

• } else if (chip->ecc.strength <= 24) {

• nand->bch = BCH_T24; /* T24 */
  

• ma35_layout_oob_table(&nand->nand_oob, mtd->oobsize,
  

• 			ma35_parity[mtd->writesize>>12][3]);
  

• } else {

• pr_warn("NAND Controller is not support this flash. (%d, %d)\n",
  

• 	mtd->writesize, mtd->oobsize);
  

• }
• chip->ecc.steps = mtd->writesize / chip->ecc.size;
• chip->ecc.bytes = nand->nand_oob.eccbytes / chip->ecc.steps;
• chip->ecc.total = nand->nand_oob.eccbytes;
• 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(ENABLE_WP, nand->regs + MA35_NFI_REG_NANDECTL);
• return 0;

+}

+static const struct nand_controller_ops ma35_nand_controller_ops = {

• .attach_chip = ma35_nand_attach_chip,

+};

+static int ma35_nand_probe(struct platform_device *pdev) +{

• struct ma35_nand_info *nand;
• struct nand_chip *chip;
• struct mtd_info *mtd;
• int retval = 0;
• nand = devm_kzalloc(&pdev->dev, sizeof(*nand), GFP_KERNEL);
• if (!nand)

• return -ENOMEM;
  

• nand_controller_init(&nand->controller);
• 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;
• mtd = nand_to_mtd(chip);
• nand_set_controller_data(chip, nand);
• nand_set_flash_node(chip, pdev->dev.of_node);
• mtd->priv = chip;
• mtd->owner = THIS_MODULE;
• mtd->dev.parent = &pdev->dev;
• nand->clk = devm_clk_get(&pdev->dev, "nand_gate");

devm_clk_get_enabled()

• if (IS_ERR(nand->clk))

• return dev_err_probe(&pdev->dev, PTR_ERR(nand->clk),
  

• 		     "failed to find nand clock\n");
  

• retval = clk_prepare_enable(nand->clk);
• if (retval < 0) {

• dev_err(&pdev->dev, "failed to enable clock\n");
  

• retval = -ENXIO;
  

• }
• nand->chip.controller = &nand->controller;
• chip->legacy.cmdfunc = ma35_nand_command;
• chip->legacy.waitfunc = ma35_waitfunc;
• chip->legacy.read_byte = ma35_nand_read_byte;
• chip->legacy.select_chip = ma35_nand_select_chip;
• chip->legacy.read_buf = ma35_read_buf_dma;
• chip->legacy.write_buf = ma35_write_buf_dma;
• chip->legacy.dev_ready = ma35_nand_devready;
• chip->legacy.chip_delay = 25; /* us */

Please convert this driver to ->exec_op().

No introduce of legacy hooks will be accepted.

• /* Read OOB data first, then HW read page */
• chip->ecc.hwctl = ma35_nand_enable_hwecc;
• chip->ecc.calculate = ma35_nand_calculate_ecc;
• chip->ecc.correct = ma35_nand_correct_data;
• chip->ecc.write_page = ma35_nand_write_page_hwecc;
• chip->ecc.read_page = ma35_nand_read_page_hwecc_oob_first;
• chip->ecc.read_oob = ma35_nand_read_oob_hwecc;
• chip->options |= (NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA);
• ma35_nand_initialize(nand);
• platform_set_drvdata(pdev, nand);
• nand->controller.ops = &ma35_nand_controller_ops;
• 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");
  

• retval = devm_request_irq(&pdev->dev, nand->irq, ma35_nand_irq,

• 		  IRQF_TRIGGER_HIGH, "ma35d1-nand", nand);
  

• if (retval) {

• dev_err(&pdev->dev, "failed to request NAND irq\n");
  

• clk_disable_unprepare(nand->clk);
  

• return -ENXIO;
  

• }
• retval = nand_scan(chip, 1);

s/retval/ret/

• if (retval)

• return retval;
  

• if (mtd_device_register(mtd, nand->parts, nand->nr_parts)) {

ret = if (ret)

• nand_cleanup(chip);
  

• devm_kfree(&pdev->dev, nand);
  

• return retval;
  

• }
• return retval;

+}

+static void ma35_nand_remove(struct platform_device *pdev) +{

• struct ma35_nand_info *nand = platform_get_drvdata(pdev);
• int ret;
• devm_free_irq(&pdev->dev, nand->irq, nand);
• ret = mtd_device_unregister(nand_to_mtd(&nand->chip));
• WARN_ON(ret);
• nand_cleanup(&nand->chip);
• clk_disable_unprepare(nand->clk);

+}

+/* 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);
• unsigned long timeo = jiffies + HZ/2;
• /* wait DMAC to ready */
• while (1) {

• if ((readl(nand->regs + MA35_NFI_REG_DMACTL) & DMA_BUSY) == 0)
  

• 	break;
  

• if (time_after(jiffies, timeo))
  

• 	return -ETIMEDOUT;
  

• }
• clk_disable(nand->clk);
• return 0;

+}

+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_new = 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 hpchen0nvt@gmail.com"); +MODULE_LICENSE("GPL");

Thanks, Miquèl