[PATCH v3 20/25] irqchip/gic-v5: Add GICv5 PPI support

[Lorenzo Pieralisi]

On Wed, May 07, 2025 at 04:57:07PM +0200, Thomas Gleixner wrote:
> On Wed, May 07 2025 at 14:52, Marc Zyngier wrote:
> > On Wed, 07 May 2025 14:42:42 +0100,
> > Thomas Gleixner <tglx at linutronix.de> wrote:
> >> 
> >> On Wed, May 07 2025 at 10:14, Marc Zyngier wrote:
> >> > On Tue, 06 May 2025 16:00:31 +0100,
> >> > Thomas Gleixner <tglx at linutronix.de> wrote:
> >> >> 
> >> >> How does this test distinguish between LEVEL_LOW and LEVEL_HIGH? It only
> >> >> tests for level, no? So the test is interesting at best ...
> >> >
> >> > There is no distinction between HIGH and LOW, RISING and FALLING, in
> >> > any revision of the GIC architecture.
> >> 
> >> Then pretending that there is a set_type() functionality is pretty daft
> >
> > You still need to distinguish between level and edge when this is
> > programmable (which is the case for a subset of the PPIs).
> 
> Fair enough, but can we please add a comment to this function which
> explains this oddity.

Getting back to this, I would need your/Marc's input on this.

I think it is fair to remove the irq_set_type() irqchip callback for
GICv5 PPIs because there is nothing to set, as I said handling mode
for these IRQs is fixed. I don't think this can cause any trouble
(IIUC a value within the IRQF_TRIGGER_MASK should be set on requesting
an IRQ to "force" the trigger to be programmed and even then core code
would not fail if the irq_set_type() irqchip callback is not
implemented).

I am thinking about *existing* drivers that request GICv3 PPIs with
values in IRQF_TRIGGER_MASK set (are there any ? Don't think so but you
know better than I do), when we switch over to GICv5 we would have no
irq_set_type() callback for PPIs but I think we are still fine, not
implementing irqchip.irq_set_type() is correct IMO.

On the other hand, given that on GICv5 PPI handling mode is fixed,
do you think that in the ppi_irq_domain_ops.translate() callback,
I should check the type the firmware provided and fail the translation
if it does not match the HW hardcoded value ?

Obviously if firmware exposes the wrong type that's a firmware bug
but I was wondering whether it is better to fail the firmware-to-Linux
IRQ translation if the firmware provided type is wrong rather than carry
on pretending that the type is correct (I was abusing the irq_set_type()
callback to do just that - namely, check that the type provided by
firmware matches HW but I think that's the wrong place to put it).

Thanks !
Lorenzo