[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181129230454.GF178809@google.com>
Date: Thu, 29 Nov 2018 17:04:54 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Lukas Wunner <lukas@...ner.de>
Cc: Alex_Gagniuc@...lteam.com, mr.nuke.me@...il.com,
Austin.Bolen@...l.com, keith.busch@...el.com, Shyam.Iyer@...l.com,
mika.westerberg@...ux.intel.com, okaya@...eaurora.org,
rafael.j.wysocki@...el.com, poza@...eaurora.org,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: pciehp: Report degraded links via link bandwidth
notification
On Thu, Nov 29, 2018 at 08:13:12PM +0100, Lukas Wunner wrote:
> On Thu, Nov 29, 2018 at 06:57:37PM +0000, Alex_Gagniuc@...lteam.com wrote:
> > On 11/29/2018 11:36 AM, Bjorn Helgaas wrote:
> > > On Wed, Nov 28, 2018 at 06:08:24PM -0600, Alexandru Gagniuc wrote:
> > >> A warning is generated when a PCIe device is probed with a degraded
> > >> link, but there was no similar mechanism to warn when the link becomes
> > >> degraded after probing. The Link Bandwidth Notification provides this
> > >> mechanism.
> > >>
> > >> Use the link bandwidth notification interrupt to detect bandwidth
> > >> changes, and rescan the bandwidth, looking for the weakest point. This
> > >> is the same logic used in probe().
> > >
> > > I like the concept of this. What I don't like is the fact that it's
> > > tied to pciehp, since I don't think the concept of Link Bandwidth
> > > Notification is related to hotplug. So I think we'll only notice this
> > > for ports that support hotplug. Maybe it's worth doing it this way
> > > anyway, even if it could be generalized in the future?
> >
> > That makes sense. At first, I thought that BW notification was tied to
> > hotplug, but our PCIe spec writer disagreed with that assertion. I'm
> > just not sure where to handle the interrupt otherwise.
>
> I guess the interrupt is shared with hotplug and PME? In that case write
> a separate pcie_port_service_driver and request the interrupt with
> IRQF_SHARED. Define a new service type in drivers/pci/pcie/portdrv.h.
> Amend get_port_device_capability() to check for PCI_EXP_LNKCAP_LBNC.
I really don't like the port driver design. I'd rather integrate
those services more tightly into the PCI core. But realistically
that's wishful thinking and may never happen, so this might be the
most expedient approach.
Bjorn
Powered by blists - more mailing lists