Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1612622 > unrolled thread
| Started by | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| First post | 2017-03-30 09:00 +0200 |
| Last post | 2017-03-31 10:30 +0200 |
| Articles | 20 on this page of 23 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v3 00/37] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 25/37] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
Re: [PATCH v3 25/37] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-30 18:00 +0200
[PATCH v3 30/37] mtd: nand: denali: use flag instead of register macro for direction Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 04/37] mtd: nand: denali: fix bitflips calculation in handle_ecc() Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 27/37] mtd: nand: denali: use interrupt instead of polling for bank reset Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 23/37] mtd: nand: denali: fix NAND_CMD_STATUS handling Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 22/37] mtd: nand: denali: rework interrupt handling Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 08/37] mtd: nand: denali: support 64bit capable DMA engine Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 18/37] mtd: nand: denali: do not propagate NAND_STATUS_FAIL to waitfunc() Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
Re: [PATCH v3 18/37] mtd: nand: denali: do not propagate NAND_STATUS_FAIL to waitfunc() Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-30 17:20 +0200
[PATCH v3 09/37] mtd: nand: denali_dt: remove dma-mask DT property Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 02/37] mtd: nand: denali: allow to override mtd->name from label DT property Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 12/37] mtd: nand: denali: support 1024 byte ECC step size Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
Re: [PATCH v3 12/37] mtd: nand: denali: support 1024 byte ECC step size Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-04-01 10:50 +0200
[PATCH v3 19/37] mtd: nand: denali: use BIT() and GENMASK() for register macros Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 06/37] mtd: nand: denali: support HW_ECC_FIXUP capability Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 28/37] mtd: nand: denali: propagate page to helpers via function argument Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
[PATCH v3 16/37] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-30 09:00 +0200
Re: [PATCH v3 16/37] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants Rob Herring <robh@kernel.org> - 2017-04-03 17:50 +0200
Re: [PATCH v3 00/37] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-30 18:40 +0200
Re: [PATCH v3 00/37] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-03-31 06:10 +0200
Re: [PATCH v3 00/37] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-31 10:30 +0200
Page 1 of 2 [1] 2 Next page →
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 00/37] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb |
| Message-ID | <tqC49-2hn-3@gated-at.bofh.it> |
This driver includes many problems.
One of the biggest one is a bunch of hard-coded parameters. This IP
has many parameters that can be customized when a delivery RTL is
generated. However, this driver was upstreamed by Intel, with
Intel parameters hard-coded. Later, Altera added denali_dt.c to use
this driver for embedded boards, but they did not fix the code in
denali.c So, this driver has never worked. Even some DT bindings
actually turned out wrong.
There are more problems: [1] The driver just retrieves the OOB area as-is
whereas the controller uses syndrome page layout. [2] Many NAND chip
specific parameters are hard-coded in the driver. [3] ONFi devices are
not working [4] It can not read Bad Block Marker
This patch series intends to solve those problems.
Outstanding changes are:
- Fix raw/oob callbacks for syndrome page layout
- Implement setup_data_interface() callback
- Fix/implement more commands for ONFi devices
- Allow to skip the driver internal bounce buffer
- Support PIO in case DMA is not supported
- Switch from ->cmdfunc over to ->cmd_ctrl
18 patches were merged at v2.
Here is the rest of the series.
v1: https://lkml.org/lkml/2016/11/26/144
v2: https://lkml.org/lkml/2017/3/22/804
Masahiro Yamada (37):
mtd: nand: relax ecc.read_page() return value for uncorrectable ECC
mtd: nand: denali: allow to override mtd->name from label DT property
mtd: nand: denali: remove meaningless pipeline read-ahead operation
mtd: nand: denali: fix bitflips calculation in handle_ecc()
mtd: nand: denali: fix erased page checking
mtd: nand: denali: support HW_ECC_FIXUP capability
mtd: nand: denali_dt: enable HW_ECC_FIXUP for Altera SOCFPGA variant
mtd: nand: denali: support 64bit capable DMA engine
mtd: nand: denali_dt: remove dma-mask DT property
mtd: nand: denali_dt: use pdev instead of ofdev for platform_device
mtd: nand: denali: allow to override revision number
mtd: nand: denali: support 1024 byte ECC step size
mtd: nand: denali: avoid hard-coding ecc.strength and ecc.bytes
mtd: nand: denali: support "nand-ecc-strength" DT property
mtd: nand: denali: remove Toshiba and Hynix specific fixup code
mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants
mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS
mtd: nand: denali: do not propagate NAND_STATUS_FAIL to waitfunc()
mtd: nand: denali: use BIT() and GENMASK() for register macros
mtd: nand: denali: remove unneeded find_valid_banks()
mtd: nand: denali: handle timing parameters by setup_data_interface()
mtd: nand: denali: rework interrupt handling
mtd: nand: denali: fix NAND_CMD_STATUS handling
mtd: nand: denali: fix NAND_CMD_PARAM handling
mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc
mtd: nand: denali: fix bank reset function
mtd: nand: denali: use interrupt instead of polling for bank reset
mtd: nand: denali: propagate page to helpers via function argument
mtd: nand: denali: merge struct nand_buf into struct denali_nand_info
mtd: nand: denali: use flag instead of register macro for direction
mtd: nand: denali: fix raw and oob accessors for syndrome page layout
mtd: nand: denali: support hardware-assisted erased page detection
mtd: nand: allocate aligned buffers if NAND_OWN_BUFFERS is unset
mtd: nand: allow drivers to request minimum alignment for passed
buffer
mtd: nand: denali: skip driver internal bounce buffer when possible
mtd: nand: denali: use non-managed kmalloc() for DMA buffer
mtd: nand: denali: enable bad block table scan
.../devicetree/bindings/mtd/denali-nand.txt | 24 +-
drivers/mtd/nand/denali.c | 1971 ++++++++++----------
drivers/mtd/nand/denali.h | 308 +--
drivers/mtd/nand/denali_dt.c | 90 +-
drivers/mtd/nand/denali_pci.c | 10 +-
drivers/mtd/nand/nand_base.c | 49 +-
include/linux/mtd/nand.h | 4 +-
7 files changed, 1234 insertions(+), 1222 deletions(-)
--
2.7.4
[toc] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 25/37] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc |
| Message-ID | <tqCdP-2l8-9@gated-at.bofh.it> |
| In reply to | #1612622 |
The NAND_CMD_SET_FEATURES support is missing from denali_cmdfunc().
This is needed for nand_onfi_set_features().
Besides, we see /* TODO: Read OOB data */ comment line.
It would be possible to add more commands along with the current
implementation, but having ->cmd_ctrl() seems a better approach from
the discussion with Boris [1].
Rely on the default ->cmdfunc() from the framework and implement the
driver's own ->cmd_ctrl().
Also add ->write_byte(), which is needed for write direction commands.
[1] https://lkml.org/lkml/2017/3/15/97
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Newly added
drivers/mtd/nand/denali.c | 104 +++++++++++++++++++++++-----------------------
1 file changed, 52 insertions(+), 52 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 0e5e490..86b7c75 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -230,20 +230,16 @@ static uint32_t denali_wait_for_irq(struct denali_nand_info *denali,
return denali->irq_status;
}
-/* resets a specific device connected to the core */
-static void reset_bank(struct denali_nand_info *denali)
+static uint32_t denali_check_irq(struct denali_nand_info *denali)
{
+ unsigned long flags;
uint32_t irq_status;
- denali_reset_irq(denali);
-
- iowrite32(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
-
- irq_status = denali_wait_for_irq(denali,
- INTR__RST_COMP | INTR__TIME_OUT);
+ spin_lock_irqsave(&denali->irq_lock, flags);
+ irq_status = denali->irq_status;
+ spin_unlock_irqrestore(&denali->irq_lock, flags);
- if (!(irq_status & INTR__RST_COMP))
- dev_err(denali->dev, "reset bank failed.\n");
+ return irq_status;
}
/*
@@ -273,6 +269,42 @@ static uint8_t denali_read_byte(struct mtd_info *mtd)
return ioread32(denali->flash_mem + 0x10);
}
+static void denali_write_byte(struct mtd_info *mtd, uint8_t byte)
+{
+ struct denali_nand_info *denali = mtd_to_denali(mtd);
+
+ index_addr(denali, MODE_11 | BANK(denali->flash_bank) | 2, byte);
+}
+
+static void denali_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
+{
+ struct denali_nand_info *denali = mtd_to_denali(mtd);
+ uint32_t type;
+
+ if (ctrl & NAND_CLE)
+ type = 0;
+ else if (ctrl & NAND_ALE)
+ type = 1;
+ else
+ return;
+
+ /*
+ * Some commands are followed by chip->dev_ready or chip->waitfunc.
+ * irq_status must be cleared here to catch the R/B# interrupt later.
+ */
+ if (ctrl & NAND_CTRL_CHANGE)
+ denali_reset_irq(denali);
+
+ index_addr(denali, MODE_11 | BANK(denali->flash_bank) | type, dat);
+}
+
+static int denali_dev_ready(struct mtd_info *mtd)
+{
+ struct denali_nand_info *denali = mtd_to_denali(mtd);
+
+ return !!(denali_check_irq(denali) & INTR__INT_ACT);
+}
+
/*
* sends a pipeline command operation to the controller. See the Denali NAND
* controller's user guide for more information (section 4.2.3.6).
@@ -824,7 +856,13 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
{
- return 0;
+ struct denali_nand_info *denali = mtd_to_denali(mtd);
+ uint32_t irq_status;
+
+ /* R/B# pin transitioned from low to high? */
+ irq_status = denali_wait_for_irq(denali, INTR__INT_ACT);
+
+ return irq_status & INTR__INT_ACT ? 0 : NAND_STATUS_FAIL;
}
static int denali_erase(struct mtd_info *mtd, int page)
@@ -845,46 +883,6 @@ static int denali_erase(struct mtd_info *mtd, int page)
return irq_status & INTR__ERASE_COMP ? 0 : NAND_STATUS_FAIL;
}
-static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
- int page)
-{
- struct denali_nand_info *denali = mtd_to_denali(mtd);
- uint32_t addr, irq_status;
- int wait_ready = 0;
-
- switch (cmd) {
- case NAND_CMD_PARAM:
- wait_ready = 1;
- break;
- case NAND_CMD_STATUS:
- case NAND_CMD_READID:
- break;
- case NAND_CMD_RESET:
- reset_bank(denali);
- break;
- case NAND_CMD_READOOB:
- /* TODO: Read OOB data */
- return;
- default:
- pr_err(": unsupported command received 0x%x\n", cmd);
- return;
- }
-
- denali_reset_irq(denali);
-
- addr = MODE_11 | BANK(denali->flash_bank);
- index_addr(denali, addr | 0, cmd);
- if (col != -1)
- index_addr(denali, addr | 1, col);
-
- if (!wait_ready)
- return;
-
- irq_status = denali_wait_for_irq(denali, INTR__INT_ACT);
- if (!(irq_status & INTR__INT_ACT))
- dev_err(denali->dev, "failed to issue command 0x%x\n", cmd);
-}
-
#define DIV_ROUND_DOWN_ULL(ll, d) \
({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; })
@@ -1230,8 +1228,10 @@ int denali_init(struct denali_nand_info *denali)
/* register the driver with the NAND core subsystem */
chip->select_chip = denali_select_chip;
- chip->cmdfunc = denali_cmdfunc;
chip->read_byte = denali_read_byte;
+ chip->write_byte = denali_write_byte;
+ chip->cmd_ctrl = denali_cmd_ctrl;
+ chip->dev_ready = denali_dev_ready;
chip->waitfunc = denali_waitfunc;
/* clk rate info is needed for setup_data_interface */
if (denali->clk_x_rate)
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Boris Brezillon <boris.brezillon@free-electrons.com> |
|---|---|
| Date | 2017-03-30 18:00 +0200 |
| Subject | Re: [PATCH v3 25/37] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc |
| Message-ID | <tqKEr-8v3-23@gated-at.bofh.it> |
| In reply to | #1612623 |
On Thu, 30 Mar 2017 15:46:11 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> /*
> * sends a pipeline command operation to the controller. See the Denali NAND
> * controller's user guide for more information (section 4.2.3.6).
> @@ -824,7 +856,13 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
>
> static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
> {
> - return 0;
> + struct denali_nand_info *denali = mtd_to_denali(mtd);
> + uint32_t irq_status;
> +
> + /* R/B# pin transitioned from low to high? */
> + irq_status = denali_wait_for_irq(denali, INTR__INT_ACT);
> +
> + return irq_status & INTR__INT_ACT ? 0 : NAND_STATUS_FAIL;
Okay, you fix the ->waitfunc() implementation here. Great! Then I'm
fine with patch 18.
> }
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 30/37] mtd: nand: denali: use flag instead of register macro for direction |
| Message-ID | <tqCdQ-2l8-29@gated-at.bofh.it> |
| In reply to | #1612622 |
It is not a good idea to re-use macros that represent a specific
register bit field for the transfer direction.
It is true that bit 8 indicates the direction for the MAP10 pipeline
operation and the data DMA operation, but this is not valid across
the IP.
Use a simple flag (write: 1, read: 0) for the direction.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Newly added
drivers/mtd/nand/denali.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 5feeaf5..6843cfc 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -53,9 +53,6 @@ static inline struct denali_nand_info *mtd_to_denali(struct mtd_info *mtd)
#define MAIN_ACCESS 0x42
#define MAIN_SPARE_ACCESS 0x43
-#define DENALI_READ 0
-#define DENALI_WRITE 0x100
-
#define DENALI_NR_BANKS 4
/*
@@ -284,7 +281,7 @@ static int denali_dev_ready(struct mtd_info *mtd)
*/
static int denali_send_pipeline_cmd(struct denali_nand_info *denali, int page,
bool ecc_en, bool transfer_spare,
- int access_type, int op)
+ int access_type, int write)
{
int status = PASS;
uint32_t addr, cmd;
@@ -295,17 +292,17 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali, int page,
addr = BANK(denali->flash_bank) | page;
- if (op == DENALI_WRITE && access_type != SPARE_ACCESS) {
+ if (write && access_type != SPARE_ACCESS) {
cmd = MODE_01 | addr;
iowrite32(cmd, denali->flash_mem);
- } else if (op == DENALI_WRITE && access_type == SPARE_ACCESS) {
+ } else if (write && access_type == SPARE_ACCESS) {
/* read spare area */
cmd = MODE_10 | addr;
index_addr(denali, cmd, access_type);
cmd = MODE_01 | addr;
iowrite32(cmd, denali->flash_mem);
- } else if (op == DENALI_READ) {
+ } else {
/* setup page read request for access type */
cmd = MODE_10 | addr;
index_addr(denali, cmd, access_type);
@@ -367,7 +364,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
int status = 0;
if (denali_send_pipeline_cmd(denali, page, false, false, SPARE_ACCESS,
- DENALI_WRITE) == PASS) {
+ 1) == PASS) {
write_data_to_flash_mem(denali, buf, mtd->oobsize);
/* wait for operation to complete */
@@ -392,7 +389,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
uint32_t irq_status, addr, cmd;
if (denali_send_pipeline_cmd(denali, page, false, true, SPARE_ACCESS,
- DENALI_READ) == PASS) {
+ 0) == PASS) {
read_data_from_flash_mem(denali, buf, mtd->oobsize);
/*
@@ -578,7 +575,7 @@ static void denali_enable_dma(struct denali_nand_info *denali, bool en)
}
static void denali_setup_dma64(struct denali_nand_info *denali,
- dma_addr_t dma_addr, int page, int op)
+ dma_addr_t dma_addr, int page, int write)
{
uint32_t mode;
const int page_count = 1;
@@ -591,7 +588,8 @@ static void denali_setup_dma64(struct denali_nand_info *denali,
* 1. setup transfer type, interrupt when complete,
* burst len = 64 bytes, the number of pages
*/
- index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);
+ index_addr(denali, mode,
+ 0x01002000 | (64 << 16) | (write << 8) | page_count);
/* 2. set memory low address */
index_addr(denali, mode, dma_addr);
@@ -601,7 +599,7 @@ static void denali_setup_dma64(struct denali_nand_info *denali,
}
static void denali_setup_dma32(struct denali_nand_info *denali,
- dma_addr_t dma_addr, int page, int op)
+ dma_addr_t dma_addr, int page, int write)
{
uint32_t mode;
const int page_count = 1;
@@ -611,7 +609,7 @@ static void denali_setup_dma32(struct denali_nand_info *denali,
/* DMA is a four step process */
/* 1. setup transfer type and # of pages */
- index_addr(denali, mode | page, 0x2000 | op | page_count);
+ index_addr(denali, mode | page, 0x2000 | (write << 8) | page_count);
/* 2. set memory high address bits 23:8 */
index_addr(denali, mode | ((dma_addr >> 16) << 8), 0x2200);
@@ -624,12 +622,12 @@ static void denali_setup_dma32(struct denali_nand_info *denali,
}
static void denali_setup_dma(struct denali_nand_info *denali,
- dma_addr_t dma_addr, int page, int op)
+ dma_addr_t dma_addr, int page, int write)
{
if (denali->caps & DENALI_CAP_DMA_64BIT)
- denali_setup_dma64(denali, dma_addr, page, op);
+ denali_setup_dma64(denali, dma_addr, page, write);
else
- denali_setup_dma32(denali, dma_addr, page, op);
+ denali_setup_dma32(denali, dma_addr, page, write);
}
/*
@@ -668,7 +666,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
denali_reset_irq(denali);
denali_enable_dma(denali, true);
- denali_setup_dma(denali, addr, page, DENALI_WRITE);
+ denali_setup_dma(denali, addr, page, 1);
/* wait for operation to complete */
irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -750,7 +748,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
denali_reset_irq(denali);
- denali_setup_dma(denali, addr, page, DENALI_READ);
+ denali_setup_dma(denali, addr, page, 0);
/* wait for operation to complete */
irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -793,7 +791,7 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
denali_reset_irq(denali);
- denali_setup_dma(denali, addr, page, DENALI_READ);
+ denali_setup_dma(denali, addr, page, 0);
/* wait for operation to complete */
irq_status = denali_wait_for_irq(denali, irq_mask);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 04/37] mtd: nand: denali: fix bitflips calculation in handle_ecc() |
| Message-ID | <tqCdQ-2l8-31@gated-at.bofh.it> |
| In reply to | #1612622 |
This function is wrong in multiple ways:
[1] Counting corrected bytes instead of corrected bits.
The following code is counting the number of corrected _bytes_.
/* correct the ECC error */
buf[offset] ^= err_cor_value;
mtd->ecc_stats.corrected++;
bitflips++;
What the core framework expects is the number of corrected _bits_.
They can be different if multiple bitflips occur within one byte.
[2] total number of errors instead of max of per-sector errors
The core framework expects that corrected errors are counted per
sector, then the max value should be taken. The current code simply
iterates over the whole page, i.e. counts the total number of
correction in the page. This means "too many bitflips" is triggered
earlier than it should be, i.e. the NAND device is worn out sooner.
Besides those bugs, this function is unreadable due to the deep
nesting. Notice the whole code in this function is wrapped in
if (irq_status & INTR__ECC_ERR), so this conditional can be moved
out of the function. Also, use shorter names for local variables.
Re-work the function to fix all the issues.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3:
- Adjust function arguments for the next commit
Changes in v2:
- Use shorter names for local variables.
- Fix bugs addressed by [1], [2]
drivers/mtd/nand/denali.c | 141 +++++++++++++++++++++++-----------------------
1 file changed, 71 insertions(+), 70 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 65cf7cc..c5c150a 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -836,80 +836,80 @@ static bool is_erased(uint8_t *buf, int len)
#define ECC_SECTOR(x) (((x) & ECC_ERROR_ADDRESS__SECTOR_NR) >> 12)
#define ECC_BYTE(x) (((x) & ECC_ERROR_ADDRESS__OFFSET))
#define ECC_CORRECTION_VALUE(x) ((x) & ERR_CORRECTION_INFO__BYTEMASK)
-#define ECC_ERROR_CORRECTABLE(x) (!((x) & ERR_CORRECTION_INFO__ERROR_TYPE))
+#define ECC_ERROR_UNCORRECTABLE(x) ((x) & ERR_CORRECTION_INFO__ERROR_TYPE)
#define ECC_ERR_DEVICE(x) (((x) & ERR_CORRECTION_INFO__DEVICE_NR) >> 8)
#define ECC_LAST_ERR(x) ((x) & ERR_CORRECTION_INFO__LAST_ERR_INFO)
-static bool handle_ecc(struct denali_nand_info *denali, uint8_t *buf,
- uint32_t irq_status, unsigned int *max_bitflips)
+static int handle_ecc(struct mtd_info *mtd, struct denali_nand_info *denali,
+ uint8_t *buf, bool *check_erased_page)
{
- bool check_erased_page = false;
unsigned int bitflips = 0;
+ unsigned int max_bitflips = 0;
+ uint32_t err_addr, err_cor_info;
+ unsigned int err_byte, err_sector, err_device;
+ uint8_t err_cor_value;
+ unsigned int prev_sector = 0;
- if (irq_status & INTR__ECC_ERR) {
- /* read the ECC errors. we'll ignore them for now */
- uint32_t err_address, err_correction_info, err_byte,
- err_sector, err_device, err_correction_value;
- denali_set_intr_modes(denali, false);
-
- do {
- err_address = ioread32(denali->flash_reg +
- ECC_ERROR_ADDRESS);
- err_sector = ECC_SECTOR(err_address);
- err_byte = ECC_BYTE(err_address);
-
- err_correction_info = ioread32(denali->flash_reg +
- ERR_CORRECTION_INFO);
- err_correction_value =
- ECC_CORRECTION_VALUE(err_correction_info);
- err_device = ECC_ERR_DEVICE(err_correction_info);
-
- if (ECC_ERROR_CORRECTABLE(err_correction_info)) {
- /*
- * If err_byte is larger than ECC_SECTOR_SIZE,
- * means error happened in OOB, so we ignore
- * it. It's no need for us to correct it
- * err_device is represented the NAND error
- * bits are happened in if there are more
- * than one NAND connected.
- */
- if (err_byte < ECC_SECTOR_SIZE) {
- struct mtd_info *mtd =
- nand_to_mtd(&denali->nand);
- int offset;
-
- offset = (err_sector *
- ECC_SECTOR_SIZE +
- err_byte) *
- denali->devnum +
- err_device;
- /* correct the ECC error */
- buf[offset] ^= err_correction_value;
- mtd->ecc_stats.corrected++;
- bitflips++;
- }
- } else {
- /*
- * if the error is not correctable, need to
- * look at the page to see if it is an erased
- * page. if so, then it's not a real ECC error
- */
- check_erased_page = true;
- }
- } while (!ECC_LAST_ERR(err_correction_info));
- /*
- * Once handle all ecc errors, controller will triger
- * a ECC_TRANSACTION_DONE interrupt, so here just wait
- * for a while for this interrupt
- */
- while (!(read_interrupt_status(denali) &
- INTR__ECC_TRANSACTION_DONE))
- cpu_relax();
- clear_interrupts(denali);
- denali_set_intr_modes(denali, true);
- }
- *max_bitflips = bitflips;
- return check_erased_page;
+ /* read the ECC errors. we'll ignore them for now */
+ denali_set_intr_modes(denali, false);
+
+ do {
+ err_addr = ioread32(denali->flash_reg + ECC_ERROR_ADDRESS);
+ err_sector = ECC_SECTOR(err_addr);
+ err_byte = ECC_BYTE(err_addr);
+
+ err_cor_info = ioread32(denali->flash_reg + ERR_CORRECTION_INFO);
+ err_cor_value = ECC_CORRECTION_VALUE(err_cor_info);
+ err_device = ECC_ERR_DEVICE(err_cor_info);
+
+ /* reset the bitflip counter when crossing ECC sector */
+ if (err_sector != prev_sector)
+ bitflips = 0;
+
+ if (ECC_ERROR_UNCORRECTABLE(err_cor_info)) {
+ /*
+ * if the error is not correctable, need to look at the
+ * page to see if it is an erased page. if so, then
+ * it's not a real ECC error
+ */
+ *check_erased_page = true;
+ } else if (err_byte < ECC_SECTOR_SIZE) {
+ /*
+ * If err_byte is larger than ECC_SECTOR_SIZE, means error
+ * happened in OOB, so we ignore it. It's no need for
+ * us to correct it err_device is represented the NAND
+ * error bits are happened in if there are more than
+ * one NAND connected.
+ */
+ int offset;
+ unsigned int flips_in_byte;
+
+ offset = (err_sector * ECC_SECTOR_SIZE + err_byte) *
+ denali->devnum + err_device;
+
+ /* correct the ECC error */
+ flips_in_byte = hweight8(buf[offset] ^ err_cor_value);
+ buf[offset] ^= err_cor_value;
+ mtd->ecc_stats.corrected += flips_in_byte;
+ bitflips += flips_in_byte;
+
+ max_bitflips = max(max_bitflips, bitflips);
+ }
+
+ prev_sector = err_sector;
+ } while (!ECC_LAST_ERR(err_cor_info));
+
+ /*
+ * Once handle all ecc errors, controller will trigger a
+ * ECC_TRANSACTION_DONE interrupt, so here just wait for
+ * a while for this interrupt
+ */
+ while (!(read_interrupt_status(denali) & INTR__ECC_TRANSACTION_DONE))
+ cpu_relax();
+ clear_interrupts(denali);
+ denali_set_intr_modes(denali, true);
+
+ return max_bitflips;
}
/* programs the controller to either enable/disable DMA transfers */
@@ -1045,7 +1045,7 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
uint8_t *buf, int oob_required, int page)
{
- unsigned int max_bitflips;
+ unsigned int max_bitflips = 0;
struct denali_nand_info *denali = mtd_to_denali(mtd);
dma_addr_t addr = denali->buf.dma_buf;
@@ -1077,7 +1077,8 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
memcpy(buf, denali->buf.buf, mtd->writesize);
- check_erased_page = handle_ecc(denali, buf, irq_status, &max_bitflips);
+ if (irq_status & INTR__ECC_ERR)
+ max_bitflips = handle_ecc(mtd, denali, buf, &check_erased_page);
denali_enable_dma(denali, false);
if (check_erased_page) {
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 27/37] mtd: nand: denali: use interrupt instead of polling for bank reset |
| Message-ID | <tqCdR-2l8-37@gated-at.bofh.it> |
| In reply to | #1612622 |
The current bank reset implementation polls the INTR_STATUS register
until interested bits are set. This is not good because:
- it simply wastes time-slice of the thread which the driver module
is loaded from.
- The while() loop may continue eternally if no bit is set, for
example, due to the controller problem. The denali_wait_for_irq()
uses wait_for_completion_timeout(), which is safer.
We can use interrupt by moving the denali_reset_bank() call below
the interrupt setup.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Newly added
drivers/mtd/nand/denali.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index e4f2699..9dfed20 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -974,24 +974,25 @@ static int denali_setup_data_interface(struct mtd_info *mtd,
static void denali_reset_banks(struct denali_nand_info *denali)
{
+ u32 irq_status;
int i;
- denali_clear_irq_all(denali);
-
for (i = 0; i < denali->max_banks; i++) {
- iowrite32(1 << i, denali->flash_reg + DEVICE_RESET);
- while (!(ioread32(denali->flash_reg + INTR_STATUS(i)) &
- (INTR__RST_COMP | INTR__TIME_OUT)))
- cpu_relax();
- if (!(ioread32(denali->flash_reg + INTR_STATUS(i)) &
- INTR__INT_ACT))
+ denali->flash_bank = i;
+
+ denali_reset_irq(denali);
+
+ iowrite32(DEVICE_RESET__BANK(i),
+ denali->flash_reg + DEVICE_RESET);
+
+ irq_status = denali_wait_for_irq(denali,
+ INTR__RST_COMP | INTR__INT_ACT | INTR__TIME_OUT);
+ if (!(irq_status & INTR__INT_ACT))
break;
}
dev_dbg(denali->dev, "%d chips connected\n", i);
denali->max_banks = i;
-
- denali_clear_irq_all(denali);
}
static void denali_hw_init(struct denali_nand_info *denali)
@@ -1013,7 +1014,6 @@ static void denali_hw_init(struct denali_nand_info *denali)
denali->bbtskipbytes = ioread32(denali->flash_reg +
SPARE_AREA_SKIP_BYTES);
detect_max_banks(denali);
- denali_reset_banks(denali);
iowrite32(0x0F, denali->flash_reg + RB_PIN_ENABLED);
iowrite32(CHIP_EN_DONT_CARE__FLAG,
denali->flash_reg + CHIP_ENABLE_DONT_CARE);
@@ -1137,9 +1137,6 @@ static void denali_drv_init(struct denali_nand_info *denali)
* element that might be access shared data (interrupt status)
*/
spin_lock_init(&denali->irq_lock);
-
- /* indicate that MTD has not selected a valid bank yet */
- denali->flash_bank = CHIP_SELECT_INVALID;
}
static int denali_multidev_fixup(struct denali_nand_info *denali)
@@ -1214,6 +1211,9 @@ int denali_init(struct denali_nand_info *denali)
}
denali_enable_irq(denali);
+ denali_reset_banks(denali);
+
+ denali->flash_bank = CHIP_SELECT_INVALID;
nand_set_flash_node(chip, denali->dev->of_node);
/* Fallback to the default name if DT did not give "label" property */
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 23/37] mtd: nand: denali: fix NAND_CMD_STATUS handling |
| Message-ID | <tqCdQ-2l8-27@gated-at.bofh.it> |
| In reply to | #1612622 |
The current NAND_CMD_STATUS handling is weird; it just reads the
WRITE_PROTECT register, and returns NAND_STATUS_WP if it is set.
It does not send Read Status (0x70) command, so it is not helpful
for checking the current device status.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Newly added
drivers/mtd/nand/denali.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 75b045e0..bca4fcd 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -107,21 +107,6 @@ static void write_byte_to_buf(struct denali_nand_info *denali, uint8_t byte)
denali->buf.buf[denali->buf.tail++] = byte;
}
-/* reads the status of the device */
-static void read_status(struct denali_nand_info *denali)
-{
- uint32_t cmd;
-
- /* initialize the data buffer to store status */
- reset_buf(denali);
-
- cmd = ioread32(denali->flash_reg + WRITE_PROTECT);
- if (cmd)
- write_byte_to_buf(denali, NAND_STATUS_WP);
- else
- write_byte_to_buf(denali, 0);
-}
-
/* Reset the flash controller */
static uint16_t denali_nand_reset(struct denali_nand_info *denali)
{
@@ -893,7 +878,11 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
switch (cmd) {
case NAND_CMD_STATUS:
- read_status(denali);
+ reset_buf(denali);
+ addr = MODE_11 | BANK(denali->flash_bank);
+ index_addr(denali, addr | 0, cmd);
+ index_addr_read_data(denali, addr | 2, &id);
+ write_byte_to_buf(denali, id);
break;
case NAND_CMD_READID:
case NAND_CMD_PARAM:
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 22/37] mtd: nand: denali: rework interrupt handling |
| Message-ID | <tqCdQ-2l8-33@gated-at.bofh.it> |
| In reply to | #1612622 |
Simplify the interrupt handling and fix issues:
- The register field view of INTR_EN / INTR_STATUS is different
among IP versions. The global macro DENALI_IRQ_ALL is hard-coded
for Intel platforms. The interrupt mask should be determined at
run-time depending on the running platform.
- wait_for_irq() loops do {} while() until interested flags are
asserted. The logic can be simplified.
- The spin_lock() guard seems too complex (and suspicious in a race
condition if wait_for_completion_timeout() bails out by timeout).
- denali->complete is reused again and again, but reinit_completion()
is missing. Add it.
Re-work the code to make it more robust and easier to handle.
While we are here, also rename the jump label "failed_req_irq" to
more appropriate "disable_irq".
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Newly added
drivers/mtd/nand/denali.c | 320 +++++++++++++++++-----------------------------
drivers/mtd/nand/denali.h | 1 +
2 files changed, 118 insertions(+), 203 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index ea566ea..75b045e0 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -31,21 +31,6 @@ MODULE_LICENSE("GPL");
#define DENALI_NAND_NAME "denali-nand"
/*
- * We define a macro here that combines all interrupts this driver uses into
- * a single constant value, for convenience.
- */
-#define DENALI_IRQ_ALL (INTR__DMA_CMD_COMP | \
- INTR__ECC_TRANSACTION_DONE | \
- INTR__ECC_ERR | \
- INTR__PROGRAM_FAIL | \
- INTR__LOAD_COMP | \
- INTR__PROGRAM_COMP | \
- INTR__TIME_OUT | \
- INTR__ERASE_FAIL | \
- INTR__RST_COMP | \
- INTR__ERASE_COMP)
-
-/*
* indicates whether or not the internal value for the flash bank is
* valid or not
*/
@@ -71,20 +56,14 @@ static inline struct denali_nand_info *mtd_to_denali(struct mtd_info *mtd)
#define DENALI_READ 0
#define DENALI_WRITE 0x100
+#define DENALI_NR_BANKS 4
+
/*
* this is a helper macro that allows us to
* format the bank into the proper bits for the controller
*/
#define BANK(x) ((x) << 24)
-/* forward declarations */
-static void clear_interrupts(struct denali_nand_info *denali);
-static uint32_t wait_for_irq(struct denali_nand_info *denali,
- uint32_t irq_mask);
-static void denali_irq_enable(struct denali_nand_info *denali,
- uint32_t int_mask);
-static uint32_t read_interrupt_status(struct denali_nand_info *denali);
-
/*
* The bus interface clock, clk_x, is phase aligned with the core clock. The
* clk_x is an integral multiple N of the core clk. The value N is configured
@@ -143,22 +122,6 @@ static void read_status(struct denali_nand_info *denali)
write_byte_to_buf(denali, 0);
}
-/* resets a specific device connected to the core */
-static void reset_bank(struct denali_nand_info *denali)
-{
- uint32_t irq_status;
- uint32_t irq_mask = INTR__RST_COMP | INTR__TIME_OUT;
-
- clear_interrupts(denali);
-
- iowrite32(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
-
- irq_status = wait_for_irq(denali, irq_mask);
-
- if (irq_status & INTR__TIME_OUT)
- dev_err(denali->dev, "reset bank failed.\n");
-}
-
/* Reset the flash controller */
static uint16_t denali_nand_reset(struct denali_nand_info *denali)
{
@@ -201,169 +164,123 @@ static void detect_max_banks(struct denali_nand_info *denali)
denali->max_banks <<= 1;
}
-static void denali_set_intr_modes(struct denali_nand_info *denali,
- uint16_t INT_ENABLE)
+static void denali_enable_irq(struct denali_nand_info *denali)
{
- if (INT_ENABLE)
- iowrite32(1, denali->flash_reg + GLOBAL_INT_ENABLE);
- else
- iowrite32(0, denali->flash_reg + GLOBAL_INT_ENABLE);
-}
+ int i;
-/*
- * validation function to verify that the controlling software is making
- * a valid request
- */
-static inline bool is_flash_bank_valid(int flash_bank)
-{
- return flash_bank >= 0 && flash_bank < 4;
+ for (i = 0; i < DENALI_NR_BANKS; i++)
+ iowrite32(U32_MAX, denali->flash_reg + INTR_EN(i));
+ iowrite32(GLOBAL_INT_EN_FLAG, denali->flash_reg + GLOBAL_INT_ENABLE);
}
-static void denali_irq_init(struct denali_nand_info *denali)
+static void denali_disable_irq(struct denali_nand_info *denali)
{
- uint32_t int_mask;
int i;
- /* Disable global interrupts */
- denali_set_intr_modes(denali, false);
-
- int_mask = DENALI_IRQ_ALL;
-
- /* Clear all status bits */
- for (i = 0; i < denali->max_banks; ++i)
- iowrite32(0xFFFF, denali->flash_reg + INTR_STATUS(i));
-
- denali_irq_enable(denali, int_mask);
+ for (i = 0; i < DENALI_NR_BANKS; i++)
+ iowrite32(0, denali->flash_reg + INTR_EN(i));
+ iowrite32(0, denali->flash_reg + GLOBAL_INT_ENABLE);
}
-static void denali_irq_cleanup(int irqnum, struct denali_nand_info *denali)
+static void denali_clear_irq(struct denali_nand_info *denali,
+ int bank, uint32_t irq_status)
{
- denali_set_intr_modes(denali, false);
+ /* write one to clear bits */
+ iowrite32(irq_status, denali->flash_reg + INTR_STATUS(bank));
}
-static void denali_irq_enable(struct denali_nand_info *denali,
- uint32_t int_mask)
+static void denali_clear_irq_all(struct denali_nand_info *denali)
{
int i;
- for (i = 0; i < denali->max_banks; ++i)
- iowrite32(int_mask, denali->flash_reg + INTR_EN(i));
+ for (i = 0; i < DENALI_NR_BANKS; i++)
+ denali_clear_irq(denali, i, U32_MAX);
}
-/*
- * This function only returns when an interrupt that this driver cares about
- * occurs. This is to reduce the overhead of servicing interrupts
- */
-static inline uint32_t denali_irq_detected(struct denali_nand_info *denali)
+static irqreturn_t denali_isr(int irq, void *dev_id)
{
- return read_interrupt_status(denali) & DENALI_IRQ_ALL;
-}
+ struct denali_nand_info *denali = dev_id;
+ irqreturn_t ret = IRQ_NONE;
+ uint32_t irq_status;
+ int i;
-/* Interrupts are cleared by writing a 1 to the appropriate status bit */
-static inline void clear_interrupt(struct denali_nand_info *denali,
- uint32_t irq_mask)
-{
- uint32_t intr_status_reg;
+ spin_lock(&denali->irq_lock);
- intr_status_reg = INTR_STATUS(denali->flash_bank);
+ for (i = 0; i < DENALI_NR_BANKS; i++) {
+ irq_status = ioread32(denali->flash_reg + INTR_STATUS(i));
+ if (irq_status)
+ ret = IRQ_HANDLED;
- iowrite32(irq_mask, denali->flash_reg + intr_status_reg);
-}
+ denali_clear_irq(denali, i, irq_status);
-static void clear_interrupts(struct denali_nand_info *denali)
-{
- uint32_t status;
+ if (i != denali->flash_bank)
+ continue;
- spin_lock_irq(&denali->irq_lock);
+ denali->irq_status |= irq_status;
- status = read_interrupt_status(denali);
- clear_interrupt(denali, status);
+ if (denali->irq_status & denali->irq_mask)
+ complete(&denali->complete);
+ }
+
+ spin_unlock(&denali->irq_lock);
- denali->irq_status = 0x0;
- spin_unlock_irq(&denali->irq_lock);
+ return ret;
}
-static uint32_t read_interrupt_status(struct denali_nand_info *denali)
+static void denali_reset_irq(struct denali_nand_info *denali)
{
- uint32_t intr_status_reg;
-
- intr_status_reg = INTR_STATUS(denali->flash_bank);
+ unsigned long flags;
- return ioread32(denali->flash_reg + intr_status_reg);
+ spin_lock_irqsave(&denali->irq_lock, flags);
+ denali->irq_status = 0;
+ denali->irq_mask = 0;
+ spin_unlock_irqrestore(&denali->irq_lock, flags);
}
-/*
- * This is the interrupt service routine. It handles all interrupts
- * sent to this device. Note that on CE4100, this is a shared interrupt.
- */
-static irqreturn_t denali_isr(int irq, void *dev_id)
+static uint32_t denali_wait_for_irq(struct denali_nand_info *denali,
+ uint32_t irq_mask)
{
- struct denali_nand_info *denali = dev_id;
+ unsigned long time_left, flags;
uint32_t irq_status;
- irqreturn_t result = IRQ_NONE;
- spin_lock(&denali->irq_lock);
+ spin_lock_irqsave(&denali->irq_lock, flags);
- /* check to see if a valid NAND chip has been selected. */
- if (is_flash_bank_valid(denali->flash_bank)) {
- /*
- * check to see if controller generated the interrupt,
- * since this is a shared interrupt
- */
- irq_status = denali_irq_detected(denali);
- if (irq_status != 0) {
- /* handle interrupt */
- /* first acknowledge it */
- clear_interrupt(denali, irq_status);
- /*
- * store the status in the device context for someone
- * to read
- */
- denali->irq_status |= irq_status;
- /* notify anyone who cares that it happened */
- complete(&denali->complete);
- /* tell the OS that we've handled this */
- result = IRQ_HANDLED;
- }
+ irq_status = denali->irq_status;
+
+ if (irq_mask & irq_status) {
+ spin_unlock_irqrestore(&denali->irq_lock, flags);
+ return irq_status;
}
- spin_unlock(&denali->irq_lock);
- return result;
+
+ denali->irq_mask = irq_mask;
+ reinit_completion(&denali->complete);
+ spin_unlock_irqrestore(&denali->irq_lock, flags);
+
+ time_left = wait_for_completion_timeout(&denali->complete,
+ msecs_to_jiffies(1000));
+ if (!time_left) {
+ dev_err(denali->dev, "timeout while waiting for irq 0x%x\n",
+ denali->irq_mask);
+ return 0;
+ }
+
+ return denali->irq_status;
}
-static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
+/* resets a specific device connected to the core */
+static void reset_bank(struct denali_nand_info *denali)
{
- unsigned long comp_res;
- uint32_t intr_status;
- unsigned long timeout = msecs_to_jiffies(1000);
+ uint32_t irq_status;
- do {
- comp_res =
- wait_for_completion_timeout(&denali->complete, timeout);
- spin_lock_irq(&denali->irq_lock);
- intr_status = denali->irq_status;
-
- if (intr_status & irq_mask) {
- denali->irq_status &= ~irq_mask;
- spin_unlock_irq(&denali->irq_lock);
- /* our interrupt was detected */
- break;
- }
+ denali_reset_irq(denali);
- /*
- * these are not the interrupts you are looking for -
- * need to wait again
- */
- spin_unlock_irq(&denali->irq_lock);
- } while (comp_res != 0);
+ iowrite32(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
- if (comp_res == 0) {
- /* timeout */
- pr_err("timeout occurred, status = 0x%x, mask = 0x%x\n",
- intr_status, irq_mask);
+ irq_status = denali_wait_for_irq(denali,
+ INTR__RST_COMP | INTR__TIME_OUT);
- intr_status = 0;
- }
- return intr_status;
+ if (!(irq_status & INTR__RST_COMP))
+ dev_err(denali->dev, "reset bank failed.\n");
}
/*
@@ -397,7 +314,7 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
setup_ecc_for_xfer(denali, ecc_en, transfer_spare);
- clear_interrupts(denali);
+ denali_reset_irq(denali);
addr = BANK(denali->flash_bank) | denali->page;
@@ -479,9 +396,9 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
write_data_to_flash_mem(denali, buf, mtd->oobsize);
/* wait for operation to complete */
- irq_status = wait_for_irq(denali, irq_mask);
+ irq_status = denali_wait_for_irq(denali, irq_mask);
- if (irq_status == 0) {
+ if (!(irq_status & INTR__PROGRAM_COMP)) {
dev_err(denali->dev, "OOB write failed\n");
status = -EIO;
}
@@ -510,9 +427,9 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
* can always use status0 bit as the
* mask is identical for each bank.
*/
- irq_status = wait_for_irq(denali, irq_mask);
+ irq_status = denali_wait_for_irq(denali, irq_mask);
- if (irq_status == 0)
+ if (!(irq_status & INTR__LOAD_COMP))
dev_err(denali->dev, "page on OOB timeout %d\n",
denali->page);
@@ -620,9 +537,9 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
unsigned int err_byte, err_sector, err_device;
uint8_t err_cor_value;
unsigned int prev_sector = 0;
+ uint32_t irq_status;
- /* read the ECC errors. we'll ignore them for now */
- denali_set_intr_modes(denali, false);
+ denali_reset_irq(denali);
do {
err_addr = ioread32(denali->flash_reg + ECC_ERROR_ADDRESS);
@@ -674,10 +591,9 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
* ECC_TRANSACTION_DONE interrupt, so here just wait for
* a while for this interrupt
*/
- while (!(read_interrupt_status(denali) & INTR__ECC_TRANSACTION_DONE))
- cpu_relax();
- clear_interrupts(denali);
- denali_set_intr_modes(denali, true);
+ irq_status = denali_wait_for_irq(denali, INTR__ECC_TRANSACTION_DONE);
+ if (!(irq_status & INTR__ECC_TRANSACTION_DONE))
+ return -EIO;
return max_bitflips;
}
@@ -778,15 +694,14 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
dma_sync_single_for_device(denali->dev, addr, size, DMA_TO_DEVICE);
- clear_interrupts(denali);
+ denali_reset_irq(denali);
denali_enable_dma(denali, true);
denali_setup_dma(denali, DENALI_WRITE);
/* wait for operation to complete */
- irq_status = wait_for_irq(denali, irq_mask);
-
- if (irq_status == 0) {
+ irq_status = denali_wait_for_irq(denali, irq_mask);
+ if (!(irq_status & INTR__DMA_CMD_COMP)) {
dev_err(denali->dev, "timeout on write_page (type = %d)\n",
raw_xfer);
ret = -EIO;
@@ -865,11 +780,11 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
denali_enable_dma(denali, true);
dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
- clear_interrupts(denali);
+ denali_reset_irq(denali);
denali_setup_dma(denali, DENALI_READ);
/* wait for operation to complete */
- irq_status = wait_for_irq(denali, irq_mask);
+ irq_status = denali_wait_for_irq(denali, irq_mask);
dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
@@ -901,6 +816,7 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
dma_addr_t addr = denali->buf.dma_buf;
size_t size = mtd->writesize + mtd->oobsize;
uint32_t irq_mask = INTR__DMA_CMD_COMP;
+ uint32_t irq_status;
denali->page = page;
@@ -909,11 +825,13 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
- clear_interrupts(denali);
+ denali_reset_irq(denali);
denali_setup_dma(denali, DENALI_READ);
/* wait for operation to complete */
- wait_for_irq(denali, irq_mask);
+ irq_status = denali_wait_for_irq(denali, irq_mask);
+ if (irq_status & INTR__DMA_CMD_COMP)
+ return -ETIMEDOUT;
dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
@@ -940,9 +858,7 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
{
struct denali_nand_info *denali = mtd_to_denali(mtd);
- spin_lock_irq(&denali->irq_lock);
denali->flash_bank = chip;
- spin_unlock_irq(&denali->irq_lock);
}
static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
@@ -953,19 +869,19 @@ static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
static int denali_erase(struct mtd_info *mtd, int page)
{
struct denali_nand_info *denali = mtd_to_denali(mtd);
-
uint32_t cmd, irq_status;
- clear_interrupts(denali);
+ denali_reset_irq(denali);
/* setup page read request for access type */
cmd = MODE_10 | BANK(denali->flash_bank) | page;
index_addr(denali, cmd, 0x1);
/* wait for erase to complete or failure to occur */
- irq_status = wait_for_irq(denali, INTR__ERASE_COMP | INTR__ERASE_FAIL);
+ irq_status = denali_wait_for_irq(denali,
+ INTR__ERASE_COMP | INTR__ERASE_FAIL);
- return irq_status & INTR__ERASE_FAIL ? NAND_STATUS_FAIL : PASS;
+ return irq_status & INTR__ERASE_COMP ? 0 : NAND_STATUS_FAIL;
}
static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
@@ -1153,7 +1069,6 @@ static void denali_hw_init(struct denali_nand_info *denali)
/* Should set value for these registers when init */
iowrite32(0, denali->flash_reg + TWO_ROW_ADDR_CYCLES);
iowrite32(1, denali->flash_reg + ECC_ENABLE);
- denali_irq_init(denali);
}
static int denali_calc_ecc_bytes(int ecc_size, int ecc_strength)
@@ -1271,9 +1186,6 @@ static void denali_drv_init(struct denali_nand_info *denali)
/* indicate that MTD has not selected a valid bank yet */
denali->flash_bank = CHIP_SELECT_INVALID;
-
- /* initialize our irq_status variable to indicate no interrupts */
- denali->irq_status = 0;
}
static int denali_multidev_fixup(struct denali_nand_info *denali)
@@ -1343,6 +1255,8 @@ int denali_init(struct denali_nand_info *denali)
denali_hw_init(denali);
denali_drv_init(denali);
+ denali_clear_irq_all(denali);
+
/* Request IRQ after all the hardware initialization is finished */
ret = devm_request_irq(denali->dev, denali->irq, denali_isr,
IRQF_SHARED, DENALI_NAND_NAME, denali);
@@ -1351,8 +1265,8 @@ int denali_init(struct denali_nand_info *denali)
return ret;
}
- /* now that our ISR is registered, we can enable interrupts */
- denali_set_intr_modes(denali, true);
+ denali_enable_irq(denali);
+
nand_set_flash_node(chip, denali->dev->of_node);
/* Fallback to the default name if DT did not give "label" property */
if (!mtd->name)
@@ -1374,7 +1288,7 @@ int denali_init(struct denali_nand_info *denali)
*/
ret = nand_scan_ident(mtd, denali->max_banks, NULL);
if (ret)
- goto failed_req_irq;
+ goto disable_irq;
/* allocate the right size buffer now */
devm_kfree(denali->dev, denali->buf.buf);
@@ -1383,7 +1297,7 @@ int denali_init(struct denali_nand_info *denali)
GFP_KERNEL);
if (!denali->buf.buf) {
ret = -ENOMEM;
- goto failed_req_irq;
+ goto disable_irq;
}
ret = dma_set_mask(denali->dev,
@@ -1391,7 +1305,7 @@ int denali_init(struct denali_nand_info *denali)
64 : 32));
if (ret) {
dev_err(denali->dev, "No usable DMA configuration\n");
- goto failed_req_irq;
+ goto disable_irq;
}
denali->buf.dma_buf = dma_map_single(denali->dev, denali->buf.buf,
@@ -1400,7 +1314,7 @@ int denali_init(struct denali_nand_info *denali)
if (dma_mapping_error(denali->dev, denali->buf.dma_buf)) {
dev_err(denali->dev, "Failed to map DMA buffer\n");
ret = -EIO;
- goto failed_req_irq;
+ goto disable_irq;
}
/*
@@ -1442,7 +1356,7 @@ int denali_init(struct denali_nand_info *denali)
!(chip->ecc.size == 1024 && denali->caps & DENALI_CAP_ECC_SIZE_1024)) {
dev_err(denali->dev, "ECC size %d is not supported on this controller",
chip->ecc.size);
- goto failed_req_irq;
+ goto disable_irq;
}
if (!chip->ecc.strength && !(chip->ecc.options & NAND_ECC_MAXIMIZE)) {
@@ -1454,13 +1368,13 @@ int denali_init(struct denali_nand_info *denali)
if (chip->ecc.options & NAND_ECC_MAXIMIZE) {
ret = denali_set_max_ecc_strength(denali);
if (ret)
- goto failed_req_irq;
+ goto disable_irq;
} else if (!(denali->ecc_strength_avail & BIT(chip->ecc.strength))) {
dev_err(denali->dev,
"ECC strength %d is not supported on this controller.\n",
chip->ecc.strength);
ret = -EINVAL;
- goto failed_req_irq;
+ goto disable_irq;
}
chip->ecc.bytes = denali_calc_ecc_bytes(chip->ecc.size,
@@ -1493,21 +1407,21 @@ int denali_init(struct denali_nand_info *denali)
ret = denali_multidev_fixup(denali);
if (ret)
- goto failed_req_irq;
+ goto disable_irq;
ret = nand_scan_tail(mtd);
if (ret)
- goto failed_req_irq;
+ goto disable_irq;
ret = mtd_device_register(mtd, NULL, 0);
if (ret) {
dev_err(denali->dev, "Failed to register MTD: %d\n", ret);
- goto failed_req_irq;
+ goto disable_irq;
}
return 0;
-failed_req_irq:
- denali_irq_cleanup(denali->irq, denali);
+disable_irq:
+ denali_disable_irq(denali);
return ret;
}
@@ -1525,7 +1439,7 @@ void denali_remove(struct denali_nand_info *denali)
int bufsize = mtd->writesize + mtd->oobsize;
nand_release(mtd);
- denali_irq_cleanup(denali->irq, denali);
+ denali_disable_irq(denali);
dma_unmap_single(denali->dev, denali->buf.dma_buf, bufsize,
DMA_BIDIRECTIONAL);
}
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index e55bb2e..7797340 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -326,6 +326,7 @@ struct denali_nand_info {
/* elements used by ISR */
struct completion complete;
spinlock_t irq_lock;
+ uint32_t irq_mask;
uint32_t irq_status;
int irq;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 08/37] mtd: nand: denali: support 64bit capable DMA engine |
| Message-ID | <tqCdR-2l8-39@gated-at.bofh.it> |
| In reply to | #1612622 |
The current driver only supports the DMA engine up to 32 bit
physical address, but there also exists 64 bit capable DMA engine
for this IP.
The data DMA setup sequence is completely different, so I added the
64 bit DMA code as a new function denali_setup_dma64(). The 32 bit
one has been renamed to denali_setup_dma32().
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3:
- Move DENALI_CAP_ define out of struct denali_nand_info
Changes in v2:
- Change the capability prefix DENALI_CAPS_ -> DENALI_CAP_
drivers/mtd/nand/denali.c | 39 +++++++++++++++++++++++++++++++++++----
drivers/mtd/nand/denali.h | 1 +
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index b95e33a..417a895 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -979,8 +979,30 @@ static void denali_enable_dma(struct denali_nand_info *denali, bool en)
ioread32(denali->flash_reg + DMA_ENABLE);
}
-/* setups the HW to perform the data DMA */
-static void denali_setup_dma(struct denali_nand_info *denali, int op)
+static void denali_setup_dma64(struct denali_nand_info *denali, int op)
+{
+ uint32_t mode;
+ const int page_count = 1;
+ uint64_t addr = denali->buf.dma_buf;
+
+ mode = MODE_10 | BANK(denali->flash_bank) | denali->page;
+
+ /* DMA is a three step process */
+
+ /*
+ * 1. setup transfer type, interrupt when complete,
+ * burst len = 64 bytes, the number of pages
+ */
+ index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);
+
+ /* 2. set memory low address */
+ index_addr(denali, mode, addr);
+
+ /* 3. set memory high address */
+ index_addr(denali, mode, addr >> 32);
+}
+
+static void denali_setup_dma32(struct denali_nand_info *denali, int op)
{
uint32_t mode;
const int page_count = 1;
@@ -1003,6 +1025,14 @@ static void denali_setup_dma(struct denali_nand_info *denali, int op)
index_addr(denali, mode | 0x14000, 0x2400);
}
+static void denali_setup_dma(struct denali_nand_info *denali, int op)
+{
+ if (denali->caps & DENALI_CAP_DMA_64BIT)
+ denali_setup_dma64(denali, op);
+ else
+ denali_setup_dma32(denali, op);
+}
+
/*
* writes a page. user specifies type, and this function handles the
* configuration details.
@@ -1518,8 +1548,9 @@ int denali_init(struct denali_nand_info *denali)
goto failed_req_irq;
}
- /* Is 32-bit DMA supported? */
- ret = dma_set_mask(denali->dev, DMA_BIT_MASK(32));
+ ret = dma_set_mask(denali->dev,
+ DMA_BIT_MASK(denali->caps & DENALI_CAP_DMA_64BIT ?
+ 64 : 32));
if (ret) {
dev_err(denali->dev, "No usable DMA configuration\n");
goto failed_req_irq;
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index e532956..1f413d0 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -347,6 +347,7 @@ struct denali_nand_info {
};
#define DENALI_CAP_HW_ECC_FIXUP BIT(0)
+#define DENALI_CAP_DMA_64BIT BIT(1)
extern int denali_init(struct denali_nand_info *denali);
extern void denali_remove(struct denali_nand_info *denali);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 18/37] mtd: nand: denali: do not propagate NAND_STATUS_FAIL to waitfunc() |
| Message-ID | <tqCdR-2l8-41@gated-at.bofh.it> |
| In reply to | #1612622 |
Currently, the error handling of denali_write_page(_raw) is a bit
complicated. If the program command fails, NAND_STATUS_FAIL is set
to the driver internal denali->status, then read out later by
denali_waitfunc().
We can avoid it by exploiting the nand_write_page() implementation.
If chip->ecc.write_page(_raw) returns negative code (i.e. -EIO), it
errors out immediately. This gives the same result as returning
NAND_STATUS_FAIL from chip->waitfunc. In either way, -EIO is
returned to the upper MTD layer.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Newly added
drivers/mtd/nand/denali.c | 12 ++++--------
drivers/mtd/nand/denali.h | 1 -
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 79851ca..5da8156 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1005,6 +1005,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
size_t size = mtd->writesize + mtd->oobsize;
uint32_t irq_status;
uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
+ int ret = 0;
denali->page = page;
@@ -1038,13 +1039,13 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
if (irq_status == 0) {
dev_err(denali->dev, "timeout on write_page (type = %d)\n",
raw_xfer);
- denali->status = NAND_STATUS_FAIL;
+ ret = -EIO;
}
denali_enable_dma(denali, false);
dma_sync_single_for_cpu(denali->dev, addr, size, DMA_TO_DEVICE);
- return 0;
+ return ret;
}
/* NAND core entry points */
@@ -1196,12 +1197,7 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
{
- struct denali_nand_info *denali = mtd_to_denali(mtd);
- int status = denali->status;
-
- denali->status = 0;
-
- return status;
+ return 0;
}
static int denali_erase(struct mtd_info *mtd, int page)
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 00ce04e..9e2b787 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -329,7 +329,6 @@ struct nand_buf {
struct denali_nand_info {
struct nand_chip nand;
int flash_bank; /* currently selected chip */
- int status;
int platform;
struct nand_buf buf;
struct device *dev;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Boris Brezillon <boris.brezillon@free-electrons.com> |
|---|---|
| Date | 2017-03-30 17:20 +0200 |
| Subject | Re: [PATCH v3 18/37] mtd: nand: denali: do not propagate NAND_STATUS_FAIL to waitfunc() |
| Message-ID | <tqK1H-8eB-3@gated-at.bofh.it> |
| In reply to | #1612630 |
On Thu, 30 Mar 2017 15:46:04 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Currently, the error handling of denali_write_page(_raw) is a bit
> complicated. If the program command fails, NAND_STATUS_FAIL is set
> to the driver internal denali->status, then read out later by
> denali_waitfunc().
>
> We can avoid it by exploiting the nand_write_page() implementation.
> If chip->ecc.write_page(_raw) returns negative code (i.e. -EIO), it
> errors out immediately. This gives the same result as returning
> NAND_STATUS_FAIL from chip->waitfunc. In either way, -EIO is
> returned to the upper MTD layer.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Newly added
>
> drivers/mtd/nand/denali.c | 12 ++++--------
> drivers/mtd/nand/denali.h | 1 -
> 2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 79851ca..5da8156 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1005,6 +1005,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
> size_t size = mtd->writesize + mtd->oobsize;
> uint32_t irq_status;
> uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
> + int ret = 0;
>
> denali->page = page;
>
> @@ -1038,13 +1039,13 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
> if (irq_status == 0) {
> dev_err(denali->dev, "timeout on write_page (type = %d)\n",
> raw_xfer);
> - denali->status = NAND_STATUS_FAIL;
> + ret = -EIO;
> }
>
> denali_enable_dma(denali, false);
> dma_sync_single_for_cpu(denali->dev, addr, size, DMA_TO_DEVICE);
>
> - return 0;
> + return ret;
> }
>
> /* NAND core entry points */
> @@ -1196,12 +1197,7 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
>
> static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
> {
> - struct denali_nand_info *denali = mtd_to_denali(mtd);
> - int status = denali->status;
> -
> - denali->status = 0;
> -
> - return status;
> + return 0;
I know it's not your fault, and the existing denali_waitfunc() is
already buggy, but it's definitely wrong to return 0 here without
checking the NAND status.
->waitfunc() is not only used to wait for a page-program operation,
it's used every time the NAND enters the busy state (lock, unlock,
set_features, ...).
Anyway, I'll review the remaining patches before taking a decision.
> }
>
> static int denali_erase(struct mtd_info *mtd, int page)
> diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
> index 00ce04e..9e2b787 100644
> --- a/drivers/mtd/nand/denali.h
> +++ b/drivers/mtd/nand/denali.h
> @@ -329,7 +329,6 @@ struct nand_buf {
> struct denali_nand_info {
> struct nand_chip nand;
> int flash_bank; /* currently selected chip */
> - int status;
> int platform;
> struct nand_buf buf;
> struct device *dev;
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 09/37] mtd: nand: denali_dt: remove dma-mask DT property |
| Message-ID | <tqCdQ-2l8-23@gated-at.bofh.it> |
| In reply to | #1612622 |
The driver sets appropriate DMA mask. Delete the "dma-mask" DT
property. See [1] for negative comments for this binding.
[1] https://lkml.org/lkml/2016/2/8/57
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/mtd/denali-nand.txt | 2 --
drivers/mtd/nand/denali_dt.c | 9 ---------
2 files changed, 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index 6f4ab4c..e593bbe 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -6,7 +6,6 @@ Required properties:
- reg : should contain registers location and length for data and reg.
- reg-names: Should contain the reg names "nand_data" and "denali_reg"
- interrupts : The interrupt number.
- - dm-mask : DMA bit mask
The device tree may optionally contain sub-nodes describing partitions of the
address space. See partition.txt for more detail.
@@ -20,5 +19,4 @@ nand: nand@ff900000 {
reg = <0xff900000 0x100000>, <0xffb80000 0x10000>;
reg-names = "nand_data", "denali_reg";
interrupts = <0 144 4>;
- dma-mask = <0xffffffff>;
};
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index 9577bfd..b8a8284 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -46,8 +46,6 @@ static const struct of_device_id denali_nand_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, denali_nand_dt_ids);
-static u64 denali_dma_mask;
-
static int denali_dt_probe(struct platform_device *ofdev)
{
struct resource *denali_reg, *nand_data;
@@ -83,13 +81,6 @@ static int denali_dt_probe(struct platform_device *ofdev)
if (IS_ERR(denali->flash_mem))
return PTR_ERR(denali->flash_mem);
- if (!of_property_read_u32(ofdev->dev.of_node,
- "dma-mask", (u32 *)&denali_dma_mask)) {
- denali->dev->dma_mask = &denali_dma_mask;
- } else {
- denali->dev->dma_mask = NULL;
- }
-
dt->clk = devm_clk_get(&ofdev->dev, NULL);
if (IS_ERR(dt->clk)) {
dev_err(&ofdev->dev, "no clk available\n");
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 02/37] mtd: nand: denali: allow to override mtd->name from label DT property |
| Message-ID | <tqCdR-2l8-45@gated-at.bofh.it> |
| In reply to | #1612622 |
Commit 28309572aac4 ("mtd: name the mtd device with an optional
label property") allow us to identify a chip in a user-friendly way.
If nand_set_flash_node() picks up the "label" from DT, let's respect
it. Otherwise, let it fallback to the current name "denali-nand".
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
Changes in v3:
- Replaced "mtd: nand: denali: do not set mtd->name" in v2
Changes in v2: None
drivers/mtd/nand/denali.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 4ca75d3..4e6d03d 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1462,8 +1462,10 @@ int denali_init(struct denali_nand_info *denali)
/* now that our ISR is registered, we can enable interrupts */
denali_set_intr_modes(denali, true);
- mtd->name = "denali-nand";
nand_set_flash_node(chip, denali->dev->of_node);
+ /* Fallback to the default name if DT did not give "label" property */
+ if (!mtd->name)
+ mtd->name = "denali-nand";
/* register the driver with the NAND core subsystem */
chip->select_chip = denali_select_chip;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 12/37] mtd: nand: denali: support 1024 byte ECC step size |
| Message-ID | <tqCdR-2l8-43@gated-at.bofh.it> |
| In reply to | #1612622 |
This driver was originally written for the Intel MRST platform with
several platform specific parameters hard-coded. Another thing we
need to fix is the hard-coded ECC step size. Currently, it is
defined as follows:
#define ECC_SECTOR_SIZE 512
(somehow, it is defined in both denali.c and denali.h)
This must be avoided because the Denali IP supports 1024B ECC size
as well. The Denali User's Guide also says supporting both 512B and
1024B ECC sectors is possible, though it would require instantiation
of two different ECC circuits. So, possible cases are:
[1] only 512B ECC size is supported
[2] only 1024B ECC size is supported
[3] both 512B and 1024B ECC sizes are supported
Newer versions of this IP need ecc.size and ecc.steps explicitly
set up via the following registers:
CFG_DATA_BLOCK_SIZE (0x6b0)
CFG_LAST_DATA_BLOCK_SIZE (0x6c0)
CFG_NUM_DATA_BLOCKS (0x6d0)
Older versions do not have such registers (they were reserved), so
write accesses are safely ignored.
This commit adds new flags DENALI_CAP_ECC_SIZE_{512,1024}.
The DT property "nand-ecc-step-size" is still optional; a reasonable
default will be chosen for [1] and [2]. For case [3], users can
force ECC size via DT in case firmware hard-codes ECC settings.
If not specified, the driver will use chip's ECC requirement as a
hint to decide the ECC size.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes in v3:
- Move DENALI_CAP_ define out of struct denali_nand_info
- Use chip->ecc_step_ds as a hint to choose chip->ecc.size
where possible
Changes in v2:
- Change the capability prefix DENALI_CAPS_ -> DENALI_CAP_
- Make ECC 512 cap and ECC 1024 cap independent
- Set up three CFG_... registers
.../devicetree/bindings/mtd/denali-nand.txt | 5 +++
drivers/mtd/nand/denali.c | 51 ++++++++++++++++------
drivers/mtd/nand/denali.h | 12 ++++-
drivers/mtd/nand/denali_dt.c | 3 +-
drivers/mtd/nand/denali_pci.c | 2 +
5 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index e593bbe..25313c7 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -7,6 +7,11 @@ Required properties:
- reg-names: Should contain the reg names "nand_data" and "denali_reg"
- interrupts : The interrupt number.
+Optional properties:
+ - nand-ecc-step-size: must be 512 or 1024. If not specified, default to:
+ 512 for "altr,socfpga-denali-nand"
+ see nand.txt for details.
+
The device tree may optionally contain sub-nodes describing partitions of the
address space. See partition.txt for more detail.
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 16634df..78d3b18 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -886,8 +886,6 @@ static int denali_hw_ecc_fixup(struct mtd_info *mtd,
return max_bitflips;
}
-#define ECC_SECTOR_SIZE 512
-
#define ECC_SECTOR(x) (((x) & ECC_ERROR_ADDRESS__SECTOR_NR) >> 12)
#define ECC_BYTE(x) (((x) & ECC_ERROR_ADDRESS__OFFSET))
#define ECC_CORRECTION_VALUE(x) ((x) & ERR_CORRECTION_INFO__BYTEMASK)
@@ -899,6 +897,7 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
struct denali_nand_info *denali,
unsigned long *uncor_ecc_flags, uint8_t *buf)
{
+ unsigned int ecc_size = denali->nand.ecc.size;
unsigned int bitflips = 0;
unsigned int max_bitflips = 0;
uint32_t err_addr, err_cor_info;
@@ -928,9 +927,9 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
* an erased sector.
*/
*uncor_ecc_flags |= BIT(err_sector);
- } else if (err_byte < ECC_SECTOR_SIZE) {
+ } else if (err_byte < ecc_size) {
/*
- * If err_byte is larger than ECC_SECTOR_SIZE, means error
+ * If err_byte is larger than ecc_size, means error
* happened in OOB, so we ignore it. It's no need for
* us to correct it err_device is represented the NAND
* error bits are happened in if there are more than
@@ -939,7 +938,7 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
int offset;
unsigned int flips_in_byte;
- offset = (err_sector * ECC_SECTOR_SIZE + err_byte) *
+ offset = (err_sector * ecc_size + err_byte) *
denali->devnum + err_device;
/* correct the ECC error */
@@ -1587,21 +1586,43 @@ int denali_init(struct denali_nand_info *denali)
chip->options |= NAND_NO_SUBPAGE_WRITE;
/*
+ * If the controller supports both 512B and 1024B ECC, and DT does not
+ * specify "nand-ecc-step-size", use the chip's requirement for a hint
+ * to choose ECC size.
+ */
+ if (!chip->ecc.size &&
+ denali->caps & DENALI_CAP_ECC_SIZE_512 &&
+ denali->caps & DENALI_CAP_ECC_SIZE_1024)
+ chip->ecc.size = chip->ecc_step_ds;
+
+ if (!chip->ecc.size) {
+ if (denali->caps & DENALI_CAP_ECC_SIZE_512)
+ chip->ecc.size = 512;
+ if (denali->caps & DENALI_CAP_ECC_SIZE_1024)
+ chip->ecc.size = 1024;
+ }
+
+ if (!(chip->ecc.size == 512 && denali->caps & DENALI_CAP_ECC_SIZE_512) &&
+ !(chip->ecc.size == 1024 && denali->caps & DENALI_CAP_ECC_SIZE_1024)) {
+ dev_err(denali->dev, "ECC size %d is not supported on this controller",
+ chip->ecc.size);
+ goto failed_req_irq;
+ }
+
+ /*
* Denali Controller only support 15bit and 8bit ECC in MRST,
* so just let controller do 15bit ECC for MLC and 8bit ECC for
* SLC if possible.
* */
if (!nand_is_slc(chip) &&
- (mtd->oobsize > (denali->bbtskipbytes +
- ECC_15BITS * (mtd->writesize /
- ECC_SECTOR_SIZE)))) {
+ mtd->oobsize > denali->bbtskipbytes +
+ ECC_15BITS * (mtd->writesize / chip->ecc.size)) {
/* if MLC OOB size is large enough, use 15bit ECC*/
chip->ecc.strength = 15;
chip->ecc.bytes = ECC_15BITS;
iowrite32(15, denali->flash_reg + ECC_CORRECTION);
- } else if (mtd->oobsize < (denali->bbtskipbytes +
- ECC_8BITS * (mtd->writesize /
- ECC_SECTOR_SIZE))) {
+ } else if (mtd->oobsize <
+ denali->bbtskipbytes + ECC_8BITS * (mtd->writesize / chip->ecc.size)) {
pr_err("Your NAND chip OOB is not large enough to contain 8bit ECC correction codes");
goto failed_req_irq;
} else {
@@ -1610,10 +1631,14 @@ int denali_init(struct denali_nand_info *denali)
iowrite32(8, denali->flash_reg + ECC_CORRECTION);
}
+ iowrite32(chip->ecc.size, denali->flash_reg + CFG_DATA_BLOCK_SIZE);
+ iowrite32(chip->ecc.size, denali->flash_reg + CFG_LAST_DATA_BLOCK_SIZE);
+ /* chip->ecc.steps is set by nand_scan_tail(); not available here */
+ iowrite32(mtd->writesize / chip->ecc.size,
+ denali->flash_reg + CFG_NUM_DATA_BLOCKS);
+
mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
- /* override the default read operations */
- chip->ecc.size = ECC_SECTOR_SIZE;
chip->ecc.read_page = denali_read_page;
chip->ecc.read_page_raw = denali_read_page_raw;
chip->ecc.write_page = denali_write_page;
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index ec00485..7c24d82 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -265,6 +265,14 @@
#define ECC_COR_INFO__MAX_ERRORS 0x007f
#define ECC_COR_INFO__UNCOR_ERR 0x0080
+#define CFG_DATA_BLOCK_SIZE 0x6b0
+
+#define CFG_LAST_DATA_BLOCK_SIZE 0x6c0
+
+#define CFG_NUM_DATA_BLOCKS 0x6d0
+
+#define CFG_META_DATA_SIZE 0x6e0
+
#define DMA_ENABLE 0x700
#define DMA_ENABLE__FLAG 0x0001
@@ -307,8 +315,6 @@
#define MODE_10 0x08000000
#define MODE_11 0x0C000000
-#define ECC_SECTOR_SIZE 512
-
struct nand_buf {
int head;
int tail;
@@ -347,6 +353,8 @@ struct denali_nand_info {
#define DENALI_CAP_HW_ECC_FIXUP BIT(0)
#define DENALI_CAP_DMA_64BIT BIT(1)
+#define DENALI_CAP_ECC_SIZE_512 BIT(2)
+#define DENALI_CAP_ECC_SIZE_1024 BIT(3)
extern int denali_init(struct denali_nand_info *denali);
extern void denali_remove(struct denali_nand_info *denali);
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index df9ef36..1681a30 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -35,7 +35,8 @@ struct denali_dt_data {
};
static const struct denali_dt_data denali_socfpga_data = {
- .caps = DENALI_CAP_HW_ECC_FIXUP,
+ .caps = DENALI_CAP_HW_ECC_FIXUP |
+ DENALI_CAP_ECC_SIZE_512,
};
static const struct of_device_id denali_nand_dt_ids[] = {
diff --git a/drivers/mtd/nand/denali_pci.c b/drivers/mtd/nand/denali_pci.c
index ac84323..5202a11 100644
--- a/drivers/mtd/nand/denali_pci.c
+++ b/drivers/mtd/nand/denali_pci.c
@@ -85,6 +85,8 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
goto failed_remap_reg;
}
+ denali->caps |= DENALI_CAP_ECC_SIZE_512;
+
ret = denali_init(denali);
if (ret)
goto failed_remap_mem;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-04-01 10:50 +0200 |
| Subject | Re: [PATCH v3 12/37] mtd: nand: denali: support 1024 byte ECC step size |
| Message-ID | <trmTn-cv-3@gated-at.bofh.it> |
| In reply to | #1612633 |
2017-03-30 15:45 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> This driver was originally written for the Intel MRST platform with
> several platform specific parameters hard-coded. Another thing we
> need to fix is the hard-coded ECC step size. Currently, it is
> defined as follows:
>
> #define ECC_SECTOR_SIZE 512
>
> (somehow, it is defined in both denali.c and denali.h)
>
> This must be avoided because the Denali IP supports 1024B ECC size
> as well. The Denali User's Guide also says supporting both 512B and
> 1024B ECC sectors is possible, though it would require instantiation
> of two different ECC circuits. So, possible cases are:
>
> [1] only 512B ECC size is supported
> [2] only 1024B ECC size is supported
> [3] both 512B and 1024B ECC sizes are supported
>
> Newer versions of this IP need ecc.size and ecc.steps explicitly
> set up via the following registers:
> CFG_DATA_BLOCK_SIZE (0x6b0)
> CFG_LAST_DATA_BLOCK_SIZE (0x6c0)
> CFG_NUM_DATA_BLOCKS (0x6d0)
>
> Older versions do not have such registers (they were reserved), so
> write accesses are safely ignored.
>
> This commit adds new flags DENALI_CAP_ECC_SIZE_{512,1024}.
>
> The DT property "nand-ecc-step-size" is still optional; a reasonable
> default will be chosen for [1] and [2]. For case [3], users can
> force ECC size via DT in case firmware hard-codes ECC settings.
> If not specified, the driver will use chip's ECC requirement as a
> hint to decide the ECC size.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>
> Changes in v3:
> - Move DENALI_CAP_ define out of struct denali_nand_info
> - Use chip->ecc_step_ds as a hint to choose chip->ecc.size
> where possible
>
Please hold back this patch
until we decide how to handle 14.
--
Best Regards
Masahiro Yamada
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 19/37] mtd: nand: denali: use BIT() and GENMASK() for register macros |
| Message-ID | <tqCdR-2l8-35@gated-at.bofh.it> |
| In reply to | #1612622 |
Make register field macros more readable especially for comparing the macros and the register description in the Denali User's Guide. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v3: None Changes in v2: - Newly added drivers/mtd/nand/denali.h | 244 ++++++++++++++++++++++------------------------ 1 file changed, 119 insertions(+), 125 deletions(-) diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h index 9e2b787..610b013 100644 --- a/drivers/mtd/nand/denali.h +++ b/drivers/mtd/nand/denali.h @@ -24,246 +24,240 @@ #include <linux/mtd/nand.h> #define DEVICE_RESET 0x0 -#define DEVICE_RESET__BANK0 0x0001 -#define DEVICE_RESET__BANK1 0x0002 -#define DEVICE_RESET__BANK2 0x0004 -#define DEVICE_RESET__BANK3 0x0008 +#define DEVICE_RESET__BANK(bank) BIT(bank) #define TRANSFER_SPARE_REG 0x10 -#define TRANSFER_SPARE_REG__FLAG 0x0001 +#define TRANSFER_SPARE_REG__FLAG BIT(0) #define LOAD_WAIT_CNT 0x20 -#define LOAD_WAIT_CNT__VALUE 0xffff +#define LOAD_WAIT_CNT__VALUE GENMASK(15, 0) #define PROGRAM_WAIT_CNT 0x30 -#define PROGRAM_WAIT_CNT__VALUE 0xffff +#define PROGRAM_WAIT_CNT__VALUE GENMASK(15, 0) #define ERASE_WAIT_CNT 0x40 -#define ERASE_WAIT_CNT__VALUE 0xffff +#define ERASE_WAIT_CNT__VALUE GENMASK(15, 0) #define INT_MON_CYCCNT 0x50 -#define INT_MON_CYCCNT__VALUE 0xffff +#define INT_MON_CYCCNT__VALUE GENMASK(15, 0) #define RB_PIN_ENABLED 0x60 -#define RB_PIN_ENABLED__BANK0 0x0001 -#define RB_PIN_ENABLED__BANK1 0x0002 -#define RB_PIN_ENABLED__BANK2 0x0004 -#define RB_PIN_ENABLED__BANK3 0x0008 +#define RB_PIN_ENABLED__BANK(bank) BIT(bank) #define MULTIPLANE_OPERATION 0x70 -#define MULTIPLANE_OPERATION__FLAG 0x0001 +#define MULTIPLANE_OPERATION__FLAG BIT(0) #define MULTIPLANE_READ_ENABLE 0x80 -#define MULTIPLANE_READ_ENABLE__FLAG 0x0001 +#define MULTIPLANE_READ_ENABLE__FLAG BIT(0) #define COPYBACK_DISABLE 0x90 -#define COPYBACK_DISABLE__FLAG 0x0001 +#define COPYBACK_DISABLE__FLAG BIT(0) #define CACHE_WRITE_ENABLE 0xa0 -#define CACHE_WRITE_ENABLE__FLAG 0x0001 +#define CACHE_WRITE_ENABLE__FLAG BIT(0) #define CACHE_READ_ENABLE 0xb0 -#define CACHE_READ_ENABLE__FLAG 0x0001 +#define CACHE_READ_ENABLE__FLAG BIT(0) #define PREFETCH_MODE 0xc0 -#define PREFETCH_MODE__PREFETCH_EN 0x0001 -#define PREFETCH_MODE__PREFETCH_BURST_LENGTH 0xfff0 +#define PREFETCH_MODE__PREFETCH_EN BIT(0) +#define PREFETCH_MODE__PREFETCH_BURST_LENGTH GENMASK(15, 4) #define CHIP_ENABLE_DONT_CARE 0xd0 -#define CHIP_EN_DONT_CARE__FLAG 0x01 +#define CHIP_EN_DONT_CARE__FLAG BIT(0) #define ECC_ENABLE 0xe0 -#define ECC_ENABLE__FLAG 0x0001 +#define ECC_ENABLE__FLAG BIT(0) #define GLOBAL_INT_ENABLE 0xf0 -#define GLOBAL_INT_EN_FLAG 0x01 +#define GLOBAL_INT_EN_FLAG BIT(0) #define WE_2_RE 0x100 -#define WE_2_RE__VALUE 0x003f +#define WE_2_RE__VALUE GENMASK(5, 0) #define ADDR_2_DATA 0x110 -#define ADDR_2_DATA__VALUE 0x003f +#define ADDR_2_DATA__VALUE GENMASK(5, 0) #define RE_2_WE 0x120 -#define RE_2_WE__VALUE 0x003f +#define RE_2_WE__VALUE GENMASK(5, 0) #define ACC_CLKS 0x130 -#define ACC_CLKS__VALUE 0x000f +#define ACC_CLKS__VALUE GENMASK(3, 0) #define NUMBER_OF_PLANES 0x140 -#define NUMBER_OF_PLANES__VALUE 0x0007 +#define NUMBER_OF_PLANES__VALUE GENMASK(2, 0) #define PAGES_PER_BLOCK 0x150 -#define PAGES_PER_BLOCK__VALUE 0xffff +#define PAGES_PER_BLOCK__VALUE GENMASK(15, 0) #define DEVICE_WIDTH 0x160 -#define DEVICE_WIDTH__VALUE 0x0003 +#define DEVICE_WIDTH__VALUE GENMASK(1, 0) #define DEVICE_MAIN_AREA_SIZE 0x170 -#define DEVICE_MAIN_AREA_SIZE__VALUE 0xffff +#define DEVICE_MAIN_AREA_SIZE__VALUE GENMASK(15, 0) #define DEVICE_SPARE_AREA_SIZE 0x180 -#define DEVICE_SPARE_AREA_SIZE__VALUE 0xffff +#define DEVICE_SPARE_AREA_SIZE__VALUE GENMASK(15, 0) #define TWO_ROW_ADDR_CYCLES 0x190 -#define TWO_ROW_ADDR_CYCLES__FLAG 0x0001 +#define TWO_ROW_ADDR_CYCLES__FLAG BIT(0) #define MULTIPLANE_ADDR_RESTRICT 0x1a0 -#define MULTIPLANE_ADDR_RESTRICT__FLAG 0x0001 +#define MULTIPLANE_ADDR_RESTRICT__FLAG BIT(0) #define ECC_CORRECTION 0x1b0 -#define ECC_CORRECTION__VALUE 0x001f +#define ECC_CORRECTION__VALUE GENMASK(4, 0) #define READ_MODE 0x1c0 -#define READ_MODE__VALUE 0x000f +#define READ_MODE__VALUE GENMASK(3, 0) #define WRITE_MODE 0x1d0 -#define WRITE_MODE__VALUE 0x000f +#define WRITE_MODE__VALUE GENMASK(3, 0) #define COPYBACK_MODE 0x1e0 -#define COPYBACK_MODE__VALUE 0x000f +#define COPYBACK_MODE__VALUE GENMASK(3, 0) #define RDWR_EN_LO_CNT 0x1f0 -#define RDWR_EN_LO_CNT__VALUE 0x001f +#define RDWR_EN_LO_CNT__VALUE GENMASK(4, 0) #define RDWR_EN_HI_CNT 0x200 -#define RDWR_EN_HI_CNT__VALUE 0x001f +#define RDWR_EN_HI_CNT__VALUE GENMASK(4, 0) #define MAX_RD_DELAY 0x210 -#define MAX_RD_DELAY__VALUE 0x000f +#define MAX_RD_DELAY__VALUE GENMASK(3, 0) #define CS_SETUP_CNT 0x220 -#define CS_SETUP_CNT__VALUE 0x001f +#define CS_SETUP_CNT__VALUE GENMASK(4, 0) #define SPARE_AREA_SKIP_BYTES 0x230 -#define SPARE_AREA_SKIP_BYTES__VALUE 0x003f +#define SPARE_AREA_SKIP_BYTES__VALUE GENMASK(5, 0) #define SPARE_AREA_MARKER 0x240 -#define SPARE_AREA_MARKER__VALUE 0xffff +#define SPARE_AREA_MARKER__VALUE GENMASK(15, 0) #define DEVICES_CONNECTED 0x250 -#define DEVICES_CONNECTED__VALUE 0x0007 +#define DEVICES_CONNECTED__VALUE GENMASK(2, 0) #define DIE_MASK 0x260 -#define DIE_MASK__VALUE 0x00ff +#define DIE_MASK__VALUE GENMASK(7, 0) #define FIRST_BLOCK_OF_NEXT_PLANE 0x270 -#define FIRST_BLOCK_OF_NEXT_PLANE__VALUE 0xffff +#define FIRST_BLOCK_OF_NEXT_PLANE__VALUE GENMASK(15, 0) #define WRITE_PROTECT 0x280 -#define WRITE_PROTECT__FLAG 0x0001 +#define WRITE_PROTECT__FLAG BIT(0) #define RE_2_RE 0x290 -#define RE_2_RE__VALUE 0x003f +#define RE_2_RE__VALUE GENMASK(5, 0) #define MANUFACTURER_ID 0x300 -#define MANUFACTURER_ID__VALUE 0x00ff +#define MANUFACTURER_ID__VALUE GENMASK(7, 0) #define DEVICE_ID 0x310 -#define DEVICE_ID__VALUE 0x00ff +#define DEVICE_ID__VALUE GENMASK(7, 0) #define DEVICE_PARAM_0 0x320 -#define DEVICE_PARAM_0__VALUE 0x00ff +#define DEVICE_PARAM_0__VALUE GENMASK(7, 0) #define DEVICE_PARAM_1 0x330 -#define DEVICE_PARAM_1__VALUE 0x00ff +#define DEVICE_PARAM_1__VALUE GENMASK(7, 0) #define DEVICE_PARAM_2 0x340 -#define DEVICE_PARAM_2__VALUE 0x00ff +#define DEVICE_PARAM_2__VALUE GENMASK(7, 0) #define LOGICAL_PAGE_DATA_SIZE 0x350 -#define LOGICAL_PAGE_DATA_SIZE__VALUE 0xffff +#define LOGICAL_PAGE_DATA_SIZE__VALUE GENMASK(15, 0) #define LOGICAL_PAGE_SPARE_SIZE 0x360 -#define LOGICAL_PAGE_SPARE_SIZE__VALUE 0xffff +#define LOGICAL_PAGE_SPARE_SIZE__VALUE GENMASK(15, 0) #define REVISION 0x370 -#define REVISION__VALUE 0xffff +#define REVISION__VALUE GENMASK(15, 0) #define ONFI_DEVICE_FEATURES 0x380 -#define ONFI_DEVICE_FEATURES__VALUE 0x003f +#define ONFI_DEVICE_FEATURES__VALUE GENMASK(5, 0) #define ONFI_OPTIONAL_COMMANDS 0x390 -#define ONFI_OPTIONAL_COMMANDS__VALUE 0x003f +#define ONFI_OPTIONAL_COMMANDS__VALUE GENMASK(5, 0) #define ONFI_TIMING_MODE 0x3a0 -#define ONFI_TIMING_MODE__VALUE 0x003f +#define ONFI_TIMING_MODE__VALUE GENMASK(5, 0) #define ONFI_PGM_CACHE_TIMING_MODE 0x3b0 -#define ONFI_PGM_CACHE_TIMING_MODE__VALUE 0x003f +#define ONFI_PGM_CACHE_TIMING_MODE__VALUE GENMASK(5, 0) #define ONFI_DEVICE_NO_OF_LUNS 0x3c0 -#define ONFI_DEVICE_NO_OF_LUNS__NO_OF_LUNS 0x00ff -#define ONFI_DEVICE_NO_OF_LUNS__ONFI_DEVICE 0x0100 +#define ONFI_DEVICE_NO_OF_LUNS__NO_OF_LUNS GENMASK(7, 0) +#define ONFI_DEVICE_NO_OF_LUNS__ONFI_DEVICE BIT(8) #define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_L 0x3d0 -#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_L__VALUE 0xffff +#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_L__VALUE GENMASK(15, 0) #define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_U 0x3e0 -#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_U__VALUE 0xffff - -#define FEATURES 0x3f0 -#define FEATURES__N_BANKS 0x0003 -#define FEATURES__ECC_MAX_ERR 0x003c -#define FEATURES__DMA 0x0040 -#define FEATURES__CMD_DMA 0x0080 -#define FEATURES__PARTITION 0x0100 -#define FEATURES__XDMA_SIDEBAND 0x0200 -#define FEATURES__GPREG 0x0400 -#define FEATURES__INDEX_ADDR 0x0800 +#define ONFI_DEVICE_NO_OF_BLOCKS_PER_LUN_U__VALUE GENMASK(15, 0) + +#define FEATURES 0x3f0 +#define FEATURES__N_BANKS GENMASK(1, 0) +#define FEATURES__ECC_MAX_ERR GENMASK(5, 2) +#define FEATURES__DMA BIT(6) +#define FEATURES__CMD_DMA BIT(7) +#define FEATURES__PARTITION BIT(8) +#define FEATURES__XDMA_SIDEBAND BIT(9) +#define FEATURES__GPREG BIT(10) +#define FEATURES__INDEX_ADDR BIT(11) #define TRANSFER_MODE 0x400 -#define TRANSFER_MODE__VALUE 0x0003 +#define TRANSFER_MODE__VALUE GENMASK(1, 0) -#define INTR_STATUS(__bank) (0x410 + ((__bank) * 0x50)) -#define INTR_EN(__bank) (0x420 + ((__bank) * 0x50)) +#define INTR_STATUS(bank) (0x410 + (bank) * 0x50) +#define INTR_EN(bank) (0x420 + (bank) * 0x50) /* bit[1:0] is used differently depending on IP version */ -#define INTR__ECC_UNCOR_ERR 0x0001 /* new IP */ -#define INTR__ECC_TRANSACTION_DONE 0x0001 /* old IP */ -#define INTR__ECC_ERR 0x0002 /* old IP */ -#define INTR__DMA_CMD_COMP 0x0004 -#define INTR__TIME_OUT 0x0008 -#define INTR__PROGRAM_FAIL 0x0010 -#define INTR__ERASE_FAIL 0x0020 -#define INTR__LOAD_COMP 0x0040 -#define INTR__PROGRAM_COMP 0x0080 -#define INTR__ERASE_COMP 0x0100 -#define INTR__PIPE_CPYBCK_CMD_COMP 0x0200 -#define INTR__LOCKED_BLK 0x0400 -#define INTR__UNSUP_CMD 0x0800 -#define INTR__INT_ACT 0x1000 -#define INTR__RST_COMP 0x2000 -#define INTR__PIPE_CMD_ERR 0x4000 -#define INTR__PAGE_XFER_INC 0x8000 - -#define PAGE_CNT(__bank) (0x430 + ((__bank) * 0x50)) -#define ERR_PAGE_ADDR(__bank) (0x440 + ((__bank) * 0x50)) -#define ERR_BLOCK_ADDR(__bank) (0x450 + ((__bank) * 0x50)) +#define INTR__ECC_UNCOR_ERR BIT(0) /* new IP */ +#define INTR__ECC_TRANSACTION_DONE BIT(0) /* old IP */ +#define INTR__ECC_ERR BIT(1) /* old IP */ +#define INTR__DMA_CMD_COMP BIT(2) +#define INTR__TIME_OUT BIT(3) +#define INTR__PROGRAM_FAIL BIT(4) +#define INTR__ERASE_FAIL BIT(5) +#define INTR__LOAD_COMP BIT(6) +#define INTR__PROGRAM_COMP BIT(7) +#define INTR__ERASE_COMP BIT(8) +#define INTR__PIPE_CPYBCK_CMD_COMP BIT(9) +#define INTR__LOCKED_BLK BIT(10) +#define INTR__UNSUP_CMD BIT(11) +#define INTR__INT_ACT BIT(12) +#define INTR__RST_COMP BIT(13) +#define INTR__PIPE_CMD_ERR BIT(14) +#define INTR__PAGE_XFER_INC BIT(15) + +#define PAGE_CNT(bank) (0x430 + (bank) * 0x50) +#define ERR_PAGE_ADDR(bank) (0x440 + (bank) * 0x50) +#define ERR_BLOCK_ADDR(bank) (0x450 + (bank) * 0x50) #define ECC_THRESHOLD 0x600 -#define ECC_THRESHOLD__VALUE 0x03ff +#define ECC_THRESHOLD__VALUE GENMASK(9, 0) #define ECC_ERROR_BLOCK_ADDRESS 0x610 -#define ECC_ERROR_BLOCK_ADDRESS__VALUE 0xffff +#define ECC_ERROR_BLOCK_ADDRESS__VALUE GENMASK(15, 0) #define ECC_ERROR_PAGE_ADDRESS 0x620 -#define ECC_ERROR_PAGE_ADDRESS__VALUE 0x0fff -#define ECC_ERROR_PAGE_ADDRESS__BANK 0xf000 +#define ECC_ERROR_PAGE_ADDRESS__VALUE GENMASK(11, 0) +#define ECC_ERROR_PAGE_ADDRESS__BANK GENMASK(15, 12) #define ECC_ERROR_ADDRESS 0x630 -#define ECC_ERROR_ADDRESS__OFFSET 0x0fff -#define ECC_ERROR_ADDRESS__SECTOR_NR 0xf000 +#define ECC_ERROR_ADDRESS__OFFSET GENMASK(11, 0) +#define ECC_ERROR_ADDRESS__SECTOR_NR GENMASK(15, 12) #define ERR_CORRECTION_INFO 0x640 -#define ERR_CORRECTION_INFO__BYTEMASK 0x00ff -#define ERR_CORRECTION_INFO__DEVICE_NR 0x0f00 -#define ERR_CORRECTION_INFO__ERROR_TYPE 0x4000 -#define ERR_CORRECTION_INFO__LAST_ERR_INFO 0x8000 +#define ERR_CORRECTION_INFO__BYTEMASK GENMASK(7, 0) +#define ERR_CORRECTION_INFO__DEVICE_NR GENMASK(11, 8) +#define ERR_CORRECTION_INFO__ERROR_TYPE BIT(14) +#define ERR_CORRECTION_INFO__LAST_ERR_INFO BIT(15) #define ECC_COR_INFO(bank) (0x650 + (bank) / 2 * 0x10) #define ECC_COR_INFO__SHIFT(bank) ((bank) % 2 * 8) -#define ECC_COR_INFO__MAX_ERRORS 0x007f -#define ECC_COR_INFO__UNCOR_ERR 0x0080 +#define ECC_COR_INFO__MAX_ERRORS GENMASK(6, 0) +#define ECC_COR_INFO__UNCOR_ERR BIT(7) #define CFG_DATA_BLOCK_SIZE 0x6b0 @@ -274,31 +268,31 @@ #define CFG_META_DATA_SIZE 0x6e0 #define DMA_ENABLE 0x700 -#define DMA_ENABLE__FLAG 0x0001 +#define DMA_ENABLE__FLAG BIT(0) #define IGNORE_ECC_DONE 0x710 -#define IGNORE_ECC_DONE__FLAG 0x0001 +#define IGNORE_ECC_DONE__FLAG BIT(0) #define DMA_INTR 0x720 #define DMA_INTR_EN 0x730 -#define DMA_INTR__TARGET_ERROR 0x0001 -#define DMA_INTR__DESC_COMP_CHANNEL0 0x0002 -#define DMA_INTR__DESC_COMP_CHANNEL1 0x0004 -#define DMA_INTR__DESC_COMP_CHANNEL2 0x0008 -#define DMA_INTR__DESC_COMP_CHANNEL3 0x0010 -#define DMA_INTR__MEMCOPY_DESC_COMP 0x0020 +#define DMA_INTR__TARGET_ERROR BIT(0) +#define DMA_INTR__DESC_COMP_CHANNEL0 BIT(1) +#define DMA_INTR__DESC_COMP_CHANNEL1 BIT(2) +#define DMA_INTR__DESC_COMP_CHANNEL2 BIT(3) +#define DMA_INTR__DESC_COMP_CHANNEL3 BIT(4) +#define DMA_INTR__MEMCOPY_DESC_COMP BIT(5) #define TARGET_ERR_ADDR_LO 0x740 -#define TARGET_ERR_ADDR_LO__VALUE 0xffff +#define TARGET_ERR_ADDR_LO__VALUE GENMASK(15, 0) #define TARGET_ERR_ADDR_HI 0x750 -#define TARGET_ERR_ADDR_HI__VALUE 0xffff +#define TARGET_ERR_ADDR_HI__VALUE GENMASK(15, 0) #define CHNL_ACTIVE 0x760 -#define CHNL_ACTIVE__CHANNEL0 0x0001 -#define CHNL_ACTIVE__CHANNEL1 0x0002 -#define CHNL_ACTIVE__CHANNEL2 0x0004 -#define CHNL_ACTIVE__CHANNEL3 0x0008 +#define CHNL_ACTIVE__CHANNEL0 BIT(0) +#define CHNL_ACTIVE__CHANNEL1 BIT(1) +#define CHNL_ACTIVE__CHANNEL2 BIT(2) +#define CHNL_ACTIVE__CHANNEL3 BIT(3) #define FAIL 1 /*failed flag*/ #define PASS 0 /*success flag*/ -- 2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 06/37] mtd: nand: denali: support HW_ECC_FIXUP capability |
| Message-ID | <tqCdR-2l8-49@gated-at.bofh.it> |
| In reply to | #1612622 |
Some old versions of the Denali IP (perhaps used only for Intel?)
detects ECC errors and provides correct data via a register, but
does not touch the transferred data. So, the software must fixup
the data in the buffer according to the provided ECC correction
information.
Newer versions perform ECC correction before transferring the data.
No more software intervention is needed. The ECC_ERROR_ADDRESS and
ECC_CORRECTION_INFO registers were deprecated. Instead, the number
of corrected bit-flips are reported via the ECC_COR_INFO register.
When an uncorrectable ECC error happens, a status flag is set to the
INTR_STATUS and ECC_COR_INFO registers.
As is often the case with this IP, the register view of INTR_STATUS
had broken compatibility.
For older versions (SW ECC fixup):
bit 0: ECC_TRANSACTION_DONE
bit 1: ECC_ERR
For newer versions (HW ECC fixup):
bit 0: ECC_UNCOR_ERR
bit 1: Reserved
Due to this difference, the irq_mask must be fixed too.
The existing handle_ecc() has been renamed to denali_sw_ecc_fixup()
for clarification.
What is unfortunate with this feature is we can not know the total
number of corrected/uncorrected errors in a page. The register
ECC_COR_INFO reports the maximum of per-sector bitflips. This is
useful for ->read_page return value, but ecc_stats.{corrected,failed}
increments may not be precise.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3:
- Adjusted for the correct erased page check.
- Move DENALI_CAP_ define out of struct denali_nand_info
Changes in v2:
- Change the capability prefix DENALI_CAPS_ -> DENALI_CAP_
drivers/mtd/nand/denali.c | 52 ++++++++++++++++++++++++++++++++++++++++-------
drivers/mtd/nand/denali.h | 14 +++++++++++--
2 files changed, 57 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 64a3bdc..b95e33a 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -856,6 +856,41 @@ static int denali_check_erased_page(struct mtd_info *mtd,
return max_bitflips;
}
+static int denali_hw_ecc_fixup(struct mtd_info *mtd,
+ struct denali_nand_info *denali,
+ unsigned long *uncor_ecc_flags)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ int bank = denali->flash_bank;
+ uint32_t ecc_cor;
+ unsigned int max_bitflips;
+
+ ecc_cor = ioread32(denali->flash_reg + ECC_COR_INFO(bank));
+ ecc_cor >>= ECC_COR_INFO__SHIFT(bank);
+
+ if (ecc_cor & ECC_COR_INFO__UNCOR_ERR) {
+ /*
+ * This flag is set when uncorrectable error occurs at least in
+ * one ECC sector. We can not know "how many sectors", or
+ * "which sector(s)". We need erase-page check for all sectors.
+ */
+ *uncor_ecc_flags = GENMASK(chip->ecc.steps - 1, 0);
+ return 0;
+ }
+
+ max_bitflips = ecc_cor & ECC_COR_INFO__MAX_ERRORS;
+
+ /*
+ * The register holds the maximum of per-sector corrected bitflips.
+ * This is suitable for the return value of the ->read_page() callback.
+ * Unfortunately, we can not know the total number of corrected bits in
+ * the page. Increase the stats by max_bitflips. (compromised solution)
+ */
+ mtd->ecc_stats.corrected += max_bitflips;
+
+ return max_bitflips;
+}
+
#define ECC_SECTOR_SIZE 512
#define ECC_SECTOR(x) (((x) & ECC_ERROR_ADDRESS__SECTOR_NR) >> 12)
@@ -865,8 +900,9 @@ static int denali_check_erased_page(struct mtd_info *mtd,
#define ECC_ERR_DEVICE(x) (((x) & ERR_CORRECTION_INFO__DEVICE_NR) >> 8)
#define ECC_LAST_ERR(x) ((x) & ERR_CORRECTION_INFO__LAST_ERR_INFO)
-static int handle_ecc(struct mtd_info *mtd, struct denali_nand_info *denali,
- uint8_t *buf, unsigned long *uncor_ecc_flags)
+static int denali_sw_ecc_fixup(struct mtd_info *mtd,
+ struct denali_nand_info *denali,
+ unsigned long *uncor_ecc_flags, uint8_t *buf)
{
unsigned int bitflips = 0;
unsigned int max_bitflips = 0;
@@ -1070,12 +1106,12 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
uint8_t *buf, int oob_required, int page)
{
struct denali_nand_info *denali = mtd_to_denali(mtd);
-
dma_addr_t addr = denali->buf.dma_buf;
size_t size = mtd->writesize + mtd->oobsize;
-
uint32_t irq_status;
- uint32_t irq_mask = INTR__ECC_TRANSACTION_DONE | INTR__ECC_ERR;
+ uint32_t irq_mask = denali->caps & DENALI_CAP_HW_ECC_FIXUP ?
+ INTR__DMA_CMD_COMP | INTR__ECC_UNCOR_ERR :
+ INTR__ECC_TRANSACTION_DONE | INTR__ECC_ERR;
unsigned long uncor_ecc_flags = 0;
int stat = 0;
@@ -1101,8 +1137,10 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
memcpy(buf, denali->buf.buf, mtd->writesize);
- if (irq_status & INTR__ECC_ERR)
- stat = handle_ecc(mtd, denali, buf, &uncor_ecc_flags);
+ if (denali->caps & DENALI_CAP_HW_ECC_FIXUP)
+ stat = denali_hw_ecc_fixup(mtd, denali, &uncor_ecc_flags);
+ else if (irq_status & INTR__ECC_ERR)
+ stat = denali_sw_ecc_fixup(mtd, denali, &uncor_ecc_flags, buf);
denali_enable_dma(denali, false);
if (stat < 0)
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 483c0e9..e532956 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -20,6 +20,7 @@
#ifndef __DENALI_H__
#define __DENALI_H__
+#include <linux/bitops.h>
#include <linux/mtd/nand.h>
#define DEVICE_RESET 0x0
@@ -218,8 +219,10 @@
#define INTR_STATUS(__bank) (0x410 + ((__bank) * 0x50))
#define INTR_EN(__bank) (0x420 + ((__bank) * 0x50))
-#define INTR__ECC_TRANSACTION_DONE 0x0001
-#define INTR__ECC_ERR 0x0002
+/* bit[1:0] is used differently depending on IP version */
+#define INTR__ECC_UNCOR_ERR 0x0001 /* new IP */
+#define INTR__ECC_TRANSACTION_DONE 0x0001 /* old IP */
+#define INTR__ECC_ERR 0x0002 /* old IP */
#define INTR__DMA_CMD_COMP 0x0004
#define INTR__TIME_OUT 0x0008
#define INTR__PROGRAM_FAIL 0x0010
@@ -259,6 +262,11 @@
#define ERR_CORRECTION_INFO__ERROR_TYPE 0x4000
#define ERR_CORRECTION_INFO__LAST_ERR_INFO 0x8000
+#define ECC_COR_INFO(bank) (0x650 + (bank) / 2 * 0x10)
+#define ECC_COR_INFO__SHIFT(bank) ((bank) % 2 * 8)
+#define ECC_COR_INFO__MAX_ERRORS 0x007f
+#define ECC_COR_INFO__UNCOR_ERR 0x0080
+
#define DMA_ENABLE 0x700
#define DMA_ENABLE__FLAG 0x0001
@@ -338,6 +346,8 @@ struct denali_nand_info {
unsigned int caps;
};
+#define DENALI_CAP_HW_ECC_FIXUP BIT(0)
+
extern int denali_init(struct denali_nand_info *denali);
extern void denali_remove(struct denali_nand_info *denali);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 28/37] mtd: nand: denali: propagate page to helpers via function argument |
| Message-ID | <tqCdR-2l8-47@gated-at.bofh.it> |
| In reply to | #1612622 |
This driver stores the currently addressed page into denali->page,
which is later read out by helper functions. While I am tackling on
this driver, I often missed to insert "denali->page = page;" where
necessary. This makes page_read/write callbacks to get access to a
wrong page, which is a bug hard to figure out.
Instead, I'd rather pass the page via function argument because the
compiler's prototype checks will help to detect bugs.
For the same reason, propagate dma_addr to the DMA helpers instead
of denali->buf.dma_buf .
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Newly added
drivers/mtd/nand/denali.c | 58 ++++++++++++++++++++---------------------------
drivers/mtd/nand/denali.h | 1 -
2 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 9dfed20..00d0e9d 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -282,7 +282,7 @@ static int denali_dev_ready(struct mtd_info *mtd)
* sends a pipeline command operation to the controller. See the Denali NAND
* controller's user guide for more information (section 4.2.3.6).
*/
-static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
+static int denali_send_pipeline_cmd(struct denali_nand_info *denali, int page,
bool ecc_en, bool transfer_spare,
int access_type, int op)
{
@@ -293,7 +293,7 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
denali_reset_irq(denali);
- addr = BANK(denali->flash_bank) | denali->page;
+ addr = BANK(denali->flash_bank) | page;
if (op == DENALI_WRITE && access_type != SPARE_ACCESS) {
cmd = MODE_01 | addr;
@@ -366,9 +366,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
uint32_t irq_mask = INTR__PROGRAM_COMP | INTR__PROGRAM_FAIL;
int status = 0;
- denali->page = page;
-
- if (denali_send_pipeline_cmd(denali, false, false, SPARE_ACCESS,
+ if (denali_send_pipeline_cmd(denali, page, false, false, SPARE_ACCESS,
DENALI_WRITE) == PASS) {
write_data_to_flash_mem(denali, buf, mtd->oobsize);
@@ -393,9 +391,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
uint32_t irq_mask = INTR__LOAD_COMP;
uint32_t irq_status, addr, cmd;
- denali->page = page;
-
- if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
+ if (denali_send_pipeline_cmd(denali, page, false, true, SPARE_ACCESS,
DENALI_READ) == PASS) {
read_data_from_flash_mem(denali, buf, mtd->oobsize);
@@ -407,8 +403,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
irq_status = denali_wait_for_irq(denali, irq_mask);
if (!(irq_status & INTR__LOAD_COMP))
- dev_err(denali->dev, "page on OOB timeout %d\n",
- denali->page);
+ dev_err(denali->dev, "page on OOB timeout %d\n", page);
/*
* We set the device back to MAIN_ACCESS here as I observed
@@ -417,7 +412,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
* is reliable (according to the MTD test infrastructure)
* if you are in MAIN_ACCESS.
*/
- addr = BANK(denali->flash_bank) | denali->page;
+ addr = BANK(denali->flash_bank) | page;
cmd = MODE_10 | addr;
index_addr(denali, cmd, MAIN_ACCESS);
}
@@ -582,13 +577,13 @@ static void denali_enable_dma(struct denali_nand_info *denali, bool en)
ioread32(denali->flash_reg + DMA_ENABLE);
}
-static void denali_setup_dma64(struct denali_nand_info *denali, int op)
+static void denali_setup_dma64(struct denali_nand_info *denali,
+ dma_addr_t dma_addr, int page, int op)
{
uint32_t mode;
const int page_count = 1;
- uint64_t addr = denali->buf.dma_buf;
- mode = MODE_10 | BANK(denali->flash_bank) | denali->page;
+ mode = MODE_10 | BANK(denali->flash_bank) | page;
/* DMA is a three step process */
@@ -599,41 +594,42 @@ static void denali_setup_dma64(struct denali_nand_info *denali, int op)
index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);
/* 2. set memory low address */
- index_addr(denali, mode, addr);
+ index_addr(denali, mode, dma_addr);
/* 3. set memory high address */
- index_addr(denali, mode, addr >> 32);
+ index_addr(denali, mode, (uint64_t)dma_addr >> 32);
}
-static void denali_setup_dma32(struct denali_nand_info *denali, int op)
+static void denali_setup_dma32(struct denali_nand_info *denali,
+ dma_addr_t dma_addr, int page, int op)
{
uint32_t mode;
const int page_count = 1;
- uint32_t addr = denali->buf.dma_buf;
mode = MODE_10 | BANK(denali->flash_bank);
/* DMA is a four step process */
/* 1. setup transfer type and # of pages */
- index_addr(denali, mode | denali->page, 0x2000 | op | page_count);
+ index_addr(denali, mode | page, 0x2000 | op | page_count);
/* 2. set memory high address bits 23:8 */
- index_addr(denali, mode | ((addr >> 16) << 8), 0x2200);
+ index_addr(denali, mode | ((dma_addr >> 16) << 8), 0x2200);
/* 3. set memory low address bits 23:8 */
- index_addr(denali, mode | ((addr & 0xffff) << 8), 0x2300);
+ index_addr(denali, mode | ((dma_addr & 0xffff) << 8), 0x2300);
/* 4. interrupt when complete, burst len = 64 bytes */
index_addr(denali, mode | 0x14000, 0x2400);
}
-static void denali_setup_dma(struct denali_nand_info *denali, int op)
+static void denali_setup_dma(struct denali_nand_info *denali,
+ dma_addr_t dma_addr, int page, int op)
{
if (denali->caps & DENALI_CAP_DMA_64BIT)
- denali_setup_dma64(denali, op);
+ denali_setup_dma64(denali, dma_addr, page, op);
else
- denali_setup_dma32(denali, op);
+ denali_setup_dma32(denali, dma_addr, page, op);
}
/*
@@ -650,8 +646,6 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
int ret = 0;
- denali->page = page;
-
/*
* if it is a raw xfer, we want to disable ecc and send the spare area.
* !raw_xfer - enable ecc
@@ -674,7 +668,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
denali_reset_irq(denali);
denali_enable_dma(denali, true);
- denali_setup_dma(denali, DENALI_WRITE);
+ denali_setup_dma(denali, addr, page, DENALI_WRITE);
/* wait for operation to complete */
irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -750,15 +744,13 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
unsigned long uncor_ecc_flags = 0;
int stat = 0;
- denali->page = page;
-
setup_ecc_for_xfer(denali, true, false);
denali_enable_dma(denali, true);
dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
denali_reset_irq(denali);
- denali_setup_dma(denali, DENALI_READ);
+ denali_setup_dma(denali, addr, page, DENALI_READ);
/* wait for operation to complete */
irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -777,7 +769,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
return stat;
if (uncor_ecc_flags) {
- read_oob_data(mtd, chip->oob_poi, denali->page);
+ read_oob_data(mtd, chip->oob_poi, page);
stat = denali_check_erased_page(mtd, chip, buf,
uncor_ecc_flags, stat);
@@ -795,15 +787,13 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
uint32_t irq_mask = INTR__DMA_CMD_COMP;
uint32_t irq_status;
- denali->page = page;
-
setup_ecc_for_xfer(denali, false, true);
denali_enable_dma(denali, true);
dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
denali_reset_irq(denali);
- denali_setup_dma(denali, DENALI_READ);
+ denali_setup_dma(denali, addr, page, DENALI_READ);
/* wait for operation to complete */
irq_status = denali_wait_for_irq(denali, irq_mask);
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 7d64167..5f917a0 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -317,7 +317,6 @@ struct denali_nand_info {
int flash_bank; /* currently selected chip */
struct nand_buf buf;
struct device *dev;
- int page;
void __iomem *flash_reg; /* Register Interface */
void __iomem *flash_mem; /* Host Data/Command Interface */
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Masahiro Yamada <yamada.masahiro@socionext.com> |
|---|---|
| Date | 2017-03-30 09:00 +0200 |
| Subject | [PATCH v3 16/37] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants |
| Message-ID | <tqCdR-2l8-51@gated-at.bofh.it> |
| In reply to | #1612622 |
Add two compatible strings for UniPhier SoCs.
"socionext,uniphier-denali-nand-v5a" is used on UniPhier sLD3, LD4,
Pro4, sLD8 SoCs.
"socionext,uniphier-denali-nand-v5b" is used on UniPhier Pro5, PXs2,
LD6b, LD11, LD20 SoCs.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3: None
Changes in v2:
- Change the compatible strings
- Fix the ecc_strength_capability
- Override revision number for the newer one
.../devicetree/bindings/mtd/denali-nand.txt | 6 ++++++
drivers/mtd/nand/denali_dt.c | 23 ++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index 647618e..0b08ea5 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -3,6 +3,8 @@
Required properties:
- compatible : should be one of the following:
"altr,socfpga-denali-nand" - for Altera SOCFPGA
+ "socionext,uniphier-denali-nand-v5a" - for Socionext UniPhier (v5a)
+ "socionext,uniphier-denali-nand-v5b" - for Socionext UniPhier (v5b)
- reg : should contain registers location and length for data and reg.
- reg-names: Should contain the reg names "nand_data" and "denali_reg"
- interrupts : The interrupt number.
@@ -10,9 +12,13 @@ Required properties:
Optional properties:
- nand-ecc-step-size: must be 512 or 1024. If not specified, default to:
512 for "altr,socfpga-denali-nand"
+ 1024 for "socionext,uniphier-denali-nand-v5a"
+ 1024 for "socionext,uniphier-denali-nand-v5b"
see nand.txt for details.
- nand-ecc-strength: see nand.txt for details. Available values are:
8, 15 for "altr,socfpga-denali-nand"
+ 8, 16, 24 for "socionext,uniphier-denali-nand-v5a"
+ 8, 16 for "socionext,uniphier-denali-nand-v5b"
- nand-ecc-maximize: see nand.txt for details
Note:
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index c3bc333..1f2f68a 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -41,11 +41,34 @@ static const struct denali_dt_data denali_socfpga_data = {
DENALI_CAP_ECC_SIZE_512,
};
+static const struct denali_dt_data denali_uniphier_v5a_data = {
+ .ecc_strength_avail = BIT(24) | BIT(16) | BIT(8),
+ .caps = DENALI_CAP_HW_ECC_FIXUP |
+ DENALI_CAP_DMA_64BIT |
+ DENALI_CAP_ECC_SIZE_1024,
+};
+
+static const struct denali_dt_data denali_uniphier_v5b_data = {
+ .revision = 0x0501,
+ .ecc_strength_avail = BIT(16) | BIT(8),
+ .caps = DENALI_CAP_HW_ECC_FIXUP |
+ DENALI_CAP_DMA_64BIT |
+ DENALI_CAP_ECC_SIZE_1024,
+};
+
static const struct of_device_id denali_nand_dt_ids[] = {
{
.compatible = "altr,socfpga-denali-nand",
.data = &denali_socfpga_data,
},
+ {
+ .compatible = "socionext,uniphier-denali-nand-v5a",
+ .data = &denali_uniphier_v5a_data,
+ },
+ {
+ .compatible = "socionext,uniphier-denali-nand-v5b",
+ .data = &denali_uniphier_v5b_data,
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, denali_nand_dt_ids);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-04-03 17:50 +0200 |
| Subject | Re: [PATCH v3 16/37] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants |
| Message-ID | <tscoV-jE-1@gated-at.bofh.it> |
| In reply to | #1612637 |
On Thu, Mar 30, 2017 at 03:46:02PM +0900, Masahiro Yamada wrote: > Add two compatible strings for UniPhier SoCs. > > "socionext,uniphier-denali-nand-v5a" is used on UniPhier sLD3, LD4, > Pro4, sLD8 SoCs. > > "socionext,uniphier-denali-nand-v5b" is used on UniPhier Pro5, PXs2, > LD6b, LD11, LD20 SoCs. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > Changes in v3: None > Changes in v2: > - Change the compatible strings > - Fix the ecc_strength_capability > - Override revision number for the newer one > > .../devicetree/bindings/mtd/denali-nand.txt | 6 ++++++ > drivers/mtd/nand/denali_dt.c | 23 ++++++++++++++++++++++ > 2 files changed, 29 insertions(+) Acked-by: Rob Herring <robh@kernel.org>
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web