2020-04-16 00:14:44

by 王文虎

[permalink] [raw]
Subject: [PATCH v2,1/5] powerpc: 85xx: make FSL_85XX_CACHE_SRAM configurable

Enable FSL_85XX_CACHE_SRAM selection. On e500 platforms, the cache
could be configured and used as a piece of SRAM which is hignly
friendly for some user level application performances.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Scott Wood <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Signed-off-by: Wang Wenhu <[email protected]>
---
Changes since v1:
* None
---
arch/powerpc/platforms/85xx/Kconfig | 2 +-
arch/powerpc/platforms/Kconfig.cputype | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
index fa3d29dcb57e..6debb4f1b9cc 100644
--- a/arch/powerpc/platforms/85xx/Kconfig
+++ b/arch/powerpc/platforms/85xx/Kconfig
@@ -17,7 +17,7 @@ if FSL_SOC_BOOKE
if PPC32

config FSL_85XX_CACHE_SRAM
- bool
+ bool "Freescale 85xx Cache-Sram"
select PPC_LIB_RHEAP
help
When selected, this option enables cache-sram support
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 0c3c1902135c..1921e9a573e8 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
config PPC32
- bool
+ bool "32-bit kernel"
default y if !PPC64
select KASAN_VMALLOC if KASAN && MODULES

@@ -15,6 +15,7 @@ config PPC_BOOK3S_32
bool

menu "Processor support"
+
choice
prompt "Processor Type"
depends on PPC32
@@ -211,9 +212,9 @@ config PPC_BOOK3E
depends on PPC_BOOK3E_64

config E500
+ bool "e500 Support"
select FSL_EMB_PERFMON
select PPC_FSL_BOOK3E
- bool

config PPC_E500MC
bool "e500mc Support"
--
2.17.1


2020-04-16 00:23:08

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v2,1/5] powerpc: 85xx: make FSL_85XX_CACHE_SRAM configurable



Le 15/04/2020 à 17:24, Wang Wenhu a écrit :
> Enable FSL_85XX_CACHE_SRAM selection. On e500 platforms, the cache
> could be configured and used as a piece of SRAM which is hignly
> friendly for some user level application performances.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: Scott Wood <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: [email protected]
> Signed-off-by: Wang Wenhu <[email protected]>
> ---
> Changes since v1:
> * None
> ---
> arch/powerpc/platforms/85xx/Kconfig | 2 +-
> arch/powerpc/platforms/Kconfig.cputype | 5 +++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index fa3d29dcb57e..6debb4f1b9cc 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -17,7 +17,7 @@ if FSL_SOC_BOOKE
> if PPC32
>
> config FSL_85XX_CACHE_SRAM
> - bool
> + bool "Freescale 85xx Cache-Sram"
> select PPC_LIB_RHEAP
> help
> When selected, this option enables cache-sram support
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 0c3c1902135c..1921e9a573e8 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> config PPC32
> - bool
> + bool "32-bit kernel"

Why make that user selectable ?

Either a kernel is 64-bit or it is 32-bit. So having PPC64 user
selectable is all we need.

And what is the link between this change and the description in the log ?

> default y if !PPC64
> select KASAN_VMALLOC if KASAN && MODULES
>
> @@ -15,6 +15,7 @@ config PPC_BOOK3S_32
> bool
>
> menu "Processor support"
> +

Why adding this space ?

> choice
> prompt "Processor Type"
> depends on PPC32
> @@ -211,9 +212,9 @@ config PPC_BOOK3E
> depends on PPC_BOOK3E_64
>
> config E500
> + bool "e500 Support"
> select FSL_EMB_PERFMON
> select PPC_FSL_BOOK3E
> - bool

Why make this user-selectable ? This is already selected by the
processors requiring it, ie 8500, e5500 and e6500.

Is there any other case where we need E500 ?

And again, what's the link between this change and the description in
the log ?


>
> config PPC_E500MC
> bool "e500mc Support"
>

Christophe

2020-04-16 01:01:43

by Crystal Wood

[permalink] [raw]
Subject: Re: [PATCH v2,1/5] powerpc: 85xx: make FSL_85XX_CACHE_SRAM configurable

On Wed, 2020-04-15 at 08:24 -0700, Wang Wenhu wrote:
> Enable FSL_85XX_CACHE_SRAM selection. On e500 platforms, the cache
> could be configured and used as a piece of SRAM which is hignly
> friendly for some user level application performances.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: Scott Wood <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: [email protected]
> Signed-off-by: Wang Wenhu <[email protected]>
> ---
> Changes since v1:
> * None
> ---
> arch/powerpc/platforms/85xx/Kconfig | 2 +-
> arch/powerpc/platforms/Kconfig.cputype | 5 +++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/Kconfig
> b/arch/powerpc/platforms/85xx/Kconfig
> index fa3d29dcb57e..6debb4f1b9cc 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -17,7 +17,7 @@ if FSL_SOC_BOOKE
> if PPC32
>
> config FSL_85XX_CACHE_SRAM
> - bool
> + bool "Freescale 85xx Cache-Sram"
> select PPC_LIB_RHEAP
> help
> When selected, this option enables cache-sram support

NACK

As discussed before, the driver that uses this API should "select" this
symbol.

-Scott


2020-04-16 01:49:26

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v2,1/5] powerpc: 85xx: make FSL_85XX_CACHE_SRAM configurable



Le 15/04/2020 à 17:24, Wang Wenhu a écrit :
> Enable FSL_85XX_CACHE_SRAM selection. On e500 platforms, the cache
> could be configured and used as a piece of SRAM which is hignly
> friendly for some user level application performances.

It looks like following patches are fixing errors generated by selecting
FSL_85XX_CACHE_SRAM.

So this patch should go after the patches which fixes the errors, ie it
should be patch 4 in the series.

Christophe

2020-04-16 04:13:52

by 王文虎

[permalink] [raw]
Subject: Re: [PATCH v2,1/5] powerpc: 85xx: make FSL_85XX_CACHE_SRAM configurable

From: Scott Wood <[email protected]>

>> + bool "32-bit kernel"
>
>Why make that user selectable ?
>
>Either a kernel is 64-bit or it is 32-bit. So having PPC64 user
>selectable is all we need.
>
>And what is the link between this change and the description in the log ?
>
>> default y if !PPC64
>> select KASAN_VMALLOC if KASAN && MODULES
>>
>> @@ -15,6 +15,7 @@ config PPC_BOOK3S_32
>> bool
>>
>> menu "Processor support"
>> +
>
>Why adding this space ?
>
>> choice
>> prompt "Processor Type"
>> depends on PPC32
>> @@ -211,9 +212,9 @@ config PPC_BOOK3E
>> depends on PPC_BOOK3E_64
>>
>> config E500
>> + bool "e500 Support"
>> select FSL_EMB_PERFMON
>> select PPC_FSL_BOOK3E
>> - bool
>
>Why make this user-selectable ? This is already selected by the
>processors requiring it, ie 8500, e5500 and e6500.
>
>Is there any other case where we need E500 ?
>
>And again, what's the link between this change and the description in
>the log ?
>
>
>>
>> config PPC_E500MC
>> bool "e500mc Support"
>>
>
>Christophe

Hi, Scott, Christophe!

I find that I did not get the point well of the defferences between
configurability and selectability(maybe words I created) of Kconfig items.

You are right that FSL_85XX_CACHE_SRAM should only be selected by a caller
but never enable it seperately.

Same answer for the comments from Christophe. I will drop this patch in v3.

Thanks,
Wenhu