[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [Patch v2 1/1] Add Odroid-XU (Exynos 5410)
Hi Suriyan, Thank you for the patch. On 26/07/14 01:02, Suriyan Ramasami wrote: XEN/ARM: Add Odroid-XU support The Odroid-Xu from hardkernel is an Exynos 5410 based board. This patch addds support to the above said board. We usually separate the commit message and the change log via "---" and a newline. So when one of the committers will apply the patch, everything after the "---" will be stripped. v2: a. Set startup address in exynos5_smp_init() only once. b. Turn on power to each core in exynos5_cpu_up(). c. Create single PLATFORM with smp_init, and cpu_up which dispatches to correct code. d. Check return code of io_remap for power. e. Use read* write* calls for MMIO mapped addresses. f. Xen coding style changes. v1: Add Odroid-XU board support Signed-off-by: Suriyan Ramasami <suriyan.r@xxxxxxxxx> Of course, you will have to move you signed-off-by before the "---" :). --- xen/arch/arm/platforms/exynos5.c | 61 +++++++++++++++++++++++++++++++-- xen/include/asm-arm/platforms/exynos5.h | 8 +++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c index b65c2c2..e61d9be 100644 --- a/xen/arch/arm/platforms/exynos5.c +++ b/xen/arch/arm/platforms/exynos5.c @@ -26,6 +26,7 @@ #include <asm/platforms/exynos5.h> #include <asm/platform.h> #include <asm/io.h> +#include <asm/delay.h> static int exynos5_init_time(void) { @@ -67,8 +68,19 @@ static int exynos5_specific_mapping(struct domain *d) static int __init exynos5_smp_init(void) { void __iomem *sysram; + unsigned long pa_sysram; - sysram = ioremap_nocache(S5P_PA_SYSRAM, PAGE_SIZE); + if ( dt_machine_is_compatible(EXYNOS5250_COMPAT) ) + pa_sysram = S5P_PA_SYSRAM; + else if ( dt_machine_is_compatible(EXYNOS5410_COMPAT) ) + pa_sysram = EXYNOS5410_PA_SYSRAM; I dislike this solution. We've introduced the platform API to avoid checking the compatible string every time we call callbacks and make them very simply. Rather than if/else stuff I would prefer if you define a different platform, and create a common function which will be called with the right parameters. On another side, Linux has introduced recently a new bindings to retrieve the sysram from the device tree. See commit b3205dea "ARM: EXYNOS: Map SYSRAM through generic DT bindings" in the Linux upstream tree. I would definitely prefer to use this way on Xen to make simpler adding a new exynos support. The drawback is we don't support older kernel on Xen for those processors. For odroid XU (aka exynos 5410) it's perfectly fine as we don't have yet a support. For the exynos5250, I think it's arguable. I don't think this board is used in production but we may want to keep binding compatibility support with Xen 4.4. I will let the ARM maintainers decide on this point. If we decide to keep compatibility, then I still would prefer a clean support for Odroid Xu. Even if it's mean to rework the current exynos platform code. + else + { + dprintk(XENLOG_ERR, "Machine not compatible!\n"); + return -EFAULT; + } + + sysram = ioremap_nocache(pa_sysram, PAGE_SIZE); if ( !sysram ) { dprintk(XENLOG_ERR, "Unable to map exynos5 MMIO\n"); @@ -84,6 +96,48 @@ static int __init exynos5_smp_init(void) return 0; } +static int __init exynos5_cpu_up(int cpu) +{ + void __iomem *power; + char byte0, byte4; + int rc; + + if ( dt_machine_is_compatible(EXYNOS5250_COMPAT) ) { + rc = cpu_up_send_sgi(cpu); + return rc; + } Introducing 2 differents platform (one for the exynos5250 and the other for the 5410) would permit to have a separate callbacks for the cpu up and avoid again the if/else. + /* EXYNOS5410: Power the secondary core */ + power = ioremap_nocache(EXYNOS5410_POWER_CPU_BASE + + cpu * EXYNOS5410_POWER_CPU_OFFSET, PAGE_SIZE); Is it really exynos5410 specific? Linux upstream seems to have a generic way to bring up secondary CPUs. It would be better if we have a similar one. So bring up a new exynos platform will be more easier. + if ( !power ) + { + dprintk(XENLOG_ERR, "Unable to map exynos5410 MMIO\n"); + return -EFAULT; + } + + byte0 = readb(power); + byte4 = readb(power + 4); + dprintk(XENLOG_INFO, "Power: %x status: %x\n", byte0, byte4); + + writeb(EXYNOS5410_POWER_ENABLE, power); + dprintk(XENLOG_INFO, "Waiting for power status to change to %x\n", + EXYNOS5410_POWER_ENABLE); + + do + { + udelay(1); + byte4 = readb(power + 4); Xen will go in an infinite loop, if the CPU has not been correctly enabled. I think you should add a timeout there. + } while ( byte4 != EXYNOS5410_POWER_ENABLE ); + + dprintk(XENLOG_INFO, "Power status changed to %x!\n", byte4); + + iounmap(power); + + rc = cpu_up_send_sgi(cpu); + return rc; +} + static void exynos5_reset(void) { void __iomem *pmu; @@ -103,7 +157,8 @@ static void exynos5_reset(void) static const char * const exynos5_dt_compat[] __initconst = { - "samsung,exynos5250", + EXYNOS5250_COMPAT, + EXYNOS5410_COMPAT, NULL }; @@ -122,7 +177,7 @@ PLATFORM_START(exynos5, "SAMSUNG EXYNOS5") .init_time = exynos5_init_time, .specific_mapping = exynos5_specific_mapping, I'm not sure if those specific mapping are relevant for the odroid XU... Hence IIRC, they are now defined in the device tree. Regards, .smp_init = exynos5_smp_init, - .cpu_up = cpu_up_send_sgi, + .cpu_up = exynos5_cpu_up, .reset = exynos5_reset, .blacklist_dev = exynos5_blacklist_dev, PLATFORM_END diff --git a/xen/include/asm-arm/platforms/exynos5.h b/xen/include/asm-arm/platforms/exynos5.h index ea941e7..4d84fe8 100644 --- a/xen/include/asm-arm/platforms/exynos5.h +++ b/xen/include/asm-arm/platforms/exynos5.h @@ -13,6 +13,14 @@ #define EXYNOS5_SWRESET 0x0400 /* Relative to PA_PMU */ #define S5P_PA_SYSRAM 0x02020000 +#define EXYNOS5250_COMPAT "samsung,exynos5250" + +/* Exynos5410 specific */ +#define EXYNOS5410_PA_SYSRAM 0x0207301c +#define EXYNOS5410_POWER_CPU_BASE (0x10040000 + 0x2000) +#define EXYNOS5410_POWER_CPU_OFFSET (0x80) +#define EXYNOS5410_POWER_ENABLE (0x03) +#define EXYNOS5410_COMPAT "samsung,exynos5410" #endif /* __ASM_ARM_PLATFORMS_EXYNOS5_H */ /* -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |