Re: [PATCH 11/9] PCI/MSI: Phase out pci_enable_msi_block()

From: Bjorn Helgaas
Date: Wed Jan 29 2014 - 17:22:20 EST


On Fri, Jan 17, 2014 at 10:27:38AM +0100, Alexander Gordeev wrote:
> This update obsoletes pci_enable_msi_block() function
> in favor of pci_enable_msi_range().
>
> Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> ---
> drivers/pci/msi.c | 17 +++++++++++++++--
> include/linux/pci.h | 7 ++-----
> 2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 7a0fec6..49088d0 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -889,7 +889,7 @@ EXPORT_SYMBOL(pci_msi_vec_count);
> * updates the @dev's irq member to the lowest new interrupt number; the
> * other interrupt numbers allocated to this device are consecutive.
> */
> -int pci_enable_msi_block(struct pci_dev *dev, int nvec)
> +static int pci_enable_msi_block(struct pci_dev *dev, int nvec)
> {
> int status, maxvec;
>
> @@ -918,7 +918,6 @@ int pci_enable_msi_block(struct pci_dev *dev, int nvec)
> status = msi_capability_init(dev, nvec);
> return status;
> }
> -EXPORT_SYMBOL(pci_enable_msi_block);
>
> void pci_msi_shutdown(struct pci_dev *dev)
> {
> @@ -1148,6 +1147,20 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
> EXPORT_SYMBOL(pci_enable_msi_range);
>
> /**
> + * pci_enable_msi - configure device's MSI capability structure
> + * @dev: device to configure
> + *
> + * This function initializes the single MSI mode. It returns a negative errno
> + * if an error occurs. If it succeeds, it returns 0 and updates the @dev's
> + * irq member to the new interrupt number;
> + **/
> +int pci_enable_msi(struct pci_dev *dev)
> +{
> + return pci_enable_msi_block(dev, 1);

I like un-exporting pci_enable_msi_block(), but I don't like the fact that
we keep it internally. I'd rather fold it into pci_enable_msi_range()
directly, and then make pci_enable_msi() a wrapper around *that*.

pci_enable_msi_block() has some one-time checks (vector count, already
enabled) that should not be inside the loop in pci_enable_msi_range().
So I think some rearrangement could lead to cleaner code here.

> +}
> +EXPORT_SYMBOL(pci_enable_msi);
> +
> +/**
> * pci_enable_msix_range - configure device's MSI-X capability structure
> * @dev: pointer to the pci_dev data structure of MSI-X device function
> * @entries: pointer to an array of MSI-X entries
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fa959aa..5b58078 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1157,7 +1157,7 @@ struct msix_entry {
>
> #ifdef CONFIG_PCI_MSI
> int pci_msi_vec_count(struct pci_dev *dev);
> -int pci_enable_msi_block(struct pci_dev *dev, int nvec);
> +int pci_enable_msi(struct pci_dev *dev);
> void pci_msi_shutdown(struct pci_dev *dev);
> void pci_disable_msi(struct pci_dev *dev);
> int pci_msix_vec_count(struct pci_dev *dev);
> @@ -1172,8 +1172,7 @@ int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
> int minvec, int maxvec);
> #else
> static inline int pci_msi_vec_count(struct pci_dev *dev) { return -ENOSYS; }
> -static inline int pci_enable_msi_block(struct pci_dev *dev, int nvec)
> -{ return -ENOSYS; }
> +static inline int pci_enable_msi(struct pci_dev *dev) { return -ENOSYS; }
> static inline void pci_msi_shutdown(struct pci_dev *dev) { }
> static inline void pci_disable_msi(struct pci_dev *dev) { }
> static inline int pci_msix_vec_count(struct pci_dev *dev) { return -ENOSYS; }
> @@ -1223,8 +1222,6 @@ static inline void pcie_set_ecrc_checking(struct pci_dev *dev) { }
> static inline void pcie_ecrc_get_policy(char *str) { }
> #endif
>
> -#define pci_enable_msi(pdev) pci_enable_msi_block(pdev, 1)
> -
> #ifdef CONFIG_HT_IRQ
> /* The functions a driver should call */
> int ht_create_irq(struct pci_dev *dev, int idx);
> --
> 1.7.7.6
>
> --
> Regards,
> Alexander Gordeev
> agordeev@xxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/