All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] PCI LNKCTL2 RMW Accessor Cleanup
@ 2024-02-15 13:31 Ilpo Järvinen
  2024-02-15 13:31 ` [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2 Ilpo Järvinen
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2024-02-15 13:31 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Jason Gunthorpe, Leon Romanovsky,
	linux-kernel, linux-rdma, Pan, Xinhui, Christian König
  Cc: Ilpo Järvinen

This series converts open-coded LNKCTL2 RMW accesses to use RMW
accessor. These patches used to be part of PCIe BW controller series
[1] which will require RMW ops to LNKCTL2 be properly locked.

However, since these RMW accessor patches are useful as cleanups on
their own I chose to send them now separately to reduce the size of the
BW controller series.

[1] https://lore.kernel.org/linux-pci/20240105112547.7301-1-ilpo.jarvinen@linux.intel.com/

Ilpo Järvinen (3):
  drm/radeon: Use RMW accessors for changing LNKCTL2
  drm/amdgpu: Use RMW accessors for changing LNKCTL2
  RDMA/hfi1: Use RMW accessors for changing LNKCTL2

 drivers/gpu/drm/amd/amdgpu/cik.c  | 41 +++++++++++--------------------
 drivers/gpu/drm/amd/amdgpu/si.c   | 41 +++++++++++--------------------
 drivers/gpu/drm/radeon/cik.c      | 40 +++++++++++-------------------
 drivers/gpu/drm/radeon/si.c       | 40 +++++++++++-------------------
 drivers/infiniband/hw/hfi1/pcie.c | 30 ++++++----------------
 5 files changed, 68 insertions(+), 124 deletions(-)

-- 
2.39.2


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

* [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
  2024-02-15 13:31 [PATCH 0/3] PCI LNKCTL2 RMW Accessor Cleanup Ilpo Järvinen
@ 2024-02-15 13:31 ` Ilpo Järvinen
  2024-02-15 17:32   ` Deucher, Alexander
  2024-02-15 13:31 ` [PATCH 2/3] drm/amdgpu: " Ilpo Järvinen
  2024-02-15 13:31 ` [PATCH 3/3] RDMA/hfi1: " Ilpo Järvinen
  2 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2024-02-15 13:31 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Jason Gunthorpe, Leon Romanovsky,
	linux-kernel, linux-rdma, Pan, Xinhui, Christian König
  Cc: Ilpo Järvinen, Lukas Wunner

Convert open coded RMW accesses for LNKCTL2 to use
pcie_capability_clear_and_set_word() which makes its easier to
understand what the code tries to do.

LNKCTL2 is not really owned by any driver because it is a collection of
control bits that PCI core might need to touch. RMW accessors already
have support for proper locking for a selected set of registers
(LNKCTL2 is not yet among them but likely will be in the future) to
avoid losing concurrent updates.

Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/gpu/drm/radeon/cik.c | 40 ++++++++++++++----------------------
 drivers/gpu/drm/radeon/si.c  | 40 ++++++++++++++----------------------
 2 files changed, 30 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 10be30366c2b..b5e96a8fc2c1 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -9592,28 +9592,18 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev)
 								   PCI_EXP_LNKCTL_HAWD);
 
 				/* linkctl2 */
-				pcie_capability_read_word(root, PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (bridge_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(root,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
-
-				pcie_capability_read_word(rdev->pdev,
-							  PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (gpu_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(rdev->pdev,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
+				pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   bridge_cfg2 |
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
+				pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   gpu_cfg2 |
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
 
 				tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
 				tmp &= ~LC_SET_QUIESCE;
@@ -9627,15 +9617,15 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev)
 	speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE;
 	WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);
 
-	pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL2, &tmp16);
-	tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
+	tmp16 = 0;
 	if (speed_cap == PCIE_SPEED_8_0GT)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
 	else if (speed_cap == PCIE_SPEED_5_0GT)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
 	else
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
-	pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL2, tmp16);
+	pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
+					   PCI_EXP_LNKCTL2_TLS, tmp16);
 
 	speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
 	speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE;
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 85e9cba49cec..8eeaea64c75d 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -7193,28 +7193,18 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev)
 								   PCI_EXP_LNKCTL_HAWD);
 
 				/* linkctl2 */
-				pcie_capability_read_word(root, PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (bridge_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(root,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
-
-				pcie_capability_read_word(rdev->pdev,
-							  PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (gpu_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(rdev->pdev,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
+				pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   bridge_cfg2 &
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
+				pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   gpu_cfg2 &
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
 
 				tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
 				tmp &= ~LC_SET_QUIESCE;
@@ -7228,15 +7218,15 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev)
 	speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE;
 	WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);
 
-	pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL2, &tmp16);
-	tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
+	tmp16 = 0;
 	if (speed_cap == PCIE_SPEED_8_0GT)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
 	else if (speed_cap == PCIE_SPEED_5_0GT)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
 	else
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
-	pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL2, tmp16);
+	pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
+					   PCI_EXP_LNKCTL2_TLS, tmp16);
 
 	speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
 	speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE;
-- 
2.39.2


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

* [PATCH 2/3] drm/amdgpu: Use RMW accessors for changing LNKCTL2
  2024-02-15 13:31 [PATCH 0/3] PCI LNKCTL2 RMW Accessor Cleanup Ilpo Järvinen
  2024-02-15 13:31 ` [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2 Ilpo Järvinen
@ 2024-02-15 13:31 ` Ilpo Järvinen
  2024-02-15 13:31 ` [PATCH 3/3] RDMA/hfi1: " Ilpo Järvinen
  2 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2024-02-15 13:31 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Jason Gunthorpe, Leon Romanovsky,
	linux-kernel, linux-rdma, Pan, Xinhui, Christian König
  Cc: Ilpo Järvinen, Lukas Wunner

Convert open coded RMW accesses for LNKCTL2 to use
pcie_capability_clear_and_set_word() which makes its easier to
understand what the code tries to do.

LNKCTL2 is not really owned by any driver because it is a collection of
control bits that PCI core might need to touch. RMW accessors already
have support for proper locking for a selected set of registers
(LNKCTL2 is not yet among them but likely will be in the future) to
avoid losing concurrent updates.

Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/gpu/drm/amd/amdgpu/cik.c | 41 ++++++++++++--------------------
 drivers/gpu/drm/amd/amdgpu/si.c  | 41 ++++++++++++--------------------
 2 files changed, 30 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index 4dfaa017cf7f..a3a643254d7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1638,28 +1638,18 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev)
 								   PCI_EXP_LNKCTL_HAWD);
 
 				/* linkctl2 */
-				pcie_capability_read_word(root, PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (bridge_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(root,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
-
-				pcie_capability_read_word(adev->pdev,
-							  PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (gpu_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(adev->pdev,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
+				pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   bridge_cfg2 &
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
+				pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   gpu_cfg2 &
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
 
 				tmp = RREG32_PCIE(ixPCIE_LC_CNTL4);
 				tmp &= ~PCIE_LC_CNTL4__LC_SET_QUIESCE_MASK;
@@ -1674,16 +1664,15 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev)
 	speed_cntl &= ~PCIE_LC_SPEED_CNTL__LC_FORCE_DIS_SW_SPEED_CHANGE_MASK;
 	WREG32_PCIE(ixPCIE_LC_SPEED_CNTL, speed_cntl);
 
-	pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL2, &tmp16);
-	tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
-
+	tmp16 = 0;
 	if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
 	else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
 	else
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
-	pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL2, tmp16);
+	pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL2,
+					   PCI_EXP_LNKCTL2_TLS, tmp16);
 
 	speed_cntl = RREG32_PCIE(ixPCIE_LC_SPEED_CNTL);
 	speed_cntl |= PCIE_LC_SPEED_CNTL__LC_INITIATE_LINK_SPEED_CHANGE_MASK;
diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c
index a757526153e5..23e4ef4fff7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/si.c
+++ b/drivers/gpu/drm/amd/amdgpu/si.c
@@ -2331,28 +2331,18 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev)
 								   gpu_cfg &
 								   PCI_EXP_LNKCTL_HAWD);
 
-				pcie_capability_read_word(root, PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (bridge_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(root,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
-
-				pcie_capability_read_word(adev->pdev,
-							  PCI_EXP_LNKCTL2,
-							  &tmp16);
-				tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN);
-				tmp16 |= (gpu_cfg2 &
-					  (PCI_EXP_LNKCTL2_ENTER_COMP |
-					   PCI_EXP_LNKCTL2_TX_MARGIN));
-				pcie_capability_write_word(adev->pdev,
-							   PCI_EXP_LNKCTL2,
-							   tmp16);
+				pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   bridge_cfg2 &
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
+				pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL2,
+								   PCI_EXP_LNKCTL2_ENTER_COMP |
+								   PCI_EXP_LNKCTL2_TX_MARGIN,
+								   gpu_cfg2 &
+								   (PCI_EXP_LNKCTL2_ENTER_COMP |
+								    PCI_EXP_LNKCTL2_TX_MARGIN));
 
 				tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
 				tmp &= ~LC_SET_QUIESCE;
@@ -2365,16 +2355,15 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev)
 	speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE;
 	WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);
 
-	pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL2, &tmp16);
-	tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
-
+	tmp16 = 0;
 	if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
 	else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
 	else
 		tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
-	pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL2, tmp16);
+	pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL2,
+					   PCI_EXP_LNKCTL2_TLS, tmp16);
 
 	speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
 	speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE;
-- 
2.39.2


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

* [PATCH 3/3] RDMA/hfi1: Use RMW accessors for changing LNKCTL2
  2024-02-15 13:31 [PATCH 0/3] PCI LNKCTL2 RMW Accessor Cleanup Ilpo Järvinen
  2024-02-15 13:31 ` [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2 Ilpo Järvinen
  2024-02-15 13:31 ` [PATCH 2/3] drm/amdgpu: " Ilpo Järvinen
@ 2024-02-15 13:31 ` Ilpo Järvinen
  2024-05-03 10:18   ` Ilpo Järvinen
  2 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2024-02-15 13:31 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Jason Gunthorpe, Leon Romanovsky,
	linux-kernel, linux-rdma, Pan, Xinhui, Christian König
  Cc: Ilpo Järvinen, Lukas Wunner, Dean Luick

Convert open coded RMW accesses for LNKCTL2 to use
pcie_capability_clear_and_set_word() which makes its easier to
understand what the code tries to do.

LNKCTL2 is not really owned by any driver because it is a collection of
control bits that PCI core might need to touch. RMW accessors already
have support for proper locking for a selected set of registers
(LNKCTL2 is not yet among them but likely will be in the future) to
avoid losing concurrent updates.

Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Dean Luick <dean.luick@cornelisnetworks.com>
---
 drivers/infiniband/hw/hfi1/pcie.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 119ec2f1382b..7133964749f8 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -1207,14 +1207,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
 		    (u32)lnkctl2);
 	/* only write to parent if target is not as high as ours */
 	if ((lnkctl2 & PCI_EXP_LNKCTL2_TLS) < target_vector) {
-		lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS;
-		lnkctl2 |= target_vector;
-		dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__,
-			    (u32)lnkctl2);
-		ret = pcie_capability_write_word(parent,
-						 PCI_EXP_LNKCTL2, lnkctl2);
+		ret = pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL2,
+							 PCI_EXP_LNKCTL2_TLS,
+							 target_vector);
 		if (ret) {
-			dd_dev_err(dd, "Unable to write to PCI config\n");
+			dd_dev_err(dd, "Unable to change parent PCI target speed\n");
 			return_error = 1;
 			goto done;
 		}
@@ -1223,22 +1220,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
 	}
 
 	dd_dev_info(dd, "%s: setting target link speed\n", __func__);
-	ret = pcie_capability_read_word(dd->pcidev, PCI_EXP_LNKCTL2, &lnkctl2);
+	ret = pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL2,
+						 PCI_EXP_LNKCTL2_TLS,
+						 target_vector);
 	if (ret) {
-		dd_dev_err(dd, "Unable to read from PCI config\n");
-		return_error = 1;
-		goto done;
-	}
-
-	dd_dev_info(dd, "%s: ..old link control2: 0x%x\n", __func__,
-		    (u32)lnkctl2);
-	lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS;
-	lnkctl2 |= target_vector;
-	dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__,
-		    (u32)lnkctl2);
-	ret = pcie_capability_write_word(dd->pcidev, PCI_EXP_LNKCTL2, lnkctl2);
-	if (ret) {
-		dd_dev_err(dd, "Unable to write to PCI config\n");
+		dd_dev_err(dd, "Unable to change device PCI target speed\n");
 		return_error = 1;
 		goto done;
 	}
-- 
2.39.2


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

* RE: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
  2024-02-15 13:31 ` [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2 Ilpo Järvinen
@ 2024-02-15 17:32   ` Deucher, Alexander
  2024-02-16  9:22     ` Ilpo Järvinen
  0 siblings, 1 reply; 10+ messages in thread
From: Deucher, Alexander @ 2024-02-15 17:32 UTC (permalink / raw)
  To: Ilpo Järvinen, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Jason Gunthorpe, Leon Romanovsky,
	linux-kernel, linux-rdma, Pan, Xinhui, Koenig, Christian
  Cc: Lukas Wunner

[Public]

> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Thursday, February 15, 2024 8:32 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> gfx@lists.freedesktop.org; Daniel Vetter <daniel@ffwll.ch>; David Airlie
> <airlied@gmail.com>; Dennis Dalessandro
> <dennis.dalessandro@cornelisnetworks.com>; dri-
> devel@lists.freedesktop.org; Jason Gunthorpe <jgg@ziepe.ca>; Leon
> Romanovsky <leon@kernel.org>; linux-kernel@vger.kernel.org; linux-
> rdma@vger.kernel.org; Pan, Xinhui <Xinhui.Pan@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>
> Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>; Lukas Wunner
> <lukas@wunner.de>
> Subject: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
>
> Convert open coded RMW accesses for LNKCTL2 to use
> pcie_capability_clear_and_set_word() which makes its easier to understand
> what the code tries to do.
>
> LNKCTL2 is not really owned by any driver because it is a collection of control
> bits that PCI core might need to touch. RMW accessors already have support
> for proper locking for a selected set of registers
> (LNKCTL2 is not yet among them but likely will be in the future) to avoid losing
> concurrent updates.
>
> Suggested-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

The radeon and amdgpu patches are:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

Are you looking for me to pick them up or do you want to land them as part of some larger change?  Either way is fine with me.

Alex

> ---
>  drivers/gpu/drm/radeon/cik.c | 40 ++++++++++++++----------------------
>  drivers/gpu/drm/radeon/si.c  | 40 ++++++++++++++----------------------
>  2 files changed, 30 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 10be30366c2b..b5e96a8fc2c1 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -9592,28 +9592,18 @@ static void cik_pcie_gen3_enable(struct
> radeon_device *rdev)
>
> PCI_EXP_LNKCTL_HAWD);
>
>                               /* linkctl2 */
> -                             pcie_capability_read_word(root,
> PCI_EXP_LNKCTL2,
> -                                                       &tmp16);
> -                             tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
> |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN);
> -                             tmp16 |= (bridge_cfg2 &
> -                                       (PCI_EXP_LNKCTL2_ENTER_COMP |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN));
> -                             pcie_capability_write_word(root,
> -                                                        PCI_EXP_LNKCTL2,
> -                                                        tmp16);
> -
> -                             pcie_capability_read_word(rdev->pdev,
> -                                                       PCI_EXP_LNKCTL2,
> -                                                       &tmp16);
> -                             tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
> |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN);
> -                             tmp16 |= (gpu_cfg2 &
> -                                       (PCI_EXP_LNKCTL2_ENTER_COMP |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN));
> -                             pcie_capability_write_word(rdev->pdev,
> -                                                        PCI_EXP_LNKCTL2,
> -                                                        tmp16);
> +                             pcie_capability_clear_and_set_word(root,
> PCI_EXP_LNKCTL2,
> +
> PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN,
> +                                                                bridge_cfg2
> |
> +
> (PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN));
> +                             pcie_capability_clear_and_set_word(rdev-
> >pdev, PCI_EXP_LNKCTL2,
> +
> PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN,
> +                                                                gpu_cfg2 |
> +
> (PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN));
>
>                               tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
>                               tmp &= ~LC_SET_QUIESCE;
> @@ -9627,15 +9617,15 @@ static void cik_pcie_gen3_enable(struct
> radeon_device *rdev)
>       speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE;
>       WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);
>
> -     pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL2, &tmp16);
> -     tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
> +     tmp16 = 0;
>       if (speed_cap == PCIE_SPEED_8_0GT)
>               tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
>       else if (speed_cap == PCIE_SPEED_5_0GT)
>               tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
>       else
>               tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
> -     pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL2, tmp16);
> +     pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
> +                                        PCI_EXP_LNKCTL2_TLS, tmp16);
>
>       speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
>       speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE; diff --git
> a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index
> 85e9cba49cec..8eeaea64c75d 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -7193,28 +7193,18 @@ static void si_pcie_gen3_enable(struct
> radeon_device *rdev)
>
> PCI_EXP_LNKCTL_HAWD);
>
>                               /* linkctl2 */
> -                             pcie_capability_read_word(root,
> PCI_EXP_LNKCTL2,
> -                                                       &tmp16);
> -                             tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
> |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN);
> -                             tmp16 |= (bridge_cfg2 &
> -                                       (PCI_EXP_LNKCTL2_ENTER_COMP |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN));
> -                             pcie_capability_write_word(root,
> -                                                        PCI_EXP_LNKCTL2,
> -                                                        tmp16);
> -
> -                             pcie_capability_read_word(rdev->pdev,
> -                                                       PCI_EXP_LNKCTL2,
> -                                                       &tmp16);
> -                             tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
> |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN);
> -                             tmp16 |= (gpu_cfg2 &
> -                                       (PCI_EXP_LNKCTL2_ENTER_COMP |
> -                                        PCI_EXP_LNKCTL2_TX_MARGIN));
> -                             pcie_capability_write_word(rdev->pdev,
> -                                                        PCI_EXP_LNKCTL2,
> -                                                        tmp16);
> +                             pcie_capability_clear_and_set_word(root,
> PCI_EXP_LNKCTL2,
> +
> PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN,
> +                                                                bridge_cfg2
> &
> +
> (PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN));
> +                             pcie_capability_clear_and_set_word(rdev-
> >pdev, PCI_EXP_LNKCTL2,
> +
> PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN,
> +                                                                gpu_cfg2 &
> +
> (PCI_EXP_LNKCTL2_ENTER_COMP |
> +
> PCI_EXP_LNKCTL2_TX_MARGIN));
>
>                               tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
>                               tmp &= ~LC_SET_QUIESCE;
> @@ -7228,15 +7218,15 @@ static void si_pcie_gen3_enable(struct
> radeon_device *rdev)
>       speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE;
>       WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);
>
> -     pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL2, &tmp16);
> -     tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
> +     tmp16 = 0;
>       if (speed_cap == PCIE_SPEED_8_0GT)
>               tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
>       else if (speed_cap == PCIE_SPEED_5_0GT)
>               tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
>       else
>               tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
> -     pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL2, tmp16);
> +     pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
> +                                        PCI_EXP_LNKCTL2_TLS, tmp16);
>
>       speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
>       speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE;
> --
> 2.39.2


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

* RE: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
  2024-02-15 17:32   ` Deucher, Alexander
@ 2024-02-16  9:22     ` Ilpo Järvinen
  2024-02-16 16:59       ` Alex Deucher
  0 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2024-02-16  9:22 UTC (permalink / raw)
  To: Deucher, Alexander
  Cc: amd-gfx, Daniel Vetter, David Airlie, Dennis Dalessandro,
	dri-devel, Jason Gunthorpe, Leon Romanovsky, linux-kernel,
	linux-rdma, Pan, Xinhui, Koenig, Christian, Lukas Wunner

[-- Attachment #1: Type: text/plain, Size: 1986 bytes --]

On Thu, 15 Feb 2024, Deucher, Alexander wrote:

> [Public]
> 
> > -----Original Message-----
> > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Sent: Thursday, February 15, 2024 8:32 AM
> > To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> > gfx@lists.freedesktop.org; Daniel Vetter <daniel@ffwll.ch>; David Airlie
> > <airlied@gmail.com>; Dennis Dalessandro
> > <dennis.dalessandro@cornelisnetworks.com>; dri-
> > devel@lists.freedesktop.org; Jason Gunthorpe <jgg@ziepe.ca>; Leon
> > Romanovsky <leon@kernel.org>; linux-kernel@vger.kernel.org; linux-
> > rdma@vger.kernel.org; Pan, Xinhui <Xinhui.Pan@amd.com>; Koenig, Christian
> > <Christian.Koenig@amd.com>
> > Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>; Lukas Wunner
> > <lukas@wunner.de>
> > Subject: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
> >
> > Convert open coded RMW accesses for LNKCTL2 to use
> > pcie_capability_clear_and_set_word() which makes its easier to understand
> > what the code tries to do.
> >
> > LNKCTL2 is not really owned by any driver because it is a collection of control
> > bits that PCI core might need to touch. RMW accessors already have support
> > for proper locking for a selected set of registers
> > (LNKCTL2 is not yet among them but likely will be in the future) to avoid losing
> > concurrent updates.
> >
> > Suggested-by: Lukas Wunner <lukas@wunner.de>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> The radeon and amdgpu patches are:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> 
> Are you looking for me to pick them up or do you want to land them as 
> part of some larger change?  Either way is fine with me. 

Hi,

You please take them, I intentionally took them apart from the BW 
controller series so they can go through the usual trees, not along with 
the BW controller. (I don't expect the BW controller to be accepted during 
this cycle).

-- 
 i.

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

* Re: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
  2024-02-16  9:22     ` Ilpo Järvinen
@ 2024-02-16 16:59       ` Alex Deucher
  0 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2024-02-16 16:59 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Deucher, Alexander, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Jason Gunthorpe, Leon Romanovsky,
	linux-kernel, linux-rdma, Pan, Xinhui, Koenig, Christian,
	Lukas Wunner

Applied.  Thanks.

Alex

On Fri, Feb 16, 2024 at 5:38 AM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Thu, 15 Feb 2024, Deucher, Alexander wrote:
>
> > [Public]
> >
> > > -----Original Message-----
> > > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Sent: Thursday, February 15, 2024 8:32 AM
> > > To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
> > > gfx@lists.freedesktop.org; Daniel Vetter <daniel@ffwll.ch>; David Airlie
> > > <airlied@gmail.com>; Dennis Dalessandro
> > > <dennis.dalessandro@cornelisnetworks.com>; dri-
> > > devel@lists.freedesktop.org; Jason Gunthorpe <jgg@ziepe.ca>; Leon
> > > Romanovsky <leon@kernel.org>; linux-kernel@vger.kernel.org; linux-
> > > rdma@vger.kernel.org; Pan, Xinhui <Xinhui.Pan@amd.com>; Koenig, Christian
> > > <Christian.Koenig@amd.com>
> > > Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>; Lukas Wunner
> > > <lukas@wunner.de>
> > > Subject: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
> > >
> > > Convert open coded RMW accesses for LNKCTL2 to use
> > > pcie_capability_clear_and_set_word() which makes its easier to understand
> > > what the code tries to do.
> > >
> > > LNKCTL2 is not really owned by any driver because it is a collection of control
> > > bits that PCI core might need to touch. RMW accessors already have support
> > > for proper locking for a selected set of registers
> > > (LNKCTL2 is not yet among them but likely will be in the future) to avoid losing
> > > concurrent updates.
> > >
> > > Suggested-by: Lukas Wunner <lukas@wunner.de>
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >
> > The radeon and amdgpu patches are:
> > Acked-by: Alex Deucher <alexander.deucher@amd.com>
> >
> > Are you looking for me to pick them up or do you want to land them as
> > part of some larger change?  Either way is fine with me.
>
> Hi,
>
> You please take them, I intentionally took them apart from the BW
> controller series so they can go through the usual trees, not along with
> the BW controller. (I don't expect the BW controller to be accepted during
> this cycle).
>
> --
>  i.

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

* Re: [PATCH 3/3] RDMA/hfi1: Use RMW accessors for changing LNKCTL2
  2024-02-15 13:31 ` [PATCH 3/3] RDMA/hfi1: " Ilpo Järvinen
@ 2024-05-03 10:18   ` Ilpo Järvinen
  2024-05-03 13:04     ` Jason Gunthorpe
  0 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2024-05-03 10:18 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Alex Deucher, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Jason Gunthorpe, Leon Romanovsky,
	LKML, linux-rdma, Pan, Xinhui, Christian König,
	Lukas Wunner, Dean Luick

[-- Attachment #1: Type: text/plain, Size: 3472 bytes --]

On Thu, 15 Feb 2024, Ilpo Järvinen wrote:

> Convert open coded RMW accesses for LNKCTL2 to use
> pcie_capability_clear_and_set_word() which makes its easier to
> understand what the code tries to do.
> 
> LNKCTL2 is not really owned by any driver because it is a collection of
> control bits that PCI core might need to touch. RMW accessors already
> have support for proper locking for a selected set of registers
> (LNKCTL2 is not yet among them but likely will be in the future) to
> avoid losing concurrent updates.
> 
> Suggested-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Reviewed-by: Dean Luick <dean.luick@cornelisnetworks.com>

I found out from Linux RDMA and InfiniBand patchwork that this patch had 
been silently closed as "Not Applicable". Is there some reason for that?

I was sending this change independently out (among 2 similar ones that 
already got applied) so I wouldn't need to keep carrying it within my PCIe 
bandwidth controller series. It seemed useful enough as cleanups to stand 
on its own legs w/o requiring it to be part of PCIe bw controller series.

Should I resend the patch or do RDMA/IB maintainers prefer it to remain 
as a part of PCIe BW controller series?

-- 
 i.

> ---
>  drivers/infiniband/hw/hfi1/pcie.c | 30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
> index 119ec2f1382b..7133964749f8 100644
> --- a/drivers/infiniband/hw/hfi1/pcie.c
> +++ b/drivers/infiniband/hw/hfi1/pcie.c
> @@ -1207,14 +1207,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
>  		    (u32)lnkctl2);
>  	/* only write to parent if target is not as high as ours */
>  	if ((lnkctl2 & PCI_EXP_LNKCTL2_TLS) < target_vector) {
> -		lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS;
> -		lnkctl2 |= target_vector;
> -		dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__,
> -			    (u32)lnkctl2);
> -		ret = pcie_capability_write_word(parent,
> -						 PCI_EXP_LNKCTL2, lnkctl2);
> +		ret = pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL2,
> +							 PCI_EXP_LNKCTL2_TLS,
> +							 target_vector);
>  		if (ret) {
> -			dd_dev_err(dd, "Unable to write to PCI config\n");
> +			dd_dev_err(dd, "Unable to change parent PCI target speed\n");
>  			return_error = 1;
>  			goto done;
>  		}
> @@ -1223,22 +1220,11 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
>  	}
>  
>  	dd_dev_info(dd, "%s: setting target link speed\n", __func__);
> -	ret = pcie_capability_read_word(dd->pcidev, PCI_EXP_LNKCTL2, &lnkctl2);
> +	ret = pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL2,
> +						 PCI_EXP_LNKCTL2_TLS,
> +						 target_vector);
>  	if (ret) {
> -		dd_dev_err(dd, "Unable to read from PCI config\n");
> -		return_error = 1;
> -		goto done;
> -	}
> -
> -	dd_dev_info(dd, "%s: ..old link control2: 0x%x\n", __func__,
> -		    (u32)lnkctl2);
> -	lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS;
> -	lnkctl2 |= target_vector;
> -	dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__,
> -		    (u32)lnkctl2);
> -	ret = pcie_capability_write_word(dd->pcidev, PCI_EXP_LNKCTL2, lnkctl2);
> -	if (ret) {
> -		dd_dev_err(dd, "Unable to write to PCI config\n");
> +		dd_dev_err(dd, "Unable to change device PCI target speed\n");
>  		return_error = 1;
>  		goto done;
>  	}
> 

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

* Re: [PATCH 3/3] RDMA/hfi1: Use RMW accessors for changing LNKCTL2
  2024-05-03 10:18   ` Ilpo Järvinen
@ 2024-05-03 13:04     ` Jason Gunthorpe
  2024-05-05 13:09       ` Leon Romanovsky
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2024-05-03 13:04 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Alex Deucher, amd-gfx, Daniel Vetter, David Airlie,
	Dennis Dalessandro, dri-devel, Leon Romanovsky, LKML, linux-rdma,
	Pan, Xinhui, Christian König, Lukas Wunner, Dean Luick

On Fri, May 03, 2024 at 01:18:35PM +0300, Ilpo Järvinen wrote:
> On Thu, 15 Feb 2024, Ilpo Järvinen wrote:
> 
> > Convert open coded RMW accesses for LNKCTL2 to use
> > pcie_capability_clear_and_set_word() which makes its easier to
> > understand what the code tries to do.
> > 
> > LNKCTL2 is not really owned by any driver because it is a collection of
> > control bits that PCI core might need to touch. RMW accessors already
> > have support for proper locking for a selected set of registers
> > (LNKCTL2 is not yet among them but likely will be in the future) to
> > avoid losing concurrent updates.
> > 
> > Suggested-by: Lukas Wunner <lukas@wunner.de>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Reviewed-by: Dean Luick <dean.luick@cornelisnetworks.com>
> 
> I found out from Linux RDMA and InfiniBand patchwork that this patch had 
> been silently closed as "Not Applicable". Is there some reason for
> that?

It is part of a series that crosses subsystems, series like that
usually go through some other trees.

If you want single patches applied then please send single
patches.. It is hard to understand intent from mixed series.

Jason

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

* Re: [PATCH 3/3] RDMA/hfi1: Use RMW accessors for changing LNKCTL2
  2024-05-03 13:04     ` Jason Gunthorpe
@ 2024-05-05 13:09       ` Leon Romanovsky
  0 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2024-05-05 13:09 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Ilpo Järvinen, Alex Deucher, amd-gfx, Daniel Vetter,
	David Airlie, Dennis Dalessandro, dri-devel, LKML, linux-rdma,
	Pan, Xinhui, Christian König, Lukas Wunner, Dean Luick

On Fri, May 03, 2024 at 10:04:16AM -0300, Jason Gunthorpe wrote:
> On Fri, May 03, 2024 at 01:18:35PM +0300, Ilpo Järvinen wrote:
> > On Thu, 15 Feb 2024, Ilpo Järvinen wrote:
> > 
> > > Convert open coded RMW accesses for LNKCTL2 to use
> > > pcie_capability_clear_and_set_word() which makes its easier to
> > > understand what the code tries to do.
> > > 
> > > LNKCTL2 is not really owned by any driver because it is a collection of
> > > control bits that PCI core might need to touch. RMW accessors already
> > > have support for proper locking for a selected set of registers
> > > (LNKCTL2 is not yet among them but likely will be in the future) to
> > > avoid losing concurrent updates.
> > > 
> > > Suggested-by: Lukas Wunner <lukas@wunner.de>
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Reviewed-by: Dean Luick <dean.luick@cornelisnetworks.com>
> > 
> > I found out from Linux RDMA and InfiniBand patchwork that this patch had 
> > been silently closed as "Not Applicable". Is there some reason for
> > that?
> 
> It is part of a series that crosses subsystems, series like that
> usually go through some other trees.

Exactly, this is why I marked it as "Not Applicable".

> 
> If you want single patches applied then please send single
> patches.. It is hard to understand intent from mixed series.
> 
> Jason

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

end of thread, other threads:[~2024-05-05 13:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-15 13:31 [PATCH 0/3] PCI LNKCTL2 RMW Accessor Cleanup Ilpo Järvinen
2024-02-15 13:31 ` [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2 Ilpo Järvinen
2024-02-15 17:32   ` Deucher, Alexander
2024-02-16  9:22     ` Ilpo Järvinen
2024-02-16 16:59       ` Alex Deucher
2024-02-15 13:31 ` [PATCH 2/3] drm/amdgpu: " Ilpo Järvinen
2024-02-15 13:31 ` [PATCH 3/3] RDMA/hfi1: " Ilpo Järvinen
2024-05-03 10:18   ` Ilpo Järvinen
2024-05-03 13:04     ` Jason Gunthorpe
2024-05-05 13:09       ` Leon Romanovsky

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.