[PATCH] ARM: add Calxeda Midway platform

[PATCH] ARM: add Calxeda Midway platform

[Andre Przywara]

Calxeda Midway is an ARMv7 server platform with Cortex-A15 cores.
The peripheral side has many similarities with the machine known as
Highbank.
Add Calxeda Midway to the list of supported platforms to avoid a
warning on boot and provide the proper reset method.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/platforms/Makefile        |  1 +
 xen/arch/arm/platforms/midway.c        | 62 ++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/platforms/midway.h | 21 ++++++++++++
 3 files changed, 84 insertions(+)
 create mode 100644 xen/arch/arm/platforms/midway.c
 create mode 100644 xen/include/asm-arm/platforms/midway.h

diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index ff2b65b..6ee8e6a 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -1,2 +1,3 @@
 obj-y += vexpress.o
 obj-y += exynos5.o
+obj-y += midway.o
diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
new file mode 100644
index 0000000..2d3be1b
--- /dev/null
+++ b/xen/arch/arm/platforms/midway.c
@@ -0,0 +1,62 @@
+/*
+ * xen/arch/arm/platforms/midway.c
+ *
+ * Calxeda Midway specific settings
+ *
+ * Andre Przywara <andre.przywara@linaro.org>
+ * Copyright (c) 2013 Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/mm.h>
+#include <xen/vmap.h>
+#include <asm/platforms/midway.h>
+#include <asm/platform.h>
+
+static void midway_reset(void)
+{
+    void __iomem *pmu;
+
+    BUILD_BUG_ON((MW_SREG_PWR_REQ & PAGE_MASK) !=
+                 (MW_SREG_A15_PWR_CTRL & PAGE_MASK));
+
+    pmu = ioremap_nocache(MW_SREG_PWR_REQ & PAGE_MASK, PAGE_SIZE);
+    if ( !pmu )
+    {
+        dprintk(XENLOG_ERR, "Unable to map PMU\n");
+        return;
+    }
+
+    iowritel(pmu + (MW_SREG_PWR_REQ & ~PAGE_MASK), MW_PWR_HARD_RESET);
+    iowritel(pmu + (MW_SREG_A15_PWR_CTRL & ~PAGE_MASK), 1);
+    iounmap(pmu);
+}
+
+static const char const *midway_dt_compat[] __initdata =
+{
+    "calxeda,ecx-2000",
+    NULL
+};
+
+PLATFORM_START(midway, "CALXEDA MIDWAY")
+    .compatible = midway_dt_compat,
+    .reset = midway_reset,
+PLATFORM_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/platforms/midway.h b/xen/include/asm-arm/platforms/midway.h
new file mode 100644
index 0000000..099e435
--- /dev/null
+++ b/xen/include/asm-arm/platforms/midway.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_PLATFORMS_MIDWAY_H
+#define __ASM_ASM_PLATFORMS_MIDWAY_H
+
+/* addresses of SREG registers for resetting the SoC */
+#define MW_SREG_PWR_REQ             0xfff3cf00
+#define MW_SREG_A15_PWR_CTRL        0xfff3c200
+
+#define MW_PWR_SUSPEND              0
+#define MW_PWR_SOFT_RESET           1
+#define MW_PWR_HARD_RESET           2
+#define MW_PWR_SHUTDOWN             3
+
+#endif /* __ASM_ARM_PLATFORMS_MIDWAY_H */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.7.12.1

Re: [PATCH] ARM: add Calxeda Midway platform

[Ian Campbell]

On Tue, 2013-08-13 at 17:13 +0200, Andre Przywara wrote:
> avoid a warning on boot and provide the proper reset method.

I've been wondering about taming that warning down to something less
alarming like "Using generic platform fallback for platform
$some_dt_name", or something like that. There are likely (going to be)
many platforms where no specific code is really needed and the big
WARNING is needlessly scary IMHO.

Ian.

Re: [PATCH] ARM: add Calxeda Midway platform

[Julien Grall]

On 13 August 2013 23:02, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Tue, 2013-08-13 at 17:13 +0200, Andre Przywara wrote:
>> avoid a warning on boot and provide the proper reset method.
>
> I've been wondering about taming that warning down to something less
> alarming like "Using generic platform fallback for platform
> $some_dt_name", or something like that. There are likely (going to be)
> many platforms where no specific code is really needed and the big
> WARNING is needlessly scary IMHO.

All the platforms need at least the reset callback.
So, I think this WARNING should stay to let the developper know that
something is missing.

-- 
Julien Grall

Re: [PATCH] ARM: add Calxeda Midway platform

[Ian Campbell]

On Wed, 2013-08-14 at 10:33 +0100, Julien Grall wrote:
> On 13 August 2013 23:02, Ian Campbell <ian.campbell@citrix.com> wrote:
> > On Tue, 2013-08-13 at 17:13 +0200, Andre Przywara wrote:
> >> avoid a warning on boot and provide the proper reset method.
> >
> > I've been wondering about taming that warning down to something less
> > alarming like "Using generic platform fallback for platform
> > $some_dt_name", or something like that. There are likely (going to be)
> > many platforms where no specific code is really needed and the big
> > WARNING is needlessly scary IMHO.
> 
> All the platforms need at least the reset callback.

Doesn't PSCI eventually cover this?

> So, I think this WARNING should stay to let the developper know that
> something is missing.

A warning if the reset hook was NULL when Xen tries to call it would
serve much the same purpose without worrying people needlessly during
boot.

Ian

Re: [PATCH] ARM: add Calxeda Midway platform

[Julien Grall]

On 13 August 2013 16:13, Andre Przywara <andre.przywara@linaro.org> wrote:
> Calxeda Midway is an ARMv7 server platform with Cortex-A15 cores.
> The peripheral side has many similarities with the machine known as
> Highbank.
> Add Calxeda Midway to the list of supported platforms to avoid a
> warning on boot and provide the proper reset method.

Thanks for this patch. With the 2 others patches, are you able to boot
further Xen on the board? :)

>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/arch/arm/platforms/Makefile        |  1 +
>  xen/arch/arm/platforms/midway.c        | 62 ++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/platforms/midway.h | 21 ++++++++++++
>  3 files changed, 84 insertions(+)
>  create mode 100644 xen/arch/arm/platforms/midway.c
>  create mode 100644 xen/include/asm-arm/platforms/midway.h
>
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index ff2b65b..6ee8e6a 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -1,2 +1,3 @@
>  obj-y += vexpress.o
>  obj-y += exynos5.o
> +obj-y += midway.o
> diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
> new file mode 100644
> index 0000000..2d3be1b
> --- /dev/null
> +++ b/xen/arch/arm/platforms/midway.c
> @@ -0,0 +1,62 @@
> +/*
> + * xen/arch/arm/platforms/midway.c
> + *
> + * Calxeda Midway specific settings
> + *
> + * Andre Przywara <andre.przywara@linaro.org>
> + * Copyright (c) 2013 Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/mm.h>
> +#include <xen/vmap.h>
> +#include <asm/platforms/midway.h>
> +#include <asm/platform.h>
> +
> +static void midway_reset(void)
> +{
> +    void __iomem *pmu;
> +
> +    BUILD_BUG_ON((MW_SREG_PWR_REQ & PAGE_MASK) !=
> +                 (MW_SREG_A15_PWR_CTRL & PAGE_MASK));
> +
> +    pmu = ioremap_nocache(MW_SREG_PWR_REQ & PAGE_MASK, PAGE_SIZE);
> +    if ( !pmu )
> +    {
> +        dprintk(XENLOG_ERR, "Unable to map PMU\n");
> +        return;
> +    }
> +
> +    iowritel(pmu + (MW_SREG_PWR_REQ & ~PAGE_MASK), MW_PWR_HARD_RESET);
> +    iowritel(pmu + (MW_SREG_A15_PWR_CTRL & ~PAGE_MASK), 1);
> +    iounmap(pmu);
> +}
> +
> +static const char const *midway_dt_compat[] __initdata =

const char * const. This is an error on the other platform code.

> +{
> +    "calxeda,ecx-2000",
> +    NULL
> +};
> +
> +PLATFORM_START(midway, "CALXEDA MIDWAY")
> +    .compatible = midway_dt_compat,
> +    .reset = midway_reset,
> +PLATFORM_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/platforms/midway.h b/xen/include/asm-arm/platforms/midway.h
> new file mode 100644
> index 0000000..099e435
> --- /dev/null
> +++ b/xen/include/asm-arm/platforms/midway.h
> @@ -0,0 +1,21 @@
> +#ifndef __ASM_ARM_PLATFORMS_MIDWAY_H
> +#define __ASM_ASM_PLATFORMS_MIDWAY_H
> +
> +/* addresses of SREG registers for resetting the SoC */
> +#define MW_SREG_PWR_REQ             0xfff3cf00
> +#define MW_SREG_A15_PWR_CTRL        0xfff3c200
> +
> +#define MW_PWR_SUSPEND              0
> +#define MW_PWR_SOFT_RESET           1
> +#define MW_PWR_HARD_RESET           2
> +#define MW_PWR_SHUTDOWN             3
> +
> +#endif /* __ASM_ARM_PLATFORMS_MIDWAY_H */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */

-- 
Julien Grall

Re: [PATCH] ARM: add Calxeda Midway platform

[Andre Przywara]

On 08/14/2013 11:44 AM, Julien Grall wrote:
> On 13 August 2013 16:13, Andre Przywara <andre.przywara@linaro.org> wrote:
>> Calxeda Midway is an ARMv7 server platform with Cortex-A15 cores.
>> The peripheral side has many similarities with the machine known as
>> Highbank.
>> Add Calxeda Midway to the list of supported platforms to avoid a
>> warning on boot and provide the proper reset method.
>
> Thanks for this patch. With the 2 others patches, are you able to boot
> further Xen on the board? :)

Well, I get to the point where it loads and starts Dom0. Then it breaks 
with a Data Abort, will investigate this next.

If there are no other complaints, I will send a v2 with the fix for 
below later today.

Regards,
Andre.

>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   xen/arch/arm/platforms/Makefile        |  1 +
>>   xen/arch/arm/platforms/midway.c        | 62 ++++++++++++++++++++++++++++++++++
>>   xen/include/asm-arm/platforms/midway.h | 21 ++++++++++++
>>   3 files changed, 84 insertions(+)
>>   create mode 100644 xen/arch/arm/platforms/midway.c
>>   create mode 100644 xen/include/asm-arm/platforms/midway.h
>>
>> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
>> index ff2b65b..6ee8e6a 100644
>> --- a/xen/arch/arm/platforms/Makefile
>> +++ b/xen/arch/arm/platforms/Makefile
>> @@ -1,2 +1,3 @@
>>   obj-y += vexpress.o
>>   obj-y += exynos5.o
>> +obj-y += midway.o
>> diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
>> new file mode 100644
>> index 0000000..2d3be1b
>> --- /dev/null
>> +++ b/xen/arch/arm/platforms/midway.c
>> @@ -0,0 +1,62 @@
>> +/*
>> + * xen/arch/arm/platforms/midway.c
>> + *
>> + * Calxeda Midway specific settings
>> + *
>> + * Andre Przywara <andre.przywara@linaro.org>
>> + * Copyright (c) 2013 Linaro Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <xen/mm.h>
>> +#include <xen/vmap.h>
>> +#include <asm/platforms/midway.h>
>> +#include <asm/platform.h>
>> +
>> +static void midway_reset(void)
>> +{
>> +    void __iomem *pmu;
>> +
>> +    BUILD_BUG_ON((MW_SREG_PWR_REQ & PAGE_MASK) !=
>> +                 (MW_SREG_A15_PWR_CTRL & PAGE_MASK));
>> +
>> +    pmu = ioremap_nocache(MW_SREG_PWR_REQ & PAGE_MASK, PAGE_SIZE);
>> +    if ( !pmu )
>> +    {
>> +        dprintk(XENLOG_ERR, "Unable to map PMU\n");
>> +        return;
>> +    }
>> +
>> +    iowritel(pmu + (MW_SREG_PWR_REQ & ~PAGE_MASK), MW_PWR_HARD_RESET);
>> +    iowritel(pmu + (MW_SREG_A15_PWR_CTRL & ~PAGE_MASK), 1);
>> +    iounmap(pmu);
>> +}
>> +
>> +static const char const *midway_dt_compat[] __initdata =
>
> const char * const. This is an error on the other platform code.

>
>> +{
>> +    "calxeda,ecx-2000",
>> +    NULL
>> +};
>> +
>> +PLATFORM_START(midway, "CALXEDA MIDWAY")
>> +    .compatible = midway_dt_compat,
>> +    .reset = midway_reset,
>> +PLATFORM_END
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/include/asm-arm/platforms/midway.h b/xen/include/asm-arm/platforms/midway.h
>> new file mode 100644
>> index 0000000..099e435
>> --- /dev/null
>> +++ b/xen/include/asm-arm/platforms/midway.h
>> @@ -0,0 +1,21 @@
>> +#ifndef __ASM_ARM_PLATFORMS_MIDWAY_H
>> +#define __ASM_ASM_PLATFORMS_MIDWAY_H
>> +
>> +/* addresses of SREG registers for resetting the SoC */
>> +#define MW_SREG_PWR_REQ             0xfff3cf00
>> +#define MW_SREG_A15_PWR_CTRL        0xfff3c200
>> +
>> +#define MW_PWR_SUSPEND              0
>> +#define MW_PWR_SOFT_RESET           1
>> +#define MW_PWR_HARD_RESET           2
>> +#define MW_PWR_SHUTDOWN             3
>> +
>> +#endif /* __ASM_ARM_PLATFORMS_MIDWAY_H */
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>

Re: [PATCH] ARM: add Calxeda Midway platform

[Ian Campbell]

On Tue, 2013-08-13 at 17:13 +0200, Andre Przywara wrote:
> Calxeda Midway is an ARMv7 server platform with Cortex-A15 cores.
> The peripheral side has many similarities with the machine known as
> Highbank.
> Add Calxeda Midway to the list of supported platforms to avoid a
> warning on boot and provide the proper reset method.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

acked + applied, thanks.

Re: [PATCH] ARM: add Calxeda Midway platform

[Julien Grall]

On 20 August 2013 16:03, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-08-13 at 17:13 +0200, Andre Przywara wrote:
>> Calxeda Midway is an ARMv7 server platform with Cortex-A15 cores.
>> The peripheral side has many similarities with the machine known as
>> Highbank.
>> Add Calxeda Midway to the list of supported platforms to avoid a
>> warning on boot and provide the proper reset method.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>
> acked + applied, thanks.
>
>

Few mails before, I asked Andre to change one thing in his patch.
He planned to send a new version later.
Cheers,

-- 
Julien Grall

Re: [PATCH] ARM: add Calxeda Midway platform

[Ian Campbell]

On Tue, 2013-08-20 at 20:01 +0100, Julien Grall wrote:
> On 20 August 2013 16:03, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2013-08-13 at 17:13 +0200, Andre Przywara wrote:
> >> Calxeda Midway is an ARMv7 server platform with Cortex-A15 cores.
> >> The peripheral side has many similarities with the machine known as
> >> Highbank.
> >> Add Calxeda Midway to the list of supported platforms to avoid a
> >> warning on boot and provide the proper reset method.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> >
> > acked + applied, thanks.
> >
> >
> 
> Few mails before, I asked Andre to change one thing in his patch.
> He planned to send a new version later.

Sorry, I skimmed the thread but missed your request for a change.
(trimming quotes would be a massive help here)

Andre, can you send an incremental patch please?

Julien suggested replacing "static const char const *" with "const char
* const", was the lack of "static" in the replacement deliberate?

Julien suggested that the other platform code was similarly broken,
could fix that at the same time too?

Ian.