| Message ID | 5821a6dbe413b5a217ca1e24ddf8ebfa63ba6ef0.1527244201.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
[Cc Biju Das] On Fri, May 25, 2018 at 04:01:59PM +0530, Viresh Kumar wrote: > 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. 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 create an OPP > table. > > Add such missing properties. > > Fix other missing property (clock latency) as well to make it all > work. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Thanks, this looks good to me and it looks like it should have: Fixes: 0417814ea140 ("ARM: dts: r8a7743: Add OPP table for frequency scaling") Biju, as the author of the above patch could you take a look over this fix? > --- > arch/arm/boot/dts/r8a7743.dtsi | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi > index 142949d7066f..e4fb31c4f0ee 100644 > --- a/arch/arm/boot/dts/r8a7743.dtsi > +++ b/arch/arm/boot/dts/r8a7743.dtsi > @@ -98,8 +98,17 @@ > reg = <1>; > clock-frequency = <1500000000>; > clocks = <&cpg CPG_CORE R8A7743_CLK_Z>; > + clock-latency = <300000>; /* 300 us */ > power-domains = <&sysc R8A7743_PD_CA15_CPU1>; > next-level-cache = <&L2_CA15>; > + > + /* kHz - uV - OPPs unknown yet */ > + operating-points = <1500000 1000000>, > + <1312500 1000000>, > + <1125000 1000000>, > + < 937500 1000000>, > + < 750000 1000000>, > + < 375000 1000000>; > }; > > L2_CA15: cache-controller-0 { > -- > 2.15.0.194.g9af6a3dea062 >
On 28-05-18, 11:23, Simon Horman wrote: > [Cc Biju Das] > > On Fri, May 25, 2018 at 04:01:59PM +0530, Viresh Kumar wrote: > > 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. 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 create an OPP > > table. > > > > Add such missing properties. > > > > Fix other missing property (clock latency) as well to make it all > > work. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > Thanks, this looks good to me and it looks like it should have: > > Fixes: 0417814ea140 ("ARM: dts: r8a7743: Add OPP table for frequency scaling") Sure. Will you be picking this patch directly and send it part of your pull request ? Maybe add Fixes tag then only ?
On Mon, May 28, 2018 at 04:28:31PM +0530, Viresh Kumar wrote: > On 28-05-18, 11:23, Simon Horman wrote: > > [Cc Biju Das] > > > > On Fri, May 25, 2018 at 04:01:59PM +0530, Viresh Kumar wrote: > > > 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. 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 create an OPP > > > table. > > > > > > Add such missing properties. > > > > > > Fix other missing property (clock latency) as well to make it all > > > work. > > > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > Thanks, this looks good to me and it looks like it should have: > > > > Fixes: 0417814ea140 ("ARM: dts: r8a7743: Add OPP table for frequency scaling") > > Sure. > > Will you be picking this patch directly and send it part of your pull > request ? Maybe add Fixes tag then only ? Yes, that is my plan. I can handle adding the Fixes tag. But I'll wait to see if Bjiu has an feedback first.
Hi All, I have tested this patch on RZ/G1M and I didn't find any issues. r8a7743 is similar to r8a7791. So I assume you will apply the same patch for other R-SoC devices as well. Apart from this, maybe we need to update the OPP binding documentation. i.e., extend the operating- point usage to other cores in the cluster (Binding 1: operating-points). Regards, Biju > -----Original Message----- > From: Simon Horman [mailto:horms@verge.net.au] > Sent: 28 May 2018 12:59 > To: Viresh Kumar <viresh.kumar@linaro.org> > Cc: arm@kernel.org; Magnus Damm <magnus.damm@gmail.com>; Rob > Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; > Vincent Guittot <vincent.guittot@linaro.org>; ionela.voinescu@arm.com; > Daniel Lezcano <daniel.lezcano@linaro.org>; chris.redpath@arm.com; linux- > renesas-soc@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Biju Das <biju.das@bp.renesas.com> > Subject: Re: [PATCH 13/15] arm: dts: r8a7743: Add missing OPP properties for > CPUs > > On Mon, May 28, 2018 at 04:28:31PM +0530, Viresh Kumar wrote: > > On 28-05-18, 11:23, Simon Horman wrote: > > > [Cc Biju Das] > > > > > > On Fri, May 25, 2018 at 04:01:59PM +0530, Viresh Kumar wrote: > > > > 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. 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 create an OPP table. > > > > > > > > Add such missing properties. > > > > > > > > Fix other missing property (clock latency) as well to make it all > > > > work. > > > > > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > > > Thanks, this looks good to me and it looks like it should have: > > > > > > Fixes: 0417814ea140 ("ARM: dts: r8a7743: Add OPP table for frequency > > > scaling") > > > > Sure. > > > > Will you be picking this patch directly and send it part of your pull > > request ? Maybe add Fixes tag then only ? > > Yes, that is my plan. I can handle adding the Fixes tag. > But I'll wait to see if Bjiu has an feedback first. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
On 29-05-18, 13:33, Biju Das wrote: > Hi All, > > I have tested this patch on RZ/G1M and I didn't find any issues. > r8a7743 is similar to r8a7791. So I assume you will apply the same > patch for other R-SoC devices as well. Okay, I have sent V2 for this and it fixes all those platforms as well. > Apart from this, maybe we need to update the OPP binding > documentation. i.e., extend the operating- point usage to other > cores in the cluster (Binding 1: operating-points). Its already done, you need to start using operating-points-v2 property to avoid duplication like this.
diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi index 142949d7066f..e4fb31c4f0ee 100644 --- a/arch/arm/boot/dts/r8a7743.dtsi +++ b/arch/arm/boot/dts/r8a7743.dtsi @@ -98,8 +98,17 @@ reg = <1>; clock-frequency = <1500000000>; clocks = <&cpg CPG_CORE R8A7743_CLK_Z>; + clock-latency = <300000>; /* 300 us */ power-domains = <&sysc R8A7743_PD_CA15_CPU1>; next-level-cache = <&L2_CA15>; + + /* kHz - uV - OPPs unknown yet */ + operating-points = <1500000 1000000>, + <1312500 1000000>, + <1125000 1000000>, + < 937500 1000000>, + < 750000 1000000>, + < 375000 1000000>; }; L2_CA15: cache-controller-0 {
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. 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 create an OPP table. Add such missing properties. Fix other missing property (clock latency) as well to make it all work. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- arch/arm/boot/dts/r8a7743.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+)