[U-Boot] [PATCH 0/9] arm: zynq: ps7* consolidation
Michal Simek
michal.simek at xilinx.com
Tue Nov 14 07:11:07 UTC 2017
Hi,
On 13.11.2017 16:27, Gerald Van Baren wrote:
> Hi Michal, Mike,
>
> On Mon, Nov 13, 2017 at 9:48 AM, Michal Simek <michal.simek at xilinx.com>
> wrote:
>> On 13.11.2017 15:35, Mike Looijmans wrote:
>>> On 10-11-17 11:58, Michal Simek wrote:
>>>> Hi,
>>>>
>>>> this series is trying to cleanup ps7_init* file that we don't need to
>>>> have the same copy of the same functions in different locations.
>>>> This work is done based on solution from Topic.nl for miami boards
>>>> where format was changed a little bit to save one word in config data
>>>> segment.
>
> [snip]
>
>>> This "constant table" approach could also be implemented for the zynqmp
>>> platforms, cutting down the size of the SPL considerably.
>>
>> Probably.
>
> Adding onto the Vivado generation wish list, it would be nice to eliminate
> the
> FPGA versions you know you don't have. We have a v3 FPGA so the v1
> and v2 tables are wasted space.
As you see we have done this for topic and syzygy board that code in
u-boot is just removed. For zc702/zc706 and similar which are xilinx
boards you can find different silicons on this board. I think I have v1
and v2 here.
For zynqmp case zcu102 can contain all silicon versions too.
>
> We've created a patch that we apply to the ps7_init_gpl.c file that #ifdefs
> the unused sections out.
>
> (sorry for the crappy paste, I'm using gmail for this post):
no. Does that mean that you are also using SPL?
>
> Strip out the tables based on the version:
> -----------------------------------------------------
> --- ps7_init_gpl.c 2017-10-27 11:30:48.000000000 -0400
> +++ ps7_init_gpl.c 2017-10-27 14:42:48.469914486 -0400
> @@ -25,8 +25,11 @@
> *
> *****************************************************************************/
>
> +#define VERSION 3 /* Target a specific version */
> +
> #include "ps7_init_gpl.h"
>
> +#if !defined(VERSION) || (VERSION == 3)
> unsigned long ps7_pll_init_data_3_0[] = {
> // START: top
> // .. START: SLCR SETTINGS
> @@ -4242,7 +4245,9 @@
>
> //
> };
> +#endif
>
> +#if !defined(VERSION) || (VERSION == 2)
> unsigned long ps7_pll_init_data_2_0[] = {
> // START: top
> // .. START: SLCR SETTINGS
> @@ -8617,7 +8622,9 @@
>
> //
> };
> +#endif
>
> +#if !defined(VERSION) || (VERSION == 1)
> unsigned long ps7_pll_init_data_1_0[] = {
> // START: top
> // .. START: SLCR SETTINGS
> @@ -12925,6 +12932,7 @@
>
> //
> };
> +#endif
>
>
> #include "xil_io.h"
> -----------------------------------------------------
> Now we can remove the code that uses the version information:
> -----------------------------------------------------
>
> --- ps7_init_gpl.c 2017-10-27 11:30:48.000000000 -0400
> +++ ps7_init_gpl.c 2017-10-27 14:44:55.142353664 -0400
> @@ -12961,6 +12961,7 @@
> return err_msg;
> }
>
> +#ifndef VERSION
> unsigned long
> ps7GetSiliconVersion () {
> // Read PS version from MCTRL register [31:28]
> @@ -12969,6 +12970,7 @@
> unsigned long ps_version = (*addr & mask) >> 28;
> return ps_version;
> }
> +#endif
>
> void mask_write (unsigned long add , unsigned long mask, unsigned long
> val ) {
> volatile unsigned long *addr = (volatile unsigned long*) add;
> @@ -13069,8 +13069,9 @@
> unsigned long *ps7_peripherals_init_data = ps7_peripherals_init_data_3_0;
>
> int
> -ps7_post_config()
> +ps7_post_config()
> {
> +#ifndef VERSION
> // Get the PS_VERSION on run time
> unsigned long si_ver = ps7GetSiliconVersion ();
> int ret = -1;
> @@ -13085,11 +13086,19 @@
> if (ret != PS7_INIT_SUCCESS) return ret;
> }
> return PS7_INIT_SUCCESS;
> +#elif(VERSION == 1)
> + return ps7_config (ps7_post_config_1_0);
> +#elif(VERSION == 2)
> + return ps7_config (ps7_post_config_2_0);
> +#elif(VERSION == 3)
> + return ps7_config (ps7_post_config_3_0);
> +#endif
> }
>
> int
> -ps7_debug()
> +ps7_debug()
> {
> +#ifndef VERSION
> // Get the PS_VERSION on run time
> unsigned long si_ver = ps7GetSiliconVersion ();
> int ret = -1;
> @@ -13104,17 +13113,27 @@
> if (ret != PS7_INIT_SUCCESS) return ret;
> }
> return PS7_INIT_SUCCESS;
> +#elif (VERSION == 1)
> + return ps7_config (ps7_debug_1_0);
> +#elif (VERSION == 2)
> + return ps7_config (ps7_debug_2_0);
> +#elif (VERSION == 3)
> + return ps7_config (ps7_debug_3_0);
> +#endif
> }
>
>
> int
> -ps7_init()
> +ps7_init()
> {
> +#if !defined(VERSION)
> // Get the PS_VERSION on run time
> unsigned long si_ver = ps7GetSiliconVersion ();
> +#endif
> int ret;
> //int pcw_ver = 0;
>
> +#if !defined(VERSION)
> if (si_ver == PCW_SILICON_VERSION_1) {
> ps7_mio_init_data = ps7_mio_init_data_1_0;
> ps7_pll_init_data = ps7_pll_init_data_1_0;
> @@ -13139,6 +13158,28 @@
> ps7_peripherals_init_data = ps7_peripherals_init_data_3_0;
> //pcw_ver = 3;
> }
> +#elif (VERSION == 1)
> + ps7_mio_init_data = ps7_mio_init_data_1_0;
> + ps7_pll_init_data = ps7_pll_init_data_1_0;
> + ps7_clock_init_data = ps7_clock_init_data_1_0;
> + ps7_ddr_init_data = ps7_ddr_init_data_1_0;
> + ps7_peripherals_init_data = ps7_peripherals_init_data_1_0;
> + //pcw_ver = 1;
> +#elif (VERSION == 2)
> + ps7_mio_init_data = ps7_mio_init_data_2_0;
> + ps7_pll_init_data = ps7_pll_init_data_2_0;
> + ps7_clock_init_data = ps7_clock_init_data_2_0;
> + ps7_ddr_init_data = ps7_ddr_init_data_2_0;
> + ps7_peripherals_init_data = ps7_peripherals_init_data_2_0;
> + //pcw_ver = 2;
> +#elif (VERSION == 3)
> + ps7_mio_init_data = ps7_mio_init_data_3_0;
> + ps7_pll_init_data = ps7_pll_init_data_3_0;
> + ps7_clock_init_data = ps7_clock_init_data_3_0;
> + ps7_ddr_init_data = ps7_ddr_init_data_3_0;
> + ps7_peripherals_init_data = ps7_peripherals_init_data_3_0;
> + //pcw_ver = 3;
> +#endif
>
> // MIO init
> ret = ps7_config (ps7_mio_init_data);
> @@ -13201,7 +13242,7 @@
> *(volatile unsigned int*)SCU_GLOBAL_TIMER_CONTROL = 0;
> }
>
> -void perf_reset_and_start_timer()
> +void perf_reset_and_start_timer()
> {
> perf_reset_clock();
> perf_start_clock();
>
> -----------------------------------------------------
> Oh, yeah, and add "void" to the functions that don't take arguments to keep
> lint from complaining,
> -----------------------------------------------------
> --- ps7_init_gpl.c 2017-10-27 16:00:09.963459068 -0400
> +++ ps7_init_gpl.c 2017-10-27 16:02:00.843517518 -0400
> @@ -12963,7 +12963,7 @@
>
> #ifndef VERSION
> unsigned long
> -ps7GetSiliconVersion () {
> +ps7GetSiliconVersion (void) {
> // Read PS version from MCTRL register [31:28]
> unsigned long mask = 0xF0000000;
> unsigned long *addr = (unsigned long*) 0XF8007080;
> @@ -13085,7 +13085,7 @@
> unsigned long *ps7_peripherals_init_data = ps7_peripherals_init_data_3_0;
>
> int
> -ps7_post_config()
> +ps7_post_config(void)
> {
> #ifndef VERSION
> // Get the PS_VERSION on run time
> @@ -13112,7 +13112,7 @@
> }
>
> int
> -ps7_debug()
> +ps7_debug(void)
> {
> #ifndef VERSION
> // Get the PS_VERSION on run time
> @@ -13140,7 +13140,7 @@
>
>
> int
> -ps7_init()
> +ps7_init(void)
> {
> #if !defined(VERSION)
> // Get the PS_VERSION on run time
> @@ -13258,7 +13258,7 @@
> *(volatile unsigned int*)SCU_GLOBAL_TIMER_CONTROL = 0;
> }
>
> -void perf_reset_and_start_timer()
> +void perf_reset_and_start_timer(void)
> {
> perf_reset_clock();
> perf_start_clock();
> --- ps7_init_gpl.h 2017-10-27 16:00:11.691459805 -0400
> +++ ps7_init_gpl.h 2017-10-27 16:01:22.547494873 -0400
> @@ -121,15 +121,15 @@
> #define SCU_GLOBAL_TIMER_AUTO_INC 0xF8F00218
>
> int ps7_config( unsigned long*);
> -int ps7_init();
> -int ps7_post_config();
> -int ps7_debug();
> +int ps7_init(void);
> +int ps7_post_config(void);
> +int ps7_debug(void);
> char* getPS7MessageInfo(unsigned key);
>
> void perf_start_clock(void);
> void perf_disable_clock(void);
> void perf_reset_clock(void);
> -void perf_reset_and_start_timer();
> +void perf_reset_and_start_timer(void);
> int get_number_of_cycles_for_delay(unsigned int delay);
> #ifdef __cplusplus
> }
This patchset should eliminate the whole ps7_init_gpl.h file.
And for .c file we should probably document what to do.
Remove all comments, remove all functions which are in common location,
remove silicon detection code if you have only one version, remove
ps7_debug function which is not called.
>
>
>>> (Only thing remaining is to have Vivado output these files properly...)
>>
>> Which is the biggest problem. I don't think this will happen.
>
> :-(
> Post-processing the output files with patch has been working for us. We
> are doing it as a manual step, but we don't have to do it often so it isn't
> too bad.
In general this processing is just reducing size of SPL which shouldn't
be a big problem on zynq. It means you will save some space but if you
don't need it then why to do it.
I have tested origin testcases that you simply copy ps7_init file to
u-boot and build it and there is not an issue and that steps above
should be really just for boards which you want to support upstream.
Thanks,
Michal
More information about the U-Boot
mailing list