[PATCH 0/5] x1e80100: Enable PDC wake GPIOs and deepest idle state

[PATCH 0/5] x1e80100: Enable PDC wake GPIOs and deepest idle state

[Maulik Shah]

There are two modes PDC irqchip can work in
        - pass through mode
        - secondary controller mode

All PDC irqchip supports pass through mode in which both Direct SPIs and
GPIO IRQs (as SPIs) are sent to GIC without latching at PDC, PDC only does
inversion when needed for falling edge to rising edge or level low to level
high, as the GIC do not support falling edge/level low interrupts.

Newer PDCs (v3.0 onwards) also support additional secondary controller mode
where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
still works same as pass through mode without latching at PDC even in
secondary controller mode.

All the SoCs defaulted to pass through mode with the exception of some x1e.

x1e PDC may be set to secondary controller mode for builds on CRD boards
whereas it may be set to pass through mode for IoT-EVK boards.

There is no way to read which current mode it is set to and make PDC work
in respective mode as the read access is not opened up for non secure
world. There is though write access opened up via SCM write API to set the
mode.

As the linux only ever makes use of pass through mode, set the IRQ mask
meant specifically for secondary controller mode to mask all the IRQs to be
forwarded to GIC irrespective of the mode PDC is set to. Writing the mask
is do not care when the PDC works in pass through mode, which is always
the case except for some of x1e platforms.

Configure PDC mode to pass through mode for all x1e based boards via SCM
write.

For successful write:
        - Nothing more to be done
For unsuccessful write:
        - Inform TLMM to monitor GPIO IRQs (same as MPM)
        - Prevent SoC low power mode (CxPC) as PDC is not monitoring GPIO
          IRQs which may be needed to wake the SoC from low power mode.

As the deepest idle state as the PDC can now wake up SoC from GPIOs and
revert 602cb14e310a ("pinctrl: qcom: x1e80100: Bypass PDC wakeup parent
for now").

Note:
For testing this series on x1e80100 CRD, interconnect nodes from SCM
device are removed as PDC requires SCM APIs early in the boot up and
interconnect nodes delays the probe of SCM device which results in early
boot NULL pointer derefernce. Looking at documentation interconnect are
added to get additional performance boost and are optional to add. Removing
them for now allows this series to go through until proper fix from SCM
device is found.

The series has been tested on x1e80100 CRD with both old and new firmware
and also on kaanapali.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
Maulik Shah (5):
      arm64: dts: qcom: x1e80100: Remove interconnect from SCM device
      dt-bindings: interrupt-controller: qcom,pdc: Document reg and QMP
      irqchip/qcom-pdc: Configure PDC to pass through mode
      arm64: dts: qcom: x1e80100: Add deepest idle state
      Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"

 .../bindings/interrupt-controller/qcom,pdc.yaml    |   5 +
 arch/arm64/boot/dts/qcom/hamoa.dtsi                |  19 +++-
 drivers/irqchip/Kconfig                            |   1 +
 drivers/irqchip/qcom-pdc.c                         | 119 +++++++++++++++++++--
 drivers/pinctrl/qcom/pinctrl-x1e80100.c            |   4 +-
 5 files changed, 131 insertions(+), 17 deletions(-)
---
base-commit: f90aadf1c67c8b4969d1e5e6d4fd7227adb6e4d7
change-id: 20260312-hamoa_pdc-8c44e70b1517

Best regards,
-- 
Maulik Shah <maulik.shah@oss.qualcomm.com>

[PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Maulik Shah]

Interconnect from SCM device are optional and were added to get
additional performance benefit. These nodes however delays the
SCM firmware device probe due to dependency on interconnect and
results in NULL pointer dereference for the users of SCM device
driver APIs, such as PDC driver.

Remove them from the scm device to unblock the user.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/hamoa.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index d7596ccf63b90a8a002ad6e77c0fb2c1b32ec9c8..ebecf43e0d462c431540257e299e3ace054901fd 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -308,8 +308,7 @@ eud_in: endpoint {
 	firmware {
 		scm: scm {
 			compatible = "qcom,scm-x1e80100", "qcom,scm";
-			interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
-					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+			/* TODO: add interconnects */
 			qcom,dload-mode = <&tcsr 0x19000>;
 		};
 

-- 
2.34.1

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
> Interconnect from SCM device are optional and were added to get
> additional performance benefit. These nodes however delays the
> SCM firmware device probe due to dependency on interconnect and
> results in NULL pointer dereference for the users of SCM device
> driver APIs, such as PDC driver.

This sounds like a bug in the PDC driver. It should reject being probed
before SCM is available.

> 
> Remove them from the scm device to unblock the user.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/hamoa.dtsi | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index d7596ccf63b90a8a002ad6e77c0fb2c1b32ec9c8..ebecf43e0d462c431540257e299e3ace054901fd 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -308,8 +308,7 @@ eud_in: endpoint {
>  	firmware {
>  		scm: scm {
>  			compatible = "qcom,scm-x1e80100", "qcom,scm";
> -			interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
> -					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> +			/* TODO: add interconnects */

Somebody will try to fix this TODO, reverting this patch. Let's find a
better way to handle it (which would also fit other platforms).
Originaly this was proposed by Sibi ([1]) to speed up PAS
authentication. Other platforms require RPM or GCC clocks to let the
firmware access crypto core.

One of the (stupid) ideas would be to add a separate SCM (child?) device
which would be used for crypto-related SCM calls. I'd like to point out
that currently we bump those clocks or NoC bandwidth, but at the same
time we don't vote on the CX rail. I'm not sure of the firmware handles
that somehow or not.

[1] https://lore.kernel.org/all/1653289258-17699-1-git-send-email-quic_sibis@quicinc.com/

>  			qcom,dload-mode = <&tcsr 0x19000>;
>  		};
>  
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Maulik Shah (mkshah)]

On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
>> Interconnect from SCM device are optional and were added to get
>> additional performance benefit. These nodes however delays the
>> SCM firmware device probe due to dependency on interconnect and
>> results in NULL pointer dereference for the users of SCM device
>> driver APIs, such as PDC driver.
> 
> This sounds like a bug in the PDC driver. It should reject being probed
> before SCM is available.

The SCM driver provides no way to check if its ready or not to decide to reject/defer the probe.
A new API like below would be needed here,

int qcom_scm_ready(void)
{
        if (__scm == NULL || __scm->dev == NULL)
                return -EPROBE_DEFER;
        return 0;
}
EXPORT_SYMBOL_GPL(qcom_scm_ready);

This is inline with what cmd-db does today with cmd_db_ready() API.
(drivers/soc/qcom/cmd-db.c).

> 
>>
>> Remove them from the scm device to unblock the user.
>>
>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>> ---
>>  arch/arm64/boot/dts/qcom/hamoa.dtsi | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
>> index d7596ccf63b90a8a002ad6e77c0fb2c1b32ec9c8..ebecf43e0d462c431540257e299e3ace054901fd 100644
>> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
>> @@ -308,8 +308,7 @@ eud_in: endpoint {
>>  	firmware {
>>  		scm: scm {
>>  			compatible = "qcom,scm-x1e80100", "qcom,scm";
>> -			interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
>> -					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>> +			/* TODO: add interconnects */
> 
> Somebody will try to fix this TODO, reverting this patch. Let's find a
> better way to handle it (which would also fit other platforms).
> Originaly this was proposed by Sibi ([1]) to speed up PAS
> authentication. Other platforms require RPM or GCC clocks to let the
> firmware access crypto core.
> 
> One of the (stupid) ideas would be to add a separate SCM (child?) device
> which would be used for crypto-related SCM calls. I'd like to point out
> that currently we bump those clocks or NoC bandwidth, but at the same
> time we don't vote on the CX rail. I'm not sure of the firmware handles
> that somehow or not.

Nice catch, AFAIK firmware don't handle voting for CX rail during SCM call.

> 
> [1] https://lore.kernel.org/all/1653289258-17699-1-git-send-email-quic_sibis@quicinc.com/

yes, I had already seen this,

So remoteproc PAS driver gets performance benefit with crypto vote and interesting choice was
made to place it from SCM driver. It was evaluated and considered reasonable one at that time,
pasting from [2],
The clocking needs for the CE relates to the SCM and not the remoteproc, and it's in line with
the management of CE clocks from the SCM driver.

With my limited understanding of remoteproc, SCM and crypto,

- A crypto vote would no way bump up the performance of CPU jumping from/to non-secure and secure world.
  (actual "path" of SCM driver).

  if remoteproc requires the crypto vote for image validation/authentication then remoteproc should
  place the vote for crypto path before invoking SCM APIs, SCM don't really use this vote for itself.
  SCM driver though today adds/removes vote within remoteproc APIs keeping vote placement limited
  to remoteproc usage only.

- Firmware could have put the crypto vote if firmware is doing image validation/authentication
  after the SCM call lands in firmware and remove it before returning to non-secure world.
  clearly not a choice now to update firmware.

- I see crypto device too places same vote (at least on x1e) so i must be missing something and
  both SCM and crypto device vote are needed here. I was thinking if remoteproc should route the
  SCM call via crypto driver (which would places the required crypto vote) and crypto driver
  should then invoke the crypto related SMC calls.

  crypto: crypto@1dfa000 {
  	compatible = "qcom,x1e80100-qce", "qcom,sm8150-qce", "qcom,qce";
	..
        interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
                        &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
  };

Let me know any preferences from below options or any other.

a) Add the API like qcom_scm_ready(), this has been tested and works fine.
b) Move interconnects from SCM to remoteproc PAS driver for all devices
   Take the vote before invoking SCM API and release after return.
c) Remove the interconnects from SCM and rely on crypto driver already
   placing the vote, Route the remote proc to SCM call via crypto API,
   This would ensure crpyto is being used and it would have placed the required vote.
d) Add separate SCM child device (with interconnects) under SoC.

[2] https://lore.kernel.org/all/Yr0Os5TOITY7f0Wk@builder.lan/

Thanks,
Maulik

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Konrad Dybcio]

On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
> 
> 
> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
>>> Interconnect from SCM device are optional and were added to get
>>> additional performance benefit. These nodes however delays the
>>> SCM firmware device probe due to dependency on interconnect and
>>> results in NULL pointer dereference for the users of SCM device
>>> driver APIs, such as PDC driver.
>>
>> This sounds like a bug in the PDC driver. It should reject being probed
>> before SCM is available.
> 
> The SCM driver provides no way to check if its ready or not to decide to reject/defer the probe.
> A new API like below would be needed here,

There is, qcom_scm_is_available()


> Let me know any preferences from below options or any other.
> 
> a) Add the API like qcom_scm_ready(), this has been tested and works fine.
> b) Move interconnects from SCM to remoteproc PAS driver for all devices
>    Take the vote before invoking SCM API and release after return.

I think this is not the right decision. The crypto path is only necessary,
because cryptographic checks must be carried out in the TZ in order to
(dis)allow a certain firmware binary. This is not a characteristic of the
remoteprocs themselves, as with a non-prudent TZ, the firmware loading
would amount to a memcpy() (and some SMMU/XPU configs via reg writes)

> c) Remove the interconnects from SCM and rely on crypto driver already
>    placing the vote, Route the remote proc to SCM call via crypto API,
>    This would ensure crpyto is being used and it would have placed the required vote.

I think this would make things even worse, because instead of waiting on
the interconnect driver, we'd now have to wait on the interconnect driver,
the clock driver and the crypto driver

> d) Add separate SCM child device (with interconnects) under SoC.

We'd then have to probe it as an aux device or something, which would
either delay the probing of SCM, or introduce the need to ping-pong for
PAS availability between the API provider and consumer, since some calls
work perfectly fine without the ICC path, while others could really use
it

Konrad

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Dmitry Baryshkov]

On Fri, Mar 13, 2026 at 12:59:46PM +0100, Konrad Dybcio wrote:
> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
> > On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
> >> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
> 
> > d) Add separate SCM child device (with interconnects) under SoC.
> 
> We'd then have to probe it as an aux device or something, which would
> either delay the probing of SCM, or introduce the need to ping-pong for
> PAS availability between the API provider and consumer, since some calls
> work perfectly fine without the ICC path, while others could really use
> it

qcom_scm_pas_is_available() ?

-- 
With best wishes
Dmitry

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Konrad Dybcio]

On 3/13/26 3:48 PM, Dmitry Baryshkov wrote:
> On Fri, Mar 13, 2026 at 12:59:46PM +0100, Konrad Dybcio wrote:
>> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
>>> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
>>>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
>>
>>> d) Add separate SCM child device (with interconnects) under SoC.
>>
>> We'd then have to probe it as an aux device or something, which would
>> either delay the probing of SCM, or introduce the need to ping-pong for
>> PAS availability between the API provider and consumer, since some calls
>> work perfectly fine without the ICC path, while others could really use
>> it
> 
> qcom_scm_pas_is_available() ?

This comes back to either having to wait for the interconnect provider
anyway, or allowing the ICC-enhanced calls to take place before they that
happens, stripping us of the benefits.

Konrad

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Dmitry Baryshkov]

On Mon, Mar 16, 2026 at 10:39:09AM +0100, Konrad Dybcio wrote:
> On 3/13/26 3:48 PM, Dmitry Baryshkov wrote:
> > On Fri, Mar 13, 2026 at 12:59:46PM +0100, Konrad Dybcio wrote:
> >> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
> >>> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
> >>>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
> >>
> >>> d) Add separate SCM child device (with interconnects) under SoC.
> >>
> >> We'd then have to probe it as an aux device or something, which would
> >> either delay the probing of SCM, or introduce the need to ping-pong for
> >> PAS availability between the API provider and consumer, since some calls
> >> work perfectly fine without the ICC path, while others could really use
> >> it
> > 
> > qcom_scm_pas_is_available() ?
> 
> This comes back to either having to wait for the interconnect provider
> anyway, or allowing the ICC-enhanced calls to take place before they that
> happens, stripping us of the benefits.

Yes. However this way only the PAS users will have to wait (i.e.
remoteprocs, venus, IPA, etc.). All the basic providers would be able to
probe.

-- 
With best wishes
Dmitry

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Konrad Dybcio]

On 3/16/26 3:25 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 16, 2026 at 10:39:09AM +0100, Konrad Dybcio wrote:
>> On 3/13/26 3:48 PM, Dmitry Baryshkov wrote:
>>> On Fri, Mar 13, 2026 at 12:59:46PM +0100, Konrad Dybcio wrote:
>>>> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
>>>>> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
>>>>>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
>>>>
>>>>> d) Add separate SCM child device (with interconnects) under SoC.
>>>>
>>>> We'd then have to probe it as an aux device or something, which would
>>>> either delay the probing of SCM, or introduce the need to ping-pong for
>>>> PAS availability between the API provider and consumer, since some calls
>>>> work perfectly fine without the ICC path, while others could really use
>>>> it
>>>
>>> qcom_scm_pas_is_available() ?
>>
>> This comes back to either having to wait for the interconnect provider
>> anyway, or allowing the ICC-enhanced calls to take place before they that
>> happens, stripping us of the benefits.
> 
> Yes. However this way only the PAS users will have to wait (i.e.
> remoteprocs, venus, IPA, etc.). All the basic providers would be able to
> probe.

Do you then envision a separate qcom_scm_pil_is_available()?

Konrad

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Dmitry Baryshkov]

On Wed, Mar 18, 2026 at 10:33:28AM +0100, Konrad Dybcio wrote:
> On 3/16/26 3:25 PM, Dmitry Baryshkov wrote:
> > On Mon, Mar 16, 2026 at 10:39:09AM +0100, Konrad Dybcio wrote:
> >> On 3/13/26 3:48 PM, Dmitry Baryshkov wrote:
> >>> On Fri, Mar 13, 2026 at 12:59:46PM +0100, Konrad Dybcio wrote:
> >>>> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
> >>>>> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
> >>>>>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
> >>>>
> >>>>> d) Add separate SCM child device (with interconnects) under SoC.
> >>>>
> >>>> We'd then have to probe it as an aux device or something, which would
> >>>> either delay the probing of SCM, or introduce the need to ping-pong for
> >>>> PAS availability between the API provider and consumer, since some calls
> >>>> work perfectly fine without the ICC path, while others could really use
> >>>> it
> >>>
> >>> qcom_scm_pas_is_available() ?
> >>
> >> This comes back to either having to wait for the interconnect provider
> >> anyway, or allowing the ICC-enhanced calls to take place before they that
> >> happens, stripping us of the benefits.
> > 
> > Yes. However this way only the PAS users will have to wait (i.e.
> > remoteprocs, venus, IPA, etc.). All the basic providers would be able to
> > probe.
> 
> Do you then envision a separate qcom_scm_pil_is_available()?

Which calls would it guard?

> 
> Konrad

-- 
With best wishes
Dmitry

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Konrad Dybcio]

On 3/18/26 11:38 AM, Dmitry Baryshkov wrote:
> On Wed, Mar 18, 2026 at 10:33:28AM +0100, Konrad Dybcio wrote:
>> On 3/16/26 3:25 PM, Dmitry Baryshkov wrote:
>>> On Mon, Mar 16, 2026 at 10:39:09AM +0100, Konrad Dybcio wrote:
>>>> On 3/13/26 3:48 PM, Dmitry Baryshkov wrote:
>>>>> On Fri, Mar 13, 2026 at 12:59:46PM +0100, Konrad Dybcio wrote:
>>>>>> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
>>>>>>> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
>>>>>>>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
>>>>>>
>>>>>>> d) Add separate SCM child device (with interconnects) under SoC.
>>>>>>
>>>>>> We'd then have to probe it as an aux device or something, which would
>>>>>> either delay the probing of SCM, or introduce the need to ping-pong for
>>>>>> PAS availability between the API provider and consumer, since some calls
>>>>>> work perfectly fine without the ICC path, while others could really use
>>>>>> it
>>>>>
>>>>> qcom_scm_pas_is_available() ?
>>>>
>>>> This comes back to either having to wait for the interconnect provider
>>>> anyway, or allowing the ICC-enhanced calls to take place before they that
>>>> happens, stripping us of the benefits.
>>>
>>> Yes. However this way only the PAS users will have to wait (i.e.
>>> remoteprocs, venus, IPA, etc.). All the basic providers would be able to
>>> probe.
>>
>> Do you then envision a separate qcom_scm_pil_is_available()?
> 
> Which calls would it guard?

pil/pas/whatever.. auth_and_reset(), probably

Konrad

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Dmitry Baryshkov]

On Wed, Mar 18, 2026 at 11:39:12AM +0100, Konrad Dybcio wrote:
> On 3/18/26 11:38 AM, Dmitry Baryshkov wrote:
> > On Wed, Mar 18, 2026 at 10:33:28AM +0100, Konrad Dybcio wrote:
> >> On 3/16/26 3:25 PM, Dmitry Baryshkov wrote:
> >>> On Mon, Mar 16, 2026 at 10:39:09AM +0100, Konrad Dybcio wrote:
> >>>> On 3/13/26 3:48 PM, Dmitry Baryshkov wrote:
> >>>>> On Fri, Mar 13, 2026 at 12:59:46PM +0100, Konrad Dybcio wrote:
> >>>>>> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
> >>>>>>> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
> >>>>>>>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
> >>>>>>
> >>>>>>> d) Add separate SCM child device (with interconnects) under SoC.
> >>>>>>
> >>>>>> We'd then have to probe it as an aux device or something, which would
> >>>>>> either delay the probing of SCM, or introduce the need to ping-pong for
> >>>>>> PAS availability between the API provider and consumer, since some calls
> >>>>>> work perfectly fine without the ICC path, while others could really use
> >>>>>> it
> >>>>>
> >>>>> qcom_scm_pas_is_available() ?
> >>>>
> >>>> This comes back to either having to wait for the interconnect provider
> >>>> anyway, or allowing the ICC-enhanced calls to take place before they that
> >>>> happens, stripping us of the benefits.
> >>>
> >>> Yes. However this way only the PAS users will have to wait (i.e.
> >>> remoteprocs, venus, IPA, etc.). All the basic providers would be able to
> >>> probe.
> >>
> >> Do you then envision a separate qcom_scm_pil_is_available()?
> > 
> > Which calls would it guard?
> 
> pil/pas/whatever.. auth_and_reset(), probably

I see only PAS calls.

-- 
With best wishes
Dmitry

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Maulik Shah (mkshah)]

On 3/13/2026 5:29 PM, Konrad Dybcio wrote:
> On 3/13/26 11:12 AM, Maulik Shah (mkshah) wrote:
>>
>>
>> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
>>> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
>>>> Interconnect from SCM device are optional and were added to get
>>>> additional performance benefit. These nodes however delays the
>>>> SCM firmware device probe due to dependency on interconnect and
>>>> results in NULL pointer dereference for the users of SCM device
>>>> driver APIs, such as PDC driver.
>>>
>>> This sounds like a bug in the PDC driver. It should reject being probed
>>> before SCM is available.
>>
>> The SCM driver provides no way to check if its ready or not to decide to reject/defer the probe.
>> A new API like below would be needed here,
> 
> There is, qcom_scm_is_available()

Thanks, i will use this API in v2 to defer the probe and drop this patch.
Deferring still delays PDC probe significantly but it would unblock this series.

> 
> 
>> Let me know any preferences from below options or any other.
>>
>> a) Add the API like qcom_scm_ready(), this has been tested and works fine.
>> b) Move interconnects from SCM to remoteproc PAS driver for all devices
>>    Take the vote before invoking SCM API and release after return.
> 
> I think this is not the right decision. The crypto path is only necessary,
> because cryptographic checks must be carried out in the TZ in order to
> (dis)allow a certain firmware binary. This is not a characteristic of the
> remoteprocs themselves, as with a non-prudent TZ, the firmware loading
> would amount to a memcpy() (and some SMMU/XPU configs via reg writes)

This does not seem to be a characteristic of SCM either.

Loading and booting the firmware is part of remoteproc and not SCM.
(Documentation/devicetree/bindings/remoteproc/*)
The votes required to (dis)allow loading them faster (such as crpyto) should also fall
under remoteproc otherwise any driver requiring SCM API (for other purposes) would put
the burden of placing votes on SCM driver?

> 
>> c) Remove the interconnects from SCM and rely on crypto driver already
>>    placing the vote, Route the remote proc to SCM call via crypto API,
>>    This would ensure crpyto is being used and it would have placed the required vote.
> 
> I think this would make things even worse, because instead of waiting on
> the interconnect driver, we'd now have to wait on the interconnect driver,
> the clock driver and the crypto driver

okay, i was just wondering if crypto vote can somehow be leveraged so SCM do not need
to place the vote.

Thanks,
Maulik

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Dmitry Baryshkov]

On Fri, Mar 13, 2026 at 03:42:32PM +0530, Maulik Shah (mkshah) wrote:
> 
> 
> On 3/13/2026 7:41 AM, Dmitry Baryshkov wrote:
> > On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
> >> Interconnect from SCM device are optional and were added to get
> >> additional performance benefit. These nodes however delays the
> >> SCM firmware device probe due to dependency on interconnect and
> >> results in NULL pointer dereference for the users of SCM device
> >> driver APIs, such as PDC driver.
> > 
> > This sounds like a bug in the PDC driver. It should reject being probed
> > before SCM is available.
> 
> The SCM driver provides no way to check if its ready or not to decide to reject/defer the probe.
> A new API like below would be needed here,

qcom_scm_is_available() ?

> 
> int qcom_scm_ready(void)
> {
>         if (__scm == NULL || __scm->dev == NULL)
>                 return -EPROBE_DEFER;
>         return 0;
> }
> EXPORT_SYMBOL_GPL(qcom_scm_ready);
> 
> This is inline with what cmd-db does today with cmd_db_ready() API.
> (drivers/soc/qcom/cmd-db.c).
> 
> > 
> >>
> >> Remove them from the scm device to unblock the user.
> >>
> >> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> >> ---
> >>  arch/arm64/boot/dts/qcom/hamoa.dtsi | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> >> index d7596ccf63b90a8a002ad6e77c0fb2c1b32ec9c8..ebecf43e0d462c431540257e299e3ace054901fd 100644
> >> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> >> @@ -308,8 +308,7 @@ eud_in: endpoint {
> >>  	firmware {
> >>  		scm: scm {
> >>  			compatible = "qcom,scm-x1e80100", "qcom,scm";
> >> -			interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
> >> -					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> >> +			/* TODO: add interconnects */
> > 
> > Somebody will try to fix this TODO, reverting this patch. Let's find a
> > better way to handle it (which would also fit other platforms).
> > Originaly this was proposed by Sibi ([1]) to speed up PAS
> > authentication. Other platforms require RPM or GCC clocks to let the
> > firmware access crypto core.
> > 
> > One of the (stupid) ideas would be to add a separate SCM (child?) device
> > which would be used for crypto-related SCM calls. I'd like to point out
> > that currently we bump those clocks or NoC bandwidth, but at the same
> > time we don't vote on the CX rail. I'm not sure of the firmware handles
> > that somehow or not.
> 
> Nice catch, AFAIK firmware don't handle voting for CX rail during SCM call.
> 
> > 
> > [1] https://lore.kernel.org/all/1653289258-17699-1-git-send-email-quic_sibis@quicinc.com/
> 
> yes, I had already seen this,
> 
> So remoteproc PAS driver gets performance benefit with crypto vote and interesting choice was
> made to place it from SCM driver. It was evaluated and considered reasonable one at that time,
> pasting from [2],
> The clocking needs for the CE relates to the SCM and not the remoteproc, and it's in line with
> the management of CE clocks from the SCM driver.

I agree that those clocks must be managed, but I think it was a hack to
reuse SCM's iface / bus clocks for crypto. Originally, *I suppose* were
added for very old platforms which had separate DAYTONA NoC clock, most
likely controlling access to some of the backing hardware, but not
necessarily crypto hardware.

> 
> With my limited understanding of remoteproc, SCM and crypto,
> 
> - A crypto vote would no way bump up the performance of CPU jumping from/to non-secure and secure world.
>   (actual "path" of SCM driver).
> 
>   if remoteproc requires the crypto vote for image validation/authentication then remoteproc should
>   place the vote for crypto path before invoking SCM APIs, SCM don't really use this vote for itself.
>   SCM driver though today adds/removes vote within remoteproc APIs keeping vote placement limited
>   to remoteproc usage only.

Looking at the code, I'd assume that once we start testing HDCP we'd
perform the same for the HDCP-related calls. The problem is that this
kind of management also doesn't seem to belong to the remoteproc driver:
it doesn't know and it should be of no concern for it if the firmware
uses crypto behind its back or not.

> - Firmware could have put the crypto vote if firmware is doing image validation/authentication
>   after the SCM call lands in firmware and remove it before returning to non-secure world.
>   clearly not a choice now to update firmware.
> 
> - I see crypto device too places same vote (at least on x1e) so i must be missing something and
>   both SCM and crypto device vote are needed here. I was thinking if remoteproc should route the
>   SCM call via crypto driver (which would places the required crypto vote) and crypto driver
>   should then invoke the crypto related SMC calls.

I think, this also looks like a hammer plumbing. The use of crypto
device for those calls is a firmware implementation detail.

> 
>   crypto: crypto@1dfa000 {
>   	compatible = "qcom,x1e80100-qce", "qcom,sm8150-qce", "qcom,qce";
> 	..
>         interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
>                         &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>   };
> 
> Let me know any preferences from below options or any other.
> 
> a) Add the API like qcom_scm_ready(), this has been tested and works fine.

We already have qcom_scm_is_available().

> b) Move interconnects from SCM to remoteproc PAS driver for all devices
>    Take the vote before invoking SCM API and release after return.
> c) Remove the interconnects from SCM and rely on crypto driver already
>    placing the vote, Route the remote proc to SCM call via crypto API,
>    This would ensure crpyto is being used and it would have placed the required vote.
> d) Add separate SCM child device (with interconnects) under SoC.

This is going to be my preference, but I'm ready to listen for other
opinions.

> 
> [2] https://lore.kernel.org/all/Yr0Os5TOITY7f0Wk@builder.lan/
> 
> Thanks,
> Maulik

-- 
With best wishes
Dmitry

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Krzysztof Kozlowski]

On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
> Interconnect from SCM device are optional and were added to get
> additional performance benefit. These nodes however delays the
> SCM firmware device probe due to dependency on interconnect and

So fix drivers.

> results in NULL pointer dereference for the users of SCM device
> driver APIs, such as PDC driver.
> 
> Remove them from the scm device to unblock the user.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/hamoa.dtsi | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index d7596ccf63b90a8a002ad6e77c0fb2c1b32ec9c8..ebecf43e0d462c431540257e299e3ace054901fd 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -308,8 +308,7 @@ eud_in: endpoint {
>  	firmware {
>  		scm: scm {
>  			compatible = "qcom,scm-x1e80100", "qcom,scm";
> -			interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
> -					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> +			/* TODO: add interconnects */

NAK, interconnects were there already. So after applying your patch I
can just revert it immediately solving the TODO.

Best regards,
Krzysztof

Re: [PATCH 1/5] arm64: dts: qcom: x1e80100: Remove interconnect from SCM device

[Maulik Shah (mkshah)]

On 3/13/2026 7:26 PM, Krzysztof Kozlowski wrote:
> On Thu, Mar 12, 2026 at 09:26:35PM +0530, Maulik Shah wrote:
>> Interconnect from SCM device are optional and were added to get
>> additional performance benefit. These nodes however delays the
>> SCM firmware device probe due to dependency on interconnect and
> 
> So fix drivers.

Yes, will address in v2.

> 
>> results in NULL pointer dereference for the users of SCM device
>> driver APIs, such as PDC driver.
>>
>> Remove them from the scm device to unblock the user.
>>
>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>> ---
>>  arch/arm64/boot/dts/qcom/hamoa.dtsi | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
>> index d7596ccf63b90a8a002ad6e77c0fb2c1b32ec9c8..ebecf43e0d462c431540257e299e3ace054901fd 100644
>> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
>> @@ -308,8 +308,7 @@ eud_in: endpoint {
>>  	firmware {
>>  		scm: scm {
>>  			compatible = "qcom,scm-x1e80100", "qcom,scm";
>> -			interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
>> -					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>> +			/* TODO: add interconnects */
> 
> NAK, interconnects were there already. So after applying your patch I
> can just revert it immediately solving the TODO.

Yes, this change will be dropped from v2 and instead will be using qcom_scm_is_available()
to handle the dependencies.

Thanks,
Maulik

[PATCH 2/5] dt-bindings: interrupt-controller: qcom,pdc: Document reg and QMP

[Maulik Shah]

Document PDC reg to configure pass through or secondary controller mode
for GPIO IRQs.
Document QMP handle for action concerning global resources.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
index 5ad68b2c6fc630fb4044c7224e6791d3bf4c2937..00eb9b28170c29c811c17b1f02f1f4f14779752f 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
@@ -62,6 +62,7 @@ properties:
     items:
       - description: PDC base register region
       - description: Edge or Level config register for SPI interrupts
+      - description: PDC config for pass through or secondary IRQ mode for GPIOs
 
   '#interrupt-cells':
     const: 2
@@ -82,6 +83,10 @@ properties:
       The tuples indicates the valid mapping of valid PDC ports
       and their hwirq mapping.
 
+  qcom,qmp:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Reference to the AOSS side-channel message RAM.
+
 required:
   - compatible
   - reg

-- 
2.34.1

Re: [PATCH 2/5] dt-bindings: interrupt-controller: qcom,pdc: Document reg and QMP

[Krzysztof Kozlowski]

On Thu, Mar 12, 2026 at 09:26:36PM +0530, Maulik Shah wrote:
> Document PDC reg to configure pass through or secondary controller mode
> for GPIO IRQs.
> Document QMP handle for action concerning global resources.

Don't explain what you did. We can read the diff. Explain WHY you are
doing this.

> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
> index 5ad68b2c6fc630fb4044c7224e6791d3bf4c2937..00eb9b28170c29c811c17b1f02f1f4f14779752f 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
> @@ -62,6 +62,7 @@ properties:
>      items:
>        - description: PDC base register region
>        - description: Edge or Level config register for SPI interrupts
> +      - description: PDC config for pass through or secondary IRQ mode for GPIOs

I do not understand why all devices have suddenly grown one more MMIO
region, including devices for few years in the market. Nothing in commit
msg helped me to understand that.

Best regards,
Krzysztof

[PATCH 3/5] irqchip/qcom-pdc: Configure PDC to pass through mode

[Maulik Shah]

There are two modes PDC irqchip supports pass through mode and secondary
controller mode.

All PDC irqchip supports pass through mode in which both Direct SPIs and
GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.

Newer PDCs (v3.0 onwards) also support additional secondary controller mode
where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
still works same as pass through mode without latching at PDC even in
secondary controller mode.

All the SoCs so far default uses pass through mode with the exception of
x1e. x1e PDC may be set to secondary controller mode for builds on CRD
boards whereas it may be set to pass through mode for IoT-EVK.

There is no way to read which current mode it is set to and make PDC work
in respective mode as the read access is not opened up for non secure
world. There is though write access opened up via SCM write API to set the
mode.

Configure PDC mode to pass through mode for all x1e based boards via SCM
write.

Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 drivers/irqchip/Kconfig    |   1 +
 drivers/irqchip/qcom-pdc.c | 119 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 111 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 83d333f8bf63d78827800e0de724f81e6aa2f1df..89caddf6e5c569a0e867cda1838c870b967fb13d 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -505,6 +505,7 @@ config GOLDFISH_PIC
 config QCOM_PDC
 	tristate "QCOM PDC"
 	depends on ARCH_QCOM
+	depends on QCOM_AOSS_QMP
 	select IRQ_DOMAIN_HIERARCHY
 	help
 	  Power Domain Controller driver to manage and configure wakeup
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 32b77fa93f730416edf120710bcdcdce33fa39a7..051700d672471c092e8cda4d7f5aa6d2032157f7 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -19,6 +19,8 @@
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/soc/qcom/qcom_aoss.h>
 
 #define PDC_MAX_GPIO_IRQS	256
 #define PDC_DRV_OFFSET		0x10000
@@ -26,9 +28,11 @@
 /* Valid only on HW version < 3.2 */
 #define IRQ_ENABLE_BANK		0x10
 #define IRQ_ENABLE_BANK_MAX	(IRQ_ENABLE_BANK + BITS_TO_BYTES(PDC_MAX_GPIO_IRQS))
+#define IRQ_i_CFG_IRQ_MASK_3_0	3
 #define IRQ_i_CFG		0x110
 
 /* Valid only on HW version >= 3.2 */
+#define IRQ_i_CFG_IRQ_MASK_3_2	4
 #define IRQ_i_CFG_IRQ_ENABLE	3
 
 #define IRQ_i_CFG_TYPE_MASK	GENMASK(2, 0)
@@ -36,8 +40,11 @@
 #define PDC_VERSION_REG		0x1000
 
 /* Notable PDC versions */
+#define PDC_VERSION_3_0		0x30000
 #define PDC_VERSION_3_2		0x30200
 
+#define PDC_PASS_THROUGH_MODE	0
+
 struct pdc_pin_region {
 	u32 pin_base;
 	u32 parent_base;
@@ -97,6 +104,33 @@ static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
 	pdc_base_reg_write(base, IRQ_ENABLE_BANK, bank, enable);
 }
 
+/*
+ * The new mask bit controls whether the interrupt is to be forwarded to the
+ * parent GIC in secondary controller mode. Writing the mask is do not care
+ * when the PDC is set to pass through mode.
+ *
+ * As linux only makes so far make use of pass through mode set all IRQs
+ * masked during probe.
+ */
+static void __pdc_mask_intr(int pin_out, bool mask)
+{
+	unsigned long irq_cfg;
+	int mask_bit;
+
+	/* Mask bit available from v3.0 */
+	if (pdc_version < PDC_VERSION_3_0)
+		return;
+
+	if (pdc_version < PDC_VERSION_3_2)
+		mask_bit = IRQ_i_CFG_IRQ_MASK_3_0;
+	else
+		mask_bit = IRQ_i_CFG_IRQ_MASK_3_2;
+
+	irq_cfg = pdc_reg_read(IRQ_i_CFG, pin_out);
+	__assign_bit(mask_bit, &irq_cfg, mask);
+	pdc_reg_write(IRQ_i_CFG, pin_out, irq_cfg);
+}
+
 static void __pdc_enable_intr(int pin_out, bool on)
 {
 	unsigned long enable;
@@ -312,7 +346,6 @@ static const struct irq_domain_ops qcom_pdc_ops = {
 static int pdc_setup_pin_mapping(struct device_node *np)
 {
 	int ret, n, i;
-
 	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
 	if (n <= 0 || n % 3)
 		return -EINVAL;
@@ -341,8 +374,10 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 		if (ret)
 			return ret;
 
-		for (i = 0; i < pdc_region[n].cnt; i++)
+		for (i = 0; i < pdc_region[n].cnt; i++) {
 			__pdc_enable_intr(i + pdc_region[n].pin_base, 0);
+			__pdc_mask_intr(i + pdc_region[n].pin_base, true);
+		}
 	}
 
 	return 0;
@@ -352,10 +387,13 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 
 static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *parent)
 {
+	static const char buf[64] = "{class: cx_mol, res: cx, val: mol}";
+	unsigned int domain_flag = IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP;
 	struct irq_domain *parent_domain, *pdc_domain;
 	struct device_node *node = pdev->dev.of_node;
 	resource_size_t res_size;
 	struct resource res;
+	struct qmp *pdc_qmp;
 	int ret;
 
 	/* compat with old sm8150 DT which had very small region for PDC */
@@ -366,6 +404,13 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 	if (res_size > resource_size(&res))
 		pr_warn("%pOF: invalid reg size, please fix DT\n", node);
 
+	pdc_base = ioremap(res.start, res_size);
+	if (!pdc_base) {
+		pr_err("%pOF: unable to map PDC registers\n", node);
+		ret = -ENXIO;
+		goto fail;
+	}
+
 	/*
 	 * PDC has multiple DRV regions, each one provides the same set of
 	 * registers for a particular client in the system. Due to a hardware
@@ -382,15 +427,71 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 		}
 
 		pdc_x1e_quirk = true;
-	}
 
-	pdc_base = ioremap(res.start, res_size);
-	if (!pdc_base) {
-		pr_err("%pOF: unable to map PDC registers\n", node);
-		ret = -ENXIO;
-		goto fail;
+		/*
+		 * There are two modes PDC irqchip can work in
+		 *	- pass through mode
+		 *	- secondary controller mode
+		 *
+		 * All PDC irqchip supports pass through mode in which both
+		 * Direct SPIs and GPIO IRQs (as SPIs) are sent to GIC
+		 * without latching at PDC.
+		 *
+		 * Newer PDCs (v3.0 onwards) also support additional
+		 * secondary controller mode where PDC latches GPIO IRQs
+		 * and sends to GIC as level type IRQ. Direct SPIs still
+		 * works same as pass through mode without latching at PDC
+		 * even in secondary controller mode.
+		 *
+		 * All the SoCs so far default uses pass through mode with
+		 * the exception of x1e.
+		 *
+		 * x1e modes:
+		 *
+		 * x1e PDC may be set to secondary controller mode for
+		 * builds on CRD boards whereas it may be set to pass
+		 * through mode for IoT-EVK boards.
+		 *
+		 * There is no way to read which current mode it is set to
+		 * and make PDC work in respective mode as the read access
+		 * is not opened up for non secure world. There is though
+		 * write access opened up via SCM write API to set the mode.
+		 *
+		 * Configure PDC mode to pass through mode for all x1e based
+		 * boards.
+		 *
+		 * For successful write:
+		 *	- Nothing more to be done
+		 *
+		 * For unsuccessful write:
+		 *	- Inform TLMM to monitor GPIO IRQs (same as MPM)
+		 *	- Prevent SoC low power mode (CxPC) as PDC is not
+		 *	  monitoring GPIO IRQs which may be needed to wake
+		 *	  the SoC from low power mode.
+		 */
+		ret = of_address_to_resource(node, 2, &res);
+		if (ret) {
+			domain_flag = IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP;
+			goto skip_scm_write;
+		}
+
+		ret = qcom_scm_io_writel(res.start, PDC_PASS_THROUGH_MODE);
+		if (ret) {
+			pdc_qmp = qmp_get(&pdev->dev);
+			if (IS_ERR(pdc_qmp)) {
+				ret = PTR_ERR(pdc_qmp);
+				goto fail;
+			} else {
+				ret = qmp_send(pdc_qmp, buf, sizeof(buf));
+				qmp_put(pdc_qmp);
+				if (ret)
+					goto fail;
+			}
+			domain_flag = IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP;
+		}
 	}
 
+skip_scm_write:
 	pdc_version = pdc_reg_read(PDC_VERSION_REG, 0);
 
 	parent_domain = irq_find_host(parent);
@@ -407,7 +508,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 	}
 
 	pdc_domain = irq_domain_create_hierarchy(parent_domain,
-					IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
+					domain_flag,
 					PDC_MAX_GPIO_IRQS,
 					of_fwnode_handle(node),
 					&qcom_pdc_ops, NULL);

-- 
2.34.1

Re: [PATCH 3/5] irqchip/qcom-pdc: Configure PDC to pass through mode

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 09:26:37PM +0530, Maulik Shah wrote:
> There are two modes PDC irqchip supports pass through mode and secondary
> controller mode.

Can't parse this, excuse me.

> 
> All PDC irqchip supports pass through mode in which both Direct SPIs and
> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
> 
> Newer PDCs (v3.0 onwards) also support additional secondary controller mode

It would help to mention the platforms, not everybody has the core docs.

> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
> still works same as pass through mode without latching at PDC even in
> secondary controller mode.
> 
> All the SoCs so far default uses pass through mode with the exception of

Is it something that must be configured by the bootloaders?

> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
> boards whereas it may be set to pass through mode for IoT-EVK.
> 
> There is no way to read which current mode it is set to and make PDC work
> in respective mode as the read access is not opened up for non secure
> world. There is though write access opened up via SCM write API to set the
> mode.

What are going to loose? The ability to latch the wakeup sources on the
CRD?

> Configure PDC mode to pass through mode for all x1e based boards via SCM
> write.

Would it make sense to always use the secondary mode instead?

> 
> Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  drivers/irqchip/Kconfig    |   1 +
>  drivers/irqchip/qcom-pdc.c | 119 +++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 111 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 83d333f8bf63d78827800e0de724f81e6aa2f1df..89caddf6e5c569a0e867cda1838c870b967fb13d 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -505,6 +505,7 @@ config GOLDFISH_PIC
>  config QCOM_PDC
>  	tristate "QCOM PDC"
>  	depends on ARCH_QCOM
> +	depends on QCOM_AOSS_QMP
>  	select IRQ_DOMAIN_HIERARCHY
>  	help
>  	  Power Domain Controller driver to manage and configure wakeup
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 32b77fa93f730416edf120710bcdcdce33fa39a7..051700d672471c092e8cda4d7f5aa6d2032157f7 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -19,6 +19,8 @@
>  #include <linux/spinlock.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/soc/qcom/qcom_aoss.h>
>  
>  #define PDC_MAX_GPIO_IRQS	256
>  #define PDC_DRV_OFFSET		0x10000
> @@ -26,9 +28,11 @@
>  /* Valid only on HW version < 3.2 */
>  #define IRQ_ENABLE_BANK		0x10
>  #define IRQ_ENABLE_BANK_MAX	(IRQ_ENABLE_BANK + BITS_TO_BYTES(PDC_MAX_GPIO_IRQS))
> +#define IRQ_i_CFG_IRQ_MASK_3_0	3
>  #define IRQ_i_CFG		0x110
>  
>  /* Valid only on HW version >= 3.2 */
> +#define IRQ_i_CFG_IRQ_MASK_3_2	4
>  #define IRQ_i_CFG_IRQ_ENABLE	3
>  
>  #define IRQ_i_CFG_TYPE_MASK	GENMASK(2, 0)
> @@ -36,8 +40,11 @@
>  #define PDC_VERSION_REG		0x1000
>  
>  /* Notable PDC versions */
> +#define PDC_VERSION_3_0		0x30000
>  #define PDC_VERSION_3_2		0x30200
>  
> +#define PDC_PASS_THROUGH_MODE	0
> +
>  struct pdc_pin_region {
>  	u32 pin_base;
>  	u32 parent_base;
> @@ -97,6 +104,33 @@ static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
>  	pdc_base_reg_write(base, IRQ_ENABLE_BANK, bank, enable);
>  }
>  
> +/*
> + * The new mask bit controls whether the interrupt is to be forwarded to the
> + * parent GIC in secondary controller mode. Writing the mask is do not care
> + * when the PDC is set to pass through mode.
> + *
> + * As linux only makes so far make use of pass through mode set all IRQs
> + * masked during probe.
> + */
> +static void __pdc_mask_intr(int pin_out, bool mask)
> +{
> +	unsigned long irq_cfg;
> +	int mask_bit;
> +
> +	/* Mask bit available from v3.0 */
> +	if (pdc_version < PDC_VERSION_3_0)
> +		return;
> +
> +	if (pdc_version < PDC_VERSION_3_2)
> +		mask_bit = IRQ_i_CFG_IRQ_MASK_3_0;
> +	else
> +		mask_bit = IRQ_i_CFG_IRQ_MASK_3_2;
> +
> +	irq_cfg = pdc_reg_read(IRQ_i_CFG, pin_out);
> +	__assign_bit(mask_bit, &irq_cfg, mask);
> +	pdc_reg_write(IRQ_i_CFG, pin_out, irq_cfg);
> +}
> +
>  static void __pdc_enable_intr(int pin_out, bool on)
>  {
>  	unsigned long enable;
> @@ -312,7 +346,6 @@ static const struct irq_domain_ops qcom_pdc_ops = {
>  static int pdc_setup_pin_mapping(struct device_node *np)
>  {
>  	int ret, n, i;
> -
>  	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
>  	if (n <= 0 || n % 3)
>  		return -EINVAL;
> @@ -341,8 +374,10 @@ static int pdc_setup_pin_mapping(struct device_node *np)
>  		if (ret)
>  			return ret;
>  
> -		for (i = 0; i < pdc_region[n].cnt; i++)
> +		for (i = 0; i < pdc_region[n].cnt; i++) {
>  			__pdc_enable_intr(i + pdc_region[n].pin_base, 0);
> +			__pdc_mask_intr(i + pdc_region[n].pin_base, true);
> +		}
>  	}
>  
>  	return 0;
> @@ -352,10 +387,13 @@ static int pdc_setup_pin_mapping(struct device_node *np)
>  
>  static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *parent)
>  {
> +	static const char buf[64] = "{class: cx_mol, res: cx, val: mol}";
> +	unsigned int domain_flag = IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP;
>  	struct irq_domain *parent_domain, *pdc_domain;
>  	struct device_node *node = pdev->dev.of_node;
>  	resource_size_t res_size;
>  	struct resource res;
> +	struct qmp *pdc_qmp;
>  	int ret;
>  
>  	/* compat with old sm8150 DT which had very small region for PDC */
> @@ -366,6 +404,13 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
>  	if (res_size > resource_size(&res))
>  		pr_warn("%pOF: invalid reg size, please fix DT\n", node);
>  
> +	pdc_base = ioremap(res.start, res_size);
> +	if (!pdc_base) {
> +		pr_err("%pOF: unable to map PDC registers\n", node);
> +		ret = -ENXIO;
> +		goto fail;
> +	}
> +
>  	/*
>  	 * PDC has multiple DRV regions, each one provides the same set of
>  	 * registers for a particular client in the system. Due to a hardware
> @@ -382,15 +427,71 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
>  		}
>  
>  		pdc_x1e_quirk = true;
> -	}
>  
> -	pdc_base = ioremap(res.start, res_size);
> -	if (!pdc_base) {
> -		pr_err("%pOF: unable to map PDC registers\n", node);
> -		ret = -ENXIO;
> -		goto fail;
> +		/*
> +		 * There are two modes PDC irqchip can work in
> +		 *	- pass through mode
> +		 *	- secondary controller mode
> +		 *
> +		 * All PDC irqchip supports pass through mode in which both
> +		 * Direct SPIs and GPIO IRQs (as SPIs) are sent to GIC
> +		 * without latching at PDC.
> +		 *
> +		 * Newer PDCs (v3.0 onwards) also support additional
> +		 * secondary controller mode where PDC latches GPIO IRQs
> +		 * and sends to GIC as level type IRQ. Direct SPIs still
> +		 * works same as pass through mode without latching at PDC
> +		 * even in secondary controller mode.

I'd say, there is no need to duplicate the commit message.

> +		 *
> +		 * All the SoCs so far default uses pass through mode with
> +		 * the exception of x1e.
> +		 *
> +		 * x1e modes:
> +		 *
> +		 * x1e PDC may be set to secondary controller mode for
> +		 * builds on CRD boards whereas it may be set to pass
> +		 * through mode for IoT-EVK boards.
> +		 *
> +		 * There is no way to read which current mode it is set to
> +		 * and make PDC work in respective mode as the read access
> +		 * is not opened up for non secure world. There is though
> +		 * write access opened up via SCM write API to set the mode.
> +		 *
> +		 * Configure PDC mode to pass through mode for all x1e based
> +		 * boards.
> +		 *
> +		 * For successful write:
> +		 *	- Nothing more to be done
> +		 *
> +		 * For unsuccessful write:

Why would it fail?

> +		 *	- Inform TLMM to monitor GPIO IRQs (same as MPM)
> +		 *	- Prevent SoC low power mode (CxPC) as PDC is not
> +		 *	  monitoring GPIO IRQs which may be needed to wake
> +		 *	  the SoC from low power mode.

This doesn't quite match the description of "latches the GPIO IRQs".

> +		 */
> +		ret = of_address_to_resource(node, 2, &res);
> +		if (ret) {
> +			domain_flag = IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP;
> +			goto skip_scm_write;
> +		}
> +
> +		ret = qcom_scm_io_writel(res.start, PDC_PASS_THROUGH_MODE);
> +		if (ret) {
> +			pdc_qmp = qmp_get(&pdev->dev);
> +			if (IS_ERR(pdc_qmp)) {
> +				ret = PTR_ERR(pdc_qmp);
> +				goto fail;
> +			} else {
> +				ret = qmp_send(pdc_qmp, buf, sizeof(buf));
> +				qmp_put(pdc_qmp);
> +				if (ret)
> +					goto fail;
> +			}
> +			domain_flag = IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP;
> +		}
>  	}
>  
> +skip_scm_write:
>  	pdc_version = pdc_reg_read(PDC_VERSION_REG, 0);
>  
>  	parent_domain = irq_find_host(parent);
> @@ -407,7 +508,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
>  	}
>  
>  	pdc_domain = irq_domain_create_hierarchy(parent_domain,
> -					IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
> +					domain_flag,
>  					PDC_MAX_GPIO_IRQS,
>  					of_fwnode_handle(node),
>  					&qcom_pdc_ops, NULL);
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 3/5] irqchip/qcom-pdc: Configure PDC to pass through mode

[Maulik Shah (mkshah)]

On 3/13/2026 7:52 AM, Dmitry Baryshkov wrote:
> On Thu, Mar 12, 2026 at 09:26:37PM +0530, Maulik Shah wrote:
>> There are two modes PDC irqchip supports pass through mode and secondary
>> controller mode.
> 
> Can't parse this, excuse me.

Ok, I can drop this in v2.

> 
>>
>> All PDC irqchip supports pass through mode in which both Direct SPIs and
>> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
>>
>> Newer PDCs (v3.0 onwards) also support additional secondary controller mode
> 
> It would help to mention the platforms, not everybody has the core docs.

Sure, i can update the platforms which are v3.0 or higher.

> 
>> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
>> still works same as pass through mode without latching at PDC even in
>> secondary controller mode.
>>
>> All the SoCs so far default uses pass through mode with the exception of
> 
> Is it something that must be configured by the bootloaders?

yes, currently changing the the mode can be done from secure world either at boot
or after boot via scm write.

> 
>> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
>> boards whereas it may be set to pass through mode for IoT-EVK.
>>
>> There is no way to read which current mode it is set to and make PDC work
>> in respective mode as the read access is not opened up for non secure
>> world. There is though write access opened up via SCM write API to set the
>> mode.
> 
> What are going to loose? The ability to latch the wakeup sources on the
> CRD?

CXPC (SoC level low power mode) would be lost if the device can not wake up from GPIO wakeup sources.

> 
>> Configure PDC mode to pass through mode for all x1e based boards via SCM
>> write.
> 
> Would it make sense to always use the secondary mode instead?

No, it would not make sense to support the secondary mode in Linux.

> 
..
..

>>  
>> -	pdc_base = ioremap(res.start, res_size);
>> -	if (!pdc_base) {
>> -		pr_err("%pOF: unable to map PDC registers\n", node);
>> -		ret = -ENXIO;
>> -		goto fail;
>> +		/*
>> +		 * There are two modes PDC irqchip can work in
>> +		 *	- pass through mode
>> +		 *	- secondary controller mode
>> +		 *
>> +		 * All PDC irqchip supports pass through mode in which both
>> +		 * Direct SPIs and GPIO IRQs (as SPIs) are sent to GIC
>> +		 * without latching at PDC.
>> +		 *
>> +		 * Newer PDCs (v3.0 onwards) also support additional
>> +		 * secondary controller mode where PDC latches GPIO IRQs
>> +		 * and sends to GIC as level type IRQ. Direct SPIs still
>> +		 * works same as pass through mode without latching at PDC
>> +		 * even in secondary controller mode.
> 
> I'd say, there is no need to duplicate the commit message.

Sure, i can remove from comments.

> 
>> +		 *
>> +		 * All the SoCs so far default uses pass through mode with
>> +		 * the exception of x1e.
>> +		 *
>> +		 * x1e modes:
>> +		 *
>> +		 * x1e PDC may be set to secondary controller mode for
>> +		 * builds on CRD boards whereas it may be set to pass
>> +		 * through mode for IoT-EVK boards.
>> +		 *
>> +		 * There is no way to read which current mode it is set to
>> +		 * and make PDC work in respective mode as the read access
>> +		 * is not opened up for non secure world. There is though
>> +		 * write access opened up via SCM write API to set the mode.
>> +		 *
>> +		 * Configure PDC mode to pass through mode for all x1e based
>> +		 * boards.
>> +		 *
>> +		 * For successful write:
>> +		 *	- Nothing more to be done
>> +		 *
>> +		 * For unsuccessful write:
> 
> Why would it fail?

It can fail if the write is denied by firmware.
As i understand the older firmware had neither read/write as such firmware
was meant to be used for non-linux (windows only and not for the dual boot).

> 
>> +		 *	- Inform TLMM to monitor GPIO IRQs (same as MPM)
>> +		 *	- Prevent SoC low power mode (CxPC) as PDC is not
>> +		 *	  monitoring GPIO IRQs which may be needed to wake
>> +		 *	  the SoC from low power mode.
> 
> This doesn't quite match the description of "latches the GPIO IRQs".

It does, PDC would continue to still latch the GPIO IRQs (as the mode change failed)
but PDC won't forward them to parent GIC as they are masked at PDC with __pdc_mask_intr().

In summary,

Below is what x1e users get today if they boot up Linux:
 - All the GPIO interrupts works fine for them as they don't get forwarded to PDC
   due to commit 602cb14e310a ("pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now")
 - SS3 idle state (CPU level deepest low power mode) not added in device tree due to
   above commit.
 - This prevents CXPC (SoC level low power mode) as the CPU subsystem cannot hit deepest low power mode.

Below is what would x1e users would get from this series,
 - GPIO interrupts continue to work fine after reverting commit 602cb14e310a
   ("pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"), PATCH 5/5 of this series.
 - SS3 idle state (CPU level deepest idle state) is added, PATCH 4/4 of this series.
   Adding the SS3 idle states opens up the path for the SoC to achieve CXPC (SoC level low power mode)
   (This again depends on drivers removing all the global resources vote)

   While all global resources votes can get removed, if the PDC still could not wake the SoC from GPIO
   interrupts, the CX is kept at MoL (minimum operating level) and TLMM (which is on Cx rail) would then
   wake up the CPUs from SS3 CPUidle / suspend (s2idle) state with GPIO interrupts.

Thanks,
Maulik

Re: [PATCH 3/5] irqchip/qcom-pdc: Configure PDC to pass through mode

[Konrad Dybcio]

On 3/13/26 7:40 AM, Maulik Shah (mkshah) wrote:
> 
> 
> On 3/13/2026 7:52 AM, Dmitry Baryshkov wrote:
>> On Thu, Mar 12, 2026 at 09:26:37PM +0530, Maulik Shah wrote:

[...]

>>> All the SoCs so far default uses pass through mode with the exception of
>>
>> Is it something that must be configured by the bootloaders?
> 
> yes, currently changing the the mode can be done from secure world either at boot
> or after boot via scm write.

..which won't work on almost any X1E devices, except CRD and IOT..

>>> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
>>> boards whereas it may be set to pass through mode for IoT-EVK.
>>>
>>> There is no way to read which current mode it is set to and make PDC work
>>> in respective mode as the read access is not opened up for non secure
>>> world. There is though write access opened up via SCM write API to set the
>>> mode.
>>
>> What are going to loose? The ability to latch the wakeup sources on the
>> CRD?
> 
> CXPC (SoC level low power mode) would be lost if the device can not wake up from GPIO wakeup sources.

To the best of my understanding, that's only because your approach chooses
to ignore supporting the secondary controller mode and force-reconfigure,
since GPIO wakeup functionality is otherwise available regardless of the
mode.

>>> Configure PDC mode to pass through mode for all x1e based boards via SCM
>>> write.
>>
>> Would it make sense to always use the secondary mode instead?
> 
> No, it would not make sense to support the secondary mode in Linux.

Why?

[...]

>>> +		 *	- Inform TLMM to monitor GPIO IRQs (same as MPM)
>>> +		 *	- Prevent SoC low power mode (CxPC) as PDC is not
>>> +		 *	  monitoring GPIO IRQs which may be needed to wake
>>> +		 *	  the SoC from low power mode.
>>
>> This doesn't quite match the description of "latches the GPIO IRQs".
> 
> It does, PDC would continue to still latch the GPIO IRQs (as the mode change failed)
> but PDC won't forward them to parent GIC as they are masked at PDC with __pdc_mask_intr().

Can you not refrain from masking them then, and clear them upon reception,
with a write to IRQ_i_CFG[IRQ_STATUS]?

The HPG states that this mechanism is only engaged for GPIO IRQs and that
the forwarded interrupt will be of LEVEL_HIGH type (which is what TLMM
accepts anyway)

FWIW, some related work:
c7984dc0a2b9 ("pinctrl: qcom: Add test case for TLMM interrupt handling")

Konrad

Re: [PATCH 3/5] irqchip/qcom-pdc: Configure PDC to pass through mode

[Bjorn Andersson]

On Thu, Mar 12, 2026 at 09:26:37PM +0530, Maulik Shah wrote:
> There are two modes PDC irqchip supports pass through mode and secondary
> controller mode.
> 
> All PDC irqchip supports pass through mode in which both Direct SPIs and
> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
> 
> Newer PDCs (v3.0 onwards) also support additional secondary controller mode
> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
> still works same as pass through mode without latching at PDC even in
> secondary controller mode.
> 
> All the SoCs so far default uses pass through mode with the exception of
> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
> boards whereas it may be set to pass through mode for IoT-EVK.
> 
> There is no way to read which current mode it is set to and make PDC work
> in respective mode as the read access is not opened up for non secure
> world. There is though write access opened up via SCM write API to set the
> mode.
> 
> Configure PDC mode to pass through mode for all x1e based boards via SCM
> write.
> 

You're failing to mention that the SCM interface was not present in
initially shipping Windows firmware, which would result in you breaking
those devices.

If you're certain that this change is available to all users, you can
argue that this is acceptable - but omitting this from the commit
message isn't.

Regards,
Bjorn

> Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  drivers/irqchip/Kconfig    |   1 +
>  drivers/irqchip/qcom-pdc.c | 119 +++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 111 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 83d333f8bf63d78827800e0de724f81e6aa2f1df..89caddf6e5c569a0e867cda1838c870b967fb13d 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -505,6 +505,7 @@ config GOLDFISH_PIC
>  config QCOM_PDC
>  	tristate "QCOM PDC"
>  	depends on ARCH_QCOM
> +	depends on QCOM_AOSS_QMP
>  	select IRQ_DOMAIN_HIERARCHY
>  	help
>  	  Power Domain Controller driver to manage and configure wakeup
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 32b77fa93f730416edf120710bcdcdce33fa39a7..051700d672471c092e8cda4d7f5aa6d2032157f7 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -19,6 +19,8 @@
>  #include <linux/spinlock.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/soc/qcom/qcom_aoss.h>
>  
>  #define PDC_MAX_GPIO_IRQS	256
>  #define PDC_DRV_OFFSET		0x10000
> @@ -26,9 +28,11 @@
>  /* Valid only on HW version < 3.2 */
>  #define IRQ_ENABLE_BANK		0x10
>  #define IRQ_ENABLE_BANK_MAX	(IRQ_ENABLE_BANK + BITS_TO_BYTES(PDC_MAX_GPIO_IRQS))
> +#define IRQ_i_CFG_IRQ_MASK_3_0	3
>  #define IRQ_i_CFG		0x110
>  
>  /* Valid only on HW version >= 3.2 */
> +#define IRQ_i_CFG_IRQ_MASK_3_2	4
>  #define IRQ_i_CFG_IRQ_ENABLE	3
>  
>  #define IRQ_i_CFG_TYPE_MASK	GENMASK(2, 0)
> @@ -36,8 +40,11 @@
>  #define PDC_VERSION_REG		0x1000
>  
>  /* Notable PDC versions */
> +#define PDC_VERSION_3_0		0x30000
>  #define PDC_VERSION_3_2		0x30200
>  
> +#define PDC_PASS_THROUGH_MODE	0
> +
>  struct pdc_pin_region {
>  	u32 pin_base;
>  	u32 parent_base;
> @@ -97,6 +104,33 @@ static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
>  	pdc_base_reg_write(base, IRQ_ENABLE_BANK, bank, enable);
>  }
>  
> +/*
> + * The new mask bit controls whether the interrupt is to be forwarded to the
> + * parent GIC in secondary controller mode. Writing the mask is do not care
> + * when the PDC is set to pass through mode.
> + *
> + * As linux only makes so far make use of pass through mode set all IRQs
> + * masked during probe.
> + */
> +static void __pdc_mask_intr(int pin_out, bool mask)
> +{
> +	unsigned long irq_cfg;
> +	int mask_bit;
> +
> +	/* Mask bit available from v3.0 */
> +	if (pdc_version < PDC_VERSION_3_0)
> +		return;
> +
> +	if (pdc_version < PDC_VERSION_3_2)
> +		mask_bit = IRQ_i_CFG_IRQ_MASK_3_0;
> +	else
> +		mask_bit = IRQ_i_CFG_IRQ_MASK_3_2;
> +
> +	irq_cfg = pdc_reg_read(IRQ_i_CFG, pin_out);
> +	__assign_bit(mask_bit, &irq_cfg, mask);
> +	pdc_reg_write(IRQ_i_CFG, pin_out, irq_cfg);
> +}
> +
>  static void __pdc_enable_intr(int pin_out, bool on)
>  {
>  	unsigned long enable;
> @@ -312,7 +346,6 @@ static const struct irq_domain_ops qcom_pdc_ops = {
>  static int pdc_setup_pin_mapping(struct device_node *np)
>  {
>  	int ret, n, i;
> -
>  	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
>  	if (n <= 0 || n % 3)
>  		return -EINVAL;
> @@ -341,8 +374,10 @@ static int pdc_setup_pin_mapping(struct device_node *np)
>  		if (ret)
>  			return ret;
>  
> -		for (i = 0; i < pdc_region[n].cnt; i++)
> +		for (i = 0; i < pdc_region[n].cnt; i++) {
>  			__pdc_enable_intr(i + pdc_region[n].pin_base, 0);
> +			__pdc_mask_intr(i + pdc_region[n].pin_base, true);
> +		}
>  	}
>  
>  	return 0;
> @@ -352,10 +387,13 @@ static int pdc_setup_pin_mapping(struct device_node *np)
>  
>  static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *parent)
>  {
> +	static const char buf[64] = "{class: cx_mol, res: cx, val: mol}";
> +	unsigned int domain_flag = IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP;
>  	struct irq_domain *parent_domain, *pdc_domain;
>  	struct device_node *node = pdev->dev.of_node;
>  	resource_size_t res_size;
>  	struct resource res;
> +	struct qmp *pdc_qmp;
>  	int ret;
>  
>  	/* compat with old sm8150 DT which had very small region for PDC */
> @@ -366,6 +404,13 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
>  	if (res_size > resource_size(&res))
>  		pr_warn("%pOF: invalid reg size, please fix DT\n", node);
>  
> +	pdc_base = ioremap(res.start, res_size);
> +	if (!pdc_base) {
> +		pr_err("%pOF: unable to map PDC registers\n", node);
> +		ret = -ENXIO;
> +		goto fail;
> +	}
> +
>  	/*
>  	 * PDC has multiple DRV regions, each one provides the same set of
>  	 * registers for a particular client in the system. Due to a hardware
> @@ -382,15 +427,71 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
>  		}
>  
>  		pdc_x1e_quirk = true;
> -	}
>  
> -	pdc_base = ioremap(res.start, res_size);
> -	if (!pdc_base) {
> -		pr_err("%pOF: unable to map PDC registers\n", node);
> -		ret = -ENXIO;
> -		goto fail;
> +		/*
> +		 * There are two modes PDC irqchip can work in
> +		 *	- pass through mode
> +		 *	- secondary controller mode
> +		 *
> +		 * All PDC irqchip supports pass through mode in which both
> +		 * Direct SPIs and GPIO IRQs (as SPIs) are sent to GIC
> +		 * without latching at PDC.
> +		 *
> +		 * Newer PDCs (v3.0 onwards) also support additional
> +		 * secondary controller mode where PDC latches GPIO IRQs
> +		 * and sends to GIC as level type IRQ. Direct SPIs still
> +		 * works same as pass through mode without latching at PDC
> +		 * even in secondary controller mode.
> +		 *
> +		 * All the SoCs so far default uses pass through mode with
> +		 * the exception of x1e.
> +		 *
> +		 * x1e modes:
> +		 *
> +		 * x1e PDC may be set to secondary controller mode for
> +		 * builds on CRD boards whereas it may be set to pass
> +		 * through mode for IoT-EVK boards.
> +		 *
> +		 * There is no way to read which current mode it is set to
> +		 * and make PDC work in respective mode as the read access
> +		 * is not opened up for non secure world. There is though
> +		 * write access opened up via SCM write API to set the mode.
> +		 *
> +		 * Configure PDC mode to pass through mode for all x1e based
> +		 * boards.
> +		 *
> +		 * For successful write:
> +		 *	- Nothing more to be done
> +		 *
> +		 * For unsuccessful write:
> +		 *	- Inform TLMM to monitor GPIO IRQs (same as MPM)
> +		 *	- Prevent SoC low power mode (CxPC) as PDC is not
> +		 *	  monitoring GPIO IRQs which may be needed to wake
> +		 *	  the SoC from low power mode.
> +		 */
> +		ret = of_address_to_resource(node, 2, &res);
> +		if (ret) {
> +			domain_flag = IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP;
> +			goto skip_scm_write;
> +		}
> +
> +		ret = qcom_scm_io_writel(res.start, PDC_PASS_THROUGH_MODE);
> +		if (ret) {
> +			pdc_qmp = qmp_get(&pdev->dev);
> +			if (IS_ERR(pdc_qmp)) {
> +				ret = PTR_ERR(pdc_qmp);
> +				goto fail;
> +			} else {
> +				ret = qmp_send(pdc_qmp, buf, sizeof(buf));
> +				qmp_put(pdc_qmp);
> +				if (ret)
> +					goto fail;
> +			}
> +			domain_flag = IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP;
> +		}
>  	}
>  
> +skip_scm_write:
>  	pdc_version = pdc_reg_read(PDC_VERSION_REG, 0);
>  
>  	parent_domain = irq_find_host(parent);
> @@ -407,7 +508,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
>  	}
>  
>  	pdc_domain = irq_domain_create_hierarchy(parent_domain,
> -					IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
> +					domain_flag,
>  					PDC_MAX_GPIO_IRQS,
>  					of_fwnode_handle(node),
>  					&qcom_pdc_ops, NULL);
> 
> -- 
> 2.34.1
> 

[PATCH 4/5] arm64: dts: qcom: x1e80100: Add deepest idle state

[Maulik Shah]

Add deepest idle state along with pdc config reg to make GPIO IRQs work
as wakeup capable interrupts in deepest idle state.

Add QMP handle to allow PDC device to place a SoC level low power mode
restriction.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/hamoa.dtsi | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index ebecf43e0d462c431540257e299e3ace054901fd..8f560fd140661ad720fec979eabe3ca8ffb34273 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -290,6 +290,14 @@ cluster_cl5: cluster-sleep-1 {
 				exit-latency-us = <4000>;
 				min-residency-us = <7000>;
 			};
+
+			domain_ss3: domain-sleep-0 {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x0200c354>;
+				entry-latency-us = <2800>;
+				exit-latency-us = <4400>;
+				min-residency-us = <9000>;
+			};
 		};
 	};
 
@@ -447,7 +455,7 @@ cluster_pd2: power-domain-cpu-cluster2 {
 
 		system_pd: power-domain-system {
 			#power-domain-cells = <0>;
-			/* TODO: system-wide idle states */
+			domain-idle-states = <&domain_ss3>;
 		};
 	};
 
@@ -6013,8 +6021,10 @@ dispcc: clock-controller@af00000 {
 
 		pdc: interrupt-controller@b220000 {
 			compatible = "qcom,x1e80100-pdc", "qcom,pdc";
-			reg = <0 0x0b220000 0 0x30000>, <0 0x174000f0 0 0x64>;
-
+			reg = <0 0x0b220000 0 0x30000>,
+			      <0 0x174000f0 0 0x64>,
+			      <0 0x0b2045e8 0 0x4>;
+			qcom,qmp = <&aoss_qmp>;
 			qcom,pdc-ranges = <0 480 42>, <42 251 5>,
 					  <47 522 52>, <99 609 32>,
 					  <131 717 12>, <143 816 19>;

-- 
2.34.1

Re: [PATCH 4/5] arm64: dts: qcom: x1e80100: Add deepest idle state

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 09:26:38PM +0530, Maulik Shah wrote:
> Add deepest idle state along with pdc config reg to make GPIO IRQs work
> as wakeup capable interrupts in deepest idle state.
> 
> Add QMP handle to allow PDC device to place a SoC level low power mode
> restriction.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/hamoa.dtsi | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index ebecf43e0d462c431540257e299e3ace054901fd..8f560fd140661ad720fec979eabe3ca8ffb34273 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -290,6 +290,14 @@ cluster_cl5: cluster-sleep-1 {
>  				exit-latency-us = <4000>;
>  				min-residency-us = <7000>;
>  			};
> +
> +			domain_ss3: domain-sleep-0 {
> +				compatible = "domain-idle-state";
> +				arm,psci-suspend-param = <0x0200c354>;
> +				entry-latency-us = <2800>;
> +				exit-latency-us = <4400>;
> +				min-residency-us = <9000>;
> +			};
>  		};
>  	};
>  
> @@ -447,7 +455,7 @@ cluster_pd2: power-domain-cpu-cluster2 {
>  
>  		system_pd: power-domain-system {
>  			#power-domain-cells = <0>;
> -			/* TODO: system-wide idle states */
> +			domain-idle-states = <&domain_ss3>;
>  		};
>  	};
>  
> @@ -6013,8 +6021,10 @@ dispcc: clock-controller@af00000 {
>  
>  		pdc: interrupt-controller@b220000 {
>  			compatible = "qcom,x1e80100-pdc", "qcom,pdc";
> -			reg = <0 0x0b220000 0 0x30000>, <0 0x174000f0 0 0x64>;
> -
> +			reg = <0 0x0b220000 0 0x30000>,

As you are touching these lines, 0x0 instead of just 0, please.

> +			      <0 0x174000f0 0 0x64>,
> +			      <0 0x0b2045e8 0 0x4>;
> +			qcom,qmp = <&aoss_qmp>;
>  			qcom,pdc-ranges = <0 480 42>, <42 251 5>,
>  					  <47 522 52>, <99 609 32>,
>  					  <131 717 12>, <143 816 19>;
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 4/5] arm64: dts: qcom: x1e80100: Add deepest idle state

[Maulik Shah (mkshah)]

On 3/13/2026 8:00 AM, Dmitry Baryshkov wrote:
> On Thu, Mar 12, 2026 at 09:26:38PM +0530, Maulik Shah wrote:

>> -			reg = <0 0x0b220000 0 0x30000>, <0 0x174000f0 0 0x64>;
>> -
>> +			reg = <0 0x0b220000 0 0x30000>,
> 
> As you are touching these lines, 0x0 instead of just 0, please.

Sure, Will update in v2.

Thanks,
Maulik

Re: [PATCH 4/5] arm64: dts: qcom: x1e80100: Add deepest idle state

[Krzysztof Kozlowski]

On Thu, Mar 12, 2026 at 09:26:38PM +0530, Maulik Shah wrote:
> Add deepest idle state along with pdc config reg to make GPIO IRQs work
> as wakeup capable interrupts in deepest idle state.
> 
> Add QMP handle to allow PDC device to place a SoC level low power mode
> restriction.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/hamoa.dtsi | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index ebecf43e0d462c431540257e299e3ace054901fd..8f560fd140661ad720fec979eabe3ca8ffb34273 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -290,6 +290,14 @@ cluster_cl5: cluster-sleep-1 {
>  				exit-latency-us = <4000>;
>  				min-residency-us = <7000>;
>  			};
> +
> +			domain_ss3: domain-sleep-0 {
> +				compatible = "domain-idle-state";
> +				arm,psci-suspend-param = <0x0200c354>;
> +				entry-latency-us = <2800>;
> +				exit-latency-us = <4400>;
> +				min-residency-us = <9000>;
> +			};
>  		};
>  	};
>  
> @@ -447,7 +455,7 @@ cluster_pd2: power-domain-cpu-cluster2 {
>  
>  		system_pd: power-domain-system {
>  			#power-domain-cells = <0>;
> -			/* TODO: system-wide idle states */
> +			domain-idle-states = <&domain_ss3>;
>  		};
>  	};
>  
> @@ -6013,8 +6021,10 @@ dispcc: clock-controller@af00000 {
>  
>  		pdc: interrupt-controller@b220000 {
>  			compatible = "qcom,x1e80100-pdc", "qcom,pdc";
> -			reg = <0 0x0b220000 0 0x30000>, <0 0x174000f0 0 0x64>;
> -
> +			reg = <0 0x0b220000 0 0x30000>,
> +			      <0 0x174000f0 0 0x64>,
> +			      <0 0x0b2045e8 0 0x4>;

One register is not device's address space.

Best regards,
Krzysztof

Re: [PATCH 4/5] arm64: dts: qcom: x1e80100: Add deepest idle state

[Maulik Shah (mkshah)]

On 3/13/2026 7:27 PM, Krzysztof Kozlowski wrote:
> On Thu, Mar 12, 2026 at 09:26:38PM +0530, Maulik Shah wrote:
>> Add deepest idle state along with pdc config reg to make GPIO IRQs work
>> as wakeup capable interrupts in deepest idle state.
>>
...
...
>>  
>>  		pdc: interrupt-controller@b220000 {
>>  			compatible = "qcom,x1e80100-pdc", "qcom,pdc";
>> -			reg = <0 0x0b220000 0 0x30000>, <0 0x174000f0 0 0x64>;
>> -
>> +			reg = <0 0x0b220000 0 0x30000>,
>> +			      <0 0x174000f0 0 0x64>,
>> +			      <0 0x0b2045e8 0 0x4>;
> 
> One register is not device's address space.
> 

I can move this one register as #define in driver in v2, SCM API is the only use
for this reg.

Thanks,
Maulik

[PATCH 5/5] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"

[Maulik Shah]

This reverts commit 602cb14e310a ("pinctrl: qcom: x1e80100: Bypass PDC
wakeup parent for now").

PDC interrupts no more break GPIOs. PDC is now set to pass through mode
which allows GPIO interrupts to setup as wakeup capable at PDC and pass
them to GIC as SPIs. Update nwakeirq_map to reflect the GPIO to PDC irq
map size.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-x1e80100.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-x1e80100.c b/drivers/pinctrl/qcom/pinctrl-x1e80100.c
index bb36f40b19fa53eedf68d46d02986410d07a733c..04e08680f996bb06f1e3123c45863c184a3fb205 100644
--- a/drivers/pinctrl/qcom/pinctrl-x1e80100.c
+++ b/drivers/pinctrl/qcom/pinctrl-x1e80100.c
@@ -1839,9 +1839,7 @@ static const struct msm_pinctrl_soc_data x1e80100_pinctrl = {
 	.ngroups = ARRAY_SIZE(x1e80100_groups),
 	.ngpios = 239,
 	.wakeirq_map = x1e80100_pdc_map,
-	/* TODO: Enabling PDC currently breaks GPIO interrupts */
-	.nwakeirq_map = 0,
-	/* .nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map), */
+	.nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map),
 	.egpio_func = 9,
 };
 

-- 
2.34.1