| Message ID | 20190819034240.6569-1-shrirang.bagul@canonical.com |
|---|---|
| Headers | show |
| Series | Support cpufreq, thermal sensors & cooling cells on iMX6Q based Nitrogen6x board | expand |
On Mon, Aug 19, 2019 at 5:43 AM Shrirang Bagul <shrirang.bagul@canonical.com> wrote: > > BugLink: https://bugs.launchpad.net/bugs/1840437 There's something weird between patch 2 and 3. Patch 2: ... The OPP properties, like "operating-points", 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. ... and adds such property to cpu1: --- a/arch/arm/boot/dts/imx7d.dtsi +++ b/arch/arm/boot/dts/imx7d.dtsi @@ -59,6 +59,11 @@ compatible = "arm,cortex-a7"; device_type = "cpu"; reg = <1>; + operating-points = < + /* KHz uV */ + 996000 1075000 + 792000 975000 + >; ... afterward, in patch 3, "operating points" is replaced with "operating-points-v2", but that is only done for cpu0: --- a/arch/arm/boot/dts/imx7d.dtsi +++ b/arch/arm/boot/dts/imx7d.dtsi @@ -47,12 +47,8 @@ / { cpus { cpu0: cpu@0 { - operating-points = < - /* KHz uV */ - 996000 1075000 - 792000 975000 - >; clock-frequency = <996000000>; + operating-points-v2 = <&cpu0_opp_table>; }; cpu1: cpu@1 { @@ -65,6 +61,25 @@ 792000 975000 >; clock-frequency = <996000000>; + operating-points-v2 = <&cpu0_opp_table>; ... and this is the final result: ... cpus { cpu0: cpu@0 { clock-frequency = <996000000>; operating-points-v2 = <&cpu0_opp_table>; #cooling-cells = <2>; }; cpu1: cpu@1 { compatible = "arm,cortex-a7"; device_type = "cpu"; reg = <1>; operating-points = < /* KHz uV */ 996000 1075000 792000 975000 >; clock-frequency = <996000000>; operating-points-v2 = <&cpu0_opp_table>; }; }; ... Could you check that?
On Mon, 2019-08-19 at 10:59 +0200, Paolo Pisati wrote: > On Mon, Aug 19, 2019 at 5:43 AM Shrirang Bagul > <shrirang.bagul@canonical.com> wrote: > > > > BugLink: https://bugs.launchpad.net/bugs/1840437 > > There's something weird between patch 2 and 3. > > Patch 2: > ... > The OPP properties, like "operating-points", 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. > ... > > and adds such property to cpu1: > > --- a/arch/arm/boot/dts/imx7d.dtsi > +++ b/arch/arm/boot/dts/imx7d.dtsi > @@ -59,6 +59,11 @@ > compatible = "arm,cortex-a7"; > device_type = "cpu"; > reg = <1>; > + operating-points = < > + /* KHz uV */ > + 996000 1075000 > + 792000 975000 > + >; > ... > > afterward, in patch 3, "operating points" is replaced with > "operating-points-v2", but that is only done for cpu0: > > --- a/arch/arm/boot/dts/imx7d.dtsi > +++ b/arch/arm/boot/dts/imx7d.dtsi > @@ -47,12 +47,8 @@ > / { > cpus { > cpu0: cpu@0 { > - operating-points = < > - /* KHz uV */ > - 996000 1075000 > - 792000 975000 > - >; > clock-frequency = <996000000>; > + operating-points-v2 = <&cpu0_opp_table>; > }; > > cpu1: cpu@1 { > @@ -65,6 +61,25 @@ > 792000 975000 > >; > clock-frequency = <996000000>; > + operating-points-v2 = <&cpu0_opp_table>; > ... > > and this is the final result: > > ... > cpus { > cpu0: cpu@0 { > clock-frequency = <996000000>; > operating-points-v2 = <&cpu0_opp_table>; > #cooling-cells = <2>; > }; > > cpu1: cpu@1 { > compatible = "arm,cortex-a7"; > device_type = "cpu"; > reg = <1>; > operating-points = < > /* KHz uV */ > 996000 1075000 > 792000 975000 > >; > clock-frequency = <996000000>; > operating-points-v2 = <&cpu0_opp_table>; > }; > }; > ... > > Could you check that? Right, requires another upstream patch to fix this anomaly. commit 33a8d5a595dd0f9b7f801c1cddb26dc05bc33a73 Author: Anson Huang <Anson.Huang@nxp.com> Date: Thu Jul 19 16:24:19 2018 +0800 ARM: dts: imx7d: remove "operating-points" property for cpu1 Commit b97872d4eb22 ("ARM: dts: imx: Add missing OPP properties for CPUs") added "operating-points" property for all CPUs, but i.MX7D already has "operating-points-v2" property on both CPUs, so no need to add "operating-points" property again, this patch removes it. Fixes: b97872d4eb22 ("ARM: dts: imx: Add missing OPP properties for CPUs") Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Signed-off-by: Shawn Guo <shawnguo@kernel.org> Thank you for the review. NAK'ing this series. Will resend with necessary updates.
Superseded by v3, with updates to imx7d cpufreq properties. On Mon, 2019-08-19 at 11:42 +0800, Shrirang Bagul wrote: > BugLink: https://bugs.launchpad.net/bugs/1840437 > > [Impact] > We ship the armhf flavour of the Bionic kernel which supports the iMX6Q > based Nitrogen6x board. The cpufreq, thermal sensors and cooling cells > should be enabled for this board. > > [Fix] > Backport upstream device tree patches to define the device nodes properly. > > [Test] > Tested and verified on the Nitrogen6x board. > > [Regression Potential] > Low. These patches update DT nodes with required properties for the > imx6q_cpufreq and imx_thermal drivers function properly on the platform. > > Anson Huang (2): > ARM: dts: imx7d: use operating-points-v2 for cpu > ARM: dts: imx: add cooling-cells for cpufreq cooling device > > Lucas Stach (1): > ARM: dts: imx6: add thermal sensor and cooling cells > > Nicolas Chauvet (1): > arm: imx: Add MODULE_ALIAS for cpufreq > > Viresh Kumar (1): > ARM: dts: imx: Add missing OPP properties for CPUs > > arch/arm/boot/dts/imx6dl.dtsi | 24 ++++++++ > arch/arm/boot/dts/imx6q-cm-fx6.dts | 66 ++++++++++++++++++++++ > arch/arm/boot/dts/imx6q.dtsi | 89 +++++++++++++++++++++++++++++- > arch/arm/boot/dts/imx6qdl.dtsi | 3 + > arch/arm/boot/dts/imx6sl.dtsi | 1 + > arch/arm/boot/dts/imx6sx.dtsi | 1 + > arch/arm/boot/dts/imx6ul.dtsi | 1 + > arch/arm/boot/dts/imx7d.dtsi | 31 +++++++++-- > drivers/cpufreq/imx6q-cpufreq.c | 1 + > 9 files changed, 209 insertions(+), 8 deletions(-) > > -- > 2.17.1 > >