* [PATCH V2 0/4] PCI: code clean up on pci configuration space
@ 2015-06-30 1:16 Wei Yang
2015-06-30 1:16 ` [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Wei Yang @ 2015-06-30 1:16 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 three are to use the exact type to access the pci cap and pcie ext
cap.
Tested on x86 and powerpc on top of v4.1.
The original thread could be referenced in below link:
http://comments.gmane.org/gmane.linux.kernel.pci/35931
---
v1->v2:
* define PCI_FIND_CAP_TTL in drivers/pci/pci.h instead of
include/linux/pci.h
* split the change for return position check from second one, and make the
forth patch
Wei Yang (4):
PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
PCI: use u8 to represent pci configuration space pos and cap
PCI: use u16 to represent pci express extended capabilities pos and
cap
PCI: consolidate return value check for pci_find_(ext_)capability
drivers/pci/iov.c | 2 +-
drivers/pci/pci.c | 68 +++++++++++++++++++++++++-------------------------
drivers/pci/pci.h | 2 ++
drivers/pci/probe.c | 10 ++++----
drivers/pci/quirks.c | 19 +++++++-------
include/linux/pci.h | 26 +++++++++----------
6 files changed, 65 insertions(+), 62 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
2015-06-30 1:16 [PATCH V2 0/4] PCI: code clean up on pci configuration space Wei Yang
@ 2015-06-30 1:16 ` Wei Yang
2015-07-14 21:57 ` Bjorn Helgaas
2015-06-30 1:16 ` [PATCH V2 2/4] PCI: use u8 to represent pci configuration space pos and cap Wei Yang
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Wei Yang @ 2015-06-30 1:16 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
macro 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>
---
v1 -> v2:
define PCI_FIND_CAP_TTL in drivers/pci/pci.h instead of include/linux/pci.h
---
drivers/pci/pci.c | 1 -
drivers/pci/pci.h | 2 ++
drivers/pci/quirks.c | 8 ++++----
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index acc4b6e..24ebb1b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -140,7 +140,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/pci.h b/drivers/pci/pci.h
index 9bd762c2..1b0d20e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -4,6 +4,8 @@
#define PCI_CFG_SPACE_SIZE 256
#define PCI_CFG_SPACE_EXP_SIZE 4096
+#define PCI_FIND_CAP_TTL 48
+
extern const unsigned char pcie_link_speed[];
bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index c6dc1df..ad3b62c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2250,7 +2250,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--) {
@@ -2309,7 +2309,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--) {
@@ -2388,7 +2388,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 */
@@ -2513,7 +2513,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--) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2 2/4] PCI: use u8 to represent pci configuration space pos and cap
2015-06-30 1:16 [PATCH V2 0/4] PCI: code clean up on pci configuration space Wei Yang
2015-06-30 1:16 ` [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
@ 2015-06-30 1:16 ` Wei Yang
2015-06-30 1:16 ` [PATCH V2 3/4] PCI: use u16 to represent pci express extended capabilities " Wei Yang
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2015-06-30 1:16 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 represent the Cap's position and ID.
This patch does some clean up for the Cap position and ID by replacing the
int/char type with u8.
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
drivers/pci/pci.c | 49 +++++++++++++++++++++++++------------------------
drivers/pci/probe.c | 8 ++++----
drivers/pci/quirks.c | 19 ++++++++++---------
include/linux/pci.h | 20 ++++++++++----------
4 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 24ebb1b..3f7770a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -141,8 +141,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;
u16 ent;
@@ -165,22 +165,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;
@@ -221,9 +221,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)
@@ -246,9 +246,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);
@@ -333,7 +333,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;
@@ -373,7 +373,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);
}
@@ -390,9 +390,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)
@@ -907,7 +907,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);
}
@@ -967,7 +967,7 @@ 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);
@@ -988,7 +988,8 @@ 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;
@@ -2068,7 +2069,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);
@@ -2173,7 +2174,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);
}
@@ -3110,7 +3111,7 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
*/
void pci_msi_off(struct pci_dev *dev)
{
- int pos;
+ u8 pos;
u16 control;
/*
@@ -3182,7 +3183,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);
@@ -3978,7 +3979,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);
@@ -4001,7 +4002,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);
@@ -4026,7 +4027,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 6675a7a..ea253fa 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)
@@ -971,7 +971,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);
@@ -1059,7 +1059,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;
@@ -1100,7 +1100,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 ad3b62c..fd8a528 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2250,7 +2250,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--) {
@@ -2309,7 +2309,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--) {
@@ -2388,7 +2388,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 */
@@ -2417,7 +2417,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;
@@ -2450,7 +2450,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;
@@ -2475,7 +2476,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;
@@ -2513,7 +2514,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--) {
@@ -2534,7 +2535,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())
@@ -3337,7 +3338,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 353db8d..b36de1f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -825,12 +825,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,
@@ -1023,10 +1023,10 @@ int pci_load_saved_state(struct pci_dev *dev,
struct pci_saved_state *state);
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);
@@ -1064,7 +1064,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);
@@ -1404,10 +1404,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] 12+ messages in thread
* [PATCH V2 3/4] PCI: use u16 to represent pci express extended capabilities pos and cap
2015-06-30 1:16 [PATCH V2 0/4] PCI: code clean up on pci configuration space Wei Yang
2015-06-30 1:16 ` [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
2015-06-30 1:16 ` [PATCH V2 2/4] PCI: use u8 to represent pci configuration space pos and cap Wei Yang
@ 2015-06-30 1:16 ` Wei Yang
2015-06-30 1:16 ` [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability Wei Yang
2015-07-14 21:37 ` [PATCH V2 0/4] PCI: code clean up on pci configuration space Bjorn Helgaas
4 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2015-06-30 1:16 UTC (permalink / raw)
To: linux-pci, bhelgaas; +Cc: Wei Yang
For pci express devices, it could have extended capabilities. The position
of extended 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 ee0ebff..db85fbe 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -529,7 +529,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 3f7770a..4bd3a0c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -272,11 +272,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;
@@ -327,7 +327,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);
}
@@ -2151,7 +2151,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)
@@ -2265,7 +2265,7 @@ void pci_request_acs(void)
*/
static int pci_std_enable_acs(struct pci_dev *dev)
{
- int pos;
+ u16 pos;
u16 cap;
u16 ctrl;
@@ -2310,7 +2310,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 ea253fa..092cf93 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1577,7 +1577,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 b36de1f..223f253 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -827,8 +827,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);
@@ -1409,7 +1409,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] 12+ messages in thread
* [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability
2015-06-30 1:16 [PATCH V2 0/4] PCI: code clean up on pci configuration space Wei Yang
` (2 preceding siblings ...)
2015-06-30 1:16 ` [PATCH V2 3/4] PCI: use u16 to represent pci express extended capabilities " Wei Yang
@ 2015-06-30 1:16 ` Wei Yang
2015-07-14 22:00 ` Bjorn Helgaas
2015-07-14 21:37 ` [PATCH V2 0/4] PCI: code clean up on pci configuration space Bjorn Helgaas
4 siblings, 1 reply; 12+ messages in thread
From: Wei Yang @ 2015-06-30 1:16 UTC (permalink / raw)
To: linux-pci, bhelgaas; +Cc: Wei Yang
The return value of the pci_find_(ext_)capability is the position of this
Cap. After previous two patches clean up, the position returned is an
unsigned value. Only 0 indicates the Cap is not presented.
This patch consolidates the form of check from (pos <= 0)to (!pos).
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
drivers/pci/pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4bd3a0c..e1282aa 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -971,7 +971,7 @@ static int pci_save_pcix_state(struct pci_dev *dev)
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);
@@ -995,7 +995,7 @@ static void pci_restore_pcix_state(struct pci_dev *dev)
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];
@@ -2159,7 +2159,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);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/4] PCI: code clean up on pci configuration space
2015-06-30 1:16 [PATCH V2 0/4] PCI: code clean up on pci configuration space Wei Yang
` (3 preceding siblings ...)
2015-06-30 1:16 ` [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability Wei Yang
@ 2015-07-14 21:37 ` Bjorn Helgaas
2015-07-15 2:08 ` Wei Yang
4 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2015-07-14 21:37 UTC (permalink / raw)
To: Wei Yang; +Cc: linux-pci
On Tue, Jun 30, 2015 at 09:16:40AM +0800, Wei Yang wrote:
> 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 three are to use the exact type to access the pci cap and pcie ext
> cap.
>
> Tested on x86 and powerpc on top of v4.1.
>
> The original thread could be referenced in below link:
> http://comments.gmane.org/gmane.linux.kernel.pci/35931
>
> ---
> v1->v2:
> * define PCI_FIND_CAP_TTL in drivers/pci/pci.h instead of
> include/linux/pci.h
> * split the change for return position check from second one, and make the
> forth patch
>
> Wei Yang (4):
> PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
> PCI: use u8 to represent pci configuration space pos and cap
> PCI: use u16 to represent pci express extended capabilities pos and
> cap
> PCI: consolidate return value check for pci_find_(ext_)capability
The first and last seem fine to me. As far as I can tell, the last does
not actually depend on the u8 and u16 changes.
The u8 and u16 patches change the signatures of several exported functions.
Do they fix some problem? Unless there's something broken, these seem like
pretty minor changes, and I don't think they're worth it.
Bjorn
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
2015-06-30 1:16 ` [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
@ 2015-07-14 21:57 ` Bjorn Helgaas
0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2015-07-14 21:57 UTC (permalink / raw)
To: Wei Yang; +Cc: linux-pci
On Tue, Jun 30, 2015 at 09:16:41AM +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
> macro 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>
Applied to pci/misc for v4.3 with changelog as follows. Original patch
touched some code that was removed in v4.2, so I adjusted for that, too.
commit 875803b6ef5becdcc2d43b014531279dfaa978af
Author: Wei Yang <weiyang@linux.vnet.ibm.com>
Date: Tue Jun 30 09:16:41 2015 +0800
PCI: Move PCI_FIND_CAP_TTL to pci.h and use it in quirks
Some quirks search for a HyperTransport capability and use a hard-coded TTL
value of 48 to avoid an infinite loop.
Move the definition of PCI_FIND_CAP_TTL to pci.h and use it instead of the
hard-coded TTL values.
[bhelgaas: changelog]
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability
2015-06-30 1:16 ` [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability Wei Yang
@ 2015-07-14 22:00 ` Bjorn Helgaas
2015-07-15 2:02 ` Wei Yang
0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2015-07-14 22:00 UTC (permalink / raw)
To: Wei Yang; +Cc: linux-pci
On Tue, Jun 30, 2015 at 09:16:44AM +0800, Wei Yang wrote:
> The return value of the pci_find_(ext_)capability is the position of this
> Cap. After previous two patches clean up, the position returned is an
> unsigned value. Only 0 indicates the Cap is not presented.
>
> This patch consolidates the form of check from (pos <= 0)to (!pos).
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Applied to pci/misc with changelog as below.
It seems pretty clear to me that pci_find_capability() returns either 0 or
a u8 value. pci_find_ext_capability() does return an int. It looks to me
like it can never be negative, but if you wanted it to be even more clear,
you could easily change just pci_find_next_ext_capability() to use a u16
for "pos". That would be very simple and wouldn't change any interfaces.
commit d5fa86074987b1b5fcbfba8c9315e75ff7262f71
Author: Wei Yang <weiyang@linux.vnet.ibm.com>
Date: Tue Jun 30 09:16:44 2015 +0800
PCI: Simplify pci_find_(ext_)capability() return value checks
The return value of the pci_find_(ext_)capability() is either zero or the
position of a capability. It is never negative.
This patch consolidates the form of check from (pos <= 0) to (!pos).
Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability
2015-07-14 22:00 ` Bjorn Helgaas
@ 2015-07-15 2:02 ` Wei Yang
2015-07-15 2:11 ` Bjorn Helgaas
0 siblings, 1 reply; 12+ messages in thread
From: Wei Yang @ 2015-07-15 2:02 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Wei Yang, linux-pci
On Tue, Jul 14, 2015 at 05:00:21PM -0500, Bjorn Helgaas wrote:
>On Tue, Jun 30, 2015 at 09:16:44AM +0800, Wei Yang wrote:
>> The return value of the pci_find_(ext_)capability is the position of this
>> Cap. After previous two patches clean up, the position returned is an
>> unsigned value. Only 0 indicates the Cap is not presented.
>>
>> This patch consolidates the form of check from (pos <= 0)to (!pos).
>>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>
>Applied to pci/misc with changelog as below.
>
>It seems pretty clear to me that pci_find_capability() returns either 0 or
>a u8 value. pci_find_ext_capability() does return an int. It looks to me
>like it can never be negative, but if you wanted it to be even more clear,
>you could easily change just pci_find_next_ext_capability() to use a u16
>for "pos". That would be very simple and wouldn't change any interfaces.
pci_find_capability() will return either 0 or a u8 value, while in the code
the return value is an "int" type. So for the first sight, it may not that
immediate. The same as pci_find_ext_capability().
This is the reason for patch 2/3. The purpose is to make the return type
reflect the value it will return.
Patch 3 does exactly what you said, use a u16 for "pos" in
pci_find_next_ext_capability().
>
>commit d5fa86074987b1b5fcbfba8c9315e75ff7262f71
>Author: Wei Yang <weiyang@linux.vnet.ibm.com>
>Date: Tue Jun 30 09:16:44 2015 +0800
>
> PCI: Simplify pci_find_(ext_)capability() return value checks
>
> The return value of the pci_find_(ext_)capability() is either zero or the
> position of a capability. It is never negative.
>
> This patch consolidates the form of check from (pos <= 0) to (!pos).
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
--
Richard Yang
Help you, Help me
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/4] PCI: code clean up on pci configuration space
2015-07-14 21:37 ` [PATCH V2 0/4] PCI: code clean up on pci configuration space Bjorn Helgaas
@ 2015-07-15 2:08 ` Wei Yang
0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2015-07-15 2:08 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Wei Yang, linux-pci
On Tue, Jul 14, 2015 at 04:37:38PM -0500, Bjorn Helgaas wrote:
>On Tue, Jun 30, 2015 at 09:16:40AM +0800, Wei Yang wrote:
>> 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 three are to use the exact type to access the pci cap and pcie ext
>> cap.
>>
>> Tested on x86 and powerpc on top of v4.1.
>>
>> The original thread could be referenced in below link:
>> http://comments.gmane.org/gmane.linux.kernel.pci/35931
>>
>> ---
>> v1->v2:
>> * define PCI_FIND_CAP_TTL in drivers/pci/pci.h instead of
>> include/linux/pci.h
>> * split the change for return position check from second one, and make the
>> forth patch
>>
>> Wei Yang (4):
>> PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks
>> PCI: use u8 to represent pci configuration space pos and cap
>> PCI: use u16 to represent pci express extended capabilities pos and
>> cap
>> PCI: consolidate return value check for pci_find_(ext_)capability
>
>The first and last seem fine to me. As far as I can tell, the last does
>not actually depend on the u8 and u16 changes.
>
With patch 2/3, changing the return value, it makes sure pos will not be
negative. So that patch 4 could make sure there is not cap when pos is 0. And
no possibility to check the negative value.
>The u8 and u16 patches change the signatures of several exported functions.
>Do they fix some problem? Unless there's something broken, these seem like
>pretty minor changes, and I don't think they're worth it.
>
You are right, no functional change and currently no bug related.
While without 2/3, it would not be that clear the pos return value must not
be negative. Then then the change in patch 4 would not be that obvious.
>Bjorn
--
Richard Yang
Help you, Help me
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability
2015-07-15 2:02 ` Wei Yang
@ 2015-07-15 2:11 ` Bjorn Helgaas
2015-07-15 5:46 ` Wei Yang
0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2015-07-15 2:11 UTC (permalink / raw)
To: Wei Yang; +Cc: linux-pci@vger.kernel.org
On Tue, Jul 14, 2015 at 9:02 PM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> On Tue, Jul 14, 2015 at 05:00:21PM -0500, Bjorn Helgaas wrote:
>>On Tue, Jun 30, 2015 at 09:16:44AM +0800, Wei Yang wrote:
>>> The return value of the pci_find_(ext_)capability is the position of this
>>> Cap. After previous two patches clean up, the position returned is an
>>> unsigned value. Only 0 indicates the Cap is not presented.
>>>
>>> This patch consolidates the form of check from (pos <= 0)to (!pos).
>>>
>>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>
>>Applied to pci/misc with changelog as below.
>>
>>It seems pretty clear to me that pci_find_capability() returns either 0 or
>>a u8 value. pci_find_ext_capability() does return an int. It looks to me
>>like it can never be negative, but if you wanted it to be even more clear,
>>you could easily change just pci_find_next_ext_capability() to use a u16
>>for "pos". That would be very simple and wouldn't change any interfaces.
>
> pci_find_capability() will return either 0 or a u8 value, while in the code
> the return value is an "int" type. So for the first sight, it may not that
> immediate. The same as pci_find_ext_capability().
>
> This is the reason for patch 2/3. The purpose is to make the return type
> reflect the value it will return.
>
> Patch 3 does exactly what you said, use a u16 for "pos" in
> pci_find_next_ext_capability().
Yes. Patch 3 contains this hunk:
@@ -272,11 +272,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;
I think the second piece (changing "pos" from int to u16) is enough to
make it easy to analyze, and it only affects this function, so that
seems obviously fine.
The first piece (changing the return type) changes the signature of an
exported interface. That is more work, e.g., it's more of a hassle
for distros to backport if they want to preserve interfaces, so it's
not quite as obvious that it's worthwhile.
Bjorn
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability
2015-07-15 2:11 ` Bjorn Helgaas
@ 2015-07-15 5:46 ` Wei Yang
0 siblings, 0 replies; 12+ messages in thread
From: Wei Yang @ 2015-07-15 5:46 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Wei Yang, linux-pci@vger.kernel.org
On Tue, Jul 14, 2015 at 09:11:54PM -0500, Bjorn Helgaas wrote:
>On Tue, Jul 14, 2015 at 9:02 PM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>> On Tue, Jul 14, 2015 at 05:00:21PM -0500, Bjorn Helgaas wrote:
>>>On Tue, Jun 30, 2015 at 09:16:44AM +0800, Wei Yang wrote:
>>>> The return value of the pci_find_(ext_)capability is the position of this
>>>> Cap. After previous two patches clean up, the position returned is an
>>>> unsigned value. Only 0 indicates the Cap is not presented.
>>>>
>>>> This patch consolidates the form of check from (pos <= 0)to (!pos).
>>>>
>>>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>>
>>>Applied to pci/misc with changelog as below.
>>>
>>>It seems pretty clear to me that pci_find_capability() returns either 0 or
>>>a u8 value. pci_find_ext_capability() does return an int. It looks to me
>>>like it can never be negative, but if you wanted it to be even more clear,
>>>you could easily change just pci_find_next_ext_capability() to use a u16
>>>for "pos". That would be very simple and wouldn't change any interfaces.
>>
>> pci_find_capability() will return either 0 or a u8 value, while in the code
>> the return value is an "int" type. So for the first sight, it may not that
>> immediate. The same as pci_find_ext_capability().
>>
>> This is the reason for patch 2/3. The purpose is to make the return type
>> reflect the value it will return.
>>
>> Patch 3 does exactly what you said, use a u16 for "pos" in
>> pci_find_next_ext_capability().
>
>Yes. Patch 3 contains this hunk:
>
>@@ -272,11 +272,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;
>
>I think the second piece (changing "pos" from int to u16) is enough to
>make it easy to analyze, and it only affects this function, so that
>seems obviously fine.
>
>The first piece (changing the return type) changes the signature of an
>exported interface. That is more work, e.g., it's more of a hassle
>for distros to backport if they want to preserve interfaces, so it's
>not quite as obvious that it's worthwhile.
OK, I got your concern.
>
>Bjorn
>--
>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] 12+ messages in thread
end of thread, other threads:[~2015-07-15 5:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-30 1:16 [PATCH V2 0/4] PCI: code clean up on pci configuration space Wei Yang
2015-06-30 1:16 ` [PATCH V2 1/4] PCI: move PCI_FIND_CAP_TTL to pci.h and use it in quirks Wei Yang
2015-07-14 21:57 ` Bjorn Helgaas
2015-06-30 1:16 ` [PATCH V2 2/4] PCI: use u8 to represent pci configuration space pos and cap Wei Yang
2015-06-30 1:16 ` [PATCH V2 3/4] PCI: use u16 to represent pci express extended capabilities " Wei Yang
2015-06-30 1:16 ` [PATCH V2 4/4] PCI: consolidate return value check for pci_find_(ext_)capability Wei Yang
2015-07-14 22:00 ` Bjorn Helgaas
2015-07-15 2:02 ` Wei Yang
2015-07-15 2:11 ` Bjorn Helgaas
2015-07-15 5:46 ` Wei Yang
2015-07-14 21:37 ` [PATCH V2 0/4] PCI: code clean up on pci configuration space Bjorn Helgaas
2015-07-15 2:08 ` Wei Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.