From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v4 05/16] xen/arm: use ioremap to map gic-v2 registers Date: Mon, 26 May 2014 14:10:48 +0100 Message-ID: <53833D58.8060204@linaro.org> References: <1401100009-7326-1-git-send-email-vijay.kilari@gmail.com> <1401100009-7326-6-git-send-email-vijay.kilari@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1401100009-7326-6-git-send-email-vijay.kilari@gmail.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: vijay.kilari@gmail.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, stefano.stabellini@citrix.com, xen-devel@lists.xen.org Cc: Prasun.Kapoor@caviumnetworks.com, vijaya.kumar@caviumnetworks.com List-Id: xen-devel@lists.xenproject.org Hi Vijay, Thank you for the patch. Did you verify that GICv2 is correctly working with theses changes? On 26/05/14 11:26, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K > > gic-v2 driver uses fixmap to map the registers. > Instead use ioremap to access mmio registers. > > With this patch, gic-v2 register definitions are updated > to use obsolute offset address instead of dividing the > register offset by 4. > > Update vgic driver logic to compute using obsolute register s/obsolute/obsolete/ > /* Local settings: interface controller */ > - GICC[GICC_PMR] = 0xff; /* Don't mask by priority */ > - GICC[GICC_BPR] = 0; /* Finest granularity of priority */ > - GICC[GICC_CTLR] = GICC_CTL_ENABLE|GICC_CTL_EOI; /* Turn on delivery */ > + writel_relaxed(0xff, GICC + GICC_PMR); /* Don't mask by priority */ > + writel_relaxed(0x0, GICC + GICC_BPR); /* Finest granularity of priority */ > + writel_relaxed(GICC_CTL_ENABLE|GICC_CTL_EOI, GICC + GICC_CTLR); /* Turn on delivery */ The coding style requests 80 characters per line. > @@ -700,7 +713,7 @@ static void gic_update_one_lr(struct vcpu *v, int i) > irq, v->domain->domain_id, v->vcpu_id, i); > #endif > } else { > - GICH[GICH_LR + i] = 0; > + writel_relaxed(0, GICH + GICH_LR + i * 4); > clear_bit(i, &this_cpu(lr_mask)); > > if ( p->desc != NULL ) > @@ -808,7 +821,7 @@ int gic_events_need_delivery(void) > struct pending_irq *p; > unsigned long flags; > > - mask_priority = (GICH[GICH_VMCR] >> GICH_VMCR_PRIORITY_SHIFT) & GICH_VMCR_PRIORITY_MASK; > + mask_priority = (readl_relaxed(GICH + GICH_VMCR) >> GICH_VMCR_PRIORITY_SHIFT) & GICH_VMCR_PRIORITY_MASK; Same remark here. > /* Number of ranks of interrupt registers for a domain */ > #define DOMAIN_NR_RANKS(d) (((d)->arch.vgic.nr_lines+31)/32) > @@ -55,7 +55,7 @@ static inline int REG_RANK_NR(int b, uint32_t n) > * Offset of GICD_ with its rank, for GICD_ with > * -bits-per-interrupt. > */ > -#define REG_RANK_INDEX(b, n) ((n) & ((b)-1)) > +#define REG_RANK_INDEX(b, n) (((n) >> 2) & ((b)-1)) > > /* > * Returns rank corresponding to a GICD_ register for > @@ -63,7 +63,9 @@ static inline int REG_RANK_NR(int b, uint32_t n) > */ > static struct vgic_irq_rank *vgic_irq_rank(struct vcpu *v, int b, int n) > { > - int rank = REG_RANK_NR(b, n); > + int rank; > + n = n >> 2; > + rank = REG_RANK_NR(b, n); As n is only used for REG_RANK_NR, I would do: int rank = REG_RANK_NR(b, n >> 2) Otherwise than the few changes here, the patch looks good to me, assuming you did some tests on GICv2. Regards, -- Julien Grall