On 20/04/2022 21:33, Bhupesh Sharma wrote:
Hi Konrad,
Thanks for your comments. Please see my comments below:
On Wed, 20 Apr 2022 at 22:30, Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxx> wrote:
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@xxxxxxxxxx/
Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
Cc: konrad.dybcio@xxxxxxxxxxxxxx
Cc: Vinod Koul <vkoul@xxxxxxxxxx>
Cc: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxxxxxx>
---
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..
The complete set of regulator enablement was something on my mind and
which I tried before sending out the smaller (only relevant to USB)
patch.
My main concern was leaving the upstream kernel on the board in an
unbootable form, as I don't have the sagami board to test the changes
on.
Yeah, messing with power rails on somebody else's device is stressful in
general, especially with downstream kernels #including 9999 other DTs,
it's really easy to make a mistake.. Personally, I sometimes even dump
the regulator settings from syfs on a running device just to be ultra sure.
That said, if you intend to send the complete regulator enablement
patch for the sagami board, it would be probably better in the longer
run (as currently any change to the common binding, dts or driver
files is held up by "make dtbs_check" warnings during upstream
review).
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 agree. If you want, you can include my simple patch as the followup
(just enabling the right regulators for the usb qmp phy), to your big
patch (which enables the complete regulator support). It should be
fine as well, as we eventually want to use the regulators defined in
the 'big' patch for some 'real' peripheral nodes like the usb qmp phy.
Please let me know your views.
Regards,
Bhupesh
I will send this patch (of course rebased) along with the
add-all-regulators one, once the .c driver is fixed then.
Thanks,
Konrad
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@xxxxxxxxxxxxxx>
*/
+#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>;
};
