"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