linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] PCI: code clean up on pci configuration space
@ 2014-10-14  6:47 Wei Yang
  2014-10-14  6:47 ` [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Wei Yang @ 2014-10-14  6:47 UTC (permalink / raw)
  To: linux-pci, bhelgaas; +Cc: Wei Yang

This series is a clean up in the pci subsystem when accessing the pci
configuration space.

The first one is to re-use the PCI_FIND_CAP_TTL to limit the times iterating
in pci configuration space.

The next two is to use the exact type to access the pci cap and pcie ext cap.

Tested on x86 and powerpc.

Wei Yang (3):
  PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
  PCI: use u8 instead of int for pci configuration space pos and cap
  PCI: use u16 instead of int for pci express extended capabilities pos
    and cap

 drivers/pci/iov.c    |    2 +-
 drivers/pci/pci.c    |   68 +++++++++++++++++++++++++-------------------------
 drivers/pci/probe.c  |   10 ++++----
 drivers/pci/quirks.c |   19 +++++++-------
 include/linux/pci.h  |   28 +++++++++++----------
 5 files changed, 65 insertions(+), 62 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
  2014-10-14  6:47 [PATCH 0/3] PCI: code clean up on pci configuration space Wei Yang
@ 2014-10-14  6:47 ` Wei Yang
  2014-10-22 23:18   ` Bjorn Helgaas
  2014-10-14  6:47 ` [PATCH 2/3] PCI: use u8 instead of int for pci configuration space pos and cap Wei Yang
  2014-10-14  6:47 ` [PATCH 3/3] PCI: use u16 instead of int for pci express extended capabilities " Wei Yang
  2 siblings, 1 reply; 10+ messages in thread
From: Wei Yang @ 2014-10-14  6:47 UTC (permalink / raw)
  To: linux-pci, bhelgaas; +Cc: Wei Yang

In some quirks, it tries to search a pci cap and use a ttl value to avoid
infinite loop. While the value is hard coded to 48, which is the same as marco
PCI_FIND_CAP_TTL.

This patch moves the definition of PCI_FIND_CAP_TTL to pci.h and replace the
hard coded value with it.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/pci/pci.c    |    1 -
 drivers/pci/quirks.c |    8 ++++----
 include/linux/pci.h  |    2 ++
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 2c9ac70..76b002b1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -137,7 +137,6 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
 EXPORT_SYMBOL_GPL(pci_ioremap_bar);
 #endif
 
-#define PCI_FIND_CAP_TTL	48
 
 static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
 				   u8 pos, int cap, int *ttl)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 80c2d01..a5f46b8 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
  * return 1 if a HT MSI capability is found and enabled */
 static int msi_ht_cap_enabled(struct pci_dev *dev)
 {
-	int pos, ttl = 48;
+	int pos, ttl = PCI_FIND_CAP_TTL;
 
 	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
 	while (pos && ttl--) {
@@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
 /* Force enable MSI mapping capability on HT bridges */
 static void ht_enable_msi_mapping(struct pci_dev *dev)
 {
-	int pos, ttl = 48;
+	int pos, ttl = PCI_FIND_CAP_TTL;
 
 	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
 	while (pos && ttl--) {
@@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
 
 static int ht_check_msi_mapping(struct pci_dev *dev)
 {
-	int pos, ttl = 48;
+	int pos, ttl = PCI_FIND_CAP_TTL;
 	int found = 0;
 
 	/* check if there is HT MSI cap or enabled on this device */
@@ -2439,7 +2439,7 @@ out:
 
 static void ht_disable_msi_mapping(struct pci_dev *dev)
 {
-	int pos, ttl = 48;
+	int pos, ttl = PCI_FIND_CAP_TTL;
 
 	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
 	while (pos && ttl--) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 96453f9..b27b79e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -33,6 +33,8 @@
 
 #include <linux/pci_ids.h>
 
+#define PCI_FIND_CAP_TTL	48
+
 /*
  * The PCI interface treats multi-function devices as independent
  * devices.  The slot/function address of each device is encoded
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] PCI: use u8 instead of int for pci configuration space pos and cap
  2014-10-14  6:47 [PATCH 0/3] PCI: code clean up on pci configuration space Wei Yang
  2014-10-14  6:47 ` [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
@ 2014-10-14  6:47 ` Wei Yang
  2014-10-22 23:23   ` Bjorn Helgaas
  2014-10-14  6:47 ` [PATCH 3/3] PCI: use u16 instead of int for pci express extended capabilities " Wei Yang
  2 siblings, 1 reply; 10+ messages in thread
From: Wei Yang @ 2014-10-14  6:47 UTC (permalink / raw)
  To: linux-pci, bhelgaas; +Cc: Wei Yang

For pci devices complying with PCI LB 3.0, the configuration space size is
256 and the Cap ID is represented by a 8bit field. This means a type of u8 is
enough to repsent the Cap's position and ID.

This patch does some clean up for the Cap position and ID by replacing the int
type with u8. And consolidate the check of whether a Cap is represented from
(pos <= 0) to (!pos).

And two functions use char to represent cap, it is changed to u8 in this
patch.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/pci/pci.c    |   55 +++++++++++++++++++++++++-------------------------
 drivers/pci/probe.c  |    8 ++++----
 drivers/pci/quirks.c |   19 ++++++++---------
 include/linux/pci.h  |   20 +++++++++---------
 4 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 76b002b1..47d8d0c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -138,8 +138,8 @@ EXPORT_SYMBOL_GPL(pci_ioremap_bar);
 #endif
 
 
-static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
-				   u8 pos, int cap, int *ttl)
+static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
+				   u8 pos, u8 cap, int *ttl)
 {
 	u8 id;
 
@@ -159,22 +159,22 @@ static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
 	return 0;
 }
 
-static int __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
-			       u8 pos, int cap)
+static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
+			       u8 pos, u8 cap)
 {
 	int ttl = PCI_FIND_CAP_TTL;
 
 	return __pci_find_next_cap_ttl(bus, devfn, pos, cap, &ttl);
 }
 
-int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap)
+u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, u8 cap)
 {
 	return __pci_find_next_cap(dev->bus, dev->devfn,
 				   pos + PCI_CAP_LIST_NEXT, cap);
 }
 EXPORT_SYMBOL_GPL(pci_find_next_capability);
 
-static int __pci_bus_find_cap_start(struct pci_bus *bus,
+static u8 __pci_bus_find_cap_start(struct pci_bus *bus,
 				    unsigned int devfn, u8 hdr_type)
 {
 	u16 status;
@@ -215,9 +215,9 @@ static int __pci_bus_find_cap_start(struct pci_bus *bus,
  *  %PCI_CAP_ID_PCIX         PCI-X
  *  %PCI_CAP_ID_EXP          PCI Express
  */
-int pci_find_capability(struct pci_dev *dev, int cap)
+u8 pci_find_capability(struct pci_dev *dev, u8 cap)
 {
-	int pos;
+	u8 pos;
 
 	pos = __pci_bus_find_cap_start(dev->bus, dev->devfn, dev->hdr_type);
 	if (pos)
@@ -240,9 +240,9 @@ EXPORT_SYMBOL(pci_find_capability);
  * device's PCI configuration space or 0 in case the device does not
  * support it.
  */
-int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
+u8 pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, u8 cap)
 {
-	int pos;
+	u8 pos;
 	u8 hdr_type;
 
 	pci_bus_read_config_byte(bus, devfn, PCI_HEADER_TYPE, &hdr_type);
@@ -327,7 +327,7 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap)
 }
 EXPORT_SYMBOL_GPL(pci_find_ext_capability);
 
-static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
+static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
 {
 	int rc, ttl = PCI_FIND_CAP_TTL;
 	u8 cap, mask;
@@ -367,7 +367,7 @@ static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
  * NB. To be 100% safe against broken PCI devices, the caller should take
  * steps to avoid an infinite loop.
  */
-int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap)
+u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap)
 {
 	return __pci_find_next_ht_cap(dev, pos + PCI_CAP_LIST_NEXT, ht_cap);
 }
@@ -384,9 +384,9 @@ EXPORT_SYMBOL_GPL(pci_find_next_ht_capability);
  * The address points to the PCI capability, of type PCI_CAP_ID_HT,
  * which has a Hypertransport capability matching @ht_cap.
  */
-int pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
+u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
 {
-	int pos;
+	u8 pos;
 
 	pos = __pci_bus_find_cap_start(dev->bus, dev->devfn, dev->hdr_type);
 	if (pos)
@@ -896,7 +896,7 @@ static struct pci_cap_saved_state *_pci_find_saved_cap(struct pci_dev *pci_dev,
 	return NULL;
 }
 
-struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, char cap)
+struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, u8 cap)
 {
 	return _pci_find_saved_cap(dev, cap, false);
 }
@@ -956,11 +956,11 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
 
 static int pci_save_pcix_state(struct pci_dev *dev)
 {
-	int pos;
+	u8 pos;
 	struct pci_cap_saved_state *save_state;
 
 	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
-	if (pos <= 0)
+	if (!pos)
 		return 0;
 
 	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
@@ -977,13 +977,14 @@ static int pci_save_pcix_state(struct pci_dev *dev)
 
 static void pci_restore_pcix_state(struct pci_dev *dev)
 {
-	int i = 0, pos;
+	int i = 0;
+	u8 pos;
 	struct pci_cap_saved_state *save_state;
 	u16 *cap;
 
 	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
 	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
-	if (!save_state || pos <= 0)
+	if (!save_state || !pos)
 		return;
 	cap = (u16 *)&save_state->cap.data[0];
 
@@ -2036,7 +2037,7 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
  */
 void pci_pm_init(struct pci_dev *dev)
 {
-	int pm;
+	u8 pm;
 	u16 pmc;
 
 	pm_runtime_forbid(&dev->dev);
@@ -2126,7 +2127,7 @@ static int _pci_add_cap_save_buffer(struct pci_dev *dev, u16 cap,
 	else
 		pos = pci_find_capability(dev, cap);
 
-	if (pos <= 0)
+	if (!pos)
 		return 0;
 
 	save_state = kzalloc(sizeof(*save_state) + size, GFP_KERNEL);
@@ -2141,7 +2142,7 @@ static int _pci_add_cap_save_buffer(struct pci_dev *dev, u16 cap,
 	return 0;
 }
 
-int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size)
+int pci_add_cap_save_buffer(struct pci_dev *dev, u8 cap, unsigned int size)
 {
 	return _pci_add_cap_save_buffer(dev, cap, false, size);
 }
@@ -3046,7 +3047,7 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
  */
 void pci_msi_off(struct pci_dev *dev)
 {
-	int pos;
+	u8 pos;
 	u16 control;
 
 	/*
@@ -3120,7 +3121,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
 
 static int pci_af_flr(struct pci_dev *dev, int probe)
 {
-	int pos;
+	u8 pos;
 	u8 cap;
 
 	pos = pci_find_capability(dev, PCI_CAP_ID_AF);
@@ -3888,7 +3889,7 @@ EXPORT_SYMBOL_GPL(pci_try_reset_bus);
  */
 int pcix_get_max_mmrbc(struct pci_dev *dev)
 {
-	int cap;
+	u8 cap;
 	u32 stat;
 
 	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
@@ -3911,7 +3912,7 @@ EXPORT_SYMBOL(pcix_get_max_mmrbc);
  */
 int pcix_get_mmrbc(struct pci_dev *dev)
 {
-	int cap;
+	u8 cap;
 	u16 cmd;
 
 	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
@@ -3936,7 +3937,7 @@ EXPORT_SYMBOL(pcix_get_mmrbc);
  */
 int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
 {
-	int cap;
+	u8 cap;
 	u32 stat, v, o;
 	u16 cmd;
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4170113..9b316bf 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -606,7 +606,7 @@ static enum pci_bus_speed agp_speed(int agp3, int agpstat)
 static void pci_set_bus_speed(struct pci_bus *bus)
 {
 	struct pci_dev *bridge = bus->self;
-	int pos;
+	u8 pos;
 
 	pos = pci_find_capability(bridge, PCI_CAP_ID_AGP);
 	if (!pos)
@@ -958,7 +958,7 @@ static void pci_read_irq(struct pci_dev *dev)
 
 void set_pcie_port_type(struct pci_dev *pdev)
 {
-	int pos;
+	u8 pos;
 	u16 reg16;
 
 	pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
@@ -1046,7 +1046,7 @@ static int pci_cfg_space_size_ext(struct pci_dev *dev)
 
 int pci_cfg_space_size(struct pci_dev *dev)
 {
-	int pos;
+	u8 pos;
 	u32 status;
 	u16 class;
 
@@ -1087,7 +1087,7 @@ int pci_setup_device(struct pci_dev *dev)
 	u32 class;
 	u8 hdr_type;
 	struct pci_slot *slot;
-	int pos = 0;
+	u8 pos = 0;
 	struct pci_bus_region region;
 	struct resource *res;
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index a5f46b8..15b76a0 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
  * return 1 if a HT MSI capability is found and enabled */
 static int msi_ht_cap_enabled(struct pci_dev *dev)
 {
-	int pos, ttl = PCI_FIND_CAP_TTL;
+	u8 pos, ttl = PCI_FIND_CAP_TTL;
 
 	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
 	while (pos && ttl--) {
@@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
 /* Force enable MSI mapping capability on HT bridges */
 static void ht_enable_msi_mapping(struct pci_dev *dev)
 {
-	int pos, ttl = PCI_FIND_CAP_TTL;
+	u8 pos, ttl = PCI_FIND_CAP_TTL;
 
 	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
 	while (pos && ttl--) {
@@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
 
 static int ht_check_msi_mapping(struct pci_dev *dev)
 {
-	int pos, ttl = PCI_FIND_CAP_TTL;
+	u8 pos, ttl = PCI_FIND_CAP_TTL;
 	int found = 0;
 
 	/* check if there is HT MSI cap or enabled on this device */
@@ -2343,7 +2343,7 @@ static int ht_check_msi_mapping(struct pci_dev *dev)
 static int host_bridge_with_leaf(struct pci_dev *host_bridge)
 {
 	struct pci_dev *dev;
-	int pos;
+	u8 pos;
 	int i, dev_no;
 	int found = 0;
 
@@ -2376,7 +2376,8 @@ static int host_bridge_with_leaf(struct pci_dev *host_bridge)
 
 static int is_end_of_ht_chain(struct pci_dev *dev)
 {
-	int pos, ctrl_off;
+	int ctrl_off;
+	u8 pos;
 	int end = 0;
 	u16 flags, ctrl;
 
@@ -2401,7 +2402,7 @@ out:
 static void nv_ht_enable_msi_mapping(struct pci_dev *dev)
 {
 	struct pci_dev *host_bridge;
-	int pos;
+	u8 pos;
 	int i, dev_no;
 	int found = 0;
 
@@ -2439,7 +2440,7 @@ out:
 
 static void ht_disable_msi_mapping(struct pci_dev *dev)
 {
-	int pos, ttl = PCI_FIND_CAP_TTL;
+	u8 pos, ttl = PCI_FIND_CAP_TTL;
 
 	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
 	while (pos && ttl--) {
@@ -2460,7 +2461,7 @@ static void ht_disable_msi_mapping(struct pci_dev *dev)
 static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all)
 {
 	struct pci_dev *host_bridge;
-	int pos;
+	u8 pos;
 	int found;
 
 	if (!pci_msi_enabled())
@@ -3226,7 +3227,7 @@ fs_initcall_sync(pci_apply_final_quirks);
  */
 static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
 {
-	int pos;
+	u8 pos;
 
 	/* only implement PCI_CLASS_SERIAL_USB at present */
 	if (dev->class == PCI_CLASS_SERIAL_USB) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b27b79e..399a18a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -818,12 +818,12 @@ enum pci_lost_interrupt_reason {
 	PCI_LOST_IRQ_DISABLE_ACPI,
 };
 enum pci_lost_interrupt_reason pci_lost_interrupt(struct pci_dev *dev);
-int pci_find_capability(struct pci_dev *dev, int cap);
-int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
+u8 pci_find_capability(struct pci_dev *dev, u8 cap);
+u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, u8 cap);
 int pci_find_ext_capability(struct pci_dev *dev, int cap);
 int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
-int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
-int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
+u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
+u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap);
 struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
 
 struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device,
@@ -1004,10 +1004,10 @@ void pci_restore_state(struct pci_dev *dev);
 struct pci_saved_state *pci_store_saved_state(struct pci_dev *dev);
 int pci_load_and_free_saved_state(struct pci_dev *dev,
 				  struct pci_saved_state **state);
-struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, char cap);
+struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, u8 cap);
 struct pci_cap_saved_state *pci_find_saved_ext_cap(struct pci_dev *dev,
 						   u16 cap);
-int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size);
+int pci_add_cap_save_buffer(struct pci_dev *dev, u8 cap, unsigned int size);
 int pci_add_ext_cap_save_buffer(struct pci_dev *dev,
 				u16 cap, unsigned int size);
 int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state);
@@ -1045,7 +1045,7 @@ void set_pcie_port_type(struct pci_dev *pdev);
 void set_pcie_hotplug_bridge(struct pci_dev *pdev);
 
 /* Functions for PCI Hotplug drivers to use */
-int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
+u8 pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, u8 cap);
 unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge);
 unsigned int pci_rescan_bus(struct pci_bus *bus);
 void pci_lock_rescan_remove(void);
@@ -1360,10 +1360,10 @@ static inline int __pci_register_driver(struct pci_driver *drv,
 static inline int pci_register_driver(struct pci_driver *drv)
 { return 0; }
 static inline void pci_unregister_driver(struct pci_driver *drv) { }
-static inline int pci_find_capability(struct pci_dev *dev, int cap)
+static inline u8 pci_find_capability(struct pci_dev *dev, u8 cap)
 { return 0; }
-static inline int pci_find_next_capability(struct pci_dev *dev, u8 post,
-					   int cap)
+static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post,
+					   u8 cap)
 { return 0; }
 static inline int pci_find_ext_capability(struct pci_dev *dev, int cap)
 { return 0; }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] PCI: use u16 instead of int for pci express extended capabilities pos and cap
  2014-10-14  6:47 [PATCH 0/3] PCI: code clean up on pci configuration space Wei Yang
  2014-10-14  6:47 ` [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
  2014-10-14  6:47 ` [PATCH 2/3] PCI: use u8 instead of int for pci configuration space pos and cap Wei Yang
@ 2014-10-14  6:47 ` Wei Yang
  2 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2014-10-14  6:47 UTC (permalink / raw)
  To: linux-pci, bhelgaas; +Cc: Wei Yang

For pci express devices, it could have extended capabilities. The position of
extened capabilities is 12bit and the cap is 16bit.

This patch does a clean up for pci express extended capabilities by replacing
type int with u16.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/pci/iov.c   |    2 +-
 drivers/pci/pci.c   |   12 ++++++------
 drivers/pci/probe.c |    2 +-
 include/linux/pci.h |    6 +++---
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index cb6f247..fffab81 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -453,7 +453,7 @@ static void sriov_restore_state(struct pci_dev *dev)
  */
 int pci_iov_init(struct pci_dev *dev)
 {
-	int pos;
+	u16 pos;
 
 	if (!pci_is_pcie(dev))
 		return -ENODEV;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 47d8d0c..023af40 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -266,11 +266,11 @@ EXPORT_SYMBOL(pci_bus_find_capability);
  * not support it.  Some capabilities can occur several times, e.g., the
  * vendor-specific capability, and this provides a way to find them all.
  */
-int pci_find_next_ext_capability(struct pci_dev *dev, int start, int cap)
+u16 pci_find_next_ext_capability(struct pci_dev *dev, int start, u16 cap)
 {
 	u32 header;
 	int ttl;
-	int pos = PCI_CFG_SPACE_SIZE;
+	u16 pos = PCI_CFG_SPACE_SIZE;
 
 	/* minimum 8 bytes per capability */
 	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
@@ -321,7 +321,7 @@ EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
  *  %PCI_EXT_CAP_ID_DSN		Device Serial Number
  *  %PCI_EXT_CAP_ID_PWR		Power Budgeting
  */
-int pci_find_ext_capability(struct pci_dev *dev, int cap)
+u16 pci_find_ext_capability(struct pci_dev *dev, u16 cap)
 {
 	return pci_find_next_ext_capability(dev, 0, cap);
 }
@@ -2119,7 +2119,7 @@ static void pci_add_saved_cap(struct pci_dev *pci_dev,
 static int _pci_add_cap_save_buffer(struct pci_dev *dev, u16 cap,
 				    bool extended, unsigned int size)
 {
-	int pos;
+	u16 pos;
 	struct pci_cap_saved_state *save_state;
 
 	if (extended)
@@ -2233,7 +2233,7 @@ void pci_request_acs(void)
  */
 static int pci_std_enable_acs(struct pci_dev *dev)
 {
-	int pos;
+	u16 pos;
 	u16 cap;
 	u16 ctrl;
 
@@ -2278,7 +2278,7 @@ void pci_enable_acs(struct pci_dev *dev)
 
 static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
 {
-	int pos;
+	u16 pos;
 	u16 cap, ctrl;
 
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ACS);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 9b316bf..86147ba 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1425,7 +1425,7 @@ EXPORT_SYMBOL(pci_scan_single_device);
 
 static unsigned next_fn(struct pci_bus *bus, struct pci_dev *dev, unsigned fn)
 {
-	int pos;
+	u16 pos;
 	u16 cap = 0;
 	unsigned next_fn;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 399a18a..b9a4f40 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -820,8 +820,8 @@ enum pci_lost_interrupt_reason {
 enum pci_lost_interrupt_reason pci_lost_interrupt(struct pci_dev *dev);
 u8 pci_find_capability(struct pci_dev *dev, u8 cap);
 u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, u8 cap);
-int pci_find_ext_capability(struct pci_dev *dev, int cap);
-int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
+u16 pci_find_ext_capability(struct pci_dev *dev, u16 cap);
+u16 pci_find_next_ext_capability(struct pci_dev *dev, int pos, u16 cap);
 u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
 u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap);
 struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
@@ -1365,7 +1365,7 @@ static inline u8 pci_find_capability(struct pci_dev *dev, u8 cap)
 static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post,
 					   u8 cap)
 { return 0; }
-static inline int pci_find_ext_capability(struct pci_dev *dev, int cap)
+static inline u16 pci_find_ext_capability(struct pci_dev *dev, u16 cap)
 { return 0; }
 
 /* Power management related routines */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
  2014-10-14  6:47 ` [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
@ 2014-10-22 23:18   ` Bjorn Helgaas
  2014-10-24  3:44     ` Wei Yang
  2014-11-06  3:12     ` Wei Yang
  0 siblings, 2 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2014-10-22 23:18 UTC (permalink / raw)
  To: Wei Yang; +Cc: linux-pci, Yinghai Lu

[+cc Yinghai]

On Tue, Oct 14, 2014 at 02:47:30PM +0800, Wei Yang wrote:
> In some quirks, it tries to search a pci cap and use a ttl value to avoid
> infinite loop. While the value is hard coded to 48, which is the same as marco
> PCI_FIND_CAP_TTL.
> 
> This patch moves the definition of PCI_FIND_CAP_TTL to pci.h and replace the
> hard coded value with it.

This seems reasonable (though I added Yinghai in case he knows of any
reason why HT capabilities should be different from plain PCI capabilities
in this respect).

But I'd prefer to have the definition in drivers/pci/pci.h rather than in
include/linux/pci.h, because both users already include drivers/pci/pci.h,
and it is less visible.

Bjorn

> 
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  drivers/pci/pci.c    |    1 -
>  drivers/pci/quirks.c |    8 ++++----
>  include/linux/pci.h  |    2 ++
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 2c9ac70..76b002b1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -137,7 +137,6 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
>  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
>  #endif
>  
> -#define PCI_FIND_CAP_TTL	48
>  
>  static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>  				   u8 pos, int cap, int *ttl)
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 80c2d01..a5f46b8 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
>   * return 1 if a HT MSI capability is found and enabled */
>  static int msi_ht_cap_enabled(struct pci_dev *dev)
>  {
> -	int pos, ttl = 48;
> +	int pos, ttl = PCI_FIND_CAP_TTL;
>  
>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>  	while (pos && ttl--) {
> @@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
>  /* Force enable MSI mapping capability on HT bridges */
>  static void ht_enable_msi_mapping(struct pci_dev *dev)
>  {
> -	int pos, ttl = 48;
> +	int pos, ttl = PCI_FIND_CAP_TTL;
>  
>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>  	while (pos && ttl--) {
> @@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
>  
>  static int ht_check_msi_mapping(struct pci_dev *dev)
>  {
> -	int pos, ttl = 48;
> +	int pos, ttl = PCI_FIND_CAP_TTL;
>  	int found = 0;
>  
>  	/* check if there is HT MSI cap or enabled on this device */
> @@ -2439,7 +2439,7 @@ out:
>  
>  static void ht_disable_msi_mapping(struct pci_dev *dev)
>  {
> -	int pos, ttl = 48;
> +	int pos, ttl = PCI_FIND_CAP_TTL;
>  
>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>  	while (pos && ttl--) {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 96453f9..b27b79e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -33,6 +33,8 @@
>  
>  #include <linux/pci_ids.h>
>  
> +#define PCI_FIND_CAP_TTL	48
> +
>  /*
>   * The PCI interface treats multi-function devices as independent
>   * devices.  The slot/function address of each device is encoded
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] PCI: use u8 instead of int for pci configuration space pos and cap
  2014-10-14  6:47 ` [PATCH 2/3] PCI: use u8 instead of int for pci configuration space pos and cap Wei Yang
@ 2014-10-22 23:23   ` Bjorn Helgaas
  2014-10-24  3:49     ` Wei Yang
  0 siblings, 1 reply; 10+ messages in thread
From: Bjorn Helgaas @ 2014-10-22 23:23 UTC (permalink / raw)
  To: Wei Yang; +Cc: linux-pci, Yinghai Lu

[+cc Yinghai]

On Tue, Oct 14, 2014 at 02:47:31PM +0800, Wei Yang wrote:
> For pci devices complying with PCI LB 3.0, the configuration space size is
> 256 and the Cap ID is represented by a 8bit field. This means a type of u8 is
> enough to repsent the Cap's position and ID.
> 
> This patch does some clean up for the Cap position and ID by replacing the int
> type with u8. And consolidate the check of whether a Cap is represented from
> (pos <= 0) to (!pos).
> 
> And two functions use char to represent cap, it is changed to u8 in this
> patch.

Please spell-check the changelog.  Please also split the return value check
changes ("pos <= 0" to "!pos") into a separate patch.  That part is not
obviously correct.

> 
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  drivers/pci/pci.c    |   55 +++++++++++++++++++++++++-------------------------
>  drivers/pci/probe.c  |    8 ++++----
>  drivers/pci/quirks.c |   19 ++++++++---------
>  include/linux/pci.h  |   20 +++++++++---------
>  4 files changed, 52 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 76b002b1..47d8d0c 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -138,8 +138,8 @@ EXPORT_SYMBOL_GPL(pci_ioremap_bar);
>  #endif
>  
>  
> -static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
> -				   u8 pos, int cap, int *ttl)
> +static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
> +				   u8 pos, u8 cap, int *ttl)
>  {
>  	u8 id;
>  
> @@ -159,22 +159,22 @@ static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>  	return 0;
>  }
>  
> -static int __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
> -			       u8 pos, int cap)
> +static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
> +			       u8 pos, u8 cap)
>  {
>  	int ttl = PCI_FIND_CAP_TTL;
>  
>  	return __pci_find_next_cap_ttl(bus, devfn, pos, cap, &ttl);
>  }
>  
> -int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap)
> +u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, u8 cap)
>  {
>  	return __pci_find_next_cap(dev->bus, dev->devfn,
>  				   pos + PCI_CAP_LIST_NEXT, cap);
>  }
>  EXPORT_SYMBOL_GPL(pci_find_next_capability);
>  
> -static int __pci_bus_find_cap_start(struct pci_bus *bus,
> +static u8 __pci_bus_find_cap_start(struct pci_bus *bus,
>  				    unsigned int devfn, u8 hdr_type)
>  {
>  	u16 status;
> @@ -215,9 +215,9 @@ static int __pci_bus_find_cap_start(struct pci_bus *bus,
>   *  %PCI_CAP_ID_PCIX         PCI-X
>   *  %PCI_CAP_ID_EXP          PCI Express
>   */
> -int pci_find_capability(struct pci_dev *dev, int cap)
> +u8 pci_find_capability(struct pci_dev *dev, u8 cap)
>  {
> -	int pos;
> +	u8 pos;
>  
>  	pos = __pci_bus_find_cap_start(dev->bus, dev->devfn, dev->hdr_type);
>  	if (pos)
> @@ -240,9 +240,9 @@ EXPORT_SYMBOL(pci_find_capability);
>   * device's PCI configuration space or 0 in case the device does not
>   * support it.
>   */
> -int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
> +u8 pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, u8 cap)
>  {
> -	int pos;
> +	u8 pos;
>  	u8 hdr_type;
>  
>  	pci_bus_read_config_byte(bus, devfn, PCI_HEADER_TYPE, &hdr_type);
> @@ -327,7 +327,7 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap)
>  }
>  EXPORT_SYMBOL_GPL(pci_find_ext_capability);
>  
> -static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
> +static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>  {
>  	int rc, ttl = PCI_FIND_CAP_TTL;
>  	u8 cap, mask;
> @@ -367,7 +367,7 @@ static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
>   * NB. To be 100% safe against broken PCI devices, the caller should take
>   * steps to avoid an infinite loop.
>   */
> -int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap)
> +u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap)
>  {
>  	return __pci_find_next_ht_cap(dev, pos + PCI_CAP_LIST_NEXT, ht_cap);
>  }
> @@ -384,9 +384,9 @@ EXPORT_SYMBOL_GPL(pci_find_next_ht_capability);
>   * The address points to the PCI capability, of type PCI_CAP_ID_HT,
>   * which has a Hypertransport capability matching @ht_cap.
>   */
> -int pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
> +u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
>  {
> -	int pos;
> +	u8 pos;
>  
>  	pos = __pci_bus_find_cap_start(dev->bus, dev->devfn, dev->hdr_type);
>  	if (pos)
> @@ -896,7 +896,7 @@ static struct pci_cap_saved_state *_pci_find_saved_cap(struct pci_dev *pci_dev,
>  	return NULL;
>  }
>  
> -struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, char cap)
> +struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, u8 cap)
>  {
>  	return _pci_find_saved_cap(dev, cap, false);
>  }
> @@ -956,11 +956,11 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
>  
>  static int pci_save_pcix_state(struct pci_dev *dev)
>  {
> -	int pos;
> +	u8 pos;
>  	struct pci_cap_saved_state *save_state;
>  
>  	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> -	if (pos <= 0)
> +	if (!pos)
>  		return 0;
>  
>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
> @@ -977,13 +977,14 @@ static int pci_save_pcix_state(struct pci_dev *dev)
>  
>  static void pci_restore_pcix_state(struct pci_dev *dev)
>  {
> -	int i = 0, pos;
> +	int i = 0;
> +	u8 pos;
>  	struct pci_cap_saved_state *save_state;
>  	u16 *cap;
>  
>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
>  	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> -	if (!save_state || pos <= 0)
> +	if (!save_state || !pos)
>  		return;
>  	cap = (u16 *)&save_state->cap.data[0];
>  
> @@ -2036,7 +2037,7 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
>   */
>  void pci_pm_init(struct pci_dev *dev)
>  {
> -	int pm;
> +	u8 pm;
>  	u16 pmc;
>  
>  	pm_runtime_forbid(&dev->dev);
> @@ -2126,7 +2127,7 @@ static int _pci_add_cap_save_buffer(struct pci_dev *dev, u16 cap,
>  	else
>  		pos = pci_find_capability(dev, cap);
>  
> -	if (pos <= 0)
> +	if (!pos)
>  		return 0;
>  
>  	save_state = kzalloc(sizeof(*save_state) + size, GFP_KERNEL);
> @@ -2141,7 +2142,7 @@ static int _pci_add_cap_save_buffer(struct pci_dev *dev, u16 cap,
>  	return 0;
>  }
>  
> -int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size)
> +int pci_add_cap_save_buffer(struct pci_dev *dev, u8 cap, unsigned int size)
>  {
>  	return _pci_add_cap_save_buffer(dev, cap, false, size);
>  }
> @@ -3046,7 +3047,7 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
>   */
>  void pci_msi_off(struct pci_dev *dev)
>  {
> -	int pos;
> +	u8 pos;
>  	u16 control;
>  
>  	/*
> @@ -3120,7 +3121,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>  
>  static int pci_af_flr(struct pci_dev *dev, int probe)
>  {
> -	int pos;
> +	u8 pos;
>  	u8 cap;
>  
>  	pos = pci_find_capability(dev, PCI_CAP_ID_AF);
> @@ -3888,7 +3889,7 @@ EXPORT_SYMBOL_GPL(pci_try_reset_bus);
>   */
>  int pcix_get_max_mmrbc(struct pci_dev *dev)
>  {
> -	int cap;
> +	u8 cap;
>  	u32 stat;
>  
>  	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> @@ -3911,7 +3912,7 @@ EXPORT_SYMBOL(pcix_get_max_mmrbc);
>   */
>  int pcix_get_mmrbc(struct pci_dev *dev)
>  {
> -	int cap;
> +	u8 cap;
>  	u16 cmd;
>  
>  	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> @@ -3936,7 +3937,7 @@ EXPORT_SYMBOL(pcix_get_mmrbc);
>   */
>  int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
>  {
> -	int cap;
> +	u8 cap;
>  	u32 stat, v, o;
>  	u16 cmd;
>  
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4170113..9b316bf 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -606,7 +606,7 @@ static enum pci_bus_speed agp_speed(int agp3, int agpstat)
>  static void pci_set_bus_speed(struct pci_bus *bus)
>  {
>  	struct pci_dev *bridge = bus->self;
> -	int pos;
> +	u8 pos;
>  
>  	pos = pci_find_capability(bridge, PCI_CAP_ID_AGP);
>  	if (!pos)
> @@ -958,7 +958,7 @@ static void pci_read_irq(struct pci_dev *dev)
>  
>  void set_pcie_port_type(struct pci_dev *pdev)
>  {
> -	int pos;
> +	u8 pos;
>  	u16 reg16;
>  
>  	pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
> @@ -1046,7 +1046,7 @@ static int pci_cfg_space_size_ext(struct pci_dev *dev)
>  
>  int pci_cfg_space_size(struct pci_dev *dev)
>  {
> -	int pos;
> +	u8 pos;
>  	u32 status;
>  	u16 class;
>  
> @@ -1087,7 +1087,7 @@ int pci_setup_device(struct pci_dev *dev)
>  	u32 class;
>  	u8 hdr_type;
>  	struct pci_slot *slot;
> -	int pos = 0;
> +	u8 pos = 0;
>  	struct pci_bus_region region;
>  	struct resource *res;
>  
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index a5f46b8..15b76a0 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
>   * return 1 if a HT MSI capability is found and enabled */
>  static int msi_ht_cap_enabled(struct pci_dev *dev)
>  {
> -	int pos, ttl = PCI_FIND_CAP_TTL;
> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>  
>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>  	while (pos && ttl--) {
> @@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
>  /* Force enable MSI mapping capability on HT bridges */
>  static void ht_enable_msi_mapping(struct pci_dev *dev)
>  {
> -	int pos, ttl = PCI_FIND_CAP_TTL;
> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>  
>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>  	while (pos && ttl--) {
> @@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
>  
>  static int ht_check_msi_mapping(struct pci_dev *dev)
>  {
> -	int pos, ttl = PCI_FIND_CAP_TTL;
> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>  	int found = 0;
>  
>  	/* check if there is HT MSI cap or enabled on this device */
> @@ -2343,7 +2343,7 @@ static int ht_check_msi_mapping(struct pci_dev *dev)
>  static int host_bridge_with_leaf(struct pci_dev *host_bridge)
>  {
>  	struct pci_dev *dev;
> -	int pos;
> +	u8 pos;
>  	int i, dev_no;
>  	int found = 0;
>  
> @@ -2376,7 +2376,8 @@ static int host_bridge_with_leaf(struct pci_dev *host_bridge)
>  
>  static int is_end_of_ht_chain(struct pci_dev *dev)
>  {
> -	int pos, ctrl_off;
> +	int ctrl_off;
> +	u8 pos;
>  	int end = 0;
>  	u16 flags, ctrl;
>  
> @@ -2401,7 +2402,7 @@ out:
>  static void nv_ht_enable_msi_mapping(struct pci_dev *dev)
>  {
>  	struct pci_dev *host_bridge;
> -	int pos;
> +	u8 pos;
>  	int i, dev_no;
>  	int found = 0;
>  
> @@ -2439,7 +2440,7 @@ out:
>  
>  static void ht_disable_msi_mapping(struct pci_dev *dev)
>  {
> -	int pos, ttl = PCI_FIND_CAP_TTL;
> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>  
>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>  	while (pos && ttl--) {
> @@ -2460,7 +2461,7 @@ static void ht_disable_msi_mapping(struct pci_dev *dev)
>  static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all)
>  {
>  	struct pci_dev *host_bridge;
> -	int pos;
> +	u8 pos;
>  	int found;
>  
>  	if (!pci_msi_enabled())
> @@ -3226,7 +3227,7 @@ fs_initcall_sync(pci_apply_final_quirks);
>   */
>  static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
>  {
> -	int pos;
> +	u8 pos;
>  
>  	/* only implement PCI_CLASS_SERIAL_USB at present */
>  	if (dev->class == PCI_CLASS_SERIAL_USB) {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index b27b79e..399a18a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -818,12 +818,12 @@ enum pci_lost_interrupt_reason {
>  	PCI_LOST_IRQ_DISABLE_ACPI,
>  };
>  enum pci_lost_interrupt_reason pci_lost_interrupt(struct pci_dev *dev);
> -int pci_find_capability(struct pci_dev *dev, int cap);
> -int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
> +u8 pci_find_capability(struct pci_dev *dev, u8 cap);
> +u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, u8 cap);
>  int pci_find_ext_capability(struct pci_dev *dev, int cap);
>  int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
> -int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
> -int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
> +u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
> +u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap);
>  struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
>  
>  struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device,
> @@ -1004,10 +1004,10 @@ void pci_restore_state(struct pci_dev *dev);
>  struct pci_saved_state *pci_store_saved_state(struct pci_dev *dev);
>  int pci_load_and_free_saved_state(struct pci_dev *dev,
>  				  struct pci_saved_state **state);
> -struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, char cap);
> +struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, u8 cap);
>  struct pci_cap_saved_state *pci_find_saved_ext_cap(struct pci_dev *dev,
>  						   u16 cap);
> -int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size);
> +int pci_add_cap_save_buffer(struct pci_dev *dev, u8 cap, unsigned int size);
>  int pci_add_ext_cap_save_buffer(struct pci_dev *dev,
>  				u16 cap, unsigned int size);
>  int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state);
> @@ -1045,7 +1045,7 @@ void set_pcie_port_type(struct pci_dev *pdev);
>  void set_pcie_hotplug_bridge(struct pci_dev *pdev);
>  
>  /* Functions for PCI Hotplug drivers to use */
> -int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
> +u8 pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, u8 cap);
>  unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge);
>  unsigned int pci_rescan_bus(struct pci_bus *bus);
>  void pci_lock_rescan_remove(void);
> @@ -1360,10 +1360,10 @@ static inline int __pci_register_driver(struct pci_driver *drv,
>  static inline int pci_register_driver(struct pci_driver *drv)
>  { return 0; }
>  static inline void pci_unregister_driver(struct pci_driver *drv) { }
> -static inline int pci_find_capability(struct pci_dev *dev, int cap)
> +static inline u8 pci_find_capability(struct pci_dev *dev, u8 cap)
>  { return 0; }
> -static inline int pci_find_next_capability(struct pci_dev *dev, u8 post,
> -					   int cap)
> +static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post,
> +					   u8 cap)
>  { return 0; }
>  static inline int pci_find_ext_capability(struct pci_dev *dev, int cap)
>  { return 0; }
> -- 
> 1.7.9.5
> 
> --
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
  2014-10-22 23:18   ` Bjorn Helgaas
@ 2014-10-24  3:44     ` Wei Yang
  2014-11-06  3:12     ` Wei Yang
  1 sibling, 0 replies; 10+ messages in thread
From: Wei Yang @ 2014-10-24  3:44 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Wei Yang, linux-pci, Yinghai Lu

On Wed, Oct 22, 2014 at 05:18:30PM -0600, Bjorn Helgaas wrote:
>[+cc Yinghai]
>
>On Tue, Oct 14, 2014 at 02:47:30PM +0800, Wei Yang wrote:
>> In some quirks, it tries to search a pci cap and use a ttl value to avoid
>> infinite loop. While the value is hard coded to 48, which is the same as marco
>> PCI_FIND_CAP_TTL.
>> 
>> This patch moves the definition of PCI_FIND_CAP_TTL to pci.h and replace the
>> hard coded value with it.
>
>This seems reasonable (though I added Yinghai in case he knows of any
>reason why HT capabilities should be different from plain PCI capabilities
>in this respect).
>
>But I'd prefer to have the definition in drivers/pci/pci.h rather than in
>include/linux/pci.h, because both users already include drivers/pci/pci.h,
>and it is less visible.

Got it.

>
>Bjorn
>
>> 
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/pci.c    |    1 -
>>  drivers/pci/quirks.c |    8 ++++----
>>  include/linux/pci.h  |    2 ++
>>  3 files changed, 6 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 2c9ac70..76b002b1 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -137,7 +137,6 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
>>  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
>>  #endif
>>  
>> -#define PCI_FIND_CAP_TTL	48
>>  
>>  static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>>  				   u8 pos, int cap, int *ttl)
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 80c2d01..a5f46b8 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
>>   * return 1 if a HT MSI capability is found and enabled */
>>  static int msi_ht_cap_enabled(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> @@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
>>  /* Force enable MSI mapping capability on HT bridges */
>>  static void ht_enable_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> @@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
>>  
>>  static int ht_check_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  	int found = 0;
>>  
>>  	/* check if there is HT MSI cap or enabled on this device */
>> @@ -2439,7 +2439,7 @@ out:
>>  
>>  static void ht_disable_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 96453f9..b27b79e 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -33,6 +33,8 @@
>>  
>>  #include <linux/pci_ids.h>
>>  
>> +#define PCI_FIND_CAP_TTL	48
>> +
>>  /*
>>   * The PCI interface treats multi-function devices as independent
>>   * devices.  The slot/function address of each device is encoded
>> -- 
>> 1.7.9.5
>> 

-- 
Richard Yang
Help you, Help me


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] PCI: use u8 instead of int for pci configuration space pos and cap
  2014-10-22 23:23   ` Bjorn Helgaas
@ 2014-10-24  3:49     ` Wei Yang
  0 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2014-10-24  3:49 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Wei Yang, linux-pci, Yinghai Lu

On Wed, Oct 22, 2014 at 05:23:07PM -0600, Bjorn Helgaas wrote:
>[+cc Yinghai]
>
>On Tue, Oct 14, 2014 at 02:47:31PM +0800, Wei Yang wrote:
>> For pci devices complying with PCI LB 3.0, the configuration space size is
>> 256 and the Cap ID is represented by a 8bit field. This means a type of u8 is
>> enough to repsent the Cap's position and ID.
>> 
>> This patch does some clean up for the Cap position and ID by replacing the int
>> type with u8. And consolidate the check of whether a Cap is represented from
>> (pos <= 0) to (!pos).
>> 
>> And two functions use char to represent cap, it is changed to u8 in this
>> patch.
>
>Please spell-check the changelog.  Please also split the return value check
>changes ("pos <= 0" to "!pos") into a separate patch.  That part is not
>obviously correct.

Sorry for my wrong word :-(

Will seperate it them.

>
>> 
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/pci.c    |   55 +++++++++++++++++++++++++-------------------------
>>  drivers/pci/probe.c  |    8 ++++----
>>  drivers/pci/quirks.c |   19 ++++++++---------
>>  include/linux/pci.h  |   20 +++++++++---------
>>  4 files changed, 52 insertions(+), 50 deletions(-)
>> 
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 76b002b1..47d8d0c 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -138,8 +138,8 @@ EXPORT_SYMBOL_GPL(pci_ioremap_bar);
>>  #endif
>>  
>>  
>> -static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>> -				   u8 pos, int cap, int *ttl)
>> +static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>> +				   u8 pos, u8 cap, int *ttl)
>>  {
>>  	u8 id;
>>  
>> @@ -159,22 +159,22 @@ static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>>  	return 0;
>>  }
>>  
>> -static int __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
>> -			       u8 pos, int cap)
>> +static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
>> +			       u8 pos, u8 cap)
>>  {
>>  	int ttl = PCI_FIND_CAP_TTL;
>>  
>>  	return __pci_find_next_cap_ttl(bus, devfn, pos, cap, &ttl);
>>  }
>>  
>> -int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap)
>> +u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, u8 cap)
>>  {
>>  	return __pci_find_next_cap(dev->bus, dev->devfn,
>>  				   pos + PCI_CAP_LIST_NEXT, cap);
>>  }
>>  EXPORT_SYMBOL_GPL(pci_find_next_capability);
>>  
>> -static int __pci_bus_find_cap_start(struct pci_bus *bus,
>> +static u8 __pci_bus_find_cap_start(struct pci_bus *bus,
>>  				    unsigned int devfn, u8 hdr_type)
>>  {
>>  	u16 status;
>> @@ -215,9 +215,9 @@ static int __pci_bus_find_cap_start(struct pci_bus *bus,
>>   *  %PCI_CAP_ID_PCIX         PCI-X
>>   *  %PCI_CAP_ID_EXP          PCI Express
>>   */
>> -int pci_find_capability(struct pci_dev *dev, int cap)
>> +u8 pci_find_capability(struct pci_dev *dev, u8 cap)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  
>>  	pos = __pci_bus_find_cap_start(dev->bus, dev->devfn, dev->hdr_type);
>>  	if (pos)
>> @@ -240,9 +240,9 @@ EXPORT_SYMBOL(pci_find_capability);
>>   * device's PCI configuration space or 0 in case the device does not
>>   * support it.
>>   */
>> -int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
>> +u8 pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, u8 cap)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  	u8 hdr_type;
>>  
>>  	pci_bus_read_config_byte(bus, devfn, PCI_HEADER_TYPE, &hdr_type);
>> @@ -327,7 +327,7 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap)
>>  }
>>  EXPORT_SYMBOL_GPL(pci_find_ext_capability);
>>  
>> -static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
>> +static u8 __pci_find_next_ht_cap(struct pci_dev *dev, u8 pos, int ht_cap)
>>  {
>>  	int rc, ttl = PCI_FIND_CAP_TTL;
>>  	u8 cap, mask;
>> @@ -367,7 +367,7 @@ static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap)
>>   * NB. To be 100% safe against broken PCI devices, the caller should take
>>   * steps to avoid an infinite loop.
>>   */
>> -int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap)
>> +u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap)
>>  {
>>  	return __pci_find_next_ht_cap(dev, pos + PCI_CAP_LIST_NEXT, ht_cap);
>>  }
>> @@ -384,9 +384,9 @@ EXPORT_SYMBOL_GPL(pci_find_next_ht_capability);
>>   * The address points to the PCI capability, of type PCI_CAP_ID_HT,
>>   * which has a Hypertransport capability matching @ht_cap.
>>   */
>> -int pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
>> +u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  
>>  	pos = __pci_bus_find_cap_start(dev->bus, dev->devfn, dev->hdr_type);
>>  	if (pos)
>> @@ -896,7 +896,7 @@ static struct pci_cap_saved_state *_pci_find_saved_cap(struct pci_dev *pci_dev,
>>  	return NULL;
>>  }
>>  
>> -struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, char cap)
>> +struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, u8 cap)
>>  {
>>  	return _pci_find_saved_cap(dev, cap, false);
>>  }
>> @@ -956,11 +956,11 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
>>  
>>  static int pci_save_pcix_state(struct pci_dev *dev)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  	struct pci_cap_saved_state *save_state;
>>  
>>  	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>> -	if (pos <= 0)
>> +	if (!pos)
>>  		return 0;
>>  
>>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
>> @@ -977,13 +977,14 @@ static int pci_save_pcix_state(struct pci_dev *dev)
>>  
>>  static void pci_restore_pcix_state(struct pci_dev *dev)
>>  {
>> -	int i = 0, pos;
>> +	int i = 0;
>> +	u8 pos;
>>  	struct pci_cap_saved_state *save_state;
>>  	u16 *cap;
>>  
>>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
>>  	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>> -	if (!save_state || pos <= 0)
>> +	if (!save_state || !pos)
>>  		return;
>>  	cap = (u16 *)&save_state->cap.data[0];
>>  
>> @@ -2036,7 +2037,7 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
>>   */
>>  void pci_pm_init(struct pci_dev *dev)
>>  {
>> -	int pm;
>> +	u8 pm;
>>  	u16 pmc;
>>  
>>  	pm_runtime_forbid(&dev->dev);
>> @@ -2126,7 +2127,7 @@ static int _pci_add_cap_save_buffer(struct pci_dev *dev, u16 cap,
>>  	else
>>  		pos = pci_find_capability(dev, cap);
>>  
>> -	if (pos <= 0)
>> +	if (!pos)
>>  		return 0;
>>  
>>  	save_state = kzalloc(sizeof(*save_state) + size, GFP_KERNEL);
>> @@ -2141,7 +2142,7 @@ static int _pci_add_cap_save_buffer(struct pci_dev *dev, u16 cap,
>>  	return 0;
>>  }
>>  
>> -int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size)
>> +int pci_add_cap_save_buffer(struct pci_dev *dev, u8 cap, unsigned int size)
>>  {
>>  	return _pci_add_cap_save_buffer(dev, cap, false, size);
>>  }
>> @@ -3046,7 +3047,7 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
>>   */
>>  void pci_msi_off(struct pci_dev *dev)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  	u16 control;
>>  
>>  	/*
>> @@ -3120,7 +3121,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>>  
>>  static int pci_af_flr(struct pci_dev *dev, int probe)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  	u8 cap;
>>  
>>  	pos = pci_find_capability(dev, PCI_CAP_ID_AF);
>> @@ -3888,7 +3889,7 @@ EXPORT_SYMBOL_GPL(pci_try_reset_bus);
>>   */
>>  int pcix_get_max_mmrbc(struct pci_dev *dev)
>>  {
>> -	int cap;
>> +	u8 cap;
>>  	u32 stat;
>>  
>>  	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>> @@ -3911,7 +3912,7 @@ EXPORT_SYMBOL(pcix_get_max_mmrbc);
>>   */
>>  int pcix_get_mmrbc(struct pci_dev *dev)
>>  {
>> -	int cap;
>> +	u8 cap;
>>  	u16 cmd;
>>  
>>  	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>> @@ -3936,7 +3937,7 @@ EXPORT_SYMBOL(pcix_get_mmrbc);
>>   */
>>  int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
>>  {
>> -	int cap;
>> +	u8 cap;
>>  	u32 stat, v, o;
>>  	u16 cmd;
>>  
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 4170113..9b316bf 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -606,7 +606,7 @@ static enum pci_bus_speed agp_speed(int agp3, int agpstat)
>>  static void pci_set_bus_speed(struct pci_bus *bus)
>>  {
>>  	struct pci_dev *bridge = bus->self;
>> -	int pos;
>> +	u8 pos;
>>  
>>  	pos = pci_find_capability(bridge, PCI_CAP_ID_AGP);
>>  	if (!pos)
>> @@ -958,7 +958,7 @@ static void pci_read_irq(struct pci_dev *dev)
>>  
>>  void set_pcie_port_type(struct pci_dev *pdev)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  	u16 reg16;
>>  
>>  	pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>> @@ -1046,7 +1046,7 @@ static int pci_cfg_space_size_ext(struct pci_dev *dev)
>>  
>>  int pci_cfg_space_size(struct pci_dev *dev)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  	u32 status;
>>  	u16 class;
>>  
>> @@ -1087,7 +1087,7 @@ int pci_setup_device(struct pci_dev *dev)
>>  	u32 class;
>>  	u8 hdr_type;
>>  	struct pci_slot *slot;
>> -	int pos = 0;
>> +	u8 pos = 0;
>>  	struct pci_bus_region region;
>>  	struct resource *res;
>>  
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index a5f46b8..15b76a0 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
>>   * return 1 if a HT MSI capability is found and enabled */
>>  static int msi_ht_cap_enabled(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = PCI_FIND_CAP_TTL;
>> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> @@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
>>  /* Force enable MSI mapping capability on HT bridges */
>>  static void ht_enable_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = PCI_FIND_CAP_TTL;
>> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> @@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
>>  
>>  static int ht_check_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = PCI_FIND_CAP_TTL;
>> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>>  	int found = 0;
>>  
>>  	/* check if there is HT MSI cap or enabled on this device */
>> @@ -2343,7 +2343,7 @@ static int ht_check_msi_mapping(struct pci_dev *dev)
>>  static int host_bridge_with_leaf(struct pci_dev *host_bridge)
>>  {
>>  	struct pci_dev *dev;
>> -	int pos;
>> +	u8 pos;
>>  	int i, dev_no;
>>  	int found = 0;
>>  
>> @@ -2376,7 +2376,8 @@ static int host_bridge_with_leaf(struct pci_dev *host_bridge)
>>  
>>  static int is_end_of_ht_chain(struct pci_dev *dev)
>>  {
>> -	int pos, ctrl_off;
>> +	int ctrl_off;
>> +	u8 pos;
>>  	int end = 0;
>>  	u16 flags, ctrl;
>>  
>> @@ -2401,7 +2402,7 @@ out:
>>  static void nv_ht_enable_msi_mapping(struct pci_dev *dev)
>>  {
>>  	struct pci_dev *host_bridge;
>> -	int pos;
>> +	u8 pos;
>>  	int i, dev_no;
>>  	int found = 0;
>>  
>> @@ -2439,7 +2440,7 @@ out:
>>  
>>  static void ht_disable_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = PCI_FIND_CAP_TTL;
>> +	u8 pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> @@ -2460,7 +2461,7 @@ static void ht_disable_msi_mapping(struct pci_dev *dev)
>>  static void __nv_msi_ht_cap_quirk(struct pci_dev *dev, int all)
>>  {
>>  	struct pci_dev *host_bridge;
>> -	int pos;
>> +	u8 pos;
>>  	int found;
>>  
>>  	if (!pci_msi_enabled())
>> @@ -3226,7 +3227,7 @@ fs_initcall_sync(pci_apply_final_quirks);
>>   */
>>  static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
>>  {
>> -	int pos;
>> +	u8 pos;
>>  
>>  	/* only implement PCI_CLASS_SERIAL_USB at present */
>>  	if (dev->class == PCI_CLASS_SERIAL_USB) {
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index b27b79e..399a18a 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -818,12 +818,12 @@ enum pci_lost_interrupt_reason {
>>  	PCI_LOST_IRQ_DISABLE_ACPI,
>>  };
>>  enum pci_lost_interrupt_reason pci_lost_interrupt(struct pci_dev *dev);
>> -int pci_find_capability(struct pci_dev *dev, int cap);
>> -int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
>> +u8 pci_find_capability(struct pci_dev *dev, u8 cap);
>> +u8 pci_find_next_capability(struct pci_dev *dev, u8 pos, u8 cap);
>>  int pci_find_ext_capability(struct pci_dev *dev, int cap);
>>  int pci_find_next_ext_capability(struct pci_dev *dev, int pos, int cap);
>> -int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
>> -int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap);
>> +u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
>> +u8 pci_find_next_ht_capability(struct pci_dev *dev, u8 pos, int ht_cap);
>>  struct pci_bus *pci_find_next_bus(const struct pci_bus *from);
>>  
>>  struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device,
>> @@ -1004,10 +1004,10 @@ void pci_restore_state(struct pci_dev *dev);
>>  struct pci_saved_state *pci_store_saved_state(struct pci_dev *dev);
>>  int pci_load_and_free_saved_state(struct pci_dev *dev,
>>  				  struct pci_saved_state **state);
>> -struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, char cap);
>> +struct pci_cap_saved_state *pci_find_saved_cap(struct pci_dev *dev, u8 cap);
>>  struct pci_cap_saved_state *pci_find_saved_ext_cap(struct pci_dev *dev,
>>  						   u16 cap);
>> -int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size);
>> +int pci_add_cap_save_buffer(struct pci_dev *dev, u8 cap, unsigned int size);
>>  int pci_add_ext_cap_save_buffer(struct pci_dev *dev,
>>  				u16 cap, unsigned int size);
>>  int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state);
>> @@ -1045,7 +1045,7 @@ void set_pcie_port_type(struct pci_dev *pdev);
>>  void set_pcie_hotplug_bridge(struct pci_dev *pdev);
>>  
>>  /* Functions for PCI Hotplug drivers to use */
>> -int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
>> +u8 pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, u8 cap);
>>  unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge);
>>  unsigned int pci_rescan_bus(struct pci_bus *bus);
>>  void pci_lock_rescan_remove(void);
>> @@ -1360,10 +1360,10 @@ static inline int __pci_register_driver(struct pci_driver *drv,
>>  static inline int pci_register_driver(struct pci_driver *drv)
>>  { return 0; }
>>  static inline void pci_unregister_driver(struct pci_driver *drv) { }
>> -static inline int pci_find_capability(struct pci_dev *dev, int cap)
>> +static inline u8 pci_find_capability(struct pci_dev *dev, u8 cap)
>>  { return 0; }
>> -static inline int pci_find_next_capability(struct pci_dev *dev, u8 post,
>> -					   int cap)
>> +static inline u8 pci_find_next_capability(struct pci_dev *dev, u8 post,
>> +					   u8 cap)
>>  { return 0; }
>>  static inline int pci_find_ext_capability(struct pci_dev *dev, int cap)
>>  { return 0; }
>> -- 
>> 1.7.9.5
>> 
>> --
>> 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

-- 
Richard Yang
Help you, Help me


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
  2014-10-22 23:18   ` Bjorn Helgaas
  2014-10-24  3:44     ` Wei Yang
@ 2014-11-06  3:12     ` Wei Yang
  2014-11-18  9:16       ` Wei Yang
  1 sibling, 1 reply; 10+ messages in thread
From: Wei Yang @ 2014-11-06  3:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Yinghai Lu; +Cc: Wei Yang, linux-pci

On Wed, Oct 22, 2014 at 05:18:30PM -0600, Bjorn Helgaas wrote:
>[+cc Yinghai]
>
>On Tue, Oct 14, 2014 at 02:47:30PM +0800, Wei Yang wrote:
>> In some quirks, it tries to search a pci cap and use a ttl value to avoid
>> infinite loop. While the value is hard coded to 48, which is the same as marco
>> PCI_FIND_CAP_TTL.
>> 
>> This patch moves the definition of PCI_FIND_CAP_TTL to pci.h and replace the
>> hard coded value with it.
>
>This seems reasonable (though I added Yinghai in case he knows of any
>reason why HT capabilities should be different from plain PCI capabilities
>in this respect).

Hi, Yinghai,

Do you have some historical story on the definition?

>
>But I'd prefer to have the definition in drivers/pci/pci.h rather than in
>include/linux/pci.h, because both users already include drivers/pci/pci.h,
>and it is less visible.
>
>Bjorn
>
>> 
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/pci.c    |    1 -
>>  drivers/pci/quirks.c |    8 ++++----
>>  include/linux/pci.h  |    2 ++
>>  3 files changed, 6 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 2c9ac70..76b002b1 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -137,7 +137,6 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
>>  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
>>  #endif
>>  
>> -#define PCI_FIND_CAP_TTL	48
>>  
>>  static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>>  				   u8 pos, int cap, int *ttl)
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 80c2d01..a5f46b8 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
>>   * return 1 if a HT MSI capability is found and enabled */
>>  static int msi_ht_cap_enabled(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> @@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
>>  /* Force enable MSI mapping capability on HT bridges */
>>  static void ht_enable_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> @@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
>>  
>>  static int ht_check_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  	int found = 0;
>>  
>>  	/* check if there is HT MSI cap or enabled on this device */
>> @@ -2439,7 +2439,7 @@ out:
>>  
>>  static void ht_disable_msi_mapping(struct pci_dev *dev)
>>  {
>> -	int pos, ttl = 48;
>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>  
>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>  	while (pos && ttl--) {
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 96453f9..b27b79e 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -33,6 +33,8 @@
>>  
>>  #include <linux/pci_ids.h>
>>  
>> +#define PCI_FIND_CAP_TTL	48
>> +
>>  /*
>>   * The PCI interface treats multi-function devices as independent
>>   * devices.  The slot/function address of each device is encoded
>> -- 
>> 1.7.9.5
>> 

-- 
Richard Yang
Help you, Help me


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
  2014-11-06  3:12     ` Wei Yang
@ 2014-11-18  9:16       ` Wei Yang
  0 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2014-11-18  9:16 UTC (permalink / raw)
  To: Wei Yang; +Cc: Bjorn Helgaas, Yinghai Lu, linux-pci

On Thu, Nov 06, 2014 at 11:12:02AM +0800, Wei Yang wrote:
>On Wed, Oct 22, 2014 at 05:18:30PM -0600, Bjorn Helgaas wrote:
>>[+cc Yinghai]
>>
>>On Tue, Oct 14, 2014 at 02:47:30PM +0800, Wei Yang wrote:
>>> In some quirks, it tries to search a pci cap and use a ttl value to avoid
>>> infinite loop. While the value is hard coded to 48, which is the same as marco
>>> PCI_FIND_CAP_TTL.
>>> 
>>> This patch moves the definition of PCI_FIND_CAP_TTL to pci.h and replace the
>>> hard coded value with it.
>>
>>This seems reasonable (though I added Yinghai in case he knows of any
>>reason why HT capabilities should be different from plain PCI capabilities
>>in this respect).
>
>Hi, Yinghai,
>
>Do you have some historical story on the definition?
>

Hi,

I am not sure if I missed some mail. This is the last mail I sent, but no
reply I got yet. Is is ok to send an updated version?

>>
>>But I'd prefer to have the definition in drivers/pci/pci.h rather than in
>>include/linux/pci.h, because both users already include drivers/pci/pci.h,
>>and it is less visible.
>>
>>Bjorn
>>
>>> 
>>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>> ---
>>>  drivers/pci/pci.c    |    1 -
>>>  drivers/pci/quirks.c |    8 ++++----
>>>  include/linux/pci.h  |    2 ++
>>>  3 files changed, 6 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>> index 2c9ac70..76b002b1 100644
>>> --- a/drivers/pci/pci.c
>>> +++ b/drivers/pci/pci.c
>>> @@ -137,7 +137,6 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
>>>  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
>>>  #endif
>>>  
>>> -#define PCI_FIND_CAP_TTL	48
>>>  
>>>  static int __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>>>  				   u8 pos, int cap, int *ttl)
>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>> index 80c2d01..a5f46b8 100644
>>> --- a/drivers/pci/quirks.c
>>> +++ b/drivers/pci/quirks.c
>>> @@ -2176,7 +2176,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x9601, quirk_amd_780_apc_msi);
>>>   * return 1 if a HT MSI capability is found and enabled */
>>>  static int msi_ht_cap_enabled(struct pci_dev *dev)
>>>  {
>>> -	int pos, ttl = 48;
>>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>>  
>>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>>  	while (pos && ttl--) {
>>> @@ -2235,7 +2235,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_CK804_PCIE,
>>>  /* Force enable MSI mapping capability on HT bridges */
>>>  static void ht_enable_msi_mapping(struct pci_dev *dev)
>>>  {
>>> -	int pos, ttl = 48;
>>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>>  
>>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>>  	while (pos && ttl--) {
>>> @@ -2314,7 +2314,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
>>>  
>>>  static int ht_check_msi_mapping(struct pci_dev *dev)
>>>  {
>>> -	int pos, ttl = 48;
>>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>>  	int found = 0;
>>>  
>>>  	/* check if there is HT MSI cap or enabled on this device */
>>> @@ -2439,7 +2439,7 @@ out:
>>>  
>>>  static void ht_disable_msi_mapping(struct pci_dev *dev)
>>>  {
>>> -	int pos, ttl = 48;
>>> +	int pos, ttl = PCI_FIND_CAP_TTL;
>>>  
>>>  	pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
>>>  	while (pos && ttl--) {
>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>> index 96453f9..b27b79e 100644
>>> --- a/include/linux/pci.h
>>> +++ b/include/linux/pci.h
>>> @@ -33,6 +33,8 @@
>>>  
>>>  #include <linux/pci_ids.h>
>>>  
>>> +#define PCI_FIND_CAP_TTL	48
>>> +
>>>  /*
>>>   * The PCI interface treats multi-function devices as independent
>>>   * devices.  The slot/function address of each device is encoded
>>> -- 
>>> 1.7.9.5
>>> 
>
>-- 
>Richard Yang
>Help you, Help me

-- 
Richard Yang
Help you, Help me


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-11-18  9:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-14  6:47 [PATCH 0/3] PCI: code clean up on pci configuration space Wei Yang
2014-10-14  6:47 ` [PATCH 1/3] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
2014-10-22 23:18   ` Bjorn Helgaas
2014-10-24  3:44     ` Wei Yang
2014-11-06  3:12     ` Wei Yang
2014-11-18  9:16       ` Wei Yang
2014-10-14  6:47 ` [PATCH 2/3] PCI: use u8 instead of int for pci configuration space pos and cap Wei Yang
2014-10-22 23:23   ` Bjorn Helgaas
2014-10-24  3:49     ` Wei Yang
2014-10-14  6:47 ` [PATCH 3/3] PCI: use u16 instead of int for pci express extended capabilities " Wei Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).