On 05.11.2024 16:22, Philipp Stanner wrote: > On Mon, 2024-11-04 at 14:23 +0100, Heiner Kallweit wrote: >> On 04.11.2024 09:30, Niklas Cassel wrote: >>> On Fri, Nov 01, 2024 at 11:38:53PM +0100, Heiner Kallweit wrote: >>>> pci_intx() should be called by PCI core and some virtualization >>>> code >>>> only. In PCI device drivers use the appropriate >>>> pci_alloc_irq_vectors() >>>> call. >>> >>> Hello Heiner, >>> >>> as you might or might not know, this patch conflicts with a >>> Philipp's >>> already acked patch: >>> https://lore.kernel.org/linux-ide/20241015185124.64726-10-pstanner@xxxxxxxxxx/ >>> >> I know, therefore he's on cc. Fully migrating PCI device drivers to >> the >> pci_alloc_irq_vectors() should be done anyway and is the cleaner >> alternative to changing pci_intx(). However for some drivers this is >> a rather >> complex task, therefore I understand Philipp's approach to adjust >> pci_intx() >> first. He's incorporating other review feedback in his series, so >> with the >> next re-spin he could remove the ahci patch from his series. > > As I have stated before, this is not just about cleaning up pci_intx(). > > Again: > pci_alloc_irq_vectors() USES pci_intx(), and my series does address > that in its MSI patch. > That's clear. The point is that in the end only PCI core and some virtualization stuff should use a low-level function like pci_intx(). Device drivers should never use pci_intx() directly, managed or not. I think all the hassle with managed intx could be avoided if PCI core would unconditionally reset register PCI_COMMAND (or at least the most relevant bits) to the initial value on driver exit. > If you want to help, a careful review of the MSI bits would be helpful. > Your patch here uses pci_intx(), you just don't see it anymore. > > That said, in principle it's of course possible for me to drop patches > while we're still in review, but I tend to think that it's causing both > you and me more work if the pci_intx() vs. pci_alloc_irq_vectors() is > worked on out of all times right now. > > It also causes more work load for the reviewers, since they'd have > reviewed my patch for nothing and would have to review yours then. > > Also be aware that I am not yet sure whether the devres aspect should > also be removed or made more explicit in MSI. Take a look at > pcim_setup_msi_release(). > > In the worst case you might just replace one problem with another > before we figured it all out. > > Regards, > P. > >>> >>> Kind regards, >>> Niklas >> >> Heiner >> >
