Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Fahmy Hassan <fahmymohammed@gmail.com>
Cc: helgaas@kernel.org, bhelgaas@google.com, scott@spiteful.org,
	kees@kernel.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 4/5] PCI: pciehp: Check pci_hp_add_bridge() return value
Date: Wed, 9 Sep 2026 08:42:37 +0200	[thread overview]
Message-ID: <aqD_3ULr3ls4S9IN@wunner.de> (raw)
In-Reply-To: <20260909022227.620217-5-fahmymohammed@gmail.com>

On Tue, Sep 08, 2026 at 08:22:25PM -0600, Fahmy Hassan wrote:
> pciehp_configure_device() calls pci_hp_add_bridge() for each bridge on
> the newly added slot without checking its return value.
> pci_hp_add_bridge() already logs an error for one failure path (no
> bus number available for the hot-added bridge), but returns silently
> if the bridge's subordinate bus isn't created after scanning -- that
> path goes completely unreported, and either way the caller currently
> has no way to notice or react to the failure.
> 
> Log an error via the driver's existing ctrl_err() macro when
> pci_hp_add_bridge() fails, identifying the device involved.

The only change here is to log something on error.  That can be done
in pci_hp_add_bridge() itself without having to amend every caller.

However pci_hp_add_bridge() already logs an error for the one failure
case that merits an error message.

The return value is normally evaluated to do something, such as
bailing out or unwinding some earlier actions.  But in this case,
I'm not seeing a need to do that.

So I believe this change isn't necessary, at least as far as pciehp
is concerned.

Thanks,

Lukas

  reply	other threads:[~2026-09-09  6:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-09  1:30 [PATCH v1] PCI: cpqphp: Check pci_hp_add_bridge() return value Fahmy Hassan
2026-09-09  1:41 ` Bjorn Helgaas
2026-09-09  2:17   ` Fahmy Hassan
2026-09-09  2:22   ` [PATCH v2 0/5] PCI: hotplug: " Fahmy Hassan
2026-09-09  2:22     ` [PATCH v2 1/5] PCI: cpqphp: " Fahmy Hassan
2026-09-09  2:22     ` [PATCH v2 2/5] PCI: cpcihp: " Fahmy Hassan
2026-09-09  2:22     ` [PATCH v2 3/5] PCI: ibmphp: " Fahmy Hassan
2026-09-09  2:22     ` [PATCH v2 4/5] PCI: pciehp: " Fahmy Hassan
2026-09-09  6:42       ` Lukas Wunner [this message]
2026-09-09  2:22     ` [PATCH v2 5/5] PCI: shpchp: " Fahmy Hassan
2026-09-09  1:42 ` [PATCH v1] PCI: cpqphp: " sashiko-bot

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=aqD_3ULr3ls4S9IN@wunner.de \
    --to=lukas@wunner.de \
    --cc=bhelgaas@google.com \
    --cc=fahmymohammed@gmail.com \
    --cc=helgaas@kernel.org \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=scott@spiteful.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox