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

[Marc Zyngier <maz@kernel.org>]

On Fri, 09 May 2025 09:35:25 +0100,
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> 
> On Fri, May 09, 2025 at 10:07:44AM +0200, Lorenzo Pieralisi wrote:
> > On Thu, May 08, 2025 at 12:44:45PM +0200, Lorenzo Pieralisi wrote:
> > 
> > [...]
> > 
> > > I noticed that, if the irq_set_type() function is not implemented,
> > > we don't execute (in __irq_set_trigger()):
> > > 
> > > irq_settings_set_level(desc);
> > > irqd_set(&desc->irq_data, IRQD_LEVEL);
> > 
> > I don't get why the settings above are written only if the irqchip
> > has an irq_set_type() method, maybe they should be updated in
> > irqdomain code (?) where:
> > 
> > irqd_set_trigger_type()
> > 
> > is executed after creating the fwspec mapping ?
> > 
> > Is it possible we never noticed because we have always had irqchips that
> > do implement irq_set_type() ?
> > 
> > Again, I don't know the history behind the IRQD_LEVEL flag so it is just
> > a question, I'd need to get this clarified though please if I remove the
> > PPI irq_set_type() callback.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/52xx/mpc52xx_pic.c?h=v6.15-rc5#n218
> 
> There are other examples in powerpc, this does not look right to me.

I don't see what's wrong with this, given that PPC is about 15 years
behind the curve when it comes to interrupt management. Better than
Sparc or Alpha, though. So they do whatever was possible at the time
this code was written.

It doesn't mean that you need to align with the worse we have in the tree!

	M.

-- 
Without deviation from the norm, progress is not possible.