linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v4 1/5] PCI: rockchip: Use standard PCIe defines
       [not found] <cover.1749826250.git.geraldogabriel@gmail.com>
@ 2025-06-13 14:51 ` Geraldo Nascimento
  2025-06-13 14:51 ` [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields Geraldo Nascimento
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Geraldo Nascimento @ 2025-06-13 14:51 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	Vinod Koul, Kishon Vijay Abraham I, linux-phy, linux-pci,
	linux-arm-kernel, linux-kernel

Current code uses custom-defined register offsets
and bitfields for standard PCIe registers. Change
to using standard PCIe defines.

Suggested-By: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 44 ++++++++++-----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index b9e7a8710cf0..65653218b9ab 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -40,18 +40,18 @@ static void rockchip_pcie_enable_bw_int(struct rockchip_pcie *rockchip)
 {
 	u32 status;
 
-	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
+	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 	status |= (PCI_EXP_LNKCTL_LBMIE | PCI_EXP_LNKCTL_LABIE);
-	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
+	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 }
 
 static void rockchip_pcie_clr_bw_int(struct rockchip_pcie *rockchip)
 {
 	u32 status;
 
-	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
+	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 	status |= (PCI_EXP_LNKSTA_LBMS | PCI_EXP_LNKSTA_LABS) << 16;
-	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
+	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 }
 
 static void rockchip_pcie_update_txcredit_mui(struct rockchip_pcie *rockchip)
@@ -269,7 +269,7 @@ static void rockchip_pcie_set_power_limit(struct rockchip_pcie *rockchip)
 	scale = 3; /* 0.001x */
 	curr = curr / 1000; /* convert to mA */
 	power = (curr * 3300) / 1000; /* milliwatt */
-	while (power > PCIE_RC_CONFIG_DCR_CSPL_LIMIT) {
+	while (power > FIELD_MAX(PCI_EXP_DEVCAP_PWR_VAL)) {
 		if (!scale) {
 			dev_warn(rockchip->dev, "invalid power supply\n");
 			return;
@@ -278,10 +278,10 @@ static void rockchip_pcie_set_power_limit(struct rockchip_pcie *rockchip)
 		power = power / 10;
 	}
 
-	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_DCR);
-	status |= (power << PCIE_RC_CONFIG_DCR_CSPL_SHIFT) |
-		  (scale << PCIE_RC_CONFIG_DCR_CPLS_SHIFT);
-	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_DCR);
+	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_DEVCAP);
+	status |= FIELD_PREP(PCI_EXP_DEVCAP_PWR_VAL, power);
+	status |= FIELD_PREP(PCI_EXP_DEVCAP_PWR_SCL, scale);
+	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_DEVCAP);
 }
 
 /**
@@ -309,14 +309,14 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 	rockchip_pcie_set_power_limit(rockchip);
 
 	/* Set RC's clock architecture as common clock */
-	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
+	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 	status |= PCI_EXP_LNKSTA_SLC << 16;
-	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
+	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 
 	/* Set RC's RCB to 128 */
-	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
+	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 	status |= PCI_EXP_LNKCTL_RCB;
-	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
+	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 
 	/* Enable Gen1 training */
 	rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
@@ -341,9 +341,9 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 		 * Enable retrain for gen2. This should be configured only after
 		 * gen1 finished.
 		 */
-		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
+		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 		status |= PCI_EXP_LNKCTL_RL;
-		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
+		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 
 		err = readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
 					 status, PCIE_LINK_IS_GEN2(status), 20,
@@ -380,15 +380,15 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 
 	/* Clear L0s from RC's link cap */
 	if (of_property_read_bool(dev->of_node, "aspm-no-l0s")) {
-		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LINK_CAP);
-		status &= ~PCIE_RC_CONFIG_LINK_CAP_L0S;
-		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LINK_CAP);
+		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCAP);
+		status &= ~PCI_EXP_LNKCAP_ASPM_L0S;
+		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCAP);
 	}
 
-	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_DCSR);
-	status &= ~PCIE_RC_CONFIG_DCSR_MPS_MASK;
-	status |= PCIE_RC_CONFIG_DCSR_MPS_256;
-	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_DCSR);
+	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_DEVCTL);
+	status &= ~PCI_EXP_DEVCTL_PAYLOAD;
+	status |= PCI_EXP_DEVCTL_PAYLOAD_256B;
+	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_DEVCTL);
 
 	return 0;
 err_power_off_phy:
-- 
2.49.0


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

* [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields
       [not found] <cover.1749826250.git.geraldogabriel@gmail.com>
  2025-06-13 14:51 ` [RFC PATCH v4 1/5] PCI: rockchip: Use standard PCIe defines Geraldo Nascimento
@ 2025-06-13 14:51 ` Geraldo Nascimento
  2025-06-13 15:03   ` Ilpo Järvinen
  2025-06-13 14:51 ` [RFC PATCH v4 3/5] PCI: rockchip: Set Target Link Speed before retraining Geraldo Nascimento
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Geraldo Nascimento @ 2025-06-13 14:51 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	Vinod Koul, Kishon Vijay Abraham I, linux-phy, linux-pci,
	linux-arm-kernel, linux-kernel

Since we are now using standard PCIe defines, drop
unused custom-defined ones, which are now referenced
from offset at added Capabilities Register.

Suggested-By: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip.h | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 5864a20323f2..f611599988d7 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -155,16 +155,7 @@
 #define PCIE_EP_CONFIG_DID_VID		(PCIE_EP_CONFIG_BASE + 0x00)
 #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
 #define PCIE_RC_CONFIG_RID_CCR		(PCIE_RC_CONFIG_BASE + 0x08)
-#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_BASE + 0xc4)
-#define   PCIE_RC_CONFIG_DCR_CSPL_SHIFT		18
-#define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
-#define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26
-#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_BASE + 0xc8)
-#define   PCIE_RC_CONFIG_DCSR_MPS_MASK		GENMASK(7, 5)
-#define   PCIE_RC_CONFIG_DCSR_MPS_256		(0x1 << 5)
-#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_BASE + 0xcc)
-#define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
-#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
+#define PCIE_RC_CONFIG_CR		(PCIE_RC_CONFIG_BASE + 0xc0)
 #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
 #define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
 #define PCIE_RC_CONFIG_THP_CAP		(PCIE_RC_CONFIG_BASE + 0x274)
-- 
2.49.0


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

* [RFC PATCH v4 3/5] PCI: rockchip: Set Target Link Speed before retraining
       [not found] <cover.1749826250.git.geraldogabriel@gmail.com>
  2025-06-13 14:51 ` [RFC PATCH v4 1/5] PCI: rockchip: Use standard PCIe defines Geraldo Nascimento
  2025-06-13 14:51 ` [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields Geraldo Nascimento
@ 2025-06-13 14:51 ` Geraldo Nascimento
  2025-06-13 14:51 ` [RFC PATCH v4 4/5] phy: rockchip-pcie: Enable all four lanes Geraldo Nascimento
  2025-06-13 14:52 ` [RFC PATCH v4 5/5] phy: rockchip-pcie: Adjust read mask and write Geraldo Nascimento
  4 siblings, 0 replies; 8+ messages in thread
From: Geraldo Nascimento @ 2025-06-13 14:51 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	Vinod Koul, Kishon Vijay Abraham I, linux-phy, linux-pci,
	linux-arm-kernel, linux-kernel

Current code may fail Gen2 retraining if Target Link Speed
is set to 2.5 GT/s in Link Control and Status Register 2.
Set it to 5.0 GT/s accordingly.

Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index 65653218b9ab..7a0b6ebb7c27 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -341,6 +341,10 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 		 * Enable retrain for gen2. This should be configured only after
 		 * gen1 finished.
 		 */
+		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
+		status &= ~PCI_EXP_LNKCTL2_TLS;
+		status |= PCI_EXP_LNKCTL2_TLS_5_0GT;
+		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
 		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
 		status |= PCI_EXP_LNKCTL_RL;
 		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL);
-- 
2.49.0


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

* [RFC PATCH v4 4/5] phy: rockchip-pcie: Enable all four lanes
       [not found] <cover.1749826250.git.geraldogabriel@gmail.com>
                   ` (2 preceding siblings ...)
  2025-06-13 14:51 ` [RFC PATCH v4 3/5] PCI: rockchip: Set Target Link Speed before retraining Geraldo Nascimento
@ 2025-06-13 14:51 ` Geraldo Nascimento
  2025-06-13 14:52 ` [RFC PATCH v4 5/5] phy: rockchip-pcie: Adjust read mask and write Geraldo Nascimento
  4 siblings, 0 replies; 8+ messages in thread
From: Geraldo Nascimento @ 2025-06-13 14:51 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	Vinod Koul, Kishon Vijay Abraham I, linux-phy, linux-pci,
	linux-arm-kernel, linux-kernel

Current code enables only Lane 0 because pwr_cnt will be incremented
on first call to the function. Use for-loop to enable all 4 lanes
through GRF.

Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/phy/rockchip/phy-rockchip-pcie.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
index bd44af36c67a..48bcc7d2b33b 100644
--- a/drivers/phy/rockchip/phy-rockchip-pcie.c
+++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
@@ -176,11 +176,13 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)
 				   PHY_CFG_ADDR_MASK,
 				   PHY_CFG_ADDR_SHIFT));
 
-	regmap_write(rk_phy->reg_base,
-		     rk_phy->phy_data->pcie_laneoff,
-		     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
-				   PHY_LANE_IDLE_MASK,
-				   PHY_LANE_IDLE_A_SHIFT + inst->index));
+	for (int i=0; i < PHY_MAX_LANE_NUM; i++) {
+		regmap_write(rk_phy->reg_base,
+			     rk_phy->phy_data->pcie_laneoff,
+			     HIWORD_UPDATE(!PHY_LANE_IDLE_OFF,
+					   PHY_LANE_IDLE_MASK,
+					   PHY_LANE_IDLE_A_SHIFT + i));
+	}
 
 	/*
 	 * No documented timeout value for phy operation below,
-- 
2.49.0


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

* [RFC PATCH v4 5/5] phy: rockchip-pcie: Adjust read mask and write
       [not found] <cover.1749826250.git.geraldogabriel@gmail.com>
                   ` (3 preceding siblings ...)
  2025-06-13 14:51 ` [RFC PATCH v4 4/5] phy: rockchip-pcie: Enable all four lanes Geraldo Nascimento
@ 2025-06-13 14:52 ` Geraldo Nascimento
  4 siblings, 0 replies; 8+ messages in thread
From: Geraldo Nascimento @ 2025-06-13 14:52 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Shawn Lin, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
	Vinod Koul, Kishon Vijay Abraham I, linux-phy, linux-pci,
	linux-arm-kernel, linux-kernel

Section 17.6.10 of the RK3399 TRM "PCIe PIPE PHY registers Description"
defines asynchronous strobe TEST_WRITE which should be enabled then
disabled and seems to have been copy-pasted as of current. Adjust it.
While at it, adjust read mask which should be the same as write mask.

Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/phy/rockchip/phy-rockchip-pcie.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-pcie.c b/drivers/phy/rockchip/phy-rockchip-pcie.c
index 48bcc7d2b33b..35d2523ee776 100644
--- a/drivers/phy/rockchip/phy-rockchip-pcie.c
+++ b/drivers/phy/rockchip/phy-rockchip-pcie.c
@@ -30,9 +30,9 @@
 #define PHY_CFG_ADDR_SHIFT    1
 #define PHY_CFG_DATA_MASK     0xf
 #define PHY_CFG_ADDR_MASK     0x3f
-#define PHY_CFG_RD_MASK       0x3ff
+#define PHY_CFG_RD_MASK       0x3f
 #define PHY_CFG_WR_ENABLE     1
-#define PHY_CFG_WR_DISABLE    1
+#define PHY_CFG_WR_DISABLE    0
 #define PHY_CFG_WR_SHIFT      0
 #define PHY_CFG_WR_MASK       1
 #define PHY_CFG_PLL_LOCK      0x10
-- 
2.49.0


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

* Re: [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields
  2025-06-13 14:51 ` [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields Geraldo Nascimento
@ 2025-06-13 15:03   ` Ilpo Järvinen
  2025-06-13 15:19     ` Geraldo Nascimento
  0 siblings, 1 reply; 8+ messages in thread
From: Ilpo Järvinen @ 2025-06-13 15:03 UTC (permalink / raw)
  To: Geraldo Nascimento
  Cc: linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I,
	linux-phy, linux-pci, linux-arm-kernel, LKML

On Fri, 13 Jun 2025, Geraldo Nascimento wrote:

> Since we are now using standard PCIe defines, drop
> unused custom-defined ones, which are now referenced
> from offset at added Capabilities Register.

These are quite short lines, please reflow the changelog paragraphs to the 
usual length.

> Suggested-By: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip.h | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index 5864a20323f2..f611599988d7 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -155,16 +155,7 @@
>  #define PCIE_EP_CONFIG_DID_VID		(PCIE_EP_CONFIG_BASE + 0x00)
>  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
>  #define PCIE_RC_CONFIG_RID_CCR		(PCIE_RC_CONFIG_BASE + 0x08)
> -#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_BASE + 0xc4)
> -#define   PCIE_RC_CONFIG_DCR_CSPL_SHIFT		18
> -#define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
> -#define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26
> -#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_BASE + 0xc8)
> -#define   PCIE_RC_CONFIG_DCSR_MPS_MASK		GENMASK(7, 5)
> -#define   PCIE_RC_CONFIG_DCSR_MPS_256		(0x1 << 5)
> -#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_BASE + 0xcc)
> -#define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
> -#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
> +#define PCIE_RC_CONFIG_CR		(PCIE_RC_CONFIG_BASE + 0xc0)

This will cause a build failure because PCIE_RC_CONFIG_CR is used in 1/5 
but only introduced here so you'll need to do this in the same patch as 
any step within a series must build too. IMO it would anyway make sense to 
combine patches 1 & 2.

>  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)

Aren't you going to convert this as well?

>  #define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
>  #define PCIE_RC_CONFIG_THP_CAP		(PCIE_RC_CONFIG_BASE + 0x274)
> 

-- 
 i.


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

* Re: [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields
  2025-06-13 15:03   ` Ilpo Järvinen
@ 2025-06-13 15:19     ` Geraldo Nascimento
  2025-06-13 15:25       ` Ilpo Järvinen
  0 siblings, 1 reply; 8+ messages in thread
From: Geraldo Nascimento @ 2025-06-13 15:19 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I,
	linux-phy, linux-pci, linux-arm-kernel, LKML

On Fri, Jun 13, 2025 at 06:03:14PM +0300, Ilpo Järvinen wrote:
> On Fri, 13 Jun 2025, Geraldo Nascimento wrote:
> 
> > Since we are now using standard PCIe defines, drop
> > unused custom-defined ones, which are now referenced
> > from offset at added Capabilities Register.
> 
> These are quite short lines, please reflow the changelog paragraphs to the 
> usual length.

Hi Ilpo,

I'll reflow for v5.

> 
> > Suggested-By: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  drivers/pci/controller/pcie-rockchip.h | 11 +----------
> >  1 file changed, 1 insertion(+), 10 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> > index 5864a20323f2..f611599988d7 100644
> > --- a/drivers/pci/controller/pcie-rockchip.h
> > +++ b/drivers/pci/controller/pcie-rockchip.h
> > @@ -155,16 +155,7 @@
> >  #define PCIE_EP_CONFIG_DID_VID		(PCIE_EP_CONFIG_BASE + 0x00)
> >  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
> >  #define PCIE_RC_CONFIG_RID_CCR		(PCIE_RC_CONFIG_BASE + 0x08)
> > -#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_BASE + 0xc4)
> > -#define   PCIE_RC_CONFIG_DCR_CSPL_SHIFT		18
> > -#define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
> > -#define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26
> > -#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_BASE + 0xc8)
> > -#define   PCIE_RC_CONFIG_DCSR_MPS_MASK		GENMASK(7, 5)
> > -#define   PCIE_RC_CONFIG_DCSR_MPS_256		(0x1 << 5)
> > -#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_BASE + 0xcc)
> > -#define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
> > -#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
> > +#define PCIE_RC_CONFIG_CR		(PCIE_RC_CONFIG_BASE + 0xc0)
> 
> This will cause a build failure because PCIE_RC_CONFIG_CR is used in 1/5 
> but only introduced here so you'll need to do this in the same patch as 
> any step within a series must build too. IMO it would anyway make sense to 
> combine patches 1 & 2.

Ah, interesting angle. I'll fix it.

> 
> >  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
> 
> Aren't you going to convert this as well?

I can, but I can't test it however! But I'll Cc: someone who hopefully
can.

Thanks,
Geraldo Nascimento
> 
> >  #define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
> >  #define PCIE_RC_CONFIG_THP_CAP		(PCIE_RC_CONFIG_BASE + 0x274)
> > 
> 
> -- 
>  i.
> 

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

* Re: [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields
  2025-06-13 15:19     ` Geraldo Nascimento
@ 2025-06-13 15:25       ` Ilpo Järvinen
  0 siblings, 0 replies; 8+ messages in thread
From: Ilpo Järvinen @ 2025-06-13 15:25 UTC (permalink / raw)
  To: Geraldo Nascimento
  Cc: linux-rockchip, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner, Vinod Koul, Kishon Vijay Abraham I,
	linux-phy, linux-pci, linux-arm-kernel, LKML

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

On Fri, 13 Jun 2025, Geraldo Nascimento wrote:

> On Fri, Jun 13, 2025 at 06:03:14PM +0300, Ilpo Järvinen wrote:
> > On Fri, 13 Jun 2025, Geraldo Nascimento wrote:
> > 
> > > Since we are now using standard PCIe defines, drop
> > > unused custom-defined ones, which are now referenced
> > > from offset at added Capabilities Register.
> > 
> > These are quite short lines, please reflow the changelog paragraphs to the 
> > usual length.
> 
> Hi Ilpo,
> 
> I'll reflow for v5.
> 
> > 
> > > Suggested-By: Bjorn Helgaas <bhelgaas@google.com>
> > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > > ---
> > >  drivers/pci/controller/pcie-rockchip.h | 11 +----------
> > >  1 file changed, 1 insertion(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> > > index 5864a20323f2..f611599988d7 100644
> > > --- a/drivers/pci/controller/pcie-rockchip.h
> > > +++ b/drivers/pci/controller/pcie-rockchip.h
> > > @@ -155,16 +155,7 @@
> > >  #define PCIE_EP_CONFIG_DID_VID		(PCIE_EP_CONFIG_BASE + 0x00)
> > >  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
> > >  #define PCIE_RC_CONFIG_RID_CCR		(PCIE_RC_CONFIG_BASE + 0x08)
> > > -#define PCIE_RC_CONFIG_DCR		(PCIE_RC_CONFIG_BASE + 0xc4)
> > > -#define   PCIE_RC_CONFIG_DCR_CSPL_SHIFT		18
> > > -#define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
> > > -#define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26
> > > -#define PCIE_RC_CONFIG_DCSR		(PCIE_RC_CONFIG_BASE + 0xc8)
> > > -#define   PCIE_RC_CONFIG_DCSR_MPS_MASK		GENMASK(7, 5)
> > > -#define   PCIE_RC_CONFIG_DCSR_MPS_256		(0x1 << 5)
> > > -#define PCIE_RC_CONFIG_LINK_CAP		(PCIE_RC_CONFIG_BASE + 0xcc)
> > > -#define   PCIE_RC_CONFIG_LINK_CAP_L0S		BIT(10)
> > > -#define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
> > > +#define PCIE_RC_CONFIG_CR		(PCIE_RC_CONFIG_BASE + 0xc0)
> > 
> > This will cause a build failure because PCIE_RC_CONFIG_CR is used in 1/5 
> > but only introduced here so you'll need to do this in the same patch as 
> > any step within a series must build too. IMO it would anyway make sense to 
> > combine patches 1 & 2.
> 
> Ah, interesting angle. I'll fix it.
> 
> > 
> > >  #define PCIE_EP_CONFIG_LCS		(PCIE_EP_CONFIG_BASE + 0xd0)
> > 
> > Aren't you going to convert this as well?
> 
> I can, but I can't test it however! But I'll Cc: someone who hopefully
> can.

TBH, the risk getting it wrong / changing the resulting object is pretty 
low. :-)

It might be that scripts/objdiff tool could prove there were no changes in 
the binary code output as it looks just a pre-preprocessor change.

-- 
 i.

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

end of thread, other threads:[~2025-06-13 15:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1749826250.git.geraldogabriel@gmail.com>
2025-06-13 14:51 ` [RFC PATCH v4 1/5] PCI: rockchip: Use standard PCIe defines Geraldo Nascimento
2025-06-13 14:51 ` [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields Geraldo Nascimento
2025-06-13 15:03   ` Ilpo Järvinen
2025-06-13 15:19     ` Geraldo Nascimento
2025-06-13 15:25       ` Ilpo Järvinen
2025-06-13 14:51 ` [RFC PATCH v4 3/5] PCI: rockchip: Set Target Link Speed before retraining Geraldo Nascimento
2025-06-13 14:51 ` [RFC PATCH v4 4/5] phy: rockchip-pcie: Enable all four lanes Geraldo Nascimento
2025-06-13 14:52 ` [RFC PATCH v4 5/5] phy: rockchip-pcie: Adjust read mask and write Geraldo Nascimento

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