Re: [PATCH 1/8] arm64: dts: sdm845: Remove redundant u-boot DT properties
On Tue, Jul 5, 2022 at 11:57 AM Daniel Thompson 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 > > 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
Re: [PATCH 1/8] arm64: dts: sdm845: Remove redundant u-boot DT properties
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 > 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.
Re: [PATCH 1/8] arm64: dts: sdm845: Remove redundant u-boot DT properties
Hi Daniel,
Thanks for your review.
On Mon, 4 Jul 2022 at 21:28, Daniel Thompson 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
> > ---
> > 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@10 {
> > - u-boot,dm-pre-reloc;
> > compatible = "qcom,gcc-sdm845";
> > reg = <0x10 0x1f>;
> > #clock-cells = <1>;
> > @@ -27,7 +26,6 @@
> > };
> >
> > gpio_north: gpio_north@390 {
> > - u-boot,dm-pre-reloc;
> > #gpio-cells = <2>;
> > compatible = "qcom,sdm845-pinctrl";
> > reg = <0x390 0x40>;
> > @@ -38,7 +36,6 @@
> > };
> >
> > tlmm_north: pinctrl_north@390 {
> > - u-boot,dm-pre-reloc;
> > compatible = "qcom,tlmm-sdm845";
> > reg = <0x390 0x40>;
> > gpio-count = <150>;
> > --
> > 2.25.1
> >
Re: [PATCH 1/8] arm64: dts: sdm845: Remove redundant u-boot DT properties
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
> ---
> 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@10 {
> - u-boot,dm-pre-reloc;
> compatible = "qcom,gcc-sdm845";
> reg = <0x10 0x1f>;
> #clock-cells = <1>;
> @@ -27,7 +26,6 @@
> };
>
> gpio_north: gpio_north@390 {
> - u-boot,dm-pre-reloc;
> #gpio-cells = <2>;
> compatible = "qcom,sdm845-pinctrl";
> reg = <0x390 0x40>;
> @@ -38,7 +36,6 @@
> };
>
> tlmm_north: pinctrl_north@390 {
> - u-boot,dm-pre-reloc;
> compatible = "qcom,tlmm-sdm845";
> reg = <0x390 0x40>;
> gpio-count = <150>;
> --
> 2.25.1
>
[PATCH 1/8] arm64: dts: sdm845: Remove redundant u-boot DT properties
U-boot specific DT properties belong to *-uboot.dtsi, so remove
corresponding redundant properties.
Signed-off-by: Sumit Garg
---
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@10 {
- u-boot,dm-pre-reloc;
compatible = "qcom,gcc-sdm845";
reg = <0x10 0x1f>;
#clock-cells = <1>;
@@ -27,7 +26,6 @@
};
gpio_north: gpio_north@390 {
- u-boot,dm-pre-reloc;
#gpio-cells = <2>;
compatible = "qcom,sdm845-pinctrl";
reg = <0x390 0x40>;
@@ -38,7 +36,6 @@
};
tlmm_north: pinctrl_north@390 {
- u-boot,dm-pre-reloc;
compatible = "qcom,tlmm-sdm845";
reg = <0x390 0x40>;
gpio-count = <150>;
--
2.25.1
