[PATCH 0/2] Realtek ECC engine

[PATCH 0/2] Realtek ECC engine

[Markus Stockhausen]

This series adds support for the ECC engine on Realtek RTL93xx switch SoCs.



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/2] dt-bindings: mtd: Add realtek,rtl9301-ecc

[Markus Stockhausen]

Add a dtschema for the ECC engine on the Realtek RTL93xx SoCs.
The engine supports BCH6 and BCH12 parity for 512 byte blocks.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 .../bindings/mtd/realtek,rtl9301-ecc.yaml     | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.yaml

diff --git a/Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.yaml b/Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.yaml
new file mode 100644
index 000000000000..9ec69e661812
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/realtek,rtl9301-ecc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek SoCs NAND ECC engine
+
+maintainers:
+  - Markus Stockhausen <markus.stockhausen@gmx.de>
+
+properties:
+  compatible:
+    const: realtek,rtl9301-ecc
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        ecc0: ecc@1a600 {
+            compatible = "realtek,rtl9301-ecc";
+            reg = <0x1a600 0x54>;
+        };
+    };
-- 
2.47.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/2] mtd: nand: realtek-ecc: Add Realtek external ECC engine support

[Markus Stockhausen]

The Realtek RTl93xx switch SoC series has a built in ECC controller
that can provide BCH6 or BCH12 over 512 data and 6 tag bytes. It
generates 10 (BCH6) or 20 (BCH12) bytes of parity.

This engine will most likely work in conjunction with the Realtek
spi-mem based NAND controller but can work on its own. Therefore
the initial implementation will be of type external.

Remark! The engine can support any data blocks that are multiples
of 512 bytes. For now limit it to data+oob layouts that have been
analyzed from existing devices. This way it keeps compatibility
and pre-existing vendor data can be read.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/mtd/nand/Kconfig       |   8 +
 drivers/mtd/nand/Makefile      |   1 +
 drivers/mtd/nand/ecc-realtek.c | 433 +++++++++++++++++++++++++++++++++
 3 files changed, 442 insertions(+)
 create mode 100644 drivers/mtd/nand/ecc-realtek.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b0c2c95f10c..4a17271076bc 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -61,6 +61,14 @@ config MTD_NAND_ECC_MEDIATEK
 	help
 	  This enables support for the hardware ECC engine from Mediatek.
 
+config MTD_NAND_ECC_REALTEK
+        tristate "Realtek RTL93xx hardware ECC engine"
+        depends on HAS_IOMEM
+        depends on MACH_REALTEK_RTL || COMPILE_TEST
+        select MTD_NAND_ECC
+        help
+          This enables support for the hardware ECC engine from Realtek.
+
 endmenu
 
 endmenu
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 44913ff1bf12..2e0e56267718 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -3,6 +3,7 @@
 nandcore-objs := core.o bbt.o
 obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
 obj-$(CONFIG_MTD_NAND_ECC_MEDIATEK) += ecc-mtk.o
+obj-$(CONFIG_MTD_NAND_ECC_REALTEK) += ecc-realtek.o
 obj-$(CONFIG_SPI_QPIC_SNAND) += qpic_common.o
 obj-$(CONFIG_MTD_NAND_QCOM) += qpic_common.o
 obj-y	+= onenand/
diff --git a/drivers/mtd/nand/ecc-realtek.c b/drivers/mtd/nand/ecc-realtek.c
new file mode 100644
index 000000000000..a9c2a3a23b78
--- /dev/null
+++ b/drivers/mtd/nand/ecc-realtek.c
@@ -0,0 +1,433 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for Realtek hardware ECC engine in RTL93xx SoCs
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/mtd/nand.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/*
+ * The Realtek ECC engine has two operation modes.
+ *
+ * - BCH6 : Generate 10 ECC bytes from 512 data bytes plus 6 tag bytes
+ * - BCH12: Generate 20 ECC bytes from 512 data bytes plus 6 tag bytes
+ *
+ * It can run for arbitrary NAND flash chips with different block and OOB sizes. Currently there
+ * are only two known devices in the wild that have NAND flash and make use of this ECC engine
+ * (Linksys LGS328C & LGS352C). To keep compatibility with vendor firmware, new modes can only
+ * be added when new data layouts have been analyzed. For now allow BCH6 on flash with 2048 byte
+ * blocks and 64 bytes oob.
+ */
+
+#define RTL_ECC_ALLOWED_PAGE_SIZE 	2048
+#define RTL_ECC_ALLOWED_OOB_SIZE	64
+#define RTL_ECC_ALLOWED_STRENGTH	6
+
+#define RTL_ECC_BLOCK_SIZE 		512
+#define RTL_ECC_TAG_SIZE 		6
+#define RTL_ECC_PARITY_SIZE_BCH6	10
+#define RTL_ECC_PARITY_SIZE_BCH12	20
+
+/*
+ * The engine is fed with two DMA regions. One for data (always 512 bytes) and one for tag and
+ * parity (either 16 bytes for BCH6 or 26 bytes for BCH12). Start and length of each must be
+ * aligned to a multiple of 4.
+ */
+
+#define RTL_ECC_DMA_TAG_PARITY_SIZE	ALIGN(RTL_ECC_TAG_SIZE + RTL_ECC_PARITY_SIZE_BCH12, 4)
+#define RTL_ECC_DMA_SIZE		(RTL_ECC_BLOCK_SIZE + RTL_ECC_DMA_TAG_PARITY_SIZE)
+
+#define RTL_ECC_CFG			0x00
+#define   RTL_ECC_BCH6			0
+#define   RTL_ECC_BCH12			BIT(28)
+#define   RTL_ECC_DMA_PRECISE		BIT(12)
+#define   RTL_ECC_BURST_128		GENMASK(1, 0)
+#define RTL_ECC_DMA_TRIGGER 		0x08
+#define   RTL_ECC_OP_DECODE		0
+#define   RTL_ECC_OP_ENCODE		BIT(0)
+#define RTL_ECC_DMA_START		0x0c
+#define RTL_ECC_DMA_TAG			0x10
+#define RTL_ECC_STATUS			0x14
+#define   RTL_ECC_CORR_COUNT		GENMASK(19, 12)
+#define   RTL_ECC_RESULT		BIT(8)
+#define   RTL_ECC_ALL_ONE		BIT(4)
+#define   RTL_ECC_OP_STATUS		BIT(0)
+
+struct rtl_ecc_engine {
+	struct device *dev;
+	struct nand_ecc_engine engine;
+	struct mutex lock;
+	char *buf;
+	dma_addr_t buf_dma;
+	struct regmap *regmap;
+};
+
+struct rtl_ecc_ctx {
+	struct rtl_ecc_engine * rtlc;
+	struct nand_ecc_req_tweak_ctx req_ctx;
+	int steps;
+	int bch_mode;
+	int parity_size;
+	int rc_bitflips;
+	int rc_status;
+};
+
+static const struct regmap_config rtl_ecc_regmap_config = {
+	.reg_bits	= 32,
+	.val_bits	= 32,
+	.reg_stride	= 4,
+};
+
+static inline void *nand_to_ctx(struct nand_device *nand)
+{
+	return nand->ecc.ctx.priv;
+}
+
+static inline struct rtl_ecc_engine *nand_to_rtlc(struct nand_device *nand)
+{
+	struct nand_ecc_engine *eng = nand->ecc.engine;
+
+	return container_of(eng, struct rtl_ecc_engine, engine);
+}
+
+static int rtl_ecc_ooblayout_ecc(struct mtd_info *mtd, int section,
+				 struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
+
+	if (section < 0 || section >= ctx->steps)
+		return -ERANGE;
+
+	oobregion->offset = ctx->steps * RTL_ECC_TAG_SIZE + section * ctx->parity_size;
+	oobregion->length = ctx->parity_size;
+
+	return 0;
+}
+
+static int rtl_ecc_ooblayout_free(struct mtd_info *mtd, int section,
+				struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
+
+	if (section < 0 || section >= ctx->steps)
+		return -ERANGE;
+
+	/* Tags can hold arbitrary values, so they are free for user data */
+	oobregion->offset = section * RTL_ECC_TAG_SIZE;
+	oobregion->length = RTL_ECC_TAG_SIZE;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops rtl_ecc_ooblayout_ops = {
+	.ecc = rtl_ecc_ooblayout_ecc,
+	.free = rtl_ecc_ooblayout_free,
+};
+
+static void rtl_ecc_kick_engine(struct rtl_ecc_ctx *ctx, int operation)
+{
+	struct rtl_ecc_engine *rtlc = ctx->rtlc;
+
+	regmap_write(rtlc->regmap, RTL_ECC_CFG,
+		     ctx->bch_mode | RTL_ECC_BURST_128 | RTL_ECC_DMA_PRECISE);
+
+	regmap_write(rtlc->regmap, RTL_ECC_DMA_START, rtlc->buf_dma);
+	regmap_write(rtlc->regmap, RTL_ECC_DMA_TAG, rtlc->buf_dma + RTL_ECC_BLOCK_SIZE);
+	regmap_write(rtlc->regmap, RTL_ECC_DMA_TRIGGER, operation);
+}
+
+static void rtl_ecc_wait_for_engine(struct rtl_ecc_ctx *ctx)
+{
+	struct rtl_ecc_engine *rtlc = ctx->rtlc;
+	int ret, status;
+	bool all_one;
+
+	/*
+	 * The ECC engine needs 6-8 us to encode/decode a BCH6 syndrome for 512 bytes of data
+	 * and 6 tag bytes. In case the NAND area has been erased and all data and oob is
+	 * set to 0xff, decoding takes 30us (reason unknown). Although the engine can trigger
+	 * interrupts when finished, use active polling for now. 12 us maximum wait time has
+	 * proven to be a good tradeoff between performance and overhead.
+	 */
+
+	ret = regmap_read_poll_timeout(rtlc->regmap, RTL_ECC_STATUS, status,
+				       !(status & RTL_ECC_OP_STATUS), 12, 600);
+	if (ret) {
+		ctx->rc_status = ret;
+		return;
+	}
+
+	all_one = FIELD_GET(RTL_ECC_ALL_ONE, status);
+	ctx->rc_status = FIELD_GET(RTL_ECC_RESULT, status);
+	ctx->rc_bitflips = FIELD_GET(RTL_ECC_CORR_COUNT, status);
+
+	/* For erased blocks (all bits one) error status can be ignored */
+	if (ctx->rc_status && all_one)
+		ctx->rc_status = 0;
+}
+
+static void rtl_ecc_run_engine(struct rtl_ecc_ctx *ctx, char *data, char *tag,
+			       char *parity, int operation)
+{
+	struct rtl_ecc_engine *rtlc = ctx->rtlc;
+	char *buf_parity = rtlc->buf + RTL_ECC_BLOCK_SIZE + RTL_ECC_TAG_SIZE;
+	char *buf_tag = rtlc->buf + RTL_ECC_BLOCK_SIZE;
+	char *buf_data = rtlc->buf;
+
+	mutex_lock(&rtlc->lock);
+
+	memcpy(buf_data, data, RTL_ECC_BLOCK_SIZE);
+	memcpy(buf_tag, tag, RTL_ECC_TAG_SIZE);
+	memcpy(buf_parity, parity, ctx->parity_size);
+
+	dma_sync_single_for_device(rtlc->dev, rtlc->buf_dma, RTL_ECC_DMA_SIZE, DMA_TO_DEVICE);
+	rtl_ecc_kick_engine(ctx, operation);
+	rtl_ecc_wait_for_engine(ctx);
+	dma_sync_single_for_cpu(rtlc->dev, rtlc->buf_dma, RTL_ECC_DMA_SIZE, DMA_FROM_DEVICE);
+
+	if (!ctx->rc_status) {
+		memcpy(data, buf_data, RTL_ECC_BLOCK_SIZE);
+		memcpy(tag, buf_tag, RTL_ECC_TAG_SIZE);
+		memcpy(parity, buf_parity, ctx->parity_size);
+	}
+
+	mutex_unlock(&rtlc->lock);
+}
+
+static int rtl_ecc_prepare_io_req(struct nand_device *nand, struct nand_page_io_req *req)
+{
+	struct rtl_ecc_engine *rtlc = nand_to_rtlc(nand);
+	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
+	char *data, *tag, *parity;
+	int failure = 0;
+
+	if (req->mode == MTD_OPS_RAW)
+		return 0;
+
+	nand_ecc_tweak_req(&ctx->req_ctx, req);
+
+	if (req->type == NAND_PAGE_READ)
+		return 0;
+
+	tag = req->oobbuf.in;
+	data = req->databuf.in;
+	parity = req->oobbuf.in + ctx->steps * RTL_ECC_TAG_SIZE;
+
+	for (int i = 0; i < ctx->steps; i++) {
+		rtl_ecc_run_engine(ctx, data, tag, parity, RTL_ECC_OP_ENCODE);
+		failure |= ctx->rc_status;
+
+		tag += RTL_ECC_TAG_SIZE;
+		data += RTL_ECC_BLOCK_SIZE;
+		parity += ctx->parity_size;
+	}
+
+	if (unlikely(failure))
+		dev_err(rtlc->dev, "ECC calculation failed\n");
+
+	return failure ? -EBADMSG : 0;
+}
+
+static int rtl_ecc_finish_io_req(struct nand_device *nand, struct nand_page_io_req *req)
+{
+	struct rtl_ecc_engine *rtlc = nand_to_rtlc(nand);
+	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	char *data, *tag, *parity;
+	unsigned int bitflips = 0;
+	int failure = 0;
+
+	if (req->mode == MTD_OPS_RAW)
+		return 0;
+
+	if (req->type == NAND_PAGE_WRITE) {
+		nand_ecc_restore_req(&ctx->req_ctx, req);
+		return 0;
+	}
+
+	tag = req->oobbuf.in;
+	data = req->databuf.in;
+	parity = req->oobbuf.in + ctx->steps * RTL_ECC_TAG_SIZE;
+
+	for (int i = 0 ; i < ctx->steps; i++) {
+		rtl_ecc_run_engine(ctx, data, tag, parity, RTL_ECC_OP_DECODE);
+		failure |= ctx->rc_status;
+		mtd->ecc_stats.failed += !!ctx->rc_status;
+		bitflips = max_t(unsigned int, ctx->rc_bitflips, bitflips);
+
+		tag += RTL_ECC_TAG_SIZE;
+		data += RTL_ECC_BLOCK_SIZE;
+		parity += ctx->parity_size;
+	}
+
+	nand_ecc_restore_req(&ctx->req_ctx, req);
+
+	if (unlikely(failure))
+		dev_err(rtlc->dev, "ECC correction failed\n");
+	else if (unlikely(bitflips > 2))
+		dev_warn(rtlc->dev, "%d bitflips detected\n", ctx->rc_bitflips);
+
+	return failure ? -EBADMSG : bitflips;
+}
+
+static int rtl_ecc_check_support(struct nand_device *nand)
+{
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	struct device *dev = nand->ecc.engine->dev;
+
+	if (mtd->oobsize != RTL_ECC_ALLOWED_OOB_SIZE ||
+	    mtd->writesize != RTL_ECC_ALLOWED_PAGE_SIZE) {
+		dev_err(dev, "only flash geometry data=%d, oob=%d supported\n",
+			RTL_ECC_ALLOWED_PAGE_SIZE, RTL_ECC_ALLOWED_OOB_SIZE);
+		return -EINVAL;
+	}
+
+	if (nand->ecc.user_conf.algo != NAND_ECC_ALGO_BCH ||
+	    nand->ecc.user_conf.strength != RTL_ECC_ALLOWED_STRENGTH ||
+	    nand->ecc.user_conf.placement != NAND_ECC_PLACEMENT_OOB ||
+	    nand->ecc.user_conf.step_size != RTL_ECC_BLOCK_SIZE) {
+		dev_err(dev, "only algo=bch, strength=%d, placement=oob, step=%d supported\n",
+			RTL_ECC_ALLOWED_STRENGTH, RTL_ECC_BLOCK_SIZE);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rtl_ecc_init_ctx(struct nand_device *nand)
+{
+	struct nand_ecc_props *conf = &nand->ecc.ctx.conf;
+	struct rtl_ecc_engine *rtlc = nand_to_rtlc(nand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	int strength = nand->ecc.user_conf.strength;
+	struct device *dev = nand->ecc.engine->dev;
+	struct rtl_ecc_ctx *ctx;
+	int ret;
+
+	ret = rtl_ecc_check_support(nand);
+	if (ret)
+		return ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	nand->ecc.ctx.priv = ctx;
+	mtd_set_ooblayout(mtd, &rtl_ecc_ooblayout_ops);
+
+	conf->algo = NAND_ECC_ALGO_BCH;
+	conf->strength = strength;
+	conf->step_size = RTL_ECC_BLOCK_SIZE;
+	conf->engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
+
+	ctx->rtlc = rtlc;
+	ctx->steps = mtd->writesize / RTL_ECC_BLOCK_SIZE;
+	ctx->bch_mode = strength == 6 ? RTL_ECC_BCH6 : RTL_ECC_BCH12;
+	ctx->parity_size = strength == 6 ? RTL_ECC_PARITY_SIZE_BCH6 : RTL_ECC_PARITY_SIZE_BCH12;
+
+	ret = nand_ecc_init_req_tweaking(&ctx->req_ctx, nand);
+	if (ret)
+		return ret;
+
+	dev_info(dev, "using bch%d with geometry data=%dx%d, tag=%dx6, parity=%dx%d",
+		 conf->strength, ctx->steps, conf->step_size,
+		 ctx->steps, ctx->steps, ctx->parity_size);
+
+	return 0;
+}
+
+static void rtl_ecc_cleanup_ctx(struct nand_device *nand)
+{
+	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
+
+	if (ctx)
+		nand_ecc_cleanup_req_tweaking(&ctx->req_ctx);
+}
+
+static struct nand_ecc_engine_ops rtl_ecc_engine_ops = {
+	.init_ctx = rtl_ecc_init_ctx,
+	.cleanup_ctx = rtl_ecc_cleanup_ctx,
+	.prepare_io_req = rtl_ecc_prepare_io_req,
+	.finish_io_req = rtl_ecc_finish_io_req,
+};
+
+static int rtl_ecc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rtl_ecc_engine *rtlc;
+	void __iomem *base;
+	int ret;
+
+	rtlc = devm_kzalloc(dev, sizeof(*rtlc), GFP_KERNEL);
+	if (!rtlc)
+		return -ENOMEM;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	ret = devm_mutex_init(dev, &rtlc->lock);
+	if (ret)
+		return ret;
+
+	rtlc->regmap = devm_regmap_init_mmio(dev, base, &rtl_ecc_regmap_config);
+	if (IS_ERR(rtlc->regmap))
+		return PTR_ERR(rtlc->regmap);
+
+	/*
+	 * Focus on simplicity and use a preallocated DMA buffer for data exchange with the
+	 * engine. Make it a noncoherent memory model as invalidating/flushing caches is
+	 * faster than reading/writing uncached memory.
+	 */
+
+	rtlc->buf = dma_alloc_noncoherent(dev, RTL_ECC_DMA_SIZE, &rtlc->buf_dma,
+					  DMA_BIDIRECTIONAL, GFP_KERNEL);
+	if (IS_ERR(rtlc->buf))
+		return PTR_ERR(rtlc->buf);
+
+	rtlc->dev = dev;
+	rtlc->engine.dev = dev;
+	rtlc->engine.ops = &rtl_ecc_engine_ops;
+	rtlc->engine.integration = NAND_ECC_ENGINE_INTEGRATION_EXTERNAL;
+
+	nand_ecc_register_on_host_hw_engine(&rtlc->engine);
+
+	platform_set_drvdata(pdev, rtlc);
+
+	return 0;
+}
+
+static void rtl_ecc_remove(struct platform_device *pdev)
+{
+	struct rtl_ecc_engine *rtlc = platform_get_drvdata(pdev);
+
+	nand_ecc_unregister_on_host_hw_engine(&rtlc->engine);
+	dma_free_noncoherent(rtlc->dev, RTL_ECC_DMA_SIZE, rtlc->buf, rtlc->buf_dma,
+			     DMA_BIDIRECTIONAL);
+}
+
+static const struct of_device_id rtl_ecc_of_ids[] = {
+	{
+		.compatible = "realtek,rtl9301-ecc",
+	},
+	{ /* sentinel */ },
+};
+
+static struct platform_driver rtl_ecc_driver = {
+	.driver	= {
+		.name = "rtl-nand-ecc-engine",
+		.of_match_table = rtl_ecc_of_ids,
+	},
+	.probe = rtl_ecc_probe,
+	.remove = rtl_ecc_remove,
+};
+module_platform_driver(rtl_ecc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Markus Stockhausen <markus.stockhausen@gmx.de>");
+MODULE_DESCRIPTION("Realtek NAND hardware ECC controller");
-- 
2.47.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/2] dt-bindings: mtd: Add realtek,rtl9301-ecc

[Rob Herring (Arm)]

On Mon, 18 Aug 2025 05:27:24 -0400, Markus Stockhausen wrote:
> Add a dtschema for the ECC engine on the Realtek RTL93xx SoCs.
> The engine supports BCH6 and BCH12 parity for 512 byte blocks.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> ---
>  .../bindings/mtd/realtek,rtl9301-ecc.yaml     | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.example.dts:24.17-38: Warning (reg_format): /example-0/soc/ecc@1a600:reg: property has invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)
Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/realtek,rtl9301-ecc.example.dtb: soc: ecc@1a600:reg:0: [108032, 84] is too short
	from schema $id: http://devicetree.org/schemas/reg.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250818092725.1977105-2-markus.stockhausen@gmx.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: nand: realtek-ecc: Add Realtek external ECC engine support

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on mtd/nand/next]
[also build test ERROR on linus/master v6.17-rc2 next-20250818]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Stockhausen/dt-bindings-mtd-Add-realtek-rtl9301-ecc/20250818-173253
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
patch link:    https://lore.kernel.org/r/20250818092725.1977105-3-markus.stockhausen%40gmx.de
patch subject: [PATCH 2/2] mtd: nand: realtek-ecc: Add Realtek external ECC engine support
config: openrisc-allyesconfig (https://download.01.org/0day-ci/archive/20250819/202508191251.z4mFuzA6-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250819/202508191251.z4mFuzA6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508191251.z4mFuzA6-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/mtd/nand/ecc-realtek.c: In function 'rtl_ecc_wait_for_engine':
>> drivers/mtd/nand/ecc-realtek.c:165:19: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration]
     165 |         all_one = FIELD_GET(RTL_ECC_ALL_ONE, status);
         |                   ^~~~~~~~~


vim +/FIELD_GET +165 drivers/mtd/nand/ecc-realtek.c

   143	
   144	static void rtl_ecc_wait_for_engine(struct rtl_ecc_ctx *ctx)
   145	{
   146		struct rtl_ecc_engine *rtlc = ctx->rtlc;
   147		int ret, status;
   148		bool all_one;
   149	
   150		/*
   151		 * The ECC engine needs 6-8 us to encode/decode a BCH6 syndrome for 512 bytes of data
   152		 * and 6 tag bytes. In case the NAND area has been erased and all data and oob is
   153		 * set to 0xff, decoding takes 30us (reason unknown). Although the engine can trigger
   154		 * interrupts when finished, use active polling for now. 12 us maximum wait time has
   155		 * proven to be a good tradeoff between performance and overhead.
   156		 */
   157	
   158		ret = regmap_read_poll_timeout(rtlc->regmap, RTL_ECC_STATUS, status,
   159					       !(status & RTL_ECC_OP_STATUS), 12, 600);
   160		if (ret) {
   161			ctx->rc_status = ret;
   162			return;
   163		}
   164	
 > 165		all_one = FIELD_GET(RTL_ECC_ALL_ONE, status);
   166		ctx->rc_status = FIELD_GET(RTL_ECC_RESULT, status);
   167		ctx->rc_bitflips = FIELD_GET(RTL_ECC_CORR_COUNT, status);
   168	
   169		/* For erased blocks (all bits one) error status can be ignored */
   170		if (ctx->rc_status && all_one)
   171			ctx->rc_status = 0;
   172	}
   173	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: nand: realtek-ecc: Add Realtek external ECC engine support

[Miquel Raynal]

Hi Markus,

On 18/08/2025 at 05:27:25 -04, Markus Stockhausen <markus.stockhausen@gmx.de> wrote:

> The Realtek RTl93xx switch SoC series has a built in ECC controller
> that can provide BCH6 or BCH12 over 512 data and 6 tag bytes. It
> generates 10 (BCH6) or 20 (BCH12) bytes of parity.
>
> This engine will most likely work in conjunction with the Realtek
> spi-mem based NAND controller but can work on its own. Therefore
> the initial implementation will be of type external.
>
> Remark! The engine can support any data blocks that are multiples
> of 512 bytes. For now limit it to data+oob layouts that have been
> analyzed from existing devices. This way it keeps compatibility
> and pre-existing vendor data can be read.
>
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
> ---

Very happy to see a new external ECC driver!

>  drivers/mtd/nand/Kconfig       |   8 +
>  drivers/mtd/nand/Makefile      |   1 +
>  drivers/mtd/nand/ecc-realtek.c | 433 +++++++++++++++++++++++++++++++++
>  3 files changed, 442 insertions(+)
>  create mode 100644 drivers/mtd/nand/ecc-realtek.c
>
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b0c2c95f10c..4a17271076bc 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -61,6 +61,14 @@ config MTD_NAND_ECC_MEDIATEK
>  	help
>  	  This enables support for the hardware ECC engine from Mediatek.
>  
> +config MTD_NAND_ECC_REALTEK
> +        tristate "Realtek RTL93xx hardware ECC engine"
> +        depends on HAS_IOMEM
> +        depends on MACH_REALTEK_RTL || COMPILE_TEST
> +        select MTD_NAND_ECC
> +        help
> +          This enables support for the hardware ECC engine from Realtek.
> +
>  endmenu
>  
>  endmenu
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 44913ff1bf12..2e0e56267718 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -3,6 +3,7 @@
>  nandcore-objs := core.o bbt.o
>  obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
>  obj-$(CONFIG_MTD_NAND_ECC_MEDIATEK) += ecc-mtk.o
> +obj-$(CONFIG_MTD_NAND_ECC_REALTEK) += ecc-realtek.o
>  obj-$(CONFIG_SPI_QPIC_SNAND) += qpic_common.o
>  obj-$(CONFIG_MTD_NAND_QCOM) += qpic_common.o
>  obj-y	+= onenand/
> diff --git a/drivers/mtd/nand/ecc-realtek.c b/drivers/mtd/nand/ecc-realtek.c
> new file mode 100644
> index 000000000000..a9c2a3a23b78
> --- /dev/null
> +++ b/drivers/mtd/nand/ecc-realtek.c
> @@ -0,0 +1,433 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Support for Realtek hardware ECC engine in RTL93xx SoCs
> + */
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +/*
> + * The Realtek ECC engine has two operation modes.
> + *
> + * - BCH6 : Generate 10 ECC bytes from 512 data bytes plus 6 tag bytes
> + * - BCH12: Generate 20 ECC bytes from 512 data bytes plus 6 tag
>  bytes

What is a tag? I believe it is Realtek wording and we need to find a
more generic translation.

> + *
> + * It can run for arbitrary NAND flash chips with different block and OOB sizes. Currently there
> + * are only two known devices in the wild that have NAND flash and make use of this ECC engine
> + * (Linksys LGS328C & LGS352C). To keep compatibility with vendor firmware, new modes can only
> + * be added when new data layouts have been analyzed. For now allow BCH6 on flash with 2048 byte
> + * blocks and 64 bytes oob.
> + */
> +
> +#define RTL_ECC_ALLOWED_PAGE_SIZE 	2048
> +#define RTL_ECC_ALLOWED_OOB_SIZE	64
> +#define RTL_ECC_ALLOWED_STRENGTH	6
> +
> +#define RTL_ECC_BLOCK_SIZE 		512
> +#define RTL_ECC_TAG_SIZE 		6
> +#define RTL_ECC_PARITY_SIZE_BCH6	10
> +#define RTL_ECC_PARITY_SIZE_BCH12	20
> +
> +/*
> + * The engine is fed with two DMA regions. One for data (always 512 bytes) and one for tag and
> + * parity (either 16 bytes for BCH6 or 26 bytes for BCH12). Start and length of each must be
> + * aligned to a multiple of 4.
> + */
> +
> +#define RTL_ECC_DMA_TAG_PARITY_SIZE	ALIGN(RTL_ECC_TAG_SIZE + RTL_ECC_PARITY_SIZE_BCH12, 4)
> +#define RTL_ECC_DMA_SIZE		(RTL_ECC_BLOCK_SIZE + RTL_ECC_DMA_TAG_PARITY_SIZE)
> +
> +#define RTL_ECC_CFG			0x00
> +#define   RTL_ECC_BCH6			0
> +#define   RTL_ECC_BCH12			BIT(28)
> +#define   RTL_ECC_DMA_PRECISE		BIT(12)
> +#define   RTL_ECC_BURST_128		GENMASK(1, 0)
> +#define RTL_ECC_DMA_TRIGGER 		0x08
> +#define   RTL_ECC_OP_DECODE		0
> +#define   RTL_ECC_OP_ENCODE		BIT(0)
> +#define RTL_ECC_DMA_START		0x0c
> +#define RTL_ECC_DMA_TAG			0x10
> +#define RTL_ECC_STATUS			0x14
> +#define   RTL_ECC_CORR_COUNT		GENMASK(19, 12)
> +#define   RTL_ECC_RESULT		BIT(8)
> +#define   RTL_ECC_ALL_ONE		BIT(4)
> +#define   RTL_ECC_OP_STATUS		BIT(0)
> +
> +struct rtl_ecc_engine {
> +	struct device *dev;
> +	struct nand_ecc_engine engine;
> +	struct mutex lock;
> +	char *buf;
> +	dma_addr_t buf_dma;
> +	struct regmap *regmap;
> +};
> +
> +struct rtl_ecc_ctx {
> +	struct rtl_ecc_engine * rtlc;
> +	struct nand_ecc_req_tweak_ctx req_ctx;
> +	int steps;
> +	int bch_mode;
> +	int parity_size;
> +	int rc_bitflips;
> +	int rc_status;
> +};
> +
> +static const struct regmap_config rtl_ecc_regmap_config = {
> +	.reg_bits	= 32,
> +	.val_bits	= 32,
> +	.reg_stride	= 4,
> +};
> +
> +static inline void *nand_to_ctx(struct nand_device *nand)
> +{
> +	return nand->ecc.ctx.priv;
> +}
> +
> +static inline struct rtl_ecc_engine *nand_to_rtlc(struct nand_device *nand)
> +{
> +	struct nand_ecc_engine *eng = nand->ecc.engine;
> +
> +	return container_of(eng, struct rtl_ecc_engine, engine);
> +}
> +
> +static int rtl_ecc_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				 struct mtd_oob_region *oobregion)
> +{
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
> +
> +	if (section < 0 || section >= ctx->steps)
> +		return -ERANGE;
> +
> +	oobregion->offset = ctx->steps * RTL_ECC_TAG_SIZE + section * ctx->parity_size;
> +	oobregion->length = ctx->parity_size;

Please reserve 2 bytes for BBM.

For me it looks like you have a bit free section and a bit ECC
section. If that simplifies your life, you can just return a single
section for both.

> +
> +	return 0;
> +}
> +
> +static int rtl_ecc_ooblayout_free(struct mtd_info *mtd, int section,
> +				struct mtd_oob_region *oobregion)
> +{
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
> +
> +	if (section < 0 || section >= ctx->steps)
> +		return -ERANGE;
> +
> +	/* Tags can hold arbitrary values, so they are free for user data */
> +	oobregion->offset = section * RTL_ECC_TAG_SIZE;
> +	oobregion->length = RTL_ECC_TAG_SIZE;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops rtl_ecc_ooblayout_ops = {
> +	.ecc = rtl_ecc_ooblayout_ecc,
> +	.free = rtl_ecc_ooblayout_free,
> +};
> +
> +static void rtl_ecc_kick_engine(struct rtl_ecc_ctx *ctx, int operation)
> +{
> +	struct rtl_ecc_engine *rtlc = ctx->rtlc;
> +
> +	regmap_write(rtlc->regmap, RTL_ECC_CFG,
> +		     ctx->bch_mode | RTL_ECC_BURST_128 | RTL_ECC_DMA_PRECISE);
> +
> +	regmap_write(rtlc->regmap, RTL_ECC_DMA_START, rtlc->buf_dma);
> +	regmap_write(rtlc->regmap, RTL_ECC_DMA_TAG, rtlc->buf_dma + RTL_ECC_BLOCK_SIZE);
> +	regmap_write(rtlc->regmap, RTL_ECC_DMA_TRIGGER, operation);
> +}
> +
> +static void rtl_ecc_wait_for_engine(struct rtl_ecc_ctx *ctx)
> +{
> +	struct rtl_ecc_engine *rtlc = ctx->rtlc;
> +	int ret, status;
> +	bool all_one;
> +
> +	/*
> +	 * The ECC engine needs 6-8 us to encode/decode a BCH6 syndrome for 512 bytes of data
> +	 * and 6 tag bytes. In case the NAND area has been erased and all data and oob is
> +	 * set to 0xff, decoding takes 30us (reason unknown). Although the engine can trigger
> +	 * interrupts when finished, use active polling for now. 12 us maximum wait time has
> +	 * proven to be a good tradeoff between performance and overhead.
> +	 */
> +
> +	ret = regmap_read_poll_timeout(rtlc->regmap, RTL_ECC_STATUS, status,
> +				       !(status & RTL_ECC_OP_STATUS), 12, 600);
> +	if (ret) {
> +		ctx->rc_status = ret;
> +		return;

Please make this function return an int and use it in the caller instead
of using an intermediate status variable for that.

> +	}
> +
> +	all_one = FIELD_GET(RTL_ECC_ALL_ONE, status);
> +	ctx->rc_status = FIELD_GET(RTL_ECC_RESULT, status);
> +	ctx->rc_bitflips = FIELD_GET(RTL_ECC_CORR_COUNT, status);
> +
> +	/* For erased blocks (all bits one) error status can be ignored */
> +	if (ctx->rc_status && all_one)
> +		ctx->rc_status = 0;
> +}
> +
> +static void rtl_ecc_run_engine(struct rtl_ecc_ctx *ctx, char *data, char *tag,
> +			       char *parity, int operation)
> +{
> +	struct rtl_ecc_engine *rtlc = ctx->rtlc;
> +	char *buf_parity = rtlc->buf + RTL_ECC_BLOCK_SIZE + RTL_ECC_TAG_SIZE;
> +	char *buf_tag = rtlc->buf + RTL_ECC_BLOCK_SIZE;
> +	char *buf_data = rtlc->buf;
> +
> +	mutex_lock(&rtlc->lock);
> +
> +	memcpy(buf_data, data, RTL_ECC_BLOCK_SIZE);
> +	memcpy(buf_tag, tag, RTL_ECC_TAG_SIZE);
> +	memcpy(buf_parity, parity, ctx->parity_size);
> +
> +	dma_sync_single_for_device(rtlc->dev, rtlc->buf_dma, RTL_ECC_DMA_SIZE, DMA_TO_DEVICE);
> +	rtl_ecc_kick_engine(ctx, operation);
> +	rtl_ecc_wait_for_engine(ctx);
> +	dma_sync_single_for_cpu(rtlc->dev, rtlc->buf_dma, RTL_ECC_DMA_SIZE, DMA_FROM_DEVICE);
> +
> +	if (!ctx->rc_status) {
> +		memcpy(data, buf_data, RTL_ECC_BLOCK_SIZE);
> +		memcpy(tag, buf_tag, RTL_ECC_TAG_SIZE);
> +		memcpy(parity, buf_parity, ctx->parity_size);
> +	}
> +
> +	mutex_unlock(&rtlc->lock);
> +}
> +
> +static int rtl_ecc_prepare_io_req(struct nand_device *nand, struct nand_page_io_req *req)
> +{
> +	struct rtl_ecc_engine *rtlc = nand_to_rtlc(nand);
> +	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
> +	char *data, *tag, *parity;
> +	int failure = 0;
> +
> +	if (req->mode == MTD_OPS_RAW)
> +		return 0;
> +
> +	nand_ecc_tweak_req(&ctx->req_ctx, req);
> +
> +	if (req->type == NAND_PAGE_READ)
> +		return 0;
> +
> +	tag = req->oobbuf.in;
> +	data = req->databuf.in;
> +	parity = req->oobbuf.in + ctx->steps * RTL_ECC_TAG_SIZE;
> +
> +	for (int i = 0; i < ctx->steps; i++) {
> +		rtl_ecc_run_engine(ctx, data, tag, parity, RTL_ECC_OP_ENCODE);
> +		failure |= ctx->rc_status;
> +
> +		tag += RTL_ECC_TAG_SIZE;
> +		data += RTL_ECC_BLOCK_SIZE;
> +		parity += ctx->parity_size;
> +	}
> +
> +	if (unlikely(failure))
> +		dev_err(rtlc->dev, "ECC calculation failed\n");

This is not helpful as the other layers will complain if that's
relevant. For instance in case you use this driver with a NAND with read
retry support, it would be very noisy. Please turn this into a dev_dbg
call at most.

> +
> +	return failure ? -EBADMSG : 0;
> +}
> +
> +static int rtl_ecc_finish_io_req(struct nand_device *nand, struct nand_page_io_req *req)
> +{
> +	struct rtl_ecc_engine *rtlc = nand_to_rtlc(nand);
> +	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
> +	struct mtd_info *mtd = nanddev_to_mtd(nand);
> +	char *data, *tag, *parity;
> +	unsigned int bitflips = 0;
> +	int failure = 0;
> +
> +	if (req->mode == MTD_OPS_RAW)
> +		return 0;
> +
> +	if (req->type == NAND_PAGE_WRITE) {
> +		nand_ecc_restore_req(&ctx->req_ctx, req);
> +		return 0;
> +	}
> +
> +	tag = req->oobbuf.in;
> +	data = req->databuf.in;
> +	parity = req->oobbuf.in + ctx->steps * RTL_ECC_TAG_SIZE;
> +
> +	for (int i = 0 ; i < ctx->steps; i++) {
> +		rtl_ecc_run_engine(ctx, data, tag, parity, RTL_ECC_OP_DECODE);
> +		failure |= ctx->rc_status;
> +		mtd->ecc_stats.failed += !!ctx->rc_status;
> +		bitflips = max_t(unsigned int, ctx->rc_bitflips, bitflips);
> +
> +		tag += RTL_ECC_TAG_SIZE;
> +		data += RTL_ECC_BLOCK_SIZE;
> +		parity += ctx->parity_size;
> +	}
> +
> +	nand_ecc_restore_req(&ctx->req_ctx, req);
> +
> +	if (unlikely(failure))
> +		dev_err(rtlc->dev, "ECC correction failed\n");
> +	else if (unlikely(bitflips > 2))
> +		dev_warn(rtlc->dev, "%d bitflips detected\n", ctx->rc_bitflips);

Same here. Way too noisy for something (a bitflip) that happens normally.

> +
> +	return failure ? -EBADMSG : bitflips;
> +}
> +
> +static int rtl_ecc_check_support(struct nand_device *nand)
> +{
> +	struct mtd_info *mtd = nanddev_to_mtd(nand);
> +	struct device *dev = nand->ecc.engine->dev;
> +
> +	if (mtd->oobsize != RTL_ECC_ALLOWED_OOB_SIZE ||
> +	    mtd->writesize != RTL_ECC_ALLOWED_PAGE_SIZE) {
> +		dev_err(dev, "only flash geometry data=%d, oob=%d supported\n",
> +			RTL_ECC_ALLOWED_PAGE_SIZE, RTL_ECC_ALLOWED_OOB_SIZE);
> +		return -EINVAL;
> +	}
> +
> +	if (nand->ecc.user_conf.algo != NAND_ECC_ALGO_BCH ||
> +	    nand->ecc.user_conf.strength != RTL_ECC_ALLOWED_STRENGTH ||
> +	    nand->ecc.user_conf.placement != NAND_ECC_PLACEMENT_OOB ||
> +	    nand->ecc.user_conf.step_size != RTL_ECC_BLOCK_SIZE) {
> +		dev_err(dev, "only algo=bch, strength=%d, placement=oob, step=%d supported\n",
> +			RTL_ECC_ALLOWED_STRENGTH, RTL_ECC_BLOCK_SIZE);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rtl_ecc_init_ctx(struct nand_device *nand)
> +{
> +	struct nand_ecc_props *conf = &nand->ecc.ctx.conf;
> +	struct rtl_ecc_engine *rtlc = nand_to_rtlc(nand);
> +	struct mtd_info *mtd = nanddev_to_mtd(nand);
> +	int strength = nand->ecc.user_conf.strength;
> +	struct device *dev = nand->ecc.engine->dev;
> +	struct rtl_ecc_ctx *ctx;
> +	int ret;
> +
> +	ret = rtl_ecc_check_support(nand);
> +	if (ret)
> +		return ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	nand->ecc.ctx.priv = ctx;
> +	mtd_set_ooblayout(mtd, &rtl_ecc_ooblayout_ops);
> +
> +	conf->algo = NAND_ECC_ALGO_BCH;
> +	conf->strength = strength;
> +	conf->step_size = RTL_ECC_BLOCK_SIZE;
> +	conf->engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
> +
> +	ctx->rtlc = rtlc;
> +	ctx->steps = mtd->writesize / RTL_ECC_BLOCK_SIZE;
> +	ctx->bch_mode = strength == 6 ? RTL_ECC_BCH6 : RTL_ECC_BCH12;
> +	ctx->parity_size = strength == 6 ? RTL_ECC_PARITY_SIZE_BCH6 : RTL_ECC_PARITY_SIZE_BCH12;
> +
> +	ret = nand_ecc_init_req_tweaking(&ctx->req_ctx, nand);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "using bch%d with geometry data=%dx%d, tag=%dx6, parity=%dx%d",
> +		 conf->strength, ctx->steps, conf->step_size,
> +		 ctx->steps, ctx->steps, ctx->parity_size);

Same, dev_dbg is fine, but otherwise no need.

> +
> +	return 0;
> +}
> +
> +static void rtl_ecc_cleanup_ctx(struct nand_device *nand)
> +{
> +	struct rtl_ecc_ctx *ctx = nand_to_ctx(nand);
> +
> +	if (ctx)
> +		nand_ecc_cleanup_req_tweaking(&ctx->req_ctx);
> +}
> +
> +static struct nand_ecc_engine_ops rtl_ecc_engine_ops = {
> +	.init_ctx = rtl_ecc_init_ctx,
> +	.cleanup_ctx = rtl_ecc_cleanup_ctx,
> +	.prepare_io_req = rtl_ecc_prepare_io_req,
> +	.finish_io_req = rtl_ecc_finish_io_req,
> +};
> +
> +static int rtl_ecc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rtl_ecc_engine *rtlc;
> +	void __iomem *base;
> +	int ret;
> +
> +	rtlc = devm_kzalloc(dev, sizeof(*rtlc), GFP_KERNEL);
> +	if (!rtlc)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	ret = devm_mutex_init(dev, &rtlc->lock);
> +	if (ret)
> +		return ret;
> +
> +	rtlc->regmap = devm_regmap_init_mmio(dev, base, &rtl_ecc_regmap_config);
> +	if (IS_ERR(rtlc->regmap))
> +		return PTR_ERR(rtlc->regmap);
> +
> +	/*
> +	 * Focus on simplicity and use a preallocated DMA buffer for data exchange with the
> +	 * engine. Make it a noncoherent memory model as invalidating/flushing caches is
> +	 * faster than reading/writing uncached memory.
> +	 */

I guess it depends on the architecture, but that's fine.

> +
> +	rtlc->buf = dma_alloc_noncoherent(dev, RTL_ECC_DMA_SIZE, &rtlc->buf_dma,
> +					  DMA_BIDIRECTIONAL, GFP_KERNEL);
> +	if (IS_ERR(rtlc->buf))
> +		return PTR_ERR(rtlc->buf);
> +
> +	rtlc->dev = dev;
> +	rtlc->engine.dev = dev;
> +	rtlc->engine.ops = &rtl_ecc_engine_ops;
> +	rtlc->engine.integration = NAND_ECC_ENGINE_INTEGRATION_EXTERNAL;
> +
> +	nand_ecc_register_on_host_hw_engine(&rtlc->engine);
> +
> +	platform_set_drvdata(pdev, rtlc);
> +
> +	return 0;
> +}
> +
> +static void rtl_ecc_remove(struct platform_device *pdev)
> +{
> +	struct rtl_ecc_engine *rtlc = platform_get_drvdata(pdev);
> +
> +	nand_ecc_unregister_on_host_hw_engine(&rtlc->engine);
> +	dma_free_noncoherent(rtlc->dev, RTL_ECC_DMA_SIZE, rtlc->buf, rtlc->buf_dma,
> +			     DMA_BIDIRECTIONAL);
> +}
> +
> +static const struct of_device_id rtl_ecc_of_ids[] = {
> +	{
> +		.compatible = "realtek,rtl9301-ecc",
> +	},
> +	{ /* sentinel */ },
> +};
> +
> +static struct platform_driver rtl_ecc_driver = {
> +	.driver	= {
> +		.name = "rtl-nand-ecc-engine",
> +		.of_match_table = rtl_ecc_of_ids,
> +	},
> +	.probe = rtl_ecc_probe,
> +	.remove = rtl_ecc_remove,
> +};
> +module_platform_driver(rtl_ecc_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Markus Stockhausen <markus.stockhausen@gmx.de>");
> +MODULE_DESCRIPTION("Realtek NAND hardware ECC controller");

Great work nevertheless! Looking forward to get it upstreamed.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

AW: [PATCH 2/2] mtd: nand: realtek-ecc: Add Realtek external ECC engine support

[markus.stockhausen]

> Von: Miquel Raynal <miquel.raynal@bootlin.com> 
> Gesendet: Sonntag, 24. August 2025 18:51
> ...
> +/*
> + * The Realtek ECC engine has two operation modes.
> + *
> + * - BCH6 : Generate 10 ECC bytes from 512 data bytes plus 6 tag bytes
> + * - BCH12: Generate 20 ECC bytes from 512 data bytes plus 6 tag bytes
>
> What is a tag? I believe it is Realtek wording and we need to find a
> more generic translation.

I have no real idea. They simply call it "tag". To be more precise

- tag (lowercase): means 6 oob bytes for one 512 byte block
- syndrome: means 10 ECC bytes for one 512 byte block.
- TAG (uppercase): means 16 oob bytes (tag+syndrome)

Even the register is named ECDTAR (ECC Controller DMA TAG 
address register). For me tag (lowercase) is simply additional 
input data that can contain arbitrary content. It can be used
for extended/auxiliary/meta data or simply padding I assume.

Looking at the data on my devices I see that tags are always 
filled with 0xff. So it seems as this gets populated after erasing
data and is never overwritten afterwards.

What naming do you advise?

> > +	oobregion->offset = ctx->steps * RTL_ECC_TAG_SIZE + section * ctx->parity_size;
> > +	oobregion->length = ctx->parity_size;
>
> Please reserve 2 bytes for BBM.
>
> For me it looks like you have a bit free section and a bit ECC
> section. If that simplifies your life, you can just return a single
> section for both.

The first two "tag" bytes seem to be designated for BBM according
to my information. Will adapt the first oob region offset/size.

Do I need to declare the BBM area somehow additionally?

Thanks for the other feedback. Will implement that in v2.

Markus


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: AW: [PATCH 2/2] mtd: nand: realtek-ecc: Add Realtek external ECC engine support

[Miquel Raynal]

Hi Markus,

On 24/08/2025 at 21:16:39 +02, <markus.stockhausen@gmx.de> wrote:

>> Von: Miquel Raynal <miquel.raynal@bootlin.com> 
>> Gesendet: Sonntag, 24. August 2025 18:51
>> ...
>> +/*
>> + * The Realtek ECC engine has two operation modes.
>> + *
>> + * - BCH6 : Generate 10 ECC bytes from 512 data bytes plus 6 tag bytes
>> + * - BCH12: Generate 20 ECC bytes from 512 data bytes plus 6 tag bytes
>>
>> What is a tag? I believe it is Realtek wording and we need to find a
>> more generic translation.
>
> I have no real idea. They simply call it "tag". To be more precise
>
> - tag (lowercase): means 6 oob bytes for one 512 byte block
> - syndrome: means 10 ECC bytes for one 512 byte block.
> - TAG (uppercase): means 16 oob bytes (tag+syndrome)

Ok. Based on your feedback, I believe that might be what we more
commonly call "free bytes". "free" as in "available for the user". In the
OOB area you'll mostly find:
- 2 bytes reserved by MTD to store bad block markers
- Bytes available for the user (end user, or more commonly upper layers
  and filesystems. jffs2 does use these free bytes, whereas UBI does not.
    * your "tag" and "TAG", difference is probably that "tag" is not ECC
      protected (subject to bitflip) whereas "TAG" is part of the
      protected area and will be corrected if needed.
- Bytes not available for the user, ie. ECC bytes, which we are being
  read/written at a fixed offset by the ECC engine.

> Do I need to declare the BBM area somehow additionally?

No, they are not user accessible nor ECC bytes. They are not exposed by
the ooblayout callbacks we are discussing.

> Thanks for the other feedback. Will implement that in v2.

Cheers!

Good luck,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/