2015-07-01 12:09:39

by Vitaly Andrianov

[permalink] [raw]
Subject: [PATCH] ARM: psci: keystone: remove keystone_smp_ops code if psci is supported

If we enable psci support we don't need keystone_smp_ops anymore.

Signed-off-by: Vitaly Andrianov <[email protected]>
---
arch/arm/mach-keystone/Makefile | 2 ++
arch/arm/mach-keystone/keystone.c | 2 ++
2 files changed, 4 insertions(+)

diff --git a/arch/arm/mach-keystone/Makefile b/arch/arm/mach-keystone/Makefile
index 25d9239..5cc5ca4 100644
--- a/arch/arm/mach-keystone/Makefile
+++ b/arch/arm/mach-keystone/Makefile
@@ -3,7 +3,9 @@ obj-y := keystone.o smc.o
plus_sec := $(call as-instr,.arch_extension sec,+sec)
AFLAGS_smc.o :=-Wa,-march=armv7-a$(plus_sec)

+ifneq ($(CONFIG_ARM_PSCI),y)
obj-$(CONFIG_SMP) += platsmp.o
+endif

# PM domain driver for Keystone SOCs
obj-$(CONFIG_ARCH_KEYSTONE) += pm_domain.o
diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-keystone/keystone.c
index e2880105..9cc489c 100644
--- a/arch/arm/mach-keystone/keystone.c
+++ b/arch/arm/mach-keystone/keystone.c
@@ -105,7 +105,9 @@ DT_MACHINE_START(KEYSTONE, "Keystone")
#if defined(CONFIG_ZONE_DMA) && defined(CONFIG_ARM_LPAE)
.dma_zone_size = SZ_2G,
#endif
+#ifndef CONFIG_ARM_PSCI
.smp = smp_ops(keystone_smp_ops),
+#endif
.init_machine = keystone_init,
.dt_compat = keystone_match,
.pv_fixup = keystone_pv_fixup,
--
1.9.1


2015-07-01 12:09:44

by Vitaly Andrianov

[permalink] [raw]
Subject: [PATCH] keystone: dts: add psci command definition

This commit adds definition for cpu_on, cpu_off and cpu_suspend commands.
These definitions must match the corresponding PSCI definitions in
boot monitor.

Signed-off-by: Vitaly Andrianov <[email protected]>
---
arch/arm/boot/dts/keystone.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index c06542b..ab60fca 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -58,6 +58,14 @@
<GIC_SPI 23 IRQ_TYPE_EDGE_RISING>;
};

+ psci {
+ compatible = "arm,psci";
+ method = "smc";
+ cpu_suspend = <0x84000001>;
+ cpu_off = <0x84000002>;
+ cpu_on = <0x84000003>;
+ };
+
soc {
#address-cells = <1>;
#size-cells = <1>;
--
1.9.1

2015-07-01 12:39:38

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH] ARM: psci: keystone: remove keystone_smp_ops code if psci is supported

On Wed, Jul 01, 2015 at 01:13:03PM +0100, Vitaly Andrianov wrote:
> If we enable psci support we don't need keystone_smp_ops anymore.

You need them for existing users.

>
> Signed-off-by: Vitaly Andrianov <[email protected]>
> ---
> arch/arm/mach-keystone/Makefile | 2 ++
> arch/arm/mach-keystone/keystone.c | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/arm/mach-keystone/Makefile b/arch/arm/mach-keystone/Makefile
> index 25d9239..5cc5ca4 100644
> --- a/arch/arm/mach-keystone/Makefile
> +++ b/arch/arm/mach-keystone/Makefile
> @@ -3,7 +3,9 @@ obj-y := keystone.o smc.o
> plus_sec := $(call as-instr,.arch_extension sec,+sec)
> AFLAGS_smc.o :=-Wa,-march=armv7-a$(plus_sec)
>
> +ifneq ($(CONFIG_ARM_PSCI),y)
> obj-$(CONFIG_SMP) += platsmp.o
> +endif
>
> # PM domain driver for Keystone SOCs
> obj-$(CONFIG_ARCH_KEYSTONE) += pm_domain.o
> diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-keystone/keystone.c
> index e2880105..9cc489c 100644
> --- a/arch/arm/mach-keystone/keystone.c
> +++ b/arch/arm/mach-keystone/keystone.c
> @@ -105,7 +105,9 @@ DT_MACHINE_START(KEYSTONE, "Keystone")
> #if defined(CONFIG_ZONE_DMA) && defined(CONFIG_ARM_LPAE)
> .dma_zone_size = SZ_2G,
> #endif
> +#ifndef CONFIG_ARM_PSCI
> .smp = smp_ops(keystone_smp_ops),
> +#endif

This will prevent building a kernel that supports PSCI and your legacy
SMP bringup.

I don't think this is necessary, given the code in setup_arch and the
absence of mdesc->smp_init for keystone. A single kernel can support
both, and it should support both.

Thanks,
Mark.

> .init_machine = keystone_init,
> .dt_compat = keystone_match,
> .pv_fixup = keystone_pv_fixup,
> --
> 1.9.1
>

2015-07-01 12:41:28

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH] keystone: dts: add psci command definition

On Wed, Jul 01, 2015 at 01:13:04PM +0100, Vitaly Andrianov wrote:
> This commit adds definition for cpu_on, cpu_off and cpu_suspend commands.
> These definitions must match the corresponding PSCI definitions in
> boot monitor.
>
> Signed-off-by: Vitaly Andrianov <[email protected]>
> ---
> arch/arm/boot/dts/keystone.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
> index c06542b..ab60fca 100644
> --- a/arch/arm/boot/dts/keystone.dtsi
> +++ b/arch/arm/boot/dts/keystone.dtsi
> @@ -58,6 +58,14 @@
> <GIC_SPI 23 IRQ_TYPE_EDGE_RISING>;
> };
>
> + psci {
> + compatible = "arm,psci";
> + method = "smc";
> + cpu_suspend = <0x84000001>;
> + cpu_off = <0x84000002>;
> + cpu_on = <0x84000003>;
> + };

It would be nice to have this injected automatically by the bootlaoder
when PSCI is present, as that way users of existing systems can upgrade
their DTB and still have things boot.

Thanks,
Mark.

> +
> soc {
> #address-cells = <1>;
> #size-cells = <1>;
> --
> 1.9.1
>

2015-07-01 13:01:39

by Vitaly Andrianov

[permalink] [raw]
Subject: Re: [PATCH] keystone: dts: add psci command definition



On 07/01/2015 08:41 AM, Mark Rutland wrote:
> On Wed, Jul 01, 2015 at 01:13:04PM +0100, Vitaly Andrianov wrote:
>> This commit adds definition for cpu_on, cpu_off and cpu_suspend commands.
>> These definitions must match the corresponding PSCI definitions in
>> boot monitor.
>>
>> Signed-off-by: Vitaly Andrianov <[email protected]>
>> ---
>> arch/arm/boot/dts/keystone.dtsi | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
>> index c06542b..ab60fca 100644
>> --- a/arch/arm/boot/dts/keystone.dtsi
>> +++ b/arch/arm/boot/dts/keystone.dtsi
>> @@ -58,6 +58,14 @@
>> <GIC_SPI 23 IRQ_TYPE_EDGE_RISING>;
>> };
>>
>> + psci {
>> + compatible = "arm,psci";
>> + method = "smc";
>> + cpu_suspend = <0x84000001>;
>> + cpu_off = <0x84000002>;
>> + cpu_on = <0x84000003>;
>> + };
>
> It would be nice to have this injected automatically by the bootlaoder
> when PSCI is present, as that way users of existing systems can upgrade
> their DTB and still have things boot.
>
> Thanks,
> Mark.
>

So, KS2 kernel doesn't need that commit and has to wait this from
u-boot? Actually PSCI support is a part of KS2 boot-monitor. U-boot is
just responsible to load and start it, but doesn't know whether PSCI is
supported.

In any case you are right. The DTS also doesn't know whether boot-
monitor supports PSCI. It is better to make u-boot to fix up the dts.
Let's forget about this patch. I'll work on dts fix-up in u-boot.

Thanks,
Vitaly

>> +
>> soc {
>> #address-cells = <1>;
>> #size-cells = <1>;
>> --
>> 1.9.1
>>

2015-07-01 14:12:39

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] ARM: psci: keystone: remove keystone_smp_ops code if psci is supported

On Wed, Jul 01, 2015 at 08:13:03AM -0400, Vitaly Andrianov wrote:
> If we enable psci support we don't need keystone_smp_ops anymore.
>
> Signed-off-by: Vitaly Andrianov <[email protected]>

However, this is not necessary. If you look at arch/arm/kernel/setup.c,
it contains this:

psci_init();
#ifdef CONFIG_SMP
if (is_smp()) {
if (!mdesc->smp_init || !mdesc->smp_init()) {
if (psci_smp_available())
smp_set_ops(&psci_smp_ops);
else if (mdesc->smp)
smp_set_ops(mdesc->smp);
}
smp_init_cpus();
smp_build_mpidr_hash();
}
#endif

So, the platforms' SMP ops won't be used even if they're provided
when PSCI is available.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-07-01 15:04:18

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [PATCH] keystone: dts: add psci command definition

On 7/1/2015 6:05 AM, Vitaly Andrianov wrote:
>
>
> On 07/01/2015 08:41 AM, Mark Rutland wrote:
>> On Wed, Jul 01, 2015 at 01:13:04PM +0100, Vitaly Andrianov wrote:
>>> This commit adds definition for cpu_on, cpu_off and cpu_suspend
>>> commands.
>>> These definitions must match the corresponding PSCI definitions in
>>> boot monitor.
>>>
>>> Signed-off-by: Vitaly Andrianov <[email protected]>
>>> ---
>>> arch/arm/boot/dts/keystone.dtsi | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/keystone.dtsi
>>> b/arch/arm/boot/dts/keystone.dtsi
>>> index c06542b..ab60fca 100644
>>> --- a/arch/arm/boot/dts/keystone.dtsi
>>> +++ b/arch/arm/boot/dts/keystone.dtsi
>>> @@ -58,6 +58,14 @@
>>> <GIC_SPI 23 IRQ_TYPE_EDGE_RISING>;
>>> };
>>>
>>> + psci {
>>> + compatible = "arm,psci";
>>> + method = "smc";
>>> + cpu_suspend = <0x84000001>;
>>> + cpu_off = <0x84000002>;
>>> + cpu_on = <0x84000003>;
>>> + };
>>
>> It would be nice to have this injected automatically by the bootlaoder
>> when PSCI is present, as that way users of existing systems can upgrade
>> their DTB and still have things boot.
>>
>> Thanks,
>> Mark.
>>
>
> So, KS2 kernel doesn't need that commit and has to wait this from
> u-boot? Actually PSCI support is a part of KS2 boot-monitor. U-boot is
> just responsible to load and start it, but doesn't know whether PSCI is
> supported.
>
> In any case you are right. The DTS also doesn't know whether boot-
> monitor supports PSCI. It is better to make u-boot to fix up the dts.
> Let's forget about this patch. I'll work on dts fix-up in u-boot.
>
Good. We can't break existing users as already pointed out earlier.
Thanks Vitaly for following it up.

Regards,
Santosh