| Message ID | 4c3e7a50d6e70f408812cd7ab199c6b4b326f9de.1676408572.git.reinette.chatre@intel.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [RFC] PCI/MSI: Clarify usage of pci_msix_free_irq() | expand |
> From: Chatre, Reinette <reinette.chatre@intel.com> > Sent: Wednesday, February 15, 2023 5:13 AM > > pci_msix_free_irq() is used to free an interrupt on > a PCI/MSI-X interrupt domain. > > The API description specifies that the interrupt to > be freed was allocated via pci_msix_alloc_irq_at(). > This description limits the usage of pci_msix_free_irq() > since pci_msix_free_irq() can also be used to free > MSI-X interrupts allocated with, for example, > pci_alloc_irq_vectors(). > > Remove the text stating that the interrupt to be freed had > to be allocated with pci_msix_alloc_irq_at(). The needed > struct msi_map need not be from pci_msix_alloc_irq_at() > but can be created from scratch using pci_irq_vector() > to obtain the Linux IRQ number. Highlight that > pci_msix_free_irq() cannot be used to disable MSI-X to > guide users that, for example, pci_free_irq_vectors() > remains to be needed. > > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> > --- > Dear Maintainers, > > Is my understanding correct? > > For example, from my understanding the following is > correct API usage: > > /* > * Enable MSI-X and allocate num interrupts. > */ > pci_alloc_irq_vectors() > > /* > * Free one of the interrupts allocated via pci_alloc_irq_vectors(). > * Possibly called num times, but not required. > */ > pci_msix_free_irq() > > /* > * Ensure all remaining interrupts freed and MSI-X disabled. > */ > pci_free_irq_vectors() > > This could be interpreted as an asymmetric usage of the API, yet > found to be practical when trying to use these new calls. > > Your feedback is appreciated. I asked same question before. Thomas gave exactly same thought. [1] " The preallocated descriptors are only relevant during setup, but not post setup. So yes, you can do: pci_alloc_irq_vectors(dev, 10, 10, PCI_IRQ_MSIX); and then map = { .index = 9, .virq = $IRQ }; pci_msix_free_irq(dev, &map); It just works. " but looks the comment was not adjusted accordingly. So, Reviewed-by: Kevin Tian <kevin.tian@intel.com> [1] https://lore.kernel.org/lkml/87r0xsd8j4.ffs@tglx/ > > Regards, > > Reinette > > drivers/pci/msi/api.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c > index b8009aa11f3c..be679aa5db64 100644 > --- a/drivers/pci/msi/api.c > +++ b/drivers/pci/msi/api.c > @@ -163,11 +163,11 @@ EXPORT_SYMBOL_GPL(pci_msix_alloc_irq_at); > > /** > * pci_msix_free_irq - Free an interrupt on a PCI/MSIX interrupt domain > - * which was allocated via pci_msix_alloc_irq_at() > * > * @dev: The PCI device to operate on > * @map: A struct msi_map describing the interrupt to free > - * as returned from the allocation function. > + * > + * Undo an interrupt vector allocation. Does not disable MSI-X. > */ > void pci_msix_free_irq(struct pci_dev *dev, struct msi_map map) > { > -- > 2.34.1
diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c index b8009aa11f3c..be679aa5db64 100644 --- a/drivers/pci/msi/api.c +++ b/drivers/pci/msi/api.c @@ -163,11 +163,11 @@ EXPORT_SYMBOL_GPL(pci_msix_alloc_irq_at); /** * pci_msix_free_irq - Free an interrupt on a PCI/MSIX interrupt domain - * which was allocated via pci_msix_alloc_irq_at() * * @dev: The PCI device to operate on * @map: A struct msi_map describing the interrupt to free - * as returned from the allocation function. + * + * Undo an interrupt vector allocation. Does not disable MSI-X. */ void pci_msix_free_irq(struct pci_dev *dev, struct msi_map map) {
pci_msix_free_irq() is used to free an interrupt on a PCI/MSI-X interrupt domain. The API description specifies that the interrupt to be freed was allocated via pci_msix_alloc_irq_at(). This description limits the usage of pci_msix_free_irq() since pci_msix_free_irq() can also be used to free MSI-X interrupts allocated with, for example, pci_alloc_irq_vectors(). Remove the text stating that the interrupt to be freed had to be allocated with pci_msix_alloc_irq_at(). The needed struct msi_map need not be from pci_msix_alloc_irq_at() but can be created from scratch using pci_irq_vector() to obtain the Linux IRQ number. Highlight that pci_msix_free_irq() cannot be used to disable MSI-X to guide users that, for example, pci_free_irq_vectors() remains to be needed. Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> --- Dear Maintainers, Is my understanding correct? For example, from my understanding the following is correct API usage: /* * Enable MSI-X and allocate num interrupts. */ pci_alloc_irq_vectors() /* * Free one of the interrupts allocated via pci_alloc_irq_vectors(). * Possibly called num times, but not required. */ pci_msix_free_irq() /* * Ensure all remaining interrupts freed and MSI-X disabled. */ pci_free_irq_vectors() This could be interpreted as an asymmetric usage of the API, yet found to be practical when trying to use these new calls. Your feedback is appreciated. Regards, Reinette drivers/pci/msi/api.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)