Re: [U-Boot] [PATCH v5 4/4] riscv: Remove redundant a2 store on DRAM base in start.S
Currently, the RISC-V U-Boot is saving a2 register at CONFIG_SYS_DRAM_BASE in start.S which does not make sense because there is no information passed by previous booting stage in a2 register.
This patch removes redundant a2 store on DRAM base.
Signed-off-by: Anup Patel anup@brainfault.org
arch/riscv/cpu/start.S | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 704190f946..e4276e8e19 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -38,8 +38,6 @@ _start: mv s0, a0 mv s1, a1
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0) la t0, trap_entry#ifdef CONFIG_RISCV_SMODE csrw stvec, t0 --
This is weird. I remember these two lines were already removed by Lukas's patch series before? Did not have time to dig out the history though.
Regards, Bin
You are correct, however I removed it again, because I did not want to break Rick's board. He did add a commit to the last pull request that removes these two lines and adjusts his board accordingly, but it is not in the current one.
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
B.R Rick
Thanks, Lukas
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote:
Currently, the RISC-V U-Boot is saving a2 register at CONFIG_SYS_DRAM_BASE in start.S which does not make sense because there is no information passed by previous booting stage in a2 register.
This patch removes redundant a2 store on DRAM base.
Signed-off-by: Anup Patel anup@brainfault.org
arch/riscv/cpu/start.S | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 704190f946..e4276e8e19 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -38,8 +38,6 @@ _start: mv s0, a0 mv s1, a1
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0) la t0, trap_entry#ifdef CONFIG_RISCV_SMODE csrw stvec, t0 --
This is weird. I remember these two lines were already removed by Lukas's patch series before? Did not have time to dig out the history though.
Regards, Bin
You are correct, however I removed it again, because I did not want to break Rick's board. He did add a commit to the last pull request that removes these two lines and adjusts his board accordingly, but it is not in the current one.
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
My apologies for the noise.
Regards, Anup
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道：
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote:
Currently, the RISC-V U-Boot is saving a2 register at CONFIG_SYS_DRAM_BASE in start.S which does not make sense because there is no information passed by previous booting stage in a2 register.
This patch removes redundant a2 store on DRAM base.
Signed-off-by: Anup Patel anup@brainfault.org
arch/riscv/cpu/start.S | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 704190f946..e4276e8e19 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -38,8 +38,6 @@ _start: mv s0, a0 mv s1, a1
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0) la t0, trap_entry#ifdef CONFIG_RISCV_SMODE csrw stvec, t0 --
This is weird. I remember these two lines were already removed by Lukas's patch series before? Did not have time to dig out the history though.
Regards, Bin
You are correct, however I removed it again, because I did not want to break Rick's board. He did add a commit to the last pull request that removes these two lines and adjusts his board accordingly, but it is not in the current one.
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S - li t0, CONFIG_SYS_SDRAM_BASE - SREG a2, 0(t0)
diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) { - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; + void **ptr = (void *)&prior_stage_fdt_address;
in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
B.R Rick
My apologies for the noise.
Regards, Anup
On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道：
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote:
> Currently, the RISC-V U-Boot is saving a2 register at > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > there is no information passed by previous booting stage in a2 > register. > > This patch removes redundant a2 store on DRAM base. > > Signed-off-by: Anup Patel anup@brainfault.org > --- > arch/riscv/cpu/start.S | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > 704190f946..e4276e8e19 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -38,8 +38,6 @@ _start: > mv s0, a0 > mv s1, a1 > > - li t0, CONFIG_SYS_SDRAM_BASE > - SREG a2, 0(t0) > la t0, trap_entry > #ifdef CONFIG_RISCV_SMODE > csrw stvec, t0 > --
This is weird. I remember these two lines were already removed by Lukas's patch series before? Did not have time to dig out the history though.
Regards, Bin
You are correct, however I removed it again, because I did not want to break Rick's board. He did add a commit to the last pull request that removes these two lines and adjusts his board accordingly, but it is not in the current one.
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in flash to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the FDT passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
In your board_fdt_blob_setup(), you can safely do: void **ptr = (void *)CONFIG_RISCV_PRIOR_FDT_BASE;
Regards, Anup
On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道：
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote:
> > Currently, the RISC-V U-Boot is saving a2 register at > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > there is no information passed by previous booting stage in a2 > > register. > > > > This patch removes redundant a2 store on DRAM base. > > > > Signed-off-by: Anup Patel anup@brainfault.org > > --- > > arch/riscv/cpu/start.S | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > 704190f946..e4276e8e19 100644 > > --- a/arch/riscv/cpu/start.S > > +++ b/arch/riscv/cpu/start.S > > @@ -38,8 +38,6 @@ _start: > > mv s0, a0 > > mv s1, a1 > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > - SREG a2, 0(t0) > > la t0, trap_entry > > #ifdef CONFIG_RISCV_SMODE > > csrw stvec, t0 > > -- > > This is weird. I remember these two lines were already removed by > Lukas's patch series before? Did not have time to dig out the history > though. >
> Regards, > Bin
You are correct, however I removed it again, because I did not want to break Rick's board. He did add a commit to the last pull request that removes these two lines and adjusts his board accordingly, but it is not in the current one.
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in flash to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the FDT passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
I think you can copy-over FDT in C code too. You don't need to do in start.S.
In your board_fdt_blob_setup(), you can safely do: void **ptr = (void *)CONFIG_RISCV_PRIOR_FDT_BASE;
Regards, Anup
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道：
On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道：
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote:
> > > Currently, the RISC-V U-Boot is saving a2 register at > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > there is no information passed by previous booting stage in a2 > > > register. > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > --- > > > arch/riscv/cpu/start.S | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > 704190f946..e4276e8e19 100644 > > > --- a/arch/riscv/cpu/start.S > > > +++ b/arch/riscv/cpu/start.S > > > @@ -38,8 +38,6 @@ _start: > > > mv s0, a0 > > > mv s1, a1 > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > - SREG a2, 0(t0) > > > la t0, trap_entry > > > #ifdef CONFIG_RISCV_SMODE > > > csrw stvec, t0 > > > -- > > > > This is weird. I remember these two lines were already removed by > > Lukas's patch series before? Did not have time to dig out the history > > though. > >
> > Regards, > > Bin > > You are correct, however I removed it again, because I did not want to break > Rick's board. He did add a commit to the last pull request that removes these > two lines and adjusts his board accordingly, but it is not in the current one. >
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in flash to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the FDT passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
Hi Anup
It can not achieve dtb delivery at runtime.
Rick
I think you can copy-over FDT in C code too. You don't need to do in start.S.
In your board_fdt_blob_setup(), you can safely do: void **ptr = (void *)CONFIG_RISCV_PRIOR_FDT_BASE;
Regards, Anup
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道：
On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com
wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道：
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com
wrote:
> > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make
sense because
> > > > there is no information passed by previous booting stage
in a2
> > > > register. > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > --- > > > > arch/riscv/cpu/start.S | 2 -- > > > > 1 file changed, 2 deletions(-) > > > > > > > > diff --git a/arch/riscv/cpu/start.S
b/arch/riscv/cpu/start.S index
> > > > 704190f946..e4276e8e19 100644 > > > > --- a/arch/riscv/cpu/start.S > > > > +++ b/arch/riscv/cpu/start.S > > > > @@ -38,8 +38,6 @@ _start: > > > > mv s0, a0 > > > > mv s1, a1 > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > - SREG a2, 0(t0) > > > > la t0, trap_entry > > > > #ifdef CONFIG_RISCV_SMODE > > > > csrw stvec, t0 > > > > -- > > > > > > This is weird. I remember these two lines were already
removed by
> > > Lukas's patch series before? Did not have time to dig out
the history
> > > though. > > >
> > > Regards, > > > Bin > > > > You are correct, however I removed it again, because I did
not want to break
> > Rick's board. He did add a commit to the last pull request
that removes these
> > two lines and adjusts his board accordingly, but it is not
in the current one.
> >
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around
#ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1
register
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in
flash
to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the
FDT
passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
Hi Anup
It can not achieve dtb delivery at runtime.
Can you elaborate why?
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
Regards, Anup
Rick
I think you can copy-over FDT in C code too. You don't need to do in start.S.
In your board_fdt_blob_setup(), you can safely do: void **ptr = (void *)CONFIG_RISCV_PRIOR_FDT_BASE;
Regards, Anup
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道：
On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道：
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > there is no information passed by previous booting stage in a2 > > > > > register. > > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > --- > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > 704190f946..e4276e8e19 100644 > > > > > --- a/arch/riscv/cpu/start.S > > > > > +++ b/arch/riscv/cpu/start.S > > > > > @@ -38,8 +38,6 @@ _start: > > > > > mv s0, a0 > > > > > mv s1, a1 > > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > - SREG a2, 0(t0) > > > > > la t0, trap_entry > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > csrw stvec, t0 > > > > > -- > > > > > > > > This is weird. I remember these two lines were already removed by > > > > Lukas's patch series before? Did not have time to dig out the history > > > > though. > > > > > > > > > Regards, > > > > Bin > > > > > > You are correct, however I removed it again, because I did not want to break > > > Rick's board. He did add a commit to the last pull request that removes these > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > Hi Likas > > Thanks for your explanation. > > RIck's commit as below > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in flash to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the FDT passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
Hi Anup
It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Regards, Anup
Rick
I think you can copy-over FDT in C code too. You don't need to do in start.S.
In your board_fdt_blob_setup(), you can safely do: void **ptr = (void *)CONFIG_RISCV_PRIOR_FDT_BASE;
Regards, Anup
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道：
On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > there is no information passed by previous booting stage in a2 > > > > > > register. > > > > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > --- > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > 704190f946..e4276e8e19 100644 > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > mv s0, a0 > > > > > > mv s1, a1 > > > > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > - SREG a2, 0(t0) > > > > > > la t0, trap_entry > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > csrw stvec, t0 > > > > > > -- > > > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > though. > > > > > > > > > > > > Regards, > > > > > Bin > > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > > Hi Likas > > > > Thanks for your explanation. > > > > RIck's commit as below > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > two lines corrupts BBL instructions. > > If this is important for some board then please have it around #ifdef. >
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in flash to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the FDT passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
Hi Anup
It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
My intention is to have these two lines removed from start.S so that U-Boot S-mode works fine on QEMU.
Regards, Anup
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道：
On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > > register. > > > > > > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > > --- > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > > mv s0, a0 > > > > > > > mv s1, a1 > > > > > > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > - SREG a2, 0(t0) > > > > > > > la t0, trap_entry > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > > csrw stvec, t0 > > > > > > > -- > > > > > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > though. > > > > > > > > > > > > > > > Regards, > > > > > > Bin > > > > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > > > > > Hi Likas > > > > > > Thanks for your explanation. > > > > > > RIck's commit as below > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > two lines corrupts BBL instructions. > > > > If this is important for some board then please have it around #ifdef. > > > > Hi Anup > > In the discussion as below : > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > I try to solve this issue with the aptch > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > - li t0, CONFIG_SYS_SDRAM_BASE > - SREG a2, 0(t0) > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > b/board/AndesTech/ax25-ae350/ax25-ae350.c > void *board_fdt_blob_setup(void) > { > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > + void **ptr = (void *)&prior_stage_fdt_address; > > in the previous pull request. > > But Bin do not agree with that I use prior_stage_fdt_address in > board_fdt_blob_setup( ) > I try to explain why I use it like that way. > Then Bin have not any reply in the following mail. > Finally I decide to drop this patch in the next pull request. > > Hi Bin > > How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in flash to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the FDT passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
Hi Anup
It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
My intention is to have these two lines removed from start.S so that U-Boot S-mode works fine on QEMU.
Regards, Anup
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道：
On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > > > register. > > > > > > > > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > > > --- > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > > > mv s0, a0 > > > > > > > > mv s1, a1 > > > > > > > > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > > - SREG a2, 0(t0) > > > > > > > > la t0, trap_entry > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > > > csrw stvec, t0 > > > > > > > > -- > > > > > > > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > > though. > > > > > > > > > > > > > > > > > > Regards, > > > > > > > Bin > > > > > > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > > > > > > > > Hi Likas > > > > > > > > Thanks for your explanation. > > > > > > > > RIck's commit as below > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > two lines corrupts BBL instructions. > > > > > > If this is important for some board then please have it around #ifdef. > > > > > > > Hi Anup > > > > In the discussion as below : > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > I try to solve this issue with the aptch > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > - li t0, CONFIG_SYS_SDRAM_BASE > > - SREG a2, 0(t0) > > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > void *board_fdt_blob_setup(void) > > { > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > in the previous pull request. > > > > But Bin do not agree with that I use prior_stage_fdt_address in > > board_fdt_blob_setup( ) > > I try to explain why I use it like that way. > > Then Bin have not any reply in the following mail. > > Finally I decide to drop this patch in the next pull request. > > > > Hi Bin > > > > How do you think about I recovery this patch to fix this issue ? > > Actually, previous booting stage can pass location of FDT stored in flash > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > in-place before passing on to Linux kernel so we should relocate the FDT > passed by previous booting stage to some board specific DRAM location. > > My suggestion is as follows: > > Instead of SDRAM_BASE, we can have new board specific config > CONFIG_RISCV_PRIOR_FDT_BASE > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > config then in start.S copy-over the FDT from location pointed by > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
Hi Anup
It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
Regards, Anup
On Tue, Nov 27, 2018 at 1:26 PM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > > > > register. > > > > > > > > > > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > > > > --- > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > > > > mv s0, a0 > > > > > > > > > mv s1, a1 > > > > > > > > > > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > > > - SREG a2, 0(t0) > > > > > > > > > la t0, trap_entry > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > > > > csrw stvec, t0 > > > > > > > > > -- > > > > > > > > > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > > > though. > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > Bin > > > > > > > > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > > > > > > > > > > > Hi Likas > > > > > > > > > > Thanks for your explanation. > > > > > > > > > > RIck's commit as below > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > two lines corrupts BBL instructions. > > > > > > > > If this is important for some board then please have it around #ifdef. > > > > > > > > > > Hi Anup > > > > > > In the discussion as below : > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > I try to solve this issue with the aptch > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > - SREG a2, 0(t0) > > > > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > void *board_fdt_blob_setup(void) > > > { > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > > > in the previous pull request. > > > > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > board_fdt_blob_setup( ) > > > I try to explain why I use it like that way. > > > Then Bin have not any reply in the following mail. > > > Finally I decide to drop this patch in the next pull request. > > > > > > Hi Bin > > > > > > How do you think about I recovery this patch to fix this issue ? > > > > Actually, previous booting stage can pass location of FDT stored in flash > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > in-place before passing on to Linux kernel so we should relocate the FDT > > passed by previous booting stage to some board specific DRAM location. > > > > My suggestion is as follows: > > > > Instead of SDRAM_BASE, we can have new board specific config > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > config then in start.S copy-over the FDT from location pointed by > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. >
Hi Anup
It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
I think I got your problem with CONFIG_OF_PRIOR_STAGE. U-Boot cannot update prior_stage_fdt_address when running from ROM/Flash.
Instead of storing FDT pointer on CONFIG_SYS_SDRAM_BASE, you can have a board specific location to save FDT pointer when booting from flash.
Here's a simple reference implementation (untested):
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..e83b9295d6 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -60,6 +60,19 @@ config RISCV_SMODE help Enable this option to build U-Boot for RISC-V S-Mode
+config RISCV_FDTPTR_SAVE + bool "Save FDT pointer for flash booting" + help + Enable this option to save FDT pointer passed to U-Boot + booting from ROM or Flash. + +config RISCV_FDTPTR_SAVE_ADDR + hex "Location for saving FDT pointer" + depends on RISCV_FDTPTR_SAVE + help + Set this to provide board specific location for saving + FDT pointer. + config 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index e4276e8e19..57c4070e0a 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -38,6 +38,11 @@ _start: mv s0, a0 mv s1, a1
+#ifdef CONFIG_RISCV_FDTPTR_SAVE + li t0, CONFIG_RISCV_FDTPTR_SAVE_ADDR + SREG s1, 0(t0) +#endif + la t0, trap_entry #ifdef CONFIG_RISCV_SMODE csrw stvec, t0 diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 5f4ca0f5a7..f3535a1d52 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -66,9 +66,9 @@ ulong board_flash_get_legacy(ulong base, int banknum, flash_info_t *info)
void *board_fdt_blob_setup(void) { - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; + void **ptr = (void *)CONFIG_RISCV_FDTPTR_SAVE_ADDR; if (fdt_magic(*ptr) == FDT_MAGIC) - return (void *)*ptr; + return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; } diff --git a/configs/ax25-ae350_64_defconfig b/configs/ax25-ae350_64_defconfig index b250d3fc7e..b4ba2ed18b 100644 --- a/configs/ax25-ae350_64_defconfig +++ b/configs/ax25-ae350_64_defconfig @@ -35,3 +35,5 @@ CONFIG_SYS_NS16550=y CONFIG_SPI=y CONFIG_ATCSPI200_SPI=y CONFIG_ATCPIT100_TIMER=y +CONFIG_RISCV_FDTPTR_SAVE=y +CONFIG_RISCV_FDTPTR_SAVE_ADDR=0x80000000
Regards, Anup
On 27.11.18 09:42, Anup Patel wrote:
On Tue, Nov 27, 2018 at 1:26 PM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： >> >> On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: >>> >>> On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: >>>> >>>> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： >>>>> >>>>> On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: >>>>>> >>>>>>>>>> Currently, the RISC-V U-Boot is saving a2 register at >>>>>>>>>> CONFIG_SYS_DRAM_BASE in start.S which does not make sense because >>>>>>>>>> there is no information passed by previous booting stage in a2 >>>>>>>>>> register. >>>>>>>>>> >>>>>>>>>> This patch removes redundant a2 store on DRAM base. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Anup Patel anup@brainfault.org >>>>>>>>>> --- >>>>>>>>>> arch/riscv/cpu/start.S | 2 -- >>>>>>>>>> 1 file changed, 2 deletions(-) >>>>>>>>>> >>>>>>>>>> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index >>>>>>>>>> 704190f946..e4276e8e19 100644 >>>>>>>>>> --- a/arch/riscv/cpu/start.S >>>>>>>>>> +++ b/arch/riscv/cpu/start.S >>>>>>>>>> @@ -38,8 +38,6 @@ _start: >>>>>>>>>> mv s0, a0 >>>>>>>>>> mv s1, a1 >>>>>>>>>> >>>>>>>>>> - li t0, CONFIG_SYS_SDRAM_BASE >>>>>>>>>> - SREG a2, 0(t0) >>>>>>>>>> la t0, trap_entry >>>>>>>>>> #ifdef CONFIG_RISCV_SMODE >>>>>>>>>> csrw stvec, t0 >>>>>>>>>> -- >>>>>>>>> >>>>>>>>> This is weird. I remember these two lines were already removed by >>>>>>>>> Lukas's patch series before? Did not have time to dig out the history >>>>>>>>> though. >>>>>>>>> >>>>>> >>>>>>>>> Regards, >>>>>>>>> Bin >>>>>>>> >>>>>>>> You are correct, however I removed it again, because I did not want to break >>>>>>>> Rick's board. He did add a commit to the last pull request that removes these >>>>>>>> two lines and adjusts his board accordingly, but it is not in the current one. >>>>>>>> >>>>>> >>>>>> Hi Likas >>>>>> >>>>>> Thanks for your explanation. >>>>>> >>>>>> RIck's commit as below >>>>>> https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html >>>>> >>>>> When we run U-Boot in S-mode the BBL runs from 0x80000000 so this >>>>> two lines corrupts BBL instructions. >>>>> >>>>> If this is important for some board then please have it around #ifdef. >>>>> >>>> >>>> Hi Anup >>>> >>>> In the discussion as below : >>>> https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html >>>> >>>> I try to solve this issue with the aptch >>>> [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register >>>> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S >>>> - li t0, CONFIG_SYS_SDRAM_BASE >>>> - SREG a2, 0(t0) >>>> >>>> diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c >>>> b/board/AndesTech/ax25-ae350/ax25-ae350.c >>>> void *board_fdt_blob_setup(void) >>>> { >>>> - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; >>>> + void **ptr = (void *)&prior_stage_fdt_address; >>>> >>>> in the previous pull request. >>>> >>>> But Bin do not agree with that I use prior_stage_fdt_address in >>>> board_fdt_blob_setup( ) >>>> I try to explain why I use it like that way. >>>> Then Bin have not any reply in the following mail. >>>> Finally I decide to drop this patch in the next pull request. >>>> >>>> Hi Bin >>>> >>>> How do you think about I recovery this patch to fix this issue ? >>> >>> Actually, previous booting stage can pass location of FDT stored in flash >>> to U-Boot. U-Boot requires FDT at a DRAM location which it can modify >>> in-place before passing on to Linux kernel so we should relocate the FDT >>> passed by previous booting stage to some board specific DRAM location. >>> >>> My suggestion is as follows: >>> >>> Instead of SDRAM_BASE, we can have new board specific config >>> CONFIG_RISCV_PRIOR_FDT_BASE >>> >>> If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by >>> config then in start.S copy-over the FDT from location pointed by >>> "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. >> > > Hi Anup > > It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
I think I got your problem with CONFIG_OF_PRIOR_STAGE. U-Boot cannot update prior_stage_fdt_address when running from ROM/Flash.
Instead of storing FDT pointer on CONFIG_SYS_SDRAM_BASE, you can have a board specific location to save FDT pointer when booting from flash.
I feel like I'm missing something obvious here. Why can't we just preserve a2 in a callee saved register and then eventually write it straight into gd?
For example, how about something like this (untested, probably mangled by mailer):
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index ae57fb8313..2281ec0537 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -47,3 +47,8 @@ int print_cpuinfo(void)
return 0; } + +void arch_setup_gd(gd_t *new_gd, ulong arg) +{ + new_gd->fdt_blob = (const void *)arg; +} diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 7cd7755190..2acbd15f7c 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -44,8 +44,7 @@ trap_vector:
.global trap_entry handle_reset: - li t0, CONFIG_SYS_SDRAM_BASE - SREG a2, 0(t0) + mv s0, a2 la t0, trap_entry csrw mtvec, t0 csrwi mstatus, 0 @@ -75,6 +74,7 @@ call_board_init_f_0: mv a0, sp jal board_init_f_alloc_reserve mv sp, a0 + mv a1, s0 jal board_init_f_init_reserve
mv a0, zero /* a0 <-- boot_flags = 0 */ diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index 208ef08562..c370ae0808 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -113,7 +113,7 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries) asm volatile("lgdtl %0\n" : : "m" (gdt)); }
-void arch_setup_gd(gd_t *new_gd) +void arch_setup_gd(gd_t *new_gd, ulong arg) { u64 *gdt_addr;
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c index 6c063e8200..a17ee5fd78 100644 --- a/arch/x86/cpu/x86_64/cpu.c +++ b/arch/x86/cpu/x86_64/cpu.c @@ -16,7 +16,7 @@ */ struct global_data *global_data_ptr = (struct global_data *)~0;
-void arch_setup_gd(gd_t *new_gd) +void arch_setup_gd(gd_t *new_gd, ulong arg) { global_data_ptr = new_gd; } diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 7d290740bf..4313ff1e74 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -72,7 +72,7 @@ static int x86_spl_init(void) */ gd->new_gd = (struct global_data *)ptr; memcpy(gd->new_gd, gd, sizeof(*gd)); - arch_setup_gd(gd->new_gd); + arch_setup_gd(gd->new_gd, 0); gd->start_addr_sp = (ulong)ptr;
/* Cache the SPI flash. Otherwise copying the code to RAM takes ages */ diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 5f4ca0f5a7..74f2d40ec5 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -66,9 +66,8 @@ ulong board_flash_get_legacy(ulong base, int banknum, flash_info_t *info)
void *board_fdt_blob_setup(void) { - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; - if (fdt_magic(*ptr) == FDT_MAGIC) - return (void *)*ptr; + if (fdt_magic(gd->fdt_blob) == FDT_MAGIC) + return (void *)gd->fdt_blob;
return (void *)CONFIG_SYS_FDT_BASE; } diff --git a/common/board_f.c b/common/board_f.c index f1a1432d86..d853480ae7 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -726,7 +726,7 @@ static int jump_to_copy(void) * with the stack in SDRAM and Global Data in temporary memory * (CPU cache) */ - arch_setup_gd(gd->new_gd); + arch_setup_gd(gd->new_gd, 0); board_init_f_r_trampoline(gd->start_addr_sp); #else relocate_code(gd->start_addr_sp, gd->new_gd, gd->relocaddr); diff --git a/common/board_r.c b/common/board_r.c index c55e33eec2..39ac04d528 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -856,7 +856,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) * dropping the new_gd parameter. */ #if CONFIG_IS_ENABLED(X86_64) - arch_setup_gd(new_gd); + arch_setup_gd(new_gd, 0); #endif
#ifdef CONFIG_NEEDS_MANUAL_RELOC diff --git a/common/init/board_init.c b/common/init/board_init.c index 526fee35ff..be57b422f8 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -12,7 +12,7 @@ DECLARE_GLOBAL_DATA_PTR;
/* Unfortunately x86 or ARM can't compile this code as gd cannot be assigned */ #if !defined(CONFIG_X86) && !defined(CONFIG_ARM) -__weak void arch_setup_gd(struct global_data *gd_ptr) +__weak void arch_setup_gd(struct global_data *gd_ptr, ulong arg) { gd = gd_ptr; } @@ -96,7 +96,7 @@ ulong board_init_f_alloc_reserve(ulong top) * (seemingly useless) incrementation causes no code increase. */
-void board_init_f_init_reserve(ulong base) +void board_init_f_init_reserve(ulong base, ulong arg) { struct global_data *gd_ptr;
@@ -110,7 +110,7 @@ void board_init_f_init_reserve(ulong base) memset(gd_ptr, '\0', sizeof(*gd)); /* set GD unless architecture did it already */ #if !defined(CONFIG_ARM) - arch_setup_gd(gd_ptr); + arch_setup_gd(gd_ptr, arg); #endif /* next alloc will be higher by one GD plus 16-byte alignment */ base += roundup(sizeof(struct global_data), 16); diff --git a/include/init.h b/include/init.h index afc953d51e..d80fa75ed3 100644 --- a/include/init.h +++ b/include/init.h @@ -152,6 +152,7 @@ void board_init_f_init_reserve(ulong base); /** * arch_setup_gd() - Set up the global_data pointer * @gd_ptr: Pointer to global data + * @arg: Architecture specific opaque data * * This pointer is special in some architectures and cannot easily be assigned * to. For example on x86 it is implemented by adding a specific record to its @@ -160,7 +161,7 @@ void board_init_f_init_reserve(ulong base); * * gd = gd_ptr; */ -void arch_setup_gd(gd_t *gd_ptr); +void arch_setup_gd(gd_t *gd_ptr, ulong arg);
/* common/board_r.c */ void board_init_r(gd_t *id, ulong dest_addr) __attribute__ ((noreturn));
Alexander Graf agraf@suse.de 於 2018年11月27日 週二 下午6:42寫道：
On 27.11.18 09:42, Anup Patel wrote:
On Tue, Nov 27, 2018 at 1:26 PM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: >> >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： >>> >>> On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: >>>> >>>> On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: >>>>> >>>>> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： >>>>>> >>>>>> On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: >>>>>>> >>>>>>>>>>> Currently, the RISC-V U-Boot is saving a2 register at >>>>>>>>>>> CONFIG_SYS_DRAM_BASE in start.S which does not make sense because >>>>>>>>>>> there is no information passed by previous booting stage in a2 >>>>>>>>>>> register. >>>>>>>>>>> >>>>>>>>>>> This patch removes redundant a2 store on DRAM base. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Anup Patel anup@brainfault.org >>>>>>>>>>> --- >>>>>>>>>>> arch/riscv/cpu/start.S | 2 -- >>>>>>>>>>> 1 file changed, 2 deletions(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index >>>>>>>>>>> 704190f946..e4276e8e19 100644 >>>>>>>>>>> --- a/arch/riscv/cpu/start.S >>>>>>>>>>> +++ b/arch/riscv/cpu/start.S >>>>>>>>>>> @@ -38,8 +38,6 @@ _start: >>>>>>>>>>> mv s0, a0 >>>>>>>>>>> mv s1, a1 >>>>>>>>>>> >>>>>>>>>>> - li t0, CONFIG_SYS_SDRAM_BASE >>>>>>>>>>> - SREG a2, 0(t0) >>>>>>>>>>> la t0, trap_entry >>>>>>>>>>> #ifdef CONFIG_RISCV_SMODE >>>>>>>>>>> csrw stvec, t0 >>>>>>>>>>> -- >>>>>>>>>> >>>>>>>>>> This is weird. I remember these two lines were already removed by >>>>>>>>>> Lukas's patch series before? Did not have time to dig out the history >>>>>>>>>> though. >>>>>>>>>> >>>>>>> >>>>>>>>>> Regards, >>>>>>>>>> Bin >>>>>>>>> >>>>>>>>> You are correct, however I removed it again, because I did not want to break >>>>>>>>> Rick's board. He did add a commit to the last pull request that removes these >>>>>>>>> two lines and adjusts his board accordingly, but it is not in the current one. >>>>>>>>> >>>>>>> >>>>>>> Hi Likas >>>>>>> >>>>>>> Thanks for your explanation. >>>>>>> >>>>>>> RIck's commit as below >>>>>>> https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html >>>>>> >>>>>> When we run U-Boot in S-mode the BBL runs from 0x80000000 so this >>>>>> two lines corrupts BBL instructions. >>>>>> >>>>>> If this is important for some board then please have it around #ifdef. >>>>>> >>>>> >>>>> Hi Anup >>>>> >>>>> In the discussion as below : >>>>> https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html >>>>> >>>>> I try to solve this issue with the aptch >>>>> [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register >>>>> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S >>>>> - li t0, CONFIG_SYS_SDRAM_BASE >>>>> - SREG a2, 0(t0) >>>>> >>>>> diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c >>>>> b/board/AndesTech/ax25-ae350/ax25-ae350.c >>>>> void *board_fdt_blob_setup(void) >>>>> { >>>>> - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; >>>>> + void **ptr = (void *)&prior_stage_fdt_address; >>>>> >>>>> in the previous pull request. >>>>> >>>>> But Bin do not agree with that I use prior_stage_fdt_address in >>>>> board_fdt_blob_setup( ) >>>>> I try to explain why I use it like that way. >>>>> Then Bin have not any reply in the following mail. >>>>> Finally I decide to drop this patch in the next pull request. >>>>> >>>>> Hi Bin >>>>> >>>>> How do you think about I recovery this patch to fix this issue ? >>>> >>>> Actually, previous booting stage can pass location of FDT stored in flash >>>> to U-Boot. U-Boot requires FDT at a DRAM location which it can modify >>>> in-place before passing on to Linux kernel so we should relocate the FDT >>>> passed by previous booting stage to some board specific DRAM location. >>>> >>>> My suggestion is as follows: >>>> >>>> Instead of SDRAM_BASE, we can have new board specific config >>>> CONFIG_RISCV_PRIOR_FDT_BASE >>>> >>>> If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by >>>> config then in start.S copy-over the FDT from location pointed by >>>> "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. >>> >> >> Hi Anup >> >> It can not achieve dtb delivery at runtime. > > > Can you elaborate why? >
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
> I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
I think I got your problem with CONFIG_OF_PRIOR_STAGE. U-Boot cannot update prior_stage_fdt_address when running from ROM/Flash.
Instead of storing FDT pointer on CONFIG_SYS_SDRAM_BASE, you can have a board specific location to save FDT pointer when booting from flash.
I feel like I'm missing something obvious here. Why can't we just preserve a2 in a callee saved register and then eventually write it straight into gd?
For example, how about something like this (untested, probably mangled by mailer):
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index ae57fb8313..2281ec0537 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -47,3 +47,8 @@ int print_cpuinfo(void)
return 0;}
+void arch_setup_gd(gd_t *new_gd, ulong arg) +{
new_gd->fdt_blob = (const void *)arg;+} diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 7cd7755190..2acbd15f7c 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -44,8 +44,7 @@ trap_vector:
.global trap_entry handle_reset:
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)
mv s0, a2 la t0, trap_entry csrw mtvec, t0 csrwi mstatus, 0@@ -75,6 +74,7 @@ call_board_init_f_0: mv a0, sp jal board_init_f_alloc_reserve mv sp, a0
mv a1, s0 jal board_init_f_init_reserve mv a0, zero /* a0 <-- boot_flags = 0 */diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index 208ef08562..c370ae0808 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -113,7 +113,7 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries) asm volatile("lgdtl %0\n" : : "m" (gdt)); }
-void arch_setup_gd(gd_t *new_gd) +void arch_setup_gd(gd_t *new_gd, ulong arg) { u64 *gdt_addr;
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c index 6c063e8200..a17ee5fd78 100644 --- a/arch/x86/cpu/x86_64/cpu.c +++ b/arch/x86/cpu/x86_64/cpu.c @@ -16,7 +16,7 @@ */ struct global_data *global_data_ptr = (struct global_data *)~0;
-void arch_setup_gd(gd_t *new_gd) +void arch_setup_gd(gd_t *new_gd, ulong arg) { global_data_ptr = new_gd; } diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 7d290740bf..4313ff1e74 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -72,7 +72,7 @@ static int x86_spl_init(void) */ gd->new_gd = (struct global_data *)ptr; memcpy(gd->new_gd, gd, sizeof(*gd));
arch_setup_gd(gd->new_gd);
arch_setup_gd(gd->new_gd, 0); gd->start_addr_sp = (ulong)ptr; /* Cache the SPI flash. Otherwise copying the code to RAM takes ages */diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 5f4ca0f5a7..74f2d40ec5 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -66,9 +66,8 @@ ulong board_flash_get_legacy(ulong base, int banknum, flash_info_t *info)
void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;if (fdt_magic(*ptr) == FDT_MAGIC)return (void *)*ptr;
if (fdt_magic(gd->fdt_blob) == FDT_MAGIC)return (void *)gd->fdt_blob; return (void *)CONFIG_SYS_FDT_BASE;} diff --git a/common/board_f.c b/common/board_f.c index f1a1432d86..d853480ae7 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -726,7 +726,7 @@ static int jump_to_copy(void) * with the stack in SDRAM and Global Data in temporary memory * (CPU cache) */
arch_setup_gd(gd->new_gd);
arch_setup_gd(gd->new_gd, 0); board_init_f_r_trampoline(gd->start_addr_sp);#else relocate_code(gd->start_addr_sp, gd->new_gd, gd->relocaddr); diff --git a/common/board_r.c b/common/board_r.c index c55e33eec2..39ac04d528 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -856,7 +856,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) * dropping the new_gd parameter. */ #if CONFIG_IS_ENABLED(X86_64)
arch_setup_gd(new_gd);
arch_setup_gd(new_gd, 0);#endif
#ifdef CONFIG_NEEDS_MANUAL_RELOC diff --git a/common/init/board_init.c b/common/init/board_init.c index 526fee35ff..be57b422f8 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -12,7 +12,7 @@ DECLARE_GLOBAL_DATA_PTR;
/* Unfortunately x86 or ARM can't compile this code as gd cannot be assigned */ #if !defined(CONFIG_X86) && !defined(CONFIG_ARM) -__weak void arch_setup_gd(struct global_data *gd_ptr) +__weak void arch_setup_gd(struct global_data *gd_ptr, ulong arg) { gd = gd_ptr; } @@ -96,7 +96,7 @@ ulong board_init_f_alloc_reserve(ulong top)
- (seemingly useless) incrementation causes no code increase.
*/
-void board_init_f_init_reserve(ulong base) +void board_init_f_init_reserve(ulong base, ulong arg) { struct global_data *gd_ptr;
@@ -110,7 +110,7 @@ void board_init_f_init_reserve(ulong base) memset(gd_ptr, '\0', sizeof(*gd)); /* set GD unless architecture did it already */ #if !defined(CONFIG_ARM)
arch_setup_gd(gd_ptr);
arch_setup_gd(gd_ptr, arg);#endif /* next alloc will be higher by one GD plus 16-byte alignment */ base += roundup(sizeof(struct global_data), 16); diff --git a/include/init.h b/include/init.h index afc953d51e..d80fa75ed3 100644 --- a/include/init.h +++ b/include/init.h @@ -152,6 +152,7 @@ void board_init_f_init_reserve(ulong base); /**
- arch_setup_gd() - Set up the global_data pointer
- @gd_ptr: Pointer to global data
- @arg: Architecture specific opaque data
- This pointer is special in some architectures and cannot easily be
assigned
- to. For example on x86 it is implemented by adding a specific record
to its @@ -160,7 +161,7 @@ void board_init_f_init_reserve(ulong base);
- gd = gd_ptr;
*/ -void arch_setup_gd(gd_t *gd_ptr); +void arch_setup_gd(gd_t *gd_ptr, ulong arg);
/* common/board_r.c */ void board_init_r(gd_t *id, ulong dest_addr) __attribute__ ((noreturn));
Hi Alex
I will try and test it. Thanks a lot
B.R Rick
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道：
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > > > > register. > > > > > > > > > > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > > > > --- > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > > > > mv s0, a0 > > > > > > > > > mv s1, a1 > > > > > > > > > > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > > > - SREG a2, 0(t0) > > > > > > > > > la t0, trap_entry > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > > > > csrw stvec, t0 > > > > > > > > > -- > > > > > > > > > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > > > though. > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > Bin > > > > > > > > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > > > > > > > > > > > Hi Likas > > > > > > > > > > Thanks for your explanation. > > > > > > > > > > RIck's commit as below > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > two lines corrupts BBL instructions. > > > > > > > > If this is important for some board then please have it around #ifdef. > > > > > > > > > > Hi Anup > > > > > > In the discussion as below : > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > I try to solve this issue with the aptch > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > - SREG a2, 0(t0) > > > > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > void *board_fdt_blob_setup(void) > > > { > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > > > in the previous pull request. > > > > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > board_fdt_blob_setup( ) > > > I try to explain why I use it like that way. > > > Then Bin have not any reply in the following mail. > > > Finally I decide to drop this patch in the next pull request. > > > > > > Hi Bin > > > > > > How do you think about I recovery this patch to fix this issue ? > > > > Actually, previous booting stage can pass location of FDT stored in flash > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > in-place before passing on to Linux kernel so we should relocate the FDT > > passed by previous booting stage to some board specific DRAM location. > > > > My suggestion is as follows: > > > > Instead of SDRAM_BASE, we can have new board specific config > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > config then in start.S copy-over the FDT from location pointed by > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. >
Hi Anup
It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
Regards, Anup
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道：
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道：
On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > > > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > > > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > > > > > register. > > > > > > > > > > > > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > > > > > --- > > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > > > > > mv s0, a0 > > > > > > > > > > mv s1, a1 > > > > > > > > > > > > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > > > > - SREG a2, 0(t0) > > > > > > > > > > la t0, trap_entry > > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > > > > > csrw stvec, t0 > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > > > > though. > > > > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > Bin > > > > > > > > > > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > > > > > > > > > > > > > > Hi Likas > > > > > > > > > > > > Thanks for your explanation. > > > > > > > > > > > > RIck's commit as below > > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > > two lines corrupts BBL instructions. > > > > > > > > > > If this is important for some board then please have it around #ifdef. > > > > > > > > > > > > > Hi Anup > > > > > > > > In the discussion as below : > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > > I try to solve this issue with the aptch > > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > - SREG a2, 0(t0) > > > > > > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > void *board_fdt_blob_setup(void) > > > > { > > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > > > > > in the previous pull request. > > > > > > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > > board_fdt_blob_setup( ) > > > > I try to explain why I use it like that way. > > > > Then Bin have not any reply in the following mail. > > > > Finally I decide to drop this patch in the next pull request. > > > > > > > > Hi Bin > > > > > > > > How do you think about I recovery this patch to fix this issue ? > > > > > > Actually, previous booting stage can pass location of FDT stored in flash > > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > > in-place before passing on to Linux kernel so we should relocate the FDT > > > passed by previous booting stage to some board specific DRAM location. > > > > > > My suggestion is as follows: > > > > > > Instead of SDRAM_BASE, we can have new board specific config > > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > > config then in start.S copy-over the FDT from location pointed by > > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > > > Hi Anup > > It can not achieve dtb delivery at runtime.
Can you elaborate why?
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
If I understand correctly, so far we have two scenarios to support:
1. U-Boot is booting directly from reset vector from ROM. This canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
2. U-Boot is booting from previous bootloader, and DT is passed in a1 register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
Regards, Bin
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道：
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道：
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: >> >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： >> > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: >> > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: >> > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： >> > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: >> > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because >> > > > > > > > > > there is no information passed by previous booting stage in a2 >> > > > > > > > > > register. >> > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. >> > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org >> > > > > > > > > > --- >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- >> > > > > > > > > > 1 file changed, 2 deletions(-) >> > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index >> > > > > > > > > > 704190f946..e4276e8e19 100644 >> > > > > > > > > > --- a/arch/riscv/cpu/start.S >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: >> > > > > > > > > > mv s0, a0 >> > > > > > > > > > mv s1, a1 >> > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE >> > > > > > > > > > - SREG a2, 0(t0) >> > > > > > > > > > la t0, trap_entry >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE >> > > > > > > > > > csrw stvec, t0 >> > > > > > > > > > -- >> > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history >> > > > > > > > > though. >> > > > > > > > > >> > > > > > >> > > > > > > > > Regards, >> > > > > > > > > Bin >> > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. >> > > > > > > > >> > > > > > >> > > > > > Hi Likas >> > > > > > >> > > > > > Thanks for your explanation. >> > > > > > >> > > > > > RIck's commit as below >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html >> > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this >> > > > > two lines corrupts BBL instructions. >> > > > > >> > > > > If this is important for some board then please have it around #ifdef. >> > > > > >> > > > >> > > > Hi Anup >> > > > >> > > > In the discussion as below : >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html >> > > > >> > > > I try to solve this issue with the aptch >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S >> > > > - li t0, CONFIG_SYS_SDRAM_BASE >> > > > - SREG a2, 0(t0) >> > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c >> > > > void *board_fdt_blob_setup(void) >> > > > { >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; >> > > > + void **ptr = (void *)&prior_stage_fdt_address; >> > > > >> > > > in the previous pull request. >> > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in >> > > > board_fdt_blob_setup( ) >> > > > I try to explain why I use it like that way. >> > > > Then Bin have not any reply in the following mail. >> > > > Finally I decide to drop this patch in the next pull request. >> > > > >> > > > Hi Bin >> > > > >> > > > How do you think about I recovery this patch to fix this issue ? >> > > >> > > Actually, previous booting stage can pass location of FDT stored in flash >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify >> > > in-place before passing on to Linux kernel so we should relocate the FDT >> > > passed by previous booting stage to some board specific DRAM location. >> > > >> > > My suggestion is as follows: >> > > >> > > Instead of SDRAM_BASE, we can have new board specific config >> > > CONFIG_RISCV_PRIOR_FDT_BASE >> > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by >> > > config then in start.S copy-over the FDT from location pointed by >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. >> > >> >> Hi Anup >> >> It can not achieve dtb delivery at runtime. > > > Can you elaborate why? >
CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
> I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there.
My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
If I understand correctly, so far we have two scenarios to support:
- U-Boot is booting directly from reset vector from ROM. This
canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
- U-Boot is booting from previous bootloader, and DT is passed in a1
register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
prior_stage_fdt_address is just a way that a variable was declared in data section. It offer a temp memory to preserve a1 for fdt relocation. If I rename prior_stage_fdt_address as fdt_preserve_address, then it will look like not belong to CONFIG_OF_PRIOR_STAGE. But the code will be somehow duplicate. That is why I recycle it and try to use prior_stage_fdt_address in board_fdt_blob_setup( )
B.R Rick
Regards, Bin
Rick Chen rickchen36@gmail.com 於 2018年11月29日 週四 下午6:42寫道：
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道：
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道：
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > >> > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > >> > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > >> > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > >> > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > >> > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > >> > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > >> > > > > > > > > > register. > >> > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > >> > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > >> > > > > > > > > > --- > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > >> > > > > > > > > > 1 file changed, 2 deletions(-) > >> > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > >> > > > > > > > > > mv s0, a0 > >> > > > > > > > > > mv s1, a1 > >> > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > >> > > > > > > > > > - SREG a2, 0(t0) > >> > > > > > > > > > la t0, trap_entry > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > >> > > > > > > > > > csrw stvec, t0 > >> > > > > > > > > > -- > >> > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > >> > > > > > > > > though. > >> > > > > > > > > > >> > > > > > > >> > > > > > > > > Regards, > >> > > > > > > > > Bin > >> > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > >> > > > > > > > > >> > > > > > > >> > > > > > Hi Likas > >> > > > > > > >> > > > > > Thanks for your explanation. > >> > > > > > > >> > > > > > RIck's commit as below > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > >> > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > >> > > > > two lines corrupts BBL instructions. > >> > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > >> > > > > > >> > > > > >> > > > Hi Anup > >> > > > > >> > > > In the discussion as below : > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > >> > > > > >> > > > I try to solve this issue with the aptch > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > >> > > > - SREG a2, 0(t0) > >> > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > >> > > > void *board_fdt_blob_setup(void) > >> > > > { > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > >> > > > > >> > > > in the previous pull request. > >> > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > >> > > > board_fdt_blob_setup( ) > >> > > > I try to explain why I use it like that way. > >> > > > Then Bin have not any reply in the following mail. > >> > > > Finally I decide to drop this patch in the next pull request. > >> > > > > >> > > > Hi Bin > >> > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > >> > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > >> > > passed by previous booting stage to some board specific DRAM location. > >> > > > >> > > My suggestion is as follows: > >> > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > >> > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > >> > > config then in start.S copy-over the FDT from location pointed by > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > >> > > >> > >> Hi Anup > >> > >> It can not achieve dtb delivery at runtime. > > > > > > Can you elaborate why? > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
> > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
If I understand correctly, so far we have two scenarios to support:
- U-Boot is booting directly from reset vector from ROM. This
canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
- U-Boot is booting from previous bootloader, and DT is passed in a1
register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
prior_stage_fdt_address is just a way that a variable was declared in data section. It offer a temp memory to preserve a1 for fdt relocation. If I rename prior_stage_fdt_address as fdt_preserve_address, then it will look like not belong to CONFIG_OF_PRIOR_STAGE. But the code will be somehow duplicate. That is why I recycle it and try to use prior_stage_fdt_address in board_fdt_blob_setup( )
If you still think it is not ok. I can try another way to overcome it. :)
Maybe I can try Alex's way.
Rick
B.R Rick
Regards, Bin
Hi Rick,
On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道：
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道：
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道：
On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > >> > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > >> > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > >> > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > >> > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > >> > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > >> > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > >> > > > > > > > > > register. > >> > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > >> > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > >> > > > > > > > > > --- > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > >> > > > > > > > > > 1 file changed, 2 deletions(-) > >> > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > >> > > > > > > > > > mv s0, a0 > >> > > > > > > > > > mv s1, a1 > >> > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > >> > > > > > > > > > - SREG a2, 0(t0) > >> > > > > > > > > > la t0, trap_entry > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > >> > > > > > > > > > csrw stvec, t0 > >> > > > > > > > > > -- > >> > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > >> > > > > > > > > though. > >> > > > > > > > > > >> > > > > > > >> > > > > > > > > Regards, > >> > > > > > > > > Bin > >> > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > >> > > > > > > > > >> > > > > > > >> > > > > > Hi Likas > >> > > > > > > >> > > > > > Thanks for your explanation. > >> > > > > > > >> > > > > > RIck's commit as below > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > >> > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > >> > > > > two lines corrupts BBL instructions. > >> > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > >> > > > > > >> > > > > >> > > > Hi Anup > >> > > > > >> > > > In the discussion as below : > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > >> > > > > >> > > > I try to solve this issue with the aptch > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > >> > > > - SREG a2, 0(t0) > >> > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > >> > > > void *board_fdt_blob_setup(void) > >> > > > { > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > >> > > > > >> > > > in the previous pull request. > >> > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > >> > > > board_fdt_blob_setup( ) > >> > > > I try to explain why I use it like that way. > >> > > > Then Bin have not any reply in the following mail. > >> > > > Finally I decide to drop this patch in the next pull request. > >> > > > > >> > > > Hi Bin > >> > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > >> > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > >> > > passed by previous booting stage to some board specific DRAM location. > >> > > > >> > > My suggestion is as follows: > >> > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > >> > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > >> > > config then in start.S copy-over the FDT from location pointed by > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > >> > > >> > >> Hi Anup > >> > >> It can not achieve dtb delivery at runtime. > > > > > > Can you elaborate why? > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > I am wondering how it can be modified at run time ?
I think you miss-understood my suggestion. I did not meant changing CONFIG_RISCV_PRIOR_FDT_BASE at runtime.
> > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > My original patch also can achieve that dtb passed by a1 and relocated and boot.
Great !!!
Why not update your original patch to relocate FDT and use FDT from safer location?
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
If I understand correctly, so far we have two scenarios to support:
- U-Boot is booting directly from reset vector from ROM. This
canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
- U-Boot is booting from previous bootloader, and DT is passed in a1
register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
What I don't understand is that if U-Boot is booting directly from the reset vector, there is no a1 passed by anyone. U-Boot itself should figure out a way to determine the DT and that's how CONFIG_OF_EMBED or CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration?
prior_stage_fdt_address is just a way that a variable was declared in data section. It offer a temp memory to preserve a1 for fdt relocation. If I rename prior_stage_fdt_address as fdt_preserve_address, then it will look like not belong to CONFIG_OF_PRIOR_STAGE. But the code will be somehow duplicate. That is why I recycle it and try to use prior_stage_fdt_address in board_fdt_blob_setup( )
+ Simon's comment to hear his thoughts.
Regards, Bin
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 上午9:48寫道：
Hi Rick,
On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道：
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道：
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道： > > On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > >> > > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > >> > > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > >> > > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > >> > > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > >> > > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > >> > > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > > >> > > > > > > > > > register. > > >> > > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > >> > > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > >> > > > > > > > > > --- > > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > >> > > > > > > > > > 1 file changed, 2 deletions(-) > > >> > > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > >> > > > > > > > > > mv s0, a0 > > >> > > > > > > > > > mv s1, a1 > > >> > > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > >> > > > > > > > > > - SREG a2, 0(t0) > > >> > > > > > > > > > la t0, trap_entry > > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > >> > > > > > > > > > csrw stvec, t0 > > >> > > > > > > > > > -- > > >> > > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > >> > > > > > > > > though. > > >> > > > > > > > > > > >> > > > > > > > >> > > > > > > > > Regards, > > >> > > > > > > > > Bin > > >> > > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > >> > > > > > > > > > >> > > > > > > > >> > > > > > Hi Likas > > >> > > > > > > > >> > > > > > Thanks for your explanation. > > >> > > > > > > > >> > > > > > RIck's commit as below > > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > >> > > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > >> > > > > two lines corrupts BBL instructions. > > >> > > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > > >> > > > > > > >> > > > > > >> > > > Hi Anup > > >> > > > > > >> > > > In the discussion as below : > > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > >> > > > > > >> > > > I try to solve this issue with the aptch > > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > > >> > > > - SREG a2, 0(t0) > > >> > > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > >> > > > void *board_fdt_blob_setup(void) > > >> > > > { > > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > > >> > > > > > >> > > > in the previous pull request. > > >> > > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > > >> > > > board_fdt_blob_setup( ) > > >> > > > I try to explain why I use it like that way. > > >> > > > Then Bin have not any reply in the following mail. > > >> > > > Finally I decide to drop this patch in the next pull request. > > >> > > > > > >> > > > Hi Bin > > >> > > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > > >> > > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > > >> > > passed by previous booting stage to some board specific DRAM location. > > >> > > > > >> > > My suggestion is as follows: > > >> > > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > > >> > > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > >> > > config then in start.S copy-over the FDT from location pointed by > > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > >> > > > >> > > >> Hi Anup > > >> > > >> It can not achieve dtb delivery at runtime. > > > > > > > > > Can you elaborate why? > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > > I am wondering how it can be modified at run time ? > > I think you miss-understood my suggestion. I did not meant changing > CONFIG_RISCV_PRIOR_FDT_BASE at runtime. > > > > > > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > > > My original patch also can achieve that dtb passed by a1 and relocated and boot. > > Great !!! > > Why not update your original patch to relocate FDT and use FDT from > safer location? >
Good question. Above can see the question I ask for Bin : How do you think about I recovery this patch to fix this issue ?
Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
If I understand correctly, so far we have two scenarios to support:
- U-Boot is booting directly from reset vector from ROM. This
canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
- U-Boot is booting from previous bootloader, and DT is passed in a1
register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
What I don't understand is that if U-Boot is booting directly from the reset vector, there is no a1 passed by anyone. U-Boot itself should figure out a way to determine the DT and that's how CONFIG_OF_EMBED or CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration?
I use CONFIG_OF_BOARD and board_fdt_blob_setup as below:
void *board_fdt_blob_setup(void) { void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; if (fdt_magic(*ptr) == FDT_MAGIC) return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; }
1. When booting from RAM a1 can be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE temporarily.
2. When booting from FLASH a1 can NOT be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE temporarily.
So dtb shall be get from CONFIG_SYS_FDT_BASE (flash base) which the dtb shall be pre-burned in this flash space.. Or CONFIG_SYS_FDT_BASE maybe a memory map space (NOT RAM or ROM) provided by HW.
That is why I mean using CONFIG_OF_BOARD can support both boot from ram or rom with one configuration.
Regards, Rick
prior_stage_fdt_address is just a way that a variable was declared in data section. It offer a temp memory to preserve a1 for fdt relocation. If I rename prior_stage_fdt_address as fdt_preserve_address, then it will look like not belong to CONFIG_OF_PRIOR_STAGE. But the code will be somehow duplicate. That is why I recycle it and try to use prior_stage_fdt_address in board_fdt_blob_setup( )
- Simon's comment to hear his thoughts.
Regards, Bin
Hi Rick,
On Fri, Nov 30, 2018 at 2:05 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 上午9:48寫道：
Hi Rick,
On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道：
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道：
On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote: > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道： > > > > On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > > >> > > > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > > >> > > > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > >> > > > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > >> > > > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > >> > > > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > >> > > > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > > > >> > > > > > > > > > register. > > > >> > > > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > >> > > > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > >> > > > > > > > > > --- > > > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > >> > > > > > > > > > 1 file changed, 2 deletions(-) > > > >> > > > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > > > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > >> > > > > > > > > > mv s0, a0 > > > >> > > > > > > > > > mv s1, a1 > > > >> > > > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > >> > > > > > > > > > - SREG a2, 0(t0) > > > >> > > > > > > > > > la t0, trap_entry > > > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > >> > > > > > > > > > csrw stvec, t0 > > > >> > > > > > > > > > -- > > > >> > > > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > > > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > >> > > > > > > > > though. > > > >> > > > > > > > > > > > >> > > > > > > > > >> > > > > > > > > Regards, > > > >> > > > > > > > > Bin > > > >> > > > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > > > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > >> > > > > > > > > > > >> > > > > > > > > >> > > > > > Hi Likas > > > >> > > > > > > > > >> > > > > > Thanks for your explanation. > > > >> > > > > > > > > >> > > > > > RIck's commit as below > > > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > >> > > > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > >> > > > > two lines corrupts BBL instructions. > > > >> > > > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > > > >> > > > > > > > >> > > > > > > >> > > > Hi Anup > > > >> > > > > > > >> > > > In the discussion as below : > > > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > >> > > > > > > >> > > > I try to solve this issue with the aptch > > > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > >> > > > - SREG a2, 0(t0) > > > >> > > > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > >> > > > void *board_fdt_blob_setup(void) > > > >> > > > { > > > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > >> > > > > > > >> > > > in the previous pull request. > > > >> > > > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > >> > > > board_fdt_blob_setup( ) > > > >> > > > I try to explain why I use it like that way. > > > >> > > > Then Bin have not any reply in the following mail. > > > >> > > > Finally I decide to drop this patch in the next pull request. > > > >> > > > > > > >> > > > Hi Bin > > > >> > > > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > > > >> > > > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > > > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > > > >> > > passed by previous booting stage to some board specific DRAM location. > > > >> > > > > > >> > > My suggestion is as follows: > > > >> > > > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > > > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > > > >> > > > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > > >> > > config then in start.S copy-over the FDT from location pointed by > > > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > > >> > > > > >> > > > >> Hi Anup > > > >> > > > >> It can not achieve dtb delivery at runtime. > > > > > > > > > > > > Can you elaborate why? > > > > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > > > I am wondering how it can be modified at run time ? > > > > I think you miss-understood my suggestion. I did not meant changing > > CONFIG_RISCV_PRIOR_FDT_BASE at runtime. > > > > > > > > > > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > > > > > My original patch also can achieve that dtb passed by a1 and relocated and boot. > > > > Great !!! > > > > Why not update your original patch to relocate FDT and use FDT from > > safer location? > > > > Good question. > Above can see the question I ask for Bin : > How do you think about I recovery this patch to fix this issue ? > > Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE !
Can you explain why you need CONFIG_OF_BOARD ? Why can you not use CONFIG_OF_PRIOR_STAGE ?
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
If I understand correctly, so far we have two scenarios to support:
- U-Boot is booting directly from reset vector from ROM. This
canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
- U-Boot is booting from previous bootloader, and DT is passed in a1
register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
What I don't understand is that if U-Boot is booting directly from the reset vector, there is no a1 passed by anyone. U-Boot itself should figure out a way to determine the DT and that's how CONFIG_OF_EMBED or CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration?
I use CONFIG_OF_BOARD and board_fdt_blob_setup as below:
void *board_fdt_blob_setup(void) { void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; if (fdt_magic(*ptr) == FDT_MAGIC) return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; }
- When booting from RAM a1 can be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE
temporarily.
- When booting from FLASH a1 can NOT be passed by loader and reserved in
CONFIG_SYS_SDRAM_BASE temporarily.
So dtb shall be get from CONFIG_SYS_FDT_BASE (flash base) whichthe dtb shall be pre-burned in this flash space.. Or CONFIG_SYS_FDT_BASE maybe a memory map space (NOT RAM or ROM) provided by HW.
That is why I mean using CONFIG_OF_BOARD can support both boot from ram or rom with one configuration.
Thanks for the info. So with the latest info, I now understand you wanted to use CONFIG_OF_BOARD to support both configurations. That makes sense to me, however I still wanted to point out the canonical way to support booting from ROM is via CONFIG_OF_SEPARATE or CONFIG_OF_EMBED. Since your board can support booting from reset vector directly, what's the need to support the booting from RAM configuration? In that configuration, what is the first stage bootloader?
Regards, Bin
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:21寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:05 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 上午9:48寫道：
Hi Rick,
On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道：
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道： > > On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote: > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道： > > > > > > On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > > > > > > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > > > >> > > > > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > > > >> > > > > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > >> > > > > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > >> > > > > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > >> > > > > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > >> > > > > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > >> > > > > > > > > > register. > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > >> > > > > > > > > > --- > > > > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > >> > > > > > > > > > 1 file changed, 2 deletions(-) > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > >> > > > > > > > > > mv s0, a0 > > > > >> > > > > > > > > > mv s1, a1 > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > >> > > > > > > > > > - SREG a2, 0(t0) > > > > >> > > > > > > > > > la t0, trap_entry > > > > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > >> > > > > > > > > > csrw stvec, t0 > > > > >> > > > > > > > > > -- > > > > >> > > > > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > > > > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > >> > > > > > > > > though. > > > > >> > > > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > Regards, > > > > >> > > > > > > > > Bin > > > > >> > > > > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > >> > > > > > > > > > > > >> > > > > > > > > > >> > > > > > Hi Likas > > > > >> > > > > > > > > > >> > > > > > Thanks for your explanation. > > > > >> > > > > > > > > > >> > > > > > RIck's commit as below > > > > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > >> > > > > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > >> > > > > two lines corrupts BBL instructions. > > > > >> > > > > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > > > > >> > > > > > > > > >> > > > > > > > >> > > > Hi Anup > > > > >> > > > > > > > >> > > > In the discussion as below : > > > > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > >> > > > > > > > >> > > > I try to solve this issue with the aptch > > > > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > >> > > > - SREG a2, 0(t0) > > > > >> > > > > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > >> > > > void *board_fdt_blob_setup(void) > > > > >> > > > { > > > > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > >> > > > > > > > >> > > > in the previous pull request. > > > > >> > > > > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > > >> > > > board_fdt_blob_setup( ) > > > > >> > > > I try to explain why I use it like that way. > > > > >> > > > Then Bin have not any reply in the following mail. > > > > >> > > > Finally I decide to drop this patch in the next pull request. > > > > >> > > > > > > > >> > > > Hi Bin > > > > >> > > > > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > > > > >> > > > > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > > > > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > > > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > > > > >> > > passed by previous booting stage to some board specific DRAM location. > > > > >> > > > > > > >> > > My suggestion is as follows: > > > > >> > > > > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > > > > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > >> > > > > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > > > >> > > config then in start.S copy-over the FDT from location pointed by > > > > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > > > >> > > > > > >> > > > > >> Hi Anup > > > > >> > > > > >> It can not achieve dtb delivery at runtime. > > > > > > > > > > > > > > > Can you elaborate why? > > > > > > > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > > > > I am wondering how it can be modified at run time ? > > > > > > I think you miss-understood my suggestion. I did not meant changing > > > CONFIG_RISCV_PRIOR_FDT_BASE at runtime. > > > > > > > > > > > > > > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > > > > > > > My original patch also can achieve that dtb passed by a1 and relocated and boot. > > > > > > Great !!! > > > > > > Why not update your original patch to relocate FDT and use FDT from > > > safer location? > > > > > > > Good question. > > Above can see the question I ask for Bin : > > How do you think about I recovery this patch to fix this issue ? > > > > Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE ! > > Can you explain why you need CONFIG_OF_BOARD ? > Why can you not use CONFIG_OF_PRIOR_STAGE ? >
You can find the discussion as below: https://patchwork.ozlabs.org/patch/988884/
If I understand correctly, so far we have two scenarios to support:
- U-Boot is booting directly from reset vector from ROM. This
canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
- U-Boot is booting from previous bootloader, and DT is passed in a1
register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
What I don't understand is that if U-Boot is booting directly from the reset vector, there is no a1 passed by anyone. U-Boot itself should figure out a way to determine the DT and that's how CONFIG_OF_EMBED or CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration?
I use CONFIG_OF_BOARD and board_fdt_blob_setup as below:
void *board_fdt_blob_setup(void) { void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; if (fdt_magic(*ptr) == FDT_MAGIC) return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; }
- When booting from RAM a1 can be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE
temporarily.
- When booting from FLASH a1 can NOT be passed by loader and reserved in
CONFIG_SYS_SDRAM_BASE temporarily.
So dtb shall be get from CONFIG_SYS_FDT_BASE (flash base) whichthe dtb shall be pre-burned in this flash space.. Or CONFIG_SYS_FDT_BASE maybe a memory map space (NOT RAM or ROM) provided by HW.
That is why I mean using CONFIG_OF_BOARD can support both boot from ram or rom with one configuration.
Thanks for the info. So with the latest info, I now understand you wanted to use CONFIG_OF_BOARD to support both configurations. That makes sense to me, however I still wanted to point out the canonical way to support booting from ROM is via CONFIG_OF_SEPARATE or CONFIG_OF_EMBED. Since your board can support booting from reset vector directly, what's the need to support the booting from RAM configuration?
I will develop u-boot via booting from ram. It will easy for debugging. After develop complete. It will be good for demo via booting from flash. With one configuration, it can decrease to make mistake with wrong configuration between 2 configuration for ram or rom individually.
In that configuration, what is the first stage
bootloader?
When booting from ram GDB will be the first stage to load u-boot. So it is not good for demo, because it need a pc to fire gdb.
B.R Rick
Regards, Bin
Hi Rick,
On Fri, Nov 30, 2018 at 2:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:21寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:05 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 上午9:48寫道：
Hi Rick,
On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道：
Hi Rick,
On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote: > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道： > > > > On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道： > > > > > > > > On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > > > > >> > > > > > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > > > > >> > > > > > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > > >> > > > > > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > >> > > > > > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > >> > > > > > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > >> > > > > > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > >> > > > > > > > > > register. > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > >> > > > > > > > > > --- > > > > > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > >> > > > > > > > > > 1 file changed, 2 deletions(-) > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > >> > > > > > > > > > mv s0, a0 > > > > > >> > > > > > > > > > mv s1, a1 > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > >> > > > > > > > > > - SREG a2, 0(t0) > > > > > >> > > > > > > > > > la t0, trap_entry > > > > > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > >> > > > > > > > > > csrw stvec, t0 > > > > > >> > > > > > > > > > -- > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > >> > > > > > > > > though. > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > Regards, > > > > > >> > > > > > > > > Bin > > > > > >> > > > > > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > Hi Likas > > > > > >> > > > > > > > > > > >> > > > > > Thanks for your explanation. > > > > > >> > > > > > > > > > > >> > > > > > RIck's commit as below > > > > > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > >> > > > > > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > > >> > > > > two lines corrupts BBL instructions. > > > > > >> > > > > > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > Hi Anup > > > > > >> > > > > > > > > >> > > > In the discussion as below : > > > > > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > >> > > > > > > > > >> > > > I try to solve this issue with the aptch > > > > > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > > > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > > > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > >> > > > - SREG a2, 0(t0) > > > > > >> > > > > > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > >> > > > void *board_fdt_blob_setup(void) > > > > > >> > > > { > > > > > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > > > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > > >> > > > > > > > > >> > > > in the previous pull request. > > > > > >> > > > > > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > > > >> > > > board_fdt_blob_setup( ) > > > > > >> > > > I try to explain why I use it like that way. > > > > > >> > > > Then Bin have not any reply in the following mail. > > > > > >> > > > Finally I decide to drop this patch in the next pull request. > > > > > >> > > > > > > > > >> > > > Hi Bin > > > > > >> > > > > > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > > > > > >> > > > > > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > > > > > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > > > > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > > > > > >> > > passed by previous booting stage to some board specific DRAM location. > > > > > >> > > > > > > > >> > > My suggestion is as follows: > > > > > >> > > > > > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > > > > > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > > >> > > > > > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > > > > >> > > config then in start.S copy-over the FDT from location pointed by > > > > > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > > > > >> > > > > > > >> > > > > > >> Hi Anup > > > > > >> > > > > > >> It can not achieve dtb delivery at runtime. > > > > > > > > > > > > > > > > > > Can you elaborate why? > > > > > > > > > > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > > > > > I am wondering how it can be modified at run time ? > > > > > > > > I think you miss-understood my suggestion. I did not meant changing > > > > CONFIG_RISCV_PRIOR_FDT_BASE at runtime. > > > > > > > > > > > > > > > > > > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > > > > > > > > > My original patch also can achieve that dtb passed by a1 and relocated and boot. > > > > > > > > Great !!! > > > > > > > > Why not update your original patch to relocate FDT and use FDT from > > > > safer location? > > > > > > > > > > Good question. > > > Above can see the question I ask for Bin : > > > How do you think about I recovery this patch to fix this issue ? > > > > > > Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE ! > > > > Can you explain why you need CONFIG_OF_BOARD ? > > Why can you not use CONFIG_OF_PRIOR_STAGE ? > > > > You can find the discussion as below: > https://patchwork.ozlabs.org/patch/988884/ >
If I understand correctly, so far we have two scenarios to support:
- U-Boot is booting directly from reset vector from ROM. This
canonical way to support DT is via CONFIG_OF_EMBED or CONFIG_OF_SEPARATE. In such configuration, there is no any previous bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply.
- U-Boot is booting from previous bootloader, and DT is passed in a1
register. Such configuration we can use CONFIG_OF_PRIOR_STAGE.
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
What I don't understand is that if U-Boot is booting directly from the reset vector, there is no a1 passed by anyone. U-Boot itself should figure out a way to determine the DT and that's how CONFIG_OF_EMBED or CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration?
I use CONFIG_OF_BOARD and board_fdt_blob_setup as below:
void *board_fdt_blob_setup(void) { void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; if (fdt_magic(*ptr) == FDT_MAGIC) return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; }
- When booting from RAM a1 can be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE
temporarily.
- When booting from FLASH a1 can NOT be passed by loader and reserved in
CONFIG_SYS_SDRAM_BASE temporarily.
So dtb shall be get from CONFIG_SYS_FDT_BASE (flash base) whichthe dtb shall be pre-burned in this flash space.. Or CONFIG_SYS_FDT_BASE maybe a memory map space (NOT RAM or ROM) provided by HW.
That is why I mean using CONFIG_OF_BOARD can support both boot from ram or rom with one configuration.
Thanks for the info. So with the latest info, I now understand you wanted to use CONFIG_OF_BOARD to support both configurations. That makes sense to me, however I still wanted to point out the canonical way to support booting from ROM is via CONFIG_OF_SEPARATE or CONFIG_OF_EMBED. Since your board can support booting from reset vector directly, what's the need to support the booting from RAM configuration?
I will develop u-boot via booting from ram. It will easy for debugging. After develop complete. It will be good for demo via booting from flash. With one configuration, it can decrease to make mistake with wrong configuration between 2 configuration for ram or rom individually.
In that configuration, what is the first stage
bootloader?
When booting from ram GDB will be the first stage to load u-boot. So it is not good for demo, because it need a pc to fire gdb.
OK, thanks for the info. For this patch, let's go with your proposal then, but please add some comments in the codes there for people to understand later.
Regards, Bin
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:57寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:21寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:05 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 上午9:48寫道：
Hi Rick,
On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道： > > Hi Rick, > > On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote: > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道： > > > > > > On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道： > > > > > > > > > > On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > > > > > >> > > > > > > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > > > > > >> > > > > > > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > > > >> > > > > > > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > >> > > > > > > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > > >> > > > > > > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > >> > > > > > > > > > register. > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > >> > > > > > > > > > --- > > > > > > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > >> > > > > > > > > > 1 file changed, 2 deletions(-) > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > >> > > > > > > > > > mv s0, a0 > > > > > > >> > > > > > > > > > mv s1, a1 > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > >> > > > > > > > > > - SREG a2, 0(t0) > > > > > > >> > > > > > > > > > la t0, trap_entry > > > > > > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > >> > > > > > > > > > csrw stvec, t0 > > > > > > >> > > > > > > > > > -- > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > >> > > > > > > > > though. > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > Regards, > > > > > > >> > > > > > > > > Bin > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > Hi Likas > > > > > > >> > > > > > > > > > > > >> > > > > > Thanks for your explanation. > > > > > > >> > > > > > > > > > > > >> > > > > > RIck's commit as below > > > > > > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > >> > > > > > > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > > > >> > > > > two lines corrupts BBL instructions. > > > > > > >> > > > > > > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > Hi Anup > > > > > > >> > > > > > > > > > >> > > > In the discussion as below : > > > > > > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > >> > > > > > > > > > >> > > > I try to solve this issue with the aptch > > > > > > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > > > > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > > > > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > >> > > > - SREG a2, 0(t0) > > > > > > >> > > > > > > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > > >> > > > void *board_fdt_blob_setup(void) > > > > > > >> > > > { > > > > > > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > > > > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > > > >> > > > > > > > > > >> > > > in the previous pull request. > > > > > > >> > > > > > > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > > > > >> > > > board_fdt_blob_setup( ) > > > > > > >> > > > I try to explain why I use it like that way. > > > > > > >> > > > Then Bin have not any reply in the following mail. > > > > > > >> > > > Finally I decide to drop this patch in the next pull request. > > > > > > >> > > > > > > > > > >> > > > Hi Bin > > > > > > >> > > > > > > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > > > > > > >> > > > > > > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > > > > > > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > > > > > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > > > > > > >> > > passed by previous booting stage to some board specific DRAM location. > > > > > > >> > > > > > > > > >> > > My suggestion is as follows: > > > > > > >> > > > > > > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > > > > > > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > > > >> > > > > > > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > > > > > >> > > config then in start.S copy-over the FDT from location pointed by > > > > > > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > > > > > >> > > > > > > > >> > > > > > > >> Hi Anup > > > > > > >> > > > > > > >> It can not achieve dtb delivery at runtime. > > > > > > > > > > > > > > > > > > > > > Can you elaborate why? > > > > > > > > > > > > > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > > > > > > I am wondering how it can be modified at run time ? > > > > > > > > > > I think you miss-understood my suggestion. I did not meant changing > > > > > CONFIG_RISCV_PRIOR_FDT_BASE at runtime. > > > > > > > > > > > > > > > > > > > > > > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > > > > > > > > > > > My original patch also can achieve that dtb passed by a1 and relocated and boot. > > > > > > > > > > Great !!! > > > > > > > > > > Why not update your original patch to relocate FDT and use FDT from > > > > > safer location? > > > > > > > > > > > > > Good question. > > > > Above can see the question I ask for Bin : > > > > How do you think about I recovery this patch to fix this issue ? > > > > > > > > Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE ! > > > > > > Can you explain why you need CONFIG_OF_BOARD ? > > > Why can you not use CONFIG_OF_PRIOR_STAGE ? > > > > > > > You can find the discussion as below: > > https://patchwork.ozlabs.org/patch/988884/ > > > > If I understand correctly, so far we have two scenarios to support: > > 1. U-Boot is booting directly from reset vector from ROM. This > canonical way to support DT is via CONFIG_OF_EMBED or > CONFIG_OF_SEPARATE. In such configuration, there is no any previous > bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply. > > 2. U-Boot is booting from previous bootloader, and DT is passed in a1 > register. Such configuration we can use CONFIG_OF_PRIOR_STAGE. >
I use the 3rd way, CONFIG_OF_BOARD. It can be found in README. And can be chosen by make menuconfig
It is the most flexible way that I can implement my own board_fdt_blob_setup( ) which can both support boot from RAM or ROM without any configuration change. I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage.
What I don't understand is that if U-Boot is booting directly from the reset vector, there is no a1 passed by anyone. U-Boot itself should figure out a way to determine the DT and that's how CONFIG_OF_EMBED or CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration?
I use CONFIG_OF_BOARD and board_fdt_blob_setup as below:
void *board_fdt_blob_setup(void) { void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; if (fdt_magic(*ptr) == FDT_MAGIC) return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; }
- When booting from RAM a1 can be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE
temporarily.
- When booting from FLASH a1 can NOT be passed by loader and reserved in
CONFIG_SYS_SDRAM_BASE temporarily.
So dtb shall be get from CONFIG_SYS_FDT_BASE (flash base) whichthe dtb shall be pre-burned in this flash space.. Or CONFIG_SYS_FDT_BASE maybe a memory map space (NOT RAM or ROM) provided by HW.
That is why I mean using CONFIG_OF_BOARD can support both boot from ram or rom with one configuration.
Thanks for the info. So with the latest info, I now understand you wanted to use CONFIG_OF_BOARD to support both configurations. That makes sense to me, however I still wanted to point out the canonical way to support booting from ROM is via CONFIG_OF_SEPARATE or CONFIG_OF_EMBED. Since your board can support booting from reset vector directly, what's the need to support the booting from RAM configuration?
I will develop u-boot via booting from ram. It will easy for debugging. After develop complete. It will be good for demo via booting from flash. With one configuration, it can decrease to make mistake with wrong configuration between 2 configuration for ram or rom individually.
In that configuration, what is the first stage
bootloader?
When booting from ram GDB will be the first stage to load u-boot. So it is not good for demo, because it need a pc to fire gdb.
OK, thanks for the info. For this patch, let's go with your proposal then, but please add some comments in the codes there for people to understand later.
Hi Bin
You say go with your proposal : Does that mean you agree with the following modification ? arch/riscv/cpu/start.S - li t0, CONFIG_SYS_SDRAM_BASE - SREG a2, 0(t0)
board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) { - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; + void **ptr = (void *)&prior_stage_fdt_address;
And add some comments in this patch for people to understand it.
Or I still can NOT borrow prior_stage_fdt_address and shall find another way to overcome the problem after remove the two line in start.S ?
B.R Rick
Regards, Bin
Hi Rick,
On Fri, Nov 30, 2018 at 3:15 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:57寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:21寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:05 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 上午9:48寫道：
Hi Rick,
On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote: > > Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道： > > > > Hi Rick, > > > > On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道： > > > > > > > > On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道： > > > > > > > > > > > > On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > > > > > > >> > > > > > > > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > > > > > > >> > > > > > > > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > > > > >> > > > > > > > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > >> > > > > > > > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > > > >> > > > > > > > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > > >> > > > > > > > > > register. > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > > >> > > > > > > > > > --- > > > > > > > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > > >> > > > > > > > > > 1 file changed, 2 deletions(-) > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > > >> > > > > > > > > > mv s0, a0 > > > > > > > >> > > > > > > > > > mv s1, a1 > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > >> > > > > > > > > > - SREG a2, 0(t0) > > > > > > > >> > > > > > > > > > la t0, trap_entry > > > > > > > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > > >> > > > > > > > > > csrw stvec, t0 > > > > > > > >> > > > > > > > > > -- > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > > >> > > > > > > > > though. > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > Regards, > > > > > > > >> > > > > > > > > Bin > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > > > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > > > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > Hi Likas > > > > > > > >> > > > > > > > > > > > > >> > > > > > Thanks for your explanation. > > > > > > > >> > > > > > > > > > > > > >> > > > > > RIck's commit as below > > > > > > > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > >> > > > > > > > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > > > > >> > > > > two lines corrupts BBL instructions. > > > > > > > >> > > > > > > > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > Hi Anup > > > > > > > >> > > > > > > > > > > >> > > > In the discussion as below : > > > > > > > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > >> > > > > > > > > > > >> > > > I try to solve this issue with the aptch > > > > > > > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > > > > > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > > > > > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > >> > > > - SREG a2, 0(t0) > > > > > > > >> > > > > > > > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > > > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > > > >> > > > void *board_fdt_blob_setup(void) > > > > > > > >> > > > { > > > > > > > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > > > > > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > > > > >> > > > > > > > > > > >> > > > in the previous pull request. > > > > > > > >> > > > > > > > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > > > > > >> > > > board_fdt_blob_setup( ) > > > > > > > >> > > > I try to explain why I use it like that way. > > > > > > > >> > > > Then Bin have not any reply in the following mail. > > > > > > > >> > > > Finally I decide to drop this patch in the next pull request. > > > > > > > >> > > > > > > > > > > >> > > > Hi Bin > > > > > > > >> > > > > > > > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > > > > > > > >> > > > > > > > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > > > > > > > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > > > > > > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > > > > > > > >> > > passed by previous booting stage to some board specific DRAM location. > > > > > > > >> > > > > > > > > > >> > > My suggestion is as follows: > > > > > > > >> > > > > > > > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > > > > > > > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > > > > >> > > > > > > > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > > > > > > >> > > config then in start.S copy-over the FDT from location pointed by > > > > > > > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > > > > > > >> > > > > > > > > >> > > > > > > > >> Hi Anup > > > > > > > >> > > > > > > > >> It can not achieve dtb delivery at runtime. > > > > > > > > > > > > > > > > > > > > > > > > Can you elaborate why? > > > > > > > > > > > > > > > > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > > > > > > > I am wondering how it can be modified at run time ? > > > > > > > > > > > > I think you miss-understood my suggestion. I did not meant changing > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE at runtime. > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > > > > > > > > > > > > > My original patch also can achieve that dtb passed by a1 and relocated and boot. > > > > > > > > > > > > Great !!! > > > > > > > > > > > > Why not update your original patch to relocate FDT and use FDT from > > > > > > safer location? > > > > > > > > > > > > > > > > Good question. > > > > > Above can see the question I ask for Bin : > > > > > How do you think about I recovery this patch to fix this issue ? > > > > > > > > > > Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE ! > > > > > > > > Can you explain why you need CONFIG_OF_BOARD ? > > > > Why can you not use CONFIG_OF_PRIOR_STAGE ? > > > > > > > > > > You can find the discussion as below: > > > https://patchwork.ozlabs.org/patch/988884/ > > > > > > > If I understand correctly, so far we have two scenarios to support: > > > > 1. U-Boot is booting directly from reset vector from ROM. This > > canonical way to support DT is via CONFIG_OF_EMBED or > > CONFIG_OF_SEPARATE. In such configuration, there is no any previous > > bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply. > > > > 2. U-Boot is booting from previous bootloader, and DT is passed in a1 > > register. Such configuration we can use CONFIG_OF_PRIOR_STAGE. > > > > I use the 3rd way, CONFIG_OF_BOARD. > It can be found in README. > And can be chosen by make menuconfig > > It is the most flexible way that I can implement my own board_fdt_blob_setup( ) > which can both support boot from RAM or ROM without any configuration change. > I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage. >
What I don't understand is that if U-Boot is booting directly from the reset vector, there is no a1 passed by anyone. U-Boot itself should figure out a way to determine the DT and that's how CONFIG_OF_EMBED or CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration?
I use CONFIG_OF_BOARD and board_fdt_blob_setup as below:
void *board_fdt_blob_setup(void) { void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; if (fdt_magic(*ptr) == FDT_MAGIC) return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; }
- When booting from RAM a1 can be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE
temporarily.
- When booting from FLASH a1 can NOT be passed by loader and reserved in
CONFIG_SYS_SDRAM_BASE temporarily.
So dtb shall be get from CONFIG_SYS_FDT_BASE (flash base) whichthe dtb shall be pre-burned in this flash space.. Or CONFIG_SYS_FDT_BASE maybe a memory map space (NOT RAM or ROM) provided by HW.
That is why I mean using CONFIG_OF_BOARD can support both boot from ram or rom with one configuration.
Thanks for the info. So with the latest info, I now understand you wanted to use CONFIG_OF_BOARD to support both configurations. That makes sense to me, however I still wanted to point out the canonical way to support booting from ROM is via CONFIG_OF_SEPARATE or CONFIG_OF_EMBED. Since your board can support booting from reset vector directly, what's the need to support the booting from RAM configuration?
I will develop u-boot via booting from ram. It will easy for debugging. After develop complete. It will be good for demo via booting from flash. With one configuration, it can decrease to make mistake with wrong configuration between 2 configuration for ram or rom individually.
In that configuration, what is the first stage
bootloader?
When booting from ram GDB will be the first stage to load u-boot. So it is not good for demo, because it need a pc to fire gdb.
OK, thanks for the info. For this patch, let's go with your proposal then, but please add some comments in the codes there for people to understand later.
Hi Bin
You say go with your proposal : Does that mean you agree with the following modification ? arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;And add some comments in this patch for people to understand it.
This one.
Or I still can NOT borrow prior_stage_fdt_address and shall find another way to overcome the problem after remove the two line in start.S ?
Regards, Bin
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午3:26寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 3:15 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:57寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:41 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 下午2:21寫道：
Hi Rick,
On Fri, Nov 30, 2018 at 2:05 PM Rick Chen rickchen36@gmail.com wrote:
Bin Meng bmeng.cn@gmail.com 於 2018年11月30日 週五 上午9:48寫道： > > Hi Rick, > > On Thu, Nov 29, 2018 at 6:41 PM Rick Chen rickchen36@gmail.com wrote: > > > > Bin Meng bmeng.cn@gmail.com 於 2018年11月27日 週二 下午6:07寫道： > > > > > > Hi Rick, > > > > > > On Tue, Nov 27, 2018 at 4:43 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午3:56寫道： > > > > > > > > > > On Tue, Nov 27, 2018 at 12:30 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:40寫道： > > > > > > > > > > > > > > On Tue, Nov 27, 2018 at 12:00 PM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > > > > > > > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午2:14寫道： > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 27 Nov, 2018, 11:38 AM Rick Chen <rickchen36@gmail.com wrote: > > > > > > > > >> > > > > > > > > >> Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午1:47寫道： > > > > > > > > >> > > > > > > > > > >> > On Tue, Nov 27, 2018 at 11:14 AM Anup Patel anup@brainfault.org wrote: > > > > > > > > >> > > > > > > > > > > >> > > On Tue, Nov 27, 2018 at 10:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > >> > > > > > > > > > > > >> > > > Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道： > > > > > > > > >> > > > > > > > > > > > > >> > > > > On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote: > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > Currently, the RISC-V U-Boot is saving a2 register at > > > > > > > > >> > > > > > > > > > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > > > > > > > > >> > > > > > > > > > there is no information passed by previous booting stage in a2 > > > > > > > > >> > > > > > > > > > register. > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > This patch removes redundant a2 store on DRAM base. > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > > > >> > > > > > > > > > --- > > > > > > > > >> > > > > > > > > > arch/riscv/cpu/start.S | 2 -- > > > > > > > > >> > > > > > > > > > 1 file changed, 2 deletions(-) > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > > > > > > > > >> > > > > > > > > > 704190f946..e4276e8e19 100644 > > > > > > > > >> > > > > > > > > > --- a/arch/riscv/cpu/start.S > > > > > > > > >> > > > > > > > > > +++ b/arch/riscv/cpu/start.S > > > > > > > > >> > > > > > > > > > @@ -38,8 +38,6 @@ _start: > > > > > > > > >> > > > > > > > > > mv s0, a0 > > > > > > > > >> > > > > > > > > > mv s1, a1 > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > > >> > > > > > > > > > - SREG a2, 0(t0) > > > > > > > > >> > > > > > > > > > la t0, trap_entry > > > > > > > > >> > > > > > > > > > #ifdef CONFIG_RISCV_SMODE > > > > > > > > >> > > > > > > > > > csrw stvec, t0 > > > > > > > > >> > > > > > > > > > -- > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > This is weird. I remember these two lines were already removed by > > > > > > > > >> > > > > > > > > Lukas's patch series before? Did not have time to dig out the history > > > > > > > > >> > > > > > > > > though. > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > Regards, > > > > > > > > >> > > > > > > > > Bin > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > You are correct, however I removed it again, because I did not want to break > > > > > > > > >> > > > > > > > Rick's board. He did add a commit to the last pull request that removes these > > > > > > > > >> > > > > > > > two lines and adjusts his board accordingly, but it is not in the current one. > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > Hi Likas > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > Thanks for your explanation. > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > RIck's commit as below > > > > > > > > >> > > > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > > >> > > > > > > > > > > > > >> > > > > When we run U-Boot in S-mode the BBL runs from 0x80000000 so this > > > > > > > > >> > > > > two lines corrupts BBL instructions. > > > > > > > > >> > > > > > > > > > > > > >> > > > > If this is important for some board then please have it around #ifdef. > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > Hi Anup > > > > > > > > >> > > > > > > > > > > > >> > > > In the discussion as below : > > > > > > > > >> > > > https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html > > > > > > > > >> > > > > > > > > > > > >> > > > I try to solve this issue with the aptch > > > > > > > > >> > > > [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register > > > > > > > > >> > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > > > > > > > >> > > > - li t0, CONFIG_SYS_SDRAM_BASE > > > > > > > > >> > > > - SREG a2, 0(t0) > > > > > > > > >> > > > > > > > > > > > >> > > > diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > > > > >> > > > b/board/AndesTech/ax25-ae350/ax25-ae350.c > > > > > > > > >> > > > void *board_fdt_blob_setup(void) > > > > > > > > >> > > > { > > > > > > > > >> > > > - void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; > > > > > > > > >> > > > + void **ptr = (void *)&prior_stage_fdt_address; > > > > > > > > >> > > > > > > > > > > > >> > > > in the previous pull request. > > > > > > > > >> > > > > > > > > > > > >> > > > But Bin do not agree with that I use prior_stage_fdt_address in > > > > > > > > >> > > > board_fdt_blob_setup( ) > > > > > > > > >> > > > I try to explain why I use it like that way. > > > > > > > > >> > > > Then Bin have not any reply in the following mail. > > > > > > > > >> > > > Finally I decide to drop this patch in the next pull request. > > > > > > > > >> > > > > > > > > > > > >> > > > Hi Bin > > > > > > > > >> > > > > > > > > > > > >> > > > How do you think about I recovery this patch to fix this issue ? > > > > > > > > >> > > > > > > > > > > >> > > Actually, previous booting stage can pass location of FDT stored in flash > > > > > > > > >> > > to U-Boot. U-Boot requires FDT at a DRAM location which it can modify > > > > > > > > >> > > in-place before passing on to Linux kernel so we should relocate the FDT > > > > > > > > >> > > passed by previous booting stage to some board specific DRAM location. > > > > > > > > >> > > > > > > > > > > >> > > My suggestion is as follows: > > > > > > > > >> > > > > > > > > > > >> > > Instead of SDRAM_BASE, we can have new board specific config > > > > > > > > >> > > CONFIG_RISCV_PRIOR_FDT_BASE > > > > > > > > >> > > > > > > > > > > >> > > If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by > > > > > > > > >> > > config then in start.S copy-over the FDT from location pointed by > > > > > > > > >> > > "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE. > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> Hi Anup > > > > > > > > >> > > > > > > > > >> It can not achieve dtb delivery at runtime. > > > > > > > > > > > > > > > > > > > > > > > > > > > Can you elaborate why? > > > > > > > > > > > > > > > > > > > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE is determined at compile time. > > > > > > > > I am wondering how it can be modified at run time ? > > > > > > > > > > > > > > I think you miss-understood my suggestion. I did not meant changing > > > > > > > CONFIG_RISCV_PRIOR_FDT_BASE at runtime. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am sure we can always relocate FDT passed by previous stage to some safer location and use it from there. > > > > > > > > > > > > > > > > My original patch also can achieve that dtb passed by a1 and relocated and boot. > > > > > > > > > > > > > > Great !!! > > > > > > > > > > > > > > Why not update your original patch to relocate FDT and use FDT from > > > > > > > safer location? > > > > > > > > > > > > > > > > > > > Good question. > > > > > > Above can see the question I ask for Bin : > > > > > > How do you think about I recovery this patch to fix this issue ? > > > > > > > > > > > > Before you talking about CONFIG_RISCV_PRIOR_FDT_BASE ! > > > > > > > > > > Can you explain why you need CONFIG_OF_BOARD ? > > > > > Why can you not use CONFIG_OF_PRIOR_STAGE ? > > > > > > > > > > > > > You can find the discussion as below: > > > > https://patchwork.ozlabs.org/patch/988884/ > > > > > > > > > > If I understand correctly, so far we have two scenarios to support: > > > > > > 1. U-Boot is booting directly from reset vector from ROM. This > > > canonical way to support DT is via CONFIG_OF_EMBED or > > > CONFIG_OF_SEPARATE. In such configuration, there is no any previous > > > bootloader so the concept of CONFIG_OF_PRIOR_STAGE does not apply. > > > > > > 2. U-Boot is booting from previous bootloader, and DT is passed in a1 > > > register. Such configuration we can use CONFIG_OF_PRIOR_STAGE. > > > > > > > I use the 3rd way, CONFIG_OF_BOARD. > > It can be found in README. > > And can be chosen by make menuconfig > > > > It is the most flexible way that I can implement my own board_fdt_blob_setup( ) > > which can both support boot from RAM or ROM without any configuration change. > > I will indeed use CONFIG_OF_EMBED when u-boot is in debug stage. > > > > What I don't understand is that if U-Boot is booting directly from the > reset vector, there is no a1 passed by anyone. U-Boot itself should > figure out a way to determine the DT and that's how CONFIG_OF_EMBED or > CONFIG_OF_SEPARATE is doing. Can you elaborate this configuration? >
I use CONFIG_OF_BOARD and board_fdt_blob_setup as below:
void *board_fdt_blob_setup(void) { void **ptr = (void *)CONFIG_SYS_SDRAM_BASE; if (fdt_magic(*ptr) == FDT_MAGIC) return (void *)*ptr;
return (void *)CONFIG_SYS_FDT_BASE; }
- When booting from RAM a1 can be passed by loader and reserved in CONFIG_SYS_SDRAM_BASE
temporarily.
- When booting from FLASH a1 can NOT be passed by loader and reserved in
CONFIG_SYS_SDRAM_BASE temporarily.
So dtb shall be get from CONFIG_SYS_FDT_BASE (flash base) whichthe dtb shall be pre-burned in this flash space.. Or CONFIG_SYS_FDT_BASE maybe a memory map space (NOT RAM or ROM) provided by HW.
That is why I mean using CONFIG_OF_BOARD can support both boot from ram or rom with one configuration.
Thanks for the info. So with the latest info, I now understand you wanted to use CONFIG_OF_BOARD to support both configurations. That makes sense to me, however I still wanted to point out the canonical way to support booting from ROM is via CONFIG_OF_SEPARATE or CONFIG_OF_EMBED. Since your board can support booting from reset vector directly, what's the need to support the booting from RAM configuration?
I will develop u-boot via booting from ram. It will easy for debugging. After develop complete. It will be good for demo via booting from flash. With one configuration, it can decrease to make mistake with wrong configuration between 2 configuration for ram or rom individually.
In that configuration, what is the first stage
bootloader?
When booting from ram GDB will be the first stage to load u-boot. So it is not good for demo, because it need a pc to fire gdb.
OK, thanks for the info. For this patch, let's go with your proposal then, but please add some comments in the codes there for people to understand later.
Hi Bin
You say go with your proposal : Does that mean you agree with the following modification ? arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;And add some comments in this patch for people to understand it.
This one.
Hi Bin
Thanks for your opinion. :)
Rick
Or I still can NOT borrow prior_stage_fdt_address and shall find another way to overcome the problem after remove the two line in start.S ?
Regards, Bin
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
Hi Anup
You said Your patchset based upon git://git.denx.de/u-boot-riscv.git
Why you announce this problem in [PATCH v5 4/4] riscv: Remove redundant a2 store on DRAM base in start.S Why you do not find this proble in v1, v2, v3, v4 ?
Rick
If this is important for some board then please have it around #ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Actually, previous booting stage can pass location of FDT stored in flash to U-Boot. U-Boot requires FDT at a DRAM location which it can modify in-place before passing on to Linux kernel so we should relocate the FDT passed by previous booting stage to some board specific DRAM location.
My suggestion is as follows:
Instead of SDRAM_BASE, we can have new board specific config CONFIG_RISCV_PRIOR_FDT_BASE
If CONFIG_RISCV_PRIOR_FDT_BASE is defined/selected by config then in start.S copy-over the FDT from location pointed by "a1" register to location pointed by CONFIG_RISCV_PRIOR_FDT_BASE.
Hi Anup
It can not achieve dtb delivery at runtime.
Rick
I think you can copy-over FDT in C code too. You don't need to do in start.S.
In your board_fdt_blob_setup(), you can safely do: void **ptr = (void *)CONFIG_RISCV_PRIOR_FDT_BASE;
Regards, Anup
On Tue, Nov 27, 2018 at 12:14 PM Rick Chen rickchen36@gmail.com wrote:
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
Hi Anup
You said Your patchset based upon git://git.denx.de/u-boot-riscv.git
Why you announce this problem in [PATCH v5 4/4] riscv: Remove redundant a2 store on DRAM base in start.S Why you do not find this proble in v1, v2, v3, v4 ?
I had this change locally. I thought some would definitely remove this lines but did not see that happening so I send patch to remove these lines myself.
Regards, Anup
Hi Rick,
On Tue, Nov 27, 2018 at 1:20 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 上午11:28寫道：
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote:
> Currently, the RISC-V U-Boot is saving a2 register at > CONFIG_SYS_DRAM_BASE in start.S which does not make sense because > there is no information passed by previous booting stage in a2 > register. > > This patch removes redundant a2 store on DRAM base. > > Signed-off-by: Anup Patel anup@brainfault.org > --- > arch/riscv/cpu/start.S | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > 704190f946..e4276e8e19 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -38,8 +38,6 @@ _start: > mv s0, a0 > mv s1, a1 > > - li t0, CONFIG_SYS_SDRAM_BASE > - SREG a2, 0(t0) > la t0, trap_entry > #ifdef CONFIG_RISCV_SMODE > csrw stvec, t0 > --
This is weird. I remember these two lines were already removed by Lukas's patch series before? Did not have time to dig out the history though.
Regards, Bin
You are correct, however I removed it again, because I did not want to break Rick's board. He did add a commit to the last pull request that removes these two lines and adjusts his board accordingly, but it is not in the current one.
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
Hi Anup
In the discussion as below : https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
I try to solve this issue with the aptch [PATCH] riscv: ax25-ae350: Pass dtb address to u-boot with a1 register diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0)diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c void *board_fdt_blob_setup(void) {
void **ptr = (void *)CONFIG_SYS_SDRAM_BASE;
void **ptr = (void *)&prior_stage_fdt_address;in the previous pull request.
But Bin do not agree with that I use prior_stage_fdt_address in board_fdt_blob_setup( ) I try to explain why I use it like that way. Then Bin have not any reply in the following mail. Finally I decide to drop this patch in the next pull request.
Hi Bin
How do you think about I recovery this patch to fix this issue ?
Do you mean removing the two lines in start.S? Yes, let's do this. However for the ax25-ae350 board, in order to support booting from NOR, we should introduce CONFIG_OF_EMBED or CONFIG_OF_SEPARATE, and leave prior_stage_fdt_address support only for cases which U-Boot is loaded by previous stage bootloaders.
Regards, Bin
On Tue, Nov 27, 2018 at 11:28 AM Anup Patel anup@brainfault.org wrote:
On Tue, Nov 27, 2018 at 8:50 AM Rick Chen rickchen36@gmail.com wrote:
Currently, the RISC-V U-Boot is saving a2 register at CONFIG_SYS_DRAM_BASE in start.S which does not make sense because there is no information passed by previous booting stage in a2 register.
This patch removes redundant a2 store on DRAM base.
Signed-off-by: Anup Patel anup@brainfault.org
arch/riscv/cpu/start.S | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 704190f946..e4276e8e19 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -38,8 +38,6 @@ _start: mv s0, a0 mv s1, a1
li t0, CONFIG_SYS_SDRAM_BASESREG a2, 0(t0) la t0, trap_entry#ifdef CONFIG_RISCV_SMODE csrw stvec, t0 --
This is weird. I remember these two lines were already removed by Lukas's patch series before? Did not have time to dig out the history though.
Regards, Bin
You are correct, however I removed it again, because I did not want to break Rick's board. He did add a commit to the last pull request that removes these two lines and adjusts his board accordingly, but it is not in the current one.
Hi Likas
Thanks for your explanation.
RIck's commit as below https://www.mail-archive.com/u-boot@lists.denx.de/msg305880.html
When we run U-Boot in S-mode the BBL runs from 0x80000000 so this two lines corrupts BBL instructions.
If this is important for some board then please have it around #ifdef.
Please don't do any #ifdef here. That codes should be removed, and we should figure out a way that does not break the ax25-ae350 board.
My apologies for the noise.
Regards, Bin
participants (4)
-
Alexander Graf -
Anup Patel -
Bin Meng -
Rick Chen