[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 13/13] PCI: Deprecate pci_intx(), pcim_intx()
On Wed, 2024-10-16 at 10:43 +0200, Heiner Kallweit wrote: > On 16.10.2024 08:57, Philipp Stanner wrote: > > On Tue, 2024-10-15 at 13:53 -0600, Alex Williamson wrote: > > > On Tue, 15 Oct 2024 20:51:23 +0200 > > > Philipp Stanner <pstanner@xxxxxxxxxx> wrote: > > > > > > > pci_intx() and its managed counterpart pcim_intx() only exist > > > > for > > > > older > > > > drivers which have not been ported yet for various reasons. > > > > Future > > > > drivers should preferably use pci_alloc_irq_vectors(). > > > > > > > > Mark pci_intx() and pcim_intx() as deprecated and encourage > > > > usage > > > > of > > > > pci_alloc_irq_vectors() in its place. > > > > > > I don't really understand this. As we've discussed previously > > > pci_alloc_irq_vectors() is, unsurprisingly, for allocating PCI > > > IRQ > > > vectors while pci_intx() is for manipulating the INTx disable bit > > > on > > > PCI devices. The latter is a generic mechanism for preventing > > > PCI > > > devices from generating INTx, regardless of whether there's a > > > vector > > > allocated for it. How does the former replace the latter and why > > > do > > > we > > > feel the need to deprecate the latter? > > > > > > It feels like this fits some narrow narrative and makes all users > > > of > > > these now deprecated functions second class citizens. Why? At > > > it's > > > root these are simply providing mask and set or mask and clear > > > register > > > bit operations. Thanks, > > > > I got the feeling from the RFC discussion that that was basically > > the > > consensus: people should use pci_alloc_irq_vectors(). Or did I > > misunderstand Andy and Heiner? > > > I think there are two different use cases for pci_intx(). > At first there are several drivers where the direct usage of > pci_intx() > can be eliminated by switching to the pci_alloc_irq_vectors() API. > > And then there's usage of pci_intx() in > drivers/vfio/pci/vfio_pci_intrs.c > drivers/xen/xen-pciback/conf_space_header.c > There we have to keep the (AFAICS unmanaged) pci_intx() calls. There is also the usage within PCI itself, in MSI. Patch №8 touches that. It's why I think this series should land before anyone should port direct pci_intx() users to the irq vectors function, because the latter also uses pci_intx() and its own devres, which sounds explosive to me. P. > > > I'm perfectly happy with dropping this patch and continue offering > > pci{m}_intx() to users, since after removing that hybrid hazzard I > > don't see any harm in them anymore. > > > > > > P. > > > > > > > > Alex > > > > > > > Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx> > > > > --- > > > > drivers/pci/devres.c | 5 ++++- > > > > drivers/pci/pci.c | 5 ++++- > > > > 2 files changed, 8 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > > > > index 6f8f712fe34e..4c76fc063104 100644 > > > > --- a/drivers/pci/devres.c > > > > +++ b/drivers/pci/devres.c > > > > @@ -435,7 +435,7 @@ static struct pcim_intx_devres > > > > *get_or_create_intx_devres(struct device *dev) > > > > } > > > > > > > > /** > > > > - * pcim_intx - managed pci_intx() > > > > + * pcim_intx - managed pci_intx() (DEPRECATED) > > > > * @pdev: the PCI device to operate on > > > > * @enable: boolean: whether to enable or disable PCI INTx > > > > * > > > > @@ -443,6 +443,9 @@ static struct pcim_intx_devres > > > > *get_or_create_intx_devres(struct device *dev) > > > > * > > > > * Enable/disable PCI INTx for device @pdev. > > > > * Restore the original state on driver detach. > > > > + * > > > > + * This function is DEPRECATED. Do not use it in new code. > > > > + * Use pci_alloc_irq_vectors() instead (there is no managed > > > > version, currently). > > > > */ > > > > int pcim_intx(struct pci_dev *pdev, int enable) > > > > { > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > > index 7ce1d0e3a1d5..dc69e23b8982 100644 > > > > --- a/drivers/pci/pci.c > > > > +++ b/drivers/pci/pci.c > > > > @@ -4477,11 +4477,14 @@ void pci_disable_parity(struct pci_dev > > > > *dev) > > > > } > > > > > > > > /** > > > > - * pci_intx - enables/disables PCI INTx for device dev > > > > + * pci_intx - enables/disables PCI INTx for device dev > > > > (DEPRECATED) > > > > * @pdev: the PCI device to operate on > > > > * @enable: boolean: whether to enable or disable PCI INTx > > > > * > > > > * Enables/disables PCI INTx for device @pdev > > > > + * > > > > + * This function is DEPRECATED. Do not use it in new code. > > > > + * Use pci_alloc_irq_vectors() instead. > > > > */ > > > > void pci_intx(struct pci_dev *pdev, int enable) > > > > { > > > > > > > >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |