diff mbox series

[U-Boot,22/30] riscv: remove unused labels in start.S

Message ID 20181019220743.15020-23-lukas.auer@aisec.fraunhofer.de
State Superseded
Delegated to: Andes
Headers show
Series General fixes / cleanup for RISC-V and improvements to qemu-riscv | expand

Commit Message

Lukas Auer Oct. 19, 2018, 10:07 p.m. UTC
The labels nmi_vector, trap_vector and handle_reset in start.S are not
used for RISC-V. Remove them.

While we are here, also remove the code from the beginning of start.S,
which stores the contents of a2 to memory. Only registers a0 and a1
contain information from the previous boot stage. There is therefore no
reason for saving a2.

Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
---

 arch/riscv/cpu/start.S | 11 -----------
 1 file changed, 11 deletions(-)

Comments

Bin Meng Oct. 22, 2018, 9:19 a.m. UTC | #1
On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> The labels nmi_vector, trap_vector and handle_reset in start.S are not
> used for RISC-V. Remove them.
>
> While we are here, also remove the code from the beginning of start.S,
> which stores the contents of a2 to memory. Only registers a0 and a1
> contain information from the previous boot stage. There is therefore no
> reason for saving a2.
>
> Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> ---
>
>  arch/riscv/cpu/start.S | 11 -----------
>  1 file changed, 11 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Rick Chen Oct. 24, 2018, 2:38 a.m. UTC | #2
> > > The labels nmi_vector, trap_vector and handle_reset in start.S are not
> > > used for RISC-V. Remove them.
> > >

Hi Lukas

Agree with the above part.

> > > While we are here, also remove the code from the beginning of start.S,
> > > which stores the contents of a2 to memory. Only registers a0 and a1
> > > contain information from the previous boot stage. There is therefore
> > > no reason for saving a2.

NOT agree with this part.
Saving a2 is trying to support dynamically dtb pass at runtime.

You can see it in the following commit :

commit d58717e42559189a226ea800173147399c8edef9
Author: Rick Chen <rick@andestech.com>
Date:   Thu Mar 29 10:08:33 2018 +0800

    riscv: ae250: Support DT provided by the board at runtime

B.R
Rick

> > >
> > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > > ---
> > >
> > >  arch/riscv/cpu/start.S | 11 -----------
> > >  1 file changed, 11 deletions(-)
> > >
> >
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Bin Meng Oct. 24, 2018, 3:34 a.m. UTC | #3
Hi Rich,

On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <rickchen36@gmail.com> wrote:
>
> > > > The labels nmi_vector, trap_vector and handle_reset in start.S are not
> > > > used for RISC-V. Remove them.
> > > >
>
> Hi Lukas
>
> Agree with the above part.
>
> > > > While we are here, also remove the code from the beginning of start.S,
> > > > which stores the contents of a2 to memory. Only registers a0 and a1
> > > > contain information from the previous boot stage. There is therefore
> > > > no reason for saving a2.
>
> NOT agree with this part.
> Saving a2 is trying to support dynamically dtb pass at runtime.
>

Could you please describe in more details on what the use case is here?

> You can see it in the following commit :
>
> commit d58717e42559189a226ea800173147399c8edef9
> Author: Rick Chen <rick@andestech.com>
> Date:   Thu Mar 29 10:08:33 2018 +0800
>
>     riscv: ae250: Support DT provided by the board at runtime
>

Regards,
Bin
Rick Chen Oct. 24, 2018, 5:20 a.m. UTC | #4
Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
>
> Hi Rich,
>
> On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > > > > The labels nmi_vector, trap_vector and handle_reset in start.S are not
> > > > > used for RISC-V. Remove them.
> > > > >
> >
> > Hi Lukas
> >
> > Agree with the above part.
> >
> > > > > While we are here, also remove the code from the beginning of start.S,
> > > > > which stores the contents of a2 to memory. Only registers a0 and a1
> > > > > contain information from the previous boot stage. There is therefore
> > > > > no reason for saving a2.
> >
> > NOT agree with this part.
> > Saving a2 is trying to support dynamically dtb pass at runtime.
> >
>
> Could you please describe in more details on what the use case is here?
>

Hi Bin and Lukas

After I study [PATCH 24/30] riscv: save hart ID and device tree passed
by prior boot stage.
I found it is the same thing.
I will accepted this patch.
So this patch is

Reviewed-by: Rick Chen <rick@andestech.com>

But I shall send a patch to fix ax25-ae350 to work as well later.
And it will be as below :

ax5-ae350.c
void *board_fdt_blob_setup(void)
{
void **ptr = (void *)&prior_stage_fdt_address;
if (fdt_magic(*ptr) == FDT_MAGIC)
return (void *)*ptr;

return (void *)CONFIG_SYS_FDT_BASE;
}


> > You can see it in the following commit :
> >
> > commit d58717e42559189a226ea800173147399c8edef9
> > Author: Rick Chen <rick@andestech.com>
> > Date:   Thu Mar 29 10:08:33 2018 +0800
> >
> >     riscv: ae250: Support DT provided by the board at runtime
> >
>
> Regards,
> Bin
Rick Chen Oct. 24, 2018, 5:47 a.m. UTC | #5
Rick Chen <rickchen36@gmail.com> 於 2018年10月24日 週三 下午1:20寫道:
>
> Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
> >
> > Hi Rich,
> >
> > On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <rickchen36@gmail.com> wrote:
> > >
> > > > > > The labels nmi_vector, trap_vector and handle_reset in start.S are not
> > > > > > used for RISC-V. Remove them.
> > > > > >
> > >
> > > Hi Lukas
> > >
> > > Agree with the above part.
> > >
> > > > > > While we are here, also remove the code from the beginning of start.S,
> > > > > > which stores the contents of a2 to memory. Only registers a0 and a1
> > > > > > contain information from the previous boot stage. There is therefore
> > > > > > no reason for saving a2.
> > >
> > > NOT agree with this part.
> > > Saving a2 is trying to support dynamically dtb pass at runtime.
> > >
> >
> > Could you please describe in more details on what the use case is here?
> >
>
> Hi Bin and Lukas
>
> After I study [PATCH 24/30] riscv: save hart ID and device tree passed
> by prior boot stage.
> I found it is the same thing.
> I will accepted this patch.
> So this patch is
>
> Reviewed-by: Rick Chen <rick@andestech.com>
>
> But I shall send a patch to fix ax25-ae350 to work as well later.
> And it will be as below :
>
> ax5-ae350.c
> void *board_fdt_blob_setup(void)
> {
> void **ptr = (void *)&prior_stage_fdt_address;
> if (fdt_magic(*ptr) == FDT_MAGIC)
> return (void *)*ptr;
>
> return (void *)CONFIG_SYS_FDT_BASE;
> }
>

Sorry!

I shall send a fixed patch as below:
riscv: ax25-ae350: use device tree passed by prior boot stage

--- a/configs/ax25-ae350_defconfig
+++ b/configs/ax25-ae350_defconfig
@@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_BOOTP_PREFER_SERVERIP=y
 CONFIG_CMD_CACHE=y
-CONFIG_OF_BOARD=y
+CONFIG_OF_PRIOR_STAGE=y

+++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
@@ -64,15 +64,6 @@ ulong board_flash_get_legacy(ulong base, int
banknum, flash_info_t *info)
        return 0;
 }

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

Rick

>
> > > You can see it in the following commit :
> > >
> > > commit d58717e42559189a226ea800173147399c8edef9
> > > Author: Rick Chen <rick@andestech.com>
> > > Date:   Thu Mar 29 10:08:33 2018 +0800
> > >
> > >     riscv: ae250: Support DT provided by the board at runtime
> > >
> >
> > Regards,
> > Bin
Lukas Auer Oct. 24, 2018, 2:13 p.m. UTC | #6
Hi Rick,

On Wed, 2018-10-24 at 13:47 +0800, Rick Chen wrote:
> Rick Chen <rickchen36@gmail.com> 於 2018年10月24日 週三 下午1:20寫道:
> > 
> > Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
> > > 
> > > Hi Rich,
> > > 
> > > On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <rickchen36@gmail.com>
> > > wrote:
> > > > 
> > > > > > > The labels nmi_vector, trap_vector and handle_reset in
> > > > > > > start.S are not
> > > > > > > used for RISC-V. Remove them.
> > > > > > > 
> > > > 
> > > > Hi Lukas
> > > > 
> > > > Agree with the above part.
> > > > 
> > > > > > > While we are here, also remove the code from the
> > > > > > > beginning of start.S,
> > > > > > > which stores the contents of a2 to memory. Only registers
> > > > > > > a0 and a1
> > > > > > > contain information from the previous boot stage. There
> > > > > > > is therefore
> > > > > > > no reason for saving a2.
> > > > 
> > > > NOT agree with this part.
> > > > Saving a2 is trying to support dynamically dtb pass at runtime.
> > > > 
> > > 
> > > Could you please describe in more details on what the use case is
> > > here?
> > > 
> > 
> > Hi Bin and Lukas
> > 
> > After I study [PATCH 24/30] riscv: save hart ID and device tree
> > passed
> > by prior boot stage.
> > I found it is the same thing.
> > I will accepted this patch.
> > So this patch is
> > 
> > Reviewed-by: Rick Chen <rick@andestech.com>
> > 
> > But I shall send a patch to fix ax25-ae350 to work as well later.
> > And it will be as below :
> > 
> > ax5-ae350.c
> > void *board_fdt_blob_setup(void)
> > {
> > void **ptr = (void *)&prior_stage_fdt_address;
> > if (fdt_magic(*ptr) == FDT_MAGIC)
> > return (void *)*ptr;
> > 
> > return (void *)CONFIG_SYS_FDT_BASE;
> > }
> > 
> 
> Sorry!
> 
> I shall send a fixed patch as below:
> riscv: ax25-ae350: use device tree passed by prior boot stage
> 
> --- a/configs/ax25-ae350_defconfig
> +++ b/configs/ax25-ae350_defconfig
> @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_BOOTP_PREFER_SERVERIP=y
>  CONFIG_CMD_CACHE=y
> -CONFIG_OF_BOARD=y
> +CONFIG_OF_PRIOR_STAGE=y
> 
> +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
> @@ -64,15 +64,6 @@ ulong board_flash_get_legacy(ulong base, int
> banknum, flash_info_t *info)
>         return 0;
>  }
> 
> -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;
> -}
> -
> 
> Rick
> 

Sorry, I have missed your commit. Just to clarify, does the ax25 pass
the device tree in a2 instead of a1? In that case, my patch must be
changed since it assumes the device tree to be in a1.
Related to this, can I assume that a0 stores the hart ID?

If you want, I can add this patch to my series.

Thanks,
Lukas

> > 
> > > > You can see it in the following commit :
> > > > 
> > > > commit d58717e42559189a226ea800173147399c8edef9
> > > > Author: Rick Chen <rick@andestech.com>
> > > > Date:   Thu Mar 29 10:08:33 2018 +0800
> > > > 
> > > >     riscv: ae250: Support DT provided by the board at runtime
> > > > 
> > > 
> > > Regards,
> > > Bin
Rick Chen Oct. 25, 2018, 1:16 a.m. UTC | #7
Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年10月24日 週三 下午10:14寫道:
>
> Hi Rick,
>
> On Wed, 2018-10-24 at 13:47 +0800, Rick Chen wrote:
> > Rick Chen <rickchen36@gmail.com> 於 2018年10月24日 週三 下午1:20寫道:
> > >
> > > Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
> > > >
> > > > Hi Rich,
> > > >
> > > > On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <rickchen36@gmail.com>
> > > > wrote:
> > > > >
> > > > > > > > The labels nmi_vector, trap_vector and handle_reset in
> > > > > > > > start.S are not
> > > > > > > > used for RISC-V. Remove them.
> > > > > > > >
> > > > >
> > > > > Hi Lukas
> > > > >
> > > > > Agree with the above part.
> > > > >
> > > > > > > > While we are here, also remove the code from the
> > > > > > > > beginning of start.S,
> > > > > > > > which stores the contents of a2 to memory. Only registers
> > > > > > > > a0 and a1
> > > > > > > > contain information from the previous boot stage. There
> > > > > > > > is therefore
> > > > > > > > no reason for saving a2.
> > > > >
> > > > > NOT agree with this part.
> > > > > Saving a2 is trying to support dynamically dtb pass at runtime.
> > > > >
> > > >
> > > > Could you please describe in more details on what the use case is
> > > > here?
> > > >
> > >
> > > Hi Bin and Lukas
> > >
> > > After I study [PATCH 24/30] riscv: save hart ID and device tree
> > > passed
> > > by prior boot stage.
> > > I found it is the same thing.
> > > I will accepted this patch.
> > > So this patch is
> > >
> > > Reviewed-by: Rick Chen <rick@andestech.com>
> > >
> > > But I shall send a patch to fix ax25-ae350 to work as well later.
> > > And it will be as below :
> > >
> > > ax5-ae350.c
> > > void *board_fdt_blob_setup(void)
> > > {
> > > void **ptr = (void *)&prior_stage_fdt_address;
> > > if (fdt_magic(*ptr) == FDT_MAGIC)
> > > return (void *)*ptr;
> > >
> > > return (void *)CONFIG_SYS_FDT_BASE;
> > > }
> > >
> >
> > Sorry!
> >
> > I shall send a fixed patch as below:
> > riscv: ax25-ae350: use device tree passed by prior boot stage
> >
> > --- a/configs/ax25-ae350_defconfig
> > +++ b/configs/ax25-ae350_defconfig
> > @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
> >  # CONFIG_CMD_SETEXPR is not set
> >  CONFIG_BOOTP_PREFER_SERVERIP=y
> >  CONFIG_CMD_CACHE=y
> > -CONFIG_OF_BOARD=y
> > +CONFIG_OF_PRIOR_STAGE=y
> >
> > +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
> > @@ -64,15 +64,6 @@ ulong board_flash_get_legacy(ulong base, int
> > banknum, flash_info_t *info)
> >         return 0;
> >  }
> >
> > -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;
> > -}
> > -
> >
> > Rick
> >
>
> Sorry, I have missed your commit. Just to clarify, does the ax25 pass
> the device tree in a2 instead of a1? In that case, my patch must be
> changed since it assumes the device tree to be in a1.
> Related to this, can I assume that a0 stores the hart ID?
>
> If you want, I can add this patch to my series.
>

Hi Lukas

Your patch is correct.
I will send out a patch to fix the mismatch in ax25-ae350 later.
And maybe you can put this patch into your v2 series.

Thanks

Rick

> Thanks,
> Lukas
>
> > >
> > > > > You can see it in the following commit :
> > > > >
> > > > > commit d58717e42559189a226ea800173147399c8edef9
> > > > > Author: Rick Chen <rick@andestech.com>
> > > > > Date:   Thu Mar 29 10:08:33 2018 +0800
> > > > >
> > > > >     riscv: ae250: Support DT provided by the board at runtime
> > > > >
> > > >
> > > > Regards,
> > > > Bin
Lukas Auer Oct. 25, 2018, 3:56 p.m. UTC | #8
Hi Rick,

On Thu, 2018-10-25 at 09:16 +0800, Rick Chen wrote:
> Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年10月24日 週三
> 下午10:14寫道:
> > 
> > Hi Rick,
> > 
> > On Wed, 2018-10-24 at 13:47 +0800, Rick Chen wrote:
> > > Rick Chen <rickchen36@gmail.com> 於 2018年10月24日 週三 下午1:20寫道:
> > > > 
> > > > Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
> > > > > 
> > > > > Hi Rich,
> > > > > 
> > > > > On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <
> > > > > rickchen36@gmail.com>
> > > > > wrote:
> > > > > > 
> > > > > > > > > The labels nmi_vector, trap_vector and handle_reset
> > > > > > > > > in
> > > > > > > > > start.S are not
> > > > > > > > > used for RISC-V. Remove them.
> > > > > > > > > 
> > > > > > 
> > > > > > Hi Lukas
> > > > > > 
> > > > > > Agree with the above part.
> > > > > > 
> > > > > > > > > While we are here, also remove the code from the
> > > > > > > > > beginning of start.S,
> > > > > > > > > which stores the contents of a2 to memory. Only
> > > > > > > > > registers
> > > > > > > > > a0 and a1
> > > > > > > > > contain information from the previous boot stage.
> > > > > > > > > There
> > > > > > > > > is therefore
> > > > > > > > > no reason for saving a2.
> > > > > > 
> > > > > > NOT agree with this part.
> > > > > > Saving a2 is trying to support dynamically dtb pass at
> > > > > > runtime.
> > > > > > 
> > > > > 
> > > > > Could you please describe in more details on what the use
> > > > > case is
> > > > > here?
> > > > > 
> > > > 
> > > > Hi Bin and Lukas
> > > > 
> > > > After I study [PATCH 24/30] riscv: save hart ID and device tree
> > > > passed
> > > > by prior boot stage.
> > > > I found it is the same thing.
> > > > I will accepted this patch.
> > > > So this patch is
> > > > 
> > > > Reviewed-by: Rick Chen <rick@andestech.com>
> > > > 
> > > > But I shall send a patch to fix ax25-ae350 to work as well
> > > > later.
> > > > And it will be as below :
> > > > 
> > > > ax5-ae350.c
> > > > void *board_fdt_blob_setup(void)
> > > > {
> > > > void **ptr = (void *)&prior_stage_fdt_address;
> > > > if (fdt_magic(*ptr) == FDT_MAGIC)
> > > > return (void *)*ptr;
> > > > 
> > > > return (void *)CONFIG_SYS_FDT_BASE;
> > > > }
> > > > 
> > > 
> > > Sorry!
> > > 
> > > I shall send a fixed patch as below:
> > > riscv: ax25-ae350: use device tree passed by prior boot stage
> > > 
> > > --- a/configs/ax25-ae350_defconfig
> > > +++ b/configs/ax25-ae350_defconfig
> > > @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
> > >  # CONFIG_CMD_SETEXPR is not set
> > >  CONFIG_BOOTP_PREFER_SERVERIP=y
> > >  CONFIG_CMD_CACHE=y
> > > -CONFIG_OF_BOARD=y
> > > +CONFIG_OF_PRIOR_STAGE=y
> > > 
> > > +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
> > > @@ -64,15 +64,6 @@ ulong board_flash_get_legacy(ulong base, int
> > > banknum, flash_info_t *info)
> > >         return 0;
> > >  }
> > > 
> > > -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;
> > > -}
> > > -
> > > 
> > > Rick
> > > 
> > 
> > Sorry, I have missed your commit. Just to clarify, does the ax25
> > pass
> > the device tree in a2 instead of a1? In that case, my patch must be
> > changed since it assumes the device tree to be in a1.
> > Related to this, can I assume that a0 stores the hart ID?
> > 
> > If you want, I can add this patch to my series.
> > 
> 
> Hi Lukas
> 
> Your patch is correct.
> I will send out a patch to fix the mismatch in ax25-ae350 later.
> And maybe you can put this patch into your v2 series.
> 
> Thanks
> 
> Rick
> 

Ok. Yes, I will pick up your patch in my v2.

Thanks,
Lukas

> > Thanks,
> > Lukas
> > 
> > > > 
> > > > > > You can see it in the following commit :
> > > > > > 
> > > > > > commit d58717e42559189a226ea800173147399c8edef9
> > > > > > Author: Rick Chen <rick@andestech.com>
> > > > > > Date:   Thu Mar 29 10:08:33 2018 +0800
> > > > > > 
> > > > > >     riscv: ae250: Support DT provided by the board at
> > > > > > runtime
> > > > > > 
> > > > > 
> > > > > Regards,
> > > > > Bin
Lukas Auer Oct. 29, 2018, 4:43 p.m. UTC | #9
Hi Rick,

On Thu, 2018-10-25 at 15:56 +0000, Auer, Lukas wrote:
> Hi Rick,
> 
> On Thu, 2018-10-25 at 09:16 +0800, Rick Chen wrote:
> > Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年10月24日 週三
> > 下午10:14寫道:
> > > 
> > > Hi Rick,
> > > 
> > > On Wed, 2018-10-24 at 13:47 +0800, Rick Chen wrote:
> > > > Rick Chen <rickchen36@gmail.com> 於 2018年10月24日 週三 下午1:20寫道:
> > > > > 
> > > > > Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
> > > > > > 
> > > > > > Hi Rich,
> > > > > > 
> > > > > > On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <
> > > > > > rickchen36@gmail.com>
> > > > > > wrote:
> > > > > > > 
> > > > > > > > > > The labels nmi_vector, trap_vector and handle_reset
> > > > > > > > > > in
> > > > > > > > > > start.S are not
> > > > > > > > > > used for RISC-V. Remove them.
> > > > > > > > > > 
> > > > > > > 
> > > > > > > Hi Lukas
> > > > > > > 
> > > > > > > Agree with the above part.
> > > > > > > 
> > > > > > > > > > While we are here, also remove the code from the
> > > > > > > > > > beginning of start.S,
> > > > > > > > > > which stores the contents of a2 to memory. Only
> > > > > > > > > > registers
> > > > > > > > > > a0 and a1
> > > > > > > > > > contain information from the previous boot stage.
> > > > > > > > > > There
> > > > > > > > > > is therefore
> > > > > > > > > > no reason for saving a2.
> > > > > > > 
> > > > > > > NOT agree with this part.
> > > > > > > Saving a2 is trying to support dynamically dtb pass at
> > > > > > > runtime.
> > > > > > > 
> > > > > > 
> > > > > > Could you please describe in more details on what the use
> > > > > > case is
> > > > > > here?
> > > > > > 
> > > > > 
> > > > > Hi Bin and Lukas
> > > > > 
> > > > > After I study [PATCH 24/30] riscv: save hart ID and device
> > > > > tree
> > > > > passed
> > > > > by prior boot stage.
> > > > > I found it is the same thing.
> > > > > I will accepted this patch.
> > > > > So this patch is
> > > > > 
> > > > > Reviewed-by: Rick Chen <rick@andestech.com>
> > > > > 
> > > > > But I shall send a patch to fix ax25-ae350 to work as well
> > > > > later.
> > > > > And it will be as below :
> > > > > 
> > > > > ax5-ae350.c
> > > > > void *board_fdt_blob_setup(void)
> > > > > {
> > > > > void **ptr = (void *)&prior_stage_fdt_address;
> > > > > if (fdt_magic(*ptr) == FDT_MAGIC)
> > > > > return (void *)*ptr;
> > > > > 
> > > > > return (void *)CONFIG_SYS_FDT_BASE;
> > > > > }
> > > > > 
> > > > 
> > > > Sorry!
> > > > 
> > > > I shall send a fixed patch as below:
> > > > riscv: ax25-ae350: use device tree passed by prior boot stage
> > > > 
> > > > --- a/configs/ax25-ae350_defconfig
> > > > +++ b/configs/ax25-ae350_defconfig
> > > > @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
> > > >  # CONFIG_CMD_SETEXPR is not set
> > > >  CONFIG_BOOTP_PREFER_SERVERIP=y
> > > >  CONFIG_CMD_CACHE=y
> > > > -CONFIG_OF_BOARD=y
> > > > +CONFIG_OF_PRIOR_STAGE=y
> > > > 
> > > > +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
> > > > @@ -64,15 +64,6 @@ ulong board_flash_get_legacy(ulong base, int
> > > > banknum, flash_info_t *info)
> > > >         return 0;
> > > >  }
> > > > 
> > > > -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;
> > > > -}
> > > > -
> > > > 
> > > > Rick
> > > > 
> > > 
> > > Sorry, I have missed your commit. Just to clarify, does the ax25
> > > pass
> > > the device tree in a2 instead of a1? In that case, my patch must
> > > be
> > > changed since it assumes the device tree to be in a1.
> > > Related to this, can I assume that a0 stores the hart ID?
> > > 
> > > If you want, I can add this patch to my series.
> > > 
> > 
> > Hi Lukas
> > 
> > Your patch is correct.
> > I will send out a patch to fix the mismatch in ax25-ae350 later.
> > And maybe you can put this patch into your v2 series.
> > 
> > Thanks
> > 
> > Rick
> > 
> 
> Ok. Yes, I will pick up your patch in my v2.
> 

Do you want me to wait with my patch series until it is clear how ax25-
ae350 handles the device tree? Otherwise, I will drop the part of this
patch that removes the code storing a2 to avoid breaking your board and
send v2 of my series.

Thanks,
Lukas
Rick Chen Oct. 30, 2018, 1:49 a.m. UTC | #10
Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年10月30日 週二 上午12:43寫道:
>
> Hi Rick,
>
> On Thu, 2018-10-25 at 15:56 +0000, Auer, Lukas wrote:
> > Hi Rick,
> >
> > On Thu, 2018-10-25 at 09:16 +0800, Rick Chen wrote:
> > > Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年10月24日 週三
> > > 下午10:14寫道:
> > > >
> > > > Hi Rick,
> > > >
> > > > On Wed, 2018-10-24 at 13:47 +0800, Rick Chen wrote:
> > > > > Rick Chen <rickchen36@gmail.com> 於 2018年10月24日 週三 下午1:20寫道:
> > > > > >
> > > > > > Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
> > > > > > >
> > > > > > > Hi Rich,
> > > > > > >
> > > > > > > On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <
> > > > > > > rickchen36@gmail.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > > > The labels nmi_vector, trap_vector and handle_reset
> > > > > > > > > > > in
> > > > > > > > > > > start.S are not
> > > > > > > > > > > used for RISC-V. Remove them.
> > > > > > > > > > >
> > > > > > > >
> > > > > > > > Hi Lukas
> > > > > > > >
> > > > > > > > Agree with the above part.
> > > > > > > >
> > > > > > > > > > > While we are here, also remove the code from the
> > > > > > > > > > > beginning of start.S,
> > > > > > > > > > > which stores the contents of a2 to memory. Only
> > > > > > > > > > > registers
> > > > > > > > > > > a0 and a1
> > > > > > > > > > > contain information from the previous boot stage.
> > > > > > > > > > > There
> > > > > > > > > > > is therefore
> > > > > > > > > > > no reason for saving a2.
> > > > > > > >
> > > > > > > > NOT agree with this part.
> > > > > > > > Saving a2 is trying to support dynamically dtb pass at
> > > > > > > > runtime.
> > > > > > > >
> > > > > > >
> > > > > > > Could you please describe in more details on what the use
> > > > > > > case is
> > > > > > > here?
> > > > > > >
> > > > > >
> > > > > > Hi Bin and Lukas
> > > > > >
> > > > > > After I study [PATCH 24/30] riscv: save hart ID and device
> > > > > > tree
> > > > > > passed
> > > > > > by prior boot stage.
> > > > > > I found it is the same thing.
> > > > > > I will accepted this patch.
> > > > > > So this patch is
> > > > > >
> > > > > > Reviewed-by: Rick Chen <rick@andestech.com>
> > > > > >
> > > > > > But I shall send a patch to fix ax25-ae350 to work as well
> > > > > > later.
> > > > > > And it will be as below :
> > > > > >
> > > > > > ax5-ae350.c
> > > > > > void *board_fdt_blob_setup(void)
> > > > > > {
> > > > > > void **ptr = (void *)&prior_stage_fdt_address;
> > > > > > if (fdt_magic(*ptr) == FDT_MAGIC)
> > > > > > return (void *)*ptr;
> > > > > >
> > > > > > return (void *)CONFIG_SYS_FDT_BASE;
> > > > > > }
> > > > > >
> > > > >
> > > > > Sorry!
> > > > >
> > > > > I shall send a fixed patch as below:
> > > > > riscv: ax25-ae350: use device tree passed by prior boot stage
> > > > >
> > > > > --- a/configs/ax25-ae350_defconfig
> > > > > +++ b/configs/ax25-ae350_defconfig
> > > > > @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
> > > > >  # CONFIG_CMD_SETEXPR is not set
> > > > >  CONFIG_BOOTP_PREFER_SERVERIP=y
> > > > >  CONFIG_CMD_CACHE=y
> > > > > -CONFIG_OF_BOARD=y
> > > > > +CONFIG_OF_PRIOR_STAGE=y
> > > > >
> > > > > +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
> > > > > @@ -64,15 +64,6 @@ ulong board_flash_get_legacy(ulong base, int
> > > > > banknum, flash_info_t *info)
> > > > >         return 0;
> > > > >  }
> > > > >
> > > > > -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;
> > > > > -}
> > > > > -
> > > > >
> > > > > Rick
> > > > >
> > > >
> > > > Sorry, I have missed your commit. Just to clarify, does the ax25
> > > > pass
> > > > the device tree in a2 instead of a1? In that case, my patch must
> > > > be
> > > > changed since it assumes the device tree to be in a1.
> > > > Related to this, can I assume that a0 stores the hart ID?
> > > >
> > > > If you want, I can add this patch to my series.
> > > >
> > >
> > > Hi Lukas
> > >
> > > Your patch is correct.
> > > I will send out a patch to fix the mismatch in ax25-ae350 later.
> > > And maybe you can put this patch into your v2 series.
> > >
> > > Thanks
> > >
> > > Rick
> > >
> >
> > Ok. Yes, I will pick up your patch in my v2.
> >
>
> Do you want me to wait with my patch series until it is clear how ax25-
> ae350 handles the device tree? Otherwise, I will drop the part of this
> patch that removes the code storing a2 to avoid breaking your board and
> send v2 of my series.
>

Hi Likas

It is ok to keep this patch into v2
[PATCH 22/30] riscv: remove unused labels in start.S

I will send a patch to fix it after your series.

Rick

> Thanks,
> Lukas
Lukas Auer Oct. 30, 2018, 12:51 p.m. UTC | #11
Hi Rick,

On Tue, 2018-10-30 at 09:49 +0800, Rick Chen wrote:
> Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年10月30日 週二
> 上午12:43寫道:
> > 
> > Hi Rick,
> > 
> > On Thu, 2018-10-25 at 15:56 +0000, Auer, Lukas wrote:
> > > Hi Rick,
> > > 
> > > On Thu, 2018-10-25 at 09:16 +0800, Rick Chen wrote:
> > > > Auer, Lukas <lukas.auer@aisec.fraunhofer.de> 於 2018年10月24日 週三
> > > > 下午10:14寫道:
> > > > > 
> > > > > Hi Rick,
> > > > > 
> > > > > On Wed, 2018-10-24 at 13:47 +0800, Rick Chen wrote:
> > > > > > Rick Chen <rickchen36@gmail.com> 於 2018年10月24日 週三 下午1:20寫道:
> > > > > > > 
> > > > > > > Bin Meng <bmeng.cn@gmail.com> 於 2018年10月24日 週三 上午11:34寫道:
> > > > > > > > 
> > > > > > > > Hi Rich,
> > > > > > > > 
> > > > > > > > On Wed, Oct 24, 2018 at 10:37 AM Rick Chen <
> > > > > > > > rickchen36@gmail.com>
> > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > > > > The labels nmi_vector, trap_vector and
> > > > > > > > > > > > handle_reset
> > > > > > > > > > > > in
> > > > > > > > > > > > start.S are not
> > > > > > > > > > > > used for RISC-V. Remove them.
> > > > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Hi Lukas
> > > > > > > > > 
> > > > > > > > > Agree with the above part.
> > > > > > > > > 
> > > > > > > > > > > > While we are here, also remove the code from
> > > > > > > > > > > > the
> > > > > > > > > > > > beginning of start.S,
> > > > > > > > > > > > which stores the contents of a2 to memory. Only
> > > > > > > > > > > > registers
> > > > > > > > > > > > a0 and a1
> > > > > > > > > > > > contain information from the previous boot
> > > > > > > > > > > > stage.
> > > > > > > > > > > > There
> > > > > > > > > > > > is therefore
> > > > > > > > > > > > no reason for saving a2.
> > > > > > > > > 
> > > > > > > > > NOT agree with this part.
> > > > > > > > > Saving a2 is trying to support dynamically dtb pass
> > > > > > > > > at
> > > > > > > > > runtime.
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Could you please describe in more details on what the
> > > > > > > > use
> > > > > > > > case is
> > > > > > > > here?
> > > > > > > > 
> > > > > > > 
> > > > > > > Hi Bin and Lukas
> > > > > > > 
> > > > > > > After I study [PATCH 24/30] riscv: save hart ID and
> > > > > > > device
> > > > > > > tree
> > > > > > > passed
> > > > > > > by prior boot stage.
> > > > > > > I found it is the same thing.
> > > > > > > I will accepted this patch.
> > > > > > > So this patch is
> > > > > > > 
> > > > > > > Reviewed-by: Rick Chen <rick@andestech.com>
> > > > > > > 
> > > > > > > But I shall send a patch to fix ax25-ae350 to work as
> > > > > > > well
> > > > > > > later.
> > > > > > > And it will be as below :
> > > > > > > 
> > > > > > > ax5-ae350.c
> > > > > > > void *board_fdt_blob_setup(void)
> > > > > > > {
> > > > > > > void **ptr = (void *)&prior_stage_fdt_address;
> > > > > > > if (fdt_magic(*ptr) == FDT_MAGIC)
> > > > > > > return (void *)*ptr;
> > > > > > > 
> > > > > > > return (void *)CONFIG_SYS_FDT_BASE;
> > > > > > > }
> > > > > > > 
> > > > > > 
> > > > > > Sorry!
> > > > > > 
> > > > > > I shall send a fixed patch as below:
> > > > > > riscv: ax25-ae350: use device tree passed by prior boot
> > > > > > stage
> > > > > > 
> > > > > > --- a/configs/ax25-ae350_defconfig
> > > > > > +++ b/configs/ax25-ae350_defconfig
> > > > > > @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
> > > > > >  # CONFIG_CMD_SETEXPR is not set
> > > > > >  CONFIG_BOOTP_PREFER_SERVERIP=y
> > > > > >  CONFIG_CMD_CACHE=y
> > > > > > -CONFIG_OF_BOARD=y
> > > > > > +CONFIG_OF_PRIOR_STAGE=y
> > > > > > 
> > > > > > +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
> > > > > > @@ -64,15 +64,6 @@ ulong board_flash_get_legacy(ulong base,
> > > > > > int
> > > > > > banknum, flash_info_t *info)
> > > > > >         return 0;
> > > > > >  }
> > > > > > 
> > > > > > -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;
> > > > > > -}
> > > > > > -
> > > > > > 
> > > > > > Rick
> > > > > > 
> > > > > 
> > > > > Sorry, I have missed your commit. Just to clarify, does the
> > > > > ax25
> > > > > pass
> > > > > the device tree in a2 instead of a1? In that case, my patch
> > > > > must
> > > > > be
> > > > > changed since it assumes the device tree to be in a1.
> > > > > Related to this, can I assume that a0 stores the hart ID?
> > > > > 
> > > > > If you want, I can add this patch to my series.
> > > > > 
> > > > 
> > > > Hi Lukas
> > > > 
> > > > Your patch is correct.
> > > > I will send out a patch to fix the mismatch in ax25-ae350
> > > > later.
> > > > And maybe you can put this patch into your v2 series.
> > > > 
> > > > Thanks
> > > > 
> > > > Rick
> > > > 
> > > 
> > > Ok. Yes, I will pick up your patch in my v2.
> > > 
> > 
> > Do you want me to wait with my patch series until it is clear how
> > ax25-
> > ae350 handles the device tree? Otherwise, I will drop the part of
> > this
> > patch that removes the code storing a2 to avoid breaking your board
> > and
> > send v2 of my series.
> > 
> 
> Hi Likas
> 
> It is ok to keep this patch into v2
> [PATCH 22/30] riscv: remove unused labels in start.S
> 
> I will send a patch to fix it after your series.
> 
> Rick
> 

I ended up removing that part of the patch to make sure I don't break
anything. You can re-add it together with your patch :)

Thanks,
Lukas
diff mbox series

Patch

diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index f375a9316e..851a1d0870 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -34,17 +34,6 @@ 
 .section .text
 .globl _start
 _start:
-	j	handle_reset
-
-nmi_vector:
-	j	nmi_vector
-
-trap_vector:
-	j	trap_entry
-
-handle_reset:
-	li	t0, CONFIG_SYS_SDRAM_BASE
-	SREG	a2, 0(t0)
 	la	t0, trap_entry
 	csrw	mtvec, t0
 	csrwi	mstatus, 0