Re: [PATCH v3 05/10] s390/pci: Restore IRQ unconditionally for the zPCI device

[Niklas Schnelle]

On Mon, 2025-09-15 at 10:42 -0700, Farhan Ali wrote:
> On 9/15/2025 1:39 AM, Niklas Schnelle wrote:
> > On Thu, 2025-09-11 at 11:33 -0700, Farhan Ali wrote:
> > > Commit c1e18c17bda6 ("s390/pci: add zpci_set_irq()/zpci_clear_irq()"),
> > > introduced the zpci_set_irq() and zpci_clear_irq(), to be used while
> > > resetting a zPCI device.
> > > 
> > > Commit da995d538d3a ("s390/pci: implement reset_slot for hotplug slot"),
> > > mentions zpci_clear_irq() being called in the path for zpci_hot_reset_device().
> > > But that is not the case anymore and these functions are not called
> > > outside of this file.
> > If you're doing another version I think you could add a bit more
> > information on why this still works for existing recovery based on my
> > investigation in
> > https://lore.kernel.org/lkml/052ebdbb6f2d38025ca4345ee51e4857e19bb0e4.camel@xxxxxxxxxxxxx/
> > 
> > Even if you don't add more explanations, I'd tend to just drop the
> > above paragraph as it doesn't seem relevant and sounds like
> > zpci_hot_reset_device() doesn't clear IRQs. As explained in the linked
> > mail there really is no need to call zpci_clear_irq() in
> > zpci_hot_reset_device() as the CLP disable does disable IRQs. It's
> > really only the state tracking that can get screwed up but is also fine
> > for drivers which end up doing the tear down.
> 
> I referenced commit da995d538d3a as that commit introduced the 
> arch_restore_msi_irqs and describes the reasoning as to why we need it. 
> It also mentions about zpci_clear_irq being called by 
> zpci_hot_reset_device. IMHO the message was confusing as it took me my 
> down the path of trying to identify any commit that changed the behavior 
> since da995d538d3a. But that wasn't the case and it was an error in the 
> commit message. I want to keep a reference here to at least clarify that.

Ok that makes sense, maybe you could add asentence like:
"… these functions are not called outside of this file. Instead
zpci_hot_reset_device() relies on zpci_disable_device() also clearing
the IRQs, but misses to reset the zdev->irqs_registered flag.