Re: [PATCH v3 3/4] mtd: nand: omap: optimize chip->ecc.hwctl() for H/W ECC schemes

[Brian Norris <computersforpeace@gmail.com>]

On Sat, Nov 02, 2013 at 03:16:15PM +0530, Pekon Gupta wrote:
> chip->ecc.hwctl() is used for preparing the H/W controller before read/write
> NAND accesses (like assigning data-buf, enabling ECC scheme configs, etc.)
> 
> Though all ECC schemes in OMAP NAND driver use GPMC controller for generating
> ECC syndrome (for both Read/Write accesses). But but in current code

s/But but/but/

:)

> HAM1_ECC and BCHx_ECC schemes implement individual function to achieve this.
> This patch merges the GPMC configuration code for all ECC schemes into
> single omap_enable_hwecc(), thus adding scalability for future ECC schemes.
> 
>  omap_enable_hwecc() + omap3_enable_hwecc_bch() -> omap_enable_hwecc()
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/nand/omap2.c | 212 +++++++++++++++++------------------------------
>  1 file changed, 74 insertions(+), 138 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 1f59505..3a99e29 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -33,6 +33,10 @@
>  #define	DRIVER_NAME	"omap2-nand"
>  #define	OMAP_NAND_TIMEOUT_MS	5000
>  
> +#define GPMC_ECC_READ		0 /* Reset Hardware ECC for read */
> +#define GPMC_ECC_WRITE		1 /* Reset Hardware ECC for write */
> +#define GPMC_ECC_READSYN	2 /* Reset before syndrom is read back */
> +
>  #define NAND_Ecc_P1e		(1 << 0)
>  #define NAND_Ecc_P2e		(1 << 1)
>  #define NAND_Ecc_P4e		(1 << 2)
> @@ -101,13 +105,9 @@
>  #define P4o_s(a)	(TF(a & NAND_Ecc_P4o)		<< 1)
>  
>  #define	PREFETCH_CONFIG1_CS_SHIFT	24
> -#define	ECC_CONFIG_CS_SHIFT		1

You stopped using this macro in a different patch, so this removal is
unrelated to this patch. But I'm not sure the code is better without
these macros anyway... I'll leave it to your discretion, I guess.

>  #define	CS_MASK				0x7
>  #define	ENABLE_PREFETCH			(0x1 << 7)
>  #define	DMA_MPU_MODE_SHIFT		2
> -#define	ECCSIZE0_SHIFT			12
> -#define	ECCSIZE1_SHIFT			22
> -#define	ECC1RESULTSIZE			0x1
>  #define	ECCCLEAR			0x100
>  #define	ECC1				0x1
>  #define	PREFETCH_FIFOTHRESHOLD_MAX	0x40

...

The rest looks OK but very hard for me to verify. Do we have any
testers? Any platforms to check for regressions?

Brian