diff mbox series

[2/2] configs: socfpga: Add QSPI boot for Arria 10 SoCDK

Message ID 20200221012506.5042-2-ley.foon.tan@intel.com
State New
Headers show
Series [1/2] configs: socfpga: Add QSPI support for Cyclone 5 | expand

Commit Message

Tan, Ley Foon Feb. 21, 2020, 1:25 a.m. UTC
Add QSPI boot settings for Arria 10 SoCDK.

Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
---
 include/configs/socfpga_arria10_socdk.h | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Ley Foon Tan March 2, 2020, 9:33 a.m. UTC | #1
On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
>
> Add QSPI boot settings for Arria 10 SoCDK.
>
> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> ---
>  include/configs/socfpga_arria10_socdk.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/include/configs/socfpga_arria10_socdk.h b/include/configs/socfpga_arria10_socdk.h
> index 645e66e6b0..e1d01c095f 100644
> --- a/include/configs/socfpga_arria10_socdk.h
> +++ b/include/configs/socfpga_arria10_socdk.h
> @@ -39,6 +39,15 @@
>  /* SPL memory allocation configuration, this is for FAT implementation */
>  #define CONFIG_SYS_SPL_MALLOC_SIZE     0x00015000
>
> +#define KERNEL_FIT_ADDR                __stringify(0x1200000)
> +
> +#define SOCFPGA_BOOT_SETTINGS \
> +       "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
> +       "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
> +                       "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
> +                       "bootm ${scriptaddr}\0" \
> +       "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
> +
>  /* The rest of the configuration is shared */
>  #include <configs/socfpga_common.h>
>

Any comment on this patch?

Regards
Ley Foon
Marek Vasut March 2, 2020, 10:20 a.m. UTC | #2
On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
>>
>> Add QSPI boot settings for Arria 10 SoCDK.
>>
>> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
>> ---
>>  include/configs/socfpga_arria10_socdk.h | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/include/configs/socfpga_arria10_socdk.h b/include/configs/socfpga_arria10_socdk.h
>> index 645e66e6b0..e1d01c095f 100644
>> --- a/include/configs/socfpga_arria10_socdk.h
>> +++ b/include/configs/socfpga_arria10_socdk.h
>> @@ -39,6 +39,15 @@
>>  /* SPL memory allocation configuration, this is for FAT implementation */
>>  #define CONFIG_SYS_SPL_MALLOC_SIZE     0x00015000
>>
>> +#define KERNEL_FIT_ADDR                __stringify(0x1200000)
>> +
>> +#define SOCFPGA_BOOT_SETTINGS \
>> +       "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
>> +       "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
>> +                       "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
>> +                       "bootm ${scriptaddr}\0" \
>> +       "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
>> +
>>  /* The rest of the configuration is shared */
>>  #include <configs/socfpga_common.h>
>>
> 
> Any comment on this patch?

Can we get rid of rootfstype=jffs2 ? It's archaic and you can use UBI on
top of SPI NOR too. Also, isn't there already some kernel_addr_r for the
kernel address ?

Finally, can't we switch to distro boot command on socfpga, to handle
all the various devices ?
Ley Foon Tan March 3, 2020, 9:21 a.m. UTC | #3
On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote:
>
> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> > On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
> >>
> >> Add QSPI boot settings for Arria 10 SoCDK.
> >>
> >> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> >> ---
> >>  include/configs/socfpga_arria10_socdk.h | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/include/configs/socfpga_arria10_socdk.h b/include/configs/socfpga_arria10_socdk.h
> >> index 645e66e6b0..e1d01c095f 100644
> >> --- a/include/configs/socfpga_arria10_socdk.h
> >> +++ b/include/configs/socfpga_arria10_socdk.h
> >> @@ -39,6 +39,15 @@
> >>  /* SPL memory allocation configuration, this is for FAT implementation */
> >>  #define CONFIG_SYS_SPL_MALLOC_SIZE     0x00015000
> >>
> >> +#define KERNEL_FIT_ADDR                __stringify(0x1200000)
> >> +
> >> +#define SOCFPGA_BOOT_SETTINGS \
> >> +       "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
> >> +       "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
> >> +                       "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
> >> +                       "bootm ${scriptaddr}\0" \
> >> +       "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
> >> +
> >>  /* The rest of the configuration is shared */
> >>  #include <configs/socfpga_common.h>
> >>
> >
> > Any comment on this patch?
>
> Can we get rid of rootfstype=jffs2 ? It's archaic and you can use UBI on
> top of SPI NOR too. Also, isn't there already some kernel_addr_r for the
> kernel address ?
Yes, I know SPI flash can use UBI FS too, but we only enable jffs2 now.
kernelfit_addr is for kernel fit image offset in SPI flash, it is
different from kernel_addr_r.
Maybe change kernelfit_addr to qspi_kernelfit_addr to avoid confusion.
>
> Finally, can't we switch to distro boot command on socfpga, to handle
> all the various devices ?
socfpga_common.h already use distro boot command, right?

In patch "[PATCH 1/2] configs: socfpga: Add QSPI support for Cyclone
5", it added QSPI to the list:
+       BOOT_TARGET_DEVICES_QSPI(func) \
Marek Vasut March 3, 2020, 12:14 p.m. UTC | #4
On 3/3/20 10:21 AM, Ley Foon Tan wrote:
> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote:
>>
>> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
>>> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
>>>>
>>>> Add QSPI boot settings for Arria 10 SoCDK.
>>>>
>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
>>>> ---
>>>>  include/configs/socfpga_arria10_socdk.h | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/include/configs/socfpga_arria10_socdk.h b/include/configs/socfpga_arria10_socdk.h
>>>> index 645e66e6b0..e1d01c095f 100644
>>>> --- a/include/configs/socfpga_arria10_socdk.h
>>>> +++ b/include/configs/socfpga_arria10_socdk.h
>>>> @@ -39,6 +39,15 @@
>>>>  /* SPL memory allocation configuration, this is for FAT implementation */
>>>>  #define CONFIG_SYS_SPL_MALLOC_SIZE     0x00015000
>>>>
>>>> +#define KERNEL_FIT_ADDR                __stringify(0x1200000)
>>>> +
>>>> +#define SOCFPGA_BOOT_SETTINGS \
>>>> +       "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
>>>> +       "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
>>>> +                       "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
>>>> +                       "bootm ${scriptaddr}\0" \
>>>> +       "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
>>>> +
>>>>  /* The rest of the configuration is shared */
>>>>  #include <configs/socfpga_common.h>
>>>>
>>>
>>> Any comment on this patch?
>>
>> Can we get rid of rootfstype=jffs2 ? It's archaic and you can use UBI on
>> top of SPI NOR too. Also, isn't there already some kernel_addr_r for the
>> kernel address ?
> Yes, I know SPI flash can use UBI FS too, but we only enable jffs2 now.

JFFS2 is dead for a very long time. I only ever met it on some archaic
altera hardware, everywhere else it's UBI, which makes me wonder -- what
is the reason Altera is sticking to this antique ?

> kernelfit_addr is for kernel fit image offset in SPI flash, it is
> different from kernel_addr_r.

Shouldn't the MTD layout of the SPI NOR be described in mtdparts ?

> Maybe change kernelfit_addr to qspi_kernelfit_addr to avoid confusion.
>>
>> Finally, can't we switch to distro boot command on socfpga, to handle
>> all the various devices ?
> socfpga_common.h already use distro boot command, right?
> 
> In patch "[PATCH 1/2] configs: socfpga: Add QSPI support for Cyclone
> 5", it added QSPI to the list:
> +       BOOT_TARGET_DEVICES_QSPI(func) \

OK
Ley Foon Tan March 4, 2020, 9:31 a.m. UTC | #5
On Tue, Mar 3, 2020 at 8:16 PM Marek Vasut <marex at denx.de> wrote:
>
> On 3/3/20 10:21 AM, Ley Foon Tan wrote:
> > On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote:
> >>
> >> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> >>> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
> >>>>
> >>>> Add QSPI boot settings for Arria 10 SoCDK.
> >>>>
> >>>> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> >>>> ---
> >>>>  include/configs/socfpga_arria10_socdk.h | 9 +++++++++
> >>>>  1 file changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/include/configs/socfpga_arria10_socdk.h b/include/configs/socfpga_arria10_socdk.h
> >>>> index 645e66e6b0..e1d01c095f 100644
> >>>> --- a/include/configs/socfpga_arria10_socdk.h
> >>>> +++ b/include/configs/socfpga_arria10_socdk.h
> >>>> @@ -39,6 +39,15 @@
> >>>>  /* SPL memory allocation configuration, this is for FAT implementation */
> >>>>  #define CONFIG_SYS_SPL_MALLOC_SIZE     0x00015000
> >>>>
> >>>> +#define KERNEL_FIT_ADDR                __stringify(0x1200000)
> >>>> +
> >>>> +#define SOCFPGA_BOOT_SETTINGS \
> >>>> +       "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
> >>>> +       "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
> >>>> +                       "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
> >>>> +                       "bootm ${scriptaddr}\0" \
> >>>> +       "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
> >>>> +
> >>>>  /* The rest of the configuration is shared */
> >>>>  #include <configs/socfpga_common.h>
> >>>>
> >>>
> >>> Any comment on this patch?
> >>
> >> Can we get rid of rootfstype=jffs2 ? It's archaic and you can use UBI on
> >> top of SPI NOR too. Also, isn't there already some kernel_addr_r for the
> >> kernel address ?
> > Yes, I know SPI flash can use UBI FS too, but we only enable jffs2 now.
>
> JFFS2 is dead for a very long time. I only ever met it on some archaic
> altera hardware, everywhere else it's UBI, which makes me wonder -- what
> is the reason Altera is sticking to this antique ?

We haven't enable UBI for Gen5. Tien Fong did enable UBI for A10 and
submit the patches before.
But, he is busy on other tasks now and haven't continue rework the patches.
But, Gen5 haven't enable UBI yet.

>
> > kernelfit_addr is for kernel fit image offset in SPI flash, it is
> > different from kernel_addr_r.
>
> Shouldn't the MTD layout of the SPI NOR be described in mtdparts ?
It doesn't use mtdparts now. I will try enable it.


Regards
Ley Foon
Marek Vasut March 4, 2020, 12:17 p.m. UTC | #6
On 3/4/20 10:31 AM, Ley Foon Tan wrote:
> On Tue, Mar 3, 2020 at 8:16 PM Marek Vasut <marex at denx.de> wrote:
>>
>> On 3/3/20 10:21 AM, Ley Foon Tan wrote:
>>> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut <marex at denx.de> wrote:
>>>>
>>>> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
>>>>> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
>>>>>>
>>>>>> Add QSPI boot settings for Arria 10 SoCDK.
>>>>>>
>>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
>>>>>> ---
>>>>>>  include/configs/socfpga_arria10_socdk.h | 9 +++++++++
>>>>>>  1 file changed, 9 insertions(+)
>>>>>>
>>>>>> diff --git a/include/configs/socfpga_arria10_socdk.h b/include/configs/socfpga_arria10_socdk.h
>>>>>> index 645e66e6b0..e1d01c095f 100644
>>>>>> --- a/include/configs/socfpga_arria10_socdk.h
>>>>>> +++ b/include/configs/socfpga_arria10_socdk.h
>>>>>> @@ -39,6 +39,15 @@
>>>>>>  /* SPL memory allocation configuration, this is for FAT implementation */
>>>>>>  #define CONFIG_SYS_SPL_MALLOC_SIZE     0x00015000
>>>>>>
>>>>>> +#define KERNEL_FIT_ADDR                __stringify(0x1200000)
>>>>>> +
>>>>>> +#define SOCFPGA_BOOT_SETTINGS \
>>>>>> +       "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
>>>>>> +       "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
>>>>>> +                       "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
>>>>>> +                       "bootm ${scriptaddr}\0" \
>>>>>> +       "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
>>>>>> +
>>>>>>  /* The rest of the configuration is shared */
>>>>>>  #include <configs/socfpga_common.h>
>>>>>>
>>>>>
>>>>> Any comment on this patch?
>>>>
>>>> Can we get rid of rootfstype=jffs2 ? It's archaic and you can use UBI on
>>>> top of SPI NOR too. Also, isn't there already some kernel_addr_r for the
>>>> kernel address ?
>>> Yes, I know SPI flash can use UBI FS too, but we only enable jffs2 now.
>>
>> JFFS2 is dead for a very long time. I only ever met it on some archaic
>> altera hardware, everywhere else it's UBI, which makes me wonder -- what
>> is the reason Altera is sticking to this antique ?
> 
> We haven't enable UBI for Gen5. Tien Fong did enable UBI for A10 and
> submit the patches before.
> But, he is busy on other tasks now and haven't continue rework the patches.
> But, Gen5 haven't enable UBI yet.

Well, just enable it, it's two config options.

>>> kernelfit_addr is for kernel fit image offset in SPI flash, it is
>>> different from kernel_addr_r.
>>
>> Shouldn't the MTD layout of the SPI NOR be described in mtdparts ?
> It doesn't use mtdparts now. I will try enable it.

Thanks!
diff mbox series

Patch

diff --git a/include/configs/socfpga_arria10_socdk.h b/include/configs/socfpga_arria10_socdk.h
index 645e66e6b0..e1d01c095f 100644
--- a/include/configs/socfpga_arria10_socdk.h
+++ b/include/configs/socfpga_arria10_socdk.h
@@ -39,6 +39,15 @@ 
 /* SPL memory allocation configuration, this is for FAT implementation */
 #define CONFIG_SYS_SPL_MALLOC_SIZE	0x00015000
 
+#define KERNEL_FIT_ADDR		__stringify(0x1200000)
+
+#define SOCFPGA_BOOT_SETTINGS \
+	"kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
+	"qspiboot=setenv bootargs " CONFIG_BOOTARGS \
+			"root=/dev/mtdblock1 rw rootfstype=jffs2;" \
+			"bootm ${scriptaddr}\0" \
+	"qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
+
 /* The rest of the configuration is shared */
 #include <configs/socfpga_common.h>