[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230615183754.GA1483387@bhelgaas>
Date: Thu, 15 Jun 2023 13:37:54 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: "Maciej W. Rozycki" <macro@...am.me.uk>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
Eric Dumazet <edumazet@...gle.com>,
Oliver O'Halloran <oohall@...il.com>,
Stefan Roese <sr@...x.de>, Leon Romanovsky <leon@...nel.org>,
linux-rdma@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Jim Wilson <wilson@...iptree.org>,
Nicholas Piggin <npiggin@...il.com>,
Alex Williamson <alex.williamson@...hat.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
David Abdurachmanov <david.abdurachmanov@...il.com>,
linuxppc-dev@...ts.ozlabs.org,
Mahesh J Salgaonkar <mahesh@...ux.ibm.com>,
"David S. Miller" <davem@...emloft.net>,
Lukas Wunner <lukas@...ner.de>, netdev@...r.kernel.org,
Pali Rohár <pali@...nel.org>,
Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [PATCH v9 00/14] pci: Work around ASMedia ASM2824 PCIe link
training failures
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
Powered by blists - more mailing lists