Re: [PATCH] lspci: Don't report PCIe link downgrades for downstream ports

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Mar 18, 2021 at 12:02:44PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> After b47b5bd408e1 ("lspci: Report if the PCIe link speed/width is full or
> downgraded"), we report "downgraded" or "ok" for PCIe links operating at
> lower speed or width than they're capable of:
> 
>   LnkCap: Port #1, Speed 8GT/s, Width x1, ASPM L1, Exit Latency L1 <16us
>   LnkSta: Speed 5GT/s (downgraded), Width x1 (ok)
> 
> Previously we did this for both ends of the link, but I don't think it's
> very useful for Downstream Ports (the upstream end of the link) because we
> claim the link is downgraded even if (1) there's no device on the other end
> or (2) the other device doesn't support anything faster/wider.
> 
> Drop the "downgraded" reporting for Downstream Ports.  If there is a device
> below, we'll still complain at that end if it supports a faster/wider link
> than is available.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  ls-caps.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/ls-caps.c b/ls-caps.c
> index db56556..dd17c6b 100644
> --- a/ls-caps.c
> +++ b/ls-caps.c
> @@ -758,13 +758,16 @@ static char *link_speed(int speed)
>      }
>  }
>  
> -static char *link_compare(int sta, int cap)
> +static char *link_compare(int type, int sta, int cap)
>  {
> +  if ((type == PCI_EXP_TYPE_ROOT_PORT) || (type == PCI_EXP_TYPE_DOWNSTREAM) ||
> +      (type == PCI_EXP_TYPE_PCIE_BRIDGE))
> +    return "";
>    if (sta < cap)
> -    return "downgraded";
> +    return " (downgraded)";
>    if (sta > cap)
> -    return "strange";
> -  return "ok";
> +    return " (strange)";
> +  return " (ok)";

Personally I would probably just return "" by default and drop the
"(ok)" notes because they don't really seem necessary.  But either way
is OK.

>  }
>  
>  static char *aspm_support(int code)
> @@ -837,11 +840,11 @@ static void cap_express_link(struct device *d, int where, int type)
>    w = get_conf_word(d, where + PCI_EXP_LNKSTA);
>    sta_speed = w & PCI_EXP_LNKSTA_SPEED;
>    sta_width = (w & PCI_EXP_LNKSTA_WIDTH) >> 4;
> -  printf("\t\tLnkSta:\tSpeed %s (%s), Width x%d (%s)\n",
> +  printf("\t\tLnkSta:\tSpeed %s%s, Width x%d%s\n",
>  	link_speed(sta_speed),
> -	link_compare(sta_speed, cap_speed),
> +	link_compare(type, sta_speed, cap_speed),
>  	sta_width,
> -	link_compare(sta_width, cap_width));
> +	link_compare(type, sta_width, cap_width));
>    printf("\t\t\tTrErr%c Train%c SlotClk%c DLActive%c BWMgmt%c ABWMgmt%c\n",
>  	FLAG(w, PCI_EXP_LNKSTA_TR_ERR),
>  	FLAG(w, PCI_EXP_LNKSTA_TRAIN),
> -- 
> 2.25.1
>