[U-Boot-Users] Need the lists opinion on the "right" way to fix something (long)

[Bruce_Leonard at selinc.com]

Not long ago Grant put in a fix to .../common/cmd_fpga.c that was the 
right thing to do but introduced another problem.  What he did was 
essentially this:

-       #if CFG_FPGA_XILINX
+       #if (CONFIG_FPGA & CFG_FPGA_XILINX)

It's the right thing to do because the first always evaluated to true and 
(in Grant's opinion and mine) the real intent is to check CONFIG_FPGA and 
see if it's a Xilinx part.

However, it introduces the following problem.  CONFIG_FPGA is defined in 
the board specific config file and is usually going to be something like 
this:

#define CONFIG_FPGA             CFG_XILINX_SPARTAN3

Following CFG_XILINX_SPARTAN3 to .../include/xilinx.h you find:

#define CFG_XILINX_SPARTAN3     CFG_FPGA_XILINX | CFG_SPARTAN3

Now as to the problem:  .../common/cmd_fpga.c includes .../include/fpga.h 
but NOT .../include/xilinx.h.  This makes sense because 
.../common/cmd_fpga.c is a common file not a Xilinx specific file.  But, 
when the preprocessor hits Grant's fix it tries to substitute 
CFG_XILINX_SPARTAN3 for CONFIG_FPGA and can't because CFG_XILINX_SPARTAN3 
is defined in .../include/xilinx.h.  The simple solution is to include 
.../include/xilinx.h (which includes .../include/fpga.h) and life is 
happy.  The reason I don't like doing that is because 
.../common/cmd_fpga.c is a common file and I believe it would be a "bad" 
thing to put Xilinx specific stuff into a common file.

Of course the flip side of this is that the function fpga_loadbitsream() 
in .../common/cmd_fpga.c is already Xilinx specific.  IMHO, it shouldn't 
be there in the first place but I understand why it's there.  It's doing 
some pre-processing on the Xilinx BIT file prior to handing it to 
fpga_load(), which is a common high-level function that then starts 
drilling down to determine Altera vs. Xilinx, serial vs. parallel loading, 
etc.

Since I've only been working with u-boot/open source for about 9 months, 
I'm not really sure what the design philosophies are and what the 
acceptable and/or "righ" way of fixing this would be.  Any input or 
direction would be greatly appreciated.  Thanks.

Bruce