RE: Big Endian/Little Endian headache in cfi_cmdset_0002.c

RE: Big Endian/Little Endian headache in cfi_cmdset_0002.c

[mark.langsdorf]

> In data writes and reads, endianness should not be a concern 
> except for the unaligned bytes which should follow the
> cfi_cmdset_0001.c code example instead of the assumption of
> a particular endianness that exists now in cfi_amdext_write_bytes_32.

	I'm not sure what you mean - as far as I can tell,
cfi_cmdset_0001.c does deal with endianness except in 3
specific incidences, two of which cfi_cmdset_0002.c doesn't
use and the third which we don't currently implement (but
should, and probably could trivially).
	Is there something else I am missing, or can I take
all these endian references out of my code with a clear
conscience?

Mark Langsdorf
Advanced Micro Devices, Inc             Tel: 512.602.3756
5204 E. Ben White Blvd. M/S 590         Fax: 512.602.5051
Austin, TX 78741                        mark.langsdorf@amd.com
	



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

RE: Big Endian/Little Endian headache in cfi_cmdset_0002.c

[Nicolas Pitre]

On Thu, 16 Nov 2000 mark.langsdorf@amd.com wrote:

> > In data writes and reads, endianness should not be a concern
> > except for the unaligned bytes which should follow the
> > cfi_cmdset_0001.c code example instead of the assumption of
> > a particular endianness that exists now in cfi_amdext_write_bytes_32.
>
> 	I'm not sure what you mean - as far as I can tell,
> cfi_cmdset_0001.c does deal with endianness except in 3
> specific incidences, ...

Could you please point them out so I could fix them please?
AFAIK cfi_cmdset_0001.c is completely endian aware.


Nicolas




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

Re: Big Endian/Little Endian headache in cfi_cmdset_0002.c

[Alice Hennessy]

mark.langsdorf@amd.com wrote:

> > In data writes and reads, endianness should not be a concern
> > except for the unaligned bytes which should follow the
> > cfi_cmdset_0001.c code example instead of the assumption of
> > a particular endianness that exists now in cfi_amdext_write_bytes_32.
>
>         I'm not sure what you mean - as far as I can tell,
> cfi_cmdset_0001.c does deal with endianness except in 3
> specific incidences, two of which cfi_cmdset_0002.c doesn't
> use and the third which we don't currently implement (but
> should, and probably could trivially).
>         Is there something else I am missing, or can I take
> all these endian references out of my code with a clear
> conscience?
>
> Mark Langsdorf
> Advanced Micro Devices, Inc             Tel: 512.602.3756
> 5204 E. Ben White Blvd. M/S 590         Fax: 512.602.5051
> Austin, TX 78741                        mark.langsdorf@amd.com
>

Sorry, I think I confused the issue with my big endian concerns.

The code that you have a problem with in cfi_amdext_write_bytes_32:

                /* if write address is aligned */
                if ((ofs & 0x03) == 0) {
                        tmp = map->read32(map, ofs);
                        tmp = be32_to_cpu(tmp);
                        memcpy((u_char *)&tmp, buf, len);
                        tmp = cpu_to_be32(tmp);
                        size = len;
                }

doesn't look like it should work for either little endian
or big endian - it just slams buf into big endian on a little endian
machine  (my big endian library
has be32_to_cpu as a nop).   This looks like a bug to me or for some
reason the coder wanted
the bytes to land in the flash in big endian order.  But this would only
be for the last bytes so it
doesn't make sense.

As far as the unaligned part of that code:

           else if ((ofs & 0x03) == 3) {
                        tmp = map->read32(map, ofs - 3);
                        tmp = (be32_to_cpu(tmp) & 0xFFFFFF00) | buf[0];
                        tmp = cpu_to_be32(tmp);
                        size = 1;
                }
                else if ((ofs & 0x03) == 2) {
                        tmp = map->read32(map, ofs - 2);
                        tmp = (be32_to_cpu(tmp) & 0xFFFF0000) | buf[1] |
(buf[0] << 8);
                        tmp = cpu_to_be32(tmp);
                        size = 2;
                }
                else if ((ofs & 0x03) == 1) {
                        tmp = map->read32(map, ofs - 1);
                        tmp = (be32_to_cpu(tmp) & 0xFF000000) | buf[2] |
(buf[1] << 8) | buf[0] << 16;
                        tmp = cpu_to_be32(tmp);
                        size = 3;
                }

This look ok except that unaligned data with lengths 1 or 2 would fail.
 Does this part work for you?

The code I was  referring to  in cfi_cmdset_0001.c is  for unaligned
writes in either write buffer or write words:

if (ofs & (CFIDEV_BUSWIDTH-1)) {
                unsigned long bus_ofs = ofs & ~(CFIDEV_BUSWIDTH-1);
                int gap = ofs - bus_ofs;
                int i = 0, n = 0;
                u_char tmp_buf[4];
                __u32 datum;

                while (gap--)
                        tmp_buf[i++] = 0xff;
                while (len && i < CFIDEV_BUSWIDTH)
                        tmp_buf[i++] = buf[n++], len--;
                while (i < CFIDEV_BUSWIDTH)
                        tmp_buf[i++] = 0xff;

                if (cfi_buswidth_is_2()) {
                        datum = *(__u16*)tmp_buf;
                } else if (cfi_buswidth_is_4()) {
                        datum = *(__u32*)tmp_buf;
                } else {
                        return -EINVAL;  /* should never happen, but be
safe */
                }

                ret = do_write_oneword(map, &cfi->chips[chipnum],
                                               bus_ofs, datum);
                if (ret)
                        return ret;

                ofs += n;
                buf += n;
                (*retlen) += n;

                ........

which just kepts the buf in data order (little endian) and also works for
unaligned data of lengths 1 or 2.

I altered it slightly for cfi_cmdset_0002.c  cfi_amdext_write_bytes_32()
because of the eventual check for read value matching write value:

 /* If it's not word-aligned, do the first bytes write */
                        if (ofs & 3) {
                                __u8 tmpbuf[4];
                                int i;
                                unsigned int data;

                                data = map->read32(map, ofs & 0xfffffffc);

                                for (i=0; i< (ofs & 3); i++)
                                        tmpbuf[i] = 0xff;
                                for (; i < min(4, len + (ofs & 3)) ; i++)
                                        tmpbuf[i] = buf[i - (ofs & 3)];
                                for (; i < 4; i++)
                                        tmpbuf[i] = 0xff;
                                data &= *(__u32 *)tmpbuf;
                                ret = do_write_oneword(map, chip, ofs &
0xfffffffc, (__u32 *) &data, 0);
                                if (ret)
                                        return ret;

                                buf += min(4-(ofs & 3), len);
                                ofs += min(4-(ofs & 3), len);
                                len -= min(4-(ofs & 3), len);


As far as my big endian environment,  I need to change the code slightly
in do_write_oneword()
so the data lands in flash in little endian order.  It seems the read
(memcpy_fromio)
is a byte read and therefore expects little endian order  - unless I am
missing something.

Alice



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

Big Endian issues

[Alice Hennessy]

Hello all,

I need some advice regarding my big endian arch (ppc). I am using
cfi_cmdset_0002.c  and there is
a built-in assumption that  words will be swapped when reading and writing to
a little endian device.
This assumption is present in cfi_build_cmd and in cfi_probe_new_chip.  Both
of these (in my configuration)
use map->write32 and map->read32 when dealing with commands and status
words.   This seems reasonable.

In dealing with data, however, map->write32 is used in writes but
map->copy_from is used in reads.
So, the assumption I assume is that map->copy_from will also swap bytes.
But in my environment,
map->copy_from (memcpy_fromio) doesn't swap bytes.  I don't think it needs to
if we make use of
map->copy_to (memcpy_toio)  when writing data (in amd_write_val and in the
immediate data check). In this case, copy_from and copy_to functions just
need to be consistent.

Any opinions?   Anyone else get this to work with big endian?
Is a change needed in my memcpy_fromio?

Alice



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

Re: Big Endian issues

[Nicolas Pitre]

On Tue, 28 Nov 2000, Alice Hennessy wrote:

> In dealing with data, however, map->write32 is used in writes but
> map->copy_from is used in reads.
> So, the assumption I assume is that map->copy_from will also swap bytes.
> But in my environment,
> map->copy_from (memcpy_fromio) doesn't swap bytes.  I don't think it needs to
> if we make use of
> map->copy_to (memcpy_toio)  when writing data (in amd_write_val and in the
> immediate data check). In this case, copy_from and copy_to functions just
> need to be consistent.

Neither of the write nor copy map methods is supposed to swap bytes.

Bytes might be swapped in cfi_build_cmd so you actually write commands and
compare with status in the device's endianness.  However data shouldn't be
swapped at all.


Nicolas



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