linux-kernel - Re: [PATCH] mtd-nand: davinci: Correct 4-bit error correction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <1257865502.21596.773.camel@localhost>
Date:	Tue, 10 Nov 2009 17:05:02 +0200
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Sudhakar Rajashekhara <sudhakar.raj@...com>
Cc:	linux-mtd@...ts.infradead.org, akpm@...ux-foundation.org,
	nsnehaprabha@...com,
	davinci-linux-open-source@...ux.davincidsp.com,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd-nand: davinci: Correct 4-bit error correction

On Tue, 2009-11-03 at 16:01 +0530, Sudhakar Rajashekhara wrote:
> On TI's DA830/OMAP-L137, DA850/OMAP-L138 and DM365, after
> setting the 4BITECC_ADD_CALC_START bit in the NAND Flash
> control register to 1 and before waiting for the NAND Flash
> status register to be equal to 1, 2 or 3, we have to wait
> till the ECC HW goes to correction state. Without this wait,
> ECC correction calculations will not be proper.
> 
> This has been tested on DA830/OMAP-L137, DA850/OMAP-L138,
> DM355 and DM365 EVMs.
> 
> Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@...com>
> Acked-by: Sneha Narnakaje <nsnehaprabha@...com>
> ---
>  drivers/mtd/nand/davinci_nand.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index fe3eba8..8a32999 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -310,6 +310,7 @@ static int nand_davinci_correct_4bit(struct mtd_info *mtd,
>  	unsigned short ecc10[8];
>  	unsigned short *ecc16;
>  	u32 syndrome[4];
> +	u32 ecc_state;
>  	unsigned num_errors, corrected;
>  
>  	/* All bytes 0xff?  It's an erased page; ignore its ECC. */
> @@ -360,6 +361,21 @@ compare:
>  	 */
>  	davinci_nand_writel(info, NANDFCR_OFFSET,
>  			davinci_nand_readl(info, NANDFCR_OFFSET) | BIT(13));
> +
> +	/*
> +	 * ECC_STATE field reads 0x3 (Error correction complete) immediately
> +	 * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately
> +	 * begin trying to poll for the state, you may fall right out of your
> +	 * loop without any of the correction calculations having taken place.
> +	 * The recommendation from the hardware team is to wait till ECC_STATE
> +	 * reads less than 4, which means ECC HW has entered correction state.
> +	 */
> +	do {
> +		ecc_state = (davinci_nand_readl(info,
> +				NANDFSR_OFFSET) >> 8) & 0x0f;
> +		cpu_relax();
> +	} while (ecc_state < 4);
> +
>  	for (;;) {
>  		u32	fsr = davinci_nand_readl(info, NANDFSR_OFFSET);

I see a lot of constructs like this in many mtd drivers. I wonder, what
happens if ecc_state never becomes < 4? Should there be some protection
against this, e.g., exit the loop with an error message if we are
looping for more than 1 or several seconds?

What is the "official" / "right" way for this kind of loops?

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/