On 10/03/2012 03:48 PM, Tom Warren wrote: > Stephen, > > On Wed, Oct 3, 2012 at 1:31 PM, Stephen Warren <swar...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot