| Message ID | 1376406787-23771-1-git-send-email-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
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.
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.
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: > + */
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: >> + */ >
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
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.
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,
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.
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: + */
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