Re: 4byte aligned com(4) and PCI_MAPREG_TYPE_MEM

[Dennis Ferguson <dennis.c.ferguson%gmail.com@localhost>]

On 9 Feb, 2014, at 10:31 , SAITOH Masanobu <msaitoh%execsw.org@localhost> wrote:
> +#if BYTE_ORDER == BIG_ENDIAN
> +#define COM_INIT_REGS_OFFSET 3
> +#else
> +#define COM_INIT_REGS_OFFSET 0
> +#endif
[...]
> +             regs.cr_nports = COM_NPORTS * (align);                  \
> +             for (int i = 0; i < __arraycount(regs.cr_map); i++)     \
> +                     regs.cr_map[i] = com_std_map[i] * (align)       \
> +                         + COM_INIT_REGS_OFFSET;                     \

Is this correct for PCI on a big endian machine?  I'm not positive
but I thought PCI was defined to always be little endian on the bus,
so that the address of the low order byte of a register always had
the low order bits clear, and that big endian machines needed to deal
with this by byte-swapping I/O to multibyte registers, either in software
or in the PCI bridge.  Note that this is not to imply that there might
not be some other bus where the above is correct (say, obio on a big-endian
machine), I just have some doubt that it is correct for PCI.

This does suggest that it might be better to reserve single byte I/O
operations for when the hardware registers are defined to be single bytes,
and to use 4 byte operations instead when the hardware registers are defined
as being 4 bytes wide, since then the bus-dependent code should do the
right thing for you.  I realize this would do yet more damage to the driver
you are working on, however.


> +#define      COM_INIT_REGS(regs, tag, hdl, addr, align)      \
>       do {                                            \
>               regs.cr_iot = tag;                      \
>               regs.cr_ioh = hdl;                      \
>               regs.cr_iobase = addr;                  \
> -             regs.cr_nports = COM_NPORTS;            \
> +             regs.cr_nports = COM_NPORTS);           \
>       } while (0)

The ')' insertion looks odd to me.

Dennis Ferguson