| Message ID | 20220704125845.1077037-2-sumit.garg@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | New boards support: db845c and qcs404-evb | expand |
On Mon, Jul 04, 2022 at 06:28:38PM +0530, Sumit Garg wrote: > U-boot specific DT properties belong to *-uboot.dtsi ... and are already included in starqltechn-uboot.dtsi (which is the only current consumer of sdm845.dtsi). Adding fuller comments, such as the above, makes things much easier to review: it makes clear why you consider the properties redundant rather then misfiled. Daniel. > , so remove > corresponding redundant properties. > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > --- > arch/arm/dts/sdm845.dtsi | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi > index 6f2fb20d68..88030156d9 100644 > --- a/arch/arm/dts/sdm845.dtsi > +++ b/arch/arm/dts/sdm845.dtsi > @@ -18,7 +18,6 @@ > compatible = "simple-bus"; > > gcc: clock-controller@100000 { > - u-boot,dm-pre-reloc; > compatible = "qcom,gcc-sdm845"; > reg = <0x100000 0x1f0000>; > #clock-cells = <1>; > @@ -27,7 +26,6 @@ > }; > > gpio_north: gpio_north@3900000 { > - u-boot,dm-pre-reloc; > #gpio-cells = <2>; > compatible = "qcom,sdm845-pinctrl"; > reg = <0x3900000 0x400000>; > @@ -38,7 +36,6 @@ > }; > > tlmm_north: pinctrl_north@3900000 { > - u-boot,dm-pre-reloc; > compatible = "qcom,tlmm-sdm845"; > reg = <0x3900000 0x400000>; > gpio-count = <150>; > -- > 2.25.1 >
Hi Daniel, Thanks for your review. On Mon, 4 Jul 2022 at 21:28, Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Mon, Jul 04, 2022 at 06:28:38PM +0530, Sumit Garg wrote: > > U-boot specific DT properties belong to *-uboot.dtsi > > ... and are already included in starqltechn-uboot.dtsi (which is the > only current consumer of sdm845.dtsi). > > > Adding fuller comments, such as the above, makes things much easier to > review: it makes clear why you consider the properties redundant rather > then misfiled. > I would rather say that this change is to follow the u-boot DT recommendation [1]. I will update the commit message accordingly. BTW, it looks like u-boot DT properties are incorrectly specified in starqltechn-uboot.dtsi here [2] as there aren't any subnodes for the "gcc" node. I will correct that too. [1] https://u-boot.readthedocs.io/en/latest/develop/devicetree/control.html#adding-tweaks-for-u-boot [2] https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/starqltechn-uboot.dtsi#L19 -Sumit > > Daniel. > > > > , so remove > > corresponding redundant properties. > > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > --- > > arch/arm/dts/sdm845.dtsi | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi > > index 6f2fb20d68..88030156d9 100644 > > --- a/arch/arm/dts/sdm845.dtsi > > +++ b/arch/arm/dts/sdm845.dtsi > > @@ -18,7 +18,6 @@ > > compatible = "simple-bus"; > > > > gcc: clock-controller@100000 { > > - u-boot,dm-pre-reloc; > > compatible = "qcom,gcc-sdm845"; > > reg = <0x100000 0x1f0000>; > > #clock-cells = <1>; > > @@ -27,7 +26,6 @@ > > }; > > > > gpio_north: gpio_north@3900000 { > > - u-boot,dm-pre-reloc; > > #gpio-cells = <2>; > > compatible = "qcom,sdm845-pinctrl"; > > reg = <0x3900000 0x400000>; > > @@ -38,7 +36,6 @@ > > }; > > > > tlmm_north: pinctrl_north@3900000 { > > - u-boot,dm-pre-reloc; > > compatible = "qcom,tlmm-sdm845"; > > reg = <0x3900000 0x400000>; > > gpio-count = <150>; > > -- > > 2.25.1 > >
On Tue, Jul 05, 2022 at 11:05:04AM +0530, Sumit Garg wrote: > Hi Daniel, > > Thanks for your review. > > On Mon, 4 Jul 2022 at 21:28, Daniel Thompson <daniel.thompson@linaro.org> wrote: > > > > On Mon, Jul 04, 2022 at 06:28:38PM +0530, Sumit Garg wrote: > > > U-boot specific DT properties belong to *-uboot.dtsi > > > > ... and are already included in starqltechn-uboot.dtsi (which is the > > only current consumer of sdm845.dtsi). > > > > > > Adding fuller comments, such as the above, makes things much easier to > > review: it makes clear why you consider the properties redundant rather > > then misfiled. > > > > I would rather say that this change is to follow the u-boot DT > recommendation [1]. I will update the commit message accordingly. BTW, > it looks like u-boot DT properties are incorrectly specified in > starqltechn-uboot.dtsi here [2] as there aren't any subnodes for the > "gcc" node. I will correct that too. That's fine. The wording was just an example and we written before I reviewed patch 4 and spotted the inconsistancies there. Daniel.
On Tue, Jul 5, 2022 at 11:57 AM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Tue, Jul 05, 2022 at 11:05:04AM +0530, Sumit Garg wrote: > > Hi Daniel, > > > > Thanks for your review. > > > > On Mon, 4 Jul 2022 at 21:28, Daniel Thompson <daniel.thompson@linaro.org> wrote: > > > > > > On Mon, Jul 04, 2022 at 06:28:38PM +0530, Sumit Garg wrote: > > > > U-boot specific DT properties belong to *-uboot.dtsi > > > > > > ... and are already included in starqltechn-uboot.dtsi (which is the > > > only current consumer of sdm845.dtsi). > > > > > > > > > Adding fuller comments, such as the above, makes things much easier to > > > review: it makes clear why you consider the properties redundant rather > > > then misfiled. > > > > > > > I would rather say that this change is to follow the u-boot DT > > recommendation [1]. I will update the commit message accordingly. BTW, > > it looks like u-boot DT properties are incorrectly specified in > > starqltechn-uboot.dtsi here [2] as there aren't any subnodes for the > > "gcc" node. I will correct that too. > > That's fine. The wording was just an example and we written before I > reviewed patch 4 and spotted the inconsistancies there. > > > Daniel. Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi index 6f2fb20d68..88030156d9 100644 --- a/arch/arm/dts/sdm845.dtsi +++ b/arch/arm/dts/sdm845.dtsi @@ -18,7 +18,6 @@ compatible = "simple-bus"; gcc: clock-controller@100000 { - u-boot,dm-pre-reloc; compatible = "qcom,gcc-sdm845"; reg = <0x100000 0x1f0000>; #clock-cells = <1>; @@ -27,7 +26,6 @@ }; gpio_north: gpio_north@3900000 { - u-boot,dm-pre-reloc; #gpio-cells = <2>; compatible = "qcom,sdm845-pinctrl"; reg = <0x3900000 0x400000>; @@ -38,7 +36,6 @@ }; tlmm_north: pinctrl_north@3900000 { - u-boot,dm-pre-reloc; compatible = "qcom,tlmm-sdm845"; reg = <0x3900000 0x400000>; gpio-count = <150>;
U-boot specific DT properties belong to *-uboot.dtsi, so remove corresponding redundant properties. Signed-off-by: Sumit Garg <sumit.garg@linaro.org> --- arch/arm/dts/sdm845.dtsi | 3 --- 1 file changed, 3 deletions(-)