From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthias Fuchs Date: Wed, 15 Aug 2007 16:24:59 +0200 Subject: [U-Boot-Users] RFC: Xilinx Spartan3 relocation code In-Reply-To: References: Message-ID: <200708151624.59770.matthias.fuchs@esd-electronics.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 -----------------------------------------------------------------------