On 13 December 2014 at 21:23, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Dec 13, 2014 at 12:53 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon
On Sat, Dec 13, 2014 at 5:27 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 12 December 2014 at 06:05, Bin Meng bmeng.cn@gmail.com wrote:
This is the initial import from Intel FSP release for Queensbay platform (Tunnel Creek processor and Topcliff Platform Controller Hub), which can be downloaded from Intel website.
For more details, check http://www.intel.com/fsp.
Note: U-Boot coding convention was applied to these codes, so it looks completely different from the original Intel release. Also update FSP support codes license header to use SPDX ID.
I'm sorry to report that now that you have moved it to U-Boot coding conventions various other issues have been revealed. I would really like to get this patch cleaned up at some point. If you'd like to do it by respinning the next patch in the series, or by sending a new patch I don't mind. But at the moment, it's not very nice code - I wonder if it was originally an entry in an obfuscation competition :-)
I know you have already done a lot to improve it, hopefully what I am asking for will not take too long.
I only got part way down the below code review. Maybe we can tidy it up later. Let me know what you think.
I think I can fix those issues in a follow-on patch. Will you apply this series for now?
These issues are fixed in the follow-on patch @ http://patchwork.ozlabs.org/patch/420827/, except the following:
- find_fsp_header() is not rewritten to remove those casts, because
only register variable can be used in a stackless environment
- 'status' parameter cannot removed in the fsp_continue() as there is
a check on the status
- params_ptr cannot be dropped in fsp_init() due to different ABI call
into the FSP
- init(¶ms) cannot be removed due to compiler optimization will
break this function
I've added some comments in the codes to explain. Please have a look.
Regards, Bin
Applied to u-boot-x86, thanks!