Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings

[Taniya Das]

On 8/8/2018 12:54 AM, skannan@xxxxxxxxxxxxxx wrote:
> On 2018-08-07 04:12, Sudeep Holla wrote:
> > On Mon, Aug 06, 2018 at 01:54:24PM -0700, skannan@xxxxxxxxxxxxxx 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....>;

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--