Re: [flashrom] Fix feature detection when fdti requires libusb for linking

[Carl-Daniel Hailfinger]

Hi Jörg,

thanks for your patch.
Review follows.

On 23.08.2009 07:52, Joerg Mayer wrote:
> Index: Makefile
>
> --- Makefile	(revision 696)
> +++ Makefile	(working copy)
> @@ -75,12 +75,13 @@
>  endif
>  endif
>
> +FTDILIBS = $(shell pkg-config --libs libftdi)

AFAICS this breaks if pkg-config is not installed. Maybe use -lftdi as
fallback? Besides that, on my machine it looks like this:
# pkg-config --libs libftdi
-L/usr/local/lib -lftdi -lusb

However, compiling and linking with -lftdi (and no other linker
parameter) works fine here. I'm not familiar enough with pkg-config to
know why this is the case.

Wouldn't we need
# pkg-config --cflags libftdi
for FEATURE_CFLAGS as well?
> FEATURE_CFLAGS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-D'FT2232_SPI_SUPPORT=1'")
>
> -FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-lftdi")
> +FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && echo $(FTDILIBS))
>
> $(PROGRAM): $(OBJS)
> $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(LIBS) $(FEATURE_LIBS)
> $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(FEATURE_LIBS) $(LIBS)

Is this one just cosmetic or am I missing some deep meaning?
> # TAROPTIONS reduces information leakage from the packager's system.
>  # If other tar programs support command line arguments for setting uid/gid of
> @@ -133,7 +134,7 @@
>    	   echo "struct ftdi_context *ftdic = NULL;";	   \
>    	   echo "int main(int argc, char **argv)"; \
>    	   echo "{ return ftdi_init(ftdic); }"; ) > .featuretest.c )
> @$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(LIBS) -lftdi >/dev/null 2>&1 &&	\
> @$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(FTDILIBS) $(LIBS)>/dev/null 2>&1 &&	\

OK.
> ( echo "found."; echo "FTDISUPPORT := yes" >> .features.tmp ) ||	\
> ( echo "not found."; echo "FTDISUPPORT := no" >> .features.tmp )
>
> @$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features

When you resubmit (or simply answer the questions above), please add a
Signed-off-by statement as well. Details of this procedure are explained at
http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure

Regards,
Carl-Daniel