* Fix to jedec_probe unlock addresses
@ 2004-09-20 23:44 Ben Dooks
2004-09-23 1:48 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Ben Dooks @ 2004-09-20 23:44 UTC (permalink / raw)
To: linux-mtd; +Cc: ben
Fix unlock address calculation for non-x8 chips for the
cfi cmdset code, which assumes always X8
Patch against 2.6.9-rc2 with 19th September CVS
Signed-off-by: Ben Dooks <ben-mtd@fluff.org>
--- linux-2.6.9-rc2-bk6-mtd20040919/drivers/mtd/chips/jedec_probe.c 2004-09-20 13:02:46.000000000 +0100
+++ linux-2.6.9-rc2-bk6-mtd20040919-work/drivers/mtd/chips/jedec_probe.c 2004-09-20 23:21:40.000000000 +0100
@@ -2046,7 +2046,15 @@
printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit bank\n",
map->name, cfi_interleave(cfi), cfi->device_type*8, base,
map->bankwidth*8);
-
+
+ /* fixup unlock addresses for the cmdset */
+
+ cfi->addr_unlock1 *= cfi_interleave(cfi) * cfi->device_type;
+ cfi->addr_unlock2 *= cfi_interleave(cfi) * cfi->device_type;
+
+ printk(KERN_DEBUG "unlocks at %08x,%08x\n",
+ cfi->addr_unlock1, cfi->addr_unlock2);
+
return 1;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix to jedec_probe unlock addresses
2004-09-20 23:44 Fix to jedec_probe unlock addresses Ben Dooks
@ 2004-09-23 1:48 ` Eric W. Biederman
2004-09-23 21:01 ` Ben Dooks
0 siblings, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2004-09-23 1:48 UTC (permalink / raw)
To: Ben Dooks; +Cc: ben, linux-mtd
Ben Dooks <ben-mtd@fluff.org> writes:
> Fix unlock address calculation for non-x8 chips for the
> cfi cmdset code, which assumes always X8
>
> Patch against 2.6.9-rc2 with 19th September CVS
>
> Signed-off-by: Ben Dooks <ben-mtd@fluff.org>
>
> --- linux-2.6.9-rc2-bk6-mtd20040919/drivers/mtd/chips/jedec_probe.c 2004-09-20
> 13:02:46.000000000 +0100
>
> +++ linux-2.6.9-rc2-bk6-mtd20040919-work/drivers/mtd/chips/jedec_probe.c
> 2004-09-20 23:21:40.000000000 +0100
>
> @@ -2046,7 +2046,15 @@
> printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit bank\n",
> map->name, cfi_interleave(cfi), cfi->device_type*8, base,
> map->bankwidth*8);
> -
> +
> + /* fixup unlock addresses for the cmdset */
> +
> + cfi->addr_unlock1 *= cfi_interleave(cfi) * cfi->device_type;
> + cfi->addr_unlock2 *= cfi_interleave(cfi) * cfi->device_type;
> +
> + printk(KERN_DEBUG "unlocks at %08x,%08x\n",
> + cfi->addr_unlock1, cfi->addr_unlock2);
> +
> return 1;
> }
Hmm. I think that points out a real issue but I think the bug actually
lies in cfi_cmdset_0002.
That fix makes below comment from cfi_cmdset_0002 clearly wrong.
/*
* The CFI_DEVICETYPE_X8 argument is needed even when
* cfi->device_type != CFI_DEVICETYPE_X8. The addresses for
* command sequences don't scale even when the device is
* wider. This is the case for many of the cfi_send_gen_cmd()
* below. I'm not sure, however, why some use
* cfi->device_type.
*/
So I think it is more likely we need to change this hunk of code below.
/*
* These might already be setup (more correctly) by
* jedec_probe.c - still need it for cfi_probe.c path.
*/
if ( ! (cfi->addr_unlock1 && cfi->addr_unlock2) ) {
switch (cfi->device_type) {
case CFI_DEVICETYPE_X8:
cfi->addr_unlock1 = 0x555;
cfi->addr_unlock2 = 0x2aa;
break;
case CFI_DEVICETYPE_X16:
cfi->addr_unlock1 = 0xaaa;
if (map_bankwidth(map) == cfi_interleave(cfi)) {
/* X16 chip(s) in X8 mode */
cfi->addr_unlock2 = 0x555;
} else {
cfi->addr_unlock2 = 0x554;
}
break;
case CFI_DEVICETYPE_X32:
cfi->addr_unlock1 = 0x1554;
if (map_bankwidth(map) == cfi_interleave(cfi)*2) {
/* X32 chip(s) in X16 mode */
cfi->addr_unlock1 = 0xaaa;
} else {
cfi->addr_unlock2 = 0xaa8;
}
break;
default:
printk(KERN_WARNING
"MTD %s(): Unsupported device type %d\n",
__func__, cfi->device_type);
kfree(mtd);
kfree(extp);
return NULL;
}
}
Does anyone have cfi cmdset 0002 devices in an interleaved configuration
who would care to comment?
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Fix to jedec_probe unlock addresses
2004-09-23 1:48 ` Eric W. Biederman
@ 2004-09-23 21:01 ` Ben Dooks
2004-09-23 21:36 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Ben Dooks @ 2004-09-23 21:01 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-mtd
On Wed, Sep 22, 2004 at 07:48:40PM -0600, Eric W. Biederman wrote:
> Ben Dooks <ben-mtd@fluff.org> writes:
>
> > Fix unlock address calculation for non-x8 chips for the
> > cfi cmdset code, which assumes always X8
> >
> > Patch against 2.6.9-rc2 with 19th September CVS
> >
> > Signed-off-by: Ben Dooks <ben-mtd@fluff.org>
> >
> > --- linux-2.6.9-rc2-bk6-mtd20040919/drivers/mtd/chips/jedec_probe.c 2004-09-20
> > 13:02:46.000000000 +0100
> >
> > +++ linux-2.6.9-rc2-bk6-mtd20040919-work/drivers/mtd/chips/jedec_probe.c
> > 2004-09-20 23:21:40.000000000 +0100
> >
> > @@ -2046,7 +2046,15 @@
> > printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit bank\n",
> > map->name, cfi_interleave(cfi), cfi->device_type*8, base,
> > map->bankwidth*8);
> > -
> > +
> > + /* fixup unlock addresses for the cmdset */
> > +
> > + cfi->addr_unlock1 *= cfi_interleave(cfi) * cfi->device_type;
> > + cfi->addr_unlock2 *= cfi_interleave(cfi) * cfi->device_type;
> > +
> > + printk(KERN_DEBUG "unlocks at %08x,%08x\n",
> > + cfi->addr_unlock1, cfi->addr_unlock2);
> > +
> > return 1;
> > }
>
> Hmm. I think that points out a real issue but I think the bug actually
> lies in cfi_cmdset_0002.
>
> That fix makes below comment from cfi_cmdset_0002 clearly wrong.
> /*
> * The CFI_DEVICETYPE_X8 argument is needed even when
> * cfi->device_type != CFI_DEVICETYPE_X8. The addresses for
> * command sequences don't scale even when the device is
> * wider. This is the case for many of the cfi_send_gen_cmd()
> * below. I'm not sure, however, why some use
> * cfi->device_type.
> */
[snip]
> Does anyone have cfi cmdset 0002 devices in an interleaved configuration
> who would care to comment?
I took the jedec_probe alterations as I was unsure wether I was going
to damage anything else by changing the cfi cmdset 0002 code.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fix to jedec_probe unlock addresses
2004-09-23 21:01 ` Ben Dooks
@ 2004-09-23 21:36 ` Eric W. Biederman
2004-09-24 4:29 ` [CFT] FIX CFI cmdset 0002 for x16 and x32 devices Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2004-09-23 21:36 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-mtd
Ben Dooks <ben-mtd@fluff.org> writes:
> I took the jedec_probe alterations as I was unsure wether I was going
> to damage anything else by changing the cfi cmdset 0002 code.
Your patch is broken with respect to interleaving. The interleave
gets applied twice. Once in jedec_probe, and then again in
cfi_send_gen_cmd.
The bottom line is that we need to be consistent with how we do this.
Either we have magic addresses specified in terms as byte addresses
or we have magic addresses specified as device visible addresses.
Right now cfi_cmdset_0002 is the only place that does not use the
device width.
A 16bit device should not even be able to see a non-16bit
aligned address, and we have to play games with the address
on the bus to handle interleaving so I don't see a reason
why we would want to disable multiplying by device size.
So it looks like the TODO item is to just look up what cfi standard
says are the unlock addresses for cfi command set 2. And make certain
the cfi hard codes are correct and nothing should be broken.
But even without that I would rather see cfi_cmdset_0002 fixed
then seeing this disagreement about how we should pass parameters
to cfi_cmdset_0002. At least if we agree on conventions the broken
cases can be incrementally fixed until the code works properly for
everyone.
It is nice to see that this problem has nothing to do with
interleaving so that anyone with a 16bit or wider device can test
this.
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* [CFT] FIX CFI cmdset 0002 for x16 and x32 devices.
2004-09-23 21:36 ` Eric W. Biederman
@ 2004-09-24 4:29 ` Eric W. Biederman
2004-09-28 12:15 ` Ben Dooks
0 siblings, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2004-09-24 4:29 UTC (permalink / raw)
To: Ben Dooks, linux-mtd; +Cc: Christopher Hoover, David Woodhouse, Thayne Harbaugh
ebiederman@lnxi.com (Eric W. Biederman) writes:
> So it looks like the TODO item is to just look up what cfi standard
> says are the unlock addresses for cfi command set 2. And make certain
> the cfi hard codes are correct and nothing should be broken.
Looking at cfi specification and the AMD/Fujitsu specific
portion of it. This is what the CFI address assignment code
needs to look like:
/* Set the default CFI lock/unlock addresses */
cfi->addr_unlock1 = 0x555;
cfi->addr_unlock2 = 0x2aa;
/* Modify the unlock address if we are in compatibility mode */
if ( /* x16 in x8 mode */
((cfi->device_type == CFI_DEVICETYPE_X8) &&
(cfi->cfiq->InterfaceDesc == 2)) ||
/* x32 in x16 mode */
((cfi->device_type == CFI_DEVICETYPE_X16) &&
(cfi->cfiq->InterfaceDesc == 4)))
{
cfi->addr_unlock1 = 0xaaa;
cfi->addr_unlock2 = 0x555;
}
The previous code to handle this was quite bogus. Unless I completely
cannot read the code.
I have committed this change and corresponding change to use
cfi_send_gen_cmd.
Will people please test this?
If I don't hear any bug reports I will assume everything is perfect
and start pushing this code upstream.
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [CFT] FIX CFI cmdset 0002 for x16 and x32 devices.
2004-09-24 4:29 ` [CFT] FIX CFI cmdset 0002 for x16 and x32 devices Eric W. Biederman
@ 2004-09-28 12:15 ` Ben Dooks
0 siblings, 0 replies; 6+ messages in thread
From: Ben Dooks @ 2004-09-28 12:15 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-mtd
On Thu, Sep 23, 2004 at 10:29:03PM -0600, Eric W. Biederman wrote:
> ebiederman@lnxi.com (Eric W. Biederman) writes:
>
> > So it looks like the TODO item is to just look up what cfi standard
> > says are the unlock addresses for cfi command set 2. And make certain
> > the cfi hard codes are correct and nothing should be broken.
>
> Looking at cfi specification and the AMD/Fujitsu specific
> portion of it. This is what the CFI address assignment code
> needs to look like:
>
> /* Set the default CFI lock/unlock addresses */
> cfi->addr_unlock1 = 0x555;
> cfi->addr_unlock2 = 0x2aa;
> /* Modify the unlock address if we are in compatibility mode */
> if ( /* x16 in x8 mode */
> ((cfi->device_type == CFI_DEVICETYPE_X8) &&
> (cfi->cfiq->InterfaceDesc == 2)) ||
> /* x32 in x16 mode */
> ((cfi->device_type == CFI_DEVICETYPE_X16) &&
> (cfi->cfiq->InterfaceDesc == 4)))
> {
> cfi->addr_unlock1 = 0xaaa;
> cfi->addr_unlock2 = 0x555;
> }
>
>
> The previous code to handle this was quite bogus. Unless I completely
> cannot read the code.
>
> I have committed this change and corresponding change to use
> cfi_send_gen_cmd.
>
> Will people please test this?
I've just had a chance to test this on my EB2410ITX (BAST) and it
seems to work (erased and then zeroed the spare partition). I'll
try and bootloader upgraded with it later.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-09-28 12:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-20 23:44 Fix to jedec_probe unlock addresses Ben Dooks
2004-09-23 1:48 ` Eric W. Biederman
2004-09-23 21:01 ` Ben Dooks
2004-09-23 21:36 ` Eric W. Biederman
2004-09-24 4:29 ` [CFT] FIX CFI cmdset 0002 for x16 and x32 devices Eric W. Biederman
2004-09-28 12:15 ` Ben Dooks
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox