Tom, On Thu, Sep 13, 2012 at 3:04 PM, Tom Rini <tr...@ti.com> wrote: > On Thu, Sep 13, 2012 at 02:21:54PM -0700, Tom Warren wrote: >> Tom, >> >> On Thu, Sep 13, 2012 at 1:33 PM, Tom Rini <tr...@ti.com> wrote: >> > On 09/13/2012 01:30 PM, Stephen Warren wrote: >> >> On 09/13/2012 02:16 PM, Tom Warren wrote: >> >>> Stephen, >> >>> >> >>> On Thu, Sep 13, 2012 at 1:03 PM, Stephen Warren <swar...@wwwdotorg.org> >> >>> wrote: >> >>>> On 09/12/2012 04:10 PM, Tom Warren wrote: >> >>>> >> >>>>> diff --git a/arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c >> >>>>> b/arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c >> >>>> >> >>>> This whole file is definitely common with Tegra20. >> >>> >> >>> I'm going through your previous comments, but I'll just reply quickly >> >>> to this one since it needs clearing up. >> >>> >> >>> The intent of this first series of patches for Tegra30 was just to get >> >>> to the command prompt on T30 in the quickest way, while impacting >> >>> Tegra20 code as little as possible. Hence, I used Tegra20 files to >> >>> create a Tegra30 build, and as I ported it to T30 HW, I tried to >> >>> eliminate what I could that I knew for sure was T20-specific and not >> >>> useful. But I've made no effort to combine common files/code in this >> >>> initial pass. I think it's much easier to understand and review these >> >>> files as a separate SoC build, rather than having to parse >> >>> common/combined files and code. I intend to do the >> >>> combination/common-izing of the TegraXX builds once I have a >> >>> reasonable T30 build in u-boot-tegra, perhaps even before I start >> >>> porting the drivers. But this is the initial approach I took. >> >>> Hopefully it'll be an acceptable course - I'd hate to have to >> >>> back-track. >> >> >> >> To be honest, it seems like the patch to add the Tegra30 deltas to the >> >> existing Tegra20 code would be massively smaller than duplicating all of >> >> Tegra20 as Tegra30 and applying those same changes. In the kernel, we >> >> have both Tegra20 and Tegra30 support with run-time differentiation, and >> >> the number of places where we have to do something different is not that >> >> large at all. With the current patch series, there's a huge amount of >> >> code to wade through, so spotting any places that haven't been updated >> >> for Tegra30, or weren't intended to be updated yet, is somewhat painful. >> > >> > Since we know that the delta can be small, yes, let's just do this right >> > the first time (or so). incremental moves, additions and we can work >> > out run-vs-build time a little further down the road. >> >> Sorry, Tom. I'm not clear on exactly which way you'd like to see this go. >> >> Are you advising that I re-cast this patchset as a set of common Tegra >> files/code, with deltas/diffs for the Tegra30 changes? That implies, I >> think, that I first have to do a patchset that re-orgs Tegra20 code >> into common code, and then submit a smaller version of this patchset >> that is just deltas for Tegra30. That means that I'll be touching >> everyone's Tegra20 code, and will need Ack's from all the T20 vendors >> before I can move forward w/T30 code. > > As far as I'm conerend to do a: > git mv arch/arm/cpu/armv7/tegra20/cmd_enterrcm.c > arch/arm/cpu/armv7/tegra-common/cmd_enterrcm.c (just looking at top of > tree mainline) needs just the overall Tegra maintainer to Ack. The > Custodians page says that's you, and so long as you MAKEALL -s tegra20 > before and after, that's good to go. By that same token, breaking out a > hypothetical set of common functions from tegra20/usb.c into > tegra-common/usb.c and leaving the T20 specific bits in tegra20/usb.c > and adding tegra30/usb.c later just needs you and MAKEALL happy. > >> The other approach, which is still a 2-(or more)-patchset process, is >> to continue with this patchset for T30, with corrections as per >> review, and then immediately work on a 'merge-to-common-code' set of >> patches to common-ize Tegra20/30. That way Tegra20 is unaffected, I >> can keep moving forward, and I think the end result will be the same >> as the approach above. > > As has been noted, when you copy files you get a lot of re-review and > it's hard to tell what's new and what's not. It IS good to post what > you have posted and say "please check areas X/Y/Z, it's new code". But > reviewing code that will be dropped as soon as you switch to the > 'merge-to-common-code' branch being the reviewed one is hard, especially > when folks with less Tegra background try and read the patches for > general issues. > >> I can see value in both approaches, and it shouldn't surprise you that >> I'd favor the 2nd approach, since it's less chaotic for me. Let me >> know what you think, > > Well, I'd argue that since you're going to need to do the > 'merge-to-common-code' path at some point, it's going to save you work > to do that now rather than fixup issues in two places. And again, if > you don't change the code, just where the code is, MAKEALL will catch > your problems for you.
Moving the code via git as you pointed out above is fine, but how do I hook 'tegra-common' into the boards.cfg/Makefile/MAKEALL process? I'd need a 'tegra-common' subdir for arm720t, armv7 and an 'arch-tegra' subdir for arch/arm/include/asm/. The arch-tegra common subdir is easy, as I can just have the <asm/arch/blah.h> file do an 'include <asm/arch-tegra/blah.h> when the include file is 100% common and flesh it out when it's SoC-specific. But I don't see how I can build the SPL and 'normal' U-Boot sections via boards.cfg w/common code, without having a SPL.C that says 'include ../tegra-common/spl.c', which seems messy. Not to mention that we have arch/arm/cpu/tegra20-common and ../tegra30-common, too. How do you do it on OMAP? Tom > > -- > Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
