[PATCH] PCI: dw-rockchip: Remove PCIE_L0S_ENTRY check from rockchip_pcie_link_up()

[Niklas Cassel]

On Wed, Apr 09, 2025 at 02:40:33PM +0800, Shawn Lin wrote:
> Two mistakes here:
> 1. 0x11 is L0 not L0S, so the naming is wrong from the very beginning.
> 2. It's totally broken if enabling ASPM as rockchip_pcie_link_up() treat other
> states, for instance, L0S or L1 as link down which is obvioult wrong.
> 
> Remove the check.
> 
> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com>
> ---
> 
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index c624b7e..21dc99c 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -44,7 +44,6 @@
>  #define PCIE_LINKUP			(PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
>  #define PCIE_RDLH_LINK_UP_CHGED		BIT(1)
>  #define PCIE_LINK_REQ_RST_NOT_INT	BIT(2)
> -#define PCIE_L0S_ENTRY			0x11
>  #define PCIE_CLIENT_GENERAL_CONTROL	0x0
>  #define PCIE_CLIENT_INTR_STATUS_LEGACY	0x8
>  #define PCIE_CLIENT_INTR_MASK_LEGACY	0x1c
> @@ -177,8 +176,7 @@ static int rockchip_pcie_link_up(struct dw_pcie *pci)
>  	struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>  	u32 val = rockchip_pcie_get_ltssm(rockchip);
>  
> -	if ((val & PCIE_LINKUP) == PCIE_LINKUP &&
> -	    (val & PCIE_LTSSM_STATUS_MASK) == PCIE_L0S_ENTRY)
> +	if ((val & PCIE_LINKUP) == PCIE_LINKUP)
>  		return 1;
>  
>  	return 0;
> -- 
> 2.7.4
> 

You should probably also add:
Fixes: 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver")


Considering that dw_pcie_link_up() looks like this:
https://github.com/torvalds/linux/blob/v6.15-rc1/drivers/pci/controller/dwc/pcie-designware.c#L714-L725

Why not simply remove the rockchip_pcie_link_up() callback completely?

Is there any advantage of using a rockchip specific way to read link up,
rather than just reading link up via the DWC PCIE_PORT_DEBUG1 register?


Kind regards,
Niklas