Re: [PATCH v2] mtd: fsl_upm: Support NAND ECC DTS properties

On 11/08/2014 04:11 PM, Aaron Sierra wrote:
[..]

+
+	/* We know mode is either NAND_ECC_SOFT or NAND_ECC_SOFT_BCH */
+	if (strength < 0 && mode == NAND_ECC_SOFT_BCH) {
+		dev_err(fun->dev,
+			"ECC BCH mode requires nand-ecc-strength property");
+		ret = -EINVAL;
+		goto err;
+	} else if (strength == 0) {
+		dev_err(fun->dev, "ECC strength of 0 bits is unsupported");
+		ret = -EINVAL;
+		goto err;
+	} else if (strength == 1 && mode == NAND_ECC_SOFT_BCH) {
+		dev_err(fun->dev, "ECC BCH mode requires > 1-bit strength");
+		ret = -EINVAL;
+		goto err;
+	} else if (strength > 1 && mode == NAND_ECC_SOFT) {
+		dev_warn(fun->dev,
+			"Forcing ECC BCH due to %d-bit strength\n", strength);
+		mode = NAND_ECC_SOFT_BCH;
+	}
+	fun->chip.ecc.mode = mode;
+	fun->chip.ecc.strength = strength;
+

Aside from my comment about the lack of ECC specification in the
binding, I think the above is wrong.

You don't have hardware ECC, but software ECC (either hamming or BCH).
So, you don't need to specify any nand_ecc_ctrl.strength (i.e.
ecc.strength above).

It'll be set by the NAND core and override any value you set
See nand_scan_tail.

So, I'd say you just need to specify the nand-ecc-mode in the devicetree
binding.

The nand-ecc-strength and nand-ecc-step-size are meant for controllers
with hardware ECC support.
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html