Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: "Stefan Mätje" <Stefan.Maetje@esd.eu>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Cc: "bhelgaas@google.com" <bhelgaas@google.com>,
	"rafael.j.wysocki@intel.com" <rafael.j.wysocki@intel.com>,
	"ptalbert@redhat.com" <ptalbert@redhat.com>,
	"lukas@wunner.de" <lukas@wunner.de>,
	"fred@fredlawl.com" <fred@fredlawl.com>,
	"bhull@redhat.com" <bhull@redhat.com>
Subject: Re: [PATCH V2 1/1] PCI/ASPM: Work around link retrain errata of Pericom PCIe-to-PCI bridges
Date: Thu, 7 Mar 2019 16:16:01 +0100
Message-ID: <adca5e40-f349-9692-84da-84d45ec828ca@esd.eu> (raw)
In-Reply-To: <20190306070341.GY9224@smile.fi.intel.com>

Am 06.03.19 um 08:03 schrieb Andy Shevchenko:
> On Tue, Mar 05, 2019 at 06:31:22PM +0100, Stefan Mätje wrote:
:
:
>> The patch introduces a new flag clear_retrain_link in the struct pci_dev. This
>> flag is set in quirk_enable_clear_retrain_link() for the affected devices in
>> the pci_fixup_header in quirks.c
>>
>> The link retraining code is moved to pcie_retrain_link(). This function now
>> applies the work around to clear the PCI_EXP_LNKCTL_RL bit again if
>> clear_retrain_link bit is set in the pci_dev structure of the link parent
>> device.
> 
> 
>> +	/* Wait for link training end. Break out after waiting for timeout */
>> +	start_jiffies = jiffies;
>> +	for (;;) {
>> +		pcie_capability_read_word(parent, PCI_EXP_LNKSTA, &reg16);
>> +		if (!(reg16 & PCI_EXP_LNKSTA_LT))
>> +			break;
>> +		if (time_after(jiffies, start_jiffies + LINK_RETRAIN_TIMEOUT))
>> +			break;
>> +		msleep(1);
>> +	}
> 
> I see this is existing code, though two comments (perhaps for future clean up):
> 	- msleep(1) is not guaranteed to be 1 ms at all (in some cases it might be less)

	Even if msleep(1) would return after a very short time it would not do any harm
	here. It is only needed to do "some" sleeping in this status reading loop.

> 	- better to use do {} while (time_before()) loop

	I will change that.

> 
>> +static void quirk_enable_clear_retrain_link(struct pci_dev *dev)
>> +{
>> +	dev->clear_retrain_link = 1;
> 
>> +	pci_info(dev, "Enable Pericom link retrain quirk\n");
> 
> If some other device would appear with same issue, but from another vendor this
> becomes misleading.
> 
> I would recommend to refer to an issue the quirk is about, and not the vendor.

I think that would be better, too. I'll change that.

>> +}
> 


      reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-05 17:31 [PATCH V2 0/1] Proposal of a patch to work around link retrain errata of Pericom PCIe brigdes Stefan Mätje
2019-03-05 17:31 ` [PATCH V2 1/1] PCI/ASPM: Work around link retrain errata of Pericom PCIe-to-PCI bridges Stefan Mätje
2019-03-06  7:03   ` Andy Shevchenko
2019-03-07 15:16     ` Stefan Mätje [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=adca5e40-f349-9692-84da-84d45ec828ca@esd.eu \
    --to=stefan.maetje@esd.eu \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=bhull@redhat.com \
    --cc=fred@fredlawl.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=ptalbert@redhat.com \
    --cc=rafael.j.wysocki@intel.com \
    /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

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git