Re: [XEN/ARM PATCH v6 1/1] Add OdroidXU board (Exynos 5410)

[Julien Grall <julien.grall@linaro.org>]

Hi all,

This patch breaks Xen boot on the Arndale.

It would have been nice to test this patch on the Arndale before pushing 
it on master... I was unable to do it because I was away.

I would prefer to keep a separate smp_init function with hardcoded value 
as long as someone as time to check if we can effectively use the new value.

Regards,

On 04/09/14 15:57, Suriyan Ramasami wrote:
>      XEN/ARM: Add support for the Odroid-XU board.
>
>      The Odroid-XU from hardkernel is an Exynos 5410 based board.
>      This patch adds support to the above said board.
>
> Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com>
> ---
>
> Changes between versions as follows:
>     v6:
>         a. Folded all *_smp_init() functions into exynos5_smp_init()
>            Arndale now shall use exynos5_smp_init() as well getting rid
>            of S5P_PA_SYSRAM
>     v5:
>         a. Get MCT base address from DT.
>         b. Create function exynos5_get_pmu_base_addr() so it can be
>            used by exynos5_reset() and exynos5_cpu_up() functions.
>
>     v4:
>         a. Lots of Xen coding style changes.
>         b. Directly assign cpu_up_send_sgi for 5250.
>         c. S5P_PA_SYSRAM renamed to EXYNOS5250_PA_SYSRAM so it is clear
>            that newer platforms should use the device tree for information.
>         d. Use __raw_readl and _raw_writel instead of readl and writel
>         e. Leave function names consistent with what Linux uses so that it
>            avoids confusion and is easier to backport in the future.
>         f. Remove "samsung,exynos5422-pmu" as no such thing exists.
>         g. Use XENLOG_DEBUG instead of XENLOG_INFO for power_base_address.
>         h. iounmap(power) in the failure case.
>         i. As its generic, call the new platform exynos5.
>     v3:
>         a. Separate commit message and change log.
>         b. Define odroid-xu as a separate platform API.
>         c. Use mainline linux's way of retrieving sysram from DT.
>         d. Use mainline linux's way of bringing up secondary CPUs.
>         e. Keep the #defines in the local C file.
>         f. Bringing up newer Exynos platforms should be easier.
>
>     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
>
> ---
>   xen/arch/arm/platforms/exynos5.c        | 197 +++++++++++++++++++++++++++++---
>   xen/include/asm-arm/platforms/exynos5.h |   3 -
>   2 files changed, 184 insertions(+), 16 deletions(-)
>
> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
> index b65c2c2..f3cbfce 100644
> --- a/xen/arch/arm/platforms/exynos5.c
> +++ b/xen/arch/arm/platforms/exynos5.c
> @@ -23,18 +23,45 @@
>   #include <xen/domain_page.h>
>   #include <xen/mm.h>
>   #include <xen/vmap.h>
> +#include <xen/delay.h>
>   #include <asm/platforms/exynos5.h>
>   #include <asm/platform.h>
>   #include <asm/io.h>
>
> +#define EXYNOS_ARM_CORE0_CONFIG     0x2000
> +#define EXYNOS_ARM_CORE_CONFIG(_nr) (0x80 * (_nr))
> +#define EXYNOS_ARM_CORE_STATUS(_nr) (EXYNOS_ARM_CORE_CONFIG(_nr) + 0x4)
> +#define S5P_CORE_LOCAL_PWR_EN       0x3
> +
>   static int exynos5_init_time(void)
>   {
>       uint32_t reg;
>       void __iomem *mct;
> +    int rc;
> +    struct dt_device_node *node;
> +    u64 mct_base_addr;
> +    u64 size;
>
>       BUILD_BUG_ON(EXYNOS5_MCT_G_TCON >= PAGE_SIZE);
>
> -    mct = ioremap_attr(EXYNOS5_MCT_BASE, PAGE_SIZE, PAGE_HYPERVISOR_NOCACHE);
> +    node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-mct");
> +    if ( !node )
> +    {
> +        dprintk(XENLOG_ERR, "samsung,exynos4210-mct missing in DT\n");
> +        return -ENXIO;
> +    }
> +
> +    rc = dt_device_get_address(node, 0, &mct_base_addr, &size);
> +    if ( rc )
> +    {
> +        dprintk(XENLOG_ERR, "Error in \"samsung,exynos4210-mct\"\n");
> +        return -ENXIO;
> +    }
> +
> +    dprintk(XENLOG_INFO, "mct_base_addr: %016llx size: %016llx\n",
> +            mct_base_addr, size);
> +
> +    mct = ioremap_attr(mct_base_addr, PAGE_SIZE, PAGE_HYPERVISOR_NOCACHE);
>       if ( !mct )
>       {
>           dprintk(XENLOG_ERR, "Unable to map MCT\n");
> @@ -51,7 +78,7 @@ static int exynos5_init_time(void)
>   }
>
>   /* Additional mappings for dom0 (Not in the DTS) */
> -static int exynos5_specific_mapping(struct domain *d)
> +static int exynos5250_specific_mapping(struct domain *d)
>   {
>       /* Map the chip ID */
>       map_mmio_regions(d, paddr_to_pfn(EXYNOS5_PA_CHIPID), 1,
> @@ -67,8 +94,29 @@ static int exynos5_specific_mapping(struct domain *d)
>   static int __init exynos5_smp_init(void)
>   {
>       void __iomem *sysram;
> +    struct dt_device_node *node;
> +    u64 sysram_ns_base_addr;
> +    u64 size;
> +    int rc;
> +
> +    node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-sysram-ns");
> +    if ( !node )
> +    {
> +        dprintk(XENLOG_ERR, "samsung,exynos4210-sysram-ns missing in DT\n");
> +        return -ENXIO;
> +    }
>
> -    sysram = ioremap_nocache(S5P_PA_SYSRAM, PAGE_SIZE);
> +    rc = dt_device_get_address(node, 0, &sysram_ns_base_addr, &size);
> +    if ( rc )
> +    {
> +        dprintk(XENLOG_ERR, "Error in \"samsung,exynos4210-sysram-ns\"\n");
> +        return -ENXIO;
> +    }
> +
> +    dprintk(XENLOG_INFO, "sysram_ns_base_addr: %016llx size: %016llx\n",
> +            sysram_ns_base_addr, size);
> +
> +    sysram = ioremap_nocache(sysram_ns_base_addr, PAGE_SIZE);
>       if ( !sysram )
>       {
>           dprintk(XENLOG_ERR, "Unable to map exynos5 MMIO\n");
> @@ -77,20 +125,127 @@ static int __init exynos5_smp_init(void)
>
>       printk("Set SYSRAM to %"PRIpaddr" (%p)\n",
>              __pa(init_secondary), init_secondary);
> -    writel(__pa(init_secondary), sysram);
> +    writel(__pa(init_secondary), sysram + 0x1c);
>
>       iounmap(sysram);
>
>       return 0;
>   }
>
> +static int exynos_cpu_power_state(void __iomem *power, int cpu)
> +{
> +    return __raw_readl(power + EXYNOS_ARM_CORE_STATUS(cpu)) &
> +                       S5P_CORE_LOCAL_PWR_EN;
> +}
> +
> +static void exynos_cpu_power_up(void __iomem *power, int cpu)
> +{
> +    __raw_writel(S5P_CORE_LOCAL_PWR_EN,
> +                 power + EXYNOS_ARM_CORE_CONFIG(cpu));
> +}
> +
> +static int exynos5_cpu_power_up(void __iomem *power, int cpu)
> +{
> +    unsigned int timeout;
> +
> +    if ( !exynos_cpu_power_state(power, cpu) )
> +    {
> +        exynos_cpu_power_up(power, cpu);
> +        timeout = 10;
> +
> +        /* wait max 10 ms until cpu is on */
> +        while ( exynos_cpu_power_state(power, cpu) != S5P_CORE_LOCAL_PWR_EN )
> +        {
> +            if ( timeout-- == 0 )
> +                break;
> +
> +            mdelay(1);
> +        }
> +
> +        if ( timeout == 0 )
> +        {
> +            dprintk(XENLOG_ERR, "CPU%d power enable failed", cpu);
> +            return -ETIMEDOUT;
> +        }
> +    }
> +    return 0;
> +}
> +
> +static int exynos5_get_pmu_base_addr(u64 *power_base_addr) {
> +    u64 size;
> +    struct dt_device_node *node;
> +    int rc;
> +    static const struct dt_device_match exynos_dt_pmu_matches[] __initconst =
> +    {
> +        DT_MATCH_COMPATIBLE("samsung,exynos5250-pmu"),
> +        DT_MATCH_COMPATIBLE("samsung,exynos5410-pmu"),
> +        DT_MATCH_COMPATIBLE("samsung,exynos5420-pmu"),
> +        { /*sentinel*/ },
> +    };
> +
> +    node = dt_find_matching_node(NULL, exynos_dt_pmu_matches);
> +    if ( !node )
> +    {
> +        dprintk(XENLOG_ERR, "samsung,exynos5XXX-pmu missing in DT\n");
> +        return -ENXIO;
> +    }
> +
> +    rc = dt_device_get_address(node, 0, power_base_addr, &size);
> +    if ( rc )
> +    {
> +        dprintk(XENLOG_ERR, "Error in \"samsung,exynos5XXXX-pmu\"\n");
> +        return -ENXIO;
> +    }
> +
> +    dprintk(XENLOG_DEBUG, "power_base_addr: %016llx size: %016llx\n",
> +            *power_base_addr, size);
> +
> +    return 0;
> +}
> +
> +static int exynos5_cpu_up(int cpu)
> +{
> +    u64 power_base_addr;
> +    void __iomem *power;
> +    int rc;
> +
> +    rc = exynos5_get_pmu_base_addr(&power_base_addr);
> +    if ( rc )
> +        return rc;
> +
> +    power = ioremap_nocache(power_base_addr +
> +                            EXYNOS_ARM_CORE0_CONFIG, PAGE_SIZE);
> +    if ( !power )
> +    {
> +        dprintk(XENLOG_ERR, "Unable to map power MMIO\n");
> +        return -EFAULT;
> +    }
> +
> +    rc = exynos5_cpu_power_up(power, cpu);
> +    if ( rc )
> +    {
> +        iounmap(power);
> +        return -ETIMEDOUT;
> +    }
> +
> +    iounmap(power);
> +
> +    return cpu_up_send_sgi(cpu);
> +}
> +
>   static void exynos5_reset(void)
>   {
> +    u64 power_base_addr;
>       void __iomem *pmu;
> +    int rc;
>
>       BUILD_BUG_ON(EXYNOS5_SWRESET >= PAGE_SIZE);
>
> -    pmu = ioremap_nocache(EXYNOS5_PA_PMU, PAGE_SIZE);
> +    rc = exynos5_get_pmu_base_addr(&power_base_addr);
> +    if ( rc )
> +        return;
> +
> +    pmu = ioremap_nocache(power_base_addr, PAGE_SIZE);
>       if ( !pmu )
>       {
>           dprintk(XENLOG_ERR, "Unable to map PMU\n");
> @@ -98,15 +253,10 @@ static void exynos5_reset(void)
>       }
>
>       writel(1, pmu + EXYNOS5_SWRESET);
> +
>       iounmap(pmu);
>   }
>
> -static const char * const exynos5_dt_compat[] __initconst =
> -{
> -    "samsung,exynos5250",
> -    NULL
> -};
> -
>   static const struct dt_device_match exynos5_blacklist_dev[] __initconst =
>   {
>       /* Multi core Timer
> @@ -117,12 +267,33 @@ static const struct dt_device_match exynos5_blacklist_dev[] __initconst =
>       { /* sentinel */ },
>   };
>
> +static const char * const exynos5250_dt_compat[] __initconst =
> +{
> +    "samsung,exynos5250",
> +    NULL
> +};
> +
> +static const char * const exynos5_dt_compat[] __initconst =
> +{
> +    "samsung,exynos5410",
> +    NULL
> +};
> +
> +PLATFORM_START(exynos5250, "SAMSUNG EXYNOS5250")
> +    .compatible = exynos5250_dt_compat,
> +    .init_time = exynos5_init_time,
> +    .specific_mapping = exynos5250_specific_mapping,
> +    .smp_init = exynos5_smp_init,
> +    .cpu_up = cpu_up_send_sgi,
> +    .reset = exynos5_reset,
> +    .blacklist_dev = exynos5_blacklist_dev,
> +PLATFORM_END
> +
>   PLATFORM_START(exynos5, "SAMSUNG EXYNOS5")
>       .compatible = exynos5_dt_compat,
>       .init_time = exynos5_init_time,
> -    .specific_mapping = exynos5_specific_mapping,
>       .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..c88455a 100644
> --- a/xen/include/asm-arm/platforms/exynos5.h
> +++ b/xen/include/asm-arm/platforms/exynos5.h
> @@ -1,7 +1,6 @@
>   #ifndef __ASM_ARM_PLATFORMS_EXYNOS5_H
>   #define __ASM_ARM_PLATFORMS_EXYNOS5_H
>
> -#define EXYNOS5_MCT_BASE            0x101c0000
>   #define EXYNOS5_MCT_G_TCON          0x240       /* Relative to MCT_BASE */
>   #define EXYNOS5_MCT_G_TCON_START    (1 << 8)
>
> @@ -12,8 +11,6 @@
>
>   #define EXYNOS5_SWRESET             0x0400      /* Relative to PA_PMU */
>
> -#define S5P_PA_SYSRAM   0x02020000
> -
>   #endif /* __ASM_ARM_PLATFORMS_EXYNOS5_H */
>   /*
>    * Local variables:
>

-- 
Julien Grall