On Wed, Jan 11, 2017 at 12:55:23PM +0200, Laurent Pinchart wrote: > Hi Simon, > > On Wednesday 11 Jan 2017 11:33:17 Simon Horman wrote: > > On Tue, Jan 10, 2017 at 09:58:19PM +0200, Laurent Pinchart wrote: > > > On Tuesday 10 Jan 2017 16:07:01 Geert Uytterhoeven wrote: > > >> On Mon, Jan 9, 2017 at 8:31 PM, Jacopo Mondi wrote: > > >>> From: Magnus Damm <d...@opensource.se> > > >>> > > >>> This is a squash of several commits, adding peripherals groups > > >>> configuration to r7s72100 device tree, and enabling some of them on > > >>> Genmai evaluation board > > >>> > > >>> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org> > > >> > > >> Thanks for the rework! > > >> > > >>> arch/arm/boot/dts/r7s72100-genmai.dts | 51 ++++++++++++ > > >>> arch/arm/boot/dts/r7s72100.dtsi | 151 ++++++++++++++++++++++++++ > > >> > > >> This path should be split in multiple parts: > > >> - Add the pfc node to r7s72100.dtsi, > > >> - Add the gpio nodes to r7s72100.dtsi, > > >> - 4 patches for r7s72100-genmai.dts, adding support for LEDs, SCIF, > > >> Ethernet, and SPI. > > > > > > I can agree about the .dtsi/.dts split, but isn't this going a bit > > > overboard ? > > > > I would like the split so that different patches touch different files > > to be made. > > That's usually what I do, at least when it comes to device tree files. When > reworking core code in a subsystem patches often have to touch multiple > files, > but that's different. > > > I am willing to be flexible regarding adding more than one IP block in a > > single patch if the patches would otherwise be very small and unlikely to > > lead to breakage. > > Splitting the GPIO and PFC nodes in two patches would be fine with me, but > given that they're tightly related, I think it makes more sense to keep them > in one patch in this particular case. > > > From my PoV a key motivation for splitting things up is to make it easier > > to selectively revert or backport individual features. I personally don't > > have much cause to do either on a fine-grained basis of late. So I'm happy > > to consider being more flexible with regards to patch granularity. > > I usually try to split addition of unrelated IP cores in multiple patches. On > the other hand, when adding multiple IP cores in one series, they often end > up > one next to the other in the source, creating conflicts if you try to > backport > selectively. I don't think that's ideal either. > > In this particular case, the changes to arch/arm/boot/dts/r7s72100-genmai.dts > are twofold : > > - add a GPIO LEDs node > - configure pinctrl for the ethernet, spi and scif devices > > We could split that in two patches, but I probably wouldn't split the > last one in three patches.
I agree that makes sense.
