| Message ID | cfd8de4b2ad1cfd7b0f1a0706c0b6b918e1ebffc.1527244201.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, May 25, 2018 at 12:31 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > The cooling device properties, like "#cooling-cells" and > "dynamic-power-coefficient", should either be present for all the CPUs > of a cluster or none. If these are present only for a subset of CPUs of > a cluster then things will start falling apart as soon as the CPUs are > brought online in a different order. Thanks for the patch. In case of Exynos, the booting CPU always has these information in DT and the booting CPU cannot be changed (chosen by firmware/hardware configuration). Therefore there is no real risk of falling although for correctness of DT your change makes sense. It is too late for this cycle for me so I'll pick it up after merge window. Alternatively, arm-soc guys can pick it up directly with my tag: Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof > For example, this will happen > because the operating system looks for such properties in the CPU node > it is trying to bring up, so that it can register a cooling device. > > Add such missing properties. > > Fix other missing properties (clocks, OPP, clock latency) as well to > make it all work. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > arch/arm/boot/dts/exynos3250.dtsi | 16 ++++++++++++++++ > arch/arm/boot/dts/exynos4210.dtsi | 13 +++++++++++++ > arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++ > arch/arm/boot/dts/exynos5250.dtsi | 23 +++++++++++++++++++++++ > 4 files changed, 61 insertions(+) > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29-05-18, 15:18, Krzysztof Kozlowski wrote: > Thanks for the patch. > > In case of Exynos, the booting CPU always has these information in DT > and the booting CPU cannot be changed (chosen by firmware/hardware > configuration). But can the booting CPU be offlined ? If yes, then this is how things are broken right now. Build cpufreq driver as module, boot kernel, hotplug out CPU0, insert cpufreq driver and that will try to find these properties in CPU1. > Therefore there is no real risk of falling although > for correctness of DT your change makes sense. > > It is too late for this cycle for me so I'll pick it up after merge window. > Alternatively, arm-soc guys can pick it up directly with my tag: > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> No worries, you can pick it up later on.
On Wed, May 30, 2018 at 6:38 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 29-05-18, 15:18, Krzysztof Kozlowski wrote: >> Thanks for the patch. >> >> In case of Exynos, the booting CPU always has these information in DT >> and the booting CPU cannot be changed (chosen by firmware/hardware >> configuration). > > But can the booting CPU be offlined ? > > If yes, then this is how things are broken right now. > > Build cpufreq driver as module, boot kernel, hotplug out CPU0, insert > cpufreq driver and that will try to find these properties in CPU1. OK, I see the possibility although it is still far away from use cases. You cannot hotplug booting CPU (CPU0) on Exynos kernels. It never worked. Strictly speaking - offlining will work. But bringing it online will likely hang the system. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30-05-18, 14:32, Krzysztof Kozlowski wrote: > OK, I see the possibility although it is still far away from use > cases. You cannot hotplug booting CPU (CPU0) on Exynos kernels. It > never worked. Strictly speaking - offlining will work. But bringing it > online will likely hang the system. True and I used the following out of tree patch for a long time for my dual A15 exynos to make hotplug work. https://git.linaro.org/people/vireshk/backup/linux.git/commit/?h=bkp/exynos/hotplug&id=aab8a906a70b8f1fb15a4b7bd2ee27e6dcabf79d
On Fri, May 25, 2018 at 04:01:53PM +0530, Viresh Kumar wrote: > The cooling device properties, like "#cooling-cells" and > "dynamic-power-coefficient", should either be present for all the CPUs > of a cluster or none. If these are present only for a subset of CPUs of > a cluster then things will start falling apart as soon as the CPUs are > brought online in a different order. For example, this will happen > because the operating system looks for such properties in the CPU node > it is trying to bring up, so that it can register a cooling device. > > Add such missing properties. > > Fix other missing properties (clocks, OPP, clock latency) as well to > make it all work. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > arch/arm/boot/dts/exynos3250.dtsi | 16 ++++++++++++++++ > arch/arm/boot/dts/exynos4210.dtsi | 13 +++++++++++++ > arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++ > arch/arm/boot/dts/exynos5250.dtsi | 23 +++++++++++++++++++++++ > 4 files changed, 61 insertions(+) > Thanks, applied. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi index 962af97c1883..aff5d66ae058 100644 --- a/arch/arm/boot/dts/exynos3250.dtsi +++ b/arch/arm/boot/dts/exynos3250.dtsi @@ -78,6 +78,22 @@ compatible = "arm,cortex-a7"; reg = <1>; clock-frequency = <1000000000>; + clocks = <&cmu CLK_ARM_CLK>; + clock-names = "cpu"; + #cooling-cells = <2>; + + operating-points = < + 1000000 1150000 + 900000 1112500 + 800000 1075000 + 700000 1037500 + 600000 1000000 + 500000 962500 + 400000 925000 + 300000 887500 + 200000 850000 + 100000 850000 + >; }; }; diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index 88fb47cef9a8..b6091c27f155 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -55,6 +55,19 @@ device_type = "cpu"; compatible = "arm,cortex-a9"; reg = <0x901>; + clocks = <&clock CLK_ARM_CLK>; + clock-names = "cpu"; + clock-latency = <160000>; + + operating-points = < + 1200000 1250000 + 1000000 1150000 + 800000 1075000 + 500000 975000 + 400000 975000 + 200000 950000 + >; + #cooling-cells = <2>; /* min followed by max */ }; }; diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index 7b43c10c510b..51f72f0327e5 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -49,21 +49,30 @@ device_type = "cpu"; compatible = "arm,cortex-a9"; reg = <0xA01>; + clocks = <&clock CLK_ARM_CLK>; + clock-names = "cpu"; operating-points-v2 = <&cpu0_opp_table>; + #cooling-cells = <2>; /* min followed by max */ }; cpu@a02 { device_type = "cpu"; compatible = "arm,cortex-a9"; reg = <0xA02>; + clocks = <&clock CLK_ARM_CLK>; + clock-names = "cpu"; operating-points-v2 = <&cpu0_opp_table>; + #cooling-cells = <2>; /* min followed by max */ }; cpu@a03 { device_type = "cpu"; compatible = "arm,cortex-a9"; reg = <0xA03>; + clocks = <&clock CLK_ARM_CLK>; + clock-names = "cpu"; operating-points-v2 = <&cpu0_opp_table>; + #cooling-cells = <2>; /* min followed by max */ }; }; diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 2daf505b3d08..69648f83b8b4 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -84,6 +84,29 @@ compatible = "arm,cortex-a15"; reg = <1>; clock-frequency = <1700000000>; + clocks = <&clock CLK_ARM_CLK>; + clock-names = "cpu"; + clock-latency = <140000>; + + operating-points = < + 1700000 1300000 + 1600000 1250000 + 1500000 1225000 + 1400000 1200000 + 1300000 1150000 + 1200000 1125000 + 1100000 1100000 + 1000000 1075000 + 900000 1050000 + 800000 1025000 + 700000 1012500 + 600000 1000000 + 500000 975000 + 400000 950000 + 300000 937500 + 200000 925000 + >; + #cooling-cells = <2>; /* min followed by max */ }; };
The cooling device properties, like "#cooling-cells" and "dynamic-power-coefficient", should either be present for all the CPUs of a cluster or none. If these are present only for a subset of CPUs of a cluster then things will start falling apart as soon as the CPUs are brought online in a different order. For example, this will happen because the operating system looks for such properties in the CPU node it is trying to bring up, so that it can register a cooling device. Add such missing properties. Fix other missing properties (clocks, OPP, clock latency) as well to make it all work. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- arch/arm/boot/dts/exynos3250.dtsi | 16 ++++++++++++++++ arch/arm/boot/dts/exynos4210.dtsi | 13 +++++++++++++ arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++ arch/arm/boot/dts/exynos5250.dtsi | 23 +++++++++++++++++++++++ 4 files changed, 61 insertions(+)