| Message ID | 1513698900-10638-16-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19-12-17, 21:25, Sricharan R wrote: > + cpu@0 { > + compatible = "qcom,krait"; > + enable-method = "qcom,kpss-acc-v1"; > + device_type = "cpu"; > + reg = <0>; > + qcom,acc = <&acc0>; > + qcom,saw = <&saw0>; > + clocks = <&kraitcc 0>; > + clock-names = "cpu"; > + cpu-supply = <&smb208_s2a>; > + operating-points-v2 = <&cpu_opp_table>; > + }; > + > + qcom,pvs { > + qcom,pvs-format-a; > + }; Not sure what Rob is going to say on that :) > + > + > + cpu_opp_table: opp_table { > + compatible = "operating-points-v2"; > + > + /* > + * Missing opp-shared property means CPUs switch DVFS states > + * independently. > + */ > + > + opp-1400000000 { > + opp-hz = /bits/ 64 <1400000000>; > + opp-microvolt-speed0-pvs0-v0 = <1250000>; Why speed0 and v0 in all the names ?
Hi Viresh, On 12/20/2017 8:56 AM, Viresh Kumar wrote: > On 19-12-17, 21:25, Sricharan R wrote: >> + cpu@0 { >> + compatible = "qcom,krait"; >> + enable-method = "qcom,kpss-acc-v1"; >> + device_type = "cpu"; >> + reg = <0>; >> + qcom,acc = <&acc0>; >> + qcom,saw = <&saw0>; >> + clocks = <&kraitcc 0>; >> + clock-names = "cpu"; >> + cpu-supply = <&smb208_s2a>; >> + operating-points-v2 = <&cpu_opp_table>; >> + }; >> + >> + qcom,pvs { >> + qcom,pvs-format-a; >> + }; > > Not sure what Rob is going to say on that :) > Yes. Would be good to know the best way. >> + >> + >> + cpu_opp_table: opp_table { >> + compatible = "operating-points-v2"; >> + >> + /* >> + * Missing opp-shared property means CPUs switch DVFS states >> + * independently. >> + */ >> + >> + opp-1400000000 { >> + opp-hz = /bits/ 64 <1400000000>; >> + opp-microvolt-speed0-pvs0-v0 = <1250000>; > > Why speed0 and v0 in all the names ? > Ya, all the three (speed, pvs and version) are read from efuse. So all the three can vary. Regards, Sricharan
On 20-12-17, 11:55, Sricharan R wrote: > >> + opp-1400000000 { > >> + opp-hz = /bits/ 64 <1400000000>; > >> + opp-microvolt-speed0-pvs0-v0 = <1250000>; > > > > Why speed0 and v0 in all the names ? > > > > Ya, all the three (speed, pvs and version) are read from efuse. So all the three > can vary. Okay, so may be in the example you should have a mix of all the combinations to show how these things work ? You only showed values for a single efuse configuration currently.
Hi Viresh, On 12/20/2017 11:57 AM, Viresh Kumar wrote: > On 20-12-17, 11:55, Sricharan R wrote: >>>> + opp-1400000000 { >>>> + opp-hz = /bits/ 64 <1400000000>; >>>> + opp-microvolt-speed0-pvs0-v0 = <1250000>; >>> >>> Why speed0 and v0 in all the names ? >>> >> >> Ya, all the three (speed, pvs and version) are read from efuse. So all the three >> can vary. > > Okay, so may be in the example you should have a mix of all the > combinations to show how these things work ? You only showed values > for a single efuse configuration currently. > Ha ok. Will add other examples as well. Regards, Sricharan
On Wed, Dec 20, 2017 at 11:55:33AM +0530, Sricharan R wrote: > Hi Viresh, > > On 12/20/2017 8:56 AM, Viresh Kumar wrote: > > On 19-12-17, 21:25, Sricharan R wrote: > >> + cpu@0 { > >> + compatible = "qcom,krait"; > >> + enable-method = "qcom,kpss-acc-v1"; > >> + device_type = "cpu"; > >> + reg = <0>; > >> + qcom,acc = <&acc0>; > >> + qcom,saw = <&saw0>; > >> + clocks = <&kraitcc 0>; > >> + clock-names = "cpu"; > >> + cpu-supply = <&smb208_s2a>; > >> + operating-points-v2 = <&cpu_opp_table>; > >> + }; > >> + > >> + qcom,pvs { > >> + qcom,pvs-format-a; > >> + }; > > > > Not sure what Rob is going to say on that :) > > > > Yes. Would be good to know the best way. Seems like this should be a property of an efuse node either implied by the compatible or a separate property. What determines format A vs. B? Rob
Hi Rob, On 12/21/2017 2:48 AM, Rob Herring wrote: > On Wed, Dec 20, 2017 at 11:55:33AM +0530, Sricharan R wrote: >> Hi Viresh, >> >> On 12/20/2017 8:56 AM, Viresh Kumar wrote: >>> On 19-12-17, 21:25, Sricharan R wrote: >>>> + cpu@0 { >>>> + compatible = "qcom,krait"; >>>> + enable-method = "qcom,kpss-acc-v1"; >>>> + device_type = "cpu"; >>>> + reg = <0>; >>>> + qcom,acc = <&acc0>; >>>> + qcom,saw = <&saw0>; >>>> + clocks = <&kraitcc 0>; >>>> + clock-names = "cpu"; >>>> + cpu-supply = <&smb208_s2a>; >>>> + operating-points-v2 = <&cpu_opp_table>; >>>> + }; >>>> + >>>> + qcom,pvs { >>>> + qcom,pvs-format-a; >>>> + }; >>> >>> Not sure what Rob is going to say on that :) >>> >> >> Yes. Would be good to know the best way. > > Seems like this should be a property of an efuse node either implied by > the compatible or a separate property. What determines format A vs. B? > Yes, this efuse registers are part of the eeprom (qfprom) tied to the soc. So this property (details like bitfields and register offsets that it represents) can be put soc specific and nvmem apis can be used to read the registers. Does something like below look ok ? qcom,pvs { compatible = "qcom,pvs-ipq8064"; nvmem-cells = <&pvs_efuse>; } qfprom: qfprom@700000 { compatible = "qcom,qfprom"; reg = <0x00700000 0x1000>; #address-cells = <1>; #size-cells = <1>; ranges; pvs_efuse: pvs { reg = <0xc0 0x8>; }; }; Regards, Sricharan
On Thu, Dec 21, 2017 at 5:53 AM, Sricharan R <sricharan@codeaurora.org> wrote: > Hi Rob, > > On 12/21/2017 2:48 AM, Rob Herring wrote: >> On Wed, Dec 20, 2017 at 11:55:33AM +0530, Sricharan R wrote: >>> Hi Viresh, >>> >>> On 12/20/2017 8:56 AM, Viresh Kumar wrote: >>>> On 19-12-17, 21:25, Sricharan R wrote: >>>>> + cpu@0 { >>>>> + compatible = "qcom,krait"; >>>>> + enable-method = "qcom,kpss-acc-v1"; >>>>> + device_type = "cpu"; >>>>> + reg = <0>; >>>>> + qcom,acc = <&acc0>; >>>>> + qcom,saw = <&saw0>; >>>>> + clocks = <&kraitcc 0>; >>>>> + clock-names = "cpu"; >>>>> + cpu-supply = <&smb208_s2a>; >>>>> + operating-points-v2 = <&cpu_opp_table>; >>>>> + }; >>>>> + >>>>> + qcom,pvs { >>>>> + qcom,pvs-format-a; >>>>> + }; >>>> >>>> Not sure what Rob is going to say on that :) >>>> >>> >>> Yes. Would be good to know the best way. >> >> Seems like this should be a property of an efuse node either implied by >> the compatible or a separate property. What determines format A vs. B? >> > > Yes, this efuse registers are part of the eeprom (qfprom) tied to the soc. > So this property (details like bitfields and register offsets that it represents) > can be put soc specific and nvmem apis can be used to read > the registers. Does something like below look ok ? > > qcom,pvs { > compatible = "qcom,pvs-ipq8064"; > nvmem-cells = <&pvs_efuse>; > } Why do you need this node? It doesn't look like it corresponds to a h/w block. It looks like you are just creating it to instantiate a driver. > qfprom: qfprom@700000 { > compatible = "qcom,qfprom"; Either this or... > reg = <0x00700000 0x1000>; > #address-cells = <1>; > #size-cells = <1>; > ranges; > pvs_efuse: pvs { a compatible here should be specific enough so the OS can know what the bits are. > reg = <0xc0 0x8>; > }; > };
Hi Rob, On 12/26/2017 11:06 PM, Rob Herring wrote: > On Thu, Dec 21, 2017 at 5:53 AM, Sricharan R <sricharan@codeaurora.org> wrote: >> Hi Rob, >> >> On 12/21/2017 2:48 AM, Rob Herring wrote: >>> On Wed, Dec 20, 2017 at 11:55:33AM +0530, Sricharan R wrote: >>>> Hi Viresh, >>>> >>>> On 12/20/2017 8:56 AM, Viresh Kumar wrote: >>>>> On 19-12-17, 21:25, Sricharan R wrote: >>>>>> + cpu@0 { >>>>>> + compatible = "qcom,krait"; >>>>>> + enable-method = "qcom,kpss-acc-v1"; >>>>>> + device_type = "cpu"; >>>>>> + reg = <0>; >>>>>> + qcom,acc = <&acc0>; >>>>>> + qcom,saw = <&saw0>; >>>>>> + clocks = <&kraitcc 0>; >>>>>> + clock-names = "cpu"; >>>>>> + cpu-supply = <&smb208_s2a>; >>>>>> + operating-points-v2 = <&cpu_opp_table>; >>>>>> + }; >>>>>> + >>>>>> + qcom,pvs { >>>>>> + qcom,pvs-format-a; >>>>>> + }; >>>>> >>>>> Not sure what Rob is going to say on that :) >>>>> >>>> >>>> Yes. Would be good to know the best way. >>> >>> Seems like this should be a property of an efuse node either implied by >>> the compatible or a separate property. What determines format A vs. B? >>> >> >> Yes, this efuse registers are part of the eeprom (qfprom) tied to the soc. >> So this property (details like bitfields and register offsets that it represents) >> can be put soc specific and nvmem apis can be used to read >> the registers. Does something like below look ok ? >> >> qcom,pvs { >> compatible = "qcom,pvs-ipq8064"; >> nvmem-cells = <&pvs_efuse>; >> } > > Why do you need this node? It doesn't look like it corresponds to a > h/w block. It looks like you are just creating it to instantiate a > driver. > >> qfprom: qfprom@700000 { >> compatible = "qcom,qfprom"; > > Either this or... > >> reg = <0x00700000 0x1000>; >> #address-cells = <1>; >> #size-cells = <1>; >> ranges; >> pvs_efuse: pvs { > > a compatible here should be specific enough so the OS can know what > the bits are. Infact the above "qcom,pvs" node is required mainly to act as a consumer for the nvmem data provider ("qcom,qfprom") (using nvmem-cells = <&pvs_efuse>) Then "qfprom" can be made to contain a "format_a" or "format_b" specific cell. So all that is needed is, nvmem-cells = <&pvs_efuse_phandle> needs to be available somewhere. The requirement is similar what is now done by "operating-points-v2-ti-cpu" and the ti-cpufreq.c. There "operating-points-v2-ti-cpu" node, contains the syscon register to read the efuse values. Similarly does defining a new "operating-points-v2-krait-cpu" which would contain the nvmem-cells property look ok ? This would avoid defining a new qcom,pvs node. cpu@0 { compatible = "qcom,krait"; enable-method = "qcom,kpss-acc-v1"; device_type = "cpu"; reg = <0>; qcom,acc = <&acc0>; qcom,saw = <&saw0>; clocks = <&kraitcc 0>; clock-names = "cpu"; cpu-supply = <&smb208_s2a>; operating-points-v2 = <&cpu_opp_table>; }; cpu_opp_table: opp_table { compatible = "operating-points-v2-krait-cpu"; nvmem-cells = <&pvs_efuse_format_a>; /* * Missing opp-shared property means CPUs switch DVFS states * independently. */ opp-1400000000 { opp-hz = /bits/ 64 <1400000000>; opp-microvolt-speed0-pvs0-v0 = <1250000>; opp-microvolt-speed0-pvs1-v0 = <1175000>; opp-microvolt-speed0-pvs2-v0 = <1125000>; opp-microvolt-speed0-pvs3-v0 = <1050000>; }; ... } qfprom: qfprom@700000 { compatible = "qcom,qfprom"; reg = <0x00700000 0x1000>; #address-cells = <1>; #size-cells = <1>; ranges; pvs_efuse_format_a: pvs { reg = <0xc0 0x8>; }; } Regards, Sricharan
On Wed, Dec 27, 2017 at 4:20 AM, Sricharan R <sricharan@codeaurora.org> wrote: > Hi Rob, > > On 12/26/2017 11:06 PM, Rob Herring wrote: >> On Thu, Dec 21, 2017 at 5:53 AM, Sricharan R <sricharan@codeaurora.org> wrote: >>> Hi Rob, >>> >>> On 12/21/2017 2:48 AM, Rob Herring wrote: >>>> On Wed, Dec 20, 2017 at 11:55:33AM +0530, Sricharan R wrote: >>>>> Hi Viresh, >>>>> >>>>> On 12/20/2017 8:56 AM, Viresh Kumar wrote: >>>>>> On 19-12-17, 21:25, Sricharan R wrote: >>>>>>> + cpu@0 { >>>>>>> + compatible = "qcom,krait"; >>>>>>> + enable-method = "qcom,kpss-acc-v1"; >>>>>>> + device_type = "cpu"; >>>>>>> + reg = <0>; >>>>>>> + qcom,acc = <&acc0>; >>>>>>> + qcom,saw = <&saw0>; >>>>>>> + clocks = <&kraitcc 0>; >>>>>>> + clock-names = "cpu"; >>>>>>> + cpu-supply = <&smb208_s2a>; >>>>>>> + operating-points-v2 = <&cpu_opp_table>; >>>>>>> + }; >>>>>>> + >>>>>>> + qcom,pvs { >>>>>>> + qcom,pvs-format-a; >>>>>>> + }; >>>>>> >>>>>> Not sure what Rob is going to say on that :) >>>>>> >>>>> >>>>> Yes. Would be good to know the best way. >>>> >>>> Seems like this should be a property of an efuse node either implied by >>>> the compatible or a separate property. What determines format A vs. B? >>>> >>> >>> Yes, this efuse registers are part of the eeprom (qfprom) tied to the soc. >>> So this property (details like bitfields and register offsets that it represents) >>> can be put soc specific and nvmem apis can be used to read >>> the registers. Does something like below look ok ? >>> >>> qcom,pvs { >>> compatible = "qcom,pvs-ipq8064"; >>> nvmem-cells = <&pvs_efuse>; >>> } >> >> Why do you need this node? It doesn't look like it corresponds to a >> h/w block. It looks like you are just creating it to instantiate a >> driver. >> >>> qfprom: qfprom@700000 { >>> compatible = "qcom,qfprom"; >> >> Either this or... >> >>> reg = <0x00700000 0x1000>; >>> #address-cells = <1>; >>> #size-cells = <1>; >>> ranges; >>> pvs_efuse: pvs { >> >> a compatible here should be specific enough so the OS can know what >> the bits are. > > Infact the above "qcom,pvs" node is required mainly to act as a consumer > for the nvmem data provider ("qcom,qfprom") (using nvmem-cells = <&pvs_efuse>) > Then "qfprom" can be made to contain a "format_a" or "format_b" specific cell. > > So all that is needed is, nvmem-cells = <&pvs_efuse_phandle> needs to be available > somewhere. The requirement is similar what is now done by "operating-points-v2-ti-cpu" > and the ti-cpufreq.c. There "operating-points-v2-ti-cpu" node, contains the syscon > register to read the efuse values. Similarly does defining a new > "operating-points-v2-krait-cpu" which would contain the nvmem-cells property look ok ? > This would avoid defining a new qcom,pvs node. Yes, this seems reasonable. > > cpu@0 { > compatible = "qcom,krait"; > enable-method = "qcom,kpss-acc-v1"; > device_type = "cpu"; > reg = <0>; > qcom,acc = <&acc0>; > qcom,saw = <&saw0>; > clocks = <&kraitcc 0>; > clock-names = "cpu"; > cpu-supply = <&smb208_s2a>; > operating-points-v2 = <&cpu_opp_table>; > }; > > cpu_opp_table: opp_table { > compatible = "operating-points-v2-krait-cpu"; > > nvmem-cells = <&pvs_efuse_format_a>; > /* > * Missing opp-shared property means CPUs switch DVFS states > * independently. > */ > > opp-1400000000 { > opp-hz = /bits/ 64 <1400000000>; > opp-microvolt-speed0-pvs0-v0 = <1250000>; > opp-microvolt-speed0-pvs1-v0 = <1175000>; > opp-microvolt-speed0-pvs2-v0 = <1125000>; > opp-microvolt-speed0-pvs3-v0 = <1050000>; > > }; > ... > } > > qfprom: qfprom@700000 { > compatible = "qcom,qfprom"; > reg = <0x00700000 0x1000>; > #address-cells = <1>; > #size-cells = <1>; > ranges; > pvs_efuse_format_a: pvs { > reg = <0xc0 0x8>; > }; > } > > Regards, > Sricharan > > -- > "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,pvs.txt b/Documentation/devicetree/bindings/arm/msm/qcom,pvs.txt new file mode 100644 index 0000000..260f537 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/qcom,pvs.txt @@ -0,0 +1,91 @@ +Qualcomm Process Voltage Scaling Tables + +The node name is required to be "qcom,pvs". There shall only be one +such node present in the root of the tree. + +PROPERTIES + +- qcom,pvs-format-a or qcom,pvs-format-b: + Usage: required + Value type: <empty> + Definition: Indicates where and how to read and interpret the efuse registers. + Based on that the opp-microvolt-<name> is extended with the right + speedXX-PVSXX-versionXX string. The cpu opp-table should be populated + with the operating-points-v2 binding and each opp must have the voltage + specified for all combinations of opp-microvolt-<speedXX-pvsXX-versionXX>. + +Example: + + cpu@0 { + compatible = "qcom,krait"; + enable-method = "qcom,kpss-acc-v1"; + device_type = "cpu"; + reg = <0>; + qcom,acc = <&acc0>; + qcom,saw = <&saw0>; + clocks = <&kraitcc 0>; + clock-names = "cpu"; + cpu-supply = <&smb208_s2a>; + operating-points-v2 = <&cpu_opp_table>; + }; + + qcom,pvs { + qcom,pvs-format-a; + }; + + + cpu_opp_table: opp_table { + compatible = "operating-points-v2"; + + /* + * Missing opp-shared property means CPUs switch DVFS states + * independently. + */ + + opp-1400000000 { + opp-hz = /bits/ 64 <1400000000>; + opp-microvolt-speed0-pvs0-v0 = <1250000>; + opp-microvolt-speed0-pvs1-v0 = <1175000>; + opp-microvolt-speed0-pvs2-v0 = <1125000>; + opp-microvolt-speed0-pvs3-v0 = <1050000>; + + }; + opp-800000000 { + opp-hz = /bits/ 64 <800000000>; + opp-microvolt-speed0-pvs0-v0 = <1100000>; + opp-microvolt-speed0-pvs1-v0 = <1025000>; + opp-microvolt-speed0-pvs2-v0 = <995000>; + opp-microvolt-speed0-pvs3-v0 = <900000>; + + }; + opp-384000000 { + opp-hz = /bits/ 64 <384000000>; + opp-microvolt-speed0-pvs0-v0 = <1000000>; + opp-microvolt-speed0-pvs1-v0 = <925000>; + opp-microvolt-speed0-pvs2-v0 = <875000>; + opp-microvolt-speed0-pvs3-v0 = <800000>; + }; + opp-1000000000 { + opp-hz = /bits/ 64 <1000000000>; + opp-microvolt-speed0-pvs0-v0 = <1150000>; + opp-microvolt-speed0-pvs1-v0 = <1075000>; + opp-microvolt-speed0-pvs2-v0 = <1025000>; + opp-microvolt-speed0-pvs3-v0 = <950000>; + + }; + opp-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt-speed0-pvs0-v0 = <1050000>; + opp-microvolt-speed0-pvs1-v0 = <975000>; + opp-microvolt-speed0-pvs2-v0 = <925000>; + opp-microvolt-speed0-pvs3-v0 = <850000>; + }; + opp-1200000000 { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt-speed0-pvs0-v0 = <1200000>; + opp-microvolt-speed0-pvs1-v0 = <1125000>; + opp-microvolt-speed0-pvs2-v0 = <1075000>; + opp-microvolt-speed0-pvs3-v0 = <1000000>; + }; + }; +