Hi Naga, Naga Sureshkumar Relli <naga.sureshkumar.relli at xilinx.com> wrote on Wed, 11 Jul 2018 13:06:52 +0530: > Add driver for arm pl353 static memory controller nand interface with HW ECC > support. This controller is used in Xilinx Zynq SoC for interfacing the NAND > flash memory. > > Signed-off-by: Naga Sureshkumar Relli <naga.sureshkumar.relli at xilinx.com> > --- > Changes in v11: > - Removed Documentation patch and added the required info in driver as > per Boris comments. > - Removed unwanted variables from pl353_nand_info as per Miquel comments > - Removed IO_ADDR_R/W. > - Replaced onhot() with hweight32() > - Defined macros for static values in function pl353_nand_correct_data() > - Removed all unnecessary delays > - Used nand_wait_ready() where ever is required > - Modifed the pl353_setup_data_interface() logic as per Miquel comments. > - Taken array instead of 7 values in pl353_setup_data_interface() and pass > it to smc driver. > - Added check to collect the return value of mtd_device_register(). > Changes in 10: > - Typos correction like nand to NAND and soc to SOC etc.. > - Defined macros for the values in pl353_nand_calculate_hwecc() > - Modifed ecc_status from int to char in pl353_nand_calculate_hwecc() > - Changed the return type form int to bool to the function > onehot() > - Removed udelay(1000) in pl353_cmd_function, as it is not required > - Dropped ecc->hwctl = NULL in pl353_ecc_init() > - Added an error message in pl353_ecc_init(), when there is no matching > oobsize > - Changed the variable from xnand to xnfc > - Added logic to get mtd->name from DT, if it is specified in DT > Changes in v9: > - Addressed the below comments given by Miquel > - instead of using pl353_nand_write32, use directly writel_relaxed > - Fixed check patch warnings > - Renamed write_buf/read_buf to write_data_op/read_data_op > - use BIT macro instead of 1 << nr > - Use NAND_ROW_ADDR_3 flag > - Use nand_wait_ready() > - Removed swecc functions > - Use address cycles as per size, instead of reading it from Parameter page > - Instead of writing too many patterns, use optional property > Changes in v8: > - Added exec_op() implementation > - Fixed the below v7 review comments > - removed mtd_info from pl353_nand_info struct > - Corrected ecc layout offsets > - Added on-die ecc support > Changes in v7: > - Currently not implemented the memclk rate adjustments. I will > look into this later and once the basic driver is accepted. > - Fixed GPL licence ident > Changes in v6: > - Fixed the checkpatch.pl reported warnings > - Using the address cycles information from the onfi param page > earlier it is hardcoded to 5 in driver > Changes in v5: > - Configure the nand timing parameters as per the onfi spec Changes in v4: > - Updated the driver to sync with pl353_smc driver APIs > Changes in v3: > - implemented the proper error codes > - further breakdown this patch to multiple sets > - added the controller and driver details to Documentation section > - updated the licenece to GPLv2 > - reorganized the pl353_nand_ecc_init function > Changes in v2: > - use "depends on" rather than "select" option in kconfig > - remove unused variable parts > --- > drivers/mtd/nand/raw/Kconfig | 7 + > drivers/mtd/nand/raw/Makefile | 1 + > drivers/mtd/nand/raw/pl353_nand.c | 1314 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 1322 insertions(+) > create mode 100644 drivers/mtd/nand/raw/pl353_nand.c > > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig > index 6871ff0..1c5d528 100644 > --- a/drivers/mtd/nand/raw/Kconfig > +++ b/drivers/mtd/nand/raw/Kconfig > @@ -530,4 +530,11 @@ config MTD_NAND_MTK > Enables support for NAND controller on MTK SoCs. > This controller is found on mt27xx, mt81xx, mt65xx SoCs. > > +config MTD_NAND_PL353 > + tristate "ARM Pl353 NAND flash driver" > + depends on MTD_NAND && ARM > + depends on PL353_SMC > + help > + Enables support for PrimeCell Static Memory Controller PL353. > + > endif # MTD_NAND > diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile > index 165b7ef..1c702e1 100644 > --- a/drivers/mtd/nand/raw/Makefile > +++ b/drivers/mtd/nand/raw/Makefile > @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504) += hisi504_nand.o > obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/ > obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o > obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o > +obj-$(CONFIG_MTD_NAND_PL353) += pl353_nand.o > > nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o > nand-objs += nand_amd.o > diff --git a/drivers/mtd/nand/raw/pl353_nand.c b/drivers/mtd/nand/raw/pl353_nand.c > new file mode 100644 > index 0000000..3fa9ba07 > --- /dev/null > +++ b/drivers/mtd/nand/raw/pl353_nand.c > @@ -0,0 +1,1314 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ARM PL353 NAND flash controller driver > + * > + * Copyright (C) 2017 Xilinx, Inc > + * Author: Punnaiah chowdary kalluri <punnaiah at xilinx.com> > + * Author: Naga Sureshkumar Relli <nagasure at xilinx.com> > + * > + */ > + > +#include <linux/err.h> > +#include <linux/delay.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/ioport.h> > +#include <linux/irq.h> > +#include <linux/module.h> > +#include <linux/moduleparam.h> > +#include <linux/mtd/mtd.h> > +#include <linux/mtd/rawnand.h> > +#include <linux/mtd/nand_ecc.h> > +#include <linux/mtd/partitions.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/pl353-smc.h> > +#include <linux/clk.h> > + > +#define PL353_NAND_DRIVER_NAME "pl353-nand" > + > +/* NAND flash driver defines */ > +#define PL353_NAND_CMD_PHASE 1 /* End command valid in command phase */ > +#define PL353_NAND_DATA_PHASE 2 /* End command valid in data phase */ > +#define PL353_NAND_ECC_SIZE 512 /* Size of data for ECC operation */ > + > +/* Flash memory controller operating parameters */ > + > +#define PL353_NAND_ECC_CONFIG (BIT(4) | /* ECC read at end of page */ \ > + (0 << 5)) /* No Jumping */ > + > +/* AXI Address definitions */ > +#define START_CMD_SHIFT 3 > +#define END_CMD_SHIFT 11 > +#define END_CMD_VALID_SHIFT 20 > +#define ADDR_CYCLES_SHIFT 21 > +#define CLEAR_CS_SHIFT 21 > +#define ECC_LAST_SHIFT 10 > +#define COMMAND_PHASE (0 << 19) > +#define DATA_PHASE BIT(19) > + > +#define PL353_NAND_ECC_LAST BIT(ECC_LAST_SHIFT) /* Set ECC_Last */ > +#define PL353_NAND_CLEAR_CS BIT(CLEAR_CS_SHIFT) /* Clear chip select */ > + > +#define ONDIE_ECC_FEATURE_ADDR 0x90 > +#define PL353_NAND_ECC_BUSY_TIMEOUT (1 * HZ) > +#define PL353_NAND_DEV_BUSY_TIMEOUT (1 * HZ) > +#define PL353_NAND_LAST_TRANSFER_LENGTH 4 > +#define PL353_NAND_ECC_VALID_SHIFT 24 > +#define PL353_NAND_ECC_VALID_MASK 0x40 > +#define PL353_ECC_BITS_BYTEOFF_MASK 0x1FF > +#define PL353_ECC_BITS_BITOFF_MASK 0x7 > +#define PL353_ECC_BIT_MASK 0xFFF > +#define PL353_TREA_MAX_VALUE 1 > + > +struct pl353_nfc_op { > + u32 cmnds[4]; > + u32 thirdrow; > + u32 type; > + u32 end_cmd; > + u32 addrs; > + u32 len; > + u32 naddrs; > + u32 addr5; > + u32 addr6; > + unsigned int data_instr_idx; > + unsigned int rdy_timeout_ms; > + unsigned int rdy_delay_ns; > + unsigned int cle_ale_delay_ns; > + const struct nand_op_instr *data_instr; > +}; > + > +/** > + * struct pl353_nand_info - Defines the NAND flash driver instance I suppose you mean "NAND flash controller driver instance", in this case, pl353_nand_info could be pl353_nand_controller? > + * @chip: NAND chip information structure > + * @dev: Parent device (used to print error messages) > + * @nand_base: Virtual address of the NAND flash device > + * @nandaddr: Virtual address of the NAND flash device for > + * data read/writes > + * @addr_cycles: Address cycles > + * @mclk: Memory controller clock > + */ > +struct pl353_nand_info { > + struct nand_chip chip; > + struct device *dev; > + void __iomem *nand_base; > + void __iomem *nandaddr; Maybe nand_base and nandaddr are not very descriptive names. What about 'regs' intead of nand_base? And if nandaddr is a buffer address, what about "buf_addr"? > + u8 addr_cycles; > + struct clk *mclk; > +}; > + > +static int pl353_ecc_ooblayout16_ecc(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (section >= chip->ecc.steps) > + return -ERANGE; > + > + oobregion->offset = (section * chip->ecc.bytes); > + oobregion->length = chip->ecc.bytes; > + > + return 0; > +} > + > +static int pl353_ecc_ooblayout16_free(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (section >= chip->ecc.steps) > + return -ERANGE; > + > + oobregion->offset = (section * chip->ecc.bytes) + 8; > + oobregion->length = 8; > + > + return 0; > +} > + > +static const struct mtd_ooblayout_ops pl353_ecc_ooblayout16_ops = { > + .ecc = pl353_ecc_ooblayout16_ecc, > + .free = pl353_ecc_ooblayout16_free, > +}; > + > +static int pl353_ecc_ooblayout64_ecc(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (section >= chip->ecc.steps) > + return -ERANGE; > + > + oobregion->offset = (section * chip->ecc.bytes) + 52; > + oobregion->length = chip->ecc.bytes; > + > + return 0; > +} > + > +static int pl353_ecc_ooblayout64_free(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + > + if (section) > + return -ERANGE; > + > + if (section >= chip->ecc.steps) > + return -ERANGE; > + > + oobregion->offset = (section * chip->ecc.bytes) + 2; > + oobregion->length = 50; > + > + return 0; > +} > + > +static const struct mtd_ooblayout_ops pl353_ecc_ooblayout64_ops = { > + .ecc = pl353_ecc_ooblayout64_ecc, > + .free = pl353_ecc_ooblayout64_free, > +}; > + > +/* Generic flash bbt decriptors */ > +static u8 bbt_pattern[] = { 'B', 'b', 't', '0' }; > +static u8 mirror_pattern[] = { '1', 't', 'b', 'B' }; > + > +static struct nand_bbt_descr bbt_main_descr = { > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE > + | NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP, > + .offs = 4, > + .len = 4, > + .veroffs = 20, > + .maxblocks = 4, > + .pattern = bbt_pattern > +}; > + > +static struct nand_bbt_descr bbt_mirror_descr = { > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE > + | NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP, > + .offs = 4, > + .len = 4, > + .veroffs = 20, > + .maxblocks = 4, > + .pattern = mirror_pattern > +}; > + > +/** > + * pl353_nand_read_data_op - read chip data into buffer > + * @chip: Pointer to the NAND chip info structure > + * @in: Pointer to the buffer to store read data > + * @len: Number of bytes to read > + * Return: Always return zero > + */ > +static int pl353_nand_read_data_op(struct nand_chip *chip, > + u8 *in, > + unsigned int len) > +{ > + int i; > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + > + if (IS_ALIGNED((uint32_t)in, sizeof(uint32_t)) && > + IS_ALIGNED(len, sizeof(uint32_t))) { > + u32 *ptr = (u32 *)in; > + > + len /= 4; > + for (i = 0; i < len; i++) > + ptr[i] = readl(xnfc->nandaddr); > + } else { > + for (i = 0; i < len; i++) > + in[i] = readb(xnfc->nandaddr); > + } What about reading 0-3 bytes with readb if the driver is not aligned, then doing aligned access with readl until readb must be used again for the last 0-3 bytes? > + > + return 0; > +} > + > +/** > + * pl353_nand_write_buf - write buffer to chip > + * @mtd: Pointer to the mtd info structure > + * @buf: Pointer to the buffer to store write data > + * @len: Number of bytes to write > + */ > +static void pl353_nand_write_data_op(struct mtd_info *mtd, const u8 *buf, > + int len) > +{ > + int i; > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + > + if (IS_ALIGNED((uint32_t)buf, sizeof(uint32_t)) && > + IS_ALIGNED(len, sizeof(uint32_t))) { > + u32 *ptr = (u32 *)buf; > + > + len /= 4; > + for (i = 0; i < len; i++) > + writel(ptr[i], xnfc->nandaddr); > + } else { > + for (i = 0; i < len; i++) > + writeb(buf[i], xnfc->nandaddr); > + } Ditto. > +} > + > +/** > + * pl353_nand_calculate_hwecc - Calculate Hardware ECC > + * @mtd: Pointer to the mtd_info structure > + * @data: Pointer to the page data > + * @ecc: Pointer to the ECC buffer where ECC data needs to be stored > + * > + * This function retrieves the Hardware ECC data from the controller and returns > + * ECC data back to the MTD subsystem. > + * It operates on a number of 512 byte blocks of NAND memory and can be > + * programmed to store the ECC codes after the data in memory. For writes, > + * the ECC is written to the spare area of the page. For reads, the result of > + * a block ECC check are made available to the device driver. > + * > + * ------------------------------------------------------------------------ > + * | n * 512 blocks | extra | ecc | | > + * | | block | codes | | > + * ------------------------------------------------------------------------ > + * > + * The ECC calculation uses a simple Hamming code, using 1-bit correction 2-bit > + * detection. It starts when a valid read or write command with a 512 byte > + * aligned address is detected on the memory interface. > + * > + * Return: 0 on success or error value on failure > + */ > +static int pl353_nand_calculate_hwecc(struct mtd_info *mtd, > + const u8 *data, u8 *ecc) > +{ > + u32 ecc_value; > + u8 ecc_reg, ecc_byte, ecc_status; > + unsigned long timeout = jiffies + PL353_NAND_ECC_BUSY_TIMEOUT; > + > + /* Wait till the ECC operation is complete or timeout */ > + do { > + if (pl353_smc_ecc_is_busy()) > + cpu_relax(); > + else > + break; > + } while (!time_after_eq(jiffies, timeout)); > + > + if (time_after_eq(jiffies, timeout)) { > + pr_err("%s timed out\n", __func__); > + return -ETIMEDOUT; > + } All of this should be done by the function calling nand_calculate_hwecc(), not here. Plus, it should deserve a function on its own. > + > + for (ecc_reg = 0; ecc_reg < 4; ecc_reg++) { > + /* Read ECC value for each block */ So you assume here that there are 4, and only 4 "blocks" (I prefer "ECC chunks" or "subpages"). Is it always the case? Or is it just how it works in your situation? I would rather prefer a to define this value anyway. > + ecc_value = pl353_smc_get_ecc_val(ecc_reg); > + ecc_status = (ecc_value >> PL353_NAND_ECC_VALID_SHIFT); Space > + /* ECC value valid */ > + if (ecc_status & PL353_NAND_ECC_VALID_MASK) { > + for (ecc_byte = 0; ecc_byte < 3; ecc_byte++) { Define 3 > + /* Copy ECC bytes to MTD buffer */ > + *ecc = ~ecc_value & 0xFF; > + ecc_value = ecc_value >> 8; > + ecc++; > + } > + } else { > + pr_warn("%s status failed\n", __func__); > + return -1; > + } > + } > + > + return 0; > +} > + > +/** > + * pl353_nand_correct_data - ECC correction function > + * @mtd: Pointer to the mtd_info structure > + * @buf: Pointer to the page data > + * @read_ecc: Pointer to the ECC value read from spare data area > + * @calc_ecc: Pointer to the calculated ECC value > + * > + * This function corrects the ECC single bit errors & detects 2-bit errors. > + * > + * Return: 0 if no ECC errors found > + * 1 if single bit error found and corrected. > + * -1 if multiple uncorrectable ECC errors found. > + */ > +static int pl353_nand_correct_data(struct mtd_info *mtd, unsigned char *buf, > + unsigned char *read_ecc, > + unsigned char *calc_ecc) > +{ > + unsigned char bit_addr; > + unsigned int byte_addr; > + unsigned short ecc_odd, ecc_even, read_ecc_lower, read_ecc_upper; > + unsigned short calc_ecc_lower, calc_ecc_upper; > + > + read_ecc_lower = (read_ecc[0] | (read_ecc[1] << 8)) & > + PL353_ECC_BIT_MASK; > + read_ecc_upper = ((read_ecc[1] >> 4) | (read_ecc[2] << 4)) & > + PL353_ECC_BIT_MASK; > + > + calc_ecc_lower = (calc_ecc[0] | (calc_ecc[1] << 8)) & > + PL353_ECC_BIT_MASK; > + calc_ecc_upper = ((calc_ecc[1] >> 4) | (calc_ecc[2] << 4)) & > + PL353_ECC_BIT_MASK; > + > + ecc_odd = read_ecc_lower ^ calc_ecc_lower; > + ecc_even = read_ecc_upper ^ calc_ecc_upper; > + > + /* no error */ > + if (!ecc_odd && !ecc_even) > + return 0; > + > + if (ecc_odd == (~ecc_even & PL353_ECC_BIT_MASK)) { > + /* bits [11:3] of error code is byte offset */ > + byte_addr = (ecc_odd >> 3) & PL353_ECC_BITS_BYTEOFF_MASK; > + /* bits [2:0] of error code is bit offset */ > + bit_addr = ecc_odd & PL353_ECC_BITS_BITOFF_MASK; > + /* Toggling error bit */ > + buf[byte_addr] ^= (BIT(bit_addr)); > + return 1; > + } > + > + /* one error in parity */ > + if (hweight32(ecc_odd | ecc_even) == 1) > + return 1; > + > + /* Uncorrectable error */ > + return -1; > +} > + > +static void pl353_prepare_cmd(struct mtd_info *mtd, struct nand_chip *chip, > + int page, int column, int start_cmd, int end_cmd, > + bool read) > +{ > + unsigned long data_phase_addr; > + u32 end_cmd_valid = 0; > + unsigned long cmd_phase_addr = 0, cmd_data = 0; > + > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + > + end_cmd_valid = read ? 1 : 0; > + > + cmd_phase_addr = (unsigned long __force)xnfc->nand_base + do you really need this cast? > + ((xnfc->addr_cycles > + << ADDR_CYCLES_SHIFT) | > + (end_cmd_valid << END_CMD_VALID_SHIFT) | > + (COMMAND_PHASE) | > + (end_cmd << END_CMD_SHIFT) | > + (start_cmd << START_CMD_SHIFT)); So the number of address cycles changes the address where you should write the address cycles? that's weird, no? > + > + /* Get the data phase address */ > + data_phase_addr = (unsigned long __force)xnfc->nand_base + > + ((0x0 << CLEAR_CS_SHIFT) | > + (0 << END_CMD_VALID_SHIFT) | > + (DATA_PHASE) | > + (end_cmd << END_CMD_SHIFT) | > + (0x0 << ECC_LAST_SHIFT)); > + Same question here? > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; This should be done only once in the probe > + > + if (chip->options & NAND_BUSWIDTH_16) > + column /= 2; > + cmd_data = column; Please rename cmd_data because assigning a column (ie, an address cycle) to a variable name "cmd_data" is weird. > + if (mtd->writesize > PL353_NAND_ECC_SIZE) { > + cmd_data |= page << 16; > + /* Another address cycle for devices > 128MiB */ > + if (chip->options & NAND_ROW_ADDR_3) { > + writel_relaxed(cmd_data, > + (void __iomem * __force)cmd_phase_addr); > + cmd_data = (page >> 16); > + } > + } else { > + cmd_data |= page << 8; > + } > + > + writel_relaxed(cmd_data, (void __iomem * __force)cmd_phase_addr); > +} > + > +/** > + * pl353_nand_read_oob - [REPLACEABLE] the most common OOB data read function > + * @mtd: Pointer to the mtd info structure NAND and MTD are always upper case in plain english. Here I suppose you refer to the structure name, the use the exact name "mtd_info". > + * @chip: Pointer to the NAND chip info structure > + * @page: Page number to read > + * > + * Return: Always return zero > + */ > +static int pl353_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip, > + int page) > +{ > + unsigned long data_phase_addr; > + u8 *p; > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + unsigned long nand_offset = (unsigned long __force)xnfc->nand_base; > + > + chip->pagebuf = -1; > + if (mtd->writesize < PL353_NAND_ECC_SIZE) > + return 0; > + > + pl353_prepare_cmd(mtd, chip, page, mtd->writesize, NAND_CMD_READ0, > + NAND_CMD_READSTART, 1); > + > + nand_wait_ready(mtd); > + > + p = chip->oob_poi; > + pl353_nand_read_data_op(chip, p, > + (mtd->oobsize - > + PL353_NAND_LAST_TRANSFER_LENGTH)); > + p += (mtd->oobsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_CLEAR_CS; > + data_phase_addr += nand_offset; This is still a foreign language for me, why do you do the math everytime? And please explain what's done here, it's absolutely not clear for me. > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; You should not need the cast. > + pl353_nand_read_data_op(chip, p, PL353_NAND_LAST_TRANSFER_LENGTH); > + > + return 0; > +} > + > +/** > + * pl353_nand_write_oob - [REPLACEABLE] the most common OOB data write function > + * @mtd: Pointer to the mtd info structure > + * @chip: Pointer to the NAND chip info structure > + * @page: Page number to write > + * > + * Return: Zero on success and EIO on failure > + */ > +static int pl353_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip, > + int page) > +{ > + const u8 *buf = chip->oob_poi; > + unsigned long data_phase_addr; > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + unsigned long nand_offset = (unsigned long __force)xnfc->nand_base; > + u32 addrcycles = 0; > + > + chip->pagebuf = -1; > + addrcycles = xnfc->addr_cycles; > + pl353_prepare_cmd(mtd, chip, page, mtd->writesize, NAND_CMD_SEQIN, > + NAND_CMD_PAGEPROG, 0); > + > + pl353_nand_write_data_op(mtd, buf, > + (mtd->oobsize - > + PL353_NAND_LAST_TRANSFER_LENGTH)); > + buf += (mtd->oobsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_CLEAR_CS; > + data_phase_addr |= (1 << END_CMD_VALID_SHIFT); > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + pl353_nand_write_data_op(mtd, buf, PL353_NAND_LAST_TRANSFER_LENGTH); > + nand_wait_ready(mtd); > + > + return 0; > +} > + > +/** > + * pl353_nand_read_page_raw - [Intern] read raw page data without ecc > + * @mtd: Pointer to the mtd info structure > + * @chip: Pointer to the NAND chip info structure > + * @buf: Pointer to the data buffer > + * @oob_required: Caller requires OOB data read to chip->oob_poi > + * @page: Page number to read > + * > + * Return: Always return zero > + */ > +static int pl353_nand_read_page_raw(struct mtd_info *mtd, > + struct nand_chip *chip, > + u8 *buf, int oob_required, int page) > +{ > + unsigned long data_phase_addr; > + u8 *p; > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + unsigned long nand_offset = (unsigned long __force)xnfc->nand_base; > + > + pl353_nand_read_data_op(chip, buf, mtd->writesize); > + p = chip->oob_poi; > + pl353_nand_read_data_op(chip, p, > + (mtd->oobsize - > + PL353_NAND_LAST_TRANSFER_LENGTH)); > + p += (mtd->oobsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_CLEAR_CS; > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + > + pl353_nand_read_data_op(chip, p, PL353_NAND_LAST_TRANSFER_LENGTH); > + > + return 0; > +} > + > +/** > + * pl353_nand_write_page_raw - [Intern] raw page write function > + * @mtd: Pointer to the mtd info structure > + * @chip: Pointer to the NAND chip info structure > + * @buf: Pointer to the data buffer > + * @oob_required: Caller requires OOB data read to chip->oob_poi > + * @page: Page number to write > + * > + * Return: Always return zero > + */ > +static int pl353_nand_write_page_raw(struct mtd_info *mtd, > + struct nand_chip *chip, > + const u8 *buf, int oob_required, > + int page) > +{ > + unsigned long data_phase_addr; > + u8 *p; > + > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + unsigned long nand_offset = (unsigned long __force)xnfc->nand_base; > + > + pl353_nand_write_data_op(mtd, buf, mtd->writesize); > + p = chip->oob_poi; > + pl353_nand_write_data_op(mtd, p, > + (mtd->oobsize - > + PL353_NAND_LAST_TRANSFER_LENGTH)); > + p += (mtd->oobsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_CLEAR_CS; > + data_phase_addr |= (1 << END_CMD_VALID_SHIFT); > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + pl353_nand_write_data_op(mtd, p, PL353_NAND_LAST_TRANSFER_LENGTH); > + > + return 0; > +} > + > +/** > + * nand_write_page_hwecc - Hardware ECC based page write function > + * @mtd: Pointer to the mtd info structure > + * @chip: Pointer to the NAND chip info structure > + * @buf: Pointer to the data buffer > + * @oob_required: Caller requires OOB data read to chip->oob_poi > + * @page: Page number to write > + * > + * This functions writes data and hardware generated ECC values in to the page. > + * > + * Return: Always return zero > + */ > +static int pl353_nand_write_page_hwecc(struct mtd_info *mtd, > + struct nand_chip *chip, > + const u8 *buf, int oob_required, > + int page) > +{ > + int eccsize = chip->ecc.size; > + int eccsteps = chip->ecc.steps; > + u8 *ecc_calc = chip->ecc.calc_buf; > + u8 *oob_ptr; > + const u8 *p = buf; > + u32 ret; > + unsigned long data_phase_addr; > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + unsigned long nand_offset = (unsigned long __force)xnfc->nand_base; > + > + pl353_prepare_cmd(mtd, chip, page, 0, NAND_CMD_SEQIN, > + NAND_CMD_PAGEPROG, 0); > + > + for ( ; (eccsteps - 1); eccsteps--) { > + pl353_nand_write_data_op(mtd, p, eccsize); > + p += eccsize; > + } > + pl353_nand_write_data_op(mtd, p, > + (eccsize - PL353_NAND_LAST_TRANSFER_LENGTH)); > + p += (eccsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + > + /* Set ECC Last bit to 1 */ > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_ECC_LAST; > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + pl353_nand_write_data_op(mtd, p, PL353_NAND_LAST_TRANSFER_LENGTH); > + > + p = buf; > + chip->ecc.calculate(mtd, p, &ecc_calc[0]); You should check the returned value? > + > + /* Wait for ECC to be calculated and read the error values */ > + ret = mtd_ooblayout_set_eccbytes(mtd, ecc_calc, chip->oob_poi, > + 0, chip->ecc.total); > + if (ret) > + return ret; > + /* Clear ECC last bit */ > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr &= ~PL353_NAND_ECC_LAST; > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + > + /* Write the spare area with ECC bytes */ > + oob_ptr = chip->oob_poi; > + pl353_nand_write_data_op(mtd, oob_ptr, > + (mtd->oobsize - > + PL353_NAND_LAST_TRANSFER_LENGTH)); > + > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_CLEAR_CS; > + data_phase_addr |= (1 << END_CMD_VALID_SHIFT); > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + oob_ptr += (mtd->oobsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + pl353_nand_write_data_op(mtd, oob_ptr, PL353_NAND_LAST_TRANSFER_LENGTH); > + nand_wait_ready(mtd); > + > + return 0; > +} > + > +/** > + * pl353_nand_read_page_hwecc - Hardware ECC based page read function > + * @mtd: Pointer to the mtd info structure > + * @chip: Pointer to the NAND chip info structure > + * @buf: Pointer to the buffer to store read data > + * @oob_required: Caller requires OOB data read to chip->oob_poi > + * @page: Page number to read > + * > + * This functions reads data and checks the data integrity by comparing > + * hardware generated ECC values and read ECC values from spare area. > + * There is a limitation in SMC controller, that we must set ECC LAST on > + * last data phase access, to tell ECC block not to expect any data further. > + * Ex: When number of ECC STEPS are 4, then till 3 we will write to flash > + * using SMC with HW ECC enabled. And for the last ECC STEP, we will subtract > + * 4bytes from page size, and will initiate a transfer. And the remaining 4 as > + * one more transfer with ECC_LAST bit set in NAND data phase register to > + * notify ECC block not to expect any more data. The last block should be align > + * with end of 512 byte block. Because of this limitation, we are not using > + * core routines. > + * > + * Return: 0 always and updates ECC operation status in to MTD structure > + */ > +static int pl353_nand_read_page_hwecc(struct mtd_info *mtd, > + struct nand_chip *chip, > + u8 *buf, int oob_required, int page) > +{ > + int i, stat, eccsize = chip->ecc.size; > + int eccbytes = chip->ecc.bytes; > + int eccsteps = chip->ecc.steps; > + u8 *p = buf; > + u8 *ecc_calc = chip->ecc.calc_buf; > + u8 *ecc = chip->ecc.code_buf; > + unsigned int max_bitflips = 0; > + u8 *oob_ptr; > + u32 ret; > + unsigned long data_phase_addr; > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + unsigned long nand_offset = (unsigned long __force)xnfc->nand_base; > + > + pl353_prepare_cmd(mtd, chip, page, 0, NAND_CMD_READ0, > + NAND_CMD_READSTART, 1); > + nand_wait_ready(mtd); > + > + for ( ; (eccsteps - 1); eccsteps--) { > + pl353_nand_read_data_op(chip, p, eccsize); > + p += eccsize; > + } > + pl353_nand_read_data_op(chip, p, > + (eccsize - PL353_NAND_LAST_TRANSFER_LENGTH)); > + p += (eccsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + > + /* Set ECC Last bit to 1 */ > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_ECC_LAST; > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + pl353_nand_read_data_op(chip, p, PL353_NAND_LAST_TRANSFER_LENGTH); > + > + /* Read the calculated ECC value */ > + p = buf; > + chip->ecc.calculate(mtd, p, &ecc_calc[0]); > + You should check the returned value? > + /* Clear ECC last bit */ > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr &= ~PL353_NAND_ECC_LAST; > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + > + /* Read the stored ECC value */ > + oob_ptr = chip->oob_poi; > + pl353_nand_read_data_op(chip, oob_ptr, > + (mtd->oobsize - > + PL353_NAND_LAST_TRANSFER_LENGTH)); > + > + /* de-assert chip select */ > + data_phase_addr = (unsigned long __force)xnfc->nandaddr; > + data_phase_addr -= nand_offset; > + data_phase_addr |= PL353_NAND_CLEAR_CS; > + data_phase_addr += nand_offset; > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + > + oob_ptr += (mtd->oobsize - PL353_NAND_LAST_TRANSFER_LENGTH); > + pl353_nand_read_data_op(chip, oob_ptr, PL353_NAND_LAST_TRANSFER_LENGTH); > + > + ret = mtd_ooblayout_get_eccbytes(mtd, ecc, chip->oob_poi, 0, > + chip->ecc.total); > + if (ret) > + return ret; > + > + eccsteps = chip->ecc.steps; > + p = buf; > + > + /* Check ECC error for all blocks and correct if it is correctable */ > + for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { > + stat = chip->ecc.correct(mtd, p, &ecc[i], &ecc_calc[i]); > + if (stat < 0) { > + mtd->ecc_stats.failed++; > + } else { > + mtd->ecc_stats.corrected += stat; > + max_bitflips = max_t(unsigned int, max_bitflips, stat); > + } > + } > + > + return max_bitflips; > +} > + > +/** > + * pl353_nand_select_chip - Select the flash device > + * @mtd: Pointer to the mtd info structure > + * @chip: Pointer to the NAND chip info structure > + * > + * This function is empty as the NAND controller handles chip select line > + * internally based on the chip address passed in command and data phase. > + */ > +static void pl353_nand_select_chip(struct mtd_info *mtd, int chip) > +{ > +} > + > +/* NAND framework ->exec_op() hooks and related helpers */ > +static void pl353_nfc_parse_instructions(struct nand_chip *chip, > + const struct nand_subop *subop, > + struct pl353_nfc_op *nfc_op) > +{ > + const struct nand_op_instr *instr = NULL; > + unsigned int op_id, offset, naddrs; > + int i, len; > + const u8 *addrs; > + > + memset(nfc_op, 0, sizeof(struct pl353_nfc_op)); > + for (op_id = 0; op_id < subop->ninstrs; op_id++) { > + nfc_op->len = nand_subop_get_data_len(subop, op_id); > + len = nand_subop_get_data_len(subop, op_id); > + instr = &subop->instrs[op_id]; > + > + switch (instr->type) { > + case NAND_OP_CMD_INSTR: > + nfc_op->type = NAND_OP_CMD_INSTR; > + if (op_id) > + nfc_op->cmnds[1] = instr->ctx.cmd.opcode; > + else > + nfc_op->cmnds[0] = instr->ctx.cmd.opcode; > + nfc_op->cle_ale_delay_ns = instr->delay_ns; > + break; > + > + case NAND_OP_ADDR_INSTR: > + offset = nand_subop_get_addr_start_off(subop, op_id); > + naddrs = nand_subop_get_num_addr_cyc(subop, op_id); > + addrs = &instr->ctx.addr.addrs[offset]; > + nfc_op->addrs = instr->ctx.addr.addrs[offset]; > + for (i = 0; i < min_t(unsigned int, 4, naddrs); i++) { > + nfc_op->addrs |= instr->ctx.addr.addrs[i] << > + (8 * i); > + } > + > + if (naddrs >= 5) > + nfc_op->addr5 = addrs[4]; > + if (naddrs >= 6) > + nfc_op->addr6 = addrs[5]; > + nfc_op->naddrs = nand_subop_get_num_addr_cyc(subop, > + op_id); > + nfc_op->cle_ale_delay_ns = instr->delay_ns; > + break; > + > + case NAND_OP_DATA_IN_INSTR: > + nfc_op->data_instr = instr; > + nfc_op->type = NAND_OP_DATA_IN_INSTR; > + nfc_op->data_instr_idx = op_id; > + break; > + > + case NAND_OP_DATA_OUT_INSTR: > + nfc_op->data_instr = instr; > + nfc_op->type = NAND_OP_DATA_IN_INSTR; > + nfc_op->data_instr_idx = op_id; > + break; > + > + case NAND_OP_WAITRDY_INSTR: > + nfc_op->rdy_timeout_ms = instr->ctx.waitrdy.timeout_ms; > + nfc_op->rdy_delay_ns = instr->delay_ns; > + break; > + } > + } > +} > + > +static void cond_delay(unsigned int ns) > +{ > + if (!ns) > + return; > + > + if (ns < 10000) > + ndelay(ns); > + else > + udelay(DIV_ROUND_UP(ns, 1000)); > +} > + > +/** > + * pl353_nand_cmd_function - Send command to NAND device > + * @chip: Pointer to the NAND chip info structure > + * @subop: Pointer to array of instructions > + * Return: Always return zero > + */ > +static int pl353_nand_cmd_function(struct nand_chip *chip, > + const struct nand_subop *subop) Please don't use this name, too close from the cmdfunc() hook we are trying to remove. > +{ > + struct mtd_info *mtd = nand_to_mtd(chip); > + const struct nand_op_instr *instr; > + struct pl353_nfc_op nfc_op = {}; > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + unsigned long cmd_data = 0, end_cmd_valid = 0; > + unsigned long cmd_phase_addr, data_phase_addr, end_cmd; > + unsigned int op_id, len, offset; > + bool reading; > + > + pl353_nfc_parse_instructions(chip, subop, &nfc_op); > + instr = nfc_op.data_instr; > + op_id = nfc_op.data_instr_idx; > + len = nand_subop_get_data_len(subop, op_id); > + offset = nand_subop_get_data_start_off(subop, op_id); > + > + pl353_smc_clr_nand_int(); > + /* Get the command phase address */ > + if (nfc_op.cmnds[1] != 0) { > + if (nfc_op.cmnds[0] == NAND_CMD_SEQIN) > + end_cmd_valid = 0; > + else > + end_cmd_valid = 1; > + end_cmd = nfc_op.cmnds[1]; > + } else { > + end_cmd = 0x0; > + } > + > + /* > + * The SMC defines two phases of commands when transferring data to or > + * from NAND flash. > + * Command phase: Commands and optional address information are written > + * to the NAND flash.The command and address can be associated with > + * either a data phase operation to write to or read from the array, > + * or a status/ID register transfer. > + * Data phase: Data is either written to or read from the NAND flash. > + * This data can be either data transferred to or from the array, > + * or status/ID register information. > + */ > + cmd_phase_addr = (unsigned long __force)xnfc->nand_base + > + ((nfc_op.naddrs << ADDR_CYCLES_SHIFT) | > + (end_cmd_valid << END_CMD_VALID_SHIFT) | > + (COMMAND_PHASE) | > + (end_cmd << END_CMD_SHIFT) | > + (nfc_op.cmnds[0] << START_CMD_SHIFT)); > + > + /* Get the data phase address */ > + end_cmd_valid = 0; > + > + data_phase_addr = (unsigned long __force)xnfc->nand_base + > + ((0x0 << CLEAR_CS_SHIFT) | > + (end_cmd_valid << END_CMD_VALID_SHIFT) | > + (DATA_PHASE) | > + (end_cmd << END_CMD_SHIFT) | > + (0x0 << ECC_LAST_SHIFT)); > + xnfc->nandaddr = (void __iomem * __force)data_phase_addr; > + > + /* Command phase AXI Read & Write */ > + if (nfc_op.naddrs >= 5) { > + if (mtd->writesize > PL353_NAND_ECC_SIZE) { > + cmd_data = nfc_op.addrs; > + /* Another address cycle for devices > 128MiB */ > + if (chip->options & NAND_ROW_ADDR_3) { > + writel_relaxed(cmd_data, > + (void __iomem * __force) > + cmd_phase_addr); > + cmd_data = nfc_op.addr5; > + if (nfc_op.naddrs >= 6) > + cmd_data |= (nfc_op.addr6 << 8); > + } > + } > + } else { > + if (nfc_op.addrs != -1) { > + int column = nfc_op.addrs; > + /* > + * Change read/write column, read id etc > + * Adjust columns for 16 bit bus width > + */ > + if ((chip->options & NAND_BUSWIDTH_16) && > + (nfc_op.cmnds[0] == NAND_CMD_READ0 || > + nfc_op.cmnds[0] == NAND_CMD_SEQIN || > + nfc_op.cmnds[0] == NAND_CMD_RNDOUT || > + nfc_op.cmnds[0] == NAND_CMD_RNDIN)) { > + column >>= 1; > + } > + cmd_data = column; > + } > + } > + writel_relaxed(cmd_data, (void __iomem * __force)cmd_phase_addr); > + > + if (!nfc_op.data_instr) { > + if (nfc_op.rdy_timeout_ms) > + nand_wait_ready(mtd); > + return 0; > + } > + > + reading = (nfc_op.data_instr->type == NAND_OP_DATA_IN_INSTR); > + if (!reading) { > + if (nfc_op.cmnds[0] == NAND_CMD_SEQIN && > + nfc_op.cmnds[1] == NAND_CMD_PAGEPROG) { > + pl353_nand_write_page_raw(mtd, chip, > + instr->ctx.data.buf.out, 0, > + nfc_op.addrs); > + > + } else { > + pl353_nand_write_data_op(mtd, instr->ctx.data.buf.out, > + len); > + } > + if (nfc_op.rdy_timeout_ms) > + nand_wait_ready(mtd); > + cond_delay(nfc_op.rdy_delay_ns); > + } > + if (reading) { > + cond_delay(nfc_op.rdy_delay_ns); > + if (nfc_op.rdy_timeout_ms) > + nand_wait_ready(mtd); > + pl353_nand_read_data_op(chip, instr->ctx.data.buf.in, len); > + } > + > + return 0; > +} > + > +static const struct nand_op_parser pl353_nfc_op_parser = NAND_OP_PARSER > + (NAND_OP_PARSER_PATTERN > + (pl353_nand_cmd_function, > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 7), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true), > + NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 2048)), > + NAND_OP_PARSER_PATTERN > + (pl353_nand_cmd_function, > + NAND_OP_PARSER_PAT_CMD_ELEM(false), > + NAND_OP_PARSER_PAT_ADDR_ELEM(false, 7), > + NAND_OP_PARSER_PAT_CMD_ELEM(false), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(false), > + NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 2048)), > + NAND_OP_PARSER_PATTERN > + (pl353_nand_cmd_function, > + NAND_OP_PARSER_PAT_CMD_ELEM(false), > + NAND_OP_PARSER_PAT_ADDR_ELEM(true, 7), > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)), > + NAND_OP_PARSER_PATTERN > + (pl353_nand_cmd_function, > + NAND_OP_PARSER_PAT_CMD_ELEM(false), > + NAND_OP_PARSER_PAT_ADDR_ELEM(false, 8), > + NAND_OP_PARSER_PAT_DATA_OUT_ELEM(false, 2048), > + NAND_OP_PARSER_PAT_CMD_ELEM(true), > + NAND_OP_PARSER_PAT_WAITRDY_ELEM(true)), > + NAND_OP_PARSER_PATTERN > + (pl353_nand_cmd_function, > + NAND_OP_PARSER_PAT_CMD_ELEM(false)), > + ); > + > +static int pl353_nfc_exec_op(struct nand_chip *chip, > + const struct nand_operation *op, > + bool check_only) > +{ > + return nand_op_parser_exec_op(chip, &pl353_nfc_op_parser, > + op, check_only); > +} > + > +/** > + * pl353_nand_device_ready - Check device ready/busy line > + * @mtd: Pointer to the mtd_info structure > + * > + * Return: 0 on busy or 1 on ready state > + */ > +static int pl353_nand_device_ready(struct mtd_info *mtd > +{ > + if (pl353_smc_get_nand_int_status_raw()) { > + pl353_smc_clr_nand_int(); > + return 1; > + } > + > + return 0; > +} > + > +/** > + * pl353_nand_ecc_init - Initialize the ecc information as per the ecc mode > + * @mtd: Pointer to the mtd_info structure > + * @ecc: Pointer to ECC control structure > + * @ecc_mode: ondie ecc status > + * > + * This function initializes the ecc block and functional pointers as per the > + * ecc mode > + * > + * Return: 0 on success or negative errno. > + */ > +static int pl353_nand_ecc_init(struct mtd_info *mtd, struct nand_ecc_ctrl *ecc, > + int ecc_mode) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + int err = 0; > + > + if (ecc_mode == NAND_ECC_ON_DIE) { > + pl353_smc_set_ecc_mode(PL353_SMC_ECCMODE_BYPASS); > + /* > + * On-Die ECC spare bytes offset 8 is used for ECC codes > + * Use the BBT pattern descriptors > + */ > + chip->bbt_td = &bbt_main_descr; > + chip->bbt_md = &bbt_mirror_descr; > + bitmap_set(chip->parameters.get_feature_list, > + ONFI_FEATURE_ON_DIE_ECC, ONFI_FEATURE_ON_DIE_ECC_EN); > + bitmap_set(chip->parameters.set_feature_list, > + ONFI_FEATURE_ON_DIE_ECC, ONFI_FEATURE_ON_DIE_ECC_EN); > + } else { > + ecc->read_oob = pl353_nand_read_oob; > + ecc->write_oob = pl353_nand_write_oob; > + > + ecc->mode = NAND_ECC_HW; > + /* Hardware ECC generates 3 bytes ECC code for each 512 bytes */ > + ecc->bytes = 3; > + ecc->strength = 1; > + ecc->calculate = pl353_nand_calculate_hwecc; > + ecc->correct = pl353_nand_correct_data; > + ecc->read_page = pl353_nand_read_page_hwecc; > + ecc->size = PL353_NAND_ECC_SIZE; > + ecc->write_page = pl353_nand_write_page_hwecc; > + pl353_smc_set_ecc_pg_size(mtd->writesize); > + switch (mtd->writesize) { > + case SZ_512: > + case SZ_1K: > + case SZ_2K: > + pl353_smc_set_ecc_mode(PL353_SMC_ECCMODE_APB); > + break; > + default: > + ecc->calculate = nand_calculate_ecc; > + ecc->correct = nand_correct_data; > + ecc->size = 256; > + break; > + } > + if (mtd->writesize <= SZ_512) > + xnfc->addr_cycles = 1; > + else > + xnfc->addr_cycles = 2; > + > + if (chip->options & NAND_ROW_ADDR_3) > + xnfc->addr_cycles += 3; > + else > + xnfc->addr_cycles += 2; > + > + if (mtd->oobsize == 16) { > + mtd_set_ooblayout(mtd, &pl353_ecc_ooblayout16_ops); > + } else if (mtd->oobsize == 64) { > + mtd_set_ooblayout(mtd, &pl353_ecc_ooblayout64_ops); > + } else { > + err = -ENXIO; > + dev_err(xnfc->dev, "Unsupported oob Layout\n"); > + } > + } > + return err; > +} > + > +static int pl353_setup_data_interface(struct mtd_info *mtd, int csline, > + const struct nand_data_interface *conf) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct pl353_nand_info *xnfc = > + container_of(chip, struct pl353_nand_info, chip); > + const struct nand_sdr_timings *sdr; > + u32 timings[7], mckperiodps; > + > + sdr = nand_get_sdr_timings(conf); > + if (IS_ERR(sdr)) > + return PTR_ERR(sdr); > + > + /* > + * SDR timings are given in pico-seconds while NFC timings must be > + * expressed in NAND controller clock cycles. > + */ > + mckperiodps = NSEC_PER_SEC / clk_get_rate(xnfc->mclk); > + mckperiodps *= 1000; > + if (sdr->tRC_min <= 20000) > + /* > + * PL353 SMC needs one extra read cycle in SDR Mode 5 > + * This is not written anywhere in the datasheet but > + * the results observed during testing. Oh, that's bad. > + */ > + timings[0] = DIV_ROUND_UP(sdr->tRC_min, mckperiodps) + 1; > + else > + timings[0] = DIV_ROUND_UP(sdr->tRC_min, mckperiodps); > + > + timings[1] = DIV_ROUND_UP(sdr->tWC_min, mckperiodps); > + /* > + * For all SDR modes, PL353 SMC needs tREA max value as 1 > + * These results observed during testing. s/These results/Results/ > + */ > + timings[2] = PL353_TREA_MAX_VALUE; > + timings[3] = DIV_ROUND_UP(sdr->tWP_min, mckperiodps); > + timings[4] = DIV_ROUND_UP(sdr->tCLR_min, mckperiodps); > + timings[5] = DIV_ROUND_UP(sdr->tAR_min, mckperiodps); > + timings[6] = DIV_ROUND_UP(sdr->tRR_min, mckperiodps); > + > + if (csline == NAND_DATA_IFACE_CHECK_ONLY) > + return 0; You should move this to the top. > + > + pl353_smc_set_cycles(timings); > + > + return 0; > +} > +/** > + * pl353_nand_probe - Probe method for the NAND driver > + * @pdev: Pointer to the platform_device structure > + * > + * This function initializes the driver data structures and the hardware. > + * The NAND driver has dependency with the pl353_smc memory controller > + * driver for initializing the NAND timing parameters, bus width, ECC modes, > + * control and status information. > + * > + * Return: 0 on success or error value on failure > + */ > +static int pl353_nand_probe(struct platform_device *pdev) > +{ > + struct pl353_nand_info *xnfc; > + struct mtd_info *mtd; > + struct nand_chip *chip; > + struct resource *res; > + struct device_node *np; > + u32 ret; > + > + xnfc = devm_kzalloc(&pdev->dev, sizeof(*xnfc), GFP_KERNEL); > + if (!xnfc) > + return -ENOMEM; > + xnfc->dev = &pdev->dev; > + /* Map physical address of NAND flash */ > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + xnfc->nand_base = devm_ioremap_resource(xnfc->dev, res); > + if (IS_ERR(xnfc->nand_base)) > + return PTR_ERR(xnfc->nand_base); > + > + chip = &xnfc->chip; > + mtd = nand_to_mtd(chip); > + chip->exec_op = pl353_nfc_exec_op; > + nand_set_controller_data(chip, xnfc); > + mtd->priv = chip; > + mtd->owner = THIS_MODULE; > + if (!mtd->name) { > + /* > + * If the new bindings are used and the bootloader has not been > + * updated to pass a new mtdparts parameter on the cmdline, you > + * should define the following property in your NAND node, ie: > + * > + * label = "pl353-nand"; > + * > + * This way, mtd->name will be set by the core when > + * nand_set_flash_node() is called. > + */ > + mtd->name = devm_kasprintf(xnfc->dev, GFP_KERNEL, > + "%s", PL353_NAND_DRIVER_NAME); > + if (!mtd->name) { > + dev_err(xnfc->dev, "Failed to allocate mtd->name\n"); > + return -ENOMEM; > + } > + } > + nand_set_flash_node(chip, xnfc->dev->of_node); > + > + /* Set the driver entry points for MTD */ > + chip->dev_ready = pl353_nand_device_ready; > + chip->select_chip = pl353_nand_select_chip; > + /* If we don't set this delay driver sets 20us by default */ > + np = of_get_next_parent(xnfc->dev->of_node); > + xnfc->mclk = of_clk_get(np, 0); > + if (IS_ERR(xnfc->mclk)) { > + dev_err(xnfc->dev, "Failed to retrieve MCK clk\n"); > + return PTR_ERR(xnfc->mclk); > + } > + chip->chip_delay = 30; > + /* Set the device option and flash width */ > + chip->options = NAND_BUSWIDTH_AUTO; > + chip->bbt_options = NAND_BBT_USE_FLASH; > + platform_set_drvdata(pdev, xnfc); > + chip->setup_data_interface = pl353_setup_data_interface; > + /* first scan to find the device and get the page size */ > + if (nand_scan_ident(mtd, 1, NULL)) { > + dev_err(xnfc->dev, "nand_scan_ident for NAND failed\n"); > + return -ENXIO; > + } > + ret = pl353_nand_ecc_init(mtd, &chip->ecc, chip->ecc.mode); > + if (chip->options & NAND_BUSWIDTH_16) > + pl353_smc_set_buswidth(PL353_SMC_MEM_WIDTH_16); > + /* second phase scan */ > + if (nand_scan_tail(mtd)) { > + dev_err(xnfc->dev, "nand_scan_tail for NAND failed\n"); > + return -ENXIO; > + } > + I suggest you to wait for v4.19-rc1 to be out and then rebase on top of it as this cycle we changed a lot of things. Particularly, nand_scan_ident() and nand_scan_tail() are not exposed anymore, you should create nand_controller_ops with an ->attach_chip() hook that does everything between ident() and tail() and use nand_scan() instead. > + ret = mtd_device_register(mtd, NULL, 0); > + if (ret) { > + dev_err(xnfc->dev, "Failed to register mtd device: %d\n", ret); > + nand_cleanup(chip); > + return ret; > + } > + > + return 0; > +} > + > +/** > + * pl353_nand_remove - Remove method for the NAND driver > + * @pdev: Pointer to the platform_device structure > + * > + * This function is called if the driver module is being unloaded. It frees all > + * resources allocated to the device. > + * > + * Return: 0 on success or error value on failure > + */ > +static int pl353_nand_remove(struct platform_device *pdev) > +{ > + struct pl353_nand_info *xnfc = platform_get_drvdata(pdev); > + struct mtd_info *mtd = nand_to_mtd(&xnfc->chip); > + > + /* Release resources, unregister device */ > + nand_release(mtd); > + > + return 0; > +} > + > +/* Match table for device tree binding */ > +static const struct of_device_id pl353_nand_of_match[] = { > + { .compatible = "arm,pl353-nand-r2p1" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, pl353_nand_of_match); > + > +/* > + * pl353_nand_driver - This structure defines the NAND subsystem platform driver > + */ > +static struct platform_driver pl353_nand_driver = { > + .probe = pl353_nand_probe, > + .remove = pl353_nand_remove, > + .driver = { > + .name = PL353_NAND_DRIVER_NAME, > + .of_match_table = pl353_nand_of_match, > + }, > +}; > + > +module_platform_driver(pl353_nand_driver); > + > +MODULE_AUTHOR("Xilinx, Inc."); > +MODULE_ALIAS("platform:" PL353_NAND_DRIVER_NAME); > +MODULE_DESCRIPTION("ARM PL353 NAND Flash Driver"); > +MODULE_LICENSE("GPL"); Thanks, Miqu?l