Ping all, Some time ago I saw several people mentioned ARMv8 PSCI, are you interested in leaving your review comments? I know it is not so easy for reviewing assembly language patches, but code structures and assembly instructions in this patch set are not complicated. When this common framework gets merged, it will be possible for each platform to implement their platform PSCI functions in C too.
Hi Tom, Any further concerns? comments?
Thanks all.
On Wed, Sep 28, 2016 at 4:27 PM, Hongbo Zhang macro.wave.z@gmail.com wrote:
I just explained why and how I tried to place newly introduced macros in arch/arm/cpu/armv8/Kconfig and board/freescale/ls1043ardb/Kconfig.
As to the order, is it compulsory to define such a CONFIG_* before using it, even there is #ifdef test when using it?
If yes, I have to define all the four new CONFIG_* into arch/arm/cpu/armv8/Kconfig along with the adding-secure-sections patch, but default values have to be set which is bit hard to handle, for the CONFIG_ARMV8_PSCI_NR_CPUS, we can set a 4 although it isn't correct in most cases, but for the CONFIG_ARMV8_SECURE_BASE, we don't know its default value, it is really platform specific.
In my implementation, I just wanted to treat CONFIG_ARMV8_PSCI_NR_CPUS and CONFIG_CPU_PER_CLUSTER as same as CONFIG_ARMV8_SECURE_BASE, since they are all platform specific.
And for the CONFIG_ARMV8_PSCI_NR_CPUS, it is slightly different form ARMv7's, if not defined, the lds file still works: #ifdef CONFIG_ARMV8_PSCI_NR_CPUS . = . + CONFIG_ARMV8_PSCI_NR_CPUS * ARM_PSCI_STACK_SIZE; #else . = . + 4 * ARM_PSCI_STACK_SIZE; #endif
On Wed, Sep 28, 2016 at 3:16 PM, Hongbo Zhang macro.wave.z@gmail.com wrote:
On Wed, Sep 28, 2016 at 1:23 AM, Tom Rini trini@konsulko.com wrote:
On Tue, Sep 27, 2016 at 05:29:00PM +0800, macro.wave.z@gmail.com wrote:
From: Hongbo Zhang hongbo.zhang@nxp.com
This patch set introduces ARMv8 PSCI framework, all the PSCI functions are implemented a default dummy one, it is up to each platform to implement their own specific ones.
The first 1/6 patch is a prepare clean up for adding ARMv8 PSCI. Patches 2/6 to 5/6 introduce new ARMv8 framework and set it up. The last 6/6 adds a most simple implementation on NXP LS1043 platform, to verify this framework.
This patch set mainly introduces ARMv8 PSCI framework, for easier review and merge, further PSCI implementation on LS1043 is coming later.
Hongbo Zhang (6): ARMv8: LS1043A: change macro CONFIG_ARMV8_PSCI definition ARMv8: Add secure sections for PSCI text and data ARMv8: Add basic PSCI framework ARMv8: Setup PSCI memory and dt ARMv8: Enable SMC instruction ARMv8: LS1043A: Enable LS1043A default PSCI support
Conceptually this is good. I have some issues around order of the patches, and where the Kconfig entries end up. Looking over the series we introduce usage of some CONFIG symbols prior to declaring them in Kconfig. This is more of a hard no now as it will break bisecting when the test for no new CONFIG symbols is tripped. The other problem is that I think the symbols you're adding in board/freescale/ls1043ardb/Kconfig need to be in arch/arm/cpu/armv8/Kconfig and then use default ... if ... to give the right address for the layerscape boards.
Thanks Tom for quick response.
For config options introduced: CONFIG_ARMV8_PSCI CONFIG_ARMV8_PSCI_NR_CPUS CONFIG_CPU_PER_CLUSTER CONFIG_ARMV8_SECURE_BASE
I've tested adding patch one by one, there is no problem with the check-config script.
And my idea was like this: let the CONFIG_ARMV8_PSCI to be an overall switch, and if it is enabled even without the other three ones, the default PSCI still works, as I've tested, this really works because any of the other three macros, when used, there is a #ifdef to check if it exists, if no, a default value is used or it isn't used at all. The later three macros, because they are platform specific so I intended to let every platform to define them.
This is slightly different from ARMv7, plan was if this get accepted, I would like to send patch to update ARMv7's.
-- Tom