(Apologies for the delay in sending v2. As discussed previously[1],
the first patch needs to be merged in the same merge window as commit
34569d869532 from v1 of the series, even if the others don't make it.)
I have an A33 tablet with MLC NAND, and I wanted to use mainline Linux
to dump the NAND. To do that, I updated this driver's ECC ops to fully
utilize the hardware for ECC and scrambling. This made the driver
compatible with the existing (scrambled) on-flash bad block map, and I
was able to read the full NAND contents.
This series contains some cleanup from that effort. The first patch
ensures we can make good use of the hardware ECC/descrambler. The other
two patches simplify some code for managing the ECC engine.
[1]: https://lore.kernel.org/linux-mtd/20230102175351.64690aaf@xps-13/
Changes in v2:
- Update commit message to address backward-compatibility concerns
- Keep `struct sunxi_nand_hw_ecc` but change the pointer to a value
- Split ECC_CTL precomputation and structure updates to two patches
Samuel Holland (3):
mtd: rawnand: sunxi: Update OOB layout to match hardware
mtd: rawnand: sunxi: Embed sunxi_nand_hw_ecc by value
mtd: rawnand: sunxi: Precompute the ECC_CTL register value
drivers/mtd/nand/raw/sunxi_nand.c | 73 +++++++++----------------------
1 file changed, 20 insertions(+), 53 deletions(-)
--
2.37.4
When using the hardware ECC engine, the OOB data is made available in
the NFC_REG_USER_DATA registers, as one 32-bit word per ECC step. Any
additional bytes are only accessible through raw reads and software
descrambling. For efficiency, and to match the vendor driver, ignore
these extra bytes when using hardware ECC.
Note that until commit 34569d869532 ("mtd: rawnand: sunxi: Fix the size
of the last OOB region"), this extra free area was reported with length
zero, so this is not a functional change for any stable kernel user.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v2:
- Update commit message to address backward-compatibility concerns
drivers/mtd/nand/raw/sunxi_nand.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index e673ac46f2e8..3c32d31f20aa 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1604,6 +1604,13 @@ static int sunxi_nand_ooblayout_free(struct mtd_info *mtd, int section,
return 0;
}
+ /*
+ * The controller does not provide access to OOB bytes
+ * past the end of the ECC data.
+ */
+ if (section == ecc->steps && ecc->engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST)
+ return -ERANGE;
+
oobregion->offset = section * (ecc->bytes + 4);
if (section < ecc->steps)
--
2.37.4
The sunxi_nand_hw_ecc object is not shared, and it has the same lifetime
as the sunxi_nand_chip which points to it, so we can embed it in the
outer structure instead of using a pointer. This removes an unnecessary
memory allocation and simplifies the error handling code.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v2:
- Keep `struct sunxi_nand_hw_ecc` but change the pointer to a value
- New patch for v2
drivers/mtd/nand/raw/sunxi_nand.c | 45 +++++--------------------------
1 file changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 3c32d31f20aa..a0d0cb17c150 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -193,7 +193,7 @@ struct sunxi_nand_hw_ecc {
struct sunxi_nand_chip {
struct list_head node;
struct nand_chip nand;
- struct sunxi_nand_hw_ecc *ecc;
+ struct sunxi_nand_hw_ecc ecc;
unsigned long clk_rate;
u32 timing_cfg;
u32 timing_ctl;
@@ -694,7 +694,7 @@ static void sunxi_nfc_hw_ecc_enable(struct nand_chip *nand)
ecc_ctl = readl(nfc->regs + NFC_REG_ECC_CTL);
ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE |
NFC_ECC_BLOCK_SIZE_MSK);
- ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(sunxi_nand->ecc->mode) |
+ ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(sunxi_nand->ecc.mode) |
NFC_ECC_EXCEPTION | NFC_ECC_PIPELINE;
if (nand->ecc.size == 512)
@@ -1626,11 +1626,6 @@ static const struct mtd_ooblayout_ops sunxi_nand_ooblayout_ops = {
.free = sunxi_nand_ooblayout_free,
};
-static void sunxi_nand_hw_ecc_ctrl_cleanup(struct sunxi_nand_chip *sunxi_nand)
-{
- kfree(sunxi_nand->ecc);
-}
-
static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
struct nand_ecc_ctrl *ecc,
struct device_node *np)
@@ -1641,7 +1636,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
struct mtd_info *mtd = nand_to_mtd(nand);
struct nand_device *nanddev = mtd_to_nanddev(mtd);
int nsectors;
- int ret;
int i;
if (nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE_STRENGTH) {
@@ -1676,10 +1670,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
if (ecc->size != 512 && ecc->size != 1024)
return -EINVAL;
- sunxi_nand->ecc = kzalloc(sizeof(*sunxi_nand->ecc), GFP_KERNEL);
- if (!sunxi_nand->ecc)
- return -ENOMEM;
-
/* Prefer 1k ECC chunk over 512 ones */
if (ecc->size == 512 && mtd->writesize > 512) {
ecc->size = 1024;
@@ -1700,11 +1690,10 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
if (i >= ARRAY_SIZE(strengths)) {
dev_err(nfc->dev, "unsupported strength\n");
- ret = -ENOTSUPP;
- goto err;
+ return -ENOTSUPP;
}
- sunxi_nand->ecc->mode = i;
+ sunxi_nand->ecc.mode = i;
/* HW ECC always request ECC bytes for 1024 bytes blocks */
ecc->bytes = DIV_ROUND_UP(ecc->strength * fls(8 * 1024), 8);
@@ -1714,10 +1703,8 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
nsectors = mtd->writesize / ecc->size;
- if (mtd->oobsize < ((ecc->bytes + 4) * nsectors)) {
- ret = -EINVAL;
- goto err;
- }
+ if (mtd->oobsize < ((ecc->bytes + 4) * nsectors))
+ return -EINVAL;
ecc->read_oob = sunxi_nfc_hw_ecc_read_oob;
ecc->write_oob = sunxi_nfc_hw_ecc_write_oob;
@@ -1740,25 +1727,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
ecc->write_oob_raw = nand_write_oob_std;
return 0;
-
-err:
- kfree(sunxi_nand->ecc);
-
- return ret;
-}
-
-static void sunxi_nand_ecc_cleanup(struct sunxi_nand_chip *sunxi_nand)
-{
- struct nand_ecc_ctrl *ecc = &sunxi_nand->nand.ecc;
-
- switch (ecc->engine_type) {
- case NAND_ECC_ENGINE_TYPE_ON_HOST:
- sunxi_nand_hw_ecc_ctrl_cleanup(sunxi_nand);
- break;
- case NAND_ECC_ENGINE_TYPE_NONE:
- default:
- break;
- }
}
static int sunxi_nand_attach_chip(struct nand_chip *nand)
@@ -1971,7 +1939,6 @@ static void sunxi_nand_chips_cleanup(struct sunxi_nfc *nfc)
ret = mtd_device_unregister(nand_to_mtd(chip));
WARN_ON(ret);
nand_cleanup(chip);
- sunxi_nand_ecc_cleanup(sunxi_nand);
list_del(&sunxi_nand->node);
}
}
--
2.37.4
The value computed by this function never changes for a given chip.
Compute the whole register value once up front, instead of every time
the ECC engine is enabled.
Signed-off-by: Samuel Holland <[email protected]>
---
Changes in v2:
- Split ECC_CTL precomputation and structure updates to two patches
drivers/mtd/nand/raw/sunxi_nand.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index a0d0cb17c150..13e3e0198d15 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -172,10 +172,10 @@ struct sunxi_nand_chip_sel {
/**
* struct sunxi_nand_hw_ecc - stores information related to HW ECC support
*
- * @mode: the sunxi ECC mode field deduced from ECC requirements
+ * @ecc_ctl: ECC_CTL register value for this NAND chip
*/
struct sunxi_nand_hw_ecc {
- int mode;
+ u32 ecc_ctl;
};
/**
@@ -689,26 +689,15 @@ static void sunxi_nfc_hw_ecc_enable(struct nand_chip *nand)
{
struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
- u32 ecc_ctl;
-
- ecc_ctl = readl(nfc->regs + NFC_REG_ECC_CTL);
- ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE |
- NFC_ECC_BLOCK_SIZE_MSK);
- ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(sunxi_nand->ecc.mode) |
- NFC_ECC_EXCEPTION | NFC_ECC_PIPELINE;
-
- if (nand->ecc.size == 512)
- ecc_ctl |= NFC_ECC_BLOCK_512;
- writel(ecc_ctl, nfc->regs + NFC_REG_ECC_CTL);
+ writel(sunxi_nand->ecc.ecc_ctl, nfc->regs + NFC_REG_ECC_CTL);
}
static void sunxi_nfc_hw_ecc_disable(struct nand_chip *nand)
{
struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
- writel(readl(nfc->regs + NFC_REG_ECC_CTL) & ~NFC_ECC_EN,
- nfc->regs + NFC_REG_ECC_CTL);
+ writel(0, nfc->regs + NFC_REG_ECC_CTL);
}
static inline void sunxi_nfc_user_data_to_buf(u32 user_data, u8 *buf)
@@ -1693,8 +1682,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
return -ENOTSUPP;
}
- sunxi_nand->ecc.mode = i;
-
/* HW ECC always request ECC bytes for 1024 bytes blocks */
ecc->bytes = DIV_ROUND_UP(ecc->strength * fls(8 * 1024), 8);
@@ -1726,6 +1713,12 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
ecc->read_oob_raw = nand_read_oob_std;
ecc->write_oob_raw = nand_write_oob_std;
+ sunxi_nand->ecc.ecc_ctl = NFC_ECC_MODE(i) | NFC_ECC_EXCEPTION |
+ NFC_ECC_PIPELINE | NFC_ECC_EN;
+
+ if (ecc->size == 512)
+ sunxi_nand->ecc.ecc_ctl |= NFC_ECC_BLOCK_512;
+
return 0;
}
--
2.37.4
On Sat, 2023-02-04 at 14:35:20 UTC, Samuel Holland wrote:
> The value computed by this function never changes for a given chip.
> Compute the whole register value once up front, instead of every time
> the ECC engine is enabled.
>
> Signed-off-by: Samuel Holland <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Sat, 2023-02-04 at 14:35:19 UTC, Samuel Holland wrote:
> The sunxi_nand_hw_ecc object is not shared, and it has the same lifetime
> as the sunxi_nand_chip which points to it, so we can embed it in the
> outer structure instead of using a pointer. This removes an unnecessary
> memory allocation and simplifies the error handling code.
>
> Signed-off-by: Samuel Holland <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Sat, 2023-02-04 at 14:35:18 UTC, Samuel Holland wrote:
> When using the hardware ECC engine, the OOB data is made available in
> the NFC_REG_USER_DATA registers, as one 32-bit word per ECC step. Any
> additional bytes are only accessible through raw reads and software
> descrambling. For efficiency, and to match the vendor driver, ignore
> these extra bytes when using hardware ECC.
>
> Note that until commit 34569d869532 ("mtd: rawnand: sunxi: Fix the size
> of the last OOB region"), this extra free area was reported with length
> zero, so this is not a functional change for any stable kernel user.
>
> Signed-off-by: Samuel Holland <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel