Re: [PATCH 13/13] PCI: Deprecate pci_intx(), pcim_intx()

[Philipp Stanner <pstanner@xxxxxxxxxx>]

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'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)
> >  {
>