PCI: rockchip: Use standard PCIe defines
Subsystems (5)
A message that is not in the archive (sent off-list or not yet received): [email protected]
RFC PATCH v4 1/5 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.
--- drivers/pci/controller/pcie-rockchip-host.c | 44 ++++++++++----------- 1 file changed, 22 insertions(+), 22 deletions(-)
drivers/pci/controller/pcie-rockchip-host.c +22 -22
@@ -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:
Signature · 1 line
2.49.0
RFC PATCH v4 2/5 PCI: rockchip: Drop unused custom registers and bitfields
In reply to a message not in the archiveSince 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.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
drivers/pci/controller/pcie-rockchip.h +1 -10
@@ -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)
Signature · 1 line
2.49.0
Re: [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields
Quoted 3 lines · Geraldo Nascimento
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.
Quoted 15 lines
Suggested-By Bjorn Helgaas <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)drivers/pci/controller/pcie-rockchip.h +0
-1@@ -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)
drivers/pci/controller/pcie-rockchip.h line 158
Quoted 10 lines
drivers/pci/controller/pcie-rockchip.h +1
-9-#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.
drivers/pci/controller/pcie-rockchip.h line 159
Quoted 1 line
drivers/pci/controller/pcie-rockchip.h +0
-0#define PCIE_EP_CONFIG_LCS (PCIE_EP_CONFIG_BASE + 0xd0)
Aren't you going to convert this as well?
Quoted 3 lines
drivers/pci/controller/pcie-rockchip.h +0
-0#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
Signature · 1 line
i.
Re: [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields
Quoted 8 lines · Ilpo Järvinen
Quoted 3 lines · Geraldo Nascimento
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.
Quoted 23 lines
Quoted 23 lines
Suggested-By Bjorn Helgaas <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)drivers/pci/controller/pcie-rockchip.h +0
-8@@ -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)
Quoted 8 lines
drivers/pci/controller/pcie-rockchip.h line 158
Quoted 3 lines
drivers/pci/controller/pcie-rockchip.h +1
-2-#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_CRis 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.
Quoted 4 lines
drivers/pci/controller/pcie-rockchip.h line 159
Quoted 2 lines
drivers/pci/controller/pcie-rockchip.h +0
-0#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
Quoted 8 lines
Quoted 4 lines
drivers/pci/controller/pcie-rockchip.h +0
-0#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)Signature · 1 line
i.
Re: [RFC PATCH v4 2/5] PCI: rockchip: Drop unused custom registers and bitfields
Quoted 49 lines · Geraldo Nascimento
Quoted 8 lines · Ilpo Järvinen
Quoted 3 lines · Geraldo Nascimento
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.
Quoted 23 lines
Quoted 23 lines
Suggested-By Bjorn Helgaas <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)drivers/pci/controller/pcie-rockchip.h +0
-8@@ -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)Quoted 8 lines
drivers/pci/controller/pcie-rockchip.h line 158
Quoted 3 lines
drivers/pci/controller/pcie-rockchip.h +1
-2-#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_CRis 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.
Quoted 1 line
Quoted 6 lines
Quoted 3 lines
drivers/pci/controller/pcie-rockchip.h line 159
Quoted 1 line
drivers/pci/controller/pcie-rockchip.h +0
-0#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.
Signature · 1 line
i.
RFC PATCH v4 3/5 PCI: rockchip: Set Target Link Speed before retraining
In reply to a message not in the archiveCurrent 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.
--- 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
RFC PATCH v4 4/5 phy: rockchip-pcie: Enable all four lanes
In reply to a message not in the archiveCurrent 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.
--- drivers/phy/rockchip/phy-rockchip-pcie.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/phy/rockchip/phy-rockchip-pcie.c +7 -5
@@ -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,
Signature · 1 line
2.49.0
RFC PATCH v4 5/5 phy: rockchip-pcie: Adjust read mask and write
In reply to a message not in the archiveSection 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.
--- drivers/phy/rockchip/phy-rockchip-pcie.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/phy/rockchip/phy-rockchip-pcie.c +2 -2
@@ -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
Signature · 1 line
2.49.0