Big Endian/Little Endian headache in cfi_cmdset_0002.c

[Alice Hennessy]

mark.langsdorf at 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 at 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 at infradead.org