All of lore.kernel.org
 help / color / mirror / Atom feed
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: Thu, 15 Jan 2026 09:44:40 +0100	[thread overview]
Message-ID: <aWio-NB1csIhZJen@wunner.de> (raw)
In-Reply-To: <aWfuyw3JHD-1F5uZ@google.com>

On Wed, Jan 14, 2026 at 11:30:19AM -0800, Brian Norris wrote:
> On Sat, Jan 10, 2026 at 07:10:52AM +0100, Lukas Wunner wrote:
> > 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).
> 
> Seems like a reasonable suggestion. I'll try pci_clear_master() in some
> of these no-op non-failure cases.
> 
> Do you have the same concerns if pcie_init_service_irqs() falls back to
> INTx but does not fail? It seems like a potentially fraught exercise to
> guess what child services might need bus mastering though, so maybe it's
> better to limit this only to nr_service==0 cases?

Sounds reasonable to me to constrain to nr_service==0.
Basically just retain the existing behavior.

I note that pcie_portdrv_remove() calls pci_disable_device()
unconditionally.  You may need an extra struct with an extra flag
to remember whether pci_disable_device() needs to be called on remove.

Thanks,

Lukas

  reply	other threads:[~2026-01-15  8:52 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
2026-01-14 19:30   ` Brian Norris
2026-01-15  8:44     ` Lukas Wunner [this message]
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=aWio-NB1csIhZJen@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.