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

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

[Aaron Sierra]

From: Jordan Friendshuh <jfriendshuh@xes-inc.com>

Support the generic nand-ecc-mode and nand-ecc-strength device-tree
properties with the Freescale UPM NAND driver.

This patch preserves the default software ECC mode while adding the
ability to use BCH ECC for larger NAND devices.

Signed-off-by: Jordan Friendshuh <jfriendshuh@xes-inc.com>
Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
---
v2:
    * Now using ECC mode and strength helpers from of_mtd.h
    * ECC mode and strength checking is more robust

 .../devicetree/bindings/mtd/fsl-upm-nand.txt       |  2 +
 drivers/mtd/nand/Kconfig                           |  1 +
 drivers/mtd/nand/fsl_upm.c                         | 51 +++++++++++++++++++---
 3 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
index fce4894..a9906f6 100644
--- a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
@@ -18,6 +18,8 @@ Optional properties:
 - chip-delay : chip dependent delay for transferring data from array to
 	read registers (tR). Required if property "gpios" is not used
 	(R/B# pins not connected).
+- nand-ecc-mode : as defined by nand.txt ("soft" and "soft_bch", only).
+- nand-ecc-strength : as defined by nand.txt.
 
 Each flash chip described may optionally contain additional sub-nodes
 describing partitions of the address space. See partition.txt for more
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f1cf503..85c0243 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -439,6 +439,7 @@ config MTD_NAND_FSL_UPM
 	tristate "Support for NAND on Freescale UPM"
 	depends on PPC_83xx || PPC_85xx
 	select FSL_LBC
+	select MTD_NAND_ECC_BCH
 	help
 	  Enables support for NAND Flash chips wired onto Freescale PowerPC
 	  processor localbus with User-Programmable Machine support.
diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
index 4d203e8..8f38447 100644
--- a/drivers/mtd/nand/fsl_upm.c
+++ b/drivers/mtd/nand/fsl_upm.c
@@ -18,6 +18,7 @@
 #include <linux/mtd/nand_ecc.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/mtd.h>
+#include <linux/of_mtd.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
 #include <linux/of_gpio.h>
@@ -160,6 +161,11 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 	int ret;
 	struct device_node *flash_np;
 	struct mtd_part_parser_data ppdata;
+	int mode, strength;
+
+	flash_np = of_get_next_child(upm_np, NULL);
+	if (!flash_np)
+		return -ENODEV;
 
 	fun->chip.IO_ADDR_R = fun->io_base;
 	fun->chip.IO_ADDR_W = fun->io_base;
@@ -168,7 +174,46 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 	fun->chip.read_byte = fun_read_byte;
 	fun->chip.read_buf = fun_read_buf;
 	fun->chip.write_buf = fun_write_buf;
-	fun->chip.ecc.mode = NAND_ECC_SOFT;
+
+	/*
+	 * If nand-ecc-strength < 0, require ECC_SOFT, error otherwise.
+	 * If nand-ecc-strength == 0, error.
+	 * If nand-ecc-strength == 1, require ECC_SOFT, error otherwise.
+	 * If nand-ecc-strength > 1, force ECC_SOFT_BCH (warn on mode mismatch).
+	 */
+	mode = of_get_nand_ecc_mode(flash_np);
+	strength = of_get_nand_ecc_strength(flash_np);
+	if (!of_property_read_bool(flash_np, "nand-ecc-mode")) {
+		dev_info(fun->dev, "ECC mode defaulting to 'soft'");
+		mode = NAND_ECC_SOFT;
+	} else if (mode != NAND_ECC_SOFT && mode != NAND_ECC_SOFT_BCH) {
+		dev_err(fun->dev, "ECC mode in device tree is unsupported");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	/* 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;
+
 	if (fun->mchip_count > 1)
 		fun->chip.select_chip = fun_select_chip;
 
@@ -178,10 +223,6 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 	fun->mtd.priv = &fun->chip;
 	fun->mtd.owner = THIS_MODULE;
 
-	flash_np = of_get_next_child(upm_np, NULL);
-	if (!flash_np)
-		return -ENODEV;
-
 	fun->mtd.name = kasprintf(GFP_KERNEL, "0x%llx.%s", (u64)io_res->start,
 				  flash_np->name);
 	if (!fun->mtd.name) {
-- 
1.9.1

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

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

[Ezequiel Garcia]

On 11/08/2014 04:11 PM, Aaron Sierra wrote:
> From: Jordan Friendshuh <jfriendshuh-AQeFf1F/bRxBDgjK7y7TUQ@public.gmane.org>
> 
> Support the generic nand-ecc-mode and nand-ecc-strength device-tree
> properties with the Freescale UPM NAND driver.
> 
> This patch preserves the default software ECC mode while adding the
> ability to use BCH ECC for larger NAND devices.
> 
> Signed-off-by: Jordan Friendshuh <jfriendshuh-AQeFf1F/bRxBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Aaron Sierra <asierra-AQeFf1F/bRxBDgjK7y7TUQ@public.gmane.org>
> ---
> v2:
>     * Now using ECC mode and strength helpers from of_mtd.h
>     * ECC mode and strength checking is more robust
> 
>  .../devicetree/bindings/mtd/fsl-upm-nand.txt       |  2 +
>  drivers/mtd/nand/Kconfig                           |  1 +
>  drivers/mtd/nand/fsl_upm.c                         | 51 +++++++++++++++++++---
>  3 files changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> index fce4894..a9906f6 100644
> --- a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> @@ -18,6 +18,8 @@ Optional properties:
>  - chip-delay : chip dependent delay for transferring data from array to
>  	read registers (tR). Required if property "gpios" is not used
>  	(R/B# pins not connected).
> +- nand-ecc-mode : as defined by nand.txt ("soft" and "soft_bch", only).
> +- nand-ecc-strength : as defined by nand.txt.
>  

The nand.txt recommends that each binding documents the way these
properties are interpreted.

Moreover, there's no nand-ecc-step usage? Are you assuming a 512-byte size?
-- 
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

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

[Aaron Sierra]

----- Original Message -----
> From: "Ezequiel Garcia" <ezequiel@vanguardiasur.com.ar>
> Sent: Saturday, November 22, 2014 6:55:42 PM
> 
> On 11/08/2014 04:11 PM, Aaron Sierra wrote:
> > From: Jordan Friendshuh <jfriendshuh@xes-inc.com>
> > 
> > Support the generic nand-ecc-mode and nand-ecc-strength device-tree
> > properties with the Freescale UPM NAND driver.
> > 
> > This patch preserves the default software ECC mode while adding the
> > ability to use BCH ECC for larger NAND devices.
> > 
> > Signed-off-by: Jordan Friendshuh <jfriendshuh@xes-inc.com>
> > Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> > ---
> > v2:
> >     * Now using ECC mode and strength helpers from of_mtd.h
> >     * ECC mode and strength checking is more robust
> > 
> >  .../devicetree/bindings/mtd/fsl-upm-nand.txt       |  2 +
> >  drivers/mtd/nand/Kconfig                           |  1 +
> >  drivers/mtd/nand/fsl_upm.c                         | 51
> >  +++++++++++++++++++---
> >  3 files changed, 49 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> > b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> > index fce4894..a9906f6 100644
> > --- a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> > @@ -18,6 +18,8 @@ Optional properties:
> >  - chip-delay : chip dependent delay for transferring data from array to
> >  	read registers (tR). Required if property "gpios" is not used
> >  	(R/B# pins not connected).
> > +- nand-ecc-mode : as defined by nand.txt ("soft" and "soft_bch", only).
> > +- nand-ecc-strength : as defined by nand.txt.
> >  
> 
> The nand.txt recommends that each binding documents the way these
> properties are interpreted.

Ezequiel,
I assume you are referring to this statement, "implementations are
encouraged to further specify the value(s) they support."

That is the only statement I found requesting documentation. I believe
that request is satisfied, since I listed all of the modes that the driver
would support with my patch applied. I used mxc-nand.txt as a reference.

> Moreover, there's no nand-ecc-step usage? Are you assuming a 512-byte size?

The ecc.bytes value is currently left to be calculated by the
NAND_ECC_SOFT_BCH case in nand_base.c. That calculation assumes a 512-byte
step size.

-Aaron

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

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

[Ezequiel Garcia]

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

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

[Aaron Sierra]

> From: "Ezequiel Garcia" <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
> Sent: Tuesday, November 25, 2014 2:08:59 PM
> 
> 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.

Ezequiel,
This patch was originally the second of two patches. The first was applied
to l2-mtd.git on 11/5/2014:

    mtd: nand: Base BCH ECC bytes on required strength

It affects the code in nand_scan_tail that you're referring to so that it
looks like this:

		/*
		 * Board driver should supply ecc.size and ecc.bytes values to
		 * select how many bits are correctable; see nand_bch_init()
		 * for details. Otherwise, default to 4 bits for large page
		 * devices.
		 */
		if (!ecc->size && (mtd->oobsize >= 64)) {
			ecc->size = 512;
			ecc->bytes = DIV_ROUND_UP(13 * ecc->strength, 8);
		}
		ecc->priv = nand_bch_init(mtd, ecc->size, ecc->bytes,
					       &ecc->layout);
		if (!ecc->priv) {
			pr_warn("BCH ECC initialization failed!\n");
			BUG();
		}
		ecc->strength = ecc->bytes * 8 / fls(8 * ecc->size);

In this case ecc->strength is unnecessarily recalculated at the end and
ecc->strength isn't checked for zero in the initial calculation. Perhaps
this block in nand_scan_tail should be patched again to preserve the
original behaviour if ecc->strength or ecc->size are zero.

The only other in-kernel user of SOFT_BCH is sunxi_nand.c and it calculates
ecc->bytes from ecc->size and ecc->strength itself, so it has defined all
three values by this point.

> 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.

Software BCH allows controllers without hardware support for multiple
bit correction to be used with NAND devices that require multiple bit
correction, so like a hardware controller it needs to know how many
bits to correct.

> --
> 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

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

[Ezequiel Garcia]

On 11/25/2014 07:19 PM, Aaron Sierra wrote:
>> From: "Ezequiel Garcia" <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
>> Sent: Tuesday, November 25, 2014 2:08:59 PM
>>
>> 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.
> 
> Ezequiel,
> This patch was originally the second of two patches. The first was applied
> to l2-mtd.git on 11/5/2014:
> 
>     mtd: nand: Base BCH ECC bytes on required strength
> 
> It affects the code in nand_scan_tail that you're referring to so that it
> looks like this:
> 
> 		/*
> 		 * Board driver should supply ecc.size and ecc.bytes values to
> 		 * select how many bits are correctable; see nand_bch_init()
> 		 * for details. Otherwise, default to 4 bits for large page
> 		 * devices.
> 		 */
> 		if (!ecc->size && (mtd->oobsize >= 64)) {
> 			ecc->size = 512;
> 			ecc->bytes = DIV_ROUND_UP(13 * ecc->strength, 8);
> 		}
> 		ecc->priv = nand_bch_init(mtd, ecc->size, ecc->bytes,
> 					       &ecc->layout);
> 		if (!ecc->priv) {
> 			pr_warn("BCH ECC initialization failed!\n");
> 			BUG();
> 		}
> 		ecc->strength = ecc->bytes * 8 / fls(8 * ecc->size);
> 
> In this case ecc->strength is unnecessarily recalculated at the end and
> ecc->strength isn't checked for zero in the initial calculation. Perhaps
> this block in nand_scan_tail should be patched again to preserve the
> original behaviour if ecc->strength or ecc->size are zero.
> 

Exactly, I was talking about that snippet. I missed the fact that
ecc.strength is used to get bytes, and then re-calculated. Some comments
are definitely needed there.

> The only other in-kernel user of SOFT_BCH is sunxi_nand.c and it calculates
> ecc->bytes from ecc->size and ecc->strength itself, so it has defined all
> three values by this point.
> 
>> 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.
> 
> Software BCH allows controllers without hardware support for multiple
> bit correction to be used with NAND devices that require multiple bit
> correction, so like a hardware controller it needs to know how many
> bits to correct.
> 

Right, right.

I'd say you should require an ECC step size in your binding, instead of
assuming the 512-byte that gets set if the ecc.size is 0.

BTW, how does this patch deal with old devicetree files?
-- 
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

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

[Aaron Sierra]

----- Original Message -----
> From: "Ezequiel Garcia" <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
> Sent: Tuesday, November 25, 2014 4:42:47 PM
> 
> On 11/25/2014 07:19 PM, Aaron Sierra wrote:
> >> From: "Ezequiel Garcia" <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
> >> Sent: Tuesday, November 25, 2014 2:08:59 PM
> >>
> >> 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;
> >>> +
> >>
> 

[ snip ]

> I'd say you should require an ECC step size in your binding, instead of
> assuming the 512-byte that gets set if the ecc.size is 0.

Yes, I agree that the fsl_upm driver should require that nand-ecc-step-size
and nand-ecc-strength both be defined with "soft_bch".

> BTW, how does this patch deal with old devicetree files?

The fsl_upm driver previously only supported 1-bit "soft" ECC, so _this_
patch expects that existing device trees don't define nand-ecc-strength
or nand-ecc-mode. In that case "soft" ECC will be detected and used.

However, if for some reason an existing device tree defines
nand-ecc-strength, then the snippet that you provided above shows that
the ECC mode would be overridden (forcing "soft_bch" for ECC strength
values greater than 1).

-Aaron
--
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

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

[Brian Norris]

Hi Aaron,

On Sat, Nov 08, 2014 at 01:11:56PM -0600, Aaron Sierra wrote:
> From: Jordan Friendshuh <jfriendshuh-AQeFf1F/bRxBDgjK7y7TUQ@public.gmane.org>
> 
> Support the generic nand-ecc-mode and nand-ecc-strength device-tree
> properties with the Freescale UPM NAND driver.
> 
> This patch preserves the default software ECC mode while adding the
> ability to use BCH ECC for larger NAND devices.
> 
> Signed-off-by: Jordan Friendshuh <jfriendshuh-AQeFf1F/bRxBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Aaron Sierra <asierra-AQeFf1F/bRxBDgjK7y7TUQ@public.gmane.org>
> ---
> v2:
>     * Now using ECC mode and strength helpers from of_mtd.h
>     * ECC mode and strength checking is more robust

Should I be expecting an update? Ezequiel made a few comments, and I'm
not sure if you were planning any action based on them.

Brian
--
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

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

[Aaron Sierra]

----- Original Message -----
> From: "Brian Norris" <computersforpeace@gmail.com>
> Sent: Tuesday, December 16, 2014 6:35:06 PM
> 
> Hi Aaron,
> 
> On Sat, Nov 08, 2014 at 01:11:56PM -0600, Aaron Sierra wrote:
> > From: Jordan Friendshuh <jfriendshuh@xes-inc.com>
> > 
> > Support the generic nand-ecc-mode and nand-ecc-strength device-tree
> > properties with the Freescale UPM NAND driver.
> > 
> > This patch preserves the default software ECC mode while adding the
> > ability to use BCH ECC for larger NAND devices.
> > 
> > Signed-off-by: Jordan Friendshuh <jfriendshuh@xes-inc.com>
> > Signed-off-by: Aaron Sierra <asierra@xes-inc.com>
> > ---
> > v2:
> >     * Now using ECC mode and strength helpers from of_mtd.h
> >     * ECC mode and strength checking is more robust
> 
> Should I be expecting an update? Ezequiel made a few comments, and I'm
> not sure if you were planning any action based on them.

Brian,

Yes, I intend to provide an updated patch based on Ezequiel's comments.
Sorry for not being clear about this earlier. I thought I would have
done the work by now.

-Aaron

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