PCI: rockchip: Improve driver quality
Subsystems (5)
v9 cover PCI: rockchip: Improve driver quality
During a 30-day debugging-run fighting quirky PCIe devices on RK3399 some quality improvements began to take form and after feedback from community they reached more polished state.
This will ensure maximum chance of retraining to 5.0GT/s, on all four lanes and fix async strobe TEST_WRITE disablement. On top of this, standard PCIe defines are now used to reference registers from offset at Capabilities Register.
Unfortunately, it seems Rockchip-IP PCIe is unable to handle 16-bit register writes and there's risk of corrupting state of RW1C registers, an issue raised by Bjorn Helgaas. There's little I could do to fix that, so on this issue the situation remains the same.
---
V8 -> V9: modify third patch to better reflect authorship by Valmantas V7 -> V8: add Valmantas Paliksa Signed-off-by to third patch V6 -> V7: drop RFC tag as per Heiko Stuebner's reminder, update cover letter
V5 -> V6: reflow to 75 cols, use 5.0GTs instead of Gen2 nomenclature, clarify strobe write adjustment and remove PHY_CFG_RD_MASK V4 -> V5: fix build failure, reflow commit messages and also convert registers for EP operation, all suggested by Ilpo
V3 -> V4: fix setting-up of TLS in Link Control and Status Register 2, also adjust commit titles
V2 -> V3: correctly clean-up with standard PCIe defines as per Bjorn's suggestion
V1 -> V2: use standard PCIe defines as suggested by Bjorn
Geraldo Nascimento (3):
PCI: rockchip: Use standard PCIe defines
PCI: rockchip: Set Target Link Speed before retraining
phy: rockchip-pcie: Properly disable TEST_WRITE strobe signal
Valmantas Paliksa (1):
phy: rockchip-pcie: Enable all four lanes if required
drivers/pci/controller/pcie-rockchip-ep.c | 4 +- drivers/pci/controller/pcie-rockchip-host.c | 48 +++++++++++---------- drivers/pci/controller/pcie-rockchip.h | 12 +----- drivers/phy/rockchip/phy-rockchip-pcie.c | 15 +++---- 4 files changed, 36 insertions(+), 43 deletions(-)
Signature · 1 line
2.49.0
PATCH v9 1/4 PCI: rockchip: Use standard PCIe defines
Current code uses custom-defined register offsets and bitfields for standard PCIe registers. Change to using standard PCIe defines. Since we are now using standard PCIe defines, drop unused custom-defined ones, which are now referenced from offset at added Capabilities Register.
--- drivers/pci/controller/pcie-rockchip-ep.c | 4 +- drivers/pci/controller/pcie-rockchip-host.c | 44 ++++++++++----------- drivers/pci/controller/pcie-rockchip.h | 12 +----- 3 files changed, 25 insertions(+), 35 deletions(-)
drivers/pci/controller/pcie-rockchip-ep.c +2 -2
@@ -518,9 +518,9 @@ static void rockchip_pcie_ep_retrain_link(struct rockchip_pcie *rockchip) {u32 status; - status =rockchip_pcie_read (rockchip,PCIE_EP_CONFIG_LCS ); + status =rockchip_pcie_read (rockchip,PCIE_EP_CONFIG_BASE +PCI_EXP_LNKCTL ); status |=PCI_EXP_LNKCTL_RL ; -rockchip_pcie_write (rockchip, status,PCIE_EP_CONFIG_LCS ); +rockchip_pcie_write (rockchip, status,PCIE_EP_CONFIG_BASE +PCI_EXP_LNKCTL ); }static bool rockchip_pcie_ep_link_up (struct rockchip_pcie *rockchip)
drivers/pci/controller/pcie-rockchip-host.c +22 -22
95 lines not on this page · raw message
drivers/pci/controller/pcie-rockchip.h +1 -11
@@ -155,17 +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_EP_CONFIG_LCS (PCIE_EP_CONFIG_BASE + 0xd0) +#define PCIE_RC_CONFIG_CR (PCIE_RC_CONFIG_BASE + 0xc0)#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)#define PCIE_RC_CONFIG_THP_CAP_NEXT_MASK GENMASK(31, 20)
Signature · 1 line
2.49.0
PATCH v9 2/4 PCI: rockchip: Set Target Link Speed before retraining
In reply to Geraldo NascimentoCurrent code may fail 5.0GT/s 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.
--- drivers/pci/controller/pcie-rockchip-host.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/pci/controller/pcie-rockchip-host.c +4 -0
@@ -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 );
Signature · 1 line
2.49.0
PATCH v9 3/4 phy: rockchip-pcie: Enable all four lanes if required
In reply to Geraldo NascimentoCurrent code enables only Lane 0 because pwr_cnt will be incremented on first call to the function. Let's reorder the enablement code to enable all 4 lanes through GRF.
--- drivers/phy/rockchip/phy-rockchip-pcie.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/phy/rockchip/phy-rockchip-pcie.c +6 -6
@@ -160,6 +160,12 @@ static int rockchip_pcie_phy_power_on(struct phy *phy)guard (mutex)(&rk_phy->pcie_mutex); +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)); +if (rk_phy->pwr_cnt++) {return 0 ; }
@@ -176,12 +182,6 @@ 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)); -/* * No documented timeout value for phy operation below, * so we make it large enough here. And we use loop-break
Signature · 1 line
2.49.0
PATCH v9 4/4 phy: rockchip-pcie: Properly disable TEST_WRITE strobe signal
In reply to Geraldo Nascimentopcie_conf is used to touch TEST_WRITE strobe signal. This signal should be enabled, a little time waited, and then disabled. Current code clearly was copy-pasted and never disables the strobe signal. Adjust the define. While at it, remove PHY_CFG_RD_MASK which has been unused since 64cdc0360811 ("phy: rockchip-pcie: remove unused phy_rd_cfg function").
--- drivers/phy/rockchip/phy-rockchip-pcie.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/phy/rockchip/phy-rockchip-pcie.c +1 -2
@@ -30,9 +30,8 @@#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_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
Signature · 1 line
2.49.0