[U-Boot-Users] RFC: Xilinx Spartan3 relocation code

[Bruce_Leonard at selinc.com]

Hi,

Our custom board has a Spartan3 FPGA on it and we're using u-boot to 
program it.  I've come across an item (I won't call it an issue since 
there's a fairly simple work around) in the function Spartan3_sp_reloc in 
.../common/spartan3.c.  (Looks like the Spartan3_ss_reloc function has the 
same issue.)

Some of the FPGA functions are optional.  That is to say the higher level 
code checks to see if the custom function pointer is non-NULL before it 
gets used.  The 'pre' function is a good example.  I don't have any need 
for a 'pre' function in my particular setup, so in my 
Xilinx_Spartan3_Slave_Parallel_fns structure I set that pointer to NULL. 
The item comes up in the relocate functions mentioned above.  They blindly 
add gd->reloc_offset to the function pointer values in the list, making 
the relocated 'pre' function pointer non-NULL.  The 'fpga load' command 
then tries to execute the 'pre' function and causes the system to crash 
because it's trying to execute bogus code.

The simple work around is to declare a function for 'pre' that does 
nothing.  To me this defeats the purpose of the non-NULL checking that 
other code does and forces the user to add code that does nothing for 
them.  I like the non-NULL checking because it's robust and allows you to 
only include the code you actually need.

So, to the RFC: I think the code that does the relocation of the FPGA 
description structures should check for NULL before adding 
gd->reloc_offset to the function pointers.  I think it keeps things in 
line with the original intent of the authors of the FPGA and makes the 
whole thing more robust.  Comments?

I'm happy to make the changes and submit a patch, but I can only test for 
one case: the Spartan 3 parallel load method.

Bruce