Re: [RFC] [PATCH 03/13] qcom: spm: Add Subsystem Power Manager driver for QCOM chipsets

[Lina Iyer <lina.iyer@linaro.org>]

On Fri, Aug 08, 2014 at 11:16:04AM -0500, Kumar Gala wrote:
>
> On Aug 7, 2014, at 11:05 PM, Lina Iyer <lina.iyer@linaro.org> wrote:
>
  > +Required properties
> > +
> > +- compatible: "qcom,spm-v2"
> > +- reg: The physical address and the size of the SPM's memory mapped registers
> > +- qcom,cpu: phandle for the CPU that the SPM block is attached to. On targets
> > + that dont support CPU phandles the driver would support qcom,core-id.
> > + This field is required on only for SPMs that control the CPU.
> > +- qcom, core-id: This property will be deprecated once all targets start
> > + supporting CPU phandles. This field will be used to identify SPMs
> > + that control the CPU.
> > + {0..n} for cores {0..n}
>
> Why aren’t we just using cpu phandles for upstream?
Need to specify L2 index as well. I dont know, but does cpu handle
define that?
>
> > +- qcom,saw2-ver-reg: The location of the version register
>
> I
>
> > +- qcom,saw2-cfg: SAW2 configuration register
>
> What does this even mean, why is this not part of the reg property?
This is a hardware register with that name. Using the same name as the 
h/w for consistency.

>
> > +- qcom,saw2-avs-ctl: The AVS control register
>
> What does this even mean, why is this not part of the reg property?
Same.

>
> > +- qcom,saw2-avs-hysterisis: The AVS hysterisis register to delay the AVS
> > + controller requests
>
> Why do we need this?
Not all registers initialize to 0 on power on. So, its better to
initialize it. We dont use AVS currently in the chipset.
>
> > +- qcom,saw2-spm-dly: Provides the values for the SPM delay command in the SPM
> > + sequence
>
> how many values?
HW configuration value.
>
> > +- qcom,saw2-spm-ctl: The SPM control register
>
> What does this mean?  Is it the offset of the register, and if so from what base?  Also, why does this vary, this should possibly be handled by different compatible values.
The register controls the SPM. 0x1 means its ON.
>
> > +- qcom,vctl-timeout-us: The timeout value in us to wait for voltage to change
>
> us -> microseconds
>
> > + after sending the voltage command to the PMIC
>
> Have we really ever set this to any value other than 50 usec?
>
> > +- qcom,name: The name with which a SPM device is identified by the power
> > +management code.
>
> what does this even mean?
Power/idle drivers, that I am trying to split and get at, would use this
reference to match and configure the correct SPM.
>
> > +
> > +Optional properties
> > +
> > +- qcom,saw2-avs-limit: The AVS limit register
>
> same comments above (as qcom,saw2-spm-ctl)
>
> > +- qcom,saw2-avs-dly: The AVS delay register is used to specify the delay values
> > + between AVS controller requests
>
> is this a delay value, list of values, a register offset? not clear.
>
> > +- qcom,saw2-pmic-data0..7: Specify the pmic data value and the associated FTS
> > + index to send the PMIC data to
>
> what is FTS?
Fast Transient Switch of the PMIC regulator

>
> > +- qcom,vctl-port: The PVC (PMIC Virtual Channel) port used for changing
> > + voltage
> > +- qcom,phase-port: The PVC port used for changing the number of phases
> > +- qcom,pfm-port: The PVC port used for enabling PWM/PFM modes
> > +- qcom,saw2-spm-cmd-wfi: The WFI command sequence
> > +- qcom,saw2-spm-cmd-ret: The Retention command sequence
> > +- qcom,saw2-spm-cmd-spc: The Standalone PC command sequence
> > +- qcom,saw2-spm-cmd-pc-no-rpm: The Power Collapse command sequence where APPS
> > + proc won't inform the RPM.
> > +- qcom,saw2-spm-cmd-pc: The Power Collapse command sequence
> > +- qcom,saw2-spm-cmd-gdhs: L2 GDHS command sequence
>
> GDHS?
Globally Distributed Head Switch. Even though it doesnt match the state 
correctly, it has been a convention (internally) to use GDHS
to indicate that the L2 memory would be retained while the control logic
would be powered down as opposed to retention where both the memory and 
the control logic would be on.
>
> > +- qcom,cpu-vctl-mask: Mask of cpus, whose voltage the spm device can control.
> > + Depricated: Replaced with cpu-vctl-list when cpu phandles are available.
>
> if deprecated, remove it.
Will evaluate the need of this for 8064 and remove this.

- Lina