[U-Boot] [PATCH 05/10] am33xx: add ti814x specific register definitions

[Tom Rini]

On Sun, Feb 17, 2013 at 09:28:33PM +0100, Peter Korsgaard wrote:
> >>>>> "Matt" == Matt Porter <mporter at ti.com> writes:
> 
>  Matt> Support the ti814x specific register definitions within
>  Matt> arch-am33xx.
> 
>  Matt> Signed-off-by: Matt Porter <mporter at ti.com>
>  Matt> ---
>  Matt>  arch/arm/cpu/armv7/am33xx/sys_info.c        |    3 +++
>  Matt>  arch/arm/include/asm/arch-am33xx/cpu.h      |   11 +++++----
>  Matt>  arch/arm/include/asm/arch-am33xx/hardware.h |   32 +++++++++++++++++++++++++++
>  Matt>  arch/arm/include/asm/arch-am33xx/omap.h     |    7 ++++++
>  Matt>  arch/arm/include/asm/arch-am33xx/spl.h      |    5 +++++
>  Matt>  5 files changed, 54 insertions(+), 4 deletions(-)
>  
>  Matt> diff --git a/arch/arm/include/asm/arch-am33xx/hardware.h b/arch/arm/include/asm/arch-am33xx/hardware.h
>  Matt> index 41ab2c0..786c159 100644
>  Matt> --- a/arch/arm/include/asm/arch-am33xx/hardware.h
>  Matt> +++ b/arch/arm/include/asm/arch-am33xx/hardware.h
>  Matt> @@ -20,9 +20,14 @@
>  Matt>  #define __AM33XX_HARDWARE_H
>  
>  Matt>  #include <asm/arch/omap.h>
>  Matt> +#include <config.h>
> 
> Quite some of the base addresses are similar, but I wonder if it
> wouldn't be cleaner to simply have a hardware-am33xx.h /
> hardware-ti814x.h instead of all these ifdef / elif?

Since I suspect the things common from ti814x and am33xx are also common
to ti816x (which has been left as an exercise to whomever needs that
next), I think we can re-structure this into something like that, but
keeping the common parts within hardware.h still.

>  Matt>  /* Control Module Base Address */
>  Matt> +#ifdef CONFIG_AM33XX
>  Matt>  #define CTRL_BASE			0x44E10000
>  Matt>  #define CTRL_DEVICE_BASE		0x44E10600
>  Matt> +#elif defined(CONFIG_TI814X)
>  Matt> +#define CTRL_BASE			0x48140000
>  Matt> +#endif
> 
> No CTRL_DEVICE_BASE on ti814x?

I think this is a side-effect of Matt not supporting the things attached
to it (USB in the case of am335x).

>  Matt> --- a/arch/arm/include/asm/arch-am33xx/spl.h
>  Matt> +++ b/arch/arm/include/asm/arch-am33xx/spl.h
>  Matt> @@ -25,8 +25,13 @@
>  
>  Matt>  #define BOOT_DEVICE_XIP       	2
>  Matt>  #define BOOT_DEVICE_NAND	5
>  Matt> +#ifdef CONFIG_AM33XX
>  Matt>  #define BOOT_DEVICE_MMC1	8
>  Matt>  #define BOOT_DEVICE_MMC2	9	/* eMMC or daughter card */
>  Matt> +#elif defined(CONFIG_TI814X)
>  Matt> +#define BOOT_DEVICE_MMC1	9
>  Matt> +#define BOOT_DEVICE_MMC2	8	/* ROM only supports 2nd instance */
> 
> Argh! Couldn't we just swap the meaning of mmc1/mmc2 or would that be
> too confusing?

IMHO, that will lead to further confusion down the line.  I talked with
Matt about this before and well, it's funky.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130218/8a90135f/attachment.pgp>