| Message ID | 20210226155056.1068534-2-zhengdejin5@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Introduce pcim_alloc_irq_vectors() | expand |
On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote: > Introduce pcim_alloc_irq_vectors(), a device-managed version of > pci_alloc_irq_vectors(). Introducing this function can simplify > the error handling path in many drivers. > > And use pci_free_irq_vectors() to replace some code in pcim_release(), > they are equivalent, and no functional change. It is more explicit > that pcim_alloc_irq_vectors() is a device-managed function. Thanks! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> > --- > v4 -> v5: > - Remove the check of enable device in pcim_alloc_irq_vectors() > and make it as a static line function. > v3 -> v4: > - No change > v2 -> v3: > - Add some commit comments for replace some codes in > pcim_release() by pci_free_irq_vectors(). > v1 -> v2: > - Use pci_free_irq_vectors() to replace some code in > pcim_release(). > - Modify some commit messages. > > drivers/pci/pci.c | 5 +---- > include/linux/pci.h | 24 ++++++++++++++++++++++++ > 2 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 16a17215f633..fecfdc0add2f 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void *res) > struct pci_devres *this = res; > int i; > > - if (dev->msi_enabled) > - pci_disable_msi(dev); > - if (dev->msix_enabled) > - pci_disable_msix(dev); > + pci_free_irq_vectors(dev); > > for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) > if (this->region_mask & (1 << i)) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 86c799c97b77..5cafd7d65fd7 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1818,6 +1818,30 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > NULL); > } > > +/** > + * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors() > + * @dev: PCI device to operate on > + * @min_vecs: minimum number of vectors required (must be >= 1) > + * @max_vecs: maximum (desired) number of vectors > + * @flags: flags or quirks for the allocation > + * > + * Return the number of vectors allocated, (which might be smaller than > + * @max_vecs) if successful, or a negative error code on error. If less > + * than @min_vecs interrupt vectors are available for @dev the function > + * will fail with -ENOSPC. > + * > + * It depends on calling pcim_enable_device() to make IRQ resources > + * manageable. > + */ > +static inline int > +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags) > +{ > + if (!pci_is_managed(dev)) > + return -EINVAL; > + return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags); > +} > + > /* Include architecture-dependent settings and functions */ > > #include <asm/pci.h> > -- > 2.25.0 >
On Fri, Feb 26, 2021 at 06:23:02PM +0200, Andy Shevchenko wrote: > On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote: > > Introduce pcim_alloc_irq_vectors(), a device-managed version of > > pci_alloc_irq_vectors(). Introducing this function can simplify > > the error handling path in many drivers. > > > > And use pci_free_irq_vectors() to replace some code in pcim_release(), > > they are equivalent, and no functional change. It is more explicit > > that pcim_alloc_irq_vectors() is a device-managed function. > > Thanks! > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Andy, Thanks! > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> > > --- > > v4 -> v5: > > - Remove the check of enable device in pcim_alloc_irq_vectors() > > and make it as a static line function. > > v3 -> v4: > > - No change > > v2 -> v3: > > - Add some commit comments for replace some codes in > > pcim_release() by pci_free_irq_vectors(). > > v1 -> v2: > > - Use pci_free_irq_vectors() to replace some code in > > pcim_release(). > > - Modify some commit messages. > > > > drivers/pci/pci.c | 5 +---- > > include/linux/pci.h | 24 ++++++++++++++++++++++++ > > 2 files changed, 25 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 16a17215f633..fecfdc0add2f 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void *res) > > struct pci_devres *this = res; > > int i; > > > > - if (dev->msi_enabled) > > - pci_disable_msi(dev); > > - if (dev->msix_enabled) > > - pci_disable_msix(dev); > > + pci_free_irq_vectors(dev); > > > > for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) > > if (this->region_mask & (1 << i)) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 86c799c97b77..5cafd7d65fd7 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -1818,6 +1818,30 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > > NULL); > > } > > > > +/** > > + * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors() > > + * @dev: PCI device to operate on > > + * @min_vecs: minimum number of vectors required (must be >= 1) > > + * @max_vecs: maximum (desired) number of vectors > > + * @flags: flags or quirks for the allocation > > + * > > + * Return the number of vectors allocated, (which might be smaller than > > + * @max_vecs) if successful, or a negative error code on error. If less > > + * than @min_vecs interrupt vectors are available for @dev the function > > + * will fail with -ENOSPC. > > + * > > + * It depends on calling pcim_enable_device() to make IRQ resources > > + * manageable. > > + */ > > +static inline int > > +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > > + unsigned int max_vecs, unsigned int flags) > > +{ > > + if (!pci_is_managed(dev)) > > + return -EINVAL; > > + return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags); > > +} > > + > > /* Include architecture-dependent settings and functions */ > > > > #include <asm/pci.h> > > -- > > 2.25.0 > > > > -- > With Best Regards, > Andy Shevchenko > >
[+cc Christoph, Thomas, Alexander, in case you're interested] [+cc Jonathan, Kurt, Logan: vmd.c and switchtec.c use managed resources and pci_alloc_irq_vectors()] On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote: > Introduce pcim_alloc_irq_vectors(), a device-managed version of > pci_alloc_irq_vectors(). Introducing this function can simplify > the error handling path in many drivers. > > And use pci_free_irq_vectors() to replace some code in pcim_release(), > they are equivalent, and no functional change. It is more explicit > that pcim_alloc_irq_vectors() is a device-managed function. > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> Let me know if you'd like me to take the series. > --- > v4 -> v5: > - Remove the check of enable device in pcim_alloc_irq_vectors() > and make it as a static line function. > v3 -> v4: > - No change > v2 -> v3: > - Add some commit comments for replace some codes in > pcim_release() by pci_free_irq_vectors(). > v1 -> v2: > - Use pci_free_irq_vectors() to replace some code in > pcim_release(). > - Modify some commit messages. > > drivers/pci/pci.c | 5 +---- > include/linux/pci.h | 24 ++++++++++++++++++++++++ > 2 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 16a17215f633..fecfdc0add2f 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void *res) > struct pci_devres *this = res; > int i; > > - if (dev->msi_enabled) > - pci_disable_msi(dev); > - if (dev->msix_enabled) > - pci_disable_msix(dev); > + pci_free_irq_vectors(dev); > > for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) > if (this->region_mask & (1 << i)) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 86c799c97b77..5cafd7d65fd7 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1818,6 +1818,30 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > NULL); > } > > +/** > + * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors() > + * @dev: PCI device to operate on > + * @min_vecs: minimum number of vectors required (must be >= 1) > + * @max_vecs: maximum (desired) number of vectors > + * @flags: flags or quirks for the allocation > + * > + * Return the number of vectors allocated, (which might be smaller than > + * @max_vecs) if successful, or a negative error code on error. If less > + * than @min_vecs interrupt vectors are available for @dev the function > + * will fail with -ENOSPC. > + * > + * It depends on calling pcim_enable_device() to make IRQ resources > + * manageable. > + */ > +static inline int > +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > + unsigned int max_vecs, unsigned int flags) > +{ > + if (!pci_is_managed(dev)) > + return -EINVAL; > + return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags); > +} > + > /* Include architecture-dependent settings and functions */ > > #include <asm/pci.h> > -- > 2.25.0 >
On Tue, Mar 23, 2021 at 05:47:10PM -0500, Bjorn Helgaas wrote: > [+cc Christoph, Thomas, Alexander, in case you're interested] > [+cc Jonathan, Kurt, Logan: vmd.c and switchtec.c use managed resources > and pci_alloc_irq_vectors()] > > On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote: > > Introduce pcim_alloc_irq_vectors(), a device-managed version of > > pci_alloc_irq_vectors(). Introducing this function can simplify > > the error handling path in many drivers. > > > > And use pci_free_irq_vectors() to replace some code in pcim_release(), > > they are equivalent, and no functional change. It is more explicit > > that pcim_alloc_irq_vectors() is a device-managed function. > > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > Let me know if you'd like me to take the series. > Hi Bjorn, These patches are still invisible on the mainline, could you help me to take it? Thanks very much! BR, Dejin > > --- > > v4 -> v5: > > - Remove the check of enable device in pcim_alloc_irq_vectors() > > and make it as a static line function. > > v3 -> v4: > > - No change > > v2 -> v3: > > - Add some commit comments for replace some codes in > > pcim_release() by pci_free_irq_vectors(). > > v1 -> v2: > > - Use pci_free_irq_vectors() to replace some code in > > pcim_release(). > > - Modify some commit messages. > > > > drivers/pci/pci.c | 5 +---- > > include/linux/pci.h | 24 ++++++++++++++++++++++++ > > 2 files changed, 25 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 16a17215f633..fecfdc0add2f 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void *res) > > struct pci_devres *this = res; > > int i; > > > > - if (dev->msi_enabled) > > - pci_disable_msi(dev); > > - if (dev->msix_enabled) > > - pci_disable_msix(dev); > > + pci_free_irq_vectors(dev); > > > > for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) > > if (this->region_mask & (1 << i)) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 86c799c97b77..5cafd7d65fd7 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -1818,6 +1818,30 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > > NULL); > > } > > > > +/** > > + * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors() > > + * @dev: PCI device to operate on > > + * @min_vecs: minimum number of vectors required (must be >= 1) > > + * @max_vecs: maximum (desired) number of vectors > > + * @flags: flags or quirks for the allocation > > + * > > + * Return the number of vectors allocated, (which might be smaller than > > + * @max_vecs) if successful, or a negative error code on error. If less > > + * than @min_vecs interrupt vectors are available for @dev the function > > + * will fail with -ENOSPC. > > + * > > + * It depends on calling pcim_enable_device() to make IRQ resources > > + * manageable. > > + */ > > +static inline int > > +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > > + unsigned int max_vecs, unsigned int flags) > > +{ > > + if (!pci_is_managed(dev)) > > + return -EINVAL; > > + return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags); > > +} > > + > > /* Include architecture-dependent settings and functions */ > > > > #include <asm/pci.h> > > -- > > 2.25.0 > >
On Thu, May 06, 2021 at 12:27:16AM +0800, Dejin Zheng wrote: > On Tue, Mar 23, 2021 at 05:47:10PM -0500, Bjorn Helgaas wrote: > > [+cc Christoph, Thomas, Alexander, in case you're interested] > > [+cc Jonathan, Kurt, Logan: vmd.c and switchtec.c use managed resources > > and pci_alloc_irq_vectors()] > > On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote: > > > Introduce pcim_alloc_irq_vectors(), a device-managed version of > > > pci_alloc_irq_vectors(). Introducing this function can simplify > > > the error handling path in many drivers. > > > > > > And use pci_free_irq_vectors() to replace some code in pcim_release(), > > > they are equivalent, and no functional change. It is more explicit > > > that pcim_alloc_irq_vectors() is a device-managed function. > > > > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> > > > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > > Let me know if you'd like me to take the series. > > > Hi Bjorn, > > These patches are still invisible on the mainline, could you help me to > take it? Thanks very much! I guess you have to rebase them on top of the latest rc (or PCI for-next) and send with a cover letter.
On Wed, Jun 02, 2021 at 11:51:10AM +0300, Andy Shevchenko wrote: > On Thu, May 06, 2021 at 12:27:16AM +0800, Dejin Zheng wrote: > > On Tue, Mar 23, 2021 at 05:47:10PM -0500, Bjorn Helgaas wrote: > > > [+cc Christoph, Thomas, Alexander, in case you're interested] > > > [+cc Jonathan, Kurt, Logan: vmd.c and switchtec.c use managed resources > > > and pci_alloc_irq_vectors()] > > > > On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote: > > > > Introduce pcim_alloc_irq_vectors(), a device-managed version of > > > > pci_alloc_irq_vectors(). Introducing this function can simplify > > > > the error handling path in many drivers. > > > > > > > > And use pci_free_irq_vectors() to replace some code in pcim_release(), > > > > they are equivalent, and no functional change. It is more explicit > > > > that pcim_alloc_irq_vectors() is a device-managed function. > > > > > > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> > > > > > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > > > > Let me know if you'd like me to take the series. > > > > > Hi Bjorn, > > > > These patches are still invisible on the mainline, could you help me to > > take it? Thanks very much! > > I guess you have to rebase them on top of the latest rc (or PCI for-next) and > send with a cover letter. > Andy, thanks for your reminder, I will do it. > -- > With Best Regards, > Andy Shevchenko > >
On Wed, Jun 02, 2021 at 11:51:10AM +0300, Andy Shevchenko wrote: > On Thu, May 06, 2021 at 12:27:16AM +0800, Dejin Zheng wrote: > > On Tue, Mar 23, 2021 at 05:47:10PM -0500, Bjorn Helgaas wrote: > > > [+cc Christoph, Thomas, Alexander, in case you're interested] > > > [+cc Jonathan, Kurt, Logan: vmd.c and switchtec.c use managed resources > > > and pci_alloc_irq_vectors()] > > > > On Fri, Feb 26, 2021 at 11:50:53PM +0800, Dejin Zheng wrote: > > > > Introduce pcim_alloc_irq_vectors(), a device-managed version of > > > > pci_alloc_irq_vectors(). Introducing this function can simplify > > > > the error handling path in many drivers. > > > > > > > > And use pci_free_irq_vectors() to replace some code in pcim_release(), > > > > they are equivalent, and no functional change. It is more explicit > > > > that pcim_alloc_irq_vectors() is a device-managed function. > > > > > > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> > > > > > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > > > > Let me know if you'd like me to take the series. > > > > > Hi Bjorn, > > > > These patches are still invisible on the mainline, could you help me to > > take it? Thanks very much! > > I guess you have to rebase them on top of the latest rc (or PCI for-next) and > send with a cover letter. No need to rebase them. The ideal is to base them on the "main" branch from https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/ (currently v5.13-rc) because that's what I base topic branches on. But I can deal with whatever your current base is. Thanks for the reminder; I'll take a look at your v7 posting. Bjorn
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 16a17215f633..fecfdc0add2f 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1969,10 +1969,7 @@ static void pcim_release(struct device *gendev, void *res) struct pci_devres *this = res; int i; - if (dev->msi_enabled) - pci_disable_msi(dev); - if (dev->msix_enabled) - pci_disable_msix(dev); + pci_free_irq_vectors(dev); for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) if (this->region_mask & (1 << i)) diff --git a/include/linux/pci.h b/include/linux/pci.h index 86c799c97b77..5cafd7d65fd7 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1818,6 +1818,30 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, NULL); } +/** + * pcim_alloc_irq_vectors - a device-managed pci_alloc_irq_vectors() + * @dev: PCI device to operate on + * @min_vecs: minimum number of vectors required (must be >= 1) + * @max_vecs: maximum (desired) number of vectors + * @flags: flags or quirks for the allocation + * + * Return the number of vectors allocated, (which might be smaller than + * @max_vecs) if successful, or a negative error code on error. If less + * than @min_vecs interrupt vectors are available for @dev the function + * will fail with -ENOSPC. + * + * It depends on calling pcim_enable_device() to make IRQ resources + * manageable. + */ +static inline int +pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, + unsigned int max_vecs, unsigned int flags) +{ + if (!pci_is_managed(dev)) + return -EINVAL; + return pci_alloc_irq_vectors(dev, min_vecs, max_vecs, flags); +} + /* Include architecture-dependent settings and functions */ #include <asm/pci.h>
Introduce pcim_alloc_irq_vectors(), a device-managed version of pci_alloc_irq_vectors(). Introducing this function can simplify the error handling path in many drivers. And use pci_free_irq_vectors() to replace some code in pcim_release(), they are equivalent, and no functional change. It is more explicit that pcim_alloc_irq_vectors() is a device-managed function. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> --- v4 -> v5: - Remove the check of enable device in pcim_alloc_irq_vectors() and make it as a static line function. v3 -> v4: - No change v2 -> v3: - Add some commit comments for replace some codes in pcim_release() by pci_free_irq_vectors(). v1 -> v2: - Use pci_free_irq_vectors() to replace some code in pcim_release(). - Modify some commit messages. drivers/pci/pci.c | 5 +---- include/linux/pci.h | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-)