Message-ID: <3515804.QJadu78ljV@diego>
Date: Wed, 06 Nov 2024 12:24:41 +0100
From: Heiko Stübner <heiko@...ech.de>
To: Dragan Simic <dsimic@...jaro.org>
Cc: linux-rockchip@...ts.infradead.org,
 Quentin Schulz <quentin.schulz@...rry.de>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, alchark@...il.com
Subject:
 Re: [PATCH v2] arm64: dts: rockchip: Add OPP voltage ranges to RK3399 OP1 SoC
 dtsi

Am Mittwoch, 6. November 2024, 11:45:06 CET schrieb Dragan Simic:
> Hello Heiko,
> 
> On 2024-11-06 10:41, Heiko Stübner wrote:
> > Am Mittwoch, 6. November 2024, 10:32:06 CET schrieb Quentin Schulz:
> >> On 11/6/24 9:33 AM, Dragan Simic wrote:
> >> > Add support for voltage ranges to the CPU, GPU and DMC OPPs defined in the
> >> > SoC dtsi for Rockchip OP1, as a variant of the Rockchip RK3399.  This may be
> >> > useful if there are any OP1-based boards whose associated voltage regulators
> >> > are unable to deliver the exact voltages; otherwise, it causes no functional
> >> > changes to the resulting OPP voltages at runtime.
> >> >
> >> > These changes cannot cause stability issues or any kind of damage, because
> >> > it's perfectly safe to use the highest voltage from an OPP group for each OPP
> >> > in the same group.  The only possible negative effect of using higher voltages
> >> > is wasted energy in form of some additionally generated heat.
> >> >
> >> > Reported-by: Quentin Schulz <quentin.schulz@...rry.de>
> >> 
> >> Well, I merely highlighted that the voltage was different on OP1
> >> compared to RK3399 for the 600MHz OPP :)
> >> 
> >> So... If there's ONE SoC I'm pretty sure is working as expected it's 
> >> the
> >> OP1 fitted on the Gru Chromebooks with the ChromiumOS kernel fork
> >> (though yes, I believe all Gru CB are EoL since August 2023). In the 
> >> 6.1
> >> kernel fork, there's also no range:
> >> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-6.1/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
> > 
> > yeah, this somehow goes quite a bit into the "stuff that doesn't need 
> > to
> > change" area. On the one hand it does make "some" sense to unify things
> > if we're using ranges everywhere else.
> 
> I agree that it might be unneeded, but there's no possible harm, and
> unifying the things may be seen as beneficial.
> 
> > On the other hand, as Quentin noted below, all existing OP1 devices 
> > seem
> > to run just fine, and there won't be any more entering the kernel.
> 
> Hmm, why can't we add more OP1-based devices?  As I mentioned in my
> earlier response to Quentin, my plan is to implement the board dts
> for OP1-based TinkerBoard 2S, so I'd like to know is there something
> that might prevent that board dts from becoming merged?
> 
> > So what do we realisitically gain here, except hiding existing 
> > git-history
> > under another layer?
> 
> Sorry, I'm not sure what would become hidden by this patch?

When you change a part of the file, a git blame points to you,
hiding the previous blame, so it makes traversing history a tiny
bit harder.

If you're actually doing the Tinkerboard and thus adding new things this
changes the whole judgement a bit too.
Like I was on the mindset-road of rk3399 is mostly done in terms of new
boards, so what's in the kernel now will at max get some new peripherals
but is in general already mostly working.

If we're still adding new rk3399 boards, it does make more sense to go
back and make the underlying parts nicer :-)


Heiko

Powered by blists - more mailing lists