Re: [flashrom] [PATCH 03/10] Various cross-platform fixes.

[Carl-Daniel Hailfinger]

Am 10.07.2013 21:17 schrieb Stefan Tauner:
> Improve compilation with libpayload (compiling flashrom.c and
> linking is still broken):
> disable Ponyprog (which enforced serial.c compilation)
> make errno available where it is needed
>
> Fix internal.c for non-x86 and enable cb parsing on ARM.
>
> Fix mingw builds by using its __USE_MINGW_ANSI_STDIO macro
> and gnu_printf definition for printf format style checking.
> See http://sourceforge.net/apps/trac/mingw-w64/wiki/gnu%20printf
> This requires inclusion of stdio.h in flash.h.
>
> Fix order of libraries in the Makefile:
> FEATURE_LIBS needs to come *after* PCILIBS in case ZLIB is needed by it.
>
> Signed-off-by: Carl-Daniel Hailfinger 
> Signed-off-by: Stefan Tauner

Apart from the library order, I'm happy with it.
> diff --git a/Makefile b/Makefile
> index 805290c..34bbab8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -632,7 +641,7 @@ ifeq ($(ARCH), x86)
>  endif
>
> $(PROGRAM)$(EXEC_SUFFIX): $(OBJS)
> $(CC) $(LDFLAGS) -o $(PROGRAM)$(EXEC_SUFFIX) $(OBJS) $(FEATURE_LIBS) $(LIBS) $(PCILIBS) $(USBLIBS)
> $(CC) $(LDFLAGS) -o $(PROGRAM)$(EXEC_SUFFIX) $(OBJS) $(LIBS) $(PCILIBS) $(USBLIBS) $(FEATURE_LIBS)

I believe this is partially wrong. AFAICS the order should be:
LIBS, PCILIBS, FEATURE_LIBS, USBLIBS.

Explanation: The linker doesn't remember defined symbols if they were
not on its list of needed symbols. This means you have to first specify
a library and then its dependencies.

LIBS is a misnomer (not generic libraries needed for everything, but
rather stuff like getopt and socket for some OS). This confused me
initially. The place of LIBS can be pretty much anywhere because it is
not needed by any other library.
FEATURE_LIBS can contain libftdi (so it needs to come before USBLIBS
which satisifies libftdi dependencies), but it also can contain libz (so
it needs to come after PCILIBS which may have libz as dependency).

To really get this in good shape, we should move libz into LIBS instead
of FEATURE_LIBS, but that breaks other parts of the Makefile. I really
really want a separate configure script to solve this.
> libflashrom.a: $(LIBFLASHROM_OBJS)
>    $(AR) rcs $@ $^
> diff --git a/physmap.c b/physmap.c
> index 524f558..644ee35 100644
> --- a/physmap.c
> +++ b/physmap.c
> @@ -120,15 +120,6 @@ void *sys_physmap(unsigned long phys_addr, size_t len)
>  void physunmap(void *virt_addr, size_t len)
>  {
>  }
>
> -int setup_cpu_msr(int cpu)
> -{
> return 0;
>
> -}
>
> -void cleanup_cpu_msr(void)
> -{
> -}
>  #elif defined(__MACH__) && defined(__APPLE__)
>
> #define MEM_DEV "DirectHW"
> @@ -568,6 +559,15 @@ int libpayload_wrmsr(int addr, msr_t msr)
>    _wrmsr(addr, msr.lo | ((unsigned long long)msr.hi << 32));
>    return 0;
>  }
>
> +int setup_cpu_msr(int cpu)
> +{
> return 0;
>
> +}
>
> +void cleanup_cpu_msr(void)
> +{
> +}
>  #else
>  /* default MSR implementation */
>  msr_t rdmsr(int addr)

The two hunks above are what I wanted in patch 2 of this series. They
fit a lot better here, though, so don't change it. Sorry.

With the library order fixed, this is
Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[PATCH 03/10] Various cross-platform fixes.

[Stefan Tauner]

On Sat, 13 Jul 2013 18:02:28 +0200
Carl-Daniel Hailfinger  wrote:
> With the library order fixed, this is
> Acked-by: Carl-Daniel Hailfinger

Thanks, r1697.