PCI: rockchip: 5.0 GT/s speed discouraged by Rockchip
v7 cover PCI: rockchip: 5.0 GT/s speed discouraged by Rockchip
Dragan Simic already had warned me of potential issues with 5.0 GT/s speed operation in Rockchip PCIe. However, in recent interactions with Shawn Lin from Rockchip it came to my attention there's grave danger in the unknown errata regarding 5.0 GT/s operational speed of their PCIe core, including data loss.
Drop all code related to 5.0 GT/s operational speed from this driver.
Endpoint Mode driver was not tested.
Changes in v7:
- In pcie-rockchip.c (PATCH 3) drop unreachable code
- Link to v6: https://lore.kernel.org/all/[email protected]/T/
Changes in v6:
- Squashed patches 3 & 4 together as suggested by Mani
- More formal warning message as suggested by Dragan
- Set the 2.5 GT/s bit regardless of if link-speed is set to 2
- Link to v5: https://lore.kernel.org/all/[email protected]/T/
Changes in v5:
- Changed commit order to not break builds and adjusted copy pasted
commit message. (thanks Charalampos!)
- Reintroduced behavior to force 2.5 GT/s in case there's something
non-default in the DT. (thanks Dragan!)
- Link to v4: https://lore.kernel.org/linux-rockchip/[email protected]/T/
Changes in v4:
- Incorporate suggestion by Bjorn and refined by Dragan to drop the
"catastrophic" code
- Link to v3: https://lore.kernel.org/linux-rockchip/[email protected]/T/
Changes in v3:
- Clarify warning message even though Rockchip won't disclose details
- Drop DT changes as they were applied as subset by Heiko
- Link to v2: https://lore.kernel.org/all/[email protected]/T/
Changes in v2:
- hard limit to 2.5 GT/s, not just warn
- add Reported-by: and Reviewed-by: Dragan Simic
- remove redundant declaration of max-link-speed from helios64 dts
- fix Link: of helios64 patch
- simplify RC mode comment
- Link to v1: https://lore.kernel.org/all/aRhR79u5BPtRRFw3@geday/T/
Geraldo Nascimento (3):
PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
PCI: rockchip-host: do not attempt 5.0 GT/s retraining
PCI: rockchip: drive at 2.5 GT/s, error other speeds
drivers/pci/controller/pcie-rockchip-ep.c | 13 ------------- drivers/pci/controller/pcie-rockchip-host.c | 20 -------------------- drivers/pci/controller/pcie-rockchip.c | 14 ++++++-------- drivers/pci/controller/pcie-rockchip.h | 3 --- 4 files changed, 6 insertions(+), 44 deletions(-)
Signature · 1 line
2.54.0
PATCH v7 1/3 PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
Drop the 5.0 GT/s Link Speed retraining code block from Rockchip PCIe EP driver. The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0 GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
--- drivers/pci/controller/pcie-rockchip-ep.c | 13 ------------- 1 file changed, 13 deletions(-)
drivers/pci/controller/pcie-rockchip-ep.c +0 -13
@@ -553,19 +553,6 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)if (ret) goto again; -/* -* Check the current speed: if gen2 speed was requested and we are not -* at gen2 speed yet, retrain again for gen2. -*/ - val =rockchip_pcie_read (rockchip,PCIE_CORE_CTRL ); -if (!PCIE_LINK_IS_GEN2 (val) && rockchip->link_gen ==2 ) { -/* Enable retrain for gen2 */ -rockchip_pcie_ep_retrain_link (rockchip); -readl_poll_timeout (rockchip->apb_base +PCIE_CORE_CTRL , - val,PCIE_LINK_IS_GEN2 (val),50 , -LINK_TRAIN_TIMEOUT ); - } -/* Check again that the link is up */ if (!rockchip_pcie_ep_link_up (rockchip)) goto again;
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 1/3] PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 12 lines
Drop the 5.0 GT/s Link Speed retraining code block from Rockchip PCIe EP driver. The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0 GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
Quoted 7 lines
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
Reported-by Dragan Simic <[email protected]>Reported-by Shawn Lin <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH v7 1/3] PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
In reply to Geraldo NascimentoHello Geraldo,
Thanks for the v6 and v7 of this series.
Quoted 37 lines · Geraldo Nascimento
Drop the 5.0 GT/s Link Speed retraining code block from Rockchip PCIe EP driver. The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0 GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
Reported-by Dragan Simic <[email protected]>Reported-by Shawn Lin <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip-ep.c | 13 ------------- 1 file changed, 13 deletions(-)drivers/pci/controller/pcie-rockchip-ep.c +0
-6@@ -553,19 +553,6 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)if (ret) goto again; -/* -* Check the current speed: if gen2 speed was requested and we are not -* at gen2 speed yet, retrain again for gen2. -*/ - val =rockchip_pcie_read (rockchip,PCIE_CORE_CTRL ); -if (!PCIE_LINK_IS_GEN2 (val) && rockchip->link_gen ==2 ) {
drivers/pci/controller/pcie-rockchip-ep.c static void rockchip_pcie_ep_link_training(struct work_struct *work) · line 558
Quoted 10 lines
drivers/pci/controller/pcie-rockchip-ep.c +0
-7-/* Enable retrain for gen2 */ -rockchip_pcie_ep_retrain_link (rockchip); -readl_poll_timeout (rockchip->apb_base +PCIE_CORE_CTRL , - val,PCIE_LINK_IS_GEN2 (val),50 , -LINK_TRAIN_TIMEOUT ); - } -/* Check again that the link is up */ if (!rockchip_pcie_ep_link_up (rockchip)) goto again;
Looking good to me, so please feel free to include
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
PATCH v7 2/3 PCI: rockchip-host: do not attempt 5.0 GT/s retraining
In reply to Geraldo NascimentoDrop the 5.0 GT/s Link Speed retraining from Rockchip PCIe Root Complex Mode Operation, so called host driver.
The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
--- drivers/pci/controller/pcie-rockchip-host.c | 20 -------------------- 1 file changed, 20 deletions(-)
drivers/pci/controller/pcie-rockchip-host.c +0 -20
@@ -328,26 +328,6 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) goto err_power_off_phy; } -if (rockchip->link_gen ==2 ) { -/* -* 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 ); - - err =readl_poll_timeout (rockchip->apb_base +PCIE_CORE_CTRL , - status,PCIE_LINK_IS_GEN2 (status),20 , -500 *USEC_PER_MSEC ); -if (err) -dev_dbg (dev,"PCIe link training gen2 timeout, fall back to gen1!\n" ); - } -/* Check the final link width from negotiated lane counter from MGMT */ status =rockchip_pcie_read (rockchip,PCIE_CORE_CTRL ); status = 0x1 << ((status & PCIE_CORE_PL_CONF_LANE_MASK) >>
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 2/3] PCI: rockchip-host: do not attempt 5.0 GT/s retraining
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 14 lines
Drop the 5.0 GT/s Link Speed retraining from Rockchip PCIe Root Complex Mode Operation, so called host driver.
The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
Quoted 7 lines
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
Reported-by Dragan Simic <[email protected]>Reported-by Shawn Lin <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH v7 2/3] PCI: rockchip-host: do not attempt 5.0 GT/s retraining
In reply to Geraldo NascimentoHello Geraldo,
Thanks for the v6 and v7 of this series.
Quoted 46 lines · Geraldo Nascimento
Drop the 5.0 GT/s Link Speed retraining from Rockchip PCIe Root Complex Mode Operation, so called host driver.
The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
Reported-by Dragan Simic <[email protected]>Reported-by Shawn Lin <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip-host.c | 20 -------------------- 1 file changed, 20 deletions(-)drivers/pci/controller/pcie-rockchip-host.c +0
-13@@ -328,26 +328,6 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) goto err_power_off_phy; } -if (rockchip->link_gen ==2 ) { -/* -* 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 ); -
drivers/pci/controller/pcie-rockchip-host.c static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) · line 333
Quoted 10 lines
drivers/pci/controller/pcie-rockchip-host.c +0
-7- err =readl_poll_timeout (rockchip->apb_base +PCIE_CORE_CTRL , - status,PCIE_LINK_IS_GEN2 (status),20 , -500 *USEC_PER_MSEC ); -if (err) -dev_dbg (dev,"PCIe link training gen2 timeout, fall back to gen1!\n" ); - } -/* Check the final link width from negotiated lane counter from MGMT */ status =rockchip_pcie_read (rockchip,PCIE_CORE_CTRL ); status = 0x1 << ((status & PCIE_CORE_PL_CONF_LANE_MASK) >>
Looking good to me, so please feel free to include
PATCH v7 3/3 PCI: rockchip: drive at 2.5 GT/s, error other speeds
In reply to Geraldo NascimentoConfigure the core to be driven at 2.5 GT/s Link Speed and ignore any other speed with a warning. Also drop the 5.0 GT/s Link Speed defines from Rockchip PCIe header.
The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0 GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
--- drivers/pci/controller/pcie-rockchip.c | 14 ++++++-------- drivers/pci/controller/pcie-rockchip.h | 3 --- 2 files changed, 6 insertions(+), 11 deletions(-)
drivers/pci/controller/pcie-rockchip.c +6 -8
@@ -66,8 +66,10 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) } rockchip->link_gen =of_pci_get_max_link_speed (node); -if (rockchip->link_gen <0 || rockchip->link_gen >2 ) - rockchip->link_gen =2 ; +if (rockchip->link_gen <0 || rockchip->link_gen >=2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"invalid max-link-speed, limited to 2.5 GT/s\n" ); + }for (i =0 ; i <ROCKCHIP_NUM_PM_RSTS ; i++) rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
@@ -147,12 +149,8 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) goto err_exit_phy; } -if (rockchip->link_gen ==2 ) -rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_2 , -PCIE_CLIENT_CONFIG ); -else -rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_1 , -PCIE_CLIENT_CONFIG ); +rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_1 , +PCIE_CLIENT_CONFIG ); regs =PCIE_CLIENT_ARI_ENABLE |PCIE_CLIENT_CONF_LANE_NUM (rockchip->lanes);
drivers/pci/controller/pcie-rockchip.h +0 -3
@@ -42,7 +42,6 @@#define PCIE_CLIENT_MODE_RC HWORD_SET_BIT(0x0040)#define PCIE_CLIENT_MODE_EP HWORD_CLR_BIT(0x0040)#define PCIE_CLIENT_GEN_SEL_1 HWORD_CLR_BIT(0x0080) -#define PCIE_CLIENT_GEN_SEL_2 HWORD_SET_BIT(0x0080)#define PCIE_CLIENT_LEGACY_INT_CTRL (PCIE_CLIENT_BASE + 0x0c)#define PCIE_CLIENT_INT_IN_ASSERT HWORD_SET_BIT(0x0002)#define PCIE_CLIENT_INT_IN_DEASSERT HWORD_CLR_BIT(0x0002)
@@ -197,8 +196,6 @@ (((x) & PCIE_CORE_PL_CONF_LS_MASK) == PCIE_CORE_PL_CONF_LS_READY)#define PCIE_LINK_UP (x) \ (((x) & PCIE_CLIENT_LINK_STATUS_MASK) == PCIE_CLIENT_LINK_STATUS_UP) -#define PCIE_LINK_IS_GEN2 (x) \ - (((x) & PCIE_CORE_PL_CONF_SPEED_MASK) == PCIE_CORE_PL_CONF_SPEED_5G)#define RC_REGION_0_ADDR_TRANS_H 0x00000000#define RC_REGION_0_ADDR_TRANS_L 0x00000000
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v7 3/3] PCI: rockchip: drive at 2.5 GT/s, error other speeds
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Spurious warning emitted when the optional 'max-link-speed' device tree property is absent.…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Spurious warning emitted when the optional 'max-link-speed' device tree property is absent.
- [Low] The parsed max-link-speed value stored in rockchip->link_gen is dead code and entirely ignored during link initialization. --
commit 3b268bac2a052935f203b8d3ae0eea857d63f07b
Author: Geraldo Nascimento <[email protected]>
PCI: rockchip: drive at 2.5 GT/s, error other speeds
This commit configures the Rockchip PCIe core to drive at a maximum link speed of 2.5 GT/s to prevent potential data loss or hardware issues. It ignores other configured speeds with a warning and removes 5.0 GT/s defines.
Quoted 5 lines
drivers/pci/controller/pcie-rockchip.c +0
-0
drivers/pci/controller/pcie-rockchip.c int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) · line 72
Quoted 9 lines
drivers/pci/controller/pcie-rockchip.c +4
-2@@ -66,8 +66,10 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) } rockchip->link_gen =of_pci_get_max_link_speed (node); -if (rockchip->link_gen <0 || rockchip->link_gen >2 ) - rockchip->link_gen =2 ; +if (rockchip->link_gen <0 || rockchip->link_gen >=2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"invalid max-link-speed, limited to 2.5 GT/s\n" ); + }
[Severity: Medium]
Since the max-link-speed device tree property is optional, of_pci_get_max_link_speed() will return a negative error code such as -EINVAL when it is absent. Does this mean the rockchip->link_gen < 0 check will cause a spurious warning on every boot for perfectly valid device trees that omit this property?
Quoted 3 lines
drivers/pci/controller/pcie-rockchip.c +0
-0for (i =0 ; i <ROCKCHIP_NUM_PM_RSTS ; i++) rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];
[ ... ]
drivers/pci/controller/pcie-rockchip.c int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) · line 153
Quoted 12 lines
drivers/pci/controller/pcie-rockchip.c +2
-6@@ -147,12 +149,8 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) goto err_exit_phy; } -if (rockchip->link_gen ==2 ) -rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_2 , -PCIE_CLIENT_CONFIG ); -else -rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_1 , -PCIE_CLIENT_CONFIG ); +rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_1 , +PCIE_CLIENT_CONFIG );
[Severity: Low]
Does this unconditional write make the rockchip->link_gen value parsed in rockchip_pcie_parse_dt() obsolete? If link_gen is no longer used during link initialization, could the device tree parsing logic and state be removed entirely to simplify the code?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Re: [PATCH v7 3/3] PCI: rockchip: drive at 2.5 GT/s, error other speeds
In reply to Geraldo NascimentoHello Geraldo,
Thanks for the v6 and v7 of this series.
Quoted 73 lines · Geraldo Nascimento
Configure the core to be driven at 2.5 GT/s Link Speed and ignore any other speed with a warning. Also drop the 5.0 GT/s Link Speed defines from Rockchip PCIe header.
The reason is that Shawn Lin from Rockchip has reiterated that there may be danger of "catastrophic failure" in using their PCIe with 5.0 GT/s speeds.
While Rockchip has done so informally without issuing a proper errata, and the particulars are thus unknown, this may cause data loss or worse.
This change is corroborated by RK3399 official datasheet [1], which states maximum link speed for this platform is 2.5 GT/s.
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
Fixes 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")Reported-by Dragan Simic <[email protected]>Reported-by Shawn Lin <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.c | 14 ++++++-------- drivers/pci/controller/pcie-rockchip.h | 3 --- 2 files changed, 6 insertions(+), 11 deletions(-)drivers/pci/controller/pcie-rockchip.c +6
-8@@ -66,8 +66,10 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) } rockchip->link_gen =of_pci_get_max_link_speed (node); -if (rockchip->link_gen <0 || rockchip->link_gen >2 ) - rockchip->link_gen =2 ; +if (rockchip->link_gen <0 || rockchip->link_gen >=2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"invalid max-link-speed, limited to 2.5 GT/s\n" ); + }for (i =0 ; i <ROCKCHIP_NUM_PM_RSTS ; i++) rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i];@@ -147,12 +149,8 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) goto err_exit_phy; } -if (rockchip->link_gen ==2 ) -rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_2 , -PCIE_CLIENT_CONFIG ); -else -rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_1 , -PCIE_CLIENT_CONFIG ); +rockchip_pcie_write (rockchip,PCIE_CLIENT_GEN_SEL_1 , +PCIE_CLIENT_CONFIG ); regs =PCIE_CLIENT_ARI_ENABLE |PCIE_CLIENT_CONF_LANE_NUM (rockchip->lanes);drivers/pci/controller/pcie-rockchip.h +0
-1@@ -42,7 +42,6 @@#define PCIE_CLIENT_MODE_RC HWORD_SET_BIT(0x0040)#define PCIE_CLIENT_MODE_EP HWORD_CLR_BIT(0x0040)#define PCIE_CLIENT_GEN_SEL_1 HWORD_CLR_BIT(0x0080) -#define PCIE_CLIENT_GEN_SEL_2 HWORD_SET_BIT(0x0080)#define PCIE_CLIENT_LEGACY_INT_CTRL (PCIE_CLIENT_BASE + 0x0c)#define PCIE_CLIENT_INT_IN_ASSERT HWORD_SET_BIT(0x0002)#define PCIE_CLIENT_INT_IN_DEASSERT HWORD_CLR_BIT(0x0002)
drivers/pci/controller/pcie-rockchip.h line 201
Quoted 8 lines
drivers/pci/controller/pcie-rockchip.h +0
-2@@ -197,8 +196,6 @@ (((x) & PCIE_CORE_PL_CONF_LS_MASK) == PCIE_CORE_PL_CONF_LS_READY)#define PCIE_LINK_UP (x) \ (((x) & PCIE_CLIENT_LINK_STATUS_MASK) == PCIE_CLIENT_LINK_STATUS_UP) -#define PCIE_LINK_IS_GEN2 (x) \ - (((x) & PCIE_CORE_PL_CONF_SPEED_MASK) == PCIE_CORE_PL_CONF_SPEED_5G)#define RC_REGION_0_ADDR_TRANS_H 0x00000000#define RC_REGION_0_ADDR_TRANS_L 0x00000000
Looking good to me, so please feel free to include
Hi everyone,
a question to PCI maintainers:
It's been two months already since v7 with no response anymore from you guys. Is there a specific thing you're waiting, like an ACK from Shawn Lin? Or perhaps this has been forgotten about?
Thanks,
Geraldo Nascimento
Quoted 60 lines · Geraldo Nascimento
Dragan Simic already had warned me of potential issues with 5.0 GT/s speed operation in Rockchip PCIe. However, in recent interactions with Shawn Lin from Rockchip it came to my attention there's grave danger in the unknown errata regarding 5.0 GT/s operational speed of their PCIe core, including data loss.
Drop all code related to 5.0 GT/s operational speed from this driver.
Endpoint Mode driver was not tested.
Changes in v7:
- Inpcie-rockchip.c(PATCH 3) drop unreachable code
- Link to v6: https://lore.kernel.org/all/[email protected]/T/Changes in v6:
- Squashed patches 3 & 4 together as suggested by Mani
- More formal warning message as suggested by Dragan
- Set the 2.5 GT/s bit regardless of if link-speed is set to 2
- Link to v5: https://lore.kernel.org/all/[email protected]/T/Changes in v5:
- Changed commit order to not break builds and adjusted copy pasted
commit message. (thanks Charalampos!)
- Reintroduced behavior to force 2.5 GT/s in case there's something
non-default in the DT. (thanks Dragan!)
- Link to v4: https://lore.kernel.org/linux-rockchip/[email protected]/T/Changes in v4:
- Incorporate suggestion by Bjorn and refined by Dragan to drop the
"catastrophic" code
- Link to v3: https://lore.kernel.org/linux-rockchip/[email protected]/T/Changes in v3:
- Clarify warning message even though Rockchip won't disclose details
- Drop DT changes as they were applied as subset by Heiko
- Link to v2: https://lore.kernel.org/all/[email protected]/T/Changes in v2:
- hard limit to 2.5 GT/s, not just warn
- add Reported-by: and Reviewed-by: Dragan Simic
- remove redundant declaration of max-link-speed from helios64 dts
- fix Link: of helios64 patch
- simplify RC mode comment
- Link to v1: https://lore.kernel.org/all/aRhR79u5BPtRRFw3@geday/T/Geraldo Nascimento (3):
PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
PCI: rockchip-host: do not attempt 5.0 GT/s retraining
PCI: rockchip: drive at 2.5 GT/s, error other speedsdrivers/pci/controller/pcie-rockchip-ep.c | 13 ------------- drivers/pci/controller/pcie-rockchip-host.c | 20 -------------------- drivers/pci/controller/pcie-rockchip.c | 14 ++++++-------- drivers/pci/controller/pcie-rockchip.h | 3 --- 4 files changed, 6 insertions(+), 44 deletions(-)--
2.54.0
Hi Geraldo
Quoted 8 lines · Geraldo Nascimento
Dragan Simic already had warned me of potential issues with 5.0 GT/s speed operation in Rockchip PCIe. However, in recent interactions with Shawn Lin from Rockchip it came to my attention there's grave danger in the unknown errata regarding 5.0 GT/s operational speed of their PCIe core, including data loss.
Drop all code related to 5.0 GT/s operational speed from this driver.
Apologies for the very late notice that this series is still pending. FWIW,
Quoted 42 lines
Endpoint Mode driver was not tested.
Changes in v7:
- Inpcie-rockchip.c(PATCH 3) drop unreachable code
- Link to v6: https://lore.kernel.org/all/[email protected]/T/Changes in v6:
- Squashed patches 3 & 4 together as suggested by Mani
- More formal warning message as suggested by Dragan
- Set the 2.5 GT/s bit regardless of if link-speed is set to 2
- Link to v5: https://lore.kernel.org/all/[email protected]/T/Changes in v5:
- Changed commit order to not break builds and adjusted copy pasted
commit message. (thanks Charalampos!)
- Reintroduced behavior to force 2.5 GT/s in case there's something
non-default in the DT. (thanks Dragan!)
- Link to v4: https://lore.kernel.org/linux-rockchip/[email protected]/T/Changes in v4:
- Incorporate suggestion by Bjorn and refined by Dragan to drop the
"catastrophic" code
- Link to v3: https://lore.kernel.org/linux-rockchip/[email protected]/T/Changes in v3:
- Clarify warning message even though Rockchip won't disclose details
- Drop DT changes as they were applied as subset by Heiko
- Link to v2: https://lore.kernel.org/all/[email protected]/T/Changes in v2:
- hard limit to 2.5 GT/s, not just warn
- add Reported-by: and Reviewed-by: Dragan Simic
- remove redundant declaration of max-link-speed from helios64 dts
- fix Link: of helios64 patch
- simplify RC mode comment
- Link to v1: https://lore.kernel.org/all/aRhR79u5BPtRRFw3@geday/T/Geraldo Nascimento (3):
PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
PCI: rockchip-host: do not attempt 5.0 GT/s retraining
PCI: rockchip: drive at 2.5 GT/s, error other speeds
Quoted 6 lines
drivers/pci/controller/pcie-rockchip-ep.c | 13 ------------- drivers/pci/controller/pcie-rockchip-host.c | 20 -------------------- drivers/pci/controller/pcie-rockchip.c | 14 ++++++-------- drivers/pci/controller/pcie-rockchip.h | 3 --- 4 files changed, 6 insertions(+), 44 deletions(-)
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
Quoted 9 lines · Geraldo Nascimento
PCI: rockchip: 5.0 GT/s speed discouraged by Rockchip
Dragan Simic already had warned me of potential issues with 5.0 GT/s speed operation in Rockchip PCIe. However, in recent interactions with Shawn Lin from Rockchip it came to my attention there's grave danger in the unknown errata regarding 5.0 GT/s operational speed of their PCIe core, including data loss.
[...]
Applied, thanks!
[1/3] PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
commit: 39f89dde0a76b229df7eb356aaf6f444b4f8a636
[2/3] PCI: rockchip-host: do not attempt 5.0 GT/s retraining
commit: 5988a53f5bf758b2d41cb6e0832c19b9692c76d2
[3/3] PCI: rockchip: drive at 2.5 GT/s, error other speeds
commit: 62843031ca1f11d1b8e90e7f558c1b3c4cd77184Best regards,
Signature · 1 line
மணிவண்ணன் சதாசிவம்
Hi Mani,
Quoted 11 lines · Manivannan Sadhasivam
Quoted 8 lines · Geraldo Nascimento
PCI: rockchip: 5.0 GT/s speed discouraged by Rockchip
Dragan Simic already had warned me of potential issues with 5.0 GT/s speed operation in Rockchip PCIe. However, in recent interactions with Shawn Lin from Rockchip it came to my attention there's grave danger in the unknown errata regarding 5.0 GT/s operational speed of their PCIe core, including data loss.
Quoted 3 lines
Quoted 1 line
[...]
Applied, thanks!
Thank you Mani, that was an important drop.
It's a little strange I guess to have patches that were Reported-by and then Reviewed-by the same people. But in this case Dragan and Shawn deserve all the credit, I'm just the person who dropped some code.
Regards,
Geraldo Nascimento
Quoted 13 lines
[1/3] PCI: rockchip-ep: do not attempt 5.0 GT/s retraining
commit: 39f89dde0a76b229df7eb356aaf6f444b4f8a636 [2/3] PCI: rockchip-host: do not attempt 5.0 GT/s retraining commit: 5988a53f5bf758b2d41cb6e0832c19b9692c76d2 [3/3] PCI: rockchip: drive at 2.5 GT/s, error other speeds commit: 62843031ca1f11d1b8e90e7f558c1b3c4cd77184Best regards,
--
மணிவண்ணன் சதாசிவம்