Re: [PATCH v2 05/16] iio: adc: mediatek: add mt6323 PMIC AUXADC driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware initialization uses inverted logic, powering down the ADC clock and disabling its voltage reference.
--

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

iio: adc: mediatek: add mt6323 PMIC AUXADC driver

This commit adds a new driver for the 15-bit AUXADC found in the MediaTek
mt6323 PMIC. It provides support for reading channels like battery voltage,
temperatures, and accessory detection.

> diff --git a/drivers/iio/adc/mt6323-auxadc.c b/drivers/iio/adc/mt6323-auxadc.c
> new file mode 100644
> index 0000000000000..2c2b495e3d382
> --- /dev/null
> +++ b/drivers/iio/adc/mt6323-auxadc.c

[ ... ]

> +static int mt6323_auxadc_init(struct mt6323_auxadc *auxadc)
> +{
> +	struct regmap *map = auxadc->regmap;
> +	int ret;
> +
> +	ret = regmap_set_bits(map, MT6323_STRUP_CON10,
> +			      AUXADC_RSTB_SW | AUXADC_RSTB_SEL);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(map, MT6323_TOP_CKPDN2, AUXADC_CTL_CK);

Is it intentional to set AUXADC_CTL_CK here? In MediaTek PMICs, CKPDN
typically stands for Clock Power Down, meaning a value of 1 powers off the
clock. Should this use regmap_clear_bits() instead to enable the ADC clock?

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(map, MT6323_AUXADC_CON10,
> +			      AUXADC_TRIM_CH2 | AUXADC_TRIM_CH4 |
> +				      AUXADC_TRIM_CH5 | AUXADC_TRIM_CH6);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(map, MT6323_AUXADC_CON27,
> +			      AUXADC_VREF18_ENB_MD | AUXADC_MD_STATUS);

Does setting AUXADC_VREF18_ENB_MD disable the 1.8V analog voltage reference?

The _ENB suffix usually indicates an active-low enable signal. If setting
this bit to 1 disables the reference voltage, it might leave the hardware
functionally offline.

Could regmap_clear_bits() be the correct operation for the active-low enable
bar signal?

> +	if (ret)
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-mt6323-v2-0-3efcba579e88@protonmail.com?part=5