All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
@ 2020-08-11  8:01 Chee Hong Ang
  2020-08-11 11:02 ` Marek Vasut
  2020-08-11 13:06 ` Tom Rini
  0 siblings, 2 replies; 9+ messages in thread
From: Chee Hong Ang @ 2020-08-11  8:01 UTC (permalink / raw)
  To: u-boot

Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
for booting up Cyclone5/Arria10.

For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
which contains four 128KB SPL images (each 64KB SPL is followed by
64KB of zero-padding).

Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
---
 Makefile | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 4483a9b..f4631f1 100644
--- a/Makefile
+++ b/Makefile
@@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
 ifneq ($(CONFIG_ARCH_SOCFPGA),)
 quiet_cmd_socboot = SOCBOOT $@
 cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
-			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
-			u-boot.img > $@ || rm -f $@
+			spl/u-boot-spl.sfp \
+			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
+	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
 u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
 	$(call if_changed,socboot)
 
@@ -1592,8 +1593,10 @@ cmd_socnandboot =  dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \
 		   cat	spl/u-boot-spl.sfp spl/u-boot-spl.pad \
 			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
 			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
-			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
-			u-boot.img > $@ || rm -f $@ spl/u-boot-spl.pad
+			spl/u-boot-spl.sfp \
+			spl/u-boot-spl.pad > spl/u-boot-nand-splx4.sfp ; \
+		   cat	spl/u-boot-nand-splx4.sfp u-boot.img > $@ || \
+		   rm	-f $@ spl/u-boot-spl.pad
 u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
 	$(call if_changed,socnandboot)
 
-- 
2.2.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11  8:01 [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images Chee Hong Ang
@ 2020-08-11 11:02 ` Marek Vasut
  2020-08-11 11:34   ` Ang, Chee Hong
  2020-08-11 13:06 ` Tom Rini
  1 sibling, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2020-08-11 11:02 UTC (permalink / raw)
  To: u-boot

On 8/11/20 10:01 AM, Chee Hong Ang wrote:
> Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
> for booting up Cyclone5/Arria10.
> 
> For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
> 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
> which contains four 128KB SPL images (each 64KB SPL is followed by
> 64KB of zero-padding).
> 
> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> ---

What changed between V1 and V2 ? Changelog is missing.

>  Makefile | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 4483a9b..f4631f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
>  ifneq ($(CONFIG_ARCH_SOCFPGA),)
>  quiet_cmd_socboot = SOCBOOT $@
>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> -			u-boot.img > $@ || rm -f $@
> +			spl/u-boot-spl.sfp \
> +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
> +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>  	$(call if_changed,socboot)

Also, now that I look at it, if you want to generate some new target, it
should be a Makefile target, just like u-boot-with-spl.sfp is a Makefile
target. So then you can do make <target>.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11 11:02 ` Marek Vasut
@ 2020-08-11 11:34   ` Ang, Chee Hong
  2020-08-11 11:43     ` Marek Vasut
  0 siblings, 1 reply; 9+ messages in thread
From: Ang, Chee Hong @ 2020-08-11 11:34 UTC (permalink / raw)
  To: u-boot

> On 8/11/20 10:01 AM, Chee Hong Ang wrote:
> > Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
> > for booting up Cyclone5/Arria10.
> >
> > For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
> > 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
> > which contains four 128KB SPL images (each 64KB SPL is followed by
> > 64KB of zero-padding).
> >
> > Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> > ---
> 
> What changed between V1 and V2 ? Changelog is missing.
In V2, 'make u-boot-with-nand-spl.sfp' will generate spl/u-boot-nand-splx4.sfp which contains 4 x (SPL + 64KB padding).
Commit message already mentioned how to generate this SFP file with 64Kb padding for each SPL in SFP.
> 
> >  Makefile | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 4483a9b..f4631f1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
> > ifneq ($(CONFIG_ARCH_SOCFPGA),)  quiet_cmd_socboot = SOCBOOT $@
> >  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> > -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> > -			u-boot.img > $@ || rm -f $@
> > +			spl/u-boot-spl.sfp \
> > +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
> > +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
> >  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
> >  	$(call if_changed,socboot)
> 
> Also, now that I look at it, if you want to generate some new target, it
> should be a Makefile target, just like u-boot-with-spl.sfp is a Makefile target.
> So then you can do make <target>.

There is already a target 'u-boot-with-nand-spl.sfp' in Makefile:
u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
 	$(call if_changed,socnandboot)

V2 patch contains the following changes as well which are missing in this email thread:
@@ -1592,8 +1593,10 @@ cmd_socnandboot =  dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \
 		   cat	spl/u-boot-spl.sfp spl/u-boot-spl.pad \
 			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
 			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
-			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
-			u-boot.img > $@ || rm -f $@ spl/u-boot-spl.pad
+			spl/u-boot-spl.sfp \
+			spl/u-boot-spl.pad > spl/u-boot-nand-splx4.sfp ; \
+		   cat	spl/u-boot-nand-splx4.sfp u-boot.img > $@ || \
+		   rm	-f $@ spl/u-boot-spl.pad
 u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
 	$(call if_changed,socnandboot)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11 11:34   ` Ang, Chee Hong
@ 2020-08-11 11:43     ` Marek Vasut
  0 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2020-08-11 11:43 UTC (permalink / raw)
  To: u-boot

On 8/11/20 1:34 PM, Ang, Chee Hong wrote:
>> On 8/11/20 10:01 AM, Chee Hong Ang wrote:
>>> Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
>>> for booting up Cyclone5/Arria10.
>>>
>>> For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
>>> 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
>>> which contains four 128KB SPL images (each 64KB SPL is followed by
>>> 64KB of zero-padding).
>>>
>>> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
>>> ---
>>
>> What changed between V1 and V2 ? Changelog is missing.
> In V2, 'make u-boot-with-nand-spl.sfp' will generate spl/u-boot-nand-splx4.sfp which contains 4 x (SPL + 64KB padding).
> Commit message already mentioned how to generate this SFP file with 64Kb padding for each SPL in SFP.

The changelog in the patches is there so it's quickly obvious what
changed in the patch without searching for previous version and running
a diff.

>>>  Makefile | 11 +++++++----
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 4483a9b..f4631f1 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
>>> ifneq ($(CONFIG_ARCH_SOCFPGA),)  quiet_cmd_socboot = SOCBOOT $@
>>>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
>>> -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
>>> -			u-boot.img > $@ || rm -f $@
>>> +			spl/u-boot-spl.sfp \
>>> +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
>>> +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
>>>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>>>  	$(call if_changed,socboot)
>>
>> Also, now that I look at it, if you want to generate some new target, it
>> should be a Makefile target, just like u-boot-with-spl.sfp is a Makefile target.
>> So then you can do make <target>.
> 
> There is already a target 'u-boot-with-nand-spl.sfp' in Makefile:
> u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>  	$(call if_changed,socnandboot)

Right, so there should be a new one,
u-boot-with-spl-x4.sfp: spl/u-boot-spl.sfp FORCE
and then whatever target needs the -x4 variant should again depend on
it, e.g.

u-boot-with-nand-spl.sfp: spl/u-boot-spl-x4.sfp u-boot.img FORCE

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11  8:01 [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images Chee Hong Ang
  2020-08-11 11:02 ` Marek Vasut
@ 2020-08-11 13:06 ` Tom Rini
  2020-08-11 13:14   ` Marek Vasut
  1 sibling, 1 reply; 9+ messages in thread
From: Tom Rini @ 2020-08-11 13:06 UTC (permalink / raw)
  To: u-boot

On Tue, Aug 11, 2020 at 04:01:10PM +0800, Chee Hong Ang wrote:

> Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
> for booting up Cyclone5/Arria10.
> 
> For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
> 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
> which contains four 128KB SPL images (each 64KB SPL is followed by
> 64KB of zero-padding).
> 
> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> ---
>  Makefile | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 4483a9b..f4631f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
>  ifneq ($(CONFIG_ARCH_SOCFPGA),)
>  quiet_cmd_socboot = SOCBOOT $@
>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> -			u-boot.img > $@ || rm -f $@
> +			spl/u-boot-spl.sfp \
> +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
> +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>  	$(call if_changed,socboot)
>  
> @@ -1592,8 +1593,10 @@ cmd_socnandboot =  dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \
>  		   cat	spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> -			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> -			u-boot.img > $@ || rm -f $@ spl/u-boot-spl.pad
> +			spl/u-boot-spl.sfp \
> +			spl/u-boot-spl.pad > spl/u-boot-nand-splx4.sfp ; \
> +		   cat	spl/u-boot-nand-splx4.sfp u-boot.img > $@ || \
> +		   rm	-f $@ spl/u-boot-spl.pad
>  u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>  	$(call if_changed,socnandboot)

It's not immediately clear to me why we're doing this here, rather than
instructing the user to write the file 4 times when programming.  On TI
platforms, even on NAND, for forever there's been multiple locations the
ROM will check for the loader.  Is there a reason to not handle this at
that level?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200811/48b87df8/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11 13:06 ` Tom Rini
@ 2020-08-11 13:14   ` Marek Vasut
  2020-08-11 14:21     ` Tom Rini
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2020-08-11 13:14 UTC (permalink / raw)
  To: u-boot

On 8/11/20 3:06 PM, Tom Rini wrote:
> On Tue, Aug 11, 2020 at 04:01:10PM +0800, Chee Hong Ang wrote:
> 
>> Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
>> for booting up Cyclone5/Arria10.
>>
>> For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
>> 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
>> which contains four 128KB SPL images (each 64KB SPL is followed by
>> 64KB of zero-padding).
>>
>> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
>> ---
>>  Makefile | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 4483a9b..f4631f1 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
>>  ifneq ($(CONFIG_ARCH_SOCFPGA),)
>>  quiet_cmd_socboot = SOCBOOT $@
>>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
>> -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
>> -			u-boot.img > $@ || rm -f $@
>> +			spl/u-boot-spl.sfp \
>> +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
>> +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
>>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>>  	$(call if_changed,socboot)
>>  
>> @@ -1592,8 +1593,10 @@ cmd_socnandboot =  dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \
>>  		   cat	spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>> -			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>> -			u-boot.img > $@ || rm -f $@ spl/u-boot-spl.pad
>> +			spl/u-boot-spl.sfp \
>> +			spl/u-boot-spl.pad > spl/u-boot-nand-splx4.sfp ; \
>> +		   cat	spl/u-boot-nand-splx4.sfp u-boot.img > $@ || \
>> +		   rm	-f $@ spl/u-boot-spl.pad
>>  u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>>  	$(call if_changed,socnandboot)
> 
> It's not immediately clear to me why we're doing this here, rather than
> instructing the user to write the file 4 times when programming.  On TI
> platforms, even on NAND, for forever there's been multiple locations the
> ROM will check for the loader.  Is there a reason to not handle this at
> that level?

The u-boot-with-spl.sfp was there to have one U-Boot image including
SPL, other platforms do that as well. Except for the NAND case, where
the padding is different.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11 13:14   ` Marek Vasut
@ 2020-08-11 14:21     ` Tom Rini
  2020-08-11 14:29       ` Simon Goldschmidt
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2020-08-11 14:21 UTC (permalink / raw)
  To: u-boot

On Tue, Aug 11, 2020 at 03:14:39PM +0200, Marek Vasut wrote:
> On 8/11/20 3:06 PM, Tom Rini wrote:
> > On Tue, Aug 11, 2020 at 04:01:10PM +0800, Chee Hong Ang wrote:
> > 
> >> Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
> >> for booting up Cyclone5/Arria10.
> >>
> >> For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
> >> 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
> >> which contains four 128KB SPL images (each 64KB SPL is followed by
> >> 64KB of zero-padding).
> >>
> >> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> >> ---
> >>  Makefile | 11 +++++++----
> >>  1 file changed, 7 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 4483a9b..f4631f1 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
> >>  ifneq ($(CONFIG_ARCH_SOCFPGA),)
> >>  quiet_cmd_socboot = SOCBOOT $@
> >>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> >> -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> >> -			u-boot.img > $@ || rm -f $@
> >> +			spl/u-boot-spl.sfp \
> >> +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
> >> +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
> >>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
> >>  	$(call if_changed,socboot)
> >>  
> >> @@ -1592,8 +1593,10 @@ cmd_socnandboot =  dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \
> >>  		   cat	spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >> -			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >> -			u-boot.img > $@ || rm -f $@ spl/u-boot-spl.pad
> >> +			spl/u-boot-spl.sfp \
> >> +			spl/u-boot-spl.pad > spl/u-boot-nand-splx4.sfp ; \
> >> +		   cat	spl/u-boot-nand-splx4.sfp u-boot.img > $@ || \
> >> +		   rm	-f $@ spl/u-boot-spl.pad
> >>  u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
> >>  	$(call if_changed,socnandboot)
> > 
> > It's not immediately clear to me why we're doing this here, rather than
> > instructing the user to write the file 4 times when programming.  On TI
> > platforms, even on NAND, for forever there's been multiple locations the
> > ROM will check for the loader.  Is there a reason to not handle this at
> > that level?
> 
> The u-boot-with-spl.sfp was there to have one U-Boot image including
> SPL, other platforms do that as well. Except for the NAND case, where
> the padding is different.

Right, it's a convenience file and handy in some cases.  But what's the
reason for doing the 4 copy case?  It makes sense to create
"u-boot-with-stuff-plus-header" when there's a header that has to cover
the whole load, or you're going to write it to an SD card or there's
some non-trivial mangling involved.  Is the problem that
"u-boot-spl.sfp" isn't ever expected to be seen by the user and so it
would be odd to tell them to write that in the 4 places and then write
u-boot.img in the correct place, so instead we have to pad everything
out and make a large file?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200811/871d4d28/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11 14:21     ` Tom Rini
@ 2020-08-11 14:29       ` Simon Goldschmidt
  2020-08-11 14:35         ` Tom Rini
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Goldschmidt @ 2020-08-11 14:29 UTC (permalink / raw)
  To: u-boot

Am 11.08.2020 um 16:21 schrieb Tom Rini:
> On Tue, Aug 11, 2020 at 03:14:39PM +0200, Marek Vasut wrote:
>> On 8/11/20 3:06 PM, Tom Rini wrote:
>>> On Tue, Aug 11, 2020 at 04:01:10PM +0800, Chee Hong Ang wrote:
>>>
>>>> Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
>>>> for booting up Cyclone5/Arria10.
>>>>
>>>> For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
>>>> 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
>>>> which contains four 128KB SPL images (each 64KB SPL is followed by
>>>> 64KB of zero-padding).
>>>>
>>>> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
>>>> ---
>>>>  Makefile | 11 +++++++----
>>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 4483a9b..f4631f1 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
>>>>  ifneq ($(CONFIG_ARCH_SOCFPGA),)
>>>>  quiet_cmd_socboot = SOCBOOT $@
>>>>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
>>>> -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
>>>> -			u-boot.img > $@ || rm -f $@
>>>> +			spl/u-boot-spl.sfp \
>>>> +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
>>>> +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
>>>>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>>>>  	$(call if_changed,socboot)
>>>>  
>>>> @@ -1592,8 +1593,10 @@ cmd_socnandboot =  dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \
>>>>  		   cat	spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>>>>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>>>>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>>>> -			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
>>>> -			u-boot.img > $@ || rm -f $@ spl/u-boot-spl.pad
>>>> +			spl/u-boot-spl.sfp \
>>>> +			spl/u-boot-spl.pad > spl/u-boot-nand-splx4.sfp ; \
>>>> +		   cat	spl/u-boot-nand-splx4.sfp u-boot.img > $@ || \
>>>> +		   rm	-f $@ spl/u-boot-spl.pad
>>>>  u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
>>>>  	$(call if_changed,socnandboot)
>>>
>>> It's not immediately clear to me why we're doing this here, rather than
>>> instructing the user to write the file 4 times when programming.  On TI
>>> platforms, even on NAND, for forever there's been multiple locations the
>>> ROM will check for the loader.  Is there a reason to not handle this at
>>> that level?
>>
>> The u-boot-with-spl.sfp was there to have one U-Boot image including
>> SPL, other platforms do that as well. Except for the NAND case, where
>> the padding is different.
> 
> Right, it's a convenience file and handy in some cases.  But what's the
> reason for doing the 4 copy case?  It makes sense to create
> "u-boot-with-stuff-plus-header" when there's a header that has to cover
> the whole load, or you're going to write it to an SD card or there's
> some non-trivial mangling involved.  Is the problem that
> "u-boot-spl.sfp" isn't ever expected to be seen by the user and so it
> would be odd to tell them to write that in the 4 places and then write
> u-boot.img in the correct place, so instead we have to pad everything
> out and make a large file?

As U-Boot and SPL have to fit together, I think it's better to create
just one file that has to be written to SD card or flash. And writing to
NOR flash is, depending on how you do it, not too quick, so it's *very*
convenient to have this "4x SPL plus U-Boot" single file (start the
command, get a coffee and wait).

And yes, it *is* just a convenience thing. But I think it's a good thing
to have as result from the build. Even if it's only for the "dumb
standard user" not having to know the offset where U-Boot has to be put...

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images
  2020-08-11 14:29       ` Simon Goldschmidt
@ 2020-08-11 14:35         ` Tom Rini
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2020-08-11 14:35 UTC (permalink / raw)
  To: u-boot

On Tue, Aug 11, 2020 at 04:29:46PM +0200, Simon Goldschmidt wrote:
> Am 11.08.2020 um 16:21 schrieb Tom Rini:
> > On Tue, Aug 11, 2020 at 03:14:39PM +0200, Marek Vasut wrote:
> >> On 8/11/20 3:06 PM, Tom Rini wrote:
> >>> On Tue, Aug 11, 2020 at 04:01:10PM +0800, Chee Hong Ang wrote:
> >>>
> >>>> Generate spl/u-boot-splx4.sfp which consist of 4 SPL images required
> >>>> for booting up Cyclone5/Arria10.
> >>>>
> >>>> For Cyclone5 using NAND flash image layout for 128 KB memory blocks,
> >>>> 'make u-boot-with-nand-spl.sfp' to generate spl/u-boot-nand-splx4.sfp
> >>>> which contains four 128KB SPL images (each 64KB SPL is followed by
> >>>> 64KB of zero-padding).
> >>>>
> >>>> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com>
> >>>> ---
> >>>>  Makefile | 11 +++++++----
> >>>>  1 file changed, 7 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/Makefile b/Makefile
> >>>> index 4483a9b..f4631f1 100644
> >>>> --- a/Makefile
> >>>> +++ b/Makefile
> >>>> @@ -1582,8 +1582,9 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE
> >>>>  ifneq ($(CONFIG_ARCH_SOCFPGA),)
> >>>>  quiet_cmd_socboot = SOCBOOT $@
> >>>>  cmd_socboot = cat	spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> >>>> -			spl/u-boot-spl.sfp spl/u-boot-spl.sfp	\
> >>>> -			u-boot.img > $@ || rm -f $@
> >>>> +			spl/u-boot-spl.sfp \
> >>>> +			spl/u-boot-spl.sfp > spl/u-boot-splx4.sfp ; \
> >>>> +	      cat	spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@
> >>>>  u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
> >>>>  	$(call if_changed,socboot)
> >>>>  
> >>>> @@ -1592,8 +1593,10 @@ cmd_socnandboot =  dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \
> >>>>  		   cat	spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >>>>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >>>>  			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >>>> -			spl/u-boot-spl.sfp spl/u-boot-spl.pad \
> >>>> -			u-boot.img > $@ || rm -f $@ spl/u-boot-spl.pad
> >>>> +			spl/u-boot-spl.sfp \
> >>>> +			spl/u-boot-spl.pad > spl/u-boot-nand-splx4.sfp ; \
> >>>> +		   cat	spl/u-boot-nand-splx4.sfp u-boot.img > $@ || \
> >>>> +		   rm	-f $@ spl/u-boot-spl.pad
> >>>>  u-boot-with-nand-spl.sfp: spl/u-boot-spl.sfp u-boot.img FORCE
> >>>>  	$(call if_changed,socnandboot)
> >>>
> >>> It's not immediately clear to me why we're doing this here, rather than
> >>> instructing the user to write the file 4 times when programming.  On TI
> >>> platforms, even on NAND, for forever there's been multiple locations the
> >>> ROM will check for the loader.  Is there a reason to not handle this at
> >>> that level?
> >>
> >> The u-boot-with-spl.sfp was there to have one U-Boot image including
> >> SPL, other platforms do that as well. Except for the NAND case, where
> >> the padding is different.
> > 
> > Right, it's a convenience file and handy in some cases.  But what's the
> > reason for doing the 4 copy case?  It makes sense to create
> > "u-boot-with-stuff-plus-header" when there's a header that has to cover
> > the whole load, or you're going to write it to an SD card or there's
> > some non-trivial mangling involved.  Is the problem that
> > "u-boot-spl.sfp" isn't ever expected to be seen by the user and so it
> > would be odd to tell them to write that in the 4 places and then write
> > u-boot.img in the correct place, so instead we have to pad everything
> > out and make a large file?
> 
> As U-Boot and SPL have to fit together, I think it's better to create
> just one file that has to be written to SD card or flash. And writing to
> NOR flash is, depending on how you do it, not too quick, so it's *very*
> convenient to have this "4x SPL plus U-Boot" single file (start the
> command, get a coffee and wait).
> 
> And yes, it *is* just a convenience thing. But I think it's a good thing
> to have as result from the build. Even if it's only for the "dumb
> standard user" not having to know the offset where U-Boot has to be put...

Alright, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200811/f0d75422/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-08-11 14:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-11  8:01 [PATCH v2] Makefile: socfpga: Generate spl/u-boot-splx4.sfp with 4 SPL images Chee Hong Ang
2020-08-11 11:02 ` Marek Vasut
2020-08-11 11:34   ` Ang, Chee Hong
2020-08-11 11:43     ` Marek Vasut
2020-08-11 13:06 ` Tom Rini
2020-08-11 13:14   ` Marek Vasut
2020-08-11 14:21     ` Tom Rini
2020-08-11 14:29       ` Simon Goldschmidt
2020-08-11 14:35         ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.