| Message ID | 20130122144813.GD2812@S2101-09.ap.freescale.net |
|---|---|
| State | New |
| Headers | show |
Hi, On Tue, Jan 22, 2013 at 10:48:15PM +0800, Shawn Guo wrote: > Shawn Guo (4): > ARM: dts: imx: use nodes label in board dts Hmm. This patch is 1000 lines of pure churn. Sure, the style on OMAP is different, but there's no clear benefit from it -- it's actually advantageous to see some of the bus hierarchies even on the leaf-level board nodes. Would you mind respinning with this left out, please? If you still want to argue it to be included, we can do so, but I'd like to pick up the rest of the branch meanwhile. :-) Thanks! -Olof
On Mon, Jan 28, 2013 at 02:00:26PM -0800, Olof Johansson wrote: > Hi, > > On Tue, Jan 22, 2013 at 10:48:15PM +0800, Shawn Guo wrote: > > > Shawn Guo (4): > > ARM: dts: imx: use nodes label in board dts > > Hmm. > > This patch is 1000 lines of pure churn. Sure, the style on OMAP is different, > but there's no clear benefit from it -- it's actually advantageous to see some > of the bus hierarchies even on the leaf-level board nodes. > > Would you mind respinning with this left out, please? If you still want to > argue it to be included, we can do so, but I'd like to pick up the rest of the > branch meanwhile. :-) I will refresh the pull-request to leave it out, but meanwhile I'd like to argue too, as the approach has been agreed by IMX people and all the patches I queued on imx/dt are all in this way. And I will move the patch to imx/dt branch instead, if you're not strongly against the approach. The board level dts are mostly used to add/overwrite properties for nodes defined in soc dts. Therefore, what people who look at board dts care about is those properties, not really which bus the nodes are on. We go this way to have board dts focus on the things they are created for. It's much easer to read and edit those properties. I'm not sure why it's important to maintain the bus topology in board dts while we have the full one in soc dts. In the current way, people sometimes have to reassemble 3 or more levels bus hierarchies for only overwriting one property for one node. I'm pretty sure that people who work on board level dts would vote for this way. It makes their life easier without increasing the maintainer's burden. So why not? Shawn
On Tue, Jan 29, 2013 at 09:08:55AM +0800, Shawn Guo wrote: > On Mon, Jan 28, 2013 at 02:00:26PM -0800, Olof Johansson wrote: > > Hi, > > > > On Tue, Jan 22, 2013 at 10:48:15PM +0800, Shawn Guo wrote: > > > > > Shawn Guo (4): > > > ARM: dts: imx: use nodes label in board dts > > > > Hmm. > > > > This patch is 1000 lines of pure churn. Sure, the style on OMAP is different, > > but there's no clear benefit from it -- it's actually advantageous to see some > > of the bus hierarchies even on the leaf-level board nodes. > > > > Would you mind respinning with this left out, please? If you still want to > > argue it to be included, we can do so, but I'd like to pick up the rest of the > > branch meanwhile. :-) > > I will refresh the pull-request to leave it out, but meanwhile I'd like > to argue too, as the approach has been agreed by IMX people and all the > patches I queued on imx/dt are all in this way. And I will move the > patch to imx/dt branch instead, if you're not strongly against the > approach. > > The board level dts are mostly used to add/overwrite properties for > nodes defined in soc dts. Therefore, what people who look at board > dts care about is those properties, not really which bus the nodes > are on. We go this way to have board dts focus on the things they > are created for. It's much easer to read and edit those properties. > > I'm not sure why it's important to maintain the bus topology in board > dts while we have the full one in soc dts. In the current way, people > sometimes have to reassemble 3 or more levels bus hierarchies for only > overwriting one property for one node. > > I'm pretty sure that people who work on board level dts would vote for > this way. It makes their life easier without increasing the > maintainer's burden. So why not? I'm with Shawn here. With this board layout I'm now able to write board dts files without looking much at the dtsi file at all. It's debatable if existing boards have to be changed to use this layout, but since people copy-paste all the time changing it increases the chance they copy the right stuff. Sascha
On Tue, Jan 29, 2013 at 08:26:03AM +0100, Sascha Hauer wrote: > On Tue, Jan 29, 2013 at 09:08:55AM +0800, Shawn Guo wrote: > > On Mon, Jan 28, 2013 at 02:00:26PM -0800, Olof Johansson wrote: > > > Hi, > > > > > > On Tue, Jan 22, 2013 at 10:48:15PM +0800, Shawn Guo wrote: > > > > > > > Shawn Guo (4): > > > > ARM: dts: imx: use nodes label in board dts > > > > > > Hmm. > > > > > > This patch is 1000 lines of pure churn. Sure, the style on OMAP is different, > > > but there's no clear benefit from it -- it's actually advantageous to see some > > > of the bus hierarchies even on the leaf-level board nodes. > > > > > > Would you mind respinning with this left out, please? If you still want to > > > argue it to be included, we can do so, but I'd like to pick up the rest of the > > > branch meanwhile. :-) > > > > I will refresh the pull-request to leave it out, but meanwhile I'd like > > to argue too, as the approach has been agreed by IMX people and all the > > patches I queued on imx/dt are all in this way. And I will move the > > patch to imx/dt branch instead, if you're not strongly against the > > approach. > > > > The board level dts are mostly used to add/overwrite properties for > > nodes defined in soc dts. Therefore, what people who look at board > > dts care about is those properties, not really which bus the nodes > > are on. We go this way to have board dts focus on the things they > > are created for. It's much easer to read and edit those properties. > > > > I'm not sure why it's important to maintain the bus topology in board > > dts while we have the full one in soc dts. In the current way, people > > sometimes have to reassemble 3 or more levels bus hierarchies for only > > overwriting one property for one node. > > > > I'm pretty sure that people who work on board level dts would vote for > > this way. It makes their life easier without increasing the > > maintainer's burden. So why not? > > I'm with Shawn here. With this board layout I'm now able to write board > dts files without looking much at the dtsi file at all. It's debatable > if existing boards have to be changed to use this layout, but since > people copy-paste all the time changing it increases the chance they > copy the right stuff. Ok. Just to make clear: While I personally prefer the regular way of specifying the board dts files, my main concern with this patch is that it is _pure churn_. It's 1000 lines of change without a functional benefit, and without really being a cleanup that benefits long-term development. We're starting to get more comments about the volume of churn again (Linus made a couple of them the last merge window), and I'd rather be a bit more conservative on some of the larger changes that lack strong benefit for a release or two, than have him get ticked off and make life miserable for all of us. So, I'm not saying that it's a bad idea to change to labels, but I would rather hold off this while we give some of the remaining platforms still not multiplatform-enabled a chance to "use the churn quota" since they will need it for include file moves, etc, etc. -Olof
On Tue, Jan 29, 2013 at 06:18:57AM -0800, Olof Johansson wrote: > Ok. Just to make clear: While I personally prefer the regular way of specifying > the board dts files, my main concern with this patch is that it is _pure > churn_. It's 1000 lines of change without a functional benefit, and without > really being a cleanup that benefits long-term development. > You can say it's not a cleanup, but it definitely benefits long-term development. It makes board dts development a lot easier, and that's why I came up with the patch. > We're starting to get more comments about the volume of churn again (Linus made > a couple of them the last merge window), and I'd rather be a bit more > conservative on some of the larger changes that lack strong benefit for > a release or two, than have him get ticked off and make life miserable for all > of us. > > So, I'm not saying that it's a bad idea to change to labels, but I would rather > hold off this while we give some of the remaining platforms still not > multiplatform-enabled a chance to "use the churn quota" since they will need it > for include file moves, etc, etc. > Then it will hold off the while IMX device tree development. All the patches I queued on imx/dt branch are all based on that. While it looks like a "churn", it should not cause any conflict, as all IMX dts changes will go via the same branch for this cycle. So please give it a go to avoid hurting people who contribute IMX device tree development. Shawn
On Tue, Jan 29, 2013 at 1:10 PM, Shawn Guo <shawn.guo@linaro.org> wrote: > On Tue, Jan 29, 2013 at 06:18:57AM -0800, Olof Johansson wrote: >> Ok. Just to make clear: While I personally prefer the regular way of specifying >> the board dts files, my main concern with this patch is that it is _pure >> churn_. It's 1000 lines of change without a functional benefit, and without >> really being a cleanup that benefits long-term development. >> > You can say it's not a cleanup, but it definitely benefits long-term > development. It makes board dts development a lot easier, and that's > why I came up with the patch. I also agree here.
The following changes since commit a49f0d1ea3ec94fc7cf33a7c36a16343b74bd565: Linux 3.8-rc1 (2012-12-21 17:19:00 -0800) are available in the git repository at: git://git.linaro.org/people/shawnguo/linux-2.6.git tags/imx-cleanup-3.9 for you to fetch changes up to 6faa0b731160d2b0e49796191ad124f564b021f0: ARM: imx: Remove mx508 support (2013-01-22 22:26:44 +0800) ---------------------------------------------------------------- IMX cleanup for 3.9: * Board level dts shuffling to use node label * Remove lluart.c by using debug_ll_io_init() * Remove mach-mx51_3ds board support * Remove imx50 support which has been BROKEN for cycles * Other trival cleanups ---------------------------------------------------------------- Fabio Estevam (5): ARM: mach-imx: Kconfig: Do not select Babbage for MACH_IMX51_DT ARM: dts: imx27-3ds: Rename it to imx27-pdk ARM: boot: dts: Add an entry for imx27-pdk.dtb ARM: imx: Remove mach-mx51_3ds board ARM: imx: Remove mx508 support Shawn Guo (4): ARM: dts: add missing imx dtb targets ARM: dts: imx: use nodes label in board dts ARM: imx: remove unused imx6q_clock_map_io() ARM: imx: use debug_ll_io_init() for imx6q Documentation/devicetree/bindings/arm/fsl.txt | 4 + arch/arm/Kconfig.debug | 10 +- arch/arm/boot/dts/Makefile | 7 +- arch/arm/boot/dts/imx25-karo-tx25.dts | 30 +- arch/arm/boot/dts/imx25.dtsi | 2 +- arch/arm/boot/dts/imx27-apf27.dts | 82 +- arch/arm/boot/dts/{imx27-3ds.dts => imx27-pdk.dts} | 24 +- arch/arm/boot/dts/imx31-bug.dts | 12 +- arch/arm/boot/dts/imx51-babbage.dts | 456 +++++---- arch/arm/boot/dts/imx53-ard.dts | 126 ++- arch/arm/boot/dts/imx53-evk.dts | 194 ++-- arch/arm/boot/dts/imx53-qsb.dts | 380 ++++---- arch/arm/boot/dts/imx53-smd.dts | 294 +++--- arch/arm/boot/dts/imx6q-arm2.dts | 124 ++- arch/arm/boot/dts/imx6q-sabreauto.dts | 64 +- arch/arm/boot/dts/imx6q-sabrelite.dts | 216 +++-- arch/arm/boot/dts/imx6q-sabresd.dts | 102 +- arch/arm/boot/dts/imx6q.dtsi | 2 +- arch/arm/configs/imx_v6_v7_defconfig | 1 - arch/arm/include/debug/imx.S | 2 +- arch/arm/mach-imx/Kconfig | 36 - arch/arm/mach-imx/Makefile | 3 - arch/arm/mach-imx/Makefile.boot | 4 - arch/arm/mach-imx/clk-imx6q.c | 2 - arch/arm/mach-imx/common.h | 11 - arch/arm/mach-imx/cpu-imx5.c | 39 - arch/arm/mach-imx/devices-imx50.h | 33 - arch/arm/mach-imx/devices/Kconfig | 2 +- arch/arm/mach-imx/devices/platform-fec.c | 6 - arch/arm/mach-imx/devices/platform-imx-i2c.c | 10 - arch/arm/mach-imx/devices/platform-imx-uart.c | 12 - arch/arm/mach-imx/hardware.h | 6 - arch/arm/mach-imx/iomux-mx50.h | 977 -------------------- arch/arm/mach-imx/lluart.c | 47 - arch/arm/mach-imx/mach-imx6q.c | 4 +- arch/arm/mach-imx/mach-mx50_rdp.c | 225 ----- arch/arm/mach-imx/mach-mx51_3ds.c | 178 ---- arch/arm/mach-imx/mm-imx5.c | 48 - arch/arm/mach-imx/mx50.h | 290 ------ arch/arm/mach-imx/mxc.h | 13 - arch/arm/mach-imx/pm-imx5.c | 7 +- 41 files changed, 1031 insertions(+), 3054 deletions(-) rename arch/arm/boot/dts/{imx27-3ds.dts => imx27-pdk.dts} (59%) delete mode 100644 arch/arm/mach-imx/devices-imx50.h delete mode 100644 arch/arm/mach-imx/iomux-mx50.h delete mode 100644 arch/arm/mach-imx/lluart.c delete mode 100644 arch/arm/mach-imx/mach-mx50_rdp.c delete mode 100644 arch/arm/mach-imx/mach-mx51_3ds.c delete mode 100644 arch/arm/mach-imx/mx50.h