From: Lukas Wunner <lukas@wunner.de>
To: Brian Norris <briannorris@chromium.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Manivannan Sadhasivam <mani@kernel.org>
Subject: Re: [PATCH] PCI/portdrv: Allow probing even without child services
Date: Sat, 10 Jan 2026 07:10:52 +0100 [thread overview]
Message-ID: <aWHtbGzVRRpa9kd0@wunner.de> (raw)
In-Reply-To: <20260109152013.1.I5fd5d83f518681b3949d8ab2f16ba8244fd3e774@changeid>
On Fri, Jan 09, 2026 at 03:20:13PM -0800, Brian Norris wrote:
> @@ -355,29 +355,18 @@ static int pcie_port_device_register(struct pci_dev *dev)
> if (status) {
> capabilities &= PCIE_PORT_SERVICE_HP;
> if (!capabilities)
> - goto error_disable;
> + return 0;
> }
This will keep the Bus Master Enable bit set (see call to
pci_set_master() further up in the function), even though
no MSIs are expected from the device. (I *think* these
would be the only memory writes that a port would perform.)
That doesn't seem right. If there are no services, it seems
prudent to clear Bus Master Enable again (as is done by
pci_disable_device() right now).
> /* Allocate child services if any */
> - status = -ENODEV;
> - nr_service = 0;
> for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
> int service = 1 << i;
> if (!(capabilities & service))
> continue;
> - if (!pcie_device_init(dev, service, irqs[i]))
> - nr_service++;
> + pcie_device_init(dev, service, irqs[i]);
> }
> - if (!nr_service)
> - goto error_cleanup_irqs;
Same here. Why keep Bus Master Enable bit set and MSIs requested
if none of the port services probed successfully?
> The PCIe port driver fails to probe if it finds no child services,
> presumably under the assumption that the driver is not useful in that
> case. However, the driver *can* still be useful for power management
> support -- namely, it still configures the port for runtime PM / D3,
> which may be important for allowing a bridge to enter low power modes.
>
> Thus, we allow probe to succeed even if no IRQs and no child services
> are available. This also mirrors existing behavior for ports that have
> no PCIe capabilities, where we'd also probe successfully.
Nit: Please use imperative mood, i.e. "Thus, allow probe to succeed..."
Thanks,
Lukas
next prev parent reply other threads:[~2026-01-10 6:11 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-09 23:20 [PATCH] PCI/portdrv: Allow probing even without child services Brian Norris
2026-01-10 6:10 ` Lukas Wunner [this message]
2026-01-14 19:30 ` Brian Norris
2026-01-15 8:44 ` Lukas Wunner
2026-02-10 1:17 ` Brian Norris
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=aWHtbGzVRRpa9kd0@wunner.de \
--to=lukas@wunner.de \
--cc=bhelgaas@google.com \
--cc=briannorris@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mani@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.