2016-03-07 16:19:04

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 00/16] mtd: nand: sunxi: various improvements/fixes

Hi,

This patchset aims at fixing a few minor bugs, and improving performances
of NAND accesses going through the sunxi NAND controller.

Note that patch 5 exports functions provided by the core which are needed
in patch 6 to still support raw OOB accesses. Other patches are just
fixes or improvements only touching the sunxi driver itself.

Here are the main improvements:
- queue CLE/ALE requests instead of forcing the controller to issue each
cmd and address cycle separately
- benefit from ECC correction on protected OOB bytes
- use polling instead of interrupt-based waiting (avoid scheduling
overhead for short wait period)

I'm preparing another series based on this one to add page operations
and DMA support, which, according to my first tests will bring a huge
speed improvement (at least x2 on read accesses, and even more when
ONFI timing mode > 0 is used).

Stay tuned.

Best Regards,

Boris

Boris Brezillon (16):
mtd: nand: sunxi: fix call order in sunxi_nand_chip_init()
mtd: nand: sunxi: fix clk rate calculation
mtd: nand: sunxi: fix EDO mode selection
mtd: nand: sunxi: adapt clk_rate to tWB, tADL, tWHR and tRHW timings
mtd: nand: export default read/write oob functions
mtd: nand: sunxi: implement ->read_oob()/->write_oob()
mtd: nand: sunxi: implement ->read_subpage()
mtd: nand: sunxi: improve ->cmd_ctrl() function
mtd: nand: sunxi: let the NAND controller control the CE line
mtd: nand: sunxi: fix the NFC_ECC_ERR_CNT() macro
mtd: nand: sunxi: fix NFC_CTL setting
mtd: nand: sunxi: disable clks on device removal
mtd: nand: enable ECC pipelining
mtd: nand: sunxi: fix ->dev_ready() implementation
mtd: nand: sunxi: make use of readl_poll_timeout()
mtd: nand: sunxi: poll for events instead of using interrupts

drivers/mtd/nand/nand_base.c | 18 +--
drivers/mtd/nand/sunxi_nand.c | 293 ++++++++++++++++++++++++++++++------------
include/linux/mtd/nand.h | 14 ++
3 files changed, 233 insertions(+), 92 deletions(-)

--
2.1.4


2016-03-07 16:18:45

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 01/16] mtd: nand: sunxi: fix call order in sunxi_nand_chip_init()

sunxi_nand_chip_set_timings() is extracting a pointer to the nfc from the
nand->controller field, but this field is initialized after
sunxi_nand_chip_set_timings() call.
Reorder the calls to avoid any problem.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 1c03eee..4d01e65 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1536,21 +1536,6 @@ static int sunxi_nand_chip_init(struct device *dev, struct sunxi_nfc *nfc,
}
}

- timings = onfi_async_timing_mode_to_sdr_timings(0);
- if (IS_ERR(timings)) {
- ret = PTR_ERR(timings);
- dev_err(dev,
- "could not retrieve timings for ONFI mode 0: %d\n",
- ret);
- return ret;
- }
-
- ret = sunxi_nand_chip_set_timings(chip, timings);
- if (ret) {
- dev_err(dev, "could not configure chip timings: %d\n", ret);
- return ret;
- }
-
nand = &chip->nand;
/* Default tR value specified in the ONFI spec (chapter 4.15.1) */
nand->chip_delay = 200;
@@ -1570,6 +1555,21 @@ static int sunxi_nand_chip_init(struct device *dev, struct sunxi_nfc *nfc,
mtd = nand_to_mtd(nand);
mtd->dev.parent = dev;

+ timings = onfi_async_timing_mode_to_sdr_timings(0);
+ if (IS_ERR(timings)) {
+ ret = PTR_ERR(timings);
+ dev_err(dev,
+ "could not retrieve timings for ONFI mode 0: %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = sunxi_nand_chip_set_timings(chip, timings);
+ if (ret) {
+ dev_err(dev, "could not configure chip timings: %d\n", ret);
+ return ret;
+ }
+
ret = nand_scan_ident(mtd, nsels, NULL);
if (ret)
return ret;
--
2.1.4

2016-03-07 16:19:13

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 02/16] mtd: nand: sunxi: fix clk rate calculation

Unlike what is specified in the Allwinner datasheets, the NAND clock rate
is not equal to 2/T but 1/T. Fix the clock rate selection accordingly.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 4d01e65..ab66d8d 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1208,13 +1208,7 @@ static int sunxi_nand_chip_set_timings(struct sunxi_nand_chip *chip,
/* Convert min_clk_period from picoseconds to nanoseconds */
min_clk_period = DIV_ROUND_UP(min_clk_period, 1000);

- /*
- * Convert min_clk_period into a clk frequency, then get the
- * appropriate rate for the NAND controller IP given this formula
- * (specified in the datasheet):
- * nand clk_rate = 2 * min_clk_rate
- */
- chip->clk_rate = (2 * NSEC_PER_SEC) / min_clk_period;
+ chip->clk_rate = NSEC_PER_SEC / min_clk_period;

return 0;
}
--
2.1.4

2016-03-07 16:19:25

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 04/16] mtd: nand: sunxi: adapt clk_rate to tWB, tADL, tWHR and tRHW timings

Adapt the NAND controller clk rate to the tWB, tADL, tWHR and tRHW
timings instead of returning an error when the maximum clk divisor is
not big enough to provide an appropriate timing.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index a270720..2914785 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1164,6 +1164,18 @@ static int sunxi_nand_chip_set_timings(struct sunxi_nand_chip *chip,
min_clk_period = DIV_ROUND_UP(timings->tWC_min, 2);

/* T16 - T19 + tCAD */
+ if (timings->tWB_max > (min_clk_period * 20))
+ min_clk_period = DIV_ROUND_UP(timings->tWB_max, 20);
+
+ if (timings->tADL_min > (min_clk_period * 32))
+ min_clk_period = DIV_ROUND_UP(timings->tADL_min, 32);
+
+ if (timings->tWHR_min > (min_clk_period * 32))
+ min_clk_period = DIV_ROUND_UP(timings->tWHR_min, 32);
+
+ if (timings->tRHW_min > (min_clk_period * 20))
+ min_clk_period = DIV_ROUND_UP(timings->tRHW_min, 20);
+
tWB = sunxi_nand_lookup_timing(tWB_lut, timings->tWB_max,
min_clk_period);
if (tWB < 0) {
--
2.1.4

2016-03-07 16:19:33

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 06/16] mtd: nand: sunxi: implement ->read_oob()/->write_oob()

Allwinner's ECC engine is capable of protecting a few bytes of the OOB
area. Implement specific OOB functions to benefit from this capability.

Also, when in raw mode, the randomizer is disabled, which means you'll
only be able to retrieve randomized data, which is not really useful
for most applications.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 2914785..8977a5f 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1074,6 +1074,40 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
return 0;
}

+static int sunxi_nfc_hw_common_ecc_read_oob(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ int page)
+{
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+
+ chip->pagebuf = -1;
+
+ return chip->ecc.read_page(mtd, chip, chip->buffers->databuf, 1, page);
+}
+
+static int sunxi_nfc_hw_common_ecc_write_oob(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ int page)
+{
+ int ret, status;
+
+ chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+
+ chip->pagebuf = -1;
+
+ memset(chip->buffers->databuf, 0xff, mtd->writesize);
+ ret = chip->ecc.write_page(mtd, chip, chip->buffers->databuf, 1, page);
+ if (ret)
+ return ret;
+
+ /* Send command to program the OOB data */
+ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+
+ status = chip->waitfunc(mtd, chip);
+
+ return status & NAND_STATUS_FAIL ? -EIO : 0;
+}
+
static const s32 tWB_lut[] = {6, 12, 16, 20};
static const s32 tRHW_lut[] = {4, 8, 12, 20};

@@ -1315,6 +1349,8 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,

layout->eccbytes = (ecc->bytes * nsectors);

+ ecc->read_oob = sunxi_nfc_hw_common_ecc_read_oob;
+ ecc->write_oob = sunxi_nfc_hw_common_ecc_write_oob;
ecc->layout = layout;
ecc->priv = data;

@@ -1346,6 +1382,8 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,

ecc->read_page = sunxi_nfc_hw_ecc_read_page;
ecc->write_page = sunxi_nfc_hw_ecc_write_page;
+ ecc->read_oob_raw = nand_read_oob_std;
+ ecc->write_oob_raw = nand_write_oob_std;
layout = ecc->layout;
nsectors = mtd->writesize / ecc->size;

@@ -1400,6 +1438,8 @@ static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct mtd_info *mtd,
ecc->prepad = 4;
ecc->read_page = sunxi_nfc_hw_syndrome_ecc_read_page;
ecc->write_page = sunxi_nfc_hw_syndrome_ecc_write_page;
+ ecc->read_oob_raw = nand_read_oob_syndrome;
+ ecc->write_oob_raw = nand_write_oob_syndrome;

layout = ecc->layout;
nsectors = mtd->writesize / ecc->size;
--
2.1.4

2016-03-07 16:19:44

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 03/16] mtd: nand: sunxi: fix EDO mode selection

The ONFI spec says that EDO should be enabled if the host drives tRC less
than 30ns, but the code just tests for the tRC_min value extracted from
the timings exposed by the NAND chip not the timings actually configured
in the NAND controller.
Fix that by first rounding down the requested clk_rate with
clk_round_rate() and then checking if tRC is actually smaller than 30ns.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index ab66d8d..a270720 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1101,6 +1101,7 @@ static int sunxi_nand_chip_set_timings(struct sunxi_nand_chip *chip,
struct sunxi_nfc *nfc = to_sunxi_nfc(chip->nand.controller);
u32 min_clk_period = 0;
s32 tWB, tADL, tWHR, tRHW, tCAD;
+ long real_clk_rate;

/* T1 <=> tCLS */
if (timings->tCLS_min > min_clk_period)
@@ -1198,17 +1199,20 @@ static int sunxi_nand_chip_set_timings(struct sunxi_nand_chip *chip,
/* TODO: A83 has some more bits for CDQSS, CS, CLHZ, CCS, WC */
chip->timing_cfg = NFC_TIMING_CFG(tWB, tADL, tWHR, tRHW, tCAD);

+ /* Convert min_clk_period from picoseconds to nanoseconds */
+ min_clk_period = DIV_ROUND_UP(min_clk_period, 1000);
+
+ chip->clk_rate = NSEC_PER_SEC / min_clk_period;
+ real_clk_rate = clk_round_rate(nfc->mod_clk, chip->clk_rate);
+
/*
* ONFI specification 3.1, paragraph 4.15.2 dictates that EDO data
* output cycle timings shall be used if the host drives tRC less than
* 30 ns.
*/
- chip->timing_ctl = (timings->tRC_min < 30000) ? NFC_TIMING_CTL_EDO : 0;
-
- /* Convert min_clk_period from picoseconds to nanoseconds */
- min_clk_period = DIV_ROUND_UP(min_clk_period, 1000);
-
- chip->clk_rate = NSEC_PER_SEC / min_clk_period;
+ min_clk_period = NSEC_PER_SEC / real_clk_rate;
+ chip->timing_ctl = ((min_clk_period * 2) < 30) ?
+ NFC_TIMING_CTL_EDO : 0;

return 0;
}
--
2.1.4

2016-03-07 16:19:54

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 05/16] mtd: nand: export default read/write oob functions

Export the default read/write oob functions (for the standard and syndrome
scheme), so that drivers can use them for their raw implementation and
implement their own functions for the normal oob operation.

This is required if your ECC engine is capable of fixing some of the OOB
data. In this case you have to overload the ->read_oob() and ->write_oob(),
but if you don't specify the ->read/write_oob_raw() functions they are
assigned to the ->read/write_oob() implementation, which is not what you
want.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/nand_base.c | 18 ++++++++++--------
include/linux/mtd/nand.h | 14 ++++++++++++++
2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index f2c8ff3..b8cfeb7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1924,13 +1924,13 @@ static int nand_read(struct mtd_info *mtd, loff_t from, size_t len,
* @chip: nand chip info structure
* @page: page number to read
*/
-static int nand_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip,
- int page)
+int nand_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page)
{
chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
return 0;
}
+EXPORT_SYMBOL(nand_read_oob_std);

/**
* nand_read_oob_syndrome - [REPLACEABLE] OOB data read function for HW ECC
@@ -1939,8 +1939,8 @@ static int nand_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip,
* @chip: nand chip info structure
* @page: page number to read
*/
-static int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
- int page)
+int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
+ int page)
{
int length = mtd->oobsize;
int chunk = chip->ecc.bytes + chip->ecc.prepad + chip->ecc.postpad;
@@ -1968,6 +1968,7 @@ static int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,

return 0;
}
+EXPORT_SYMBOL(nand_read_oob_syndrome);

/**
* nand_write_oob_std - [REPLACEABLE] the most common OOB data write function
@@ -1975,8 +1976,7 @@ static int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
* @chip: nand chip info structure
* @page: page number to write
*/
-static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip,
- int page)
+int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page)
{
int status = 0;
const uint8_t *buf = chip->oob_poi;
@@ -1991,6 +1991,7 @@ static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip,

return status & NAND_STATUS_FAIL ? -EIO : 0;
}
+EXPORT_SYMBOL(nand_write_oob_std);

/**
* nand_write_oob_syndrome - [REPLACEABLE] OOB data write function for HW ECC
@@ -1999,8 +2000,8 @@ static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip,
* @chip: nand chip info structure
* @page: page number to write
*/
-static int nand_write_oob_syndrome(struct mtd_info *mtd,
- struct nand_chip *chip, int page)
+int nand_write_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
+ int page)
{
int chunk = chip->ecc.bytes + chip->ecc.prepad + chip->ecc.postpad;
int eccsize = chip->ecc.size, length = mtd->oobsize;
@@ -2050,6 +2051,7 @@ static int nand_write_oob_syndrome(struct mtd_info *mtd,

return status & NAND_STATUS_FAIL ? -EIO : 0;
}
+EXPORT_SYMBOL(nand_write_oob_syndrome);

/**
* nand_do_read_oob - [INTERN] NAND read out-of-band
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 7604f4b..19b2f7d 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -1070,4 +1070,18 @@ int nand_check_erased_ecc_chunk(void *data, int datalen,
void *ecc, int ecclen,
void *extraoob, int extraooblen,
int threshold);
+
+/* Default write_oob implementation */
+int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page);
+
+/* Default write_oob syndrome implementation */
+int nand_write_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
+ int page);
+
+/* Default read_oob implementation */
+int nand_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page);
+
+/* Default read_oob syndrome implementation */
+int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
+ int page);
#endif /* __LINUX_MTD_NAND_H */
--
2.1.4

2016-03-07 16:20:41

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 07/16] mtd: nand: sunxi: implement ->read_subpage()

Being able to read subpages can greatly improve read performances if the
MTD user is only interested in a small section of a NAND page.
This is particularly true with large pages (>= 8k).

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 8977a5f..df73e55 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -974,6 +974,39 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
return max_bitflips;
}

+static int sunxi_nfc_hw_ecc_read_subpage(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ u32 data_offs, u32 readlen,
+ u8 *bufpoi, int page)
+{
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+ int ret, i, cur_off = 0;
+ unsigned int max_bitflips = 0;
+
+ sunxi_nfc_hw_ecc_enable(mtd);
+
+ chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+ for (i = data_offs / ecc->size;
+ i < DIV_ROUND_UP(data_offs + readlen, ecc->size); i++) {
+ int data_off = i * ecc->size;
+ int oob_off = i * (ecc->bytes + 4);
+ u8 *data = bufpoi + data_off;
+ u8 *oob = chip->oob_poi + oob_off;
+
+ ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off,
+ oob,
+ oob_off + mtd->writesize,
+ &cur_off, &max_bitflips,
+ !i, page);
+ if (ret < 0)
+ return ret;
+ }
+
+ sunxi_nfc_hw_ecc_disable(mtd);
+
+ return max_bitflips;
+}
+
static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
struct nand_chip *chip,
const uint8_t *buf, int oob_required,
@@ -1384,6 +1417,7 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
ecc->write_page = sunxi_nfc_hw_ecc_write_page;
ecc->read_oob_raw = nand_read_oob_std;
ecc->write_oob_raw = nand_write_oob_std;
+ ecc->read_subpage = sunxi_nfc_hw_ecc_read_subpage;
layout = ecc->layout;
nsectors = mtd->writesize / ecc->size;

@@ -1630,6 +1664,8 @@ static int sunxi_nand_chip_init(struct device *dev, struct sunxi_nfc *nfc,
if (nand->options & NAND_NEED_SCRAMBLING)
nand->options |= NAND_NO_SUBPAGE_WRITE;

+ nand->options |= NAND_SUBPAGE_READ;
+
ret = sunxi_nand_chip_init_timings(chip, np);
if (ret) {
dev_err(dev, "could not configure chip timings: %d\n", ret);
--
2.1.4

2016-03-07 16:21:05

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 08/16] mtd: nand: sunxi: improve ->cmd_ctrl() function

Try to pack address and command cycles into a single NAND controller
command to avoid polling the status register for each single change
on the NAND bus.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 52 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index df73e55..db5446e 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -239,6 +239,10 @@ struct sunxi_nand_chip {
u32 timing_cfg;
u32 timing_ctl;
int selected;
+ int addr_cycles;
+ u32 addr[2];
+ int cmd_cycles;
+ u8 cmd[2];
int nsels;
struct sunxi_nand_chip_sel sels[0];
};
@@ -526,17 +530,49 @@ static void sunxi_nfc_cmd_ctrl(struct mtd_info *mtd, int dat,
writel(tmp, nfc->regs + NFC_REG_CTL);
}

- if (dat == NAND_CMD_NONE)
- return;
+ if (dat == NAND_CMD_NONE && (ctrl & NAND_NCE) &&
+ !(ctrl & (NAND_CLE | NAND_ALE))) {
+ u32 cmd = 0;

- if (ctrl & NAND_CLE) {
- writel(NFC_SEND_CMD1 | dat, nfc->regs + NFC_REG_CMD);
- } else {
- writel(dat, nfc->regs + NFC_REG_ADDR_LOW);
- writel(NFC_SEND_ADR, nfc->regs + NFC_REG_CMD);
+ if (!sunxi_nand->addr_cycles && !sunxi_nand->cmd_cycles)
+ return;
+
+ if (sunxi_nand->cmd_cycles--)
+ cmd |= NFC_SEND_CMD1 | sunxi_nand->cmd[0];
+
+ if (sunxi_nand->cmd_cycles--) {
+ cmd |= NFC_SEND_CMD2;
+ writel(sunxi_nand->cmd[1],
+ nfc->regs + NFC_REG_RCMD_SET);
+ }
+
+ sunxi_nand->cmd_cycles = 0;
+
+ if (sunxi_nand->addr_cycles) {
+ cmd |= NFC_SEND_ADR |
+ NFC_ADR_NUM(sunxi_nand->addr_cycles);
+ writel(sunxi_nand->addr[0],
+ nfc->regs + NFC_REG_ADDR_LOW);
+ }
+
+ if (sunxi_nand->addr_cycles > 4)
+ writel(sunxi_nand->addr[1],
+ nfc->regs + NFC_REG_ADDR_HIGH);
+
+ writel(cmd, nfc->regs + NFC_REG_CMD);
+ sunxi_nand->addr[0] = 0;
+ sunxi_nand->addr[1] = 0;
+ sunxi_nand->addr_cycles = 0;
+ sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
}

- sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+ if (ctrl & NAND_CLE) {
+ sunxi_nand->cmd[sunxi_nand->cmd_cycles++] = dat;
+ } else if (ctrl & NAND_ALE) {
+ sunxi_nand->addr[sunxi_nand->addr_cycles / 4] |=
+ dat << ((sunxi_nand->addr_cycles % 4) * 8);
+ sunxi_nand->addr_cycles++;
+ }
}

/* These seed values have been extracted from Allwinner's BSP */
--
2.1.4

2016-03-07 16:21:28

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 09/16] mtd: nand: sunxi: let the NAND controller control the CE line

We don't need to manually toggle the CE line since the controller handles
it for us. Moreover, keeping the CE line low when interacting with a DDR
NAND can be problematic (data loss in some corner cases).

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index db5446e..25a167f 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -515,21 +515,11 @@ static void sunxi_nfc_cmd_ctrl(struct mtd_info *mtd, int dat,
struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
int ret;
- u32 tmp;

ret = sunxi_nfc_wait_cmd_fifo_empty(nfc);
if (ret)
return;

- if (ctrl & NAND_CTRL_CHANGE) {
- tmp = readl(nfc->regs + NFC_REG_CTL);
- if (ctrl & NAND_NCE)
- tmp |= NFC_CE_CTL;
- else
- tmp &= ~NFC_CE_CTL;
- writel(tmp, nfc->regs + NFC_REG_CTL);
- }
-
if (dat == NAND_CMD_NONE && (ctrl & NAND_NCE) &&
!(ctrl & (NAND_CLE | NAND_ALE))) {
u32 cmd = 0;
--
2.1.4

2016-03-07 16:21:54

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 10/16] mtd: nand: sunxi: fix the NFC_ECC_ERR_CNT() macro

NFC_ECC_ERR_CNT() is not taking into account the case when the NAND chip
contains more than 4 ECC blocks (NANDs with 4kB+ pages).

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 25a167f..c1ab33a 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -155,7 +155,7 @@
/* define bit use in NFC_ECC_ST */
#define NFC_ECC_ERR(x) BIT(x)
#define NFC_ECC_PAT_FOUND(x) BIT(x + 16)
-#define NFC_ECC_ERR_CNT(b, x) (((x) >> ((b) * 8)) & 0xff)
+#define NFC_ECC_ERR_CNT(b, x) (((x) >> (((b) % 4) * 8)) & 0xff)

#define NFC_DEFAULT_TIMEOUT_MS 1000

--
2.1.4

2016-03-07 16:22:03

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 12/16] mtd: nand: sunxi: disable clks on device removal

mod and ahb clocks are not disabled when the NAND controller device is
removed.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 22d4d55..931c33d 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -1840,6 +1840,8 @@ static int sunxi_nfc_remove(struct platform_device *pdev)
struct sunxi_nfc *nfc = platform_get_drvdata(pdev);

sunxi_nand_chips_cleanup(nfc);
+ clk_disable_unprepare(nfc->mod_clk);
+ clk_disable_unprepare(nfc->ahb_clk);

return 0;
}
--
2.1.4

2016-03-07 16:22:14

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 13/16] mtd: nand: enable ECC pipelining

When the NAND controller operates in DMA mode it can pipeline ECC
operations which improves the throughput.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 931c33d..9816cda 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -743,7 +743,8 @@ static void sunxi_nfc_hw_ecc_enable(struct mtd_info *mtd)
ecc_ctl = readl(nfc->regs + NFC_REG_ECC_CTL);
ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE |
NFC_ECC_BLOCK_SIZE_MSK);
- ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION;
+ ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION |
+ NFC_ECC_PIPELINE;

writel(ecc_ctl, nfc->regs + NFC_REG_ECC_CTL);
}
--
2.1.4

2016-03-07 16:22:27

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 15/16] mtd: nand: sunxi: make use of readl_poll_timeout()

Replace open coded polling loops by readl_poll_timeout() calls.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 79c90fd..002194b 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -39,7 +39,7 @@
#include <linux/dmaengine.h>
#include <linux/gpio.h>
#include <linux/interrupt.h>
-#include <linux/io.h>
+#include <linux/iopoll.h>

#define NFC_REG_CTL 0x0000
#define NFC_REG_ST 0x0004
@@ -323,33 +323,33 @@ static int sunxi_nfc_wait_int(struct sunxi_nfc *nfc, u32 flags,

static int sunxi_nfc_wait_cmd_fifo_empty(struct sunxi_nfc *nfc)
{
- unsigned long timeout = jiffies +
- msecs_to_jiffies(NFC_DEFAULT_TIMEOUT_MS);
+ u32 status;
+ int ret;

- do {
- if (!(readl(nfc->regs + NFC_REG_ST) & NFC_CMD_FIFO_STATUS))
- return 0;
- } while (time_before(jiffies, timeout));
+ ret = readl_poll_timeout(nfc->regs + NFC_REG_ST, status,
+ !(status & NFC_CMD_FIFO_STATUS), 1,
+ NFC_DEFAULT_TIMEOUT_MS * 1000);
+ if (ret)
+ dev_err(nfc->dev, "wait for empty cmd FIFO timedout\n");

- dev_err(nfc->dev, "wait for empty cmd FIFO timedout\n");
- return -ETIMEDOUT;
+ return ret;
}

static int sunxi_nfc_rst(struct sunxi_nfc *nfc)
{
- unsigned long timeout = jiffies +
- msecs_to_jiffies(NFC_DEFAULT_TIMEOUT_MS);
+ u32 ctl;
+ int ret;

writel(0, nfc->regs + NFC_REG_ECC_CTL);
writel(NFC_RESET, nfc->regs + NFC_REG_CTL);

- do {
- if (!(readl(nfc->regs + NFC_REG_CTL) & NFC_RESET))
- return 0;
- } while (time_before(jiffies, timeout));
+ ret = readl_poll_timeout(nfc->regs + NFC_REG_CTL, ctl,
+ !(ctl & NFC_RESET), 1,
+ NFC_DEFAULT_TIMEOUT_MS * 1000);
+ if (ret)
+ dev_err(nfc->dev, "wait for NAND controller reset timedout\n");

- dev_err(nfc->dev, "wait for NAND controller reset timedout\n");
- return -ETIMEDOUT;
+ return ret;
}

static int sunxi_nfc_dev_ready(struct mtd_info *mtd)
--
2.1.4

2016-03-07 16:22:38

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 16/16] mtd: nand: sunxi: poll for events instead of using interrupts

Some NAND operations are so fast that it doesn't make any sense to use
interrupt based waits (the scheduling overhead is not worth it).
Rename sunxi_nfc_wait_int() into sunxi_nfc_wait_events() and add a
parameter to specify whether polling should be used or not.

Note that all sunxi_nfc_wait_int() are moved to the polling approach now,
but this should change as soon as we have more information about the
approximate time we are about to wait (can be extracted from the NAND
timings, and the type of operation).

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 45 +++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 002194b..0616f3b 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -302,23 +302,40 @@ static irqreturn_t sunxi_nfc_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static int sunxi_nfc_wait_int(struct sunxi_nfc *nfc, u32 flags,
- unsigned int timeout_ms)
+static int sunxi_nfc_wait_events(struct sunxi_nfc *nfc, u32 events,
+ bool use_polling, unsigned int timeout_ms)
{
- init_completion(&nfc->complete);
+ int ret;

- writel(flags, nfc->regs + NFC_REG_INT);
+ if (events & ~NFC_INT_MASK)
+ return -EINVAL;

if (!timeout_ms)
timeout_ms = NFC_DEFAULT_TIMEOUT_MS;

- if (!wait_for_completion_timeout(&nfc->complete,
- msecs_to_jiffies(timeout_ms))) {
- dev_err(nfc->dev, "wait interrupt timedout\n");
- return -ETIMEDOUT;
+ if (!use_polling) {
+ init_completion(&nfc->complete);
+
+ writel(events, nfc->regs + NFC_REG_INT);
+
+ ret = wait_for_completion_timeout(&nfc->complete,
+ msecs_to_jiffies(timeout_ms));
+
+ writel(0, nfc->regs + NFC_REG_INT);
+ } else {
+ u32 status;
+
+ ret = readl_poll_timeout(nfc->regs + NFC_REG_ST, status,
+ (status & events) == events, 1,
+ timeout_ms * 1000);
}

- return 0;
+ writel(events & NFC_INT_MASK, nfc->regs + NFC_REG_ST);
+
+ if (ret)
+ dev_err(nfc->dev, "wait interrupt timedout\n");
+
+ return ret;
}

static int sunxi_nfc_wait_cmd_fifo_empty(struct sunxi_nfc *nfc)
@@ -449,7 +466,7 @@ static void sunxi_nfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD;
writel(tmp, nfc->regs + NFC_REG_CMD);

- ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+ ret = sunxi_nfc_wait_events(nfc, NFC_CMD_INT_FLAG, true, 0);
if (ret)
break;

@@ -484,7 +501,7 @@ static void sunxi_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf,
NFC_ACCESS_DIR;
writel(tmp, nfc->regs + NFC_REG_CMD);

- ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+ ret = sunxi_nfc_wait_events(nfc, NFC_CMD_INT_FLAG, true, 0);
if (ret)
break;

@@ -546,7 +563,7 @@ static void sunxi_nfc_cmd_ctrl(struct mtd_info *mtd, int dat,
sunxi_nand->addr[0] = 0;
sunxi_nand->addr[1] = 0;
sunxi_nand->addr_cycles = 0;
- sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+ sunxi_nfc_wait_events(nfc, NFC_CMD_INT_FLAG, true, 0);
}

if (ctrl & NAND_CLE) {
@@ -789,7 +806,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
nfc->regs + NFC_REG_CMD);

- ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+ ret = sunxi_nfc_wait_events(nfc, NFC_CMD_INT_FLAG, true, 0);
sunxi_nfc_randomizer_disable(mtd);
if (ret)
return ret;
@@ -927,7 +944,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
NFC_ACCESS_DIR | NFC_ECC_OP,
nfc->regs + NFC_REG_CMD);

- ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+ ret = sunxi_nfc_wait_events(nfc, NFC_CMD_INT_FLAG, true, 0);
sunxi_nfc_randomizer_disable(mtd);
if (ret)
return ret;
--
2.1.4

2016-03-07 16:23:23

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 14/16] mtd: nand: sunxi: fix ->dev_ready() implementation

->dev_ready() is not supposed to wait for busy to ready solution (this is
the role of ->waitfunc()).

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 7 -------
1 file changed, 7 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 9816cda..79c90fd 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -358,7 +358,6 @@ static int sunxi_nfc_dev_ready(struct mtd_info *mtd)
struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
struct sunxi_nand_rb *rb;
- unsigned long timeo = (sunxi_nand->nand.state == FL_ERASING ? 400 : 20);
int ret;

if (sunxi_nand->selected < 0)
@@ -370,12 +369,6 @@ static int sunxi_nfc_dev_ready(struct mtd_info *mtd)
case RB_NATIVE:
ret = !!(readl(nfc->regs + NFC_REG_ST) &
NFC_RB_STATE(rb->info.nativeid));
- if (ret)
- break;
-
- sunxi_nfc_wait_int(nfc, NFC_RB_B2R, timeo);
- ret = !!(readl(nfc->regs + NFC_REG_ST) &
- NFC_RB_STATE(rb->info.nativeid));
break;
case RB_GPIO:
ret = gpio_get_value(rb->info.gpio);
--
2.1.4

2016-03-07 16:23:39

by Boris Brezillon

[permalink] [raw]
Subject: [PATCH 11/16] mtd: nand: sunxi: fix NFC_CTL setting

NFC_PAGE_SHIFT() already takes the real page_shift value and subtract 10
to it.

Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/mtd/nand/sunxi_nand.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index c1ab33a..22d4d55 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -411,7 +411,7 @@ static void sunxi_nfc_select_chip(struct mtd_info *mtd, int chip)
sel = &sunxi_nand->sels[chip];

ctl |= NFC_CE_SEL(sel->cs) | NFC_EN |
- NFC_PAGE_SHIFT(nand->page_shift - 10);
+ NFC_PAGE_SHIFT(nand->page_shift);
if (sel->rb.type == RB_NONE) {
nand->dev_ready = NULL;
} else {
--
2.1.4

2016-03-08 14:58:19

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 02/16] mtd: nand: sunxi: fix clk rate calculation

Boris,

On Mon, 7 Mar 2016 17:18:19 +0100, Boris Brezillon wrote:
> Unlike what is specified in the Allwinner datasheets, the NAND clock rate
> is not equal to 2/T but 1/T. Fix the clock rate selection accordingly.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> ---
> drivers/mtd/nand/sunxi_nand.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> index 4d01e65..ab66d8d 100644
> --- a/drivers/mtd/nand/sunxi_nand.c
> +++ b/drivers/mtd/nand/sunxi_nand.c
> @@ -1208,13 +1208,7 @@ static int sunxi_nand_chip_set_timings(struct sunxi_nand_chip *chip,
> /* Convert min_clk_period from picoseconds to nanoseconds */
> min_clk_period = DIV_ROUND_UP(min_clk_period, 1000);
>
> - /*
> - * Convert min_clk_period into a clk frequency, then get the
> - * appropriate rate for the NAND controller IP given this formula
> - * (specified in the datasheet):
> - * nand clk_rate = 2 * min_clk_rate
> - */

When some HW works in a way that is *NOT* the one specified in the
datasheet, I think you should rather add *more* comments about this
aspect, rather than removing existing comments.

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2016-03-08 15:21:18

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 02/16] mtd: nand: sunxi: fix clk rate calculation

On Tue, 8 Mar 2016 15:58:03 +0100
Thomas Petazzoni <[email protected]> wrote:

> Boris,
>
> On Mon, 7 Mar 2016 17:18:19 +0100, Boris Brezillon wrote:
> > Unlike what is specified in the Allwinner datasheets, the NAND clock rate
> > is not equal to 2/T but 1/T. Fix the clock rate selection accordingly.
> >
> > Signed-off-by: Boris Brezillon <[email protected]>
> > ---
> > drivers/mtd/nand/sunxi_nand.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> > index 4d01e65..ab66d8d 100644
> > --- a/drivers/mtd/nand/sunxi_nand.c
> > +++ b/drivers/mtd/nand/sunxi_nand.c
> > @@ -1208,13 +1208,7 @@ static int sunxi_nand_chip_set_timings(struct sunxi_nand_chip *chip,
> > /* Convert min_clk_period from picoseconds to nanoseconds */
> > min_clk_period = DIV_ROUND_UP(min_clk_period, 1000);
> >
> > - /*
> > - * Convert min_clk_period into a clk frequency, then get the
> > - * appropriate rate for the NAND controller IP given this formula
> > - * (specified in the datasheet):
> > - * nand clk_rate = 2 * min_clk_rate
> > - */
>
> When some HW works in a way that is *NOT* the one specified in the
> datasheet, I think you should rather add *more* comments about this
> aspect, rather than removing existing comments.

Fair enough (but you know how much I like documenting my code ;)). I'll
add a comment explaining why it differs from the datasheet
description :P.


--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

2016-04-13 15:27:51

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 00/16] mtd: nand: sunxi: various improvements/fixes

On Mon, 7 Mar 2016 17:18:17 +0100
Boris Brezillon <[email protected]> wrote:

> Hi,
>
> This patchset aims at fixing a few minor bugs, and improving performances
> of NAND accesses going through the sunxi NAND controller.
>
> Note that patch 5 exports functions provided by the core which are needed
> in patch 6 to still support raw OOB accesses. Other patches are just
> fixes or improvements only touching the sunxi driver itself.
>
> Here are the main improvements:
> - queue CLE/ALE requests instead of forcing the controller to issue each
> cmd and address cycle separately
> - benefit from ECC correction on protected OOB bytes
> - use polling instead of interrupt-based waiting (avoid scheduling
> overhead for short wait period)

I'm applying this series since it only touches the sunxi NAND driver,
and I don't expect to have any external reviews :).

--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com