Re: [PATCH v2] irqchip/gic-v2m: Add workaround for Broadcom NS2 GICv2m erratum

[Ray Jui]

On 5/5/2016 12:38 AM, Marc Zyngier wrote:
> On 05/05/16 00:47, Ray Jui wrote:
> > Alex Barba <alex.barba@xxxxxxxxxxxx> discovered Broadcom NS2 GICv2m
> > implementation has an erratum where the MSI data needs to be the SPI
> > number subtracted by an offset of 32, for the correct MSI interrupt
> > to be triggered.
> >
> > Here we are adding the workaround based on readings from the MSI_IIDR
> > register, which contains a value unique to Broadcom NS2 GICv2m
> >
> > Reported-by: Alex Barba <alex.barba@xxxxxxxxxxxx>
> > Signed-off-by: Ray Jui <ray.jui@xxxxxxxxxxxx>
> > ---
> >  drivers/irqchip/irq-gic-v2m.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> > index 28f047c..4c89e27 100644
> > --- a/drivers/irqchip/irq-gic-v2m.c
> > +++ b/drivers/irqchip/irq-gic-v2m.c
> > @@ -49,6 +49,9 @@
> >  /* APM X-Gene with GICv2m MSI_IIDR register value */
> >  #define XGENE_GICV2M_MSI_IIDR		0x06000170
> >
> > +/* Broadcom NS2 GICv2m MSI_IIDR register value */
> > +#define BCM_NS2_GICV2M_MSI_IIDR		0x0000013f
> > +
> >  /* List of flags for specific v2m implementation */
> >  #define GICV2M_NEEDS_SPI_OFFSET		0x00000001
> >
> > @@ -62,6 +65,7 @@ struct v2m_data {
> >  	void __iomem *base;	/* GICv2m virt address */
> >  	u32 spi_start;		/* The SPI number that MSIs start */
> >  	u32 nr_spis;		/* The number of SPIs for MSIs */
> > +	u32 spi_offset;		/* offset to be subtracted from SPI number */
> >  	unsigned long *bm;	/* MSI vector bitmap */
> >  	u32 flags;		/* v2m flags for specific implementation */
> >  };
> > @@ -102,7 +106,7 @@ static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> >  	msg->data = data->hwirq;
> >
> >  	if (v2m->flags & GICV2M_NEEDS_SPI_OFFSET)
> > -		msg->data -= v2m->spi_start;
> > +		msg->data -= v2m->spi_offset;
> >  }
> >
> >  static struct irq_chip gicv2m_irq_chip = {
> > @@ -340,9 +344,18 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
> >  	 * different from the standard GICv2m implementation where
> >  	 * the MSI data is the absolute value within the range from
> >  	 * spi_start to (spi_start + num_spis).
> > +	 *
> > +	 * Broadom NS2 GICv2m implementation has an erratum where the MSI data
> > +	 * is 'spi_number - 32'
> >  	 */
> > -	if (readl_relaxed(v2m->base + V2M_MSI_IIDR) == XGENE_GICV2M_MSI_IIDR)
> > +	if (readl_relaxed(v2m->base + V2M_MSI_IIDR) == XGENE_GICV2M_MSI_IIDR) {
> > +		v2m->flags |= GICV2M_NEEDS_SPI_OFFSET;
> > +		v2m->spi_offset = v2m->spi_start;
> > +	} else if (readl_relaxed(v2m->base + V2M_MSI_IIDR) ==
> > +		   BCM_NS2_GICV2M_MSI_IIDR) {
> >  		v2m->flags |= GICV2M_NEEDS_SPI_OFFSET;
> > +		v2m->spi_offset = 32;
> > +	}
>
> Can we make this a bit less ugly?
>
> 	switch (readl_relaxed(v2m->base + V2M_MSI_IIDR)) {
> 	case XGENE_GICV2M_MSI_IIDR:
> 		v2m->flags |= GICV2M_NEEDS_SPI_OFFSET;
> 		v2m->spi_offset = v2m->spi_start;
> 		break;
> 	case BCM_NS2_GICV2M_MSI_IIDR:
> 		v2m->flags |= GICV2M_NEEDS_SPI_OFFSET;
> 		v2m->spi_offset = 32;
> 		break;
> 	}

Okay will do that!

> Thanks,
>
> 	M.

Thanks,

Ray