Re: [PATCH 4/9] irqchip/gic-v3: Add ESPI range support

[Lokesh Vutla <lokeshvutla@ti.com>]

On 23/07/19 4:14 PM, Marc Zyngier wrote:
> Add the required support for the ESPI range, which behave exactly like
> the SPIs of old, only with new funky INTIDs.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/irqchip/irq-gic-v3.c       | 85 ++++++++++++++++++++++++------
>  include/linux/irqchip/arm-gic-v3.h | 17 +++++-
>  2 files changed, 85 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 2371e0a70215..d328a8de533f 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -51,13 +51,16 @@ struct gic_chip_data {
>  	u32			nr_redist_regions;
>  	u64			flags;
>  	bool			has_rss;
> -	unsigned int		irq_nr;
>  	struct partition_desc	*ppi_descs[16];
>  };
>  
>  static struct gic_chip_data gic_data __read_mostly;
>  static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
>  
> +#define GIC_ID_NR	(1U << GICD_TYPER_ID_BITS(gic_data.rdists.gicd_typer))
> +#define GIC_LINE_NR	GICD_TYPER_SPIS(gic_data.rdists.gicd_typer)
> +#define GIC_ESPI_NR	GICD_TYPER_ESPIS(gic_data.rdists.gicd_typer)
> +
>  /*
>   * The behaviours of RPR and PMR registers differ depending on the value of
>   * SCR_EL3.FIQ, and the behaviour of non-secure priority registers of the
> @@ -100,6 +103,7 @@ static DEFINE_PER_CPU(bool, has_rss);
>  enum gic_intid_range {
>  	PPI_RANGE,
>  	SPI_RANGE,
> +	ESPI_RANGE,
>  	LPI_RANGE,
>  	__INVALID_RANGE__
>  };
> @@ -111,6 +115,8 @@ static enum gic_intid_range __get_intid_range(irq_hw_number_t hwirq)
>  		return PPI_RANGE;
>  	case 32 ... 1019:
>  		return SPI_RANGE;
> +	case ESPI_BASE_INTID ... 8191:

as per dt documentation, shouldn't the range be
	case ESPI_BASE_INTID ... 5119:

> +		return ESPI_RANGE;
>  	case 8192 ... GENMASK(23, 0):
>  		return LPI_RANGE;
>  	default:
> @@ -141,6 +147,7 @@ static inline void __iomem *gic_dist_base(struct irq_data *d)
>  		return gic_data_rdist_sgi_base();
>  
>  	case SPI_RANGE:
> +	case ESPI_RANGE:
>  		/* SPI -> dist_base */
>  		return gic_data.dist_base;
>  
> @@ -234,6 +241,31 @@ static u32 convert_offset_index(struct irq_data *d, u32 offset, u32 *index)
>  	case SPI_RANGE:
>  		*index = d->hwirq;
>  		return offset;
> +	case ESPI_RANGE:
> +		*index = d->hwirq - ESPI_BASE_INTID;
> +		switch (offset) {
> +		case GICD_ISENABLER:
> +			return GICD_ISENABLERnE;
> +		case GICD_ICENABLER:
> +			return GICD_ICENABLERnE;
> +		case GICD_ISPENDR:
> +			return GICD_ISPENDRnE;
> +		case GICD_ICPENDR:
> +			return GICD_ICPENDRnE;
> +		case GICD_ISACTIVER:
> +			return GICD_ISACTIVERnE;
> +		case GICD_ICACTIVER:
> +			return GICD_ICACTIVERnE;
> +		case GICD_IPRIORITYR:
> +			return GICD_IPRIORITYRnE;
> +		case GICD_ICFGR:
> +			return GICD_ICFGRnE;
> +		case GICD_IROUTER:
> +			return GICD_IROUTERnE;
> +		default:
> +			break;
> +		}
> +		break;
>  	default:
>  		break;
>  	}
> @@ -316,7 +348,7 @@ static int gic_irq_set_irqchip_state(struct irq_data *d,
>  {
>  	u32 reg;
>  
> -	if (d->hwirq >= gic_data.irq_nr) /* PPI/SPI only */
> +	if (d->hwirq >= 8192) /* PPI/SPI only */
>  		return -EINVAL;
>  
>  	switch (which) {
> @@ -343,7 +375,7 @@ static int gic_irq_set_irqchip_state(struct irq_data *d,
>  static int gic_irq_get_irqchip_state(struct irq_data *d,
>  				     enum irqchip_irq_state which, bool *val)
>  {
> -	if (d->hwirq >= gic_data.irq_nr) /* PPI/SPI only */
> +	if (d->hwirq >= 8192) /* PPI/SPI only */
>  		return -EINVAL;
>  
>  	switch (which) {
> @@ -567,7 +599,12 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
>  		gic_arch_enable_irqs();
>  	}
>  
> -	if (likely(irqnr > 15 && irqnr < 1020) || irqnr >= 8192) {
> +	/* Check for special IDs first */
> +	if ((irqnr >= 1020 && irqnr <= 1023))
> +		return;

May be I am missing something here, what is special about these 4 interrupts? or
you meant to check for reserved range here?

Thanks and regards,
Lokesh