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@xxxxxxxxxxxxxxxx>
> >> ---
> >>  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@xxxxxxxxxxx/
> 
> 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@xxxxxxxxxxx/
> 
> Thanks,
> Maulik

-- 
With best wishes
Dmitry