[].. >> @@ -10,4 +10,46 @@ >> / { >> model = "Qualcomm Technologies, Inc. SDM845 MTP"; >> compatible = "qcom,sdm845-mtp"; >> + >> + aliases { >> + serial0 = &qup_uart2; >> + }; >> + >> + chosen { >> + stdout-path = "serial0"; >> + }; >> + >> + soc { > > I don't know if there's an official position, but in general I'm seen > people use the actual "soc" alias here. AKA at the top level of this > dts, just do: > > &soc { > ... > }; > > Normally doing stuff like that is useful so you don't need to > replicate the whole hierarchy. In this case that's not a huge > savings, but it can be nice to be consistent. In the very least it > saves you a level of indentation. > > >> + serial@a84000 { >> + status = "okay"; >> + }; > > Similarly here you can use the alias from the sdm845.dtsi file to > avoid replicating the hierarchy. AKA at the top level do: > > &qup_uart2 { > status = "okay"; > }; > > In this case it saves you 2 levels of indentation. Right. Andy/Bjorn, are there any preferences here? I see we don't do this for the other board files, and I not sure theres a specific reasoning for how its currently done and if we need to stick to it. > >> + >> + pinctrl@3400000 { >> + qup_uart2_default: qup_uart2_default { > > I'm not sure how persnickety I should be, but according to > <https://elinux.org/Device_Tree_Linux>: > > node names use dash "-" instead of underscore "_" > > ...but, of course, labels can't use dashes (and the same page says to > use underscore for labels). This is why, in rk3288 for instance, you > see: > > i2c2_xfer: i2c2-xfer { > rockchip,pins = <6 9 RK_FUNC_1 &pcfg_pull_none>, > <6 10 RK_FUNC_1 &pcfg_pull_none>; > }; > > AKA the label and the node name are the same but the label uses "_" > and the node names use "-". Sure, I'll fix these up. [] >> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> index 02520f19e4ca..c4ce70840acf 100644 >> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> @@ -4,6 +4,7 @@ >> */ >> >> #include <dt-bindings/interrupt-controller/arm-gic.h> >> +#include <dt-bindings/clock/qcom,gcc-sdm845.h> >> >> / { >> model = "Qualcomm Technologies, Inc. SDM845"; >> @@ -273,5 +274,25 @@ >> cell-index = <0>; >> }; >> >> + qup_1: qcom,geni_se@ac0000 { >> + compatible = "qcom,geni-se-qup"; >> + reg = <0xac0000 0x6000>; > > I think you may have mentioned this in another context, but this > doesn't match the current bindings. Some clocks should be here. > ...and it looks like the uart should be a subnode. right, these were tested with the v1 set for serial. I will update them. regards Rajendra -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html