Michael Ellerman <[EMAIL PROTECTED]> writes:
> pci_enable_msi() and pci_enable_msix() both search for the MSI/MSI-X
> capability, we can fold this into pci_msi_supported() by passing the
> type in.
>
> Update the code to match the comment for pci_msi_supported(). That is
> it returns 0 on success, and anything else indicates an error.
Ok. Looking at this one I don't see a bug exactly but there
is one very confusing piece.
Currently we have a function pci_msi_supported that sounds like it
sounds like it should return a boolean value.
However instead it returns 0 for success and -EINVAL for failure.
Reading through the code before this patch it is clear it does this
weird thing because we check for < 0.
After this patch we are simply checking to see if there was a
return value at all.
Can we please change the return value here so it is actually boolean.
1 for supported 0 for not supported.
There aren't any useful return values anyway so this would just make
the code easier to read and maintain.
Eric
>
> Signed-off-by: Michael Ellerman <[EMAIL PROTECTED]>
> ---
>
> drivers/pci/msi.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> Index: msi-new/drivers/pci/msi.c
> ===================================================================
> --- msi-new.orig/drivers/pci/msi.c
> +++ msi-new/drivers/pci/msi.c
> @@ -433,12 +433,13 @@ static int msix_capability_init(struct p
> /**
> * pci_msi_supported - check whether MSI may be enabled on device
> * @dev: pointer to the pci_dev data structure of MSI device function
> + * @type: are we checking for MSI or MSI-X ?
> *
> * Look at global flags, the device itself, and its parent busses
> * to return 0 if MSI are supported for the device.
> **/
> static
> -int pci_msi_supported(struct pci_dev * dev)
> +int pci_msi_supported(struct pci_dev * dev, int type)
> {
> struct pci_bus *bus;
>
> @@ -456,6 +457,9 @@ int pci_msi_supported(struct pci_dev * d
> if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
> return -EINVAL;
>
> + if (!pci_find_capability(dev, type))
> + return -EINVAL;
> +
> return 0;
> }
>
> @@ -471,13 +475,9 @@ int pci_msi_supported(struct pci_dev * d
> **/
> int pci_enable_msi(struct pci_dev* dev)
> {
> - int pos, status;
> -
> - if (pci_msi_supported(dev) < 0)
> - return -EINVAL;
> + int status;
>
> - pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
> - if (!pos)
> + if (pci_msi_supported(dev, PCI_CAP_ID_MSI))
> return -EINVAL;
>
> WARN_ON(!!dev->msi_enabled);
> @@ -580,13 +580,10 @@ int pci_enable_msix(struct pci_dev* dev,
> int i, j;
> u16 control;
>
> - if (!entries || pci_msi_supported(dev) < 0)
> + if (!entries || pci_msi_supported(dev, PCI_CAP_ID_MSIX))
> return -EINVAL;
>
> pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> - if (!pos)
> - return -EINVAL;
> -
> pci_read_config_word(dev, msi_control_reg(pos), &control);
> nr_entries = multi_msix_capable(control);
> if (nvec > nr_entries)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
