Re: [RFC PATCH 1/5] mtd: nand: ecc-qcom: Add support for ECC Engine Driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/3/2023 6:03 PM, Dmitry Baryshkov wrote:
On Fri, 3 Nov 2023 at 14:25, Md Sadre Alam <quic_mdalam@xxxxxxxxxxx> wrote:



On 10/31/2023 10:41 PM, Krzysztof Kozlowski wrote:
On 31/10/2023 13:03, Md Sadre Alam wrote:

Eh? Empty?

QPIC controller has the ecc pipelined so will keep the ecc support
inlined in both raw nand and serial nand driver.

Droping this driver since device node was NAK-ed
https://www.spinics.net/lists/linux-arm-msm/msg177596.html

It seems, we have to repeat the same thing again and again:

It was not the device node that was NAKed. It was the patch that was
NAKed. Please read the emails carefully.

And next time please perform dtbs_check, dt_binding_check and
checkpatch before sending the patch.

It is perfectly fine to ask questions 'like we are getting we are
getting this and that issues with the bindings, please advise'. It is
not fine to skip that step completely.

Sorry in V1 will run all basic checks.

Based on below feedback [1] and NAK on the device node patch
got idea of having separate device node for ECC is not acceptable.
Could you please help to clarify that.

Since ECC block is inlined with QPIC controller so is the below
device node acceptable ?

   bch: qpic_ecc {
                          compatible = "qcom,ipq9574-ecc";
                          status = "ok";
                  };

 [1] https://www.spinics.net/lists/linux-arm-msm/msg177525.html



Signed-off-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx>
Signed-off-by: Sricharan R <quic_srichara@xxxxxxxxxxx>
---
   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


...

+
+    return 0;
+}
+EXPORT_SYMBOL(qcom_ecc_config);
+
+void qcom_ecc_enable(struct qcom_ecc *ecc)
+{
+    ecc->use_ecc = true;
+}
+EXPORT_SYMBOL(qcom_ecc_enable);

Drop this and all other exports. Nothing here explains the need for them.

+
+void qcom_ecc_disable(struct qcom_ecc *ecc)
+{
+    ecc->use_ecc = false;
+}
+EXPORT_SYMBOL(qcom_ecc_disable);
+
+static const struct of_device_id qpic_ecc_dt_match[] = {
+    {
+            .compatible = "qcom,ipq9574-ecc",

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.

Checkpatch is preerquisite. Don't send patches which have obvious issues
pointed out by checkpatch. It's a waste of reviewers time.

+    },
+    {},
+};
+
+static int qpic_ecc_probe(struct platform_device *pdev)
+{
+    struct device *dev = &pdev->dev;
+    struct qpic_ecc *ecc;
+    u32 max_eccdata_size;
+
+    ecc = devm_kzalloc(dev, sizeof(*ecc), GFP_KERNEL);
+    if (!ecc)
+            return -ENOMEM;
+
+    ecc->caps = of_device_get_match_data(dev);
+
+    ecc->dev = dev;
+    platform_set_drvdata(pdev, ecc);
+    dev_info(dev, "probed\n");

No, no such messages.



Best regards,
Krzysztof







[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux