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

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

[Rafał Miłecki]

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

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

[Rafał Miłecki]

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

[PATCH 2/3] Documentation: devicetree: deprecate "soft_bch" nand-ecc-mode value

[Rafał Miłecki]

Now that we support nand-ecc-algo property it should be used together
with "soft" to specify software BCH ECC.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 Documentation/devicetree/bindings/mtd/nand.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index 5ac4ab7..68342ea 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -20,8 +20,10 @@ Optional NAND controller properties
 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".
+		  Supported values are: "none", "soft", "hw", "hw_syndrome",
+		  "hw_oob_first".
+		  Deprecated values:
+		  "soft_bch": use "soft" and nand-ecc-algo instead
 - 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
-- 
1.8.4.5

[PATCH 2/3] Documentation: devicetree: deprecate "soft_bch" nand-ecc-mode value

[Rafał Miłecki]

Now that we support nand-ecc-algo property it should be used together
with "soft" to specify software BCH ECC.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 Documentation/devicetree/bindings/mtd/nand.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index 5ac4ab7..68342ea 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -20,8 +20,10 @@ Optional NAND controller properties
 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".
+		  Supported values are: "none", "soft", "hw", "hw_syndrome",
+		  "hw_oob_first".
+		  Deprecated values:
+		  "soft_bch": use "soft" and nand-ecc-algo instead
 - 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
-- 
1.8.4.5

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

[Rafał Miłecki]

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

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

[Boris Brezillon]

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

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

[Boris Brezillon]

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

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

[Rob Herring]

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>

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

[Rob Herring]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 1/3] mtd: nand: add support for "nand-ecc-algo" DT property

[Rob Herring]

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>

Re: [PATCH 2/3] Documentation: devicetree: deprecate "soft_bch" nand-ecc-mode value

[Rob Herring]

On Fri, Apr 22, 2016 at 01:23:14PM +0200, Rafał Miłecki wrote:
> Now that we support nand-ecc-algo property it should be used together
> with "soft" to specify software BCH ECC.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

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

Re: [PATCH 2/3] Documentation: devicetree: deprecate "soft_bch" nand-ecc-mode value

[Rob Herring]

On Fri, Apr 22, 2016 at 01:23:14PM +0200, Rafał Miłecki wrote:
> Now that we support nand-ecc-algo property it should be used together
> with "soft" to specify software BCH ECC.
> 
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 2/3] Documentation: devicetree: deprecate "soft_bch" nand-ecc-mode value

[Rob Herring]

On Fri, Apr 22, 2016 at 01:23:14PM +0200, Rafał Miłecki wrote:
> Now that we support nand-ecc-algo property it should be used together
> with "soft" to specify software BCH ECC.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

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

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

[Boris Brezillon]

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

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

[Brian Norris]

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

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

[Brian Norris]

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

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

[Boris Brezillon]

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

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

[Boris Brezillon]

On Fri, 22 Apr 2016 13:23:13 +0200
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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
--
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 2/3] Documentation: devicetree: deprecate "soft_bch" nand-ecc-mode value

[Boris Brezillon]

On Fri, 22 Apr 2016 13:23:14 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> Now that we support nand-ecc-algo property it should be used together
> with "soft" to specify software BCH ECC.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Applied, thanks.

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

Re: [PATCH 2/3] Documentation: devicetree: deprecate "soft_bch" nand-ecc-mode value

[Boris Brezillon]

On Fri, 22 Apr 2016 13:23:14 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> Now that we support nand-ecc-algo property it should be used together
> with "soft" to specify software BCH ECC.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Applied, thanks.

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

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

[Boris Brezillon]

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

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

[Boris Brezillon]

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

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

[Rafał Miłecki]

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

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

[Rafał Miłecki]

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

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

[Boris Brezillon]

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