[U-Boot] [PATCH 4/7] Tegra30: Add arch-tegra30 include files

[Stephen Warren <swarren@wwwdotorg.org>]

On 10/03/2012 03:48 PM, Tom Warren wrote:
> Stephen,
> 
> On Wed, Oct 3, 2012 at 1:31 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/02/2012 04:45 PM, Tom Warren wrote:
>>> Common Tegra files are in arch-tegra, shared between T20 and T30.
>>> Tegra30-specific headers are in arch-tegra30. Note that some of
>>> these will be filled in as more T30 support is added (drivers,
>>> WB/LP0 support, etc.).
...
>>> diff --git a/arch/arm/include/asm/arch-tegra30/emc.h b/arch/arm/include/asm/arch-tegra30/emc.h
>>
>> This file seems to be:
>>
>> a) An exact duplicate of the Tegra20 file. Did the register layout
>> really not change at all? That seems unlikely. If so, shouldn't the file
>> be shared?
>>
>> b) Not used by anything in this patch series, so shouldn't be needed.
>>
>> c) Incorrect; struct emc_ctlr doesn't actually match the register layout
>> for Tegra20 or Tegra30 (at least, offset 0 is interrupt status in HW in
>> both chips, not cfg as in the struct). Some fields match though.
> 
> I'll revisit it in our internal U-Boot source. I tried to go through
> all the include files and make sure any not used on T30 initially were
> removed, but if it's used in a common file, I didn't want to add #if
> defined(CONFIG_TEGRA20) fencing if not necessary.

Hmm. I guess this should be conditional on something like
CONFIG_TEGRA_EMC irrespective of the new Tegra30 changes; I imagine any
EMC-related code is quite optional even on Tegra20 where there is a
driver coded up.

>>> diff --git a/arch/arm/include/asm/arch-tegra30/gpio.h b/arch/arm/include/asm/arch-tegra30/gpio.h
>>
>>> + * The Tegra 3x GPIO controller has 246 GPIOS in 8 banks of 4 ports,
>>> + *  each with 8 GPIOs.
>>
>> Extra space there after *.
>>
>>> diff --git a/arch/arm/include/asm/arch-tegra30/hardware.h b/arch/arm/include/asm/arch-tegra30/hardware.h
>>
>> This file is empty except for comments. Is it really useful? The Tegra20
>> file is empty too
> 
> It's included from include/asm/hardware.h, which is included in 3 or
> so arm720t files, so it's mandatory. I hate empty files, but I don't
> see a way around it without polluting some arm720t files with
> #ifdef's. the integratorap_cm720t does the same thing.

OK, makes sense.

>>> diff --git a/arch/arm/include/asm/arch-tegra30/pinmux-config-common.h b/arch/arm/include/asm/arch-tegra30/pinmux-config-common.h
>>
>> The content of this file presumably describes Cardhu (which revision?)
>> Surely it should be in board/nvidia/cardhu/*.c?
> 
> On the previous review cycle (before I commonized Tegra files), this
> file lived in board/nvidia/cardhu, and I think it was Tom Rini (or
> maybe Simon) that pointed out that it had no 'cardhu' identifiers in
> it, so it should be moved to a more Tegra30-centric area, hence the
> move to arch-tegra30. Again, this is from our internal repo, and I
> don't believe there's any rev info in that file (except for
> commented-out tables that aren't used, so I removed them).  I think
> it's valid to say this file is common to all Tegra30 builds, and
> additional sparse pinmux tables could be added in board files/areas
> for board-specific / rev-specific mux changes.

Sorry, there's no way those tables aren't Cardhu-specific; they're
picking specific pinmux options for all/many of the pins, and there's no
reason in general to believe that every Tegra30 board will use the exact
same pinmux setup.

In practice, I know that e.g. Waluigi and Beaver use a very similar
pinmux, but I imagine there are some differences even there. So, chunks
of the table could be re-used, but certainly not the whole thing. I
think the best way forward here is to put this current file into the
Cardhu directory, and then implement pinmux-from-device-tree before
adding support for any other boards that need a different pinmux.