Message ID | 20230411072840.2751813-1-bhupesh.sharma@linaro.org |
---|---|
Headers | show |
Series | arm64: dts: qcom: Add Qualcomm RB2 board dts | expand |
On 11.04.2023 09:28, Bhupesh Sharma wrote: > Increase the l22 and l24 load used for uSD and eMMC VMMC. > These need to be increased in order to prevent any voltage drop > issues due to limited current happening during specific operations > (e.g. write). > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- You could have simply squashed this into the patch where you enabled the controllers, so that that commit works reliably for e.g. bisect Konrad > arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > index c9c6e3787462..dc80f0bca767 100644 > --- a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > +++ b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > @@ -171,6 +171,8 @@ vreg_l21a_2p704: l21 { > vreg_l22a_2p96: l22 { > regulator-min-microvolt = <2952000>; > regulator-max-microvolt = <3304000>; > + regulator-system-load = <100000>; > + regulator-allow-set-load; > }; > > vreg_l23a_3p3: l23 { > @@ -181,6 +183,8 @@ vreg_l23a_3p3: l23 { > vreg_l24a_2p96: l24 { > regulator-min-microvolt = <2704000>; > regulator-max-microvolt = <3600000>; > + regulator-system-load = <100000>; > + regulator-allow-set-load; > }; > }; > };
On Tue, 11 Apr 2023 at 17:28, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 11.04.2023 09:28, Bhupesh Sharma wrote: > > Increase the l22 and l24 load used for uSD and eMMC VMMC. > > These need to be increased in order to prevent any voltage drop > > issues due to limited current happening during specific operations > > (e.g. write). > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > You could have simply squashed this into the patch where > you enabled the controllers, so that that commit works > reliably for e.g. bisect Yes, but Bjorn asked me to send this separately (via irc). I am fine with squashing this with the previous patch [PATCH 2/3] as well, if Bjorn is OK with it. Thanks, Bhupesh
On Tue, Apr 11, 2023 at 05:43:51PM +0530, Bhupesh Sharma wrote: > On Tue, 11 Apr 2023 at 17:28, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > > > > > On 11.04.2023 09:28, Bhupesh Sharma wrote: > > > Increase the l22 and l24 load used for uSD and eMMC VMMC. > > > These need to be increased in order to prevent any voltage drop > > > issues due to limited current happening during specific operations > > > (e.g. write). > > > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > > --- > > You could have simply squashed this into the patch where > > you enabled the controllers, so that that commit works > > reliably for e.g. bisect > > Yes, but Bjorn asked me to send this separately (via irc). > I am fine with squashing this with the previous patch [PATCH 2/3] as > well, if Bjorn is OK with it. > I was trying to say that I was fine with you just fixing the small thing I had asked for and then you could send a separate patch for this when you found the time. I can squash the two while applying, unless anyone else have any concerns with the patches. Regards, Bjorn
On 11.04.2023 09:28, Bhupesh Sharma wrote: > Add DTS for Qualcomm qrb4210-rb2 board which uses SM4250 SoC. > > This adds debug uart, emmc, uSD and tlmm support along with > regulators found on this board. > > Also defines the 'xo_board' and 'sleep_clk' frequencies for > this board. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 223 +++++++++++++++++++++++ > 2 files changed, 224 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index e0e2def48470..d42c59572ace 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -74,6 +74,7 @@ dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb > dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb > dtb-$(CONFIG_ARCH_QCOM) += qdu1000-idp.dtb > dtb-$(CONFIG_ARCH_QCOM) += qrb2210-rb1.dtb > +dtb-$(CONFIG_ARCH_QCOM) += qrb4210-rb2.dtb > dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5.dtb > dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5-vision-mezzanine.dtb > dtb-$(CONFIG_ARCH_QCOM) += qru1000-idp.dtb > diff --git a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > new file mode 100644 > index 000000000000..c9c6e3787462 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > @@ -0,0 +1,223 @@ > +// SPDX-License-Identifier: BSD-3-Clause > +/* > + * Copyright (c) 2023, Linaro Limited > + */ > + > +/dts-v1/; > + > +#include "sm4250.dtsi" > + > +/ { > + model = "Qualcomm Technologies, Inc. QRB4210 RB2"; > + compatible = "qcom,qrb4210-rb2", "qcom,qrb4210", "qcom,sm4250"; > + > + aliases { > + serial0 = &uart4; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + 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; > + }; > +}; > + > +&qupv3_id_0 { > + status = "okay"; > +}; > + > +&rpm_requests { > + regulators { > + compatible = "qcom,rpm-pm6125-regulators"; > + > + 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-l7-l17-l18-supply = <&vreg_s6a_1p352>; > + vdd-l2-l3-l4-supply = <&vreg_s6a_1p352>; > + vdd-l5-l15-l19-l20-l21-l22-supply = <&vph_pwr>; > + vdd-l6-l8-supply = <&vreg_s5a_0p848>; > + vdd-l9-l11-supply = <&vreg_s7a_2p04>; > + vdd-l10-l13-l14-supply = <&vreg_s7a_2p04>; > + vdd-l12-l16-supply = <&vreg_s7a_2p04>; > + vdd-l23-l24-supply = <&vph_pwr>; > + > + vreg_s5a_0p848: s5 { I think going with pmicname_regulatorname (e.g. pm6125_s5) here and adding: regulator-name = "vreg_s5a_0p848" would make this more maintainable. [...] > +&sdhc_1 { > + vmmc-supply = <&vreg_l24a_2p96>; > + vqmmc-supply = <&vreg_l11a_1p8>; > + no-sdio; > + non-removable; > + > + status = "okay"; > +}; > + > +&sdhc_2 { > + cd-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>; /* card detect gpio */ This comment is still pretty much spam. > + vmmc-supply = <&vreg_l22a_2p96>; > + vqmmc-supply = <&vreg_l5a_2p96>; > + no-sdio; > + > + status = "okay"; > +}; > + > +&sleep_clk { > + clock-frequency = <32000>; > +}; > + > +&tlmm { > + gpio-reserved-ranges = <37 5>, <43 2>, <47 1>, > + <49 1>, <52 1>, <54 1>, > + <56 3>, <61 2>, <64 1>, > + <68 1>, <72 8>, <96 1>; > +}; > + > +&uart4 { > + status = "okay"; > +}; This is not the correct SE for the production board. People booting this will get a tz bite. LGTM otherwise. Konrad > + > +&xo_board { > + clock-frequency = <19200000>; > +};
On Tue, 11 Apr 2023 12:58:37 +0530, Bhupesh Sharma wrote: > Changes since v2: > ----------------- > - v2 can be viewed here: https://lore.kernel.org/linux-arm-msm/20230315210145.2221116-1-bhupesh.sharma@linaro.org/ > - Addressed review comments from Bjorn about load conditions for vmmc > ldos and added [PATCH 3/3] accordingly in v3. > - Collected Krzysztof's Ack for [PATCH 1/3]. > > [...] Applied, thanks! [2/3] arm64: dts: qcom: Add base qrb4210-rb2 board dts commit: 8d58a8c0d930c52dd30bd50af24b786d55509cbf [3/3] arm64: dts: qcom: qrb4210-rb2: Increase load on l22 and l24 for uSD and eMMC (no commit info) Best regards,
On Tue, 11 Apr 2023 at 18:09, Bjorn Andersson <andersson@kernel.org> wrote: > > On Tue, Apr 11, 2023 at 05:43:51PM +0530, Bhupesh Sharma wrote: > > On Tue, 11 Apr 2023 at 17:28, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > > > > > > > > > On 11.04.2023 09:28, Bhupesh Sharma wrote: > > > > Increase the l22 and l24 load used for uSD and eMMC VMMC. > > > > These need to be increased in order to prevent any voltage drop > > > > issues due to limited current happening during specific operations > > > > (e.g. write). > > > > > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > > > --- > > > You could have simply squashed this into the patch where > > > you enabled the controllers, so that that commit works > > > reliably for e.g. bisect > > > > Yes, but Bjorn asked me to send this separately (via irc). > > I am fine with squashing this with the previous patch [PATCH 2/3] as > > well, if Bjorn is OK with it. > > > > I was trying to say that I was fine with you just fixing the small thing > I had asked for and then you could send a separate patch for this when > you found the time. > > I can squash the two while applying, unless anyone else have any > concerns with the patches. Sounds good to me. Thanks for your help Bjorn with squashing the patches. Thanks, Bhupesh
On Tue, 11 Apr 2023 at 18:26, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > On 11.04.2023 09:28, Bhupesh Sharma wrote: > > Add DTS for Qualcomm qrb4210-rb2 board which uses SM4250 SoC. > > > > This adds debug uart, emmc, uSD and tlmm support along with > > regulators found on this board. > > > > Also defines the 'xo_board' and 'sleep_clk' frequencies for > > this board. > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/Makefile | 1 + > > arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 223 +++++++++++++++++++++++ > > 2 files changed, 224 insertions(+) > > create mode 100644 arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > > > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > > index e0e2def48470..d42c59572ace 100644 > > --- a/arch/arm64/boot/dts/qcom/Makefile > > +++ b/arch/arm64/boot/dts/qcom/Makefile > > @@ -74,6 +74,7 @@ dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb > > dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb > > dtb-$(CONFIG_ARCH_QCOM) += qdu1000-idp.dtb > > dtb-$(CONFIG_ARCH_QCOM) += qrb2210-rb1.dtb > > +dtb-$(CONFIG_ARCH_QCOM) += qrb4210-rb2.dtb > > dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5.dtb > > dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5-vision-mezzanine.dtb > > dtb-$(CONFIG_ARCH_QCOM) += qru1000-idp.dtb > > diff --git a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > > new file mode 100644 > > index 000000000000..c9c6e3787462 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts > > @@ -0,0 +1,223 @@ > > +// SPDX-License-Identifier: BSD-3-Clause > > +/* > > + * Copyright (c) 2023, Linaro Limited > > + */ > > + > > +/dts-v1/; > > + > > +#include "sm4250.dtsi" > > + > > +/ { > > + model = "Qualcomm Technologies, Inc. QRB4210 RB2"; > > + compatible = "qcom,qrb4210-rb2", "qcom,qrb4210", "qcom,sm4250"; > > + > > + aliases { > > + serial0 = &uart4; > > + }; > > + > > + chosen { > > + stdout-path = "serial0:115200n8"; > > + }; > > + > > + 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; > > + }; > > +}; > > + > > +&qupv3_id_0 { > > + status = "okay"; > > +}; > > + > > +&rpm_requests { > > + regulators { > > + compatible = "qcom,rpm-pm6125-regulators"; > > + > > + 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-l7-l17-l18-supply = <&vreg_s6a_1p352>; > > + vdd-l2-l3-l4-supply = <&vreg_s6a_1p352>; > > + vdd-l5-l15-l19-l20-l21-l22-supply = <&vph_pwr>; > > + vdd-l6-l8-supply = <&vreg_s5a_0p848>; > > + vdd-l9-l11-supply = <&vreg_s7a_2p04>; > > + vdd-l10-l13-l14-supply = <&vreg_s7a_2p04>; > > + vdd-l12-l16-supply = <&vreg_s7a_2p04>; > > + vdd-l23-l24-supply = <&vph_pwr>; > > + > > + vreg_s5a_0p848: s5 { > I think going with pmicname_regulatorname (e.g. pm6125_s5) here > and adding: > > regulator-name = "vreg_s5a_0p848" > > would make this more maintainable. Ok. > > +&sdhc_1 { > > + vmmc-supply = <&vreg_l24a_2p96>; > > + vqmmc-supply = <&vreg_l11a_1p8>; > > + no-sdio; > > + non-removable; > > + > > + status = "okay"; > > +}; > > + > > +&sdhc_2 { > > + cd-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>; /* card detect gpio */ > This comment is still pretty much spam. Ok. > > + vmmc-supply = <&vreg_l22a_2p96>; > > + vqmmc-supply = <&vreg_l5a_2p96>; > > + no-sdio; > > + > > + status = "okay"; > > +}; > > + > > +&sleep_clk { > > + clock-frequency = <32000>; > > +}; > > + > > +&tlmm { > > + gpio-reserved-ranges = <37 5>, <43 2>, <47 1>, > > + <49 1>, <52 1>, <54 1>, > > + <56 3>, <61 2>, <64 1>, > > + <68 1>, <72 8>, <96 1>; > > +}; > > + > > +&uart4 { > > + status = "okay"; > > +}; > This is not the correct SE for the production board. People > booting this will get a tz bite. Hmm.. I can swap it, but the problem is that it's as the SE for my RB2 board, so I would rather provide instructions in the cover letter as to how to swap it (say for a production board) and recompile the dts. Otherwise, it might break the debug uart console for even the test folks @ Qualcomm. I will send a v4 accordingly. Thanks, Bhupesh
On 11.04.2023 19:38, Bhupesh Sharma wrote: > On Tue, 11 Apr 2023 at 18:26, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> On 11.04.2023 09:28, Bhupesh Sharma wrote: >>> Add DTS for Qualcomm qrb4210-rb2 board which uses SM4250 SoC. >>> >>> This adds debug uart, emmc, uSD and tlmm support along with >>> regulators found on this board. >>> >>> Also defines the 'xo_board' and 'sleep_clk' frequencies for >>> this board. >>> >>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> >>> --- >>> arch/arm64/boot/dts/qcom/Makefile | 1 + >>> arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 223 +++++++++++++++++++++++ >>> 2 files changed, 224 insertions(+) >>> create mode 100644 arch/arm64/boot/dts/qcom/qrb4210-rb2.dts >>> >>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile >>> index e0e2def48470..d42c59572ace 100644 >>> --- a/arch/arm64/boot/dts/qcom/Makefile >>> +++ b/arch/arm64/boot/dts/qcom/Makefile >>> @@ -74,6 +74,7 @@ dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb >>> dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb >>> dtb-$(CONFIG_ARCH_QCOM) += qdu1000-idp.dtb >>> dtb-$(CONFIG_ARCH_QCOM) += qrb2210-rb1.dtb >>> +dtb-$(CONFIG_ARCH_QCOM) += qrb4210-rb2.dtb >>> dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5.dtb >>> dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5-vision-mezzanine.dtb >>> dtb-$(CONFIG_ARCH_QCOM) += qru1000-idp.dtb >>> diff --git a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts >>> new file mode 100644 >>> index 000000000000..c9c6e3787462 >>> --- /dev/null >>> +++ b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts >>> @@ -0,0 +1,223 @@ >>> +// SPDX-License-Identifier: BSD-3-Clause >>> +/* >>> + * Copyright (c) 2023, Linaro Limited >>> + */ >>> + >>> +/dts-v1/; >>> + >>> +#include "sm4250.dtsi" >>> + >>> +/ { >>> + model = "Qualcomm Technologies, Inc. QRB4210 RB2"; >>> + compatible = "qcom,qrb4210-rb2", "qcom,qrb4210", "qcom,sm4250"; >>> + >>> + aliases { >>> + serial0 = &uart4; >>> + }; >>> + >>> + chosen { >>> + stdout-path = "serial0:115200n8"; >>> + }; >>> + >>> + 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; >>> + }; >>> +}; >>> + >>> +&qupv3_id_0 { >>> + status = "okay"; >>> +}; >>> + >>> +&rpm_requests { >>> + regulators { >>> + compatible = "qcom,rpm-pm6125-regulators"; >>> + >>> + 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-l7-l17-l18-supply = <&vreg_s6a_1p352>; >>> + vdd-l2-l3-l4-supply = <&vreg_s6a_1p352>; >>> + vdd-l5-l15-l19-l20-l21-l22-supply = <&vph_pwr>; >>> + vdd-l6-l8-supply = <&vreg_s5a_0p848>; >>> + vdd-l9-l11-supply = <&vreg_s7a_2p04>; >>> + vdd-l10-l13-l14-supply = <&vreg_s7a_2p04>; >>> + vdd-l12-l16-supply = <&vreg_s7a_2p04>; >>> + vdd-l23-l24-supply = <&vph_pwr>; >>> + >>> + vreg_s5a_0p848: s5 { >> I think going with pmicname_regulatorname (e.g. pm6125_s5) here >> and adding: >> >> regulator-name = "vreg_s5a_0p848" >> >> would make this more maintainable. > > Ok. > >>> +&sdhc_1 { >>> + vmmc-supply = <&vreg_l24a_2p96>; >>> + vqmmc-supply = <&vreg_l11a_1p8>; >>> + no-sdio; >>> + non-removable; >>> + >>> + status = "okay"; >>> +}; >>> + >>> +&sdhc_2 { >>> + cd-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>; /* card detect gpio */ >> This comment is still pretty much spam. > > Ok. > >>> + vmmc-supply = <&vreg_l22a_2p96>; >>> + vqmmc-supply = <&vreg_l5a_2p96>; >>> + no-sdio; >>> + >>> + status = "okay"; >>> +}; >>> + >>> +&sleep_clk { >>> + clock-frequency = <32000>; >>> +}; >>> + >>> +&tlmm { >>> + gpio-reserved-ranges = <37 5>, <43 2>, <47 1>, >>> + <49 1>, <52 1>, <54 1>, >>> + <56 3>, <61 2>, <64 1>, >>> + <68 1>, <72 8>, <96 1>; >>> +}; >>> + >>> +&uart4 { >>> + status = "okay"; >>> +}; >> This is not the correct SE for the production board. People >> booting this will get a tz bite. > > Hmm.. I can swap it, but the problem is that it's as the SE for my RB2 > board, so I would rather provide instructions in the cover letter as to how > to swap it (say for a production board) and recompile the dts. That's what I did for RB1 and what I believe is the correct approach. > > Otherwise, it might break the debug uart console for even the test > folks @ Qualcomm. Perhaps that'll teach them a lesson about making major design changes and sharing the details on release day.. We can take care of preproduction boards after we set up U-Boot with hw rev recognition, but that's a story for another day. > I will send a v4 accordinglyBjorn sent a "thank you" email already but I don't see the patches on his branch, not sure how he wants to proceed here. Konrad > > Thanks, > Bhupesh