[PATCH V2 0/4] mtd: nand: add enum nand_ecc_algo

[PATCH V2 0/4] mtd: nand: add enum nand_ecc_algo

[Rafał Miłecki]

Some time ago I started working on a new "nand-ecc-algo" property to allow
specifying ECC algorithm for hardware ECC mode as well.
Boris pointed out it's becoming a bit messy that way as we already have value
NAND_ECC_SOFT_BCH.
I suggested deprecating "soft_bch" value from nand-ecc-mode property and got
Boris agreed and no objections from others. So there is how I want to implement
this.
If you agree on this way & apply my patches, I'll start modifying NAND drivers
(similarly to the nandsim) and then will try to drop NAND_ECC_SOFT_BCH except
for handling backward compatibility.
Then finally we should be able to add "nand-ecc-algo" property support properly.

V2: Add of_get_nand_ecc_algo helper (in 2/4) instead of this slightly hacky code
    in nand_dt_init that would be replaced anyway.

Rafał Miłecki (4):
  mtd: nand: add new enum for storing ECC algorithm
  of: mtd: prepare helper reading NAND ECC algo from DT
  mtd: nand: set ECC algorithm in nand_dt_init
  mtd: nand: nandsim: set ECC algorithm explicitly

 drivers/mtd/nand/nand_base.c |  6 +++++-
 drivers/mtd/nand/nandsim.c   |  2 ++
 drivers/of/of_mtd.c          | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h     |  7 +++++++
 include/linux/of_mtd.h       |  6 ++++++
 5 files changed, 56 insertions(+), 1 deletion(-)

-- 
1.8.4.5

[PATCH V2 1/4] mtd: nand: add new enum for storing ECC algorithm

[Rafał Miłecki]

Our nand_ecc_modes_t is already a bit abused by value NAND_ECC_SOFT_BCH.
This enum should store ECC mode only and putting algorithm details there
is a bad idea. It would result in too many values impossible to support
in a sane way.

To solve this problem let's add a new enum. We'll have to modify all
drivers to set it properly but once it's done it'll be possible to drop
NAND_ECC_SOFT_BCH. That will result in a cleaner design and more
possibilities like setting ECC algorithm for hardware ECC mode.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 include/linux/mtd/nand.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 7604f4b..b818eb3 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -119,6 +119,12 @@ typedef enum {
 	NAND_ECC_SOFT_BCH,
 } nand_ecc_modes_t;
 
+enum nand_ecc_algo {
+	NAND_ECC_UNKNOWN,
+	NAND_ECC_HAMMING,
+	NAND_ECC_BCH,
+};
+
 /*
  * Constants for Hardware ECC
  */
@@ -508,6 +514,7 @@ struct nand_hw_control {
  */
 struct nand_ecc_ctrl {
 	nand_ecc_modes_t mode;
+	enum nand_ecc_algo algo;
 	int steps;
 	int size;
 	int bytes;
-- 
1.8.4.5

[PATCH V2 2/4] of: mtd: prepare helper reading NAND ECC algo from DT

[Rafał Miłecki]

NAND subsystem is being slightly reworked to store ECC details in
separated fields. In future we'll want to add support for more DT
properties as specifying every possible setup with a single
"nand-ecc-mode" is a pretty bad idea.
To allow this let's add a helper that will support something like
"nand-ecc-algo" in future. Right now we use it for keeping backward
compatibility.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/of/of_mtd.c    | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/of_mtd.h |  6 ++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed..15d056e 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -50,6 +50,42 @@ int of_get_nand_ecc_mode(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_get_nand_ecc_mode);
 
 /**
+ * of_get_nand_ecc_algo - Get nand ecc algorithm for given device_node
+ * @np:	Pointer to the given device_node
+ *
+ * The function gets ecc algorithm and returns its enum value, or errno in error
+ * case.
+ */
+int of_get_nand_ecc_algo(struct device_node *np)
+{
+	const char *pm;
+	int err;
+
+	/*
+	 * 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.
+	 */
+
+	/*
+	 * For backward compatibility we also read "nand-ecc-mode" checking
+	 * for some obsoleted values that were specifying ECC algorithm.
+	 */
+	err = of_property_read_string(np, "nand-ecc-mode", &pm);
+	if (err < 0)
+		return err;
+
+	if (!strcasecmp(pm, "soft"))
+		return NAND_ECC_HAMMING;
+	else if (!strcasecmp(pm, "soft_bch"))
+		return NAND_ECC_BCH;
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(of_get_nand_ecc_algo);
+
+/**
  * of_get_nand_ecc_step_size - Get ECC step size associated to
  * the required ECC strength (see below).
  * @np:	Pointer to the given device_node
diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
index e266caa..0f6aca5 100644
--- a/include/linux/of_mtd.h
+++ b/include/linux/of_mtd.h
@@ -13,6 +13,7 @@
 
 #include <linux/of.h>
 int of_get_nand_ecc_mode(struct device_node *np);
+int of_get_nand_ecc_algo(struct device_node *np);
 int of_get_nand_ecc_step_size(struct device_node *np);
 int of_get_nand_ecc_strength(struct device_node *np);
 int of_get_nand_bus_width(struct device_node *np);
@@ -25,6 +26,11 @@ static inline int of_get_nand_ecc_mode(struct device_node *np)
 	return -ENOSYS;
 }
 
+static inline int of_get_nand_ecc_algo(struct device_node *np)
+{
+	return -ENOSYS;
+}
+
 static inline int of_get_nand_ecc_step_size(struct device_node *np)
 {
 	return -ENOSYS;
-- 
1.8.4.5

[PATCH V2 3/4] mtd: nand: set ECC algorithm in nand_dt_init

[Rafał Miłecki]

Use recently added of_get_nand_ecc_algo for that.

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

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index f2c8ff3..ef977f3 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3979,7 +3979,7 @@ ident_done:
 static int nand_dt_init(struct nand_chip *chip)
 {
 	struct device_node *dn = nand_get_flash_node(chip);
-	int ecc_mode, ecc_strength, ecc_step;
+	int ecc_mode, ecc_algo, ecc_strength, ecc_step;
 
 	if (!dn)
 		return 0;
@@ -3991,6 +3991,7 @@ static int nand_dt_init(struct nand_chip *chip)
 		chip->bbt_options |= NAND_BBT_USE_FLASH;
 
 	ecc_mode = of_get_nand_ecc_mode(dn);
+	ecc_algo = of_get_nand_ecc_algo(dn);
 	ecc_strength = of_get_nand_ecc_strength(dn);
 	ecc_step = of_get_nand_ecc_step_size(dn);
 
@@ -4003,6 +4004,9 @@ static int nand_dt_init(struct nand_chip *chip)
 	if (ecc_mode >= 0)
 		chip->ecc.mode = ecc_mode;
 
+	if (ecc_algo >= 0)
+		chip->ecc.algo = ecc_algo;
+
 	if (ecc_strength >= 0)
 		chip->ecc.strength = ecc_strength;
 
-- 
1.8.4.5

[PATCH V2 4/4] mtd: nand: nandsim: set ECC algorithm explicitly

[Rafał Miłecki]

This follows recent work on switching to enum nand_ecc_algo and
deprecating NAND_ECC_SOFT_BCH.

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

diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index 1fd5195..6ff1d8d 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -2261,6 +2261,7 @@ static int __init ns_init_module(void)
 	chip->read_buf   = ns_nand_read_buf;
 	chip->read_word  = ns_nand_read_word;
 	chip->ecc.mode   = NAND_ECC_SOFT;
+	chip->ecc.algo   = NAND_ECC_HAMMING;
 	/* The NAND_SKIP_BBTSCAN option is necessary for 'overridesize' */
 	/* and 'badblocks' parameters to work */
 	chip->options   |= NAND_SKIP_BBTSCAN;
@@ -2339,6 +2340,7 @@ static int __init ns_init_module(void)
 			goto error;
 		}
 		chip->ecc.mode = NAND_ECC_SOFT_BCH;
+		chip->ecc.algo = NAND_ECC_BCH;
 		chip->ecc.size = 512;
 		chip->ecc.strength = bch;
 		chip->ecc.bytes = eccbytes;
-- 
1.8.4.5

Re: [PATCH V2 0/4] mtd: nand: add enum nand_ecc_algo

[Boris Brezillon]

On Wed, 23 Mar 2016 11:18:59 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:

> Some time ago I started working on a new "nand-ecc-algo" property to allow
> specifying ECC algorithm for hardware ECC mode as well.
> Boris pointed out it's becoming a bit messy that way as we already have value
> NAND_ECC_SOFT_BCH.
> I suggested deprecating "soft_bch" value from nand-ecc-mode property and got
> Boris agreed and no objections from others. So there is how I want to implement
> this.
> If you agree on this way & apply my patches, I'll start modifying NAND drivers
> (similarly to the nandsim) and then will try to drop NAND_ECC_SOFT_BCH except
> for handling backward compatibility.
> Then finally we should be able to add "nand-ecc-algo" property support properly.
> 
> V2: Add of_get_nand_ecc_algo helper (in 2/4) instead of this slightly hacky code
>     in nand_dt_init that would be replaced anyway.

This version looks good to me. If nobody disagree, I'll apply
those patches on my future nand/next branch.

Thanks,

Boris

> 
> Rafał Miłecki (4):
>   mtd: nand: add new enum for storing ECC algorithm
>   of: mtd: prepare helper reading NAND ECC algo from DT
>   mtd: nand: set ECC algorithm in nand_dt_init
>   mtd: nand: nandsim: set ECC algorithm explicitly
> 
>  drivers/mtd/nand/nand_base.c |  6 +++++-
>  drivers/mtd/nand/nandsim.c   |  2 ++
>  drivers/of/of_mtd.c          | 36 ++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/nand.h     |  7 +++++++
>  include/linux/of_mtd.h       |  6 ++++++
>  5 files changed, 56 insertions(+), 1 deletion(-)
> 



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

Re: [PATCH V2 0/4] mtd: nand: add enum nand_ecc_algo

[Boris Brezillon]

Hi Rafal,

On Wed, 23 Mar 2016 11:18:59 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:

> Some time ago I started working on a new "nand-ecc-algo" property to allow
> specifying ECC algorithm for hardware ECC mode as well.
> Boris pointed out it's becoming a bit messy that way as we already have value
> NAND_ECC_SOFT_BCH.
> I suggested deprecating "soft_bch" value from nand-ecc-mode property and got
> Boris agreed and no objections from others. So there is how I want to implement
> this.
> If you agree on this way & apply my patches, I'll start modifying NAND drivers
> (similarly to the nandsim) and then will try to drop NAND_ECC_SOFT_BCH except
> for handling backward compatibility.
> Then finally we should be able to add "nand-ecc-algo" property support properly.

Can you configure git-send-email to not encode your messages in base64?

Thanks,

Boris

> 
> V2: Add of_get_nand_ecc_algo helper (in 2/4) instead of this slightly hacky code
>     in nand_dt_init that would be replaced anyway.
> 
> Rafał Miłecki (4):
>   mtd: nand: add new enum for storing ECC algorithm
>   of: mtd: prepare helper reading NAND ECC algo from DT
>   mtd: nand: set ECC algorithm in nand_dt_init
>   mtd: nand: nandsim: set ECC algorithm explicitly
> 
>  drivers/mtd/nand/nand_base.c |  6 +++++-
>  drivers/mtd/nand/nandsim.c   |  2 ++
>  drivers/of/of_mtd.c          | 36 ++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/nand.h     |  7 +++++++
>  include/linux/of_mtd.h       |  6 ++++++
>  5 files changed, 56 insertions(+), 1 deletion(-)
> 



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

Re: [PATCH V2 0/4] mtd: nand: add enum nand_ecc_algo

[Boris Brezillon]

On Wed, 30 Mar 2016 15:27:10 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Hi Rafal,
> 
> On Wed, 23 Mar 2016 11:18:59 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
> 
> > Some time ago I started working on a new "nand-ecc-algo" property to allow
> > specifying ECC algorithm for hardware ECC mode as well.
> > Boris pointed out it's becoming a bit messy that way as we already have value
> > NAND_ECC_SOFT_BCH.
> > I suggested deprecating "soft_bch" value from nand-ecc-mode property and got
> > Boris agreed and no objections from others. So there is how I want to implement
> > this.
> > If you agree on this way & apply my patches, I'll start modifying NAND drivers
> > (similarly to the nandsim) and then will try to drop NAND_ECC_SOFT_BCH except
> > for handling backward compatibility.
> > Then finally we should be able to add "nand-ecc-algo" property support properly.
> 
> Can you configure git-send-email to not encode your messages in base64?


Nevermind, my mailer was showing me base64 encoded content, but after a
reboot everything is back to normal. Don't know what happened exactly.



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

Re: [PATCH V2 0/4] mtd: nand: add enum nand_ecc_algo

[Boris Brezillon]

Hi Rafal,

On Wed, 23 Mar 2016 11:18:59 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:

> Some time ago I started working on a new "nand-ecc-algo" property to allow
> specifying ECC algorithm for hardware ECC mode as well.
> Boris pointed out it's becoming a bit messy that way as we already have value
> NAND_ECC_SOFT_BCH.
> I suggested deprecating "soft_bch" value from nand-ecc-mode property and got
> Boris agreed and no objections from others. So there is how I want to implement
> this.
> If you agree on this way & apply my patches, I'll start modifying NAND drivers
> (similarly to the nandsim) and then will try to drop NAND_ECC_SOFT_BCH except
> for handling backward compatibility.
> Then finally we should be able to add "nand-ecc-algo" property support properly.
> 
> V2: Add of_get_nand_ecc_algo helper (in 2/4) instead of this slightly hacky code
>     in nand_dt_init that would be replaced anyway.

Whole series applied to nand/next [1]. I just added a comment for the
ecc->algo field in patch 1.
You can continue your rework (patch existing NAND_ECC_SOFT_BCH and
NAND_ECC_SOFT users to use ecc->algo).

Thanks,

Boris

[1]https://github.com/linux-nand/linux/tree/nand/next

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