Received-SPF: pass (google.com: domain of linux-kernel+bounces-88816-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 1 Mar 2024 17:22:37 +0000 (GMT)
From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
cc: Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org, 
    linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] PCI: Use the correct bit in Link Training not active
 check
In-Reply-To: <20240301150641.4037-1-ilpo.jarvinen@linux.intel.com>
Message-ID: <alpine.DEB.2.21.2403011622560.42226@angie.orcam.me.uk>
References: <20240301150641.4037-1-ilpo.jarvinen@linux.intel.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT

On Fri, 1 Mar 2024, Ilpo Järvinen wrote:

> Besides Link Training bit, pcie_retrain_link() can also be asked to
> wait for Data Link Layer Link Active bit (PCIe r6.1 sec 7.5.3.8) using
> 'use_lt' parameter since the merge commit 1abb47390350 ("Merge branch
> 'pci/enumeration'").

 Nope, it was added with commit 680e9c47a229 ("PCI: Add support for 
polling DLLLA to pcie_retrain_link()"), before said merge.

> pcie_retrain_link() first tries to ensure Link Training is not
> currently active (see Implementation Note in PCIe r6.1 sec 7.5.3.7)
> which must always check Link Training bit regardless of 'use_lt'.
> Correct the pcie_wait_for_link_status() parameters to only wait for
> the correct bit to ensure there is no ongoing Link Training.

 You're talking the PCIe spec here and code is talking a bad device case.

> Since waiting for Data Link Layer Link Active bit is only used for the
> Target Speed quirk, this only impacts the case when the quirk attempts
> to restore speed to higher than 2.5 GT/s (The link is Up at that point
> so pcie_retrain_link() will fail).

 NAK.  It's used for both clamping and unclamping and it will break the 
workaround, because the whole point there is to wait until DLLA has been 
set.  Using LT is not reliable because it will oscillate in the failure 
case and seeing the bit clear does not mean link has been established.  

 What are you trying to achieve here and what problem is it to fix?

  Maciej