Re: [PATCH 3/4] iio: adc: ade9000: add support for ADE9078
Jonathan Cameron <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260724011229.0cebaf62@jic23-huawei> |
On Mon, 20 Jul 2026 14:07:46 +0300 Antoniu Miclaus <[email protected]> wrote: > The ADE9078 is a polyphase energy metering device that is register > compatible with the ADE9000. The main differences relevant to the driver > are the absence of the on-chip dip/swell detection and slightly different > full-scale ADC codes. > > Add a dedicated channel table that omits the swell/dip voltage events and > a matching chip_info describing the ADE9078 full-scale codes. The RMS > voltage channel macro is split into a shared base and two variants so the > event-less table can reuse the common definition. > > Signed-off-by: Antoniu Miclaus <[email protected]> Hi Antoniu. Just a couple of minor comments below. Jonathan > --- > drivers/iio/adc/Kconfig | 14 ++--- > drivers/iio/adc/ade9000.c | 121 +++++++++++++++++++++++++------------- > 2 files changed, 86 insertions(+), 49 deletions(-) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index d1b198cb8a80..33f2c281b1aa 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -573,20 +573,20 @@ config AD9467 > called ad9467. > > config ADE9000 > - tristate "Analog Devices ADE9000 Multiphase Energy, and Power Quality Monitoring IC Driver" > + tristate "Analog Devices ADE9000/ADE9078 Multiphase Energy, and Power Quality Monitoring IC Driver" > depends on SPI > select REGMAP_SPI > select IIO_BUFFER > select IIO_KFIFO_BUF > help > - Say yes here to build support for the Analog Devices ADE9000, > - a highly accurate, multiphase energy and power quality monitoring > - integrated circuit. > + Say yes here to build support for the Analog Devices ADE9000 and > + ADE9078, highly accurate, multiphase energy and power quality > + monitoring integrated circuits. Do you think you are likely to end up adding a bunch more supported parts to this driver over time? Might be worth moving to a generic 'and similar' title and a bulleted list in here if so. > > - The device features high-precision analog-to-digital converters > + The devices feature high-precision analog-to-digital converters > and digital signal processing to compute RMS values, power factor, > - frequency, and harmonic analysis. It supports SPI communication > - and provides buffered data output through the IIO framework. > + frequency, and harmonic analysis. They support SPI communication > + and provide buffered data output through the IIO framework. > > To compile this driver as a module, choose M here: the module will > be called ade9000. > diff --git a/drivers/iio/adc/ade9000.c b/drivers/iio/adc/ade9000.c > index 38a5c7e043b3..0de55b7ad1ea 100644 > --- a/drivers/iio/adc/ade9000.c > +++ b/drivers/iio/adc/ade9000.c > @@ -502,7 +502,7 @@ static const struct iio_chan_spec_ext_info ade9000_ext_info[] = { > .scan_index = -1 \ > } > > -#define ADE9000_ALTVOLTAGE_RMS_CHANNEL(num) { \ > +#define ADE9000_ALTVOLTAGE_RMS_CHANNEL_BASE(num) \ I'd prefer we just duplicated the common stuff. It's not big enough to be worth nesting multiple macros to fill up the structure > .type = IIO_ALTVOLTAGE, \ > .channel = num, \ > .address = ADE9000_ADDR_ADJUST(ADE9000_REG_AVRMS, num), \ > @@ -512,9 +512,18 @@ static const struct iio_chan_spec_ext_info ade9000_ext_info[] = { > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > BIT(IIO_CHAN_INFO_SCALE) | \ > BIT(IIO_CHAN_INFO_CALIBBIAS), \ > + .scan_index = -1 > + > +/* With swell/dip (sag) events - ADE9000 only */ > +#define ADE9000_ALTVOLTAGE_RMS_CHANNEL(num) { \ > + ADE9000_ALTVOLTAGE_RMS_CHANNEL_BASE(num), \ > .event_spec = ade9000_rms_voltage_events, \ > .num_event_specs = ARRAY_SIZE(ade9000_rms_voltage_events), \ > - .scan_index = -1 \ > +} > + > +/* Without swell/dip events - parts lacking DIP_LVL/SWELL_LVL hardware */ > +#define ADE9000_ALTVOLTAGE_RMS_CHANNEL_NO_EVENTS(num) { \ > + ADE9000_ALTVOLTAGE_RMS_CHANNEL_BASE(num), \ > } > > #define ADE9000_POWER_ACTIVE_CHANNEL(num) { \ > @@ -598,48 +607,65 @@ static const struct iio_chan_spec_ext_info ade9000_ext_info[] = { > .scan_index = -1 \ > } > > -static const struct iio_chan_spec ade9000_channels[] = { > - /* Phase A channels */ > - ADE9000_CURRENT_CHANNEL(ADE9000_PHASE_A_NR), > - ADE9000_VOLTAGE_CHANNEL(ADE9000_PHASE_A_NR), > - ADE9000_ALTCURRENT_RMS_CHANNEL(ADE9000_PHASE_A_NR), > - ADE9000_ALTVOLTAGE_RMS_CHANNEL(ADE9000_PHASE_A_NR), > - ADE9000_POWER_ACTIVE_CHANNEL(ADE9000_PHASE_A_NR), > - ADE9000_POWER_REACTIVE_CHANNEL(ADE9000_PHASE_A_NR), > - ADE9000_POWER_APPARENT_CHANNEL(ADE9000_PHASE_A_NR), > - ADE9000_ENERGY_ACTIVE_CHANNEL(ADE9000_PHASE_A_NR, ADE9000_REG_AWATTHR_LO), > - ADE9000_ENERGY_APPARENT_CHANNEL(ADE9000_PHASE_A_NR, ADE9000_REG_AVAHR_LO), > - ADE9000_ENERGY_REACTIVE_CHANNEL(ADE9000_PHASE_A_NR, ADE9000_REG_AFVARHR_LO), > - ADE9000_POWER_FACTOR_CHANNEL(ADE9000_PHASE_A_NR), > - /* Phase B channels */ > - ADE9000_CURRENT_CHANNEL(ADE9000_PHASE_B_NR), > - ADE9000_VOLTAGE_CHANNEL(ADE9000_PHASE_B_NR), > - ADE9000_ALTCURRENT_RMS_CHANNEL(ADE9000_PHASE_B_NR), > - ADE9000_ALTVOLTAGE_RMS_CHANNEL(ADE9000_PHASE_B_NR), > - ADE9000_POWER_ACTIVE_CHANNEL(ADE9000_PHASE_B_NR), > - ADE9000_POWER_REACTIVE_CHANNEL(ADE9000_PHASE_B_NR), > - ADE9000_POWER_APPARENT_CHANNEL(ADE9000_PHASE_B_NR), > - ADE9000_ENERGY_ACTIVE_CHANNEL(ADE9000_PHASE_B_NR, ADE9000_REG_BWATTHR_LO), > - ADE9000_ENERGY_APPARENT_CHANNEL(ADE9000_PHASE_B_NR, ADE9000_REG_BVAHR_LO), > - ADE9000_ENERGY_REACTIVE_CHANNEL(ADE9000_PHASE_B_NR, ADE9000_REG_BFVARHR_LO), > - ADE9000_POWER_FACTOR_CHANNEL(ADE9000_PHASE_B_NR), > - /* Phase C channels */ > - ADE9000_CURRENT_CHANNEL(ADE9000_PHASE_C_NR), > - ADE9000_VOLTAGE_CHANNEL(ADE9000_PHASE_C_NR), > - ADE9000_ALTCURRENT_RMS_CHANNEL(ADE9000_PHASE_C_NR), > - ADE9000_ALTVOLTAGE_RMS_CHANNEL(ADE9000_PHASE_C_NR), > - ADE9000_POWER_ACTIVE_CHANNEL(ADE9000_PHASE_C_NR), > - ADE9000_POWER_REACTIVE_CHANNEL(ADE9000_PHASE_C_NR), > - ADE9000_POWER_APPARENT_CHANNEL(ADE9000_PHASE_C_NR), > - ADE9000_ENERGY_ACTIVE_CHANNEL(ADE9000_PHASE_C_NR, ADE9000_REG_CWATTHR_LO), > - ADE9000_ENERGY_APPARENT_CHANNEL(ADE9000_PHASE_C_NR, ADE9000_REG_CVAHR_LO), > - ADE9000_ENERGY_REACTIVE_CHANNEL(ADE9000_PHASE_C_NR, ADE9000_REG_CFVARHR_LO), > - ADE9000_POWER_FACTOR_CHANNEL(ADE9000_PHASE_C_NR), > -}; > +/* > + * Declare a full channel array. @altvoltage_rms picks the RMS voltage channel > + * variant so parts without dip/swell hardware (e.g. ADE9078) omit those events. > + */ > +#define ADE9000_DECLARE_CHANNELS(_name, altvoltage_rms) \ > + static const struct iio_chan_spec _name[] = { \ > + /* Phase A channels */ \ > + ADE9000_CURRENT_CHANNEL(ADE9000_PHASE_A_NR), \ > + ADE9000_VOLTAGE_CHANNEL(ADE9000_PHASE_A_NR), \ > + ADE9000_ALTCURRENT_RMS_CHANNEL(ADE9000_PHASE_A_NR), \ > + altvoltage_rms(ADE9000_PHASE_A_NR), \ > + ADE9000_POWER_ACTIVE_CHANNEL(ADE9000_PHASE_A_NR), \ > + ADE9000_POWER_REACTIVE_CHANNEL(ADE9000_PHASE_A_NR), \ > + ADE9000_POWER_APPARENT_CHANNEL(ADE9000_PHASE_A_NR), \ > + ADE9000_ENERGY_ACTIVE_CHANNEL(ADE9000_PHASE_A_NR, \ > + ADE9000_REG_AWATTHR_LO), \ > + ADE9000_ENERGY_APPARENT_CHANNEL(ADE9000_PHASE_A_NR, \ > + ADE9000_REG_AVAHR_LO), \ > + ADE9000_ENERGY_REACTIVE_CHANNEL(ADE9000_PHASE_A_NR, \ > + ADE9000_REG_AFVARHR_LO), \ > + ADE9000_POWER_FACTOR_CHANNEL(ADE9000_PHASE_A_NR), \ > + /* Phase B channels */ \ > + ADE9000_CURRENT_CHANNEL(ADE9000_PHASE_B_NR), \ > + ADE9000_VOLTAGE_CHANNEL(ADE9000_PHASE_B_NR), \ > + ADE9000_ALTCURRENT_RMS_CHANNEL(ADE9000_PHASE_B_NR), \ > + altvoltage_rms(ADE9000_PHASE_B_NR), \ > + ADE9000_POWER_ACTIVE_CHANNEL(ADE9000_PHASE_B_NR), \ > + ADE9000_POWER_REACTIVE_CHANNEL(ADE9000_PHASE_B_NR), \ > + ADE9000_POWER_APPARENT_CHANNEL(ADE9000_PHASE_B_NR), \ > + ADE9000_ENERGY_ACTIVE_CHANNEL(ADE9000_PHASE_B_NR, \ > + ADE9000_REG_BWATTHR_LO), \ > + ADE9000_ENERGY_APPARENT_CHANNEL(ADE9000_PHASE_B_NR, \ > + ADE9000_REG_BVAHR_LO), \ > + ADE9000_ENERGY_REACTIVE_CHANNEL(ADE9000_PHASE_B_NR, \ > + ADE9000_REG_BFVARHR_LO), \ > + ADE9000_POWER_FACTOR_CHANNEL(ADE9000_PHASE_B_NR), \ > + /* Phase C channels */ \ > + ADE9000_CURRENT_CHANNEL(ADE9000_PHASE_C_NR), \ > + ADE9000_VOLTAGE_CHANNEL(ADE9000_PHASE_C_NR), \ > + ADE9000_ALTCURRENT_RMS_CHANNEL(ADE9000_PHASE_C_NR), \ > + altvoltage_rms(ADE9000_PHASE_C_NR), \ > + ADE9000_POWER_ACTIVE_CHANNEL(ADE9000_PHASE_C_NR), \ > + ADE9000_POWER_REACTIVE_CHANNEL(ADE9000_PHASE_C_NR), \ > + ADE9000_POWER_APPARENT_CHANNEL(ADE9000_PHASE_C_NR), \ > + ADE9000_ENERGY_ACTIVE_CHANNEL(ADE9000_PHASE_C_NR, \ > + ADE9000_REG_CWATTHR_LO), \ > + ADE9000_ENERGY_APPARENT_CHANNEL(ADE9000_PHASE_C_NR, \ > + ADE9000_REG_CVAHR_LO), \ > + ADE9000_ENERGY_REACTIVE_CHANNEL(ADE9000_PHASE_C_NR, \ > + ADE9000_REG_CFVARHR_LO), \ > + ADE9000_POWER_FACTOR_CHANNEL(ADE9000_PHASE_C_NR), \ > + } > + > +ADE9000_DECLARE_CHANNELS(ade9000_channels, ADE9000_ALTVOLTAGE_RMS_CHANNEL); > +ADE9000_DECLARE_CHANNELS(ade9078_channels, ADE9000_ALTVOLTAGE_RMS_CHANNEL_NO_EVENTS); Given this one would be much larger to duplicate I'm fine with the slightly more complex macros here. > > /* > - * Full-scale codes referred from the datasheet. These are the digital codes > - * produced when the ADC inputs are at full scale. > + * Full-scale codes referred from the respective datasheets. These are the > + * digital codes produced when the ADC inputs are at full scale. Can just use this when bring in chip info and avoid the churn. > */ > static const struct ade9000_chip_info ade9000_chip_info = { > .name = "ade9000", > @@ -650,6 +676,15 @@ static const struct ade9000_chip_info ade9000_chip_info = { > .pcf_full_scale_codes = 74770000, > }; > > +static const struct ade9000_chip_info ade9078_chip_info = { > + .name = "ade9078", > + .channels = ade9078_channels, > + .num_channels = ARRAY_SIZE(ade9078_channels), > + .rms_full_scale_codes = 52866837, > + .watt_full_scale_codes = 20823646, > + .pcf_full_scale_codes = 74680000, > +}; > + > static const struct reg_sequence ade9000_initialization_sequence[] = { > { ADE9000_REG_PGA_GAIN, ADE9000_PGA_GAIN }, > { ADE9000_REG_CONFIG0, ADE9000_CONFIG0 }, > @@ -1800,12 +1835,14 @@ static int ade9000_probe(struct spi_device *spi) > > static const struct spi_device_id ade9000_id[] = { > { .name = "ade9000", .driver_data = (kernel_ulong_t)&ade9000_chip_info }, > + { .name = "ade9078", .driver_data = (kernel_ulong_t)&ade9078_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(spi, ade9000_id); > > static const struct of_device_id ade9000_of_match[] = { > { .compatible = "adi,ade9000", .data = &ade9000_chip_info }, > + { .compatible = "adi,ade9078", .data = &ade9078_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(of, ade9000_of_match);