Re: [PATCH v2 3/3] pmdomain: mediatek: Add support for MT6858 SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The MT6858 power domains such as VDE0, VEN0, AUDIO, and SSUSB do not include the `MTK_SCPD_KEEP_DEFAULT_OFF` capability flag. Without this flag, `scpsys_add_one_domain()` will unconditionally execute the power-on sequence during probe and initialize the domains with `is_off=false`, setting `stay_on=true`. This locks the domains in a powered-on state until `sync_state` is reached (i.e., all consumers have probed). Wasting power on unused video decoder/encoder domains is unnecessary unless explicitly required by the bootloader.
--

--- Patch [3]: [PATCH v2 3/3] pmdomain: mediatek: Add support for MT6858 SoC ---
commit 4b694c86cffa3ba63d8623fd34074d76f6348e26
Author: Nikolai Burov <nikolai.burov@jolla.com>
Subject: pmdomain: mediatek: Add support for MT6858 SoC

This commit adds support for the power domains found in the MediaTek MT6858
SoC. It defines the bus protection blocks and domain data for various hardware
subsystems.

> diff --git a/drivers/pmdomain/mediatek/mt6858-pm-domains.h b/drivers/pmdomain/mediatek/mt6858-pm-domains.h
> new file mode 100644
> index 0000000000000..177b2e86ab1eb
> --- /dev/null
> +++ b/drivers/pmdomain/mediatek/mt6858-pm-domains.h

[ ... ]

> +	[MT6858_POWER_DOMAIN_VDE0] = {
> +		.name = "vde0",
> +		.sta_mask = MT6858_PWR_ACK,
> +		.sta2nd_mask = MT6858_PWR_ACK_2ND,
> +		.ctl_offs = 0xe34,
> +		.pwr_sta_offs = 0xe34,
> +		.pwr_sta2nd_offs = 0xe34,
> +		.sram_pdn_bits = BIT(8),
> +		.sram_pdn_ack_bits = BIT(12),
> +		.bp_cfg = {
> +			BUS_PROT_WR_IGN(INFRA,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_VDE0,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_SET,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_CLR,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_RDY),
> +			BUS_PROT_WR_IGN(INFRA,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_VDE0,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_SET,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_CLR,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_RDY),
> +		},
> +	},

[Severity: Medium]
Does this domain need to be powered on unconditionally at boot?

Without the MTK_SCPD_KEEP_DEFAULT_OFF capability flag in scpsys_domain_data,
scpsys_add_one_domain() will execute the power-on sequence during probe and
initialize the domain with is_off=false.

This sets stay_on=true, locking the domain in a powered-on state until
sync_state is reached after all consumers have probed. If the consumer drivers
for the video decoder never probe, this power will be wasted indefinitely.

Other similar MediaTek SoCs typically set this flag for video and audio
domains. Could the MTK_SCPD_KEEP_DEFAULT_OFF flag be added here, as well as
for the AUDIO and SSUSB domains?

> +	[MT6858_POWER_DOMAIN_VEN0] = {
> +		.name = "ven0",
> +		.sta_mask = MT6858_PWR_ACK,
> +		.sta2nd_mask = MT6858_PWR_ACK_2ND,
> +		.ctl_offs = 0xe3c,
> +		.pwr_sta_offs = 0xe3c,
> +		.pwr_sta2nd_offs = 0xe3c,
> +		.sram_pdn_bits = BIT(8),
> +		.sram_pdn_ack_bits = BIT(12),
> +		.bp_cfg = {
> +			BUS_PROT_WR_IGN(INFRA,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_VEN0,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_SET,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_CLR,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_0_RDY),
> +			BUS_PROT_WR_IGN(INFRA,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_VEN0,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_SET,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_CLR,
> +					MT6858_TOP_AXI_PROT_EN_MMSYS_STA_1_RDY),
> +		},
> +	},

[Severity: Medium]
Similarly, does the VEN0 (Video Encoder) domain require the
MTK_SCPD_KEEP_DEFAULT_OFF flag to avoid staying powered on indefinitely
when its consumers do not probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-mt6858-pmdomain-v2-0-6293e87fc093@jolla.com?part=3