2023-04-11 07:30:07

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v3 0/3] arm64: dts: qcom: Add Qualcomm RB2 board dts

Changes since v2:
-----------------
- v2 can be viewed here: https://lore.kernel.org/linux-arm-msm/[email protected]/
- 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].

Changes since v1:
-----------------
- v1 can be viewed here: https://lore.kernel.org/linux-arm-msm/[email protected]/
- Addressed review comments from Konrad and fixed the board dts and also
added a new 'qcom,qrb4210' compatible.
- Although Krzysztof provided an Ack for [PATCH 1/2] from the v1 series,
since this series introduces the new 'qcom,qrb4210' compatible, so I
have dropped the same for now.

Add an initial device tree for Qualcomm RB2 board (see [1]).
It is based on the Robotics version of the Snapdragon SM4250
Soc, i.e. QRB4210.

Currently it enables:
- eMMC via SDHC1,
- uSD card via SDHC2,
- RPM regulators,
- Debug UART (via micro USB port).

Subsequent patchset(s) will add more peripherals like USB, etc.

This patchset is dependent on the QRB4210 SocInfo patchset sent out
earlier (see [2]).

To get a successful boot run:

$ cat arch/arm64/boot/Image.gz arch/arm64/boot/dts/qcom/\
qrb4210-rb2.dtb > ./Image-adp.gz+dtb

$ mkbootimg --kernel ./Image-adp.gz+dtb \
--ramdisk ./some-initramfs-image.rootfs.img \
--output ./rb2-boot.img --pagesize 4096 \
--base 0x80000000 --cmdline 'SOME_CMDLINE'

$ fastboot boot ./rb2-boot.img

[1]. https://www.qualcomm.com/products/internet-of-things/industrial/industrial-automation/qualcomm-robotics-rb2-platform#Overview
[2]. https://lore.kernel.org/linux-arm-msm/[email protected]/

Bhupesh Sharma (3):
dt-bindings: arm: qcom: Document the Qualcomm qrb4210-rb2 board
arm64: dts: qcom: Add base qrb4210-rb2 board dts
arm64: dts: qcom: qrb4210-rb2: Increase load on l22 and l24 for uSD
and eMMC

.../devicetree/bindings/arm/qcom.yaml | 8 +
arch/arm64/boot/dts/qcom/Makefile | 1 +
arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 227 ++++++++++++++++++
3 files changed, 236 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/qrb4210-rb2.dts

--
2.38.1


2023-04-11 07:30:28

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v3 2/3] arm64: dts: qcom: Add base qrb4210-rb2 board dts

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 <[email protected]>
---
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 {
+ regulator-min-microvolt = <920000>;
+ regulator-max-microvolt = <1128000>;
+ };
+
+ vreg_s6a_1p352: s6 {
+ regulator-min-microvolt = <304000>;
+ regulator-max-microvolt = <1456000>;
+ };
+
+ vreg_s7a_2p04: s7 {
+ regulator-min-microvolt = <1280000>;
+ regulator-max-microvolt = <2080000>;
+ };
+
+ vreg_l1a_1p0: l1 {
+ regulator-min-microvolt = <952000>;
+ regulator-max-microvolt = <1152000>;
+ };
+
+ vreg_l4a_0p9: l4 {
+ regulator-min-microvolt = <488000>;
+ regulator-max-microvolt = <1000000>;
+ };
+
+ vreg_l5a_2p96: l5 {
+ regulator-min-microvolt = <1648000>;
+ regulator-max-microvolt = <3056000>;
+ };
+
+ vreg_l6a_0p6: l6 {
+ regulator-min-microvolt = <576000>;
+ regulator-max-microvolt = <656000>;
+ };
+
+ vreg_l7a_1p256: l7 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1304000>;
+ };
+
+ vreg_l8a_0p664: l8 {
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <728000>;
+ };
+
+ vreg_l9a_1p8: l9 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <2000000>;
+ };
+
+ vreg_l10a_1p8: l10 {
+ regulator-min-microvolt = <1704000>;
+ regulator-max-microvolt = <1904000>;
+ };
+
+ vreg_l11a_1p8: l11 {
+ regulator-min-microvolt = <1704000>;
+ regulator-max-microvolt = <1952000>;
+ };
+
+ vreg_l12a_1p8: l12 {
+ regulator-min-microvolt = <1624000>;
+ regulator-max-microvolt = <1984000>;
+ };
+
+ vreg_l13a_1p8: l13 {
+ regulator-min-microvolt = <1504000>;
+ regulator-max-microvolt = <1952000>;
+ };
+
+ vreg_l14a_1p8: l14 {
+ regulator-min-microvolt = <1704000>;
+ regulator-max-microvolt = <1904000>;
+ };
+
+ vreg_l15a_3p128: l15 {
+ regulator-min-microvolt = <2920000>;
+ regulator-max-microvolt = <3232000>;
+ };
+
+ vreg_l16a_1p3: l16 {
+ regulator-min-microvolt = <1704000>;
+ regulator-max-microvolt = <1904000>;
+ };
+
+ vreg_l17a_1p3: l17 {
+ regulator-min-microvolt = <1152000>;
+ regulator-max-microvolt = <1384000>;
+ };
+
+ vreg_l18a_1p232: l18 {
+ regulator-min-microvolt = <1104000>;
+ regulator-max-microvolt = <1312000>;
+ };
+
+ vreg_l19a_1p8: l19 {
+ regulator-min-microvolt = <1624000>;
+ regulator-max-microvolt = <3304000>;
+ };
+
+ vreg_l20a_1p8: l20 {
+ regulator-min-microvolt = <1624000>;
+ regulator-max-microvolt = <3304000>;
+ };
+
+ vreg_l21a_2p704: l21 {
+ regulator-min-microvolt = <2400000>;
+ regulator-max-microvolt = <3600000>;
+ };
+
+ vreg_l22a_2p96: l22 {
+ regulator-min-microvolt = <2952000>;
+ regulator-max-microvolt = <3304000>;
+ };
+
+ vreg_l23a_3p3: l23 {
+ regulator-min-microvolt = <3200000>;
+ regulator-max-microvolt = <3400000>;
+ };
+
+ vreg_l24a_2p96: l24 {
+ regulator-min-microvolt = <2704000>;
+ regulator-max-microvolt = <3600000>;
+ };
+ };
+};
+
+&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 */
+ 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";
+};
+
+&xo_board {
+ clock-frequency = <19200000>;
+};
--
2.38.1

2023-04-11 07:30:50

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v3 1/3] dt-bindings: arm: qcom: Document the Qualcomm qrb4210-rb2 board

Document the Qualcomm qrb4210-rb2 board based on Robotics version
of the Snapdragon SM4250 Soc, i.e. QRB4210.

Acked-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/devicetree/bindings/arm/qcom.yaml | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 785b969294a8..d9dd25695c3d 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -50,6 +50,7 @@ description: |
qcm2290
qdu1000
qrb2210
+ qrb4210
qru1000
sa8155p
sa8540p
@@ -97,6 +98,7 @@ description: |
liquid
mtp
qrd
+ rb2
ride
sbc
x100
@@ -874,6 +876,12 @@ properties:
- oneplus,billie2
- const: qcom,sm4250

+ - items:
+ - enum:
+ - qcom,qrb4210-rb2
+ - const: qcom,qrb4210
+ - const: qcom,sm4250
+
- items:
- enum:
- lenovo,j606f
--
2.38.1

2023-04-11 07:31:35

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v3 3/3] arm64: dts: qcom: qrb4210-rb2: Increase load on l22 and l24 for uSD and eMMC

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 <[email protected]>
---
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;
};
};
};
--
2.38.1

2023-04-11 12:04:39

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: qcom: qrb4210-rb2: Increase load on l22 and l24 for uSD and eMMC



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 <[email protected]>
> ---
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;
> };
> };
> };

2023-04-11 12:16:08

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: qcom: qrb4210-rb2: Increase load on l22 and l24 for uSD and eMMC

On Tue, 11 Apr 2023 at 17:28, Konrad Dybcio <[email protected]> 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 <[email protected]>
> > ---
> 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

2023-04-11 12:51:56

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: qcom: qrb4210-rb2: Increase load on l22 and l24 for uSD and eMMC

On Tue, Apr 11, 2023 at 05:43:51PM +0530, Bhupesh Sharma wrote:
> On Tue, 11 Apr 2023 at 17:28, Konrad Dybcio <[email protected]> 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 <[email protected]>
> > > ---
> > 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

2023-04-11 12:58:55

by Bjorn Andersson

[permalink] [raw]
Subject: Re: (subset) [PATCH v3 0/3] arm64: dts: qcom: Add Qualcomm RB2 board dts

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/[email protected]/
> - 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,
--
Bjorn Andersson <[email protected]>

2023-04-11 12:59:19

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] arm64: dts: qcom: Add base qrb4210-rb2 board dts



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 <[email protected]>
> ---
> 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>;
> +};

2023-04-11 14:37:55

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: qcom: qrb4210-rb2: Increase load on l22 and l24 for uSD and eMMC

On Tue, 11 Apr 2023 at 18:09, Bjorn Andersson <[email protected]> wrote:
>
> On Tue, Apr 11, 2023 at 05:43:51PM +0530, Bhupesh Sharma wrote:
> > On Tue, 11 Apr 2023 at 17:28, Konrad Dybcio <[email protected]> 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 <[email protected]>
> > > > ---
> > > 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

2023-04-11 17:41:28

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] arm64: dts: qcom: Add base qrb4210-rb2 board dts

On Tue, 11 Apr 2023 at 18:26, Konrad Dybcio <[email protected]> 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 <[email protected]>
> > ---
> > 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

2023-04-11 18:08:53

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] arm64: dts: qcom: Add base qrb4210-rb2 board dts



On 11.04.2023 19:38, Bhupesh Sharma wrote:
> On Tue, 11 Apr 2023 at 18:26, Konrad Dybcio <[email protected]> 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 <[email protected]>
>>> ---
>>> 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