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

["Maulik Shah (mkshah)" <maulik.shah@oss.qualcomm.com>]

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