Simon, On Tue, Sep 18, 2012 at 12:29 PM, Simon Glass <s...@chromium.org> wrote: > 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.
As per a side-discussion with TomR and StephenW, I am in the process of recasting this T30 patchset as a move to common Tegra code. It'll mimic quite a lot of the Chromium U-Boot directory structure, I'm sure. Thanks, Tom > > 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