Devicetree
 help / color / mirror / Atom feed
From: "Maulik Shah (mkshah)" <maulik.shah@oss.qualcomm.com>
To: Thomas Gleixner <tglx@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Linus Walleij <linusw@kernel.org>
Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
	Sneh Mankad <sneh.mankad@oss.qualcomm.com>
Subject: Re: [PATCH v3 2/8] irqchip/qcom-pdc: Move all statics to struct pdc_desc
Date: Fri, 3 Jul 2026 14:16:43 +0530	[thread overview]
Message-ID: <afd17a5b-ab71-46b9-b537-c5e9b98653dc@oss.qualcomm.com> (raw)
In-Reply-To: <87jyrgqe5m.ffs@fw13>



On 6/30/2026 8:16 PM, Thomas Gleixner wrote:
> On Tue, Jun 16 2026 at 14:55, Maulik Shah wrote:
>> -		for (i = 0; i < pdc_region[n].cnt; i++)
>> -			__pdc_enable_intr(i + pdc_region[n].pin_base, 0);
>> +		for (int i = 0; i < pdc->region[n].cnt; i++)
>> +			pdc->enable_intr(i + pdc->region[n].pin_base, 0);
> 
> This needs a guard(raw_spinlock_irqsave)() when invoking
> pdc->enable_intr(). The probe function is only invoked
> with interrupts disabled during early boot. If it's called later, then
> this still works, but lockdep will be rightfully upset.
> 

Patch 3 of the series moved guard(raw_spinlock)() within the pdc->enable_intr().
I will merge patch 2 and patch 3 in v4 series so that lock movement and newly adding it at probe time is captured in single change.

Lock is required only for old PDC HW versions (v2.7 and v3.0) where enable bank is used instead of separate enable register for each IRQ.
Adding lock like below will apply the lock unnecessary on HW v3.2 specific pdc->enable_intr() as well which is initialized to pdc_enable_intr_cfg().

       guard(raw_spinlock_irqsave)(&pdc->lock);
       		pdc->enable_intr(i + pdc->region[n].pin_base, false);

To address this, lock is still kept within pdc->enable_intr() which is pointing to pdc_enable_intr_bank() for PDC HW v2.7 and v3.0.

Since probe gets invoked later during probe, i will keep guard(raw_spinlock_irqsave)() within pdc->enable_intr(),
even if chip callbacks like .irq_enable invoking pdc->enable_intr() may not need _irqsave() variant as it seems no better way as keeping
lock before calling pdc->enable_intr() will apply for PDC HW v3.2 as well.

Thanks,
Maulik

  reply	other threads:[~2026-07-03  8:46 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16  9:25 [PATCH v3 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
2026-06-16  9:25 ` [PATCH v3 1/8] irqchip/qcom-pdc: restructure version support Maulik Shah
2026-06-17 13:12   ` Konrad Dybcio
2026-06-25  9:19     ` Maulik Shah (mkshah)
2026-06-30 14:38   ` Thomas Gleixner
2026-07-03  8:18     ` Maulik Shah (mkshah)
2026-06-16  9:25 ` [PATCH v3 2/8] irqchip/qcom-pdc: Move all statics to struct pdc_desc Maulik Shah
2026-06-17 13:26   ` Konrad Dybcio
2026-06-25  9:19     ` Maulik Shah (mkshah)
2026-06-29  9:56       ` Konrad Dybcio
2026-06-30 14:46   ` Thomas Gleixner
2026-07-03  8:46     ` Maulik Shah (mkshah) [this message]
2026-06-16  9:25 ` [PATCH v3 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper Maulik Shah
2026-06-16  9:36   ` sashiko-bot
2026-06-28 17:54   ` Val Packett
2026-06-30  3:52     ` Maulik Shah (mkshah)
2026-06-16  9:25 ` [PATCH v3 4/8] irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI Maulik Shah
2026-06-18  8:02   ` Konrad Dybcio
2026-06-25  9:20     ` Maulik Shah (mkshah)
2026-06-30 14:57   ` Thomas Gleixner
2026-07-03  8:17     ` Maulik Shah (mkshah)
2026-06-16  9:25 ` [PATCH v3 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode Maulik Shah
2026-06-16  9:43   ` sashiko-bot
2026-06-18  8:18   ` Konrad Dybcio
2026-06-25  9:24     ` Maulik Shah (mkshah)
2026-06-18  8:19   ` Konrad Dybcio
2026-06-25  9:25     ` Maulik Shah (mkshah)
2026-06-30 15:07   ` Thomas Gleixner
2026-06-30 15:09     ` Thomas Gleixner
2026-07-03  9:20     ` Maulik Shah (mkshah)
2026-06-16  9:25 ` [PATCH v3 6/8] pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller Maulik Shah
2026-06-16  9:45   ` sashiko-bot
2026-06-16  9:25 ` [PATCH v3 7/8] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now" Maulik Shah
2026-06-16  9:25 ` [PATCH v3 8/8] arm64: dts: qcom: x1e80100: Add deepest idle state Maulik Shah
2026-06-18  8:25   ` Konrad Dybcio
2026-06-28 18:39 ` [PATCH v3 0/8] x1e80100: Enable PDC wake GPIOs and " Val Packett
2026-06-29  9:54   ` Konrad Dybcio
2026-07-03  9:30   ` Maulik Shah (mkshah)
2026-06-30 11:42 ` Linus Walleij
2026-06-30 14:34   ` Thomas Gleixner
2026-07-01  7:35     ` Bartosz Golaszewski

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=afd17a5b-ab71-46b9-b537-c5e9b98653dc@oss.qualcomm.com \
    --to=maulik.shah@oss.qualcomm.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sneh.mankad@oss.qualcomm.com \
    --cc=tglx@kernel.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