Patchwork [resending,MTD,NAND] nand_ecc.c: Bug fix in nand ecc

login
register
mail settings
Submitter Singh, Vimal
Date April 1, 2009, 9:53 a.m.
Message ID <34437.192.168.10.89.1238579632.squirrel@dbdmail.itg.ti.com>
Download mbox | patch
Permalink /patch/25480/
State New
Headers show

Comments

Singh, Vimal - April 1, 2009, 9:53 a.m.
I do not see this patch on tree, yet.

------------------------------ Original Message -------------------------------
Subject: [PATCH] [MTD] [NAND] nand_ecc.c: Bug fix in nand ecc
From:    "vimal singh" <vimalsingh@ti.com>
Date:    Mon, February 23, 2009 1:46 pm
To:      linux-mtd@lists.infradead.org
-------------------------------------------------------------------------------

Type of 'byte_addr' needed to change for 512 byte ecc support.

Signed-off-by: Vimal Singh <vimalsingh@ti.com>
---

 			(((struct nand_chip *)mtd->priv)->ecc.size) >> 8;
Artem Bityutskiy - April 1, 2009, 10:01 a.m.
On Wed, 2009-04-01 at 15:23 +0530, vimal singh wrote:
> I do not see this patch on tree, yet.
> 
> ------------------------------ Original Message -------------------------------
> Subject: [PATCH] [MTD] [NAND] nand_ecc.c: Bug fix in nand ecc
> From:    "vimal singh" <vimalsingh@ti.com>
> Date:    Mon, February 23, 2009 1:46 pm
> To:      linux-mtd@lists.infradead.org
> -------------------------------------------------------------------------------
> 
> Type of 'byte_addr' needed to change for 512 byte ecc support.
> 
> Signed-off-by: Vimal Singh <vimalsingh@ti.com>

Probably because you did not try to CC the author of the
code and get his ack. Isn't this logical step to do?

I've CCed Frams.

> ---
> 
> diff --git a/drivers/mtd/nand/nand_ecc.c b/drivers/mtd/nand/nand_ecc.c index
> 868147a..6b19058 100644
> --- a/drivers/mtd/nand/nand_ecc.c
> +++ b/drivers/mtd/nand/nand_ecc.c
> @@ -429,7 +429,8 @@ int nand_correct_data(struct mtd_info *mtd, unsigned char
> *buf,
>  		      unsigned char *read_ecc, unsigned char *calc_ecc)
>  {
>  	unsigned char b0, b1, b2;
> -	unsigned char byte_addr, bit_addr;
> +	uint32_t byte_addr;
> +	unsigned char bit_addr;
>  	/* 256 or 512 bytes/ecc  */
>  	const uint32_t eccsize_mult =
>  			(((struct nand_chip *)mtd->priv)->ecc.size) >> 8;
>
Artem Bityutskiy - April 1, 2009, 10:02 a.m.
On Wed, 2009-04-01 at 13:01 +0300, Artem Bityutskiy wrote:
> Probably because you did not try to CC the author of the
> code and get his ack. Isn't this logical step to do?
> 
> I've CCed Frams.

Pardon, Frans.
Singh, Vimal - April 1, 2009, 10:11 a.m.
> Probably because you did not try to CC the author of the
> code and get his ack. Isn't this logical step to do?
> 
> I've CCed Frams.

I am not so old with open source, so missed that.
Anyways, thanks... 

Best regards,
vimal
Singh, Vimal - April 24, 2009, 6:32 a.m.
It looks like Frans is not active to this list, now a days...
so can some one else review this patch:
http://lists.infradead.org/pipermail/linux-mtd/2009-April/025094.html

---
vimal
Artem Bityutskiy - April 24, 2009, 7:39 a.m.
Hi,

On Fri, 2009-04-24 at 12:02 +0530, Singh, Vimal wrote:
> It looks like Frans is not active to this list, now a days...
> so can some one else review this patch:
> http://lists.infradead.org/pipermail/linux-mtd/2009-April/025094.html


It did not apply. Please, send patches which can be applied.

Please, fix you e-mail. Is your address really
"IMCEAEX-_O=TI_OU=BD_CN=RECIPIENTS_CN=X0094262@dlee86.itg.ti.com" ?

But I fixed both - your patch and your e-mail. Please, check your other
patches and re-send them if they are corrupted. Please, take a look at
Documentation/SubmittingPatches

I've amended your patch a little. Instead of using 'uint32_t', I
made it to be 'int'. I do not see any reason to limit the compiler
and CPU by 32 bits. I do not appreciate unreasonable usage of these
'uint32_t' and similar types, and I think 'int' is perfectly fine
in your case. Could you please verify this? It is here:
http://git.infradead.org/users/dedekind/l2-mtd-2.6.git?a=commit;h=e3b8cec094d12aabbfd77c462b1932537e1c239e

Thanks!
Singh, Vimal - April 24, 2009, 8:27 a.m.
On Fri, Apr 24, 2009 at 1:09 PM, Artem Bityutskiy <dedekind@infradead.org> wrote:
> Hi,
>
> On Fri, 2009-04-24 at 12:02 +0530, Singh, Vimal wrote:
>> It looks like Frans is not active to this list, now a days...
>> so can some one else review this patch:
>> http://lists.infradead.org/pipermail/linux-mtd/2009-April/025094.html
>
>
> It did not apply. Please, send patches which can be applied.
>
> Please, fix you e-mail. Is your address really
> "IMCEAEX-_O=TI_OU=BD_CN=RECIPIENTS_CN=X0094262@dlee86.itg.ti.com" ?
>
> But I fixed both - your patch and your e-mail. Please, check your other
> patches and re-send them if they are corrupted. Please, take a look at
> Documentation/SubmittingPatches
>
> I've amended your patch a little. Instead of using 'uint32_t', I
> made it to be 'int'. I do not see any reason to limit the compiler
> and CPU by 32 bits. I do not appreciate unreasonable usage of these
> 'uint32_t' and similar types, and I think 'int' is perfectly fine

I agree to not limit it to 32bit. But Then I will prefer to use 'unsigned int'
rather just 'int', since 'byte_addr' is always a positive number.

> in your case. Could you please verify this? It is here:
> http://git.infradead.org/users/dedekind/l2-mtd-2.6.git?a=commit;h=e3b8cec094d12aabbfd77c462b1932537e1c239e

I still do not see this change in commit, this is still 'uint32_t'.

Thanks and Regards,
vimal

>
> Thanks!
>
> --
> Best regards,
> Artem Bityutskiy (Битюцкий Артём)
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
Artem Bityutskiy - April 24, 2009, 9:31 a.m.
On Fri, 2009-04-24 at 13:57 +0530, Singh, Vimal wrote:
> > in your case. Could you please verify this? It is here:
> > http://git.infradead.org/users/dedekind/l2-mtd-2.6.git?a=commit;h=e3b8cec094d12aabbfd77c462b1932537e1c239e
> 
> I still do not see this change in commit, this is still 'uint32_t'.

Fixed.

http://git.infradead.org/users/dedekind/l2-mtd-2.6.git?a=commit;h=bb36f87562cb8941aec7d132351f60b8d93b93b3
Frans Meulenbroeks - April 25, 2009, 2:55 p.m.
Vimal, Artem,

Thanks for fixing this. I thought I had acked the original patch but
apparently I didn't. Sorry.
I'm happy with the patch as it is now. Glad you solved it.

Frans.

Patch

diff --git a/drivers/mtd/nand/nand_ecc.c b/drivers/mtd/nand/nand_ecc.c index
868147a..6b19058 100644
--- a/drivers/mtd/nand/nand_ecc.c
+++ b/drivers/mtd/nand/nand_ecc.c
@@ -429,7 +429,8 @@  int nand_correct_data(struct mtd_info *mtd, unsigned char
*buf,
 		      unsigned char *read_ecc, unsigned char *calc_ecc)
 {
 	unsigned char b0, b1, b2;
-	unsigned char byte_addr, bit_addr;
+	uint32_t byte_addr;
+	unsigned char bit_addr;
 	/* 256 or 512 bytes/ecc  */
 	const uint32_t eccsize_mult =