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

[Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>]

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