All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Maulik Shah" <maulik.shah@oss.qualcomm.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v4 1/7] irqchip/qcom-pdc: restructure version support
Date: Tue, 07 Jul 2026 09:31:34 +0000	[thread overview]
Message-ID: <20260707093134.CE2FE1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260707-hamoa_pdc_v3-v4-1-dfd1f4a3ae89@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The global `pdc_version` is left uninitialized, completely breaking interrupt enablement on PDC v3.2 devices due to executing the wrong code path.
--

--- Patch [1]: [PATCH v4 1/7] irqchip/qcom-pdc: restructure version support ---
commit 9e72443d7a81acfbf2e3c820dae28eecbd9b0db1
Author: Maulik Shah <maulik.shah@oss.qualcomm.com>

irqchip/qcom-pdc: restructure version support

This commit restructures the PDC driver by introducing a pdc_desc struct to
manage the base addresses, registers, and version information per device.
This prepares the driver to handle different hardware register layouts
and secondary controller modes more cleanly.

> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 2014dbb0bc43a..a77d1d3340629 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
[ ... ]
> @@ -383,6 +462,30 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
[ ... ]
> +	pdc = devm_kzalloc(dev, sizeof(*pdc), GFP_KERNEL);
> +	if (!pdc)
> +		return -ENOMEM;
[ ... ]
> +	pdc->version = pdc_reg_read(PDC_VERSION_REG, 0);
[ ... ]
> @@ -401,48 +505,29 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
[ ... ]
> -	pdc_version = pdc_reg_read(PDC_VERSION_REG, 0);
> -

[Severity: High]
Since the global pdc_version is no longer initialized here, won't it default
to 0?

If __pdc_enable_intr() still relies on the global pdc_version variable to
branch:

	if (pdc_version < PDC_VERSION_3_2)
		pdc_enable_intr_bank(pin_out, on);
	else
		pdc_enable_intr_cfg(pin_out, on);

will PDC v3.2 hardware incorrectly take the legacy bank configuration path?

Because pdc->regs->irq_en_reg is intentionally 0 for v3.2 devices, this
looks like it might read and write to offset 0 (the PDC base) instead of
the IRQ_CFG register, potentially corrupting unrelated registers and failing
to enable interrupts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-hamoa_pdc_v3-v4-0-dfd1f4a3ae89@oss.qualcomm.com?part=1

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

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  9:21 [PATCH v4 0/7] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
2026-07-07  9:21 ` [PATCH v4 1/7] irqchip/qcom-pdc: restructure version support Maulik Shah
2026-07-07  9:31   ` sashiko-bot [this message]
2026-07-07 20:33   ` [tip: irq/drivers] irqchip/qcom-pdc: Restructure " tip-bot2 for Maulik Shah
2026-07-07  9:21 ` [PATCH v4 2/7] irqchip/qcom-pdc: Move all statics to struct pdc_desc Maulik Shah
2026-07-07 20:33   ` [tip: irq/drivers] irqchip/qcom-pdc: Move all static variables " tip-bot2 for Maulik Shah
2026-07-07  9:21 ` [PATCH v4 3/7] irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI Maulik Shah
2026-07-07 20:32   ` [tip: irq/drivers] " tip-bot2 for Maulik Shah
2026-07-07  9:21 ` [PATCH v4 4/7] irqchip/qcom-pdc: Configure PDC to pass through mode Maulik Shah
2026-07-07  9:57   ` sashiko-bot
2026-07-07 20:32   ` [tip: irq/drivers] " tip-bot2 for Maulik Shah
2026-07-07  9:21 ` [PATCH v4 5/7] pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller Maulik Shah
2026-07-07  9:21 ` [PATCH v4 6/7] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now" Maulik Shah
2026-07-07 10:00   ` Krzysztof Kozlowski
2026-07-07  9:21 ` [PATCH v4 7/7] arm64: dts: qcom: x1e80100: Add deepest idle state Maulik Shah
2026-07-11 18:07   ` Bjorn Andersson
2026-07-07 20:33 ` [PATCH v4 0/7] x1e80100: Enable PDC wake GPIOs and " Thomas Gleixner
2026-07-08 11:47   ` Bartosz Golaszewski
2026-07-08 11:48 ` (subset) " Bartosz Golaszewski
2026-07-12 13:26 ` Daniel J Blueman

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=20260707093134.CE2FE1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=maulik.shah@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.