| Message ID | 1532428970-18122-2-git-send-email-tdas@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver | expand |
Quoting Taniya Das (2018-07-24 03:42:49) > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > new file mode 100644 > index 0000000..22d4355 > --- /dev/null > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > @@ -0,0 +1,172 @@ [...] > + > + CPU7: cpu@700 { > + device_type = "cpu"; > + compatible = "qcom,kryo385"; > + reg = <0x0 0x700>; > + enable-method = "psci"; > + next-level-cache = <&L2_700>; > + qcom,freq-domain = <&freq_domain_table1>; > + L2_700: l2-cache { > + compatible = "cache"; > + next-level-cache = <&L3_0>; > + }; > + }; > + }; > + > + qcom,cpufreq-hw { > + compatible = "qcom,cpufreq-hw"; > + > + clocks = <&rpmhcc RPMH_CXO_CLK>; > + clock-names = "xo"; > + > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + freq_domain_table0: freq_table0 { > + reg = <0 0x17d43000 0 0x1400>; > + }; > + > + freq_domain_table1: freq_table1 { > + reg = <0 0x17d45800 0 0x1400>; > + }; Sorry, this is just not proper DT design. The whole node should have a reg property, and it should contain two (or three if we're handling the L3 clk domain?) different offsets for the different power clusters. The problem seems to still be that we don't have a way to map the CPUs to the clk domains they're in provided by this hardware block. Making subnodes is not the solution.
On 2018-08-03 16:46, Stephen Boyd wrote: > Quoting Taniya Das (2018-07-24 03:42:49) >> diff --git >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >> new file mode 100644 >> index 0000000..22d4355 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >> @@ -0,0 +1,172 @@ > [...] >> + >> + CPU7: cpu@700 { >> + device_type = "cpu"; >> + compatible = "qcom,kryo385"; >> + reg = <0x0 0x700>; >> + enable-method = "psci"; >> + next-level-cache = <&L2_700>; >> + qcom,freq-domain = <&freq_domain_table1>; >> + L2_700: l2-cache { >> + compatible = "cache"; >> + next-level-cache = <&L3_0>; >> + }; >> + }; >> + }; >> + >> + qcom,cpufreq-hw { >> + compatible = "qcom,cpufreq-hw"; >> + >> + clocks = <&rpmhcc RPMH_CXO_CLK>; >> + clock-names = "xo"; >> + >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + freq_domain_table0: freq_table0 { >> + reg = <0 0x17d43000 0 0x1400>; >> + }; >> + >> + freq_domain_table1: freq_table1 { >> + reg = <0 0x17d45800 0 0x1400>; >> + }; > > Sorry, this is just not proper DT design. The whole node should have a > reg property, and it should contain two (or three if we're handling the > L3 clk domain?) different offsets for the different power clusters. The > problem seems to still be that we don't have a way to map the CPUs to > the clk domains they're in provided by this hardware block. Making > subnodes is not the solution. The problem is mapping clock domains to logical CPUs that CPUfreq uses. The physical CPU to logical CPU mapping can be changed by the kernel (even through DT if I'm not mistaken). So we need to have a way to tell in DT which physical CPUs are connected to which CPU freq clock domain. As for subnodes or not, we don't have any strong opinion, but couple of other points to consider. Two or more CPUfreq policies might have a common frequency table (read from HW), but separate control of frequency. So, you also need a way to group frequency table with CPU freq policies. If you have a better design, we are open to that suggestion. Thanks, Saravana
On Mon, Aug 06, 2018 at 01:54:24PM -0700, skannan@codeaurora.org wrote: > On 2018-08-03 16:46, Stephen Boyd wrote: > >Quoting Taniya Das (2018-07-24 03:42:49) > >>diff --git > >>a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > >>b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > >>new file mode 100644 > >>index 0000000..22d4355 > >>--- /dev/null > >>+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > >>@@ -0,0 +1,172 @@ > >[...] > >>+ > >>+ CPU7: cpu@700 { > >>+ device_type = "cpu"; > >>+ compatible = "qcom,kryo385"; > >>+ reg = <0x0 0x700>; > >>+ enable-method = "psci"; > >>+ next-level-cache = <&L2_700>; > >>+ qcom,freq-domain = <&freq_domain_table1>; > >>+ L2_700: l2-cache { > >>+ compatible = "cache"; > >>+ next-level-cache = <&L3_0>; > >>+ }; > >>+ }; > >>+ }; > >>+ > >>+ qcom,cpufreq-hw { > >>+ compatible = "qcom,cpufreq-hw"; > >>+ > >>+ clocks = <&rpmhcc RPMH_CXO_CLK>; > >>+ clock-names = "xo"; > >>+ > >>+ #address-cells = <2>; > >>+ #size-cells = <2>; > >>+ ranges; > >>+ freq_domain_table0: freq_table0 { > >>+ reg = <0 0x17d43000 0 0x1400>; > >>+ }; > >>+ > >>+ freq_domain_table1: freq_table1 { > >>+ reg = <0 0x17d45800 0 0x1400>; > >>+ }; > > > >Sorry, this is just not proper DT design. The whole node should have a > >reg property, and it should contain two (or three if we're handling the > >L3 clk domain?) different offsets for the different power clusters. The > >problem seems to still be that we don't have a way to map the CPUs to > >the clk domains they're in provided by this hardware block. Making > >subnodes is not the solution. > > The problem is mapping clock domains to logical CPUs that CPUfreq uses. The > physical CPU to logical CPU mapping can be changed by the kernel (even > through DT if I'm not mistaken). So we need to have a way to tell in DT > which physical CPUs are connected to which CPU freq clock domain. > How about passing CPU freq clock domain id as along with phandle in qcom,freq-domain ? -- Regards, Sudeep
On 2018-08-07 04:12, Sudeep Holla wrote: > On Mon, Aug 06, 2018 at 01:54:24PM -0700, skannan@codeaurora.org wrote: >> On 2018-08-03 16:46, Stephen Boyd wrote: >> >Quoting Taniya Das (2018-07-24 03:42:49) >> >>diff --git >> >>a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >> >>b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >> >>new file mode 100644 >> >>index 0000000..22d4355 >> >>--- /dev/null >> >>+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >> >>@@ -0,0 +1,172 @@ >> >[...] >> >>+ >> >>+ CPU7: cpu@700 { >> >>+ device_type = "cpu"; >> >>+ compatible = "qcom,kryo385"; >> >>+ reg = <0x0 0x700>; >> >>+ enable-method = "psci"; >> >>+ next-level-cache = <&L2_700>; >> >>+ qcom,freq-domain = <&freq_domain_table1>; >> >>+ L2_700: l2-cache { >> >>+ compatible = "cache"; >> >>+ next-level-cache = <&L3_0>; >> >>+ }; >> >>+ }; >> >>+ }; >> >>+ >> >>+ qcom,cpufreq-hw { >> >>+ compatible = "qcom,cpufreq-hw"; >> >>+ >> >>+ clocks = <&rpmhcc RPMH_CXO_CLK>; >> >>+ clock-names = "xo"; >> >>+ >> >>+ #address-cells = <2>; >> >>+ #size-cells = <2>; >> >>+ ranges; >> >>+ freq_domain_table0: freq_table0 { >> >>+ reg = <0 0x17d43000 0 0x1400>; >> >>+ }; >> >>+ >> >>+ freq_domain_table1: freq_table1 { >> >>+ reg = <0 0x17d45800 0 0x1400>; >> >>+ }; >> > >> >Sorry, this is just not proper DT design. The whole node should have a >> >reg property, and it should contain two (or three if we're handling the >> >L3 clk domain?) different offsets for the different power clusters. The >> >problem seems to still be that we don't have a way to map the CPUs to >> >the clk domains they're in provided by this hardware block. Making >> >subnodes is not the solution. >> >> The problem is mapping clock domains to logical CPUs that CPUfreq >> uses. The >> physical CPU to logical CPU mapping can be changed by the kernel (even >> through DT if I'm not mistaken). So we need to have a way to tell in >> DT >> which physical CPUs are connected to which CPU freq clock domain. >> > > How about passing CPU freq clock domain id as along with phandle in > qcom,freq-domain ? Now sure what you mean here. There's no such this as CPUfreq clock domain id. It has policies that are made up of logical CPU numbers. Logical CPU is not something that you can fix in DT. -Saravana
On 8/8/2018 12:54 AM, skannan@codeaurora.org wrote: > On 2018-08-07 04:12, Sudeep Holla wrote: >> On Mon, Aug 06, 2018 at 01:54:24PM -0700, skannan@codeaurora.org wrote: >>> On 2018-08-03 16:46, Stephen Boyd wrote: >>> >Quoting Taniya Das (2018-07-24 03:42:49) >>> >>diff --git >>> >>a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >>> >>b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >>> >>new file mode 100644 >>> >>index 0000000..22d4355 >>> >>--- /dev/null >>> >>+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt >>> >>@@ -0,0 +1,172 @@ >>> >[...] >>> >>+ >>> >>+ CPU7: cpu@700 { >>> >>+ device_type = "cpu"; >>> >>+ compatible = "qcom,kryo385"; >>> >>+ reg = <0x0 0x700>; >>> >>+ enable-method = "psci"; >>> >>+ next-level-cache = <&L2_700>; >>> >>+ qcom,freq-domain = <&freq_domain_table1>; >>> >>+ L2_700: l2-cache { >>> >>+ compatible = "cache"; >>> >>+ next-level-cache = <&L3_0>; >>> >>+ }; >>> >>+ }; >>> >>+ }; >>> >>+ >>> >>+ qcom,cpufreq-hw { >>> >>+ compatible = "qcom,cpufreq-hw"; >>> >>+ >>> >>+ clocks = <&rpmhcc RPMH_CXO_CLK>; >>> >>+ clock-names = "xo"; >>> >>+ >>> >>+ #address-cells = <2>; >>> >>+ #size-cells = <2>; >>> >>+ ranges; >>> >>+ freq_domain_table0: freq_table0 { >>> >>+ reg = <0 0x17d43000 0 0x1400>; >>> >>+ }; >>> >>+ >>> >>+ freq_domain_table1: freq_table1 { >>> >>+ reg = <0 0x17d45800 0 0x1400>; >>> >>+ }; >>> > >>> >Sorry, this is just not proper DT design. The whole node should have a >>> >reg property, and it should contain two (or three if we're handling the >>> >L3 clk domain?) different offsets for the different power clusters. The >>> >problem seems to still be that we don't have a way to map the CPUs to >>> >the clk domains they're in provided by this hardware block. Making >>> >subnodes is not the solution. >>> >>> The problem is mapping clock domains to logical CPUs that CPUfreq >>> uses. The >>> physical CPU to logical CPU mapping can be changed by the kernel (even >>> through DT if I'm not mistaken). So we need to have a way to tell in DT >>> which physical CPUs are connected to which CPU freq clock domain. >>> >> >> How about passing CPU freq clock domain id as along with phandle in >> qcom,freq-domain ? > > Now sure what you mean here. There's no such this as CPUfreq clock > domain id. It has policies that are made up of logical CPU numbers. > Logical CPU is not something that you can fix in DT. > > -Saravana Sudeep, Earlier the design was the freq_domain would take the CPU phandles freq_domain: cpus = <&cpu0 &cpu1....>;
Quoting Taniya Das (2018-08-07 19:46:01) > > > On 8/8/2018 12:54 AM, skannan@codeaurora.org wrote: > > On 2018-08-07 04:12, Sudeep Holla wrote: > >> On Mon, Aug 06, 2018 at 01:54:24PM -0700, skannan@codeaurora.org wrote: > >>> On 2018-08-03 16:46, Stephen Boyd wrote: > >>> >Quoting Taniya Das (2018-07-24 03:42:49) > >>> >>diff --git > >>> >>a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > >>> >>b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > >>> >>new file mode 100644 > >>> >>index 0000000..22d4355 > >>> >>--- /dev/null > >>> >>+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > >>> >>@@ -0,0 +1,172 @@ > >>> >[...] > >>> >>+ > >>> >>+ CPU7: cpu@700 { > >>> >>+ device_type = "cpu"; > >>> >>+ compatible = "qcom,kryo385"; > >>> >>+ reg = <0x0 0x700>; > >>> >>+ enable-method = "psci"; > >>> >>+ next-level-cache = <&L2_700>; > >>> >>+ qcom,freq-domain = <&freq_domain_table1>; > >>> >>+ L2_700: l2-cache { > >>> >>+ compatible = "cache"; > >>> >>+ next-level-cache = <&L3_0>; > >>> >>+ }; > >>> >>+ }; > >>> >>+ }; > >>> >>+ > >>> >>+ qcom,cpufreq-hw { > >>> >>+ compatible = "qcom,cpufreq-hw"; > >>> >>+ > >>> >>+ clocks = <&rpmhcc RPMH_CXO_CLK>; > >>> >>+ clock-names = "xo"; > >>> >>+ > >>> >>+ #address-cells = <2>; > >>> >>+ #size-cells = <2>; > >>> >>+ ranges; > >>> >>+ freq_domain_table0: freq_table0 { > >>> >>+ reg = <0 0x17d43000 0 0x1400>; > >>> >>+ }; > >>> >>+ > >>> >>+ freq_domain_table1: freq_table1 { > >>> >>+ reg = <0 0x17d45800 0 0x1400>; > >>> >>+ }; > >>> > > >>> >Sorry, this is just not proper DT design. The whole node should have a > >>> >reg property, and it should contain two (or three if we're handling the > >>> >L3 clk domain?) different offsets for the different power clusters. The > >>> >problem seems to still be that we don't have a way to map the CPUs to > >>> >the clk domains they're in provided by this hardware block. Making > >>> >subnodes is not the solution. > >>> > >>> The problem is mapping clock domains to logical CPUs that CPUfreq > >>> uses. The > >>> physical CPU to logical CPU mapping can be changed by the kernel (even > >>> through DT if I'm not mistaken). So we need to have a way to tell in DT > >>> which physical CPUs are connected to which CPU freq clock domain. > >>> > >> > >> How about passing CPU freq clock domain id as along with phandle in > >> qcom,freq-domain ? > > > > Now sure what you mean here. There's no such this as CPUfreq clock > > domain id. It has policies that are made up of logical CPU numbers. > > Logical CPU is not something that you can fix in DT. > > > > -Saravana > > Sudeep, > > Earlier the design was the freq_domain would take the CPU phandles > > freq_domain: > cpus = <&cpu0 &cpu1....>; > I believe Sudeep is recommending something I recommended earlier. It would look like: cpu7 { qcom,freq-domain = <&cpufreq_hw 1>; } to indicate that cpu7 is in cpufreq_hw's frequency domain #1. That should probably be called clk domain BTW. If that was done with a phandle and a single cell, then we should have something similar on the cpufreq_hw node side indicating how to parse the cells in qcom,freq-domain. A property like #qcom,freq-domain-cells = <1> to indicate that one u32 follows the phandle.
On Tue, Aug 07, 2018 at 11:15:02PM -0700, Stephen Boyd wrote: > Quoting Taniya Das (2018-08-07 19:46:01) [...] > > Sudeep, > > > > Earlier the design was the freq_domain would take the CPU phandles > > > > freq_domain: > > cpus = <&cpu0 &cpu1....>; > > > > I believe Sudeep is recommending something I recommended earlier. It > would look like: > > cpu7 { > qcom,freq-domain = <&cpufreq_hw 1>; > } > > to indicate that cpu7 is in cpufreq_hw's frequency domain #1. That > should probably be called clk domain BTW. > Thanks Stephen, that's exactly what I meant. You have explained all the details saving me time :) > If that was done with a phandle and a single cell, then we should have > something similar on the cpufreq_hw node side indicating how to parse > the cells in qcom,freq-domain. A property like #qcom,freq-domain-cells = > <1> to indicate that one u32 follows the phandle. > As Saravana mentions in the other email I can't believe it's just made up of logical CPU numbers. If that's the case is it software configurable. -- Regards, Sudeep
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file mode 100644 index 0000000..22d4355 --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt @@ -0,0 +1,172 @@ +Qualcomm Technologies, Inc. CPUFREQ Bindings + +CPUFREQ HW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI) +SoCs to manage frequency in hardware. It is capable of controlling frequency +for multiple clusters. + +Properties: +- compatible + Usage: required + Value type: <string> + Definition: must be "qcom,cpufreq-hw". + +- clocks + Usage: required + Value type: <phandle> From common clock binding. + Definition: clock handle for XO clock. + +- clock-names + Usage: required + Value type: <string> From common clock binding. + Definition: must be "xo". + +* Property qcom,freq-domain +Devices supporting freq-domain must set their "qcom,freq-domain" property with +phandle to a freq_domain_table in their DT node. + +* Frequency Domain Table Node + +This describes the frequency domain belonging to a device. +This node can have following properties: + +- reg + Usage: required + Value type: <prop-encoded-array> + Definition: Addresses and sizes for the memory of the HW bases. + +Example: + +Example 1: Dual-cluster, Quad-core per cluster. CPUs within a cluster switch +DCVS state together. + +/ { + cpus { + #address-cells = <2>; + #size-cells = <0>; + + CPU0: cpu@0 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x0>; + enable-method = "psci"; + next-level-cache = <&L2_0>; + qcom,freq-domain = <&freq_domain_table0>; + L2_0: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + L3_0: l3-cache { + compatible = "cache"; + }; + }; + }; + + CPU1: cpu@100 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x100>; + enable-method = "psci"; + next-level-cache = <&L2_100>; + qcom,freq-domain = <&freq_domain_table0>; + L2_100: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU2: cpu@200 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x200>; + enable-method = "psci"; + next-level-cache = <&L2_200>; + qcom,freq-domain = <&freq_domain_table0>; + L2_200: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU3: cpu@300 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x300>; + enable-method = "psci"; + next-level-cache = <&L2_300>; + qcom,freq-domain = <&freq_domain_table0>; + L2_300: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU4: cpu@400 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x400>; + enable-method = "psci"; + next-level-cache = <&L2_400>; + qcom,freq-domain = <&freq_domain_table1>; + L2_400: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU5: cpu@500 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x500>; + enable-method = "psci"; + next-level-cache = <&L2_500>; + qcom,freq-domain = <&freq_domain_table1>; + L2_500: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU6: cpu@600 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x600>; + enable-method = "psci"; + next-level-cache = <&L2_600>; + qcom,freq-domain = <&freq_domain_table1>; + L2_600: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU7: cpu@700 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x700>; + enable-method = "psci"; + next-level-cache = <&L2_700>; + qcom,freq-domain = <&freq_domain_table1>; + L2_700: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + }; + + qcom,cpufreq-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>; + }; + }; +};
Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's SoCs. This is required for managing the cpu frequency transitions which are controlled by the hardware engine. Signed-off-by: Taniya Das <tdas@codeaurora.org> --- .../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 +++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt