* [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-12 18:11 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-12 18:11 UTC (permalink / raw)
To: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree-u79uwXL29TY76Z2rM5mHXA,
Rafał Miłecki
This allows specifying algorithm used for NAND ECC. There are two
available values: "bch" and "hamming". It's important as having just
ECC parameters (step, strength) isn't enough to determine algorithm,
e.g. you can't distinct BCH-1 and Hamming.
Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
include/linux/mtd/nand.h | 5 ++++
include/linux/of_mtd.h | 6 +++++
4 files changed, 47 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index b53f92e..a2c2df5 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -3,6 +3,9 @@
- 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: "bch", "hamming". The default one is
+ "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/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed..f2a6630 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -50,6 +50,39 @@ int of_get_nand_ecc_mode(struct device_node *np)
EXPORT_SYMBOL_GPL(of_get_nand_ecc_mode);
/**
+ * It maps 'enum nand_ecc_algo' found in include/linux/mtd/nand.h into the
+ * device tree binding of 'nand-ecc-algo'.
+ */
+static const char * const nand_ecc_algos[] = {
+ [NAND_ECC_BCH] = "bch",
+ [NAND_ECC_HAMMING] = "hamming",
+};
+
+/**
+ * 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 string from property 'nand-ecc-algo' and
+ * returns its index in nand_ecc_algos table, or errno in error case.
+ */
+int of_get_nand_ecc_algo(struct device_node *np)
+{
+ const char *pm;
+ int err, i;
+
+ err = of_property_read_string(np, "nand-ecc-algo", &pm);
+ if (err < 0)
+ return err;
+
+ for (i = 0; i < ARRAY_SIZE(nand_ecc_algos); i++)
+ if (!strcasecmp(pm, nand_ecc_algos[i]))
+ return i;
+
+ 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/mtd/nand.h b/include/linux/mtd/nand.h
index 7604f4b..25854d2 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -119,6 +119,11 @@ typedef enum {
NAND_ECC_SOFT_BCH,
} nand_ecc_modes_t;
+enum nand_ecc_algo {
+ NAND_ECC_BCH,
+ NAND_ECC_HAMMING,
+};
+
/*
* Constants for Hardware ECC
*/
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
--
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
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-12 18:11 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-12 18:11 UTC (permalink / raw)
To: Brian Norris, linux-mtd
Cc: Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree, Rafał Miłecki
This allows specifying algorithm used for NAND ECC. There are two
available values: "bch" and "hamming". It's important as having just
ECC parameters (step, strength) isn't enough to determine algorithm,
e.g. you can't distinct BCH-1 and Hamming.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
include/linux/mtd/nand.h | 5 ++++
include/linux/of_mtd.h | 6 +++++
4 files changed, 47 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
index b53f92e..a2c2df5 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -3,6 +3,9 @@
- 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: "bch", "hamming". The default one is
+ "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/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed..f2a6630 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -50,6 +50,39 @@ int of_get_nand_ecc_mode(struct device_node *np)
EXPORT_SYMBOL_GPL(of_get_nand_ecc_mode);
/**
+ * It maps 'enum nand_ecc_algo' found in include/linux/mtd/nand.h into the
+ * device tree binding of 'nand-ecc-algo'.
+ */
+static const char * const nand_ecc_algos[] = {
+ [NAND_ECC_BCH] = "bch",
+ [NAND_ECC_HAMMING] = "hamming",
+};
+
+/**
+ * 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 string from property 'nand-ecc-algo' and
+ * returns its index in nand_ecc_algos table, or errno in error case.
+ */
+int of_get_nand_ecc_algo(struct device_node *np)
+{
+ const char *pm;
+ int err, i;
+
+ err = of_property_read_string(np, "nand-ecc-algo", &pm);
+ if (err < 0)
+ return err;
+
+ for (i = 0; i < ARRAY_SIZE(nand_ecc_algos); i++)
+ if (!strcasecmp(pm, nand_ecc_algos[i]))
+ return i;
+
+ 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/mtd/nand.h b/include/linux/mtd/nand.h
index 7604f4b..25854d2 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -119,6 +119,11 @@ typedef enum {
NAND_ECC_SOFT_BCH,
} nand_ecc_modes_t;
+enum nand_ecc_algo {
+ NAND_ECC_BCH,
+ NAND_ECC_HAMMING,
+};
+
/*
* Constants for Hardware ECC
*/
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
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-02-12 18:11 ` Rafał Miłecki
-1 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-12 18:11 UTC (permalink / raw)
To: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree-u79uwXL29TY76Z2rM5mHXA,
Rafał Miłecki
This will allow drivers handle ECC properly.
Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/mtd/nand/nand_base.c | 6 +++++-
include/linux/mtd/nand.h | 1 +
2 files changed, 6 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;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 25854d2..8deca1b 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -513,6 +513,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
--
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
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
@ 2016-02-12 18:11 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-12 18:11 UTC (permalink / raw)
To: Brian Norris, linux-mtd
Cc: Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree, Rafał Miłecki
This will allow drivers handle ECC properly.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
drivers/mtd/nand/nand_base.c | 6 +++++-
include/linux/mtd/nand.h | 1 +
2 files changed, 6 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;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 25854d2..8deca1b 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -513,6 +513,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
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 3/3] mtd: brcmnand: fix check for Hamming algorithm
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-02-12 18:11 ` Rafał Miłecki
-1 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-12 18:11 UTC (permalink / raw)
To: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree-u79uwXL29TY76Z2rM5mHXA,
Rafał Miłecki
So far we were treating ECC strength 1 as Hamming algorithm. It didn't
supporting some less common devices with BCH-1 (e.g. D-Link DIR-885L).
Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
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 844fc07..b8055da 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1842,7 +1842,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
--
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
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 3/3] mtd: brcmnand: fix check for Hamming algorithm
@ 2016-02-12 18:11 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-12 18:11 UTC (permalink / raw)
To: Brian Norris, linux-mtd
Cc: Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree, Rafał Miłecki
So far we were treating ECC strength 1 as Hamming algorithm. It didn't
supporting some less common 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 844fc07..b8055da 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1842,7 +1842,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
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-12 18:11 ` Rafał Miłecki
(?)
@ 2016-02-15 21:28 ` Kamal Dasu
-1 siblings, 0 replies; 43+ messages in thread
From: Kamal Dasu @ 2016-02-15 21:28 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, Hauke Mehrtens, Rob Herring,
Frank Rowand, Grant Likely, devicetree
[-- Attachment #1: Type: text/plain, Size: 4333 bytes --]
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>
Reviewed-by: Kamal Dasu <kdasu.kdev@gmail.com>
Thanks Rafal
>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> drivers/of/of_mtd.c | 33
> ++++++++++++++++++++++++++
> include/linux/mtd/nand.h | 5 ++++
> include/linux/of_mtd.h | 6 +++++
> 4 files changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt
> b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..a2c2df5 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -3,6 +3,9 @@
> - 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: "bch", "hamming". The default one
> is
> + "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/of/of_mtd.c b/drivers/of/of_mtd.c
> index b7361ed..f2a6630 100644
> --- a/drivers/of/of_mtd.c
> +++ b/drivers/of/of_mtd.c
> @@ -50,6 +50,39 @@ int of_get_nand_ecc_mode(struct device_node *np)
> EXPORT_SYMBOL_GPL(of_get_nand_ecc_mode);
>
> /**
> + * It maps 'enum nand_ecc_algo' found in include/linux/mtd/nand.h into the
> + * device tree binding of 'nand-ecc-algo'.
> + */
> +static const char * const nand_ecc_algos[] = {
> + [NAND_ECC_BCH] = "bch",
> + [NAND_ECC_HAMMING] = "hamming",
> +};
> +
> +/**
> + * 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 string from property 'nand-ecc-algo'
> and
> + * returns its index in nand_ecc_algos table, or errno in error case.
> + */
> +int of_get_nand_ecc_algo(struct device_node *np)
> +{
> + const char *pm;
> + int err, i;
> +
> + err = of_property_read_string(np, "nand-ecc-algo", &pm);
> + if (err < 0)
> + return err;
> +
> + for (i = 0; i < ARRAY_SIZE(nand_ecc_algos); i++)
> + if (!strcasecmp(pm, nand_ecc_algos[i]))
> + return i;
> +
> + 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/mtd/nand.h b/include/linux/mtd/nand.h
> index 7604f4b..25854d2 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -119,6 +119,11 @@ typedef enum {
> NAND_ECC_SOFT_BCH,
> } nand_ecc_modes_t;
>
> +enum nand_ecc_algo {
> + NAND_ECC_BCH,
> + NAND_ECC_HAMMING,
> +};
> +
> /*
> * Constants for Hardware ECC
> */
> 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
>
>
[-- Attachment #2: Type: text/html, Size: 6039 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 3/3] mtd: brcmnand: fix check for Hamming algorithm
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-02-15 21:30 ` Kamal Dasu
-1 siblings, 0 replies; 43+ messages in thread
From: Kamal Dasu @ 2016-02-15 21:30 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hauke Mehrtens, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> So far we were treating ECC strength 1 as Hamming algorithm. It didn't
> supporting some less common devices with BCH-1 (e.g. D-Link DIR-885L).
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thanks Rafal
> ---
> 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 844fc07..b8055da 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1842,7 +1842,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
>
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 3/3] mtd: brcmnand: fix check for Hamming algorithm
@ 2016-02-15 21:30 ` Kamal Dasu
0 siblings, 0 replies; 43+ messages in thread
From: Kamal Dasu @ 2016-02-15 21:30 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, Hauke Mehrtens, Rob Herring,
Frank Rowand, Grant Likely, devicetree
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> So far we were treating ECC strength 1 as Hamming algorithm. It didn't
> supporting some less common devices with BCH-1 (e.g. D-Link DIR-885L).
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Reviewed-by: Kamal Dasu <kdasu.kdev@gmail.com>
Thanks Rafal
> ---
> 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 844fc07..b8055da 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1842,7 +1842,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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-02-15 21:31 ` Kamal Dasu
-1 siblings, 0 replies; 43+ messages in thread
From: Kamal Dasu @ 2016-02-15 21:31 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hauke Mehrtens, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>
> This will allow drivers handle ECC properly.
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thanks Rafal
>
> ---
> drivers/mtd/nand/nand_base.c | 6 +++++-
> include/linux/mtd/nand.h | 1 +
> 2 files changed, 6 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;
>
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 25854d2..8deca1b 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -513,6 +513,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
>
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
@ 2016-02-15 21:31 ` Kamal Dasu
0 siblings, 0 replies; 43+ messages in thread
From: Kamal Dasu @ 2016-02-15 21:31 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, Hauke Mehrtens, Rob Herring,
Frank Rowand, Grant Likely, devicetree
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>
> This will allow drivers handle ECC properly.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Reviewed-by: Kamal Dasu <kdasu.kdev@gmail.com>
Thanks Rafal
>
> ---
> drivers/mtd/nand/nand_base.c | 6 +++++-
> include/linux/mtd/nand.h | 1 +
> 2 files changed, 6 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;
>
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 25854d2..8deca1b 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -513,6 +513,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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-02-15 21:43 ` Kamal Dasu
-1 siblings, 0 replies; 43+ messages in thread
From: Kamal Dasu @ 2016-02-15 21:43 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hauke Mehrtens, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thanks Rafal
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> include/linux/mtd/nand.h | 5 ++++
> include/linux/of_mtd.h | 6 +++++
> 4 files changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..a2c2df5 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -3,6 +3,9 @@
> - 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: "bch", "hamming". The default one is
> + "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/of/of_mtd.c b/drivers/of/of_mtd.c
> index b7361ed..f2a6630 100644
> --- a/drivers/of/of_mtd.c
> +++ b/drivers/of/of_mtd.c
> @@ -50,6 +50,39 @@ int of_get_nand_ecc_mode(struct device_node *np)
> EXPORT_SYMBOL_GPL(of_get_nand_ecc_mode);
>
> /**
> + * It maps 'enum nand_ecc_algo' found in include/linux/mtd/nand.h into the
> + * device tree binding of 'nand-ecc-algo'.
> + */
> +static const char * const nand_ecc_algos[] = {
> + [NAND_ECC_BCH] = "bch",
> + [NAND_ECC_HAMMING] = "hamming",
> +};
> +
> +/**
> + * 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 string from property 'nand-ecc-algo' and
> + * returns its index in nand_ecc_algos table, or errno in error case.
> + */
> +int of_get_nand_ecc_algo(struct device_node *np)
> +{
> + const char *pm;
> + int err, i;
> +
> + err = of_property_read_string(np, "nand-ecc-algo", &pm);
> + if (err < 0)
> + return err;
> +
> + for (i = 0; i < ARRAY_SIZE(nand_ecc_algos); i++)
> + if (!strcasecmp(pm, nand_ecc_algos[i]))
> + return i;
> +
> + 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/mtd/nand.h b/include/linux/mtd/nand.h
> index 7604f4b..25854d2 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -119,6 +119,11 @@ typedef enum {
> NAND_ECC_SOFT_BCH,
> } nand_ecc_modes_t;
>
> +enum nand_ecc_algo {
> + NAND_ECC_BCH,
> + NAND_ECC_HAMMING,
> +};
> +
> /*
> * Constants for Hardware ECC
> */
> 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
>
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-15 21:43 ` Kamal Dasu
0 siblings, 0 replies; 43+ messages in thread
From: Kamal Dasu @ 2016-02-15 21:43 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, Hauke Mehrtens, Rob Herring,
Frank Rowand, Grant Likely, devicetree
On Fri, Feb 12, 2016 at 1:11 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Reviewed-by: Kamal Dasu <kdasu.kdev@gmail.com>
Thanks Rafal
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> include/linux/mtd/nand.h | 5 ++++
> include/linux/of_mtd.h | 6 +++++
> 4 files changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..a2c2df5 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -3,6 +3,9 @@
> - 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: "bch", "hamming". The default one is
> + "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/of/of_mtd.c b/drivers/of/of_mtd.c
> index b7361ed..f2a6630 100644
> --- a/drivers/of/of_mtd.c
> +++ b/drivers/of/of_mtd.c
> @@ -50,6 +50,39 @@ int of_get_nand_ecc_mode(struct device_node *np)
> EXPORT_SYMBOL_GPL(of_get_nand_ecc_mode);
>
> /**
> + * It maps 'enum nand_ecc_algo' found in include/linux/mtd/nand.h into the
> + * device tree binding of 'nand-ecc-algo'.
> + */
> +static const char * const nand_ecc_algos[] = {
> + [NAND_ECC_BCH] = "bch",
> + [NAND_ECC_HAMMING] = "hamming",
> +};
> +
> +/**
> + * 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 string from property 'nand-ecc-algo' and
> + * returns its index in nand_ecc_algos table, or errno in error case.
> + */
> +int of_get_nand_ecc_algo(struct device_node *np)
> +{
> + const char *pm;
> + int err, i;
> +
> + err = of_property_read_string(np, "nand-ecc-algo", &pm);
> + if (err < 0)
> + return err;
> +
> + for (i = 0; i < ARRAY_SIZE(nand_ecc_algos); i++)
> + if (!strcasecmp(pm, nand_ecc_algos[i]))
> + return i;
> +
> + 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/mtd/nand.h b/include/linux/mtd/nand.h
> index 7604f4b..25854d2 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -119,6 +119,11 @@ typedef enum {
> NAND_ECC_SOFT_BCH,
> } nand_ecc_modes_t;
>
> +enum nand_ecc_algo {
> + NAND_ECC_BCH,
> + NAND_ECC_HAMMING,
> +};
> +
> /*
> * Constants for Hardware ECC
> */
> 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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-02-22 2:54 ` Rob Herring
-1 siblings, 0 replies; 43+ messages in thread
From: Rob Herring @ 2016-02-22 2:54 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hauke Mehrtens, Kamal Dasu, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 07:11:23PM +0100, Rafał Miłecki wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> include/linux/mtd/nand.h | 5 ++++
> include/linux/of_mtd.h | 6 +++++
> 4 files changed, 47 insertions(+)
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-22 2:54 ` Rob Herring
0 siblings, 0 replies; 43+ messages in thread
From: Rob Herring @ 2016-02-22 2:54 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, Hauke Mehrtens, Kamal Dasu,
Frank Rowand, Grant Likely, devicetree
On Fri, Feb 12, 2016 at 07:11:23PM +0100, Rafał Miłecki wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
Acked-by: Rob Herring <robh@kernel.org>
> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> include/linux/mtd/nand.h | 5 ++++
> include/linux/of_mtd.h | 6 +++++
> 4 files changed, 47 insertions(+)
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-02-24 13:46 ` Boris Brezillon
-1 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-24 13:46 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
Hi Rafal,
On Fri, 12 Feb 2016 19:11:23 +0100
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> include/linux/mtd/nand.h | 5 ++++
> include/linux/of_mtd.h | 6 +++++
> 4 files changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..a2c2df5 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -3,6 +3,9 @@
> - 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: "bch", "hamming". The default one is
> + "bch".
I like the idea of specifying which ECC algorithm should be used, and
this is IMO clearer than putting the information directly in
nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
which are respectively encoding software hamming and software bch
implementations).
But shouldn't we take this even further and add new DT properties
to encode the ECC layout (syndrome, oob_first), and another one to
specify the implementation type (software or hardware)?
nand-ecc-layout = "nornal", "syndrome" or "oob_first"
nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
Note that it's not something I ask you to do right now, I just want to
bring the topic on the table.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel 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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-24 13:46 ` Boris Brezillon
0 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-24 13:46 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, devicetree, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
Hi Rafal,
On Fri, 12 Feb 2016 19:11:23 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> include/linux/mtd/nand.h | 5 ++++
> include/linux/of_mtd.h | 6 +++++
> 4 files changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index b53f92e..a2c2df5 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -3,6 +3,9 @@
> - 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: "bch", "hamming". The default one is
> + "bch".
I like the idea of specifying which ECC algorithm should be used, and
this is IMO clearer than putting the information directly in
nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
which are respectively encoding software hamming and software bch
implementations).
But shouldn't we take this even further and add new DT properties
to encode the ECC layout (syndrome, oob_first), and another one to
specify the implementation type (software or hardware)?
nand-ecc-layout = "nornal", "syndrome" or "oob_first"
nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
Note that it's not something I ask you to do right now, I just want to
bring the topic on the table.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-24 13:46 ` Boris Brezillon
@ 2016-02-25 19:56 ` Rafał Miłecki
-1 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-25 19:56 UTC (permalink / raw)
To: Boris Brezillon
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On 24 February 2016 at 14:46, Boris Brezillon
<boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Fri, 12 Feb 2016 19:11:23 +0100
> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> This allows specifying algorithm used for NAND ECC. There are two
>> available values: "bch" and "hamming". It's important as having just
>> ECC parameters (step, strength) isn't enough to determine algorithm,
>> e.g. you can't distinct BCH-1 and Hamming.
>>
>> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
>> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
>> include/linux/mtd/nand.h | 5 ++++
>> include/linux/of_mtd.h | 6 +++++
>> 4 files changed, 47 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
>> index b53f92e..a2c2df5 100644
>> --- a/Documentation/devicetree/bindings/mtd/nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
>> @@ -3,6 +3,9 @@
>> - 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: "bch", "hamming". The default one is
>> + "bch".
>
> I like the idea of specifying which ECC algorithm should be used, and
> this is IMO clearer than putting the information directly in
> nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
> which are respectively encoding software hamming and software bch
> implementations).
>
> But shouldn't we take this even further and add new DT properties
> to encode the ECC layout (syndrome, oob_first), and another one to
> specify the implementation type (software or hardware)?
>
> nand-ecc-layout = "nornal", "syndrome" or "oob_first"
> nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
>
> Note that it's not something I ask you to do right now, I just want to
> bring the topic on the table.
So far I was fine with whatever drvier/NAND core picked, didn't have
any issue with it. If at some point we'll see a real need of
specifying it manually as well, I guess we should do as you suggested.
--
Rafał
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-25 19:56 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-25 19:56 UTC (permalink / raw)
To: Boris Brezillon
Cc: Brian Norris, linux-mtd, devicetree, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On 24 February 2016 at 14:46, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Fri, 12 Feb 2016 19:11:23 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> This allows specifying algorithm used for NAND ECC. There are two
>> available values: "bch" and "hamming". It's important as having just
>> ECC parameters (step, strength) isn't enough to determine algorithm,
>> e.g. you can't distinct BCH-1 and Hamming.
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
>> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
>> include/linux/mtd/nand.h | 5 ++++
>> include/linux/of_mtd.h | 6 +++++
>> 4 files changed, 47 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
>> index b53f92e..a2c2df5 100644
>> --- a/Documentation/devicetree/bindings/mtd/nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
>> @@ -3,6 +3,9 @@
>> - 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: "bch", "hamming". The default one is
>> + "bch".
>
> I like the idea of specifying which ECC algorithm should be used, and
> this is IMO clearer than putting the information directly in
> nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
> which are respectively encoding software hamming and software bch
> implementations).
>
> But shouldn't we take this even further and add new DT properties
> to encode the ECC layout (syndrome, oob_first), and another one to
> specify the implementation type (software or hardware)?
>
> nand-ecc-layout = "nornal", "syndrome" or "oob_first"
> nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
>
> Note that it's not something I ask you to do right now, I just want to
> bring the topic on the table.
So far I was fine with whatever drvier/NAND core picked, didn't have
any issue with it. If at some point we'll see a real need of
specifying it manually as well, I guess we should do as you suggested.
--
Rafał
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-25 19:56 ` Rafał Miłecki
@ 2016-02-25 20:09 ` Boris Brezillon
-1 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-25 20:09 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On Thu, 25 Feb 2016 20:56:36 +0100
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 24 February 2016 at 14:46, Boris Brezillon
> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Fri, 12 Feb 2016 19:11:23 +0100
> > Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> This allows specifying algorithm used for NAND ECC. There are two
> >> available values: "bch" and "hamming". It's important as having just
> >> ECC parameters (step, strength) isn't enough to determine algorithm,
> >> e.g. you can't distinct BCH-1 and Hamming.
> >>
> >> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> >> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> >> include/linux/mtd/nand.h | 5 ++++
> >> include/linux/of_mtd.h | 6 +++++
> >> 4 files changed, 47 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> >> index b53f92e..a2c2df5 100644
> >> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> >> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> >> @@ -3,6 +3,9 @@
> >> - 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: "bch", "hamming". The default one is
> >> + "bch".
> >
> > I like the idea of specifying which ECC algorithm should be used, and
> > this is IMO clearer than putting the information directly in
> > nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
> > which are respectively encoding software hamming and software bch
> > implementations).
> >
> > But shouldn't we take this even further and add new DT properties
> > to encode the ECC layout (syndrome, oob_first), and another one to
> > specify the implementation type (software or hardware)?
> >
> > nand-ecc-layout = "nornal", "syndrome" or "oob_first"
> > nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
> >
> > Note that it's not something I ask you to do right now, I just want to
> > bring the topic on the table.
>
> So far I was fine with whatever drvier/NAND core picked, didn't have
> any issue with it. If at some point we'll see a real need of
> specifying it manually as well, I guess we should do as you suggested.
>
My point is that it's kind of weird to have the same information
duplicated in two different properties with possibly some conflicting
configurations, like:
nand-ecc-mode = "soft_bch"; /* software BCH implementation... */
nand-ecc-algo = "hamming"; /* ... and here you choose Hamming */
Of course this won't be a real problem until NAND core starts using
this property to decide which soft implementation should be used, but
providing a consistent binding is important IMO.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel 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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-25 20:09 ` Boris Brezillon
0 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-25 20:09 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, devicetree, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On Thu, 25 Feb 2016 20:56:36 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:
> On 24 February 2016 at 14:46, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Fri, 12 Feb 2016 19:11:23 +0100
> > Rafał Miłecki <zajec5@gmail.com> wrote:
> >
> >> This allows specifying algorithm used for NAND ECC. There are two
> >> available values: "bch" and "hamming". It's important as having just
> >> ECC parameters (step, strength) isn't enough to determine algorithm,
> >> e.g. you can't distinct BCH-1 and Hamming.
> >>
> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >> ---
> >> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> >> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> >> include/linux/mtd/nand.h | 5 ++++
> >> include/linux/of_mtd.h | 6 +++++
> >> 4 files changed, 47 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> >> index b53f92e..a2c2df5 100644
> >> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> >> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> >> @@ -3,6 +3,9 @@
> >> - 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: "bch", "hamming". The default one is
> >> + "bch".
> >
> > I like the idea of specifying which ECC algorithm should be used, and
> > this is IMO clearer than putting the information directly in
> > nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
> > which are respectively encoding software hamming and software bch
> > implementations).
> >
> > But shouldn't we take this even further and add new DT properties
> > to encode the ECC layout (syndrome, oob_first), and another one to
> > specify the implementation type (software or hardware)?
> >
> > nand-ecc-layout = "nornal", "syndrome" or "oob_first"
> > nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
> >
> > Note that it's not something I ask you to do right now, I just want to
> > bring the topic on the table.
>
> So far I was fine with whatever drvier/NAND core picked, didn't have
> any issue with it. If at some point we'll see a real need of
> specifying it manually as well, I guess we should do as you suggested.
>
My point is that it's kind of weird to have the same information
duplicated in two different properties with possibly some conflicting
configurations, like:
nand-ecc-mode = "soft_bch"; /* software BCH implementation... */
nand-ecc-algo = "hamming"; /* ... and here you choose Hamming */
Of course this won't be a real problem until NAND core starts using
this property to decide which soft implementation should be used, but
providing a consistent binding is important IMO.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-25 20:09 ` Boris Brezillon
@ 2016-02-26 16:30 ` Rafał Miłecki
-1 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-26 16:30 UTC (permalink / raw)
To: Boris Brezillon
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On 25 February 2016 at 21:09, Boris Brezillon
<boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Thu, 25 Feb 2016 20:56:36 +0100
> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On 24 February 2016 at 14:46, Boris Brezillon
>> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> > On Fri, 12 Feb 2016 19:11:23 +0100
>> > Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >
>> >> This allows specifying algorithm used for NAND ECC. There are two
>> >> available values: "bch" and "hamming". It's important as having just
>> >> ECC parameters (step, strength) isn't enough to determine algorithm,
>> >> e.g. you can't distinct BCH-1 and Hamming.
>> >>
>> >> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >> ---
>> >> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
>> >> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
>> >> include/linux/mtd/nand.h | 5 ++++
>> >> include/linux/of_mtd.h | 6 +++++
>> >> 4 files changed, 47 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
>> >> index b53f92e..a2c2df5 100644
>> >> --- a/Documentation/devicetree/bindings/mtd/nand.txt
>> >> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
>> >> @@ -3,6 +3,9 @@
>> >> - 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: "bch", "hamming". The default one is
>> >> + "bch".
>> >
>> > I like the idea of specifying which ECC algorithm should be used, and
>> > this is IMO clearer than putting the information directly in
>> > nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
>> > which are respectively encoding software hamming and software bch
>> > implementations).
>> >
>> > But shouldn't we take this even further and add new DT properties
>> > to encode the ECC layout (syndrome, oob_first), and another one to
>> > specify the implementation type (software or hardware)?
>> >
>> > nand-ecc-layout = "nornal", "syndrome" or "oob_first"
>> > nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
>> >
>> > Note that it's not something I ask you to do right now, I just want to
>> > bring the topic on the table.
>>
>> So far I was fine with whatever drvier/NAND core picked, didn't have
>> any issue with it. If at some point we'll see a real need of
>> specifying it manually as well, I guess we should do as you suggested.
>>
>
> My point is that it's kind of weird to have the same information
> duplicated in two different properties with possibly some conflicting
> configurations, like:
>
> nand-ecc-mode = "soft_bch"; /* software BCH implementation... */
> nand-ecc-algo = "hamming"; /* ... and here you choose Hamming */
>
> Of course this won't be a real problem until NAND core starts using
> this property to decide which soft implementation should be used, but
> providing a consistent binding is important IMO.
Sorry, I didn't pay enough attention to NAND properties and missed
your point. I was a bit surprised to see NAND_ECC_SOFT_BCH value and
"soft_bch" as mapping. This is indeed a bit confusing.
So I'm trying to find a proper way to split nand-ecc-mode. What you
suggested isn't exactly clear to me. AFAIR both enums: SYNDROME and
OOB_FIRST specify they way ECC info has to be accessed. For example
when using NAND_ECC_HW_OOB_FIRST you have to read OOB and after that
read data. I don't see how it's really related to the ECC layout (you
suggested "oob_first" as possible value of nand-ecc-layout).
What about:
1) Adding new nand-ecc-algo as this patch does
2) Deprecating "soft_bch" value from nand-ecc-mode property
--
Rafał
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-26 16:30 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-26 16:30 UTC (permalink / raw)
To: Boris Brezillon
Cc: Brian Norris, linux-mtd, devicetree, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On 25 February 2016 at 21:09, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Thu, 25 Feb 2016 20:56:36 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> On 24 February 2016 at 14:46, Boris Brezillon
>> <boris.brezillon@free-electrons.com> wrote:
>> > On Fri, 12 Feb 2016 19:11:23 +0100
>> > Rafał Miłecki <zajec5@gmail.com> wrote:
>> >
>> >> This allows specifying algorithm used for NAND ECC. There are two
>> >> available values: "bch" and "hamming". It's important as having just
>> >> ECC parameters (step, strength) isn't enough to determine algorithm,
>> >> e.g. you can't distinct BCH-1 and Hamming.
>> >>
>> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> >> ---
>> >> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
>> >> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
>> >> include/linux/mtd/nand.h | 5 ++++
>> >> include/linux/of_mtd.h | 6 +++++
>> >> 4 files changed, 47 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
>> >> index b53f92e..a2c2df5 100644
>> >> --- a/Documentation/devicetree/bindings/mtd/nand.txt
>> >> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
>> >> @@ -3,6 +3,9 @@
>> >> - 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: "bch", "hamming". The default one is
>> >> + "bch".
>> >
>> > I like the idea of specifying which ECC algorithm should be used, and
>> > this is IMO clearer than putting the information directly in
>> > nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
>> > which are respectively encoding software hamming and software bch
>> > implementations).
>> >
>> > But shouldn't we take this even further and add new DT properties
>> > to encode the ECC layout (syndrome, oob_first), and another one to
>> > specify the implementation type (software or hardware)?
>> >
>> > nand-ecc-layout = "nornal", "syndrome" or "oob_first"
>> > nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
>> >
>> > Note that it's not something I ask you to do right now, I just want to
>> > bring the topic on the table.
>>
>> So far I was fine with whatever drvier/NAND core picked, didn't have
>> any issue with it. If at some point we'll see a real need of
>> specifying it manually as well, I guess we should do as you suggested.
>>
>
> My point is that it's kind of weird to have the same information
> duplicated in two different properties with possibly some conflicting
> configurations, like:
>
> nand-ecc-mode = "soft_bch"; /* software BCH implementation... */
> nand-ecc-algo = "hamming"; /* ... and here you choose Hamming */
>
> Of course this won't be a real problem until NAND core starts using
> this property to decide which soft implementation should be used, but
> providing a consistent binding is important IMO.
Sorry, I didn't pay enough attention to NAND properties and missed
your point. I was a bit surprised to see NAND_ECC_SOFT_BCH value and
"soft_bch" as mapping. This is indeed a bit confusing.
So I'm trying to find a proper way to split nand-ecc-mode. What you
suggested isn't exactly clear to me. AFAIR both enums: SYNDROME and
OOB_FIRST specify they way ECC info has to be accessed. For example
when using NAND_ECC_HW_OOB_FIRST you have to read OOB and after that
read data. I don't see how it's really related to the ECC layout (you
suggested "oob_first" as possible value of nand-ecc-layout).
What about:
1) Adding new nand-ecc-algo as this patch does
2) Deprecating "soft_bch" value from nand-ecc-mode property
--
Rafał
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-26 16:30 ` Rafał Miłecki
@ 2016-02-26 16:55 ` Boris Brezillon
-1 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-26 16:55 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On Fri, 26 Feb 2016 17:30:47 +0100
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 25 February 2016 at 21:09, Boris Brezillon
> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Thu, 25 Feb 2016 20:56:36 +0100
> > Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> On 24 February 2016 at 14:46, Boris Brezillon
> >> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> >> > On Fri, 12 Feb 2016 19:11:23 +0100
> >> > Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> >
> >> >> This allows specifying algorithm used for NAND ECC. There are two
> >> >> available values: "bch" and "hamming". It's important as having just
> >> >> ECC parameters (step, strength) isn't enough to determine algorithm,
> >> >> e.g. you can't distinct BCH-1 and Hamming.
> >> >>
> >> >> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >> ---
> >> >> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> >> >> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> >> >> include/linux/mtd/nand.h | 5 ++++
> >> >> include/linux/of_mtd.h | 6 +++++
> >> >> 4 files changed, 47 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> >> >> index b53f92e..a2c2df5 100644
> >> >> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> >> >> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> >> >> @@ -3,6 +3,9 @@
> >> >> - 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: "bch", "hamming". The default one is
> >> >> + "bch".
> >> >
> >> > I like the idea of specifying which ECC algorithm should be used, and
> >> > this is IMO clearer than putting the information directly in
> >> > nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
> >> > which are respectively encoding software hamming and software bch
> >> > implementations).
> >> >
> >> > But shouldn't we take this even further and add new DT properties
> >> > to encode the ECC layout (syndrome, oob_first), and another one to
> >> > specify the implementation type (software or hardware)?
> >> >
> >> > nand-ecc-layout = "nornal", "syndrome" or "oob_first"
> >> > nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
> >> >
> >> > Note that it's not something I ask you to do right now, I just want to
> >> > bring the topic on the table.
> >>
> >> So far I was fine with whatever drvier/NAND core picked, didn't have
> >> any issue with it. If at some point we'll see a real need of
> >> specifying it manually as well, I guess we should do as you suggested.
> >>
> >
> > My point is that it's kind of weird to have the same information
> > duplicated in two different properties with possibly some conflicting
> > configurations, like:
> >
> > nand-ecc-mode = "soft_bch"; /* software BCH implementation... */
> > nand-ecc-algo = "hamming"; /* ... and here you choose Hamming */
> >
> > Of course this won't be a real problem until NAND core starts using
> > this property to decide which soft implementation should be used, but
> > providing a consistent binding is important IMO.
>
> Sorry, I didn't pay enough attention to NAND properties and missed
> your point. I was a bit surprised to see NAND_ECC_SOFT_BCH value and
> "soft_bch" as mapping. This is indeed a bit confusing.
>
> So I'm trying to find a proper way to split nand-ecc-mode. What you
> suggested isn't exactly clear to me. AFAIR both enums: SYNDROME and
> OOB_FIRST specify they way ECC info has to be accessed. For example
> when using NAND_ECC_HW_OOB_FIRST you have to read OOB and after that
> read data. I don't see how it's really related to the ECC layout (you
> suggested "oob_first" as possible value of nand-ecc-layout).
Because it's encoding where the ECC bytes are stored in a NAND page.
Maybe ecc-layout is not the correct name (how about nand-page-layout),
but neither ecc-mode is ;-).
>
> What about:
> 1) Adding new nand-ecc-algo as this patch does
> 2) Deprecating "soft_bch" value from nand-ecc-mode property
>
Sounds good to me.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel 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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-26 16:55 ` Boris Brezillon
0 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-26 16:55 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, devicetree, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On Fri, 26 Feb 2016 17:30:47 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:
> On 25 February 2016 at 21:09, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Thu, 25 Feb 2016 20:56:36 +0100
> > Rafał Miłecki <zajec5@gmail.com> wrote:
> >
> >> On 24 February 2016 at 14:46, Boris Brezillon
> >> <boris.brezillon@free-electrons.com> wrote:
> >> > On Fri, 12 Feb 2016 19:11:23 +0100
> >> > Rafał Miłecki <zajec5@gmail.com> wrote:
> >> >
> >> >> This allows specifying algorithm used for NAND ECC. There are two
> >> >> available values: "bch" and "hamming". It's important as having just
> >> >> ECC parameters (step, strength) isn't enough to determine algorithm,
> >> >> e.g. you can't distinct BCH-1 and Hamming.
> >> >>
> >> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >> >> ---
> >> >> Documentation/devicetree/bindings/mtd/nand.txt | 3 +++
> >> >> drivers/of/of_mtd.c | 33 ++++++++++++++++++++++++++
> >> >> include/linux/mtd/nand.h | 5 ++++
> >> >> include/linux/of_mtd.h | 6 +++++
> >> >> 4 files changed, 47 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> >> >> index b53f92e..a2c2df5 100644
> >> >> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> >> >> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> >> >> @@ -3,6 +3,9 @@
> >> >> - 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: "bch", "hamming". The default one is
> >> >> + "bch".
> >> >
> >> > I like the idea of specifying which ECC algorithm should be used, and
> >> > this is IMO clearer than putting the information directly in
> >> > nand-ecc-mode (as is already done for the "soft" and "soft_bch" modes,
> >> > which are respectively encoding software hamming and software bch
> >> > implementations).
> >> >
> >> > But shouldn't we take this even further and add new DT properties
> >> > to encode the ECC layout (syndrome, oob_first), and another one to
> >> > specify the implementation type (software or hardware)?
> >> >
> >> > nand-ecc-layout = "nornal", "syndrome" or "oob_first"
> >> > nand-ecc-engine = "none", "soft" or "hw" ("on-die" ?)
> >> >
> >> > Note that it's not something I ask you to do right now, I just want to
> >> > bring the topic on the table.
> >>
> >> So far I was fine with whatever drvier/NAND core picked, didn't have
> >> any issue with it. If at some point we'll see a real need of
> >> specifying it manually as well, I guess we should do as you suggested.
> >>
> >
> > My point is that it's kind of weird to have the same information
> > duplicated in two different properties with possibly some conflicting
> > configurations, like:
> >
> > nand-ecc-mode = "soft_bch"; /* software BCH implementation... */
> > nand-ecc-algo = "hamming"; /* ... and here you choose Hamming */
> >
> > Of course this won't be a real problem until NAND core starts using
> > this property to decide which soft implementation should be used, but
> > providing a consistent binding is important IMO.
>
> Sorry, I didn't pay enough attention to NAND properties and missed
> your point. I was a bit surprised to see NAND_ECC_SOFT_BCH value and
> "soft_bch" as mapping. This is indeed a bit confusing.
>
> So I'm trying to find a proper way to split nand-ecc-mode. What you
> suggested isn't exactly clear to me. AFAIR both enums: SYNDROME and
> OOB_FIRST specify they way ECC info has to be accessed. For example
> when using NAND_ECC_HW_OOB_FIRST you have to read OOB and after that
> read data. I don't see how it's really related to the ECC layout (you
> suggested "oob_first" as possible value of nand-ecc-layout).
Because it's encoding where the ECC bytes are stored in a NAND page.
Maybe ecc-layout is not the correct name (how about nand-page-layout),
but neither ecc-mode is ;-).
>
> What about:
> 1) Adding new nand-ecc-algo as this patch does
> 2) Deprecating "soft_bch" value from nand-ecc-mode property
>
Sounds good to me.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-26 16:55 ` Boris Brezillon
@ 2016-02-26 21:24 ` Rafał Miłecki
-1 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-26 21:24 UTC (permalink / raw)
To: Boris Brezillon
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On 26 February 2016 at 17:55, Boris Brezillon
<boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Fri, 26 Feb 2016 17:30:47 +0100
> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> What about:
>> 1) Adding new nand-ecc-algo as this patch does
>> 2) Deprecating "soft_bch" value from nand-ecc-mode property
>>
>
> Sounds good to me.
I have problems understanding "soft" values which maps to the
NAND_ECC_SOFT. What exactly algorithm is that? It uses functions from
drivers/mtd/nand/nand_ecc.c but this file isn't really helpful. I was
only able to find "3-byte ECC" phase. Kconfig config MTD_NAND_ECC also
doesn't have any description. I also tried looking at
Documentation/mtd/nand_ecc.txt but still no luck.
Does this algorithm has any name?
--
Rafał
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-26 21:24 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-02-26 21:24 UTC (permalink / raw)
To: Boris Brezillon
Cc: Brian Norris, linux-mtd, devicetree, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On 26 February 2016 at 17:55, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Fri, 26 Feb 2016 17:30:47 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>> What about:
>> 1) Adding new nand-ecc-algo as this patch does
>> 2) Deprecating "soft_bch" value from nand-ecc-mode property
>>
>
> Sounds good to me.
I have problems understanding "soft" values which maps to the
NAND_ECC_SOFT. What exactly algorithm is that? It uses functions from
drivers/mtd/nand/nand_ecc.c but this file isn't really helpful. I was
only able to find "3-byte ECC" phase. Kconfig config MTD_NAND_ECC also
doesn't have any description. I also tried looking at
Documentation/mtd/nand_ecc.txt but still no luck.
Does this algorithm has any name?
--
Rafał
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-26 21:24 ` Rafał Miłecki
@ 2016-02-26 21:28 ` Boris Brezillon
-1 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-26 21:28 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On Fri, 26 Feb 2016 22:24:21 +0100
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 26 February 2016 at 17:55, Boris Brezillon
> <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Fri, 26 Feb 2016 17:30:47 +0100
> > Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> What about:
> >> 1) Adding new nand-ecc-algo as this patch does
> >> 2) Deprecating "soft_bch" value from nand-ecc-mode property
> >>
> >
> > Sounds good to me.
>
> I have problems understanding "soft" values which maps to the
> NAND_ECC_SOFT. What exactly algorithm is that? It uses functions from
> drivers/mtd/nand/nand_ecc.c but this file isn't really helpful. I was
> only able to find "3-byte ECC" phase. Kconfig config MTD_NAND_ECC also
> doesn't have any description. I also tried looking at
> Documentation/mtd/nand_ecc.txt but still no luck.
>
> Does this algorithm has any name?
>
AFAIR, nand_ecc.c is implementing Hamming ECC.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel 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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-02-26 21:28 ` Boris Brezillon
0 siblings, 0 replies; 43+ messages in thread
From: Boris Brezillon @ 2016-02-26 21:28 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Brian Norris, linux-mtd, devicetree, Kamal Dasu, Hauke Mehrtens,
Rob Herring, Grant Likely, Frank Rowand
On Fri, 26 Feb 2016 22:24:21 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:
> On 26 February 2016 at 17:55, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Fri, 26 Feb 2016 17:30:47 +0100
> > Rafał Miłecki <zajec5@gmail.com> wrote:
> >> What about:
> >> 1) Adding new nand-ecc-algo as this patch does
> >> 2) Deprecating "soft_bch" value from nand-ecc-mode property
> >>
> >
> > Sounds good to me.
>
> I have problems understanding "soft" values which maps to the
> NAND_ECC_SOFT. What exactly algorithm is that? It uses functions from
> drivers/mtd/nand/nand_ecc.c but this file isn't really helpful. I was
> only able to find "3-byte ECC" phase. Kconfig config MTD_NAND_ECC also
> doesn't have any description. I also tried looking at
> Documentation/mtd/nand_ecc.txt but still no luck.
>
> Does this algorithm has any name?
>
AFAIR, nand_ecc.c is implementing Hamming ECC.
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-04-01 16:01 ` Brian Norris
-1 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:01 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Hauke Mehrtens,
Kamal Dasu, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 07:11:23PM +0100, Rafał Miłecki wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Brian Norris <computersforpeace-dVM3bRS+LDk@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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT
@ 2016-04-01 16:01 ` Brian Norris
0 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:01 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd, Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree
On Fri, Feb 12, 2016 at 07:11:23PM +0100, Rafał Miłecki wrote:
> This allows specifying algorithm used for NAND ECC. There are two
> available values: "bch" and "hamming". It's important as having just
> ECC parameters (step, strength) isn't enough to determine algorithm,
> e.g. you can't distinct BCH-1 and Hamming.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Brian Norris <computersforpeace@gmai.com>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-04-01 16:02 ` Brian Norris
-1 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:02 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Hauke Mehrtens,
Kamal Dasu, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
> This will allow drivers handle ECC properly.
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Brian Norris <computersforpeace-dVM3bRS+LDk@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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
@ 2016-04-01 16:02 ` Brian Norris
0 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:02 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd, Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree
On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
> This will allow drivers handle ECC properly.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Brian Norris <computersforpeace@gmai.com>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-04-01 16:07 ` Brian Norris
-1 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:07 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Hauke Mehrtens,
Kamal Dasu, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
> This will allow drivers handle ECC properly.
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/mtd/nand/nand_base.c | 6 +++++-
> include/linux/mtd/nand.h | 1 +
> 2 files changed, 6 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;
> +
While this might appear to handle the absence of the nand-ecc-algo
property correctly, this isn't safe. This means that any existing DT
without the property will get treated as Hamming ECC. Perhaps we need
the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
that?
Brian
> if (ecc_strength >= 0)
> chip->ecc.strength = ecc_strength;
>
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 25854d2..8deca1b 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -513,6 +513,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
>
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
@ 2016-04-01 16:07 ` Brian Norris
0 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:07 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd, Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree
On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
> This will allow drivers handle ECC properly.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> drivers/mtd/nand/nand_base.c | 6 +++++-
> include/linux/mtd/nand.h | 1 +
> 2 files changed, 6 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;
> +
While this might appear to handle the absence of the nand-ecc-algo
property correctly, this isn't safe. This means that any existing DT
without the property will get treated as Hamming ECC. Perhaps we need
the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
that?
Brian
> if (ecc_strength >= 0)
> chip->ecc.strength = ecc_strength;
>
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 25854d2..8deca1b 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -513,6 +513,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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 3/3] mtd: brcmnand: fix check for Hamming algorithm
2016-02-12 18:11 ` Rafał Miłecki
@ 2016-04-01 16:07 ` Brian Norris
-1 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:07 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Hauke Mehrtens,
Kamal Dasu, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Feb 12, 2016 at 07:11:25PM +0100, Rafał Miłecki wrote:
> So far we were treating ECC strength 1 as Hamming algorithm. It didn't
> supporting some less common devices with BCH-1 (e.g. D-Link DIR-885L).
>
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> 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 844fc07..b8055da 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1842,7 +1842,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)
It's probably best we do a little more than this. Right now, this allows
someone to specify:
nand-ecc-strength = <20>;
nand-ecc-stepsize = <512>;
nand-ecc-algo = "hamming";
nand-ecc-mode = "hw";
And they'll end up with 1-bit hamming ECC, except nand_base will still
think we have 20-bit correction. I think we need to add a check in this
driver to be sure we haven't selected >1-bit correction with hamming
ECC.
Brian
> cfg->ecc_level = 15;
> else
> cfg->ecc_level = chip->ecc.strength;
> --
> 1.8.4.5
>
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 3/3] mtd: brcmnand: fix check for Hamming algorithm
@ 2016-04-01 16:07 ` Brian Norris
0 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 16:07 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd, Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree
On Fri, Feb 12, 2016 at 07:11:25PM +0100, Rafał Miłecki wrote:
> So far we were treating ECC strength 1 as Hamming algorithm. It didn't
> supporting some less common 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 844fc07..b8055da 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1842,7 +1842,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)
It's probably best we do a little more than this. Right now, this allows
someone to specify:
nand-ecc-strength = <20>;
nand-ecc-stepsize = <512>;
nand-ecc-algo = "hamming";
nand-ecc-mode = "hw";
And they'll end up with 1-bit hamming ECC, except nand_base will still
think we have 20-bit correction. I think we need to add a check in this
driver to be sure we haven't selected >1-bit correction with hamming
ECC.
Brian
> cfg->ecc_level = 15;
> else
> cfg->ecc_level = chip->ecc.strength;
> --
> 1.8.4.5
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
2016-04-01 16:07 ` Brian Norris
@ 2016-04-01 19:32 ` Rafał Miłecki
-1 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-04-01 19:32 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Hauke Mehrtens,
Kamal Dasu, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On 1 April 2016 at 18:07, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
>> This will allow drivers handle ECC properly.
>>
>> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> drivers/mtd/nand/nand_base.c | 6 +++++-
>> include/linux/mtd/nand.h | 1 +
>> 2 files changed, 6 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;
>> +
>
> While this might appear to handle the absence of the nand-ecc-algo
> property correctly, this isn't safe. This means that any existing DT
> without the property will get treated as Hamming ECC. Perhaps we need
> the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
> that?
You're commenting on an old series. If you take a look at:
https://patchwork.ozlabs.org/patch/601180/
that also got applied to the:
https://github.com/linux-nand/linux/commits/nand/next
repo, you'll see we have NAND_ECC_UNKNOWN there.
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
@ 2016-04-01 19:32 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-04-01 19:32 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd, Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree
On 1 April 2016 at 18:07, Brian Norris <computersforpeace@gmail.com> wrote:
> On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
>> This will allow drivers handle ECC properly.
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>> drivers/mtd/nand/nand_base.c | 6 +++++-
>> include/linux/mtd/nand.h | 1 +
>> 2 files changed, 6 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;
>> +
>
> While this might appear to handle the absence of the nand-ecc-algo
> property correctly, this isn't safe. This means that any existing DT
> without the property will get treated as Hamming ECC. Perhaps we need
> the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
> that?
You're commenting on an old series. If you take a look at:
https://patchwork.ozlabs.org/patch/601180/
that also got applied to the:
https://github.com/linux-nand/linux/commits/nand/next
repo, you'll see we have NAND_ECC_UNKNOWN there.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
2016-04-01 19:32 ` Rafał Miłecki
@ 2016-04-01 20:07 ` Brian Norris
-1 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 20:07 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Hauke Mehrtens,
Kamal Dasu, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Fri, Apr 01, 2016 at 09:32:40PM +0200, Rafał Miłecki wrote:
> On 1 April 2016 at 18:07, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
> >> This will allow drivers handle ECC properly.
> >>
> >> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >> drivers/mtd/nand/nand_base.c | 6 +++++-
> >> include/linux/mtd/nand.h | 1 +
> >> 2 files changed, 6 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;
> >> +
> >
> > While this might appear to handle the absence of the nand-ecc-algo
> > property correctly, this isn't safe. This means that any existing DT
> > without the property will get treated as Hamming ECC. Perhaps we need
> > the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
> > that?
>
> You're commenting on an old series. If you take a look at:
> https://patchwork.ozlabs.org/patch/601180/
> that also got applied to the:
> https://github.com/linux-nand/linux/commits/nand/next
> repo, you'll see we have NAND_ECC_UNKNOWN there.
Ah, I guess I missed that because I was searching for the patch to
brcmnand (i.e., fixing the original problem you saw). AFAICT, this v2
series doesn't actually resolve your issue with brcmnand, no?
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
@ 2016-04-01 20:07 ` Brian Norris
0 siblings, 0 replies; 43+ messages in thread
From: Brian Norris @ 2016-04-01 20:07 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd, Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree
On Fri, Apr 01, 2016 at 09:32:40PM +0200, Rafał Miłecki wrote:
> On 1 April 2016 at 18:07, Brian Norris <computersforpeace@gmail.com> wrote:
> > On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
> >> This will allow drivers handle ECC properly.
> >>
> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >> ---
> >> drivers/mtd/nand/nand_base.c | 6 +++++-
> >> include/linux/mtd/nand.h | 1 +
> >> 2 files changed, 6 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;
> >> +
> >
> > While this might appear to handle the absence of the nand-ecc-algo
> > property correctly, this isn't safe. This means that any existing DT
> > without the property will get treated as Hamming ECC. Perhaps we need
> > the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
> > that?
>
> You're commenting on an old series. If you take a look at:
> https://patchwork.ozlabs.org/patch/601180/
> that also got applied to the:
> https://github.com/linux-nand/linux/commits/nand/next
> repo, you'll see we have NAND_ECC_UNKNOWN there.
Ah, I guess I missed that because I was searching for the patch to
brcmnand (i.e., fixing the original problem you saw). AFAICT, this v2
series doesn't actually resolve your issue with brcmnand, no?
Brian
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
2016-04-01 20:07 ` Brian Norris
@ 2016-04-01 21:23 ` Rafał Miłecki
-1 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-04-01 21:23 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Hauke Mehrtens,
Kamal Dasu, Rob Herring, Frank Rowand, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA
On 1 April 2016 at 22:07, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Apr 01, 2016 at 09:32:40PM +0200, Rafał Miłecki wrote:
>> On 1 April 2016 at 18:07, Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
>> >> This will allow drivers handle ECC properly.
>> >>
>> >> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >> ---
>> >> drivers/mtd/nand/nand_base.c | 6 +++++-
>> >> include/linux/mtd/nand.h | 1 +
>> >> 2 files changed, 6 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;
>> >> +
>> >
>> > While this might appear to handle the absence of the nand-ecc-algo
>> > property correctly, this isn't safe. This means that any existing DT
>> > without the property will get treated as Hamming ECC. Perhaps we need
>> > the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
>> > that?
>>
>> You're commenting on an old series. If you take a look at:
>> https://patchwork.ozlabs.org/patch/601180/
>> that also got applied to the:
>> https://github.com/linux-nand/linux/commits/nand/next
>> repo, you'll see we have NAND_ECC_UNKNOWN there.
>
> Ah, I guess I missed that because I was searching for the patch to
> brcmnand (i.e., fixing the original problem you saw). AFAICT, this v2
> series doesn't actually resolve your issue with brcmnand, no?
That's correct. I'm planning to clean NAND subsystem first, then patch brcmnand.
--
Rafał
--
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
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm
@ 2016-04-01 21:23 ` Rafał Miłecki
0 siblings, 0 replies; 43+ messages in thread
From: Rafał Miłecki @ 2016-04-01 21:23 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd, Hauke Mehrtens, Kamal Dasu, Rob Herring, Frank Rowand,
Grant Likely, devicetree
On 1 April 2016 at 22:07, Brian Norris <computersforpeace@gmail.com> wrote:
> On Fri, Apr 01, 2016 at 09:32:40PM +0200, Rafał Miłecki wrote:
>> On 1 April 2016 at 18:07, Brian Norris <computersforpeace@gmail.com> wrote:
>> > On Fri, Feb 12, 2016 at 07:11:24PM +0100, Rafał Miłecki wrote:
>> >> This will allow drivers handle ECC properly.
>> >>
>> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> >> ---
>> >> drivers/mtd/nand/nand_base.c | 6 +++++-
>> >> include/linux/mtd/nand.h | 1 +
>> >> 2 files changed, 6 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;
>> >> +
>> >
>> > While this might appear to handle the absence of the nand-ecc-algo
>> > property correctly, this isn't safe. This means that any existing DT
>> > without the property will get treated as Hamming ECC. Perhaps we need
>> > the enum to have a 0 value of 'NAND_ECC_ALGO_UNKNOWN' or something like
>> > that?
>>
>> You're commenting on an old series. If you take a look at:
>> https://patchwork.ozlabs.org/patch/601180/
>> that also got applied to the:
>> https://github.com/linux-nand/linux/commits/nand/next
>> repo, you'll see we have NAND_ECC_UNKNOWN there.
>
> Ah, I guess I missed that because I was searching for the patch to
> brcmnand (i.e., fixing the original problem you saw). AFAICT, this v2
> series doesn't actually resolve your issue with brcmnand, no?
That's correct. I'm planning to clean NAND subsystem first, then patch brcmnand.
--
Rafał
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2016-04-01 21:24 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-12 18:11 [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT Rafał Miłecki
2016-02-12 18:11 ` Rafał Miłecki
2016-02-15 21:28 ` Kamal Dasu
[not found] ` <1455300685-27009-1-git-send-email-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-12 18:11 ` [PATCH 2/3] mtd: nand: read (from DT) and store ECC algorithm Rafał Miłecki
2016-02-12 18:11 ` Rafał Miłecki
[not found] ` <1455300685-27009-2-git-send-email-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-15 21:31 ` Kamal Dasu
2016-02-15 21:31 ` Kamal Dasu
2016-04-01 16:02 ` Brian Norris
2016-04-01 16:02 ` Brian Norris
2016-04-01 16:07 ` Brian Norris
2016-04-01 16:07 ` Brian Norris
[not found] ` <20160401160722.GC117117-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2016-04-01 19:32 ` Rafał Miłecki
2016-04-01 19:32 ` Rafał Miłecki
[not found] ` <CACna6rz6mYSxAr1oSjcLc__sSKM1XdUf-cGOFanGvuij0nNC5Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-04-01 20:07 ` Brian Norris
2016-04-01 20:07 ` Brian Norris
2016-04-01 21:23 ` Rafał Miłecki
2016-04-01 21:23 ` Rafał Miłecki
2016-02-12 18:11 ` [PATCH 3/3] mtd: brcmnand: fix check for Hamming algorithm Rafał Miłecki
2016-02-12 18:11 ` Rafał Miłecki
[not found] ` <1455300685-27009-3-git-send-email-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-02-15 21:30 ` Kamal Dasu
2016-02-15 21:30 ` Kamal Dasu
2016-04-01 16:07 ` Brian Norris
2016-04-01 16:07 ` Brian Norris
2016-02-15 21:43 ` [PATCH 1/3] of: mtd: add helper reading "nand-ecc-algo" from DT Kamal Dasu
2016-02-15 21:43 ` Kamal Dasu
2016-02-22 2:54 ` Rob Herring
2016-02-22 2:54 ` Rob Herring
2016-02-24 13:46 ` Boris Brezillon
2016-02-24 13:46 ` Boris Brezillon
2016-02-25 19:56 ` Rafał Miłecki
2016-02-25 19:56 ` Rafał Miłecki
[not found] ` <CACna6rxemvBOu9fy9KP0327TsqonO7dFY5EsQjf1k9Ln6hX8cw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-25 20:09 ` Boris Brezillon
2016-02-25 20:09 ` Boris Brezillon
2016-02-26 16:30 ` Rafał Miłecki
2016-02-26 16:30 ` Rafał Miłecki
[not found] ` <CACna6ryVuqSLv8WdzzGUQzpA-F35JxXoHoa2jcLzZeHaXtoexA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-26 16:55 ` Boris Brezillon
2016-02-26 16:55 ` Boris Brezillon
2016-02-26 21:24 ` Rafał Miłecki
2016-02-26 21:24 ` Rafał Miłecki
[not found] ` <CACna6rzRROfYwagsBGo5b8d_bydUBrJ=9RvGZSt1G85xP=gKaw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-26 21:28 ` Boris Brezillon
2016-02-26 21:28 ` Boris Brezillon
2016-04-01 16:01 ` Brian Norris
2016-04-01 16:01 ` Brian Norris
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.