Re: [PATCH v6 21/31] irqchip/gic-v5: Add GICv5 IRS/SPI support

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

There are more ID masks not using FIELD_PREP() in here, but
as mentioned in earlier reply, even if we decide those are worth
cleaning up, no reason it can't be as a trivial patch on top of the
series.

Otherwise just one comment walking back some feedback on an earlier
patch (as what you had there now makes sense)

Jonathan

> diff --git a/drivers/irqchip/irq-gic-v5-irs.c b/drivers/irqchip/irq-gic-v5-irs.c
> new file mode 100644
> index 000000000000..fba8efceb26e
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v5-irs.c
> @@ -0,0 +1,434 @@

> -static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> -					  struct irq_fwspec *fwspec,
> -					  irq_hw_number_t *hwirq,
> -					  unsigned int *type)
> +static const struct irq_chip gicv5_spi_irq_chip = {
> +	.name			= "GICv5-SPI",
> +	.irq_mask		= gicv5_spi_irq_mask,
> +	.irq_unmask		= gicv5_spi_irq_unmask,
> +	.irq_eoi		= gicv5_spi_irq_eoi,
> +	.irq_set_type		= gicv5_spi_irq_set_type,
> +	.irq_set_affinity	= gicv5_spi_irq_set_affinity,
> +	.irq_retrigger		= gicv5_spi_irq_retrigger,
> +	.irq_get_irqchip_state	= gicv5_spi_irq_get_irqchip_state,
> +	.irq_set_irqchip_state	= gicv5_spi_irq_set_irqchip_state,
> +	.flags			= IRQCHIP_SET_TYPE_MASKED |
> +				  IRQCHIP_SKIP_SET_WAKE	  |
> +				  IRQCHIP_MASK_ON_SUSPEND,
> +};
> +
> +static __always_inline int gicv5_irq_domain_translate(struct irq_domain *d,
> +						      struct irq_fwspec *fwspec,
> +						      irq_hw_number_t *hwirq,
> +						      unsigned int *type,
> +						      const u8 hwirq_typ)

>  {
>  	if (!is_of_node(fwspec->fwnode))
>  		return -EINVAL;
> @@ -235,20 +428,39 @@ static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
>  	if (fwspec->param_count < 3)
>  		return -EINVAL;
>  
> -	if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> +	if (fwspec->param[0] != hwirq_type)
>  		return -EINVAL;
>  
>  	*hwirq = fwspec->param[1];
>  
> -	/*
> -	 * Handling mode is hardcoded for PPIs, set the type using
> -	 * HW reported value.
> -	 */
> -	*type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> +	switch (hwirq_type) {
> +	case GICV5_HWIRQ_TYPE_PPI:
> +		/*
> +		 * Handling mode is hardcoded for PPIs, set the type using
> +		 * HW reported value.
> +		 */
> +		*type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW :
> +							 IRQ_TYPE_EDGE_RISING;
> +		break;
> +	case GICV5_HWIRQ_TYPE_SPI:
> +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;

Ah fair enough in earlier patches on just enforcing 3 parameters for all cases.
Seems like a sensible simplification once this is taken into account. So ignore that one!

> +		break;
> +	default:
> +		BUILD_BUG_ON(1);
> +	}
>  
>  	return 0;
>  }