Re: [PATCH v2] arm64: dts: qcom: sm8750: mark the ADSP fastrpc node as non-secure domain

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] arm64: dts: qcom: sm8750: v2 revision note placed in commit
  message body
- [Low] arm64: dts: qcom: sm8750: qcom,non-secure-domain placed before
  memory-region
--

commit a18d45b6dd4c9437dfa6368a1222360ec6bb808a
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

> arm64: dts: qcom: sm8750: mark the ADSP fastrpc node as non-secure domain
>
> v2: add the Fixes tag requested in the sashiko review
> (<sashiko-outbox-202531@kernel.org>).  The diff is unchanged from v1.
>
> On SM8750 the ADSP fastrpc instance used by the sensor core (and by the
> other non-secure ADSP users) is the one the firmware exposes as
> non-secure; the CDSP fastrpc node already carries
> "qcom,non-secure-domain", the ADSP one does not.  With the fastrpc driver
> keying the secure/non-secure split off that property, the ADSP only
> instantiates /dev/fastrpc-adsp-secure and userspace never gets
> /dev/fastrpc-adsp, so libssc/hexagonrpcd cannot attach to the sensor PD
> (qrtr service 400) and iio-sensor-proxy reports no sensors at all.
>
> Observed on the Lenovo Legion Tab Y700 Gen4 (TB322FC), which is SM8750
> based: without this property /dev/fastrpc-adsp never appears and
> iio-sensor-proxy exposes neither accelerometer, ambient light nor
> proximity; with it, qrtr shows the "Snapdragon Sensor Core service"
> (id 400) and all three sensor types report as available.  The change is
> one property on one node, matching what the CDSP node already does.
>
> Tested with `make CHECK_DTBS=y qcom/sm8750-mtp.dtb` (no new warnings; the
> single avoid_unnecessary_addr_size warning on dsi@ae96000 is present
> before and after).
>
> *** AI-assistance disclosure ***
> This patch and its commit message were prepared with the help of an LLM
> coding assistant.  The author reviewed the change line by line, ran the
> checks quoted above on his own hardware and takes full responsibility
> for the content.
>
> Fixes: be9d54c35312479ed92c85aec3cd43153513210d ("arm64: dts: qcom: sm8750: add ADSP fastrpc-compute-cb nodes")
> Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

[Severity: Low]
This isn't a bug, but the v2 patch changelog paragraph belongs below the
--- separator line so git am strips it rather than recording it in the
permanent commit history.  Also, the Fixes: tag should use a 12-character
SHA prefix:

  Fixes: be9d54c35312 ("arm64: dts: qcom: sm8750: add ADSP fastrpc-compute-cb nodes")

> diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> index cf947acc57602..9302481b112af 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> @@ -2571,6 +2571,7 @@ fastrpc {
>  					compatible = "qcom,fastrpc";
>  					qcom,glink-channels = "fastrpcglink-apps-dsp";
>  					label = "adsp";
> +					qcom,non-secure-domain;
>  					memory-region = <&adsp_rpc_remote_heap_mem>;
>  					qcom,vmids = <QCOM_SCM_VMID_LPASS
>  						      QCOM_SCM_VMID_ADSP_HEAP>;

[Severity: Low]
This isn't a bug, but per the Devicetree coding style and
scripts/dtc/dt-check-style, standard properties such as memory-region
should precede vendor-prefixed properties, so qcom,non-secure-domain
should be placed after memory-region.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110023153.84262-1-xiaqinghuan@mail.ustc.edu.cn?part=1