Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: stuart hayes <stuart.w.hayes@gmail.com>
Cc: Krzysztof Wilczy??ski <kw@linux.com>,
	Lukas Wunner <lukas@wunner.de>,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH v2] PCI/portdrv: Use link bandwidth notification capability bit
Date: Tue, 31 Aug 2021 14:20:41 -0500	[thread overview]
Message-ID: <20210831192041.GA124936@bjorn-Precision-5520> (raw)
In-Reply-To: <53c92c86-5fd9-5db4-eacf-954f1f07cecb@gmail.com>

On Wed, May 26, 2021 at 08:12:04PM -0500, stuart hayes wrote:
> ...
> I made the patch because it was causing the config space for a downstream
> port to not get restored when a DPC event occurred, and all the NVMe drives
> under it disappeared.  I found that myself, though--I'm not aware of anyone
> else reporting the issue.

This niggles at me.  IIUC the problem you're reporting is that portdrv
didn't claim a port because portdrv incorrectly assumed the port
supported bandwidth notification interrupts.  That's all fine, and I
think this is a good fix.

But why should it matter whether portdrv claims the port?  What if
CONFIG_PCIEPORTBUS isn't even enabled?  I guess CONFIG_PCIE_DPC
wouldn't be enabled then either.

In your situation, you have CONFIG_PCIEPORTBUS=y and (I assume)
CONFIG_PCIE_DPC=y.  I guess you must have two levels of downstream
ports, e.g.,

  Root Port -> Switch Upstream Port -> Switch Downstream Port -> NVMe

and portdrv claimed the Root Port and you enabled DPC there, but it
didn't claim the Switch Downstream Port?

The failure to restore config space because portdrv didn't claim the
port seems wrong to me.

Bjorn

  parent reply	other threads:[~2021-08-31 19:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-12 21:33 [PATCH v2] PCI/portdrv: Use link bandwidth notification capability bit Stuart Hayes
2021-05-14 13:03 ` Krzysztof Wilczyński
2021-05-14 13:08   ` Lukas Wunner
2021-05-14 13:17     ` Krzysztof Wilczy??ski
2021-05-27  1:12       ` stuart hayes
2021-07-07 15:48         ` stuart hayes
2021-07-07 18:59           ` Bjorn Helgaas
2021-08-31 19:20         ` Bjorn Helgaas [this message]
2021-08-31 21:39           ` stuart hayes
2021-08-31 21:58             ` Bjorn Helgaas
2021-09-01  5:48               ` Lukas Wunner
2021-09-01 21:28                 ` Bjorn Helgaas
2021-07-16 21:56 ` Bjorn Helgaas
2021-07-17  2:43   ` stuart hayes
2021-08-31 19:11 ` Bjorn Helgaas

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=20210831192041.GA124936@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=kw@linux.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=stuart.w.hayes@gmail.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
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