[PATCH v2] arm64: dts: rockchip: Add OPP voltage ranges to RK3399 OP1 SoC dtsi

[Dragan Simic]

On 2024-11-06 12:24, Heiko Stübner wrote:
> Am Mittwoch, 6. November 2024, 11:45:06 CET schrieb Dragan Simic:
>> 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 at cherry.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.

Ah, I see, thanks for the clarification.  I'm willing to take the
resulting blame, though. :)

> If you're actually doing the Tinkerboard and thus adding new things 
> this
> changes the whole judgement a bit too.

Yes, I need an OP1-based board for my upcoming Rockchip SoC binning
endeavor, which for me basically boils down to getting a TinkerBoard
2S.  Of course, I need to have my future TinkerBoard 2S working and
running mainline kernel, and what's a better way for doing that than
having its board dts upstreamed, for everyone's benefit. :)

> 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 :-)

Yes, I see the RK3399 as an actively maintained part of the kernel. :)
With all that in mind, I hope the associated cleanups will be seen as
justified.