diff mbox series

[03/10] PCI/ASPM: Use FIELD_GET()

Message ID 20231010204436.1000644-4-helgaas@kernel.org (mailing list archive)
State Accepted
Delegated to: Bjorn Helgaas
Headers show
Series PCI: Use FIELD_GET() and FIELD_PREP() | expand

Commit Message

Bjorn Helgaas Oct. 10, 2023, 8:44 p.m. UTC 
From: Bjorn Helgaas <bhelgaas@google.com>

Add #defines for T_POWER_ON in the L1 PM Substates Capability and use
FIELD_PREP() and FIELD_GET() when possible.  These remove the need for
explicit shifts.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/aspm.c       | 31 ++++++++++++++++++-------------
 include/uapi/linux/pci_regs.h |  2 ++
 2 files changed, 20 insertions(+), 13 deletions(-)

Comments

Bjorn Helgaas Oct. 10, 2023, 9:07 p.m. UTC | #1 
On Tue, Oct 10, 2023 at 03:44:29PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Add #defines for T_POWER_ON in the L1 PM Substates Capability and use
> FIELD_PREP() and FIELD_GET() when possible.  These remove the need for
> explicit shifts.  No functional change intended.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

I see this is identical to your patch at
https://lore.kernel.org/r/20230915155752.84640-3-ilpo.jarvinen@linux.intel.com,
Ilpo, so I'll drop this one.

> ---
>  drivers/pci/pcie/aspm.c       | 31 ++++++++++++++++++-------------
>  include/uapi/linux/pci_regs.h |  2 ++
>  2 files changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 1bf630059264..06f175d8dee5 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -7,6 +7,7 @@
>   * Copyright (C) Shaohua Li (shaohua.li@intel.com)
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/kernel.h>
>  #include <linux/math.h>
>  #include <linux/module.h>
> @@ -267,7 +268,7 @@ static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
>  /* Convert L0s latency encoding to ns */
>  static u32 calc_l0s_latency(u32 lnkcap)
>  {
> -	u32 encoding = (lnkcap & PCI_EXP_LNKCAP_L0SEL) >> 12;
> +	u32 encoding = FIELD_GET(PCI_EXP_LNKCAP_L0SEL, lnkcap);
>  
>  	if (encoding == 0x7)
>  		return (5 * 1000);	/* > 4us */
> @@ -285,7 +286,7 @@ static u32 calc_l0s_acceptable(u32 encoding)
>  /* Convert L1 latency encoding to ns */
>  static u32 calc_l1_latency(u32 lnkcap)
>  {
> -	u32 encoding = (lnkcap & PCI_EXP_LNKCAP_L1EL) >> 15;
> +	u32 encoding = FIELD_GET(PCI_EXP_LNKCAP_L1EL, lnkcap);
>  
>  	if (encoding == 0x7)
>  		return (65 * 1000);	/* > 64us */
> @@ -371,11 +372,11 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
>  	link = endpoint->bus->self->link_state;
>  
>  	/* Calculate endpoint L0s acceptable latency */
> -	encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L0S) >> 6;
> +	encoding = FIELD_GET(PCI_EXP_DEVCAP_L0S, endpoint->devcap);
>  	acceptable_l0s = calc_l0s_acceptable(encoding);
>  
>  	/* Calculate endpoint L1 acceptable latency */
> -	encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L1) >> 9;
> +	encoding = FIELD_GET(PCI_EXP_DEVCAP_L1, endpoint->devcap);
>  	acceptable_l1 = calc_l1_acceptable(encoding);
>  
>  	while (link) {
> @@ -446,22 +447,24 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
>  	u32 pl1_2_enables, cl1_2_enables;
>  
>  	/* Choose the greater of the two Port Common_Mode_Restore_Times */
> -	val1 = (parent_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
> -	val2 = (child_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
> +	val1 = FIELD_GET(PCI_L1SS_CAP_CM_RESTORE_TIME, parent_l1ss_cap);
> +	val2 = FIELD_GET(PCI_L1SS_CAP_CM_RESTORE_TIME, child_l1ss_cap);
>  	t_common_mode = max(val1, val2);
>  
>  	/* Choose the greater of the two Port T_POWER_ON times */
> -	val1   = (parent_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
> -	scale1 = (parent_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
> -	val2   = (child_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
> -	scale2 = (child_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
> +	val1   = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_VALUE, parent_l1ss_cap);
> +	scale1 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_SCALE, parent_l1ss_cap);
> +	val2   = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_VALUE, child_l1ss_cap);
> +	scale2 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_SCALE, child_l1ss_cap);
>  
>  	if (calc_l12_pwron(parent, scale1, val1) >
>  	    calc_l12_pwron(child, scale2, val2)) {
> -		ctl2 |= scale1 | (val1 << 3);
> +		ctl2 |= FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_SCALE, scale1) |
> +			FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_VALUE, val1);
>  		t_power_on = calc_l12_pwron(parent, scale1, val1);
>  	} else {
> -		ctl2 |= scale2 | (val2 << 3);
> +		ctl2 |= FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_SCALE, scale2) |
> +			FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_VALUE, val2);
>  		t_power_on = calc_l12_pwron(child, scale2, val2);
>  	}
>  
> @@ -477,7 +480,9 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
>  	 */
>  	l1_2_threshold = 2 + 4 + t_common_mode + t_power_on;
>  	encode_l12_threshold(l1_2_threshold, &scale, &value);
> -	ctl1 |= t_common_mode << 8 | scale << 29 | value << 16;
> +	ctl1 |= FIELD_PREP(PCI_L1SS_CTL1_CM_RESTORE_TIME, t_common_mode) |
> +		FIELD_PREP(PCI_L1SS_CTL1_LTR_L12_TH_VALUE, value) |
> +		FIELD_PREP(PCI_L1SS_CTL1_LTR_L12_TH_SCALE, scale);
>  
>  	/* Some broken devices only support dword access to L1 SS */
>  	pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, &pctl1);
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e5f558d96493..34bf037993f3 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -1088,6 +1088,8 @@
>  #define  PCI_L1SS_CTL1_LTR_L12_TH_VALUE	0x03ff0000  /* LTR_L1.2_THRESHOLD_Value */
>  #define  PCI_L1SS_CTL1_LTR_L12_TH_SCALE	0xe0000000  /* LTR_L1.2_THRESHOLD_Scale */
>  #define PCI_L1SS_CTL2		0x0c	/* Control 2 Register */
> +#define  PCI_L1SS_CTL2_T_PWR_ON_SCALE   0x00000003  /* T_POWER_ON Scale */
> +#define  PCI_L1SS_CTL2_T_PWR_ON_VALUE   0x000000f8  /* T_POWER_ON Value */
>  
>  /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
>  #define PCI_DVSEC_HEADER1		0x4 /* Designated Vendor-Specific Header1 */
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 1bf630059264..06f175d8dee5 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -7,6 +7,7 @@ 
  * Copyright (C) Shaohua Li (shaohua.li@intel.com)
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/math.h>
 #include <linux/module.h>
@@ -267,7 +268,7 @@  static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
 /* Convert L0s latency encoding to ns */
 static u32 calc_l0s_latency(u32 lnkcap)
 {
-	u32 encoding = (lnkcap & PCI_EXP_LNKCAP_L0SEL) >> 12;
+	u32 encoding = FIELD_GET(PCI_EXP_LNKCAP_L0SEL, lnkcap);
 
 	if (encoding == 0x7)
 		return (5 * 1000);	/* > 4us */
@@ -285,7 +286,7 @@  static u32 calc_l0s_acceptable(u32 encoding)
 /* Convert L1 latency encoding to ns */
 static u32 calc_l1_latency(u32 lnkcap)
 {
-	u32 encoding = (lnkcap & PCI_EXP_LNKCAP_L1EL) >> 15;
+	u32 encoding = FIELD_GET(PCI_EXP_LNKCAP_L1EL, lnkcap);
 
 	if (encoding == 0x7)
 		return (65 * 1000);	/* > 64us */
@@ -371,11 +372,11 @@  static void pcie_aspm_check_latency(struct pci_dev *endpoint)
 	link = endpoint->bus->self->link_state;
 
 	/* Calculate endpoint L0s acceptable latency */
-	encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L0S) >> 6;
+	encoding = FIELD_GET(PCI_EXP_DEVCAP_L0S, endpoint->devcap);
 	acceptable_l0s = calc_l0s_acceptable(encoding);
 
 	/* Calculate endpoint L1 acceptable latency */
-	encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L1) >> 9;
+	encoding = FIELD_GET(PCI_EXP_DEVCAP_L1, endpoint->devcap);
 	acceptable_l1 = calc_l1_acceptable(encoding);
 
 	while (link) {
@@ -446,22 +447,24 @@  static void aspm_calc_l12_info(struct pcie_link_state *link,
 	u32 pl1_2_enables, cl1_2_enables;
 
 	/* Choose the greater of the two Port Common_Mode_Restore_Times */
-	val1 = (parent_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
-	val2 = (child_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
+	val1 = FIELD_GET(PCI_L1SS_CAP_CM_RESTORE_TIME, parent_l1ss_cap);
+	val2 = FIELD_GET(PCI_L1SS_CAP_CM_RESTORE_TIME, child_l1ss_cap);
 	t_common_mode = max(val1, val2);
 
 	/* Choose the greater of the two Port T_POWER_ON times */
-	val1   = (parent_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
-	scale1 = (parent_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
-	val2   = (child_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
-	scale2 = (child_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
+	val1   = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_VALUE, parent_l1ss_cap);
+	scale1 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_SCALE, parent_l1ss_cap);
+	val2   = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_VALUE, child_l1ss_cap);
+	scale2 = FIELD_GET(PCI_L1SS_CAP_P_PWR_ON_SCALE, child_l1ss_cap);
 
 	if (calc_l12_pwron(parent, scale1, val1) >
 	    calc_l12_pwron(child, scale2, val2)) {
-		ctl2 |= scale1 | (val1 << 3);
+		ctl2 |= FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_SCALE, scale1) |
+			FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_VALUE, val1);
 		t_power_on = calc_l12_pwron(parent, scale1, val1);
 	} else {
-		ctl2 |= scale2 | (val2 << 3);
+		ctl2 |= FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_SCALE, scale2) |
+			FIELD_PREP(PCI_L1SS_CTL2_T_PWR_ON_VALUE, val2);
 		t_power_on = calc_l12_pwron(child, scale2, val2);
 	}
 
@@ -477,7 +480,9 @@  static void aspm_calc_l12_info(struct pcie_link_state *link,
 	 */
 	l1_2_threshold = 2 + 4 + t_common_mode + t_power_on;
 	encode_l12_threshold(l1_2_threshold, &scale, &value);
-	ctl1 |= t_common_mode << 8 | scale << 29 | value << 16;
+	ctl1 |= FIELD_PREP(PCI_L1SS_CTL1_CM_RESTORE_TIME, t_common_mode) |
+		FIELD_PREP(PCI_L1SS_CTL1_LTR_L12_TH_VALUE, value) |
+		FIELD_PREP(PCI_L1SS_CTL1_LTR_L12_TH_SCALE, scale);
 
 	/* Some broken devices only support dword access to L1 SS */
 	pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, &pctl1);
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e5f558d96493..34bf037993f3 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1088,6 +1088,8 @@ 
 #define  PCI_L1SS_CTL1_LTR_L12_TH_VALUE	0x03ff0000  /* LTR_L1.2_THRESHOLD_Value */
 #define  PCI_L1SS_CTL1_LTR_L12_TH_SCALE	0xe0000000  /* LTR_L1.2_THRESHOLD_Scale */
 #define PCI_L1SS_CTL2		0x0c	/* Control 2 Register */
+#define  PCI_L1SS_CTL2_T_PWR_ON_SCALE   0x00000003  /* T_POWER_ON Scale */
+#define  PCI_L1SS_CTL2_T_PWR_ON_VALUE   0x000000f8  /* T_POWER_ON Value */
 
 /* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
 #define PCI_DVSEC_HEADER1		0x4 /* Designated Vendor-Specific Header1 */