| Message ID | 1379670763-14895-2-git-send-email-pranavkumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 09/20/2013 10:52 AM, Pranavkumar Sawargaonkar wrote: > This patch adds following things: > Early printk support for APM X-Gene platform. Does the UART is compatible with 8250? If so, can you rename the your debug-xgene-storm.inc in debug-8250.inc? Future ARM64 server with 8250-compatible UART will be able to use the same code. Also, can you divide this patch in 2 part: - Early printk support; - Initial platform support. > Initial platform stubs for APM X-Gene. > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > --- > xen/arch/arm/Rules.mk | 5 +++ > xen/arch/arm/arm64/debug-xgene-storm.inc | 55 ++++++++++++++++++++++++++++++ > xen/arch/arm/platforms/Makefile | 1 + > xen/arch/arm/platforms/xgene-storm.c | 57 ++++++++++++++++++++++++++++++++ > 4 files changed, 118 insertions(+) > create mode 100644 xen/arch/arm/arm64/debug-xgene-storm.inc > create mode 100644 xen/arch/arm/platforms/xgene-storm.c > > diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk > index bd79b26..b36120e 100644 > --- a/xen/arch/arm/Rules.mk > +++ b/xen/arch/arm/Rules.mk > @@ -77,6 +77,11 @@ EARLY_PRINTK_INC := 8250 > EARLY_UART_BASE_ADDRESS := 0x01c28000 > EARLY_UART_REG_SHIFT := 2 > endif > +ifeq ($(CONFIG_EARLY_PRINTK), xgene-storm) > +EARLY_PRINTK_INC := xgene-storm > +EARLY_PRINTK_BAUD := 115200 > +EARLY_UART_BASE_ADDRESS := 0x1c020000 > +endif > > ifneq ($(EARLY_PRINTK_INC),) > EARLY_PRINTK := y > diff --git a/xen/arch/arm/arm64/debug-xgene-storm.inc b/xen/arch/arm/arm64/debug-xgene-storm.inc > new file mode 100644 > index 0000000..ebbb8d2 > --- /dev/null > +++ b/xen/arch/arm/arm64/debug-xgene-storm.inc > @@ -0,0 +1,55 @@ > +/* > + * xen/arch/arm/arm64/debug-xgene-storm.inc > + * > + * X-Gene Storm specific debug code > + * > + * Copyright (c) 2013 Applied Micro. > + * > + * 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/8250-uart.h> > + > +#define XGENE_STORM_UART_LSR 0x14 > +#define XGENE_STORM_UART_THR 0x00 I guess these defines contains the shift (=4), right? To be more generic, you should allow a different shift via EARLY_UART_REG_SHIFT (see arm32/debug-8250.inc) and use this define. > + > +/* UART initialization > + * rb: register which contains the UART base address > + * rc: scratch register 1 > + * rd: scratch register 2 */ > +.macro early_uart_init rb rc rd > +.endm early_uart_init is only needed if EARLY_PRINTK_INIT_UART is defined. In your case it's not defined, so you can remove this macro. > + > +/* UART wait UART to be ready to transmit > + * xb: register which contains the UART base address > + * c: scratch register */ > +.macro early_uart_ready xb c > +1: > + ldrb w\c, [\xb, #XGENE_STORM_UART_LSR] > + and w\c, w\c, #UART_LSR_THRE > + cmp w\c, #UART_LSR_THRE > + b.ne 1b > +.endm > + > +/* UART transmit character > + * xb: register which contains the UART base address > + * wt: register which contains the character to transmit */ > +.macro early_uart_transmit xb wt > + /* UART_THR transmit holding */ > + strb \wt, [\xb, #XGENE_STORM_UART_THR] > +.endm > + > +/* > + * Local variables: > + * mode: ASM > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile > index 4aa82e8..8a6709f 100644 > --- a/xen/arch/arm/platforms/Makefile > +++ b/xen/arch/arm/platforms/Makefile > @@ -2,3 +2,4 @@ obj-y += vexpress.o > obj-y += exynos5.o > obj-y += midway.o > obj-y += omap5.o > +obj-y += xgene-storm.o Shouldn't it be only enabled on ARM64? > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > new file mode 100644 > index 0000000..8e2b3b6 > --- /dev/null > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -0,0 +1,57 @@ > +/* > + * xen/arch/arm/platforms/xgene-storm.c > + * > + * Applied Micro's X-Gene specific settings > + * > + * Pranavkumar Sawargaonkar <psawargaonkar@apm.com> > + * Anup Patel <apatel@apm.com> > + * Copyright (c) 2013 Applied Micro. > + * > + * 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/config.h> > +#include <xen/device_tree.h> > +#include <xen/domain_page.h> > +#include <xen/mm.h> > +#include <xen/vmap.h> > +#include <asm/platform.h> > +#include <asm/early_printk.h> > + > +static void xgene_storm_reset(void) > +{ > +} > + > +static int xgene_storm_init(void) > +{ > + return 0; > +} You don't need these empty functions, the platform code copes with empty callback. > +static const char const *xgene_storm_dt_compat[] __initdata = > +{ > + "apm,xgene-storm", > + NULL > +}; > + > +PLATFORM_START(xgene_storm, "APM X-GENE STORM") > + .compatible = xgene_storm_dt_compat, > + .init = xgene_storm_init, > + .reset = xgene_storm_reset, > +PLATFORM_END > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ >
HI Julien, On Fri, Sep 20, 2013 at 5:39 PM, Julien Grall <julien.grall@linaro.org> wrote: > On 09/20/2013 10:52 AM, Pranavkumar Sawargaonkar wrote: >> This patch adds following things: >> Early printk support for APM X-Gene platform. > > Does the UART is compatible with 8250? Yes it is compatible but it is ns16550. > > If so, can you rename the your debug-xgene-storm.inc in debug-8250.inc? > Future ARM64 server with 8250-compatible UART will be able to use the > same code. I think better name would be debug-ns16550.inc ?? > > Also, can you divide this patch in 2 part: > - Early printk support; > - Initial platform support. > Sure i will do it. >> Initial platform stubs for APM X-Gene. >> >> Signed-off-by: Anup Patel <anup.patel@linaro.org> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >> --- >> xen/arch/arm/Rules.mk | 5 +++ >> xen/arch/arm/arm64/debug-xgene-storm.inc | 55 ++++++++++++++++++++++++++++++ >> xen/arch/arm/platforms/Makefile | 1 + >> xen/arch/arm/platforms/xgene-storm.c | 57 ++++++++++++++++++++++++++++++++ >> 4 files changed, 118 insertions(+) >> create mode 100644 xen/arch/arm/arm64/debug-xgene-storm.inc >> create mode 100644 xen/arch/arm/platforms/xgene-storm.c >> >> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk >> index bd79b26..b36120e 100644 >> --- a/xen/arch/arm/Rules.mk >> +++ b/xen/arch/arm/Rules.mk >> @@ -77,6 +77,11 @@ EARLY_PRINTK_INC := 8250 >> EARLY_UART_BASE_ADDRESS := 0x01c28000 >> EARLY_UART_REG_SHIFT := 2 >> endif >> +ifeq ($(CONFIG_EARLY_PRINTK), xgene-storm) >> +EARLY_PRINTK_INC := xgene-storm >> +EARLY_PRINTK_BAUD := 115200 >> +EARLY_UART_BASE_ADDRESS := 0x1c020000 >> +endif >> >> ifneq ($(EARLY_PRINTK_INC),) >> EARLY_PRINTK := y >> diff --git a/xen/arch/arm/arm64/debug-xgene-storm.inc b/xen/arch/arm/arm64/debug-xgene-storm.inc >> new file mode 100644 >> index 0000000..ebbb8d2 >> --- /dev/null >> +++ b/xen/arch/arm/arm64/debug-xgene-storm.inc >> @@ -0,0 +1,55 @@ >> +/* >> + * xen/arch/arm/arm64/debug-xgene-storm.inc >> + * >> + * X-Gene Storm specific debug code >> + * >> + * Copyright (c) 2013 Applied Micro. >> + * >> + * 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/8250-uart.h> >> + >> +#define XGENE_STORM_UART_LSR 0x14 >> +#define XGENE_STORM_UART_THR 0x00 > > I guess these defines contains the shift (=4), right? > > To be more generic, you should allow a different shift via > EARLY_UART_REG_SHIFT (see arm32/debug-8250.inc) and use this define. Sure, i will do. > >> + >> +/* UART initialization >> + * rb: register which contains the UART base address >> + * rc: scratch register 1 >> + * rd: scratch register 2 */ >> +.macro early_uart_init rb rc rd >> +.endm > > early_uart_init is only needed if EARLY_PRINTK_INIT_UART is defined. > In your case it's not defined, so you can remove this macro. > > >> + >> +/* UART wait UART to be ready to transmit >> + * xb: register which contains the UART base address >> + * c: scratch register */ >> +.macro early_uart_ready xb c >> +1: >> + ldrb w\c, [\xb, #XGENE_STORM_UART_LSR] >> + and w\c, w\c, #UART_LSR_THRE >> + cmp w\c, #UART_LSR_THRE >> + b.ne 1b >> +.endm >> + >> +/* UART transmit character >> + * xb: register which contains the UART base address >> + * wt: register which contains the character to transmit */ >> +.macro early_uart_transmit xb wt >> + /* UART_THR transmit holding */ >> + strb \wt, [\xb, #XGENE_STORM_UART_THR] >> +.endm >> + >> +/* >> + * Local variables: >> + * mode: ASM >> + * indent-tabs-mode: nil >> + * End: >> + */ >> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile >> index 4aa82e8..8a6709f 100644 >> --- a/xen/arch/arm/platforms/Makefile >> +++ b/xen/arch/arm/platforms/Makefile >> @@ -2,3 +2,4 @@ obj-y += vexpress.o >> obj-y += exynos5.o >> obj-y += midway.o >> obj-y += omap5.o >> +obj-y += xgene-storm.o > > Shouldn't it be only enabled on ARM64? But then arm32 boards should be only compiled for arm32 ? > >> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c >> new file mode 100644 >> index 0000000..8e2b3b6 >> --- /dev/null >> +++ b/xen/arch/arm/platforms/xgene-storm.c >> @@ -0,0 +1,57 @@ >> +/* >> + * xen/arch/arm/platforms/xgene-storm.c >> + * >> + * Applied Micro's X-Gene specific settings >> + * >> + * Pranavkumar Sawargaonkar <psawargaonkar@apm.com> >> + * Anup Patel <apatel@apm.com> >> + * Copyright (c) 2013 Applied Micro. >> + * >> + * 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/config.h> >> +#include <xen/device_tree.h> >> +#include <xen/domain_page.h> >> +#include <xen/mm.h> >> +#include <xen/vmap.h> >> +#include <asm/platform.h> >> +#include <asm/early_printk.h> >> + >> +static void xgene_storm_reset(void) >> +{ >> +} >> + >> +static int xgene_storm_init(void) >> +{ >> + return 0; >> +} > > You don't need these empty functions, the platform code copes with empty > callback. > >> +static const char const *xgene_storm_dt_compat[] __initdata = >> +{ >> + "apm,xgene-storm", >> + NULL >> +}; >> + >> +PLATFORM_START(xgene_storm, "APM X-GENE STORM") >> + .compatible = xgene_storm_dt_compat, >> + .init = xgene_storm_init, >> + .reset = xgene_storm_reset, >> +PLATFORM_END >> + >> +/* >> + * Local variables: >> + * mode: C >> + * c-file-style: "BSD" >> + * c-basic-offset: 4 >> + * indent-tabs-mode: nil >> + * End: >> + */ >> > > > -- > Julien Grall > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel Thanks, Pranav
On Fri, 2013-09-20 at 18:58 +0530, Pranavkumar Sawargaonkar wrote: > HI Julien, > > On Fri, Sep 20, 2013 at 5:39 PM, Julien Grall <julien.grall@linaro.org> wrote: > > On 09/20/2013 10:52 AM, Pranavkumar Sawargaonkar wrote: > >> This patch adds following things: > >> Early printk support for APM X-Gene platform. > > > > Does the UART is compatible with 8250? > > Yes it is compatible but it is ns16550. I think you can just use the existing debug-8250.inc then. The early console is just a debug thing so does not need to be full featured.
On 09/20/2013 02:28 PM, Pranavkumar Sawargaonkar wrote: > HI Julien, > > On Fri, Sep 20, 2013 at 5:39 PM, Julien Grall <julien.grall@linaro.org> wrote: >> On 09/20/2013 10:52 AM, Pranavkumar Sawargaonkar wrote: >>> This patch adds following things: >>> Early printk support for APM X-Gene platform. >> >> Does the UART is compatible with 8250? > > Yes it is compatible but it is ns16550. [..] > I think better name would be debug-ns16550.inc ?? There are some UARTs that are 8250-compatible but not ns16550-compatible. It would be better to stay as generic as possible to support numerous UART. >> Also, can you divide this patch in 2 part: >> - Early printk support; >> - Initial platform support. >> >> Shouldn't it be only enabled on ARM64? > > But then arm32 boards should be only compiled for arm32 ? Right :). We begin to have multiple platforms that are only supported in one architecture (arm32 or arm64). So it's good time to introduce a way to compile only supported platform.
Hi Ian, On Fri, Sep 20, 2013 at 7:08 PM, Ian Campbell <ian.campbell@citrix.com> wrote: > On Fri, 2013-09-20 at 18:58 +0530, Pranavkumar Sawargaonkar wrote: >> HI Julien, >> >> On Fri, Sep 20, 2013 at 5:39 PM, Julien Grall <julien.grall@linaro.org> wrote: >> > On 09/20/2013 10:52 AM, Pranavkumar Sawargaonkar wrote: >> >> This patch adds following things: >> >> Early printk support for APM X-Gene platform. >> > >> > Does the UART is compatible with 8250? >> >> Yes it is compatible but it is ns16550. > > I think you can just use the existing debug-8250.inc then. The early > console is just a debug thing so does not need to be full featured. No because that is arm32 assembly. Probably i will add/port similar debug-8250.inc for arm64. > > Thanks, Pranav
On Fri, 2013-09-20 at 19:29 +0530, Pranavkumar Sawargaonkar wrote: > Hi Ian, > > On Fri, Sep 20, 2013 at 7:08 PM, Ian Campbell <ian.campbell@citrix.com> wrote: > > On Fri, 2013-09-20 at 18:58 +0530, Pranavkumar Sawargaonkar wrote: > >> HI Julien, > >> > >> On Fri, Sep 20, 2013 at 5:39 PM, Julien Grall <julien.grall@linaro.org> wrote: > >> > On 09/20/2013 10:52 AM, Pranavkumar Sawargaonkar wrote: > >> >> This patch adds following things: > >> >> Early printk support for APM X-Gene platform. > >> > > >> > Does the UART is compatible with 8250? > >> > >> Yes it is compatible but it is ns16550. > > > > I think you can just use the existing debug-8250.inc then. The early > > console is just a debug thing so does not need to be full featured. > > No because that is arm32 assembly. Doh, Obviously! Sorry, need to have breakfast/coffee before commenting next time! > Probably i will add/port similar debug-8250.inc for arm64. Yes, please name it as generically as possible. 8250 is probably the right thing. Ian.
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk index bd79b26..b36120e 100644 --- a/xen/arch/arm/Rules.mk +++ b/xen/arch/arm/Rules.mk @@ -77,6 +77,11 @@ EARLY_PRINTK_INC := 8250 EARLY_UART_BASE_ADDRESS := 0x01c28000 EARLY_UART_REG_SHIFT := 2 endif +ifeq ($(CONFIG_EARLY_PRINTK), xgene-storm) +EARLY_PRINTK_INC := xgene-storm +EARLY_PRINTK_BAUD := 115200 +EARLY_UART_BASE_ADDRESS := 0x1c020000 +endif ifneq ($(EARLY_PRINTK_INC),) EARLY_PRINTK := y diff --git a/xen/arch/arm/arm64/debug-xgene-storm.inc b/xen/arch/arm/arm64/debug-xgene-storm.inc new file mode 100644 index 0000000..ebbb8d2 --- /dev/null +++ b/xen/arch/arm/arm64/debug-xgene-storm.inc @@ -0,0 +1,55 @@ +/* + * xen/arch/arm/arm64/debug-xgene-storm.inc + * + * X-Gene Storm specific debug code + * + * Copyright (c) 2013 Applied Micro. + * + * 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/8250-uart.h> + +#define XGENE_STORM_UART_LSR 0x14 +#define XGENE_STORM_UART_THR 0x00 + +/* UART initialization + * rb: register which contains the UART base address + * rc: scratch register 1 + * rd: scratch register 2 */ +.macro early_uart_init rb rc rd +.endm + +/* UART wait UART to be ready to transmit + * xb: register which contains the UART base address + * c: scratch register */ +.macro early_uart_ready xb c +1: + ldrb w\c, [\xb, #XGENE_STORM_UART_LSR] + and w\c, w\c, #UART_LSR_THRE + cmp w\c, #UART_LSR_THRE + b.ne 1b +.endm + +/* UART transmit character + * xb: register which contains the UART base address + * wt: register which contains the character to transmit */ +.macro early_uart_transmit xb wt + /* UART_THR transmit holding */ + strb \wt, [\xb, #XGENE_STORM_UART_THR] +.endm + +/* + * Local variables: + * mode: ASM + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile index 4aa82e8..8a6709f 100644 --- a/xen/arch/arm/platforms/Makefile +++ b/xen/arch/arm/platforms/Makefile @@ -2,3 +2,4 @@ obj-y += vexpress.o obj-y += exynos5.o obj-y += midway.o obj-y += omap5.o +obj-y += xgene-storm.o diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c new file mode 100644 index 0000000..8e2b3b6 --- /dev/null +++ b/xen/arch/arm/platforms/xgene-storm.c @@ -0,0 +1,57 @@ +/* + * xen/arch/arm/platforms/xgene-storm.c + * + * Applied Micro's X-Gene specific settings + * + * Pranavkumar Sawargaonkar <psawargaonkar@apm.com> + * Anup Patel <apatel@apm.com> + * Copyright (c) 2013 Applied Micro. + * + * 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/config.h> +#include <xen/device_tree.h> +#include <xen/domain_page.h> +#include <xen/mm.h> +#include <xen/vmap.h> +#include <asm/platform.h> +#include <asm/early_printk.h> + +static void xgene_storm_reset(void) +{ +} + +static int xgene_storm_init(void) +{ + return 0; +} + +static const char const *xgene_storm_dt_compat[] __initdata = +{ + "apm,xgene-storm", + NULL +}; + +PLATFORM_START(xgene_storm, "APM X-GENE STORM") + .compatible = xgene_storm_dt_compat, + .init = xgene_storm_init, + .reset = xgene_storm_reset, +PLATFORM_END + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */