[V2] xen: arm: platforms: Adding reset support for xgene arm64 platform.

Message ID 1390460890-27971-1-git-send-email-pranavkumar@linaro.org
State New
Headers show

Commit Message

PranavkumarSawargaonkar Jan. 23, 2014, 7:08 a.m.
This patch adds a reset support for xgene arm64 platform.

V2:
- Removed unnecssary mdelay in code.
- Adding iounmap of the base address.
V1:
-Initial patch.

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Signed-off-by: Anup Patel <anup.patel@linaro.org>
---
 xen/arch/arm/platforms/xgene-storm.c |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Ian Campbell Jan. 23, 2014, 11:28 a.m. | #1
On Thu, 2014-01-23 at 12:38 +0530, Pranavkumar Sawargaonkar wrote:
> This patch adds a reset support for xgene arm64 platform.
> 
> V2:
> - Removed unnecssary mdelay in code.
> - Adding iounmap of the base address.
> V1:
> -Initial patch.
> 
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> ---
>  xen/arch/arm/platforms/xgene-storm.c |   24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
> index 5b0bd5f..9901bf0 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -20,6 +20,9 @@
>  
>  #include <xen/config.h>
>  #include <asm/platform.h>
> +#include <xen/mm.h>
> +#include <xen/vmap.h>
> +#include <asm/io.h>

Do you really need all 3 of these?

>  #include <asm/gic.h>
>  
>  static uint32_t xgene_storm_quirks(void)
> @@ -107,6 +110,26 @@ err:
>      return ret;
>  }
>  
> +/*
> + * TODO: Get base address and mask from the device tree

How likely are these to change during the lifetime of Xen 4.4? I think
it cannot be ruled out. Doing this based on DT shouldn't be more than a
dozen lines of code -- see e.g init_xen_time or gic_init.

You probably want to do the detection at start of day in the
platform->init hook so that you can call:
        dt_device_set_used_by(node, DOMID_XEN);
on it so it doesn't get given to dom0.

Otherwise I expect you will also want to add the relevant compatibility
string to the platform blacklist_dev list so it isn't given to dom0 for
that reason.

> + */
> +static void xgene_storm_reset(void)
> +{
> +    void __iomem *addr;
> +
> +    addr = ioremap_nocache(0x17000014UL, 0x100);

If you aren't going to use device tree please can you at least add a
#define for the address.

> +    if ( !addr )
> +    {
> +        dprintk(XENLOG_ERR, "Unable to map xgene reset address\n");
> +        return;
> +    }
> +
> +    /* Write mask 0x1 to base address */
> +    writel(0x1, addr);
> +
> +    iounmap(addr);
> +}
>  
>  static const char * const xgene_storm_dt_compat[] __initconst =
>  {
> @@ -116,6 +139,7 @@ static const char * const xgene_storm_dt_compat[] __initconst =
>  
>  PLATFORM_START(xgene_storm, "APM X-GENE STORM")
>      .compatible = xgene_storm_dt_compat,
> +    .reset = xgene_storm_reset,
>      .quirks = xgene_storm_quirks,
>      .specific_mapping = xgene_storm_specific_mapping,
>
PranavkumarSawargaonkar Jan. 23, 2014, 12:29 p.m. | #2
Hi Ian,

On 23 January 2014 16:58, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2014-01-23 at 12:38 +0530, Pranavkumar Sawargaonkar wrote:
>> This patch adds a reset support for xgene arm64 platform.
>>
>> V2:
>> - Removed unnecssary mdelay in code.
>> - Adding iounmap of the base address.
>> V1:
>> -Initial patch.
>>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> ---
>>  xen/arch/arm/platforms/xgene-storm.c |   24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
>> index 5b0bd5f..9901bf0 100644
>> --- a/xen/arch/arm/platforms/xgene-storm.c
>> +++ b/xen/arch/arm/platforms/xgene-storm.c
>> @@ -20,6 +20,9 @@
>>
>>  #include <xen/config.h>
>>  #include <asm/platform.h>
>> +#include <xen/mm.h>
>> +#include <xen/vmap.h>
>> +#include <asm/io.h>
>
> Do you really need all 3 of these?
I will remove mm.h
vmap.h is required for iounmap and io.h is for writel.

>
>>  #include <asm/gic.h>
>>
>>  static uint32_t xgene_storm_quirks(void)
>> @@ -107,6 +110,26 @@ err:
>>      return ret;
>>  }
>>
>> +/*
>> + * TODO: Get base address and mask from the device tree
>
> How likely are these to change during the lifetime of Xen 4.4? I think
> it cannot be ruled out. Doing this based on DT shouldn't be more than a
> dozen lines of code -- see e.g init_xen_time or gic_init.
>
> You probably want to do the detection at start of day in the
> platform->init hook so that you can call:
>         dt_device_set_used_by(node, DOMID_XEN);
> on it so it doesn't get given to dom0.
>
> Otherwise I expect you will also want to add the relevant compatibility
> string to the platform blacklist_dev list so it isn't given to dom0 for
> that reason.

Thanks i was not aware of this let me try to fix it using device tree only.

>
>> + */
>> +static void xgene_storm_reset(void)
>> +{
>> +    void __iomem *addr;
>> +
>> +    addr = ioremap_nocache(0x17000014UL, 0x100);
>
> If you aren't going to use device tree please can you at least add a
> #define for the address.

I think i can try using device tree only instead of hard-coding.

>
>> +    if ( !addr )
>> +    {
>> +        dprintk(XENLOG_ERR, "Unable to map xgene reset address\n");
>> +        return;
>> +    }
>> +
>> +    /* Write mask 0x1 to base address */
>> +    writel(0x1, addr);
>> +
>> +    iounmap(addr);
>> +}
>>
>>  static const char * const xgene_storm_dt_compat[] __initconst =
>>  {
>> @@ -116,6 +139,7 @@ static const char * const xgene_storm_dt_compat[] __initconst =
>>
>>  PLATFORM_START(xgene_storm, "APM X-GENE STORM")
>>      .compatible = xgene_storm_dt_compat,
>> +    .reset = xgene_storm_reset,
>>      .quirks = xgene_storm_quirks,
>>      .specific_mapping = xgene_storm_specific_mapping,
>>
>
>
Thanks,
Pranav

Patch

diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 5b0bd5f..9901bf0 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -20,6 +20,9 @@ 
 
 #include <xen/config.h>
 #include <asm/platform.h>
+#include <xen/mm.h>
+#include <xen/vmap.h>
+#include <asm/io.h>
 #include <asm/gic.h>
 
 static uint32_t xgene_storm_quirks(void)
@@ -107,6 +110,26 @@  err:
     return ret;
 }
 
+/*
+ * TODO: Get base address and mask from the device tree
+ */
+static void xgene_storm_reset(void)
+{
+    void __iomem *addr;
+
+    addr = ioremap_nocache(0x17000014UL, 0x100);
+
+    if ( !addr )
+    {
+        dprintk(XENLOG_ERR, "Unable to map xgene reset address\n");
+        return;
+    }
+
+    /* Write mask 0x1 to base address */
+    writel(0x1, addr);
+
+    iounmap(addr);
+}
 
 static const char * const xgene_storm_dt_compat[] __initconst =
 {
@@ -116,6 +139,7 @@  static const char * const xgene_storm_dt_compat[] __initconst =
 
 PLATFORM_START(xgene_storm, "APM X-GENE STORM")
     .compatible = xgene_storm_dt_compat,
+    .reset = xgene_storm_reset,
     .quirks = xgene_storm_quirks,
     .specific_mapping = xgene_storm_specific_mapping,