On Aug 8, 2014, at 4:53 PM, Lina Iyer <lina.iyer@xxxxxxxxxx> wrote:
> 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@xxxxxxxxxx> 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?
If you have the cpu-phandle, you should be able to get the L2 cache via the 'next-level-cache’ property, is that not sufficient?
>> +- 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.
Ok, we do we need to specify these in the device tree? Are these properties the register offsets or the values to program into the register?
> +- qcom,saw2-avs-ctl: The AVS control register
>>
>> What does this even mean, why is this not part of the reg property?
> Same.
Same question as above
> +- 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.
So, one there is typo in the name compares to the property. Again, trying to understand if this is the value to init the register to or the offset.
>>
>>> +- qcom,saw2-spm-dly: Provides the values for the SPM delay command in the SPM
>>> + sequence
>>
>> how many values?
> HW configuration value.
Huh? Are you saying the # of values varies?
>>
>>> +- 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.
Can you be more descriptive. Is this the value to set the saw2-spm-ctl register to?
>>
>>> +- 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.
Hmm, I’m not sure how to handle this right now. It seems a little chicken and egg case, since we don’t have support for qcom,name.
>> +
>>> +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
In general with acronyms, please do something like FTS (Fast Transient Switch) for the first reference.
> +- 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.
Some comment about acronyms
>>
>>> +- 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.
We should fixup 8064 if need by to not require this if that is feasible.
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
