Re: [PATCH v2 3/3] PCI/MSI: Remove arch_msi_check_device()

2014-09-23 Thread Bjorn Helgaas
On Sun, Sep 07, 2014 at 08:57:55PM +0200, Alexander Gordeev wrote:
> There are no archs that override arch_msi_check_device()
> hook. Remove it as it is completely redundant.
> 
> If an arch would need to check MSI/MSI-X possibility for a
> device it should make it within arch_setup_msi_irqs() hook.
> 
> Cc: Michael Ellerman 
> Cc: linuxppc-...@lists.ozlabs.org
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Alexander Gordeev 
> ---
>  drivers/pci/msi.c   | 49 +
>  include/linux/msi.h |  3 ---
>  2 files changed, 13 insertions(+), 39 deletions(-)

I like this patch a lot, but it's doing several things at once, so I split
it into several patches as follows.  I think it is equivalent, but I did
not add Thomas' tested-by.

I also reversed the sense of pci_msi_supported() so it returns true (1)
when MSI/MSI-X is supported and false (0) otherwise.

The whole thing is on my pci/msi branch at
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/msi

Let me know if you see any issues.

Bjorn


commit 28a3ec3f672d42a877d8770a33305dfe82c2cc90
Author: Alexander Gordeev 
Date:   Tue Sep 23 12:39:54 2014 -0600

PCI/MSI: Remove arch_msi_check_device()

No architectures implement arch_msi_check_device() or the struct msi_chip
.check_device() method, so remove them.

Remove the "type" parameter to pci_msi_check_device() because it was only
used to call arch_msi_check_device() and is no longer needed.

[bhelgaas: changelog, split to separate patch]
Signed-off-by: Alexander Gordeev 
Signed-off-by: Bjorn Helgaas 

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index aff384aea639..a0b623d97ad8 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -56,16 +56,6 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
chip->teardown_irq(chip, irq);
 }
 
-int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
-{
-   struct msi_chip *chip = dev->bus->msi;
-
-   if (!chip || !chip->check_device)
-   return 0;
-
-   return chip->check_device(chip, dev, nvec, type);
-}
-
 int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
struct msi_desc *entry;
@@ -788,10 +778,9 @@ out_free:
  * to determine if MSI/-X are supported for the device. If MSI/-X is
  * supported return 0, else return an error code.
  **/
-static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
+static int pci_msi_check_device(struct pci_dev *dev, int nvec)
 {
struct pci_bus *bus;
-   int ret;
 
/* MSI must be globally enabled and supported by the device */
if (!pci_msi_enable || !dev || dev->no_msi)
@@ -816,10 +805,6 @@ static int pci_msi_check_device(struct pci_dev *dev, int 
nvec, int type)
if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
return -EINVAL;
 
-   ret = arch_msi_check_device(dev, nvec, type);
-   if (ret)
-   return ret;
-
return 0;
 }
 
@@ -925,7 +910,7 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry 
*entries, int nvec)
if (!entries || !dev->msix_cap || dev->current_state != PCI_D0)
return -EINVAL;
 
-   status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSIX);
+   status = pci_msi_check_device(dev, nvec);
if (status)
return status;
 
@@ -1059,7 +1044,7 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, 
int maxvec)
nvec = maxvec;
 
do {
-   rc = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSI);
+   rc = pci_msi_check_device(dev, nvec);
if (rc < 0) {
return rc;
} else if (rc > 0) {
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 6be66f48b026..36c63cfccf5b 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -55,7 +55,6 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc 
*desc);
 void arch_teardown_msi_irq(unsigned int irq);
 int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void arch_teardown_msi_irqs(struct pci_dev *dev);
-int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
 void arch_restore_msi_irqs(struct pci_dev *dev);
 
 void default_teardown_msi_irqs(struct pci_dev *dev);
@@ -72,8 +71,6 @@ struct msi_chip {
int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev,
 struct msi_desc *desc);
void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
-   int (*check_device)(struct msi_chip *chip, struct pci_dev *dev,
-   int nvec, int type);
 };
 
 #endif /* LINUX_MSI_H */

commit 9d686dd5bbf62a249d0e4f54bffd848d573d2380
Author: Alexander Gordeev 
Date:   Tue Sep 23 14:25:11 2014 -0600

PCI/MSI: Move D0 check into pci_msi_check_device()

Both callers of pci_msi_check_device() check that the device is in D0
state, 

Re: [PATCH v2 3/3] PCI/MSI: Remove arch_msi_check_device()

2014-09-23 Thread Bjorn Helgaas
On Sun, Sep 07, 2014 at 08:57:55PM +0200, Alexander Gordeev wrote:
 There are no archs that override arch_msi_check_device()
 hook. Remove it as it is completely redundant.
 
 If an arch would need to check MSI/MSI-X possibility for a
 device it should make it within arch_setup_msi_irqs() hook.
 
 Cc: Michael Ellerman m...@ellerman.id.au
 Cc: linuxppc-...@lists.ozlabs.org
 Cc: linux-...@vger.kernel.org
 Signed-off-by: Alexander Gordeev agord...@redhat.com
 ---
  drivers/pci/msi.c   | 49 +
  include/linux/msi.h |  3 ---
  2 files changed, 13 insertions(+), 39 deletions(-)

I like this patch a lot, but it's doing several things at once, so I split
it into several patches as follows.  I think it is equivalent, but I did
not add Thomas' tested-by.

I also reversed the sense of pci_msi_supported() so it returns true (1)
when MSI/MSI-X is supported and false (0) otherwise.

The whole thing is on my pci/msi branch at
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/msi

Let me know if you see any issues.

Bjorn


commit 28a3ec3f672d42a877d8770a33305dfe82c2cc90
Author: Alexander Gordeev agord...@redhat.com
Date:   Tue Sep 23 12:39:54 2014 -0600

PCI/MSI: Remove arch_msi_check_device()

No architectures implement arch_msi_check_device() or the struct msi_chip
.check_device() method, so remove them.

Remove the type parameter to pci_msi_check_device() because it was only
used to call arch_msi_check_device() and is no longer needed.

[bhelgaas: changelog, split to separate patch]
Signed-off-by: Alexander Gordeev agord...@redhat.com
Signed-off-by: Bjorn Helgaas bhelg...@google.com

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index aff384aea639..a0b623d97ad8 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -56,16 +56,6 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
chip-teardown_irq(chip, irq);
 }
 
-int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
-{
-   struct msi_chip *chip = dev-bus-msi;
-
-   if (!chip || !chip-check_device)
-   return 0;
-
-   return chip-check_device(chip, dev, nvec, type);
-}
-
 int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
struct msi_desc *entry;
@@ -788,10 +778,9 @@ out_free:
  * to determine if MSI/-X are supported for the device. If MSI/-X is
  * supported return 0, else return an error code.
  **/
-static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
+static int pci_msi_check_device(struct pci_dev *dev, int nvec)
 {
struct pci_bus *bus;
-   int ret;
 
/* MSI must be globally enabled and supported by the device */
if (!pci_msi_enable || !dev || dev-no_msi)
@@ -816,10 +805,6 @@ static int pci_msi_check_device(struct pci_dev *dev, int 
nvec, int type)
if (bus-bus_flags  PCI_BUS_FLAGS_NO_MSI)
return -EINVAL;
 
-   ret = arch_msi_check_device(dev, nvec, type);
-   if (ret)
-   return ret;
-
return 0;
 }
 
@@ -925,7 +910,7 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry 
*entries, int nvec)
if (!entries || !dev-msix_cap || dev-current_state != PCI_D0)
return -EINVAL;
 
-   status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSIX);
+   status = pci_msi_check_device(dev, nvec);
if (status)
return status;
 
@@ -1059,7 +1044,7 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, 
int maxvec)
nvec = maxvec;
 
do {
-   rc = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSI);
+   rc = pci_msi_check_device(dev, nvec);
if (rc  0) {
return rc;
} else if (rc  0) {
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 6be66f48b026..36c63cfccf5b 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -55,7 +55,6 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc 
*desc);
 void arch_teardown_msi_irq(unsigned int irq);
 int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void arch_teardown_msi_irqs(struct pci_dev *dev);
-int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
 void arch_restore_msi_irqs(struct pci_dev *dev);
 
 void default_teardown_msi_irqs(struct pci_dev *dev);
@@ -72,8 +71,6 @@ struct msi_chip {
int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev,
 struct msi_desc *desc);
void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
-   int (*check_device)(struct msi_chip *chip, struct pci_dev *dev,
-   int nvec, int type);
 };
 
 #endif /* LINUX_MSI_H */

commit 9d686dd5bbf62a249d0e4f54bffd848d573d2380
Author: Alexander Gordeev agord...@redhat.com
Date:   Tue Sep 23 14:25:11 2014 -0600

PCI/MSI: Move D0 check into pci_msi_check_device()

[PATCH v2 3/3] PCI/MSI: Remove arch_msi_check_device()

2014-09-07 Thread Alexander Gordeev
There are no archs that override arch_msi_check_device()
hook. Remove it as it is completely redundant.

If an arch would need to check MSI/MSI-X possibility for a
device it should make it within arch_setup_msi_irqs() hook.

Cc: Michael Ellerman 
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Alexander Gordeev 
---
 drivers/pci/msi.c   | 49 +
 include/linux/msi.h |  3 ---
 2 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 5a40516..6c2cc41 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -56,16 +56,6 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
chip->teardown_irq(chip, irq);
 }
 
-int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
-{
-   struct msi_chip *chip = dev->bus->msi;
-
-   if (!chip || !chip->check_device)
-   return 0;
-
-   return chip->check_device(chip, dev, nvec, type);
-}
-
 int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
struct msi_desc *entry;
@@ -806,22 +796,23 @@ out_free:
 }
 
 /**
- * pci_msi_check_device - check whether MSI may be enabled on a device
+ * pci_msi_supported - check whether MSI may be enabled on a device
  * @dev: pointer to the pci_dev data structure of MSI device function
  * @nvec: how many MSIs have been requested ?
- * @type: are we checking for MSI or MSI-X ?
  *
  * Look at global flags, the device itself, and its parent buses
  * to determine if MSI/-X are supported for the device. If MSI/-X is
  * supported return 0, else return an error code.
  **/
-static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
+static int pci_msi_supported(struct pci_dev *dev, int nvec)
 {
struct pci_bus *bus;
-   int ret;
 
/* MSI must be globally enabled and supported by the device */
-   if (!pci_msi_enable || !dev || dev->no_msi)
+   if (!pci_msi_enable)
+   return -EINVAL;
+
+   if (!dev || dev->no_msi || dev->current_state != PCI_D0)
return -EINVAL;
 
/*
@@ -843,10 +834,6 @@ static int pci_msi_check_device(struct pci_dev *dev, int 
nvec, int type)
if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
return -EINVAL;
 
-   ret = arch_msi_check_device(dev, nvec, type);
-   if (ret)
-   return ret;
-
return 0;
 }
 
@@ -949,13 +936,13 @@ int pci_enable_msix(struct pci_dev *dev, struct 
msix_entry *entries, int nvec)
int status, nr_entries;
int i, j;
 
-   if (!entries || !dev->msix_cap || dev->current_state != PCI_D0)
-   return -EINVAL;
-
-   status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSIX);
+   status = pci_msi_supported(dev, nvec);
if (status)
return status;
 
+   if (!entries)
+   return -EINVAL;
+
nr_entries = pci_msix_vec_count(dev);
if (nr_entries < 0)
return nr_entries;
@@ -1062,8 +1049,9 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, 
int maxvec)
int nvec;
int rc;
 
-   if (dev->current_state != PCI_D0)
-   return -EINVAL;
+   rc = pci_msi_supported(dev, minvec);
+   if (rc)
+   return rc;
 
WARN_ON(!!dev->msi_enabled);
 
@@ -1086,17 +1074,6 @@ int pci_enable_msi_range(struct pci_dev *dev, int 
minvec, int maxvec)
nvec = maxvec;
 
do {
-   rc = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSI);
-   if (rc < 0) {
-   return rc;
-   } else if (rc > 0) {
-   if (rc < minvec)
-   return -ENOSPC;
-   nvec = rc;
-   }
-   } while (rc);
-
-   do {
rc = msi_capability_init(dev, nvec);
if (rc < 0) {
return rc;
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 8103f32..dbf7cc9 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -60,7 +60,6 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc 
*desc);
 void arch_teardown_msi_irq(unsigned int irq);
 int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void arch_teardown_msi_irqs(struct pci_dev *dev);
-int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
 void arch_restore_msi_irqs(struct pci_dev *dev);
 
 void default_teardown_msi_irqs(struct pci_dev *dev);
@@ -77,8 +76,6 @@ struct msi_chip {
int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev,
 struct msi_desc *desc);
void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
-   int (*check_device)(struct msi_chip *chip, struct pci_dev *dev,
-   int nvec, int type);
 };
 
 #endif /* LINUX_MSI_H */
-- 
1.9.3

--
To unsubscribe from this list: send the 

[PATCH v2 3/3] PCI/MSI: Remove arch_msi_check_device()

2014-09-07 Thread Alexander Gordeev
There are no archs that override arch_msi_check_device()
hook. Remove it as it is completely redundant.

If an arch would need to check MSI/MSI-X possibility for a
device it should make it within arch_setup_msi_irqs() hook.

Cc: Michael Ellerman m...@ellerman.id.au
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Alexander Gordeev agord...@redhat.com
---
 drivers/pci/msi.c   | 49 +
 include/linux/msi.h |  3 ---
 2 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 5a40516..6c2cc41 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -56,16 +56,6 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
chip-teardown_irq(chip, irq);
 }
 
-int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
-{
-   struct msi_chip *chip = dev-bus-msi;
-
-   if (!chip || !chip-check_device)
-   return 0;
-
-   return chip-check_device(chip, dev, nvec, type);
-}
-
 int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
struct msi_desc *entry;
@@ -806,22 +796,23 @@ out_free:
 }
 
 /**
- * pci_msi_check_device - check whether MSI may be enabled on a device
+ * pci_msi_supported - check whether MSI may be enabled on a device
  * @dev: pointer to the pci_dev data structure of MSI device function
  * @nvec: how many MSIs have been requested ?
- * @type: are we checking for MSI or MSI-X ?
  *
  * Look at global flags, the device itself, and its parent buses
  * to determine if MSI/-X are supported for the device. If MSI/-X is
  * supported return 0, else return an error code.
  **/
-static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
+static int pci_msi_supported(struct pci_dev *dev, int nvec)
 {
struct pci_bus *bus;
-   int ret;
 
/* MSI must be globally enabled and supported by the device */
-   if (!pci_msi_enable || !dev || dev-no_msi)
+   if (!pci_msi_enable)
+   return -EINVAL;
+
+   if (!dev || dev-no_msi || dev-current_state != PCI_D0)
return -EINVAL;
 
/*
@@ -843,10 +834,6 @@ static int pci_msi_check_device(struct pci_dev *dev, int 
nvec, int type)
if (bus-bus_flags  PCI_BUS_FLAGS_NO_MSI)
return -EINVAL;
 
-   ret = arch_msi_check_device(dev, nvec, type);
-   if (ret)
-   return ret;
-
return 0;
 }
 
@@ -949,13 +936,13 @@ int pci_enable_msix(struct pci_dev *dev, struct 
msix_entry *entries, int nvec)
int status, nr_entries;
int i, j;
 
-   if (!entries || !dev-msix_cap || dev-current_state != PCI_D0)
-   return -EINVAL;
-
-   status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSIX);
+   status = pci_msi_supported(dev, nvec);
if (status)
return status;
 
+   if (!entries)
+   return -EINVAL;
+
nr_entries = pci_msix_vec_count(dev);
if (nr_entries  0)
return nr_entries;
@@ -1062,8 +1049,9 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, 
int maxvec)
int nvec;
int rc;
 
-   if (dev-current_state != PCI_D0)
-   return -EINVAL;
+   rc = pci_msi_supported(dev, minvec);
+   if (rc)
+   return rc;
 
WARN_ON(!!dev-msi_enabled);
 
@@ -1086,17 +1074,6 @@ int pci_enable_msi_range(struct pci_dev *dev, int 
minvec, int maxvec)
nvec = maxvec;
 
do {
-   rc = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSI);
-   if (rc  0) {
-   return rc;
-   } else if (rc  0) {
-   if (rc  minvec)
-   return -ENOSPC;
-   nvec = rc;
-   }
-   } while (rc);
-
-   do {
rc = msi_capability_init(dev, nvec);
if (rc  0) {
return rc;
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 8103f32..dbf7cc9 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -60,7 +60,6 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc 
*desc);
 void arch_teardown_msi_irq(unsigned int irq);
 int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void arch_teardown_msi_irqs(struct pci_dev *dev);
-int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
 void arch_restore_msi_irqs(struct pci_dev *dev);
 
 void default_teardown_msi_irqs(struct pci_dev *dev);
@@ -77,8 +76,6 @@ struct msi_chip {
int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev,
 struct msi_desc *desc);
void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
-   int (*check_device)(struct msi_chip *chip, struct pci_dev *dev,
-   int nvec, int type);
 };
 
 #endif /* LINUX_MSI_H */
-- 
1.9.3

--
To unsubscribe from this