Message-ID: <297b5511-8f6a-4798-a2b4-d4c634969aed@gmail.com>
Date: Wed, 16 Oct 2024 10:43:27 +0200
From: Heiner Kallweit <hkallweit1@...il.com>
To: Philipp Stanner <pstanner@...hat.com>,
 Alex Williamson <alex.williamson@...hat.com>
Cc: Damien Le Moal <dlemoal@...nel.org>, Niklas Cassel <cassel@...nel.org>,
 Sergey Shtylyov <s.shtylyov@....ru>,
 Basavaraj Natikar <basavaraj.natikar@....com>, Jiri Kosina
 <jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>,
 Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Alex Dubov
 <oakad@...oo.com>, Sudarsana Kalluru <skalluru@...vell.com>,
 Manish Chopra <manishc@...vell.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rasesh Mody <rmody@...vell.com>, GR-Linux-NIC-Dev@...vell.com,
 Igor Mitsyanko <imitsyanko@...ntenna.com>,
 Sergey Matyukevich <geomatsi@...il.com>, Kalle Valo <kvalo@...nel.org>,
 Sanjay R Mehta <sanju.mehta@....com>,
 Shyam Sundar S K <Shyam-sundar.S-k@....com>, Jon Mason <jdmason@...zu.us>,
 Dave Jiang <dave.jiang@...el.com>, Allen Hubbe <allenbh@...il.com>,
 Bjorn Helgaas <bhelgaas@...gle.com>, Juergen Gross <jgross@...e.com>,
 Stefano Stabellini <sstabellini@...nel.org>,
 Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
 Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
 Chen Ni <nichen@...as.ac.cn>, Mario Limonciello <mario.limonciello@....com>,
 Ricky Wu <ricky_wu@...ltek.com>, Al Viro <viro@...iv.linux.org.uk>,
 Breno Leitao <leitao@...ian.org>, Kevin Tian <kevin.tian@...el.com>,
 Thomas Gleixner <tglx@...utronix.de>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Mostafa Saleh <smostafa@...gle.com>, Jason Gunthorpe <jgg@...pe.ca>,
 Yi Liu <yi.l.liu@...el.com>, Christian Brauner <brauner@...nel.org>,
 Ankit Agrawal <ankita@...dia.com>, Eric Auger <eric.auger@...hat.com>,
 Reinette Chatre <reinette.chatre@...el.com>, Ye Bin <yebin10@...wei.com>,
 Marek Marczykowski-Górecki
 <marmarek@...isiblethingslab.com>,
 Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>,
 Peter Ujfalusi <peter.ujfalusi@...ux.intel.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
 Rui Salvaterra <rsalvaterra@...il.com>, linux-ide@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
 netdev@...r.kernel.org, linux-wireless@...r.kernel.org, ntb@...ts.linux.dev,
 linux-pci@...r.kernel.org, kvm@...r.kernel.org,
 xen-devel@...ts.xenproject.org, linux-sound@...r.kernel.org
Subject: Re: [PATCH 13/13] PCI: Deprecate pci_intx(), pcim_intx()

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@...hat.com> 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.

> 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@...hat.com>
>>> ---
>>>  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)
>>>  {
>>
> 
> 

Powered by blists - more mailing lists