| Message ID | c604a8ac-8025-4078-ab90-834d95872e31@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | ata: ahci: Don't call pci_intx() directly | expand |
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@redhat.com/ Kind regards, Niklas
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@redhat.com/ > 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. > > Kind regards, > Niklas Heiner
On Mon, Nov 04, 2024 at 02:23:43PM +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@redhat.com/ > > > 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. Well, if you look at Philipp's patch it: 1) Doesn't only update drivers/ata/ahci.c, it also updates: drivers/ata/ata_piix.c drivers/ata/pata_rdc.c drivers/ata/sata_sil24.c drivers/ata/sata_sis.c drivers/ata/sata_uli.c drivers/ata/sata_vsc.c Why don't you update the other drivers in drivers/ata/* ? 2) Doesn't just bother to fix a single subsystem (drivers/ata/), it is actually part of a series that fixes all affected subsystems. Why don't you send out this fix as part of a series that fixes all the affected subsystems? Kind regards, Niklas
On 04.11.2024 19:36, Niklas Cassel wrote: > On Mon, Nov 04, 2024 at 02:23:43PM +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@redhat.com/ >>> >> 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. > > Well, if you look at Philipp's patch it: > > 1) Doesn't only update drivers/ata/ahci.c, > it also updates: > drivers/ata/ata_piix.c > drivers/ata/pata_rdc.c > drivers/ata/sata_sil24.c > drivers/ata/sata_sis.c > drivers/ata/sata_uli.c > drivers/ata/sata_vsc.c > > Why don't you update the other drivers in drivers/ata/* ? > Because I don't have hw for testing the changes and usually I'm somewhat reluctant to submit patches which are compile-tested only. > > 2) Doesn't just bother to fix a single subsystem (drivers/ata/), > it is actually part of a series that fixes all affected subsystems. > > Why don't you send out this fix as part of a series that fixes all the > affected subsystems? > Because for some drivers it's complex (e.g. bnx2x) and I don't have hw to test the changes. > > Kind regards, > Niklas
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 2d3d3d67b..09090294c 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1985,7 +1985,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) if (ahci_init_msi(pdev, n_ports, hpriv) < 0) { /* legacy intx interrupts */ - pci_intx(pdev, 1); + pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX); } hpriv->irq = pci_irq_vector(pdev, 0);
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. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/ata/ahci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)