* jedec_probe problems with 16bit devices
@ 2004-09-04 17:37 Ben Dooks
2004-09-07 13:34 ` Thayne Harbaugh
2004-09-13 13:12 ` Thayne Harbaugh
0 siblings, 2 replies; 6+ messages in thread
From: Ben Dooks @ 2004-09-04 17:37 UTC (permalink / raw)
To: linux-mtd; +Cc: ben
I have been trying to get an SST 39LF160 on an Simtec Electronics
EB2410ITX (aka Bast). I have found some problems with jedec_probe
with non-8bit devices.
Basically, the two problems are as follows:
1) A number of functions are masking out bits from the command
addresses, but the cfi_send_gen_cmd() moves the addresses up
depending on the chip type, so the masking is not needed.
2) the cfi_send_gen_cmd() is called with CFI_DEVICETYPE_X8
instead of cfi->device_type, which causes the wrong accesses to
be generated to the chip.
This is the patch generated from the 03 Sep 2004 CVS drop:
--- mtd/drivers/mtd/chips/jedec_probe.c 2004-08-25 23:00:11.000000000 +0100
+++ linux-2.6.8.1-patched-work2/drivers/mtd/chips/jedec_probe.c 2004-09-04 18:18:51.000000000 +0100
@@ -8,6 +8,15 @@
Occasionally maintained by Thayne Harbaugh tharbaugh at lnxi dot com
*/
+/* Note from Ben Dooks:
+ *
+ * There was an attempt to mask off unused address bits depending on the
+ * device width (ie, remove A0 for 16bit). This is a proble since the
+ * command functions shift the addresses up for the chip-width, thus
+ * negating the need to mask, as all bits of the address are presented
+ * to the chip.
+ */
+
#include <linux/config.h>
#include <linux/module.h>
#include <linux/init.h>
@@ -1379,6 +1388,22 @@
ERASEINFO(0x01000,256),
}
}, {
+ .mfr_id = MANUFACTURER_SST, /* should be CFI */
+ .dev_id = SST39LF160,
+ .name = "SST 39LF160",
+ .uaddr = {
+ [0] = MTD_UADDR_0x5555_0x2AAA, /* x8 */
+ [1] = MTD_UADDR_0x5555_0x2AAA /* x16 */
+ },
+ .DevSize = SIZE_2MiB,
+ .CmdSet = P_ID_AMD_STD,
+ .NumEraseRegions= 2,
+ .regions = {
+ ERASEINFO(0x1000,256),
+ ERASEINFO(0x1000,256)
+ }
+
+ }, {
.mfr_id = MANUFACTURER_ST, /* FIXME - CFI device? */
.dev_id = M29W800DT,
.name = "ST M29W800DT",
@@ -1654,8 +1679,8 @@
* the F0 is written to */
if(cfi->addr_unlock1) {
/*printk("reset unlock called %x %x \n",cfi->addr_unlock1,cfi->addr_unlock2);*/
- cfi_send_gen_cmd(0xaa, cfi->addr_unlock1, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
- cfi_send_gen_cmd(0x55, cfi->addr_unlock2, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
+ cfi_send_gen_cmd(0xaa, cfi->addr_unlock1, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x55, cfi->addr_unlock2, base, map, cfi, cfi->device_type, NULL);
}
cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, base, map, cfi, cfi->device_type, NULL);
@@ -1700,7 +1725,6 @@
static int cfi_jedec_setup(struct cfi_private *p_cfi, int index)
{
int i,num_erase_regions;
- unsigned long mask;
__u8 uaddr;
printk("Found: %s\n",jedec_table[index].name);
@@ -1735,10 +1759,11 @@
return 0;
}
- /* Mask out address bits which are smaller than the device type */
- mask = ~(p_cfi->device_type-1);
- p_cfi->addr_unlock1 = unlock_addrs[uaddr].addr1 & mask;
- p_cfi->addr_unlock2 = unlock_addrs[uaddr].addr2 & mask;
+ /* Do not mask out `unused bits` due to the command function
+ * shifting the addresses depdning on the chip type */
+
+ p_cfi->addr_unlock1 = unlock_addrs[uaddr].addr1;
+ p_cfi->addr_unlock2 = unlock_addrs[uaddr].addr2;
return 1; /* ok */
}
@@ -1759,7 +1784,6 @@
int rc = 0; /* failure until all tests pass */
u32 mfr, id;
__u8 uaddr;
- unsigned long mask;
/*
* The IDs must match. For X16 and X32 devices operating in
@@ -1810,18 +1834,17 @@
goto match_done;
}
- mask = ~(cfi->device_type-1);
DEBUG( MTD_DEBUG_LEVEL3, "MTD %s(): check unlock addrs 0x%.4x 0x%.4x\n",
__func__, cfi->addr_unlock1, cfi->addr_unlock2 );
if ( MTD_UADDR_UNNECESSARY != uaddr && MTD_UADDR_DONT_CARE != uaddr
- && ( (unlock_addrs[uaddr].addr1 & mask) != cfi->addr_unlock1 ||
- (unlock_addrs[uaddr].addr2 & mask) != cfi->addr_unlock2 ) ) {
+ && ( unlock_addrs[uaddr].addr1 != cfi->addr_unlock1 ||
+ unlock_addrs[uaddr].addr2 != cfi->addr_unlock2 ) ) {
DEBUG( MTD_DEBUG_LEVEL3,
"MTD %s(): 0x%.4lx 0x%.4lx did not match\n",
__func__,
- unlock_addrs[uaddr].addr1 & mask,
- unlock_addrs[uaddr].addr2 & mask);
+ unlock_addrs[uaddr].addr1,
+ unlock_addrs[uaddr].addr2);
goto match_done;
}
@@ -1857,10 +1880,10 @@
*/
DEBUG( MTD_DEBUG_LEVEL3, "MTD %s(): return to ID mode\n", __func__ );
if(cfi->addr_unlock1) {
- cfi_send_gen_cmd(0xaa, cfi->addr_unlock1, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
- cfi_send_gen_cmd(0x55, cfi->addr_unlock2, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
+ cfi_send_gen_cmd(0xaa, cfi->addr_unlock1, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x55, cfi->addr_unlock2, base, map, cfi, cfi->device_type, NULL);
}
- cfi_send_gen_cmd(0x90, cfi->addr_unlock1, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
+ cfi_send_gen_cmd(0x90, cfi->addr_unlock1, base, map, cfi, cfi->device_type, NULL);
/* FIXME - should have a delay before continuing */
match_done:
@@ -1876,16 +1899,13 @@
retry:
if (!cfi->numchips) {
- unsigned long mask = ~(cfi->device_type-1);
-
uaddr_idx++;
if (MTD_UADDR_UNNECESSARY == uaddr_idx)
return 0;
- /* Mask out address bits which are smaller than the device type */
- cfi->addr_unlock1 = unlock_addrs[uaddr_idx].addr1 & mask;
- cfi->addr_unlock2 = unlock_addrs[uaddr_idx].addr2 & mask;
+ cfi->addr_unlock1 = unlock_addrs[uaddr_idx].addr1;
+ cfi->addr_unlock2 = unlock_addrs[uaddr_idx].addr2;
}
/* Make certain we aren't probing past the end of map */
@@ -1916,10 +1936,10 @@
/* Autoselect Mode */
if(cfi->addr_unlock1) {
- cfi_send_gen_cmd(0xaa, cfi->addr_unlock1, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
- cfi_send_gen_cmd(0x55, cfi->addr_unlock2, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
+ cfi_send_gen_cmd(0xaa, cfi->addr_unlock1, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x55, cfi->addr_unlock2, base, map, cfi, cfi->device_type, NULL);
}
- cfi_send_gen_cmd(0x90, cfi->addr_unlock1, base, map, cfi, CFI_DEVICETYPE_X8, NULL);
+ cfi_send_gen_cmd(0x90, cfi->addr_unlock1, base, map, cfi, cfi->device_type, NULL);
/* FIXME - should have a delay before continuing */
if (!cfi->numchips) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: jedec_probe problems with 16bit devices
2004-09-04 17:37 jedec_probe problems with 16bit devices Ben Dooks
@ 2004-09-07 13:34 ` Thayne Harbaugh
2004-09-07 15:08 ` David Woodhouse
2004-09-07 23:52 ` Christopher Hoover
2004-09-13 13:12 ` Thayne Harbaugh
1 sibling, 2 replies; 6+ messages in thread
From: Thayne Harbaugh @ 2004-09-07 13:34 UTC (permalink / raw)
To: Ben Dooks; +Cc: ben, linux-mtd, ch
On Sat, 2004-09-04 at 18:37 +0100, Ben Dooks wrote:
> I have been trying to get an SST 39LF160 on an Simtec Electronics
> EB2410ITX (aka Bast). I have found some problems with jedec_probe
> with non-8bit devices.
> 1) A number of functions are masking out bits from the command
> addresses, but the cfi_send_gen_cmd() moves the addresses up
> depending on the chip type, so the masking is not needed.
Dave, can you comment on this? It seems like it was something that you
worked with - something about not probing at unaligned addresses?
> 2) the cfi_send_gen_cmd() is called with CFI_DEVICETYPE_X8
> instead of cfi->device_type, which causes the wrong accesses to
> be generated to the chip.
It seems like the CFI_DEVICETYPE_X8 was changed to cfi->device_type at
one point and it broke some 16-bit chips.
I think all of the CFI_DEVICETYPE_X8s were removed from jedec_probe.c
1.30, but then some were added back in at 1.36. I'm trying to find the
discussion as to why this happened, but what I'm finding isn't very
descriptive:
http://lists.infradead.org/pipermail/linux-mtd/2003-October/008685.html
I seem to remember IRC'ing with Christopher Hoover on this. Maybe he
can remember something? Christopher? My memory is somewhat murky.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: jedec_probe problems with 16bit devices
2004-09-07 13:34 ` Thayne Harbaugh
@ 2004-09-07 15:08 ` David Woodhouse
2004-09-07 23:52 ` Christopher Hoover
1 sibling, 0 replies; 6+ messages in thread
From: David Woodhouse @ 2004-09-07 15:08 UTC (permalink / raw)
To: tharbaugh; +Cc: ben, linux-mtd, ch
On Tue, 2004-09-07 at 07:34 -0600, Thayne Harbaugh wrote:
> On Sat, 2004-09-04 at 18:37 +0100, Ben Dooks wrote:
> > I have been trying to get an SST 39LF160 on an Simtec Electronics
> > EB2410ITX (aka Bast). I have found some problems with jedec_probe
> > with non-8bit devices.
>
> > 1) A number of functions are masking out bits from the command
> > addresses, but the cfi_send_gen_cmd() moves the addresses up
> > depending on the chip type, so the masking is not needed.
>
> Dave, can you comment on this? It seems like it was something that you
> worked with - something about not probing at unaligned addresses?
The probe code used to probe for impossible combinations at unaligned
addresses. So people hacked around it by masking out the lower bits. Now
we fixed the probe code. Those hacks can go.
> > 2) the cfi_send_gen_cmd() is called with CFI_DEVICETYPE_X8
> > instead of cfi->device_type, which causes the wrong accesses to
> > be generated to the chip.
>
> It seems like the CFI_DEVICETYPE_X8 was changed to cfi->device_type at
> one point and it broke some 16-bit chips.
>
> I think all of the CFI_DEVICETYPE_X8s were removed from jedec_probe.c
> 1.30, but then some were added back in at 1.36. I'm trying to find the
> discussion as to why this happened, but what I'm finding isn't very
> descriptive:
We need to agree --- either we can use cfi->device_type for all, or we
use CFI_DEVICETYPE_X8 for all, and we set the uaddr in the device table
accordingly.
I think we had CFI_DEVICETYPE_X8 so that we could hard-code different
addresses for different modes of the same chip, on the basis that the
addresses couldn't be calculated from a single entry in the chip table
and the cfi->device_type.
I think that's unnecessary though -- I think we should have only a
_single_ uaddr field in the device table, and use cfi->device_type with
it.
--
dwmw2
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: jedec_probe problems with 16bit devices
2004-09-07 13:34 ` Thayne Harbaugh
2004-09-07 15:08 ` David Woodhouse
@ 2004-09-07 23:52 ` Christopher Hoover
1 sibling, 0 replies; 6+ messages in thread
From: Christopher Hoover @ 2004-09-07 23:52 UTC (permalink / raw)
To: tharbaugh, 'Ben Dooks'; +Cc: ben, linux-mtd
> It seems like the CFI_DEVICETYPE_X8 was changed to cfi->device_type at
> one point and it broke some 16-bit chips.
>
> I think all of the CFI_DEVICETYPE_X8s were removed from jedec_probe.c
> 1.30, but then some were added back in at 1.36. I'm trying
> to find the
> discussion as to why this happened, but what I'm finding isn't very
> descriptive:
>
> http://lists.infradead.org/pipermail/linux-mtd/2003-October/00
> 8685.html
>
> I seem to remember IRC'ing with Christopher Hoover on this. Maybe he
> can remember something? Christopher? My memory is somewhat murky.
I don't have any new information to report -- only what is noted above, that
some regression in the mtd tree about the time the CFI_DEVICETYPE_X8's
disappeared. It was never fixed while I worked with the board. It may be
fine now, but unfortunately I am unable to check easily.
Can we locate someone with a suitable platform and coax them into testing
the latest code?
-ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: jedec_probe problems with 16bit devices
2004-09-04 17:37 jedec_probe problems with 16bit devices Ben Dooks
2004-09-07 13:34 ` Thayne Harbaugh
@ 2004-09-13 13:12 ` Thayne Harbaugh
2004-09-14 22:46 ` Ben Dooks
1 sibling, 1 reply; 6+ messages in thread
From: Thayne Harbaugh @ 2004-09-13 13:12 UTC (permalink / raw)
To: Ben Dooks; +Cc: ch, linux-mtd
On Sat, 2004-09-04 at 18:37 +0100, Ben Dooks wrote:
> I have been trying to get an SST 39LF160 on an Simtec Electronics
> EB2410ITX (aka Bast). I have found some problems with jedec_probe
> with non-8bit devices.
>
> Basically, the two problems are as follows:
>
> 1) A number of functions are masking out bits from the command
> addresses, but the cfi_send_gen_cmd() moves the addresses up
> depending on the chip type, so the masking is not needed.
>
> 2) the cfi_send_gen_cmd() is called with CFI_DEVICETYPE_X8
> instead of cfi->device_type, which causes the wrong accesses to
> be generated to the chip.
committed patch - please try it.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: jedec_probe problems with 16bit devices
2004-09-13 13:12 ` Thayne Harbaugh
@ 2004-09-14 22:46 ` Ben Dooks
0 siblings, 0 replies; 6+ messages in thread
From: Ben Dooks @ 2004-09-14 22:46 UTC (permalink / raw)
To: Thayne Harbaugh; +Cc: ch, linux-mtd
On Mon, Sep 13, 2004 at 07:12:39AM -0600, Thayne Harbaugh wrote:
> On Sat, 2004-09-04 at 18:37 +0100, Ben Dooks wrote:
> > I have been trying to get an SST 39LF160 on an Simtec Electronics
> > EB2410ITX (aka Bast). I have found some problems with jedec_probe
> > with non-8bit devices.
> >
> > Basically, the two problems are as follows:
> >
> > 1) A number of functions are masking out bits from the command
> > addresses, but the cfi_send_gen_cmd() moves the addresses up
> > depending on the chip type, so the masking is not needed.
> >
> > 2) the cfi_send_gen_cmd() is called with CFI_DEVICETYPE_X8
> > instead of cfi->device_type, which causes the wrong accesses to
> > be generated to the chip.
>
> committed patch - please try it.
I'll try this on my EB2410ITX as soon as I can, would like to
polish the NOR flash driver and get it uploaded.
--
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-14 22:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-04 17:37 jedec_probe problems with 16bit devices Ben Dooks
2004-09-07 13:34 ` Thayne Harbaugh
2004-09-07 15:08 ` David Woodhouse
2004-09-07 23:52 ` Christopher Hoover
2004-09-13 13:12 ` Thayne Harbaugh
2004-09-14 22:46 ` Ben Dooks
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox