Re: [PATCH v3 4/7] remoteproc: qcom_q6v5_mss: Use mss as regulator for MSM8953

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] remoteproc: qcom_q6v5_mss: universal probe failure on MSM8953 due to missing mss-supply
--

commit 726993c6f2fb948fb92664f68a46c91f069ed760
Author: Barnabás Czémán <barnabas.czeman@mainlining.org>

remoteproc: qcom_q6v5_mss: Use mss as regulator for MSM8953

The commit modifies the MSM8953 MSS configuration to use an "mss" regulator
rather than a power domain. The device tree definition is updated to remove
the power domain, and the driver is updated to expect the new active supply.

> diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> index 753167c3f8619..c7dc08858d542 100644
> --- a/arch/arm64/boot/dts/qcom/msm8953.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> @@ -1451,9 +1451,8 @@ mpss: remoteproc@4080000 {
>  					  "handover", "stop-ack";
>  
>  			power-domains = <&rpmpd MSM8953_VDDCX>,
> -					<&rpmpd MSM8953_VDDMX>,
> -					<&rpmpd MSM8953_VDDMD>;
> -			power-domain-names = "cx", "mx","mss";
> +					<&rpmpd MSM8953_VDDMX>;
> +			power-domain-names = "cx", "mx";

[Severity: High]
The old power domain is removed but the required mss-supply replacement to power
the MSS hardware is missing.

Is it expected that mss-supply is missing here?

Without mss-supply in the device tree, devm_regulator_get(dev, "mss") will
return either -ENODEV or a dummy regulator.

>  
>  			clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
>  				 <&gcc GCC_MSS_Q6_BIMC_AXI_CLK>,

[ ... ]

> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 2f71ed2feff69..8f6312213e1ab 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -2728,6 +2728,14 @@ static const struct rproc_hexagon_res msm8953_mss = {
>  		},
>  		{}
>  	},
> +	.active_supply = (struct qcom_mss_reg_res[]) {
> +		{
> +			.supply = "mss",
> +			.uV = 1050000,
> +			.uA = 100000,
> +		},

[Severity: High]
This makes the "mss" regulator mandatory and strictly requires a specific
voltage to be set.

Since a dummy regulator lacks voltage-setting operations, an unconditional call
to regulator_set_voltage() in q6v5_regulator_enable() will return -EINVAL,
causing the driver to unconditionally fail to probe:

drivers/remoteproc/qcom_q6v5_mss.c:q6v5_regulator_enable() {
    ...
			ret = regulator_set_voltage(regs[i].reg,
					regs[i].uV, INT_MAX);
			if (ret) {
    ...
}

Could this cause the modem driver to fail probing, making the modem unusable
on all MSM8953 devices (and SDM632 devices due to later patches in the series)?

> +		{}
> +	},

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002-sdm632-rpmpd-v3-0-95c47d0f26cf@mainlining.org?part=4