[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/4] xen: arm64: Add Basic Platform support for APM X-Gene Storm.



HI Julien,

On Fri, Sep 20, 2013 at 5:39 PM, Julien Grall <julien.grall@xxxxxxxxxx> 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@xxxxxxxxxx>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>
>> ---
>>  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@xxxxxxx>
>> + * Anup Patel <apatel@xxxxxxx>
>> + * 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@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

Thanks,
Pranav

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.