From: sashiko-bot@kernel.org
To: "Alex Elder" <elder@riscstar.com>
Cc: devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
conor+dt@kernel.org, robh@kernel.org
Subject: Re: [PATCH v6 4/4] PCI: of: introduce of_pci_verify_node()
Date: Thu, 24 Sep 2026 15:09:34 +0000 [thread overview]
Message-ID: <20260924150934.B4CE31F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260924150222.1179235-5-elder@riscstar.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: of: unintended error-level logging in of_pci_verify_node()
--
commit 0e8b20efbbe63f895449027a5712b691b9eeaa56
Author: Alex Elder <elder@riscstar.com>
PCI: of: introduce of_pci_verify_node()
This commit introduces of_pci_verify_node() to verify the device_type property
for PCI devices. It adds a runtime check during device enumeration to warn if a
bridge lacks a "pci" or "pciex" device type or if an endpoint improperly
includes one.
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a51dff91b196d..5a040ed836744 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
[ ... ]
> +void of_pci_verify_node(struct pci_dev *pdev)
> +{
> + struct device_node *np = pci_device_to_OF_node(pdev);
> + bool device_is_bridge;
> + bool device_type_pci;
> +
> + /* Nothing to check if there's no pre-existing devicetree node */
> + if (!np)
> + return;
> +
> + device_is_bridge = pci_is_bridge(pdev);
> + device_type_pci = of_node_is_type(np, "pci") ||
> + of_node_is_type(np, "pciex");
> +
> + /* Bridges should have device type "pci"; endpoints should not */
> + if (device_is_bridge == device_type_pci)
> + return;
> +
> + dev_err(&pdev->dev, "PCI %s have \"pci\" device_type property\n",
> + device_is_bridge ? "bridge should" : "endpoint should not");
[Severity: Medium]
The commit message states that these warnings are "meant to help developers;
users should never see them."
However, dev_err() logs unconditionally at the error level in
of_pci_verify_node(), making it prominently visible to end users and
potentially failing automated test suites on boards with legacy devicetree
nodes.
Should dev_dbg() or dev_info() be used here instead to match the stated
developer-only intent?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260924150222.1179235-1-elder@riscstar.com?part=4
next prev parent reply other threads:[~2026-09-24 15:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-24 15:02 [PATCH v6 0/4] PCI: of: warn on bogus device_type property Alex Elder
2026-09-24 15:02 ` [PATCH v6 1/4] PCI: of: avoid allocations in of_pci_prop_compatible() Alex Elder
2026-09-24 15:10 ` sashiko-bot
2026-09-24 15:02 ` [PATCH v6 2/4] PCI: of: drop the reg_num argument to of_pci_set_address() Alex Elder
2026-09-24 15:08 ` sashiko-bot
2026-09-24 15:02 ` [PATCH v6 3/4] PCI: of: don't zero flags in of_pci_get_addr_flags() Alex Elder
2026-09-24 15:07 ` sashiko-bot
2026-09-24 15:02 ` [PATCH v6 4/4] PCI: of: introduce of_pci_verify_node() Alex Elder
2026-09-24 15:09 ` sashiko-bot [this message]
2026-09-24 15:11 ` Alex Elder
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=20260924150934.B4CE31F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=elder@riscstar.com \
--cc=linux-pci@vger.kernel.org \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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