Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1385039 > unrolled thread

[PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property

Started byRafał Miłecki <zajec5@gmail.com>
First post2016-04-22 13:30 +0200
Last post2016-04-26 09:40 +0200
Articles 10 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property Rafał Miłecki <zajec5@gmail.com> - 2016-04-22 13:30 +0200
    [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem Rafał Miłecki <zajec5@gmail.com> - 2016-04-22 13:30 +0200
      Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND  subsystem Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-25 17:10 +0200
      Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND  subsystem Brian Norris <computersforpeace@gmail.com> - 2016-04-26 08:00 +0200
        Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND  subsystem Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-26 09:40 +0200
        Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem Rafał Miłecki <zajec5@gmail.com> - 2016-04-26 20:40 +0200
        Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND  subsystem Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-27 10:00 +0200
    Re: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT  property Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-25 12:20 +0200
    Re: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT  property Rob Herring <robh@kernel.org> - 2016-04-25 14:40 +0200
    Re: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT  property Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-26 09:40 +0200

#1385039 — [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property

FromRafał Miłecki <zajec5@gmail.com>
Date2016-04-22 13:30 +0200
Subject[PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property
Message-ID<rqHrA-1aV-19@gated-at.bofh.it>
So far it was only possible to specify ECC algorithm using "soft" and
"soft_bch" values of nand-ecc-mode prop. There wasn't a way to specify
it for a hardware ECC mode.

Now that we have independent field in NAND subsystem for storing info
about ECC algorithm we may also add support for this new DT property.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 Documentation/devicetree/bindings/mtd/nand.txt |  2 ++
 drivers/mtd/nand/nand_base.c                   | 20 +++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index a17662b..5ac4ab7 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -22,6 +22,8 @@ Optional NAND chip properties:
 - nand-ecc-mode : String, operation mode of the NAND ecc mode.
   Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
   "soft_bch".
+- nand-ecc-algo: string, algorithm of NAND ECC.
+		 Supported values are: "hamming", "bch".
 - nand-bus-width : 8 or 16 bus width if not present 8
 - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
 
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 7bc37b4..a5417a0 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4003,17 +4003,23 @@ static int of_get_nand_ecc_mode(struct device_node *np)
 	return -ENODEV;
 }
 
+static const char * const nand_ecc_algos[] = {
+	[NAND_ECC_HAMMING]	= "hamming",
+	[NAND_ECC_BCH]		= "bch",
+};
+
 static int of_get_nand_ecc_algo(struct device_node *np)
 {
 	const char *pm;
-	int err;
+	int err, i;
 
-	/*
-	 * TODO: Read ECC algo OF property and map it to enum nand_ecc_algo.
-	 * It's not implemented yet as currently NAND subsystem ignores
-	 * algorithm explicitly set this way. Once it's handled we should
-	 * document & support new property.
-	 */
+	err = of_property_read_string(np, "nand-ecc-algo", &pm);
+	if (!err) {
+		for (i = 0; i < ARRAY_SIZE(nand_ecc_algos); i++)
+			if (!strcasecmp(pm, nand_ecc_algos[i]))
+				return i;
+		return -ENODEV;
+	}
 
 	/*
 	 * For backward compatibility we also read "nand-ecc-mode" checking
-- 
1.8.4.5

[toc] | [next] | [standalone]


#1385063 — [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem

FromRafał Miłecki <zajec5@gmail.com>
Date2016-04-22 13:30 +0200
Subject[PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem
Message-ID<rqHrC-1aV-75@gated-at.bofh.it>
In reply to#1385039
It's more reliable than guessing based on ECC strength. It allows using
NAND on devices with BCH-1 (e.g. D-Link DIR-885L).

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index c3331ff..dcb22dc 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1927,7 +1927,7 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
 
 	switch (chip->ecc.size) {
 	case 512:
-		if (chip->ecc.strength == 1) /* Hamming */
+		if (chip->ecc.algo == NAND_ECC_HAMMING)
 			cfg->ecc_level = 15;
 		else
 			cfg->ecc_level = chip->ecc.strength;
-- 
1.8.4.5

[toc] | [prev] | [next] | [standalone]


#1386563 — Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-25 17:10 +0200
SubjectRe: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem
Message-ID<rrQj9-80L-25@gated-at.bofh.it>
In reply to#1385063
On Fri, 22 Apr 2016 13:23:15 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> It's more reliable than guessing based on ECC strength. It allows using
> NAND on devices with BCH-1 (e.g. D-Link DIR-885L).

Brian, Kamal, could you add your Ack on this patch.

> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> index c3331ff..dcb22dc 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1927,7 +1927,7 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>  
>  	switch (chip->ecc.size) {
>  	case 512:
> -		if (chip->ecc.strength == 1) /* Hamming */
> +		if (chip->ecc.algo == NAND_ECC_HAMMING)
>  			cfg->ecc_level = 15;
>  		else
>  			cfg->ecc_level = chip->ecc.strength;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1387146 — Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem

FromBrian Norris <computersforpeace@gmail.com>
Date2016-04-26 08:00 +0200
SubjectRe: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem
Message-ID<rs4cq-2eM-7@gated-at.bofh.it>
In reply to#1385063
On Fri, Apr 22, 2016 at 01:23:15PM +0200, Rafał Miłecki wrote:
> It's more reliable than guessing based on ECC strength. It allows using
> NAND on devices with BCH-1 (e.g. D-Link DIR-885L).
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> index c3331ff..dcb22dc 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1927,7 +1927,7 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>  
>  	switch (chip->ecc.size) {
>  	case 512:
> -		if (chip->ecc.strength == 1) /* Hamming */
> +		if (chip->ecc.algo == NAND_ECC_HAMMING)

This doesn't handle most of the problems I noted on the early version of
this series. (But thank you for following through on the algorithm
selection refactoring!)

Particularly, this change will
(a) break any existing DTs which used to have 'nand-ecc-size = <1>', and
    would assume this gets Hamming ECC; and
(b) allows DTs to specify Hamming ECC for unsupported modes, like 1024B
    sectors, or ecc_level != 1. None of these are supported in HW.

>  			cfg->ecc_level = 15;
>  		else
>  			cfg->ecc_level = chip->ecc.strength;

Something like the following probably works better (not tested):

---8<---

From: Brian Norris <computersforpeace@gmail.com>
Date: Mon, 25 Apr 2016 20:48:02 -0700
Subject: [PATCH] mtd: brcmnand: respect ECC algorithm set by the NAND
 subsystem

This is more obvious than guessing based on ECC strength. It allows
using NAND on devices with BCH-1 (e.g. D-Link DIR-885L).

This maintains DT backward compatibility by defaulting to Hamming if a
1-bit ECC algorithm is specified without a corresponding algorithm
selection. i.e., to use BCH-1, you must specify:

  nand-ecc-strength = <1>;
  nand-ecc-step-size = <512>;
  nand-ecc-algo = "bch";

Also adds a check to ensure we haven't allowed someone to get by with SW
ECC. If we want to support SW ECC, we need to refactor some other pieces
of this driver.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/brcmnand/brcmnand.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index c3331ffcaffd..b76ad7c0144f 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1925,9 +1925,31 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
 	cfg->col_adr_bytes = 2;
 	cfg->blk_adr_bytes = get_blk_adr_bytes(mtd->size, mtd->writesize);
 
+	if (chip->ecc.mode != NAND_ECC_HW) {
+		dev_err(ctrl->dev, "only HW ECC supported; selected: %d\n",
+			chip->ecc.mode);
+		return -EINVAL;
+	}
+
+	if (chip->ecc.algo == NAND_ECC_UNKNOWN) {
+		if (chip->ecc.strength == 1 && chip->ecc.size == 512)
+			/* Default to Hamming for 1-bit ECC, if unspecified */
+			chip->ecc.algo = NAND_ECC_HAMMING;
+		else
+			/* Otherwise, BCH */
+			chip->ecc.algo = NAND_ECC_BCH;
+	}
+
+	if (chip->ecc.algo == NAND_ECC_HAMMING && (chip->ecc.strength != 1 ||
+						   chip->ecc.size != 512)) {
+		dev_err(ctrl->dev, "invalid Hamming params: %d bits per %d bytes\n",
+			chip->ecc.strength, chip->ecc.size);
+		return -EINVAL;
+	}
+
 	switch (chip->ecc.size) {
 	case 512:
-		if (chip->ecc.strength == 1) /* Hamming */
+		if (chip->ecc.algo == NAND_ECC_HAMMING)
 			cfg->ecc_level = 15;
 		else
 			cfg->ecc_level = chip->ecc.strength;
-- 
2.8.0.rc3.226.g39d4020

[toc] | [prev] | [next] | [standalone]


#1387188 — Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-26 09:40 +0200
SubjectRe: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem
Message-ID<rs5Ld-3z9-25@gated-at.bofh.it>
In reply to#1387146
Hi Brian,

On Mon, 25 Apr 2016 22:53:55 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Fri, Apr 22, 2016 at 01:23:15PM +0200, Rafał Miłecki wrote:
> > It's more reliable than guessing based on ECC strength. It allows using
> > NAND on devices with BCH-1 (e.g. D-Link DIR-885L).
> > 
> > Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> > ---
> >  drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> > index c3331ff..dcb22dc 100644
> > --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> > @@ -1927,7 +1927,7 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
> >  
> >  	switch (chip->ecc.size) {
> >  	case 512:
> > -		if (chip->ecc.strength == 1) /* Hamming */
> > +		if (chip->ecc.algo == NAND_ECC_HAMMING)  
> 
> This doesn't handle most of the problems I noted on the early version of
> this series. (But thank you for following through on the algorithm
> selection refactoring!)
> 
> Particularly, this change will
> (a) break any existing DTs which used to have 'nand-ecc-size = <1>', and
>     would assume this gets Hamming ECC; and
> (b) allows DTs to specify Hamming ECC for unsupported modes, like 1024B
>     sectors, or ecc_level != 1. None of these are supported in HW.

Indeed.

> 
> >  			cfg->ecc_level = 15;
> >  		else
> >  			cfg->ecc_level = chip->ecc.strength;  
> 
> Something like the following probably works better (not tested):
> 
> ---8<---
> 
> From: Brian Norris <computersforpeace@gmail.com>
> Date: Mon, 25 Apr 2016 20:48:02 -0700
> Subject: [PATCH] mtd: brcmnand: respect ECC algorithm set by the NAND
>  subsystem
> 
> This is more obvious than guessing based on ECC strength. It allows
> using NAND on devices with BCH-1 (e.g. D-Link DIR-885L).
> 
> This maintains DT backward compatibility by defaulting to Hamming if a
> 1-bit ECC algorithm is specified without a corresponding algorithm
> selection. i.e., to use BCH-1, you must specify:
> 
>   nand-ecc-strength = <1>;
>   nand-ecc-step-size = <512>;
>   nand-ecc-algo = "bch";
> 
> Also adds a check to ensure we haven't allowed someone to get by with SW
> ECC. If we want to support SW ECC, we need to refactor some other pieces
> of this driver.

I'm waiting a bit before applying this patch (I'd like to have a
Tested-by first).

> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/nand/brcmnand/brcmnand.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> index c3331ffcaffd..b76ad7c0144f 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1925,9 +1925,31 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>  	cfg->col_adr_bytes = 2;
>  	cfg->blk_adr_bytes = get_blk_adr_bytes(mtd->size, mtd->writesize);
>  
> +	if (chip->ecc.mode != NAND_ECC_HW) {
> +		dev_err(ctrl->dev, "only HW ECC supported; selected: %d\n",
> +			chip->ecc.mode);
> +		return -EINVAL;
> +	}
> +
> +	if (chip->ecc.algo == NAND_ECC_UNKNOWN) {
> +		if (chip->ecc.strength == 1 && chip->ecc.size == 512)
> +			/* Default to Hamming for 1-bit ECC, if unspecified */
> +			chip->ecc.algo = NAND_ECC_HAMMING;
> +		else
> +			/* Otherwise, BCH */
> +			chip->ecc.algo = NAND_ECC_BCH;
> +	}
> +
> +	if (chip->ecc.algo == NAND_ECC_HAMMING && (chip->ecc.strength != 1 ||
> +						   chip->ecc.size != 512)) {
> +		dev_err(ctrl->dev, "invalid Hamming params: %d bits per %d bytes\n",
> +			chip->ecc.strength, chip->ecc.size);
> +		return -EINVAL;
> +	}
> +
>  	switch (chip->ecc.size) {
>  	case 512:
> -		if (chip->ecc.strength == 1) /* Hamming */
> +		if (chip->ecc.algo == NAND_ECC_HAMMING)
>  			cfg->ecc_level = 15;
>  		else
>  			cfg->ecc_level = chip->ecc.strength;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1387843 — Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem

FromRafał Miłecki <zajec5@gmail.com>
Date2016-04-26 20:40 +0200
SubjectRe: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem
Message-ID<rsg3U-3Yf-19@gated-at.bofh.it>
In reply to#1387146
On 26 April 2016 at 07:53, Brian Norris <computersforpeace@gmail.com> wrote:
> From: Brian Norris <computersforpeace@gmail.com>
> Date: Mon, 25 Apr 2016 20:48:02 -0700
> Subject: [PATCH] mtd: brcmnand: respect ECC algorithm set by the NAND
>  subsystem
>
> This is more obvious than guessing based on ECC strength. It allows
> using NAND on devices with BCH-1 (e.g. D-Link DIR-885L).
>
> This maintains DT backward compatibility by defaulting to Hamming if a
> 1-bit ECC algorithm is specified without a corresponding algorithm
> selection. i.e., to use BCH-1, you must specify:
>
>   nand-ecc-strength = <1>;
>   nand-ecc-step-size = <512>;
>   nand-ecc-algo = "bch";
>
> Also adds a check to ensure we haven't allowed someone to get by with SW
> ECC. If we want to support SW ECC, we need to refactor some other pieces
> of this driver.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Tested-by: Rafał Miłecki <zajec5@gmail.com>

I just needed to apply following patch first:
[PATCH] mtd: nand: fix NULL pointer dereference in of_get_nand_ecc_algo

[toc] | [prev] | [next] | [standalone]


#1388755 — Re: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-27 10:00 +0200
SubjectRe: [PATCH 3/3] mtd: brcmnand: respect ECC algorithm set by NAND subsystem
Message-ID<rssy7-5VB-39@gated-at.bofh.it>
In reply to#1387146
On Mon, 25 Apr 2016 22:53:55 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> From: Brian Norris <computersforpeace@gmail.com>
> Date: Mon, 25 Apr 2016 20:48:02 -0700
> Subject: [PATCH] mtd: brcmnand: respect ECC algorithm set by the NAND
>  subsystem
> 
> This is more obvious than guessing based on ECC strength. It allows
> using NAND on devices with BCH-1 (e.g. D-Link DIR-885L).
> 
> This maintains DT backward compatibility by defaulting to Hamming if a
> 1-bit ECC algorithm is specified without a corresponding algorithm
> selection. i.e., to use BCH-1, you must specify:
> 
>   nand-ecc-strength = <1>;
>   nand-ecc-step-size = <512>;
>   nand-ecc-algo = "bch";
> 
> Also adds a check to ensure we haven't allowed someone to get by with SW
> ECC. If we want to support SW ECC, we need to refactor some other pieces
> of this driver.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Applied, thanks.

Boris

> ---
>  drivers/mtd/nand/brcmnand/brcmnand.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> index c3331ffcaffd..b76ad7c0144f 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1925,9 +1925,31 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>  	cfg->col_adr_bytes = 2;
>  	cfg->blk_adr_bytes = get_blk_adr_bytes(mtd->size, mtd->writesize);
>  
> +	if (chip->ecc.mode != NAND_ECC_HW) {
> +		dev_err(ctrl->dev, "only HW ECC supported; selected: %d\n",
> +			chip->ecc.mode);
> +		return -EINVAL;
> +	}
> +
> +	if (chip->ecc.algo == NAND_ECC_UNKNOWN) {
> +		if (chip->ecc.strength == 1 && chip->ecc.size == 512)
> +			/* Default to Hamming for 1-bit ECC, if unspecified */
> +			chip->ecc.algo = NAND_ECC_HAMMING;
> +		else
> +			/* Otherwise, BCH */
> +			chip->ecc.algo = NAND_ECC_BCH;
> +	}
> +
> +	if (chip->ecc.algo == NAND_ECC_HAMMING && (chip->ecc.strength != 1 ||
> +						   chip->ecc.size != 512)) {
> +		dev_err(ctrl->dev, "invalid Hamming params: %d bits per %d bytes\n",
> +			chip->ecc.strength, chip->ecc.size);
> +		return -EINVAL;
> +	}
> +
>  	switch (chip->ecc.size) {
>  	case 512:
> -		if (chip->ecc.strength == 1) /* Hamming */
> +		if (chip->ecc.algo == NAND_ECC_HAMMING)
>  			cfg->ecc_level = 15;
>  		else
>  			cfg->ecc_level = chip->ecc.strength;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1386213 — Re: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-25 12:20 +0200
SubjectRe: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property
Message-ID<rrLMv-406-27@gated-at.bofh.it>
In reply to#1385039
On Fri, 22 Apr 2016 13:23:13 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> So far it was only possible to specify ECC algorithm using "soft" and
> "soft_bch" values of nand-ecc-mode prop. There wasn't a way to specify
> it for a hardware ECC mode.
> 
> Now that we have independent field in NAND subsystem for storing info
> about ECC algorithm we may also add support for this new DT property.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt |  2 ++
>  drivers/mtd/nand/nand_base.c                   | 20 +++++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index a17662b..5ac4ab7 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -22,6 +22,8 @@ Optional NAND chip properties:
>  - nand-ecc-mode : String, operation mode of the NAND ecc mode.
>    Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
>    "soft_bch".
> +- nand-ecc-algo: string, algorithm of NAND ECC.
> +		 Supported values are: "hamming", "bch".

Rob, any objection to this binding change (and the one in patch 3).
Please note that everything is backward compatible.

Thanks,

Boris

>  - nand-bus-width : 8 or 16 bus width if not present 8
>  - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
>  
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 7bc37b4..a5417a0 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -4003,17 +4003,23 @@ static int of_get_nand_ecc_mode(struct device_node *np)
>  	return -ENODEV;
>  }
>  
> +static const char * const nand_ecc_algos[] = {
> +	[NAND_ECC_HAMMING]	= "hamming",
> +	[NAND_ECC_BCH]		= "bch",
> +};
> +
>  static int of_get_nand_ecc_algo(struct device_node *np)
>  {
>  	const char *pm;
> -	int err;
> +	int err, i;
>  
> -	/*
> -	 * TODO: Read ECC algo OF property and map it to enum nand_ecc_algo.
> -	 * It's not implemented yet as currently NAND subsystem ignores
> -	 * algorithm explicitly set this way. Once it's handled we should
> -	 * document & support new property.
> -	 */
> +	err = of_property_read_string(np, "nand-ecc-algo", &pm);
> +	if (!err) {
> +		for (i = 0; i < ARRAY_SIZE(nand_ecc_algos); i++)
> +			if (!strcasecmp(pm, nand_ecc_algos[i]))
> +				return i;
> +		return -ENODEV;
> +	}
>  
>  	/*
>  	 * For backward compatibility we also read "nand-ecc-mode" checking



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1386368 — Re: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property

FromRob Herring <robh@kernel.org>
Date2016-04-25 14:40 +0200
SubjectRe: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property
Message-ID<rrNXZ-5VY-37@gated-at.bofh.it>
In reply to#1385039
On Fri, Apr 22, 2016 at 01:23:13PM +0200, Rafał Miłecki wrote:
> So far it was only possible to specify ECC algorithm using "soft" and
> "soft_bch" values of nand-ecc-mode prop. There wasn't a way to specify
> it for a hardware ECC mode.
> 
> Now that we have independent field in NAND subsystem for storing info
> about ECC algorithm we may also add support for this new DT property.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt |  2 ++
>  drivers/mtd/nand/nand_base.c                   | 20 +++++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

[toc] | [prev] | [next] | [standalone]


#1387187 — Re: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-04-26 09:40 +0200
SubjectRe: [PATCH 1/3] mtd: nand: add support for "nand-ecc-algo" DT property
Message-ID<rs5Lc-3z9-23@gated-at.bofh.it>
In reply to#1385039
On Fri, 22 Apr 2016 13:23:13 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> So far it was only possible to specify ECC algorithm using "soft" and
> "soft_bch" values of nand-ecc-mode prop. There wasn't a way to specify
> it for a hardware ECC mode.
> 
> Now that we have independent field in NAND subsystem for storing info
> about ECC algorithm we may also add support for this new DT property.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt |  2 ++
>  drivers/mtd/nand/nand_base.c                   | 20 +++++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)

Applied, thanks.

Boris
-- 
Boris Brezillon, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web