Linux PCI subsystem development
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org,  LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] PCI: Use the correct bit in Link Training not active check
Date: Fri, 15 Mar 2024 11:58:31 +0200 (EET)	[thread overview]
Message-ID: <c81ecb2a-9365-dbb0-2207-ed692c8c7487@linux.intel.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2403142145010.37739@angie.orcam.me.uk>

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

On Thu, 14 Mar 2024, Maciej W. Rozycki wrote:

> On Thu, 14 Mar 2024, Ilpo Järvinen wrote:
> 
> > One more point to add here, I started to wonder today why that use_lt 
> > parameter is needed at all for pcie_retrain_link()?
> > 
> > Once the Target Speed has been changed to 2.5GT/s which is what the quirk 
> > does before calling retraining, LT too should work "normally" after that.
> 
>  We don't know if the link is going to become stable with the TLS update 
> to 2.5GT/s and we want to ensure that the link has reached the active 
> state before claiming victory; LT clear does not mean the link is active, 
> it only means what it means, that is that the link isn't being trained at 
> the moment.

LT clear means retraining has ended which is the condition 
pcie_retrain_link() should terminate at. It tried and finished retraining 
as proven by LT clear.

>  Also we don't want to reset the TLS to the maximum before the link has 
> become active.

I'm not suggesting to change the if DLLLA check that is within the quirk 
so it will remain the same even if pcie_retrain_link() would no longer 
have the use_lt parameter.

If LT clear after retraining does not imply DLLLA set, then this again 
falls into the quirk territory and IMO the quirk itself should be what 
makes the additional call to wait for DLLLA, not pcie_retrain_link().
I suspect though that DL clear does imply DLLLA set (after the target 
speed was lowered) so my expectation is that the extra wait wouldn't be 
necessary at that point.

>  Does this answer your question?

What I'm trying to achieve is having pcie_retrain_link() focus on 
retraining and quirk on steps required by the quirk. Currently they're 
kind of mixed if we assume the assumption that LT clear doesn't imply 
active link is true. That tells quirk would need additional step, 
that is, wait for DLLLA after the retraining has completed which is 
currently hidden into pcie_retrain_link() rather than explicitly 
called by the quirk.

-- 
 i.

      reply	other threads:[~2024-03-15  9:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-01 15:06 [PATCH 1/1] PCI: Use the correct bit in Link Training not active check Ilpo Järvinen
2024-03-01 17:22 ` Maciej W. Rozycki
2024-03-04 11:21   ` Ilpo Järvinen
2024-03-06 12:23     ` Maciej W. Rozycki
2024-03-06 15:44       ` Ilpo Järvinen
2024-03-14 11:39         ` Ilpo Järvinen
2024-03-14 21:58           ` Maciej W. Rozycki
2024-03-15  9:58             ` Ilpo Järvinen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c81ecb2a-9365-dbb0-2207-ed692c8c7487@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=macro@orcam.me.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox