| Message ID | 1744332481-143011-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | [v2] PCI: dw-rockchip: Enable L0S capability | expand |
Hello Shawn, On Fri, Apr 11, 2025 at 08:48:01AM +0800, Shawn Lin wrote: > L0S capability isn't enabled on all SoCs by default, so enabling it > in order to make ASPM L0S work on Rockchip platforms. We have been > testing it for quite a long time and found the default FTS number > provided by DWC core doesn't work stable and make LTSSM switch between > L0S and Recovery, leading to long exit latency, even fail to link sometimes. > So override it to the max 255 which seems work fine under test for both PHYs > used by Rockchip platforms. > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > > Changes in v2: > - Move n_fts to probe function > - rewrite the commit message > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > index 21dc99c..c397f3a 100644 > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > @@ -185,6 +185,17 @@ static int rockchip_pcie_link_up(struct dw_pcie *pci) > static int rockchip_pcie_start_link(struct dw_pcie *pci) > { > struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 cap, lnkcap; > + > + /* Enable L0S capability for all SoCs */ > + cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > + if (cap) { > + lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP); > + lnkcap |= PCI_EXP_LNKCAP_ASPM_L0S; > + dw_pcie_dbi_ro_wr_en(pci); > + dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, lnkcap); > + dw_pcie_dbi_ro_wr_dis(pci); > + } I think it looks wrong to have this in _start_link(). Why not put this code in a new "_enable_l0s() function, and call it from: rockchip_pcie_ep_init() and rockchip_pcie_host_init(). When looking at this, we probably should also move the rockchip_pcie_ep_hide_broken_ats_cap_rk3588() call from _pre_init() to _init(), because if there is a core reset, _init() is called again, but _pre_init() is not. > > /* Reset device */ > gpiod_set_value_cansleep(rockchip->rst_gpio, 0); > @@ -598,6 +609,9 @@ static int rockchip_pcie_probe(struct platform_device *pdev) > rockchip->pci.dev = dev; > rockchip->pci.ops = &dw_pcie_ops; > rockchip->data = data; Perhaps add a newline here. > + /* Default fts number(210) is broken, override it to 255 */ Perhaps add a space between "number" and "(". Perhaps write it as N_FTS instead of fts. Perhaps use "value" instead of "number". ie. /* Default N_FTS value (210) is broken, ... */ Kind regards, Niklas
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c index 21dc99c..c397f3a 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -185,6 +185,17 @@ static int rockchip_pcie_link_up(struct dw_pcie *pci) static int rockchip_pcie_start_link(struct dw_pcie *pci) { struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); + u32 cap, lnkcap; + + /* Enable L0S capability for all SoCs */ + cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); + if (cap) { + lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP); + lnkcap |= PCI_EXP_LNKCAP_ASPM_L0S; + dw_pcie_dbi_ro_wr_en(pci); + dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, lnkcap); + dw_pcie_dbi_ro_wr_dis(pci); + } /* Reset device */ gpiod_set_value_cansleep(rockchip->rst_gpio, 0); @@ -598,6 +609,9 @@ static int rockchip_pcie_probe(struct platform_device *pdev) rockchip->pci.dev = dev; rockchip->pci.ops = &dw_pcie_ops; rockchip->data = data; + /* Default fts number(210) is broken, override it to 255 */ + rockchip->pci.n_fts[0] = 255; /* Gen1 */ + rockchip->pci.n_fts[1] = 255; /* Gen2+ */ ret = rockchip_pcie_resource_get(pdev, rockchip); if (ret)
L0S capability isn't enabled on all SoCs by default, so enabling it in order to make ASPM L0S work on Rockchip platforms. We have been testing it for quite a long time and found the default FTS number provided by DWC core doesn't work stable and make LTSSM switch between L0S and Recovery, leading to long exit latency, even fail to link sometimes. So override it to the max 255 which seems work fine under test for both PHYs used by Rockchip platforms. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- Changes in v2: - Move n_fts to probe function - rewrite the commit message drivers/pci/controller/dwc/pcie-dw-rockchip.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)