| Message ID | 1518616792-29028-10-git-send-email-ilialin@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Ilia Lin (2018-02-14 05:59:51) > diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi > index 492a011..930f68b 100644 > --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi > +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2014-2016, The Linux Foundation. All rights reserved. > + * Copyright (c) 2014-2016,2018 The Linux Foundation. All rights reserved. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 and Why? > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi > index 4b2afcc..0359197 100644 > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi > @@ -144,6 +152,182 @@ > }; > }; > > + cluster0_opp: opp_table0 { > + compatible = "operating-points-v2"; > + opp-shared; > + > + opp@307200000 { > + opp-hz = /bits/ 64 < 307200000 >; > + clock-latency-ns = <200000>; > + }; > + opp@422400000 { > + opp-hz = /bits/ 64 < 422400000 >; > + clock-latency-ns = <200000>; > + }; > + opp@480000000 { It looks like opp-480000000 now instead of opp@<freq>. > + opp-hz = /bits/ 64 < 480000000 >; > + clock-latency-ns = <200000>; > + }; > + opp@556800000 { > + opp-hz = /bits/ 64 < 556800000 >; > + clock-latency-ns = <200000>; > + }; > + opp@652800000 { > + opp-hz = /bits/ 64 < 652800000 >; > + clock-latency-ns = <200000>; > + }; > + opp@729600000 { > + opp-hz = /bits/ 64 < 729600000 >; > + clock-latency-ns = <200000>; > + }; > + opp@844800000 { > + opp-hz = /bits/ 64 < 844800000 >; > + clock-latency-ns = <200000>; > + }; > + opp@960000000 { > + opp-hz = /bits/ 64 < 960000000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1036800000 { > + opp-hz = /bits/ 64 < 1036800000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1113600000 { > + opp-hz = /bits/ 64 < 1113600000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1190400000 { > + opp-hz = /bits/ 64 < 1190400000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1228800000 { > + opp-hz = /bits/ 64 < 1228800000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1324800000 { > + opp-hz = /bits/ 64 < 1324800000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1401600000 { > + opp-hz = /bits/ 64 < 1401600000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1478400000 { > + opp-hz = /bits/ 64 < 1478400000 >; > + clock-latency-ns = <200000>; > + }; > + opp@1593600000 { [...] > + > + }; > thermal-zones { > cpu-thermal0 { > polling-delay-passive = <250>; > @@ -403,7 +587,7 @@ > }; > > kryocc: clock-controller@6400000 { > - compatible = "qcom,apcc-msm8996"; > + compatible = "qcom-msm8996-apcc"; Bad change? > reg = <0x6400000 0x90000>; > #clock-cells = <1>; > }; > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c > index 3b585e4..b6cd0ae 100644 > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > @@ -95,6 +95,9 @@ > { .compatible = "xlnx,zynq-7000", }, > { .compatible = "xlnx,zynqmp", }, > > + { .compatible = "qcom,msm8996", }, > + { .compatible = "qcom,apq8096", }, > + Why can't we base it on the kryocc node being present? Or even populate the cpufreq-dt from the kryocc driver?
> -----Original Message----- > From: Stephen Boyd <sboyd@kernel.org> > Sent: Monday, March 19, 2018 18:49 > To: Ilia Lin <ilialin@codeaurora.org>; linux-arm-kernel@lists.infradead.org; > linux-arm-msm@vger.kernel.org; linux-clk@vger.kernel.org; > sboyd@codeaurora.org > Cc: mark.rutland@arm.com; devicetree@vger.kernel.org; > rnayak@codeaurora.org; robh@kernel.org; will.deacon@arm.com; > amit.kucheria@linaro.org; tfinkel@codeaurora.org; ilialin@codeaurora.org; > nicolas.dechesne@linaro.org; celster@codeaurora.org > Subject: Re: [PATCH v3 09/10] DT: QCOM: Add cpufreq-dt to msm8996 > > Quoting Ilia Lin (2018-02-14 05:59:51) > > diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi > > b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi > > index 492a011..930f68b 100644 > > --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi > > +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi > > @@ -1,5 +1,5 @@ > > /* > > - * Copyright (c) 2014-2016, The Linux Foundation. All rights reserved. > > + * Copyright (c) 2014-2016,2018 The Linux Foundation. All rights reserved. > > * > > * This program is free software; you can redistribute it and/or modify > > * it under the terms of the GNU General Public License version 2 and > > Why? The file was changed again in 2018. I'm reflecting this in the file dates. > > > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi > > b/arch/arm64/boot/dts/qcom/msm8996.dtsi > > index 4b2afcc..0359197 100644 > > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi > > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi > > @@ -144,6 +152,182 @@ > > }; > > }; > > > > + cluster0_opp: opp_table0 { > > + compatible = "operating-points-v2"; > > + opp-shared; > > + > > + opp@307200000 { > > + opp-hz = /bits/ 64 < 307200000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@422400000 { > > + opp-hz = /bits/ 64 < 422400000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@480000000 { > > It looks like opp-480000000 now instead of opp@<freq>. Will be changed in the next spin. > > > + opp-hz = /bits/ 64 < 480000000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@556800000 { > > + opp-hz = /bits/ 64 < 556800000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@652800000 { > > + opp-hz = /bits/ 64 < 652800000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@729600000 { > > + opp-hz = /bits/ 64 < 729600000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@844800000 { > > + opp-hz = /bits/ 64 < 844800000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@960000000 { > > + opp-hz = /bits/ 64 < 960000000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1036800000 { > > + opp-hz = /bits/ 64 < 1036800000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1113600000 { > > + opp-hz = /bits/ 64 < 1113600000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1190400000 { > > + opp-hz = /bits/ 64 < 1190400000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1228800000 { > > + opp-hz = /bits/ 64 < 1228800000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1324800000 { > > + opp-hz = /bits/ 64 < 1324800000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1401600000 { > > + opp-hz = /bits/ 64 < 1401600000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1478400000 { > > + opp-hz = /bits/ 64 < 1478400000 >; > > + clock-latency-ns = <200000>; > > + }; > > + opp@1593600000 { > [...] > > + > > + }; > > thermal-zones { > > cpu-thermal0 { > > polling-delay-passive = <250>; @@ -403,7 > > +587,7 @@ > > }; > > > > kryocc: clock-controller@6400000 { > > - compatible = "qcom,apcc-msm8996"; > > + compatible = "qcom-msm8996-apcc"; > > Bad change? This was required by Rob Herring. > > > reg = <0x6400000 0x90000>; > > #clock-cells = <1>; > > }; > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c > > b/drivers/cpufreq/cpufreq-dt-platdev.c > > index 3b585e4..b6cd0ae 100644 > > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > > @@ -95,6 +95,9 @@ > > { .compatible = "xlnx,zynq-7000", }, > > { .compatible = "xlnx,zynqmp", }, > > > > + { .compatible = "qcom,msm8996", }, > > + { .compatible = "qcom,apq8096", }, > > + > > Why can't we base it on the kryocc node being present? This could be good idea, if I would writing a platform specific cpufreq driver, which may be a future option. > Or even populate > the cpufreq-dt from the kryocc driver? There is a problem that during the clock probe the OPP table still doesn't exist.
Quoting ilialin@codeaurora.org (2018-03-20 06:46:19) > > From: Stephen Boyd <sboyd@kernel.org> > > Quoting Ilia Lin (2018-02-14 05:59:51) > > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c > > > b/drivers/cpufreq/cpufreq-dt-platdev.c > > > index 3b585e4..b6cd0ae 100644 > > > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > > > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > > > @@ -95,6 +95,9 @@ > > > { .compatible = "xlnx,zynq-7000", }, > > > { .compatible = "xlnx,zynqmp", }, > > > > > > + { .compatible = "qcom,msm8996", }, > > > + { .compatible = "qcom,apq8096", }, > > > + > > > > Why can't we base it on the kryocc node being present? > This could be good idea, if I would writing a platform specific cpufreq driver, which may be a future option. Well maybe cpufreq-dt can also look at the cpus nodes for a cpu with a certain type (in this case kryo). Then we can add cpufreq dt based on that CPU node being there. > > Or even populate > > the cpufreq-dt from the kryocc driver? > There is a problem that during the clock probe the OPP table still doesn't exist. The OPP table is in DT? Why doesn't it exist?
Understood, what you mean. I assumed that the cpufreq-dt is generic driver, and adding architecture specific code to it is no not correct from architectural point of view. Anyway, I'm considering adding a platform specific cpufreq driver, which will handle the efuse data as well. Meanwhile this is the way to use the generic driver. > -----Original Message----- > From: Stephen Boyd <sboyd@kernel.org> > Sent: Tuesday, March 20, 2018 22:06 > To: ilialin@codeaurora.org; linux-arm-kernel@lists.infradead.org; linux-arm- > msm@vger.kernel.org; linux-clk@vger.kernel.org; sboyd@codeaurora.org > Cc: mark.rutland@arm.com; devicetree@vger.kernel.org; > rnayak@codeaurora.org; robh@kernel.org; will.deacon@arm.com; > amit.kucheria@linaro.org; tfinkel@codeaurora.org; > nicolas.dechesne@linaro.org; celster@codeaurora.org > Subject: RE: [PATCH v3 09/10] DT: QCOM: Add cpufreq-dt to msm8996 > > Quoting ilialin@codeaurora.org (2018-03-20 06:46:19) > > > From: Stephen Boyd <sboyd@kernel.org> Quoting Ilia Lin (2018-02-14 > > > 05:59:51) > > > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c > > > > b/drivers/cpufreq/cpufreq-dt-platdev.c > > > > index 3b585e4..b6cd0ae 100644 > > > > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > > > > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > > > > @@ -95,6 +95,9 @@ > > > > { .compatible = "xlnx,zynq-7000", }, > > > > { .compatible = "xlnx,zynqmp", }, > > > > > > > > + { .compatible = "qcom,msm8996", }, > > > > + { .compatible = "qcom,apq8096", }, > > > > + > > > > > > Why can't we base it on the kryocc node being present? > > This could be good idea, if I would writing a platform specific cpufreq driver, > which may be a future option. > > Well maybe cpufreq-dt can also look at the cpus nodes for a cpu with a > certain type (in this case kryo). Then we can add cpufreq dt based on that > CPU node being there. > > > > Or even populate > > > the cpufreq-dt from the kryocc driver? > > There is a problem that during the clock probe the OPP table still doesn't > exist. > > The OPP table is in DT? Why doesn't it exist?
Quoting ilialin@codeaurora.org (2018-03-20 13:34:04) > Understood, what you mean. I assumed that the cpufreq-dt is generic driver, and adding architecture specific code to it is no not correct from architectural point of view. Anyway, I'm considering adding a platform specific cpufreq driver, which will handle the efuse data as well. > > Meanwhile this is the way to use the generic driver. > You'll need to Cc Viresh Kumar, who can comment further on cpufreq-dt. I wasn't recommending adding architecture specific code to cpufreq-dt, but at least updating how it handles CPU clk on/off state around hotplug would be good to do.
diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dts b/arch/arm64/boot/dts/qcom/apq8096-db820c.dts index 230e9c8..632cb42 100644 --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dts +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dts @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2016, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2016,2018 The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -17,5 +17,5 @@ / { model = "Qualcomm Technologies, Inc. DB820c"; - compatible = "arrow,apq8096-db820c", "qcom,apq8096-sbc"; + compatible = "arrow,apq8096-db820c", "qcom,apq8096-sbc", "qcom,apq8096"; }; diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi index 492a011..930f68b 100644 --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2016, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2016,2018 The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi index 4b2afcc..0359197 100644 --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi @@ -1,4 +1,4 @@ -/* Copyright (c) 2014-2015, The Linux Foundation. All rights reserved. +/* Copyright (c) 2014-2015,2018 The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -86,6 +86,8 @@ compatible = "qcom,kryo"; reg = <0x0 0x0>; enable-method = "psci"; + clocks = <&kryocc 0>; + operating-points-v2 = <&cluster0_opp>; next-level-cache = <&L2_0>; L2_0: l2-cache { compatible = "cache"; @@ -98,6 +100,8 @@ compatible = "qcom,kryo"; reg = <0x0 0x1>; enable-method = "psci"; + clocks = <&kryocc 0>; + operating-points-v2 = <&cluster0_opp>; next-level-cache = <&L2_0>; }; @@ -106,6 +110,8 @@ compatible = "qcom,kryo"; reg = <0x0 0x100>; enable-method = "psci"; + clocks = <&kryocc 1>; + operating-points-v2 = <&cluster1_opp>; next-level-cache = <&L2_1>; L2_1: l2-cache { compatible = "cache"; @@ -118,6 +124,8 @@ compatible = "qcom,kryo"; reg = <0x0 0x101>; enable-method = "psci"; + clocks = <&kryocc 1>; + operating-points-v2 = <&cluster1_opp>; next-level-cache = <&L2_1>; }; @@ -144,6 +152,182 @@ }; }; + cluster0_opp: opp_table0 { + compatible = "operating-points-v2"; + opp-shared; + + opp@307200000 { + opp-hz = /bits/ 64 < 307200000 >; + clock-latency-ns = <200000>; + }; + opp@422400000 { + opp-hz = /bits/ 64 < 422400000 >; + clock-latency-ns = <200000>; + }; + opp@480000000 { + opp-hz = /bits/ 64 < 480000000 >; + clock-latency-ns = <200000>; + }; + opp@556800000 { + opp-hz = /bits/ 64 < 556800000 >; + clock-latency-ns = <200000>; + }; + opp@652800000 { + opp-hz = /bits/ 64 < 652800000 >; + clock-latency-ns = <200000>; + }; + opp@729600000 { + opp-hz = /bits/ 64 < 729600000 >; + clock-latency-ns = <200000>; + }; + opp@844800000 { + opp-hz = /bits/ 64 < 844800000 >; + clock-latency-ns = <200000>; + }; + opp@960000000 { + opp-hz = /bits/ 64 < 960000000 >; + clock-latency-ns = <200000>; + }; + opp@1036800000 { + opp-hz = /bits/ 64 < 1036800000 >; + clock-latency-ns = <200000>; + }; + opp@1113600000 { + opp-hz = /bits/ 64 < 1113600000 >; + clock-latency-ns = <200000>; + }; + opp@1190400000 { + opp-hz = /bits/ 64 < 1190400000 >; + clock-latency-ns = <200000>; + }; + opp@1228800000 { + opp-hz = /bits/ 64 < 1228800000 >; + clock-latency-ns = <200000>; + }; + opp@1324800000 { + opp-hz = /bits/ 64 < 1324800000 >; + clock-latency-ns = <200000>; + }; + opp@1401600000 { + opp-hz = /bits/ 64 < 1401600000 >; + clock-latency-ns = <200000>; + }; + opp@1478400000 { + opp-hz = /bits/ 64 < 1478400000 >; + clock-latency-ns = <200000>; + }; + opp@1593600000 { + opp-hz = /bits/ 64 < 1593600000 >; + clock-latency-ns = <200000>; + }; + }; + + cluster1_opp: opp_table1 { + compatible = "operating-points-v2"; + opp-shared; + + opp@307200000 { + opp-hz = /bits/ 64 < 307200000 >; + clock-latency-ns = <200000>; + }; + opp@403200000 { + opp-hz = /bits/ 64 < 403200000 >; + clock-latency-ns = <200000>; + }; + opp@480000000 { + opp-hz = /bits/ 64 < 480000000 >; + clock-latency-ns = <200000>; + }; + opp@556800000 { + opp-hz = /bits/ 64 < 556800000 >; + clock-latency-ns = <200000>; + }; + opp@652800000 { + opp-hz = /bits/ 64 < 652800000 >; + clock-latency-ns = <200000>; + }; + opp@729600000 { + opp-hz = /bits/ 64 < 729600000 >; + clock-latency-ns = <200000>; + }; + opp@806400000 { + opp-hz = /bits/ 64 < 806400000 >; + clock-latency-ns = <200000>; + }; + opp@883200000 { + opp-hz = /bits/ 64 < 883200000 >; + clock-latency-ns = <200000>; + }; + opp@940800000 { + opp-hz = /bits/ 64 < 940800000 >; + clock-latency-ns = <200000>; + }; + opp@1036800000 { + opp-hz = /bits/ 64 < 1036800000 >; + clock-latency-ns = <200000>; + }; + opp@1113600000 { + opp-hz = /bits/ 64 < 1113600000 >; + clock-latency-ns = <200000>; + }; + opp@1190400000 { + opp-hz = /bits/ 64 < 1190400000 >; + clock-latency-ns = <200000>; + }; + opp@1248000000 { + opp-hz = /bits/ 64 < 1248000000 >; + clock-latency-ns = <200000>; + }; + opp@1324800000 { + opp-hz = /bits/ 64 < 1324800000 >; + clock-latency-ns = <200000>; + }; + opp@1401600000 { + opp-hz = /bits/ 64 < 1401600000 >; + clock-latency-ns = <200000>; + }; + opp@1478400000 { + opp-hz = /bits/ 64 < 1478400000 >; + clock-latency-ns = <200000>; + }; + opp@1552000000 { + opp-hz = /bits/ 64 < 1552000000 >; + clock-latency-ns = <200000>; + }; + opp@1632000000 { + opp-hz = /bits/ 64 < 1632000000 >; + clock-latency-ns = <200000>; + }; + opp@1708800000 { + opp-hz = /bits/ 64 < 1708800000 >; + clock-latency-ns = <200000>; + }; + opp@1785600000 { + opp-hz = /bits/ 64 < 1785600000 >; + clock-latency-ns = <200000>; + }; + opp@1824000000 { + opp-hz = /bits/ 64 < 1824000000 >; + clock-latency-ns = <200000>; + }; + opp@1920000000 { + opp-hz = /bits/ 64 < 1920000000 >; + clock-latency-ns = <200000>; + }; + opp@1996800000 { + opp-hz = /bits/ 64 < 1996800000 >; + clock-latency-ns = <200000>; + }; + opp@2073600000 { + opp-hz = /bits/ 64 < 2073600000 >; + clock-latency-ns = <200000>; + }; + opp@2150400000 { + opp-hz = /bits/ 64 < 2150400000 >; + clock-latency-ns = <200000>; + }; + + }; thermal-zones { cpu-thermal0 { polling-delay-passive = <250>; @@ -403,7 +587,7 @@ }; kryocc: clock-controller@6400000 { - compatible = "qcom,apcc-msm8996"; + compatible = "qcom-msm8996-apcc"; reg = <0x6400000 0x90000>; #clock-cells = <1>; }; diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index 3b585e4..b6cd0ae 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -95,6 +95,9 @@ { .compatible = "xlnx,zynq-7000", }, { .compatible = "xlnx,zynqmp", }, + { .compatible = "qcom,msm8996", }, + { .compatible = "qcom,apq8096", }, + { } };
Add device tree frequency table for the MSM8996 to be used by the upstream cpufreq-dt driver with the clk-cpu-8996 driver as infrastructure. Signed-off-by: Ilia Lin <ilialin@codeaurora.org> --- arch/arm64/boot/dts/qcom/apq8096-db820c.dts | 4 +- arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 +- arch/arm64/boot/dts/qcom/msm8996.dtsi | 188 ++++++++++++++++++++++++++- drivers/cpufreq/cpufreq-dt-platdev.c | 3 + 4 files changed, 192 insertions(+), 5 deletions(-)