On 26.05.2023 01:39, Konrad Dybcio wrote:
>
>
> On 17.05.2023 20:48, Stephan Gerhold wrote:
>> Some of the regulators must be always-on to ensure correct operation of
>> the system, e.g. PM8916 L2 for the LPDDR RAM, L5 for most digital I/O
>> and L7 for the CPU PLL (strictly speaking the CPU PLL might only need
>> an active-only vote but this is not supported for regulators in
>> mainline currently).
> Would you be interested in implementing this?
Actually, I think currently all votes are active-only votes and what
we're missing is sleep-only (and active-sleep if we vote on both)
Konrad
>
> Ancient downstream defines a second device (vregname_ao) and basically
> seems to select QCOM_SMD_(ACTIVE/SLEEP)_STATE based on that..
>
> Looks like `struct regulator` stores voltage in an array that wouldn't
> you know it, depends on the PM state. Perhaps that could be something
> to explore!
>
> Konrad
>
>>
>> The RPM firmware seems to enforce that internally, these supplies stay
>> on even if we vote for them to power off (and there is no other
>> processor running). This means it's pointless to keep sending
>> enable/disable requests because they will just be ignored.
>> Also, drivers are much more likely to get a wrong impression of the
>> regulator status, because regulator_is_enabled() will return false when
>> there are no users, even though the regulator is always on.
>>
>> Describe this properly by marking the regulators as always-on.
>>
>> Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
>> ---
>> arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 5 -----
>> arch/arm64/boot/dts/qcom/msm8916-pm8916.dtsi | 5 +++++
>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
>> index ab8dfd858025..1c5d55854893 100644
>> --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
>> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
>> @@ -358,11 +358,6 @@ pm8916_l17: l17 {
>> };
>> };
>>
>> -&pm8916_s4 {
>> - regulator-always-on;
>> - regulator-boot-on;
>> -};
>> -
>> &sdhc_1 {
>> status = "okay";
>>
>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-pm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pm8916.dtsi
>> index b38eecbd6253..64d7228bee07 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8916-pm8916.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8916-pm8916.dtsi
>> @@ -72,11 +72,13 @@ pm8916_rpm_regulators: regulators {
>> pm8916_s3: s3 {
>> regulator-min-microvolt = <1250000>;
>> regulator-max-microvolt = <1350000>;
>> + regulator-always-on; /* Needed for L2 */
>> };
>>
>> pm8916_s4: s4 {
>> regulator-min-microvolt = <1850000>;
>> regulator-max-microvolt = <2150000>;
>> + regulator-always-on; /* Needed for L5/L7 */
>> };
>>
>> /*
>> @@ -93,6 +95,7 @@ pm8916_s4: s4 {
>> pm8916_l2: l2 {
>> regulator-min-microvolt = <1200000>;
>> regulator-max-microvolt = <1200000>;
>> + regulator-always-on; /* Needed for LPDDR RAM */
>> };
>>
>> /* pm8916_l3 is managed by rpmpd (MSM8916_VDDMX) */
>> @@ -102,6 +105,7 @@ pm8916_l2: l2 {
>> pm8916_l5: l5 {
>> regulator-min-microvolt = <1800000>;
>> regulator-max-microvolt = <1800000>;
>> + regulator-always-on; /* Needed for most digital I/O */
>> };
>>
>> pm8916_l6: l6 {
>> @@ -112,6 +116,7 @@ pm8916_l6: l6 {
>> pm8916_l7: l7 {
>> regulator-min-microvolt = <1800000>;
>> regulator-max-microvolt = <1800000>;
>> + regulator-always-on; /* Needed for CPU PLL */
>> };
>>
>> pm8916_l8: l8 {
>>
