Re: [PATCH v2 06/10] PCI: tegra194: Check for L1SS capability before programming

[Frank Li]

On Wed, Oct 21, 2026 at 11:43:45AM +0530, Manikanta Maddireddy wrote:
> init_host_aspm() programs the L1 PM Substates capability without checking
> whether the controller advertises it. If the capability is absent, the
> code writes offsets from zero instead of L1SS registers.
>
> Check for the L1SS capability before programming L1SS-specific fields.
> Keep RAS-DES discovery, ASPM counter enablement and AFR latency
> programming outside that guard because they do not depend on L1SS.
> Only create the ASPM counter debugfs file when the RAS-DES capability is
> present.
>
> Fixes: 56e15a238d92 ("PCI: tegra: Add Tegra194 PCIe support")
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> Changes V1 -> V2:
> - Use RAS_DES_EVENT_COUNTER_CONTROL to match the preceding DWC register
>   renames. No functional changes.
>
>  drivers/pci/controller/dwc/pcie-tegra194.c | 72 ++++++++++++----------
>  1 file changed, 41 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 14031cfaee8b..7c349478556d 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -673,37 +673,46 @@ static void init_host_aspm(struct tegra_pcie_dw *pcie)
>
>  	pcie->ras_des_cap = dw_pcie_find_ext_capability(&pcie->pci,
>  							PCI_EXT_CAP_ID_VNDR);
> -
> -	/* Enable ASPM counters */
> -	val = FIELD_PREP(EVENT_COUNTER_ENABLE_MASK, EVENT_COUNTER_ENABLE_ALL);
> -	val |= FIELD_PREP(EVENT_COUNTER_GROUP_SEL_MASK, EVENT_COUNTER_GROUP_5);
> -	dw_pcie_writel_dbi(pci, pcie->ras_des_cap +
> -			   RAS_DES_EVENT_COUNTER_CONTROL, val);
> -
> -	/* Program T_cmrt and T_pwr_on values */
> -	val = dw_pcie_readl_dbi(pci, l1ss + PCI_L1SS_CAP);
> -	FIELD_MODIFY(PCI_L1SS_CAP_CM_RESTORE_TIME, &val, pcie->aspm_cmrt);
> -	FIELD_MODIFY(PCI_L1SS_CAP_P_PWR_ON_VALUE, &val, pcie->aspm_pwr_on_t);
> -	dw_pcie_writel_dbi(pci, l1ss + PCI_L1SS_CAP, val);
> -
> -	if (pcie->supports_clkreq)
> -		pci->l1ss_support = true;
> -
> -	/*
> -	 * Disable L1.2 capability advertisement for Tegra234 Endpoint mode.
> -	 * Tegra234 has a hardware bug where during L1.2 exit, the UPHY PLL is
> -	 * powered up immediately without waiting for REFCLK to stabilize. This
> -	 * causes the PLL to fail to lock to the correct frequency, resulting in
> -	 * PCIe link loss. Since there is no hardware fix available, we prevent
> -	 * the Endpoint from advertising L1.2 support by clearing the L1.2 bits
> -	 * in the L1 PM Substates Capabilities register. This ensures the host
> -	 * will not attempt to enter L1.2 state with this Endpoint.
> -	 */
> -	if (pcie->of_data->disable_l1_2 &&
> -	    pcie->of_data->mode == DW_PCIE_EP_TYPE) {
> +	if (pcie->ras_des_cap) {
> +		/* Enable ASPM counters */
> +		val = FIELD_PREP(EVENT_COUNTER_ENABLE_MASK,
> +				 EVENT_COUNTER_ENABLE_ALL);
> +		val |= FIELD_PREP(EVENT_COUNTER_GROUP_SEL_MASK,
> +				  EVENT_COUNTER_GROUP_5);
> +		dw_pcie_writel_dbi(pci, pcie->ras_des_cap +
> +				   RAS_DES_EVENT_COUNTER_CONTROL, val);
> +	}
> +
> +	if (l1ss) {
> +		/* Program T_cmrt and T_pwr_on values */
>  		val = dw_pcie_readl_dbi(pci, l1ss + PCI_L1SS_CAP);
> -		val &= ~(PCI_L1SS_CAP_PCIPM_L1_2 | PCI_L1SS_CAP_ASPM_L1_2);
> +		FIELD_MODIFY(PCI_L1SS_CAP_CM_RESTORE_TIME, &val,
> +			     pcie->aspm_cmrt);
> +		FIELD_MODIFY(PCI_L1SS_CAP_P_PWR_ON_VALUE, &val,
> +			     pcie->aspm_pwr_on_t);
>  		dw_pcie_writel_dbi(pci, l1ss + PCI_L1SS_CAP, val);
> +
> +		if (pcie->supports_clkreq)
> +			pci->l1ss_support = true;
> +
> +		/*
> +		 * Disable L1.2 capability advertisement for Tegra234 Endpoint
> +		 * mode. Tegra234 has a hardware bug where during L1.2 exit,
> +		 * the UPHY PLL is powered up immediately without waiting for
> +		 * REFCLK to stabilize. This causes the PLL to fail to lock to
> +		 * the correct frequency, resulting in PCIe link loss. Since
> +		 * there is no hardware fix available, prevent the Endpoint
> +		 * from advertising L1.2 support by clearing the L1.2 bits in
> +		 * the L1 PM Substates Capabilities register. This ensures the
> +		 * host will not attempt to enter L1.2 with this Endpoint.
> +		 */
> +		if (pcie->of_data->disable_l1_2 &&
> +		    pcie->of_data->mode == DW_PCIE_EP_TYPE) {
> +			val = dw_pcie_readl_dbi(pci, l1ss + PCI_L1SS_CAP);
> +			val &= ~(PCI_L1SS_CAP_PCIPM_L1_2 |
> +				 PCI_L1SS_CAP_ASPM_L1_2);
> +			dw_pcie_writel_dbi(pci, l1ss + PCI_L1SS_CAP, val);
> +		}
>  	}
>
>  	/* Program L0s and L1 entrance latencies */
> @@ -727,8 +736,9 @@ static void init_debugfs(struct tegra_pcie_dw *pcie)
>
>  	pcie->debugfs = debugfs_create_dir(name, NULL);
>
> -	debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs,
> -				    aspm_state_cnt);
> +	if (pcie->ras_des_cap)
> +		debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs,
> +					    aspm_state_cnt);
>  }
>  #else
>  static inline void init_host_aspm(struct tegra_pcie_dw *pcie) { return; }
> --
> 2.34.1
>