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

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

[Vitaly Andrianov]

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

Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
---
 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

[PATCH] keystone: dts: add psci command definition

[Vitaly Andrianov]

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 <vitalya@ti.com>
---
 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

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

[Mark Rutland]

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 <vitalya@ti.com>
> ---
>  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
> 

[PATCH] keystone: dts: add psci command definition

[Mark Rutland]

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 <vitalya@ti.com>
> ---
>  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
> 

[PATCH] keystone: dts: add psci command definition

[Vitaly Andrianov]

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 <vitalya@ti.com>
>> ---
>>   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
>>

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

[Russell King - ARM Linux]

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 <vitalya@ti.com>

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.

[PATCH] keystone: dts: add psci command definition

[santosh shilimkar]

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 <vitalya@ti.com>
>>> ---
>>>   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