On 10/31/2023 8:58 PM, Miquel Raynal wrote:
Hi,
quic_mdalam@xxxxxxxxxxx wrote on Tue, 31 Oct 2023 17:33:03 +0530:
Commit log is missing.
Having a separate device node for ECC was NAK-ed
https://www.spinics.net/lists/linux-arm-msm/msg177596.html
It is fine to drop this patch ? keep ECC support inlined in both
raw nand and Serial nand driver.
Signed-off-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx>
Signed-off-by: Sricharan R <quic_srichara@xxxxxxxxxxx>
If Sricharan is a co developer you need to use the right tags. Please
have a look at the documentation. Using the two SoB here does not mean
anything
Ok will fix
---
drivers/mtd/nand/Kconfig | 7 ++
drivers/mtd/nand/Makefile | 1 +
drivers/mtd/nand/ecc-qcom.c | 198 ++++++++++++++++++++++++++++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/mtd/nand/ecc-qcom.c
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b0c2c95f10c..333cec8187c8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -61,6 +61,13 @@ config MTD_NAND_ECC_MEDIATEK
help
This enables support for the hardware ECC engine from Mediatek.
+config MTD_NAND_ECC_QCOM
+ tristate "Qualcomm hardware ECC engine"
+ depends on ARCH_QCOM
Same comment as Mark regarding COMPILE_TEST
Ok
+ select MTD_NAND_ECC
+ help
+ This enables support for the hardware ECC engine from Qualcomm.
+
endmenu
endmenu
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 19e1291ac4d5..c73b8a3456ec 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_QCOM) += ecc-qcom.o qpic_common.o
obj-y += onenand/
obj-y += raw/
diff --git a/drivers/mtd/nand/ecc-qcom.c b/drivers/mtd/nand/ecc-qcom.c
new file mode 100644
index 000000000000..a85423ed368a
--- /dev/null
+++ b/drivers/mtd/nand/ecc-qcom.c
@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * QCOM ECC Engine Driver.
+ * Copyright (C) 2023 Qualcomm Inc.
+ * Authors: Md sadre Alam <quic_mdalam@xxxxxxxxxxx>
+ * Sricharan R <quic_srichara@xxxxxxxxxxx>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/iopoll.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/mutex.h>
+#include <linux/mtd/nand-qpic-common.h>
+
+
+
+/* ECC modes supported by the controller */
+#define ECC_NONE BIT(0)
+#define ECC_RS_4BIT BIT(1)
+#define ECC_BCH_4BIT BIT(2)
+#define ECC_BCH_8BIT BIT(3)
+
+struct qpic_ecc_caps {
+ u32 err_mask;
+ u32 err_shift;
+ const u8 *ecc_strength;
+ const u32 *ecc_regs;
+ u8 num_ecc_strength;
+ u8 ecc_mode_shift;
+ u32 parity_bits;
+ int pg_irq_sel;
+};
+
+
+struct qcom_nand_host *to_qcom_nand_host(struct nand_chip *chip)
+{
+ return container_of(chip, struct qcom_nand_host, chip);
+}
+EXPORT_SYMBOL(to_qcom_nand_host);
+
+struct qcom_nand_controller *
+get_qcom_nand_controller(struct nand_chip *chip)
+{
+ return container_of(chip->controller, struct qcom_nand_controller,
+ controller);
+}
+EXPORT_SYMBOL(get_qcom_nand_controller);
+
+static struct qpic_ecc *qpic_ecc_get(struct device_node *np)
+{
+ struct platform_device *pdev;
+ struct qpic_ecc *ecc;
+
+ pdev = of_find_device_by_node(np);
+ if (!pdev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ ecc = platform_get_drvdata(pdev);
+ if (!ecc) {
+ put_device(&pdev->dev);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ return ecc;
+}
+
+struct qpic_ecc *of_qpic_ecc_get(struct device_node *of_node)
+{
+ struct qpic_ecc *ecc = NULL;
+ struct device_node *np;
+
+ np = of_parse_phandle(of_node, "nand-ecc-engine", 0);
+ /* for backward compatibility */
There is no backward compatibility to handle upstream
Ok will fix in V1
+ if (!np)
+ np = of_parse_phandle(of_node, "ecc-engine", 0);
+ if (np) {
+ ecc = qpic_ecc_get(np);
+ of_node_put(np);
+ }
+
+ return ecc;
+}
+EXPORT_SYMBOL(of_qpic_ecc_get);
+
+int qcom_ecc_config(struct qpic_ecc *ecc, int ecc_strength,
+ bool wide_bus)
+{
+ ecc->ecc_modes = (ECC_RS_4BIT | ECC_BCH_8BIT);
+
+ if (ecc_strength >= 8) {
If your engine does not support more than an 8-bit strength this
condition seems a bit strange.
Max ECC supported 8-bit only, forcing it to 8-bit.
+ /* 8 bit ECC defaults to BCH ECC on all platforms */
+ ecc->bch_enabled = true;
+ ecc->ecc_mode = 1;
ecc_modes above, ecc_mode here, not very clear what this is.
Please give meaningful names to your variables, not just the bit name
that this is capturing because here it's unclear what this is.
ok will fix in V1
+
+ if (wide_bus) {
+ ecc->ecc_bytes_hw = 14;
+ ecc->spare_bytes = 0;
Spare bytes depend on the flash, you can't use constant values like
that.
Ok will fix in V1
I also don't understand what wide_bus is and why it has an impact of
only 1 on the number of ECC bytes. Please make all this more explicit.
wide_bus 1 means 16-bit wide and wide_bus 0 means 8-bit wide.
there different configuration for ecc config for 16-bit wide bus
and 8-bit wide bus. This is recommended configuration by IP team,
Will reconfirm this with IP folks and come back.
Regards,
Alam.
