[U-Boot,v2,06/20] riscv: ax25: Hide the ax25-specific Kconfig option

Message ID 1544192072-28764-7-git-send-email-bmeng.cn@gmail.com
State Superseded
Delegated to: Andes
Headers show
Series
  • riscv: Adding RISC-V CPU and timer driver
Related show

Commit Message

Bin Meng Dec. 7, 2018, 2:14 p.m.
There is no need to expose RISCV_NDS to the Kconfig menu as it is
an ax25-specific option.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

Changes in v2: None

 arch/riscv/cpu/ax25/Kconfig | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Auer, Lukas Dec. 10, 2018, 11:18 p.m. | #1
On Fri, 2018-12-07 at 06:14 -0800, Bin Meng wrote:
> There is no need to expose RISCV_NDS to the Kconfig menu as it is
> an ax25-specific option.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
> Changes in v2: None
> 
>  arch/riscv/cpu/ax25/Kconfig | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 

Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
Rick Chen Dec. 11, 2018, 7:06 a.m. | #2
> > Subject: [PATCH v2 06/20] riscv: ax25: Hide the ax25-specific Kconfig option
> >
> > There is no need to expose RISCV_NDS to the Kconfig menu as it is an
> > ax25-specific option.
> >

Hi Bin

Can you explain why there is no need to expose RISCV_NDS here ?

Rick

> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> > Changes in v2: None
> >
> >  arch/riscv/cpu/ax25/Kconfig | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/riscv/cpu/ax25/Kconfig b/arch/riscv/cpu/ax25/Kconfig index
> > 6c7022f..5ff9e5c 100644
> > --- a/arch/riscv/cpu/ax25/Kconfig
> > +++ b/arch/riscv/cpu/ax25/Kconfig
> > @@ -1,7 +1,5 @@
> >  config RISCV_NDS
> > -     bool "AndeStar V5 ISA support"
> > -     default n
> > +     bool
> >       help
> > -             Say Y here if you plan to run U-Boot on AndeStar v5
> > -             platforms and use some specific features which are
> > -             provided by Andes Technology AndeStar V5 Families.
> > +       Run U-Boot on AndeStar v5 platforms and use some specific features
> > +       which are provided by Andes Technology AndeStar V5 Families.
> > --
> > 2.7.4
>
Bin Meng Dec. 11, 2018, 7:17 a.m. | #3
Hi Rick,

On Tue, Dec 11, 2018 at 3:06 PM Rick Chen <rickchen36@gmail.com> wrote:
>
> > > Subject: [PATCH v2 06/20] riscv: ax25: Hide the ax25-specific Kconfig option
> > >
> > > There is no need to expose RISCV_NDS to the Kconfig menu as it is an
> > > ax25-specific option.
> > >
>
> Hi Bin
>
> Can you explain why there is no need to expose RISCV_NDS here ?
>

This is specific to AX25, and there is no need to appear in the
Kconfig menu when people are building U-Boot for some other RISC-V
platforms. Also even if you select Y in the Kconfig menu for this
option for platforms other than AX25, it just does not help since all
its logic is within arch/riscv/cpu/ax25.

Regards,
Bin
Rick Chen Dec. 12, 2018, 9:02 a.m. | #4
Hi Bin

Bin Meng <bmeng.cn@gmail.com> 於 2018年12月11日 週二 下午3:17寫道:
>
> Hi Rick,
>
> On Tue, Dec 11, 2018 at 3:06 PM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > > > Subject: [PATCH v2 06/20] riscv: ax25: Hide the ax25-specific Kconfig option
> > > >
> > > > There is no need to expose RISCV_NDS to the Kconfig menu as it is an
> > > > ax25-specific option.
> > > >
> >
> > Hi Bin
> >
> > Can you explain why there is no need to expose RISCV_NDS here ?
> >
>
> This is specific to AX25, and there is no need to appear in the
> Kconfig menu when people are building U-Boot for some other RISC-V
> platforms. Also even if you select Y in the Kconfig menu for this
> option for platforms other than AX25, it just does not help since all
> its logic is within arch/riscv/cpu/ax25.
>

AX25 can not select RISCV_NDS by default, it may cause build fail problem.
I still prefer to enable it by make menuconfig.
Can you drop this patch ?

Thanks
Rick

> Regards,
> Bin
Bin Meng Dec. 12, 2018, 9:37 a.m. | #5
Hi Rick,

On Wed, Dec 12, 2018 at 5:02 PM Rick Chen <rickchen36@gmail.com> wrote:
>
> Hi Bin
>
> Bin Meng <bmeng.cn@gmail.com> 於 2018年12月11日 週二 下午3:17寫道:
> >
> > Hi Rick,
> >
> > On Tue, Dec 11, 2018 at 3:06 PM Rick Chen <rickchen36@gmail.com> wrote:
> > >
> > > > > Subject: [PATCH v2 06/20] riscv: ax25: Hide the ax25-specific Kconfig option
> > > > >
> > > > > There is no need to expose RISCV_NDS to the Kconfig menu as it is an
> > > > > ax25-specific option.
> > > > >
> > >
> > > Hi Bin
> > >
> > > Can you explain why there is no need to expose RISCV_NDS here ?
> > >
> >
> > This is specific to AX25, and there is no need to appear in the
> > Kconfig menu when people are building U-Boot for some other RISC-V
> > platforms. Also even if you select Y in the Kconfig menu for this
> > option for platforms other than AX25, it just does not help since all
> > its logic is within arch/riscv/cpu/ax25.
> >
>
> AX25 can not select RISCV_NDS by default, it may cause build fail problem.
> I still prefer to enable it by make menuconfig.
> Can you drop this patch ?
>

I prefer not to drop this patch since it's not supposed to be exposed
to other platforms.

Do you mean the build fail problem is custom CSR numbers like
mcache_ctl? Can we use hardcoded CSR number instead?

Regards,
Bin
Rick Chen Dec. 12, 2018, 9:57 a.m. | #6
Bin Meng <bmeng.cn@gmail.com> 於 2018年12月12日 週三 下午5:37寫道:
>
> Hi Rick,
>
> On Wed, Dec 12, 2018 at 5:02 PM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > Hi Bin
> >
> > Bin Meng <bmeng.cn@gmail.com> 於 2018年12月11日 週二 下午3:17寫道:
> > >
> > > Hi Rick,
> > >
> > > On Tue, Dec 11, 2018 at 3:06 PM Rick Chen <rickchen36@gmail.com> wrote:
> > > >
> > > > > > Subject: [PATCH v2 06/20] riscv: ax25: Hide the ax25-specific Kconfig option
> > > > > >
> > > > > > There is no need to expose RISCV_NDS to the Kconfig menu as it is an
> > > > > > ax25-specific option.
> > > > > >
> > > >
> > > > Hi Bin
> > > >
> > > > Can you explain why there is no need to expose RISCV_NDS here ?
> > > >
> > >
> > > This is specific to AX25, and there is no need to appear in the
> > > Kconfig menu when people are building U-Boot for some other RISC-V
> > > platforms. Also even if you select Y in the Kconfig menu for this
> > > option for platforms other than AX25, it just does not help since all
> > > its logic is within arch/riscv/cpu/ax25.
> > >
> >
> > AX25 can not select RISCV_NDS by default, it may cause build fail problem.
> > I still prefer to enable it by make menuconfig.
> > Can you drop this patch ?
> >
>
> I prefer not to drop this patch since it's not supposed to be exposed
> to other platforms.
>
> Do you mean the build fail problem is custom CSR numbers like
> mcache_ctl? Can we use hardcoded CSR number instead?
>

Thanks for your suggestion about hardcoded CSR number.

But actually I hope the mcache_ctl will be disabled by default in this stage.
Because some drivers of ae350 (like spi, smc flash driver, mac driver)
still have some access problems when cache is enable.
I am fixing it now.
But mmc driver is ready when cache is enable.
That is why I prefer enable cache by make menuconfig.
It will be easy to switch cache enable or disable by make menuconfig
without modifying Kconfig.

How do you think about it ?

B.R
Rick


> Regards,
> Bin
Bin Meng Dec. 12, 2018, 2:15 p.m. | #7
Hi Rick,

On Wed, Dec 12, 2018 at 5:56 PM Rick Chen <rickchen36@gmail.com> wrote:
>
> Bin Meng <bmeng.cn@gmail.com> 於 2018年12月12日 週三 下午5:37寫道:
> >
> > Hi Rick,
> >
> > On Wed, Dec 12, 2018 at 5:02 PM Rick Chen <rickchen36@gmail.com> wrote:
> > >
> > > Hi Bin
> > >
> > > Bin Meng <bmeng.cn@gmail.com> 於 2018年12月11日 週二 下午3:17寫道:
> > > >
> > > > Hi Rick,
> > > >
> > > > On Tue, Dec 11, 2018 at 3:06 PM Rick Chen <rickchen36@gmail.com> wrote:
> > > > >
> > > > > > > Subject: [PATCH v2 06/20] riscv: ax25: Hide the ax25-specific Kconfig option
> > > > > > >
> > > > > > > There is no need to expose RISCV_NDS to the Kconfig menu as it is an
> > > > > > > ax25-specific option.
> > > > > > >
> > > > >
> > > > > Hi Bin
> > > > >
> > > > > Can you explain why there is no need to expose RISCV_NDS here ?
> > > > >
> > > >
> > > > This is specific to AX25, and there is no need to appear in the
> > > > Kconfig menu when people are building U-Boot for some other RISC-V
> > > > platforms. Also even if you select Y in the Kconfig menu for this
> > > > option for platforms other than AX25, it just does not help since all
> > > > its logic is within arch/riscv/cpu/ax25.
> > > >
> > >
> > > AX25 can not select RISCV_NDS by default, it may cause build fail problem.
> > > I still prefer to enable it by make menuconfig.
> > > Can you drop this patch ?
> > >
> >
> > I prefer not to drop this patch since it's not supposed to be exposed
> > to other platforms.
> >
> > Do you mean the build fail problem is custom CSR numbers like
> > mcache_ctl? Can we use hardcoded CSR number instead?
> >
>
> Thanks for your suggestion about hardcoded CSR number.
>
> But actually I hope the mcache_ctl will be disabled by default in this stage.
> Because some drivers of ae350 (like spi, smc flash driver, mac driver)
> still have some access problems when cache is enable.
> I am fixing it now.
> But mmc driver is ready when cache is enable.
> That is why I prefer enable cache by make menuconfig.
> It will be easy to switch cache enable or disable by make menuconfig
> without modifying Kconfig.
>
> How do you think about it ?
>

Please check the v5 patch [1] which should satisfy your requirement.

[1] http://patchwork.ozlabs.org/patch/1011983/

Regards,
Bin
Rick Chen Dec. 13, 2018, 5:09 a.m. | #8
Hi Bin

Bin Meng <bmeng.cn@gmail.com> 於 2018年12月12日 週三 下午10:16寫道:
>
> Hi Rick,
>
> On Wed, Dec 12, 2018 at 5:56 PM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > Bin Meng <bmeng.cn@gmail.com> 於 2018年12月12日 週三 下午5:37寫道:
> > >
> > > Hi Rick,
> > >
> > > On Wed, Dec 12, 2018 at 5:02 PM Rick Chen <rickchen36@gmail.com> wrote:
> > > >
> > > > Hi Bin
> > > >
> > > > Bin Meng <bmeng.cn@gmail.com> 於 2018年12月11日 週二 下午3:17寫道:
> > > > >
> > > > > Hi Rick,
> > > > >
> > > > > On Tue, Dec 11, 2018 at 3:06 PM Rick Chen <rickchen36@gmail.com> wrote:
> > > > > >
> > > > > > > > Subject: [PATCH v2 06/20] riscv: ax25: Hide the ax25-specific Kconfig option
> > > > > > > >
> > > > > > > > There is no need to expose RISCV_NDS to the Kconfig menu as it is an
> > > > > > > > ax25-specific option.
> > > > > > > >
> > > > > >
> > > > > > Hi Bin
> > > > > >
> > > > > > Can you explain why there is no need to expose RISCV_NDS here ?
> > > > > >
> > > > >
> > > > > This is specific to AX25, and there is no need to appear in the
> > > > > Kconfig menu when people are building U-Boot for some other RISC-V
> > > > > platforms. Also even if you select Y in the Kconfig menu for this
> > > > > option for platforms other than AX25, it just does not help since all
> > > > > its logic is within arch/riscv/cpu/ax25.
> > > > >
> > > >
> > > > AX25 can not select RISCV_NDS by default, it may cause build fail problem.
> > > > I still prefer to enable it by make menuconfig.
> > > > Can you drop this patch ?
> > > >
> > >
> > > I prefer not to drop this patch since it's not supposed to be exposed
> > > to other platforms.
> > >
> > > Do you mean the build fail problem is custom CSR numbers like
> > > mcache_ctl? Can we use hardcoded CSR number instead?
> > >
> >
> > Thanks for your suggestion about hardcoded CSR number.
> >
> > But actually I hope the mcache_ctl will be disabled by default in this stage.
> > Because some drivers of ae350 (like spi, smc flash driver, mac driver)
> > still have some access problems when cache is enable.
> > I am fixing it now.
> > But mmc driver is ready when cache is enable.
> > That is why I prefer enable cache by make menuconfig.
> > It will be easy to switch cache enable or disable by make menuconfig
> > without modifying Kconfig.
> >
> > How do you think about it ?
> >
>
> Please check the v5 patch [1] which should satisfy your requirement.
>
> [1] http://patchwork.ozlabs.org/patch/1011983/
>

Yes
Thanks for this better solution.
It is good to me.

B.R
Rick

> Regards,
> Bin

Patch

diff --git a/arch/riscv/cpu/ax25/Kconfig b/arch/riscv/cpu/ax25/Kconfig
index 6c7022f..5ff9e5c 100644
--- a/arch/riscv/cpu/ax25/Kconfig
+++ b/arch/riscv/cpu/ax25/Kconfig
@@ -1,7 +1,5 @@ 
 config RISCV_NDS
-	bool "AndeStar V5 ISA support"
-	default n
+	bool
 	help
-		Say Y here if you plan to run U-Boot on AndeStar v5
-		platforms and use some specific features which are
-		provided by Andes Technology AndeStar V5 Families.
+	  Run U-Boot on AndeStar v5 platforms and use some specific features
+	  which are provided by Andes Technology AndeStar V5 Families.