Hi Hui-Ping,

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


Thanks,
Miquèl

Reply via email to