Patchwork [RFC,v2,08/29] PCI/MSI: Make pci_enable_msix() 'nvec' argument unsigned int

login
register
mail settings
Submitter Alexander Gordeev
Date Oct. 18, 2013, 5:12 p.m.
Message ID <93e145deeb238ad805196762cae696d3ae67f812.1382103786.git.agordeev@redhat.com>
Download mbox | patch
Permalink /patch/284838/
State Superseded
Headers show

Comments

Alexander Gordeev - Oct. 18, 2013, 5:12 p.m.
Make pci_enable_msix() and pci_enable_msi_block() consistent
with regard to the type of 'nvec' argument. Indeed, a number
of vectors to allocate is a natural value, so make it unsigned.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 Documentation/PCI/MSI-HOWTO.txt |    3 ++-
 drivers/pci/msi.c               |    3 ++-
 include/linux/pci.h             |    5 +++--
 3 files changed, 7 insertions(+), 4 deletions(-)
Tejun Heo - Nov. 20, 2013, 4:14 p.m.
Hello,

On Fri, Oct 18, 2013 at 07:12:08PM +0200, Alexander Gordeev wrote:
> Make pci_enable_msix() and pci_enable_msi_block() consistent
> with regard to the type of 'nvec' argument. Indeed, a number
> of vectors to allocate is a natural value, so make it unsigned.

I'm personally not a big fan of using unsigneds where they're just
used as numbers unless strictly necessary.  They don't really provide
any meaningful protection and we often end up in situations where we
want to encode error conditions in the same type variable as return
value or whatever and end up with silly situations like functions
which take unsigned and returns integer or having a separate err
variable when the high bit of the orignal variable would have worked
just fine.  Also, people have different thresholds for what should be
unsigned and we end up with half things unsigned and the other signed.

I'll defer the decision to Bjorn but I'd vote for converting things to
int.

Thanks.
Alexander Gordeev - Nov. 22, 2013, 6:34 p.m.
On Wed, Nov 20, 2013 at 11:14:12AM -0500, Tejun Heo wrote:
> I'm personally not a big fan of using unsigneds where they're just
> used as numbers unless strictly necessary.  They don't really provide
> any meaningful protection and we often end up in situations where we
> want to encode error conditions in the same type variable as return
> value or whatever and end up with silly situations like functions
> which take unsigned and returns integer or having a separate err
> variable when the high bit of the orignal variable would have worked
> just fine.  Also, people have different thresholds for what should be
> unsigned and we end up with half things unsigned and the other signed.

That sounds convincing. Given that arch hooks do use signed int's 
there is one more reason to convert to signed.

> I'll defer the decision to Bjorn but I'd vote for converting things to
> int.

I will re-post with int's in next version unless Bjorn (someone) speaks
up against.

> Thanks.
> 
> -- 
> tejun

Patch

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index a091780..845edb5 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -190,7 +190,8 @@  same number.
 
 4.3.1 pci_enable_msix
 
-int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
+int pci_enable_msix(struct pci_dev *dev,
+		    struct msix_entry *entries, unsigned int nvec)
 
 Calling this function asks the PCI subsystem to allocate 'nvec' MSIs.
 The 'entries' argument is a pointer to an array of msix_entry structs
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index bbe3d3d..fdae3a4 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -932,7 +932,8 @@  int pci_msix_table_size(struct pci_dev *dev)
  * of irqs or MSI-X vectors available. Driver should use the returned value to
  * re-send its request.
  **/
-int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
+int pci_enable_msix(struct pci_dev *dev,
+		    struct msix_entry *entries, unsigned int nvec)
 {
 	int status, nr_entries;
 	int i, j;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5cfe54c..d6832de 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1167,7 +1167,7 @@  static inline int pci_msix_table_size(struct pci_dev *dev)
 	return 0;
 }
 static inline int pci_enable_msix(struct pci_dev *dev,
-				  struct msix_entry *entries, int nvec)
+				  struct msix_entry *entries, int unsigned nvec)
 {
 	return -ENOSYS;
 }
@@ -1192,7 +1192,8 @@  int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
 void pci_msi_shutdown(struct pci_dev *dev);
 void pci_disable_msi(struct pci_dev *dev);
 int pci_msix_table_size(struct pci_dev *dev);
-int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec);
+int pci_enable_msix(struct pci_dev *dev,
+		    struct msix_entry *entries, unsigned int nvec);
 void pci_msix_shutdown(struct pci_dev *dev);
 void pci_disable_msix(struct pci_dev *dev);
 void msi_remove_pci_irq_vectors(struct pci_dev *dev);