| Message ID | 447517b3acece13b7e7ce86fdd4d73abe28d81f4.1345124063.git.agordeev@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, Aug 16, 2012 at 8:49 AM, Alexander Gordeev <agordeev@redhat.com> wrote: > The new function pci_enable_msi_block_auto() tries to allocate maximum > possible number of MSIs up to the number the device supports. It > generalizes a pattern when pci_enable_msi_block() contiguously called > until it succeeds or fails. > > Opposite to pci_enable_msi_block() which takes the number of MSIs to > allocate as a input parameter, pci_enable_msi_block_auto() could be used > by device drivers to obtain the number of assigned MSIs. > > The function could also be used by device drivers which do not care > about the exact number of assigned MSIs. > > Signed-off-by: Alexander Gordeev <agordeev@redhat.com> > --- > Documentation/PCI/MSI-HOWTO.txt | 41 ++++++++++++++++++++++++++++++++++---- > drivers/pci/msi.c | 29 +++++++++++++++++++++++++++ > include/linux/pci.h | 7 ++++++ > 3 files changed, 72 insertions(+), 5 deletions(-) > > diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt > index 53e6fca..d5ab6d7 100644 > --- a/Documentation/PCI/MSI-HOWTO.txt > +++ b/Documentation/PCI/MSI-HOWTO.txt > @@ -127,15 +127,46 @@ on the number of vectors that can be allocated; pci_enable_msi_block() > returns as soon as it finds any constraint that doesn't allow the > call to succeed. > > -4.2.3 pci_disable_msi > +4.2.3 pci_enable_msi_block_auto > + > +int pci_enable_msi_block_auto(struct pci_dev *dev, int *count) > + > +This variation on pci_enable_msi() call allows a device driver to request > +the maximum possible number of MSIs. The MSI specification only allows > +interrupts to be allocated in powers of two, up to a maximum of 2^5 (32). > + > +If this function returns 0, it has succeeded in allocating as many > +interrupts as the device supports. > + > +If this function returns a positive number, it indicates that it has > +succeeded, but the number of allocated interrupts is less than the number > +of interrupts the device supports. The returned value in this case is the > +number of allocated interrupts. Seems like it would be simpler to avoid the special case of returning zero. You could return a negative value for failure, otherwise return the number of interrupts allocated. Then you could also dispense with the "int *count" argument, because the caller could just look at the return value. > +In both cases above, the function enables MSI on this device and updates > +dev->irq to be the lowest of the new interrupts assigned to it. The other > +interrupts assigned to the device are in the range dev->irq to dev->irq + > +number of allocated interrupts - 1. > + > +If the device driver needs to know the number of allocated interrupts it > +can pass the pointer count where that number is stored. If the device driver > +has other means to obtain that number or does not need it, NULL pointer can > +be passed instead. > + > +If this function returns a negative number, it indicates an error and the > +driver should not attempt to request any more MSI interrupts for this > +device. > + > +4.2.4 pci_disable_msi > > void pci_disable_msi(struct pci_dev *dev) > > This function should be used to undo the effect of pci_enable_msi() or > -pci_enable_msi_block(). Calling it restores dev->irq to the pin-based > -interrupt number and frees the previously allocated message signaled > -interrupt(s). The interrupt may subsequently be assigned to another > -device, so drivers should not cache the value of dev->irq. > +pci_enable_msi_block() or pci_enable_msi_block_auto(). Calling it restores > +dev->irq to the pin-based interrupt number and frees the previously > +allocated message signaled interrupt(s). The interrupt may subsequently be > +assigned to another device, so drivers should not cache the value of > +dev->irq. > > Before calling this function, a device driver must always call free_irq() > on any interrupt for which it previously called request_irq(). > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index f0752d1..ef982c1 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -845,6 +845,35 @@ int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec) > } > EXPORT_SYMBOL(pci_enable_msi_block); > > +int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *nvec) > +{ > + int ret, pos, envec, maxvec; > + u16 msgctl; > + > + pos = pci_find_capability(dev, PCI_CAP_ID_MSI); > + if (!pos) > + return -EINVAL; > + > + pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &msgctl); > + maxvec = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1); > + ret = maxvec; > + > + do { > + envec = ret; > + ret = pci_enable_msi_block(dev, envec); > + } while (ret > 0); > + > + if (ret < 0) > + return ret; > + if (nvec) > + *nvec = envec; > + if (envec < maxvec) > + return envec; > + return 0; > + > +} > +EXPORT_SYMBOL(pci_enable_msi_block_auto); > + > void pci_msi_shutdown(struct pci_dev *dev) > { > struct msi_desc *desc; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index d8c379d..14788d0 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1024,6 +1024,12 @@ static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec) > return -1; > } > > +static inline int > +pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *nvec) > +{ > + return -1; > +} > + > static inline void pci_msi_shutdown(struct pci_dev *dev) > { } > static inline void pci_disable_msi(struct pci_dev *dev) > @@ -1055,6 +1061,7 @@ static inline int pci_msi_enabled(void) > } > #else > extern int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec); > +extern int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *nvec); > extern void pci_msi_shutdown(struct pci_dev *dev); > extern void pci_disable_msi(struct pci_dev *dev); > extern int pci_msix_table_size(struct pci_dev *dev); > -- > 1.7.7.6 > > > -- > Regards, > Alexander Gordeev > agordeev@redhat.com -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 16, 2012 at 10:00:39AM -0600, Bjorn Helgaas wrote: > On Thu, Aug 16, 2012 at 8:49 AM, Alexander Gordeev <agordeev@redhat.com> wrote: > > -4.2.3 pci_disable_msi > > +4.2.3 pci_enable_msi_block_auto > > + > > +int pci_enable_msi_block_auto(struct pci_dev *dev, int *count) > > + > > +This variation on pci_enable_msi() call allows a device driver to request > > +the maximum possible number of MSIs. The MSI specification only allows > > +interrupts to be allocated in powers of two, up to a maximum of 2^5 (32). > > + > > +If this function returns 0, it has succeeded in allocating as many > > +interrupts as the device supports. > > + > > +If this function returns a positive number, it indicates that it has > > +succeeded, but the number of allocated interrupts is less than the number > > +of interrupts the device supports. The returned value in this case is the > > +number of allocated interrupts. > > Seems like it would be simpler to avoid the special case of returning > zero. You could return a negative value for failure, otherwise return > the number of interrupts allocated. But this special case is important, because some drivers would not get satisfied with just any number of interrupts allocated (i.e. few Intel AHCI chips (seems) have hardware logic that compares qmask vs qsize and simply falls back to single interrupt if they are not equal). So I see the fact that maximum possible number of interrupts were allocated at least as important than the number itself. > Then you could also dispense with the "int *count" argument, because > the caller could just look at the return value. What about returning the number of allocated interrtupts while storing the number of supported interrupts to "int *count" (or maxcount)?
On Fri, Aug 17, 2012 at 2:19 AM, Alexander Gordeev <agordeev@redhat.com> wrote: > On Thu, Aug 16, 2012 at 10:00:39AM -0600, Bjorn Helgaas wrote: >> On Thu, Aug 16, 2012 at 8:49 AM, Alexander Gordeev <agordeev@redhat.com> wrote: >> > -4.2.3 pci_disable_msi >> > +4.2.3 pci_enable_msi_block_auto >> > + >> > +int pci_enable_msi_block_auto(struct pci_dev *dev, int *count) >> > + >> > +This variation on pci_enable_msi() call allows a device driver to request >> > +the maximum possible number of MSIs. The MSI specification only allows >> > +interrupts to be allocated in powers of two, up to a maximum of 2^5 (32). >> > + >> > +If this function returns 0, it has succeeded in allocating as many >> > +interrupts as the device supports. >> > + >> > +If this function returns a positive number, it indicates that it has >> > +succeeded, but the number of allocated interrupts is less than the number >> > +of interrupts the device supports. The returned value in this case is the >> > +number of allocated interrupts. >> >> Seems like it would be simpler to avoid the special case of returning >> zero. You could return a negative value for failure, otherwise return >> the number of interrupts allocated. > > But this special case is important, because some drivers would not get > satisfied with just any number of interrupts allocated (i.e. few Intel AHCI > chips (seems) have hardware logic that compares qmask vs qsize and simply > falls back to single interrupt if they are not equal). > > So I see the fact that maximum possible number of interrupts were allocated > at least as important than the number itself. > >> Then you could also dispense with the "int *count" argument, because >> the caller could just look at the return value. > > What about returning the number of allocated interrtupts while storing the > number of supported interrupts to "int *count" (or maxcount)? I like that idea a lot better because then you don't need a special case to interpret the return value. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt index 53e6fca..d5ab6d7 100644 --- a/Documentation/PCI/MSI-HOWTO.txt +++ b/Documentation/PCI/MSI-HOWTO.txt @@ -127,15 +127,46 @@ on the number of vectors that can be allocated; pci_enable_msi_block() returns as soon as it finds any constraint that doesn't allow the call to succeed. -4.2.3 pci_disable_msi +4.2.3 pci_enable_msi_block_auto + +int pci_enable_msi_block_auto(struct pci_dev *dev, int *count) + +This variation on pci_enable_msi() call allows a device driver to request +the maximum possible number of MSIs. The MSI specification only allows +interrupts to be allocated in powers of two, up to a maximum of 2^5 (32). + +If this function returns 0, it has succeeded in allocating as many +interrupts as the device supports. + +If this function returns a positive number, it indicates that it has +succeeded, but the number of allocated interrupts is less than the number +of interrupts the device supports. The returned value in this case is the +number of allocated interrupts. + +In both cases above, the function enables MSI on this device and updates +dev->irq to be the lowest of the new interrupts assigned to it. The other +interrupts assigned to the device are in the range dev->irq to dev->irq + +number of allocated interrupts - 1. + +If the device driver needs to know the number of allocated interrupts it +can pass the pointer count where that number is stored. If the device driver +has other means to obtain that number or does not need it, NULL pointer can +be passed instead. + +If this function returns a negative number, it indicates an error and the +driver should not attempt to request any more MSI interrupts for this +device. + +4.2.4 pci_disable_msi void pci_disable_msi(struct pci_dev *dev) This function should be used to undo the effect of pci_enable_msi() or -pci_enable_msi_block(). Calling it restores dev->irq to the pin-based -interrupt number and frees the previously allocated message signaled -interrupt(s). The interrupt may subsequently be assigned to another -device, so drivers should not cache the value of dev->irq. +pci_enable_msi_block() or pci_enable_msi_block_auto(). Calling it restores +dev->irq to the pin-based interrupt number and frees the previously +allocated message signaled interrupt(s). The interrupt may subsequently be +assigned to another device, so drivers should not cache the value of +dev->irq. Before calling this function, a device driver must always call free_irq() on any interrupt for which it previously called request_irq(). diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index f0752d1..ef982c1 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -845,6 +845,35 @@ int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec) } EXPORT_SYMBOL(pci_enable_msi_block); +int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *nvec) +{ + int ret, pos, envec, maxvec; + u16 msgctl; + + pos = pci_find_capability(dev, PCI_CAP_ID_MSI); + if (!pos) + return -EINVAL; + + pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &msgctl); + maxvec = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1); + ret = maxvec; + + do { + envec = ret; + ret = pci_enable_msi_block(dev, envec); + } while (ret > 0); + + if (ret < 0) + return ret; + if (nvec) + *nvec = envec; + if (envec < maxvec) + return envec; + return 0; + +} +EXPORT_SYMBOL(pci_enable_msi_block_auto); + void pci_msi_shutdown(struct pci_dev *dev) { struct msi_desc *desc; diff --git a/include/linux/pci.h b/include/linux/pci.h index d8c379d..14788d0 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1024,6 +1024,12 @@ static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec) return -1; } +static inline int +pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *nvec) +{ + return -1; +} + static inline void pci_msi_shutdown(struct pci_dev *dev) { } static inline void pci_disable_msi(struct pci_dev *dev) @@ -1055,6 +1061,7 @@ static inline int pci_msi_enabled(void) } #else extern int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec); +extern int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *nvec); extern void pci_msi_shutdown(struct pci_dev *dev); extern void pci_disable_msi(struct pci_dev *dev); extern int pci_msix_table_size(struct pci_dev *dev);
The new function pci_enable_msi_block_auto() tries to allocate maximum possible number of MSIs up to the number the device supports. It generalizes a pattern when pci_enable_msi_block() contiguously called until it succeeds or fails. Opposite to pci_enable_msi_block() which takes the number of MSIs to allocate as a input parameter, pci_enable_msi_block_auto() could be used by device drivers to obtain the number of assigned MSIs. The function could also be used by device drivers which do not care about the exact number of assigned MSIs. Signed-off-by: Alexander Gordeev <agordeev@redhat.com> --- Documentation/PCI/MSI-HOWTO.txt | 41 ++++++++++++++++++++++++++++++++++---- drivers/pci/msi.c | 29 +++++++++++++++++++++++++++ include/linux/pci.h | 7 ++++++ 3 files changed, 72 insertions(+), 5 deletions(-)