| Message ID | 1368152307-598-34-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
> @@ -91,6 +93,47 @@ out: > } > > /* > + * TODO: Get base address from the device tree > + * See arm,vexpress-reset node > + */ > +static void vexpress_reset(void) > +{ > + void __iomem *base; > + void __iomem *sp810; > + > + /* Use the SP810 system controller to force a reset */ > + base = ioremap_attr(SP810_ADDRESS & PAGE_MASK, PAGE_SIZE, > + PAGE_HYPERVISOR_NOCACHE); > + if ( !base ) > + { > + dprintk(XENLOG_ERR, "Unable to map SP810\n"); > + return; > + } > + > + sp810 = base + (SP810_ADDRESS & ~PAGE_MASK); Didn't I see you making vunmap, which iounmap is based on, take care of the page offsets itself in an earlier patch? Or is that not the reason you are going through the base + offset dance? > + > + /* switch to slow mode */ > + iowritel(sp810, 0x3); > + dsb(); isb(); > + /* writing any value to SCSYSSTAT reg will reset the system */ > + iowritel(sp810 + 4, 0x1); > + dsb(); isb(); Hopefully we don't get here ;-) > + iounmap(base); > +} > +
On 05/10/2013 10:44 AM, Ian Campbell wrote: >> @@ -91,6 +93,47 @@ out: >> } >> >> /* >> + * TODO: Get base address from the device tree >> + * See arm,vexpress-reset node >> + */ >> +static void vexpress_reset(void) >> +{ >> + void __iomem *base; >> + void __iomem *sp810; >> + >> + /* Use the SP810 system controller to force a reset */ >> + base = ioremap_attr(SP810_ADDRESS & PAGE_MASK, PAGE_SIZE, >> + PAGE_HYPERVISOR_NOCACHE); >> + if ( !base ) >> + { >> + dprintk(XENLOG_ERR, "Unable to map SP810\n"); >> + return; >> + } >> + >> + sp810 = base + (SP810_ADDRESS & ~PAGE_MASK); > > Didn't I see you making vunmap, which iounmap is based on, take care of > the page offsets itself in an earlier patch? Or is that not the reason > you are going through the base + offset dance? I wasn't not sure if you will accept ioremap(..., 8); I will fix the patch. > >> + >> + /* switch to slow mode */ >> + iowritel(sp810, 0x3); >> + dsb(); isb(); >> + /* writing any value to SCSYSSTAT reg will reset the system */ >> + iowritel(sp810 + 4, 0x1); >> + dsb(); isb(); > > Hopefully we don't get here ;-) > >> + iounmap(base); >> +} >> + >
On Fri, 2013-05-10 at 15:00 +0100, Julien Grall wrote: > On 05/10/2013 10:44 AM, Ian Campbell wrote: > > >> @@ -91,6 +93,47 @@ out: > >> } > >> > >> /* > >> + * TODO: Get base address from the device tree > >> + * See arm,vexpress-reset node > >> + */ > >> +static void vexpress_reset(void) > >> +{ > >> + void __iomem *base; > >> + void __iomem *sp810; > >> + > >> + /* Use the SP810 system controller to force a reset */ > >> + base = ioremap_attr(SP810_ADDRESS & PAGE_MASK, PAGE_SIZE, > >> + PAGE_HYPERVISOR_NOCACHE); > >> + if ( !base ) > >> + { > >> + dprintk(XENLOG_ERR, "Unable to map SP810\n"); > >> + return; > >> + } > >> + > >> + sp810 = base + (SP810_ADDRESS & ~PAGE_MASK); > > > > Didn't I see you making vunmap, which iounmap is based on, take care of > > the page offsets itself in an earlier patch? Or is that not the reason > > you are going through the base + offset dance? > > > I wasn't not sure if you will accept ioremap(..., 8); I will fix the patch. I'm confused, what does (..., 8) mean? What I was asking about was why the code couldn't be: sp810 = ioremap_..(SP810_ADDRESS, PAGE_SIZE,..) iowritel(sp810,...) iounmap(sp810) Ian.
On 05/10/2013 03:06 PM, Ian Campbell wrote: > On Fri, 2013-05-10 at 15:00 +0100, Julien Grall wrote: >> On 05/10/2013 10:44 AM, Ian Campbell wrote: >> >>>> @@ -91,6 +93,47 @@ out: >>>> } >>>> >>>> /* >>>> + * TODO: Get base address from the device tree >>>> + * See arm,vexpress-reset node >>>> + */ >>>> +static void vexpress_reset(void) >>>> +{ >>>> + void __iomem *base; >>>> + void __iomem *sp810; >>>> + >>>> + /* Use the SP810 system controller to force a reset */ >>>> + base = ioremap_attr(SP810_ADDRESS & PAGE_MASK, PAGE_SIZE, >>>> + PAGE_HYPERVISOR_NOCACHE); >>>> + if ( !base ) >>>> + { >>>> + dprintk(XENLOG_ERR, "Unable to map SP810\n"); >>>> + return; >>>> + } >>>> + >>>> + sp810 = base + (SP810_ADDRESS & ~PAGE_MASK); >>> >>> Didn't I see you making vunmap, which iounmap is based on, take care of >>> the page offsets itself in an earlier patch? Or is that not the reason >>> you are going through the base + offset dance? >> >> >> I wasn't not sure if you will accept ioremap(..., 8); I will fix the patch. > > I'm confused, what does (..., 8) mean? Because we only need to the first 8 bytes. > What I was asking about was why the code couldn't be: > sp810 = ioremap_..(SP810_ADDRESS, PAGE_SIZE,..) > iowritel(sp810,...) > iounmap(sp810) Indeed. I will use this solution.
On Fri, 2013-05-10 at 15:10 +0100, Julien Grall wrote: > On 05/10/2013 03:06 PM, Ian Campbell wrote: > > > On Fri, 2013-05-10 at 15:00 +0100, Julien Grall wrote: > >> On 05/10/2013 10:44 AM, Ian Campbell wrote: > >> > >>>> @@ -91,6 +93,47 @@ out: > >>>> } > >>>> > >>>> /* > >>>> + * TODO: Get base address from the device tree > >>>> + * See arm,vexpress-reset node > >>>> + */ > >>>> +static void vexpress_reset(void) > >>>> +{ > >>>> + void __iomem *base; > >>>> + void __iomem *sp810; > >>>> + > >>>> + /* Use the SP810 system controller to force a reset */ > >>>> + base = ioremap_attr(SP810_ADDRESS & PAGE_MASK, PAGE_SIZE, > >>>> + PAGE_HYPERVISOR_NOCACHE); > >>>> + if ( !base ) > >>>> + { > >>>> + dprintk(XENLOG_ERR, "Unable to map SP810\n"); > >>>> + return; > >>>> + } > >>>> + > >>>> + sp810 = base + (SP810_ADDRESS & ~PAGE_MASK); > >>> > >>> Didn't I see you making vunmap, which iounmap is based on, take care of > >>> the page offsets itself in an earlier patch? Or is that not the reason > >>> you are going through the base + offset dance? > >> > >> > >> I wasn't not sure if you will accept ioremap(..., 8); I will fix the patch. > > > > I'm confused, what does (..., 8) mean? > > Because we only need to the first 8 bytes. If ioremap is happy with sub-page sizes then I'm ok with this or using PAGE_SIZE. Ian.
diff --git a/xen/arch/arm/platforms/vexpress.c b/xen/arch/arm/platforms/vexpress.c index fd4ce74..9b35e9d 100644 --- a/xen/arch/arm/platforms/vexpress.c +++ b/xen/arch/arm/platforms/vexpress.c @@ -18,7 +18,9 @@ */ #include <asm/platforms/vexpress.h> +#include <asm/platform.h> #include <xen/mm.h> +#include <xen/vmap.h> #define DCC_SHIFT 26 #define FUNCTION_SHIFT 20 @@ -91,6 +93,47 @@ out: } /* + * TODO: Get base address from the device tree + * See arm,vexpress-reset node + */ +static void vexpress_reset(void) +{ + void __iomem *base; + void __iomem *sp810; + + /* Use the SP810 system controller to force a reset */ + base = ioremap_attr(SP810_ADDRESS & PAGE_MASK, PAGE_SIZE, + PAGE_HYPERVISOR_NOCACHE); + if ( !base ) + { + dprintk(XENLOG_ERR, "Unable to map SP810\n"); + return; + } + + sp810 = base + (SP810_ADDRESS & ~PAGE_MASK); + + /* switch to slow mode */ + iowritel(sp810, 0x3); + dsb(); isb(); + /* writing any value to SCSYSSTAT reg will reset the system */ + iowritel(sp810 + 4, 0x1); + dsb(); isb(); + + iounmap(base); +} + +static const char const *vexpress_dt_compat[] __initdata = +{ + "arm,vexpress", + NULL +}; + +PLATFORM_START(vexpress, "VERSATILE EXPRESS") + .compatible = vexpress_dt_compat, + .reset = vexpress_reset, +PLATFORM_END + +/* * Local variables: * mode: C * c-file-style: "BSD" diff --git a/xen/arch/arm/shutdown.c b/xen/arch/arm/shutdown.c index 0903842..767cc12 100644 --- a/xen/arch/arm/shutdown.c +++ b/xen/arch/arm/shutdown.c @@ -3,25 +3,11 @@ #include <xen/cpu.h> #include <xen/delay.h> #include <xen/lib.h> -#include <xen/mm.h> #include <xen/smp.h> #include <asm/platform.h> static void raw_machine_reset(void) { - /* XXX get this from device tree */ -#ifdef SP810_ADDRESS - /* Use the SP810 system controller to force a reset */ - volatile uint32_t *sp810; - set_fixmap(FIXMAP_MISC, SP810_ADDRESS >> PAGE_SHIFT, DEV_SHARED); - sp810 = ((uint32_t *) - (FIXMAP_ADDR(FIXMAP_MISC) + (SP810_ADDRESS & ~PAGE_MASK))); - sp810[0] = 0x3; /* switch to slow mode */ - dsb(); isb(); - sp810[1] = 0x1; /* writing any value to SCSYSSTAT reg will reset system */ - dsb(); isb(); - clear_fixmap(FIXMAP_MISC); -#endif platform_reset(); } diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index 8ed72f5..7599202 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -149,9 +149,6 @@ extern unsigned long frametable_virt_end; #define GIC_CR_OFFSET 0x2000 #define GIC_HR_OFFSET 0x4000 /* Guess work http://lists.infradead.org/pipermail/linux-arm-kernel/2011-September/064219.html */ #define GIC_VR_OFFSET 0x6000 /* Virtual Machine CPU interface) */ -/* Board-specific: base address of system controller */ -#define SP810_ADDRESS 0x1C020000 - #endif /* __ARM_CONFIG_H__ */ /* diff --git a/xen/include/asm-arm/platforms/vexpress.h b/xen/include/asm-arm/platforms/vexpress.h index 67f8fef..5cf3aba 100644 --- a/xen/include/asm-arm/platforms/vexpress.h +++ b/xen/include/asm-arm/platforms/vexpress.h @@ -23,6 +23,9 @@ #define V2M_SYS_CFG_OSC4 4 #define V2M_SYS_CFG_OSC5 5 +/* Board-specific: base address of system controller */ +#define SP810_ADDRESS 0x1C020000 + #ifndef __ASSEMBLY__ #include <xen/inttypes.h>
This platform contains nearly nothing specific except the reset function. Signed-off-by: Julien Grall <julien.grall@linaro.org> Changes in v3: - Use ioremap_attr instead of fixmap Changes in v2: - Add TODO to retrieve reset base address in the DT --- xen/arch/arm/platforms/vexpress.c | 43 ++++++++++++++++++++++++++++++ xen/arch/arm/shutdown.c | 14 ---------- xen/include/asm-arm/config.h | 3 --- xen/include/asm-arm/platforms/vexpress.h | 3 +++ 4 files changed, 46 insertions(+), 17 deletions(-)