diff mbox

[RESEND,1/2] ARM: dts: tegra: Add labels to Tegra114 nodes

Message ID 1432036279-6318-1-git-send-email-k.kozlowski.k@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Krzysztof Kozlowski May 19, 2015, 11:51 a.m. UTC
Add new labels to certain nodes on Tegra114 so they could be easily
referenced by board DTS files.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>

---

Tested by comparison of generated DTB and decompiled DTB->DTS for each
commit. Each output was the same.
---
 arch/arm/boot/dts/tegra114.dtsi | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Stephen Warren May 19, 2015, 1:53 p.m. UTC | #1
On 05/19/2015 05:51 AM, Krzysztof Kozlowski wrote:
> Add new labels to certain nodes on Tegra114 so they could be easily
> referenced by board DTS files.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>

As I think I mentioned last time, this would make the Tegra114 files 
inconsistent with all other Tegra files. Either we should convert 
everything, or none at all. I'm personally not sure the churn this will 
introduce is worth it, but maybe.
Krzysztof Kozlowski May 19, 2015, 11:35 p.m. UTC | #2
2015-05-19 22:53 GMT+09:00 Stephen Warren <swarren@wwwdotorg.org>:
> On 05/19/2015 05:51 AM, Krzysztof Kozlowski wrote:
>>
>> Add new labels to certain nodes on Tegra114 so they could be easily
>> referenced by board DTS files.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
>
>
> As I think I mentioned last time, this would make the Tegra114 files
> inconsistent with all other Tegra files.

I am sorry but I missed you response from my first mail.

> Either we should convert
> everything, or none at all. I'm personally not sure the churn this will
> introduce is worth it, but maybe.

Okay, I understand.

Best regards,
Krzysztof
Alexandre Courbot May 20, 2015, 4:05 a.m. UTC | #3
On Tue, May 19, 2015 at 8:51 PM, Krzysztof Kozlowski
<k.kozlowski.k@gmail.com> wrote:
> Usage of labels instead of full paths reduces possible mistakes when
> overriding nodes.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>

Indentation seems to be off by one tab in the added code (hence the
huge size of this patch ; most lines should not change), can you
check?
Krzysztof Kozlowski May 20, 2015, 5:03 a.m. UTC | #4
2015-05-20 13:05 GMT+09:00 Alexandre Courbot <gnurou@gmail.com>:
> On Tue, May 19, 2015 at 8:51 PM, Krzysztof Kozlowski
> <k.kozlowski.k@gmail.com> wrote:
>> Usage of labels instead of full paths reduces possible mistakes when
>> overriding nodes.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
>
> Indentation seems to be off by one tab in the added code (hence the
> huge size of this patch ; most lines should not change), can you
> check?

It is correct. This change in indentation is an effect of moving nodes
out of first bracket ("\ {"). That is the convention.

It us up to you guys if this is worth the effort. For exynos we use
label-convention and now I am converting old DTS to it. I think the
label-convention is less error-prone when extending or overriding
nodes. Also it removes duplicated addresses.

Best regards,
Krzysztof
Alexandre Courbot May 20, 2015, 5:05 a.m. UTC | #5
On Wed, May 20, 2015 at 2:03 PM, Krzysztof Koz?owski
<k.kozlowski.k@gmail.com> wrote:
> 2015-05-20 13:05 GMT+09:00 Alexandre Courbot <gnurou@gmail.com>:
>> On Tue, May 19, 2015 at 8:51 PM, Krzysztof Kozlowski
>> <k.kozlowski.k@gmail.com> wrote:
>>> Usage of labels instead of full paths reduces possible mistakes when
>>> overriding nodes.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
>>
>> Indentation seems to be off by one tab in the added code (hence the
>> huge size of this patch ; most lines should not change), can you
>> check?
>
> It is correct. This change in indentation is an effect of moving nodes
> out of first bracket ("\ {"). That is the convention.
>
> It us up to you guys if this is worth the effort. For exynos we use
> label-convention and now I am converting old DTS to it. I think the
> label-convention is less error-prone when extending or overriding
> nodes. Also it removes duplicated addresses.

I don't really have a strong opinion on this - Stephen and Thierry
have worked with DT much more, let's see what they think...
Thierry Reding May 20, 2015, 12:40 p.m. UTC | #6
On Wed, May 20, 2015 at 02:05:38PM +0900, Alexandre Courbot wrote:
> On Wed, May 20, 2015 at 2:03 PM, Krzysztof Koz?owski
> <k.kozlowski.k@gmail.com> wrote:
> > 2015-05-20 13:05 GMT+09:00 Alexandre Courbot <gnurou@gmail.com>:
> >> On Tue, May 19, 2015 at 8:51 PM, Krzysztof Kozlowski
> >> <k.kozlowski.k@gmail.com> wrote:
> >>> Usage of labels instead of full paths reduces possible mistakes when
> >>> overriding nodes.
> >>>
> >>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
> >>
> >> Indentation seems to be off by one tab in the added code (hence the
> >> huge size of this patch ; most lines should not change), can you
> >> check?
> >
> > It is correct. This change in indentation is an effect of moving nodes
> > out of first bracket ("\ {"). That is the convention.
> >
> > It us up to you guys if this is worth the effort. For exynos we use
> > label-convention and now I am converting old DTS to it. I think the
> > label-convention is less error-prone when extending or overriding
> > nodes. Also it removes duplicated addresses.
> 
> I don't really have a strong opinion on this - Stephen and Thierry
> have worked with DT much more, let's see what they think...

I agree with Stephen that this is unnecessary churn. I understand the
reason why people prefer to use labels, but I don't think it's enough of
an issue to warrent rewriting all of the DTS files. I personally don't
like the convention very much because it makes the otherwise very neatly
structured DTS files hard to read.

Thierry
diff mbox

Patch

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index f58a3d9d5f13..1b31cefd1047 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -10,7 +10,7 @@ 
 	compatible = "nvidia,tegra114";
 	interrupt-parent = <&lic>;
 
-	host1x@50000000 {
+	host1x: host1x@50000000 {
 		compatible = "nvidia,tegra114-host1x", "simple-bus";
 		reg = <0x50000000 0x00028000>;
 		interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, /* syncpt */
@@ -318,7 +318,7 @@ 
 		status = "disabled";
 	};
 
-	i2c@7000c000 {
+	i2c1: i2c@7000c000 {
 		compatible = "nvidia,tegra114-i2c";
 		reg = <0x7000c000 0x100>;
 		interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
@@ -363,7 +363,7 @@ 
 		status = "disabled";
 	};
 
-	i2c@7000c700 {
+	i2c4: i2c@7000c700 {
 		compatible = "nvidia,tegra114-i2c";
 		reg = <0x7000c700 0x100>;
 		interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
@@ -378,7 +378,7 @@ 
 		status = "disabled";
 	};
 
-	i2c@7000d000 {
+	i2c5: i2c@7000d000 {
 		compatible = "nvidia,tegra114-i2c";
 		reg = <0x7000d000 0x100>;
 		interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
@@ -438,7 +438,7 @@ 
 		status = "disabled";
 	};
 
-	spi@7000da00 {
+	spi4: spi@7000da00 {
 		compatible = "nvidia,tegra114-spi";
 		reg = <0x7000da00 0x200>;
 		interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
@@ -500,7 +500,7 @@ 
 		status = "disabled";
 	};
 
-	pmc@7000e400 {
+	pmc: pmc@7000e400 {
 		compatible = "nvidia,tegra114-pmc";
 		reg = <0x7000e400 0x400>;
 		clocks = <&tegra_car TEGRA114_CLK_PCLK>, <&clk32k_in>;
@@ -527,7 +527,7 @@ 
 		#iommu-cells = <1>;
 	};
 
-	ahub@70080000 {
+	ahub: ahub@70080000 {
 		compatible = "nvidia,tegra114-ahub";
 		reg = <0x70080000 0x200>,
 		      <0x70080200 0x100>,
@@ -648,7 +648,7 @@ 
 		status = "disabled";
 	};
 
-	sdhci@78000400 {
+	sdhci3: sdhci@78000400 {
 		compatible = "nvidia,tegra114-sdhci", "nvidia,tegra30-sdhci";
 		reg = <0x78000400 0x200>;
 		interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
@@ -658,7 +658,7 @@ 
 		status = "disabled";
 	};
 
-	sdhci@78000600 {
+	sdhci4: sdhci@78000600 {
 		compatible = "nvidia,tegra114-sdhci", "nvidia,tegra30-sdhci";
 		reg = <0x78000600 0x200>;
 		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
@@ -668,7 +668,7 @@ 
 		status = "disabled";
 	};
 
-	usb@7d000000 {
+	usb1: usb@7d000000 {
 		compatible = "nvidia,tegra30-ehci", "usb-ehci";
 		reg = <0x7d000000 0x4000>;
 		interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
@@ -704,7 +704,7 @@ 
 		status = "disabled";
 	};
 
-	usb@7d008000 {
+	usb3: usb@7d008000 {
 		compatible = "nvidia,tegra30-ehci", "usb-ehci";
 		reg = <0x7d008000 0x4000>;
 		interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;