ACK: [PATCH] aspm: Only require ASPM for devices with an actual link

[ivanhu]

Thanks!

Acked-by: Ivan Hu <ivan.hu at canonical.com>


On 7/2/24 00:49, superm1 at kernel.org wrote:
> From: Mario Limonciello <mario.limonciello at amd.com>
> 
> USB4 switches don't actually have any link between upstream and
> downstream ports.  Adjust the check to only check related devices
> with physical links.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>   src/lib/include/fwts_pci.h | 12 ++++++++++++
>   src/pci/aspm/aspm.c        |  9 +++++++++
>   2 files changed, 21 insertions(+)
> 
> diff --git a/src/lib/include/fwts_pci.h b/src/lib/include/fwts_pci.h
> index ba9cfac1..a3428a86 100644
> --- a/src/lib/include/fwts_pci.h
> +++ b/src/lib/include/fwts_pci.h
> @@ -109,6 +109,18 @@
>   #define FWTS_PCI_CONFIG_TYPE2_SUBSYSTEM_VENDOR_ID	(0x42)
>   #define FWTS_PCI_CONFIG_TYPE2_LEGACY_MODE_BASE_ADDRESS	(0x44)
>   
> +/* PCI config device port type */
> +#define FWTS_PCI_EXP_FLAGS_TYPE				0x00f0
> +#define  FWTS_PCI_EXP_TYPE_ENDPOINT			(0x0)
> +#define  FWTS_PCI_EXP_TYPE_LEGACY_ENDPOINT		(0x1)
> +#define  FWTS_PCI_EXP_TYPE_ROOT_PORT			(0x4)
> +#define  FWTS_PCI_EXP_TYPE_UPSTREAM_PORT		(0x5)
> +#define  FWTS_PCI_EXP_TYPE_DOWNSTREAM_PORT		(0x6)
> +#define  FWTS_PCI_EXP_TYPE_PCI_BRIDGE			(0x7)
> +#define  FWTS_PCI_EXP_TYPE_PCIE_BRIDGE			(0x8)
> +#define  FWTS_PCI_EXP_TYPE_RC_ENDPOINT			(0x9)
> +#define  FWTS_PCI_EXP_TYPE_RC_EVENT_COLLECTOR		(0xa)
> +
>   /* PCI config header types */
>   #define FWTS_PCI_CONFIG_HEADER_TYPE_NON_BRIDGE		(0x00)
>   #define FWTS_PCI_CONFIG_HEADER_TYPE_PCI_BRIDGE		(0x01)
> diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
> index c274f092..e50215d7 100644
> --- a/src/pci/aspm/aspm.c
> +++ b/src/pci/aspm/aspm.c
> @@ -87,6 +87,15 @@ static int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw,
>   		next_cap = rp_cap->next_cap_point;
>   		rp_cap = (fwts_pcie_capability *) &rp->config[next_cap];
>   	}
> +	if (rp_cap) {
> +		uint8_t device_type = (rp_cap->pcie_cap_reg & FWTS_PCI_EXP_FLAGS_TYPE) >> 4;
> +
> +		if ((device_type != FWTS_PCI_EXP_TYPE_ROOT_PORT) &&
> +		    (device_type != FWTS_PCI_EXP_TYPE_DOWNSTREAM_PORT) &&
> +		    (device_type != FWTS_PCI_EXP_TYPE_PCIE_BRIDGE)) {
> +			return ret;
> +		}
> +	}
>   
>   	next_cap = dev->config[FWTS_PCI_CONFIG_TYPE1_CAPABILITY_POINTER];
>   	device_cap = (fwts_pcie_capability *)&dev->config[next_cap];