Re: [Xen-devel] [PATCH V2] xen: arm: platforms: Adding reset support for xgene arm64 platform.

[Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>]

Hi Ian,

On 23 January 2014 16:58, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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@xxxxxxxxxx>
>> Signed-off-by: Anup Patel <anup.patel@xxxxxxxxxx>
>> ---
>>  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

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