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

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

[Tom Rini]

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@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reported-by: Nishanth Menon <nm@ti.com>
Tested-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Tom Rini <trini@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
-
-	. = .;
 
 	_image_binary_end = .;
 
-- 
1.7.9.5

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

[Michal Simek]

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@xilinx.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reported-by: Nishanth Menon <nm@ti.com>
> Tested-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Tom Rini <trini@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?

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.

Thanks,
Michal

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

[Tom Rini]

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@xilinx.com>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Reported-by: Nishanth Menon <nm@ti.com>
> > Tested-by: Nishanth Menon <nm@ti.com>
> > Signed-off-by: Tom Rini <trini@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.

> 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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160315/725c9648/attachment.sig>

[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@xilinx.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> Reported-by: Nishanth Menon <nm@ti.com>
>>> Tested-by: Nishanth Menon <nm@ti.com>
>>> Signed-off-by: Tom Rini <trini@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