Bug in cfi_cmdset_0002.c in linux-2.6.8.1

Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Jochen Karrer]

Hi,
jffs2 did not work in linux-2.6.8.1 because single word writes at the end
of a buffer write are done with the wrong address:

--- /home/jkarrer/kernel/linux-2.6.8.1/drivers/mtd/chips/cfi_cmdset_0002.c
Sat Aug 14 12:55:20 2004
+++ cfi_cmdset_0002.c	Thu Aug 26 14:33:42 2004
@@ -1128,7 +1128,7 @@
 	if (len) {
 		size_t retlen_dregs = 0;

-		ret = cfi_amdstd_write_words(mtd, to, len, &retlen_dregs, buf);
+		ret = cfi_amdstd_write_words(mtd, ofs + (chipnum<<cfi->chipshift), len,
&retlen_dregs, buf);

 		*retlen += retlen_dregs;
 		return ret;


Jochen

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Steven Scholz]

Jochen Karrer wrote:

> Hi,
> jffs2 did not work in linux-2.6.8.1 because single word writes at the end
> of a buffer write are done with the wrong address:
> 

Aah! Thanks a million! I already started think that my hardware was broken...

-- 
Steven

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[David Woodhouse]

On Thu, 2004-08-26 at 15:11 +0200, Jochen Karrer wrote:
> Hi,
> jffs2 did not work in linux-2.6.8.1 because single word writes at the end
> of a buffer write are done with the wrong address:

Thanks. If this is needed though -- don't we _also_ need the same fix a
few lines up?

-- 
dwmw2

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Jochen Karrer]

On Thu, 2004-08-26 at 15:11 +0200, Jochen Karrer wrote:

Hi,
> > jffs2 did not work in linux-2.6.8.1 because single word writes at the
end
> > of a buffer write are done with the wrong address:

> Thanks. If this is needed though -- don't we _also_ need the same fix a
> few lines up?

Its not required because for the first write "ofs + chipnum <<
cfi->chipshift"
should be the same like "to". "ofs" is not yet incremented. But may its
better
style.

Jochen

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Thayne Harbaugh]

On Thu, 2004-08-26 at 15:11 +0200, Jochen Karrer wrote:
> Hi,
> jffs2 did not work in linux-2.6.8.1 because single word writes at the end
> of a buffer write are done with the wrong address:
> 
> --- /home/jkarrer/kernel/linux-2.6.8.1/drivers/mtd/chips/cfi_cmdset_0002.c
> Sat Aug 14 12:55:20 2004
> +++ cfi_cmdset_0002.c	Thu Aug 26 14:33:42 2004
> @@ -1128,7 +1128,7 @@
>  	if (len) {
>  		size_t retlen_dregs = 0;
> 
> -		ret = cfi_amdstd_write_words(mtd, to, len, &retlen_dregs, buf);
> +		ret = cfi_amdstd_write_words(mtd, ofs + (chipnum<<cfi->chipshift), len,
> &retlen_dregs, buf);
> 
>  		*retlen += retlen_dregs;
>  		return ret;


I never saw that this patch was committed.  I didn't do anything with it
because I'm not so familiar with that piece in cfi_cmdset_0002.c (and am
too lazy to read the function).  I did notice that Steven Scholz
confirmed that it fixed problems he was seeing.

Anyone else that cares should comment now or I'll commit it so that it
doesn't get forgotten.

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[David Woodhouse]

On Tue, 2004-09-14 at 13:41 -0600, Thayne Harbaugh wrote:
> Anyone else that cares should comment now or I'll commit it so that it
> doesn't get forgotten.

Looks good -- best to commit it. Are there other places where we need
the same fix?

-- 
dwmw2

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Thayne Harbaugh]

On Tue, 2004-09-14 at 21:47 +0100, David Woodhouse wrote:
> On Tue, 2004-09-14 at 13:41 -0600, Thayne Harbaugh wrote:
> > Anyone else that cares should comment now or I'll commit it so that it
> > doesn't get forgotten.
> 
> Looks good -- best to commit it. Are there other places where we need
> the same fix?

You asked that before and the response was, "no, unless we just want
things to be consistent."

Like I said, I'm just not very familiar with the buffer functions and
I'm a bit too lazy (and busy) to read the code.  Guess I'll have to look
- then I'll commit it.

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[David Woodhouse]

On Wed, 2004-09-15 at 11:19 -0600, Thayne Harbaugh wrote:
> On Tue, 2004-09-14 at 21:47 +0100, David Woodhouse wrote:
> > On Tue, 2004-09-14 at 13:41 -0600, Thayne Harbaugh wrote:
> > > Anyone else that cares should comment now or I'll commit it so that it
> > > doesn't get forgotten.
> > 
> > Looks good -- best to commit it. Are there other places where we need
> > the same fix?
> 
> You asked that before and the response was, "no, unless we just want
> things to be consistent."

If I was going to be reliable enough to remember such things I'd be
reliable enough to have committed the patch in the first place. Don't
taunt me :)

> Like I said, I'm just not very familiar with the buffer functions and
> I'm a bit too lazy (and busy) to read the code.  Guess I'll have to look
> - then I'll commit it.

Thanks. I completely accept your excuse. :)

-- 
dwmw2

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Thayne Harbaugh]

On Tue, 2004-09-14 at 21:47 +0100, David Woodhouse wrote:
> On Tue, 2004-09-14 at 13:41 -0600, Thayne Harbaugh wrote:
> > Anyone else that cares should comment now or I'll commit it so that it
> > doesn't get forgotten.
> 
> Looks good -- best to commit it. Are there other places where we need
> the same fix?

I looked and didn't see any other places (other than right before in the
same function - I changed that for consistency).

Anyone that cares, please test what's in CVS!

BTW, Eric Biederman is going to push this and other patches to the
kernel.  Better check everything before it's too late!

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Manfred Gruber]

Am Dienstag, 14. September 2004 21:41 schrieb Thayne Harbaugh:
> On Thu, 2004-08-26 at 15:11 +0200, Jochen Karrer wrote:
> > Hi,
> > jffs2 did not work in linux-2.6.8.1 because single word writes at the end
> > of a buffer write are done with the wrong address:
> >
> > ---
> > /home/jkarrer/kernel/linux-2.6.8.1/drivers/mtd/chips/cfi_cmdset_0002.c
> > Sat Aug 14 12:55:20 2004
> > +++ cfi_cmdset_0002.c	Thu Aug 26 14:33:42 2004
> > @@ -1128,7 +1128,7 @@
> >  	if (len) {
> >  		size_t retlen_dregs = 0;
> >
> > -		ret = cfi_amdstd_write_words(mtd, to, len, &retlen_dregs, buf);
> > +		ret = cfi_amdstd_write_words(mtd, ofs + (chipnum<<cfi->chipshift),
> > len, &retlen_dregs, buf);
> >
> >  		*retlen += retlen_dregs;
> >  		return ret;
>
> I never saw that this patch was committed.  I didn't do anything with it
> because I'm not so familiar with that piece in cfi_cmdset_0002.c (and am
> too lazy to read the function).  I did notice that Steven Scholz
> confirmed that it fixed problems he was seeing.
>
> Anyone else that cares should comment now or I'll commit it so that it
> doesn't get forgotten.

At my point this patch works good for my AMD flash, i dont get timeouts now.

Hope it helps regards manfred

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Steven Scholz]

Jochen Karrer wrote:
> Hi,
> jffs2 did not work in linux-2.6.8.1 because single word writes at the end
> of a buffer write are done with the wrong address:
> 
> --- /home/jkarrer/kernel/linux-2.6.8.1/drivers/mtd/chips/cfi_cmdset_0002.c
> Sat Aug 14 12:55:20 2004
> +++ cfi_cmdset_0002.c	Thu Aug 26 14:33:42 2004
> @@ -1128,7 +1128,7 @@
>  	if (len) {
>  		size_t retlen_dregs = 0;
> 
> -		ret = cfi_amdstd_write_words(mtd, to, len, &retlen_dregs, buf);
> +		ret = cfi_amdstd_write_words(mtd, ofs + (chipnum<<cfi->chipshift), len,
> &retlen_dregs, buf);
> 
>  		*retlen += retlen_dregs;
>  		return ret;

As far as I can see this bug is still not fixed in 2.6.9!
Is it done in newer versions?

--
Steven

Re: Bug in cfi_cmdset_0002.c in linux-2.6.8.1

[Thayne Harbaugh]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

On Thu, 2004-11-11 at 17:10 +0100, Steven Scholz wrote:
> Jochen Karrer wrote:
> > Hi,
> > jffs2 did not work in linux-2.6.8.1 because single word writes at the end
> > of a buffer write are done with the wrong address:
> > 
> > --- /home/jkarrer/kernel/linux-2.6.8.1/drivers/mtd/chips/cfi_cmdset_0002.c
> > Sat Aug 14 12:55:20 2004
> > +++ cfi_cmdset_0002.c	Thu Aug 26 14:33:42 2004
> > @@ -1128,7 +1128,7 @@
> >  	if (len) {
> >  		size_t retlen_dregs = 0;
> > 
> > -		ret = cfi_amdstd_write_words(mtd, to, len, &retlen_dregs, buf);
> > +		ret = cfi_amdstd_write_words(mtd, ofs + (chipnum<<cfi->chipshift), len,
> > &retlen_dregs, buf);
> > 
> >  		*retlen += retlen_dregs;
> >  		return ret;
> 
> As far as I can see this bug is still not fixed in 2.6.9!
> Is it done in newer versions?

It's fixed in MTD CVS.  I don't know if it has been pushed to Linus.

-- 
Thayne Harbaugh
Linux Networx

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]