| Message ID | 1390564129-32611-1-git-send-email-pranavkumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi, On 24 January 2014 17:18, Pranavkumar Sawargaonkar <pranavkumar@linaro.org> wrote: > This patch adds a reset support for xgene arm64 platform. > > V3: > - Retriving reset base address and reset mask from device tree. > - Removed unnecssary header files included earlier. > 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 | 70 ++++++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > index 5b0bd5f..986284c 100644 > --- a/xen/arch/arm/platforms/xgene-storm.c > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -20,8 +20,17 @@ > > #include <xen/config.h> > #include <asm/platform.h> > +#include <xen/vmap.h> > +#include <asm/io.h> > #include <asm/gic.h> > > +#define DT_MATCH_RESET \ > + DT_MATCH_COMPATIBLE("apm,xgene-reboot") > + > +/* Variables to save reset address of soc during platform initialization. */ > +static u64 reset_addr, reset_size; > +static u32 reset_mask; > + > static uint32_t xgene_storm_quirks(void) > { > return PLATFORM_QUIRK_GIC_64K_STRIDE; > @@ -107,6 +116,65 @@ err: > return ret; > } > > +/* > + * TODO: Get base address and mask from the device tree > + */ Will resend a patch with removal of this comment. > +static void xgene_storm_reset(void) > +{ > + void __iomem *addr; > + > + addr = ioremap_nocache(reset_addr, reset_size); > + > + if ( !addr ) > + { > + printk("Unable to map xgene reset address\n"); > + return; > + } > + > + /* Write mask 0x1 to base address */ > + writel(reset_mask, addr); > + > + iounmap(addr); > +} > + > +static int xgene_storm_init(void) > +{ > + static const struct dt_device_match reset_ids[] __initconst = > + { > + DT_MATCH_RESET, > + {}, > + }; > + struct dt_device_node *dev; > + int res; > + > + dev = dt_find_matching_node(NULL, reset_ids); > + if ( !dev ) > + { > + printk("Unable to find a compatible reset node in " > + "the device tree"); > + return -ENODEV; > + } > + > + dt_device_set_used_by(dev, DOMID_XEN); > + > + /* Retrieve base address and size */ > + res = dt_device_get_address(dev, 0, &reset_addr, &reset_size); > + if ( res ) > + { > + printk("Unable to retrieve the base address for reset\n"); > + return res; > + } > + > + /* Get reset mask */ > + res = dt_property_read_u32(dev, "mask", &reset_mask); > + if ( !res ) > + { > + printk("Unable to retrieve the reset mask\n"); > + return res; > + } > + > + return 0; > +} > > static const char * const xgene_storm_dt_compat[] __initconst = > { > @@ -116,6 +184,8 @@ static const char * const xgene_storm_dt_compat[] __initconst = > > PLATFORM_START(xgene_storm, "APM X-GENE STORM") > .compatible = xgene_storm_dt_compat, > + .init = xgene_storm_init, > + .reset = xgene_storm_reset, > .quirks = xgene_storm_quirks, > .specific_mapping = xgene_storm_specific_mapping, > > -- > 1.7.9.5 > Thanks, Pranav
On Fri, 2014-01-24 at 17:18 +0530, Pranavkumar Sawargaonkar wrote: > This patch adds a reset support for xgene arm64 platform. > > V3: > - Retriving reset base address and reset mask from device tree. > - Removed unnecssary header files included earlier. > 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 | 70 ++++++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > index 5b0bd5f..986284c 100644 > --- a/xen/arch/arm/platforms/xgene-storm.c > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -20,8 +20,17 @@ > > #include <xen/config.h> > #include <asm/platform.h> > +#include <xen/vmap.h> > +#include <asm/io.h> > #include <asm/gic.h> > > +#define DT_MATCH_RESET \ > + DT_MATCH_COMPATIBLE("apm,xgene-reboot") The gic and timer use a #define here because it is needed in multiple places, for this use case you can just inline it into the array in xgene_storm_init. i.e.: > +static int xgene_storm_init(void) > +{ > + static const struct dt_device_match reset_ids[] __initconst = > + { > + DT_MATCH_RESET, DT_MATCH_COMPATIBLE("apm,xgene-reboot") is fine IMHO. > + {}, > + }; > + struct dt_device_node *dev; > + int res; > + > + dev = dt_find_matching_node(NULL, reset_ids); > + if ( !dev ) > + { > + printk("Unable to find a compatible reset node in " > + "the device tree"); Please don't wrap string constants, it makes it hard to grep and I'd rather have a long line (in this case it's not too long either). Please can you add an xgene: (or whatever is appropriate) prefix too. > + return -ENODEV; I wonder if it is worth returning success here? The system would be mostly functional after all. (You could apply this logic to the other returns if you wish, although if the node is present then an error in the content could be considered more critical to abort on) > + } > + > + dt_device_set_used_by(dev, DOMID_XEN); > + > + /* Retrieve base address and size */ > + res = dt_device_get_address(dev, 0, &reset_addr, &reset_size); > + if ( res ) > + { > + printk("Unable to retrieve the base address for reset\n"); > + return res; > + } > + > + /* Get reset mask */ > + res = dt_property_read_u32(dev, "mask", &reset_mask); > + if ( !res ) > + { > + printk("Unable to retrieve the reset mask\n"); > + return res; > + } All looks good, thanks. > + > + return 0; > +} > > static const char * const xgene_storm_dt_compat[] __initconst = > { > @@ -116,6 +184,8 @@ static const char * const xgene_storm_dt_compat[] __initconst = > > PLATFORM_START(xgene_storm, "APM X-GENE STORM") > .compatible = xgene_storm_dt_compat, > + .init = xgene_storm_init, > + .reset = xgene_storm_reset, > .quirks = xgene_storm_quirks, > .specific_mapping = xgene_storm_specific_mapping, >
Hi Ian, On 24 January 2014 17:31, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Fri, 2014-01-24 at 17:18 +0530, Pranavkumar Sawargaonkar wrote: >> This patch adds a reset support for xgene arm64 platform. >> >> V3: >> - Retriving reset base address and reset mask from device tree. >> - Removed unnecssary header files included earlier. >> 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 | 70 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 70 insertions(+) >> >> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c >> index 5b0bd5f..986284c 100644 >> --- a/xen/arch/arm/platforms/xgene-storm.c >> +++ b/xen/arch/arm/platforms/xgene-storm.c >> @@ -20,8 +20,17 @@ >> >> #include <xen/config.h> >> #include <asm/platform.h> >> +#include <xen/vmap.h> >> +#include <asm/io.h> >> #include <asm/gic.h> >> >> +#define DT_MATCH_RESET \ >> + DT_MATCH_COMPATIBLE("apm,xgene-reboot") > > The gic and timer use a #define here because it is needed in multiple > places, for this use case you can just inline it into the array in > xgene_storm_init. i.e.: > >> +static int xgene_storm_init(void) >> +{ >> + static const struct dt_device_match reset_ids[] __initconst = >> + { >> + DT_MATCH_RESET, > > DT_MATCH_COMPATIBLE("apm,xgene-reboot") > > is fine IMHO. Sure i will fix this, > >> + {}, >> + }; >> + struct dt_device_node *dev; >> + int res; >> + >> + dev = dt_find_matching_node(NULL, reset_ids); >> + if ( !dev ) >> + { >> + printk("Unable to find a compatible reset node in " >> + "the device tree"); > > Please don't wrap string constants, it makes it hard to grep and I'd > rather have a long line (in this case it's not too long either). > > Please can you add an xgene: (or whatever is appropriate) prefix too. Yes will do it. > >> + return -ENODEV; > > I wonder if it is worth returning success here? The system would be > mostly functional after all. > > (You could apply this logic to the other returns if you wish, although > if the node is present then an error in the content could be considered > more critical to abort on) Yeah actually i also wonder about correct return value since system is mostly functional without this. I will return a success here. > >> + } >> + >> + dt_device_set_used_by(dev, DOMID_XEN); >> + >> + /* Retrieve base address and size */ >> + res = dt_device_get_address(dev, 0, &reset_addr, &reset_size); >> + if ( res ) >> + { >> + printk("Unable to retrieve the base address for reset\n"); >> + return res; >> + } >> + >> + /* Get reset mask */ >> + res = dt_property_read_u32(dev, "mask", &reset_mask); >> + if ( !res ) >> + { >> + printk("Unable to retrieve the reset mask\n"); >> + return res; >> + } > > All looks good, thanks. Thanks, will send out a new patch today. > > >> + >> + return 0; >> +} >> >> static const char * const xgene_storm_dt_compat[] __initconst = >> { >> @@ -116,6 +184,8 @@ static const char * const xgene_storm_dt_compat[] __initconst = >> >> PLATFORM_START(xgene_storm, "APM X-GENE STORM") >> .compatible = xgene_storm_dt_compat, >> + .init = xgene_storm_init, >> + .reset = xgene_storm_reset, >> .quirks = xgene_storm_quirks, >> .specific_mapping = xgene_storm_specific_mapping, >> > > Thanks, Pranav
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c index 5b0bd5f..986284c 100644 --- a/xen/arch/arm/platforms/xgene-storm.c +++ b/xen/arch/arm/platforms/xgene-storm.c @@ -20,8 +20,17 @@ #include <xen/config.h> #include <asm/platform.h> +#include <xen/vmap.h> +#include <asm/io.h> #include <asm/gic.h> +#define DT_MATCH_RESET \ + DT_MATCH_COMPATIBLE("apm,xgene-reboot") + +/* Variables to save reset address of soc during platform initialization. */ +static u64 reset_addr, reset_size; +static u32 reset_mask; + static uint32_t xgene_storm_quirks(void) { return PLATFORM_QUIRK_GIC_64K_STRIDE; @@ -107,6 +116,65 @@ 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(reset_addr, reset_size); + + if ( !addr ) + { + printk("Unable to map xgene reset address\n"); + return; + } + + /* Write mask 0x1 to base address */ + writel(reset_mask, addr); + + iounmap(addr); +} + +static int xgene_storm_init(void) +{ + static const struct dt_device_match reset_ids[] __initconst = + { + DT_MATCH_RESET, + {}, + }; + struct dt_device_node *dev; + int res; + + dev = dt_find_matching_node(NULL, reset_ids); + if ( !dev ) + { + printk("Unable to find a compatible reset node in " + "the device tree"); + return -ENODEV; + } + + dt_device_set_used_by(dev, DOMID_XEN); + + /* Retrieve base address and size */ + res = dt_device_get_address(dev, 0, &reset_addr, &reset_size); + if ( res ) + { + printk("Unable to retrieve the base address for reset\n"); + return res; + } + + /* Get reset mask */ + res = dt_property_read_u32(dev, "mask", &reset_mask); + if ( !res ) + { + printk("Unable to retrieve the reset mask\n"); + return res; + } + + return 0; +} static const char * const xgene_storm_dt_compat[] __initconst = { @@ -116,6 +184,8 @@ static const char * const xgene_storm_dt_compat[] __initconst = PLATFORM_START(xgene_storm, "APM X-GENE STORM") .compatible = xgene_storm_dt_compat, + .init = xgene_storm_init, + .reset = xgene_storm_reset, .quirks = xgene_storm_quirks, .specific_mapping = xgene_storm_specific_mapping,