| Message ID | 1479345215-31467-11-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
On 11/16/2016 06:13 PM, Simon Glass wrote: > With the new device-tree rules it is possible to put device-tree changes > needed by U-Boot into their own file. As an example of this approach, move > Tegra over to use it. Sounds like a good idea. > diff --git a/arch/arm/dts/tegra20-u-boot.dtsi b/arch/arm/dts/tegra20-u-boot.dtsi > +/ { > + compatible = "nvidia,tegra20"; > + interrupt-parent = <&lic>; I don't think either of those lines is specific to U-Boot. I'd expect to see more "U-Boot overlay" DTs than this; I recall there being more differences between U-Boot and kernel DTS files when I last sync'd the two.
Hi Stephen, On 17 November 2016 at 12:45, Stephen Warren <swarren@wwwdotorg.org> wrote: > > On 11/16/2016 06:13 PM, Simon Glass wrote: >> >> With the new device-tree rules it is possible to put device-tree changes >> needed by U-Boot into their own file. As an example of this approach, move >> Tegra over to use it. > > > Sounds like a good idea. > >> diff --git a/arch/arm/dts/tegra20-u-boot.dtsi b/arch/arm/dts/tegra20-u-boot.dtsi > > >> +/ { >> + compatible = "nvidia,tegra20"; >> + interrupt-parent = <&lic>; > > > I don't think either of those lines is specific to U-Boot. Yes they should not be there - fixed in v6. > > I'd expect to see more "U-Boot overlay" DTs than this; I recall there being more differences between U-Boot and kernel DTS files when I last sync'd the two. Yes but most of those changes should be dropped. I did a partial sync a few months back but if you recall there were still differences. Is this something the Tegra maintainer might look at? I don't want to immortalise those differences in a separate U-Boot file when really we should just get rid of them. Regards, Simon
On 11/28/2016 03:09 PM, Simon Glass wrote: > Hi Stephen, > > On 17 November 2016 at 12:45, Stephen Warren <swarren@wwwdotorg.org> wrote: >> >> On 11/16/2016 06:13 PM, Simon Glass wrote: >>> >>> With the new device-tree rules it is possible to put device-tree changes >>> needed by U-Boot into their own file. As an example of this approach, move >>> Tegra over to use it. >> >> >> Sounds like a good idea. >> >>> diff --git a/arch/arm/dts/tegra20-u-boot.dtsi b/arch/arm/dts/tegra20-u-boot.dtsi >> I'd expect to see more "U-Boot overlay" DTs than this; I recall there being more differences between U-Boot and kernel DTS files when I last sync'd the two. > > Yes but most of those changes should be dropped. I did a partial sync > a few months back but if you recall there were still differences. Is > this something the Tegra maintainer might look at? > > I don't want to immortalise those differences in a separate U-Boot > file when really we should just get rid of them. From my perspective, we should have two files: 1) The base DT. This should not contain any U-Boot modifications, and should exactly match the DT used elsewhere, such as in mainline Linux. Since this should always match other DTs, we should pretty much always be able to over-write it with any updated DT from other sources. 2) The U-Boot modifications. This always contain /all/ local modifications applied by U-Boot. It shouldn't matter why the change was made, or how long we hope/expect the delta to continue to exist. This will isolate all U-Boot changes into this file so it's obvious what local changes exist. If some changes are intended to be temporary, we can add a comment to that effect, and eventually submit a patch to remove the delta. I don't think that putting a change into this "U-Boot local overlay" should in any way imply that the change is by definition correct and long-term; some changes may satisfy that decription and others won't. Just like we sometimes have C code that we wish we didn't and eventually clean up.
Hi Stephen, On 29 November 2016 at 21:09, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 11/28/2016 03:09 PM, Simon Glass wrote: >> >> Hi Stephen, >> >> On 17 November 2016 at 12:45, Stephen Warren <swarren@wwwdotorg.org> >> wrote: >>> >>> >>> On 11/16/2016 06:13 PM, Simon Glass wrote: >>>> >>>> >>>> With the new device-tree rules it is possible to put device-tree changes >>>> needed by U-Boot into their own file. As an example of this approach, >>>> move >>>> Tegra over to use it. >>> >>> >>> >>> Sounds like a good idea. >>> >>>> diff --git a/arch/arm/dts/tegra20-u-boot.dtsi >>>> b/arch/arm/dts/tegra20-u-boot.dtsi > > >>> I'd expect to see more "U-Boot overlay" DTs than this; I recall there >>> being more differences between U-Boot and kernel DTS files when I last >>> sync'd the two. >> >> >> Yes but most of those changes should be dropped. I did a partial sync >> a few months back but if you recall there were still differences. Is >> this something the Tegra maintainer might look at? >> >> I don't want to immortalise those differences in a separate U-Boot >> file when really we should just get rid of them. > > > From my perspective, we should have two files: > > 1) The base DT. > > This should not contain any U-Boot modifications, and should exactly match > the DT used elsewhere, such as in mainline Linux. Since this should always > match other DTs, we should pretty much always be able to over-write it with > any updated DT from other sources. > > 2) The U-Boot modifications. > > This always contain /all/ local modifications applied by U-Boot. It > shouldn't matter why the change was made, or how long we hope/expect the > delta to continue to exist. This will isolate all U-Boot changes into this > file so it's obvious what local changes exist. If some changes are intended > to be temporary, we can add a comment to that effect, and eventually submit > a patch to remove the delta. > > I don't think that putting a change into this "U-Boot local overlay" should > in any way imply that the change is by definition correct and long-term; > some changes may satisfy that decription and others won't. Just like we > sometimes have C code that we wish we didn't and eventually clean up. That's fine with me. What do you want to do with this patch? Regards, Simon
On 11/30/2016 07:19 PM, Simon Glass wrote: > Hi Stephen, > > On 29 November 2016 at 21:09, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 11/28/2016 03:09 PM, Simon Glass wrote: >>> >>> Hi Stephen, >>> >>> On 17 November 2016 at 12:45, Stephen Warren <swarren@wwwdotorg.org> >>> wrote: >>>> >>>> >>>> On 11/16/2016 06:13 PM, Simon Glass wrote: >>>>> >>>>> >>>>> With the new device-tree rules it is possible to put device-tree changes >>>>> needed by U-Boot into their own file. As an example of this approach, >>>>> move >>>>> Tegra over to use it. >>>> >>>> >>>> >>>> Sounds like a good idea. >>>> >>>>> diff --git a/arch/arm/dts/tegra20-u-boot.dtsi >>>>> b/arch/arm/dts/tegra20-u-boot.dtsi >> >> >>>> I'd expect to see more "U-Boot overlay" DTs than this; I recall there >>>> being more differences between U-Boot and kernel DTS files when I last >>>> sync'd the two. >>> >>> >>> Yes but most of those changes should be dropped. I did a partial sync >>> a few months back but if you recall there were still differences. Is >>> this something the Tegra maintainer might look at? >>> >>> I don't want to immortalise those differences in a separate U-Boot >>> file when really we should just get rid of them. >> >> >> From my perspective, we should have two files: >> >> 1) The base DT. >> >> This should not contain any U-Boot modifications, and should exactly match >> the DT used elsewhere, such as in mainline Linux. Since this should always >> match other DTs, we should pretty much always be able to over-write it with >> any updated DT from other sources. >> >> 2) The U-Boot modifications. >> >> This always contain /all/ local modifications applied by U-Boot. It >> shouldn't matter why the change was made, or how long we hope/expect the >> delta to continue to exist. This will isolate all U-Boot changes into this >> file so it's obvious what local changes exist. If some changes are intended >> to be temporary, we can add a comment to that effect, and eventually submit >> a patch to remove the delta. >> >> I don't think that putting a change into this "U-Boot local overlay" should >> in any way imply that the change is by definition correct and long-term; >> some changes may satisfy that decription and others won't. Just like we >> sometimes have C code that we wish we didn't and eventually clean up. > > That's fine with me. What do you want to do with this patch? IIRC the patch content is fine as far as it goes, but it'd be nice to take it all the way and move all the U-Boot diffs into arch/arm/dts/tegra20-u-boot.dtsi if possible.
Hi Stephen, On 2 December 2016 at 12:19, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 11/30/2016 07:19 PM, Simon Glass wrote: >> >> Hi Stephen, >> >> On 29 November 2016 at 21:09, Stephen Warren <swarren@wwwdotorg.org> >> wrote: >>> >>> On 11/28/2016 03:09 PM, Simon Glass wrote: >>>> >>>> >>>> Hi Stephen, >>>> >>>> On 17 November 2016 at 12:45, Stephen Warren <swarren@wwwdotorg.org> >>>> wrote: >>>>> >>>>> >>>>> >>>>> On 11/16/2016 06:13 PM, Simon Glass wrote: >>>>>> >>>>>> >>>>>> >>>>>> With the new device-tree rules it is possible to put device-tree >>>>>> changes >>>>>> needed by U-Boot into their own file. As an example of this approach, >>>>>> move >>>>>> Tegra over to use it. >>>>> >>>>> >>>>> >>>>> >>>>> Sounds like a good idea. >>>>> >>>>>> diff --git a/arch/arm/dts/tegra20-u-boot.dtsi >>>>>> b/arch/arm/dts/tegra20-u-boot.dtsi >>> >>> >>> >>>>> I'd expect to see more "U-Boot overlay" DTs than this; I recall there >>>>> being more differences between U-Boot and kernel DTS files when I last >>>>> sync'd the two. >>>> >>>> >>>> >>>> Yes but most of those changes should be dropped. I did a partial sync >>>> a few months back but if you recall there were still differences. Is >>>> this something the Tegra maintainer might look at? >>>> >>>> I don't want to immortalise those differences in a separate U-Boot >>>> file when really we should just get rid of them. >>> >>> >>> >>> From my perspective, we should have two files: >>> >>> 1) The base DT. >>> >>> This should not contain any U-Boot modifications, and should exactly >>> match >>> the DT used elsewhere, such as in mainline Linux. Since this should >>> always >>> match other DTs, we should pretty much always be able to over-write it >>> with >>> any updated DT from other sources. >>> >>> 2) The U-Boot modifications. >>> >>> This always contain /all/ local modifications applied by U-Boot. It >>> shouldn't matter why the change was made, or how long we hope/expect the >>> delta to continue to exist. This will isolate all U-Boot changes into >>> this >>> file so it's obvious what local changes exist. If some changes are >>> intended >>> to be temporary, we can add a comment to that effect, and eventually >>> submit >>> a patch to remove the delta. >>> >>> I don't think that putting a change into this "U-Boot local overlay" >>> should >>> in any way imply that the change is by definition correct and long-term; >>> some changes may satisfy that decription and others won't. Just like we >>> sometimes have C code that we wish we didn't and eventually clean up. >> >> >> That's fine with me. What do you want to do with this patch? > > > IIRC the patch content is fine as far as it goes, but it'd be nice to take > it all the way and move all the U-Boot diffs into > arch/arm/dts/tegra20-u-boot.dtsi if possible. OK, can you please add a review or test tag? I will see if I can take a look in January. Is there any chance that Nvidia might put some effort into this? Regards, Simon
diff --git a/arch/arm/dts/tegra124-nyan-big-u-boot.dtsi b/arch/arm/dts/tegra124-nyan-big-u-boot.dtsi new file mode 100644 index 0000000..fff1d78 --- /dev/null +++ b/arch/arm/dts/tegra124-nyan-big-u-boot.dtsi @@ -0,0 +1,15 @@ +/* + * Copyright (C) 2016 Google, Inc + * Written by Simon Glass <sjg@chromium.org> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +/ { + host1x@50000000 { + u-boot,dm-pre-reloc; + dc@54200000 { + u-boot,dm-pre-reloc; + }; + }; +}; diff --git a/arch/arm/dts/tegra124-nyan-big.dts b/arch/arm/dts/tegra124-nyan-big.dts index 3758395..62f89d0 100644 --- a/arch/arm/dts/tegra124-nyan-big.dts +++ b/arch/arm/dts/tegra124-nyan-big.dts @@ -27,9 +27,7 @@ }; host1x@50000000 { - u-boot,dm-pre-reloc; dc@54200000 { - u-boot,dm-pre-reloc; display-timings { timing@0 { clock-frequency = <69500000>; diff --git a/arch/arm/dts/tegra20-u-boot.dtsi b/arch/arm/dts/tegra20-u-boot.dtsi new file mode 100644 index 0000000..87b16eb --- /dev/null +++ b/arch/arm/dts/tegra20-u-boot.dtsi @@ -0,0 +1,11 @@ +/ { + compatible = "nvidia,tegra20"; + interrupt-parent = <&lic>; + + host1x@50000000 { + u-boot,dm-pre-reloc; + dc@54200000 { + u-boot,dm-pre-reloc; + }; + }; +}; diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index 84bb1b0..e21ee25 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -10,7 +10,6 @@ interrupt-parent = <&lic>; host1x@50000000 { - u-boot,dm-pre-reloc; compatible = "nvidia,tegra20-host1x", "simple-bus"; reg = <0x50000000 0x00024000>; interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, /* syncpt */ @@ -78,7 +77,6 @@ }; dc@54200000 { - u-boot,dm-pre-reloc; compatible = "nvidia,tegra20-dc"; reg = <0x54200000 0x00040000>; interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
With the new device-tree rules it is possible to put device-tree changes needed by U-Boot into their own file. As an example of this approach, move Tegra over to use it. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v5: - Add a new tegra patch to use an automatically included .dtsi file Changes in v4: None Changes in v3: None Changes in v2: None arch/arm/dts/tegra124-nyan-big-u-boot.dtsi | 15 +++++++++++++++ arch/arm/dts/tegra124-nyan-big.dts | 2 -- arch/arm/dts/tegra20-u-boot.dtsi | 11 +++++++++++ arch/arm/dts/tegra20.dtsi | 2 -- 4 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 arch/arm/dts/tegra124-nyan-big-u-boot.dtsi create mode 100644 arch/arm/dts/tegra20-u-boot.dtsi