Re: [PATCH 1/1] PCI/PME+pciehp: Request IRQF_ONESHOT because bwctrl shares IRQ

[Ilpo Järvinen]

On Fri, 15 Nov 2024, Lukas Wunner wrote:

> On Thu, Nov 14, 2024 at 04:20:34PM +0200, Ilpo Järvinen wrote:
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -68,7 +68,8 @@ static inline int pciehp_request_irq(struct controller *ctrl)
> >  
> >  	/* Installs the interrupt handler */
> >  	retval = request_threaded_irq(irq, pciehp_isr, pciehp_ist,
> > -				      IRQF_SHARED, "pciehp", ctrl);
> > +				      IRQF_SHARED | IRQF_ONESHOT,
> > +				      "pciehp", ctrl);
> >  	if (retval)
> >  		ctrl_err(ctrl, "Cannot get irq %d for the hotplug controller\n",
> >  			 irq);
> 
> I don't think this will work.  The IRQ thread pciehp_ist() may write
> to the Slot Control register and await a Command Completed event,
> e.g. when turning Slot Power on/off, changing LEDs, etc.
> 
> What happens then is, the hardware sets the Command Completed bit in
> the Slot Status register and signals an interrupt.  The hardirq handler
> pciehp_isr() reads the Slot Status register, acknowledges the
> Command Completed event, sets "ctrl->cmd_busy = 0" and wakes up the
> waiting IRQ thread.
> 
> In other words, pciehp does need the interrupt to stay enabled while
> the IRQ thread is running so that the hardirq handler can receive
> Command Completed interrupts.
> 
> Note that DPC also does not use IRQF_ONESHOT, so you'd have to change
> that as well in this patch.  The Raspberry Pi happens to not support
> DPC, so Stefan didn't see an error related to it.
> 
> I'm afraid you need to amend bwctrl to work without IRQF_ONESHOT rather
> than changing all the others.

That isn't complicated. The current irq thread handler is simple enough 
that it will just work as hardirq handler without any changes.

-- 
 i.