Re: [PATCH v6 1/2] arm64: dts: qcom: sm8250: sort out Iris power domains

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The Venus driver ignores the newly added 'mmcx' power domain, preventing its voltage levels from being scaled or enabled.
- [Low] The addition of a fourth power domain violates the existing device tree binding schema for the Venus node.
- [Low] Duplicate phandles to the same OPP table in `required-opps` violate the `opp-v2` device tree binding schema.
--

commit b2dff816f619c94b6f1db957c2bb928b6d9248f2
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

arm64: dts: qcom: sm8250: sort out Iris power domains

This commit adds the MMCX power domain to the Iris device node on SM8250
to support MMCX voltage levels. It updates the power-domains,
power-domain-names, and required-opps properties.

> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 7076720413ab2..1fcbd400a3be3 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4326,8 +4326,12 @@ venus: video-codec@aa00000 {
>  			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>  			power-domains = <&videocc MVS0C_GDSC>,
>  					<&videocc MVS0_GDSC>,
> -					<&rpmhpd RPMHPD_MX>;
> -			power-domain-names = "venus", "vcodec0", "mx";
> +					<&rpmhpd RPMHPD_MX>,
> +					<&rpmhpd RPMHPD_MMCX>;
> +			power-domain-names = "venus",
> +					     "vcodec0",
> +					     "mx",
> +					     "mmcx";

Does this addition cause a device tree schema regression? The schema in
Documentation/devicetree/bindings/media/qcom,sm8250-venus.yaml currently
enforces maxItems: 3 for power-domains and restricts power-domain-names
to exactly "venus", "vcodec0", and "mx".

Will the qcom/venus driver actually attach to this new mmcx power domain?
Looking at drivers/media/platform/qcom/venus/pm_helpers.c,
vcodec_domains_get() initializes opp_pd_data with num_pd_names = 1, and the
hardware resource struct sm8250_res.opp_pmdomain in core.c only lists
{ "mx" }.

Since "mmcx" is not in this array, devm_pm_domain_attach_list() will only
attach "mx". This would cause opp_table->required_devs[1] to remain NULL,
and the OPP core might silently skip setting the performance state for MMCX.
Does the driver need an update to properly manage the MMCX rail?

[ ... ]

> @@ -4353,22 +4357,26 @@ venus_opp_table: opp-table {
>  
>  				opp-720000000 {
>  					opp-hz = /bits/ 64 <720000000>;
> -					required-opps = <&rpmhpd_opp_low_svs>;
> +					required-opps = <&rpmhpd_opp_low_svs>,
> +							<&rpmhpd_opp_low_svs>;

Could these duplicate phandles cause a schema validation regression?
The opp-v2 bindings in Documentation/devicetree/bindings/opp/opp-v2-base.yaml
explicitly state that required-opps should not contain multiple phandles to
the OPP nodes in the same OPP table.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-iris-venus-fix-sm8250-v6-0-51856dbef83c@oss.qualcomm.com?part=1