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

2020-03-04 Thread Marek Vasut 
On 3/4/20 10:31 AM, Ley Foon Tan wrote:
> On Tue, Mar 3, 2020 at 8:16 PM Marek Vasut  wrote:
>>
>> On 3/3/20 10:21 AM, Ley Foon Tan wrote:
>>> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut  wrote:

 On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan  
> wrote:
>>
>> Add QSPI boot settings for Arria 10 SoCDK.
>>
>> Signed-off-by: Ley Foon Tan 
>> ---
>>  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(0x120)
>> +
>> +#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 
>>
>
> 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!

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

2020-03-04 Thread Ley Foon Tan 
On Tue, Mar 3, 2020 at 8:16 PM Marek Vasut  wrote:
>
> On 3/3/20 10:21 AM, Ley Foon Tan wrote:
> > On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut  wrote:
> >>
> >> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> >>> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan  
> >>> wrote:
> 
>  Add QSPI boot settings for Arria 10 SoCDK.
> 
>  Signed-off-by: Ley Foon Tan 
>  ---
>   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(0x120)
>  +
>  +#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 
> 
> >>>
> >>> 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

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

2020-03-03 Thread Marek Vasut 
On 3/3/20 10:21 AM, Ley Foon Tan wrote:
> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut  wrote:
>>
>> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
>>> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan  wrote:

 Add QSPI boot settings for Arria 10 SoCDK.

 Signed-off-by: Ley Foon Tan 
 ---
  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(0x120)
 +
 +#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 

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

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

2020-03-03 Thread Ley Foon Tan 
On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut  wrote:
>
> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> > On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan  wrote:
> >>
> >> Add QSPI boot settings for Arria 10 SoCDK.
> >>
> >> Signed-off-by: Ley Foon Tan 
> >> ---
> >>  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(0x120)
> >> +
> >> +#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 
> >>
> >
> > 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) \

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

2020-03-02 Thread Marek Vasut 
On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan  wrote:
>>
>> Add QSPI boot settings for Arria 10 SoCDK.
>>
>> Signed-off-by: Ley Foon Tan 
>> ---
>>  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(0x120)
>> +
>> +#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 
>>
> 
> 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 ?

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

2020-03-02 Thread Ley Foon Tan 
On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan  wrote:
>
> Add QSPI boot settings for Arria 10 SoCDK.
>
> Signed-off-by: Ley Foon Tan 
> ---
>  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(0x120)
> +
> +#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 
>

Any comment on this patch?

Regards
Ley Foon

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

2020-02-20 Thread Ley Foon Tan 
Add QSPI boot settings for Arria 10 SoCDK.

Signed-off-by: Ley Foon Tan 
---
 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(0x120)
+
+#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 
 
-- 
2.19.0