From: Bjorn Helgaas <helgaas@kernel.org>
To: Ryder Lee <ryder.lee@mediatek.com>,
Jianjun Wang <jianjun.wang@mediatek.com>
Cc: linux-pci@vger.kernel.org
Subject: possible issue in mtk_pcie_power_up()
Date: Tue, 5 May 2026 11:59:12 -0500 [thread overview]
Message-ID: <20260505165912.GA738617@bhelgaas> (raw)
Sashiko noticed this when reviewing another patchset:
https://sashiko.dev/#/patchset/20260430162420.42839-1-18255117159%40163.com
I'm just forwarding to you as an FYI.
Author: Hans Zhang <18255117159@163.com>
PCI: mediatek-gen3: Use FIELD_MODIFY()
This commit replaces open-coded bit manipulation with FIELD_MODIFY() in the
Mediatek Gen3 PCIe driver. It intends to introduce no functional changes.
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index b0accd8285892..f36a616a8b52e 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -494,8 +494,7 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
> /* Set Link Control 2 (LNKCTL2) speed restriction, if any */
> if (pcie->max_link_speed) {
> val = readl_relaxed(pcie->base + PCIE_CONF_LINK2_CTL_STS);
> - val &= ~PCIE_CONF_LINK2_LCR2_LINK_SPEED;
> - val |= FIELD_PREP(PCIE_CONF_LINK2_LCR2_LINK_SPEED, pcie->max_link_speed);
> + FIELD_MODIFY(PCIE_CONF_LINK2_LCR2_LINK_SPEED, &val, pcie->max_link_speed);
> writel_relaxed(val, pcie->base + PCIE_CONF_LINK2_CTL_STS);
> }
This isn't an issue introduced by this patch, but while reviewing
the driver I noticed a pre-existing issue in mtk_pcie_power_up() and
mtk_pcie_en7581_power_up().
Both functions call pm_runtime_get_sync(dev) without checking its
return value. If the device fails to resume, the function continues
execution and accesses hardware registers via readl_relaxed() and
writel_relaxed().
Could accessing unclocked registers here cause a synchronous
external abort and a kernel panic?
Additionally, pm_runtime_get_sync() increments the device usage
count even on failure. Since the current error paths do not call
pm_runtime_put_noidle(), does this leak the PM reference count?
Would it be better to use pm_runtime_resume_and_get(dev) instead,
and cleanly abort the initialization on failure?
reply other threads:[~2026-05-05 16:59 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20260505165912.GA738617@bhelgaas \
--to=helgaas@kernel.org \
--cc=jianjun.wang@mediatek.com \
--cc=linux-pci@vger.kernel.org \
--cc=ryder.lee@mediatek.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