Tango NAND Flash controller driver (and dependencies)
Subsystems (2)
v6 cover Tango NAND Flash controller driver (and dependencies)
This patch series imports a driver for the Tango NAND Flash controller and a few dependent patches, included for easy reference.
Boris Brezillon (2):
mtd: nand: Add a few more timings to nand_sdr_timings
mtd: nand: Wait tCCS after a column change
Marc Gonzalez (2):
mtd: nand: Actually apply the optimal timings
mtd: nand: tango: import driver for tango chips
drivers/mtd/nand/Kconfig | 6 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/nand_base.c | 28 +- drivers/mtd/nand/nand_timings.c | 26 +- drivers/mtd/nand/tango_nand.c | 654 ++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/nand.h | 18 ++ 6 files changed, 730 insertions(+), 3 deletions(-) create mode 100644 drivers/mtd/nand/tango_nand.c
PATCH v6 1/4 mtd: nand: Add a few more timings to nand_sdr_timings
From: Boris Brezillon <[email protected]>
Add the tR_max, tBERS_max, tPROG_max and tCCS_min timings to the nand_sdr_timings struct.
Assign default/safe values for the statically defined timings, and extract them from the ONFI parameter table if the NAND is ONFI compliant.
--- drivers/mtd/nand/nand_timings.c | 26 +++++++++++++++++++++++++- include/linux/mtd/nand.h | 8 ++++++++ 2 files changed, 33 insertions(+), 1 deletion(-)
drivers/mtd/nand/nand_timings.c +25 -1
@@ -18,6 +18,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = { { .type = NAND_SDR_IFACE, .timings.sdr = { + .tCCS_min = 500000, + .tR_max = 200000000, .tADL_min = 400000, .tALH_min = 20000, .tALS_min = 50000,
@@ -58,6 +60,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = { { .type = NAND_SDR_IFACE, .timings.sdr = { + .tCCS_min = 500000, + .tR_max = 200000000, .tADL_min = 400000, .tALH_min = 10000, .tALS_min = 25000,
@@ -98,6 +102,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = { { .type = NAND_SDR_IFACE, .timings.sdr = { + .tCCS_min = 500000, + .tR_max = 200000000, .tADL_min = 400000, .tALH_min = 10000, .tALS_min = 15000,
@@ -138,6 +144,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = { { .type = NAND_SDR_IFACE, .timings.sdr = { + .tCCS_min = 500000, + .tR_max = 200000000, .tADL_min = 400000, .tALH_min = 5000, .tALS_min = 10000,
@@ -178,6 +186,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = { { .type = NAND_SDR_IFACE, .timings.sdr = { + .tCCS_min = 500000, + .tR_max = 200000000, .tADL_min = 400000, .tALH_min = 5000, .tALS_min = 10000,
@@ -218,6 +228,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = { { .type = NAND_SDR_IFACE, .timings.sdr = { + .tCCS_min = 500000, + .tR_max = 200000000, .tADL_min = 400000, .tALH_min = 5000, .tALS_min = 10000,
@@ -290,10 +302,22 @@ int onfi_init_data_interface(struct nand_chip *chip, *iface = onfi_sdr_timings[timing_mode];/* -* TODO: initialize timings that cannot be deduced from timing mode: +* Initialize timings that cannot be deduced from timing mode: * tR, tPROG, tCCS, ... * These information are part of the ONFI parameter page. */ +if (chip->onfi_version) { +struct nand_onfi_params *params = &chip->onfi_params; +struct nand_sdr_timings *timings = &iface->timings.sdr; + +/* microseconds -> picoseconds */ + timings->tPROG_max =1000000UL *le16_to_cpu (params->t_prog); + timings->tBERS_max =1000000UL *le16_to_cpu (params->t_bers); + timings->tR_max =1000000UL *le16_to_cpu (params->t_r); + +/* nanoseconds -> picoseconds */ + timings->tCCS_min =1000UL *le16_to_cpu (params->t_ccs); + }return 0 ; }
include/linux/mtd/nand.h +8 -0
@@ -584,6 +584,10 @@ struct nand_buffers {* * All these timings are expressed in picoseconds. * +* @tBERS_max: Block erase time +* @tCCS_min: Change column setup time +* @tPROG_max: Page program time +* @tR_max: Page read time * @tALH_min: ALE hold time * @tADL_min: ALE to data loading time * @tALS_min: ALE setup time
@@ -621,6 +625,10 @@ struct nand_buffers {* @tWW_min: WP# transition to WE# low */ struct nand_sdr_timings { +u32 tBERS_max; +u32 tCCS_min; +u32 tPROG_max; +u32 tR_max;u32 tALH_min;u32 tADL_min;u32 tALS_min;
Signature · 1 line
2.9.0
PATCH v6 2/4 mtd: nand: Wait tCCS after a column change
In reply to Marc GonzalezFrom: Boris Brezillon <[email protected]>
Drivers implementing ->cmd_ctrl() and relying on the default ->cmdfunc() implementation usually don't wait tCCS when a column change (RNDIN or RNDOUT) is requested.
Add an option flag to ask the core to do so (note that we keep this as an opt-in to avoid breaking existing implementations), and make use of the ->data_interface information is available (otherwise, wait 500ns).
--- drivers/mtd/nand/nand_base.c | 26 +++++++++++++++++++++++++- include/linux/mtd/nand.h | 10 ++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
drivers/mtd/nand/nand_base.c +25 -1
@@ -709,6 +709,25 @@ static void nand_command(struct mtd_info *mtd, unsigned int command,nand_wait_ready (mtd); } +static void nand_ccs_delay (struct nand_chip *chip) +{ +/* +* The controller already takes care of waiting for tCCS when the RNDIN +* or RNDOUT command is sent, return directly. +*/ +if (!(chip->options &NAND_WAIT_TCCS )) +return ; + +/* +* Wait tCCS_min if it is correctly defined, otherwise wait 500ns +* (which should be safe for all NANDs). +*/ +if (chip->data_interface && chip->data_interface->timings.sdr.tCCS_min) +ndelay (chip->data_interface->timings.sdr.tCCS_min /1000 ); +else +ndelay (500 ); +} +/** * nand_command_lp - [DEFAULT] Send command to NAND large page device * @mtd: MTD device structure
@@ -773,10 +792,13 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,case NAND_CMD_ERASE1 :case NAND_CMD_ERASE2 :case NAND_CMD_SEQIN : -case NAND_CMD_RNDIN :case NAND_CMD_STATUS :return ; +case NAND_CMD_RNDIN : +nand_ccs_delay (chip); +return ; +case NAND_CMD_RESET :if (chip->dev_ready)break ;
@@ -795,6 +817,8 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE); chip->cmd_ctrl (mtd,NAND_CMD_NONE ,NAND_NCE |NAND_CTRL_CHANGE ); + +nand_ccs_delay (chip);return ;case NAND_CMD_READ0 :
include/linux/mtd/nand.h +10 -0
@@ -210,6 +210,16 @@ enum nand_ecc_algo {*/ #define NAND_USE_BOUNCE_BUFFER 0x00100000 +/* +* In case your controller is implementing ->cmd_ctrl() and is relying on the +* default ->cmdfunc() implementation, you may want to let the core handle the +* tCCS delay which is required when a column change (RNDIN or RNDOUT) is +* requested. +* If your controller already takes care of this delay, you don't need to set +* this flag. +*/ +#define NAND_WAIT_TCCS 0x00200000 +/* Options set by nand scan */ /* Nand scan has allocated controller struct */ #define NAND_CONTROLLER_ALLOC 0x80000000
Signature · 1 line
2.9.0
PATCH v6 3/4 mtd: nand: Actually apply the optimal timings
In reply to Marc Gonzalez--- drivers/mtd/nand/nand_base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/nand/nand_base.c +1 -1
@@ -1108,7 +1108,7 @@ static int nand_init_data_interface(struct nand_chip *chip) } } -return 0 ; +return nand_setup_data_interface (chip); }static void nand_release_data_interface (struct nand_chip *chip)
Signature · 1 line
2.9.0
PATCH v6 4/4 mtd: nand: tango: import driver for tango chips
In reply to Marc GonzalezThis driver supports the NAND Flash controller embedded in recent Tango chips, such as SMP8758 and SMP8759.
--- drivers/mtd/nand/Kconfig | 6 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/tango_nand.c | 654 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 661 insertions(+) create mode 100644 drivers/mtd/nand/tango_nand.c
drivers/mtd/nand/Kconfig +6 -0
@@ -205,6 +205,12 @@ config MTD_NAND_S3C2410_CLKSTOP when the is NAND chip selected or released, but will save approximately 5mA of power when there is nothing happening. +config MTD_NAND_TANGO + tristate "NAND Flash support for Tango chips" + depends on ARCH_TANGO + help + Enables the NAND Flash controller on Tango chips. + config MTD_NAND_DISKONCHIP tristate "DiskOnChip 2000, Millennium and Millennium Plus (NAND reimplementation)" depends on HAS_IOMEM
drivers/mtd/nand/Makefile +1 -0
@@ -16,6 +16,7 @@ obj-$(CONFIG_MTD_NAND_DENALI_DT) += denali_dt.o obj-$(CONFIG_MTD_NAND_AU1550) += au1550nd.o obj-$(CONFIG_MTD_NAND_BF5XX) += bf5xx_nand.o obj-$(CONFIG_MTD_NAND_S3C2410) += s3c2410.o +obj-$(CONFIG_MTD_NAND_TANGO) += tango_nand.o obj-$(CONFIG_MTD_NAND_DAVINCI) += davinci_nand.o obj-$(CONFIG_MTD_NAND_DISKONCHIP) += diskonchip.o obj-$(CONFIG_MTD_NAND_DOCG4) += docg4.o
drivers/mtd/nand/tango_nand.c new file +654 -0
654 lines not on this page · raw message
Signature · 1 line
2.9.0
Re: [PATCH v6 4/4] mtd: nand: tango: import driver for tango chips
Hi Marc,
Quoted 2 lines · On Mon, 24 Oct 2016 14:55:16 +0200 Marc Gonzalez…
This driver supports the NAND Flash controller embedded in recent Tango chips, such as SMP8758 and SMP8759.
A few comments below (most of them are about coding style).
Quoted 143 lines
Signed-off-by Marc Gonzalez <[email protected]>--- drivers/mtd/nand/Kconfig | 6 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/tango_nand.c | 654 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 661 insertions(+) create mode 100644 drivers/mtd/nand/tango_nand.cdrivers/mtd/nand/Kconfig +6
-0@@ -205,6 +205,12 @@ config MTD_NAND_S3C2410_CLKSTOP when the is NAND chip selected or released, but will save approximately 5mA of power when there is nothing happening. +config MTD_NAND_TANGO + tristate "NAND Flash support for Tango chips" + depends on ARCH_TANGO + help + Enables the NAND Flash controller on Tango chips. + config MTD_NAND_DISKONCHIP tristate "DiskOnChip 2000, Millennium and Millennium Plus (NAND reimplementation)" depends on HAS_IOMEMdrivers/mtd/nand/Makefile +1
-0@@ -16,6 +16,7 @@ obj-$(CONFIG_MTD_NAND_DENALI_DT) += denali_dt.o obj-$(CONFIG_MTD_NAND_AU1550) += au1550nd.o obj-$(CONFIG_MTD_NAND_BF5XX) += bf5xx_nand.o obj-$(CONFIG_MTD_NAND_S3C2410) += s3c2410.o +obj-$(CONFIG_MTD_NAND_TANGO) += tango_nand.o obj-$(CONFIG_MTD_NAND_DAVINCI) += davinci_nand.o obj-$(CONFIG_MTD_NAND_DISKONCHIP) += diskonchip.o obj-$(CONFIG_MTD_NAND_DOCG4) += docg4.odrivers/mtd/nand/tango_nand.c new file +99
-0@@ -0,0 +1,654 @@ +#include <linux/io.h> +#include <linux/of.h> +#include <linux/clk.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/mtd/nand.h> +#include <linux/dmaengine.h> +#include <linux/dma-mapping.h> +#include <linux/platform_device.h> + +/* Offsets relative to chip->base */ +#define PBUS_CMD 0 +#define PBUS_ADDR 4 +#define PBUS_DATA 8 + +/* Offsets relative to reg_base */ +#define NFC_STATUS 0x00 +#define NFC_FLASH_CMD 0x04 +#define NFC_DEVICE_CFG 0x08 +#define NFC_TIMING1 0x0c +#define NFC_TIMING2 0x10 +#define NFC_XFER_CFG 0x14 +#define NFC_PKT_0_CFG 0x18 +#define NFC_PKT_N_CFG 0x1c +#define NFC_BB_CFG 0x20 +#define NFC_ADDR_PAGE 0x24 +#define NFC_ADDR_OFFSET 0x28 +#define NFC_XFER_STATUS 0x2c + +/* NFC_STATUS values */ +#define CMD_READY BIT(31) + +/* NFC_FLASH_CMD values */ +#define NFC_READ 1 +#define NFC_WRITE 2 + +/* NFC_XFER_STATUS values */ +#define PAGE_IS_EMPTY BIT(16) + +/* Offsets relative to mem_base */ +#define METADATA 0x000 +#define ERROR_REPORT 0x1c0 + +/* +* Error reports are split in two bytes: +* byte 0 for the first packet in the page (PKT_0) +* byte 1 for other packets in the page (PKT_N, for N > 0) +* ERR_COUNT_PKT_N is the max error count over all but the first packet. +*/ +#define DECODE_OK_PKT_0 (v) (v & BIT(7)) +#define DECODE_OK_PKT_N (v) (v & BIT(15)) +#define ERR_COUNT_PKT_0 (v) ((v >> 0) & 0x3f) +#define ERR_COUNT_PKT_N (v) ((v >> 8) & 0x3f) + +/* Offsets relative to pbus_base */ +#define PBUS_CS_CTRL 0x83c +#define PBUS_PAD_MODE 0x8f0 + +/* PBUS_CS_CTRL values */ +#define PBUS_IORDY BIT(31) + +/* +* PBUS_PAD_MODE values +* In raw mode, the driver communicates directly with the NAND chips. +* In NFC mode, the NAND Flash controller manages the communication. +* We use NFC mode for read and write; raw mode for everything else. +*/ +#define MODE_RAW 0 +#define MODE_NFC BIT(31) + +#define METADATA_SIZE 4 +#define BBM_SIZE 6 +#define FIELD_ORDER 15 + +#define MAX_CS 4 + +struct tango_nfc { +struct nand_hw_control hw; +void __iomem *reg_base; +void __iomem *mem_base; +void __iomem *pbus_base; +struct tango_chip *chips[MAX_CS ]; +struct dma_chan *chan; +int freq_kHz; +}; + +#define to_tango_nfc (ptr) container_of(ptr, struct tango_nfc, hw) + +struct tango_chip { +struct nand_chip nand_chip; +void __iomem *base; +u32 timing1; +u32 timing2; +u32 xfer_cfg; +u32 pkt_0_cfg; +u32 pkt_n_cfg; +u32 bb_cfg; +}; +
drivers/mtd/nand/tango_nand.c line 109
Quoted 10 lines
drivers/mtd/nand/tango_nand.c new file +10
-0+#define to_tango_chip (ptr) container_of(ptr, struct tango_chip, nand_chip) + +#define XFER_CFG (cs, page_count, steps, metadata_size) \ + ((cs) << 24 | (page_count) << 16 | (steps) << 8 | (metadata_size) << 0) + +#define PKT_CFG (size, strength) ((size) << 16 | (strength) << 0) + +#define BB_CFG (bb_offset, bb_size) ((bb_offset) << 16 | (bb_size) << 0) + +#define TIMING (t0, t1, t2, t3) (t0 << 24 | t1 << 16 | t2 << 8 | t3 << 0)
Wrap tX params with parenthesis to make sure the shift operation is applied to the correct value (for example, if t0 = a + b, then the resulting operation will be a + (b << 24) instead of (a + b) << 24 if you don't put those parenthesis).
Quoted 124 lines
drivers/mtd/nand/tango_nand.c new file +124
-0+ +static void tango_cmd_ctrl (struct mtd_info *mtd,int dat,unsigned int ctrl) +{ +struct tango_chip *tchip =to_tango_chip (mtd_to_nand (mtd)); + +if (ctrl &NAND_CLE ) +writeb_relaxed (dat, tchip->base +PBUS_CMD ); + +if (ctrl &NAND_ALE ) +writeb_relaxed (dat, tchip->base +PBUS_ADDR ); +} + +static int tango_dev_ready (struct mtd_info *mtd) +{ +struct nand_chip *chip =mtd_to_nand (mtd); +struct tango_nfc *nfc =to_tango_nfc (chip->controller); + +return readl_relaxed (nfc->pbus_base +PBUS_CS_CTRL ) &PBUS_IORDY ; +} + +static uint8_t tango_read_byte (struct mtd_info *mtd) +{ +struct tango_chip *tchip =to_tango_chip (mtd_to_nand (mtd)); + +return readb_relaxed (tchip->base +PBUS_DATA ); +} + +static void tango_read_buf (struct mtd_info *mtd,uint8_t *buf,int len) +{ +struct tango_chip *tchip =to_tango_chip (mtd_to_nand (mtd)); + +ioread8_rep (tchip->base +PBUS_DATA , buf, len); +} + +static void tango_write_buf (struct mtd_info *mtd,const uint8_t *buf,int len) +{ +struct tango_chip *tchip =to_tango_chip (mtd_to_nand (mtd)); + +iowrite8_rep (tchip->base +PBUS_DATA , buf, len); +} + +static void tango_select_chip (struct mtd_info *mtd,int idx) +{ +struct nand_chip *chip =mtd_to_nand (mtd); +struct tango_nfc *nfc =to_tango_nfc (chip->controller); +struct tango_chip *tchip =to_tango_chip (chip); + +if (idx <0 ) +return ;/* No "chip unselect" function */ + +writel_relaxed (tchip->timing1, nfc->reg_base +NFC_TIMING1 ); +writel_relaxed (tchip->timing2, nfc->reg_base +NFC_TIMING2 ); +writel_relaxed (tchip->xfer_cfg, nfc->reg_base +NFC_XFER_CFG ); +writel_relaxed (tchip->pkt_0_cfg, nfc->reg_base +NFC_PKT_0_CFG ); +writel_relaxed (tchip->pkt_n_cfg, nfc->reg_base +NFC_PKT_N_CFG ); +writel_relaxed (tchip->bb_cfg, nfc->reg_base +NFC_BB_CFG ); +} + +/* +* The controller does not check for bitflips in erased pages, +* therefore software must check instead. +*/ +static int check_erased_page (struct nand_chip *chip,u8 *buf) +{ +u8 *meta = chip->oob_poi +BBM_SIZE ; +u8 *ecc = chip->oob_poi +BBM_SIZE +METADATA_SIZE ; +const int ecc_size = chip->ecc.bytes; +const int pkt_size = chip->ecc.size; +int i, res, meta_len, bitflips =0 ; + +for (i =0 ; i < chip->ecc.steps; ++i) + { + meta_len = i ?0 :METADATA_SIZE ; + res =nand_check_erased_ecc_chunk (buf, pkt_size, ecc, ecc_size, + meta, meta_len, chip->ecc.strength); +if (res <0 ) +return res; + + bitflips =max (res, bitflips); + buf += pkt_size; + ecc += ecc_size; + } + +return bitflips; +} + +static int decode_error_report (struct tango_nfc *nfc) +{ +u32 status, res; + + status =readl_relaxed (nfc->reg_base +NFC_XFER_STATUS ); +if (status &PAGE_IS_EMPTY ) +return 0 ; + + res =readl_relaxed (nfc->mem_base +ERROR_REPORT ); + +if (DECODE_OK_PKT_0 (res) &&DECODE_OK_PKT_N (res)) +return max (ERR_COUNT_PKT_0 (res),ERR_COUNT_PKT_N (res)); + +return -EBADMSG ; +} + +static void tango_dma_callback (void *arg) +{ +complete (arg); +} + +static int do_dma (struct tango_nfc *nfc,int dir,int cmd, +const void *buf,int len,int page) +{ +struct dma_chan *chan = nfc->chan; +struct dma_async_tx_descriptor *desc; +struct scatterlist sg; +struct completion tx_done; +int err = -EIO ; +u32 res, val; + +sg_init_one (&sg, buf, len); +if (dma_map_sg (chan->device->dev, &sg,1 , dir) !=1 ) +return -EIO ; + + desc =dmaengine_prep_slave_sg (chan, &sg,1 , dir,DMA_PREP_INTERRUPT ); +if (!desc) + goto dma_unmap;
drivers/mtd/nand/tango_nand.c line 243
Quoted 10 lines
drivers/mtd/nand/tango_nand.c new file +10
-0+ + desc->callback = tango_dma_callback; + desc->callback_param = &tx_done; +init_completion (&tx_done); + +writel_relaxed (MODE_NFC , nfc->pbus_base +PBUS_PAD_MODE ); + +writel_relaxed (page, nfc->reg_base +NFC_ADDR_PAGE ); +writel_relaxed (0 , nfc->reg_base +NFC_ADDR_OFFSET ); +writel_relaxed ( cmd, nfc->reg_base +NFC_FLASH_CMD );
A vestige of you weird alignment policy ;). Please fix that.
drivers/mtd/nand/tango_nand.c line 252
Quoted 9 lines
drivers/mtd/nand/tango_nand.c new file +9
-0+ +dmaengine_submit (desc); +dma_async_issue_pending (chan); + + res =wait_for_completion_timeout (&tx_done,HZ ); +if (res >0 ) { +void __iomem *addr = nfc->reg_base +NFC_STATUS ; + err =readl_poll_timeout (addr, val, val &CMD_READY ,0 ,1000 ); + }
I'd still prefer:
if (res > 0) err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
Quoted 17 lines
drivers/mtd/nand/tango_nand.c new file +17
-0+ +writel_relaxed (MODE_RAW , nfc->pbus_base +PBUS_PAD_MODE ); + +dma_unmap: +dma_unmap_sg (chan->device->dev, &sg,1 , dir); + +return err; +} + +static int tango_read_page (struct mtd_info *mtd,struct nand_chip *chip, +uint8_t *buf,int oob_required,int page) +{ +struct tango_nfc *nfc =to_tango_nfc (chip->controller); +int err, res, len = mtd->writesize; + +if (oob_required) + chip->ecc.read_oob (mtd, chip, page);
drivers/mtd/nand/tango_nand.c line 279
Quoted 10 lines
drivers/mtd/nand/tango_nand.c new file +10
-0+ + err =do_dma (nfc,DMA_FROM_DEVICE ,NFC_READ , buf, len, page); +if (err) +return err; + + res =decode_error_report (nfc); +if (res <0 ) { + chip->ecc.read_oob_raw (mtd, chip, page); + res =check_erased_page (chip, buf); + }
You should not return an error, when the ECC engine detected uncorrectable errors. You should just increment ecc->failed.
Quoted 7 lines
drivers/mtd/nand/tango_nand.c new file +7
-0+ +return res; +} + +static int tango_write_page (struct mtd_info *mtd,struct nand_chip *chip, +const uint8_t *buf,int oob_required,int page) +{
drivers/mtd/nand/tango_nand.c line 296
Quoted 10 lines
drivers/mtd/nand/tango_nand.c new file +10
-0+struct tango_nfc *nfc =to_tango_nfc (chip->controller); +int err, len = mtd->writesize; + +writel_relaxed (0xffffffff , nfc->mem_base +METADATA ); + err =do_dma (nfc,DMA_TO_DEVICE ,NFC_WRITE , buf, len, page); +if (err) +return err; + +if (oob_required) +return -ENOTSUPP ;/* Sending PAGEPROG twice is forbidden */
Please put the comment before the oob_required test. You should probably test that before lauching the DMA transfer.
Quoted 4 lines
drivers/mtd/nand/tango_nand.c new file +4
-0+ +return 0 ; +} +
drivers/mtd/nand/tango_nand.c line 310
Quoted 10 lines
drivers/mtd/nand/tango_nand.c new file +10
-0+static void aux_read (struct nand_chip *chip,u8 **buf,int len,int *pos) +{ + *pos += len; + +if (*buf ==NULL )/* skip over len bytes */ + chip->cmdfunc (&chip->mtd,NAND_CMD_RNDOUT , *pos,-1 ); +else { +tango_read_buf (&chip->mtd, *buf, len); + *buf += len; + }
if (*buf == NULL) {
/* skip over len bytes */
chip->cmdfunc(&chip->mtd, NAND_CMD_RNDOUT, *pos, -1);
} else {
tango_read_buf(&chip->mtd, *buf, len);
*buf += len;
}drivers/mtd/nand/tango_nand.c line 322
Quoted 12 lines
drivers/mtd/nand/tango_nand.c new file +12
-0+} + +static void aux_write (struct nand_chip *chip,const u8 **buf,int len,int *pos) +{ + *pos += len; + +if (*buf ==NULL )/* skip over len bytes */ + chip->cmdfunc (&chip->mtd,NAND_CMD_SEQIN , *pos,-1 ); +else { +tango_write_buf (&chip->mtd, *buf, len); + *buf += len; + }
Ditto.
Quoted 198 lines
drivers/mtd/nand/tango_nand.c new file +198
-0+} + +/* +* Physical page layout (not drawn to scale) +* +* NB: Bad Block Marker area splits PKT_N in two (N1, N2). +* +* +---+-----------------+-------+-----+-----------+-----+----+-------+ +* | M | PKT_0 | ECC_0 | ... | N1 | BBM | N2 | ECC_N | +* +---+-----------------+-------+-----+-----------+-----+----+-------+ +* +* Logical page layout: +* +* +-----+---+-------+-----+-------+ +* oob = | BBM | M | ECC_0 | ... | ECC_N | +* +-----+---+-------+-----+-------+ +* +* +-----------------+-----+-----------------+ +* buf = | PKT_0 | ... | PKT_N | +* +-----------------+-----+-----------------+ +*/ +static int raw_read (struct nand_chip *chip,u8 *buf,u8 *oob) +{ +u8 *oob_orig = oob; +const int page_size = chip->mtd.writesize; +const int ecc_size = chip->ecc.bytes; +const int pkt_size = chip->ecc.size; +int pos =0 ;/* position within physical page */ +int rem = page_size;/* bytes remaining until BBM area */ + +if (oob !=NULL ) + oob +=BBM_SIZE ; + +aux_read (chip, &oob,METADATA_SIZE , &pos); + +while (rem > pkt_size) { +aux_read (chip, &buf, pkt_size, &pos); +aux_read (chip, &oob, ecc_size, &pos); + rem = page_size - pos; + } + +aux_read (chip, &buf, rem, &pos); +aux_read (chip, &oob_orig,BBM_SIZE , &pos); +aux_read (chip, &buf, pkt_size - rem, &pos); +aux_read (chip, &oob, ecc_size, &pos); + +return 0 ; +} + +static int raw_write (struct nand_chip *chip,const u8 *buf,const u8 *oob) +{ +const u8 *oob_orig = oob; +const int page_size = chip->mtd.writesize; +const int ecc_size = chip->ecc.bytes; +const int pkt_size = chip->ecc.size; +int pos =0 ;/* position within physical page */ +int rem = page_size;/* bytes remaining until BBM area */ + +if (oob !=NULL ) + oob +=BBM_SIZE ; + +aux_write (chip, &oob,METADATA_SIZE , &pos); + +while (rem > pkt_size) { +aux_write (chip, &buf, pkt_size, &pos); +aux_write (chip, &oob, ecc_size, &pos); + rem = page_size - pos; + } + +aux_write (chip, &buf, rem, &pos); +aux_write (chip, &oob_orig,BBM_SIZE , &pos); +aux_write (chip, &buf, pkt_size - rem, &pos); +aux_write (chip, &oob, ecc_size, &pos); + +return 0 ; +} + +static int tango_read_page_raw (struct mtd_info *mtd,struct nand_chip *chip, +uint8_t *buf,int oob_required,int page) +{ +return raw_read (chip, buf, chip->oob_poi); +} + +static int tango_write_page_raw (struct mtd_info *mtd,struct nand_chip *chip, +const uint8_t *buf,int oob_required,int page) +{ +return raw_write (chip, buf, chip->oob_poi); +} + +static int tango_read_oob (struct mtd_info *mtd,struct nand_chip *chip,int page) +{ + chip->cmdfunc (mtd,NAND_CMD_READ0 ,0 , page); +return raw_read (chip,NULL , chip->oob_poi); +} + +static int tango_write_oob (struct mtd_info *mtd,struct nand_chip *chip,int page) +{ + chip->cmdfunc (mtd,NAND_CMD_SEQIN ,0 , page); +raw_write (chip,NULL , chip->oob_poi); + chip->cmdfunc (mtd,NAND_CMD_PAGEPROG ,-1 ,-1 ); + chip->waitfunc (mtd, chip); +return 0 ; +} + +static int oob_ecc (struct mtd_info *mtd,int idx,struct mtd_oob_region *res) +{ +struct nand_chip *chip =mtd_to_nand (mtd); +struct nand_ecc_ctrl *ecc = &chip->ecc; + +if (idx >= ecc->steps) +return -ERANGE ; + + res->offset =BBM_SIZE +METADATA_SIZE + ecc->bytes * idx; + res->length = ecc->bytes; + +return 0 ; +} + +static int oob_free (struct mtd_info *mtd,int idx,struct mtd_oob_region *res) +{ +return -ERANGE ;/* no free space in spare area */ +} + +static const struct mtd_ooblayout_ops tango_nand_ooblayout_ops = { + .ecc = oob_ecc, + .free = oob_free, +}; + +static u32 to_ticks (int kHz,int ps) +{ +return DIV_ROUND_UP_ULL ((u64 )kHz * ps,NSEC_PER_SEC ); +} + +static int tango_set_timings (struct mtd_info *mtd, +const struct nand_data_interface *conf,bool check_only) +{ +const struct nand_sdr_timings *sdr =nand_get_sdr_timings (conf); +struct nand_chip *chip =mtd_to_nand (mtd); +struct tango_nfc *nfc =to_tango_nfc (chip->controller); +struct tango_chip *tchip =to_tango_chip (chip); +u32 Trdy, Textw, Twc, Twpw, Tacc, Thold, Trpw, Textr; +int kHz = nfc->freq_kHz; + +if (IS_ERR (sdr)) +return PTR_ERR (sdr); + +if (check_only) +return 0 ; + + Trdy =to_ticks (kHz, sdr->tCEA_max - sdr->tREA_max); + Textw =to_ticks (kHz, sdr->tWB_max); + Twc =to_ticks (kHz, sdr->tWC_min); + Twpw =to_ticks (kHz, sdr->tWC_min - sdr->tWP_min); + + Tacc =to_ticks (kHz, sdr->tREA_max); + Thold =to_ticks (kHz, sdr->tREH_min); + Trpw =to_ticks (kHz, sdr->tRC_min - sdr->tREH_min); + Textr =to_ticks (kHz, sdr->tRHZ_max); + + tchip->timing1 =TIMING (Trdy, Textw, Twc, Twpw); + tchip->timing2 =TIMING (Tacc, Thold, Trpw, Textr); + +return 0 ; +} + +static int chip_init (struct device *dev,struct device_node *np) +{ +int err, res; +u32 cs, ecc_bits; +struct mtd_info *mtd; +struct nand_chip *chip; +struct tango_chip *tchip; +struct nand_ecc_ctrl *ecc; +struct tango_nfc *nfc =dev_get_drvdata (dev); + + tchip =devm_kzalloc (dev,sizeof (*tchip),GFP_KERNEL ); +if (!tchip) +return -ENOMEM ; + + res =of_property_count_u32_elems (np,"reg" ); +if (res <0 ) +return res; + +if (res !=1 ) +return -ENOTSUPP ;/* Multi-CS chips are not supported */ + + err =of_property_read_u32_index (np,"reg" ,0 , &cs); +if (err) +return err; + +if (cs >=MAX_CS ) +return -EINVAL ; + + chip = &tchip->nand_chip; + ecc = &chip->ecc; + mtd = &chip->mtd; + + chip->read_byte = tango_read_byte;
drivers/mtd/nand/tango_nand.c line 530
Quoted 10 lines
drivers/mtd/nand/tango_nand.c new file +10
-0+ chip->write_buf = tango_write_buf; + chip->read_buf = tango_read_buf; + chip->select_chip = tango_select_chip; + chip->cmd_ctrl = tango_cmd_ctrl; + chip->dev_ready = tango_dev_ready; + chip->setup_data_interface = tango_set_timings; + chip->options =0 + |NAND_USE_BOUNCE_BUFFER + |NAND_NO_SUBPAGE_WRITE + |NAND_WAIT_TCCS ;
chip->options = NAND_USE_BOUNCE_BUFFER | NAND_NO_SUBPAGE_WRITE | NAND_WAIT_TCCS;
drivers/mtd/nand/tango_nand.c line 539
Quoted 9 lines
drivers/mtd/nand/tango_nand.c new file +9
-0+ chip->controller = &nfc->hw; + tchip->base = nfc->pbus_base + (cs *256 ); + +nand_set_flash_node (chip, np); +mtd_set_ooblayout (mtd, &tango_nand_ooblayout_ops); + mtd->dev.parent = dev; + + ecc->mode =NAND_ECC_HW ; + ecc->algo =NAND_ECC_BCH ;
You only support HW ECC, so please put these two line after nand_scan_ident(). Otherwise, the DT parsing done in nand_base.c might override the ->mode and ->algo values.
Quoted 33 lines
drivers/mtd/nand/tango_nand.c new file +33
-0+ + err =nand_scan_ident (mtd,1 ,NULL ); +if (err) +return err; + + ecc->read_page_raw = tango_read_page_raw; + ecc->write_page_raw = tango_write_page_raw; + ecc->read_page = tango_read_page; + ecc->write_page = tango_write_page; + ecc->read_oob = tango_read_oob; + ecc->write_oob = tango_write_oob; + + ecc_bits = ecc->strength *FIELD_ORDER ; + ecc->bytes =DIV_ROUND_UP (ecc_bits,BITS_PER_BYTE ); + + err =nand_scan_tail (mtd); +if (err) +return err; + + tchip->xfer_cfg =XFER_CFG (cs,1 , ecc->steps,METADATA_SIZE ); + tchip->pkt_0_cfg =PKT_CFG (ecc->size +METADATA_SIZE , ecc->strength); + tchip->pkt_n_cfg =PKT_CFG (ecc->size, ecc->strength); + tchip->bb_cfg =BB_CFG (mtd->writesize,BBM_SIZE ); + + err =mtd_device_register (mtd,NULL ,0 ); +if (err) +return err; + + nfc->chips[cs] = tchip; + +return 0 ; +} +
drivers/mtd/nand/tango_nand.c line 582
Quoted 10 lines
drivers/mtd/nand/tango_nand.c new file +10
-0+static int tango_nand_remove (struct platform_device *pdev) +{ +int cs; +struct tango_nfc *nfc =platform_get_drvdata (pdev); + +dma_release_channel (nfc->chan); + +for (cs =0 ; cs <MAX_CS ; ++cs) +if (nfc->chips[cs] !=NULL ) +nand_release (&nfc->chips[cs]->nand_chip.mtd);
Please add curly braces around the for loop.
Quoted 72 lines
drivers/mtd/nand/tango_nand.c new file +72
-0+ +return 0 ; +} + +static int tango_nand_probe (struct platform_device *pdev) +{ +struct clk *clk; +struct resource *res; +struct tango_nfc *nfc; +struct device_node *np; + + nfc =devm_kzalloc (&pdev->dev,sizeof (*nfc),GFP_KERNEL ); +if (!nfc) +return -ENOMEM ; + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + nfc->reg_base =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (nfc->reg_base)) +return PTR_ERR (nfc->reg_base); + + res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); + nfc->mem_base =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (nfc->mem_base)) +return PTR_ERR (nfc->mem_base); + + res =platform_get_resource (pdev,IORESOURCE_MEM ,2 ); + nfc->pbus_base =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (nfc->pbus_base)) +return PTR_ERR (nfc->pbus_base); + + clk =clk_get (&pdev->dev,NULL ); +if (IS_ERR (clk)) +return PTR_ERR (clk); + + nfc->chan =dma_request_chan (&pdev->dev,"nfc_sbox" ); +if (IS_ERR (nfc->chan)) +return PTR_ERR (nfc->chan); + +platform_set_drvdata (pdev, nfc); +nand_hw_control_init (&nfc->hw); + nfc->freq_kHz =clk_get_rate (clk) /1000 ; + +for_each_child_of_node (pdev->dev.of_node, np) { +int err =chip_init (&pdev->dev, np); +if (err) { +tango_nand_remove (pdev); +return err; + } + } + +return 0 ; +} + +static const struct of_device_id tango_nand_ids[] = { + { .compatible ="sigma,smp8758-nand" }, + {/* sentinel */ } +}; + +static struct platform_driver tango_nand_driver = { + .probe = tango_nand_probe, + .remove = tango_nand_remove, + .driver = { + .name ="tango-nand" , + .of_match_table = tango_nand_ids, + }, +}; + +module_platform_driver (tango_nand_driver); + +MODULE_LICENSE ("GPL" ); +MODULE_AUTHOR ("Sigma Designs" ); +MODULE_DESCRIPTION ("Tango4 NAND Flash controller driver" );
Re: [PATCH v6 4/4] mtd: nand: tango: import driver for tango chips
Heya :-)
Quoted 6 lines · Boris Brezillon
Quoted 2 lines · Marc Gonzalez
This driver supports the NAND Flash controller embedded in recent Tango chips, such as SMP8758 and SMP8759.
A few comments below (most of them are about coding style).
20c25f430311 tweak chip->optionscd1f53e6d2b8 Move ECC init code aroundb90efb09a921 Increment ecc_stats.failed on error2236f67af8d6 move brace for checkpatch83c1c0311c2a braces in for loop13faec8fa36b proper braces for if-blockf879bf99d5c3 Move test before DMA writea2d319b18702 no brackets for readl_poll_timeout67acc45e26e1 Remove nice alignmentdd71ae037721 fix TIMING macro
Time to squash and send v7, along with the bindings doc.
Quoted 6 lines
Quoted 1 line
+#define
TIMING(t0, t1, t2, t3)(t0 << 24 | t1 << 16 | t2 << 8 | t3 << 0)Wrap tX params with parenthesis to make sure the shift operation is applied to the correct value (for example, if t0 = a + b, then the resulting operation will be a + (b << 24) instead of (a + b) << 24 if you don't put those parenthesis).
Fixed.
For the record, addition has higher precedence than shift. Thus a + b << n = (a + b) << n
But your point is valid for bit-wise operations, e.g. a & b << n = a & (b << n)
Quoted 5 lines
drivers/mtd/nand/tango_nand.c line 243
Quoted 3 lines
drivers/mtd/nand/tango_nand.c new file +3
-0+writel_relaxed (page, nfc->reg_base +NFC_ADDR_PAGE ); +writel_relaxed (0 , nfc->reg_base +NFC_ADDR_OFFSET ); +writel_relaxed ( cmd, nfc->reg_base +NFC_FLASH_CMD );A vestige of you weird alignment policy ;). Please fix that.
Fixed. Even checkpatch complains...
(Sad puppy face)
Regards.