Re: [flashrom] [PATCH] Try to handle non-x86 sanely

[Uwe Hermann]

On Wed, May 26, 2010 at 12:34:49AM +0200, Carl-Daniel Hailfinger wrote:
> Handle the following architectures in generic flashrom code:
> x86/x86_64 (little endian)
> PowerPC (big endian)
> MIPS (big+little endian)

Build-tested on x86 and x86_64. Runtime-tested on a random
FWH-board/-chip (PREW) on Linux x86_64, still works fine.
> Signed-off-by: Carl-Daniel Hailfinger 
> Acked-by: Vladimir 'phcoder/φ-coder' Serbinenko

Acked-by: Uwe Hermann
> Index: flashrom-arch_abstraction/hwaccess.c
>
> --- flashrom-arch_abstraction/hwaccess.c	(Revision 1008)
> +++ flashrom-arch_abstraction/hwaccess.c	(Arbeitskopie)

[...]
> +#elif defined(__powerpc__) || defined(__powerpc64__) || defined(__ppc__) || defined(__ppc64__)
>
> +static inline void sync_primitive(void)
> +{
> asm("eieio" : : : "memory");

This asm could use a nice long comment about what it does exactly and
why it's needed etc.
> +#if !defined (__FLASHROM_BIG_ENDIAN__) && !defined (__FLASHROM_LITTLE_ENDIAN__)
> +#error Unable to determine endianness. Please add support for your arch or libc.
> +#endif
>
> +#define ___constant_swab8(x) ((uint8_t)(				\

^
                                  space here, please
> (((uint8_t)(x) & (uint8_t)0xffU))))
>
> +#define ___constant_swab16(x) ((uint16_t)(				\

^
> (((uint16_t)(x) & (uint16_t)0x00ffU) << 8) |			\
> (((uint16_t)(x) & (uint16_t)0xff00U) >> 8)))
>
> +#define ___constant_swab32(x) ((uint32_t)(				\

^
> (((uint32_t)(x) & (uint32_t)0x000000ffUL) << 24) |		\
> (((uint32_t)(x) & (uint32_t)0x0000ff00UL) <<  8) |		\
> (((uint32_t)(x) & (uint32_t)0x00ff0000UL) >>  8) |		\
> (((uint32_t)(x) & (uint32_t)0xff000000UL) >> 24)))
>
> +#define ___constant_swab64(x) ((uint64_t)(				\

^
Ditto for these other 3 places.
> +#elif defined(__powerpc__) || defined(__powerpc64__) || defined(__ppc__) || defined(__ppc64__)
>
> +/* PCI port I/O is not yet implemented on PowerPC. */
>
> +#elif defined (__mips) || defined (__mips__) || defined (_mips) || defined (mips)
>
> +/* PCI port I/O is not yet implemented on MIPS. */
>
> +#else
>
> +#error Unknown architecture, please check if it supports PCI port IO.

^^
"IO" -> "I/O" for consistency.
> Index: flashrom-arch_abstraction/nic3com.c
>
> --- flashrom-arch_abstraction/nic3com.c	(Revision 1008)
> +++ flashrom-arch_abstraction/nic3com.c	(Arbeitskopie)
> @@ -23,6 +23,8 @@
>  #include <sys/types.h>
>  #include "flash.h"
>
> +#if defined(__i386__) || defined(__x86_64__)
>
> #define BIOS_ROM_ADDR		0x04
>  #define BIOS_ROM_DATA		0x08
>  #define INT_STATUS		0x0e
> @@ -112,3 +114,7 @@
>    OUTL((uint32_t)addr, io_base_addr + BIOS_ROM_ADDR);
>    return INB(io_base_addr + BIOS_ROM_DATA);
>  }
>
> +#else
> +#error Unsupported architecture for nic3com. You need a way to emulate port IO access.

"IO" -> "I/O" for consistency.

Also, wouldn't the nicrealtek code need the same #ifdefs and #error?

Uwe.