Re: [PATCH v2 3/3] soc: mediatek: pwrap: add mt6572 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unconditional access to `wrp->slave->dew_regs` in `pwrap_common_init_reg_clock` for `PWRAP_MT6572` can lead to a NULL pointer dereference or unintended PMIC register writes.
--

--- Patch [3]: [PATCH v2 3/3] soc: mediatek: pwrap: add mt6572 support ---
commit cbf0689188d0318bc80532d5ebc3a4d9de137810
Author: Roman Vivchar <rva333@protonmail.com>

soc: mediatek: pwrap: add mt6572 support

This commit adds mt6572 registers and platform data for the pmic wrapper,
omitting registers not included in the mt2701 array. It reuses the mt2701
SoC-specific initialization function.

> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index a4b10b0a97cbb..3f05fc724e15b 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
[ ... ]
> @@ -1733,6 +1734,11 @@ static void pwrap_init_chip_select_ext(struct pmic_wrapper *wrp, u8 hext_write,
>  static int pwrap_common_init_reg_clock(struct pmic_wrapper *wrp)
>  {
>  	switch (wrp->master->type) {
> +	case PWRAP_MT6572:
> +		pwrap_writel(wrp, 0x8, PWRAP_RDDMY);
> +		pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_RDDMY_NO], 0x8);

[Severity: Medium]
Could this unconditional access to wrp->slave->dew_regs cause a NULL pointer
dereference or unintended register writes if a malformed device tree is
provided?

If mediatek,mt6572-pwrap is paired with a PMIC that lacks dew_regs (such as
mediatek,mt6380-regulator) or lacks PWRAP_DEW_RDDMY_NO, pwrap_common_init_reg_clock
will blindly dereference the array.

Other cases in this switch block, such as PWRAP_MT6795, verify the slave type
before accessing dew_regs:

	case PWRAP_MT6795:
		if (wrp->slave->type == PMIC_MT6331) {
			const u32 *dew_regs = wrp->slave->dew_regs;
			pwrap_write(wrp, dew_regs[PWRAP_DEW_RDDMY_NO], 0x8);
            ...

Would it be safer to add a similar slave type validation here to ensure we fail
gracefully on misconfigured device trees?

> +		pwrap_init_chip_select_ext(wrp, 5, 0, 0, 0);
> +		break;
>  	case PWRAP_MT6795:
>  		if (wrp->slave->type == PMIC_MT6331) {
>  			const u32 *dew_regs = wrp->slave->dew_regs;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-6572-pwrap-v2-0-bea1d801c81b@protonmail.com?part=3