[3/3] iio: meter: add ADE9078 driver

Commit Message

Ciprian Hegbeli Feb. 17, 2022, 1:51 p.m. UTC

The ADE9078 is a highly accurate, fully integrated energy metering
device. It allows the monitoring of three independent phases
simultaneously, by using seven high performances ADCs and a flexible
DSP core for a fixed fundamental signal frequency of either 50Hz or
60Hz.

Datasheet:
www.analog.com/media/en/technical-documentation/data-sheets/ADE9078.pdf

Signed-off-by: chegbeli <[e-mail]>
---
 MAINTAINERS                 |    8 +
 drivers/iio/meter/Kconfig   |   13 +
 drivers/iio/meter/Makefile  |    2 +
 drivers/iio/meter/ade9078.c | 1666 +++++++++++++++++++++++++++++++++++
 4 files changed, 1689 insertions(+)
 create mode 100644 drivers/iio/meter/ade9078.c

Comments

Jonathan Cameron Feb. 17, 2022, 6:33 p.m. UTC | #1

On Thu, 17 Feb 2022 15:51:40 +0200
chegbeli <[e-mail]> wrote:

> The ADE9078 is a highly accurate, fully integrated energy metering
> device. It allows the monitoring of three independent phases
> simultaneously, by using seven high performances ADCs and a flexible
> DSP core for a fixed fundamental signal frequency of either 50Hz or
> 60Hz.
> 
> Datasheet:
> www.analog.com/media/en/technical-documentation/data-sheets/ADE9078.pdf
> 
> Signed-off-by: chegbeli <[e-mail]>

Hi Ciprian

I took a very quick end of day look so probably missed some stuff I'll
pick up on in v2.  Biggest issue is new ABI without docs. Various
other comments inline. Plus as already observed, please use full name
in from and SoB as they form part of the Developer Certificate of Origin.

Thanks,

Jonathan

> ---
>  MAINTAINERS                 |    8 +
>  drivers/iio/meter/Kconfig   |   13 +
>  drivers/iio/meter/Makefile  |    2 +
>  drivers/iio/meter/ade9078.c | 1666 +++++++++++++++++++++++++++++++++++
>  4 files changed, 1689 insertions(+)
>  create mode 100644 drivers/iio/meter/ade9078.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a2c8699e9e41..72fac9f4f837 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1101,6 +1101,14 @@ L:	[e-mail]
>  S:	Maintained
>  F:	drivers/media/i2c/ad9389b*
>  
> +ANALOG DEVICES INC ADE9078 DRIVER
> +M:	Ciprian Hegbeli <[e-mail]>
> +L:	[e-mail]
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
> +F:	drivers/iio/meter/ade9078.c
> +
>  ANALOG DEVICES INC ADGS1408 DRIVER
>  M:	Mircea Caprioru <[e-mail]>
>  S:	Supported
> diff --git a/drivers/iio/meter/Kconfig b/drivers/iio/meter/Kconfig
> index d1c91eed5283..b46e93af1052 100644
> --- a/drivers/iio/meter/Kconfig
> +++ b/drivers/iio/meter/Kconfig
> @@ -6,4 +6,17 @@
>  
>  menu "Active energy metering"
>  
> +config ADE9078
> +	tristate "Analog Devices ADE9078 Poly Phase Multifunction Energy Metering IC Driver"
> +	depends on SPI
> +	select REGMAP_SPI
> +	select IIO_BUFFER
> +	select IIO_KFIFO_BUF
> +	help
> +	  Support driver for the ADE9078Poly Phase Multifunction Energy Meter,
> +	  select y to build the driver.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called ade9078
> +
>  endmenu
> diff --git a/drivers/iio/meter/Makefile b/drivers/iio/meter/Makefile
> index c27cba44fc0b..866b7f7dc703 100644
> --- a/drivers/iio/meter/Makefile
> +++ b/drivers/iio/meter/Makefile
> @@ -3,3 +3,5 @@
>  # Makefile for enrgy metering drivers
>  #
>  # When adding new entries keep the list in alphabetical order
> +
> +obj-$(CONFIG_ADE9078) += ade9078.o
> diff --git a/drivers/iio/meter/ade9078.c b/drivers/iio/meter/ade9078.c
> new file mode 100644
> index 000000000000..cfcb093e4c27
> --- /dev/null
> +++ b/drivers/iio/meter/ade9078.c
> @@ -0,0 +1,1666 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADE9078 driver
> + *
> + * Copyright 2021 Analog Devices Inc.
> + */
> +
> +#include <asm/unaligned.h>

Preference to put asm includes after the linux ones.

> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +/* Address of ADE90XX registers */
> +#define	ADE9078_REG_AIGAIN		0x000
> +#define	ADE9078_REG_AVGAIN		0x00B
> +#define	ADE9078_REG_AIRMSOS		0x00C
> +#define	ADE9078_REG_AVRMSOS		0x00D
> +#define	ADE9078_REG_APGAIN		0x00E
> +#define	ADE9078_REG_AWATTOS		0x00F
> +#define	ADE9078_REG_AVAROS		0x010
> +#define	ADE9078_REG_AFVAROS		0x012
> +#define	ADE9078_REG_CONFIG0		0x060
> +#define	ADE9078_REG_DICOEFF		0x072
> +#define	ADE9078_REG_AI_PCF		0x20A
> +#define	ADE9078_REG_AV_PCF		0x20B
> +#define	ADE9078_REG_AIRMS		0x20C
> +#define	ADE9078_REG_AVRMS		0x20D
> +#define	ADE9078_REG_AWATT		0x210
> +#define	ADE9078_REG_AVAR		0x211
> +#define	ADE9078_REG_AVA			0x212
> +#define ADE9078_REG_AFVAR		0x214
> +#define	ADE9078_REG_APF			0x216
> +#define	ADE9078_REG_BI_PCF		0x22A
> +#define	ADE9078_REG_BV_PCF		0x22B
> +#define	ADE9078_REG_BIRMS		0x22C
> +#define	ADE9078_REG_BVRMS		0x22D
> +#define	ADE9078_REG_CI_PCF		0x24A
> +#define	ADE9078_REG_CV_PCF		0x24B
> +#define	ADE9078_REG_CIRMS		0x24C
> +#define	ADE9078_REG_CVRMS		0x24D
> +#define	ADE9078_REG_AWATT_ACC		0x2E5
> +#define	ADE9078_REG_STATUS0		0x402
> +#define	ADE9078_REG_STATUS1		0x403
> +#define	ADE9078_REG_MASK0		0x405
> +#define	ADE9078_REG_MASK1		0x406
> +#define	ADE9078_REG_EVENT_MASK		0x407
> +#define	ADE9078_REG_VLEVEL		0x40F
> +#define	ADE9078_REG_RUN			0x480
> +#define ADE9078_REG_CONFIG1		0x481
> +#define	ADE9078_REG_ACCMODE		0x492
> +#define	ADE9078_REG_CONFIG3		0x493
> +#define	ADE9078_REG_ZX_LP_SEL		0x49A
> +#define	ADE9078_REG_WFB_CFG		0x4A0
> +#define	ADE9078_REG_WFB_PG_IRQEN	0x4A1
> +#define	ADE9078_REG_WFB_TRG_CFG		0x4A2
> +#define	ADE9078_REG_WFB_TRG_STAT	0x4A3
> +#define	ADE9078_REG_CONFIG2		0x4AF
> +#define	ADE9078_REG_EP_CFG		0x4B0
> +#define	ADE9078_REG_EGY_TIME		0x4B2
> +#define	ADE9078_REG_PGA_GAIN		0x4B9
> +#define	ADE9078_REG_VERSION		0x4FE
> +#define ADE9078_REG_WF_BUFF		0x800
> +
> +#define ADE9078_REG_ADDR_MASK		GENMASK(15, 4)
> +#define ADE9078_REG_READ_BIT_MASK	BIT(3)
> +#define ADE9078_RX_DEPTH		6
> +#define ADE9078_TX_DEPTH		10
> +
> +#define ADE9078_WF_CAP_EN_MASK		BIT(4)
> +#define ADE9078_WF_CAP_SEL_MASK		BIT(5)
> +#define ADE9078_WF_MODE_MASK		GENMASK(7, 6)
> +#define ADE9078_WF_SRC_MASK		GENMASK(9, 8)
> +#define ADE9078_WF_IN_EN_MASK		BIT(12)
> +
> +/*
> + * Configuration registers
> + * PGA@0x0000. Gain of all channels=1
> + */
> +#define ADE9078_PGA_GAIN		0x0000
> +
> +/* Default configuration */
> +#define ADE9078_CONFIG0			0x00000000
> +
> +/* CF3/ZX pin outputs Zero crossing */
> +#define ADE9078_CONFIG1			0x0002

This looks like it could be better expressed in terms
of defines for the fields contained in this register.
Same for many of the ones that follow.

> +
> +/* Default High pass corner frequency of 1.25Hz */
> +#define ADE9078_CONFIG2			0x0A00
> +
> +/* Peak and overcurrent detection disabled */
> +#define ADE9078_CONFIG3			0x0000
> +
> +/*
> + * 50Hz operation, 3P4W Wye configuration, signed accumulation
> + * Clear bit 8 i.e. ACCMODE=0x00xx for 50Hz operation
> + * ACCMODE=0x0x9x for 3Wire delta when phase B is used as reference
> + */
> +#define ADE9078_ACCMODE			0x0000
> +
> +/*Line period and zero crossing obtained from VA */
> +#define ADE9078_ZX_LP_SEL		0x0000
> +
> +/* Disable all interrupts */
> +#define ADE9078_MASK0			0x00000000
> +
> +/* Disable all interrupts */
> +#define ADE9078_MASK1			0x00000000
> +
> +/* Events disabled */
> +#define ADE9078_EVENT_MASK		0x00000000
> +
> +/*
> + * Assuming Vnom=1/2 of full scale.
> + * Refer to Technical reference manual for detailed calculations.
> + */
> +#define ADE9078_VLEVEL			0x0022EA28
> +
> +/* Set DICOEFF= 0xFFFFE000 when integrator is enabled */
> +#define ADE9078_DICOEFF			0x00000000
> +
> +/* DSP ON */
> +#define ADE9078_RUN_ON			0xFFFFFFFF
> +
> +/*
> + * Energy Accumulation Settings
> + * Enable energy accumulation, accumulate samples at 8ksps
> + * latch energy accumulation after EGYRDY
> + * If accumulation is changed to half line cycle mode, change EGY_TIME
> + */
> +#define ADE9078_EP_CFG			0x0011
> +
> +/* Accumulate 4000 samples */
> +#define ADE9078_EGY_TIME		0x0FA0
> +
> +/*
> + * Constant Definitions
> + * ADE9000 FDSP: 8000sps, ADE9078 FDSP: 4000sps
> + */
> +#define ADE9078_FDSP			4000
> +#define ADE9078_WFB_CFG			0x0329
> +#define ADE9078_WFB_PAGE_SIZE		128
> +#define ADE9078_WFB_BYTES_IN_PAGE	4
> +#define ADE9078_WFB_PAGE_ARRAY_SIZE	\
> +	(ADE9078_WFB_PAGE_SIZE * ADE9078_WFB_BYTES_IN_PAGE)
> +#define ADE9078_WFB_FULL_BUFF_SIZE	\
> +	(ADE9078_WFB_PAGE_ARRAY_SIZE * 16)
> +#define ADE9078_WFB_FULL_BUFF_NR_SAMPLES \
> +	(ADE9078_WFB_PAGE_SIZE * 16)
> +
> +#define ADE9078_SWRST_BIT		BIT(0)
> +
> +/* Status and Mask register bits*/
> +#define ADE9078_ST0_WFB_TRIG_BIT	BIT(16)
> +#define ADE9078_ST0_PAGE_FULL_BIT	BIT(17)
> +
> +#define ADE9078_ST1_ZXTOVA_BIT		BIT(6)
> +#define ADE9078_ST1_ZXTOVB_BIT		BIT(7)
> +#define ADE9078_ST1_ZXTOVC_BIT		BIT(8)
> +#define ADE9078_ST1_ZXVA_BIT		BIT(9)
> +#define ADE9078_ST1_ZXVB_BIT		BIT(10)
> +#define ADE9078_ST1_ZXVC_BIT		BIT(11)
> +#define ADE9078_ST1_ZXIA_BIT		BIT(13)
> +#define ADE9078_ST1_ZXIB_BIT		BIT(14)
> +#define ADE9078_ST1_ZXIC_BIT		BIT(15)
> +#define ADE9078_ST1_RSTDONE_BIT		BIT(16)
> +#define ADE9078_ST1_ERROR0_BIT		BIT(28)
> +#define ADE9078_ST1_ERROR1_BIT		BIT(29)
> +#define ADE9078_ST1_ERROR2_BIT		BIT(30)
> +#define ADE9078_ST1_ERROR3_BIT		BIT(31)
> +#define ADE9078_ST_ERROR \
> +	(ADE9078_ST1_ERROR0 | \
> +	 ADE9078_ST1_ERROR1 | \
> +	 ADE9078_ST1_ERROR2 | \
> +	 ADE9078_ST1_ERROR3)
> +#define ADE9078_ST1_CROSSING_FIRST	6
> +#define ADE9078_ST1_CROSSING_DEPTH	16
> +
> +#define ADE9078_WFB_TRG_ZXIA_BIT	BIT(3)
> +#define ADE9078_WFB_TRG_ZXIB_BIT	BIT(4)
> +#define ADE9078_WFB_TRG_ZXIC_BIT	BIT(5)
> +#define ADE9078_WFB_TRG_ZXVA_BIT	BIT(6)
> +#define ADE9078_WFB_TRG_ZXVB_BIT	BIT(7)
> +#define ADE9078_WFB_TRG_ZXVC_BIT	BIT(8)
> +
> +/* Stop when waveform buffer is full */
> +#define ADE9078_WFB_FULL_MODE		0x0
> +/* Continuous fill—stop only on enabled trigger events */
> +#define ADE9078_WFB_EN_TRIG_MODE	0x1
> +/* Continuous filling—center capture around enabled trigger events */
> +#define ADE9078_WFB_C_EN_TRIG_MODE	0x2
> +/* Continuous fill—save event address of enabled trigger events */
> +#define ADE9078_WFB_SAVE_EN_TRIG_MODE	0x3
> +
> +#define ADE9078_MODE_0_1_PAGE_BIT	BIT(15)
> +#define ADE9078_MODE_2_PAGE_BIT		BIT(7)
> +
> +/*
> + * Full scale Codes referred from Datasheet.Respective digital codes are
> + * produced when ADC inputs are at full scale. Do not Change.
> + */
> +#define ADE9078_RMS_FULL_SCALE_CODES	52866837
> +#define ADE9000_WATT_FULL_SCALE_CODES	20694066
> +#define ADE9078_PCF_FULL_SCALE_CODES	74770000
> +
> +/* Phase and channel definitions */
> +#define ADE9078_PHASE_A_NR		0
> +#define ADE9078_PHASE_B_NR		2
> +#define ADE9078_PHASE_C_NR		4
> +#define ADE9078_PHASE_A_NAME		"A"
> +#define ADE9078_PHASE_B_NAME		"B"
> +#define ADE9078_PHASE_C_NAME		"C"

These will be passed into a new macro as below (once each)
so probably will be better just used inline there.

> +
> +#define ADE9078_SCAN_POS_IA		BIT(0)
> +#define ADE9078_SCAN_POS_VA		BIT(1)
> +#define ADE9078_SCAN_POS_IB		BIT(2)
> +#define ADE9078_SCAN_POS_VB		BIT(3)
> +#define ADE9078_SCAN_POS_IC		BIT(4)
> +#define ADE9078_SCAN_POS_VC		BIT(5)
> +
> +#define ADE9078_PHASE_B_POS_BIT		BIT(5)
> +#define ADE9078_PHASE_C_POS_BIT		BIT(6)
> +
> +#define ADE9078_MAX_PHASE_NR		3
> +#define AD9078_CHANNELS_PER_PHASE	9
> +
> +#define ADE9078_ADDR_ADJUST(addr, chan)					\
> +	(((chan) << 4) | (addr))
> +
> +#define ADE9078_CURRENT_CHANNEL(num, name) {				\
> +	.type = IIO_CURRENT,						\
> +	.channel = num,							\
> +	.extend_name = name,						\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AI_PCF, num),	\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
> +	.event_spec = ade9078_events,					\
> +	.num_event_specs = ARRAY_SIZE(ade9078_events),			\
> +	.scan_index = num,						\
> +	.scan_type = {							\
> +		.sign = 's',						\
> +		.realbits = 32,						\
> +		.storagebits = 32,					\
> +		.shift = 0,						\
> +		.endianness = IIO_BE,					\
> +	},								\
> +}
> +
> +#define ADE9078_VOLTAGE_CHANNEL(num, name) {				\
> +	.type = IIO_VOLTAGE,						\
> +	.channel = num,							\
> +	.extend_name = name,						\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AV_PCF, num),	\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
> +	.event_spec = ade9078_events,					\
> +	.num_event_specs = ARRAY_SIZE(ade9078_events),			\
> +	.scan_index = num + 1,						\
> +	.scan_type = {							\
> +		.sign = 's',						\
> +		.realbits = 32,						\
> +		.storagebits = 32,					\
> +		.shift = 0,						\
> +		.endianness = IIO_BE,					\
> +	},								\
> +}
> +
> +#define ADE9078_CURRENT_RMS_CHANNEL(num, name) {			\
> +	.type = IIO_CURRENT,						\
> +	.channel = num,							\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AIRMS, num),		\
> +	.extend_name = name "_rms",					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_OFFSET),		\
> +	.scan_index = -1						\
> +}
> +
> +#define ADE9078_VOLTAGE_RMS_CHANNEL(num, name) {			\
> +	.type = IIO_VOLTAGE,						\
> +	.channel = num,							\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AVRMS, num),		\
> +	.extend_name = name "_rms",					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_OFFSET),		\
> +	.scan_index = -1						\
> +}
> +
> +#define ADE9078_POWER_ACTIV_CHANNEL(num, name) {			\
> +	.type = IIO_POWER,						\
> +	.channel = num,							\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AWATT, num),		\
> +	.extend_name = name "_activ",					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_OFFSET) |		\
> +			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
> +	.scan_index = -1						\
> +}
> +
> +#define ADE9078_POWER_REACTIV_CHANNEL(num, name) {			\
> +	.type = IIO_POWER,						\
> +	.channel = num,							\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AVAR, num),		\
> +	.extend_name = name "_reactiv",					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_OFFSET),		\
> +	.scan_index = -1						\
> +}
> +
> +#define ADE9078_POWER_APPARENT_CHANNEL(num, name) {			\
> +	.type = IIO_POWER,						\
> +	.channel = num,							\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AVA, num),		\
> +	.extend_name = name "_apparent",				\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE),			\
> +	.scan_index = -1						\
> +}
> +
> +#define ADE9078_POWER_FUND_REACTIV_CHANNEL(num, name) {			\
> +	.type = IIO_POWER,						\
> +	.channel = num,							\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AFVAR, num),		\
> +	.extend_name = name "_fund_reactiv",				\

extend_name defines new ABI.  Needs documentation in
Documentation/ABI/testing/sysfs-bus-iio-* 

and a very strong reason why it makes sense to do it this way rather than
via modifiers or similar.

> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_OFFSET),		\
> +	.scan_index = -1						\
> +}
> +
> +#define ADE9078_POWER_FACTOR_CHANNEL(num, name) {			\
> +	.type = IIO_POWER,						\
> +	.channel = num,							\
> +	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_APF, num),		\
> +	.extend_name = name "_factor",					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE),			\
> +	.scan_index = -1						\
> +}
reorder the code so these defs are near the use of them.
> +
> +/*
> + * struct ade9078_state - ade9078 specific data
> + * @rst_done	flag for when reset sequence irq has been received
> + * @wf_mode	wave form buffer mode, read datasheet for more details,
> + *		retrieved from DT
> + * @wfb_trg	wave form buffer triger configuration, read datasheet for more
> + *		details, retrieved from DT
> + * @spi		spi device associated to the ade9078
> + * @tx		transmit buffer for the spi
> + * @rx		receive buffer for the spi
> + * @xfer	transfer setup used in iio buffer configuration
> + * @spi_msg	message transfer trough spi, used in iio buffer
> + *		configuration
> + * @regmap	register map pointer
> + * @indio_dev:	the IIO device
> + * @trig	iio trigger pointer, is connected to IRQ0 and IRQ1
> + * @rx_buff	receive buffer for the iio buffer trough spi, will
> + *		contain the samples from the IC wave form buffer
> + * @tx_buff	transmit buffer for the iio buffer trough spi, used
> + *		in iio	buffer configuration
> + */
> +
> +struct ade9078_state {
> +	bool rst_done;
> +	u8 wf_mode;
> +	u32 wfb_trg;
> +	struct spi_device *spi;
> +	u8 *tx;
> +	u8 *rx;
> +	struct spi_transfer xfer[2];
> +	struct spi_message spi_msg;
> +	struct regmap *regmap;
> +	struct iio_dev *indio_dev;

This is always a bad sign.  If you need to go from the state back
to the iio_dev then you should have passed the iio_dev in the first
place - in this particular case the iio_dev should be your private
data for the irq handlers, not the ade9078 state structure.


> +	union{
> +		u8 byte[ADE9078_WFB_FULL_BUFF_SIZE];
> +		__be32 word[ADE9078_WFB_FULL_BUFF_NR_SAMPLES];
> +	} rx_buff ____cacheline_aligned;
> +	u8 tx_buff[2];
> +};
> +
> +static const struct iio_event_spec ade9078_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
> +				 BIT(IIO_EV_INFO_VALUE),
> +	},
> +};
> +
> +/* IIO channels of the ade9078 for each phase individually */
> +static const struct iio_chan_spec ade9078_a_channels[] = {
> +	ADE9078_CURRENT_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
> +	ADE9078_VOLTAGE_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
> +	ADE9078_CURRENT_RMS_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
> +	ADE9078_VOLTAGE_RMS_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
> +	ADE9078_POWER_ACTIV_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
> +	ADE9078_POWER_REACTIV_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
> +	ADE9078_POWER_APPARENT_CHANNEL(ADE9078_PHASE_A_NR,
> +				       ADE9078_PHASE_A_NAME),
> +	ADE9078_POWER_FUND_REACTIV_CHANNEL(ADE9078_PHASE_A_NR,
> +					   ADE9078_PHASE_A_NAME),
> +	ADE9078_POWER_FACTOR_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),

These definitely look like a case for a macro defining all the channels for a phase.

> +};
> +
> +static const struct iio_chan_spec ade9078_b_channels[] = {
> +	ADE9078_CURRENT_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
> +	ADE9078_VOLTAGE_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
> +	ADE9078_CURRENT_RMS_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
> +	ADE9078_VOLTAGE_RMS_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
> +	ADE9078_POWER_ACTIV_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
> +	ADE9078_POWER_REACTIV_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
> +	ADE9078_POWER_APPARENT_CHANNEL(ADE9078_PHASE_B_NR,
> +				       ADE9078_PHASE_B_NAME),
> +	ADE9078_POWER_FUND_REACTIV_CHANNEL(ADE9078_PHASE_B_NR,
> +					   ADE9078_PHASE_B_NAME),
> +	ADE9078_POWER_FACTOR_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
> +};
> +
> +static const struct iio_chan_spec ade9078_c_channels[] = {
> +	ADE9078_CURRENT_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
> +	ADE9078_VOLTAGE_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
> +	ADE9078_CURRENT_RMS_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
> +	ADE9078_VOLTAGE_RMS_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
> +	ADE9078_POWER_ACTIV_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
> +	ADE9078_POWER_REACTIV_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
> +	ADE9078_POWER_APPARENT_CHANNEL(ADE9078_PHASE_C_NR,
> +				       ADE9078_PHASE_C_NAME),
> +	ADE9078_POWER_FUND_REACTIV_CHANNEL(ADE9078_PHASE_C_NR,
> +					   ADE9078_PHASE_C_NAME),
> +	ADE9078_POWER_FACTOR_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
> +};
> +
> +static const struct reg_sequence ade9078_reg_sequence[] = {
> +	{ ADE9078_REG_PGA_GAIN, ADE9078_PGA_GAIN },
> +	{ ADE9078_REG_CONFIG0, ADE9078_CONFIG0 },
> +	{ ADE9078_REG_CONFIG1, ADE9078_CONFIG1 },
> +	{ ADE9078_REG_CONFIG2, ADE9078_CONFIG2 },
> +	{ ADE9078_REG_CONFIG3, ADE9078_CONFIG3 },
> +	{ ADE9078_REG_ACCMODE, ADE9078_ACCMODE },
> +	{ ADE9078_REG_ZX_LP_SEL, ADE9078_ZX_LP_SEL },
> +	{ ADE9078_REG_MASK0, ADE9078_MASK0 },
> +	{ ADE9078_REG_MASK1, ADE9078_MASK1 },
> +	{ ADE9078_REG_EVENT_MASK, ADE9078_EVENT_MASK },
> +	{ ADE9078_REG_WFB_CFG, ADE9078_WFB_CFG },
> +	{ ADE9078_REG_VLEVEL, ADE9078_VLEVEL },
> +	{ ADE9078_REG_DICOEFF, ADE9078_DICOEFF },
> +	{ ADE9078_REG_EGY_TIME, ADE9078_EGY_TIME },
> +	{ ADE9078_REG_EP_CFG, ADE9078_EP_CFG },
> +	{ ADE9078_REG_RUN, ADE9078_RUN_ON }
> +};
> +
> +/*

As mentioned below, I don't immediately understand why this can't
be done with appropriate standard regmap.  Perhaps you could give
more details of what is missing.  I'd like to see that added to
regmap if possible.

> + * ade9078_spi_write_reg() - ade9078 write register over SPI
> + * the data format for communicating with the ade9078 over SPI
> + * is very specific and can access both 32bit and 16bit registers
> + * @context:	void pointer to the SPI device
> + * @reg:	address of the of desired register
> + * @val:	value to be written to the ade9078
> + */
> +static int ade9078_spi_write_reg(void *context,
> +				 unsigned int reg,
> +				 unsigned int val)
> +{
> +	struct device *dev = context;
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct ade9078_state *st = spi_get_drvdata(spi);

> +
> +	u16 addr;
> +	int ret = 0;
> +	struct spi_transfer xfer[] = {
> +		{
> +			.tx_buf = st->tx,
> +		},
> +	};
> +
> +	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg);
> +
> +	put_unaligned_be16(addr, st->tx);
> +	put_unaligned_be32(val, &st->tx[2]);
> +
> +	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION) {
> +		put_unaligned_be16(val, &st->tx[2]);
> +		xfer[0].len = 4;
> +	} else {
> +		xfer[0].len = 6;
> +	}
> +
> +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> +	if (ret) {
> +		dev_err(&st->spi->dev, "problem when writing register 0x%x",
> +			reg);
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * ade9078_spi_write_reg() - ade9078 read register over SPI
> + * the data format for communicating with the ade9078 over SPI
> + * is very specific and can access both 32bit and 16bit registers
> + * @context:	void pointer to the SPI device
> + * @reg:	address of the of desired register
> + * @val:	value to be read to the ade9078
> + */
> +static int ade9078_spi_read_reg(void *context,
> +				unsigned int reg,
> +				unsigned int *val)
> +{
> +	struct device *dev = context;
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct ade9078_state *st = spi_get_drvdata(spi);
> +
> +	u16 addr;
> +	int ret = 0;
> +	struct spi_transfer xfer[] = {
> +		{
> +			.tx_buf = st->tx,
> +			.len = 2,
> +		},
> +		{
> +			.rx_buf = st->rx,
> +		},
> +	};
> +
> +	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg) |
> +	       ADE9078_REG_READ_BIT_MASK;
> +
> +	put_unaligned_be16(addr, st->tx);
> +
> +	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION)
> +		xfer[1].len = 4;
> +	else
> +		xfer[1].len = 6;

This doesn't look like a fixed length register which is expected
for regmap...  Also that len should just be the rx register which
you treat below as 16 bit and 32 bit (so 2 and 4, not 4 and 6).

Can the larger registers be treated as bulk reads of pairs of smaller ones?

> +
> +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> +	if (ret) {
> +		dev_err(&st->spi->dev, "problem when reading register 0x%x",
> +			reg);
> +		goto err_ret;
> +	}
> +
> +	//registers which are 16 bits
> +	if (reg > 0x480 && reg < 0x4FE)
> +		*val = get_unaligned_be16(st->rx);
> +	else
> +		*val = get_unaligned_be32(st->rx);
> +
> +err_ret:
> +	return ret;
> +}
> +
> +/*
> + * ade9078_is_volatile_reg() - list of ade9078 registers which should use
> + * caching
> + * @dev:	device data
> + * @reg:	address of the of desired register
> + */
> +static bool ade9078_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case ADE9078_REG_STATUS0:
> +	case ADE9078_REG_STATUS1:
> +	case ADE9078_REG_MASK0:
> +	case ADE9078_REG_MASK1:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +/*
> + * ade9078_en_wfb() - enables or disables the WFBuffer in the ADE9078
> + * @st:		ade9078 device data
> + * @state:	true for enabled; false for disabled
> + */
> +static int ade9078_en_wfb(struct ade9078_state *st, bool state)
> +{
> +	return regmap_update_bits(st->regmap, ADE9078_REG_WFB_CFG, BIT(4),
> +				  state ? BIT(4) : 0);
> +}
> +
> +/*
> + * ade9078_iio_push_buffer() - reads out the content of the waveform buffer and
> + * pushes it to the IIO buffer.
> + * @st:		ade9078 device data
> + */
> +static int ade9078_iio_push_buffer(struct ade9078_state *st)
> +{
> +	int ret;
> +	u32 i;
> +
> +	ret = spi_sync(st->spi, &st->spi_msg);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "SPI fail in trigger handler");
> +		return ret;
> +	}
> +
> +	for (i = 0; i < ADE9078_WFB_FULL_BUFF_NR_SAMPLES; i++)
> +		iio_push_to_buffers(st->indio_dev, &st->rx_buff.word[i]);
> +
> +	return 0;
> +}
> +
> +/*
> + * ade9078_irq0_thread() - Thread for IRQ0. It reads Status register 0 and
> + * checks for the IRQ activation. This is configured to acquire samples in to
> + * the IC buffer and dump it in to the iio_buffer according to Stop When Buffer
> + * Is Full Mode, Stop Filling on Trigger and Capture Around Trigger from the
> + * ADE9078 Datasheet
> + */
> +static irqreturn_t ade9078_irq0_thread(int irq, void *data)
> +{
> +	struct ade9078_state *st = data;
> +	u32 handled_irq = 0;
> +	u32 interrupts;
> +	u32 status;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, ADE9078_REG_STATUS0, &status);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "IRQ0 read status fail");
> +		goto irq0_done;
> +	}
> +
> +	ret = regmap_read(st->regmap, ADE9078_REG_MASK0, &interrupts);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "IRQ0 read status fail");
> +		goto irq0_done;
> +	}
> +
> +	if ((status & ADE9078_ST0_PAGE_FULL_BIT) &&
> +	    (interrupts & ADE9078_ST0_PAGE_FULL_BIT)) {
> +		//Stop Filling on Trigger and Center Capture Around Trigger
> +		if (st->wf_mode) {
> +			ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG,
> +					   st->wfb_trg);
> +			if (ret) {
> +				dev_err(&st->spi->dev, "IRQ0 WFB write fail");
> +				goto irq0_done;
> +			}
> +
> +			interrupts |= ADE9078_ST0_WFB_TRIG_BIT;
> +
> +		} else {
> +			//Stop When Buffer Is Full Mode
> +			ret = ade9078_en_wfb(st, false);
> +			if (ret) {
> +				dev_err(&st->spi->dev, "IRQ0 WFB stop fail");
> +				goto irq0_done;
> +			}
> +			ret = ade9078_iio_push_buffer(st);
> +			if (ret) {
> +				dev_err(&st->spi->dev, "IRQ0 IIO push fail");
> +				goto irq0_done;
> +			}
> +		}
> +
> +		//disable Page full interrupt
> +		interrupts &= ~ADE9078_ST0_PAGE_FULL_BIT;
> +
> +		ret = regmap_write(st->regmap, ADE9078_REG_MASK0, interrupts);
> +		if (ret) {
> +			dev_err(&st->spi->dev, "IRQ0 MAKS0 write fail");
> +			goto irq0_done;
> +		}
> +
> +		handled_irq |= ADE9078_ST0_PAGE_FULL_BIT;
> +	}
> +
> +	if ((status & ADE9078_ST0_WFB_TRIG_BIT) &&
> +	    (interrupts & ADE9078_ST0_WFB_TRIG_BIT)) {
> +		//Stop Filling on Trigger and Center Capture Around Trigger
> +		ret = ade9078_en_wfb(st, false);
> +		if (ret) {
> +			dev_err(&st->spi->dev, "IRQ0 WFB fail");
> +			goto irq0_done;
> +		}
> +
> +		ret = ade9078_iio_push_buffer(st);
> +		if (ret) {
> +			dev_err(&st->spi->dev, "IRQ0 IIO push fail @ WFB TRIG");
> +			goto irq0_done;
> +		}
> +
> +		handled_irq |= ADE9078_ST0_WFB_TRIG_BIT;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, handled_irq);
> +	if (ret)
> +		dev_err(&st->spi->dev, "IRQ0 write status fail");
> +
> +irq0_done:

As below. return wherever you have a goto.  This common exit just makes
the code less readable.

> +	return IRQ_HANDLED;
> +}
> +
> +/*
> + * ade9078_irq1_thread() - Thread for IRQ1. It reads Status register 1 and
> + * checks for the IRQ activation. This thread handles the reset condition and
> + * the zero-crossing conditions for all 3 phases on Voltage and Current
> + */
> +static irqreturn_t ade9078_irq1_thread(int irq, void *data)
> +{
> +	struct ade9078_state *st = data;
> +	struct iio_dev *indio_dev = st->indio_dev;
> +	struct iio_chan_spec const *chan = indio_dev->channels;
> +	unsigned int bit = ADE9078_ST1_CROSSING_FIRST;
> +	s64 timestamp = iio_get_time_ns(indio_dev);
> +	u32 interrupts;
> +	u32 result;
> +	u32 status;
> +	u32 tmp;
> +	int ret;
> +
> +	//reset
/* reset */ etc
// isn't used for comments in IIO (or most of the rest of the kernel)

> +	if (!st->rst_done) {
> +		ret = regmap_read(st->regmap, ADE9078_REG_STATUS1, &result);
> +		if (ret)
> +			return ret;
> +
> +		if (result & ADE9078_ST1_RSTDONE_BIT)
> +			st->rst_done = true;
> +		else
> +			dev_err(&st->spi->dev, "Error testing reset done");
> +
> +		goto irq1_done;
> +	}
> +
> +	ret = regmap_read(st->regmap, ADE9078_REG_STATUS1, &status);
> +	if (ret)
> +		goto irq1_done;
> +
> +	ret = regmap_read(st->regmap, ADE9078_REG_MASK1, &interrupts);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "IRQ1 read status fail");
> +		goto irq1_done;

return IRQ_HANDLED; in all these paths.  No point in going to a common
exit that doesn't do anything.

> +	}
> +
> +	//crossings
> +	for_each_set_bit_from(bit, (unsigned long *)&interrupts,
> +			      ADE9078_ST1_CROSSING_DEPTH) {
> +		tmp = status & BIT(bit);
> +
> +		switch (tmp) {
> +		case ADE9078_ST1_ZXVA_BIT:
> +		case ADE9078_ST1_ZXTOVA_BIT:
> +		case ADE9078_ST1_ZXIA_BIT:
> +			iio_push_event(indio_dev,
> +				       IIO_UNMOD_EVENT_CODE(chan->type,
> +							    ADE9078_PHASE_A_NR,
> +							    IIO_EV_TYPE_THRESH,
> +							    IIO_EV_DIR_EITHER),
> +				       timestamp);
> +			break;
> +		case ADE9078_ST1_ZXVB_BIT:
> +		case ADE9078_ST1_ZXTOVB_BIT:
> +		case ADE9078_ST1_ZXIB_BIT:
> +			iio_push_event(indio_dev,
> +				       IIO_UNMOD_EVENT_CODE(chan->type,
> +							    ADE9078_PHASE_B_NR,
> +							    IIO_EV_TYPE_THRESH,
> +							    IIO_EV_DIR_EITHER),
> +				       timestamp);
> +			break;
> +		case ADE9078_ST1_ZXVC_BIT:
> +		case ADE9078_ST1_ZXTOVC_BIT:
> +		case ADE9078_ST1_ZXIC_BIT:
> +			iio_push_event(indio_dev,
> +				       IIO_UNMOD_EVENT_CODE(chan->type,
> +							    ADE9078_PHASE_C_NR,
> +							    IIO_EV_TYPE_THRESH,
> +							    IIO_EV_DIR_EITHER),
> +				       timestamp);
> +			break;
> +		default:
> +			goto irq1_done;
> +		}
> +	}
> +
> +irq1_done:
> +	return IRQ_HANDLED;
> +}
> +
> +/*
> + * ade9078_configure_scan() - sets up the transfer parameters
> + * as well as the tx and rx buffers
> + * @indio_dev:	the IIO device
> + */
> +static int ade9078_configure_scan(struct iio_dev *indio_dev)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	u16 addr;
> +
> +	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, ADE9078_REG_WF_BUFF) |
> +	       ADE9078_REG_READ_BIT_MASK;
> +
> +	put_unaligned_be16(addr, st->tx_buff);
> +
> +	st->xfer[0].tx_buf = &st->tx_buff[0];
> +	st->xfer[0].len = 2;
> +
> +	st->xfer[1].rx_buf = &st->rx_buff.byte[0];
> +	st->xfer[1].len = ADE9078_WFB_FULL_BUFF_SIZE;
> +
> +	spi_message_init_with_transfers(&st->spi_msg, st->xfer, 2);
> +	return 0;
> +}
> +
> +/*
> + * ade9078_read_raw() - IIO read function
> + * @indio_dev:	the IIO device
> + * @chan:	channel specs of the ade9078
> + * @val:	first half of the read value
> + * @val2:	second half of the read value
> + * @mask:	info mask of the channel
> + */
> +static int ade9078_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
> +			    long mask)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	int measured;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_read(st->regmap, chan->address, &measured);

> +
> +		iio_device_release_direct_mode(indio_dev);
> +		*val = measured;
Convention is to only set values on no error.
so

		if (ret)
			return ret;

		*val = measured;

		return IIO_VAL_INT;


> +
> +		return ret ?: IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_CURRENT || chan->type == IIO_VOLTAGE) {
> +			switch (chan->address) {
> +			case ADE9078_REG_AI_PCF:
> +			case ADE9078_REG_AV_PCF:
> +			case ADE9078_REG_BI_PCF:
> +			case ADE9078_REG_BV_PCF:
> +			case ADE9078_REG_CI_PCF:
> +			case ADE9078_REG_CV_PCF:
> +				*val = 1;
> +				*val2 = ADE9078_PCF_FULL_SCALE_CODES;
> +				return IIO_VAL_FRACTIONAL;
> +			case ADE9078_REG_AIRMS:
> +			case ADE9078_REG_AVRMS:
> +			case ADE9078_REG_BIRMS:
> +			case ADE9078_REG_BVRMS:
> +			case ADE9078_REG_CIRMS:
> +			case ADE9078_REG_CVRMS:
> +				*val = 1;
> +				*val2 = ADE9078_RMS_FULL_SCALE_CODES;
> +				return IIO_VAL_FRACTIONAL;
> +			default:
> +				return -EINVAL;
> +			}
> +		} else if (chan->type == IIO_POWER) {
> +			*val = 1;
> +			*val2 = ADE9000_WATT_FULL_SCALE_CODES;
> +			return IIO_VAL_FRACTIONAL;
> +		} else {
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
I don't think you can reach this.

> +}
> +
> +/*
> + * ade9078_write_raw() - IIO write function
> + * @indio_dev:	the IIO device
> + * @chan:	channel specs of the ade9078
> + * @val:	first half of the read value
> + * @val2:	second half of the read value
> + * @mask:	info mask of the channel
> + */
> +static int ade9078_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val,
> +			     int val2,
> +			     long mask)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	u32 addr = 0xFFFFF;

If that value is used, something went wrong.  I don't think it is, so don't
assign it.

> +	u32 tmp;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_OFFSET:
> +		switch (chan->type) {
> +		case IIO_CURRENT:
> +			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AIRMSOS,
> +						   chan->channel);
> +			break;
> +		case IIO_VOLTAGE:
> +			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AVRMSOS,
> +						   chan->channel);
> +			break;
> +		case IIO_POWER:
> +			tmp = chan->address;
> +			tmp &= ~ADE9078_PHASE_B_POS_BIT;
> +			tmp &= ~ADE9078_PHASE_C_POS_BIT;
> +
> +			switch (tmp) {
> +			case ADE9078_REG_AWATTOS:
> +				addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AWATTOS,
> +							   chan->channel);
> +				break;
> +			case ADE9078_REG_AVAR:
> +				addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AVAROS,
> +							   chan->channel);
> +				break;
> +			case ADE9078_REG_AFVAR:
> +				addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AFVAROS,
> +							   chan->channel);
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		switch (chan->type) {
> +		case IIO_CURRENT:
> +			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AIGAIN,
> +						   chan->channel);
> +			break;
> +		case IIO_VOLTAGE:
> +			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AVGAIN,
> +						   chan->channel);
> +			break;
> +		case IIO_POWER:
> +			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_APGAIN,
> +						   chan->channel);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_write(st->regmap, addr, val);
> +	return ret;

return regmap_write()

> +}
> +
> +/*
> + * ade9078_reg_access() - IIO debug register access
> + * @indio_dev:	the IIO device
> + * @reg:	register to be accessed
> + * @tx_val:	value to be transmitted
> + * @rx_val:	value to be received
> + */
> +static int ade9078_reg_access(struct iio_dev *indio_dev,
> +			      unsigned int reg,
> +			      unsigned int tx_val,
> +			      unsigned int *rx_val)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +
> +	if (rx_val)
> +		return regmap_read(st->regmap, reg, rx_val);
> +
> +	return regmap_write(st->regmap, reg, tx_val);
> +}
> +
> +/*
> + * ade9078_write_event_config() - IIO event configure to enable zero-crossing

All these function description comments should be valid kernel-doc.

> + * and zero-crossing timeout on voltage and current for each phases. These
> + * events will also influence the trigger conditions for the buffer capture.
> + */
> +static int ade9078_write_event_config(struct iio_dev *indio_dev,
> +				      const struct iio_chan_spec *chan,
> +				      enum iio_event_type type,
> +				      enum iio_event_direction dir,
> +				      int state)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	u32 interrupts;
> +	u32 number;
> +	int ret;
> +
> +	number = chan->channel;
> +
> +	switch (number) {
> +	case ADE9078_PHASE_A_NR:

I would use a lookup on the phase into an array of structures.
The structure would then have fields for which bit to set etc
for a voltage channel and for a current channel.

That way this all becomes one bit of code and some const data
rather that 3 sets of near identical code. If you can make
this sort of thing data rather than code that is almost always
the best choice.

> +		if (chan->type == IIO_VOLTAGE) {
> +			if (state) {
> +				interrupts |= ADE9078_ST1_ZXVA_BIT;
> +				interrupts |= ADE9078_ST1_ZXTOVA_BIT;
> +				st->wfb_trg |= ADE9078_WFB_TRG_ZXVA_BIT;
> +			} else {
> +				interrupts &= ~ADE9078_ST1_ZXVA_BIT;
> +				interrupts &= ~ADE9078_ST1_ZXTOVA_BIT;
> +				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXVA_BIT;
> +			}
> +		} else if (chan->type == IIO_CURRENT) {
> +			if (state) {
> +				interrupts |= ADE9078_ST1_ZXIA_BIT;
> +				st->wfb_trg |= ADE9078_WFB_TRG_ZXIA_BIT;
> +			} else {
> +				interrupts &= ~ADE9078_ST1_ZXIA_BIT;
> +				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXIA_BIT;
> +			}
> +		}
> +		break;
> +	case ADE9078_PHASE_B_NR:
> +		if (chan->type == IIO_VOLTAGE) {
> +			if (state) {
> +				interrupts |= ADE9078_ST1_ZXVB_BIT;
> +				interrupts |= ADE9078_ST1_ZXTOVB_BIT;
> +				st->wfb_trg |= ADE9078_WFB_TRG_ZXVB_BIT;
> +			} else {
> +				interrupts &= ~ADE9078_ST1_ZXVB_BIT;
> +				interrupts &= ~ADE9078_ST1_ZXTOVB_BIT;
> +				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXVB_BIT;
> +			}
> +		} else if (chan->type == IIO_CURRENT) {
> +			if (state) {
> +				interrupts |= ADE9078_ST1_ZXIB_BIT;
> +				st->wfb_trg |= ADE9078_WFB_TRG_ZXIB_BIT;
> +			} else {
> +				interrupts &= ~ADE9078_ST1_ZXIB_BIT;
> +				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXIB_BIT;
> +			}
> +		}
> +		break;
> +	case ADE9078_PHASE_C_NR:
> +		if (chan->type == IIO_VOLTAGE) {
> +			if (state) {
> +				interrupts |= ADE9078_ST1_ZXVC_BIT;
> +				interrupts |= ADE9078_ST1_ZXTOVC_BIT;
> +				st->wfb_trg |= ADE9078_WFB_TRG_ZXVC_BIT;
> +			} else {
> +				interrupts &= ~ADE9078_ST1_ZXVC_BIT;
> +				interrupts &= ~ADE9078_ST1_ZXTOVC_BIT;
> +				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXVC_BIT;
> +			}
> +		} else if (chan->type == IIO_CURRENT) {
> +			if (state) {
> +				interrupts |= ADE9078_ST1_ZXIC_BIT;
> +				st->wfb_trg |= ADE9078_WFB_TRG_ZXIC_BIT;
> +			} else {
> +				interrupts &= ~ADE9078_ST1_ZXIC_BIT;
> +				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXIC_BIT;
> +			}
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1, GENMASK(31, 0));
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK1, interrupts,
> +				  interrupts);
> +}
> +
> +/*
> + * ade9078_read_event_vlaue() - Outputs the result of the zero-crossing for
> + * voltage and current for each phase.
> + * Result:
> + * 0 - if crossing event not set
> + * 1 - if crossing event occurred
> + * -1 - if crossing timeout (only for Voltages)
> + */
> +static int ade9078_read_event_vlaue(struct iio_dev *indio_dev,

value?  If so, this should be reading the thresholds, not anything
about current status of events. 


> +				    const struct iio_chan_spec *chan,
> +				    enum iio_event_type type,
> +				    enum iio_event_direction dir,
> +				    enum iio_event_info info,
> +				    int *val, int *val2)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	u32 handeled_irq = 0;

handled

> +	u32 interrupts;
> +	u32 number;
> +	u32 status;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, ADE9078_REG_STATUS1, &status);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, ADE9078_REG_MASK1, &interrupts);
> +	if (ret)
> +		return ret;
> +
> +	*val = 0;
> +	number = chan->channel;
> +	switch (number) {
> +	case ADE9078_PHASE_A_NR:
> +		if (chan->type == IIO_VOLTAGE) {
> +			if (status & ADE9078_ST1_ZXVA_BIT) {
> +				*val = 1;
> +				handeled_irq |= ADE9078_ST1_ZXVA_BIT;
> +			} else if (status & ADE9078_ST1_ZXTOVA_BIT) {
> +				*val = -1;
> +				handeled_irq |= ADE9078_ST1_ZXTOVA_BIT;
> +			}
> +			if (!(interrupts & ADE9078_ST1_ZXTOVA_BIT))
> +				*val = 0;
> +		} else if (chan->type == IIO_CURRENT) {
> +			if (status & ADE9078_ST1_ZXIA_BIT) {
> +				*val = 1;
> +				handeled_irq |= ADE9078_ST1_ZXIA_BIT;
> +			}
> +		}
> +		break;
> +	case ADE9078_PHASE_B_NR:
> +		if (chan->type == IIO_VOLTAGE) {
> +			if (status & ADE9078_ST1_ZXVB_BIT) {
> +				*val = 1;
> +				handeled_irq |= ADE9078_ST1_ZXVB_BIT;
> +			} else if (status & ADE9078_ST1_ZXTOVB_BIT) {
> +				*val = -1;
> +				handeled_irq |= ADE9078_ST1_ZXTOVB_BIT;
> +			}
> +			if (!(interrupts & ADE9078_ST1_ZXTOVB_BIT))
> +				*val = 0;
> +		} else if (chan->type == IIO_CURRENT) {
> +			if (status & ADE9078_ST1_ZXIB_BIT) {
> +				*val = 1;
> +				handeled_irq |= ADE9078_ST1_ZXIB_BIT;
> +			}
> +		}
> +		break;
> +	case ADE9078_PHASE_C_NR:
> +		if (chan->type == IIO_VOLTAGE) {
> +			if (status & ADE9078_ST1_ZXVC_BIT) {
> +				*val = 1;
> +				handeled_irq |= ADE9078_ST1_ZXVC_BIT;
> +			} else if (status & ADE9078_ST1_ZXTOVC_BIT) {
> +				*val = -1;
> +				handeled_irq |= ADE9078_ST1_ZXTOVC_BIT;
> +			}
> +			if (!(interrupts & ADE9078_ST1_ZXTOVC_BIT))
> +				*val = 0;
> +		} else if (chan->type == IIO_CURRENT) {
> +			if (status & ADE9078_ST1_ZXIC_BIT) {
> +				*val = 1;
> +				handeled_irq |= ADE9078_ST1_ZXIC_BIT;
> +			}
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1, handeled_irq);
Are there other bits, or is this always going to write all bits that
might be set? If it is all such bits then 
> +	if (ret)
> +		return ret;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +/*
> + * ade9078_config_wfb() - reads the ade9078 node and configures the wave form
> + * buffer based on the options set. Additionally is reads the active scan mask
> + * in order to set the input data of the buffer. There are only a few available
> + * input configurations permitted by the IC, any unpermitted configuration will
> + * result in all channels being active.
> + * @indio_dev:	the IIO device

Make this valid kernel-doc.  Then check for any warnings by running the
kernel-doc build scripts.


> + */
> +static int ade9078_config_wfb(struct iio_dev *indio_dev)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	u32 wfg_cfg_val = 0;
> +	u32 tmp;
> +	int ret;
> +
> +	bitmap_to_arr32(&wfg_cfg_val, indio_dev->active_scan_mask,
> +			indio_dev->masklength);
> +
> +	switch (wfg_cfg_val) {

Use another variable so that this becomes more readable.

> +	case ADE9078_SCAN_POS_IA | ADE9078_SCAN_POS_VA:
> +		wfg_cfg_val = 0x1;
> +		break;
> +	case ADE9078_SCAN_POS_IB | ADE9078_SCAN_POS_VB:
> +		wfg_cfg_val = 0x2;
> +		break;
> +	case ADE9078_SCAN_POS_IC | ADE9078_SCAN_POS_VC:
> +		wfg_cfg_val = 0x3;
> +		break;
> +	case ADE9078_SCAN_POS_IA:
> +		wfg_cfg_val = 0x8;
> +		break;
> +	case ADE9078_SCAN_POS_VA:
> +		wfg_cfg_val = 0x9;
> +		break;
> +	case ADE9078_SCAN_POS_IB:
> +		wfg_cfg_val = 0xA;
> +		break;
> +	case ADE9078_SCAN_POS_VB:
> +		wfg_cfg_val = 0xB;
> +		break;
> +	case ADE9078_SCAN_POS_IC:
> +		wfg_cfg_val = 0xC;
> +		break;
> +	case ADE9078_SCAN_POS_VC:
> +		wfg_cfg_val = 0xD;
> +		break;
> +	default:
> +		wfg_cfg_val = 0x0;
> +		break;
> +	}
> +
> +	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-cap-sel",
> +				   &tmp);

Generic device properties form include/linux/properties.h only for new IIO drivers
unless there is a very strong reason why it won't work.


> +	if (ret) {
> +		dev_err(&st->spi->dev, "Failed to get wf-cap-sel: %d\n", ret);
> +		return ret;
> +	}
> +	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_CAP_SEL_MASK, tmp);
> +
> +	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-mode",
> +				   &tmp);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Failed to get wf-mode: %d\n", ret);
> +		return ret;
> +	}
> +	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_MODE_MASK, tmp);
> +	st->wf_mode = tmp;
> +
> +	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-src",
> +				   &tmp);
> +	if (ret) {
> +		dev_err(&st->spi->dev,
> +			"Failed to get wf-src: %d\n",
> +			ret);
> +		return ret;
> +	}
> +	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_SRC_MASK, tmp);
> +
> +	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-in-en",
> +				   &tmp);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Failed to get wf-in-en: %d\n", ret);
> +		return ret;
> +	}
> +	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_IN_EN_MASK, tmp);
> +
> +	return regmap_write(st->regmap, ADE9078_REG_WFB_CFG, wfg_cfg_val);
> +}
> +
> +/*
> + * ade9078_wfb_interrupt_setup() - Configures the wave form buffer interrupt
> + * according to modes
> + * @st:		ade9078 device data
> + * @mode:	modes according to datasheet; values [0-2]
> + *
> + * This sets the interrupt register and other registers related to the
> + * interrupts according to mode [0-2] from the datasheet
> + */
> +static int ade9078_wfb_interrupt_setup(struct ade9078_state *st, u8 mode)
> +{
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	if (mode == ADE9078_WFB_FULL_MODE || mode == ADE9078_WFB_EN_TRIG_MODE) {

A switch statement would be cleaner here.

> +		ret = regmap_write(st->regmap, ADE9078_REG_WFB_PG_IRQEN,
> +				   ADE9078_MODE_0_1_PAGE_BIT);
> +		if (ret)
> +			return ret;
> +	} else if (mode == ADE9078_WFB_C_EN_TRIG_MODE) {
> +		ret = regmap_write(st->regmap, ADE9078_REG_WFB_PG_IRQEN,
> +				   ADE9078_MODE_2_PAGE_BIT);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, GENMASK(31, 0));
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0,
> +				  ADE9078_ST0_PAGE_FULL_BIT,
> +				  ADE9078_ST0_PAGE_FULL_BIT);
> +	if (ret)

Unreachable code...

> +		return ret;
> +
> +	return 0;
> +}
> +
> +/*
> + * ade9078_buffer_preenable() - configures the waveform buffer, sets the
> + * interrupts and enables the buffer
> + * @indio_dev:	the IIO device
> + */
> +static int ade9078_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ade9078_config_wfb(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ade9078_wfb_interrupt_setup(st, st->wf_mode);
> +	if (ret)
> +		return ret;
> +
> +	ret = ade9078_en_wfb(st, true);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Post-enable wfb enable fail");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * ade9078_buffer_postdisable() - after the iio is disable
> + * this will disable the ade9078 internal buffer for acquisition
> + * @indio_dev:	the IIO device
> + */
> +static int ade9078_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct ade9078_state *st = iio_priv(indio_dev);
> +	u32 interrupts = 0;
> +	int ret;
> +
> +	ret = ade9078_en_wfb(st, false);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Post-disable wfb disable fail");
> +		return ret;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	interrupts |= ADE9078_ST0_WFB_TRIG_BIT;
> +	interrupts |= ADE9078_ST0_PAGE_FULL_BIT;
> +
> +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0, interrupts, 0);
> +	if (ret) {
> +		dev_err(&st->spi->dev, "Post-disable update maks0 fail");
> +		return ret;
> +	}
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, GENMASK(31, 0));
> +	if (ret)
> +		return ret;
> +
> +	return 0;

return regmap_write()...

> +}
> +
> +/*
> + * ade9078_setup_iio_channels() - parses the phase nodes of the device-tree and
> + * creates the iio channels based on the active phases in the DT.
> + * @st:		ade9078 device data
> + */
> +static int ade9078_setup_iio_channels(struct ade9078_state *st)
> +{
> +	struct fwnode_handle *phase_node = NULL;
> +	struct device *dev = &st->spi->dev;
> +	struct iio_chan_spec *chan;
> +	u32 phase_nr;
> +	int ret;
> +
> +	chan = devm_kcalloc(dev,
> +			    ADE9078_MAX_PHASE_NR *
> +			    ARRAY_SIZE(ade9078_a_channels),
> +			    sizeof(*ade9078_a_channels), GFP_KERNEL);
> +	if (!chan) {
> +		dev_err(dev, "Unable to allocate ADE9078 channels");
> +		return -ENOMEM;
> +	}
> +	st->indio_dev->num_channels = 0;
> +	st->indio_dev->channels = chan;
> +
> +	fwnode_for_each_available_child_node(dev_fwnode(dev), phase_node) {
> +		ret = fwnode_property_read_u32(phase_node, "reg", &phase_nr);
> +		if (ret) {
> +			dev_err(dev, "Could not read channel reg : %d\n", ret);
> +			return ret;
> +		}
> +
> +		switch (phase_nr) {
> +		case ADE9078_PHASE_A_NR:
> +			memcpy(chan, ade9078_a_channels,
> +			       sizeof(ade9078_a_channels));
> +			break;
> +		case ADE9078_PHASE_B_NR:
> +			memcpy(chan, ade9078_b_channels,
> +			       sizeof(ade9078_b_channels));
> +			break;
> +		case ADE9078_PHASE_C_NR:
> +			memcpy(chan, ade9078_c_channels,
> +			       sizeof(ade9078_c_channels));
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		chan += AD9078_CHANNELS_PER_PHASE;
> +		st->indio_dev->num_channels += AD9078_CHANNELS_PER_PHASE;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * ade9078_reset() - Reset sequence for the ADE9078, the hardware reset is
> + * optional in the DT. When no hardware reset has been declared a software
> + * reset is executed
> + * @st:		ade9078 device data
> + */
> +static int ade9078_reset(struct ade9078_state *st)
> +{
> +	struct gpio_desc *gpio_reset;
> +	int ret;
> +
> +	st->rst_done = false;
> +
> +	gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> +					     GPIOD_OUT_HIGH);
> +	if (IS_ERR(gpio_reset))
> +		return PTR_ERR(gpio_reset);
> +
> +	if (gpio_reset) {
> +		gpiod_set_value_cansleep(gpio_reset, 1);
> +		usleep_range(1, 100);
> +		gpiod_set_value_cansleep(gpio_reset, 0);
> +		msleep_interruptible(50);
> +	} else {
> +		ret = regmap_update_bits(st->regmap, ADE9078_REG_CONFIG1,
> +					 ADE9078_SWRST_BIT, ADE9078_SWRST_BIT);
> +		if (ret)
> +			return ret;
> +		usleep_range(80, 100);
> +	}
> +
> +	if (!st->rst_done)

Why EPERM?  Seems more like an IO error

> +		return -EPERM;
> +
> +	return 0;
> +}
> +
> +/*
> + * ade9078_setup() - initial register setup of the ade9078
> + * @st:		ade9078 device data
> + */
> +static int ade9078_setup(struct ade9078_state *st)
> +{
> +	int ret = 0;
The value here is never used.

Make sure you run a bunch of build tests including setting W=1 and ensuring it
is clean + run at very least sparse.  Some of those tests would warn about this.

> +
> +	ret = regmap_multi_reg_write(st->regmap, ade9078_reg_sequence,
> +				     ARRAY_SIZE(ade9078_reg_sequence));
> +	if (ret)
> +		return ret;
> +
> +	msleep_interruptible(2);
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, GENMASK(31, 0));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1, GENMASK(31, 0));

return regmap_write()

> +	if (ret)
> +		return ret;
> +
> +	return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops ade9078_buffer_ops = {
> +	.preenable = &ade9078_buffer_preenable,
> +	.postdisable = &ade9078_buffer_postdisable,
> +};
> +
> +static const struct iio_info ade9078_info = {
> +	.read_raw = &ade9078_read_raw,
> +	.write_raw = &ade9078_write_raw,
> +	.debugfs_reg_access = &ade9078_reg_access,
> +	.write_event_config = &ade9078_write_event_config,
> +	.read_event_value = &ade9078_read_event_vlaue,

> +};
> +
> +/*
> + * Regmap configuration
> + * The register access of the ade9078 requires a 16 bit address
> + * with the read flag on bit 3. This is not supported by default
> + * regmap functionality, thus reg_read and reg_write have been
> + * replaced with custom functions

How big would the changes needed to support this in the regmap
core be?   Superficially I can't immediately see why it won't work.
regmap appears to support setting flags in any of the address bytes
as it uses regmap_set_work_buf_flag_mask() internally and
that will set bits in any of the reg_bits/8 bytes.

> + */
> +static const struct regmap_config ade9078_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 32,
> +	.zero_flag_mask = true,
> +	.cache_type = REGCACHE_RBTREE,
> +	.reg_read = ade9078_spi_read_reg,
> +	.reg_write = ade9078_spi_write_reg,
> +	.volatile_reg = ade9078_is_volatile_reg,
> +};
> +
> +static int ade9078_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ade9078_state *st;
> +	unsigned long irqflags;
> +	struct regmap *regmap;
> +	int irq;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev) {
> +		dev_err(&spi->dev, "Unable to allocate ADE9078 IIO");
> +		return -ENOMEM;
> +	}
> +	st = iio_priv(indio_dev);
> +	if (!st) {
> +		dev_err(&spi->dev,
> +			"Unable to allocate ADE9078 device structure");

This can't fail. So drop this check.

> +		return -ENOMEM;
> +	}
> +
> +	st->rx = devm_kcalloc(&spi->dev, ADE9078_RX_DEPTH, sizeof(*st->rx),
> +			      GFP_KERNEL);

Avoid having multiple small allocations like this by making them part of
the iio_priv() structure. 

> +	if (!st->rx)
> +		return -ENOMEM;
> +
> +	st->tx = devm_kcalloc(&spi->dev, ADE9078_TX_DEPTH, sizeof(*st->tx),
> +			      GFP_KERNEL);
> +	if (!st->tx)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init(&spi->dev, NULL, spi, &ade9078_regmap_config);

Probably better to pass the iio_device structure and have a pointer to
spi in there.

> +	if (IS_ERR(regmap))	{
> +		dev_err(&spi->dev, "Unable to allocate ADE9078 regmap");
> +		return PTR_ERR(regmap);
> +	}
> +	spi_set_drvdata(spi, st);

This shouldn't be needed after you've changed the passed context to provide
the iio_dev structure and you can then call iio_priv() on that.


> +
> +	irq = of_irq_get_byname((&spi->dev)->of_node, "irq0");
> +	if (irq < 0) {
> +		dev_err(&spi->dev, "Unable to find irq0");
> +		return -EINVAL;
> +	}
> +	irqflags = irq_get_trigger_type(irq);
> +	ret = devm_request_threaded_irq(&spi->dev, irq, NULL,
> +					ade9078_irq0_thread,
> +					irqflags | IRQF_ONESHOT,
> +					KBUILD_MODNAME, st);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to request threaded irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	irq = of_irq_get_byname((&spi->dev)->of_node, "irq1");

A generic firmware properties version of irq_get_by_name is queued in
the i2c tree.  Please use that here and mention it in the
patch description / cover letter.  I'll probably have pulled it into
IIO shortly anyway.

https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git/commit/?h=i2c/alert-for-acpi&id=ca0acb511c21738b32386ce0f85c284b351d919e

> +	if (irq < 0) {
> +		dev_err(&spi->dev, "Unable to find irq1");
> +		return -EINVAL;
> +	}

blank line after error checks like this make things more readable (slightly)

> +	irqflags = irq_get_trigger_type(irq);
> +	ret = devm_request_threaded_irq(&spi->dev, irq, NULL,
> +					ade9078_irq1_thread,
> +					irqflags | IRQF_ONESHOT,

You shouldn't need to read and then feed through the flags.
Unless something unusual is going on this will have the same
affect as just passing IRQF_ONESHOT.


> +					KBUILD_MODNAME, st);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to request threaded irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	st->spi = spi;
> +
> +	indio_dev->dev.parent = &st->spi->dev;
> +	indio_dev->info = &ade9078_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	st->regmap = regmap;
> +	st->indio_dev = indio_dev;
> +	ade9078_setup_iio_channels(st);
> +	if (ret) {

ret = ade9078_setup_iio_channels(st);

> +		dev_err(&spi->dev, "Failed to set up IIO channels");
> +		return ret;
> +	}
> +
> +	ret = ade9078_configure_scan(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_kfifo_buffer_setup(&spi->dev, indio_dev,
> +					  INDIO_BUFFER_SOFTWARE,
> +					  &ade9078_buffer_ops);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_device_register(&spi->dev, indio_dev);

Anything happening in probe after this call is usually a bad sign because
this is the point at which userspace interfaces are exposed - hence it
normally comes last to avoid any potential race condtions.

> +	if (ret) {
> +		dev_err(&spi->dev, "Unable to register IIO device");
> +		return ret;
> +	}
> +
> +	ret = ade9078_reset(st);
> +	if (ret) {
> +		dev_err(&spi->dev, "ADE9078 reset failed");
> +		return ret;
> +	}
> +
> +	ret = ade9078_setup(st);
> +	if (ret) {
> +		dev_err(&spi->dev, "Unable to setup ADE9078");
> +		return ret;
> +	}
> +
> +	return ret;
> +};
> +
> +static const struct spi_device_id ade9078_id[] = {
> +		{"ade9078", 0},
> +		{}
> +};
> +MODULE_DEVICE_TABLE(spi, ade9078_id);
> +
> +static const struct of_device_id ade9078_of_match[] = {
> +	{ .compatible = "adi,ade9078" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ade9078_of_match);
> +
> +static struct spi_driver ade9078_driver = {
> +		.driver = {
> +			.name = "ade9078",
> +		},
> +		.probe = ade9078_probe,
> +		.id_table = ade9078_id,
> +};
> +module_spi_driver(ade9078_driver);
> +
> +MODULE_AUTHOR("Ciprian Hegbeli <[e-mail]>");
> +MODULE_DESCRIPTION("Analog Devices ADE9078 High Performance,Polyphase Energy Metering IC Driver");

Shorter might be good.  Could definitely drop "High Performance" without changing the useful
content in a driver description :)

> +MODULE_LICENSE("GPL v2");

Ciprian Hegbeli April 7, 2022, 12:56 p.m. UTC | #2

> Hi Ciprian
> 
> I took a very quick end of day look so probably missed some stuff I'll
> pick up on in v2.  Biggest issue is new ABI without docs. Various
> other comments inline. Plus as already observed, please use full name
> in from and SoB as they form part of the Developer Certificate of Origin.
> 
> Thanks,
> 
> Jonathan

Hi Jonathan,

Thank you for the review. I wanted to answer the regmap related
questions and maybe ask for some advice regarding them. There
are some details in the messages below; the short versions is that
it's doable but I will have to give up on some of the regmap 
functionality and mask some of the basic regmap functions in driver 
specific functions. These changes might make the code less
readable.  

> > +static const struct reg_sequence ade9078_reg_sequence[] = {
> > +	{ ADE9078_REG_PGA_GAIN, ADE9078_PGA_GAIN },
> > +	{ ADE9078_REG_CONFIG0, ADE9078_CONFIG0 },
> > +	{ ADE9078_REG_CONFIG1, ADE9078_CONFIG1 },
> > +	{ ADE9078_REG_CONFIG2, ADE9078_CONFIG2 },
> > +	{ ADE9078_REG_CONFIG3, ADE9078_CONFIG3 },
> > +	{ ADE9078_REG_ACCMODE, ADE9078_ACCMODE },
> > +	{ ADE9078_REG_ZX_LP_SEL, ADE9078_ZX_LP_SEL },
> > +	{ ADE9078_REG_MASK0, ADE9078_MASK0 },
> > +	{ ADE9078_REG_MASK1, ADE9078_MASK1 },
> > +	{ ADE9078_REG_EVENT_MASK, ADE9078_EVENT_MASK },
> > +	{ ADE9078_REG_WFB_CFG, ADE9078_WFB_CFG },
> > +	{ ADE9078_REG_VLEVEL, ADE9078_VLEVEL },
> > +	{ ADE9078_REG_DICOEFF, ADE9078_DICOEFF },
> > +	{ ADE9078_REG_EGY_TIME, ADE9078_EGY_TIME },
> > +	{ ADE9078_REG_EP_CFG, ADE9078_EP_CFG },
> > +	{ ADE9078_REG_RUN, ADE9078_RUN_ON }
> > +};

CONFIG0, MASK0, MASK1, EVENT_MASK, VLEVEL, DICOEFF are 
32bit registers. 

> > +/*
> 
> As mentioned below, I don't immediately understand why this can't
> be done with appropriate standard regmap.  Perhaps you could give
> more details of what is missing.  I'd like to see that added to
> regmap if possible.
> 

I started implementing the driver with a standard SPI and using 
bulk read/write for the 32 bit registers. This however renders some 
of the standard features of the regmap unusable (as far as I know).
For example I can't use regmap_update_bits for 32 bit registers,
which happens a few times, which I have marked in the code below.
I could implement a substitute function in my driver for this, but that
would complicate the code (in my opinion).   

> > + * ade9078_spi_write_reg() - ade9078 write register over SPI
> > + * the data format for communicating with the ade9078 over SPI
> > + * is very specific and can access both 32bit and 16bit registers
> > + * @context:	void pointer to the SPI device
> > + * @reg:	address of the of desired register
> > + * @val:	value to be written to the ade9078
> > + */
> > +static int ade9078_spi_write_reg(void *context,
> > +				 unsigned int reg,
> > +				 unsigned int val)
> > +{
> > +	struct device *dev = context;
> > +	struct spi_device *spi = to_spi_device(dev);
> > +	struct ade9078_state *st = spi_get_drvdata(spi);
> 
> > +
> > +	u16 addr;
> > +	int ret = 0;
> > +	struct spi_transfer xfer[] = {
> > +		{
> > +			.tx_buf = st->tx,
> > +		},
> > +	};
> > +
> > +	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg);
> > +
> > +	put_unaligned_be16(addr, st->tx);
> > +	put_unaligned_be32(val, &st->tx[2]);
> > +
> > +	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION) {
> > +		put_unaligned_be16(val, &st->tx[2]);
> > +		xfer[0].len = 4;
> > +	} else {
> > +		xfer[0].len = 6;
> > +	}
> > +
> > +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> > +	if (ret) {
> > +		dev_err(&st->spi->dev, "problem when writing register
> 0x%x",
> > +			reg);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +/*
> > + * ade9078_spi_write_reg() - ade9078 read register over SPI
> > + * the data format for communicating with the ade9078 over SPI
> > + * is very specific and can access both 32bit and 16bit registers
> > + * @context:	void pointer to the SPI device
> > + * @reg:	address of the of desired register
> > + * @val:	value to be read to the ade9078
> > + */
> > +static int ade9078_spi_read_reg(void *context,
> > +				unsigned int reg,
> > +				unsigned int *val)
> > +{
> > +	struct device *dev = context;
> > +	struct spi_device *spi = to_spi_device(dev);
> > +	struct ade9078_state *st = spi_get_drvdata(spi);
> > +
> > +	u16 addr;
> > +	int ret = 0;
> > +	struct spi_transfer xfer[] = {
> > +		{
> > +			.tx_buf = st->tx,
> > +			.len = 2,
> > +		},
> > +		{
> > +			.rx_buf = st->rx,
> > +		},
> > +	};
> > +
> > +	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg) |
> > +	       ADE9078_REG_READ_BIT_MASK;
> > +
> > +	put_unaligned_be16(addr, st->tx);
> > +
> > +	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION)
> > +		xfer[1].len = 4;
> > +	else
> > +		xfer[1].len = 6;
> 
> This doesn't look like a fixed length register which is expected
> for regmap...  Also that len should just be the rx register which
> you treat below as 16 bit and 32 bit (so 2 and 4, not 4 and 6).
> 
> Can the larger registers be treated as bulk reads of pairs of smaller ones?
> 

It could, but I would still need an if or a type of "decoder" to determine
if it is a 16 bit or 32 bit register, which means I still need a form of 
ade9078_read_reg which selects between a regmap_read function
or regmap_bulk_read. This goes for write as well. 

> > +
> > +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> > +	if (ret) {
> > +		dev_err(&st->spi->dev, "problem when reading register
> 0x%x",
> > +			reg);
> > +		goto err_ret;
> > +	}
> > +
> > +	//registers which are 16 bits
> > +	if (reg > 0x480 && reg < 0x4FE)
> > +		*val = get_unaligned_be16(st->rx);
> > +	else
> > +		*val = get_unaligned_be32(st->rx);
> > +
> > +err_ret:
> > +	return ret;
> > +}
> > +
> > +/*
> > + * ade9078_is_volatile_reg() - list of ade9078 registers which should use
> > + * caching
> > + * @dev:	device data
> > + * @reg:	address of the of desired register
> > + */
> > +static bool ade9078_is_volatile_reg(struct device *dev, unsigned int reg)
> > +{
> > +	switch (reg) {
> > +	case ADE9078_REG_STATUS0:
> > +	case ADE9078_REG_STATUS1:
> > +	case ADE9078_REG_MASK0:
> > +	case ADE9078_REG_MASK1:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +

I left this part in for context.

> > +/*
> > + * ade9078_write_event_config() - IIO event configure to enable zero-
> crossing
> 
> All these function description comments should be valid kernel-doc.
> 
> > + * and zero-crossing timeout on voltage and current for each phases.
> These
> > + * events will also influence the trigger conditions for the buffer capture.
> > + */
> > +static int ade9078_write_event_config(struct iio_dev *indio_dev,
> > +				      const struct iio_chan_spec *chan,
> > +				      enum iio_event_type type,
> > +				      enum iio_event_direction dir,
> > +				      int state)
> > +{
> > +	struct ade9078_state *st = iio_priv(indio_dev);
> > +	u32 interrupts;
> > +	u32 number;
> > +	int ret;
> > +
> > +	number = chan->channel;
> > +
> > +	switch (number) {
> > +	case ADE9078_PHASE_A_NR:
> 
> I would use a lookup on the phase into an array of structures.
> The structure would then have fields for which bit to set etc
> for a voltage channel and for a current channel.
> 
> That way this all becomes one bit of code and some const data
> rather that 3 sets of near identical code. If you can make
> this sort of thing data rather than code that is almost always
> the best choice.
> 
> > +		if (chan->type == IIO_VOLTAGE) {
> > +			if (state) {
> > +				interrupts |= ADE9078_ST1_ZXVA_BIT;
> > +				interrupts |= ADE9078_ST1_ZXTOVA_BIT;
> > +				st->wfb_trg |=
> ADE9078_WFB_TRG_ZXVA_BIT;
> > +			} else {
> > +				interrupts &= ~ADE9078_ST1_ZXVA_BIT;
> > +				interrupts &= ~ADE9078_ST1_ZXTOVA_BIT;
> > +				st->wfb_trg &=
> ~ADE9078_WFB_TRG_ZXVA_BIT;
> > +			}
> > +		} else if (chan->type == IIO_CURRENT) {
> > +			if (state) {
> > +				interrupts |= ADE9078_ST1_ZXIA_BIT;
> > +				st->wfb_trg |=
> ADE9078_WFB_TRG_ZXIA_BIT;
> > +			} else {
> > +				interrupts &= ~ADE9078_ST1_ZXIA_BIT;
> > +				st->wfb_trg &=
> ~ADE9078_WFB_TRG_ZXIA_BIT;
> > +			}
> > +		}
> > +		break;
> > +	case ADE9078_PHASE_B_NR:
> > +		if (chan->type == IIO_VOLTAGE) {
> > +			if (state) {
> > +				interrupts |= ADE9078_ST1_ZXVB_BIT;
> > +				interrupts |= ADE9078_ST1_ZXTOVB_BIT;
> > +				st->wfb_trg |=
> ADE9078_WFB_TRG_ZXVB_BIT;
> > +			} else {
> > +				interrupts &= ~ADE9078_ST1_ZXVB_BIT;
> > +				interrupts &= ~ADE9078_ST1_ZXTOVB_BIT;
> > +				st->wfb_trg &=
> ~ADE9078_WFB_TRG_ZXVB_BIT;
> > +			}
> > +		} else if (chan->type == IIO_CURRENT) {
> > +			if (state) {
> > +				interrupts |= ADE9078_ST1_ZXIB_BIT;
> > +				st->wfb_trg |=
> ADE9078_WFB_TRG_ZXIB_BIT;
> > +			} else {
> > +				interrupts &= ~ADE9078_ST1_ZXIB_BIT;
> > +				st->wfb_trg &=
> ~ADE9078_WFB_TRG_ZXIB_BIT;
> > +			}
> > +		}
> > +		break;
> > +	case ADE9078_PHASE_C_NR:
> > +		if (chan->type == IIO_VOLTAGE) {
> > +			if (state) {
> > +				interrupts |= ADE9078_ST1_ZXVC_BIT;
> > +				interrupts |= ADE9078_ST1_ZXTOVC_BIT;
> > +				st->wfb_trg |=
> ADE9078_WFB_TRG_ZXVC_BIT;
> > +			} else {
> > +				interrupts &= ~ADE9078_ST1_ZXVC_BIT;
> > +				interrupts &= ~ADE9078_ST1_ZXTOVC_BIT;
> > +				st->wfb_trg &=
> ~ADE9078_WFB_TRG_ZXVC_BIT;
> > +			}
> > +		} else if (chan->type == IIO_CURRENT) {
> > +			if (state) {
> > +				interrupts |= ADE9078_ST1_ZXIC_BIT;
> > +				st->wfb_trg |=
> ADE9078_WFB_TRG_ZXIC_BIT;
> > +			} else {
> > +				interrupts &= ~ADE9078_ST1_ZXIC_BIT;
> > +				st->wfb_trg &=
> ~ADE9078_WFB_TRG_ZXIC_BIT;
> > +			}
> > +		}
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1,
> GENMASK(31, 0));
> > +	if (ret)
> > +		return ret;
> > +
> > +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK1,
> interrupts,
> > +				  interrupts);
> > +}

MASK1 is a 32 bit registers.

> > +/*
> > + * ade9078_wfb_interrupt_setup() - Configures the wave form buffer
> interrupt
> > + * according to modes
> > + * @st:		ade9078 device data
> > + * @mode:	modes according to datasheet; values [0-2]
> > + *
> > + * This sets the interrupt register and other registers related to the
> > + * interrupts according to mode [0-2] from the datasheet
> > + */
> > +static int ade9078_wfb_interrupt_setup(struct ade9078_state *st, u8
> mode)
> > +{
> > +	int ret;
> > +
> > +	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (mode == ADE9078_WFB_FULL_MODE || mode ==
> ADE9078_WFB_EN_TRIG_MODE) {
> 
> A switch statement would be cleaner here.
> 
> > +		ret = regmap_write(st->regmap,
> ADE9078_REG_WFB_PG_IRQEN,
> > +				   ADE9078_MODE_0_1_PAGE_BIT);
> > +		if (ret)
> > +			return ret;
> > +	} else if (mode == ADE9078_WFB_C_EN_TRIG_MODE) {
> > +		ret = regmap_write(st->regmap,
> ADE9078_REG_WFB_PG_IRQEN,
> > +				   ADE9078_MODE_2_PAGE_BIT);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0,
> GENMASK(31, 0));
> > +	if (ret)
> > +		return ret;
> > +
> > +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0,
> > +				  ADE9078_ST0_PAGE_FULL_BIT,
> > +				  ADE9078_ST0_PAGE_FULL_BIT);
> > +	if (ret)
> 
> Unreachable code...
> 
> > +		return ret;
> > +
> > +	return 0;
> > +}

MASK0 is a 32 bit registers.

> > +/*
> > + * ade9078_buffer_postdisable() - after the iio is disable
> > + * this will disable the ade9078 internal buffer for acquisition
> > + * @indio_dev:	the IIO device
> > + */
> > +static int ade9078_buffer_postdisable(struct iio_dev *indio_dev)
> > +{
> > +	struct ade9078_state *st = iio_priv(indio_dev);
> > +	u32 interrupts = 0;
> > +	int ret;
> > +
> > +	ret = ade9078_en_wfb(st, false);
> > +	if (ret) {
> > +		dev_err(&st->spi->dev, "Post-disable wfb disable fail");
> > +		return ret;
> > +	}
> > +
> > +	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	interrupts |= ADE9078_ST0_WFB_TRIG_BIT;
> > +	interrupts |= ADE9078_ST0_PAGE_FULL_BIT;
> > +
> > +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0,
> interrupts, 0);
> > +	if (ret) {
> > +		dev_err(&st->spi->dev, "Post-disable update maks0 fail");
> > +		return ret;
> > +	}
> > +
> > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0,
> GENMASK(31, 0));
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> 
> return regmap_write()...
> 
> > +}

MASK0 is updated again.

> > +/*
> > + * ade9078_setup() - initial register setup of the ade9078
> > + * @st:		ade9078 device data
> > + */
> > +static int ade9078_setup(struct ade9078_state *st)
> > +{
> > +	int ret = 0;
> The value here is never used.
> 
> Make sure you run a bunch of build tests including setting W=1 and ensuring
> it
> is clean + run at very least sparse.  Some of those tests would warn about
> this.
> 
> > +
> > +	ret = regmap_multi_reg_write(st->regmap, ade9078_reg_sequence,
> > +				     ARRAY_SIZE(ade9078_reg_sequence));
> > +	if (ret)
> > +		return ret;
> > +

regmap_multi_reg_write is another function I don't think I could use at its 
full extent if I switch to standard regmap with bulk transfers, because I 
configure both 16 bit and 32 bit registres. The 32 bit registers configured
in this set are listed above, near the begging of the e-mail.

> > +	msleep_interruptible(2);
> > +
> > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0,
> GENMASK(31, 0));
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1,
> GENMASK(31, 0));
> 
> return regmap_write()
> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	return ret;
> > +}
> > +

I left this part in for context.

> > +/*
> > + * Regmap configuration
> > + * The register access of the ade9078 requires a 16 bit address
> > + * with the read flag on bit 3. This is not supported by default
> > + * regmap functionality, thus reg_read and reg_write have been
> > + * replaced with custom functions
> 
> How big would the changes needed to support this in the regmap
> core be?   Superficially I can't immediately see why it won't work.
> regmap appears to support setting flags in any of the address bytes
> as it uses regmap_set_work_buf_flag_mask() internally and
> that will set bits in any of the reg_bits/8 bytes.
> 
> > + */
> > +static const struct regmap_config ade9078_regmap_config = {
> > +	.reg_bits = 16,
> > +	.val_bits = 32,
> > +	.zero_flag_mask = true,
> > +	.cache_type = REGCACHE_RBTREE,
> > +	.reg_read = ade9078_spi_read_reg,
> > +	.reg_write = ade9078_spi_write_reg,
> > +	.volatile_reg = ade9078_is_volatile_reg,
> > +};

It dose work but as mentioned above it comes with an additional set
of driver specific functions which will ultimately mask the underlying 
regmap functions in the best case scenario.

All in all, I could implement the changes for V2 but given the fact that
this would mean replacing some regmap functions, I decide to ask for your 
opinion and some guidance first.

- Ciprian

Jonathan Cameron April 10, 2022, 3:42 p.m. UTC | #3

On Thu, 7 Apr 2022 12:56:47 +0000
"Hegbeli, Ciprian" <[e-mail]> wrote:

> > Hi Ciprian
> > 
> > I took a very quick end of day look so probably missed some stuff I'll
> > pick up on in v2.  Biggest issue is new ABI without docs. Various
> > other comments inline. Plus as already observed, please use full name
> > in from and SoB as they form part of the Developer Certificate of Origin.
> > 
> > Thanks,
> > 
> > Jonathan  
> 
> Hi Jonathan,
> 
> Thank you for the review. I wanted to answer the regmap related
> questions and maybe ask for some advice regarding them. There
> are some details in the messages below; the short versions is that
> it's doable but I will have to give up on some of the regmap 
> functionality and mask some of the basic regmap functions in driver 
> specific functions. These changes might make the code less
> readable.  
> 
> > > +static const struct reg_sequence ade9078_reg_sequence[] = {
> > > +	{ ADE9078_REG_PGA_GAIN, ADE9078_PGA_GAIN },
> > > +	{ ADE9078_REG_CONFIG0, ADE9078_CONFIG0 },
> > > +	{ ADE9078_REG_CONFIG1, ADE9078_CONFIG1 },
> > > +	{ ADE9078_REG_CONFIG2, ADE9078_CONFIG2 },
> > > +	{ ADE9078_REG_CONFIG3, ADE9078_CONFIG3 },
> > > +	{ ADE9078_REG_ACCMODE, ADE9078_ACCMODE },
> > > +	{ ADE9078_REG_ZX_LP_SEL, ADE9078_ZX_LP_SEL },
> > > +	{ ADE9078_REG_MASK0, ADE9078_MASK0 },
> > > +	{ ADE9078_REG_MASK1, ADE9078_MASK1 },
> > > +	{ ADE9078_REG_EVENT_MASK, ADE9078_EVENT_MASK },
> > > +	{ ADE9078_REG_WFB_CFG, ADE9078_WFB_CFG },
> > > +	{ ADE9078_REG_VLEVEL, ADE9078_VLEVEL },
> > > +	{ ADE9078_REG_DICOEFF, ADE9078_DICOEFF },
> > > +	{ ADE9078_REG_EGY_TIME, ADE9078_EGY_TIME },
> > > +	{ ADE9078_REG_EP_CFG, ADE9078_EP_CFG },
> > > +	{ ADE9078_REG_RUN, ADE9078_RUN_ON }
> > > +};  
> 
> CONFIG0, MASK0, MASK1, EVENT_MASK, VLEVEL, DICOEFF are 
> 32bit registers. 

Ah. I'd missed that.  Indeed changes the whole discussion.

> 
> > > +/*  
> > 
> > As mentioned below, I don't immediately understand why this can't
> > be done with appropriate standard regmap.  Perhaps you could give
> > more details of what is missing.  I'd like to see that added to
> > regmap if possible.
> >   
> 
> I started implementing the driver with a standard SPI and using 
> bulk read/write for the 32 bit registers. This however renders some 
> of the standard features of the regmap unusable (as far as I know).
> For example I can't use regmap_update_bits for 32 bit registers,
> which happens a few times, which I have marked in the code below.
> I could implement a substitute function in my driver for this, but that
> would complicate the code (in my opinion).   

Agreed - regmap doesn't fit well in my opinion when there are a mixture
of register sizes.  This may just be a case where you indeed should
just not use regmap at all.  It's challenging to make it work
and sometimes it's not sensible to bother.


> 
> > > + * ade9078_spi_write_reg() - ade9078 write register over SPI
> > > + * the data format for communicating with the ade9078 over SPI
> > > + * is very specific and can access both 32bit and 16bit registers
> > > + * @context:	void pointer to the SPI device
> > > + * @reg:	address of the of desired register
> > > + * @val:	value to be written to the ade9078
> > > + */
> > > +static int ade9078_spi_write_reg(void *context,
> > > +				 unsigned int reg,
> > > +				 unsigned int val)
> > > +{
> > > +	struct device *dev = context;
> > > +	struct spi_device *spi = to_spi_device(dev);
> > > +	struct ade9078_state *st = spi_get_drvdata(spi);  
> >   
> > > +
> > > +	u16 addr;
> > > +	int ret = 0;
> > > +	struct spi_transfer xfer[] = {
> > > +		{
> > > +			.tx_buf = st->tx,
> > > +		},
> > > +	};
> > > +
> > > +	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg);
> > > +
> > > +	put_unaligned_be16(addr, st->tx);
> > > +	put_unaligned_be32(val, &st->tx[2]);
> > > +
> > > +	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION) {
> > > +		put_unaligned_be16(val, &st->tx[2]);
> > > +		xfer[0].len = 4;
> > > +	} else {
> > > +		xfer[0].len = 6;
> > > +	}
> > > +
> > > +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> > > +	if (ret) {
> > > +		dev_err(&st->spi->dev, "problem when writing register  
> > 0x%x",  
> > > +			reg);
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +/*
> > > + * ade9078_spi_write_reg() - ade9078 read register over SPI
> > > + * the data format for communicating with the ade9078 over SPI
> > > + * is very specific and can access both 32bit and 16bit registers
> > > + * @context:	void pointer to the SPI device
> > > + * @reg:	address of the of desired register
> > > + * @val:	value to be read to the ade9078
> > > + */
> > > +static int ade9078_spi_read_reg(void *context,
> > > +				unsigned int reg,
> > > +				unsigned int *val)
> > > +{
> > > +	struct device *dev = context;
> > > +	struct spi_device *spi = to_spi_device(dev);
> > > +	struct ade9078_state *st = spi_get_drvdata(spi);
> > > +
> > > +	u16 addr;
> > > +	int ret = 0;
> > > +	struct spi_transfer xfer[] = {
> > > +		{
> > > +			.tx_buf = st->tx,
> > > +			.len = 2,
> > > +		},
> > > +		{
> > > +			.rx_buf = st->rx,
> > > +		},
> > > +	};
> > > +
> > > +	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg) |
> > > +	       ADE9078_REG_READ_BIT_MASK;
> > > +
> > > +	put_unaligned_be16(addr, st->tx);
> > > +
> > > +	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION)
> > > +		xfer[1].len = 4;
> > > +	else
> > > +		xfer[1].len = 6;  
> > 
> > This doesn't look like a fixed length register which is expected
> > for regmap...  Also that len should just be the rx register which
> > you treat below as 16 bit and 32 bit (so 2 and 4, not 4 and 6).
> > 
> > Can the larger registers be treated as bulk reads of pairs of smaller ones?
> >   
> 
> It could, but I would still need an if or a type of "decoder" to determine
> if it is a 16 bit or 32 bit register, which means I still need a form of 
> ade9078_read_reg which selects between a regmap_read function
> or regmap_bulk_read. This goes for write as well. 

My assumption (wrong :) was that this was the normal 'large register' case
where we were actually looking at 2 registers that had to always be
read as a pair to make up the large register.  That isn't true here.

> 
> > > +
> > > +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> > > +	if (ret) {
> > > +		dev_err(&st->spi->dev, "problem when reading register  
> > 0x%x",  
> > > +			reg);
> > > +		goto err_ret;
> > > +	}
> > > +
> > > +	//registers which are 16 bits
> > > +	if (reg > 0x480 && reg < 0x4FE)
> > > +		*val = get_unaligned_be16(st->rx);
> > > +	else
> > > +		*val = get_unaligned_be32(st->rx);
> > > +
> > > +err_ret:
> > > +	return ret;
> > > +}
> > > +
> > > +/*
> > > + * ade9078_is_volatile_reg() - list of ade9078 registers which should use
> > > + * caching
> > > + * @dev:	device data
> > > + * @reg:	address of the of desired register
> > > + */
> > > +static bool ade9078_is_volatile_reg(struct device *dev, unsigned int reg)
> > > +{
> > > +	switch (reg) {
> > > +	case ADE9078_REG_STATUS0:
> > > +	case ADE9078_REG_STATUS1:
> > > +	case ADE9078_REG_MASK0:
> > > +	case ADE9078_REG_MASK1:
> > > +		return true;
> > > +	default:
> > > +		return false;
> > > +	}
> > > +}
> > > +  
> 
> I left this part in for context.
> 
> > > +/*
> > > + * ade9078_write_event_config() - IIO event configure to enable zero-  
> > crossing
> > 
> > All these function description comments should be valid kernel-doc.
> >   
> > > + * and zero-crossing timeout on voltage and current for each phases.  
> > These  
> > > + * events will also influence the trigger conditions for the buffer capture.
> > > + */
> > > +static int ade9078_write_event_config(struct iio_dev *indio_dev,
> > > +				      const struct iio_chan_spec *chan,
> > > +				      enum iio_event_type type,
> > > +				      enum iio_event_direction dir,
> > > +				      int state)
> > > +{
> > > +	struct ade9078_state *st = iio_priv(indio_dev);
> > > +	u32 interrupts;
> > > +	u32 number;
> > > +	int ret;
> > > +
> > > +	number = chan->channel;
> > > +
> > > +	switch (number) {
> > > +	case ADE9078_PHASE_A_NR:  
> > 
> > I would use a lookup on the phase into an array of structures.
> > The structure would then have fields for which bit to set etc
> > for a voltage channel and for a current channel.
> > 
> > That way this all becomes one bit of code and some const data
> > rather that 3 sets of near identical code. If you can make
> > this sort of thing data rather than code that is almost always
> > the best choice.
> >   
> > > +		if (chan->type == IIO_VOLTAGE) {
> > > +			if (state) {
> > > +				interrupts |= ADE9078_ST1_ZXVA_BIT;
> > > +				interrupts |= ADE9078_ST1_ZXTOVA_BIT;
> > > +				st->wfb_trg |=  
> > ADE9078_WFB_TRG_ZXVA_BIT;  
> > > +			} else {
> > > +				interrupts &= ~ADE9078_ST1_ZXVA_BIT;
> > > +				interrupts &= ~ADE9078_ST1_ZXTOVA_BIT;
> > > +				st->wfb_trg &=  
> > ~ADE9078_WFB_TRG_ZXVA_BIT;  
> > > +			}
> > > +		} else if (chan->type == IIO_CURRENT) {
> > > +			if (state) {
> > > +				interrupts |= ADE9078_ST1_ZXIA_BIT;
> > > +				st->wfb_trg |=  
> > ADE9078_WFB_TRG_ZXIA_BIT;  
> > > +			} else {
> > > +				interrupts &= ~ADE9078_ST1_ZXIA_BIT;
> > > +				st->wfb_trg &=  
> > ~ADE9078_WFB_TRG_ZXIA_BIT;  
> > > +			}
> > > +		}
> > > +		break;
> > > +	case ADE9078_PHASE_B_NR:
> > > +		if (chan->type == IIO_VOLTAGE) {
> > > +			if (state) {
> > > +				interrupts |= ADE9078_ST1_ZXVB_BIT;
> > > +				interrupts |= ADE9078_ST1_ZXTOVB_BIT;
> > > +				st->wfb_trg |=  
> > ADE9078_WFB_TRG_ZXVB_BIT;  
> > > +			} else {
> > > +				interrupts &= ~ADE9078_ST1_ZXVB_BIT;
> > > +				interrupts &= ~ADE9078_ST1_ZXTOVB_BIT;
> > > +				st->wfb_trg &=  
> > ~ADE9078_WFB_TRG_ZXVB_BIT;  
> > > +			}
> > > +		} else if (chan->type == IIO_CURRENT) {
> > > +			if (state) {
> > > +				interrupts |= ADE9078_ST1_ZXIB_BIT;
> > > +				st->wfb_trg |=  
> > ADE9078_WFB_TRG_ZXIB_BIT;  
> > > +			} else {
> > > +				interrupts &= ~ADE9078_ST1_ZXIB_BIT;
> > > +				st->wfb_trg &=  
> > ~ADE9078_WFB_TRG_ZXIB_BIT;  
> > > +			}
> > > +		}
> > > +		break;
> > > +	case ADE9078_PHASE_C_NR:
> > > +		if (chan->type == IIO_VOLTAGE) {
> > > +			if (state) {
> > > +				interrupts |= ADE9078_ST1_ZXVC_BIT;
> > > +				interrupts |= ADE9078_ST1_ZXTOVC_BIT;
> > > +				st->wfb_trg |=  
> > ADE9078_WFB_TRG_ZXVC_BIT;  
> > > +			} else {
> > > +				interrupts &= ~ADE9078_ST1_ZXVC_BIT;
> > > +				interrupts &= ~ADE9078_ST1_ZXTOVC_BIT;
> > > +				st->wfb_trg &=  
> > ~ADE9078_WFB_TRG_ZXVC_BIT;  
> > > +			}
> > > +		} else if (chan->type == IIO_CURRENT) {
> > > +			if (state) {
> > > +				interrupts |= ADE9078_ST1_ZXIC_BIT;
> > > +				st->wfb_trg |=  
> > ADE9078_WFB_TRG_ZXIC_BIT;  
> > > +			} else {
> > > +				interrupts &= ~ADE9078_ST1_ZXIC_BIT;
> > > +				st->wfb_trg &=  
> > ~ADE9078_WFB_TRG_ZXIC_BIT;  
> > > +			}
> > > +		}
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1,  
> > GENMASK(31, 0));  
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK1,  
> > interrupts,  
> > > +				  interrupts);
> > > +}  
> 
> MASK1 is a 32 bit registers.
> 
> > > +/*
> > > + * ade9078_wfb_interrupt_setup() - Configures the wave form buffer  
> > interrupt  
> > > + * according to modes
> > > + * @st:		ade9078 device data
> > > + * @mode:	modes according to datasheet; values [0-2]
> > > + *
> > > + * This sets the interrupt register and other registers related to the
> > > + * interrupts according to mode [0-2] from the datasheet
> > > + */
> > > +static int ade9078_wfb_interrupt_setup(struct ade9078_state *st, u8  
> > mode)  
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (mode == ADE9078_WFB_FULL_MODE || mode ==  
> > ADE9078_WFB_EN_TRIG_MODE) {
> > 
> > A switch statement would be cleaner here.
> >   
> > > +		ret = regmap_write(st->regmap,  
> > ADE9078_REG_WFB_PG_IRQEN,  
> > > +				   ADE9078_MODE_0_1_PAGE_BIT);
> > > +		if (ret)
> > > +			return ret;
> > > +	} else if (mode == ADE9078_WFB_C_EN_TRIG_MODE) {
> > > +		ret = regmap_write(st->regmap,  
> > ADE9078_REG_WFB_PG_IRQEN,  
> > > +				   ADE9078_MODE_2_PAGE_BIT);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0,  
> > GENMASK(31, 0));  
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0,
> > > +				  ADE9078_ST0_PAGE_FULL_BIT,
> > > +				  ADE9078_ST0_PAGE_FULL_BIT);
> > > +	if (ret)  
> > 
> > Unreachable code...
> >   
> > > +		return ret;
> > > +
> > > +	return 0;
> > > +}  
> 
> MASK0 is a 32 bit registers.
> 
> > > +/*
> > > + * ade9078_buffer_postdisable() - after the iio is disable
> > > + * this will disable the ade9078 internal buffer for acquisition
> > > + * @indio_dev:	the IIO device
> > > + */
> > > +static int ade9078_buffer_postdisable(struct iio_dev *indio_dev)
> > > +{
> > > +	struct ade9078_state *st = iio_priv(indio_dev);
> > > +	u32 interrupts = 0;
> > > +	int ret;
> > > +
> > > +	ret = ade9078_en_wfb(st, false);
> > > +	if (ret) {
> > > +		dev_err(&st->spi->dev, "Post-disable wfb disable fail");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	interrupts |= ADE9078_ST0_WFB_TRIG_BIT;
> > > +	interrupts |= ADE9078_ST0_PAGE_FULL_BIT;
> > > +
> > > +	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0,  
> > interrupts, 0);  
> > > +	if (ret) {
> > > +		dev_err(&st->spi->dev, "Post-disable update maks0 fail");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0,  
> > GENMASK(31, 0));  
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return 0;  
> > 
> > return regmap_write()...
> >   
> > > +}  
> 
> MASK0 is updated again.
> 
> > > +/*
> > > + * ade9078_setup() - initial register setup of the ade9078
> > > + * @st:		ade9078 device data
> > > + */
> > > +static int ade9078_setup(struct ade9078_state *st)
> > > +{
> > > +	int ret = 0;  
> > The value here is never used.
> > 
> > Make sure you run a bunch of build tests including setting W=1 and ensuring
> > it
> > is clean + run at very least sparse.  Some of those tests would warn about
> > this.
> >   
> > > +
> > > +	ret = regmap_multi_reg_write(st->regmap, ade9078_reg_sequence,
> > > +				     ARRAY_SIZE(ade9078_reg_sequence));
> > > +	if (ret)
> > > +		return ret;
> > > +  
> 
> regmap_multi_reg_write is another function I don't think I could use at its 
> full extent if I switch to standard regmap with bulk transfers, because I 
> configure both 16 bit and 32 bit registres. The 32 bit registers configured
> in this set are listed above, near the begging of the e-mail.
> 
> > > +	msleep_interruptible(2);
> > > +
> > > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0,  
> > GENMASK(31, 0));  
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1,  
> > GENMASK(31, 0));
> > 
> > return regmap_write()
> >   
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return ret;
> > > +}
> > > +  
> 
> I left this part in for context.
> 
> > > +/*
> > > + * Regmap configuration
> > > + * The register access of the ade9078 requires a 16 bit address
> > > + * with the read flag on bit 3. This is not supported by default
> > > + * regmap functionality, thus reg_read and reg_write have been
> > > + * replaced with custom functions  
> > 
> > How big would the changes needed to support this in the regmap
> > core be?   Superficially I can't immediately see why it won't work.
> > regmap appears to support setting flags in any of the address bytes
> > as it uses regmap_set_work_buf_flag_mask() internally and
> > that will set bits in any of the reg_bits/8 bytes.
> >   
> > > + */
> > > +static const struct regmap_config ade9078_regmap_config = {
> > > +	.reg_bits = 16,
> > > +	.val_bits = 32,
> > > +	.zero_flag_mask = true,
> > > +	.cache_type = REGCACHE_RBTREE,
> > > +	.reg_read = ade9078_spi_read_reg,
> > > +	.reg_write = ade9078_spi_write_reg,
> > > +	.volatile_reg = ade9078_is_volatile_reg,
> > > +};  
> 
> It dose work but as mentioned above it comes with an additional set
> of driver specific functions which will ultimately mask the underlying 
> regmap functions in the best case scenario.
> 
> All in all, I could implement the changes for V2 but given the fact that
> this would mean replacing some regmap functions, I decide to ask for your 
> opinion and some guidance first.

I'd just drop regmap and do things the old fashions (and more flexible)
way.  Whilst the device is register based - mixed sized registers are
a horrible hardware quirk so it doesn't map (afaik) to regmap.

Jonathan

> 
> - Ciprian

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index a2c8699e9e41..72fac9f4f837 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1101,6 +1101,14 @@  L:	[e-mail]
 S:	Maintained
 F:	drivers/media/i2c/ad9389b*
 
+ANALOG DEVICES INC ADE9078 DRIVER
+M:	Ciprian Hegbeli <[e-mail]>
+L:	[e-mail]
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
+F:	drivers/iio/meter/ade9078.c
+
 ANALOG DEVICES INC ADGS1408 DRIVER
 M:	Mircea Caprioru <[e-mail]>
 S:	Supported
diff --git a/drivers/iio/meter/Kconfig b/drivers/iio/meter/Kconfig
index d1c91eed5283..b46e93af1052 100644
--- a/drivers/iio/meter/Kconfig
+++ b/drivers/iio/meter/Kconfig
@@ -6,4 +6,17 @@ 
 
 menu "Active energy metering"
 
+config ADE9078
+	tristate "Analog Devices ADE9078 Poly Phase Multifunction Energy Metering IC Driver"
+	depends on SPI
+	select REGMAP_SPI
+	select IIO_BUFFER
+	select IIO_KFIFO_BUF
+	help
+	  Support driver for the ADE9078Poly Phase Multifunction Energy Meter,
+	  select y to build the driver.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called ade9078
+
 endmenu
diff --git a/drivers/iio/meter/Makefile b/drivers/iio/meter/Makefile
index c27cba44fc0b..866b7f7dc703 100644
--- a/drivers/iio/meter/Makefile
+++ b/drivers/iio/meter/Makefile
@@ -3,3 +3,5 @@ 
 # Makefile for enrgy metering drivers
 #
 # When adding new entries keep the list in alphabetical order
+
+obj-$(CONFIG_ADE9078) += ade9078.o
diff --git a/drivers/iio/meter/ade9078.c b/drivers/iio/meter/ade9078.c
new file mode 100644
index 000000000000..cfcb093e4c27
--- /dev/null
+++ b/drivers/iio/meter/ade9078.c
@@ -0,0 +1,1666 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADE9078 driver
+ *
+ * Copyright 2021 Analog Devices Inc.
+ */
+
+#include <asm/unaligned.h>
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/iio/events.h>
+#include <linux/iio/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+/* Address of ADE90XX registers */
+#define	ADE9078_REG_AIGAIN		0x000
+#define	ADE9078_REG_AVGAIN		0x00B
+#define	ADE9078_REG_AIRMSOS		0x00C
+#define	ADE9078_REG_AVRMSOS		0x00D
+#define	ADE9078_REG_APGAIN		0x00E
+#define	ADE9078_REG_AWATTOS		0x00F
+#define	ADE9078_REG_AVAROS		0x010
+#define	ADE9078_REG_AFVAROS		0x012
+#define	ADE9078_REG_CONFIG0		0x060
+#define	ADE9078_REG_DICOEFF		0x072
+#define	ADE9078_REG_AI_PCF		0x20A
+#define	ADE9078_REG_AV_PCF		0x20B
+#define	ADE9078_REG_AIRMS		0x20C
+#define	ADE9078_REG_AVRMS		0x20D
+#define	ADE9078_REG_AWATT		0x210
+#define	ADE9078_REG_AVAR		0x211
+#define	ADE9078_REG_AVA			0x212
+#define ADE9078_REG_AFVAR		0x214
+#define	ADE9078_REG_APF			0x216
+#define	ADE9078_REG_BI_PCF		0x22A
+#define	ADE9078_REG_BV_PCF		0x22B
+#define	ADE9078_REG_BIRMS		0x22C
+#define	ADE9078_REG_BVRMS		0x22D
+#define	ADE9078_REG_CI_PCF		0x24A
+#define	ADE9078_REG_CV_PCF		0x24B
+#define	ADE9078_REG_CIRMS		0x24C
+#define	ADE9078_REG_CVRMS		0x24D
+#define	ADE9078_REG_AWATT_ACC		0x2E5
+#define	ADE9078_REG_STATUS0		0x402
+#define	ADE9078_REG_STATUS1		0x403
+#define	ADE9078_REG_MASK0		0x405
+#define	ADE9078_REG_MASK1		0x406
+#define	ADE9078_REG_EVENT_MASK		0x407
+#define	ADE9078_REG_VLEVEL		0x40F
+#define	ADE9078_REG_RUN			0x480
+#define ADE9078_REG_CONFIG1		0x481
+#define	ADE9078_REG_ACCMODE		0x492
+#define	ADE9078_REG_CONFIG3		0x493
+#define	ADE9078_REG_ZX_LP_SEL		0x49A
+#define	ADE9078_REG_WFB_CFG		0x4A0
+#define	ADE9078_REG_WFB_PG_IRQEN	0x4A1
+#define	ADE9078_REG_WFB_TRG_CFG		0x4A2
+#define	ADE9078_REG_WFB_TRG_STAT	0x4A3
+#define	ADE9078_REG_CONFIG2		0x4AF
+#define	ADE9078_REG_EP_CFG		0x4B0
+#define	ADE9078_REG_EGY_TIME		0x4B2
+#define	ADE9078_REG_PGA_GAIN		0x4B9
+#define	ADE9078_REG_VERSION		0x4FE
+#define ADE9078_REG_WF_BUFF		0x800
+
+#define ADE9078_REG_ADDR_MASK		GENMASK(15, 4)
+#define ADE9078_REG_READ_BIT_MASK	BIT(3)
+#define ADE9078_RX_DEPTH		6
+#define ADE9078_TX_DEPTH		10
+
+#define ADE9078_WF_CAP_EN_MASK		BIT(4)
+#define ADE9078_WF_CAP_SEL_MASK		BIT(5)
+#define ADE9078_WF_MODE_MASK		GENMASK(7, 6)
+#define ADE9078_WF_SRC_MASK		GENMASK(9, 8)
+#define ADE9078_WF_IN_EN_MASK		BIT(12)
+
+/*
+ * Configuration registers
+ * PGA@0x0000. Gain of all channels=1
+ */
+#define ADE9078_PGA_GAIN		0x0000
+
+/* Default configuration */
+#define ADE9078_CONFIG0			0x00000000
+
+/* CF3/ZX pin outputs Zero crossing */
+#define ADE9078_CONFIG1			0x0002
+
+/* Default High pass corner frequency of 1.25Hz */
+#define ADE9078_CONFIG2			0x0A00
+
+/* Peak and overcurrent detection disabled */
+#define ADE9078_CONFIG3			0x0000
+
+/*
+ * 50Hz operation, 3P4W Wye configuration, signed accumulation
+ * Clear bit 8 i.e. ACCMODE=0x00xx for 50Hz operation
+ * ACCMODE=0x0x9x for 3Wire delta when phase B is used as reference
+ */
+#define ADE9078_ACCMODE			0x0000
+
+/*Line period and zero crossing obtained from VA */
+#define ADE9078_ZX_LP_SEL		0x0000
+
+/* Disable all interrupts */
+#define ADE9078_MASK0			0x00000000
+
+/* Disable all interrupts */
+#define ADE9078_MASK1			0x00000000
+
+/* Events disabled */
+#define ADE9078_EVENT_MASK		0x00000000
+
+/*
+ * Assuming Vnom=1/2 of full scale.
+ * Refer to Technical reference manual for detailed calculations.
+ */
+#define ADE9078_VLEVEL			0x0022EA28
+
+/* Set DICOEFF= 0xFFFFE000 when integrator is enabled */
+#define ADE9078_DICOEFF			0x00000000
+
+/* DSP ON */
+#define ADE9078_RUN_ON			0xFFFFFFFF
+
+/*
+ * Energy Accumulation Settings
+ * Enable energy accumulation, accumulate samples at 8ksps
+ * latch energy accumulation after EGYRDY
+ * If accumulation is changed to half line cycle mode, change EGY_TIME
+ */
+#define ADE9078_EP_CFG			0x0011
+
+/* Accumulate 4000 samples */
+#define ADE9078_EGY_TIME		0x0FA0
+
+/*
+ * Constant Definitions
+ * ADE9000 FDSP: 8000sps, ADE9078 FDSP: 4000sps
+ */
+#define ADE9078_FDSP			4000
+#define ADE9078_WFB_CFG			0x0329
+#define ADE9078_WFB_PAGE_SIZE		128
+#define ADE9078_WFB_BYTES_IN_PAGE	4
+#define ADE9078_WFB_PAGE_ARRAY_SIZE	\
+	(ADE9078_WFB_PAGE_SIZE * ADE9078_WFB_BYTES_IN_PAGE)
+#define ADE9078_WFB_FULL_BUFF_SIZE	\
+	(ADE9078_WFB_PAGE_ARRAY_SIZE * 16)
+#define ADE9078_WFB_FULL_BUFF_NR_SAMPLES \
+	(ADE9078_WFB_PAGE_SIZE * 16)
+
+#define ADE9078_SWRST_BIT		BIT(0)
+
+/* Status and Mask register bits*/
+#define ADE9078_ST0_WFB_TRIG_BIT	BIT(16)
+#define ADE9078_ST0_PAGE_FULL_BIT	BIT(17)
+
+#define ADE9078_ST1_ZXTOVA_BIT		BIT(6)
+#define ADE9078_ST1_ZXTOVB_BIT		BIT(7)
+#define ADE9078_ST1_ZXTOVC_BIT		BIT(8)
+#define ADE9078_ST1_ZXVA_BIT		BIT(9)
+#define ADE9078_ST1_ZXVB_BIT		BIT(10)
+#define ADE9078_ST1_ZXVC_BIT		BIT(11)
+#define ADE9078_ST1_ZXIA_BIT		BIT(13)
+#define ADE9078_ST1_ZXIB_BIT		BIT(14)
+#define ADE9078_ST1_ZXIC_BIT		BIT(15)
+#define ADE9078_ST1_RSTDONE_BIT		BIT(16)
+#define ADE9078_ST1_ERROR0_BIT		BIT(28)
+#define ADE9078_ST1_ERROR1_BIT		BIT(29)
+#define ADE9078_ST1_ERROR2_BIT		BIT(30)
+#define ADE9078_ST1_ERROR3_BIT		BIT(31)
+#define ADE9078_ST_ERROR \
+	(ADE9078_ST1_ERROR0 | \
+	 ADE9078_ST1_ERROR1 | \
+	 ADE9078_ST1_ERROR2 | \
+	 ADE9078_ST1_ERROR3)
+#define ADE9078_ST1_CROSSING_FIRST	6
+#define ADE9078_ST1_CROSSING_DEPTH	16
+
+#define ADE9078_WFB_TRG_ZXIA_BIT	BIT(3)
+#define ADE9078_WFB_TRG_ZXIB_BIT	BIT(4)
+#define ADE9078_WFB_TRG_ZXIC_BIT	BIT(5)
+#define ADE9078_WFB_TRG_ZXVA_BIT	BIT(6)
+#define ADE9078_WFB_TRG_ZXVB_BIT	BIT(7)
+#define ADE9078_WFB_TRG_ZXVC_BIT	BIT(8)
+
+/* Stop when waveform buffer is full */
+#define ADE9078_WFB_FULL_MODE		0x0
+/* Continuous fill—stop only on enabled trigger events */
+#define ADE9078_WFB_EN_TRIG_MODE	0x1
+/* Continuous filling—center capture around enabled trigger events */
+#define ADE9078_WFB_C_EN_TRIG_MODE	0x2
+/* Continuous fill—save event address of enabled trigger events */
+#define ADE9078_WFB_SAVE_EN_TRIG_MODE	0x3
+
+#define ADE9078_MODE_0_1_PAGE_BIT	BIT(15)
+#define ADE9078_MODE_2_PAGE_BIT		BIT(7)
+
+/*
+ * Full scale Codes referred from Datasheet.Respective digital codes are
+ * produced when ADC inputs are at full scale. Do not Change.
+ */
+#define ADE9078_RMS_FULL_SCALE_CODES	52866837
+#define ADE9000_WATT_FULL_SCALE_CODES	20694066
+#define ADE9078_PCF_FULL_SCALE_CODES	74770000
+
+/* Phase and channel definitions */
+#define ADE9078_PHASE_A_NR		0
+#define ADE9078_PHASE_B_NR		2
+#define ADE9078_PHASE_C_NR		4
+#define ADE9078_PHASE_A_NAME		"A"
+#define ADE9078_PHASE_B_NAME		"B"
+#define ADE9078_PHASE_C_NAME		"C"
+
+#define ADE9078_SCAN_POS_IA		BIT(0)
+#define ADE9078_SCAN_POS_VA		BIT(1)
+#define ADE9078_SCAN_POS_IB		BIT(2)
+#define ADE9078_SCAN_POS_VB		BIT(3)
+#define ADE9078_SCAN_POS_IC		BIT(4)
+#define ADE9078_SCAN_POS_VC		BIT(5)
+
+#define ADE9078_PHASE_B_POS_BIT		BIT(5)
+#define ADE9078_PHASE_C_POS_BIT		BIT(6)
+
+#define ADE9078_MAX_PHASE_NR		3
+#define AD9078_CHANNELS_PER_PHASE	9
+
+#define ADE9078_ADDR_ADJUST(addr, chan)					\
+	(((chan) << 4) | (addr))
+
+#define ADE9078_CURRENT_CHANNEL(num, name) {				\
+	.type = IIO_CURRENT,						\
+	.channel = num,							\
+	.extend_name = name,						\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AI_PCF, num),	\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
+	.event_spec = ade9078_events,					\
+	.num_event_specs = ARRAY_SIZE(ade9078_events),			\
+	.scan_index = num,						\
+	.scan_type = {							\
+		.sign = 's',						\
+		.realbits = 32,						\
+		.storagebits = 32,					\
+		.shift = 0,						\
+		.endianness = IIO_BE,					\
+	},								\
+}
+
+#define ADE9078_VOLTAGE_CHANNEL(num, name) {				\
+	.type = IIO_VOLTAGE,						\
+	.channel = num,							\
+	.extend_name = name,						\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AV_PCF, num),	\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
+	.event_spec = ade9078_events,					\
+	.num_event_specs = ARRAY_SIZE(ade9078_events),			\
+	.scan_index = num + 1,						\
+	.scan_type = {							\
+		.sign = 's',						\
+		.realbits = 32,						\
+		.storagebits = 32,					\
+		.shift = 0,						\
+		.endianness = IIO_BE,					\
+	},								\
+}
+
+#define ADE9078_CURRENT_RMS_CHANNEL(num, name) {			\
+	.type = IIO_CURRENT,						\
+	.channel = num,							\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AIRMS, num),		\
+	.extend_name = name "_rms",					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_OFFSET),		\
+	.scan_index = -1						\
+}
+
+#define ADE9078_VOLTAGE_RMS_CHANNEL(num, name) {			\
+	.type = IIO_VOLTAGE,						\
+	.channel = num,							\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AVRMS, num),		\
+	.extend_name = name "_rms",					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_OFFSET),		\
+	.scan_index = -1						\
+}
+
+#define ADE9078_POWER_ACTIV_CHANNEL(num, name) {			\
+	.type = IIO_POWER,						\
+	.channel = num,							\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AWATT, num),		\
+	.extend_name = name "_activ",					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_OFFSET) |		\
+			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
+	.scan_index = -1						\
+}
+
+#define ADE9078_POWER_REACTIV_CHANNEL(num, name) {			\
+	.type = IIO_POWER,						\
+	.channel = num,							\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AVAR, num),		\
+	.extend_name = name "_reactiv",					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_OFFSET),		\
+	.scan_index = -1						\
+}
+
+#define ADE9078_POWER_APPARENT_CHANNEL(num, name) {			\
+	.type = IIO_POWER,						\
+	.channel = num,							\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AVA, num),		\
+	.extend_name = name "_apparent",				\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE),			\
+	.scan_index = -1						\
+}
+
+#define ADE9078_POWER_FUND_REACTIV_CHANNEL(num, name) {			\
+	.type = IIO_POWER,						\
+	.channel = num,							\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_AFVAR, num),		\
+	.extend_name = name "_fund_reactiv",				\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_OFFSET),		\
+	.scan_index = -1						\
+}
+
+#define ADE9078_POWER_FACTOR_CHANNEL(num, name) {			\
+	.type = IIO_POWER,						\
+	.channel = num,							\
+	.address = ADE9078_ADDR_ADJUST(ADE9078_REG_APF, num),		\
+	.extend_name = name "_factor",					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE),			\
+	.scan_index = -1						\
+}
+
+/*
+ * struct ade9078_state - ade9078 specific data
+ * @rst_done	flag for when reset sequence irq has been received
+ * @wf_mode	wave form buffer mode, read datasheet for more details,
+ *		retrieved from DT
+ * @wfb_trg	wave form buffer triger configuration, read datasheet for more
+ *		details, retrieved from DT
+ * @spi		spi device associated to the ade9078
+ * @tx		transmit buffer for the spi
+ * @rx		receive buffer for the spi
+ * @xfer	transfer setup used in iio buffer configuration
+ * @spi_msg	message transfer trough spi, used in iio buffer
+ *		configuration
+ * @regmap	register map pointer
+ * @indio_dev:	the IIO device
+ * @trig	iio trigger pointer, is connected to IRQ0 and IRQ1
+ * @rx_buff	receive buffer for the iio buffer trough spi, will
+ *		contain the samples from the IC wave form buffer
+ * @tx_buff	transmit buffer for the iio buffer trough spi, used
+ *		in iio	buffer configuration
+ */
+
+struct ade9078_state {
+	bool rst_done;
+	u8 wf_mode;
+	u32 wfb_trg;
+	struct spi_device *spi;
+	u8 *tx;
+	u8 *rx;
+	struct spi_transfer xfer[2];
+	struct spi_message spi_msg;
+	struct regmap *regmap;
+	struct iio_dev *indio_dev;
+	union{
+		u8 byte[ADE9078_WFB_FULL_BUFF_SIZE];
+		__be32 word[ADE9078_WFB_FULL_BUFF_NR_SAMPLES];
+	} rx_buff ____cacheline_aligned;
+	u8 tx_buff[2];
+};
+
+static const struct iio_event_spec ade9078_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
+				 BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
+/* IIO channels of the ade9078 for each phase individually */
+static const struct iio_chan_spec ade9078_a_channels[] = {
+	ADE9078_CURRENT_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
+	ADE9078_VOLTAGE_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
+	ADE9078_CURRENT_RMS_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
+	ADE9078_VOLTAGE_RMS_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
+	ADE9078_POWER_ACTIV_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
+	ADE9078_POWER_REACTIV_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
+	ADE9078_POWER_APPARENT_CHANNEL(ADE9078_PHASE_A_NR,
+				       ADE9078_PHASE_A_NAME),
+	ADE9078_POWER_FUND_REACTIV_CHANNEL(ADE9078_PHASE_A_NR,
+					   ADE9078_PHASE_A_NAME),
+	ADE9078_POWER_FACTOR_CHANNEL(ADE9078_PHASE_A_NR, ADE9078_PHASE_A_NAME),
+};
+
+static const struct iio_chan_spec ade9078_b_channels[] = {
+	ADE9078_CURRENT_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
+	ADE9078_VOLTAGE_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
+	ADE9078_CURRENT_RMS_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
+	ADE9078_VOLTAGE_RMS_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
+	ADE9078_POWER_ACTIV_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
+	ADE9078_POWER_REACTIV_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
+	ADE9078_POWER_APPARENT_CHANNEL(ADE9078_PHASE_B_NR,
+				       ADE9078_PHASE_B_NAME),
+	ADE9078_POWER_FUND_REACTIV_CHANNEL(ADE9078_PHASE_B_NR,
+					   ADE9078_PHASE_B_NAME),
+	ADE9078_POWER_FACTOR_CHANNEL(ADE9078_PHASE_B_NR, ADE9078_PHASE_B_NAME),
+};
+
+static const struct iio_chan_spec ade9078_c_channels[] = {
+	ADE9078_CURRENT_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
+	ADE9078_VOLTAGE_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
+	ADE9078_CURRENT_RMS_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
+	ADE9078_VOLTAGE_RMS_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
+	ADE9078_POWER_ACTIV_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
+	ADE9078_POWER_REACTIV_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
+	ADE9078_POWER_APPARENT_CHANNEL(ADE9078_PHASE_C_NR,
+				       ADE9078_PHASE_C_NAME),
+	ADE9078_POWER_FUND_REACTIV_CHANNEL(ADE9078_PHASE_C_NR,
+					   ADE9078_PHASE_C_NAME),
+	ADE9078_POWER_FACTOR_CHANNEL(ADE9078_PHASE_C_NR, ADE9078_PHASE_C_NAME),
+};
+
+static const struct reg_sequence ade9078_reg_sequence[] = {
+	{ ADE9078_REG_PGA_GAIN, ADE9078_PGA_GAIN },
+	{ ADE9078_REG_CONFIG0, ADE9078_CONFIG0 },
+	{ ADE9078_REG_CONFIG1, ADE9078_CONFIG1 },
+	{ ADE9078_REG_CONFIG2, ADE9078_CONFIG2 },
+	{ ADE9078_REG_CONFIG3, ADE9078_CONFIG3 },
+	{ ADE9078_REG_ACCMODE, ADE9078_ACCMODE },
+	{ ADE9078_REG_ZX_LP_SEL, ADE9078_ZX_LP_SEL },
+	{ ADE9078_REG_MASK0, ADE9078_MASK0 },
+	{ ADE9078_REG_MASK1, ADE9078_MASK1 },
+	{ ADE9078_REG_EVENT_MASK, ADE9078_EVENT_MASK },
+	{ ADE9078_REG_WFB_CFG, ADE9078_WFB_CFG },
+	{ ADE9078_REG_VLEVEL, ADE9078_VLEVEL },
+	{ ADE9078_REG_DICOEFF, ADE9078_DICOEFF },
+	{ ADE9078_REG_EGY_TIME, ADE9078_EGY_TIME },
+	{ ADE9078_REG_EP_CFG, ADE9078_EP_CFG },
+	{ ADE9078_REG_RUN, ADE9078_RUN_ON }
+};
+
+/*
+ * ade9078_spi_write_reg() - ade9078 write register over SPI
+ * the data format for communicating with the ade9078 over SPI
+ * is very specific and can access both 32bit and 16bit registers
+ * @context:	void pointer to the SPI device
+ * @reg:	address of the of desired register
+ * @val:	value to be written to the ade9078
+ */
+static int ade9078_spi_write_reg(void *context,
+				 unsigned int reg,
+				 unsigned int val)
+{
+	struct device *dev = context;
+	struct spi_device *spi = to_spi_device(dev);
+	struct ade9078_state *st = spi_get_drvdata(spi);
+
+	u16 addr;
+	int ret = 0;
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = st->tx,
+		},
+	};
+
+	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg);
+
+	put_unaligned_be16(addr, st->tx);
+	put_unaligned_be32(val, &st->tx[2]);
+
+	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION) {
+		put_unaligned_be16(val, &st->tx[2]);
+		xfer[0].len = 4;
+	} else {
+		xfer[0].len = 6;
+	}
+
+	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
+	if (ret) {
+		dev_err(&st->spi->dev, "problem when writing register 0x%x",
+			reg);
+	}
+
+	return ret;
+}
+
+/*
+ * ade9078_spi_write_reg() - ade9078 read register over SPI
+ * the data format for communicating with the ade9078 over SPI
+ * is very specific and can access both 32bit and 16bit registers
+ * @context:	void pointer to the SPI device
+ * @reg:	address of the of desired register
+ * @val:	value to be read to the ade9078
+ */
+static int ade9078_spi_read_reg(void *context,
+				unsigned int reg,
+				unsigned int *val)
+{
+	struct device *dev = context;
+	struct spi_device *spi = to_spi_device(dev);
+	struct ade9078_state *st = spi_get_drvdata(spi);
+
+	u16 addr;
+	int ret = 0;
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = st->tx,
+			.len = 2,
+		},
+		{
+			.rx_buf = st->rx,
+		},
+	};
+
+	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, reg) |
+	       ADE9078_REG_READ_BIT_MASK;
+
+	put_unaligned_be16(addr, st->tx);
+
+	if (reg > ADE9078_REG_RUN && reg < ADE9078_REG_VERSION)
+		xfer[1].len = 4;
+	else
+		xfer[1].len = 6;
+
+	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
+	if (ret) {
+		dev_err(&st->spi->dev, "problem when reading register 0x%x",
+			reg);
+		goto err_ret;
+	}
+
+	//registers which are 16 bits
+	if (reg > 0x480 && reg < 0x4FE)
+		*val = get_unaligned_be16(st->rx);
+	else
+		*val = get_unaligned_be32(st->rx);
+
+err_ret:
+	return ret;
+}
+
+/*
+ * ade9078_is_volatile_reg() - list of ade9078 registers which should use
+ * caching
+ * @dev:	device data
+ * @reg:	address of the of desired register
+ */
+static bool ade9078_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case ADE9078_REG_STATUS0:
+	case ADE9078_REG_STATUS1:
+	case ADE9078_REG_MASK0:
+	case ADE9078_REG_MASK1:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/*
+ * ade9078_en_wfb() - enables or disables the WFBuffer in the ADE9078
+ * @st:		ade9078 device data
+ * @state:	true for enabled; false for disabled
+ */
+static int ade9078_en_wfb(struct ade9078_state *st, bool state)
+{
+	return regmap_update_bits(st->regmap, ADE9078_REG_WFB_CFG, BIT(4),
+				  state ? BIT(4) : 0);
+}
+
+/*
+ * ade9078_iio_push_buffer() - reads out the content of the waveform buffer and
+ * pushes it to the IIO buffer.
+ * @st:		ade9078 device data
+ */
+static int ade9078_iio_push_buffer(struct ade9078_state *st)
+{
+	int ret;
+	u32 i;
+
+	ret = spi_sync(st->spi, &st->spi_msg);
+	if (ret) {
+		dev_err(&st->spi->dev, "SPI fail in trigger handler");
+		return ret;
+	}
+
+	for (i = 0; i < ADE9078_WFB_FULL_BUFF_NR_SAMPLES; i++)
+		iio_push_to_buffers(st->indio_dev, &st->rx_buff.word[i]);
+
+	return 0;
+}
+
+/*
+ * ade9078_irq0_thread() - Thread for IRQ0. It reads Status register 0 and
+ * checks for the IRQ activation. This is configured to acquire samples in to
+ * the IC buffer and dump it in to the iio_buffer according to Stop When Buffer
+ * Is Full Mode, Stop Filling on Trigger and Capture Around Trigger from the
+ * ADE9078 Datasheet
+ */
+static irqreturn_t ade9078_irq0_thread(int irq, void *data)
+{
+	struct ade9078_state *st = data;
+	u32 handled_irq = 0;
+	u32 interrupts;
+	u32 status;
+	int ret;
+
+	ret = regmap_read(st->regmap, ADE9078_REG_STATUS0, &status);
+	if (ret) {
+		dev_err(&st->spi->dev, "IRQ0 read status fail");
+		goto irq0_done;
+	}
+
+	ret = regmap_read(st->regmap, ADE9078_REG_MASK0, &interrupts);
+	if (ret) {
+		dev_err(&st->spi->dev, "IRQ0 read status fail");
+		goto irq0_done;
+	}
+
+	if ((status & ADE9078_ST0_PAGE_FULL_BIT) &&
+	    (interrupts & ADE9078_ST0_PAGE_FULL_BIT)) {
+		//Stop Filling on Trigger and Center Capture Around Trigger
+		if (st->wf_mode) {
+			ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG,
+					   st->wfb_trg);
+			if (ret) {
+				dev_err(&st->spi->dev, "IRQ0 WFB write fail");
+				goto irq0_done;
+			}
+
+			interrupts |= ADE9078_ST0_WFB_TRIG_BIT;
+
+		} else {
+			//Stop When Buffer Is Full Mode
+			ret = ade9078_en_wfb(st, false);
+			if (ret) {
+				dev_err(&st->spi->dev, "IRQ0 WFB stop fail");
+				goto irq0_done;
+			}
+			ret = ade9078_iio_push_buffer(st);
+			if (ret) {
+				dev_err(&st->spi->dev, "IRQ0 IIO push fail");
+				goto irq0_done;
+			}
+		}
+
+		//disable Page full interrupt
+		interrupts &= ~ADE9078_ST0_PAGE_FULL_BIT;
+
+		ret = regmap_write(st->regmap, ADE9078_REG_MASK0, interrupts);
+		if (ret) {
+			dev_err(&st->spi->dev, "IRQ0 MAKS0 write fail");
+			goto irq0_done;
+		}
+
+		handled_irq |= ADE9078_ST0_PAGE_FULL_BIT;
+	}
+
+	if ((status & ADE9078_ST0_WFB_TRIG_BIT) &&
+	    (interrupts & ADE9078_ST0_WFB_TRIG_BIT)) {
+		//Stop Filling on Trigger and Center Capture Around Trigger
+		ret = ade9078_en_wfb(st, false);
+		if (ret) {
+			dev_err(&st->spi->dev, "IRQ0 WFB fail");
+			goto irq0_done;
+		}
+
+		ret = ade9078_iio_push_buffer(st);
+		if (ret) {
+			dev_err(&st->spi->dev, "IRQ0 IIO push fail @ WFB TRIG");
+			goto irq0_done;
+		}
+
+		handled_irq |= ADE9078_ST0_WFB_TRIG_BIT;
+	}
+
+	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, handled_irq);
+	if (ret)
+		dev_err(&st->spi->dev, "IRQ0 write status fail");
+
+irq0_done:
+	return IRQ_HANDLED;
+}
+
+/*
+ * ade9078_irq1_thread() - Thread for IRQ1. It reads Status register 1 and
+ * checks for the IRQ activation. This thread handles the reset condition and
+ * the zero-crossing conditions for all 3 phases on Voltage and Current
+ */
+static irqreturn_t ade9078_irq1_thread(int irq, void *data)
+{
+	struct ade9078_state *st = data;
+	struct iio_dev *indio_dev = st->indio_dev;
+	struct iio_chan_spec const *chan = indio_dev->channels;
+	unsigned int bit = ADE9078_ST1_CROSSING_FIRST;
+	s64 timestamp = iio_get_time_ns(indio_dev);
+	u32 interrupts;
+	u32 result;
+	u32 status;
+	u32 tmp;
+	int ret;
+
+	//reset
+	if (!st->rst_done) {
+		ret = regmap_read(st->regmap, ADE9078_REG_STATUS1, &result);
+		if (ret)
+			return ret;
+
+		if (result & ADE9078_ST1_RSTDONE_BIT)
+			st->rst_done = true;
+		else
+			dev_err(&st->spi->dev, "Error testing reset done");
+
+		goto irq1_done;
+	}
+
+	ret = regmap_read(st->regmap, ADE9078_REG_STATUS1, &status);
+	if (ret)
+		goto irq1_done;
+
+	ret = regmap_read(st->regmap, ADE9078_REG_MASK1, &interrupts);
+	if (ret) {
+		dev_err(&st->spi->dev, "IRQ1 read status fail");
+		goto irq1_done;
+	}
+
+	//crossings
+	for_each_set_bit_from(bit, (unsigned long *)&interrupts,
+			      ADE9078_ST1_CROSSING_DEPTH) {
+		tmp = status & BIT(bit);
+
+		switch (tmp) {
+		case ADE9078_ST1_ZXVA_BIT:
+		case ADE9078_ST1_ZXTOVA_BIT:
+		case ADE9078_ST1_ZXIA_BIT:
+			iio_push_event(indio_dev,
+				       IIO_UNMOD_EVENT_CODE(chan->type,
+							    ADE9078_PHASE_A_NR,
+							    IIO_EV_TYPE_THRESH,
+							    IIO_EV_DIR_EITHER),
+				       timestamp);
+			break;
+		case ADE9078_ST1_ZXVB_BIT:
+		case ADE9078_ST1_ZXTOVB_BIT:
+		case ADE9078_ST1_ZXIB_BIT:
+			iio_push_event(indio_dev,
+				       IIO_UNMOD_EVENT_CODE(chan->type,
+							    ADE9078_PHASE_B_NR,
+							    IIO_EV_TYPE_THRESH,
+							    IIO_EV_DIR_EITHER),
+				       timestamp);
+			break;
+		case ADE9078_ST1_ZXVC_BIT:
+		case ADE9078_ST1_ZXTOVC_BIT:
+		case ADE9078_ST1_ZXIC_BIT:
+			iio_push_event(indio_dev,
+				       IIO_UNMOD_EVENT_CODE(chan->type,
+							    ADE9078_PHASE_C_NR,
+							    IIO_EV_TYPE_THRESH,
+							    IIO_EV_DIR_EITHER),
+				       timestamp);
+			break;
+		default:
+			goto irq1_done;
+		}
+	}
+
+irq1_done:
+	return IRQ_HANDLED;
+}
+
+/*
+ * ade9078_configure_scan() - sets up the transfer parameters
+ * as well as the tx and rx buffers
+ * @indio_dev:	the IIO device
+ */
+static int ade9078_configure_scan(struct iio_dev *indio_dev)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	u16 addr;
+
+	addr = FIELD_PREP(ADE9078_REG_ADDR_MASK, ADE9078_REG_WF_BUFF) |
+	       ADE9078_REG_READ_BIT_MASK;
+
+	put_unaligned_be16(addr, st->tx_buff);
+
+	st->xfer[0].tx_buf = &st->tx_buff[0];
+	st->xfer[0].len = 2;
+
+	st->xfer[1].rx_buf = &st->rx_buff.byte[0];
+	st->xfer[1].len = ADE9078_WFB_FULL_BUFF_SIZE;
+
+	spi_message_init_with_transfers(&st->spi_msg, st->xfer, 2);
+	return 0;
+}
+
+/*
+ * ade9078_read_raw() - IIO read function
+ * @indio_dev:	the IIO device
+ * @chan:	channel specs of the ade9078
+ * @val:	first half of the read value
+ * @val2:	second half of the read value
+ * @mask:	info mask of the channel
+ */
+static int ade9078_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val,
+			    int *val2,
+			    long mask)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	int measured;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+
+		ret = regmap_read(st->regmap, chan->address, &measured);
+
+		iio_device_release_direct_mode(indio_dev);
+		*val = measured;
+
+		return ret ?: IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_CURRENT || chan->type == IIO_VOLTAGE) {
+			switch (chan->address) {
+			case ADE9078_REG_AI_PCF:
+			case ADE9078_REG_AV_PCF:
+			case ADE9078_REG_BI_PCF:
+			case ADE9078_REG_BV_PCF:
+			case ADE9078_REG_CI_PCF:
+			case ADE9078_REG_CV_PCF:
+				*val = 1;
+				*val2 = ADE9078_PCF_FULL_SCALE_CODES;
+				return IIO_VAL_FRACTIONAL;
+			case ADE9078_REG_AIRMS:
+			case ADE9078_REG_AVRMS:
+			case ADE9078_REG_BIRMS:
+			case ADE9078_REG_BVRMS:
+			case ADE9078_REG_CIRMS:
+			case ADE9078_REG_CVRMS:
+				*val = 1;
+				*val2 = ADE9078_RMS_FULL_SCALE_CODES;
+				return IIO_VAL_FRACTIONAL;
+			default:
+				return -EINVAL;
+			}
+		} else if (chan->type == IIO_POWER) {
+			*val = 1;
+			*val2 = ADE9000_WATT_FULL_SCALE_CODES;
+			return IIO_VAL_FRACTIONAL;
+		} else {
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * ade9078_write_raw() - IIO write function
+ * @indio_dev:	the IIO device
+ * @chan:	channel specs of the ade9078
+ * @val:	first half of the read value
+ * @val2:	second half of the read value
+ * @mask:	info mask of the channel
+ */
+static int ade9078_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val,
+			     int val2,
+			     long mask)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	u32 addr = 0xFFFFF;
+	u32 tmp;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_OFFSET:
+		switch (chan->type) {
+		case IIO_CURRENT:
+			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AIRMSOS,
+						   chan->channel);
+			break;
+		case IIO_VOLTAGE:
+			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AVRMSOS,
+						   chan->channel);
+			break;
+		case IIO_POWER:
+			tmp = chan->address;
+			tmp &= ~ADE9078_PHASE_B_POS_BIT;
+			tmp &= ~ADE9078_PHASE_C_POS_BIT;
+
+			switch (tmp) {
+			case ADE9078_REG_AWATTOS:
+				addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AWATTOS,
+							   chan->channel);
+				break;
+			case ADE9078_REG_AVAR:
+				addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AVAROS,
+							   chan->channel);
+				break;
+			case ADE9078_REG_AFVAR:
+				addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AFVAROS,
+							   chan->channel);
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_HARDWAREGAIN:
+		switch (chan->type) {
+		case IIO_CURRENT:
+			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AIGAIN,
+						   chan->channel);
+			break;
+		case IIO_VOLTAGE:
+			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_AVGAIN,
+						   chan->channel);
+			break;
+		case IIO_POWER:
+			addr = ADE9078_ADDR_ADJUST(ADE9078_REG_APGAIN,
+						   chan->channel);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_write(st->regmap, addr, val);
+	return ret;
+}
+
+/*
+ * ade9078_reg_access() - IIO debug register access
+ * @indio_dev:	the IIO device
+ * @reg:	register to be accessed
+ * @tx_val:	value to be transmitted
+ * @rx_val:	value to be received
+ */
+static int ade9078_reg_access(struct iio_dev *indio_dev,
+			      unsigned int reg,
+			      unsigned int tx_val,
+			      unsigned int *rx_val)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+
+	if (rx_val)
+		return regmap_read(st->regmap, reg, rx_val);
+
+	return regmap_write(st->regmap, reg, tx_val);
+}
+
+/*
+ * ade9078_write_event_config() - IIO event configure to enable zero-crossing
+ * and zero-crossing timeout on voltage and current for each phases. These
+ * events will also influence the trigger conditions for the buffer capture.
+ */
+static int ade9078_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      int state)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	u32 interrupts;
+	u32 number;
+	int ret;
+
+	number = chan->channel;
+
+	switch (number) {
+	case ADE9078_PHASE_A_NR:
+		if (chan->type == IIO_VOLTAGE) {
+			if (state) {
+				interrupts |= ADE9078_ST1_ZXVA_BIT;
+				interrupts |= ADE9078_ST1_ZXTOVA_BIT;
+				st->wfb_trg |= ADE9078_WFB_TRG_ZXVA_BIT;
+			} else {
+				interrupts &= ~ADE9078_ST1_ZXVA_BIT;
+				interrupts &= ~ADE9078_ST1_ZXTOVA_BIT;
+				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXVA_BIT;
+			}
+		} else if (chan->type == IIO_CURRENT) {
+			if (state) {
+				interrupts |= ADE9078_ST1_ZXIA_BIT;
+				st->wfb_trg |= ADE9078_WFB_TRG_ZXIA_BIT;
+			} else {
+				interrupts &= ~ADE9078_ST1_ZXIA_BIT;
+				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXIA_BIT;
+			}
+		}
+		break;
+	case ADE9078_PHASE_B_NR:
+		if (chan->type == IIO_VOLTAGE) {
+			if (state) {
+				interrupts |= ADE9078_ST1_ZXVB_BIT;
+				interrupts |= ADE9078_ST1_ZXTOVB_BIT;
+				st->wfb_trg |= ADE9078_WFB_TRG_ZXVB_BIT;
+			} else {
+				interrupts &= ~ADE9078_ST1_ZXVB_BIT;
+				interrupts &= ~ADE9078_ST1_ZXTOVB_BIT;
+				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXVB_BIT;
+			}
+		} else if (chan->type == IIO_CURRENT) {
+			if (state) {
+				interrupts |= ADE9078_ST1_ZXIB_BIT;
+				st->wfb_trg |= ADE9078_WFB_TRG_ZXIB_BIT;
+			} else {
+				interrupts &= ~ADE9078_ST1_ZXIB_BIT;
+				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXIB_BIT;
+			}
+		}
+		break;
+	case ADE9078_PHASE_C_NR:
+		if (chan->type == IIO_VOLTAGE) {
+			if (state) {
+				interrupts |= ADE9078_ST1_ZXVC_BIT;
+				interrupts |= ADE9078_ST1_ZXTOVC_BIT;
+				st->wfb_trg |= ADE9078_WFB_TRG_ZXVC_BIT;
+			} else {
+				interrupts &= ~ADE9078_ST1_ZXVC_BIT;
+				interrupts &= ~ADE9078_ST1_ZXTOVC_BIT;
+				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXVC_BIT;
+			}
+		} else if (chan->type == IIO_CURRENT) {
+			if (state) {
+				interrupts |= ADE9078_ST1_ZXIC_BIT;
+				st->wfb_trg |= ADE9078_WFB_TRG_ZXIC_BIT;
+			} else {
+				interrupts &= ~ADE9078_ST1_ZXIC_BIT;
+				st->wfb_trg &= ~ADE9078_WFB_TRG_ZXIC_BIT;
+			}
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1, GENMASK(31, 0));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(st->regmap, ADE9078_REG_MASK1, interrupts,
+				  interrupts);
+}
+
+/*
+ * ade9078_read_event_vlaue() - Outputs the result of the zero-crossing for
+ * voltage and current for each phase.
+ * Result:
+ * 0 - if crossing event not set
+ * 1 - if crossing event occurred
+ * -1 - if crossing timeout (only for Voltages)
+ */
+static int ade9078_read_event_vlaue(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	u32 handeled_irq = 0;
+	u32 interrupts;
+	u32 number;
+	u32 status;
+	int ret;
+
+	ret = regmap_read(st->regmap, ADE9078_REG_STATUS1, &status);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(st->regmap, ADE9078_REG_MASK1, &interrupts);
+	if (ret)
+		return ret;
+
+	*val = 0;
+	number = chan->channel;
+	switch (number) {
+	case ADE9078_PHASE_A_NR:
+		if (chan->type == IIO_VOLTAGE) {
+			if (status & ADE9078_ST1_ZXVA_BIT) {
+				*val = 1;
+				handeled_irq |= ADE9078_ST1_ZXVA_BIT;
+			} else if (status & ADE9078_ST1_ZXTOVA_BIT) {
+				*val = -1;
+				handeled_irq |= ADE9078_ST1_ZXTOVA_BIT;
+			}
+			if (!(interrupts & ADE9078_ST1_ZXTOVA_BIT))
+				*val = 0;
+		} else if (chan->type == IIO_CURRENT) {
+			if (status & ADE9078_ST1_ZXIA_BIT) {
+				*val = 1;
+				handeled_irq |= ADE9078_ST1_ZXIA_BIT;
+			}
+		}
+		break;
+	case ADE9078_PHASE_B_NR:
+		if (chan->type == IIO_VOLTAGE) {
+			if (status & ADE9078_ST1_ZXVB_BIT) {
+				*val = 1;
+				handeled_irq |= ADE9078_ST1_ZXVB_BIT;
+			} else if (status & ADE9078_ST1_ZXTOVB_BIT) {
+				*val = -1;
+				handeled_irq |= ADE9078_ST1_ZXTOVB_BIT;
+			}
+			if (!(interrupts & ADE9078_ST1_ZXTOVB_BIT))
+				*val = 0;
+		} else if (chan->type == IIO_CURRENT) {
+			if (status & ADE9078_ST1_ZXIB_BIT) {
+				*val = 1;
+				handeled_irq |= ADE9078_ST1_ZXIB_BIT;
+			}
+		}
+		break;
+	case ADE9078_PHASE_C_NR:
+		if (chan->type == IIO_VOLTAGE) {
+			if (status & ADE9078_ST1_ZXVC_BIT) {
+				*val = 1;
+				handeled_irq |= ADE9078_ST1_ZXVC_BIT;
+			} else if (status & ADE9078_ST1_ZXTOVC_BIT) {
+				*val = -1;
+				handeled_irq |= ADE9078_ST1_ZXTOVC_BIT;
+			}
+			if (!(interrupts & ADE9078_ST1_ZXTOVC_BIT))
+				*val = 0;
+		} else if (chan->type == IIO_CURRENT) {
+			if (status & ADE9078_ST1_ZXIC_BIT) {
+				*val = 1;
+				handeled_irq |= ADE9078_ST1_ZXIC_BIT;
+			}
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1, handeled_irq);
+	if (ret)
+		return ret;
+
+	return IIO_VAL_INT;
+}
+
+/*
+ * ade9078_config_wfb() - reads the ade9078 node and configures the wave form
+ * buffer based on the options set. Additionally is reads the active scan mask
+ * in order to set the input data of the buffer. There are only a few available
+ * input configurations permitted by the IC, any unpermitted configuration will
+ * result in all channels being active.
+ * @indio_dev:	the IIO device
+ */
+static int ade9078_config_wfb(struct iio_dev *indio_dev)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	u32 wfg_cfg_val = 0;
+	u32 tmp;
+	int ret;
+
+	bitmap_to_arr32(&wfg_cfg_val, indio_dev->active_scan_mask,
+			indio_dev->masklength);
+
+	switch (wfg_cfg_val) {
+	case ADE9078_SCAN_POS_IA | ADE9078_SCAN_POS_VA:
+		wfg_cfg_val = 0x1;
+		break;
+	case ADE9078_SCAN_POS_IB | ADE9078_SCAN_POS_VB:
+		wfg_cfg_val = 0x2;
+		break;
+	case ADE9078_SCAN_POS_IC | ADE9078_SCAN_POS_VC:
+		wfg_cfg_val = 0x3;
+		break;
+	case ADE9078_SCAN_POS_IA:
+		wfg_cfg_val = 0x8;
+		break;
+	case ADE9078_SCAN_POS_VA:
+		wfg_cfg_val = 0x9;
+		break;
+	case ADE9078_SCAN_POS_IB:
+		wfg_cfg_val = 0xA;
+		break;
+	case ADE9078_SCAN_POS_VB:
+		wfg_cfg_val = 0xB;
+		break;
+	case ADE9078_SCAN_POS_IC:
+		wfg_cfg_val = 0xC;
+		break;
+	case ADE9078_SCAN_POS_VC:
+		wfg_cfg_val = 0xD;
+		break;
+	default:
+		wfg_cfg_val = 0x0;
+		break;
+	}
+
+	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-cap-sel",
+				   &tmp);
+	if (ret) {
+		dev_err(&st->spi->dev, "Failed to get wf-cap-sel: %d\n", ret);
+		return ret;
+	}
+	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_CAP_SEL_MASK, tmp);
+
+	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-mode",
+				   &tmp);
+	if (ret) {
+		dev_err(&st->spi->dev, "Failed to get wf-mode: %d\n", ret);
+		return ret;
+	}
+	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_MODE_MASK, tmp);
+	st->wf_mode = tmp;
+
+	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-src",
+				   &tmp);
+	if (ret) {
+		dev_err(&st->spi->dev,
+			"Failed to get wf-src: %d\n",
+			ret);
+		return ret;
+	}
+	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_SRC_MASK, tmp);
+
+	ret = of_property_read_u32((&st->spi->dev)->of_node, "adi,wf-in-en",
+				   &tmp);
+	if (ret) {
+		dev_err(&st->spi->dev, "Failed to get wf-in-en: %d\n", ret);
+		return ret;
+	}
+	wfg_cfg_val |= FIELD_PREP(ADE9078_WF_IN_EN_MASK, tmp);
+
+	return regmap_write(st->regmap, ADE9078_REG_WFB_CFG, wfg_cfg_val);
+}
+
+/*
+ * ade9078_wfb_interrupt_setup() - Configures the wave form buffer interrupt
+ * according to modes
+ * @st:		ade9078 device data
+ * @mode:	modes according to datasheet; values [0-2]
+ *
+ * This sets the interrupt register and other registers related to the
+ * interrupts according to mode [0-2] from the datasheet
+ */
+static int ade9078_wfb_interrupt_setup(struct ade9078_state *st, u8 mode)
+{
+	int ret;
+
+	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
+	if (ret)
+		return ret;
+
+	if (mode == ADE9078_WFB_FULL_MODE || mode == ADE9078_WFB_EN_TRIG_MODE) {
+		ret = regmap_write(st->regmap, ADE9078_REG_WFB_PG_IRQEN,
+				   ADE9078_MODE_0_1_PAGE_BIT);
+		if (ret)
+			return ret;
+	} else if (mode == ADE9078_WFB_C_EN_TRIG_MODE) {
+		ret = regmap_write(st->regmap, ADE9078_REG_WFB_PG_IRQEN,
+				   ADE9078_MODE_2_PAGE_BIT);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, GENMASK(31, 0));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0,
+				  ADE9078_ST0_PAGE_FULL_BIT,
+				  ADE9078_ST0_PAGE_FULL_BIT);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/*
+ * ade9078_buffer_preenable() - configures the waveform buffer, sets the
+ * interrupts and enables the buffer
+ * @indio_dev:	the IIO device
+ */
+static int ade9078_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ade9078_config_wfb(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = ade9078_wfb_interrupt_setup(st, st->wf_mode);
+	if (ret)
+		return ret;
+
+	ret = ade9078_en_wfb(st, true);
+	if (ret) {
+		dev_err(&st->spi->dev, "Post-enable wfb enable fail");
+		return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * ade9078_buffer_postdisable() - after the iio is disable
+ * this will disable the ade9078 internal buffer for acquisition
+ * @indio_dev:	the IIO device
+ */
+static int ade9078_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ade9078_state *st = iio_priv(indio_dev);
+	u32 interrupts = 0;
+	int ret;
+
+	ret = ade9078_en_wfb(st, false);
+	if (ret) {
+		dev_err(&st->spi->dev, "Post-disable wfb disable fail");
+		return ret;
+	}
+
+	ret = regmap_write(st->regmap, ADE9078_REG_WFB_TRG_CFG, 0x0);
+	if (ret)
+		return ret;
+
+	interrupts |= ADE9078_ST0_WFB_TRIG_BIT;
+	interrupts |= ADE9078_ST0_PAGE_FULL_BIT;
+
+	return regmap_update_bits(st->regmap, ADE9078_REG_MASK0, interrupts, 0);
+	if (ret) {
+		dev_err(&st->spi->dev, "Post-disable update maks0 fail");
+		return ret;
+	}
+
+	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, GENMASK(31, 0));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/*
+ * ade9078_setup_iio_channels() - parses the phase nodes of the device-tree and
+ * creates the iio channels based on the active phases in the DT.
+ * @st:		ade9078 device data
+ */
+static int ade9078_setup_iio_channels(struct ade9078_state *st)
+{
+	struct fwnode_handle *phase_node = NULL;
+	struct device *dev = &st->spi->dev;
+	struct iio_chan_spec *chan;
+	u32 phase_nr;
+	int ret;
+
+	chan = devm_kcalloc(dev,
+			    ADE9078_MAX_PHASE_NR *
+			    ARRAY_SIZE(ade9078_a_channels),
+			    sizeof(*ade9078_a_channels), GFP_KERNEL);
+	if (!chan) {
+		dev_err(dev, "Unable to allocate ADE9078 channels");
+		return -ENOMEM;
+	}
+	st->indio_dev->num_channels = 0;
+	st->indio_dev->channels = chan;
+
+	fwnode_for_each_available_child_node(dev_fwnode(dev), phase_node) {
+		ret = fwnode_property_read_u32(phase_node, "reg", &phase_nr);
+		if (ret) {
+			dev_err(dev, "Could not read channel reg : %d\n", ret);
+			return ret;
+		}
+
+		switch (phase_nr) {
+		case ADE9078_PHASE_A_NR:
+			memcpy(chan, ade9078_a_channels,
+			       sizeof(ade9078_a_channels));
+			break;
+		case ADE9078_PHASE_B_NR:
+			memcpy(chan, ade9078_b_channels,
+			       sizeof(ade9078_b_channels));
+			break;
+		case ADE9078_PHASE_C_NR:
+			memcpy(chan, ade9078_c_channels,
+			       sizeof(ade9078_c_channels));
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		chan += AD9078_CHANNELS_PER_PHASE;
+		st->indio_dev->num_channels += AD9078_CHANNELS_PER_PHASE;
+	}
+
+	return 0;
+}
+
+/*
+ * ade9078_reset() - Reset sequence for the ADE9078, the hardware reset is
+ * optional in the DT. When no hardware reset has been declared a software
+ * reset is executed
+ * @st:		ade9078 device data
+ */
+static int ade9078_reset(struct ade9078_state *st)
+{
+	struct gpio_desc *gpio_reset;
+	int ret;
+
+	st->rst_done = false;
+
+	gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
+					     GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio_reset))
+		return PTR_ERR(gpio_reset);
+
+	if (gpio_reset) {
+		gpiod_set_value_cansleep(gpio_reset, 1);
+		usleep_range(1, 100);
+		gpiod_set_value_cansleep(gpio_reset, 0);
+		msleep_interruptible(50);
+	} else {
+		ret = regmap_update_bits(st->regmap, ADE9078_REG_CONFIG1,
+					 ADE9078_SWRST_BIT, ADE9078_SWRST_BIT);
+		if (ret)
+			return ret;
+		usleep_range(80, 100);
+	}
+
+	if (!st->rst_done)
+		return -EPERM;
+
+	return 0;
+}
+
+/*
+ * ade9078_setup() - initial register setup of the ade9078
+ * @st:		ade9078 device data
+ */
+static int ade9078_setup(struct ade9078_state *st)
+{
+	int ret = 0;
+
+	ret = regmap_multi_reg_write(st->regmap, ade9078_reg_sequence,
+				     ARRAY_SIZE(ade9078_reg_sequence));
+	if (ret)
+		return ret;
+
+	msleep_interruptible(2);
+
+	ret = regmap_write(st->regmap, ADE9078_REG_STATUS0, GENMASK(31, 0));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, ADE9078_REG_STATUS1, GENMASK(31, 0));
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+static const struct iio_buffer_setup_ops ade9078_buffer_ops = {
+	.preenable = &ade9078_buffer_preenable,
+	.postdisable = &ade9078_buffer_postdisable,
+};
+
+static const struct iio_info ade9078_info = {
+	.read_raw = &ade9078_read_raw,
+	.write_raw = &ade9078_write_raw,
+	.debugfs_reg_access = &ade9078_reg_access,
+	.write_event_config = &ade9078_write_event_config,
+	.read_event_value = &ade9078_read_event_vlaue,
+};
+
+/*
+ * Regmap configuration
+ * The register access of the ade9078 requires a 16 bit address
+ * with the read flag on bit 3. This is not supported by default
+ * regmap functionality, thus reg_read and reg_write have been
+ * replaced with custom functions
+ */
+static const struct regmap_config ade9078_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 32,
+	.zero_flag_mask = true,
+	.cache_type = REGCACHE_RBTREE,
+	.reg_read = ade9078_spi_read_reg,
+	.reg_write = ade9078_spi_write_reg,
+	.volatile_reg = ade9078_is_volatile_reg,
+};
+
+static int ade9078_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ade9078_state *st;
+	unsigned long irqflags;
+	struct regmap *regmap;
+	int irq;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev) {
+		dev_err(&spi->dev, "Unable to allocate ADE9078 IIO");
+		return -ENOMEM;
+	}
+	st = iio_priv(indio_dev);
+	if (!st) {
+		dev_err(&spi->dev,
+			"Unable to allocate ADE9078 device structure");
+		return -ENOMEM;
+	}
+
+	st->rx = devm_kcalloc(&spi->dev, ADE9078_RX_DEPTH, sizeof(*st->rx),
+			      GFP_KERNEL);
+	if (!st->rx)
+		return -ENOMEM;
+
+	st->tx = devm_kcalloc(&spi->dev, ADE9078_TX_DEPTH, sizeof(*st->tx),
+			      GFP_KERNEL);
+	if (!st->tx)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init(&spi->dev, NULL, spi, &ade9078_regmap_config);
+	if (IS_ERR(regmap))	{
+		dev_err(&spi->dev, "Unable to allocate ADE9078 regmap");
+		return PTR_ERR(regmap);
+	}
+	spi_set_drvdata(spi, st);
+
+	irq = of_irq_get_byname((&spi->dev)->of_node, "irq0");
+	if (irq < 0) {
+		dev_err(&spi->dev, "Unable to find irq0");
+		return -EINVAL;
+	}
+	irqflags = irq_get_trigger_type(irq);
+	ret = devm_request_threaded_irq(&spi->dev, irq, NULL,
+					ade9078_irq0_thread,
+					irqflags | IRQF_ONESHOT,
+					KBUILD_MODNAME, st);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to request threaded irq: %d\n", ret);
+		return ret;
+	}
+
+	irq = of_irq_get_byname((&spi->dev)->of_node, "irq1");
+	if (irq < 0) {
+		dev_err(&spi->dev, "Unable to find irq1");
+		return -EINVAL;
+	}
+	irqflags = irq_get_trigger_type(irq);
+	ret = devm_request_threaded_irq(&spi->dev, irq, NULL,
+					ade9078_irq1_thread,
+					irqflags | IRQF_ONESHOT,
+					KBUILD_MODNAME, st);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to request threaded irq: %d\n", ret);
+		return ret;
+	}
+
+	st->spi = spi;
+
+	indio_dev->dev.parent = &st->spi->dev;
+	indio_dev->info = &ade9078_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	st->regmap = regmap;
+	st->indio_dev = indio_dev;
+	ade9078_setup_iio_channels(st);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to set up IIO channels");
+		return ret;
+	}
+
+	ret = ade9078_configure_scan(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_kfifo_buffer_setup(&spi->dev, indio_dev,
+					  INDIO_BUFFER_SOFTWARE,
+					  &ade9078_buffer_ops);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
+	if (ret) {
+		dev_err(&spi->dev, "Unable to register IIO device");
+		return ret;
+	}
+
+	ret = ade9078_reset(st);
+	if (ret) {
+		dev_err(&spi->dev, "ADE9078 reset failed");
+		return ret;
+	}
+
+	ret = ade9078_setup(st);
+	if (ret) {
+		dev_err(&spi->dev, "Unable to setup ADE9078");
+		return ret;
+	}
+
+	return ret;
+};
+
+static const struct spi_device_id ade9078_id[] = {
+		{"ade9078", 0},
+		{}
+};
+MODULE_DEVICE_TABLE(spi, ade9078_id);
+
+static const struct of_device_id ade9078_of_match[] = {
+	{ .compatible = "adi,ade9078" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ade9078_of_match);
+
+static struct spi_driver ade9078_driver = {
+		.driver = {
+			.name = "ade9078",
+		},
+		.probe = ade9078_probe,
+		.id_table = ade9078_id,
+};
+module_spi_driver(ade9078_driver);
+
+MODULE_AUTHOR("Ciprian Hegbeli <[e-mail]>");
+MODULE_DESCRIPTION("Analog Devices ADE9078 High Performance,Polyphase Energy Metering IC Driver");
+MODULE_LICENSE("GPL v2");