[U-Boot] [PATCH v2] spl: arm: Make sure to include u_boot_list_*_part_disk_*

[Michal Simek]

On 16.3.2016 01:50, Tom Rini wrote:
> On Wed, Mar 16, 2016 at 01:25:14AM +0100, Michal Simek wrote:
>> On 15.3.2016 23:29, Tom Rini wrote:
>>> Starting with 96e5b03 we use a linker list for partition table
>>> information.  However since we use this in SPL we need to make sure that
>>> the SPL linker scripts include these as well.
>>>
>>> Cc: Michal Simek <michal.simek at xilinx.com>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>> Reported-by: Nishanth Menon <nm at ti.com>
>>> Tested-by: Nishanth Menon <nm at ti.com>
>>> Signed-off-by: Tom Rini <trini at konsulko.com>
>>> ---
>>> Changes in v2:
>>> - Drop the CONFIG_SPL_DM tests for zynq as DM is the only case for zynq
>>>   and while in here drop a now useless line.
>>> ---
>>>  arch/arm/cpu/armv7/omap-common/u-boot-spl.lds |    1 +
>>>  arch/arm/cpu/u-boot-spl.lds                   |    1 +
>>>  arch/arm/mach-zynq/u-boot-spl.lds             |    4 ----
>>>  3 files changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds b/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds
>>> index ccd0c83..9dccdc0 100644
>>> --- a/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds
>>> +++ b/arch/arm/cpu/armv7/omap-common/u-boot-spl.lds
>>> @@ -35,6 +35,7 @@ SECTIONS
>>>  
>>>  	. = ALIGN(4);
>>>  	.u_boot_list : {
>>> +		KEEP(*(SORT(.u_boot_list_*_part_driver_*)));
>>>  		KEEP(*(SORT(.u_boot_list*_i2c_*)));
>>>  	} >.sram
>>>  
>>> diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
>>> index c5b4f7c..1805043 100644
>>> --- a/arch/arm/cpu/u-boot-spl.lds
>>> +++ b/arch/arm/cpu/u-boot-spl.lds
>>> @@ -40,6 +40,7 @@ SECTIONS
>>>  #endif
>>>  	. = .;
>>>  	.u_boot_list : {
>>> +		KEEP(*(SORT(.u_boot_list_*_part_driver_*)));
>>>  		KEEP(*(SORT(.u_boot_list*_i2c_*)));
>>>  	}
>>>  
>>> diff --git a/arch/arm/mach-zynq/u-boot-spl.lds b/arch/arm/mach-zynq/u-boot-spl.lds
>>> index ecdf6a0..a5c76a6 100644
>>> --- a/arch/arm/mach-zynq/u-boot-spl.lds
>>> +++ b/arch/arm/mach-zynq/u-boot-spl.lds
>>> @@ -38,16 +38,12 @@ SECTIONS
>>>  	} > .sram
>>>  
>>>  	. = ALIGN(4);
>>> -#ifdef CONFIG_SPL_DM
>>>  	.u_boot_list : {
>>>  		KEEP(*(SORT(.u_boot_list_*_driver_*)));
>>>  		KEEP(*(SORT(.u_boot_list_*_uclass_*)));
>>>  	} > .sram
>>>  
>>>  	. = ALIGN(4);
>>> -#endif
>>> -
>>> -	. = .;
>>
>> This is getting more interested than I thought.
>> We have for zynq added two lists for driver and uclass added by Simon
>> but this is missing in linker script for others SoC. Does that mean that
>> none else is using SPL with DM? Or that we have in linker script
>> something what doesn't need to be there?
> 
> Zynq is ahead of the curve here, many SoCs are not doing DM in SPL yet.
> With my TI hat on, the series that lets us do FIT in SPL (and thus pick
> one of N DTBs for u-boot.img) is what unblocks moving over because we
> can retain having 'am335x_evm' support 4+ distinct HW platforms in a
> single binary.

ok.


>> Regarding this patch. Zynq is not enabling partition support but I still
>> think that we should add also this list to zynq spl.
>> Maybe we didn't understand each other. I wanted to say remove that
>> SPL_DM and keep that part_driver lists even we are not enabling them by
>> default.
> 
> Right, but keep in mind that u_boot_list_*_driver_* will match
> u_boot_list_2_part_driver_2_dos already so we don't need to glob it in.
> 
> All of that said, I'm going to do a v3 now.  I played with the linker
> list stuff like I was thinking I should anyhow and I see things like:
>  *(SORT(.u_boot_list_*))
>  .u_boot_list_2_cmd_1
>                 0x0000000040310d50        0x0 common/built-in.o
>  .u_boot_list_2_cmd_3
>                 0x0000000040310d50        0x0 common/built-in.o
>  .u_boot_list_2_i2c_1
>                 0x0000000040310d50        0x0 drivers/built-in.o
>  .u_boot_list_2_i2c_2_omap24_0
>                 0x0000000040310d50       0x2c drivers/built-in.o
> 
> So when we include empty stuff into the file the linker still does the
> right thing.

You are right I missed that. But on the other hand maybe then make sense
for others to just add this
KEEP(*(SORT(.u_boot_list_*_driver_*)));
instead of
KEEP(*(SORT(.u_boot_list_*_part_driver_*)));

Thanks,
Michal