[PATCH v2] PCI: dw-rockchip: Enable L0S capability

[Shawn Lin]

在 2025/04/11 星期五 16:51, Niklas Cassel 写道:
> 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 at 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().
> 

Will do.

> 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.

That's a good point.

> 
> 
>>   
>>   	/* 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, ... */
> 

Sure.

> 
> 
> Kind regards,
> Niklas
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>