1. Patchew
  2. linux
  3. [PATCH v10 0/6] Refactor capability search into common macros
  4. View patch

[PATCH v10 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability

Hans Zhang posted 6 patches 9 months, 2 weeks ago
There is a newer version of this series
[PATCH v10 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability
Posted by Hans Zhang 9 months, 2 weeks ago 
Refactor the __pci_find_next_cap_ttl() to improve code clarity:
- Replace magic number 0x40 with PCI_STD_HEADER_SIZEOF.
- Use ALIGN_DOWN() for position alignment instead of manual bitmask.
- Extract PCI capability fields via FIELD_GET() with standardized masks.
- Add necessary headers (linux/align.h, uapi/linux/pci_regs.h).

The changes are purely non-functional cleanups, ensuring behavior remains
identical to the original implementation.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
Changes since v9:
- None

Changes since v8:
- Split into patch 1/6, patch 2/6.
- The
 drivers/pci/pci.c             | 10 ++++++----
 include/uapi/linux/pci_regs.h |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4d7c9f64ea24..1c29e8f20cb5 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/align.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
 #include <linux/dmi.h>
@@ -30,6 +31,7 @@
 #include <asm/dma.h>
 #include <linux/aer.h>
 #include <linux/bitfield.h>
+#include <uapi/linux/pci_regs.h>
 #include "pci.h"
 
 DEFINE_MUTEX(pci_slot_mutex);
@@ -432,17 +434,17 @@ static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
 	pci_bus_read_config_byte(bus, devfn, pos, &pos);
 
 	while ((*ttl)--) {
-		if (pos < 0x40)
+		if (pos < PCI_STD_HEADER_SIZEOF)
 			break;
-		pos &= ~3;
+		pos = ALIGN_DOWN(pos, 4);
 		pci_bus_read_config_word(bus, devfn, pos, &ent);
 
-		id = ent & 0xff;
+		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
 		if (id == 0xff)
 			break;
 		if (id == cap)
 			return pos;
-		pos = (ent >> 8);
+		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
 	}
 	return 0;
 }
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index ba326710f9c8..b59179e1210a 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -206,6 +206,8 @@
 /* 0x48-0x7f reserved */
 
 /* Capability lists */
+#define PCI_CAP_ID_MASK		0x00ff
+#define PCI_CAP_LIST_NEXT_MASK	0xff00
 
 #define PCI_CAP_LIST_ID		0	/* Capability ID */
 #define  PCI_CAP_ID_PM		0x01	/* Power Management */
-- 
2.25.1
Re: [PATCH v10 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability
Posted by Ilpo Järvinen 9 months, 2 weeks ago 
On Tue, 29 Apr 2025, Hans Zhang wrote:

> Refactor the __pci_find_next_cap_ttl() to improve code clarity:
> - Replace magic number 0x40 with PCI_STD_HEADER_SIZEOF.
> - Use ALIGN_DOWN() for position alignment instead of manual bitmask.
> - Extract PCI capability fields via FIELD_GET() with standardized masks.
> - Add necessary headers (linux/align.h, uapi/linux/pci_regs.h).
> 
> The changes are purely non-functional cleanups, ensuring behavior remains
> identical to the original implementation.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> Changes since v9:
> - None
> 
> Changes since v8:
> - Split into patch 1/6, patch 2/6.
> - The
>  drivers/pci/pci.c             | 10 ++++++----
>  include/uapi/linux/pci_regs.h |  2 ++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 4d7c9f64ea24..1c29e8f20cb5 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/acpi.h>
> +#include <linux/align.h>
>  #include <linux/kernel.h>
>  #include <linux/delay.h>
>  #include <linux/dmi.h>
> @@ -30,6 +31,7 @@
>  #include <asm/dma.h>
>  #include <linux/aer.h>
>  #include <linux/bitfield.h>
> +#include <uapi/linux/pci_regs.h>

linux/pci.h will pull this in through <uapi/linux/pci.h> so you don't need 
to add it (basically anywhere).

>  #include "pci.h"
>  
>  DEFINE_MUTEX(pci_slot_mutex);
> @@ -432,17 +434,17 @@ static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>  	pci_bus_read_config_byte(bus, devfn, pos, &pos);
>  
>  	while ((*ttl)--) {
> -		if (pos < 0x40)
> +		if (pos < PCI_STD_HEADER_SIZEOF)
>  			break;
> -		pos &= ~3;
> +		pos = ALIGN_DOWN(pos, 4);
>  		pci_bus_read_config_word(bus, devfn, pos, &ent);
>  
> -		id = ent & 0xff;
> +		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
>  		if (id == 0xff)
>  			break;
>  		if (id == cap)
>  			return pos;
> -		pos = (ent >> 8);
> +		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
>  	}
>  	return 0;
>  }
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index ba326710f9c8..b59179e1210a 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -206,6 +206,8 @@
>  /* 0x48-0x7f reserved */
>  
>  /* Capability lists */
> +#define PCI_CAP_ID_MASK		0x00ff
> +#define PCI_CAP_LIST_NEXT_MASK	0xff00

Consider adding a comment after the value as is done for most  defines in 
this file.

>  
>  #define PCI_CAP_LIST_ID		0	/* Capability ID */
>  #define  PCI_CAP_ID_PM		0x01	/* Power Management */
> 

-- 
 i.
Re: [PATCH v10 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability
Posted by Hans Zhang 9 months, 2 weeks ago 

On 2025/4/29 23:17, Ilpo Järvinen wrote:
>>   #include <linux/acpi.h>
>> +#include <linux/align.h>
>>   #include <linux/kernel.h>
>>   #include <linux/delay.h>
>>   #include <linux/dmi.h>
>> @@ -30,6 +31,7 @@
>>   #include <asm/dma.h>
>>   #include <linux/aer.h>
>>   #include <linux/bitfield.h>
>> +#include <uapi/linux/pci_regs.h>
> 
> linux/pci.h will pull this in through <uapi/linux/pci.h> so you don't need
> to add it (basically anywhere).

Dear Ilpo,

Will delete.

Best regards,
Hans
Re: [PATCH v10 2/6] PCI: Clean up __pci_find_next_cap_ttl() readability
Posted by Hans Zhang 9 months, 2 weeks ago 

On 2025/4/29 23:17, Ilpo Järvinen wrote:
> On Tue, 29 Apr 2025, Hans Zhang wrote:
> 
>> Refactor the __pci_find_next_cap_ttl() to improve code clarity:
>> - Replace magic number 0x40 with PCI_STD_HEADER_SIZEOF.
>> - Use ALIGN_DOWN() for position alignment instead of manual bitmask.
>> - Extract PCI capability fields via FIELD_GET() with standardized masks.
>> - Add necessary headers (linux/align.h, uapi/linux/pci_regs.h).
>>
>> The changes are purely non-functional cleanups, ensuring behavior remains
>> identical to the original implementation.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> Changes since v9:
>> - None
>>
>> Changes since v8:
>> - Split into patch 1/6, patch 2/6.
>> - The
>>   drivers/pci/pci.c             | 10 ++++++----
>>   include/uapi/linux/pci_regs.h |  2 ++
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 4d7c9f64ea24..1c29e8f20cb5 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -9,6 +9,7 @@
>>    */
>>   
>>   #include <linux/acpi.h>
>> +#include <linux/align.h>
>>   #include <linux/kernel.h>
>>   #include <linux/delay.h>
>>   #include <linux/dmi.h>
>> @@ -30,6 +31,7 @@
>>   #include <asm/dma.h>
>>   #include <linux/aer.h>
>>   #include <linux/bitfield.h>
>> +#include <uapi/linux/pci_regs.h>
> 
> linux/pci.h will pull this in through <uapi/linux/pci.h> so you don't need
> to add it (basically anywhere).
> 
>>   #include "pci.h"
>>   
>>   DEFINE_MUTEX(pci_slot_mutex);
>> @@ -432,17 +434,17 @@ static u8 __pci_find_next_cap_ttl(struct pci_bus *bus, unsigned int devfn,
>>   	pci_bus_read_config_byte(bus, devfn, pos, &pos);
>>   
>>   	while ((*ttl)--) {
>> -		if (pos < 0x40)
>> +		if (pos < PCI_STD_HEADER_SIZEOF)
>>   			break;
>> -		pos &= ~3;
>> +		pos = ALIGN_DOWN(pos, 4);
>>   		pci_bus_read_config_word(bus, devfn, pos, &ent);
>>   
>> -		id = ent & 0xff;
>> +		id = FIELD_GET(PCI_CAP_ID_MASK, ent);
>>   		if (id == 0xff)
>>   			break;
>>   		if (id == cap)
>>   			return pos;
>> -		pos = (ent >> 8);
>> +		pos = FIELD_GET(PCI_CAP_LIST_NEXT_MASK, ent);
>>   	}
>>   	return 0;
>>   }
>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>> index ba326710f9c8..b59179e1210a 100644
>> --- a/include/uapi/linux/pci_regs.h
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -206,6 +206,8 @@
>>   /* 0x48-0x7f reserved */
>>   
>>   /* Capability lists */
>> +#define PCI_CAP_ID_MASK		0x00ff
>> +#define PCI_CAP_LIST_NEXT_MASK	0xff00
> 
> Consider adding a comment after the value as is done for most  defines in
> this file.

Dear Ilpo,

Thank you very much for your reply. Will add.

Best regards,
Hans

> 
>>   
>>   #define PCI_CAP_LIST_ID		0	/* Capability ID */
>>   #define  PCI_CAP_ID_PM		0x01	/* Power Management */
>>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.