Re: [PATCH v2 3/4] mfd: mt6397-core: add mt6323 AUXADC support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Setting the clock power down bit disables the AUXADC clock instead of enabling it.
--

commit bab9fefa6ba4c56ff9360f203a97d192c15f6cd1
Author: Roman Vivchar <rva333@protonmail.com>

mfd: mt6397-core: add mt6323 AUXADC support

The mt6323 PMIC includes an AUXADC. Register the AUXADC in the mt6323
devices array to allow the corresponding driver to probe using compatible
string.

> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index 3e58d0764c7e0..013b0857fb548 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -125,6 +125,9 @@ static const struct resource mt6323_pwrc_resources[] = {
>  
>  static const struct mfd_cell mt6323_devs[] = {
>  	{
> +		.name = "mt6323-auxadc",
> +		.of_compatible = "mediatek,mt6323-auxadc",

[Severity: High]
Does enabling this device expose an issue in the mt6323-auxadc driver's clock
initialization?

Looking at mt6323_auxadc_init() in drivers/iio/adc/mt6323-auxadc.c, it writes
a 1 to the power down register bit:

	ret = regmap_set_bits(map, MT6323_TOP_CKPDN2, AUXADC_TOP_CKPDN2_CTL_CK);
	if (ret)
		return ret;

Since CKPDN registers in MediaTek PMICs are Clock Power Down registers,
setting a bit to 1 explicitly disables the clock, while clearing it to 0
powers it up.

Because the AUXADC clock is explicitly disabled here and never turned back
on, will ADC read requests in mt6323_auxadc_read_raw() fail or time out
waiting for AUXADC_READY_MASK?

If the entire driver is functionally broken due to this, could it
subsequently break dependent features like battery management and thermal
protection?

> +	}, {
>  		.name = "mt6323-rtc",
>  		.num_resources = ARRAY_SIZE(mt6323_rtc_resources),
>  		.resources = mt6323_rtc_resources,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-mt6323-adc-v2-0-aa93a22309f9@protonmail.com?part=3