"Mark A. Greer" <mgr...@mvista.com> writes:
> From: Mark A. Greer <mgr...@mvista.com>
>
> The da830/omap l137 is a new SoC from TI that is similar
> to the davinci line. Since its so similar to davinci,
> put the support for the da830 in the same directory as
> the davinci code.
>
> There are differences, however. Some of those differences
> prevent support for davinci and da830 platforms to work
> in the same kernel binary. Those differences are:
>
> 1) Different physical address for RAM. This is relevant
> to Makefile.boot addresses and PHYS_OFFSET. The
> Makefile.boot issue isn't truly a kernel issue but
> it means u-boot won't work with a uImage including
> both architectures. The PHYS_OFFSET issue is
> addressed by the "Allow for runtime-determined
> PHYS_OFFSET" patch by Lennert Buytenhek but it
> hasn't been accepted yet.
Other than (2), have you verified that the the PHYS_OFFSET patch
submitted can build and boot a single kernel for da8xx and the
DAVINCI_TRUE SoCs?
> 2) Different uart addresses. This is only an issue
> for the assembly 'addruart' macro when CONFIG_DEBUG_LL
> is enabled. Since the code in that macro is called
> so early (e.g., by _error_p in kernel/head.S when
> the processor lookup fails), we can't determine what
> platform the kernel is running on at runtime to use
> the correct uart address.
Maybe add an #error someplace if DEBUG_LL && DAVINCI_TRUE && DA830 so
that when the PHYS_OFFSET problem is resolved (hopefully) this
dependency is still prevented.
> These areas have (or will have) compile errors
> intentionally inserted if both CONFIG_ARCH_DAVINCI_TRUE
> and CONFIG_ARCH_DAVINCI_DA830 are enabled at the same
> time. There is also a check in mach-davinci/Kconfig
> to prevent enabling both architectures concurrently.
>
> A new config variable, CONFIG_ARCH_DAVINCI_TRUE, is
> added to simply distinction between a true davinci
> architecture and the da830 architecture.
>
> The da830 currently has an issue with writeback data
> cache so CONFIG_CPU_DCACHE_WRITETHROUGH is forced on
> when CONFIG_ARCH_DAVINCI_DA830 is enabled.
Hmm, any thoughts on how this issue might be addressed with a single
kernel? Doesn't seem too simple. I assume a writethrough kernel works
fine on the other DaVincis as well.
What exactly is the "issue" being worked around. Is it a HW bug? Is
this workaround always required?
Is this something that could be changed slightly later in the boot by
SoC specific code?
> Signed-off-by: Mark A. Greer <mgr...@mvista.com>
> ---
> arch/arm/mach-davinci/Kconfig | 12 +
> arch/arm/mach-davinci/Makefile | 3 +
> arch/arm/mach-davinci/Makefile.boot | 10 +
> arch/arm/mach-davinci/da830.c | 1034
> +++++++++++++++++++++++++++
> arch/arm/mach-davinci/devices-da830.c | 517 +++++++++++++
> arch/arm/mach-davinci/include/mach/cpu.h | 8 +
> arch/arm/mach-davinci/include/mach/da830.h | 133 ++++
> arch/arm/mach-davinci/include/mach/edma.h | 48 ++
> arch/arm/mach-davinci/include/mach/irqs.h | 110 +++-
> arch/arm/mach-davinci/include/mach/memory.h | 11 +-
> arch/arm/mach-davinci/include/mach/mux.h | 423 +++++++++++-
> arch/arm/mach-davinci/include/mach/psc.h | 42 ++
> arch/arm/mach-davinci/include/mach/serial.h | 4 +
> 13 files changed, 2349 insertions(+), 6 deletions(-)
> create mode 100644 arch/arm/mach-davinci/da830.c
> create mode 100644 arch/arm/mach-davinci/devices-da830.c
> create mode 100644 arch/arm/mach-davinci/include/mach/da830.h
>
> diff --git a/arch/arm/mach-davinci/Kconfig b/arch/arm/mach-davinci/Kconfig
> index d9bbd41..13cfc86 100644
> --- a/arch/arm/mach-davinci/Kconfig
> +++ b/arch/arm/mach-davinci/Kconfig
> @@ -6,6 +6,9 @@ config AINTC
> config CP_INTC
> bool
>
> +config ARCH_DAVINCI_TRUE
> + bool
> +
Still not crazy about the name "true" here. Seems to unnecessarily
belittle the da830. ;)
Maybe ARCH_DAVINCI_DMx or ARCH_DAVINCI_COMMON?
In any case, I think that Kconfig item could disappear once the issues
you mentioned in the description are sorted out.
> menu "TI DaVinci Implementations"
>
> comment "DaVinci Core Type"
> @@ -13,14 +16,23 @@ comment "DaVinci Core Type"
> config ARCH_DAVINCI_DM644x
> bool "DaVinci 644x based system"
> select AINTC
> + select ARCH_DAVINCI_TRUE
>
> config ARCH_DAVINCI_DM646x
> bool "DaVinci 646x based system"
> select AINTC
> + select ARCH_DAVINCI_TRUE
>
> config ARCH_DAVINCI_DM355
> bool "DaVinci 355 based system"
> select AINTC
> + select ARCH_DAVINCI_TRUE
> +
> +config ARCH_DAVINCI_DA830
> + bool "DA830/OMAP-L137 based system"
> + depends on !ARCH_DAVINCI_TRUE
I suggest you drop this restriction and let a compile be attempted.
The #errors inserted below will maybe irritate someone to help fix the
problems. :)
> + select CP_INTC
> + select CPU_DCACHE_WRITETHROUGH
>
> comment "DaVinci Board Type"
>
> diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile
> index fab706e..b50bc41 100644
> --- a/arch/arm/mach-davinci/Makefile
> +++ b/arch/arm/mach-davinci/Makefile
> @@ -15,10 +15,13 @@ obj-$(CONFIG_TI_DAVINCI_EMAC_MODULE) += mac_addr.o
> obj-$(CONFIG_ARCH_DAVINCI_DM644x) += dm644x.o
> obj-$(CONFIG_ARCH_DAVINCI_DM646x) += dm646x.o
> obj-$(CONFIG_ARCH_DAVINCI_DM355) += dm355.o
> +obj-$(CONFIG_ARCH_DAVINCI_DA830) += da830.o devices-da830.o
>
> obj-$(CONFIG_AINTC) += irq.o
> obj-$(CONFIG_CP_INTC) += cp_intc.o
>
> +obj-$(CONFIG_ARCH_DAVINCI_TRUE) += devices.o
> +
How about appending devices.o to dm*.c above and dropping this use of
DAVINCI_TRUE.
[...]
> +#ifdef CONFIG_DAVINCI_MUX
> +/*
> + * Device specific mux setup
> + *
> + * soc description mux mode mode mux dbg
> + * reg offset mask mode
> + */
> +static const struct mux_config da830_pins[] = {
As pointed out in the MUX patch, move the #ifdef here, and drop the other
#ifdefs.
> + MUX_CFG(DA830, GPIO7_14, 0, 0, 0xf, 1, false)
[...]
> --- /dev/null
> +++ b/arch/arm/mach-davinci/include/mach/da830.h
[...]
> +
> +#define DA830_ARM_INTC_BASE 0xfffee000
> +#define DA830_CP_INTC_SIZE SZ_8K
> +#define DA830_CP_INTC_VIRT (IO_VIRT - DA830_CP_INTC_SIZE - SZ_4K)
I think a comment is missing here as to the need for this virtual
mapping. I understand it now, but I know someday I'll forget why.
That would also clarify why you're not using IO_ADDRESS() here.
[...]
> +enum {
> + DA830_PDEV_EDMA,
> + DA830_PDEV_I2C_0,
> + DA830_PDEV_I2C_1,
> + DA830_PDEV_SPI_0,
> + DA830_PDEV_SPI_1,
> + DA830_PDEV_WATCHDOG,
> + DA830_PDEV_USB_20,
> + DA830_PDEV_USB_11,
> + DA830_PDEV_EMAC,
> + DA830_PDEV_MMC,
> + DA830_PDEV_RTC,
> + DA830_PDEV_EQEP_0,
> + DA830_PDEV_EQEP_1,
> + DA830_PDEV_LCDC,
> + DA830_PDEV_NUM
> +};
> +
> +struct da830_pdata {
> + int dev; /* Device to enable */
> + void *pdata; /* platform_data for this device */
> +};
> +
> +void da830_add_devices(struct da830_pdata pdata[DA830_PDEV_NUM],
> + unsigned long num);
> +
Maybe it's getting too late for me, but I don't see the point of these enums
and the extra array of dev/pdata associations.
I think it's simpler and clearer for the SoC code to just register all the
devices and platform data all the time.
[...]
> diff --git a/arch/arm/mach-davinci/include/mach/irqs.h
> b/arch/arm/mach-davinci/include/mach/irqs.h
> index bc5d6aa..8ae55a5 100644
> --- a/arch/arm/mach-davinci/include/mach/irqs.h
> +++ b/arch/arm/mach-davinci/include/mach/irqs.h
> @@ -99,9 +99,6 @@
> #define IRQ_EMUINT 63
>
> #define DAVINCI_N_AINTC_IRQ 64
> -#define DAVINCI_N_GPIO 104
> -
> -#define NR_IRQS (DAVINCI_N_AINTC_IRQ + DAVINCI_N_GPIO)
>
> #define ARCH_TIMER_IRQ IRQ_TINT1_TINT34
>
> @@ -206,4 +203,111 @@
> #define IRQ_DM355_GPIOBNK5 59
> #define IRQ_DM355_GPIOBNK6 60
>
> +/* DA830 interrupts */
> +#define IRQ_DA830_COMMTX 0
> +#define IRQ_DA830_COMMRX 1
> +#define IRQ_DA830_NINT 2
> +#define IRQ_DA830_EVTOUT0 3
> +#define IRQ_DA830_EVTOUT1 4
> +#define IRQ_DA830_EVTOUT2 5
> +#define IRQ_DA830_EVTOUT3 6
> +#define IRQ_DA830_EVTOUT4 7
> +#define IRQ_DA830_EVTOUT5 8
> +#define IRQ_DA830_EVTOUT6 9
> +#define IRQ_DA830_EVTOUT7 10
> +#define IRQ_DA830_CCINT0 11
> +#define IRQ_DA830_CCERRINT 12
> +#define IRQ_DA830_TCERRINT0 13
> +#define IRQ_DA830_AEMIFINT 14
> +#define IRQ_DA830_I2CINT0 15
> +#define IRQ_DA830_MMCSDINT0 16
> +#define IRQ_DA830_MMCSDINT1 17
> +#define IRQ_DA830_ALLINT0 18
> +#define IRQ_DA830_RTC 19
> +#define IRQ_DA830_SPINT0 20
> +#define IRQ_DA830_TINT12_0 21
> +#define IRQ_DA830_TINT34_0 22
> +#define IRQ_DA830_TINT12_1 23
> +#define IRQ_DA830_TINT34_1 24
> +#define IRQ_DA830_UARTINT0 25
> +#define IRQ_DA830_KEYMGRINT 26
> +#define IRQ_DA830_SECINT 26
> +#define IRQ_DA830_SECKEYERR 26
> +#define IRQ_DA830_MPUERR 27
> +#define IRQ_DA830_IOPUERR 27
> +#define IRQ_DA830_BOOTCFGERR 27
> +#define IRQ_DA830_CHIPINT0 28
> +#define IRQ_DA830_CHIPINT1 29
> +#define IRQ_DA830_CHIPINT2 30
> +#define IRQ_DA830_CHIPINT3 31
> +#define IRQ_DA830_TCERRINT1 32
> +#define IRQ_DA830_C0_RX_THRESH_PULSE 33
> +#define IRQ_DA830_C0_RX_PULSE 34
> +#define IRQ_DA830_C0_TX_PULSE 35
> +#define IRQ_DA830_C0_MISC_PULSE 36
> +#define IRQ_DA830_C1_RX_THRESH_PULSE 37
> +#define IRQ_DA830_C1_RX_PULSE 38
> +#define IRQ_DA830_C1_TX_PULSE 39
> +#define IRQ_DA830_C1_MISC_PULSE 40
> +#define IRQ_DA830_MEMERR 41
> +#define IRQ_DA830_GPIO0 42
> +#define IRQ_DA830_GPIO1 43
> +#define IRQ_DA830_GPIO2 44
> +#define IRQ_DA830_GPIO3 45
> +#define IRQ_DA830_GPIO4 46
> +#define IRQ_DA830_GPIO5 47
> +#define IRQ_DA830_GPIO6 48
> +#define IRQ_DA830_GPIO7 49
> +#define IRQ_DA830_GPIO8 50
> +#define IRQ_DA830_I2CINT1 51
> +#define IRQ_DA830_LCDINT 52
> +#define IRQ_DA830_UARTINT1 53
> +#define IRQ_DA830_MCASPINT 54
> +#define IRQ_DA830_ALLINT1 55
> +#define IRQ_DA830_SPINT1 56
> +#define IRQ_DA830_UHPI_INT1 57
> +#define IRQ_DA830_USB_INT 58
> +#define IRQ_DA830_IRQN 59
> +#define IRQ_DA830_RWAKEUP 60
> +#define IRQ_DA830_UARTINT2 61
> +#define IRQ_DA830_DFTSSINT 62
> +#define IRQ_DA830_EHRPWM0 63
> +#define IRQ_DA830_EHRPWM0TZ 64
> +#define IRQ_DA830_EHRPWM1 65
> +#define IRQ_DA830_EHRPWM1TZ 66
> +#define IRQ_DA830_EHRPWM2 67
> +#define IRQ_DA830_EHRPWM2TZ 68
> +#define IRQ_DA830_ECAP0 69
> +#define IRQ_DA830_ECAP1 70
> +#define IRQ_DA830_ECAP2 71
> +#define IRQ_DA830_EQEP0 72
> +#define IRQ_DA830_EQEP1 73
> +#define IRQ_DA830_T12CMPINT0_0 74
> +#define IRQ_DA830_T12CMPINT1_0 75
> +#define IRQ_DA830_T12CMPINT2_0 76
> +#define IRQ_DA830_T12CMPINT3_0 77
> +#define IRQ_DA830_T12CMPINT4_0 78
> +#define IRQ_DA830_T12CMPINT5_0 79
> +#define IRQ_DA830_T12CMPINT6_0 80
> +#define IRQ_DA830_T12CMPINT7_0 81
> +#define IRQ_DA830_T12CMPINT0_1 82
> +#define IRQ_DA830_T12CMPINT1_1 83
> +#define IRQ_DA830_T12CMPINT2_1 84
> +#define IRQ_DA830_T12CMPINT3_1 85
> +#define IRQ_DA830_T12CMPINT4_1 86
> +#define IRQ_DA830_T12CMPINT5_1 87
> +#define IRQ_DA830_T12CMPINT6_1 88
> +#define IRQ_DA830_T12CMPINT7_1 89
> +#define IRQ_DA830_ARMCLKSTOPREQ 90
> +
> +#define DA830_N_CP_INTC_IRQ 96
> +
> +#ifdef CONFIG_ARCH_DAVINCI_DA830
> +#define DAVINCI_N_GPIO 128
> +#define NR_IRQS (DA830_N_CP_INTC_IRQ +
> DAVINCI_N_GPIO)
> +#else
> +#define DAVINCI_N_GPIO 104
> +#define NR_IRQS (DAVINCI_N_AINTC_IRQ +
> DAVINCI_N_GPIO)
> +#endif
> +
No #ifdef here please. These #defines are to reflect the maximum
number on any supported SoC. If there's an #ifdef here, then the next
chip to add support will add another #ifdef and will most likely break
multi-SoC support.
Just drop the #idef and do something like:
/* Define the maximum value of any SoC */
#define DAVINCI_N_GPIO 128 /* da8xx has most GPIO lines */
#define NR_IRQS 96 /* CP_INTC has more than AINTC */
[...]
Kevin
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
