* [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6
@ 2026-03-27 14:05 Richard Genoud
2026-03-27 14:05 ` [PATCH v2 1/5] mtd: rawnand: sunxi: Replace hard coded value by a define Richard Genoud
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Richard Genoud @ 2026-03-27 14:05 UTC (permalink / raw)
To: Dario Binacchi, Michael Trimarchi, Tom Rini, Andre Przywara
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot, Richard Genoud
This series introduces the correct usage of user data length for
H6/H616.
If the older SoCs must use a fixed value of 4 user data bytes for each
ECC step, the H6 can have a different value for each step.
In the first round, the behavior was kept as the one used by A10.
But we have multiple choices:
- do like A10 like before (and not use all OOB)
- do like the vendor kernel (different from A10, but not using all OOB)
- use the whole OOB for user data bytes
In this second round, the third approach is implemented.
All patches but the 4th are code cleaning.
The 4th patch enables using a variable user data length for H6/H616
along with maximizing it.
A similar series has been accepted for Linux:
https://lore.kernel.org/lkml/20260317142437.580204-1-richard.genoud@bootlin.com/
Changes from v1:
- gather reviewed-by
- clean sunxi_nand_chip_init()
- use all possible remaining OOB space for user data bytes
Richard Genoud (5):
mtd: rawnand: sunxi: Replace hard coded value by a define
mtd: rawnand: sunxi: make the code mode self-explanatory
mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
mtd: rawnand: sunxi: introduce variable user data length
mtd: nand: raw: sunxi_spl: remove user data length reset
drivers/mtd/nand/raw/sunxi_nand.c | 292 +++++++++++++++++++-------
drivers/mtd/nand/raw/sunxi_nand.h | 3 +
drivers/mtd/nand/raw/sunxi_nand_spl.c | 129 +++++++++---
3 files changed, 324 insertions(+), 100 deletions(-)
base-commit: ba7bf918dafcd093ad733b07ba490baeb20cf5da
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/5] mtd: rawnand: sunxi: Replace hard coded value by a define
2026-03-27 14:05 [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Richard Genoud
@ 2026-03-27 14:05 ` Richard Genoud
2026-04-27 11:14 ` Andre Przywara
2026-03-27 14:05 ` [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Richard Genoud @ 2026-03-27 14:05 UTC (permalink / raw)
To: Dario Binacchi, Michael Trimarchi, Tom Rini, Andre Przywara
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot, Richard Genoud
The user data length (4) used all over the code hard coded.
And sometimes, it's not that trivial to know that it's the user data
length and not something else.
Moreover, for the H6/H616 this value is no more fixed by hardware, but
could be modified.
Using a define here makes the code more readable.
Suggested-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
drivers/mtd/nand/raw/sunxi_nand.c | 58 +++++++++++++--------------
drivers/mtd/nand/raw/sunxi_nand.h | 3 ++
drivers/mtd/nand/raw/sunxi_nand_spl.c | 7 +++-
3 files changed, 38 insertions(+), 30 deletions(-)
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index ef27a4b7a36a..91f7da2c5ce5 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -772,7 +772,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
return ret;
sunxi_nfc_reset_user_data_len(nfc);
- sunxi_nfc_set_user_data_len(nfc, 4, 0);
+ sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
sunxi_nfc_randomizer_enable(mtd);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
@@ -783,7 +783,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
if (ret)
return ret;
- *cur_off = oob_off + ecc->bytes + 4;
+ *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
@@ -794,7 +794,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
pattern = 0x0;
memset(data, pattern, ecc->size);
- memset(oob, pattern, ecc->bytes + 4);
+ memset(oob, pattern, ecc->bytes + USER_DATA_SZ);
return 1;
}
@@ -804,7 +804,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
- sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + 4, true, page);
+ sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ, true, page);
status = readl(nfc->regs + NFC_REG_ECC_ST);
if (status & NFC_ECC_ERR(0)) {
@@ -816,17 +816,17 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
nand->read_buf(mtd, data, ecc->size);
nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
- nand->read_buf(mtd, oob, ecc->bytes + 4);
+ nand->read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ);
}
ret = nand_check_erased_ecc_chunk(data, ecc->size,
- oob, ecc->bytes + 4,
+ oob, ecc->bytes + USER_DATA_SZ,
NULL, 0, ecc->strength);
if (ret >= 0)
raw_mode = 1;
} else {
/*
- * The engine protects 4 bytes of OOB data per chunk.
+ * The engine protects USER_DATA_SZ bytes of OOB data per chunk.
* Retrieve the corrected OOB bytes.
*/
sunxi_nfc_user_data_to_buf(readl(nfc->regs +
@@ -854,7 +854,7 @@ static void sunxi_nfc_hw_ecc_read_extra_oob(struct mtd_info *mtd,
{
struct nand_chip *nand = mtd_to_nand(mtd);
struct nand_ecc_ctrl *ecc = &nand->ecc;
- int offset = ((ecc->bytes + 4) * ecc->steps);
+ int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
int len = mtd->oobsize - offset;
if (len <= 0)
@@ -896,9 +896,9 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
/* Fill OOB data in */
if ((nand->options & NAND_NEED_SCRAMBLING) && bbm) {
- u8 user_data[4];
+ u8 user_data[USER_DATA_SZ];
- memcpy(user_data, oob, 4);
+ memcpy(user_data, oob, USER_DATA_SZ);
sunxi_nfc_randomize_bbm(mtd, page, user_data);
writel(sunxi_nfc_buf_to_user_data(user_data),
nfc->regs + NFC_REG_USER_DATA(nfc, 0));
@@ -915,7 +915,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
return ret;
sunxi_nfc_reset_user_data_len(nfc);
- sunxi_nfc_set_user_data_len(nfc, 4, 0);
+ sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
sunxi_nfc_randomizer_enable(mtd);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
@@ -927,7 +927,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
if (ret)
return ret;
- *cur_off = oob_off + ecc->bytes + 4;
+ *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
return 0;
}
@@ -938,7 +938,7 @@ static void sunxi_nfc_hw_ecc_write_extra_oob(struct mtd_info *mtd,
{
struct nand_chip *nand = mtd_to_nand(mtd);
struct nand_ecc_ctrl *ecc = &nand->ecc;
- int offset = ((ecc->bytes + 4) * ecc->steps);
+ int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
int len = mtd->oobsize - offset;
if (len <= 0)
@@ -966,7 +966,7 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
for (i = 0; i < ecc->steps; i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + 4);
+ int oob_off = i * (ecc->bytes + USER_DATA_SZ);
u8 *data = buf + data_off;
u8 *oob = chip->oob_poi + oob_off;
@@ -1004,7 +1004,7 @@ static int sunxi_nfc_hw_ecc_read_subpage(struct mtd_info *mtd,
for (i = data_offs / ecc->size;
i < DIV_ROUND_UP(data_offs + readlen, ecc->size); i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + 4);
+ int oob_off = i * (ecc->bytes + USER_DATA_SZ);
u8 *data = bufpoi + data_off;
u8 *oob = chip->oob_poi + oob_off;
@@ -1032,7 +1032,7 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
for (i = 0; i < ecc->steps; i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + 4);
+ int oob_off = i * (ecc->bytes + USER_DATA_SZ);
const u8 *data = buf + data_off;
const u8 *oob = chip->oob_poi + oob_off;
@@ -1066,7 +1066,7 @@ static int sunxi_nfc_hw_ecc_write_subpage(struct mtd_info *mtd,
for (i = data_offs / ecc->size;
i < DIV_ROUND_UP(data_offs + data_len, ecc->size); i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + 4);
+ int oob_off = i * (ecc->bytes + USER_DATA_SZ);
const u8 *data = buf + data_off;
const u8 *oob = chip->oob_poi + oob_off;
@@ -1095,10 +1095,10 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
sunxi_nfc_hw_ecc_enable(mtd);
for (i = 0; i < ecc->steps; i++) {
- int data_off = i * (ecc->size + ecc->bytes + 4);
+ int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
int oob_off = data_off + ecc->size;
u8 *data = buf + (i * ecc->size);
- u8 *oob = chip->oob_poi + (i * (ecc->bytes + 4));
+ u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
oob_off, &cur_off,
@@ -1129,10 +1129,10 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
sunxi_nfc_hw_ecc_enable(mtd);
for (i = 0; i < ecc->steps; i++) {
- int data_off = i * (ecc->size + ecc->bytes + 4);
+ int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
int oob_off = data_off + ecc->size;
const u8 *data = buf + (i * ecc->size);
- const u8 *oob = chip->oob_poi + (i * (ecc->bytes + 4));
+ const u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off,
oob, oob_off, &cur_off,
@@ -1390,7 +1390,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
layout = &data->layout;
nsectors = mtd->writesize / ecc->size;
- if (mtd->oobsize < ((ecc->bytes + 4) * nsectors)) {
+ if (mtd->oobsize < ((ecc->bytes + USER_DATA_SZ) * nsectors)) {
ret = -EINVAL;
goto err;
}
@@ -1440,14 +1440,14 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
layout->oobfree[i - 1].offset +
layout->oobfree[i - 1].length +
ecc->bytes;
- layout->oobfree[i].length = 4;
+ layout->oobfree[i].length = USER_DATA_SZ;
} else {
/*
* The first 2 bytes are used for BB markers, hence we
- * only have 2 bytes available in the first user data
- * section.
+ * only have USER_DATA_SZ - 2 bytes available in the
+ * first user data section.
*/
- layout->oobfree[i].length = 2;
+ layout->oobfree[i].length = USER_DATA_SZ - 2;
layout->oobfree[i].offset = 2;
}
@@ -1457,13 +1457,13 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
layout->oobfree[i].length + j;
}
- if (mtd->oobsize > (ecc->bytes + 4) * nsectors) {
+ if (mtd->oobsize > (ecc->bytes + USER_DATA_SZ) * nsectors) {
layout->oobfree[nsectors].offset =
layout->oobfree[nsectors - 1].offset +
layout->oobfree[nsectors - 1].length +
ecc->bytes;
layout->oobfree[nsectors].length = mtd->oobsize -
- ((ecc->bytes + 4) * nsectors);
+ ((ecc->bytes + USER_DATA_SZ) * nsectors);
}
return 0;
@@ -1481,7 +1481,7 @@ static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct mtd_info *mtd,
if (ret)
return ret;
- ecc->prepad = 4;
+ ecc->prepad = USER_DATA_SZ;
ecc->read_page = sunxi_nfc_hw_syndrome_ecc_read_page;
ecc->write_page = sunxi_nfc_hw_syndrome_ecc_write_page;
diff --git a/drivers/mtd/nand/raw/sunxi_nand.h b/drivers/mtd/nand/raw/sunxi_nand.h
index 6ee3ea14ee17..d7a8b3dd40c3 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.h
+++ b/drivers/mtd/nand/raw/sunxi_nand.h
@@ -181,6 +181,9 @@
#define NFC_MAX_CS 7
+/* On A10, the user data length register is 4 bytes */
+#define USER_DATA_SZ 4
+
/*
* NAND Controller capabilities structure: stores NAND controller capabilities
* for distinction between compatible strings.
diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
index 67f7d22ed2c8..3a5271b49c36 100644
--- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
@@ -269,6 +269,11 @@ static void sunxi_nfc_set_user_data_len(const struct nfc_config *nfc,
writel_nfc(val, NFC_REG_USER_DATA_LEN(nfc, step));
}
+/*
+ * Values in this table are obtained by doing:
+ * DIV_ROUND_UP(info->ecc_strength * 14, 8) + USER_DATA_SZ
+ * So it's the number of bytes needed for ECC + user data for one step.
+ */
#if defined(CONFIG_MACH_SUN50I_H616) || defined(CONFIG_MACH_SUN50I_H6)
static const int ecc_bytes[] = {
32, 46, 54, 60, 74, 82, 88, 96, 102, 110, 116, 124, 130, 138, 144
@@ -338,7 +343,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
nand_change_column(oob_off);
sunxi_nfc_reset_user_data_len(conf);
- sunxi_nfc_set_user_data_len(conf, 4, 0);
+ sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, 0);
nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
/* Get the ECC status */
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory
2026-03-27 14:05 [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Richard Genoud
2026-03-27 14:05 ` [PATCH v2 1/5] mtd: rawnand: sunxi: Replace hard coded value by a define Richard Genoud
@ 2026-03-27 14:05 ` Richard Genoud
2026-03-27 14:40 ` Michael Nazzareno Trimarchi
2026-04-27 12:23 ` Andre Przywara
2026-03-27 14:05 ` [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init() Richard Genoud
` (3 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Richard Genoud @ 2026-03-27 14:05 UTC (permalink / raw)
To: Dario Binacchi, Michael Trimarchi, Tom Rini, Andre Przywara
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot, Richard Genoud
In sunxi_nfc_hw_ecc_{read,write}_chunk(), the ECC step was force to 0,
the reason is not trivial to get when reading the code.
The explanation is that, from the NAND flash controller perspective, we
are indeed at step 0 for user data length and ECC errors.
Just add a const value with an explanation to clarify things.
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
drivers/mtd/nand/raw/sunxi_nand.c | 20 ++++++++++++--------
drivers/mtd/nand/raw/sunxi_nand_spl.c | 6 ++++--
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 91f7da2c5ce5..9fc9bc5e0198 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -758,6 +758,8 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
u32 status;
u32 pattern_found;
int ret;
+ /* From the controller point of view, we are at step 0 */
+ const int nfc_step = 0;
if (*cur_off != data_off)
nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
@@ -772,7 +774,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
return ret;
sunxi_nfc_reset_user_data_len(nfc);
- sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
+ sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
sunxi_nfc_randomizer_enable(mtd);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
@@ -787,7 +789,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
- if (pattern_found & NFC_ECC_PAT_FOUND(0)) {
+ if (pattern_found & NFC_ECC_PAT_FOUND(nfc_step)) {
u8 pattern = 0xff;
if (unlikely(!(readl(nfc->regs + NFC_REG_PAT_ID(nfc)) & 0x1)))
@@ -799,7 +801,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
return 1;
}
- ret = NFC_ECC_ERR_CNT(0, readl(nfc->regs + NFC_REG_ECC_ERR_CNT(nfc, 0)));
+ ret = NFC_ECC_ERR_CNT(nfc_step, readl(nfc->regs + NFC_REG_ECC_ERR_CNT(nfc, nfc_step)));
memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
@@ -807,7 +809,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ, true, page);
status = readl(nfc->regs + NFC_REG_ECC_ST);
- if (status & NFC_ECC_ERR(0)) {
+ if (status & NFC_ECC_ERR(nfc_step)) {
/*
* Re-read the data with the randomizer disabled to identify
* bitflips in erased pages.
@@ -830,7 +832,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
* Retrieve the corrected OOB bytes.
*/
sunxi_nfc_user_data_to_buf(readl(nfc->regs +
- NFC_REG_USER_DATA(nfc, 0)),
+ NFC_REG_USER_DATA(nfc, nfc_step)),
oob);
/* De-randomize the Bad Block Marker. */
@@ -888,6 +890,8 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
struct nand_ecc_ctrl *ecc = &nand->ecc;
int ret;
+ /* From the controller point of view, we are at step 0 */
+ const int nfc_step = 0;
if (data_off != *cur_off)
nand->cmdfunc(mtd, NAND_CMD_RNDIN, data_off, -1);
@@ -901,10 +905,10 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
memcpy(user_data, oob, USER_DATA_SZ);
sunxi_nfc_randomize_bbm(mtd, page, user_data);
writel(sunxi_nfc_buf_to_user_data(user_data),
- nfc->regs + NFC_REG_USER_DATA(nfc, 0));
+ nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
} else {
writel(sunxi_nfc_buf_to_user_data(oob),
- nfc->regs + NFC_REG_USER_DATA(nfc, 0));
+ nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
}
if (data_off + ecc->size != oob_off)
@@ -915,7 +919,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
return ret;
sunxi_nfc_reset_user_data_len(nfc);
- sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
+ sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
sunxi_nfc_randomizer_enable(mtd);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
index 3a5271b49c36..0d1f060cc425 100644
--- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
@@ -302,6 +302,8 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
int page = offs / conf->page_size;
u32 ecc_st, pattern_found;
int i;
+ /* From the controller point of view, we are at step 0 */
+ const int nfc_step = 0;
if (offs % conf->page_size || len % conf->ecc_size ||
len > conf->page_size || len < 0)
@@ -343,7 +345,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
nand_change_column(oob_off);
sunxi_nfc_reset_user_data_len(conf);
- sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, 0);
+ sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, nfc_step);
nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
/* Get the ECC status */
@@ -361,7 +363,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
pattern_found = readl_nfc(conf->caps->reg_pat_found);
pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(conf),
pattern_found);
- if (pattern_found & NFC_ECC_PAT_FOUND(0))
+ if (pattern_found & NFC_ECC_PAT_FOUND(nfc_step))
return 1;
}
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
2026-03-27 14:05 [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Richard Genoud
2026-03-27 14:05 ` [PATCH v2 1/5] mtd: rawnand: sunxi: Replace hard coded value by a define Richard Genoud
2026-03-27 14:05 ` [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
@ 2026-03-27 14:05 ` Richard Genoud
2026-03-27 14:42 ` Michael Nazzareno Trimarchi
2026-03-27 14:05 ` [PATCH v2 4/5] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Richard Genoud @ 2026-03-27 14:05 UTC (permalink / raw)
To: Dario Binacchi, Michael Trimarchi, Tom Rini, Andre Przywara
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot, Richard Genoud
In sunxi_nand_chip_init there's quite a lot of kfree/return, it's easy
to forget a kfree(), so use a goto/kfree instead.
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
drivers/mtd/nand/raw/sunxi_nand.c | 41 ++++++++++++++-----------------
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 9fc9bc5e0198..ecab9ebc9576 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1600,21 +1600,20 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
if (ret) {
dev_err(dev, "could not retrieve reg property: %d\n",
ret);
- kfree(chip);
- return ret;
+ goto out;
}
if (tmp > NFC_MAX_CS) {
dev_err(dev,
"invalid reg value: %u (max CS = 7)\n", tmp);
- kfree(chip);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
if (test_and_set_bit(tmp, &nfc->assigned_cs)) {
dev_err(dev, "CS %d already assigned\n", tmp);
- kfree(chip);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
chip->sels[i].cs = tmp;
@@ -1640,15 +1639,13 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
dev_err(dev,
"could not retrieve timings for ONFI mode 0: %d\n",
ret);
- kfree(chip);
- return ret;
+ goto out;
}
ret = sunxi_nand_chip_set_timings(nfc, chip, timings);
if (ret) {
dev_err(dev, "could not configure chip timings: %d\n", ret);
- kfree(chip);
- return ret;
+ goto out;
}
nand = &chip->nand;
@@ -1669,10 +1666,8 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
mtd = nand_to_mtd(nand);
ret = nand_scan_ident(mtd, nsels, NULL);
- if (ret) {
- kfree(chip);
- return ret;
- }
+ if (ret)
+ goto out;
if (nand->bbt_options & NAND_BBT_USE_FLASH)
nand->bbt_options |= NAND_BBT_NO_OOB;
@@ -1685,34 +1680,34 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
ret = sunxi_nand_chip_init_timings(nfc, chip);
if (ret) {
dev_err(dev, "could not configure chip timings: %d\n", ret);
- kfree(chip);
- return ret;
+ goto out;
}
ret = sunxi_nand_ecc_init(mtd, &nand->ecc);
if (ret) {
dev_err(dev, "ECC init failed: %d\n", ret);
- kfree(chip);
- return ret;
+ goto out;
}
ret = nand_scan_tail(mtd);
if (ret) {
dev_err(dev, "nand_scan_tail failed: %d\n", ret);
- kfree(chip);
- return ret;
+ goto out;
}
ret = nand_register(devnum, mtd);
if (ret) {
dev_err(dev, "failed to register mtd device: %d\n", ret);
- kfree(chip);
- return ret;
+ goto out;
}
list_add_tail(&chip->node, &nfc->chips);
- return 0;
+out:
+ if (ret)
+ kfree(chip);
+
+ return ret;
}
static int sunxi_nand_chips_init(struct udevice *dev, struct sunxi_nfc *nfc)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 4/5] mtd: rawnand: sunxi: introduce variable user data length
2026-03-27 14:05 [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Richard Genoud
` (2 preceding siblings ...)
2026-03-27 14:05 ` [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init() Richard Genoud
@ 2026-03-27 14:05 ` Richard Genoud
2026-05-01 12:46 ` Andre Przywara
2026-03-27 14:05 ` [PATCH v2 5/5] mtd: nand: raw: sunxi_spl: remove user data length reset Richard Genoud
2026-04-27 12:28 ` [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Andre Przywara
5 siblings, 1 reply; 18+ messages in thread
From: Richard Genoud @ 2026-03-27 14:05 UTC (permalink / raw)
To: Dario Binacchi, Michael Trimarchi, Tom Rini, Andre Przywara
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot, Richard Genoud
In Allwinner SoCs, user data can be added in OOB before each ECC data.
For older SoCs like A10, the user data size was the size of a register
(4 bytes) and was mandatory before each ECC step.
So, the A10 OOB Layout is:
[4Bytes USER_DATA_STEP0] [ECC_STEP0 bytes]
[4bytes USER_DATA_STEP1] [ECC_STEP1 bytes]
...
NB: the BBM is stored at the beginning of the USER_DATA_STEP0.
Now, for H6/H616 NAND flash controller, this user data can have a
different size for each step.
So, we are maximizing the user data length to use as many OOB bytes as
possible.
Fixes: 7d1de9801151 ("mtd: rawnand: sunxi_spl: add support for H6/H616 nand controller")
Fixes: f163da5e6d26 ("mtd: rawnand: sunxi: add support for H6/H616 nand controller")
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
drivers/mtd/nand/raw/sunxi_nand.c | 235 +++++++++++++++++++++-----
drivers/mtd/nand/raw/sunxi_nand_spl.c | 107 ++++++++++--
2 files changed, 287 insertions(+), 55 deletions(-)
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index ecab9ebc9576..c3bec1fe517e 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -114,6 +114,7 @@ struct sunxi_nand_hw_ecc {
* @clk_rate: clk_rate required for this NAND chip
* @timing_cfg TIMING_CFG register value for this NAND chip
* @selected: current active CS
+ * @user_data_bytes array of user data lengths for all ECC steps
* @nsels: number of CS lines required by the NAND chip
* @sels: array of CS lines descriptions
*/
@@ -128,6 +129,7 @@ struct sunxi_nand_chip {
u32 addr[2];
int cmd_cycles;
u8 cmd[2];
+ u8 *user_data_bytes;
int nsels;
struct sunxi_nand_chip_sel sels[0];
};
@@ -744,19 +746,73 @@ static void sunxi_nfc_set_user_data_len(struct sunxi_nfc *nfc,
writel(val, nfc->regs + NFC_REG_USER_DATA_LEN(nfc, step));
}
+static u8 sunxi_nfc_user_data_sz(const struct sunxi_nand_chip *sunxi_nand, int step)
+{
+ if (!sunxi_nand->user_data_bytes)
+ return USER_DATA_SZ;
+
+ return sunxi_nand->user_data_bytes[step];
+}
+
+static void sunxi_nfc_hw_ecc_get_prot_oob_bytes(struct nand_chip *nand, u8 *oob,
+ int step, bool bbm, int page,
+ unsigned int user_data_sz)
+{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
+ struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+ u32 user_data;
+
+ if (!nfc->caps->reg_user_data_len) {
+ /*
+ * For A10, the user data for step n is in the nth
+ * REG_USER_DATA
+ */
+ user_data = readl(nfc->regs + NFC_REG_USER_DATA(nfc, step));
+ sunxi_nfc_user_data_to_buf(user_data, oob);
+
+ } else {
+ /*
+ * For H6 NAND controller, the user data for all steps is
+ * contained in 32 user data registers, but not at a specific
+ * offset for each step, they are just concatenated.
+ */
+ unsigned int user_data_off = 0;
+ unsigned int reg_off;
+ u8 *ptr = oob;
+ unsigned int i;
+
+ for (i = 0; i < step; i++)
+ user_data_off += sunxi_nfc_user_data_sz(sunxi_nand, i);
+
+ user_data_off /= 4;
+ for (i = 0; i < user_data_sz / 4; i++, ptr += 4) {
+ reg_off = NFC_REG_USER_DATA(nfc, user_data_off + i);
+ user_data = readl(nfc->regs + reg_off);
+ sunxi_nfc_user_data_to_buf(user_data, ptr);
+ }
+ }
+
+ /* De-randomize the Bad Block Marker. */
+ if (bbm && nand->options & NAND_NEED_SCRAMBLING)
+ sunxi_nfc_randomize_bbm(&nand->mtd, page, oob);
+}
+
static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
u8 *data, int data_off,
u8 *oob, int oob_off,
int *cur_off,
unsigned int *max_bitflips,
- bool bbm, int page)
+ int step, int page)
{
struct nand_chip *nand = mtd_to_nand(mtd);
struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
+ unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, step);
struct nand_ecc_ctrl *ecc = &nand->ecc;
int raw_mode = 0;
u32 status;
u32 pattern_found;
+ bool bbm = !step;
int ret;
/* From the controller point of view, we are at step 0 */
const int nfc_step = 0;
@@ -773,8 +829,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
if (ret)
return ret;
- sunxi_nfc_reset_user_data_len(nfc);
- sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
+ sunxi_nfc_set_user_data_len(nfc, user_data_sz, nfc_step);
sunxi_nfc_randomizer_enable(mtd);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
@@ -785,7 +840,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
if (ret)
return ret;
- *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
+ *cur_off = oob_off + ecc->bytes + user_data_sz;
pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
@@ -796,7 +851,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
pattern = 0x0;
memset(data, pattern, ecc->size);
- memset(oob, pattern, ecc->bytes + USER_DATA_SZ);
+ memset(oob, pattern, ecc->bytes + user_data_sz);
return 1;
}
@@ -806,7 +861,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
- sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ, true, page);
+ sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + user_data_sz, true, page);
status = readl(nfc->regs + NFC_REG_ECC_ST);
if (status & NFC_ECC_ERR(nfc_step)) {
@@ -818,26 +873,21 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
nand->read_buf(mtd, data, ecc->size);
nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
- nand->read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ);
+ nand->read_buf(mtd, oob, ecc->bytes + user_data_sz);
}
ret = nand_check_erased_ecc_chunk(data, ecc->size,
- oob, ecc->bytes + USER_DATA_SZ,
+ oob, ecc->bytes + user_data_sz,
NULL, 0, ecc->strength);
if (ret >= 0)
raw_mode = 1;
} else {
/*
- * The engine protects USER_DATA_SZ bytes of OOB data per chunk.
+ * The engine protects user_data_sz bytes of OOB data per chunk.
* Retrieve the corrected OOB bytes.
*/
- sunxi_nfc_user_data_to_buf(readl(nfc->regs +
- NFC_REG_USER_DATA(nfc, nfc_step)),
- oob);
-
- /* De-randomize the Bad Block Marker. */
- if (bbm && nand->options & NAND_NEED_SCRAMBLING)
- sunxi_nfc_randomize_bbm(mtd, page, oob);
+ sunxi_nfc_hw_ecc_get_prot_oob_bytes(nand, oob, nfc_step,
+ bbm, page, user_data_sz);
}
if (ret < 0) {
@@ -850,13 +900,30 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
return raw_mode;
}
+/*
+ * Returns the offset of the OOB for each step.
+ * (it includes the user data before the ECC data.)
+ */
+static int sunxi_get_oob_offset(struct sunxi_nand_chip *sunxi_nand,
+ struct nand_ecc_ctrl *ecc, int step)
+{
+ int ecc_off = step * ecc->bytes;
+ int i;
+
+ for (i = 0; i < step; i++)
+ ecc_off += sunxi_nfc_user_data_sz(sunxi_nand, i);
+
+ return ecc_off;
+}
+
static void sunxi_nfc_hw_ecc_read_extra_oob(struct mtd_info *mtd,
u8 *oob, int *cur_off,
bool randomize, int page)
{
struct nand_chip *nand = mtd_to_nand(mtd);
struct nand_ecc_ctrl *ecc = &nand->ecc;
- int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
+ int offset = sunxi_get_oob_offset(sunxi_nand, ecc, ecc->steps);
int len = mtd->oobsize - offset;
if (len <= 0)
@@ -883,12 +950,15 @@ static inline u32 sunxi_nfc_buf_to_user_data(const u8 *buf)
static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
const u8 *data, int data_off,
const u8 *oob, int oob_off,
- int *cur_off, bool bbm,
+ int *cur_off, int step,
int page)
{
struct nand_chip *nand = mtd_to_nand(mtd);
struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
+ unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, step);
struct nand_ecc_ctrl *ecc = &nand->ecc;
+ bool bbm = !step;
int ret;
/* From the controller point of view, we are at step 0 */
const int nfc_step = 0;
@@ -900,12 +970,17 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
/* Fill OOB data in */
if ((nand->options & NAND_NEED_SCRAMBLING) && bbm) {
- u8 user_data[USER_DATA_SZ];
+ u8 *user_data;
- memcpy(user_data, oob, USER_DATA_SZ);
+ user_data = kzalloc(user_data_sz, GFP_KERNEL);
+ if (!user_data)
+ return -ENOMEM;
+
+ memcpy(user_data, oob, user_data_sz);
sunxi_nfc_randomize_bbm(mtd, page, user_data);
writel(sunxi_nfc_buf_to_user_data(user_data),
nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
+ kfree(user_data);
} else {
writel(sunxi_nfc_buf_to_user_data(oob),
nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
@@ -918,8 +993,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
if (ret)
return ret;
- sunxi_nfc_reset_user_data_len(nfc);
- sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
+ sunxi_nfc_set_user_data_len(nfc, user_data_sz, nfc_step);
sunxi_nfc_randomizer_enable(mtd);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
@@ -931,7 +1005,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
if (ret)
return ret;
- *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
+ *cur_off = oob_off + ecc->bytes + user_data_sz;
return 0;
}
@@ -942,7 +1016,8 @@ static void sunxi_nfc_hw_ecc_write_extra_oob(struct mtd_info *mtd,
{
struct nand_chip *nand = mtd_to_nand(mtd);
struct nand_ecc_ctrl *ecc = &nand->ecc;
- int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
+ int offset = sunxi_get_oob_offset(sunxi_nand, ecc, ecc->steps);
int len = mtd->oobsize - offset;
if (len <= 0)
@@ -961,6 +1036,8 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
struct nand_chip *chip, uint8_t *buf,
int oob_required, int page)
{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
+ struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
struct nand_ecc_ctrl *ecc = &chip->ecc;
unsigned int max_bitflips = 0;
int ret, i, cur_off = 0;
@@ -968,16 +1045,17 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
sunxi_nfc_hw_ecc_enable(mtd);
+ sunxi_nfc_reset_user_data_len(nfc);
for (i = 0; i < ecc->steps; i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+ int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
u8 *data = buf + data_off;
u8 *oob = chip->oob_poi + oob_off;
ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
oob_off + mtd->writesize,
&cur_off, &max_bitflips,
- !i, page);
+ i, page);
if (ret < 0)
return ret;
else if (ret)
@@ -998,23 +1076,26 @@ static int sunxi_nfc_hw_ecc_read_subpage(struct mtd_info *mtd,
uint32_t data_offs, uint32_t readlen,
uint8_t *bufpoi, int page)
{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
+ struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
struct nand_ecc_ctrl *ecc = &chip->ecc;
int ret, i, cur_off = 0;
unsigned int max_bitflips = 0;
sunxi_nfc_hw_ecc_enable(mtd);
+ sunxi_nfc_reset_user_data_len(nfc);
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
for (i = data_offs / ecc->size;
i < DIV_ROUND_UP(data_offs + readlen, ecc->size); i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+ int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
u8 *data = bufpoi + data_off;
u8 *oob = chip->oob_poi + oob_off;
ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off,
oob, oob_off + mtd->writesize,
- &cur_off, &max_bitflips, !i, page);
+ &cur_off, &max_bitflips, i, page);
if (ret < 0)
return ret;
}
@@ -1029,20 +1110,23 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
const uint8_t *buf, int oob_required,
int page)
{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
+ struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
struct nand_ecc_ctrl *ecc = &chip->ecc;
int ret, i, cur_off = 0;
sunxi_nfc_hw_ecc_enable(mtd);
+ sunxi_nfc_reset_user_data_len(nfc);
for (i = 0; i < ecc->steps; i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+ int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
const u8 *data = buf + data_off;
const u8 *oob = chip->oob_poi + oob_off;
ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off, oob,
oob_off + mtd->writesize,
- &cur_off, !i, page);
+ &cur_off, i, page);
if (ret)
return ret;
}
@@ -1062,21 +1146,24 @@ static int sunxi_nfc_hw_ecc_write_subpage(struct mtd_info *mtd,
const u8 *buf, int oob_required,
int page)
{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
+ struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
struct nand_ecc_ctrl *ecc = &chip->ecc;
int ret, i, cur_off = 0;
sunxi_nfc_hw_ecc_enable(mtd);
+ sunxi_nfc_reset_user_data_len(nfc);
for (i = data_offs / ecc->size;
i < DIV_ROUND_UP(data_offs + data_len, ecc->size); i++) {
int data_off = i * ecc->size;
- int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+ int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
const u8 *data = buf + data_off;
const u8 *oob = chip->oob_poi + oob_off;
ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off, oob,
oob_off + mtd->writesize,
- &cur_off, !i, page);
+ &cur_off, i, page);
if (ret)
return ret;
}
@@ -1091,18 +1178,23 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
uint8_t *buf, int oob_required,
int page)
{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
+ struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
struct nand_ecc_ctrl *ecc = &chip->ecc;
unsigned int max_bitflips = 0;
int ret, i, cur_off = 0;
bool raw_mode = false;
+ /* With hw_syndrome, user data length is fixed */
+ unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, 0);
sunxi_nfc_hw_ecc_enable(mtd);
+ sunxi_nfc_reset_user_data_len(nfc);
for (i = 0; i < ecc->steps; i++) {
- int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
+ int data_off = i * (ecc->size + ecc->bytes + user_data_sz);
int oob_off = data_off + ecc->size;
u8 *data = buf + (i * ecc->size);
- u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
+ u8 *oob = chip->oob_poi + (i * (ecc->bytes + user_data_sz));
ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
oob_off, &cur_off,
@@ -1127,16 +1219,19 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
const uint8_t *buf,
int oob_required, int page)
{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
struct nand_ecc_ctrl *ecc = &chip->ecc;
int ret, i, cur_off = 0;
+ /* With hw_syndrome, user data length is fixed */
+ unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, 0);
sunxi_nfc_hw_ecc_enable(mtd);
for (i = 0; i < ecc->steps; i++) {
- int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
+ int data_off = i * (ecc->size + ecc->bytes + user_data_sz);
int oob_off = data_off + ecc->size;
const u8 *data = buf + (i * ecc->size);
- const u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
+ const u8 *oob = chip->oob_poi + (i * (ecc->bytes + user_data_sz));
ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off,
oob, oob_off, &cur_off,
@@ -1338,6 +1433,34 @@ static int sunxi_nand_chip_init_timings(struct sunxi_nfc *nfc,
return sunxi_nand_chip_set_timings(nfc, chip, timings);
}
+static int sunxi_nfc_maximize_user_data(struct nand_chip *nand, uint32_t oobsize,
+ int ecc_bytes, int nsectors)
+{
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
+ struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+ const struct sunxi_nfc_caps *c = nfc->caps;
+ int remaining_bytes = oobsize - (ecc_bytes * nsectors);
+ int i, step;
+
+ sunxi_nand->user_data_bytes = devm_kzalloc(nfc->dev, nsectors,
+ GFP_KERNEL);
+ if (!sunxi_nand->user_data_bytes)
+ return -ENOMEM;
+
+ for (step = 0; (step < nsectors) && (remaining_bytes > 0); step++) {
+ for (i = 0; i < c->nuser_data_tab; i++) {
+ if (c->user_data_len_tab[i] > remaining_bytes)
+ break;
+ sunxi_nand->user_data_bytes[step] = c->user_data_len_tab[i];
+ }
+ remaining_bytes -= sunxi_nand->user_data_bytes[step];
+ if (sunxi_nand->user_data_bytes[step] == 0)
+ break;
+ }
+
+ return 0;
+}
+
static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
struct nand_ecc_ctrl *ecc)
{
@@ -1346,6 +1469,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
struct sunxi_nand_hw_ecc *data;
struct nand_ecclayout *layout;
+ unsigned int total_user_data_sz = 0;
int nsectors;
int ret;
int i;
@@ -1394,7 +1518,15 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
layout = &data->layout;
nsectors = mtd->writesize / ecc->size;
- if (mtd->oobsize < ((ecc->bytes + USER_DATA_SZ) * nsectors)) {
+ /* Use the remaining OOB space for user data */
+ if (nfc->caps->reg_user_data_len)
+ sunxi_nfc_maximize_user_data(nand, mtd->oobsize, ecc->bytes,
+ nsectors);
+
+ for (i = 0; i < nsectors; i++)
+ total_user_data_sz += sunxi_nfc_user_data_sz(sunxi_nand, i);
+
+ if (mtd->oobsize < ecc->bytes * nsectors + total_user_data_sz) {
ret = -EINVAL;
goto err;
}
@@ -1408,6 +1540,8 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
err:
kfree(data);
+ devm_kfree(nfc->dev, sunxi_nand->user_data_bytes);
+ sunxi_nand->user_data_bytes = NULL;
return ret;
}
@@ -1422,7 +1556,10 @@ static void sunxi_nand_hw_common_ecc_ctrl_cleanup(struct nand_ecc_ctrl *ecc)
static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
struct nand_ecc_ctrl *ecc)
{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
struct nand_ecclayout *layout;
+ unsigned int total_user_data_sz = 0;
int nsectors;
int i, j;
int ret;
@@ -1444,14 +1581,14 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
layout->oobfree[i - 1].offset +
layout->oobfree[i - 1].length +
ecc->bytes;
- layout->oobfree[i].length = USER_DATA_SZ;
+ layout->oobfree[i].length = sunxi_nfc_user_data_sz(sunxi_nand, i);
} else {
/*
* The first 2 bytes are used for BB markers, hence we
- * only have USER_DATA_SZ - 2 bytes available in the
+ * only have user_data_len(0) - 2 bytes available in the
* first user data section.
*/
- layout->oobfree[i].length = USER_DATA_SZ - 2;
+ layout->oobfree[i].length = sunxi_nfc_user_data_sz(sunxi_nand, i) - 2;
layout->oobfree[i].offset = 2;
}
@@ -1461,13 +1598,16 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
layout->oobfree[i].length + j;
}
- if (mtd->oobsize > (ecc->bytes + USER_DATA_SZ) * nsectors) {
+ for (i = 0; i < nsectors; i++)
+ total_user_data_sz += sunxi_nfc_user_data_sz(sunxi_nand, i);
+
+ if (mtd->oobsize > ecc->bytes * nsectors + total_user_data_sz) {
layout->oobfree[nsectors].offset =
layout->oobfree[nsectors - 1].offset +
layout->oobfree[nsectors - 1].length +
ecc->bytes;
layout->oobfree[nsectors].length = mtd->oobsize -
- ((ecc->bytes + USER_DATA_SZ) * nsectors);
+ (ecc->bytes * nsectors + total_user_data_sz);
}
return 0;
@@ -1476,6 +1616,8 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct mtd_info *mtd,
struct nand_ecc_ctrl *ecc)
{
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
struct nand_ecclayout *layout;
int nsectors;
int i;
@@ -1485,7 +1627,13 @@ static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct mtd_info *mtd,
if (ret)
return ret;
- ecc->prepad = USER_DATA_SZ;
+ for (i = 0; i < nsectors; i++)
+ if (sunxi_nfc_user_data_sz(sunxi_nand, i) !=
+ sunxi_nfc_user_data_sz(sunxi_nand, 0)) {
+ dev_err(mtd->dev, "Variable user data length not upported with NAND_ECC_HW_SYNDROME\n");
+ return -EOPNOTSUPP;
+ }
+ ecc->prepad = sunxi_nfc_user_data_sz(sunxi_nand, 0);
ecc->read_page = sunxi_nfc_hw_syndrome_ecc_read_page;
ecc->write_page = sunxi_nfc_hw_syndrome_ecc_write_page;
@@ -1735,6 +1883,7 @@ static void sunxi_nand_chips_cleanup(struct sunxi_nfc *nfc)
nand_release(&chip->mtd);
sunxi_nand_ecc_cleanup(&chip->nand.ecc);
list_del(&chip->node);
+ devm_kfree(nfc->dev, chip->user_data_bytes);
kfree(chip);
}
}
diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
index 0d1f060cc425..784ffb00cf70 100644
--- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
@@ -28,6 +28,7 @@ struct nfc_config {
bool randomize;
bool valid;
const struct sunxi_nfc_caps *caps;
+ u8 *user_data_bytes;
};
/* minimal "boot0" style NAND support for Allwinner A20 */
@@ -271,16 +272,17 @@ static void sunxi_nfc_set_user_data_len(const struct nfc_config *nfc,
/*
* Values in this table are obtained by doing:
- * DIV_ROUND_UP(info->ecc_strength * 14, 8) + USER_DATA_SZ
- * So it's the number of bytes needed for ECC + user data for one step.
+ * DIV_ROUND_UP(info->ecc_strength * 14, 8)
+ * So it's the number of bytes needed for ECC one step
+ * (not counting the user data length)
*/
#if defined(CONFIG_MACH_SUN50I_H616) || defined(CONFIG_MACH_SUN50I_H6)
static const int ecc_bytes[] = {
- 32, 46, 54, 60, 74, 82, 88, 96, 102, 110, 116, 124, 130, 138, 144
+ 28, 42, 50, 56, 70, 78, 84, 92, 98, 106, 112, 120, 126, 134, 140
};
#else
static const int ecc_bytes[] = {
- 32, 46, 54, 60, 74, 88, 102, 110, 116
+ 28, 42, 50, 56, 70, 84, 98, 106, 112
};
#endif
@@ -293,6 +295,14 @@ static void nand_readlcpy(u32 *dest, u32 * __iomem src, size_t len)
*dest++ = readl(src++);
}
+static u8 nand_user_data_sz(const struct nfc_config *conf, int step)
+{
+ if (!conf->user_data_bytes)
+ return USER_DATA_SZ;
+
+ return conf->user_data_bytes[step];
+}
+
static int nand_read_page(const struct nfc_config *conf, u32 offs,
void *dest, int len)
{
@@ -300,6 +310,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
u16 rand_seed = 0;
int oob_chunk_sz = ecc_bytes[conf->ecc_strength];
int page = offs / conf->page_size;
+ int oob_off = conf->page_size;
u32 ecc_st, pattern_found;
int i;
/* From the controller point of view, we are at step 0 */
@@ -316,9 +327,9 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
/* Retrieve data from SRAM (PIO) */
for (i = 0; i < nsectors; i++) {
int data_off = i * conf->ecc_size;
- int oob_off = conf->page_size + (i * oob_chunk_sz);
u8 *data = dest + data_off;
u32 ecc512_bit = 0;
+ unsigned int user_data_sz = nand_user_data_sz(conf, i);
if (conf->caps->has_ecc_block_512 && conf->ecc_size == 512)
ecc512_bit = NFC_ECC_BLOCK_512;
@@ -345,7 +356,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
nand_change_column(oob_off);
sunxi_nfc_reset_user_data_len(conf);
- sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, nfc_step);
+ sunxi_nfc_set_user_data_len(conf, user_data_sz, nfc_step);
nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
/* Get the ECC status */
@@ -371,13 +382,61 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
nand_readlcpy((u32 *)data,
(void *)(uintptr_t)SUNXI_NFC_BASE + NFC_RAM0_BASE,
conf->ecc_size);
-
/* Stop the ECC engine */
writel_nfc(readl_nfc(NFC_REG_ECC_CTL) & ~NFC_ECC_EN,
NFC_REG_ECC_CTL);
if (data_off + conf->ecc_size >= len)
break;
+
+ oob_off += oob_chunk_sz + user_data_sz;
+ }
+
+ return 0;
+}
+
+static int nand_min_user_data_sz(struct nfc_config *conf, int nsectors)
+{
+ const struct sunxi_nfc_caps *c = conf->caps;
+ int min_user_data_sz = 0;
+ int i;
+
+ if (!c->reg_user_data_len) {
+ for (i = 0; i < nsectors; i++)
+ min_user_data_sz += nand_user_data_sz(conf, i);
+ } else {
+ for (i = 0; i < c->nuser_data_tab; i++)
+ /* We want at least enough size for the BBM */
+ if (c->user_data_len_tab[i] >= 2)
+ break;
+ min_user_data_sz = c->user_data_len_tab[i];
+ }
+
+ return min_user_data_sz;
+}
+
+static int nand_maximize_user_data(struct nfc_config *conf, uint32_t oobsize,
+ int ecc_len, int nsectors)
+{
+ const struct sunxi_nfc_caps *c = conf->caps;
+ int remaining_bytes = oobsize - (ecc_len * nsectors);
+ int i, step;
+
+ kfree(conf->user_data_bytes);
+
+ conf->user_data_bytes = kzalloc(nsectors, GFP_KERNEL);
+ if (!conf->user_data_bytes)
+ return -ENOMEM;
+
+ for (step = 0; (step < nsectors) && (remaining_bytes > 0); step++) {
+ for (i = 0; i < c->nuser_data_tab; i++) {
+ if (c->user_data_len_tab[i] > remaining_bytes)
+ break;
+ conf->user_data_bytes[step] = c->user_data_len_tab[i];
+ }
+ remaining_bytes -= conf->user_data_bytes[step];
+ if (conf->user_data_bytes[step] == 0)
+ break;
}
return 0;
@@ -387,7 +446,8 @@ static int nand_max_ecc_strength(struct nfc_config *conf)
{
int max_oobsize, max_ecc_bytes;
int nsectors = conf->page_size / conf->ecc_size;
- int i;
+ unsigned int total_user_data_sz = 0;
+ int ecc_idx, i;
/*
* ECC strength is limited by the size of the OOB area which is
@@ -412,15 +472,38 @@ static int nand_max_ecc_strength(struct nfc_config *conf)
max_ecc_bytes = max_oobsize / nsectors;
- for (i = 0; i < ARRAY_SIZE(ecc_bytes); i++) {
- if (ecc_bytes[i] > max_ecc_bytes)
+ /*
+ * nand_min_user_data_sz() will return the total_user_data_sz in case
+ * of a fixed user data length, or the minimal usable user data size
+ * in case of variable data length (with at least enough space for the
+ * BBM.
+ */
+ total_user_data_sz = nand_min_user_data_sz(conf, nsectors);
+
+ for (ecc_idx = 0; ecc_idx < ARRAY_SIZE(ecc_bytes); ecc_idx++) {
+ if (ecc_bytes[ecc_idx] + total_user_data_sz > max_ecc_bytes)
break;
}
- if (!i)
+ if (!ecc_idx)
return -EINVAL;
- return i - 1;
+ ecc_idx--;
+
+ /*
+ * The rationale for variable data length is to prioritize maximum ECC
+ * strength, and then use the remaining space for user data.
+ */
+ if (conf->caps->reg_user_data_len) {
+ nand_maximize_user_data(conf, max_oobsize,
+ ecc_bytes[ecc_idx], nsectors);
+
+ total_user_data_sz = 0;
+ for (i = 0; i < nsectors; i++)
+ total_user_data_sz += nand_user_data_sz(conf, i);
+ }
+
+ return ecc_idx;
}
static int nand_detect_ecc_config(struct nfc_config *conf, u32 offs,
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 5/5] mtd: nand: raw: sunxi_spl: remove user data length reset
2026-03-27 14:05 [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Richard Genoud
` (3 preceding siblings ...)
2026-03-27 14:05 ` [PATCH v2 4/5] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
@ 2026-03-27 14:05 ` Richard Genoud
2026-05-01 12:47 ` Andre Przywara
2026-04-27 12:28 ` [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Andre Przywara
5 siblings, 1 reply; 18+ messages in thread
From: Richard Genoud @ 2026-03-27 14:05 UTC (permalink / raw)
To: Dario Binacchi, Michael Trimarchi, Tom Rini, Andre Przywara
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot, Richard Genoud
No need to reset user data length registers in SPL.
In SPL, only the first user data length register is used, so we don't
need to reset all of them.
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
drivers/mtd/nand/raw/sunxi_nand_spl.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
index 784ffb00cf70..cf351de4e8de 100644
--- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
@@ -224,22 +224,6 @@ static int nand_change_column(u16 column)
return 0;
}
-/*
- * On H6/H616 the user_data length has to be set in specific registers
- * before writing.
- */
-static void sunxi_nfc_reset_user_data_len(const struct nfc_config *nfc)
-{
- int loop_step = NFC_REG_USER_DATA_LEN_CAPACITY;
-
- /* not all SoCs have this register */
- if (!NFC_REG_USER_DATA_LEN(nfc, 0))
- return;
-
- for (int i = 0; i < nfc->caps->max_ecc_steps; i += loop_step)
- writel_nfc(0, NFC_REG_USER_DATA_LEN(nfc, i));
-}
-
static void sunxi_nfc_set_user_data_len(const struct nfc_config *nfc,
int len, int step)
{
@@ -355,7 +339,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
*/
nand_change_column(oob_off);
- sunxi_nfc_reset_user_data_len(conf);
sunxi_nfc_set_user_data_len(conf, user_data_sz, nfc_step);
nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory
2026-03-27 14:05 ` [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
@ 2026-03-27 14:40 ` Michael Nazzareno Trimarchi
2026-04-27 12:23 ` Andre Przywara
1 sibling, 0 replies; 18+ messages in thread
From: Michael Nazzareno Trimarchi @ 2026-03-27 14:40 UTC (permalink / raw)
To: Richard Genoud
Cc: Dario Binacchi, Tom Rini, Andre Przywara, Andrew Goodbody,
Miquel Raynal, James Hilliard, Boris Brezillon, Thomas Petazzoni,
u-boot
Hi
On Fri, Mar 27, 2026 at 3:05 PM Richard Genoud
<richard.genoud@bootlin.com> wrote:
>
> In sunxi_nfc_hw_ecc_{read,write}_chunk(), the ECC step was force to 0,
> the reason is not trivial to get when reading the code.
>
> The explanation is that, from the NAND flash controller perspective, we
> are indeed at step 0 for user data length and ECC errors.
>
> Just add a const value with an explanation to clarify things.
>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> ---
> drivers/mtd/nand/raw/sunxi_nand.c | 20 ++++++++++++--------
> drivers/mtd/nand/raw/sunxi_nand_spl.c | 6 ++++--
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index 91f7da2c5ce5..9fc9bc5e0198 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -758,6 +758,8 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> u32 status;
> u32 pattern_found;
> int ret;
> + /* From the controller point of view, we are at step 0 */
> + const int nfc_step = 0;
>
> if (*cur_off != data_off)
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
> @@ -772,7 +774,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> return ret;
>
> sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
> + sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
> @@ -787,7 +789,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
>
> pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
> pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
> - if (pattern_found & NFC_ECC_PAT_FOUND(0)) {
> + if (pattern_found & NFC_ECC_PAT_FOUND(nfc_step)) {
> u8 pattern = 0xff;
>
> if (unlikely(!(readl(nfc->regs + NFC_REG_PAT_ID(nfc)) & 0x1)))
> @@ -799,7 +801,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> return 1;
> }
>
> - ret = NFC_ECC_ERR_CNT(0, readl(nfc->regs + NFC_REG_ECC_ERR_CNT(nfc, 0)));
> + ret = NFC_ECC_ERR_CNT(nfc_step, readl(nfc->regs + NFC_REG_ECC_ERR_CNT(nfc, nfc_step)));
>
> memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
>
> @@ -807,7 +809,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ, true, page);
>
> status = readl(nfc->regs + NFC_REG_ECC_ST);
> - if (status & NFC_ECC_ERR(0)) {
> + if (status & NFC_ECC_ERR(nfc_step)) {
> /*
> * Re-read the data with the randomizer disabled to identify
> * bitflips in erased pages.
> @@ -830,7 +832,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> * Retrieve the corrected OOB bytes.
> */
> sunxi_nfc_user_data_to_buf(readl(nfc->regs +
> - NFC_REG_USER_DATA(nfc, 0)),
> + NFC_REG_USER_DATA(nfc, nfc_step)),
> oob);
>
> /* De-randomize the Bad Block Marker. */
> @@ -888,6 +890,8 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> int ret;
> + /* From the controller point of view, we are at step 0 */
> + const int nfc_step = 0;
>
> if (data_off != *cur_off)
> nand->cmdfunc(mtd, NAND_CMD_RNDIN, data_off, -1);
> @@ -901,10 +905,10 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> memcpy(user_data, oob, USER_DATA_SZ);
> sunxi_nfc_randomize_bbm(mtd, page, user_data);
> writel(sunxi_nfc_buf_to_user_data(user_data),
> - nfc->regs + NFC_REG_USER_DATA(nfc, 0));
> + nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
> } else {
> writel(sunxi_nfc_buf_to_user_data(oob),
> - nfc->regs + NFC_REG_USER_DATA(nfc, 0));
> + nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
> }
>
> if (data_off + ecc->size != oob_off)
> @@ -915,7 +919,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> return ret;
>
> sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
> + sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index 3a5271b49c36..0d1f060cc425 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -302,6 +302,8 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> int page = offs / conf->page_size;
> u32 ecc_st, pattern_found;
> int i;
> + /* From the controller point of view, we are at step 0 */
> + const int nfc_step = 0;
>
> if (offs % conf->page_size || len % conf->ecc_size ||
> len > conf->page_size || len < 0)
> @@ -343,7 +345,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> nand_change_column(oob_off);
>
> sunxi_nfc_reset_user_data_len(conf);
> - sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, 0);
> + sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, nfc_step);
>
> nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
> /* Get the ECC status */
> @@ -361,7 +363,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> pattern_found = readl_nfc(conf->caps->reg_pat_found);
> pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(conf),
> pattern_found);
> - if (pattern_found & NFC_ECC_PAT_FOUND(0))
> + if (pattern_found & NFC_ECC_PAT_FOUND(nfc_step))
> return 1;
> }
>
Reviewed-By: Michael Trimarchi <michael@amarulasolutions.com>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
2026-03-27 14:05 ` [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init() Richard Genoud
@ 2026-03-27 14:42 ` Michael Nazzareno Trimarchi
2026-04-27 12:35 ` Andre Przywara
0 siblings, 1 reply; 18+ messages in thread
From: Michael Nazzareno Trimarchi @ 2026-03-27 14:42 UTC (permalink / raw)
To: Richard Genoud
Cc: Dario Binacchi, Tom Rini, Andre Przywara, Andrew Goodbody,
Miquel Raynal, James Hilliard, Boris Brezillon, Thomas Petazzoni,
u-boot
Hi Richard
On Fri, Mar 27, 2026 at 3:05 PM Richard Genoud
<richard.genoud@bootlin.com> wrote:
>
> In sunxi_nand_chip_init there's quite a lot of kfree/return, it's easy
> to forget a kfree(), so use a goto/kfree instead.
>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> ---
> drivers/mtd/nand/raw/sunxi_nand.c | 41 ++++++++++++++-----------------
> 1 file changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index 9fc9bc5e0198..ecab9ebc9576 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -1600,21 +1600,20 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
> if (ret) {
> dev_err(dev, "could not retrieve reg property: %d\n",
> ret);
> - kfree(chip);
> - return ret;
> + goto out;
> }
>
> if (tmp > NFC_MAX_CS) {
> dev_err(dev,
> "invalid reg value: %u (max CS = 7)\n", tmp);
> - kfree(chip);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out;
> }
>
> if (test_and_set_bit(tmp, &nfc->assigned_cs)) {
> dev_err(dev, "CS %d already assigned\n", tmp);
> - kfree(chip);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out;
> }
>
> chip->sels[i].cs = tmp;
> @@ -1640,15 +1639,13 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
> dev_err(dev,
> "could not retrieve timings for ONFI mode 0: %d\n",
> ret);
> - kfree(chip);
> - return ret;
> + goto out;
> }
>
> ret = sunxi_nand_chip_set_timings(nfc, chip, timings);
> if (ret) {
> dev_err(dev, "could not configure chip timings: %d\n", ret);
> - kfree(chip);
> - return ret;
> + goto out;
> }
>
> nand = &chip->nand;
> @@ -1669,10 +1666,8 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
>
> mtd = nand_to_mtd(nand);
> ret = nand_scan_ident(mtd, nsels, NULL);
> - if (ret) {
> - kfree(chip);
> - return ret;
> - }
> + if (ret)
> + goto out;
>
> if (nand->bbt_options & NAND_BBT_USE_FLASH)
> nand->bbt_options |= NAND_BBT_NO_OOB;
> @@ -1685,34 +1680,34 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
> ret = sunxi_nand_chip_init_timings(nfc, chip);
> if (ret) {
> dev_err(dev, "could not configure chip timings: %d\n", ret);
> - kfree(chip);
> - return ret;
> + goto out;
> }
>
> ret = sunxi_nand_ecc_init(mtd, &nand->ecc);
> if (ret) {
> dev_err(dev, "ECC init failed: %d\n", ret);
> - kfree(chip);
> - return ret;
> + goto out;
> }
>
> ret = nand_scan_tail(mtd);
> if (ret) {
> dev_err(dev, "nand_scan_tail failed: %d\n", ret);
> - kfree(chip);
> - return ret;
> + goto out;
> }
>
> ret = nand_register(devnum, mtd);
> if (ret) {
> dev_err(dev, "failed to register mtd device: %d\n", ret);
> - kfree(chip);
> - return ret;
> + goto out;
> }
>
> list_add_tail(&chip->node, &nfc->chips);
>
> - return 0;
> +out:
> + if (ret)
> + kfree(chip);
> +
> + return ret;
> }
>
> static int sunxi_nand_chips_init(struct udevice *dev, struct sunxi_nfc *nfc)
I need to go a bit in the code but seems that out can happen even if
there is no error. It's
not so common to have if (ret) if exit condition in case of errot
Michael
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/5] mtd: rawnand: sunxi: Replace hard coded value by a define
2026-03-27 14:05 ` [PATCH v2 1/5] mtd: rawnand: sunxi: Replace hard coded value by a define Richard Genoud
@ 2026-04-27 11:14 ` Andre Przywara
0 siblings, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2026-04-27 11:14 UTC (permalink / raw)
To: Richard Genoud, Dario Binacchi, Michael Trimarchi, Tom Rini
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot
Hi,
On 3/27/26 15:05, Richard Genoud wrote:
> The user data length (4) used all over the code hard coded.
> And sometimes, it's not that trivial to know that it's the user data
> length and not something else.
> Moreover, for the H6/H616 this value is no more fixed by hardware, but
> could be modified.
>
> Using a define here makes the code more readable.
>
> Suggested-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
Acked-by: Andre Przywara <andre.przywara@arm.com>
Cheers,
Andre
> ---
> drivers/mtd/nand/raw/sunxi_nand.c | 58 +++++++++++++--------------
> drivers/mtd/nand/raw/sunxi_nand.h | 3 ++
> drivers/mtd/nand/raw/sunxi_nand_spl.c | 7 +++-
> 3 files changed, 38 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index ef27a4b7a36a..91f7da2c5ce5 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -772,7 +772,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> return ret;
>
> sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, 4, 0);
> + sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
> @@ -783,7 +783,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - *cur_off = oob_off + ecc->bytes + 4;
> + *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
>
> pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
> pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
> @@ -794,7 +794,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> pattern = 0x0;
>
> memset(data, pattern, ecc->size);
> - memset(oob, pattern, ecc->bytes + 4);
> + memset(oob, pattern, ecc->bytes + USER_DATA_SZ);
>
> return 1;
> }
> @@ -804,7 +804,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
>
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
> - sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + 4, true, page);
> + sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ, true, page);
>
> status = readl(nfc->regs + NFC_REG_ECC_ST);
> if (status & NFC_ECC_ERR(0)) {
> @@ -816,17 +816,17 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
> nand->read_buf(mtd, data, ecc->size);
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
> - nand->read_buf(mtd, oob, ecc->bytes + 4);
> + nand->read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ);
> }
>
> ret = nand_check_erased_ecc_chunk(data, ecc->size,
> - oob, ecc->bytes + 4,
> + oob, ecc->bytes + USER_DATA_SZ,
> NULL, 0, ecc->strength);
> if (ret >= 0)
> raw_mode = 1;
> } else {
> /*
> - * The engine protects 4 bytes of OOB data per chunk.
> + * The engine protects USER_DATA_SZ bytes of OOB data per chunk.
> * Retrieve the corrected OOB bytes.
> */
> sunxi_nfc_user_data_to_buf(readl(nfc->regs +
> @@ -854,7 +854,7 @@ static void sunxi_nfc_hw_ecc_read_extra_oob(struct mtd_info *mtd,
> {
> struct nand_chip *nand = mtd_to_nand(mtd);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> - int offset = ((ecc->bytes + 4) * ecc->steps);
> + int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
> int len = mtd->oobsize - offset;
>
> if (len <= 0)
> @@ -896,9 +896,9 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
>
> /* Fill OOB data in */
> if ((nand->options & NAND_NEED_SCRAMBLING) && bbm) {
> - u8 user_data[4];
> + u8 user_data[USER_DATA_SZ];
>
> - memcpy(user_data, oob, 4);
> + memcpy(user_data, oob, USER_DATA_SZ);
> sunxi_nfc_randomize_bbm(mtd, page, user_data);
> writel(sunxi_nfc_buf_to_user_data(user_data),
> nfc->regs + NFC_REG_USER_DATA(nfc, 0));
> @@ -915,7 +915,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> return ret;
>
> sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, 4, 0);
> + sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
> @@ -927,7 +927,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - *cur_off = oob_off + ecc->bytes + 4;
> + *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
>
> return 0;
> }
> @@ -938,7 +938,7 @@ static void sunxi_nfc_hw_ecc_write_extra_oob(struct mtd_info *mtd,
> {
> struct nand_chip *nand = mtd_to_nand(mtd);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> - int offset = ((ecc->bytes + 4) * ecc->steps);
> + int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
> int len = mtd->oobsize - offset;
>
> if (len <= 0)
> @@ -966,7 +966,7 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
>
> for (i = 0; i < ecc->steps; i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + 4);
> + int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> u8 *data = buf + data_off;
> u8 *oob = chip->oob_poi + oob_off;
>
> @@ -1004,7 +1004,7 @@ static int sunxi_nfc_hw_ecc_read_subpage(struct mtd_info *mtd,
> for (i = data_offs / ecc->size;
> i < DIV_ROUND_UP(data_offs + readlen, ecc->size); i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + 4);
> + int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> u8 *data = bufpoi + data_off;
> u8 *oob = chip->oob_poi + oob_off;
>
> @@ -1032,7 +1032,7 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
>
> for (i = 0; i < ecc->steps; i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + 4);
> + int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> const u8 *data = buf + data_off;
> const u8 *oob = chip->oob_poi + oob_off;
>
> @@ -1066,7 +1066,7 @@ static int sunxi_nfc_hw_ecc_write_subpage(struct mtd_info *mtd,
> for (i = data_offs / ecc->size;
> i < DIV_ROUND_UP(data_offs + data_len, ecc->size); i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + 4);
> + int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> const u8 *data = buf + data_off;
> const u8 *oob = chip->oob_poi + oob_off;
>
> @@ -1095,10 +1095,10 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
> sunxi_nfc_hw_ecc_enable(mtd);
>
> for (i = 0; i < ecc->steps; i++) {
> - int data_off = i * (ecc->size + ecc->bytes + 4);
> + int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
> int oob_off = data_off + ecc->size;
> u8 *data = buf + (i * ecc->size);
> - u8 *oob = chip->oob_poi + (i * (ecc->bytes + 4));
> + u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
>
> ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
> oob_off, &cur_off,
> @@ -1129,10 +1129,10 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
> sunxi_nfc_hw_ecc_enable(mtd);
>
> for (i = 0; i < ecc->steps; i++) {
> - int data_off = i * (ecc->size + ecc->bytes + 4);
> + int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
> int oob_off = data_off + ecc->size;
> const u8 *data = buf + (i * ecc->size);
> - const u8 *oob = chip->oob_poi + (i * (ecc->bytes + 4));
> + const u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
>
> ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off,
> oob, oob_off, &cur_off,
> @@ -1390,7 +1390,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
> layout = &data->layout;
> nsectors = mtd->writesize / ecc->size;
>
> - if (mtd->oobsize < ((ecc->bytes + 4) * nsectors)) {
> + if (mtd->oobsize < ((ecc->bytes + USER_DATA_SZ) * nsectors)) {
> ret = -EINVAL;
> goto err;
> }
> @@ -1440,14 +1440,14 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
> layout->oobfree[i - 1].offset +
> layout->oobfree[i - 1].length +
> ecc->bytes;
> - layout->oobfree[i].length = 4;
> + layout->oobfree[i].length = USER_DATA_SZ;
> } else {
> /*
> * The first 2 bytes are used for BB markers, hence we
> - * only have 2 bytes available in the first user data
> - * section.
> + * only have USER_DATA_SZ - 2 bytes available in the
> + * first user data section.
> */
> - layout->oobfree[i].length = 2;
> + layout->oobfree[i].length = USER_DATA_SZ - 2;
> layout->oobfree[i].offset = 2;
> }
>
> @@ -1457,13 +1457,13 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
> layout->oobfree[i].length + j;
> }
>
> - if (mtd->oobsize > (ecc->bytes + 4) * nsectors) {
> + if (mtd->oobsize > (ecc->bytes + USER_DATA_SZ) * nsectors) {
> layout->oobfree[nsectors].offset =
> layout->oobfree[nsectors - 1].offset +
> layout->oobfree[nsectors - 1].length +
> ecc->bytes;
> layout->oobfree[nsectors].length = mtd->oobsize -
> - ((ecc->bytes + 4) * nsectors);
> + ((ecc->bytes + USER_DATA_SZ) * nsectors);
> }
>
> return 0;
> @@ -1481,7 +1481,7 @@ static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - ecc->prepad = 4;
> + ecc->prepad = USER_DATA_SZ;
> ecc->read_page = sunxi_nfc_hw_syndrome_ecc_read_page;
> ecc->write_page = sunxi_nfc_hw_syndrome_ecc_write_page;
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.h b/drivers/mtd/nand/raw/sunxi_nand.h
> index 6ee3ea14ee17..d7a8b3dd40c3 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.h
> +++ b/drivers/mtd/nand/raw/sunxi_nand.h
> @@ -181,6 +181,9 @@
>
> #define NFC_MAX_CS 7
>
> +/* On A10, the user data length register is 4 bytes */
> +#define USER_DATA_SZ 4
> +
> /*
> * NAND Controller capabilities structure: stores NAND controller capabilities
> * for distinction between compatible strings.
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index 67f7d22ed2c8..3a5271b49c36 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -269,6 +269,11 @@ static void sunxi_nfc_set_user_data_len(const struct nfc_config *nfc,
> writel_nfc(val, NFC_REG_USER_DATA_LEN(nfc, step));
> }
>
> +/*
> + * Values in this table are obtained by doing:
> + * DIV_ROUND_UP(info->ecc_strength * 14, 8) + USER_DATA_SZ
> + * So it's the number of bytes needed for ECC + user data for one step.
> + */
> #if defined(CONFIG_MACH_SUN50I_H616) || defined(CONFIG_MACH_SUN50I_H6)
> static const int ecc_bytes[] = {
> 32, 46, 54, 60, 74, 82, 88, 96, 102, 110, 116, 124, 130, 138, 144
> @@ -338,7 +343,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> nand_change_column(oob_off);
>
> sunxi_nfc_reset_user_data_len(conf);
> - sunxi_nfc_set_user_data_len(conf, 4, 0);
> + sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, 0);
>
> nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
> /* Get the ECC status */
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory
2026-03-27 14:05 ` [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
2026-03-27 14:40 ` Michael Nazzareno Trimarchi
@ 2026-04-27 12:23 ` Andre Przywara
1 sibling, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2026-04-27 12:23 UTC (permalink / raw)
To: Richard Genoud, Dario Binacchi, Michael Trimarchi, Tom Rini
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot
Hi,
On 3/27/26 15:05, Richard Genoud wrote:
> In sunxi_nfc_hw_ecc_{read,write}_chunk(), the ECC step was force to 0,
> the reason is not trivial to get when reading the code.
>
> The explanation is that, from the NAND flash controller perspective, we
> are indeed at step 0 for user data length and ECC errors.
>
> Just add a const value with an explanation to clarify things.
>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
Acked-by: Andre Przywara <andre.przywara@arm.com>
Cheers,
Andre
> ---
> drivers/mtd/nand/raw/sunxi_nand.c | 20 ++++++++++++--------
> drivers/mtd/nand/raw/sunxi_nand_spl.c | 6 ++++--
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index 91f7da2c5ce5..9fc9bc5e0198 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -758,6 +758,8 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> u32 status;
> u32 pattern_found;
> int ret;
> + /* From the controller point of view, we are at step 0 */
> + const int nfc_step = 0;
>
> if (*cur_off != data_off)
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
> @@ -772,7 +774,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> return ret;
>
> sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
> + sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
> @@ -787,7 +789,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
>
> pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
> pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
> - if (pattern_found & NFC_ECC_PAT_FOUND(0)) {
> + if (pattern_found & NFC_ECC_PAT_FOUND(nfc_step)) {
> u8 pattern = 0xff;
>
> if (unlikely(!(readl(nfc->regs + NFC_REG_PAT_ID(nfc)) & 0x1)))
> @@ -799,7 +801,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> return 1;
> }
>
> - ret = NFC_ECC_ERR_CNT(0, readl(nfc->regs + NFC_REG_ECC_ERR_CNT(nfc, 0)));
> + ret = NFC_ECC_ERR_CNT(nfc_step, readl(nfc->regs + NFC_REG_ECC_ERR_CNT(nfc, nfc_step)));
>
> memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
>
> @@ -807,7 +809,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ, true, page);
>
> status = readl(nfc->regs + NFC_REG_ECC_ST);
> - if (status & NFC_ECC_ERR(0)) {
> + if (status & NFC_ECC_ERR(nfc_step)) {
> /*
> * Re-read the data with the randomizer disabled to identify
> * bitflips in erased pages.
> @@ -830,7 +832,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> * Retrieve the corrected OOB bytes.
> */
> sunxi_nfc_user_data_to_buf(readl(nfc->regs +
> - NFC_REG_USER_DATA(nfc, 0)),
> + NFC_REG_USER_DATA(nfc, nfc_step)),
> oob);
>
> /* De-randomize the Bad Block Marker. */
> @@ -888,6 +890,8 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> int ret;
> + /* From the controller point of view, we are at step 0 */
> + const int nfc_step = 0;
>
> if (data_off != *cur_off)
> nand->cmdfunc(mtd, NAND_CMD_RNDIN, data_off, -1);
> @@ -901,10 +905,10 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> memcpy(user_data, oob, USER_DATA_SZ);
> sunxi_nfc_randomize_bbm(mtd, page, user_data);
> writel(sunxi_nfc_buf_to_user_data(user_data),
> - nfc->regs + NFC_REG_USER_DATA(nfc, 0));
> + nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
> } else {
> writel(sunxi_nfc_buf_to_user_data(oob),
> - nfc->regs + NFC_REG_USER_DATA(nfc, 0));
> + nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
> }
>
> if (data_off + ecc->size != oob_off)
> @@ -915,7 +919,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> return ret;
>
> sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
> + sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index 3a5271b49c36..0d1f060cc425 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -302,6 +302,8 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> int page = offs / conf->page_size;
> u32 ecc_st, pattern_found;
> int i;
> + /* From the controller point of view, we are at step 0 */
> + const int nfc_step = 0;
>
> if (offs % conf->page_size || len % conf->ecc_size ||
> len > conf->page_size || len < 0)
> @@ -343,7 +345,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> nand_change_column(oob_off);
>
> sunxi_nfc_reset_user_data_len(conf);
> - sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, 0);
> + sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, nfc_step);
>
> nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
> /* Get the ECC status */
> @@ -361,7 +363,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> pattern_found = readl_nfc(conf->caps->reg_pat_found);
> pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(conf),
> pattern_found);
> - if (pattern_found & NFC_ECC_PAT_FOUND(0))
> + if (pattern_found & NFC_ECC_PAT_FOUND(nfc_step))
> return 1;
> }
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6
2026-03-27 14:05 [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Richard Genoud
` (4 preceding siblings ...)
2026-03-27 14:05 ` [PATCH v2 5/5] mtd: nand: raw: sunxi_spl: remove user data length reset Richard Genoud
@ 2026-04-27 12:28 ` Andre Przywara
2026-04-27 12:31 ` Michael Nazzareno Trimarchi
5 siblings, 1 reply; 18+ messages in thread
From: Andre Przywara @ 2026-04-27 12:28 UTC (permalink / raw)
To: Richard Genoud, Dario Binacchi, Michael Trimarchi, Tom Rini
Cc: Andrew Goodbody, Miquel Raynal, James Hilliard, Boris Brezillon,
Thomas Petazzoni, u-boot
Dario, Michael,
are you going to take those patches eventually, or are they supposed to
go through the sunxi tree? Somehow patchwork put this on my plate, but
MAINTAINERS says it's yours.
I will have a final look at those patches, but I think they are fine
since their counterparts passed the kernel reviews ;-)
Cheers,
Andre
On 3/27/26 15:05, Richard Genoud wrote:
> This series introduces the correct usage of user data length for
> H6/H616.
> If the older SoCs must use a fixed value of 4 user data bytes for each
> ECC step, the H6 can have a different value for each step.
>
> In the first round, the behavior was kept as the one used by A10.
> But we have multiple choices:
> - do like A10 like before (and not use all OOB)
> - do like the vendor kernel (different from A10, but not using all OOB)
> - use the whole OOB for user data bytes
>
> In this second round, the third approach is implemented.
>
> All patches but the 4th are code cleaning.
> The 4th patch enables using a variable user data length for H6/H616
> along with maximizing it.
>
> A similar series has been accepted for Linux:
> https://lore.kernel.org/lkml/20260317142437.580204-1-richard.genoud@bootlin.com/
>
> Changes from v1:
> - gather reviewed-by
> - clean sunxi_nand_chip_init()
> - use all possible remaining OOB space for user data bytes
>
> Richard Genoud (5):
> mtd: rawnand: sunxi: Replace hard coded value by a define
> mtd: rawnand: sunxi: make the code mode self-explanatory
> mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
> mtd: rawnand: sunxi: introduce variable user data length
> mtd: nand: raw: sunxi_spl: remove user data length reset
>
> drivers/mtd/nand/raw/sunxi_nand.c | 292 +++++++++++++++++++-------
> drivers/mtd/nand/raw/sunxi_nand.h | 3 +
> drivers/mtd/nand/raw/sunxi_nand_spl.c | 129 +++++++++---
> 3 files changed, 324 insertions(+), 100 deletions(-)
>
>
> base-commit: ba7bf918dafcd093ad733b07ba490baeb20cf5da
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6
2026-04-27 12:28 ` [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Andre Przywara
@ 2026-04-27 12:31 ` Michael Nazzareno Trimarchi
2026-04-27 13:47 ` Andre Przywara
0 siblings, 1 reply; 18+ messages in thread
From: Michael Nazzareno Trimarchi @ 2026-04-27 12:31 UTC (permalink / raw)
To: Andre Przywara
Cc: Richard Genoud, Dario Binacchi, Tom Rini, Andrew Goodbody,
Miquel Raynal, James Hilliard, Boris Brezillon, Thomas Petazzoni,
u-boot
Hi Andre
On Mon, Apr 27, 2026 at 2:28 PM Andre Przywara <andre.przywara@arm.com>
wrote:
> Dario, Michael,
>
> are you going to take those patches eventually, or are they supposed to
> go through the sunxi tree? Somehow patchwork put this on my plate, but
> MAINTAINERS says it's yours.
>
> I will have a final look at those patches, but I think they are fine
> since their counterparts passed the kernel reviews ;-)
>
>
Company projects are stressing me out. I will check today with Dario
Michael
Cheers,
> Andre
>
> On 3/27/26 15:05, Richard Genoud wrote:
> > This series introduces the correct usage of user data length for
> > H6/H616.
> > If the older SoCs must use a fixed value of 4 user data bytes for each
> > ECC step, the H6 can have a different value for each step.
> >
> > In the first round, the behavior was kept as the one used by A10.
> > But we have multiple choices:
> > - do like A10 like before (and not use all OOB)
> > - do like the vendor kernel (different from A10, but not using all OOB)
> > - use the whole OOB for user data bytes
> >
> > In this second round, the third approach is implemented.
> >
> > All patches but the 4th are code cleaning.
> > The 4th patch enables using a variable user data length for H6/H616
> > along with maximizing it.
> >
> > A similar series has been accepted for Linux:
> >
> https://lore.kernel.org/lkml/20260317142437.580204-1-richard.genoud@bootlin.com/
> >
> > Changes from v1:
> > - gather reviewed-by
> > - clean sunxi_nand_chip_init()
> > - use all possible remaining OOB space for user data bytes
> >
> > Richard Genoud (5):
> > mtd: rawnand: sunxi: Replace hard coded value by a define
> > mtd: rawnand: sunxi: make the code mode self-explanatory
> > mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
> > mtd: rawnand: sunxi: introduce variable user data length
> > mtd: nand: raw: sunxi_spl: remove user data length reset
> >
> > drivers/mtd/nand/raw/sunxi_nand.c | 292 +++++++++++++++++++-------
> > drivers/mtd/nand/raw/sunxi_nand.h | 3 +
> > drivers/mtd/nand/raw/sunxi_nand_spl.c | 129 +++++++++---
> > 3 files changed, 324 insertions(+), 100 deletions(-)
> >
> >
> > base-commit: ba7bf918dafcd093ad733b07ba490baeb20cf5da
>
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
2026-03-27 14:42 ` Michael Nazzareno Trimarchi
@ 2026-04-27 12:35 ` Andre Przywara
2026-05-01 12:59 ` Andre Przywara
0 siblings, 1 reply; 18+ messages in thread
From: Andre Przywara @ 2026-04-27 12:35 UTC (permalink / raw)
To: Michael Nazzareno Trimarchi, Richard Genoud
Cc: Dario Binacchi, Tom Rini, Andrew Goodbody, Miquel Raynal,
James Hilliard, Boris Brezillon, Thomas Petazzoni, u-boot
Hi,
On 3/27/26 15:42, Michael Nazzareno Trimarchi wrote:
> Hi Richard
>
> On Fri, Mar 27, 2026 at 3:05 PM Richard Genoud
> <richard.genoud@bootlin.com> wrote:
>>
>> In sunxi_nand_chip_init there's quite a lot of kfree/return, it's easy
>> to forget a kfree(), so use a goto/kfree instead.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
>> ---
>> drivers/mtd/nand/raw/sunxi_nand.c | 41 ++++++++++++++-----------------
>> 1 file changed, 18 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
>> index 9fc9bc5e0198..ecab9ebc9576 100644
>> --- a/drivers/mtd/nand/raw/sunxi_nand.c
>> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
>> @@ -1600,21 +1600,20 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
>> if (ret) {
>> dev_err(dev, "could not retrieve reg property: %d\n",
>> ret);
>> - kfree(chip);
>> - return ret;
>> + goto out;
>> }
>>
>> if (tmp > NFC_MAX_CS) {
>> dev_err(dev,
>> "invalid reg value: %u (max CS = 7)\n", tmp);
>> - kfree(chip);
>> - return -EINVAL;
>> + ret = -EINVAL;
>> + goto out;
>> }
>>
>> if (test_and_set_bit(tmp, &nfc->assigned_cs)) {
>> dev_err(dev, "CS %d already assigned\n", tmp);
>> - kfree(chip);
>> - return -EINVAL;
>> + ret = -EINVAL;
>> + goto out;
>> }
>>
>> chip->sels[i].cs = tmp;
>> @@ -1640,15 +1639,13 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
>> dev_err(dev,
>> "could not retrieve timings for ONFI mode 0: %d\n",
>> ret);
>> - kfree(chip);
>> - return ret;
>> + goto out;
>> }
>>
>> ret = sunxi_nand_chip_set_timings(nfc, chip, timings);
>> if (ret) {
>> dev_err(dev, "could not configure chip timings: %d\n", ret);
>> - kfree(chip);
>> - return ret;
>> + goto out;
>> }
>>
>> nand = &chip->nand;
>> @@ -1669,10 +1666,8 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
>>
>> mtd = nand_to_mtd(nand);
>> ret = nand_scan_ident(mtd, nsels, NULL);
>> - if (ret) {
>> - kfree(chip);
>> - return ret;
>> - }
>> + if (ret)
>> + goto out;
>>
>> if (nand->bbt_options & NAND_BBT_USE_FLASH)
>> nand->bbt_options |= NAND_BBT_NO_OOB;
>> @@ -1685,34 +1680,34 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
>> ret = sunxi_nand_chip_init_timings(nfc, chip);
>> if (ret) {
>> dev_err(dev, "could not configure chip timings: %d\n", ret);
>> - kfree(chip);
>> - return ret;
>> + goto out;
>> }
>>
>> ret = sunxi_nand_ecc_init(mtd, &nand->ecc);
>> if (ret) {
>> dev_err(dev, "ECC init failed: %d\n", ret);
>> - kfree(chip);
>> - return ret;
>> + goto out;
>> }
>>
>> ret = nand_scan_tail(mtd);
>> if (ret) {
>> dev_err(dev, "nand_scan_tail failed: %d\n", ret);
>> - kfree(chip);
>> - return ret;
>> + goto out;
>> }
>>
>> ret = nand_register(devnum, mtd);
>> if (ret) {
>> dev_err(dev, "failed to register mtd device: %d\n", ret);
>> - kfree(chip);
>> - return ret;
>> + goto out;
>> }
>>
>> list_add_tail(&chip->node, &nfc->chips);
>>
>> - return 0;
>> +out:
>> + if (ret)
>> + kfree(chip);
>> +
>> + return ret;
>> }
>>
>> static int sunxi_nand_chips_init(struct udevice *dev, struct sunxi_nfc *nfc)
>
> I need to go a bit in the code but seems that out can happen even if
> there is no error. It's
> not so common to have if (ret) if exit condition in case of errot
So I think it's fine in this case, ret is always set before we "goto",
and in the success case "ret" must be 0 from the last check. But I agree
that the construct is a bit uncommon and fragile, so what about keeping
the "return 0;" (to use the common pattern and help readability), and
rename the label to "out_err" or "out_free", to make it more obvious
that this the error path only, and we expect ret to be set?
Cheers,
Andre
>
> Michael
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6
2026-04-27 12:31 ` Michael Nazzareno Trimarchi
@ 2026-04-27 13:47 ` Andre Przywara
2026-04-27 14:10 ` Michael Nazzareno Trimarchi
0 siblings, 1 reply; 18+ messages in thread
From: Andre Przywara @ 2026-04-27 13:47 UTC (permalink / raw)
To: Michael Nazzareno Trimarchi
Cc: Richard Genoud, Dario Binacchi, Tom Rini, Andrew Goodbody,
Miquel Raynal, James Hilliard, Boris Brezillon, Thomas Petazzoni,
u-boot
Hi Michael,
On 4/27/26 14:31, Michael Nazzareno Trimarchi wrote:
> Hi Andre
>
> On Mon, Apr 27, 2026 at 2:28 PM Andre Przywara <andre.przywara@arm.com
> <mailto:andre.przywara@arm.com>> wrote:
>
> Dario, Michael,
>
> are you going to take those patches eventually, or are they supposed to
> go through the sunxi tree? Somehow patchwork put this on my plate, but
> MAINTAINERS says it's yours.
>
> I will have a final look at those patches, but I think they are fine
> since their counterparts passed the kernel reviews ;-)
>
>
> Company projects are stressing me out. I will check today with Dario
Sorry to hear that! I can easily take care of this series, if you are
fine with that. It would touch only those sunxi specific files anyway.
Cheers,
Andre
> Michael
>
> Cheers,
> Andre
>
> On 3/27/26 15:05, Richard Genoud wrote:
> > This series introduces the correct usage of user data length for
> > H6/H616.
> > If the older SoCs must use a fixed value of 4 user data bytes for
> each
> > ECC step, the H6 can have a different value for each step.
> >
> > In the first round, the behavior was kept as the one used by A10.
> > But we have multiple choices:
> > - do like A10 like before (and not use all OOB)
> > - do like the vendor kernel (different from A10, but not using
> all OOB)
> > - use the whole OOB for user data bytes
> >
> > In this second round, the third approach is implemented.
> >
> > All patches but the 4th are code cleaning.
> > The 4th patch enables using a variable user data length for H6/H616
> > along with maximizing it.
> >
> > A similar series has been accepted for Linux:
> > https://lore.kernel.org/lkml/20260317142437.580204-1-
> richard.genoud@bootlin.com/ <https://lore.kernel.org/
> lkml/20260317142437.580204-1-richard.genoud@bootlin.com/>
> >
> > Changes from v1:
> > - gather reviewed-by
> > - clean sunxi_nand_chip_init()
> > - use all possible remaining OOB space for user data bytes
> >
> > Richard Genoud (5):
> > mtd: rawnand: sunxi: Replace hard coded value by a define
> > mtd: rawnand: sunxi: make the code mode self-explanatory
> > mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
> > mtd: rawnand: sunxi: introduce variable user data length
> > mtd: nand: raw: sunxi_spl: remove user data length reset
> >
> > drivers/mtd/nand/raw/sunxi_nand.c | 292 ++++++++++++++++++
> +-------
> > drivers/mtd/nand/raw/sunxi_nand.h | 3 +
> > drivers/mtd/nand/raw/sunxi_nand_spl.c | 129 +++++++++---
> > 3 files changed, 324 insertions(+), 100 deletions(-)
> >
> >
> > base-commit: ba7bf918dafcd093ad733b07ba490baeb20cf5da
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com <mailto:michael@amarulasolutions.com>
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com <mailto:info@amarulasolutions.com>
> www.amarulasolutions.com <http://www.amarulasolutions.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6
2026-04-27 13:47 ` Andre Przywara
@ 2026-04-27 14:10 ` Michael Nazzareno Trimarchi
0 siblings, 0 replies; 18+ messages in thread
From: Michael Nazzareno Trimarchi @ 2026-04-27 14:10 UTC (permalink / raw)
To: Andre Przywara
Cc: Richard Genoud, Dario Binacchi, Tom Rini, Andrew Goodbody,
Miquel Raynal, James Hilliard, Boris Brezillon, Thomas Petazzoni,
u-boot
Hi
On Mon, Apr 27, 2026 at 3:47 PM Andre Przywara <andre.przywara@arm.com>
wrote:
> Hi Michael,
>
> On 4/27/26 14:31, Michael Nazzareno Trimarchi wrote:
> > Hi Andre
> >
> > On Mon, Apr 27, 2026 at 2:28 PM Andre Przywara <andre.przywara@arm.com
> > <mailto:andre.przywara@arm.com>> wrote:
> >
> > Dario, Michael,
> >
> > are you going to take those patches eventually, or are they supposed
> to
> > go through the sunxi tree? Somehow patchwork put this on my plate,
> but
> > MAINTAINERS says it's yours.
> >
> > I will have a final look at those patches, but I think they are fine
> > since their counterparts passed the kernel reviews ;-)
> >
> >
> > Company projects are stressing me out. I will check today with Dario
>
> Sorry to hear that! I can easily take care of this series, if you are
> fine with that. It would touch only those sunxi specific files anyway.
>
>
Yes it's ok, good that you can pick them
Michael
> Cheers,
> Andre
>
> > Michael
> >
> > Cheers,
> > Andre
> >
> > On 3/27/26 15:05, Richard Genoud wrote:
> > > This series introduces the correct usage of user data length for
> > > H6/H616.
> > > If the older SoCs must use a fixed value of 4 user data bytes for
> > each
> > > ECC step, the H6 can have a different value for each step.
> > >
> > > In the first round, the behavior was kept as the one used by A10.
> > > But we have multiple choices:
> > > - do like A10 like before (and not use all OOB)
> > > - do like the vendor kernel (different from A10, but not using
> > all OOB)
> > > - use the whole OOB for user data bytes
> > >
> > > In this second round, the third approach is implemented.
> > >
> > > All patches but the 4th are code cleaning.
> > > The 4th patch enables using a variable user data length for
> H6/H616
> > > along with maximizing it.
> > >
> > > A similar series has been accepted for Linux:
> > > https://lore.kernel.org/lkml/20260317142437.580204-1-
> > richard.genoud@bootlin.com/ <https://lore.kernel.org/
> > lkml/20260317142437.580204-1-richard.genoud@bootlin.com/>
> > >
> > > Changes from v1:
> > > - gather reviewed-by
> > > - clean sunxi_nand_chip_init()
> > > - use all possible remaining OOB space for user data bytes
> > >
> > > Richard Genoud (5):
> > > mtd: rawnand: sunxi: Replace hard coded value by a define
> > > mtd: rawnand: sunxi: make the code mode self-explanatory
> > > mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
> > > mtd: rawnand: sunxi: introduce variable user data length
> > > mtd: nand: raw: sunxi_spl: remove user data length reset
> > >
> > > drivers/mtd/nand/raw/sunxi_nand.c | 292 ++++++++++++++++++
> > +-------
> > > drivers/mtd/nand/raw/sunxi_nand.h | 3 +
> > > drivers/mtd/nand/raw/sunxi_nand_spl.c | 129 +++++++++---
> > > 3 files changed, 324 insertions(+), 100 deletions(-)
> > >
> > >
> > > base-commit: ba7bf918dafcd093ad733b07ba490baeb20cf5da
> >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > michael@amarulasolutions.com <mailto:michael@amarulasolutions.com>
> > __________________________________
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > info@amarulasolutions.com <mailto:info@amarulasolutions.com>
> > www.amarulasolutions.com <http://www.amarulasolutions.com>
>
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] mtd: rawnand: sunxi: introduce variable user data length
2026-03-27 14:05 ` [PATCH v2 4/5] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
@ 2026-05-01 12:46 ` Andre Przywara
0 siblings, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2026-05-01 12:46 UTC (permalink / raw)
To: Richard Genoud
Cc: Dario Binacchi, Michael Trimarchi, Tom Rini, Andrew Goodbody,
Miquel Raynal, James Hilliard, Boris Brezillon, Thomas Petazzoni,
u-boot
On Fri, 27 Mar 2026 15:05:07 +0100
Richard Genoud <richard.genoud@bootlin.com> wrote:
Hi,
> In Allwinner SoCs, user data can be added in OOB before each ECC data.
> For older SoCs like A10, the user data size was the size of a register
> (4 bytes) and was mandatory before each ECC step.
> So, the A10 OOB Layout is:
> [4Bytes USER_DATA_STEP0] [ECC_STEP0 bytes]
> [4bytes USER_DATA_STEP1] [ECC_STEP1 bytes]
> ...
> NB: the BBM is stored at the beginning of the USER_DATA_STEP0.
>
> Now, for H6/H616 NAND flash controller, this user data can have a
> different size for each step.
> So, we are maximizing the user data length to use as many OOB bytes as
> possible.
>
> Fixes: 7d1de9801151 ("mtd: rawnand: sunxi_spl: add support for H6/H616 nand controller")
> Fixes: f163da5e6d26 ("mtd: rawnand: sunxi: add support for H6/H616 nand controller")
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
So I just skimmed over this, as I have no clue what's going on exactly,
but the code makes some sense, and more importantly it roughly matches
the respective Linux patch that went into v7.1-rc1, so I assume that's
good. From a mainline perspective it's mostly dead code anyway, as
there is no mainline board using raw NAND outside of the two C.H.I.P.
boards from about a decade ago (hint hint: please upstream support for
your board!)
So I am just taking this as-is, as I don't expect to see proper reviews
anytime soon anyway. If people find bugs, we can always fix them.
Cheers,
Andre
> ---
> drivers/mtd/nand/raw/sunxi_nand.c | 235 +++++++++++++++++++++-----
> drivers/mtd/nand/raw/sunxi_nand_spl.c | 107 ++++++++++--
> 2 files changed, 287 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index ecab9ebc9576..c3bec1fe517e 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -114,6 +114,7 @@ struct sunxi_nand_hw_ecc {
> * @clk_rate: clk_rate required for this NAND chip
> * @timing_cfg TIMING_CFG register value for this NAND chip
> * @selected: current active CS
> + * @user_data_bytes array of user data lengths for all ECC steps
> * @nsels: number of CS lines required by the NAND chip
> * @sels: array of CS lines descriptions
> */
> @@ -128,6 +129,7 @@ struct sunxi_nand_chip {
> u32 addr[2];
> int cmd_cycles;
> u8 cmd[2];
> + u8 *user_data_bytes;
> int nsels;
> struct sunxi_nand_chip_sel sels[0];
> };
> @@ -744,19 +746,73 @@ static void sunxi_nfc_set_user_data_len(struct sunxi_nfc *nfc,
> writel(val, nfc->regs + NFC_REG_USER_DATA_LEN(nfc, step));
> }
>
> +static u8 sunxi_nfc_user_data_sz(const struct sunxi_nand_chip *sunxi_nand, int step)
> +{
> + if (!sunxi_nand->user_data_bytes)
> + return USER_DATA_SZ;
> +
> + return sunxi_nand->user_data_bytes[step];
> +}
> +
> +static void sunxi_nfc_hw_ecc_get_prot_oob_bytes(struct nand_chip *nand, u8 *oob,
> + int step, bool bbm, int page,
> + unsigned int user_data_sz)
> +{
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> + struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
> + u32 user_data;
> +
> + if (!nfc->caps->reg_user_data_len) {
> + /*
> + * For A10, the user data for step n is in the nth
> + * REG_USER_DATA
> + */
> + user_data = readl(nfc->regs + NFC_REG_USER_DATA(nfc, step));
> + sunxi_nfc_user_data_to_buf(user_data, oob);
> +
> + } else {
> + /*
> + * For H6 NAND controller, the user data for all steps is
> + * contained in 32 user data registers, but not at a specific
> + * offset for each step, they are just concatenated.
> + */
> + unsigned int user_data_off = 0;
> + unsigned int reg_off;
> + u8 *ptr = oob;
> + unsigned int i;
> +
> + for (i = 0; i < step; i++)
> + user_data_off += sunxi_nfc_user_data_sz(sunxi_nand, i);
> +
> + user_data_off /= 4;
> + for (i = 0; i < user_data_sz / 4; i++, ptr += 4) {
> + reg_off = NFC_REG_USER_DATA(nfc, user_data_off + i);
> + user_data = readl(nfc->regs + reg_off);
> + sunxi_nfc_user_data_to_buf(user_data, ptr);
> + }
> + }
> +
> + /* De-randomize the Bad Block Marker. */
> + if (bbm && nand->options & NAND_NEED_SCRAMBLING)
> + sunxi_nfc_randomize_bbm(&nand->mtd, page, oob);
> +}
> +
> static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> u8 *data, int data_off,
> u8 *oob, int oob_off,
> int *cur_off,
> unsigned int *max_bitflips,
> - bool bbm, int page)
> + int step, int page)
> {
> struct nand_chip *nand = mtd_to_nand(mtd);
> struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> + unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, step);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> int raw_mode = 0;
> u32 status;
> u32 pattern_found;
> + bool bbm = !step;
> int ret;
> /* From the controller point of view, we are at step 0 */
> const int nfc_step = 0;
> @@ -773,8 +829,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
> + sunxi_nfc_set_user_data_len(nfc, user_data_sz, nfc_step);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
> @@ -785,7 +840,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
> + *cur_off = oob_off + ecc->bytes + user_data_sz;
>
> pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
> pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
> @@ -796,7 +851,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> pattern = 0x0;
>
> memset(data, pattern, ecc->size);
> - memset(oob, pattern, ecc->bytes + USER_DATA_SZ);
> + memset(oob, pattern, ecc->bytes + user_data_sz);
>
> return 1;
> }
> @@ -806,7 +861,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
>
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
> - sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ, true, page);
> + sunxi_nfc_randomizer_read_buf(mtd, oob, ecc->bytes + user_data_sz, true, page);
>
> status = readl(nfc->regs + NFC_REG_ECC_ST);
> if (status & NFC_ECC_ERR(nfc_step)) {
> @@ -818,26 +873,21 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
> nand->read_buf(mtd, data, ecc->size);
> nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
> - nand->read_buf(mtd, oob, ecc->bytes + USER_DATA_SZ);
> + nand->read_buf(mtd, oob, ecc->bytes + user_data_sz);
> }
>
> ret = nand_check_erased_ecc_chunk(data, ecc->size,
> - oob, ecc->bytes + USER_DATA_SZ,
> + oob, ecc->bytes + user_data_sz,
> NULL, 0, ecc->strength);
> if (ret >= 0)
> raw_mode = 1;
> } else {
> /*
> - * The engine protects USER_DATA_SZ bytes of OOB data per chunk.
> + * The engine protects user_data_sz bytes of OOB data per chunk.
> * Retrieve the corrected OOB bytes.
> */
> - sunxi_nfc_user_data_to_buf(readl(nfc->regs +
> - NFC_REG_USER_DATA(nfc, nfc_step)),
> - oob);
> -
> - /* De-randomize the Bad Block Marker. */
> - if (bbm && nand->options & NAND_NEED_SCRAMBLING)
> - sunxi_nfc_randomize_bbm(mtd, page, oob);
> + sunxi_nfc_hw_ecc_get_prot_oob_bytes(nand, oob, nfc_step,
> + bbm, page, user_data_sz);
> }
>
> if (ret < 0) {
> @@ -850,13 +900,30 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
> return raw_mode;
> }
>
> +/*
> + * Returns the offset of the OOB for each step.
> + * (it includes the user data before the ECC data.)
> + */
> +static int sunxi_get_oob_offset(struct sunxi_nand_chip *sunxi_nand,
> + struct nand_ecc_ctrl *ecc, int step)
> +{
> + int ecc_off = step * ecc->bytes;
> + int i;
> +
> + for (i = 0; i < step; i++)
> + ecc_off += sunxi_nfc_user_data_sz(sunxi_nand, i);
> +
> + return ecc_off;
> +}
> +
> static void sunxi_nfc_hw_ecc_read_extra_oob(struct mtd_info *mtd,
> u8 *oob, int *cur_off,
> bool randomize, int page)
> {
> struct nand_chip *nand = mtd_to_nand(mtd);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> - int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> + int offset = sunxi_get_oob_offset(sunxi_nand, ecc, ecc->steps);
> int len = mtd->oobsize - offset;
>
> if (len <= 0)
> @@ -883,12 +950,15 @@ static inline u32 sunxi_nfc_buf_to_user_data(const u8 *buf)
> static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> const u8 *data, int data_off,
> const u8 *oob, int oob_off,
> - int *cur_off, bool bbm,
> + int *cur_off, int step,
> int page)
> {
> struct nand_chip *nand = mtd_to_nand(mtd);
> struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> + unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, step);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> + bool bbm = !step;
> int ret;
> /* From the controller point of view, we are at step 0 */
> const int nfc_step = 0;
> @@ -900,12 +970,17 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
>
> /* Fill OOB data in */
> if ((nand->options & NAND_NEED_SCRAMBLING) && bbm) {
> - u8 user_data[USER_DATA_SZ];
> + u8 *user_data;
>
> - memcpy(user_data, oob, USER_DATA_SZ);
> + user_data = kzalloc(user_data_sz, GFP_KERNEL);
> + if (!user_data)
> + return -ENOMEM;
> +
> + memcpy(user_data, oob, user_data_sz);
> sunxi_nfc_randomize_bbm(mtd, page, user_data);
> writel(sunxi_nfc_buf_to_user_data(user_data),
> nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
> + kfree(user_data);
> } else {
> writel(sunxi_nfc_buf_to_user_data(oob),
> nfc->regs + NFC_REG_USER_DATA(nfc, nfc_step));
> @@ -918,8 +993,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - sunxi_nfc_reset_user_data_len(nfc);
> - sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
> + sunxi_nfc_set_user_data_len(nfc, user_data_sz, nfc_step);
>
> sunxi_nfc_randomizer_enable(mtd);
> writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
> @@ -931,7 +1005,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - *cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
> + *cur_off = oob_off + ecc->bytes + user_data_sz;
>
> return 0;
> }
> @@ -942,7 +1016,8 @@ static void sunxi_nfc_hw_ecc_write_extra_oob(struct mtd_info *mtd,
> {
> struct nand_chip *nand = mtd_to_nand(mtd);
> struct nand_ecc_ctrl *ecc = &nand->ecc;
> - int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> + int offset = sunxi_get_oob_offset(sunxi_nand, ecc, ecc->steps);
> int len = mtd->oobsize - offset;
>
> if (len <= 0)
> @@ -961,6 +1036,8 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
> struct nand_chip *chip, uint8_t *buf,
> int oob_required, int page)
> {
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
> + struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> unsigned int max_bitflips = 0;
> int ret, i, cur_off = 0;
> @@ -968,16 +1045,17 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
>
> sunxi_nfc_hw_ecc_enable(mtd);
>
> + sunxi_nfc_reset_user_data_len(nfc);
> for (i = 0; i < ecc->steps; i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> + int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
> u8 *data = buf + data_off;
> u8 *oob = chip->oob_poi + oob_off;
>
> ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
> oob_off + mtd->writesize,
> &cur_off, &max_bitflips,
> - !i, page);
> + i, page);
> if (ret < 0)
> return ret;
> else if (ret)
> @@ -998,23 +1076,26 @@ static int sunxi_nfc_hw_ecc_read_subpage(struct mtd_info *mtd,
> uint32_t data_offs, uint32_t readlen,
> uint8_t *bufpoi, int page)
> {
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
> + struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> int ret, i, cur_off = 0;
> unsigned int max_bitflips = 0;
>
> sunxi_nfc_hw_ecc_enable(mtd);
>
> + sunxi_nfc_reset_user_data_len(nfc);
> chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> for (i = data_offs / ecc->size;
> i < DIV_ROUND_UP(data_offs + readlen, ecc->size); i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> + int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
> u8 *data = bufpoi + data_off;
> u8 *oob = chip->oob_poi + oob_off;
>
> ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off,
> oob, oob_off + mtd->writesize,
> - &cur_off, &max_bitflips, !i, page);
> + &cur_off, &max_bitflips, i, page);
> if (ret < 0)
> return ret;
> }
> @@ -1029,20 +1110,23 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
> const uint8_t *buf, int oob_required,
> int page)
> {
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
> + struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> int ret, i, cur_off = 0;
>
> sunxi_nfc_hw_ecc_enable(mtd);
>
> + sunxi_nfc_reset_user_data_len(nfc);
> for (i = 0; i < ecc->steps; i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> + int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
> const u8 *data = buf + data_off;
> const u8 *oob = chip->oob_poi + oob_off;
>
> ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off, oob,
> oob_off + mtd->writesize,
> - &cur_off, !i, page);
> + &cur_off, i, page);
> if (ret)
> return ret;
> }
> @@ -1062,21 +1146,24 @@ static int sunxi_nfc_hw_ecc_write_subpage(struct mtd_info *mtd,
> const u8 *buf, int oob_required,
> int page)
> {
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
> + struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> int ret, i, cur_off = 0;
>
> sunxi_nfc_hw_ecc_enable(mtd);
>
> + sunxi_nfc_reset_user_data_len(nfc);
> for (i = data_offs / ecc->size;
> i < DIV_ROUND_UP(data_offs + data_len, ecc->size); i++) {
> int data_off = i * ecc->size;
> - int oob_off = i * (ecc->bytes + USER_DATA_SZ);
> + int oob_off = sunxi_get_oob_offset(sunxi_nand, ecc, i);
> const u8 *data = buf + data_off;
> const u8 *oob = chip->oob_poi + oob_off;
>
> ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off, oob,
> oob_off + mtd->writesize,
> - &cur_off, !i, page);
> + &cur_off, i, page);
> if (ret)
> return ret;
> }
> @@ -1091,18 +1178,23 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
> uint8_t *buf, int oob_required,
> int page)
> {
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
> + struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> unsigned int max_bitflips = 0;
> int ret, i, cur_off = 0;
> bool raw_mode = false;
> + /* With hw_syndrome, user data length is fixed */
> + unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, 0);
>
> sunxi_nfc_hw_ecc_enable(mtd);
>
> + sunxi_nfc_reset_user_data_len(nfc);
> for (i = 0; i < ecc->steps; i++) {
> - int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
> + int data_off = i * (ecc->size + ecc->bytes + user_data_sz);
> int oob_off = data_off + ecc->size;
> u8 *data = buf + (i * ecc->size);
> - u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
> + u8 *oob = chip->oob_poi + (i * (ecc->bytes + user_data_sz));
>
> ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
> oob_off, &cur_off,
> @@ -1127,16 +1219,19 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
> const uint8_t *buf,
> int oob_required, int page)
> {
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(chip);
> struct nand_ecc_ctrl *ecc = &chip->ecc;
> int ret, i, cur_off = 0;
> + /* With hw_syndrome, user data length is fixed */
> + unsigned int user_data_sz = sunxi_nfc_user_data_sz(sunxi_nand, 0);
>
> sunxi_nfc_hw_ecc_enable(mtd);
>
> for (i = 0; i < ecc->steps; i++) {
> - int data_off = i * (ecc->size + ecc->bytes + USER_DATA_SZ);
> + int data_off = i * (ecc->size + ecc->bytes + user_data_sz);
> int oob_off = data_off + ecc->size;
> const u8 *data = buf + (i * ecc->size);
> - const u8 *oob = chip->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
> + const u8 *oob = chip->oob_poi + (i * (ecc->bytes + user_data_sz));
>
> ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off,
> oob, oob_off, &cur_off,
> @@ -1338,6 +1433,34 @@ static int sunxi_nand_chip_init_timings(struct sunxi_nfc *nfc,
> return sunxi_nand_chip_set_timings(nfc, chip, timings);
> }
>
> +static int sunxi_nfc_maximize_user_data(struct nand_chip *nand, uint32_t oobsize,
> + int ecc_bytes, int nsectors)
> +{
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> + struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
> + const struct sunxi_nfc_caps *c = nfc->caps;
> + int remaining_bytes = oobsize - (ecc_bytes * nsectors);
> + int i, step;
> +
> + sunxi_nand->user_data_bytes = devm_kzalloc(nfc->dev, nsectors,
> + GFP_KERNEL);
> + if (!sunxi_nand->user_data_bytes)
> + return -ENOMEM;
> +
> + for (step = 0; (step < nsectors) && (remaining_bytes > 0); step++) {
> + for (i = 0; i < c->nuser_data_tab; i++) {
> + if (c->user_data_len_tab[i] > remaining_bytes)
> + break;
> + sunxi_nand->user_data_bytes[step] = c->user_data_len_tab[i];
> + }
> + remaining_bytes -= sunxi_nand->user_data_bytes[step];
> + if (sunxi_nand->user_data_bytes[step] == 0)
> + break;
> + }
> +
> + return 0;
> +}
> +
> static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
> struct nand_ecc_ctrl *ecc)
> {
> @@ -1346,6 +1469,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
> struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
> struct sunxi_nand_hw_ecc *data;
> struct nand_ecclayout *layout;
> + unsigned int total_user_data_sz = 0;
> int nsectors;
> int ret;
> int i;
> @@ -1394,7 +1518,15 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
> layout = &data->layout;
> nsectors = mtd->writesize / ecc->size;
>
> - if (mtd->oobsize < ((ecc->bytes + USER_DATA_SZ) * nsectors)) {
> + /* Use the remaining OOB space for user data */
> + if (nfc->caps->reg_user_data_len)
> + sunxi_nfc_maximize_user_data(nand, mtd->oobsize, ecc->bytes,
> + nsectors);
> +
> + for (i = 0; i < nsectors; i++)
> + total_user_data_sz += sunxi_nfc_user_data_sz(sunxi_nand, i);
> +
> + if (mtd->oobsize < ecc->bytes * nsectors + total_user_data_sz) {
> ret = -EINVAL;
> goto err;
> }
> @@ -1408,6 +1540,8 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
>
> err:
> kfree(data);
> + devm_kfree(nfc->dev, sunxi_nand->user_data_bytes);
> + sunxi_nand->user_data_bytes = NULL;
>
> return ret;
> }
> @@ -1422,7 +1556,10 @@ static void sunxi_nand_hw_common_ecc_ctrl_cleanup(struct nand_ecc_ctrl *ecc)
> static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
> struct nand_ecc_ctrl *ecc)
> {
> + struct nand_chip *nand = mtd_to_nand(mtd);
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> struct nand_ecclayout *layout;
> + unsigned int total_user_data_sz = 0;
> int nsectors;
> int i, j;
> int ret;
> @@ -1444,14 +1581,14 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
> layout->oobfree[i - 1].offset +
> layout->oobfree[i - 1].length +
> ecc->bytes;
> - layout->oobfree[i].length = USER_DATA_SZ;
> + layout->oobfree[i].length = sunxi_nfc_user_data_sz(sunxi_nand, i);
> } else {
> /*
> * The first 2 bytes are used for BB markers, hence we
> - * only have USER_DATA_SZ - 2 bytes available in the
> + * only have user_data_len(0) - 2 bytes available in the
> * first user data section.
> */
> - layout->oobfree[i].length = USER_DATA_SZ - 2;
> + layout->oobfree[i].length = sunxi_nfc_user_data_sz(sunxi_nand, i) - 2;
> layout->oobfree[i].offset = 2;
> }
>
> @@ -1461,13 +1598,16 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
> layout->oobfree[i].length + j;
> }
>
> - if (mtd->oobsize > (ecc->bytes + USER_DATA_SZ) * nsectors) {
> + for (i = 0; i < nsectors; i++)
> + total_user_data_sz += sunxi_nfc_user_data_sz(sunxi_nand, i);
> +
> + if (mtd->oobsize > ecc->bytes * nsectors + total_user_data_sz) {
> layout->oobfree[nsectors].offset =
> layout->oobfree[nsectors - 1].offset +
> layout->oobfree[nsectors - 1].length +
> ecc->bytes;
> layout->oobfree[nsectors].length = mtd->oobsize -
> - ((ecc->bytes + USER_DATA_SZ) * nsectors);
> + (ecc->bytes * nsectors + total_user_data_sz);
> }
>
> return 0;
> @@ -1476,6 +1616,8 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
> static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct mtd_info *mtd,
> struct nand_ecc_ctrl *ecc)
> {
> + struct nand_chip *nand = mtd_to_nand(mtd);
> + struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
> struct nand_ecclayout *layout;
> int nsectors;
> int i;
> @@ -1485,7 +1627,13 @@ static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct mtd_info *mtd,
> if (ret)
> return ret;
>
> - ecc->prepad = USER_DATA_SZ;
> + for (i = 0; i < nsectors; i++)
> + if (sunxi_nfc_user_data_sz(sunxi_nand, i) !=
> + sunxi_nfc_user_data_sz(sunxi_nand, 0)) {
> + dev_err(mtd->dev, "Variable user data length not upported with NAND_ECC_HW_SYNDROME\n");
> + return -EOPNOTSUPP;
> + }
> + ecc->prepad = sunxi_nfc_user_data_sz(sunxi_nand, 0);
> ecc->read_page = sunxi_nfc_hw_syndrome_ecc_read_page;
> ecc->write_page = sunxi_nfc_hw_syndrome_ecc_write_page;
>
> @@ -1735,6 +1883,7 @@ static void sunxi_nand_chips_cleanup(struct sunxi_nfc *nfc)
> nand_release(&chip->mtd);
> sunxi_nand_ecc_cleanup(&chip->nand.ecc);
> list_del(&chip->node);
> + devm_kfree(nfc->dev, chip->user_data_bytes);
> kfree(chip);
> }
> }
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index 0d1f060cc425..784ffb00cf70 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -28,6 +28,7 @@ struct nfc_config {
> bool randomize;
> bool valid;
> const struct sunxi_nfc_caps *caps;
> + u8 *user_data_bytes;
> };
>
> /* minimal "boot0" style NAND support for Allwinner A20 */
> @@ -271,16 +272,17 @@ static void sunxi_nfc_set_user_data_len(const struct nfc_config *nfc,
>
> /*
> * Values in this table are obtained by doing:
> - * DIV_ROUND_UP(info->ecc_strength * 14, 8) + USER_DATA_SZ
> - * So it's the number of bytes needed for ECC + user data for one step.
> + * DIV_ROUND_UP(info->ecc_strength * 14, 8)
> + * So it's the number of bytes needed for ECC one step
> + * (not counting the user data length)
> */
> #if defined(CONFIG_MACH_SUN50I_H616) || defined(CONFIG_MACH_SUN50I_H6)
> static const int ecc_bytes[] = {
> - 32, 46, 54, 60, 74, 82, 88, 96, 102, 110, 116, 124, 130, 138, 144
> + 28, 42, 50, 56, 70, 78, 84, 92, 98, 106, 112, 120, 126, 134, 140
> };
> #else
> static const int ecc_bytes[] = {
> - 32, 46, 54, 60, 74, 88, 102, 110, 116
> + 28, 42, 50, 56, 70, 84, 98, 106, 112
> };
> #endif
>
> @@ -293,6 +295,14 @@ static void nand_readlcpy(u32 *dest, u32 * __iomem src, size_t len)
> *dest++ = readl(src++);
> }
>
> +static u8 nand_user_data_sz(const struct nfc_config *conf, int step)
> +{
> + if (!conf->user_data_bytes)
> + return USER_DATA_SZ;
> +
> + return conf->user_data_bytes[step];
> +}
> +
> static int nand_read_page(const struct nfc_config *conf, u32 offs,
> void *dest, int len)
> {
> @@ -300,6 +310,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> u16 rand_seed = 0;
> int oob_chunk_sz = ecc_bytes[conf->ecc_strength];
> int page = offs / conf->page_size;
> + int oob_off = conf->page_size;
> u32 ecc_st, pattern_found;
> int i;
> /* From the controller point of view, we are at step 0 */
> @@ -316,9 +327,9 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> /* Retrieve data from SRAM (PIO) */
> for (i = 0; i < nsectors; i++) {
> int data_off = i * conf->ecc_size;
> - int oob_off = conf->page_size + (i * oob_chunk_sz);
> u8 *data = dest + data_off;
> u32 ecc512_bit = 0;
> + unsigned int user_data_sz = nand_user_data_sz(conf, i);
>
> if (conf->caps->has_ecc_block_512 && conf->ecc_size == 512)
> ecc512_bit = NFC_ECC_BLOCK_512;
> @@ -345,7 +356,7 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> nand_change_column(oob_off);
>
> sunxi_nfc_reset_user_data_len(conf);
> - sunxi_nfc_set_user_data_len(conf, USER_DATA_SZ, nfc_step);
> + sunxi_nfc_set_user_data_len(conf, user_data_sz, nfc_step);
>
> nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
> /* Get the ECC status */
> @@ -371,13 +382,61 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> nand_readlcpy((u32 *)data,
> (void *)(uintptr_t)SUNXI_NFC_BASE + NFC_RAM0_BASE,
> conf->ecc_size);
> -
> /* Stop the ECC engine */
> writel_nfc(readl_nfc(NFC_REG_ECC_CTL) & ~NFC_ECC_EN,
> NFC_REG_ECC_CTL);
>
> if (data_off + conf->ecc_size >= len)
> break;
> +
> + oob_off += oob_chunk_sz + user_data_sz;
> + }
> +
> + return 0;
> +}
> +
> +static int nand_min_user_data_sz(struct nfc_config *conf, int nsectors)
> +{
> + const struct sunxi_nfc_caps *c = conf->caps;
> + int min_user_data_sz = 0;
> + int i;
> +
> + if (!c->reg_user_data_len) {
> + for (i = 0; i < nsectors; i++)
> + min_user_data_sz += nand_user_data_sz(conf, i);
> + } else {
> + for (i = 0; i < c->nuser_data_tab; i++)
> + /* We want at least enough size for the BBM */
> + if (c->user_data_len_tab[i] >= 2)
> + break;
> + min_user_data_sz = c->user_data_len_tab[i];
> + }
> +
> + return min_user_data_sz;
> +}
> +
> +static int nand_maximize_user_data(struct nfc_config *conf, uint32_t oobsize,
> + int ecc_len, int nsectors)
> +{
> + const struct sunxi_nfc_caps *c = conf->caps;
> + int remaining_bytes = oobsize - (ecc_len * nsectors);
> + int i, step;
> +
> + kfree(conf->user_data_bytes);
> +
> + conf->user_data_bytes = kzalloc(nsectors, GFP_KERNEL);
> + if (!conf->user_data_bytes)
> + return -ENOMEM;
> +
> + for (step = 0; (step < nsectors) && (remaining_bytes > 0); step++) {
> + for (i = 0; i < c->nuser_data_tab; i++) {
> + if (c->user_data_len_tab[i] > remaining_bytes)
> + break;
> + conf->user_data_bytes[step] = c->user_data_len_tab[i];
> + }
> + remaining_bytes -= conf->user_data_bytes[step];
> + if (conf->user_data_bytes[step] == 0)
> + break;
> }
>
> return 0;
> @@ -387,7 +446,8 @@ static int nand_max_ecc_strength(struct nfc_config *conf)
> {
> int max_oobsize, max_ecc_bytes;
> int nsectors = conf->page_size / conf->ecc_size;
> - int i;
> + unsigned int total_user_data_sz = 0;
> + int ecc_idx, i;
>
> /*
> * ECC strength is limited by the size of the OOB area which is
> @@ -412,15 +472,38 @@ static int nand_max_ecc_strength(struct nfc_config *conf)
>
> max_ecc_bytes = max_oobsize / nsectors;
>
> - for (i = 0; i < ARRAY_SIZE(ecc_bytes); i++) {
> - if (ecc_bytes[i] > max_ecc_bytes)
> + /*
> + * nand_min_user_data_sz() will return the total_user_data_sz in case
> + * of a fixed user data length, or the minimal usable user data size
> + * in case of variable data length (with at least enough space for the
> + * BBM.
> + */
> + total_user_data_sz = nand_min_user_data_sz(conf, nsectors);
> +
> + for (ecc_idx = 0; ecc_idx < ARRAY_SIZE(ecc_bytes); ecc_idx++) {
> + if (ecc_bytes[ecc_idx] + total_user_data_sz > max_ecc_bytes)
> break;
> }
>
> - if (!i)
> + if (!ecc_idx)
> return -EINVAL;
>
> - return i - 1;
> + ecc_idx--;
> +
> + /*
> + * The rationale for variable data length is to prioritize maximum ECC
> + * strength, and then use the remaining space for user data.
> + */
> + if (conf->caps->reg_user_data_len) {
> + nand_maximize_user_data(conf, max_oobsize,
> + ecc_bytes[ecc_idx], nsectors);
> +
> + total_user_data_sz = 0;
> + for (i = 0; i < nsectors; i++)
> + total_user_data_sz += nand_user_data_sz(conf, i);
> + }
> +
> + return ecc_idx;
> }
>
> static int nand_detect_ecc_config(struct nfc_config *conf, u32 offs,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 5/5] mtd: nand: raw: sunxi_spl: remove user data length reset
2026-03-27 14:05 ` [PATCH v2 5/5] mtd: nand: raw: sunxi_spl: remove user data length reset Richard Genoud
@ 2026-05-01 12:47 ` Andre Przywara
0 siblings, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2026-05-01 12:47 UTC (permalink / raw)
To: Richard Genoud
Cc: Dario Binacchi, Michael Trimarchi, Tom Rini, Andrew Goodbody,
Miquel Raynal, James Hilliard, Boris Brezillon, Thomas Petazzoni,
u-boot
On Fri, 27 Mar 2026 15:05:08 +0100
Richard Genoud <richard.genoud@bootlin.com> wrote:
> No need to reset user data length registers in SPL.
>
> In SPL, only the first user data length register is used, so we don't
> need to reset all of them.
>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
Acked-by: Andre Przywara <andre.przywara@arm.com>
Cheers,
Andre
> ---
> drivers/mtd/nand/raw/sunxi_nand_spl.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index 784ffb00cf70..cf351de4e8de 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -224,22 +224,6 @@ static int nand_change_column(u16 column)
> return 0;
> }
>
> -/*
> - * On H6/H616 the user_data length has to be set in specific registers
> - * before writing.
> - */
> -static void sunxi_nfc_reset_user_data_len(const struct nfc_config *nfc)
> -{
> - int loop_step = NFC_REG_USER_DATA_LEN_CAPACITY;
> -
> - /* not all SoCs have this register */
> - if (!NFC_REG_USER_DATA_LEN(nfc, 0))
> - return;
> -
> - for (int i = 0; i < nfc->caps->max_ecc_steps; i += loop_step)
> - writel_nfc(0, NFC_REG_USER_DATA_LEN(nfc, i));
> -}
> -
> static void sunxi_nfc_set_user_data_len(const struct nfc_config *nfc,
> int len, int step)
> {
> @@ -355,7 +339,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs,
> */
> nand_change_column(oob_off);
>
> - sunxi_nfc_reset_user_data_len(conf);
> sunxi_nfc_set_user_data_len(conf, user_data_sz, nfc_step);
>
> nand_exec_cmd(NFC_DATA_TRANS | NFC_ECC_OP);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init()
2026-04-27 12:35 ` Andre Przywara
@ 2026-05-01 12:59 ` Andre Przywara
0 siblings, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2026-05-01 12:59 UTC (permalink / raw)
To: Michael Nazzareno Trimarchi, Richard Genoud
Cc: Dario Binacchi, Tom Rini, Andrew Goodbody, Miquel Raynal,
James Hilliard, Boris Brezillon, Thomas Petazzoni, u-boot
On Mon, 27 Apr 2026 14:35:08 +0200
Andre Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> On 3/27/26 15:42, Michael Nazzareno Trimarchi wrote:
> > Hi Richard
> >
> > On Fri, Mar 27, 2026 at 3:05 PM Richard Genoud
> > <richard.genoud@bootlin.com> wrote:
> >>
> >> In sunxi_nand_chip_init there's quite a lot of kfree/return, it's easy
> >> to forget a kfree(), so use a goto/kfree instead.
> >>
> >> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> >> ---
> >> drivers/mtd/nand/raw/sunxi_nand.c | 41 ++++++++++++++-----------------
> >> 1 file changed, 18 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> >> index 9fc9bc5e0198..ecab9ebc9576 100644
> >> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> >> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> >> @@ -1600,21 +1600,20 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
> >> if (ret) {
> >> dev_err(dev, "could not retrieve reg property: %d\n",
> >> ret);
> >> - kfree(chip);
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> if (tmp > NFC_MAX_CS) {
> >> dev_err(dev,
> >> "invalid reg value: %u (max CS = 7)\n", tmp);
> >> - kfree(chip);
> >> - return -EINVAL;
> >> + ret = -EINVAL;
> >> + goto out;
> >> }
> >>
> >> if (test_and_set_bit(tmp, &nfc->assigned_cs)) {
> >> dev_err(dev, "CS %d already assigned\n", tmp);
> >> - kfree(chip);
> >> - return -EINVAL;
> >> + ret = -EINVAL;
> >> + goto out;
> >> }
> >>
> >> chip->sels[i].cs = tmp;
> >> @@ -1640,15 +1639,13 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
> >> dev_err(dev,
> >> "could not retrieve timings for ONFI mode 0: %d\n",
> >> ret);
> >> - kfree(chip);
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> ret = sunxi_nand_chip_set_timings(nfc, chip, timings);
> >> if (ret) {
> >> dev_err(dev, "could not configure chip timings: %d\n", ret);
> >> - kfree(chip);
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> nand = &chip->nand;
> >> @@ -1669,10 +1666,8 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
> >>
> >> mtd = nand_to_mtd(nand);
> >> ret = nand_scan_ident(mtd, nsels, NULL);
> >> - if (ret) {
> >> - kfree(chip);
> >> - return ret;
> >> - }
> >> + if (ret)
> >> + goto out;
> >>
> >> if (nand->bbt_options & NAND_BBT_USE_FLASH)
> >> nand->bbt_options |= NAND_BBT_NO_OOB;
> >> @@ -1685,34 +1680,34 @@ static int sunxi_nand_chip_init(struct udevice *dev, struct sunxi_nfc *nfc,
> >> ret = sunxi_nand_chip_init_timings(nfc, chip);
> >> if (ret) {
> >> dev_err(dev, "could not configure chip timings: %d\n", ret);
> >> - kfree(chip);
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> ret = sunxi_nand_ecc_init(mtd, &nand->ecc);
> >> if (ret) {
> >> dev_err(dev, "ECC init failed: %d\n", ret);
> >> - kfree(chip);
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> ret = nand_scan_tail(mtd);
> >> if (ret) {
> >> dev_err(dev, "nand_scan_tail failed: %d\n", ret);
> >> - kfree(chip);
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> ret = nand_register(devnum, mtd);
> >> if (ret) {
> >> dev_err(dev, "failed to register mtd device: %d\n", ret);
> >> - kfree(chip);
> >> - return ret;
> >> + goto out;
> >> }
> >>
> >> list_add_tail(&chip->node, &nfc->chips);
> >>
> >> - return 0;
> >> +out:
> >> + if (ret)
> >> + kfree(chip);
> >> +
> >> + return ret;
> >> }
> >>
> >> static int sunxi_nand_chips_init(struct udevice *dev, struct sunxi_nfc *nfc)
> >
> > I need to go a bit in the code but seems that out can happen even if
> > there is no error. It's
> > not so common to have if (ret) if exit condition in case of errot
>
> So I think it's fine in this case, ret is always set before we "goto",
> and in the success case "ret" must be 0 from the last check. But I agree
> that the construct is a bit uncommon and fragile, so what about keeping
> the "return 0;" (to use the common pattern and help readability), and
> rename the label to "out_err" or "out_free", to make it more obvious
> that this the error path only, and we expect ret to be set?
So FYI, in the interest of moving this patch set forward, I took the
freedom of changing this as I suggested above. That's gonna be part of
the PR now.
Cheers,
Andre
>
> Cheers,
> Andre
>
> >
> > Michael
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-05-01 12:59 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-27 14:05 [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Richard Genoud
2026-03-27 14:05 ` [PATCH v2 1/5] mtd: rawnand: sunxi: Replace hard coded value by a define Richard Genoud
2026-04-27 11:14 ` Andre Przywara
2026-03-27 14:05 ` [PATCH v2 2/5] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
2026-03-27 14:40 ` Michael Nazzareno Trimarchi
2026-04-27 12:23 ` Andre Przywara
2026-03-27 14:05 ` [PATCH v2 3/5] mtd: rawnand: sunxi: clean sunxi_nand_chip_init() Richard Genoud
2026-03-27 14:42 ` Michael Nazzareno Trimarchi
2026-04-27 12:35 ` Andre Przywara
2026-05-01 12:59 ` Andre Przywara
2026-03-27 14:05 ` [PATCH v2 4/5] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
2026-05-01 12:46 ` Andre Przywara
2026-03-27 14:05 ` [PATCH v2 5/5] mtd: nand: raw: sunxi_spl: remove user data length reset Richard Genoud
2026-05-01 12:47 ` Andre Przywara
2026-04-27 12:28 ` [PATCH v2 0/5] mtd: rawnand: sunxi: Fix user data length for H6 Andre Przywara
2026-04-27 12:31 ` Michael Nazzareno Trimarchi
2026-04-27 13:47 ` Andre Przywara
2026-04-27 14:10 ` Michael Nazzareno Trimarchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox