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

[Dragan Simic <dsimic@manjaro.org>]

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@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?

>> So not sure we need to handle theoretical cases here. Will let
>> maintainers decide on that one. FWIW, there are two other OP1 devices,
>> the RockPi4A+ and RockPi4B+ which do not change the OPP either.