Re: [PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

Konrad Dybcio
2022-04-20T17:00:49Z

[-- Attachment #1: Type: text/plain, Size: 4109 bytes --]

On 19/04/2022 22:58, Bhupesh Sharma wrote:
> As suggested by Bjorn during review of [1], the 'vdda-pll-supply' &
> 'vdda-phy-supply' supplies denote the power for the bus and the
> clock of the usb qmp phy and are used by the qcom qmp phy driver.
>
> So, its safe to assume that the two regulators are the same as on
> the MTP. So let's wire them up in the same way as the MTP.
>
> In absence of the same 'make dtbs_check' leads to following warnings:
>
> arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml:
> phy-wrapper@88e9000: 'vdda-phy-supply' is a required property
>
> arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml:
> phy-wrapper@88e9000: 'vdda-pll-supply' is a required property
>
> [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/
>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: konrad.dybcio@somainline.org
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
> Changes since v1:
> -----------------
> - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html
> - Fixed the commit message to read usb qmp phy instead of ufs phy (which
> was introduced erroraneously in the commit log).
>
> .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++
> 1 file changed, 25 insertions(+)

Hi, I actually threw together a patch with [almost] all RPMh regulators
on Sagami quite a while ago (if thunderbird doesn't explode, you should
find it as an attachment to this message), but I haven't sent it, as
pm8350c_l3 and pmr735a_s3 are broken on the .c driver side (the step
numbers and/or values are not correct), but Qualcomm in their infinite
Qualcommery will not let us, mere mortals, access the PDFs that contain
the correct specifications, so I have to wait for somebody with access
to them to pick them up. That said, the values you supplied seem correct
(say for lack of allow-set-load which will bite when (if?) UFS is ever
functional upstream on this piece of Japanese engineering), but adding
the configuration of all regulators at once just seems more complete to me..

On a note, USB - among other more or less necessary peripherals - along
with its supplies, is brought up in XBL (or bootrom if you jump to EDL),
so not setting these supplies in Linux does not bite in any way, shape
or form other than "make dtbs_check" screaming.

I'm not against this patch, but once again, even though this point may
not sound very convincing to you all, I think it would be "nice" to
configure all regulators in one go.

Konrad

> diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
> index 90b13cbe2fa6..238ac9380ca2 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
> @@ -3,6 +3,7 @@
> * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org>
> */
>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> #include "sm8350.dtsi"
> #include "pm8350.dtsi"
> #include "pm8350b.dtsi"
> @@ -75,6 +76,27 @@ ramoops@ffc00000 {
> };
> };
>
> +&apps_rsc {
> + pm8350-rpmh-regulators {
> + compatible = "qcom,pm8350-rpmh-regulators";
> + qcom,pmic-id = "b";
> +
> + vreg_l1b_0p88: ldo1 {
> + regulator-name = "vreg_l1b_0p88";
> + regulator-min-microvolt = <912000>;
> + regulator-max-microvolt = <920000>;
> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> + };
> +
> + vreg_l6b_1p2: ldo6 {
> + regulator-name = "vreg_l6b_1p2";
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1208000>;
> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> + };
> + };
> +};
> +
> &adsp {
> status = "okay";
> firmware-name = "qcom/adsp.mbn";
> @@ -256,4 +278,7 @@ &usb_1_hsphy {
>
> &usb_1_qmpphy {
> status = "okay";
> +
> + vdda-phy-supply = <&vreg_l6b_1p2>;
> + vdda-pll-supply = <&vreg_l1b_0p88>;
> };

[-- Attachment #2: sagami_regulators.patch --]
[-- Type: text/plain, Size: 10687 bytes --]

From da20c2afa2c0715ef78136cd670ee9c1db3adf6c Mon Sep 17 00:00:00 2001
From: Konrad Dybcio <konrad.dybcio@somainline.org>
Date: Sat, 5 Mar 2022 17:08:58 +0100
Subject: [PATCH] arm64: dts: qcom: sm8350-sagami: Add most RPMh regulators

Configure most RPMh-controlled regulators on SoMC Sagami. The missing
ones (on pm8350b and pm8008[ij]) will be configured when driver support
is added. Thankfully, it looks like PDX215 and PDX214 don't have any
differences when it comes to PM8350/PM8350C/PMR735a.

Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---
.../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 350 ++++++++++++++++++
1 file changed, 350 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
index 90b13cbe2fa63..81363f367d452 100644
--- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
@@ -3,6 +3,7 @@
* Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org>
*/

+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
#include "sm8350.dtsi"
#include "pm8350.dtsi"
#include "pm8350b.dtsi"
@@ -73,6 +74,16 @@
no-map;
};
};
+
+ vph_pwr: vph-pwr-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vph_pwr";
+ regulator-min-microvolt = <3700000>;
+ regulator-max-microvolt = <3700000>;
+
+ regulator-always-on;
+ regulator-boot-on;
+ };
};

&adsp {
@@ -80,6 +91,345 @@
firmware-name = "qcom/adsp.mbn";
};

+&apps_rsc {
+ pm8350-rpmh-regulators {
+ compatible = "qcom,pm8350-rpmh-regulators";
+ qcom,pmic-id = "b";
+
+ vdd-s1-supply = <&vph_pwr>;
+ vdd-s2-supply = <&vph_pwr>;
+ vdd-s3-supply = <&vph_pwr>;
+ vdd-s4-supply = <&vph_pwr>;
+ vdd-s5-supply = <&vph_pwr>;
+ vdd-s6-supply = <&vph_pwr>;
+ vdd-s7-supply = <&vph_pwr>;
+ vdd-s8-supply = <&vph_pwr>;
+ vdd-s9-supply = <&vph_pwr>;
+ vdd-s10-supply = <&vph_pwr>;
+ vdd-s11-supply = <&vph_pwr>;
+ vdd-s12-supply = <&vph_pwr>;
+
+ vdd-l1-l4-supply = <&pm8350_s11>;
+ vdd-l2-l7-supply = <&vreg_bob>;
+ vdd-l3-l5-supply = <&vreg_bob>;
+ vdd-l6-l9-l10-supply = <&pm8350_s11>;
+
+ /*
+ * ARC regulators:
+ * S5 - mx.lvl
+ * S6 - gfx.lvl
+ * S9 - mxc.lvl
+ */
+
+ pm8350_s10: smps10 {
+ regulator-name = "pm8350_s10";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350_s11: smps11 {
+ regulator-name = "pm8350_s11";
+ regulator-min-microvolt = <752000>;
+ regulator-max-microvolt = <1000000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350_s12: smps12 {
+ regulator-name = "pm8350_s12";
+ regulator-min-microvolt = <1224000>;
+ regulator-max-microvolt = <1360000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350_l1: ldo1 {
+ regulator-name = "pm8350_l1";
+ regulator-min-microvolt = <912000>;
+ regulator-max-microvolt = <920000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350_l2: ldo2 {
+ regulator-name = "pm8350_l2";
+ regulator-min-microvolt = <3072000>;
+ regulator-max-microvolt = <3072000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350_l3: ldo3 {
+ regulator-name = "pm8350_l3";
+ regulator-min-microvolt = <904000>;
+ regulator-max-microvolt = <904000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ /* L4 - lmx.lvl (ARC) */
+
+ pm8350_l5: ldo5 {
+ regulator-name = "pm8350_l5";
+ regulator-min-microvolt = <880000>;
+ regulator-max-microvolt = <888000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-allow-set-load;
+ };
+
+ pm8350_l6: ldo6 {
+ regulator-name = "pm8350_l6";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1208000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-allow-set-load;
+ };
+
+ pm8350_l7: ldo7 {
+ regulator-name = "pm8350_l7";
+ regulator-min-microvolt = <2400000>;
+ regulator-max-microvolt = <3008000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-allow-set-load;
+ };
+
+ /* L8 - lcx.lvl (ARC) */
+
+ pm8350_l9: ldo9 {
+ regulator-name = "pm8350_l9";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-allow-set-load;
+ };
+ };
+
+ pm8350c-rpmh-regulators {
+ compatible = "qcom,pm8350c-rpmh-regulators";
+ qcom,pmic-id = "c";
+
+ vdd-s1-supply = <&vph_pwr>;
+ vdd-s2-supply = <&vph_pwr>;
+ vdd-s3-supply = <&vph_pwr>;
+ vdd-s4-supply = <&vph_pwr>;
+ vdd-s5-supply = <&vph_pwr>;
+ vdd-s6-supply = <&vph_pwr>;
+ vdd-s7-supply = <&vph_pwr>;
+ vdd-s8-supply = <&vph_pwr>;
+ vdd-s9-supply = <&vph_pwr>;
+ vdd-s10-supply = <&vph_pwr>;
+
+ vdd-l1-l12-supply = <&pm8350c_s1>;
+ vdd-l2-l8-supply = <&pm8350c_s1>;
+ vdd-l3-l4-l5-l7-l13-supply = <&vreg_bob>;
+ vdd-l6-l9-l11-supply = <&vreg_bob>;
+ vdd-l10-supply = <&pm8350_s12>;
+
+ vdd-bob-supply = <&vph_pwr>;
+
+ pm8350c_s1: smps1 {
+ regulator-name = "pm8350c_s1";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1952000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ /* S2 - ebi.lvl (ARC) */
+
+ pm8350c_s3: smps3 {
+ regulator-name = "pm8350c_s3";
+ regulator-min-microvolt = <300000>;
+ regulator-max-microvolt = <704000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ /*
+ * ARC regulators:
+ * S4 - mss.lvl
+ * S6 - cx.lvl
+ * S8 - mmcx.lvl
+ */
+
+ pm8350c_s10: smps10 {
+ regulator-name = "pm8350c_s10";
+ regulator-min-microvolt = <1048000>;
+ regulator-max-microvolt = <1128000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l1: ldo1 {
+ regulator-name = "pm8350c_l1";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l2: ldo2 {
+ regulator-name = "pm8350c_l2";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l3: ldo3 {
+ regulator-name = "pm8350c_l3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l4: ldo4 {
+ regulator-name = "pm8350c_l4";
+ regulator-min-microvolt = <1704000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l5: ldo5 {
+ regulator-name = "pm8350c_l5";
+ regulator-min-microvolt = <1704000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l6: ldo6 {
+ regulator-name = "pm8350c_l6";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <2960000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l7: ldo7 {
+ regulator-name = "pm8350c_l7";
+ regulator-min-microvolt = <3008000>;
+ regulator-max-microvolt = <3008000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l8: ldo8 {
+ regulator-name = "pm8350c_l8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l9: ldo9 {
+ regulator-name = "pm8350c_l9";
+ regulator-min-microvolt = <2960000>;
+ regulator-max-microvolt = <3008000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l10: ldo10 {
+ regulator-name = "pm8350c_l10";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l11: ldo11 {
+ regulator-name = "pm8350c_l11";
+ regulator-min-microvolt = <2400000>;
+ regulator-max-microvolt = <3008000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l12: ldo12 {
+ regulator-name = "pm8350c_l12";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <2000000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ pm8350c_l13: ldo13 {
+ regulator-name = "pm8350c_l13";
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_bob: bob {
+ regulator-name = "vreg_bob";
+ regulator-min-microvolt = <3400000>;
+ regulator-max-microvolt = <3960000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+ };
+ };
+
+ /* TODO: Add pm8350b (just one ldo) once the driver part is in */
+
+ pmr735a-rpmh-regulators {
+ compatible = "qcom,pmr735a-rpmh-regulators";
+ qcom,pmic-id = "e";
+
+ vdd-s1-supply = <&vph_pwr>;
+ vdd-s2-supply = <&vph_pwr>;
+ vdd-s3-supply = <&vph_pwr>;
+
+ vdd-l1-l2-supply = <&pmr735a_s2>;
+ vdd-l3-supply = <&pmr735a_s1>;
+ vdd-l4-supply = <&pm8350c_s1>;
+ vdd-l5-l6-supply = <&pm8350c_s1>;
+ vdd-l7-bob-supply = <&vreg_bob>;
+
+ pmr735a_s1: smps1 {
+ regulator-name = "pmr735a_s1";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1280000>;
+ };
+
+ pmr735a_s2: smps2 {
+ regulator-name = "pmr735a_s2";
+ regulator-min-microvolt = <500000>;
+ regulator-max-microvolt = <976000>;
+ };
+
+ pmr735a_s3: smps3 {
+ regulator-name = "pmr735a_s3";
+ regulator-min-microvolt = <2200000>;
+ regulator-max-microvolt = <2352000>;
+ };
+
+ pmr735a_l1: ldo1 {
+ regulator-name = "pmr735a_l1";
+ regulator-min-microvolt = <912000>;
+ regulator-max-microvolt = <912000>;
+ };
+
+ pmr735a_l2: ldo2 {
+ regulator-name = "pmr735a_l2";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+
+ pmr735a_l3: ldo3 {
+ regulator-name = "pmr735a_l3";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+
+ pmr735a_l4: ldo4 {
+ regulator-name = "pmr735a_l4";
+ regulator-min-microvolt = <1776000>;
+ regulator-max-microvolt = <1872000>;
+ };
+
+ pmr735a_l5: ldo5 {
+ regulator-name = "pmr735a_l5";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <800000>;
+ };
+
+ pmr735a_l6: ldo6 {
+ regulator-name = "pmr735a_l6";
+ regulator-min-microvolt = <480000>;
+ regulator-max-microvolt = <904000>;
+ };
+
+ pmr735a_l7: ldo7 {
+ regulator-name = "pmr735a_l7";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+ };
+};
+
&cdsp {
status = "okay";
firmware-name = "qcom/cdsp.mbn";