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

[Tom Warren]

Simon,

On Tue, Sep 18, 2012 at 12:29 PM, Simon Glass <sjg at chromium.org> wrote:
> 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.

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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot