On 18-08-25, 13:52, Krishna Chaitanya Chundru wrote: > @@ -2210,45 +2213,67 @@ pcie1_opp_table: opp-table { > compatible = "operating-points-v2"; > > /* GEN 1 x1 */ > - opp-2500000 { > + opp-2500000-1 { Why mention -1 here when there is only one entry with this freq value ? > opp-hz = /bits/ 64 <2500000>; > required-opps = <&rpmhpd_opp_low_svs>; > opp-peak-kBps = <250000 1>; > + opp-level = <1>; > }; > > - /* GEN 1 x2 and GEN 2 x1 */ > - opp-5000000 { > + /* GEN 1 x2 */ > + opp-5000000-1 { > + opp-hz = /bits/ 64 <5000000>; > + required-opps = <&rpmhpd_opp_low_svs>; > + opp-peak-kBps = <500000 1>; > + opp-level = <1>; > + }; > + > + /* GEN 2 x1 */ > + opp-5000000-2 { > opp-hz = /bits/ 64 <5000000>; > required-opps = <&rpmhpd_opp_low_svs>; > opp-peak-kBps = <500000 1>; > + opp-level = <2>; > }; This looks okay. > > /* GEN 2 x2 */ > - opp-10000000 { > + opp-10000000-2 { Why -2 here ? > opp-hz = /bits/ 64 <10000000>; > required-opps = <&rpmhpd_opp_low_svs>; > opp-peak-kBps = <1000000 1>; > + opp-level = <2>; > }; > > /* GEN 3 x1 */ > - opp-8000000 { > + opp-8000000-3 { same. > opp-hz = /bits/ 64 <8000000>; > required-opps = <&rpmhpd_opp_nom>; > opp-peak-kBps = <984500 1>; > + opp-level = <3>; > + }; > + > + /* GEN 3 x2 */ > + opp-16000000-3 { Shouldn't this be opp-16000000-1 only ? This is the first occurrence 16000000. > + opp-hz = /bits/ 64 <16000000>; > + required-opps = <&rpmhpd_opp_nom>; > + opp-peak-kBps = <1969000 1>; > + opp-level = <3>; > }; > > - /* GEN 3 x2 and GEN 4 x1 */ > - opp-16000000 { > + /* GEN 4 x1 */ > + opp-16000000-4 { opp-16000000-2 ? -- viresh