On 11.04.2023 19:38, Bhupesh Sharma wrote: > On Tue, 11 Apr 2023 at 18:26, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> 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@xxxxxxxxxx> >>> --- >>> 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