PCI: rockchip: 5.0 GT/s speed may be dangerous
Subsystems (5)
v2 cover PCI: rockchip: 5.0 GT/s speed may be dangerous
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. Even if the odds are low, to contain any damage, let's cover the remaining corner-cases where the default would lead to 5.0 GT/s operation as well as add a comment to Root Complex driver core, documenting this danger. Furthermore, remove redundant declaration of max-link-speed from rk3399-nanopi-r4s.dtsi
---
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/
Geraldo Nascimento (4):
PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
PCI: rockchip-host: comment danger of 5.0 GT/s speed
arm64: dts: rockchip: remove dangerous max-link-speed from helios64
arm64: dts: rockchip: remove redundant max-link-speed from nanopi-r4s
arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 - arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dtsi | 1 - drivers/pci/controller/pcie-rockchip-host.c | 3 +++ drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 4 files changed, 11 insertions(+), 4 deletions(-)
Signature · 1 line
2.49.0
PATCH v2 1/4 PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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 | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/pci/controller/pcie-rockchip.c +8 -2
@@ -66,8 +66,14 @@ 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, set to 2.5 GT/s\n" ); + } +else if (rockchip->link_gen ==2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"5.0 GT/s is dangerous, set 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];
Signature · 1 line
2.49.0
Manivannan Sadhasivam
maintainerRe: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Quoted 28 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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]>Reviewed-by Dragan Simic <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/pci/controller/pcie-rockchip.c +0
-0@@ -66,8 +66,14 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) }
drivers/pci/controller/pcie-rockchip.c int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) · line 75
Quoted 10 lines
drivers/pci/controller/pcie-rockchip.c +7
-2rockchip->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, set to 2.5 GT/s\n" ); + } +else if (rockchip->link_gen ==2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"5.0 GT/s is dangerous, set to 2.5 GT/s\n" );
What does 'danger' really mean here? Link instability or something else?
Error messages should be precise and not fearmongering.
- Mani
Signature · 1 line
மணிவண்ணன் சதாசிவம்
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Heello Manivannan and Geraldo,
Quoted 29 lines · Manivannan Sadhasivam
Quoted 28 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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]>Reviewed-by Dragan Simic <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/pci/controller/pcie-rockchip.c +0
-0@@ -66,8 +66,14 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) }
Quoted 13 lines
drivers/pci/controller/pcie-rockchip.c int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) · line 75
Quoted 10 lines
drivers/pci/controller/pcie-rockchip.c +7
-2rockchip->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, set to 2.5 GT/s\n" ); + } +else if (rockchip->link_gen ==2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"5.0 GT/s is dangerous, set to 2.5 GT/s\n" );What does 'danger' really mean here? Link instability or something else? Error messages should be precise and not fearmongering.
I agree that the original wording is a bit suboptimal, and I'd suggest to Geraldo that the produced warning message is changed to
"5.0 GT/s may cause data corruption, limited to to 2.5 GT/s\n"
or something similar, to better reflect the actual underlying issue.
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Quoted 46 lines · Dragan Simic
Heello Manivannan and Geraldo,
Quoted 29 lines · Manivannan Sadhasivam
Quoted 28 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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]>Reviewed-by Dragan Simic <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/pci/controller/pcie-rockchip.c +0
-0@@ -66,8 +66,14 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) }Quoted 13 lines
drivers/pci/controller/pcie-rockchip.c int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) · line 75
Quoted 10 lines
drivers/pci/controller/pcie-rockchip.c +7
-2rockchip->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, set to 2.5 GT/s\n" ); + } +else if (rockchip->link_gen ==2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"5.0 GT/s is dangerous, set to 2.5 GT/s\n" );What does 'danger' really mean here? Link instability or something else? Error messages should be precise and not fearmongering.
Quoted 6 lines
I agree that the original wording is a bit suboptimal, and I'd suggest to Geraldo that the produced warning message is changed to
"5.0 GT/s may cause data corruption, limited to to 2.5 GT/s\n"
or something similar, to better reflect the actual underlying issue.
s/limited to to/therefore limit speed to/ ?
Cheers,
Diederik
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Hello Diederik,
Quoted 45 lines · Diederik de Haas
Quoted 44 lines · Dragan Simic
Quoted 42 lines · Manivannan Sadhasivam
Quoted 28 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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]>Reviewed-by Dragan Simic <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/pci/controller/pcie-rockchip.c +0
-0@@ -66,8 +66,14 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) }drivers/pci/controller/pcie-rockchip.c int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) · line 75
Quoted 10 lines
drivers/pci/controller/pcie-rockchip.c +7
-2rockchip->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, set to 2.5 GT/s\n" ); + } +else if (rockchip->link_gen ==2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"5.0 GT/s is dangerous, set to 2.5 GT/s\n" );What does 'danger' really mean here? Link instability or something else? Error messages should be precise and not fearmongering.
Quoted 8 lines
Quoted 6 lines
I agree that the original wording is a bit suboptimal, and I'd suggest to Geraldo that the produced warning message is changed to
"5.0 GT/s may cause data corruption, limited to to 2.5 GT/s\n"
or something similar, to better reflect the actual underlying issue.
s/limited to to/therefore limit speed to/ ?
That would work well in a book or an article, while slightly terse wording is usually preferred in the messages produced by the kernel, or in log messages in general. Such an approach compacts as much information as possible in as few words as possible, while still remaining (mostly) grammatically correct.
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Hello Mani, Dragan, Diederik, and all,
I'm currently facing banking issues in Brazil which means I'm refraining from fetching my email through POP3 with mutt to do proper kernel development.
I usually fetch my mail through POP3 and have the server delete it, to save space on my free tier.
But I found out I have a 20-year old bank account open that I had forgot about, and with this bureaucracy hell I can't have the server delete the messages for now.
I'll send v3 with more proper and illustrative wording once I get this done.
Thanks,
Geraldo Nascimento
Quoted 64 lines · Dragan Simic
Hello Diederik,
Quoted 45 lines · Diederik de Haas
Quoted 44 lines · Dragan Simic
Quoted 42 lines · Manivannan Sadhasivam
Quoted 28 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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]>Reviewed-by Dragan Simic <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/pci/controller/pcie-rockchip.c +0
-0@@ -66,8 +66,14 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) }drivers/pci/controller/pcie-rockchip.c int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) · line 75
Quoted 10 lines
drivers/pci/controller/pcie-rockchip.c +7
-2rockchip->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, set to 2.5 GT/s\n" ); + } +else if (rockchip->link_gen ==2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"5.0 GT/s is dangerous, set to 2.5 GT/s\n" );What does 'danger' really mean here? Link instability or something else? Error messages should be precise and not fearmongering.
Quoted 8 lines
Quoted 6 lines
I agree that the original wording is a bit suboptimal, and I'd suggest to Geraldo that the produced warning message is changed to
"5.0 GT/s may cause data corruption, limited to to 2.5 GT/s\n"
or something similar, to better reflect the actual underlying issue.
s/limited to to/therefore limit speed to/ ?
That would work well in a book or an article, while slightly terse wording is usually preferred in the messages produced by the kernel, or in log messages in general. Such an approach compacts as much information as possible in as few words as possible, while still remaining (mostly) grammatically correct.
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
In reply to Geraldo NascimentoHi Geraldo,
Quoted 11 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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
To accurately determine the operating speed, we can leverage the PCIE_CLIENT_BASIC_STATUS0/1 fields.
This provides a dynamic mechanism to resolve the issue.
[1] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-rockchip-ep.c#L533-L595
Thanks
-Anand
Quoted 36 lines
Fixes 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")Reported-by Dragan Simic <[email protected]>Reported-by Shawn Lin <[email protected]>Reviewed-by Dragan Simic <[email protected]>Signed-off-by Geraldo Nascimento <[email protected]>--- drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/pci/controller/pcie-rockchip.c +8
-3@@ -66,8 +66,14 @@ 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, set to 2.5 GT/s\n" ); + } +else if (rockchip->link_gen ==2 ) { + rockchip->link_gen =1 ; +dev_warn (dev,"5.0 GT/s is dangerous, set 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]; --2.49.0
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Quoted 16 lines · Anand Moon
Hi Geraldo,
Quoted 11 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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
Quoted 8 lines
To accurately determine the operating speed, we can leverage the PCIE_CLIENT_BASIC_STATUS0/1 fields.
This provides a dynamic mechanism to resolve the issue.[1] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-rockchip-ep.c#L533-L595
Thanks
-Anand
Hi Anand,
not to put you down but I think your approach adds unnecessary complexity.
All I care really is that the Kernel Project isn't blamed in the future if someone happens to lose their data.
Thanks,
Geraldo Nascimento
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Hi Geraldo,
Quoted 27 lines · Geraldo Nascimento
Quoted 16 lines · Anand Moon
Hi Geraldo,
Quoted 11 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says 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
Quoted 8 lines
To accurately determine the operating speed, we can leverage the PCIE_CLIENT_BASIC_STATUS0/1 fields.
This provides a dynamic mechanism to resolve the issue.[1] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-rockchip-ep.c#L533-L595
Thanks
-Anand
Quoted 7 lines
Hi Anand,
not to put you down but I think your approach adds unnecessary complexity.
All I care really is that the Kernel Project isn't blamed in the future if someone happens to lose their data.
Allow the hardware to negotiate the link speed based on the available number of lanes.
I don’t anticipate any data loss, since PCIe will automatically configure the device speed
with link training..
Quoted 2 lines
Thanks,
Geraldo Nascimento
Thanks
-Anand
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Hello Anand,
Quoted 23 lines · Anand Moon
Quoted 21 lines · Geraldo Nascimento
Quoted 10 lines · Anand Moon
Quoted 8 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says maximum link speed for this platform is 2.5 GT/s.
Quoted 7 lines
Quoted 2 lines
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
To accurately determine the operating speed, we can leverage the PCIE_CLIENT_BASIC_STATUS0/1 fields.
This provides a dynamic mechanism to resolve the issue.[1] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-rockchip-ep.c#L533-L595
not to put you down but I think your approach adds unnecessary complexity.
Quoted 7 lines
Quoted 3 lines
All I care really is that the Kernel Project isn't blamed in the future if someone happens to lose their data.
Allow the hardware to negotiate the link speed based on the available number of lanes.
I don’t anticipate any data loss, since PCIe will automatically configure the device speed with link training..
Please, note that this isn't about performing auto negotiation and following its results, but about "artificially" limiting the PCIe link speed to 2.5 GT/s on RK3399, because it's well known by Rockchip that 5 GT/s on RK3399's PCIe interface may cause issues and data corruption in certain corner cases.
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Hi Dragan,
Quoted 35 lines · Dragan Simic
Hello Anand,
Quoted 23 lines · Anand Moon
Quoted 21 lines · Geraldo Nascimento
Quoted 10 lines · Anand Moon
Quoted 8 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says maximum link speed for this platform is 2.5 GT/s.
Quoted 7 lines
Quoted 2 lines
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
To accurately determine the operating speed, we can leverage the PCIE_CLIENT_BASIC_STATUS0/1 fields.
This provides a dynamic mechanism to resolve the issue.[1] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-rockchip-ep.c#L533-L595
not to put you down but I think your approach adds unnecessary complexity.
Quoted 7 lines
Quoted 3 lines
All I care really is that the Kernel Project isn't blamed in the future if someone happens to lose their data.
Allow the hardware to negotiate the link speed based on the available number of lanes.
I don’t anticipate any data loss, since PCIe will automatically configure the device speed with link training..
Quoted 6 lines
Please, note that this isn't about performing auto negotiation and following its results, but about "artificially" limiting the PCIe link speed to 2.5 GT/s on RK3399, because it's well known by Rockchip that 5 GT/s on RK3399's PCIe interface may cause issues and data corruption in certain corner cases.
It’s possible the link speed wasn’t properly tuned. On my older development board,
which supports this configuration, I haven’t observed any data loss.
sudo lspci -vvv | grep Speed
LnkCap: Port #0, Speed 5GT/s, Width x1, ASPM L1, Exit
Latency L1 <8us
LnkSta: Speed 5GT/s, Width x1
LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
LnkCap: Port #1, Speed 5GT/s, Width x1, ASPM L0s L1,
Exit Latency L0s unlimited, L1 <2us
LnkSta: Speed 5GT/s, Width x1
LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
Thanks
-Anand
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Quoted 43 lines · Anand Moon
Quoted 32 lines · Dragan Simic
Quoted 23 lines · Anand Moon
Quoted 21 lines · Geraldo Nascimento
Quoted 10 lines · Anand Moon
Quoted 8 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says maximum link speed for this platform is 2.5 GT/s.
Quoted 7 lines
Quoted 2 lines
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
To accurately determine the operating speed, we can leverage the PCIE_CLIENT_BASIC_STATUS0/1 fields.
This provides a dynamic mechanism to resolve the issue.[1] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-rockchip-ep.c#L533-L595
not to put you down but I think your approach adds unnecessary complexity.
Quoted 7 lines
Quoted 3 lines
All I care really is that the Kernel Project isn't blamed in the future if someone happens to lose their data.
Allow the hardware to negotiate the link speed based on the available number of lanes.
I don’t anticipate any data loss, since PCIe will automatically configure the device speed with link training..Quoted 6 lines
Please, note that this isn't about performing auto negotiation and following its results, but about "artificially" limiting the PCIe link speed to 2.5 GT/s on RK3399, because it's well known by Rockchip that 5 GT/s on RK3399's PCIe interface may cause issues and data corruption in certain corner cases.
It’s possible the link speed wasn’t properly tuned. On my older development board,
which supports this configuration, I haven’t observed any data loss.
Quoted 9 lines
sudo lspci -vvv | grep Speed
LnkCap: Port #0, Speed 5GT/s, Width x1, ASPM L1, Exit
Latency L1 <8us
LnkSta: Speed 5GT/s, Width x1
LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
LnkCap: Port #1, Speed 5GT/s, Width x1, ASPM L0s L1,
Exit Latency L0s unlimited, L1 <2us
LnkSta: Speed 5GT/s, Width x1
LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
Let me clarify, please... This limitation to 2.5 GT/s came straight from Rockchip a few years ago, described back then as an undisclosed errata. Recently, we got some more details from Rockchip that confirmed 5 GT/s as having issues in certain corner cases that cannot be validated by performing some field tests or by observing the PCIe behavior under load. Those corner cases with 5 GT/s, as described by Rockchip, are quite hard to reach, but the possibility is still real.
To sum it up, yes, multiple people have reported 5 GT/s as "working for me" on their RK3399-based boards and devices, but that unfortunately means nothing in this case, due to the specific nature of the underlying issue.
Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
Quoted 63 lines · Dragan Simic
Quoted 43 lines · Anand Moon
Quoted 32 lines · Dragan Simic
Quoted 23 lines · Anand Moon
Quoted 21 lines · Geraldo Nascimento
Quoted 10 lines · Anand Moon
Quoted 8 lines · Geraldo Nascimento
Shawn Lin from Rockchip has reiterated that there may be danger in using their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change from the default and drive at 2.5 GT/s only, even if the DT max-link-speed property is invalid or inexistent.
This change is corroborated by RK3399 official datasheet [1], which says maximum link speed for this platform is 2.5 GT/s.
Quoted 7 lines
Quoted 2 lines
[1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
To accurately determine the operating speed, we can leverage the PCIE_CLIENT_BASIC_STATUS0/1 fields.
This provides a dynamic mechanism to resolve the issue.[1] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-rockchip-ep.c#L533-L595
not to put you down but I think your approach adds unnecessary complexity.
Quoted 7 lines
Quoted 3 lines
All I care really is that the Kernel Project isn't blamed in the future if someone happens to lose their data.
Allow the hardware to negotiate the link speed based on the available number of lanes.
I don’t anticipate any data loss, since PCIe will automatically configure the device speed with link training..Quoted 6 lines
Please, note that this isn't about performing auto negotiation and following its results, but about "artificially" limiting the PCIe link speed to 2.5 GT/s on RK3399, because it's well known by Rockchip that 5 GT/s on RK3399's PCIe interface may cause issues and data corruption in certain corner cases.
It’s possible the link speed wasn’t properly tuned. On my older development board,
which supports this configuration, I haven’t observed any data loss.Quoted 9 lines
sudo lspci -vvv | grep Speed
LnkCap: Port #0, Speed 5GT/s, Width x1, ASPM L1, Exit
Latency L1 <8us
LnkSta: Speed 5GT/s, Width x1
LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
LnkCap: Port #1, Speed 5GT/s, Width x1, ASPM L0s L1,
Exit Latency L0s unlimited, L1 <2us
LnkSta: Speed 5GT/s, Width x1
LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-Let me clarify, please... This limitation to 2.5 GT/s came straight from Rockchip a few years ago, described back then as an undisclosed errata. Recently, we got some more details from Rockchip that confirmed 5 GT/s as having issues in certain corner cases that cannot be validated by performing some field tests or by observing the PCIe behavior under load. Those corner cases with 5 GT/s, as described by Rockchip, are quite hard to reach, but the possibility is still real.
Quoted 4 lines
To sum it up, yes, multiple people have reported 5 GT/s as "working for me" on their RK3399-based boards and devices, but that unfortunately means nothing in this case, due to the specific nature of the underlying issue.
Not only that but the bandwidth actually earned is very small due to the inherent limited interrupt processing capability of a single core, meaning the 5 GT/s bandwidth / transfer speed is never fully utilized.
It's better than to force a slightly lower actual transfer speed than to risk the liability of someone losing their data, period.
Regards,
Geraldo Nascimento
PATCH v2 2/4 PCI: rockchip-host: comment danger of 5.0 GT/s speed
In reply to Geraldo NascimentoAccording to Rockchip sources, there is grave danger in enabling 5.0 GT/s speed for this core. Add a comment documenting that danger and discouraging end-users from forcing higher speed.
--- drivers/pci/controller/pcie-rockchip-host.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/pci/controller/pcie-rockchip-host.c +3 -0
@@ -332,6 +332,9 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)/* * Enable retrain for gen2. This should be configured only after * gen1 finished. +* +* Dangerous and may lead to catastrophic failure eventually! +* */ status =rockchip_pcie_read (rockchip,PCIE_RC_CONFIG_CR +PCI_EXP_LNKCTL2 ); status &= ~PCI_EXP_LNKCTL2_TLS ;
Signature · 1 line
2.49.0
PATCH v2 3/4 arm64: dts: rockchip: remove dangerous max-link-speed from helios64
In reply to Geraldo NascimentoShawn Lin from Rockchip strongly discourages attempts to use their RK3399 PCIe core at 5.0 GT/s speed, citing concerns about catastrophic failures that may happen. Even if the odds are low, drop from last user of this non-default property for the RK3399 platform, helios64 board dts.
--- arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 - 1 file changed, 1 deletion(-)
arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts +0 -1
@@ -424,7 +424,6 @@ &pcie_phy { &pcie0 { ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>; - max-link-speed = <2>; num-lanes = <2>; pinctrl-names = "default"; status = "okay";
Signature · 1 line
2.49.0
PATCH v2 4/4 arm64: dts: rockchip: remove redundant max-link-speed from nanopi-r4s
In reply to Geraldo NascimentoThis is already the default in rk3399-base.dtsi, remove redundant declaration from rk3399-nanopi-r4s.dtsi.
--- arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dtsi | 1 - 1 file changed, 1 deletion(-)
arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dtsi +0 -1
@@ -71,7 +71,6 @@ &i2c4 { }; &pcie0 { - max-link-speed = <1>; num-lanes = <1>; vpcie3v3-supply = <&vcc3v3_sys>; };
Signature · 1 line
2.49.0
Quoted 10 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. Even if the odds are low, to contain any damage, let's cover the remaining corner-cases where the default would lead to 5.0 GT/s operation as well as add a comment to Root Complex driver core, documenting this danger. Furthermore, remove redundant declaration of max-link-speed from
rk3399-nanopi-r4s.dtsi
Quoted 2 lines
Signed-off-by Geraldo Nascimento <[email protected]>
Thanks for fixing these.
Quoted 22 lines
---
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/Geraldo Nascimento (4):
PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage
PCI: rockchip-host: comment danger of 5.0 GT/s speed
arm64: dts: rockchip: remove dangerous max-link-speed from helios64
arm64: dts: rockchip: remove redundant max-link-speed from nanopi-r4sarch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts | 1 - arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dtsi | 1 - drivers/pci/controller/pcie-rockchip-host.c | 3 +++ drivers/pci/controller/pcie-rockchip.c | 10 ++++++++-- 4 files changed, 11 insertions(+), 4 deletions(-)
Re: (subset) [PATCH v2 0/4] PCI: rockchip: 5.0 GT/s speed may be dangerous
In reply to Geraldo NascimentoQuoted 11 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. Even if the odds are low, to contain any damage, let's cover the remaining corner-cases where the default would lead to 5.0 GT/s operation as well as add a comment to Root Complex driver core, documenting this danger. Furthermore, remove redundant declaration of max-link-speed from
rk3399-nanopi-r4s.dtsi[...]
Applied, thanks!
[3/4] arm64: dts: rockchip: remove dangerous max-link-speed from helios64
commit: 0368e4afcf20f377c81fa77b1c7d0dee4a625a44
[4/4] arm64: dts: rockchip: remove redundant max-link-speed from nanopi-r4s
commit: ce652c98a7bfa0b7c675ef5cd85c44c186db96afBest regards,
Signature · 1 line
Heiko Stuebner <[email protected]>