All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
Cc: linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH 5/6] PCI/pwrctrl: tc9563: Rename DSP3 to VDSP
Date: Sat, 25 Jul 2026 09:11:04 +0000	[thread overview]
Message-ID: <20260725091105.440AB1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260725-tc9563-fix-v1-5-ec4286e31331@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced regulator_bulk_disable in probe error path and remove function.
--

--- Patch [5]: [PATCH 5/6] PCI/pwrctrl: tc9563: Rename DSP3 to VDSP ---
commit c88ec5f207551e44e4f9a4f5673e22f3dde64e6e
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

PCI/pwrctrl: tc9563: Rename DSP3 to VDSP

TC9563 Reference Manual calls DSP3 as VDSP (Virtual Downstream Port). Hence
name it as such to avoid ambiguity.

> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index 28e9ec3e172ae..f1179737f221f 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
[ ... ]
> @@ -612,10 +612,10 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
>  			break;
>  
>  		/*
> -		 * The integrated Ethernet MAC Endpoint under DSP3 is a single
> +		 * The integrated Ethernet MAC Endpoint under VDSP is a single
>  		 * device whose functions share the same config registers.
>  		 */
> -		if (port == TC9563_DSP3) {
> +		if (port == TC9563_VDSP) {
>  			struct device_node *eth __free(device_node) =
>  					of_get_next_available_child(child, NULL);
>  			if (eth) {

[Severity: High]
This isn't a bug introduced by this patch, but does the probe error path cause
an unbalanced regulator disable?

In tc9563_pwrctrl_probe(), if devm_pci_pwrctrl_device_set_ready() fails, the
code jumps to power_off:

	ret = devm_pci_pwrctrl_device_set_ready(dev, &tc9563->pwrctrl);
	if (ret)
		goto power_off;
...
power_off:
	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);

Since tc9563_pwrctrl_power_on() is meant to be called asynchronously by the
PCI subsystem and is never invoked during probe, wouldn't calling
tc9563_pwrctrl_power_off() here invoke regulator_bulk_disable() without a
preceding regulator_bulk_enable()?

This triggers a WARN_ON_ONCE in the regulator core, which crashes systems
with panic_on_warn enabled.

The same unconditional disable appears to occur in tc9563_pwrctrl_remove()
when unbinding the driver.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-tc9563-fix-v1-0-ec4286e31331@oss.qualcomm.com?part=5

  reply	other threads:[~2026-07-25  9:11 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-25  8:59 [PATCH 0/6] PCI/pwrctrl: tc9563: Several fixes and improvements Manivannan Sadhasivam via B4 Relay
2026-07-25  8:59 ` Manivannan Sadhasivam
2026-07-25  8:59 ` [PATCH 1/6] dt-bindings: PCI: toshiba,tc9563: Restrict Tx Amplitude, DFE and N_FTS to USP, DSP1 and DSP2 Manivannan Sadhasivam via B4 Relay
2026-07-25  8:59   ` Manivannan Sadhasivam
2026-07-25  9:12   ` sashiko-bot
2026-07-25  8:59 ` [PATCH 2/6] PCI/pwrctrl: tc9563: Fix parsing the integrated Ethernet MAC Endpoint node Manivannan Sadhasivam via B4 Relay
2026-07-25  8:59   ` Manivannan Sadhasivam
2026-07-25  9:09   ` sashiko-bot
2026-07-25  8:59 ` [PATCH 3/6] PCI/pwrctrl: tc9563: Power off only the external ports in tc9563_pwrctrl_disable_port() Manivannan Sadhasivam via B4 Relay
2026-07-25  8:59   ` Manivannan Sadhasivam
2026-07-25  9:07   ` sashiko-bot
2026-07-25  8:59 ` [PATCH 4/6] PCI/pwrctrl: tc9563: Skip Tx amplitude and DFE tuning for DSP3 Manivannan Sadhasivam via B4 Relay
2026-07-25  8:59   ` Manivannan Sadhasivam
2026-07-25  9:12   ` sashiko-bot
2026-07-25  8:59 ` [PATCH 5/6] PCI/pwrctrl: tc9563: Rename DSP3 to VDSP Manivannan Sadhasivam via B4 Relay
2026-07-25  8:59   ` Manivannan Sadhasivam
2026-07-25  9:11   ` sashiko-bot [this message]
2026-07-25  8:59 ` [PATCH 6/6] PCI/pwrctrl: tc9563: Move Integrated MAC Endpoint out of 'tc9563_pwrctrl_ports' enum Manivannan Sadhasivam via B4 Relay
2026-07-25  8:59   ` Manivannan Sadhasivam
2026-07-25  9:04   ` 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=20260725091105.440AB1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=manivannan.sadhasivam@oss.qualcomm.com \
    --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 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.