From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 1/2] xen/arm: Introduce accessors for GICv2 MMIO reads/writes Date: Wed, 09 Jul 2014 13:09:09 +0100 Message-ID: <53BD30E5.8010400@linaro.org> References: <1404906858-709-1-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1404906858-709-1-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , xen-devel@lists.xen.org Cc: Vijaya Kumar K , tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 07/09/2014 12:54 PM, Ian Campbell wrote: > A future patch needs to make the accesses to GICC more complex. For consistency > introduce read/write wrappers for all three regions (GICD, GICC, GICH). > > Signed-off-by: Ian Campbell > Cc: Vijaya Kumar K > --- > xen/arch/arm/gic-v2.c | 157 +++++++++++++++++++++++++++++-------------------- > 1 file changed, 94 insertions(+), 63 deletions(-) > > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index cc60af8..3820be5 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -60,16 +60,12 @@ > #define GICH_V2_VMCR_PRIORITY_MASK 0x1f > #define GICH_V2_VMCR_PRIORITY_SHIFT 27 > > -#define GICD (gicv2.map_dbase) > -#define GICC (gicv2.map_cbase) > -#define GICH (gicv2.map_hbase) > - > /* Global state */ > static struct { > paddr_t dbase; /* Address of distributor registers */ > void __iomem * map_dbase; /* IO mapped Address of distributor registers */ > paddr_t cbase; /* Address of CPU interface registers */ > - void __iomem * map_cbase; /* IO mapped Address of CPU interface registers*/ > + void __iomem * map_cbase; /* IO mapped Address of CPU interface registers */ > paddr_t hbase; /* Address of virtual interface registers */ > void __iomem * map_hbase; /* IO Address of virtual interface registers */ > paddr_t vbase; /* Address of virtual cpu interface registers */ > @@ -87,6 +83,41 @@ static DEFINE_PER_CPU(u8, gic_cpu_id); > /* Maximum cpu interface per GIC */ > #define NR_GIC_CPU_IF 8 > > +static inline void writeb_gicd(uint32_t val, int offset) NIT: the value is 8 byte, so I would use uint8_t. Also the offset should never be negative. I would use unsigned int (same remark for the other helpers). > +{ > + writeb_relaxed(val, gicv2.map_dbase + offset); > +} The other part of the patch looks good to me. Regards, -- Julien Grall