Re: [PATCH v3 3/7] pmdomain: qcom: rpmpd: Split SDM632 pm domains from MSM8953

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pmdomain: qcom: rpmpd: DT ABI break and log spam from removed domains
- [Low] dt-bindings: power: qcom-rpmpd: duplicate ID for RPMPD_VDDMD
--

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

pmdomain: qcom: rpmpd: Split SDM632 pm domains from MSM8953

Remove modem related pm domains from MSM8953 rpmpd because MSM8953 MSS
is using mss-supply as regulator. Split SDM632 rpmpd from MSM8953 because
SDM632 MSS is using mss-supply as pm domain.

> diff --git a/drivers/pmdomain/qcom/rpmpd.c b/drivers/pmdomain/qcom/rpmpd.c
> index 5f55fc791131a..c37a79d6bc433 100644
> --- a/drivers/pmdomain/qcom/rpmpd.c
> +++ b/drivers/pmdomain/qcom/rpmpd.c

[ ... ]

> @@ -703,8 +692,6 @@ static const struct rpmpd_desc msm8917_desc = {
>  };
>  
>  static struct rpmpd *msm8953_rpmpds[] = {
> -	[MSM8953_VDDMD] =	&md_s1a_lvl,
> -	[MSM8953_VDDMD_AO] =	&md_s1a_lvl_ao,

[Severity: High]
Does removing these entries break ABI compatibility for older device trees?

By removing MSM8953_VDDMD (0) and MSM8953_VDDMD_AO (1), these array indices
become NULL. When rpmpd_probe() iterates over the array, it will encounter
these empty entries and trigger a warning:

	if (!rpmpds[i]) {
		dev_warn(&pdev->dev, "rpmpds[] with empty entry at index=%d\n",
			 i);
		continue;
	}

Will this cause unconditional boot log warnings on MSM8953 systems?

Additionally, if older device trees still reference <&rpmpd MSM8953_VDDMD>,
would of_genpd_xlate_onecell() return an error and cause consumer drivers
to indefinitely defer probing?

>  	[MSM8953_VDDCX] =	&cx_s2a_lvl,
>  	[MSM8953_VDDCX_AO] =	&cx_s2a_lvl_ao,
>  	[MSM8953_VDDCX_VFL] =	&cx_s2a_vfl,

[ ... ]

> @@ -844,6 +831,21 @@ static const struct rpmpd_desc qm215_desc = {
>  	.max_state = RPM_SMD_LEVEL_TURBO,
>  };
>  
> +static struct rpmpd *sdm632_rpmpds[] = {
> +	[RPMPD_VDDMD] =		&md_s1a_lvl,

[Severity: Low]
Is there a conflicting index assignment introduced by using RPMPD_VDDMD here?

Looking at include/dt-bindings/power/qcom-rpmpd.h, RPMPD_VDDMD is defined
as 5, which is the exact same ID assigned to RPMPD_VDDMX_VFL:

#define RPMPD_VDDMX_VFL		5
#define RPMPD_VDDMD		5

While SDM632 does not use VDDMX_VFL, could having two distinct generic PM
domains share the same integer ID break the uniqueness guarantee of generic
bindings and cause silent overwrites if a future SoC initializes both?

> +	[RPMPD_VDDCX] =		&cx_s2a_lvl,
> +	[RPMPD_VDDCX_AO] =	&cx_s2a_lvl_ao,
> +	[RPMPD_VDDCX_VFL] =	&cx_s2a_vfl,
> +	[RPMPD_VDDMX] =		&mx_s7a_lvl,
> +	[RPMPD_VDDMX_AO] =	&mx_s7a_lvl_ao,
> +};

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