On 20-03-24, 15:37, Andre Przywara wrote:
> On Wed, 20 Mar 2024 10:02:28 -0500
> Rob Herring <robh@xxxxxxxxxx> wrote:
> > On Mon, Mar 18, 2024 at 01:12:23AM +0000, Andre Przywara wrote:
> > > From: Martin Botka <martin.botka@xxxxxxxxxxxxxx>
> > > - opp-1080000000 {
> > > + opp-792000000-l {
> > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > - opp-hz = /bits/ 64 <1080000000>;
> > > + opp-hz = /bits/ 64 <792000000>;
> > >
> > > - opp-microvolt-speed0 = <1060000>;
> > > - opp-microvolt-speed1 = <880000>;
> > > - opp-microvolt-speed2 = <840000>;
> > > + opp-microvolt = <900000>;
> > > + opp-supported-hw = <0x02>;
> > > };
> > >
> > > - opp-1320000000 {
> > > + opp-792000000-h {
> > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > - opp-hz = /bits/ 64 <1320000000>;
> > > + opp-hz = /bits/ 64 <792000000>;
> > >
> > > - opp-microvolt-speed0 = <1160000>;
> > > - opp-microvolt-speed1 = <940000>;
> > > - opp-microvolt-speed2 = <900000>;
> > > + opp-microvolt = <940000>;
> > > + opp-supported-hw = <0x10>;
> >
> > So far, we've avoided multiple entries for a single frequency. I think
> > it would be good to maintain that.
>
> Fair, I wasn't super happy with that either, but it still seemed better
> than the alternatives.
>
> > Couldn't you just do:
> >
> > opp-supported-hw = <0>, <0x10>, <0x02>;
> >
> > Where the index corresponds to speed0, speed1, speed2.
> >
> > If not, then I don't understand how multiple entries of opp-supported-hw
> > are supposed to work.
>
> If I got this correctly, multiple cells in opp-supported-hw are to
> describe various levels of hierarchy for a chip version, so like silicon
> mask, metal layer revision, bin, I guess? The binding doc speaks of "cuts,
> substrate and process", not really sure what that means exactly.
Right. That basically translates to hardware versions the OPP will be parsed
for.
> I think currently we cannot easily combine microvolt suffixes and
> opp-supported-hw in one OPP node?
It should be fine.
> I think it bails out if one
> microvolt-speed<x> property is missing, but I have to double check.
> But IIRC v1 of this series somehow pulled that off, so we can maybe bring
> it back? To end up with:
> opp-792 {
> opp-hz = <792000000>;
> opp-microvolt-speed1 = <900000>;
> opp-microvolt-speed4 = <940000>;
> opp-supported-hw = <0x12>;
> };
That's what I thought too while reading your email.. Just populate the OPP for
both 0x10 and 0x02 versions and let the speedN thing get you the right voltage.
--
viresh
