On Sun, Feb 17, 2013 at 09:28:33PM +0100, Peter Korsgaard wrote:
"Matt" == Matt Porter mporter@ti.com writes:
Matt> Support the ti814x specific register definitions within Matt> arch-am33xx.
Matt> Signed-off-by: Matt Porter mporter@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.