Re: [PATCH v3 13/15] mtd: rawnand: sunxi: Add support for H616 nand controller
From: Richard GENOUD
Date: Tue Oct 28 2025 - 03:30:37 EST
Le 22/10/2025 à 11:18, Miquel Raynal a écrit :
Hello Richard,ok
On 20/10/2025 at 12:13:09 +02, Richard Genoud <richard.genoud@xxxxxxxxxxx> wrote:
The H616 nand controller has the same base as A10/A23, with some
differences:
- mdma is based on chained buffers
- its ECC supports up to 80bit per 1024bytes
- some registers layouts are a bit different, mainly due do the stronger
ECC.
- it uses USER_DATA_LEN registers along USER_DATA registers.
- it needs a specific clock for ECC and MBUS.
Introduce the basic support, with ECC and scrambling, but without
DMA/MDMA.
Tested on Whatsminer H616 board (with and without scrambling, ECC)
Signed-off-by: Richard Genoud <richard.genoud@xxxxxxxxxxx>
...
+#define NFC_TIMING_CFG2(tCDQSS, tSC, tCLHZ, tCSS, tWC) \
+ ((((tCDQSS) & 0x1) << 11) | (((tSC) & 0x3) << 12) | \
+ (((tCLHZ) & 0x3) << 14) | (((tCSS) & 0x3) << 16) | \
+ (((tWC) & 0x3) << 18))
+
/* define bit use in NFC_CMD */
#define NFC_CMD_LOW_BYTE_MSK GENMASK(7, 0)
-#define NFC_CMD_HIGH_BYTE_MSK GENMASK(15, 8)
+#define NFC_CMD_HIGH_BYTE_MSK GENMASK(15, 8) // 15-10 reserved on H6
Wrong comment type :-)
+#define NFC_CMD_ADR_NUM_MSK GENMASK(9, 8)
#define NFC_CMD(x) (x)
#define NFC_ADR_NUM_MSK GENMASK(18, 16)
#define NFC_ADR_NUM(x) (((x) - 1) << 16)
@@ -122,6 +156,7 @@
#define NFC_SEQ BIT(25)
#define NFC_DATA_SWAP_METHOD BIT(26)
#define NFC_ROW_AUTO_INC BIT(27)
+#define NFC_H6_SEND_RND_CMD2 BIT(27)
#define NFC_SEND_CMD3 BIT(28)
#define NFC_SEND_CMD4 BIT(29)
#define NFC_CMD_TYPE_MSK GENMASK(31, 30)
@@ -133,6 +168,7 @@
#define NFC_READ_CMD_MSK GENMASK(7, 0)
#define NFC_RND_READ_CMD0_MSK GENMASK(15, 8)
#define NFC_RND_READ_CMD1_MSK GENMASK(23, 16)
+#define NFC_RND_READ_CMD2_MSK GENMASK(31, 24)
...
@@ -858,6 +967,8 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
if (ret)
return ret;
+ sunxi_nfc_reset_user_data_len(nfc);
+ sunxi_nfc_set_user_data_len(nfc, 4, 0);
I'm not sure I understand this properly. Why isn't this a fixed setting?
Also, what is 4? It is not obvious to me and my require either a comment
or a define (or maybe a sizeof()).
Yes, indeed. There are '4' all over the place, maybe a bit of tidy up is needed in a separate patch.
(and some explanations)
I'll resend a v4 with that.
yes, definitely
sunxi_nfc_randomizer_config(nand, page, false);
sunxi_nfc_randomizer_enable(nand);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
@@ -968,6 +1079,8 @@ static int sunxi_nfc_hw_ecc_read_chunks_dma(struct nand_chip *nand, uint8_t *buf
return ret;
sunxi_nfc_hw_ecc_enable(nand);
+ sunxi_nfc_reset_user_data_len(nfc);
+ sunxi_nfc_set_user_data_len(nfc, 4, 0);
sunxi_nfc_randomizer_config(nand, page, false);
sunxi_nfc_randomizer_enable(nand);
@@ -1100,6 +1213,8 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct nand_chip *nand,
sunxi_nfc_randomizer_config(nand, page, false);
sunxi_nfc_randomizer_enable(nand);
+ sunxi_nfc_reset_user_data_len(nfc);
+ sunxi_nfc_set_user_data_len(nfc, 4, 0);
sunxi_nfc_hw_ecc_set_prot_oob_bytes(nand, oob, 0, bbm, page);
writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
@@ -1344,10 +1459,12 @@ static int sunxi_nfc_hw_ecc_write_page_dma(struct nand_chip *nand,
if (ret)
goto pio_fallback;
+ sunxi_nfc_reset_user_data_len(nfc);
for (i = 0; i < ecc->steps; i++) {
const u8 *oob = nand->oob_poi + (i * (ecc->bytes + 4));
sunxi_nfc_hw_ecc_set_prot_oob_bytes(nand, oob, i, !i, page);
+ sunxi_nfc_set_user_data_len(nfc, 4, i);
Here you use it differently, maybe a bit of explanation in a comment
could help.
yes, I'll remove them.}
nand_prog_page_begin_op(nand, page, 0, NULL, 0);
@@ -2148,6 +2265,10 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
if (irq < 0)
return irq;
+ nfc->caps = of_device_get_match_data(dev);
+ if (!nfc->caps)
+ return -EINVAL;
+
nfc->ahb_clk = devm_clk_get_enabled(dev, "ahb");
if (IS_ERR(nfc->ahb_clk)) {
dev_err(dev, "failed to retrieve ahb clk\n");
@@ -2160,6 +2281,22 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
return PTR_ERR(nfc->mod_clk);
}
...
static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
.has_ecc_block_512 = true,
+ .has_ecc_clk = false,
+ .has_mbus_clk = false,
As you want, but setting these fields (and below) to false is not
strictly required as they will be set to 0 (which means false,
automatically).
I'll drop this line also..reg_io_data = NFC_REG_A10_IO_DATA,
.reg_ecc_err_cnt = NFC_REG_A10_ECC_ERR_CNT,
.reg_user_data = NFC_REG_A10_USER_DATA,
@@ -2242,11 +2383,14 @@ static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
.dma_maxburst = 4,
.ecc_strengths = sunxi_ecc_strengths_a10,
.nstrengths = ARRAY_SIZE(sunxi_ecc_strengths_a10),
+ .max_ecc_steps = 16,
.sram_size = 1024,
};
+static const struct sunxi_nfc_caps sunxi_nfc_h616_caps = {
+ .has_mdma = false, // H616 supports only chained descriptors
Wrong comment type :-)
Thanks!
LGTM otherwise.
Thanks,
Miquèl
--
Richard Genoud, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com