From: Alex Williamson <alex.williamson@redhat.com>
To: Alexander Duyck <alexander.h.duyck@intel.com>
Cc: bhelgaas@google.com, linux-pci@vger.kernel.org,
ddutile@redhat.com, indou.takao@jp.fujitsu.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 8/9] pci: Tune secondary bus reset timing
Date: Tue, 06 Aug 2013 20:56:31 -0600 [thread overview]
Message-ID: <1375844191.3509.15.camel@ul30vt.home> (raw)
In-Reply-To: <52018664.1080103@intel.com>
On Tue, 2013-08-06 at 16:27 -0700, Alexander Duyck wrote:
> On 08/05/2013 12:37 PM, Alex Williamson wrote:
> > The PCI spec indicates that with stable power, reset needs to be
> > asserted for a minimum of 1ms (Trst). Seems like we should be able
> > to assume power is stable for a runtime secondary bus reset. The
> > current code has always used 100ms with no explanation where that
> > came from. The aer_do_secondary_bus_reset() function uses 2ms, but
> > that seems to be a misinterpretation of the PCIe spec, where hot
> > reset is implemented by TS1 ordered sets containing the hot reset
> > command. After a 2ms delay the state machine enters the detect state,
> > but to generate a link down, only two consecutive TS1 hot reset
> > ordered sets are requred. 1ms should be plenty for that.
>
> The reason for doing a 2ms sleep is because the are supposed to be
> sending the Hot Reset TS1 Ordered-Sets continuously for 2ms per all of
> the documents I have read.
Could you point to one of those references? In the PCIe v3 spec I'm
seeing things like 4.2.6.11 Hot Reset:
* If two consecutive TS1 Ordered Sets are received on any Lane
with the Hot Reset bit asserted and configured Link and Lane
numbers, then:
* LinkUp = 0b (False)
* If no higher Layer is directing the Physical Layer to
remain in Hot Reset, the next state is Detect
* Otherwise, all Lanes in the configured Link continue to
transmit TS1 Ordered Sets with the Hot Reset bit
asserted and the configured Link and Lane numbers.
* Otherwise, after a 2 ms timeout next state is Detect.
The next section has something similar for propagation of hot resets.
Nowhere there does it say TS1 Ordered Sets need to be sent continuously
for 2ms. A hot reset is initiated only by two consecutive TS1 Ordered
Sets with the Hot Reset bit asserted. The 2ms timeout seems to be the
delay before the link moves to the Detect state after we stop asserting
hot reset. 1ms seems like more than enough time for two TS1 Ordered
Sets to propagate down a multi-level hierarchy at 2.5GT/s.
> The 1ms number you quote is the minimum time
> for a conventional PCI bus. I'm not completely sure of that applies as
> well to PCIe, nor does it represent the maximum recommended value.
Correct, 1ms comes from conventional PCI. PCIe is designed to be
software compatible with conventional PCI so it makes sense that PCIe
would do something within the timing boundaries of conventional PCI. I
didn't see any reference to a maximum recommended value for this
parameter.
> If we stop early we risk not resetting the full device tree on the
> secondary bus which is the bug I was resolving by adding the 2ms delay.
> Previously we saw that some devices were only getting their PCIe link
> retrained without performing a hot reset when the bit was not held for
> long enough. I would prefer to keep this at 2 ms in order to account
> for the fact that PCIe has to go though link recovery states before it
> can perform the hot reset.
I'm not going to sweat over 1ms or 2ms but I do want to be able to
document why we're setting it to one or the other. If it's warm
fuzzies, so be it, but I'd prefer if we could find actual spec or
hardware examples to back it up. Thanks,
Alex
next prev parent reply other threads:[~2013-08-07 2:56 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-05 19:37 [PATCH v4 0/9] pci: bus and slot reset interfaces Alex Williamson
2013-08-05 19:37 ` [PATCH v4 1/9] pci: Create pci_reset_bridge_secondary_bus() Alex Williamson
2013-08-05 19:37 ` [PATCH v4 2/9] pci: Add hotplug_slot_ops.reset_slot() Alex Williamson
2013-08-05 19:37 ` [PATCH v4 3/9] pci: Implement reset_slot for pciehp Alex Williamson
2013-08-05 19:37 ` [PATCH v4 4/9] pci: Add slot reset option to pci_dev_reset Alex Williamson
2013-08-05 19:37 ` [PATCH v4 5/9] pci: Split out pci_dev lock/unlock and save/restore Alex Williamson
2013-08-05 19:37 ` [PATCH v4 6/9] pci: Add slot and bus reset interfaces Alex Williamson
2013-08-05 19:37 ` [PATCH v4 7/9] pci: Wake-up devices before save for reset Alex Williamson
2013-08-05 19:37 ` [PATCH v4 8/9] pci: Tune secondary bus reset timing Alex Williamson
2013-08-06 23:27 ` Alexander Duyck
2013-08-07 2:56 ` Alex Williamson [this message]
2013-08-07 18:30 ` Alexander Duyck
2013-08-08 5:23 ` Alex Williamson
2013-08-08 16:46 ` Alexander Duyck
2013-08-08 18:42 ` Alex Williamson
2013-08-05 19:37 ` [PATCH v4 9/9] pci: Remove aer_do_secondary_bus_reset() Alex Williamson
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=1375844191.3509.15.camel@ul30vt.home \
--to=alex.williamson@redhat.com \
--cc=alexander.h.duyck@intel.com \
--cc=bhelgaas@google.com \
--cc=ddutile@redhat.com \
--cc=indou.takao@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).