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

[Matthias Fuchs <matthias.fuchs@esd-electronics.com>]

Hi Bruce,

I ran into the same issue a couple of weeks ago. Currently I implemented
dummy functions for things that I do not need. But I agree with you
that relocating NULL pointers makes no sense but fixing the FPGA code as
you suggested would be fine.

I can test your patch against the spartan2/3 serial slave code with my board.

Matthias

On Tuesday 14 August 2007 22:58, Bruce_Leonard at selinc.com wrote:
> 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
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >>  http://get.splunk.com/
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
> 
> 

-- 
-----------------------------------------------------------------------
Dipl.-Ing. Matthias Fuchs             esd electronic system design gmbh
http://www.esd-electronics.com                    Vahrenwalder Str. 207
phone: +49-511-37298-0, fax: -68                30165 Hannover, Germany
-----------------------------------------------------------------------