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

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