Hi Tom, On Thu, Sep 13, 2012 at 2:10 PM, Tom Warren <twarren.nvi...@gmail.com> wrote: > Tom, > > On Thu, Sep 13, 2012 at 11:06 AM, Tom Rini <tr...@ti.com> wrote: >> On Wed, Sep 12, 2012 at 03:10:47PM -0700, Tom Warren wrote: >> >>> Signed-off-by: Tom Warren <twar...@nvidia.com> >> >> A few things: >> - I see some #define FOO[space][space]val that should be [tab] > > Probably copied over from Tegra20 files. I'll turn on whitespace > highlighting in my editor and fix 'em up. > >> - I didn't checkpatch.pl this (nor the whole series) but please do and >> let us know if it's clean or why the warnings are false positives. > > I always run checkpath before submitting. I'll put a notice to that > affect in the next version. Checkpatch ran clean w/only 1 > false-positive about 'macros with complex values should be enclosed in > parenthesis' for the "#define CONFIG_DEFAULT_DEVICE_TREE > tegra30-cardhu" line in cardhu.h. > >> - My preference is to bring in includes and C files and Makefiles and so >> on all at once, when each is needed / useful. YMMV and not a big >> deal. >> - But please make sure that you aren't adding defines / structs / etc >> that aren't used at some point by the end of the series at least. >> Removing (and correcting!) structs and defines was one of the things I >> had to do on am33xx. If it wasn't added until the corresponding >> driver work was being pushed, it'd have saved me some time. >> > > Sure, and that's good advice. I took a couple of passes during the > port to try and remove vestigial and/or useless/unsupported files, > features, and code, but I'm sure I missed some (as Stephen has already > pointed out). I'll address those in V2.
Congrats on getting this out. It is a lot of work! In the Chromium tree, we have an arch-tegra directory where a lot of the common code lives. It sounds like you are going to split that out a bit which is good. I suspect you may also want a few patches to move quite a bit of the code in arch/arm/include/arch-tegra20 to arch/arm/include/arch-tegra. Regards, Simon > > Thanks, > > Tom >> -- >> Tom > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot