From: Bjorn Helgaas <helgaas@kernel.org>
To: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"Eric Dumazet" <edumazet@google.com>,
"Oliver O'Halloran" <oohall@gmail.com>,
"Stefan Roese" <sr@denx.de>, "Leon Romanovsky" <leon@kernel.org>,
linux-rdma@vger.kernel.org, "Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Jim Wilson" <wilson@tuliptree.org>,
"Nicholas Piggin" <npiggin@gmail.com>,
"Alex Williamson" <alex.williamson@redhat.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Mika Westerberg" <mika.westerberg@linux.intel.com>,
"David Abdurachmanov" <david.abdurachmanov@gmail.com>,
linuxppc-dev@lists.ozlabs.org,
"Mahesh J Salgaonkar" <mahesh@linux.ibm.com>,
"David S. Miller" <davem@davemloft.net>,
"Lukas Wunner" <lukas@wunner.de>,
netdev@vger.kernel.org, "Pali Rohár" <pali@kernel.org>,
"Saeed Mahameed" <saeedm@nvidia.com>
Subject: Re: [PATCH v9 00/14] pci: Work around ASMedia ASM2824 PCIe link training failures
Date: Thu, 15 Jun 2023 13:37:54 -0500 [thread overview]
Message-ID: <20230615183754.GA1483387@bhelgaas> (raw)
In-Reply-To: <alpine.DEB.2.21.2306150124010.64925@angie.orcam.me.uk>
On Thu, Jun 15, 2023 at 01:41:10AM +0100, Maciej W. Rozycki wrote:
> On Wed, 14 Jun 2023, Bjorn Helgaas wrote:
>
> > > This is v9 of the change to work around a PCIe link training phenomenon
> > > where a pair of devices both capable of operating at a link speed above
> > > 2.5GT/s seems unable to negotiate the link speed and continues training
> > > indefinitely with the Link Training bit switching on and off repeatedly
> > > and the data link layer never reaching the active state.
> > >
> > > With several requests addressed and a few extra issues spotted this
> > > version has now grown to 14 patches. It has been verified for device
> > > enumeration with and without PCI_QUIRKS enabled, using the same piece of
> > > RISC-V hardware as previously. Hot plug or reset events have not been
> > > verified, as this is difficult if at all feasible with hardware in
> > > question.
> > static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
> > {
> > - bool retrain = true;
> > int delay = 1;
> > + bool retrain = false;
> > + struct pci_dev *bridge;
> > +
> > + if (pci_is_pcie(dev)) {
> > + retrain = true;
> > + bridge = pci_upstream_bridge(dev);
> > + }
>
> If doing it this way, which I actually like, I think it would be a little
> bit better performance- and style-wise if this was written as:
>
> if (pci_is_pcie(dev)) {
> bridge = pci_upstream_bridge(dev);
> retrain = !!bridge;
> }
>
> (or "retrain = bridge != NULL" if you prefer this style), and then we
> don't have to repeatedly check two variables iff (pcie && !bridge) in the
> loop below:
Done, thanks, I do like that better. I did:
bridge = pci_upstream_bridge(dev);
if (bridge)
retrain = true;
because it seems like it flows more naturally when reading.
Bjorn
next prev parent reply other threads:[~2023-06-15 18:37 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-11 17:19 [PATCH v9 00/14] pci: Work around ASMedia ASM2824 PCIe link training failures Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 01/14] PCI: pciehp: Rely on `link_active_reporting' Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 02/14] PCI: Export PCIe link retrain timeout Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 03/14] PCI: Execute `quirk_enable_clear_retrain_link' earlier Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 04/14] PCI: Initialize `link_active_reporting' earlier Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 05/14] powerpc/eeh: Rely on `link_active_reporting' Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 06/14] net/mlx5: " Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 07/14] PCI: Export `pcie_retrain_link' for use outside ASPM Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 08/14] PCI: Use distinct local vars in `pcie_retrain_link' Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 09/14] PCI: Factor our waiting for link training end Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 10/14] PCI: Add support for polling DLLLA to `pcie_retrain_link' Maciej W. Rozycki
2023-06-11 17:19 ` [PATCH v9 11/14] PCI: Use `pcie_wait_for_link_status' in `pcie_wait_for_link_delay' Maciej W. Rozycki
2023-06-11 17:20 ` [PATCH v9 12/14] PCI: Provide stub failed link recovery for device probing and hot plug Maciej W. Rozycki
2023-06-11 17:20 ` [PATCH v9 13/14] PCI: Add failed link recovery for device reset events Maciej W. Rozycki
2023-06-11 17:20 ` [PATCH v9 14/14] PCI: Work around PCIe link training failures Maciej W. Rozycki
2023-06-14 23:12 ` [PATCH v9 00/14] pci: Work around ASMedia ASM2824 " Bjorn Helgaas
2023-06-15 0:41 ` Maciej W. Rozycki
2023-06-15 18:37 ` Bjorn Helgaas [this message]
2023-06-16 12:27 ` Maciej W. Rozycki
2023-06-16 20:29 ` Bjorn Helgaas
2023-06-20 9:54 ` Maciej W. Rozycki
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=20230615183754.GA1483387@bhelgaas \
--to=helgaas@kernel.org \
--cc=alex.williamson@redhat.com \
--cc=bhelgaas@google.com \
--cc=davem@davemloft.net \
--cc=david.abdurachmanov@gmail.com \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=lukas@wunner.de \
--cc=macro@orcam.me.uk \
--cc=mahesh@linux.ibm.com \
--cc=mika.westerberg@linux.intel.com \
--cc=netdev@vger.kernel.org \
--cc=npiggin@gmail.com \
--cc=oohall@gmail.com \
--cc=pabeni@redhat.com \
--cc=pali@kernel.org \
--cc=saeedm@nvidia.com \
--cc=sr@denx.de \
--cc=wilson@tuliptree.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).