Re: [PATCH 1/2] arm: boot: dts: am4372: add operating points

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 4586 bytes --]

Hi,

On Sun, May 10, 2015 at 07:23:34PM -0500, Nishanth Menon wrote:
> On 05/08/2015 03:24 PM, Felipe Balbi wrote:
> > On Fri, May 08, 2015 at 03:12:27PM -0500, Nishanth Menon wrote:
> >> On 05/08/2015 03:09 PM, Nishanth Menon wrote:
> >>> On 05/08/2015 02:57 PM, Felipe Balbi wrote:
> >>>> By adding operating points, cpufreq-dt has a
> >>>> chance of running and doing something useful.
> >>>>
> >>>> Signed-off-by: Felipe Balbi <balbi@ti.com>
> >>>> ---
> >>>>  arch/arm/boot/dts/am4372.dtsi | 9 +++++++++
> >>>>  1 file changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> >>>> index c80a3e233792..ea1db20f64fc 100644
> >>>> --- a/arch/arm/boot/dts/am4372.dtsi
> >>>> +++ b/arch/arm/boot/dts/am4372.dtsi
> >>>> @@ -38,6 +38,15 @@
> >>>>  			clocks = <&dpll_mpu_ck>;
> >>>>  			clock-names = "cpu";
> >>>>  
> >>>> +			operating-points = <
> >>>> +				/* kHz		uV */
> >>>> +				1000000		1325000 /* OPP_NITRO */
> >>>> +				 800000		1260000 /* OPP_TURBO */
> >>>> +				 720000		1200000 /* OPP_120 */
> >>>> +				 600000		1100000 /* OPP_100 */
> >>>> +				 300000		 950000 /* OPP_50 */
> >>>> +			>;
> >>>> +
> >>>>  			clock-latency = <300000>; /* From omap-cpufreq driver */
> >>>>  		};
> >>>>  	};
> >>>>
> >>> which of these OPPs need AVS? which of these are dependent on Efuse bit
> >>> dependent?
> >>>
> >>
> >>
> >> You can use
> >> http://git.ti.com/ti-linux-kernel/ti-linux-kernel/blobs/ti-linux-3.14.y/arch/arm/mach-omap2/opp43xx_data.c
> >> for reference.
> > 
> > heh, why isn't that upstream yet ? Seems to be ready already. The point
> 
> we have tried in the past[1] - unfortunately that was just bandaid since
> the existing OPP device tree bindings have very limiting behavior across
> multiple SoCs.This was one of the reasons why we stopped adding more
> OPPs, since we are hoping to see the new bindings[2] which is under
> discussion settle down and help enable support for cases like that we
> have on TI SoCs, iMX SoCs etc.

fair enough.

> > is that as of now, u-boot will set maximum OPP it can find and, for
> > AM437x, that will be 800MHz or 1GHz depending on your board. 1GHz might
> > not be supported in all SoCs and letting that be used all the time is
> > likely going to reduce silicon lifetime.
> 
> > At least allowing ondemand governor run, we will be mostly running at
> > 300MHz and only jump to "invalid" OPPs under load which, granted, is
> > still not perfect, but better than running at 1GHz all the time, don't
> > you agree ?
> The fix for this should ideally be in u-boot - we are trying not to

right, ideally, yeah; but then we go back to the discussion regarding
kernel vs bootloader dependencies :-)

> introduce dts changes in the hopes that the new proposed bindings that
> Viresh has on discussion comes to conclusion and help introduce new dtb
> support with proper SoC description. allowing an SoC to enter an invalid
> OPP is broken by design. we must attempt to curb it. unfortunately, we
> already do have a broken implementation for AM335x, DRA7 in place which
> went with the assumption that we will be able to do modifiers on top of
> existing dt description and the hopes that [1] will eventually get
> upstream. But, as is clear now, [1] has no future path in upstream
> kernel. following the same broken path for newer SoC definitions will
> probably very short sighted by us.
> 
> in my opinion, doing a temporary hack in upstream kernel is not an
> elegant approach. I suggest helping review and approving Viresh's new

however this is not a hack, right ? If we get rid of OPP_NITRO and
OPP_TURBO, then we will more than likely always be dealing with safe
OPPs (yeah, I need to confirm this since it's not on public TRM, so as
of now, take this statement with a grain of salt :-), moreover, even
though we're trying to change opp bindings, the current situation is
still very much accepted and will remain valid even after changing
binding :-)

Not to mention that people using AM43xx today might be using it under
invalid OPPs and decreasing silicon life; I'd assume that's a very
urgent detail to sort out.

> bindings so that we have a longer term solution is more the way to do this.
> 
> Just my 2 cents. Thanks once again for identifying the urgent need for
> helping Viresh's series along - will be great if folks could help review
> and approve his series to get them upstream and help all of us ARM SoC
> variations along.

np.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]