Messages in this thread | | | Date | Wed, 15 Mar 2023 15:29:55 +0530 | Subject | Re: [PATCH 2/2] arm64: dts: qcom: Add base qrb4210-rb2 board dts | From | Bhupesh Sharma <> |
| |
On 3/15/23 2:55 AM, Konrad Dybcio wrote: > > > On 14.03.2023 22:08, 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> >> --- > [...] > >> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > This SoC does not feature RPMh, drop.
Ok.
>> +#include "sm4250.dtsi" >> + >> +/ { >> + model = "Qualcomm Technologies, Inc. QRB4210 RB2"; >> + compatible = "qcom,qrb4210-rb2", "qcom,sm4250"; > Please add a qcom,qrb4210 between the board-specific and the common SoC > compatibles so that we can address QRB-specific quirks if such ever arise.
As per the available documentation there are no qrb specific quirks as of now, but let me add a qcom,qrb4210 for future compatibility.
>> + >> + aliases { >> + serial0 = &uart4; >> + }; >> + > [...] > >> +&xo_board { >> + clock-frequency = <19200000>; >> +}; >> + >> +&sleep_clk { >> + clock-frequency = <32000>; >> +}; > Out of alphanumerical order
Ok.
>> + >> +&qupv3_id_0 { >> + status = "okay"; >> +}; >> + >> +&uart4 { >> + status = "okay"; >> +}; >> + >> +&rpm_requests { > Out of alphanumerical order
Ok.
>> + regulators-0 { > Will there be more PMICs under this node? If not, drop the -0.
Ok.
> [...] > >> +&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>; >> +}; > Are there *really* so many? Does the board refuse to boot if > you knock off any of these entries? If so, they probably > don't belong here.
Yes, these are reserved / not-connected gpios as per latest version of the board schematics.
>> + >> +&sdhc_1 { >> + status = "okay"; > Status should go last >> + >> + vmmc-supply = <&vreg_l24a_2p96>; /* emmc power line */ >> + vqmmc-supply = <&vreg_l11a_1p8>; /* emmc vddq */ > The comments are not very useful, drop please. > >> + bus-width = <8>; > This is defined in the SoC dtsi already
Ok.
>> + no-sdio; >> + non-removable; >> +}; >> + >> +&sdhc_2 { >> + status = "okay"; >> + >> + cd-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>; /* card detect gpio */ >> + vmmc-supply = <&vreg_l22a_2p96>; /* Card power line */ >> + vqmmc-supply = <&vreg_l5a_2p96>; /* IO line power */ >> + bus-width = <4>; >> + no-sdio; >> + no-emmc; > Ditto
Ok. Will send updated v2 soon.
Thanks.
| |