Re: [PATCH 3/4] iio: adc: ade9000: add support for ADE9430

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The ADE9000_REG_DICOEFF configuration register is written after the DSP has been started, violating initialization requirements for existing devices.
- [Medium] The ADE9430 full-scale ADC codes are identical to the ADE9000 codes, contradicting the commit message claim that they are different.
--

commit 95855f12cd746ba14c4e360029b507dc042d8327
Author: Antoniu Miclaus <antoniu.miclaus@analog.com>

iio: adc: ade9000: add support for ADE9430

This commit adds support for the ADE9430, noting it is register compatible
with the ADE9000 but lacks the on-chip digital integrator and has slightly
different full-scale ADC codes.

> The ADE9430 is a polyphase energy metering device that is register
> compatible with the ADE9000. The differences relevant to the driver are
> the absence of the on-chip dip/swell detection, the absence of an on-chip
> digital integrator (the ADE9430 uses an external analog integrator for
> Rogowski coils, so the DICOEFF register does not exist) and slightly
> different full-scale ADC codes.

[Severity: Medium]
The commit message mentions that the ADE9430 has "slightly different
full-scale ADC codes", but looking at the newly added ade9430_chip_info
struct below, the rms_full_scale_codes, watt_full_scale_codes, and
pcf_full_scale_codes are identical to the ADE9000 codes.

> diff --git a/drivers/iio/adc/ade9000.c b/drivers/iio/adc/ade9000.c
> index 0f64bfb221da0..2f5fe016a82f3 100644
> --- a/drivers/iio/adc/ade9000.c
> +++ b/drivers/iio/adc/ade9000.c
[ ... ]
> @@ -689,6 +693,16 @@ static const struct ade9000_chip_info ade9078_chip_info = {
>  	.rms_full_scale_codes = 52866837,
>  	.watt_full_scale_codes = 20823646,
>  	.pcf_full_scale_codes = 74680000,
> +	.has_digital_integrator = true,
> +};
> +
> +static const struct ade9000_chip_info ade9430_chip_info = {
> +	.name = "ade9430",
> +	.channels = ade9430_channels,
> +	.num_channels = ARRAY_SIZE(ade9430_channels),
> +	.rms_full_scale_codes = 52702092,
> +	.watt_full_scale_codes = 20694066,
> +	.pcf_full_scale_codes = 74532013,
>  };
>  

Are these the correct values for the ADE9430, or were they inadvertently
copied from the ADE9000 without being updated from the datasheet?

> @@ -704,7 +718,6 @@ static const struct reg_sequence ade9000_initialization_sequence[] = {
>  	{ ADE9000_REG_EVENT_MASK, ADE9000_EVENT_DISABLE },
>  	{ ADE9000_REG_WFB_CFG, ADE9000_WFB_CFG },
>  	{ ADE9000_REG_VLEVEL, ADE9000_VLEVEL },
> -	{ ADE9000_REG_DICOEFF, ADE9000_DICOEFF },
>  	{ ADE9000_REG_EGY_TIME, ADE9000_EGY_TIME },
>  	{ ADE9000_REG_EP_CFG, ADE9000_EP_CFG },
>  	/* Clear all pending status bits by writing 1s */
>  	{ ADE9000_REG_STATUS0, GENMASK(31, 0) },
>  	{ ADE9000_REG_STATUS1, GENMASK(31, 0) },
>  	{ ADE9000_REG_RUN, ADE9000_RUN_ON }
>  };
[ ... ]
> @@ -1672,6 +1685,14 @@ static int ade9000_setup(struct ade9000_state *st)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to write register sequence");
>  
> +	/* The ADE9430 has no on-chip digital integrator and lacks DICOEFF. */
> +	if (st->info->has_digital_integrator) {
> +		ret = regmap_write(st->regmap, ADE9000_REG_DICOEFF,
> +				   ADE9000_DICOEFF);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to set DICOEFF\n");
> +	}
> +
>  	fsleep(2000);
>  
>  	return 0;

[Severity: High]
Does this change cause a regression for existing devices by writing the
DICOEFF register after the DSP has already been started?

Because ADE9000_REG_DICOEFF is removed from ade9000_initialization_sequence,
the multi-register sequence now concludes with setting ADE9000_REG_RUN to
ADE9000_RUN_ON without writing DICOEFF first.

Then, ade9000_setup() conditionally writes ADE9000_REG_DICOEFF after the
initialization sequence has already started the DSP. Could this lead to the
configuration being ignored or cause unpredictable calculations since it is
written while the DSP is running?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907101623.8730-1-antoniu.miclaus@analog.com?part=3