[U-Boot] [PATCH 1/9] Tegra: T30: Add include files

[Simon Glass]

Hi Tom,

On Thu, Sep 13, 2012 at 2:10 PM, Tom Warren <twarren.nvidia at gmail.com> wrote:
> Tom,
>
> On Thu, Sep 13, 2012 at 11:06 AM, Tom Rini <trini at ti.com> wrote:
>> On Wed, Sep 12, 2012 at 03:10:47PM -0700, Tom Warren wrote:
>>
>>> Signed-off-by: Tom Warren <twarren at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot