[Patch v3 1/1] Add Odroid-XU (Exynos5410)

[Patch v3 1/1] Add Odroid-XU (Exynos5410)

[Suriyan Ramasami]

    XEN/ARM: Add Odroid-XU support

    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:
   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 | 167 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 154 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index b65c2c2..965ac8e 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -23,10 +23,16 @@
 #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;
@@ -51,7 +57,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,
@@ -64,11 +70,16 @@ static int exynos5_specific_mapping(struct domain *d)
     return 0;
 }
 
-static int __init exynos5_smp_init(void)
+static int __init exynos5250_cpu_up(int cpu)
+{
+    return cpu_up_send_sgi(cpu);
+}
+
+static int __init exynos_smp_init(unsigned long pa_sysram)
 {
     void __iomem *sysram;
 
-    sysram = ioremap_nocache(S5P_PA_SYSRAM, PAGE_SIZE);
+    sysram = ioremap_nocache(pa_sysram, PAGE_SIZE);
     if ( !sysram )
     {
         dprintk(XENLOG_ERR, "Unable to map exynos5 MMIO\n");
@@ -84,6 +95,121 @@ static int __init exynos5_smp_init(void)
     return 0;
 }
 
+static int __init exynos5250_smp_init(void)
+{
+    return exynos_smp_init(S5P_PA_SYSRAM);
+}
+
+static int __init exynos5_smp_init(void)
+{
+    struct dt_device_node *node;
+    u64 sysram_ns_base_addr = 0;
+    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;
+    }
+
+    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: %08x size: %08x\n",
+            (unsigned int) sysram_ns_base_addr, (unsigned int) size);
+
+    return exynos_smp_init(sysram_ns_base_addr + 0x1c);
+}
+
+static int exynos_cpu_power_state(void __iomem *power, int cpu)
+{
+    return readl(power + EXYNOS_ARM_CORE_STATUS(cpu)) &
+                  S5P_CORE_LOCAL_PWR_EN;
+}
+
+static void exynos_cpu_set_power_up(void __iomem *power, int cpu)
+{
+    writel(S5P_CORE_LOCAL_PWR_EN,
+        power + EXYNOS_ARM_CORE_CONFIG(cpu));
+}
+
+static int exynos_cpu_power_up(void __iomem *power, int cpu)
+{
+    int timeout;
+
+    if ( !exynos_cpu_power_state(power, cpu) ) {
+        exynos_cpu_set_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_cpu_up(int cpu)
+{
+    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"),
+        DT_MATCH_COMPATIBLE("samsung,exynos5422-pmu"),
+        { /*sentinel*/ },
+    };
+    void __iomem *power;
+    u64 power_base_addr = 0;
+    u64 size;
+    struct dt_device_node *node;
+    int rc;
+    
+    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_INFO, "power_base_addr: %08x size: %08x\n",
+            (unsigned int) power_base_addr, (unsigned int) size);
+
+    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 = exynos_cpu_power_up(power, cpu);
+    if ( rc ) {
+        return -ETIMEDOUT;
+    }
+
+    iounmap(power);
+
+    return cpu_up_send_sgi(cpu);
+}
+
 static void exynos5_reset(void)
 {
     void __iomem *pmu;
@@ -101,12 +227,6 @@ static void exynos5_reset(void)
     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 +237,33 @@ static const struct dt_device_match exynos5_blacklist_dev[] __initconst =
     { /* sentinel */ },
 };
 
-PLATFORM_START(exynos5, "SAMSUNG EXYNOS5")
-    .compatible = exynos5_dt_compat,
+static const char * const exynos5250_dt_compat[] __initconst =
+{
+    "samsung,exynos5250",
+    NULL
+};
+
+static const char * const exynos5410_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 = exynos5250_smp_init,
+    .cpu_up = exynos5250_cpu_up,
+    .reset = exynos5_reset,
+    .blacklist_dev = exynos5_blacklist_dev,
+PLATFORM_END
+
+PLATFORM_START(exynos5410, "SAMSUNG EXYNOS5410")
+    .compatible = exynos5410_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
-- 
1.9.1

Re: [Patch v3 1/1] Add Odroid-XU (Exynos5410)

[Julien Grall]

Hello Suriyan,

On 13/08/14 22:12, Suriyan Ramasami wrote:
> -static int __init exynos5_smp_init(void)
> +static int __init exynos5250_cpu_up(int cpu)
> +{
> +    return cpu_up_send_sgi(cpu);
> +}
> +

This is not necessary. You can directly assign cpu_up_send_sgi to the 
cpu_up callback of the platform structure.

> +static int __init exynos_smp_init(unsigned long pa_sysram)

paddr_t pa_sysram

>   {
>       void __iomem *sysram;
>
> -    sysram = ioremap_nocache(S5P_PA_SYSRAM, PAGE_SIZE);
> +    sysram = ioremap_nocache(pa_sysram, PAGE_SIZE);
>       if ( !sysram )
>       {
>           dprintk(XENLOG_ERR, "Unable to map exynos5 MMIO\n");
> @@ -84,6 +95,121 @@ static int __init exynos5_smp_init(void)
>       return 0;
>   }
>
> +static int __init exynos5250_smp_init(void)
> +{
> +    return exynos_smp_init(S5P_PA_SYSRAM);
> +}

I would rename S5P_PA_SYSRAM and EXYNOS5_* to something specific to 
exynos 5250. It would make clear that new platform should use the device 
tree to get information.

> +static int __init exynos5_smp_init(void)
> +{
> +    struct dt_device_node *node;
> +    u64 sysram_ns_base_addr = 0;
> +    u64 size;
> +    int rc;
> +
> +    node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-sysram-ns");
> +    if ( !node ) {

Coding style:

if ( !node )
{

> +	dprintk(XENLOG_ERR, "samsung,exynos4210-sysram-ns missing in DT\n");

The coding style doesn't allow to use hard tab.

> +        return -ENXIO;
> +    }
> +
> +    rc = dt_device_get_address(node, 0, &sysram_ns_base_addr, &size);
> +    if (rc) {

Coding style:

if ( rc )
{

> +        dprintk(XENLOG_ERR, "Error in \"samsung,exynos4210-sysram-ns\"\n");
> +        return -ENXIO;
> +    }
> +
> +    dprintk(XENLOG_INFO, "sysram_ns_base_addr: %08x size: %08x\n",
> +            (unsigned int) sysram_ns_base_addr, (unsigned int) size);

Why do you cast to unsigned int rather than right printf format?

> +
> +    return exynos_smp_init(sysram_ns_base_addr + 0x1c);
> +}
> +
> +static int exynos_cpu_power_state(void __iomem *power, int cpu)
> +{
> +    return readl(power + EXYNOS_ARM_CORE_STATUS(cpu)) &
> +                  S5P_CORE_LOCAL_PWR_EN;

Please correctly align the second line to the open branch. I.e:

return readl(power ....
              S5P_CORE_...

Also, why did you replace the __raw_readl by a readl? The former one 
doesn't use barrier while the latter does.

As Linux is using the former one, I don't understand why we would 
require barrier on Xen.

> +}
> +
> +static void exynos_cpu_set_power_up(void __iomem *power, int cpu)
> +{
> +    writel(S5P_CORE_LOCAL_PWR_EN,
> +        power + EXYNOS_ARM_CORE_CONFIG(cpu));

Same here for both coding style and writel.

> +}
> +
> +static int exynos_cpu_power_up(void __iomem *power, int cpu)
> +{
> +    int timeout;

unsigned int.

> +
> +    if ( !exynos_cpu_power_state(power, cpu) ) {

Coding style:

if ( ... )
{

> +        exynos_cpu_set_power_up(power, cpu);

I would keep the same name as Linux ie exynos_cpu_power_up to avoid 
confusion and make backporting easier.

> +        timeout = 10;
> +
> +        /* wait max 10 ms until cpu is on */
> +        while (exynos_cpu_power_state(power, cpu) != S5P_CORE_LOCAL_PWR_EN) {

Coding style:

while ( ... )
{

> +            if (timeout-- == 0)
> +                break;

Coding style:

if ( ... )

> +
> +            mdelay(1);
> +        }
> +
> +        if (timeout == 0) {

Coding style:

if ( ... )
{

> +            dprintk(XENLOG_ERR, "CPU%d power enable failed", cpu);
> +            return -ETIMEDOUT;
> +        }
> +    }
> +    return 0;
> +}
> +
> +static int exynos5_cpu_up(int cpu)
> +{
> +    static const struct dt_device_match exynos_dt_pmu_matches[] __initconst =
> +    {

[..]

> +        DT_MATCH_COMPATIBLE("samsung,exynos5422-pmu"),

Where does this compatible come from? I don't find any reference in 
Linux upstream documentation 
(Documentation/devicetree/bindings/arm/samsung/pmu.txt).

> +        { /*sentinel*/ },
> +    };
> +    void __iomem *power;
> +    u64 power_base_addr = 0;
> +    u64 size;
> +    struct dt_device_node *node;
> +    int rc;
> +
> +    node = dt_find_matching_node(NULL, exynos_dt_pmu_matches);
> +    if ( !node ) {

Coding style:

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 ) {

Coding style
if ( rc )
{

> +	    dprintk(XENLOG_ERR, "Error in \"samsung,exynos5XXXX-pmu\"\n");
> +            return -ENXIO;
> +    }
> +
> +    dprintk(XENLOG_INFO, "power_base_addr: %08x size: %08x\n",

I would use XENLOG_DEBUG

> +            (unsigned int) power_base_addr, (unsigned int) size);
> +
> +    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 = exynos_cpu_power_up(power, cpu);
> +    if ( rc ) {
> +        return -ETIMEDOUT;

If exynos_cpu_power_up is failing you never unmap the power region.

> +PLATFORM_START(exynos5410, "SAMSUNG EXYNOS5410")

Nothing looks 5410 specific in structure. I would rename it to exynos5, 
"SAMSUNG EXYNOS5". So if someone want to add a new exynos5 (such as the 
octa) it wouldn't have to rename this platform.

Regards,

-- 
Julien Grall

Re: [Patch v3 1/1] Add Odroid-XU (Exynos5410)

[Suriyan Ramasami]

Thank you Julien for your review, much appreciated!

On Mon, Aug 18, 2014 at 12:07 PM, Julien Grall <julien.grall@linaro.org> wrote:
> Hello Suriyan,
>
>
> On 13/08/14 22:12, Suriyan Ramasami wrote:
>>
>> -static int __init exynos5_smp_init(void)
>> +static int __init exynos5250_cpu_up(int cpu)
>> +{
>> +    return cpu_up_send_sgi(cpu);
>> +}
>> +
>
>
> This is not necessary. You can directly assign cpu_up_send_sgi to the cpu_up
> callback of the platform structure.
>
I shall do that.

>
>> +static int __init exynos_smp_init(unsigned long pa_sysram)
>
>
> paddr_t pa_sysram
>
>
>>   {
>>       void __iomem *sysram;
>>
>> -    sysram = ioremap_nocache(S5P_PA_SYSRAM, PAGE_SIZE);
>> +    sysram = ioremap_nocache(pa_sysram, PAGE_SIZE);
>>       if ( !sysram )
>>       {
>>           dprintk(XENLOG_ERR, "Unable to map exynos5 MMIO\n");
>> @@ -84,6 +95,121 @@ static int __init exynos5_smp_init(void)
>>       return 0;
>>   }
>>
>> +static int __init exynos5250_smp_init(void)
>> +{
>> +    return exynos_smp_init(S5P_PA_SYSRAM);
>> +}
>
>
> I would rename S5P_PA_SYSRAM and EXYNOS5_* to something specific to exynos
> 5250. It would make clear that new platform should use the device tree to
> get information.
>
I shall rename S5P_PA_SYSRAM to EXYNOS5250_PA_SYSRAM.
Do you also want me to rename the other ones, like EXYNOS5_MCT_*
defines in the exynos5.h header file as well?
If so, do you want me to get rid of exynos5.h and dump all the defines
in exynos5.c ?

>
>> +static int __init exynos5_smp_init(void)
>> +{
>> +    struct dt_device_node *node;
>> +    u64 sysram_ns_base_addr = 0;
>> +    u64 size;
>> +    int rc;
>> +
>> +    node = dt_find_compatible_node(NULL, NULL,
>> "samsung,exynos4210-sysram-ns");
>> +    if ( !node ) {
>
>
> Coding style:
>
>
> if ( !node )
> {
>
I shall correct this.

>> +       dprintk(XENLOG_ERR, "samsung,exynos4210-sysram-ns missing in
>> DT\n");
>
>
> The coding style doesn't allow to use hard tab.
>
>
I shall correct this too.

>> +        return -ENXIO;
>> +    }
>> +
>> +    rc = dt_device_get_address(node, 0, &sysram_ns_base_addr, &size);
>> +    if (rc) {
>
>
> Coding style:
>
> if ( rc )
> {
>
and this one as well.

>> +        dprintk(XENLOG_ERR, "Error in
>> \"samsung,exynos4210-sysram-ns\"\n");
>>
>> +        return -ENXIO;
>> +    }
>> +
>> +    dprintk(XENLOG_INFO, "sysram_ns_base_addr: %08x size: %08x\n",
>> +            (unsigned int) sysram_ns_base_addr, (unsigned int) size);
>
>
> Why do you cast to unsigned int rather than right printf format?
>
Cause I am an idiot :-) I shall correct this to use %016llx.
>
>> +
>> +    return exynos_smp_init(sysram_ns_base_addr + 0x1c);
>> +}
>> +
>> +static int exynos_cpu_power_state(void __iomem *power, int cpu)
>> +{
>> +    return readl(power + EXYNOS_ARM_CORE_STATUS(cpu)) &
>> +                  S5P_CORE_LOCAL_PWR_EN;
>
>
> Please correctly align the second line to the open branch. I.e:
>
> return readl(power ....
>              S5P_CORE_...
>
I shall do so.

> Also, why did you replace the __raw_readl by a readl? The former one doesn't
> use barrier while the latter does.
>
> As Linux is using the former one, I don't understand why we would require
> barrier on Xen.
>
I shall use __raw_readl()
>
>> +}
>> +
>> +static void exynos_cpu_set_power_up(void __iomem *power, int cpu)
>> +{
>> +    writel(S5P_CORE_LOCAL_PWR_EN,
>> +        power + EXYNOS_ARM_CORE_CONFIG(cpu));
>
>
> Same here for both coding style and writel.
>
Shall correct them both.

>
>> +}
>> +
>> +static int exynos_cpu_power_up(void __iomem *power, int cpu)
>> +{
>> +    int timeout;
>
>
> unsigned int.
>
>
I shall correct this one too.

>> +
>> +    if ( !exynos_cpu_power_state(power, cpu) ) {
>
>
> Coding style:
>
> if ( ... )
> {
>
This one as well.

>> +        exynos_cpu_set_power_up(power, cpu);
>
>
> I would keep the same name as Linux ie exynos_cpu_power_up to avoid
> confusion and make backporting easier.
>
OK, I shall make it exynos_cpu_power_up() like in linux, and the
caller exynos5_cpu_powerup().

>
>> +        timeout = 10;
>> +
>> +        /* wait max 10 ms until cpu is on */
>> +        while (exynos_cpu_power_state(power, cpu) !=
>> S5P_CORE_LOCAL_PWR_EN) {
>
>
> Coding style:
>
> while ( ... )
>
> {
>
Shall correct this.

>> +            if (timeout-- == 0)
>> +                break;
>
>
> Coding style:
>
> if ( ... )
>
>
>> +
>> +            mdelay(1);
>> +        }
>> +
>> +        if (timeout == 0) {
>
>
> Coding style:
>
> if ( ... )
>
> {
>
Sorry about so many coding style errors. Shall correct this.

>> +            dprintk(XENLOG_ERR, "CPU%d power enable failed", cpu);
>> +            return -ETIMEDOUT;
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>> +static int exynos5_cpu_up(int cpu)
>> +{
>> +    static const struct dt_device_match exynos_dt_pmu_matches[]
>> __initconst =
>> +    {
>
>
> [..]
>
>> +        DT_MATCH_COMPATIBLE("samsung,exynos5422-pmu"),
>
>
> Where does this compatible come from? I don't find any reference in Linux
> upstream documentation
> (Documentation/devicetree/bindings/arm/samsung/pmu.txt).
>
This was my wrong assumption. I wanted to add odroid xu3's pmu in
this, but as you pointed out there is no exynos5422-pmu. Odroid XU3
uses the "exysno5420-pmu" compatibility strings. I shall remove this
line.

>
>> +        { /*sentinel*/ },
>> +    };
>> +    void __iomem *power;
>> +    u64 power_base_addr = 0;
>> +    u64 size;
>> +    struct dt_device_node *node;
>> +    int rc;
>> +
>> +    node = dt_find_matching_node(NULL, exynos_dt_pmu_matches);
>> +    if ( !node ) {
>
>
> Coding style:
>
>
> if ( !node )
> {
>
Shall correct it.

>> +       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 ) {
>
>
> Coding style
>
> if ( rc )
> {
>
Shall correct this one too.

>> +           dprintk(XENLOG_ERR, "Error in \"samsung,exynos5XXXX-pmu\"\n");
>> +            return -ENXIO;
>> +    }
>> +
>> +    dprintk(XENLOG_INFO, "power_base_addr: %08x size: %08x\n",
>
>
> I would use XENLOG_DEBUG
>
>
Shall change it.

>> +            (unsigned int) power_base_addr, (unsigned int) size);
>> +
>> +    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 = exynos_cpu_power_up(power, cpu);
>> +    if ( rc ) {
>> +        return -ETIMEDOUT;
>
>
> If exynos_cpu_power_up is failing you never unmap the power region.
>
I shall add a iounmap(power) on failure.

>> +PLATFORM_START(exynos5410, "SAMSUNG EXYNOS5410")
>
>
> Nothing looks 5410 specific in structure. I would rename it to exynos5,
> "SAMSUNG EXYNOS5". So if someone want to add a new exynos5 (such as the
> octa) it wouldn't have to rename this platform.
>
Shall rename it

> Regards,
>
> --
> Julien Grall

Re: [Patch v3 1/1] Add Odroid-XU (Exynos5410)

[Julien Grall]

Hello,

On 18/08/14 17:21, Suriyan Ramasami wrote:
>> I would rename S5P_PA_SYSRAM and EXYNOS5_* to something specific to exynos
>> 5250. It would make clear that new platform should use the device tree to
>> get information.
>>
> I shall rename S5P_PA_SYSRAM to EXYNOS5250_PA_SYSRAM.
> Do you also want me to rename the other ones, like EXYNOS5_MCT_*
> defines in the exynos5.h header file as well?

You are using EXYNOS5_MCT_* on both board so I don't think you have to 
rename it. The best things would be using the device tree rather than an 
hardcoded value.

But it's not the scope of this patch :).

> If so, do you want me to get rid of exynos5.h and dump all the defines
> in exynos5.c ?

It would be nice but not necessary for this patch.

Regards,

-- 
Julien Grall