Re: [PATCH v3 5/9] arm64: dts: qcom: sdm670: add adsp

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] arm64: dts: qcom: sdm670: missing qcom,non-secure-domain in
  adsp fastrpc node
- [Low] arm64: dts: qcom: sdm670: overallocated reg size on aoss_qmp
  node
--

commit 09499ee58443d0f969ea496f5f01be20fd59d890
Author: Richard Acayan <mailingradian@gmail.com>

arm64: dts: qcom: sdm670: add adsp

This commit adds the Audio DSP remoteproc node and its associated SMP2P,
AOSS QMP, and APSS shared mailbox nodes to the SDM670 device tree.

> diff --git a/arch/arm64/boot/dts/qcom/sdm670.dtsi b/arch/arm64/boot/dts/qcom/sdm670.dtsi
> index 711e15b56cb8a..e99183c222083 100644
> --- a/arch/arm64/boot/dts/qcom/sdm670.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm670.dtsi

[ ... ]

> @@ -2383,6 +2412,19 @@ dispcc: clock-controller@af00000 {
>  			#power-domain-cells = <1>;
>  		};
>  
> +		aoss_qmp: power-management@c300000 {
> +			compatible = "qcom,sdm670-aoss-qmp", "qcom,aoss-qmp";
> +			reg = <0 0x0c300000 0 0x100000>;

[Severity: Low]
Should the reg size on aoss_qmp be 0x400 instead of 0x100000?

qmp_probe() calls devm_platform_ioremap_resource(), which requests
exclusive ownership of the full reg range.  Specifying 0x100000 reserves
the entire 1 MiB AOSS region rather than the 1 KiB APSS client message RAM
slice, blocking other devices in that aperture (such as RPMh sleep stats
at 0x0c3f0000) from claiming their memory resource.

[ ... ]

> @@ -2573,6 +2621,135 @@ cpufreq_hw: cpufreq@17d43000 {
>  			#freq-domain-cells = <1>;
>  		};
>  
> +		adsp_pas: remoteproc@62400000 {

[ ... ]

> +				fastrpc {
> +					compatible = "qcom,fastrpc";
> +					qcom,glink-channels = "fastrpcglink-apps-dsp";
> +					label = "adsp";
> +					#address-cells = <1>;
> +					#size-cells = <0>;

[Severity: Medium]
Is this fastrpc node missing the qcom,non-secure-domain property?

Without it, fastrpc_rpmsg_probe() defaults secure_dsp to true and
registers /dev/fastrpc-adsp-secure instead of /dev/fastrpc-adsp for
ADSP_DOMAIN_ID, breaking non-secure userspace audio and sensor FastRPC
clients:

drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
	...
	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
	data->secure = secure_dsp;
	...
	switch (domain_id) {
	case ADSP_DOMAIN_ID:
	case MDSP_DOMAIN_ID:
	case SDSP_DOMAIN_ID:
		/* Unsigned PD offloading is only supported on CDSP and GDSP */
		data->unsigned_support = false;
		err = fastrpc_device_register(rdev, data, secure_dsp, domain);
		if (err)
			goto fdev_error;
		break;
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107022453.159341-1-mailingradian@gmail.com?part=5