CFI problems with 32bit bus and 4 devices

CFI problems with 32bit bus and 4 devices

[Stéphane Laroche]

Hi,

I've just updated to the latest CVS and my CFI AMD chips are not
accessible anymore.

I lokked briefly into it and saw that the cfi->interleave and
cfi->device_type were removed from offset calculations when accessing
the devices.

So, for my setup, which has 4 x16 devices on a 32 bit bus (AMDs),  the
CFI query structure is located at offsets 0x80, 0x88, 0x90, etc.

cfi_read_query()  uses only the buswidth to calculate the offset, which
is not general enough (it used to be like that before I played with the
code a bit last summer).  It's obviously wrong in my case ( 0x10 << 2 !=
0x80 ).

Rewriting cfi_read_query like this made the CFI query structure
readable:

static inline __u8 cfi_read_query(struct map_info *map, __u32 base,
__u32 addr)
{
    struct cfi_private *cfi = map->fldrv_priv;
    addr *= cfi->interleave * cfi->device_type;  /* instead of addr <<
(buswidth / 2) */
    if (cfi_buswidth_is_1()) {
        return map->read8(map, base + addr);
    } else if (cfi_buswidth_is_2()) {
        return cfi16_to_cpu(map->read16(map, base + addr));
    } else if (cfi_buswidth_is_4()) {
        return cfi32_to_cpu(map->read32(map, base + addr));
    } else {
        return 0;
    }
}

With this change, the chips are now properly recognized.  But I can't
still not use them (reads are wrong), so I think I have to look at
cfi_cmdset_002.c to bring back the use of cfi->interleave in some
calculations...

Any comments?  Is it possible that I'm the only one using that kind of
geometry?

-Stephane



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Nicolas Pitre]

On Wed, 10 Jan 2001, Stiphane Laroche wrote:

> Hi,
>
> I've just updated to the latest CVS and my CFI AMD chips are not
> accessible anymore.
>
> I lokked briefly into it and saw that the cfi->interleave and
> cfi->device_type were removed from offset calculations when accessing
> the devices.
>
> So, for my setup, which has 4 x16 devices on a 32 bit bus (AMDs),  the
> CFI query structure is located at offsets 0x80, 0x88, 0x90, etc.

[...]

> Any comments?  Is it possible that I'm the only one using that kind of
> geometry?

No.  4 x16 Intel chips on a 32-bit bus is now broken as well.  I didn't
investigate it yet though.


Nicolas



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[David Woodhouse]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF8, Size: 1094 bytes --]

On Wed, 10 Jan 2001, [iso-8859-1] Stéphane Laroche wrote:

> So, for my setup, which has 4 x16 devices on a 32 bit bus (AMDs),  the
> CFI query structure is located at offsets 0x80, 0x88, 0x90, etc.
>
> cfi_read_query()  uses only the buswidth to calculate the offset, which
> is not general enough (it used to be like that before I played with the
> code a bit last summer).  It's obviously wrong in my case ( 0x10 << 2 !=
> 0x80 ).

Sorry, mea culpa. I'd forgotten - or just didn't realise this arrangement
was used. How did {amd,cfi}_send_gen_cmd() send a command to all four
chips? With two separate 32-bit writes?

> Any comments?  Is it possible that I'm the only one using that kind of
> geometry?

(reads responses) Evidently not. Hmmm.

Technically, I think this setup would fit into the original model as
buswidth == 8, and we should be using read64/write64 for accesses to the
flash.

Just how crap is gcc's long long support these days? Could we use __u64
for this?

-- 
dwmw2





To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Stéphane Laroche]

David Woodhouse wrote:
> Sorry, mea culpa. I'd forgotten - or just didn't realise this arrangement
> was used. How did {amd,cfi}_send_gen_cmd() send a command to all four
> chips? With two separate 32-bit writes?
> ...
> Technically, I think this setup would fit into the original model as
> buswidth == 8, and we should be using read64/write64 for accesses to the
> flash.

Bus width is still 32 bits and you can access all 4 chips with one read or
write operation.  CFI commands are 8bit wide.

It's just the CFI offset that moves, according to the device type (8 bit, 16
bit or 32 bit), the mode the device is in (a 16bit or 32 bit device can
operate in 8 bit mode) and the number of devices sitting on the bus (or the
bus width).

Although using buswidth == 8 in that specific case may work in that specific
case, you would read 4 bytes too many, which are invalid addresses for
16bit/32bit CFI devices in x8 mode (I guess nothing bad happens by reading
those but you never know).

cfi_send_gen_cmd() uses the interleave and device_type information and
therefore was and is still doing "the right thing" (tm).

-Stephane




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[David Woodhouse]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF8, Size: 1762 bytes --]

On Wed, 10 Jan 2001, [iso-8859-1] Stéphane Laroche wrote:

> Although using buswidth == 8 in that specific case may work in that specific
> case, you would read 4 bytes too many, which are invalid addresses for
> 16bit/32bit CFI devices in x8 mode (I guess nothing bad happens by reading
> those but you never know).

I'd expect the chips to just ignore the A0 line completely, so those
addresses are just aliases for the first 4 bytes. And I think that the
whole point in the map driver was to eliminate such aliasing and stuff,
and just to present the address space of the chip linearly.

It's arguable either way, really - perhaps you could claim that the CFI
code ought to know about such things and _expect_ the LSbit of the
'offset' it passes to the map driver to be equivalent to the A0 line of
the chip, even if the A0 line of the chip isn't valid. But I think doing
it in the map driver keeps the code simpler, and is probably worth it.
Shifts are cheap enough, aren't they?



NB: Some time in the future I may optimise away some of the map drivers'
access functions in certain cases - so many of them are so trivial, and so
many people compile with only one map driver.

#ifdef MAP_DRIVER_OPTIMISE_HACK
#define MAP_WRITE8(map, d, ofs) do { *(__u8 *)map->map_priv_1 + ofs = d } while(0)
#else
#define MAP_WRITE8(map, d, ofs) map->write8(map, d, ofs)

etc.

For this case, it'd be
#define MAP_WRITE32(map, d, ofs) do { *(__u32 *)map->map_priv_1 + (ofs>>1) = d } while(0)

At that point, you'd have the shift in the same function block as the
original calculation of 'ofs' and it ought to get completely optimised
away anyway.

-- 
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Nicolas Pitre]

On Wed, 10 Jan 2001, David Woodhouse wrote:

> On Wed, 10 Jan 2001, [iso-8859-1] Stéphane Laroche wrote:
>
> > So, for my setup, which has 4 x16 devices on a 32 bit bus (AMDs),  the
> > CFI query structure is located at offsets 0x80, 0x88, 0x90, etc.
> >
> > cfi_read_query()  uses only the buswidth to calculate the offset, which
> > is not general enough (it used to be like that before I played with the
> > code a bit last summer).  It's obviously wrong in my case ( 0x10 << 2 !=
> > 0x80 ).
>
> Sorry, mea culpa. I'd forgotten - or just didn't realise this arrangement
> was used. How did {amd,cfi}_send_gen_cmd() send a command to all four
> chips? With two separate 32-bit writes?

No.  Usually the actual x16 flash devices are able to operate in 8-bit mode
(selected with a pin tied to VCC or GND I think).  Then you route four of
them in parallel on a 32-bit bus.  So in this case buswidth=4, interleave=4,
chip_type=2.  That was the case that needed to be fixed for the Intel write
buffer size lately.


Nicolas



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[David Woodhouse]

On Wed, 10 Jan 2001, Nicolas Pitre wrote:

> No.  Usually the actual x16 flash devices are able to operate in 8-bit mode
> (selected with a pin tied to VCC or GND I think).  Then you route four of
> them in parallel on a 32-bit bus.

OK, sorry - I misunderstood. I was thinking of four x16 devices in x16
mode.  (Which would make a certain amount of sense - although you can't
drive them all with a single bus access, you can still have all four
writing simultaneously, hence achieving four times the write bandwidth of
a single chip.)

So in this case, you've essentially got 4 8-bit devices with the address
lines all shifted left by one bit, right?

I thought the whole point in the 8-bit emulation mode was that you just
didn't connect the chips' A0 line, and then you pretended they really were
8-bit chips, rather than having to add all this extra support to the
drivers (and all the extra probe setups, which I've only just started to
understand). Although having seen the schematics for the LART, I suppose
we ought to be grateful that the address lines are at least connected in
_order_ :)

But just like the LART, I think we should probably fix up the address line
miswiring in the map driver, by shifting the requested address back to the
right by one bit to make up for the offset in the hardware. It'll allow us
to keep the actual CFI code simpler.

This kind of thing is really what the separation between chip and map
drivers was designed for - to prevent the CFI drivers from having to know
about the variety of ways in which the chips' address lines can be
connected to the CPU.

So - are x16 devices in x8 mode _really_ compatible with x8 devices if you
shift the address lines over by one? Common sense would imply that's the
case. Bitter experience would imply that the manufacturers have
probably screwed it up in interesting ways.

--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Stéphane Laroche]

David Woodhouse wrote:

> So - are x16 devices in x8 mode _really_ compatible with x8 devices if you
> shift the address lines over by one? Common sense would imply that's the
> case. Bitter experience would imply that the manufacturers have
> probably screwed it up in interesting ways.

The only difference between the 2 modes are the CFI addresses (you have to
multiply them by 2 in x8 mode).  So you cannot cover that in the map driver,
which doesn't know the difference between a CFI and a normal operation.

-Stephane




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[David Woodhouse]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF8, Size: 903 bytes --]

On Wed, 10 Jan 2001, [iso-8859-1] Stéphane Laroche wrote:

> The only difference between the 2 modes are the CFI addresses (you have to
> multiply them by 2 in x8 mode).  So you cannot cover that in the map driver,
> which doesn't know the difference between a CFI and a normal operation.

"BYTE ENABLE: BYTE# low places the device in x8 mode. All data is then
input or output on DQ0-DQ7, while DQ8-DQ15 float. Address A0 selects
between the high and low byte. BYTE# high places the device in x16 mode,
and turns off the A0 input buffer. Address A1 then becomes the lowest
order address."

Yep, I was wrong. It couldn't work the way I was thinking of - you'd have
too few address lines to access the whole chip in x8 mode.

We do need to handle it in the CFI code, as we did before I broke it.

-- 
dwmw2






To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Nicolas Pitre]

On Wed, 10 Jan 2001, David Woodhouse wrote:

> On Wed, 10 Jan 2001, Nicolas Pitre wrote:
>
> > No.  Usually the actual x16 flash devices are able to operate in 8-bit mode
> > (selected with a pin tied to VCC or GND I think).  Then you route four of
> > them in parallel on a 32-bit bus.
>
> OK, sorry - I misunderstood. I was thinking of four x16 devices in x16
> mode.  (Which would make a certain amount of sense - although you can't
> drive them all with a single bus access, you can still have all four
> writing simultaneously, hence achieving four times the write bandwidth of
> a single chip.)

This would become ugly IMHO to interleave multiple chips over multiple bus
access.  Rather make one bank follow the other instead.

> So in this case, you've essentially got 4 8-bit devices with the address
> lines all shifted left by one bit, right?
>
> I thought the whole point in the 8-bit emulation mode was that you just
> didn't connect the chips' A0 line, and then you pretended they really were
> 8-bit chips, rather than having to add all this extra support to the
> drivers

>From what I understand that's mostly that, except for the query structure
which gets expanded on twice the address range.  All the rest should be the
same.


Nicolas



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Alice Hennessy]

Stéphane Laroche wrote:

> Hi,
>
> I've just updated to the latest CVS and my CFI AMD chips are not
> accessible anymore.
>
> I lokked briefly into it and saw that the cfi->interleave and
> cfi->device_type were removed from offset calculations when accessing
> the devices.
>
> So, for my setup, which has 4 x16 devices on a 32 bit bus (AMDs),  the
> CFI query structure is located at offsets 0x80, 0x88, 0x90, etc.
>
> cfi_read_query()  uses only the buswidth to calculate the offset, which
> is not general enough (it used to be like that before I played with the
> code a bit last summer).  It's obviously wrong in my case ( 0x10 << 2 !=
> 0x80 ).
>
> Rewriting cfi_read_query like this made the CFI query structure
> readable:
>
> static inline __u8 cfi_read_query(struct map_info *map, __u32 base,
> __u32 addr)
> {
>     struct cfi_private *cfi = map->fldrv_priv;
>     addr *= cfi->interleave * cfi->device_type;  /* instead of addr <<
> (buswidth / 2) */
>     if (cfi_buswidth_is_1()) {
>         return map->read8(map, base + addr);
>     } else if (cfi_buswidth_is_2()) {
>         return cfi16_to_cpu(map->read16(map, base + addr));
>     } else if (cfi_buswidth_is_4()) {
>         return cfi32_to_cpu(map->read32(map, base + addr));
>     } else {
>         return 0;
>     }
> }
>
> With this change, the chips are now properly recognized.  But I can't
> still not use them (reads are wrong), so I think I have to look at
> cfi_cmdset_002.c to bring back the use of cfi->interleave in some
> calculations...
>
> Any comments?  Is it possible that I'm the only one using that kind of
> geometry?
>
> -Stephane
>
> To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

I just ran into the same problem with my one 8x16 intel chip which is set
up in 8 bit mode.

I think the fix should be:

static inline __u8 cfi_read_query(struct map_info *map, __u32 base, __u32
query_addr, int int
erleave, int type)
{
        __u32 addr = base + cfi_build_query_addr(query_addr, interleave,
type);

        if (cfi_buswidth_is_1()) {
                return map->read8(map, addr);
        } else if (cfi_buswidth_is_2()) {
                return cfi16_to_cpu(map->read16(map, addr));
        } else if (cfi_buswidth_is_4()) {
                return cfi32_to_cpu(map->read32(map, addr));
        } else {
                return 0;
        }
}

with cfi_build_query_addr () the same as cfi_build_cmd_addr():

static inline __u32 cfi_build_query_addr(__u32 cmd_ofs, int interleave, int
type)
{
        return (cmd_ofs * type) * interleave;
}

So that the address of the query is adjusted for the 8 bit selection of an
8x16 chip.
I think the correct address is taken from the combo of interleave and type
( which is correctly set up to be
CFI_DEVICETYPE_X16  for this configuration).


I think we need to pass in the interleave and type because the
map->fldrv_priv
isn't set up before the first query call.

Alice







To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Alice Hennessy]

Alice Hennessy wrote:

> Stéphane Laroche wrote:
>
> > Hi,
> >
> > I've just updated to the latest CVS and my CFI AMD chips are not
> > accessible anymore.
> >
> > I lokked briefly into it and saw that the cfi->interleave and
> > cfi->device_type were removed from offset calculations when accessing
> > the devices.
> >
> > So, for my setup, which has 4 x16 devices on a 32 bit bus (AMDs),  the
> > CFI query structure is located at offsets 0x80, 0x88, 0x90, etc.
> >
> > cfi_read_query()  uses only the buswidth to calculate the offset, which
> > is not general enough (it used to be like that before I played with the
> > code a bit last summer).  It's obviously wrong in my case ( 0x10 << 2 !=
> > 0x80 ).
> >
> > Rewriting cfi_read_query like this made the CFI query structure
> > readable:
> >
> > static inline __u8 cfi_read_query(struct map_info *map, __u32 base,
> > __u32 addr)
> > {
> >     struct cfi_private *cfi = map->fldrv_priv;
> >     addr *= cfi->interleave * cfi->device_type;  /* instead of addr <<
> > (buswidth / 2) */
> >     if (cfi_buswidth_is_1()) {
> >         return map->read8(map, base + addr);
> >     } else if (cfi_buswidth_is_2()) {
> >         return cfi16_to_cpu(map->read16(map, base + addr));
> >     } else if (cfi_buswidth_is_4()) {
> >         return cfi32_to_cpu(map->read32(map, base + addr));
> >     } else {
> >         return 0;
> >     }
> > }
> >
> > With this change, the chips are now properly recognized.  But I can't
> > still not use them (reads are wrong), so I think I have to look at
> > cfi_cmdset_002.c to bring back the use of cfi->interleave in some
> > calculations...
> >
> > Any comments?  Is it possible that I'm the only one using that kind of
> > geometry?
> >
> > -Stephane
> >
> > To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org
>
> I just ran into the same problem with my one 8x16 intel chip which is set
> up in 8 bit mode.
>
> I think the fix should be:
>
> static inline __u8 cfi_read_query(struct map_info *map, __u32 base, __u32
> query_addr, int int
> erleave, int type)
> {
>         __u32 addr = base + cfi_build_query_addr(query_addr, interleave,
> type);
>
>         if (cfi_buswidth_is_1()) {
>                 return map->read8(map, addr);
>         } else if (cfi_buswidth_is_2()) {
>                 return cfi16_to_cpu(map->read16(map, addr));
>         } else if (cfi_buswidth_is_4()) {
>                 return cfi32_to_cpu(map->read32(map, addr));
>         } else {
>                 return 0;
>         }
> }
>
> with cfi_build_query_addr () the same as cfi_build_cmd_addr():
>
> static inline __u32 cfi_build_query_addr(__u32 cmd_ofs, int interleave, int
> type)
> {
>         return (cmd_ofs * type) * interleave;
> }
>
> So that the address of the query is adjusted for the 8 bit selection of an
> 8x16 chip.
> I think the correct address is taken from the combo of interleave and type
> ( which is correctly set up to be
> CFI_DEVICETYPE_X16  for this configuration).
>
> I think we need to pass in the interleave and type because the
> map->fldrv_priv
> isn't set up before the first query call.
>
> Alice

Nevermind,  better to just put back in the ofs_factor in all the
cfi_read_query calls:

int ofs_factor = cfi->interleave * cfi->device_type;
((unsigned char *)cfi.cfiq)[i] = cfi_read_query(map,base + ((0x10 +
i)*ofs_factor));

I should have checked this change with both my test boards, sorry.

Alice




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: CFI problems with 32bit bus and 4 devices

[Michael Thompson]

dwmw2 wrote:
> 
> Technically, I think this setup would fit into the original model as
> buswidth == 8, and we should be using read64/write64 for accesses to the
> flash.
> 
> Just how crap is gcc's long long support these days? Could we use __u64
> for this?

Actually, I just got buswidth==8 working on a 32-bit ppc (my configuration
is 4 devices in 16 bit mode).  However, I found that I could use __u64 for
passing data around, but __u64 would not work when accessing the devices..
For this, I had to use a little trick: floating point loads and stores.
On the ppc, these are 64 bits wide.

I'd be willing to submit patches...  Shall I send them to you?

-Michael Thompson
mickey@berkeley.innomedia.com


To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org