[U-Boot] [PATCH 0/4 V2] Add basic NVIDIA Tegra2 SoC support

[Tom Warren]

On Fri, Jan 14, 2011 at 12:59 PM, Peter Tyser <ptyser at xes-inc.com> wrote:
> Hi Tom,
>
> On Fri, 2011-01-14 at 10:11 -0700, Tom Warren wrote:
>> This series of patches adds preliminary/baseline support for NVIDIA's
>> Tegra2 SoC.  Basic CPU (AVP), RAM and UART init are covered so that the
>> system (Harmony or Seaboard) can boot to the U-Boot serial cmd prompt.
>>
>> Further support (for Cortex-A9 CPU(s), USB, SD/MMC, etc.) to follow.
>>
>> V2: Make changes based on feedback from Peter Tyser and Sandeep Paulraj.
>
> If you didn't use all the feedback to the original patches, you should
> state explicitly what you changed here, or respond to the original
> comment email as to why they weren't made.  For example, I see you
> didn't make the suggested change to use IO access functions, or allow
> compiling out of support for UARTA and UARTD.  That should be made clear
> somewhere (and the logic of why the changes weren't made) so that those
> reviewing the patches know what changed between v1 and v2 and why.  As
> is its unclear why the v1 comments weren't implemented, so they also
> apply to this series.
>
Peter,

Sorry, since this is my first patch series to U-Boot upstream, I'm
still learning the proper etiquette.
Let me respond here rather than in the patch comments since there are
only 2 unchanged areas WRT comments.

1) IO access functions - I pre-reviewed my patch series with Wolfgang
(to hopefully catch any blatant errors and smooth
the process) and he indicated that C structs and I/O accessor funcs or
macros were preferred to my base+offset original code.
Since the ARM is 32-bit, and all of our registers are I/O mapped, it
made sense just to cast the necessary HW mem-mapped
regs as volatile structs and access the members directly. Works well,
is easy to read & understand, etc. Let me know (with
examples, if possible) how I can make it better.

2) Compiling out support for UARTA or UARTD - didn't seem necessary -
size isn't an issue at this point with Tegra2 U-Boot,
and some boards (Harmony, for example) are populated w/hardware for
both UARTA and UARTD, and can have both on at
U-Boot runtime (perhaps for debug out to UARTA whilst normal console
I/O goes to UARTD), so I chose to leave the init code
for both intact. Plus I've never liked code with too many unnecessary
ifdef's - makes it less readable, IMO.

I was going to respond to your review w/a direct, inline reply, but I
thought it better to get the V2 patch out there before the
weekend (we're off for MLK, as well). I'm under some pressure to get a
baseline Tegra2 patchset in before the merge window
closes.  I'll be sure to respond to each issue directly on the list in
the future, though.

Thanks for the thorough review,

Tom
> Best,
> Peter
>
>