[PATCH v6 0/4] mtd: devices: elm: add checks ELM H/W constrains, driver code cleanup

[PATCH v6 0/4] mtd: devices: elm: add checks ELM H/W constrains, driver code cleanup

[Pekon Gupta]

*changes v5 -> v6*
[PATCH 02/04] minor cleanup

*changes v4 -> v5*
This patch series is split version from earlier series [1].
This series refactors and cleans ELM driver which is used by Hardware
based BCHx ecc-schemes.
 - Undo: introduction of 'struct mtd_info' and 'struct nand_chip'. Instead
         keep ELM driver independent of mtd_info and nand_chip structs and
         pass only required ECC configurations as elm_config() arguments
         elm_config(..., int ecc_steps, int ecc_step_size, int ecc_step_bytes)
 - Undo: re-writing of elm_load_syndrome() ECC register configurations.

*changes v3 -> v4 [1]*
 - in-corporated feedbacks from Brian Norris <computersforpeace@gmail.com>
 - updated: use 'pr_fmt(fmt)' to suffix DRIVER_NAME
 - removed: local 'eccsteps' in ELM driver, instead using nand_chip->ecc.steps
 - undo: irrelavant white-space changes

[1] http://lists.infradead.org/pipermail/linux-mtd/2013-November/050242.html

Pekon Gupta (4):
  mtd: devices: elm: check for hardware engine's design constrains
  mtd: devices: elm: clean elm_load_syndrome
  mtd: devices: elm: configure parallel channels based on ecc_steps
  mtd: devices: elm: update DRIVER_NAME as "omap-elm"

 drivers/mtd/devices/elm.c         | 44 +++++++++++++++++++++++++++++----------
 drivers/mtd/nand/omap2.c          |  9 +++++---
 include/linux/platform_data/elm.h | 10 ++-------
 3 files changed, 41 insertions(+), 22 deletions(-)

-- 
1.8.5.1.163.gd7aced9

[PATCH v6 1/4] mtd: devices: elm: check for hardware engine's design constrains

[Pekon Gupta]

ELM hardware engine is used by BCH ecc-schemes for detecting and locating
ECC errors. This patch adds following checks as per ELM design constrains:

 - ELM internal buffers are of 1K,
   So it cannot process data with ecc-step-size > 1K.

 - ELM engine can execute upto maximum of 8 threads in parallel,
   So in *page-mode* (when complete page is processed in single iteration),
   ELM cannot support ecc-steps > 8.

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/devices/elm.c         | 19 ++++++++++++++++++-
 drivers/mtd/nand/omap2.c          |  9 ++++++---
 include/linux/platform_data/elm.h |  3 ++-
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
index f160d2c..7fda50f 100644
--- a/drivers/mtd/devices/elm.c
+++ b/drivers/mtd/devices/elm.c
@@ -84,6 +84,9 @@ struct elm_info {
 	struct list_head list;
 	enum bch_ecc bch_type;
 	struct elm_registers elm_regs;
+	int ecc_steps;
+	int ecc_step_size;
+	int ecc_step_bytes;
 };
 
 static LIST_HEAD(elm_devices);
@@ -103,7 +106,8 @@ static u32 elm_read_reg(struct elm_info *info, int offset)
  * @dev:	ELM device
  * @bch_type:	Type of BCH ecc
  */
-int elm_config(struct device *dev, enum bch_ecc bch_type)
+int elm_config(struct device *dev, enum bch_ecc bch_type,
+	int ecc_steps, int ecc_step_size, int ecc_step_bytes)
 {
 	u32 reg_val;
 	struct elm_info *info = dev_get_drvdata(dev);
@@ -112,10 +116,23 @@ int elm_config(struct device *dev, enum bch_ecc bch_type)
 		dev_err(dev, "Unable to configure elm - device not probed?\n");
 		return -ENODEV;
 	}
+	/* ELM cannot detect ECC errors for chunks > 1KB */
+	if (ecc_step_size > (ELM_ECC_SIZE/2 + 1)) {
+		pr_err("unsupported config ecc-size=%d", ecc_step_size);
+		return -EINVAL;
+	}
+	/* ELM support 8 error syndrome process */
+	if (ecc_steps > ERROR_VECTOR_MAX) {
+		pr_err("unsupported config ecc-step=%d", ecc_steps);
+		return -EINVAL;
+	}
 
 	reg_val = (bch_type & ECC_BCH_LEVEL_MASK) | (ELM_ECC_SIZE << 16);
 	elm_write_reg(info, ELM_LOCATION_CONFIG, reg_val);
 	info->bch_type = bch_type;
+	info->ecc_steps		= ecc_steps;
+	info->ecc_step_size	= ecc_step_size;
+	info->ecc_step_bytes	= ecc_step_bytes;
 
 	return 0;
 }
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 5ce2097..369aee7 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1546,6 +1546,8 @@ static int is_elm_present(struct omap_nand_info *info,
 			struct device_node *elm_node, enum bch_ecc bch_type)
 {
 	struct platform_device *pdev;
+	struct nand_ecc_ctrl *ecc = &info->nand.ecc;
+	int err = 0;
 	/* check whether elm-id is passed via DT */
 	if (!elm_node) {
 		pr_err("nand: error: ELM DT node not found\n");
@@ -1559,9 +1561,10 @@ static int is_elm_present(struct omap_nand_info *info,
 	}
 	/* ELM module available, now configure it */
 	info->elm_dev = &pdev->dev;
-	if (elm_config(info->elm_dev, bch_type))
-		return -ENODEV;
-	return 0;
+	err = elm_config(info->elm_dev, bch_type,
+		(info->mtd.writesize / ecc->size), ecc->size, ecc->bytes);
+
+	return err;
 }
 #endif /* CONFIG_MTD_NAND_ECC_BCH */
 
diff --git a/include/linux/platform_data/elm.h b/include/linux/platform_data/elm.h
index bf0a83b..b824ff3 100644
--- a/include/linux/platform_data/elm.h
+++ b/include/linux/platform_data/elm.h
@@ -50,5 +50,6 @@ struct elm_errorvec {
 
 void elm_decode_bch_error_page(struct device *dev, u8 *ecc_calc,
 		struct elm_errorvec *err_vec);
-int elm_config(struct device *dev, enum bch_ecc bch_type);
+int elm_config(struct device *dev, enum bch_ecc bch_type,
+	int ecc_steps, int ecc_step_size, int ecc_step_bytes);
 #endif /* __ELM_H */
-- 
1.8.5.1.163.gd7aced9

[PATCH v6 2/4] mtd: devices: elm: clean elm_load_syndrome

[Pekon Gupta]

 - replace ECC_BYTES* macros with ecc->bytes
 - Add scalability for future ecc-schemes

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/devices/elm.c         | 14 ++++++++------
 include/linux/platform_data/elm.h |  7 -------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
index 7fda50f..f59c100 100644
--- a/drivers/mtd/devices/elm.c
+++ b/drivers/mtd/devices/elm.c
@@ -181,10 +181,8 @@ static void elm_load_syndrome(struct elm_info *info,
 			elm_configure_page_mode(info, i, true);
 			offset = ELM_SYNDROME_FRAGMENT_0 +
 				SYNDROME_FRAGMENT_REG_SIZE * i;
-
-			/* BCH8 */
-			if (info->bch_type) {
-
+			switch (info->bch_type) {
+			case BCH8_ECC:
 				/* syndrome fragment 0 = ecc[9-12B] */
 				val = cpu_to_be32(*(u32 *) &ecc[9]);
 				elm_write_reg(info, offset, val);
@@ -203,7 +201,8 @@ static void elm_load_syndrome(struct elm_info *info,
 				offset += 4;
 				val = ecc[0];
 				elm_write_reg(info, offset, val);
-			} else {
+				break;
+			case BCH4_ECC:
 				/* syndrome fragment 0 = ecc[20-52b] bits */
 				val = (cpu_to_be32(*(u32 *) &ecc[3]) >> 4) |
 					((ecc[2] & 0xf) << 28);
@@ -213,11 +212,14 @@ static void elm_load_syndrome(struct elm_info *info,
 				offset += 4;
 				val = cpu_to_be32(*(u32 *) &ecc[0]) >> 12;
 				elm_write_reg(info, offset, val);
+				break;
+			default:
+				pr_err("invalid config bch_type\n");
 			}
 		}
 
 		/* Update ecc pointer with ecc byte size */
-		ecc += info->bch_type ? BCH8_SIZE : BCH4_SIZE;
+		ecc += info->ecc_step_bytes;
 	}
 }
 
diff --git a/include/linux/platform_data/elm.h b/include/linux/platform_data/elm.h
index b824ff3..dc0218d 100644
--- a/include/linux/platform_data/elm.h
+++ b/include/linux/platform_data/elm.h
@@ -26,13 +26,6 @@ enum bch_ecc {
 /* ELM support 8 error syndrome process */
 #define ERROR_VECTOR_MAX		8
 
-#define BCH8_ECC_OOB_BYTES		13
-#define BCH4_ECC_OOB_BYTES		7
-/* RBL requires 14 byte even though BCH8 uses only 13 byte */
-#define BCH8_SIZE			(BCH8_ECC_OOB_BYTES + 1)
-/* Uses 1 extra byte to handle erased pages */
-#define BCH4_SIZE			(BCH4_ECC_OOB_BYTES + 1)
-
 /**
  * struct elm_errorvec - error vector for elm
  * @error_reported:		set true for vectors error is reported
-- 
1.8.5.1.163.gd7aced9

[PATCH v6 3/4] mtd: devices: elm: configure parallel channels based on ecc_steps

[Pekon Gupta]

Though ELM hardware can process upto maximum of 8 channels in parallel for
ECC error detection. But actual number of channels need to be processed in
parallel depends on ecc_steps (for page mode configuration).

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/devices/elm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
index f59c100..43fd81d 100644
--- a/drivers/mtd/devices/elm.c
+++ b/drivers/mtd/devices/elm.c
@@ -174,7 +174,7 @@ static void elm_load_syndrome(struct elm_info *info,
 	int i, offset;
 	u32 val;
 
-	for (i = 0; i < ERROR_VECTOR_MAX; i++) {
+	for (i = 0; i < info->ecc_steps; i++) {
 
 		/* Check error reported */
 		if (err_vec[i].error_reported) {
@@ -242,7 +242,7 @@ static void elm_start_processing(struct elm_info *info,
 	 * Set syndrome vector valid, so that ELM module
 	 * will process it for vectors error is reported
 	 */
-	for (i = 0; i < ERROR_VECTOR_MAX; i++) {
+	for (i = 0; i < info->ecc_steps; i++) {
 		if (err_vec[i].error_reported) {
 			offset = ELM_SYNDROME_FRAGMENT_6 +
 				SYNDROME_FRAGMENT_REG_SIZE * i;
@@ -271,7 +271,7 @@ static void elm_error_correction(struct elm_info *info,
 	int offset;
 	u32 reg_val;
 
-	for (i = 0; i < ERROR_VECTOR_MAX; i++) {
+	for (i = 0; i < info->ecc_steps; i++) {
 
 		/* Check error reported */
 		if (err_vec[i].error_reported) {
-- 
1.8.5.1.163.gd7aced9

[PATCH v6 4/4] mtd: devices: elm: update DRIVER_NAME as "omap-elm"

[Pekon Gupta]

use "omap-elm" as DRIVER_NAME
prefix DRIVER_NAME in debug and error messages

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/devices/elm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
index 43fd81d..16e62ae 100644
--- a/drivers/mtd/devices/elm.c
+++ b/drivers/mtd/devices/elm.c
@@ -15,6 +15,9 @@
  *
  */
 
+#define	DRIVER_NAME	"omap-elm"
+#define pr_fmt(fmt)	DRIVER_NAME ": " fmt
+
 #include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
@@ -522,7 +525,7 @@ MODULE_DEVICE_TABLE(of, elm_of_match);
 
 static struct platform_driver elm_driver = {
 	.driver	= {
-		.name	= "elm",
+		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(elm_of_match),
 		.pm	= &elm_pm_ops,
-- 
1.8.5.1.163.gd7aced9

Re: [PATCH v6 1/4] mtd: devices: elm: check for hardware engine's design constrains

[Ezequiel Garcia]

Hello Pekon,

On Mar 07, Pekon Gupta wrote:
> ELM hardware engine is used by BCH ecc-schemes for detecting and locating
> ECC errors. This patch adds following checks as per ELM design constrains:
> 

s/constrains/constraints

>  - ELM internal buffers are of 1K,
>    So it cannot process data with ecc-step-size > 1K.
> 
>  - ELM engine can execute upto maximum of 8 threads in parallel,
>    So in *page-mode* (when complete page is processed in single iteration),
>    ELM cannot support ecc-steps > 8.
> 

Altough it's just a nitpick, I think you can work a bit on your explanations.
For instance, you start with a capital letter after a comma (1K, So it..).

And you have some incomplete sentences (This patch adds *the* following).

I'm not a native english speaker, so I know this can be hard! It's not a hard
requirement for the patch, but rather just a suggestion to improve your upstream
work.

I have a few comments below.

> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/devices/elm.c         | 19 ++++++++++++++++++-
>  drivers/mtd/nand/omap2.c          |  9 ++++++---
>  include/linux/platform_data/elm.h |  3 ++-
>  3 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
> index f160d2c..7fda50f 100644
> --- a/drivers/mtd/devices/elm.c
> +++ b/drivers/mtd/devices/elm.c
> @@ -84,6 +84,9 @@ struct elm_info {
>  	struct list_head list;
>  	enum bch_ecc bch_type;
>  	struct elm_registers elm_regs;
> +	int ecc_steps;
> +	int ecc_step_size;
> +	int ecc_step_bytes;
>  };
>  
>  static LIST_HEAD(elm_devices);
> @@ -103,7 +106,8 @@ static u32 elm_read_reg(struct elm_info *info, int offset)
>   * @dev:	ELM device
>   * @bch_type:	Type of BCH ecc
>   */
> -int elm_config(struct device *dev, enum bch_ecc bch_type)
> +int elm_config(struct device *dev, enum bch_ecc bch_type,
> +	int ecc_steps, int ecc_step_size, int ecc_step_bytes)
>  {
>  	u32 reg_val;
>  	struct elm_info *info = dev_get_drvdata(dev);
> @@ -112,10 +116,23 @@ int elm_config(struct device *dev, enum bch_ecc bch_type)
>  		dev_err(dev, "Unable to configure elm - device not probed?\n");

You are using dev_err on this function...

>  		return -ENODEV;
>  	}
> +	/* ELM cannot detect ECC errors for chunks > 1KB */
> +	if (ecc_step_size > (ELM_ECC_SIZE/2 + 1)) {
> +		pr_err("unsupported config ecc-size=%d", ecc_step_size);

... but then you use pr_err?

I think it's better to simply use dev_err() whenever possible as it carries
more information.

> +		return -EINVAL;
> +	}
> +	/* ELM support 8 error syndrome process */
> +	if (ecc_steps > ERROR_VECTOR_MAX) {
> +		pr_err("unsupported config ecc-step=%d", ecc_steps);

Ditto.

> +		return -EINVAL;
> +	}
>  
>  	reg_val = (bch_type & ECC_BCH_LEVEL_MASK) | (ELM_ECC_SIZE << 16);
>  	elm_write_reg(info, ELM_LOCATION_CONFIG, reg_val);
>  	info->bch_type = bch_type;
> +	info->ecc_steps		= ecc_steps;
> +	info->ecc_step_size	= ecc_step_size;
> +	info->ecc_step_bytes	= ecc_step_bytes;
>  

You're not using this values anywhere but in elm_config, at least in this
patch. I'd suggest that you remove the new field introduction here and
instead introduce it in the same patch you're using it.

Otherwise, you just confuse reviewers :)

>  	return 0;
>  }
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 5ce2097..369aee7 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1546,6 +1546,8 @@ static int is_elm_present(struct omap_nand_info *info,
>  			struct device_node *elm_node, enum bch_ecc bch_type)
>  {
>  	struct platform_device *pdev;
> +	struct nand_ecc_ctrl *ecc = &info->nand.ecc;
> +	int err = 0;

Are you sure you need to initialize err here?

Thanks and sorry for reviewing this only now, after six rounds.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v6 4/4] mtd: devices: elm: update DRIVER_NAME as "omap-elm"

[Ezequiel Garcia]

On Mar 07, Pekon Gupta wrote:
> use "omap-elm" as DRIVER_NAME
> prefix DRIVER_NAME in debug and error messages
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/devices/elm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
> index 43fd81d..16e62ae 100644
> --- a/drivers/mtd/devices/elm.c
> +++ b/drivers/mtd/devices/elm.c
> @@ -15,6 +15,9 @@
>   *
>   */
>  
> +#define	DRIVER_NAME	"omap-elm"
> +#define pr_fmt(fmt)	DRIVER_NAME ": " fmt
> +

As per my previous comment about pr_{} vs. dev_{}, I think
you can now drop the pr_fmt and just leave the name change here.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v6 3/4] mtd: devices: elm: configure parallel channels based on ecc_steps

[Ezequiel Garcia]

On Mar 07, Pekon Gupta wrote:
> Though ELM hardware can process upto maximum of 8 channels in parallel for
> ECC error detection. But actual number of channels need to be processed in
> parallel depends on ecc_steps (for page mode configuration).
>
 
This commit log needs some love ;) How about something along these lines:

"ELM hardware can process up to ERR_VECTOR_MAX channels in parallel for
ECC error detection. However, the actual number of channels that need to
be processed is the ECC step number." ?

Perhaps you could add some more information (I'm not too familiar with ELM),
what's the impact of this change? Does it fix any bug or the over-looping
was harmless?

> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/devices/elm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
> index f59c100..43fd81d 100644
> --- a/drivers/mtd/devices/elm.c
> +++ b/drivers/mtd/devices/elm.c
> @@ -174,7 +174,7 @@ static void elm_load_syndrome(struct elm_info *info,
>  	int i, offset;
>  	u32 val;
>  
> -	for (i = 0; i < ERROR_VECTOR_MAX; i++) {
> +	for (i = 0; i < info->ecc_steps; i++) {

I think it would be less confusing if you can add the ecc_steps field on this
patch, where you start to use it.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v6 0/4] mtd: devices: elm: add checks ELM H/W constrains, driver code cleanup

[Lee Jones]

> *changes v5 -> v6*
> [PATCH 02/04] minor cleanup
> 
> *changes v4 -> v5*
> This patch series is split version from earlier series [1].
> This series refactors and cleans ELM driver which is used by Hardware
> based BCHx ecc-schemes.
>  - Undo: introduction of 'struct mtd_info' and 'struct nand_chip'. Instead
>          keep ELM driver independent of mtd_info and nand_chip structs and
>          pass only required ECC configurations as elm_config() arguments
>          elm_config(..., int ecc_steps, int ecc_step_size, int ecc_step_bytes)
>  - Undo: re-writing of elm_load_syndrome() ECC register configurations.
> 
> *changes v3 -> v4 [1]*
>  - in-corporated feedbacks from Brian Norris <computersforpeace@gmail.com>
>  - updated: use 'pr_fmt(fmt)' to suffix DRIVER_NAME
>  - removed: local 'eccsteps' in ELM driver, instead using nand_chip->ecc.steps
>  - undo: irrelavant white-space changes
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2013-November/050242.html
> 
> Pekon Gupta (4):
>   mtd: devices: elm: check for hardware engine's design constrains
>   mtd: devices: elm: clean elm_load_syndrome
>   mtd: devices: elm: configure parallel channels based on ecc_steps
>   mtd: devices: elm: update DRIVER_NAME as "omap-elm"
> 
>  drivers/mtd/devices/elm.c         | 44 +++++++++++++++++++++++++++++----------
>  drivers/mtd/nand/omap2.c          |  9 +++++---
>  include/linux/platform_data/elm.h | 10 ++-------
>  3 files changed, 41 insertions(+), 22 deletions(-)

I can't seem to be able to apply this set. What kernel release it is
based on?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [PATCH v6 0/4] mtd: devices: elm: add checks ELM H/W constrains, driver code cleanup

[Gupta, Pekon]

>From: Lee Jones [mailto:lee.jones@linaro.org]
>
>> *changes v5 -> v6*
>> [PATCH 02/04] minor cleanup
>>
>> *changes v4 -> v5*
>> This patch series is split version from earlier series [1].
>> This series refactors and cleans ELM driver which is used by Hardware
>> based BCHx ecc-schemes.
>>  - Undo: introduction of 'struct mtd_info' and 'struct nand_chip'. Instead
>>          keep ELM driver independent of mtd_info and nand_chip structs and
>>          pass only required ECC configurations as elm_config() arguments
>>          elm_config(..., int ecc_steps, int ecc_step_size, int ecc_step_bytes)
>>  - Undo: re-writing of elm_load_syndrome() ECC register configurations.
>>
>> *changes v3 -> v4 [1]*
>>  - in-corporated feedbacks from Brian Norris <computersforpeace@gmail.com>
>>  - updated: use 'pr_fmt(fmt)' to suffix DRIVER_NAME
>>  - removed: local 'eccsteps' in ELM driver, instead using nand_chip->ecc.steps
>>  - undo: irrelavant white-space changes
>>
>> [1] http://lists.infradead.org/pipermail/linux-mtd/2013-November/050242.html
>>
>> Pekon Gupta (4):
>>   mtd: devices: elm: check for hardware engine's design constrains
>>   mtd: devices: elm: clean elm_load_syndrome
>>   mtd: devices: elm: configure parallel channels based on ecc_steps
>>   mtd: devices: elm: update DRIVER_NAME as "omap-elm"
>>
>>  drivers/mtd/devices/elm.c         | 44 +++++++++++++++++++++++++++++----------
>>  drivers/mtd/nand/omap2.c          |  9 +++++---
>>  include/linux/platform_data/elm.h | 10 ++-------
>>  3 files changed, 41 insertions(+), 22 deletions(-)
>
>I can't seem to be able to apply this set. What kernel release it is
>based on?
>
This is rebased on l2-mtd/master. You can also pull from [1]

[1] http://git.ti.com/~pekon/connectivity-integration-tree/pekons-connectivity-ti-linux-kernel/commits/l2-mtd/bch16_support


with regards, pekon

Re: [PATCH v6 0/4] mtd: devices: elm: add checks ELM H/W constrains, driver code cleanup

[Lee Jones]

> >> *changes v5 -> v6*
> >> [PATCH 02/04] minor cleanup
> >>
> >> *changes v4 -> v5*
> >> This patch series is split version from earlier series [1].
> >> This series refactors and cleans ELM driver which is used by Hardware
> >> based BCHx ecc-schemes.
> >>  - Undo: introduction of 'struct mtd_info' and 'struct nand_chip'. Instead
> >>          keep ELM driver independent of mtd_info and nand_chip structs and
> >>          pass only required ECC configurations as elm_config() arguments
> >>          elm_config(..., int ecc_steps, int ecc_step_size, int ecc_step_bytes)
> >>  - Undo: re-writing of elm_load_syndrome() ECC register configurations.
> >>
> >> *changes v3 -> v4 [1]*
> >>  - in-corporated feedbacks from Brian Norris <computersforpeace@gmail.com>
> >>  - updated: use 'pr_fmt(fmt)' to suffix DRIVER_NAME
> >>  - removed: local 'eccsteps' in ELM driver, instead using nand_chip->ecc.steps
> >>  - undo: irrelavant white-space changes
> >>
> >> [1] http://lists.infradead.org/pipermail/linux-mtd/2013-November/050242.html
> >>
> >> Pekon Gupta (4):
> >>   mtd: devices: elm: check for hardware engine's design constrains
> >>   mtd: devices: elm: clean elm_load_syndrome
> >>   mtd: devices: elm: configure parallel channels based on ecc_steps
> >>   mtd: devices: elm: update DRIVER_NAME as "omap-elm"
> >>
> >>  drivers/mtd/devices/elm.c         | 44 +++++++++++++++++++++++++++++----------
> >>  drivers/mtd/nand/omap2.c          |  9 +++++---
> >>  include/linux/platform_data/elm.h | 10 ++-------
> >>  3 files changed, 41 insertions(+), 22 deletions(-)
> >
> >I can't seem to be able to apply this set. What kernel release it is
> >based on?
> >
> This is rebased on l2-mtd/master. You can also pull from [1]
> 
> [1] http://git.ti.com/~pekon/connectivity-integration-tree/pekons-connectivity-ti-linux-kernel/commits/l2-mtd/bch16_support

I'm sorry I'm not familiar with the l2-mtd tree. Which upstream
commit are these patches applied on top of?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [PATCH v6 0/4] mtd: devices: elm: add checks ELM H/W constrains, driver code cleanup

[Gupta, Pekon]

Hi,

>From: Lee Jones [mailto:lee.jones@linaro.org]
>> >> *changes v5 -> v6*
>> >> [PATCH 02/04] minor cleanup
>> >>
>> >> *changes v4 -> v5*
>> >> This patch series is split version from earlier series [1].
>> >> This series refactors and cleans ELM driver which is used by Hardware
>> >> based BCHx ecc-schemes.
>> >>  - Undo: introduction of 'struct mtd_info' and 'struct nand_chip'. Instead
>> >>          keep ELM driver independent of mtd_info and nand_chip structs and
>> >>          pass only required ECC configurations as elm_config() arguments
>> >>          elm_config(..., int ecc_steps, int ecc_step_size, int ecc_step_bytes)
>> >>  - Undo: re-writing of elm_load_syndrome() ECC register configurations.
>> >>
>> >> *changes v3 -> v4 [1]*
>> >>  - in-corporated feedbacks from Brian Norris <computersforpeace@gmail.com>
>> >>  - updated: use 'pr_fmt(fmt)' to suffix DRIVER_NAME
>> >>  - removed: local 'eccsteps' in ELM driver, instead using nand_chip->ecc.steps
>> >>  - undo: irrelavant white-space changes
>> >>
>> >> [1] http://lists.infradead.org/pipermail/linux-mtd/2013-November/050242.html
>> >>
>> >> Pekon Gupta (4):
>> >>   mtd: devices: elm: check for hardware engine's design constrains
>> >>   mtd: devices: elm: clean elm_load_syndrome
>> >>   mtd: devices: elm: configure parallel channels based on ecc_steps
>> >>   mtd: devices: elm: update DRIVER_NAME as "omap-elm"
>> >>
>> >>  drivers/mtd/devices/elm.c         | 44 +++++++++++++++++++++++++++++----------
>> >>  drivers/mtd/nand/omap2.c          |  9 +++++---
>> >>  include/linux/platform_data/elm.h | 10 ++-------
>> >>  3 files changed, 41 insertions(+), 22 deletions(-)
>> >
>> >I can't seem to be able to apply this set. What kernel release it is
>> >based on?
>> >
>> This is rebased on l2-mtd/master. You can also pull from [1]
>>
>> [1] http://git.ti.com/~pekon/connectivity-integration-tree/pekons-connectivity-ti-linux-kernel/commits/l2-mtd/bch16_support
>
>I'm sorry I'm not familiar with the l2-mtd tree. Which upstream
>commit are these patches applied on top of?
>
These are rebased on following tree for linux-mtd subsystem.
	git://git.infradead.org/users/dedekind/l2-mtd.git
	which is I think based on upstream tag of "v3.14-rc1"

If you still see conflicts (especially in drivers/mtd/nand/omap2.c) then
Please pull in following previous patches series in given order
(though each of following is functionally independently).

(0) http://lists.infradead.org/pipermail/linux-mtd/2014-February/052068.html

(1) [PATCH v8 0/6] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes
  http://lists.infradead.org/pipermail/linux-mtd/2014-February/052092.html

(2) [PATCH v6 0/4] mtd: nand: omap: optimize chip->ecc.calculate() for H/W ECC schemes
  http://lists.infradead.org/pipermail/linux-mtd/2014-February/052272.html

(3) [PATCH v5 0/4] mtd: nand: omap: optimize chip->ecc.hwctl() for H/W ECC schemes
  http://lists.infradead.org/pipermail/linux-mtd/2014-March/052327.html

 (4) [PATCH v6 0/4] mtd: devices: elm: add checks ELM H/W constrains, driver code cleanup
  http://lists.infradead.org/pipermail/linux-mtd/2014-March/052455.html
 
with regards, pekon

RE: [PATCH v6 1/4] mtd: devices: elm: check for hardware engine's design constrains

[Gupta, Pekon]

Hi Ezequiel,

>From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
 [...]
>Altough it's just a nitpick, I think you can work a bit on your explanations.
>For instance, you start with a capital letter after a comma (1K, So it..).
>
>And you have some incomplete sentences (This patch adds *the* following).
>
>I'm not a native english speaker, so I know this can be hard! It's not a hard
>requirement for the patch, but rather just a suggestion to improve your upstream
>work.
>
Thanks much for review on this and other patches. I'll try to be more careful
with my English next time. I'll incorporate your feedbacks in next version,
But before sending next version, I'll wait for:
- if Brian has any other feedbacks on this and previous series.
- And acceptance/conclusion on following discussions..
	"mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes"
	"mtd: nand: add erased-page bitflip correction" from Brain Norris


with regards, pekon