[PATCH v4 00/15] Add support for AD7091R-2/-4/-8

[PATCH v4 00/15] Add support for AD7091R-2/-4/-8

[Marcelo Schmitt]

From: Marcelo Schmitt <marcelo.schmitt1@gmail.com>

----------------- Updates -----------------

Applied changes suggested to the previous set.

Change log v3 -> v4:
- Patch 1: checkpatch patch
  * Changed __aligned regex string.
- Patch 2: alert handling fix
  * Applied David's suggestion [1] to pass iio_dev on to IRQ thread handler.
- Patches 6, 7, 9
  * Removed ad7091r prefix from callback function names.
- New Patch 7: Remove uneeded probe parameters
  * Removed id->name and regmap from probe paramenters.
- Patch 8 (now Patch 9): Enable internal vref
  * Not expecting NULL return from regulator_get_optional() anymore;
  * Reverted to previous probe defer handling.
- Patch 10 (now Patch 11): dt doc
  * Extending existing ad7091r5 dt doc instead of creating a new one;
  * Added VDD and VDRIVE supplies to dt doc;
  * Removed channel property from dt doc;
  * Interrupt description, interrupt constraint check, example indentation improvements.
- Patch 12 (now Patch 13): add ad7091r8 patch
  * Neats to macros, gpio setups, and probe parameters.
- Patch 13 (now Patch 14):
  * Made use of wild cards in MAINTAINERS file.
- New Patch (Patch 15): event configuration callbacks

[1]: https://lore.kernel.org/linux-iio/CAMknhBHCYicEL_xhumBQMUm=HBVb=7dLrYsK8Zj2o7RodvMarw@mail.gmail.com/

Thank you all for the help with this set,
Marcelo

----------------- Context -----------------

This series adds support for AD7091R-2/-4/-8 ADCs which can do single shot
or sequenced readings. Threshold events are also supported.
Overall, AD7091R-2/-4/-8 are very similar to AD7091R-5 except they use SPI interface.

Changes have been tested with raspberrypi and eval board on raspberrypi kernel
6.7-rc3 from raspberrypi fork.
Link: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad7091r8

Marcelo Schmitt (15):
  scripts: checkpatch: Add __aligned to the list of attribute notes
  iio: adc: ad7091r: Pass iio_dev to event handler
  iio: adc: ad7091r: Set alert bit in config register
  iio: adc: ad7091r: Align arguments to function call parenthesis
  iio: adc: ad7091r: Move generic AD7091R code to base driver and header
    file
  iio: adc: ad7091r: Move chip init data to container struct
  iio: adc: ad7091r: Remove unneeded probe parameters
  iio: adc: ad7091r: Set device mode through chip_info callback
  iio: adc: ad7091r: Enable internal vref if external vref is not
    supplied
  iio: adc: ad7091r: Add chip_info callback to get conversion result
    channel
  iio: adc: Split AD7091R-5 config symbol
  dt-bindings: iio: Add AD7091R-8
  iio: adc: Add support for AD7091R-8
  MAINTAINERS: Add MAINTAINERS entry for AD7091R
  iio: adc: ad7091r: Allow users to configure device events

 .../bindings/iio/adc/adi,ad7091r5.yaml        |  82 +++++-
 MAINTAINERS                                   |   8 +
 drivers/iio/adc/Kconfig                       |  16 ++
 drivers/iio/adc/Makefile                      |   4 +-
 drivers/iio/adc/ad7091r-base.c                | 255 +++++++++++------
 drivers/iio/adc/ad7091r-base.h                |  81 +++++-
 drivers/iio/adc/ad7091r5.c                    | 120 ++++----
 drivers/iio/adc/ad7091r8.c                    | 257 ++++++++++++++++++
 scripts/checkpatch.pl                         |   1 +
 9 files changed, 682 insertions(+), 142 deletions(-)
 create mode 100644 drivers/iio/adc/ad7091r8.c

-- 
2.42.0

[PATCH v4 01/15] scripts: checkpatch: Add __aligned to the list of attribute notes

[Marcelo Schmitt]

Checkpatch presumes attributes marked with __aligned(alignment) are part
of a function declaration and throws a warning stating that those
compiler attributes should have an identifier name which is not correct.
Add __aligned compiler attributes to the list of attribute notes
so they don't cause warnings anymore.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Any expression that evaluates to an integer that is a power of 2 can be
within __aligned parenthesis.
I can't see how we could use a regex to check code meets such constraint (if possible).

Some additional exotic uses of __aligned are:
drivers/net/wireless/quantenna/qtnfmac/bus.h:72:   char bus_priv[] __aligned(sizeof(void *));
include/linux/netdevice.h:225:} __aligned(4 * sizeof(unsigned long)); 

The regex
			__aligned\s*\(.*\)
seems to match all use cases. 

We might not catch invalid arguments to __aligned, but it looks like making
checkpath confidently report those wouldn't be feasible anyway.

The patch that would trigger the mentioned warning in now
patch number 13 (iio: adc: Add support for AD7091R-8).

 scripts/checkpatch.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 25fdb7fda112..d56c98146da3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -512,6 +512,7 @@ our $Attribute	= qr{
 			__ro_after_init|
 			__kprobes|
 			$InitAttribute|
+			__aligned\s*\(.*\)|
 			____cacheline_aligned|
 			____cacheline_aligned_in_smp|
 			____cacheline_internodealigned_in_smp|
-- 
2.42.0

Re: [PATCH v4 01/15] scripts: checkpatch: Add __aligned to the list of attribute notes

[Joe Perches]

On Sat, 2023-12-16 at 14:45 -0300, Marcelo Schmitt wrote:
> Checkpatch presumes attributes marked with __aligned(alignment) are part
> of a function declaration and throws a warning stating that those
> compiler attributes should have an identifier name which is not correct.
> Add __aligned compiler attributes to the list of attribute notes
> so they don't cause warnings anymore.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Any expression that evaluates to an integer that is a power of 2 can be
> within __aligned parenthesis.
> I can't see how we could use a regex to check code meets such constraint (if possible).

You can't because if a #define is used for the alignment
value it is not necessarily available to a patch fragment
or even a file if the #define is in an #include.

> Some additional exotic uses of __aligned are:
> drivers/net/wireless/quantenna/qtnfmac/bus.h:72:   char bus_priv[] __aligned(sizeof(void *));
> include/linux/netdevice.h:225:} __aligned(4 * sizeof(unsigned long)); 

Right, then there are random uses like:

drivers/firmware/qcom/qcom_qseecom_uefisecapp.c:		size_t __aligned;						\
drivers/firmware/qcom/qcom_qseecom_uefisecapp.c:			*__offset = __aligned;					\

so there's always some false positive/negative issue
with checkpatch.

Anyway:

Acked-by: Joe Perches <joe@perches.com>

> 
> The regex
> 			__aligned\s*\(.*\)
> seems to match all use cases. 
> 
> We might not catch invalid arguments to __aligned, but it looks like making
> checkpath confidently report those wouldn't be feasible anyway.
> 
> The patch that would trigger the mentioned warning in now
> patch number 13 (iio: adc: Add support for AD7091R-8).
> 
>  scripts/checkpatch.pl | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 25fdb7fda112..d56c98146da3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -512,6 +512,7 @@ our $Attribute	= qr{
>  			__ro_after_init|
>  			__kprobes|
>  			$InitAttribute|
> +			__aligned\s*\(.*\)|
>  			____cacheline_aligned|
>  			____cacheline_aligned_in_smp|
>  			____cacheline_internodealigned_in_smp|

Re: [PATCH v4 01/15] scripts: checkpatch: Add __aligned to the list of attribute notes

[Jonathan Cameron]

On Sat, 16 Dec 2023 10:07:24 -0800
Joe Perches <joe@perches.com> wrote:

> On Sat, 2023-12-16 at 14:45 -0300, Marcelo Schmitt wrote:
> > Checkpatch presumes attributes marked with __aligned(alignment) are part
> > of a function declaration and throws a warning stating that those
> > compiler attributes should have an identifier name which is not correct.
> > Add __aligned compiler attributes to the list of attribute notes
> > so they don't cause warnings anymore.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > Any expression that evaluates to an integer that is a power of 2 can be
> > within __aligned parenthesis.
> > I can't see how we could use a regex to check code meets such constraint (if possible).  
> 
> You can't because if a #define is used for the alignment
> value it is not necessarily available to a patch fragment
> or even a file if the #define is in an #include.
> 
> > Some additional exotic uses of __aligned are:
> > drivers/net/wireless/quantenna/qtnfmac/bus.h:72:   char bus_priv[] __aligned(sizeof(void *));
> > include/linux/netdevice.h:225:} __aligned(4 * sizeof(unsigned long));   
> 
> Right, then there are random uses like:
> 
> drivers/firmware/qcom/qcom_qseecom_uefisecapp.c:		size_t __aligned;						\
> drivers/firmware/qcom/qcom_qseecom_uefisecapp.c:			*__offset = __aligned;					\
> 
> so there's always some false positive/negative issue
> with checkpatch.
> 
> Anyway:
> 
> Acked-by: Joe Perches <joe@perches.com>
Given this should cut down on false postives I've picked this one up now.

Applied to the togreg branch of iio.git and pushed out briefly as testing to
see what blows up.

I'll see if I can pick up some more parts of this series to avoid us going round
again with quite so many patches.


Jonathan

> 
> > 
> > The regex
> > 			__aligned\s*\(.*\)
> > seems to match all use cases. 
> > 
> > We might not catch invalid arguments to __aligned, but it looks like making
> > checkpath confidently report those wouldn't be feasible anyway.
> > 
> > The patch that would trigger the mentioned warning in now
> > patch number 13 (iio: adc: Add support for AD7091R-8).
> > 
> >  scripts/checkpatch.pl | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 25fdb7fda112..d56c98146da3 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -512,6 +512,7 @@ our $Attribute	= qr{
> >  			__ro_after_init|
> >  			__kprobes|
> >  			$InitAttribute|
> > +			__aligned\s*\(.*\)|
> >  			____cacheline_aligned|
> >  			____cacheline_aligned_in_smp|
> >  			____cacheline_internodealigned_in_smp|  
> 
> 

[PATCH v4 02/15] iio: adc: ad7091r: Pass iio_dev to event handler

[Marcelo Schmitt]

Previous version of ad7091r event handler received the ADC state pointer
and retrieved the iio device from driver data field with dev_get_drvdata().
However, no driver data have ever been set, which led to null pointer
dereference when running the event handler.

Pass the iio device to the event handler and retrieve the ADC state struct
from it so we avoid the null pointer dereference and save the driver from
filling the driver data field.

Fixes: ca69300173b6 ("iio: adc: Add support for AD7091R5 ADC")
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index 8e252cde735b..0e5d3d2e9c98 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -174,8 +174,8 @@ static const struct iio_info ad7091r_info = {
 
 static irqreturn_t ad7091r_event_handler(int irq, void *private)
 {
-	struct ad7091r_state *st = (struct ad7091r_state *) private;
-	struct iio_dev *iio_dev = dev_get_drvdata(st->dev);
+	struct iio_dev *iio_dev = private;
+	struct ad7091r_state *st = iio_priv(iio_dev);
 	unsigned int i, read_val;
 	int ret;
 	s64 timestamp = iio_get_time_ns(iio_dev);
@@ -234,7 +234,7 @@ int ad7091r_probe(struct device *dev, const char *name,
 	if (irq) {
 		ret = devm_request_threaded_irq(dev, irq, NULL,
 				ad7091r_event_handler,
-				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, st);
+				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);
 		if (ret)
 			return ret;
 	}
-- 
2.42.0

Re: [PATCH v4 02/15] iio: adc: ad7091r: Pass iio_dev to event handler

[Jonathan Cameron]

On Sat, 16 Dec 2023 14:46:11 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Previous version of ad7091r event handler received the ADC state pointer
> and retrieved the iio device from driver data field with dev_get_drvdata().
> However, no driver data have ever been set, which led to null pointer
> dereference when running the event handler.
> 
> Pass the iio device to the event handler and retrieve the ADC state struct
> from it so we avoid the null pointer dereference and save the driver from
> filling the driver data field.
> 
> Fixes: ca69300173b6 ("iio: adc: Add support for AD7091R5 ADC")
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Given we are late in the cycle, I've applied this to the togreg branch of iio.git
and it will hopefully go in during the merge window rather than before.
Marked it for stable though so should get backported appropriately.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7091r-base.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index 8e252cde735b..0e5d3d2e9c98 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -174,8 +174,8 @@ static const struct iio_info ad7091r_info = {
>  
>  static irqreturn_t ad7091r_event_handler(int irq, void *private)
>  {
> -	struct ad7091r_state *st = (struct ad7091r_state *) private;
> -	struct iio_dev *iio_dev = dev_get_drvdata(st->dev);
> +	struct iio_dev *iio_dev = private;
> +	struct ad7091r_state *st = iio_priv(iio_dev);
>  	unsigned int i, read_val;
>  	int ret;
>  	s64 timestamp = iio_get_time_ns(iio_dev);
> @@ -234,7 +234,7 @@ int ad7091r_probe(struct device *dev, const char *name,
>  	if (irq) {
>  		ret = devm_request_threaded_irq(dev, irq, NULL,
>  				ad7091r_event_handler,
> -				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, st);
> +				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);
>  		if (ret)
>  			return ret;
>  	}

[PATCH v4 03/15] iio: adc: ad7091r: Set alert bit in config register

[Marcelo Schmitt]

The ad7091r-base driver sets up an interrupt handler for firing events
when inputs are either above or below a certain threshold.
However, for the interrupt signal to come from the device it must be
configured to enable the ALERT/BUSY/GPO pin to be used as ALERT, which
was not being done until now.
Enable interrupt signals on the ALERT/BUSY/GPO pin by setting the proper
bit in the configuration register.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index 0e5d3d2e9c98..8aaa854f816f 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -28,6 +28,7 @@
 #define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
 
 /* AD7091R_REG_CONF */
+#define AD7091R_REG_CONF_ALERT_EN   BIT(4)
 #define AD7091R_REG_CONF_AUTO   BIT(8)
 #define AD7091R_REG_CONF_CMD    BIT(10)
 
@@ -232,6 +233,11 @@ int ad7091r_probe(struct device *dev, const char *name,
 	iio_dev->channels = chip_info->channels;
 
 	if (irq) {
+		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
+					 AD7091R_REG_CONF_ALERT_EN, BIT(4));
+		if (ret)
+			return ret;
+
 		ret = devm_request_threaded_irq(dev, irq, NULL,
 				ad7091r_event_handler,
 				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);
-- 
2.42.0

Re: [PATCH v4 03/15] iio: adc: ad7091r: Set alert bit in config register

[Jonathan Cameron]

On Sat, 16 Dec 2023 14:46:37 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> The ad7091r-base driver sets up an interrupt handler for firing events
> when inputs are either above or below a certain threshold.
> However, for the interrupt signal to come from the device it must be
> configured to enable the ALERT/BUSY/GPO pin to be used as ALERT, which
> was not being done until now.
> Enable interrupt signals on the ALERT/BUSY/GPO pin by setting the proper
> bit in the configuration register.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Hi Marcelo,

In V3 review I asked if this should have a fixes tag.  I've assumed for now
the answer is no and applied it without.  If you let me know fast enough
I can probably slip on in, but if not you may want to consider requesting
a backport after this is upstream.

Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to take a look at it.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7091r-base.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index 0e5d3d2e9c98..8aaa854f816f 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -28,6 +28,7 @@
>  #define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
>  
>  /* AD7091R_REG_CONF */
> +#define AD7091R_REG_CONF_ALERT_EN   BIT(4)
>  #define AD7091R_REG_CONF_AUTO   BIT(8)
>  #define AD7091R_REG_CONF_CMD    BIT(10)
>  
> @@ -232,6 +233,11 @@ int ad7091r_probe(struct device *dev, const char *name,
>  	iio_dev->channels = chip_info->channels;
>  
>  	if (irq) {
> +		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> +					 AD7091R_REG_CONF_ALERT_EN, BIT(4));
> +		if (ret)
> +			return ret;
> +
>  		ret = devm_request_threaded_irq(dev, irq, NULL,
>  				ad7091r_event_handler,
>  				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);

Re: [PATCH v4 03/15] iio: adc: ad7091r: Set alert bit in config register

[Marcelo Schmitt]

On 12/17, Jonathan Cameron wrote:
> On Sat, 16 Dec 2023 14:46:37 -0300
> Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> 
> > The ad7091r-base driver sets up an interrupt handler for firing events
> > when inputs are either above or below a certain threshold.
> > However, for the interrupt signal to come from the device it must be
> > configured to enable the ALERT/BUSY/GPO pin to be used as ALERT, which
> > was not being done until now.
> > Enable interrupt signals on the ALERT/BUSY/GPO pin by setting the proper
> > bit in the configuration register.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> Hi Marcelo,
> 
> In V3 review I asked if this should have a fixes tag.  I've assumed for now
> the answer is no and applied it without.  If you let me know fast enough
> I can probably slip on in, but if not you may want to consider requesting
> a backport after this is upstream.

The events for these devices would not work both because of the broken
dereference fixed in patch 2 and the alert signal was not being enabled.
Patch 2 fixed a null pointer dereference that would lead to an error in the
kernel. This patch (on top of the previous one) makes the event generation
actually work although it's not fixing any errors.
I was hesitant in marking this one with a fixes tag too worrying I might be
adding too many fixes tags for a feature that never worked.
Anyway, looks like I should have added that so at least now I have learned something.
Will see how to ask backports for the fixing patches once they get in mainline.

Thanks

> 
> Applied to the togreg branch of iio.git and pushed out as testing for 0-day
> to take a look at it.
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/adc/ad7091r-base.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> > index 0e5d3d2e9c98..8aaa854f816f 100644
> > --- a/drivers/iio/adc/ad7091r-base.c
> > +++ b/drivers/iio/adc/ad7091r-base.c
> > @@ -28,6 +28,7 @@
> >  #define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
> >  
> >  /* AD7091R_REG_CONF */
> > +#define AD7091R_REG_CONF_ALERT_EN   BIT(4)
> >  #define AD7091R_REG_CONF_AUTO   BIT(8)
> >  #define AD7091R_REG_CONF_CMD    BIT(10)
> >  
> > @@ -232,6 +233,11 @@ int ad7091r_probe(struct device *dev, const char *name,
> >  	iio_dev->channels = chip_info->channels;
> >  
> >  	if (irq) {
> > +		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > +					 AD7091R_REG_CONF_ALERT_EN, BIT(4));
> > +		if (ret)
> > +			return ret;
> > +
> >  		ret = devm_request_threaded_irq(dev, irq, NULL,
> >  				ad7091r_event_handler,
> >  				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);
> 

[PATCH v4 04/15] iio: adc: ad7091r: Align arguments to function call parenthesis

[Marcelo Schmitt]

Align arguments to function call open parenthesis to comply with the
Linux kernel coding style.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index 8aaa854f816f..d3d287d3b953 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -239,8 +239,9 @@ int ad7091r_probe(struct device *dev, const char *name,
 			return ret;
 
 		ret = devm_request_threaded_irq(dev, irq, NULL,
-				ad7091r_event_handler,
-				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);
+						ad7091r_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT, name, iio_dev);
 		if (ret)
 			return ret;
 	}
-- 
2.42.0

Re: [PATCH v4 04/15] iio: adc: ad7091r: Align arguments to function call parenthesis

[Jonathan Cameron]

On Sat, 16 Dec 2023 14:47:01 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Align arguments to function call open parenthesis to comply with the
> Linux kernel coding style.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Applied
> ---
>  drivers/iio/adc/ad7091r-base.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index 8aaa854f816f..d3d287d3b953 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -239,8 +239,9 @@ int ad7091r_probe(struct device *dev, const char *name,
>  			return ret;
>  
>  		ret = devm_request_threaded_irq(dev, irq, NULL,
> -				ad7091r_event_handler,
> -				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);
> +						ad7091r_event_handler,
> +						IRQF_TRIGGER_FALLING |
> +						IRQF_ONESHOT, name, iio_dev);
>  		if (ret)
>  			return ret;
>  	}

[PATCH v4 05/15] iio: adc: ad7091r: Move generic AD7091R code to base driver and header file

[Marcelo Schmitt]

Some code generic to AD7091R devices such as channel definitions and
event spec structs were in the AD7091R-5 driver.
There was also some generic register definitions declared in the base
driver which would make more sense to be in the header file.
The device state struct will be needed for the ad7091r8 driver in a
follow up patch so that ought to be moved to the header file as well.
Lastly, a couple of regmap callback functions are also capable of
abstracting characteristics of different AD7091R devices and those are
now being exported to IIO_AD7091R name space.

Move AD7091R generic code either to the base driver or to the header
file so both the ad7091r5 and the ad7091r8 driver can use those
declaration in follow up patches.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 46 +++++++++++++++++-----------------
 drivers/iio/adc/ad7091r-base.h | 42 ++++++++++++++++++++++++++++++-
 drivers/iio/adc/ad7091r5.c     | 39 +++-------------------------
 3 files changed, 68 insertions(+), 59 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index d3d287d3b953..0d1f544de07a 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -15,14 +15,6 @@
 
 #include "ad7091r-base.h"
 
-#define AD7091R_REG_RESULT  0
-#define AD7091R_REG_CHANNEL 1
-#define AD7091R_REG_CONF    2
-#define AD7091R_REG_ALERT   3
-#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
-#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
-#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
-
 /* AD7091R_REG_RESULT */
 #define AD7091R_REG_RESULT_CH_ID(x)	    (((x) >> 13) & 0x3)
 #define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
@@ -35,20 +27,26 @@
 #define AD7091R_REG_CONF_MODE_MASK  \
 	(AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
 
-enum ad7091r_mode {
-	AD7091R_MODE_SAMPLE,
-	AD7091R_MODE_COMMAND,
-	AD7091R_MODE_AUTOCYCLE,
-};
-
-struct ad7091r_state {
-	struct device *dev;
-	struct regmap *map;
-	struct regulator *vref;
-	const struct ad7091r_chip_info *chip_info;
-	enum ad7091r_mode mode;
-	struct mutex lock; /*lock to prevent concurent reads */
+const struct iio_event_spec ad7091r_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
+	},
 };
+EXPORT_SYMBOL_NS_GPL(ad7091r_events, IIO_AD7091R);
 
 static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
 {
@@ -269,7 +267,7 @@ int ad7091r_probe(struct device *dev, const char *name,
 }
 EXPORT_SYMBOL_NS_GPL(ad7091r_probe, IIO_AD7091R);
 
-static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
+bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
 	case AD7091R_REG_RESULT:
@@ -279,8 +277,9 @@ static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
 		return true;
 	}
 }
+EXPORT_SYMBOL_NS_GPL(ad7091r_writeable_reg, IIO_AD7091R);
 
-static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
+bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
 	case AD7091R_REG_RESULT:
@@ -290,6 +289,7 @@ static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
 		return false;
 	}
 }
+EXPORT_SYMBOL_NS_GPL(ad7091r_volatile_reg, IIO_AD7091R);
 
 const struct regmap_config ad7091r_regmap_config = {
 	.reg_bits = 8,
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 509748aef9b1..1d30eeb46bcc 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -8,8 +8,43 @@
 #ifndef __DRIVERS_IIO_ADC_AD7091R_BASE_H__
 #define __DRIVERS_IIO_ADC_AD7091R_BASE_H__
 
+#define AD7091R_REG_RESULT  0
+#define AD7091R_REG_CHANNEL 1
+#define AD7091R_REG_CONF    2
+#define AD7091R_REG_ALERT   3
+
+#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
+#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
+#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
+
+#define AD7091R_CHANNEL(idx, bits, ev, num_ev) {			\
+	.type = IIO_VOLTAGE,						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.indexed = 1,							\
+	.channel = idx,							\
+	.event_spec = ev,						\
+	.num_event_specs = num_ev,					\
+	.scan_type.storagebits = 16,					\
+	.scan_type.realbits = bits,					\
+}
+
 struct device;
-struct ad7091r_state;
+
+enum ad7091r_mode {
+	AD7091R_MODE_SAMPLE,
+	AD7091R_MODE_COMMAND,
+	AD7091R_MODE_AUTOCYCLE,
+};
+
+struct ad7091r_state {
+	struct device *dev;
+	struct regmap *map;
+	struct regulator *vref;
+	const struct ad7091r_chip_info *chip_info;
+	enum ad7091r_mode mode;
+	struct mutex lock; /*lock to prevent concurent reads */
+};
 
 struct ad7091r_chip_info {
 	unsigned int num_channels;
@@ -17,10 +52,15 @@ struct ad7091r_chip_info {
 	unsigned int vref_mV;
 };
 
+extern const struct iio_event_spec ad7091r_events[3];
+
 extern const struct regmap_config ad7091r_regmap_config;
 
 int ad7091r_probe(struct device *dev, const char *name,
 		const struct ad7091r_chip_info *chip_info,
 		struct regmap *map, int irq);
 
+bool ad7091r_volatile_reg(struct device *dev, unsigned int reg);
+bool ad7091r_writeable_reg(struct device *dev, unsigned int reg);
+
 #endif /* __DRIVERS_IIO_ADC_AD7091R_BASE_H__ */
diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
index 2f048527b7b7..9d3ccfca94ec 100644
--- a/drivers/iio/adc/ad7091r5.c
+++ b/drivers/iio/adc/ad7091r5.c
@@ -12,42 +12,11 @@
 
 #include "ad7091r-base.h"
 
-static const struct iio_event_spec ad7091r5_events[] = {
-	{
-		.type = IIO_EV_TYPE_THRESH,
-		.dir = IIO_EV_DIR_RISING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-				 BIT(IIO_EV_INFO_ENABLE),
-	},
-	{
-		.type = IIO_EV_TYPE_THRESH,
-		.dir = IIO_EV_DIR_FALLING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-				 BIT(IIO_EV_INFO_ENABLE),
-	},
-	{
-		.type = IIO_EV_TYPE_THRESH,
-		.dir = IIO_EV_DIR_EITHER,
-		.mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
-	},
-};
-
-#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
-	.type = IIO_VOLTAGE, \
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
-	.indexed = 1, \
-	.channel = idx, \
-	.event_spec = ev, \
-	.num_event_specs = num_ev, \
-	.scan_type.storagebits = 16, \
-	.scan_type.realbits = bits, \
-}
 static const struct iio_chan_spec ad7091r5_channels_irq[] = {
-	AD7091R_CHANNEL(0, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
-	AD7091R_CHANNEL(1, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
-	AD7091R_CHANNEL(2, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
-	AD7091R_CHANNEL(3, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
+	AD7091R_CHANNEL(0, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(1, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(2, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(3, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
 };
 
 static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
-- 
2.42.0

Re: [PATCH v4 05/15] iio: adc: ad7091r: Move generic AD7091R code to base driver and header file

[Jonathan Cameron]

On Sat, 16 Dec 2023 14:47:25 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Some code generic to AD7091R devices such as channel definitions and
> event spec structs were in the AD7091R-5 driver.
> There was also some generic register definitions declared in the base
> driver which would make more sense to be in the header file.
> The device state struct will be needed for the ad7091r8 driver in a
> follow up patch so that ought to be moved to the header file as well.
> Lastly, a couple of regmap callback functions are also capable of
> abstracting characteristics of different AD7091R devices and those are
> now being exported to IIO_AD7091R name space.
> 
> Move AD7091R generic code either to the base driver or to the header
> file so both the ad7091r5 and the ad7091r8 driver can use those
> declaration in follow up patches.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Hi Marcelo

I'm going to stop here as can't apply next patch because of build warnings
and this one is hard to justify without the rest of the series.

So please just include this patch onwards in v5.

This looks fine to me btw.  I'll just comment on patches where I have
anything to add. If not they look fine.

Jonathan

> ---
>  drivers/iio/adc/ad7091r-base.c | 46 +++++++++++++++++-----------------
>  drivers/iio/adc/ad7091r-base.h | 42 ++++++++++++++++++++++++++++++-
>  drivers/iio/adc/ad7091r5.c     | 39 +++-------------------------
>  3 files changed, 68 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index d3d287d3b953..0d1f544de07a 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -15,14 +15,6 @@
>  
>  #include "ad7091r-base.h"
>  
> -#define AD7091R_REG_RESULT  0
> -#define AD7091R_REG_CHANNEL 1
> -#define AD7091R_REG_CONF    2
> -#define AD7091R_REG_ALERT   3
> -#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
> -#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
> -#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> -
>  /* AD7091R_REG_RESULT */
>  #define AD7091R_REG_RESULT_CH_ID(x)	    (((x) >> 13) & 0x3)
>  #define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
> @@ -35,20 +27,26 @@
>  #define AD7091R_REG_CONF_MODE_MASK  \
>  	(AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
>  
> -enum ad7091r_mode {
> -	AD7091R_MODE_SAMPLE,
> -	AD7091R_MODE_COMMAND,
> -	AD7091R_MODE_AUTOCYCLE,
> -};
> -
> -struct ad7091r_state {
> -	struct device *dev;
> -	struct regmap *map;
> -	struct regulator *vref;
> -	const struct ad7091r_chip_info *chip_info;
> -	enum ad7091r_mode mode;
> -	struct mutex lock; /*lock to prevent concurent reads */
> +const struct iio_event_spec ad7091r_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +				 BIT(IIO_EV_INFO_ENABLE),
> +	},
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +				 BIT(IIO_EV_INFO_ENABLE),
> +	},
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> +	},
>  };
> +EXPORT_SYMBOL_NS_GPL(ad7091r_events, IIO_AD7091R);
>  
>  static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
>  {
> @@ -269,7 +267,7 @@ int ad7091r_probe(struct device *dev, const char *name,
>  }
>  EXPORT_SYMBOL_NS_GPL(ad7091r_probe, IIO_AD7091R);
>  
> -static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
> +bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
>  	case AD7091R_REG_RESULT:
> @@ -279,8 +277,9 @@ static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
>  		return true;
>  	}
>  }
> +EXPORT_SYMBOL_NS_GPL(ad7091r_writeable_reg, IIO_AD7091R);
>  
> -static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
> +bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
>  	case AD7091R_REG_RESULT:
> @@ -290,6 +289,7 @@ static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
>  		return false;
>  	}
>  }
> +EXPORT_SYMBOL_NS_GPL(ad7091r_volatile_reg, IIO_AD7091R);
>  
>  const struct regmap_config ad7091r_regmap_config = {
>  	.reg_bits = 8,
> diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> index 509748aef9b1..1d30eeb46bcc 100644
> --- a/drivers/iio/adc/ad7091r-base.h
> +++ b/drivers/iio/adc/ad7091r-base.h
> @@ -8,8 +8,43 @@
>  #ifndef __DRIVERS_IIO_ADC_AD7091R_BASE_H__
>  #define __DRIVERS_IIO_ADC_AD7091R_BASE_H__
>  
> +#define AD7091R_REG_RESULT  0
> +#define AD7091R_REG_CHANNEL 1
> +#define AD7091R_REG_CONF    2
> +#define AD7091R_REG_ALERT   3
> +
> +#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
> +#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
> +#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> +
> +#define AD7091R_CHANNEL(idx, bits, ev, num_ev) {			\
> +	.type = IIO_VOLTAGE,						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.indexed = 1,							\
> +	.channel = idx,							\
> +	.event_spec = ev,						\
> +	.num_event_specs = num_ev,					\
> +	.scan_type.storagebits = 16,					\
> +	.scan_type.realbits = bits,					\
> +}
> +
>  struct device;
> -struct ad7091r_state;
> +
> +enum ad7091r_mode {
> +	AD7091R_MODE_SAMPLE,
> +	AD7091R_MODE_COMMAND,
> +	AD7091R_MODE_AUTOCYCLE,
> +};
> +
> +struct ad7091r_state {
> +	struct device *dev;
> +	struct regmap *map;
> +	struct regulator *vref;
> +	const struct ad7091r_chip_info *chip_info;
> +	enum ad7091r_mode mode;
> +	struct mutex lock; /*lock to prevent concurent reads */
> +};
>  
>  struct ad7091r_chip_info {
>  	unsigned int num_channels;
> @@ -17,10 +52,15 @@ struct ad7091r_chip_info {
>  	unsigned int vref_mV;
>  };
>  
> +extern const struct iio_event_spec ad7091r_events[3];
> +
>  extern const struct regmap_config ad7091r_regmap_config;
>  
>  int ad7091r_probe(struct device *dev, const char *name,
>  		const struct ad7091r_chip_info *chip_info,
>  		struct regmap *map, int irq);
>  
> +bool ad7091r_volatile_reg(struct device *dev, unsigned int reg);
> +bool ad7091r_writeable_reg(struct device *dev, unsigned int reg);
> +
>  #endif /* __DRIVERS_IIO_ADC_AD7091R_BASE_H__ */
> diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
> index 2f048527b7b7..9d3ccfca94ec 100644
> --- a/drivers/iio/adc/ad7091r5.c
> +++ b/drivers/iio/adc/ad7091r5.c
> @@ -12,42 +12,11 @@
>  
>  #include "ad7091r-base.h"
>  
> -static const struct iio_event_spec ad7091r5_events[] = {
> -	{
> -		.type = IIO_EV_TYPE_THRESH,
> -		.dir = IIO_EV_DIR_RISING,
> -		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -				 BIT(IIO_EV_INFO_ENABLE),
> -	},
> -	{
> -		.type = IIO_EV_TYPE_THRESH,
> -		.dir = IIO_EV_DIR_FALLING,
> -		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -				 BIT(IIO_EV_INFO_ENABLE),
> -	},
> -	{
> -		.type = IIO_EV_TYPE_THRESH,
> -		.dir = IIO_EV_DIR_EITHER,
> -		.mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> -	},
> -};
> -
> -#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
> -	.type = IIO_VOLTAGE, \
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> -	.indexed = 1, \
> -	.channel = idx, \
> -	.event_spec = ev, \
> -	.num_event_specs = num_ev, \
> -	.scan_type.storagebits = 16, \
> -	.scan_type.realbits = bits, \
> -}
>  static const struct iio_chan_spec ad7091r5_channels_irq[] = {
> -	AD7091R_CHANNEL(0, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> -	AD7091R_CHANNEL(1, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> -	AD7091R_CHANNEL(2, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> -	AD7091R_CHANNEL(3, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> +	AD7091R_CHANNEL(0, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
> +	AD7091R_CHANNEL(1, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
> +	AD7091R_CHANNEL(2, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
> +	AD7091R_CHANNEL(3, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
>  };
>  
>  static const struct iio_chan_spec ad7091r5_channels_noirq[] = {

[PATCH v4 06/15] iio: adc: ad7091r: Move chip init data to container struct

[Marcelo Schmitt]

AD7091R designs may differ on their communication protocol and resources
required for proper setup. Extract what is design specific into a
init_info struct so the base driver can use data and callback functions
from that struct rather than checking which specific chip is connected
during device initialization.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 27 ++++++++++-----------
 drivers/iio/adc/ad7091r-base.h | 15 +++++++++---
 drivers/iio/adc/ad7091r5.c     | 43 ++++++++++++++++++++++++----------
 3 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index 0d1f544de07a..ad4b4d591e1a 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -207,7 +207,7 @@ static void ad7091r_remove(void *data)
 }
 
 int ad7091r_probe(struct device *dev, const char *name,
-		const struct ad7091r_chip_info *chip_info,
+		const struct ad7091r_init_info *init_info,
 		struct regmap *map, int irq)
 {
 	struct iio_dev *iio_dev;
@@ -220,17 +220,16 @@ int ad7091r_probe(struct device *dev, const char *name,
 
 	st = iio_priv(iio_dev);
 	st->dev = dev;
-	st->chip_info = chip_info;
-	st->map = map;
+	init_info->init_adc_regmap(st, init_info->regmap_config);
+	if (IS_ERR(st->map))
+		return dev_err_probe(st->dev, PTR_ERR(st->map),
+				     "Error initializing regmap\n");
 
-	iio_dev->name = name;
 	iio_dev->info = &ad7091r_info;
 	iio_dev->modes = INDIO_DIRECT_MODE;
 
-	iio_dev->num_channels = chip_info->num_channels;
-	iio_dev->channels = chip_info->channels;
-
 	if (irq) {
+		st->chip_info = &init_info->irq_info;
 		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
 					 AD7091R_REG_CONF_ALERT_EN, BIT(4));
 		if (ret)
@@ -242,8 +241,14 @@ int ad7091r_probe(struct device *dev, const char *name,
 						IRQF_ONESHOT, name, iio_dev);
 		if (ret)
 			return ret;
+	} else {
+		st->chip_info = &init_info->info_no_irq;
 	}
 
+	iio_dev->name = st->chip_info->name;
+	iio_dev->num_channels = st->chip_info->num_channels;
+	iio_dev->channels = st->chip_info->channels;
+
 	st->vref = devm_regulator_get_optional(dev, "vref");
 	if (IS_ERR(st->vref)) {
 		if (PTR_ERR(st->vref) == -EPROBE_DEFER)
@@ -291,14 +296,6 @@ bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
 }
 EXPORT_SYMBOL_NS_GPL(ad7091r_volatile_reg, IIO_AD7091R);
 
-const struct regmap_config ad7091r_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 16,
-	.writeable_reg = ad7091r_writeable_reg,
-	.volatile_reg = ad7091r_volatile_reg,
-};
-EXPORT_SYMBOL_NS_GPL(ad7091r_regmap_config, IIO_AD7091R);
-
 MODULE_AUTHOR("Beniamin Bia <beniamin.bia@analog.com>");
 MODULE_DESCRIPTION("Analog Devices AD7091Rx multi-channel converters");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 1d30eeb46bcc..51649570bb5d 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -8,6 +8,8 @@
 #ifndef __DRIVERS_IIO_ADC_AD7091R_BASE_H__
 #define __DRIVERS_IIO_ADC_AD7091R_BASE_H__
 
+#include <linux/regmap.h>
+
 #define AD7091R_REG_RESULT  0
 #define AD7091R_REG_CHANNEL 1
 #define AD7091R_REG_CONF    2
@@ -47,17 +49,24 @@ struct ad7091r_state {
 };
 
 struct ad7091r_chip_info {
+	const char *name;
 	unsigned int num_channels;
 	const struct iio_chan_spec *channels;
 	unsigned int vref_mV;
 };
 
-extern const struct iio_event_spec ad7091r_events[3];
+struct ad7091r_init_info {
+	struct ad7091r_chip_info irq_info;
+	struct ad7091r_chip_info info_no_irq;
+	const struct regmap_config *regmap_config;
+	void (*init_adc_regmap)(struct ad7091r_state *st,
+				const struct regmap_config *regmap_conf);
+};
 
-extern const struct regmap_config ad7091r_regmap_config;
+extern const struct iio_event_spec ad7091r_events[3];
 
 int ad7091r_probe(struct device *dev, const char *name,
-		const struct ad7091r_chip_info *chip_info,
+		const struct ad7091r_init_info *init_info,
 		struct regmap *map, int irq);
 
 bool ad7091r_volatile_reg(struct device *dev, unsigned int reg);
diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
index 9d3ccfca94ec..69b436920e2d 100644
--- a/drivers/iio/adc/ad7091r5.c
+++ b/drivers/iio/adc/ad7091r5.c
@@ -27,42 +27,61 @@ static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
 };
 
 static const struct ad7091r_chip_info ad7091r5_chip_info_irq = {
+	.name = "ad7091r-5",
 	.channels = ad7091r5_channels_irq,
 	.num_channels = ARRAY_SIZE(ad7091r5_channels_irq),
 	.vref_mV = 2500,
 };
 
 static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
+	.name = "ad7091r-5",
 	.channels = ad7091r5_channels_noirq,
 	.num_channels = ARRAY_SIZE(ad7091r5_channels_noirq),
 	.vref_mV = 2500,
 };
 
+static const struct regmap_config ad7091r_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.writeable_reg = ad7091r_writeable_reg,
+	.volatile_reg = ad7091r_volatile_reg,
+};
+
+static void ad7091r5_regmap_init(struct ad7091r_state *st,
+				 const struct regmap_config *regmap_conf)
+{
+	struct i2c_client *i2c = container_of(st->dev, struct i2c_client, dev);
+
+	st->map = devm_regmap_init_i2c(i2c, regmap_conf);
+}
+
+static struct ad7091r_init_info ad7091r5_init_info = {
+	.irq_info = ad7091r5_chip_info_irq,
+	.info_no_irq = ad7091r5_chip_info_noirq,
+	.regmap_config = &ad7091r_regmap_config,
+	.init_adc_regmap = &ad7091r5_regmap_init
+};
+
 static int ad7091r5_i2c_probe(struct i2c_client *i2c)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
-	const struct ad7091r_chip_info *chip_info;
-	struct regmap *map = devm_regmap_init_i2c(i2c, &ad7091r_regmap_config);
-
-	if (IS_ERR(map))
-		return PTR_ERR(map);
+	const struct ad7091r_init_info *init_info;
 
-	if (i2c->irq)
-		chip_info = &ad7091r5_chip_info_irq;
-	else
-		chip_info = &ad7091r5_chip_info_noirq;
+	init_info = i2c_get_match_data(i2c);
+	if (!init_info)
+		return -EINVAL;
 
-	return ad7091r_probe(&i2c->dev, id->name, chip_info, map, i2c->irq);
+	return ad7091r_probe(&i2c->dev, id->name, init_info, NULL, i2c->irq);
 }
 
 static const struct of_device_id ad7091r5_dt_ids[] = {
-	{ .compatible = "adi,ad7091r5" },
+	{ .compatible = "adi,ad7091r5", .data = &ad7091r5_init_info },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ad7091r5_dt_ids);
 
 static const struct i2c_device_id ad7091r5_i2c_ids[] = {
-	{"ad7091r5", 0},
+	{"ad7091r5", (kernel_ulong_t)&ad7091r5_init_info },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ad7091r5_i2c_ids);
-- 
2.42.0

Re: [PATCH v4 6/15] iio: adc: ad7091r: Move chip init data to container struct

[kernel test robot]

Hi Marcelo,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.7-rc5 next-20231215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Marcelo-Schmitt/scripts-checkpatch-Add-__aligned-to-the-list-of-attribute-notes/20231217-055420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/9d1248860193b55e79640b2e64c21c66bd6645f9.1702746240.git.marcelo.schmitt1%40gmail.com
patch subject: [PATCH v4 6/15] iio: adc: ad7091r: Move chip init data to container struct
config: um-randconfig-r111-20231217 (https://download.01.org/0day-ci/archive/20231217/202312172006.b2fep1oG-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231217/202312172006.b2fep1oG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312172006.b2fep1oG-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/adc/ad7091r5.c:59:14: error: initializer element is not constant
     .irq_info = ad7091r5_chip_info_irq,
                 ^~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/ad7091r5.c:59:14: note: (near initialization for 'ad7091r5_init_info.irq_info')
   drivers/iio/adc/ad7091r5.c:60:17: error: initializer element is not constant
     .info_no_irq = ad7091r5_chip_info_noirq,
                    ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/ad7091r5.c:60:17: note: (near initialization for 'ad7091r5_init_info.info_no_irq')


vim +59 drivers/iio/adc/ad7091r5.c

    57	
    58	static struct ad7091r_init_info ad7091r5_init_info = {
  > 59		.irq_info = ad7091r5_chip_info_irq,
    60		.info_no_irq = ad7091r5_chip_info_noirq,
    61		.regmap_config = &ad7091r_regmap_config,
    62		.init_adc_regmap = &ad7091r5_regmap_init
    63	};
    64	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 6/15] iio: adc: ad7091r: Move chip init data to container struct

[Jonathan Cameron]

On Sun, 17 Dec 2023 21:04:50 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi Marcelo,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on jic23-iio/togreg]
> [also build test ERROR on linus/master v6.7-rc5 next-20231215]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Marcelo-Schmitt/scripts-checkpatch-Add-__aligned-to-the-list-of-attribute-notes/20231217-055420
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> patch link:    https://lore.kernel.org/r/9d1248860193b55e79640b2e64c21c66bd6645f9.1702746240.git.marcelo.schmitt1%40gmail.com
> patch subject: [PATCH v4 6/15] iio: adc: ad7091r: Move chip init data to container struct
> config: um-randconfig-r111-20231217 (https://download.01.org/0day-ci/archive/20231217/202312172006.b2fep1oG-lkp@intel.com/config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231217/202312172006.b2fep1oG-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202312172006.b2fep1oG-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/iio/adc/ad7091r5.c:59:14: error: initializer element is not constant  
>      .irq_info = ad7091r5_chip_info_irq,
>                  ^~~~~~~~~~~~~~~~~~~~~~
>    drivers/iio/adc/ad7091r5.c:59:14: note: (near initialization for 'ad7091r5_init_info.irq_info')
>    drivers/iio/adc/ad7091r5.c:60:17: error: initializer element is not constant
>      .info_no_irq = ad7091r5_chip_info_noirq,
>                     ^~~~~~~~~~~~~~~~~~~~~~~~

You should use a pointer rather than assigning the whole structure.

>    drivers/iio/adc/ad7091r5.c:60:17: note: (near initialization for 'ad7091r5_init_info.info_no_irq')
> 
> 
> vim +59 drivers/iio/adc/ad7091r5.c
> 
>     57	
>     58	static struct ad7091r_init_info ad7091r5_init_info = {
>   > 59		.irq_info = ad7091r5_chip_info_irq,  
>     60		.info_no_irq = ad7091r5_chip_info_noirq,
>     61		.regmap_config = &ad7091r_regmap_config,
>     62		.init_adc_regmap = &ad7091r5_regmap_init
>     63	};
>     64	
> 

[PATCH v4 07/15] iio: adc: ad7091r: Remove unneeded probe parameters

[Marcelo Schmitt]

With the grouping of ad7091r initialization data and callbacks into the
init_info struct, there is no more need to pass the device name and
register map through probe function parameters as those will be available
in the init_info object.
Remove probe parameters that are not needed anymore.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Maybe squash this patch with the previous one.
Wasn't sure about it and squashing is easier than to split so I left it separate.

 drivers/iio/adc/ad7091r-base.c | 8 ++++----
 drivers/iio/adc/ad7091r-base.h | 5 ++---
 drivers/iio/adc/ad7091r5.c     | 3 +--
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index ad4b4d591e1a..ed98729baf43 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -206,9 +206,8 @@ static void ad7091r_remove(void *data)
 	regulator_disable(st->vref);
 }
 
-int ad7091r_probe(struct device *dev, const char *name,
-		const struct ad7091r_init_info *init_info,
-		struct regmap *map, int irq)
+int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
+		  int irq)
 {
 	struct iio_dev *iio_dev;
 	struct ad7091r_state *st;
@@ -238,7 +237,8 @@ int ad7091r_probe(struct device *dev, const char *name,
 		ret = devm_request_threaded_irq(dev, irq, NULL,
 						ad7091r_event_handler,
 						IRQF_TRIGGER_FALLING |
-						IRQF_ONESHOT, name, iio_dev);
+						IRQF_ONESHOT,
+						st->chip_info->name, iio_dev);
 		if (ret)
 			return ret;
 	} else {
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 51649570bb5d..3b3c81904ac2 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -65,9 +65,8 @@ struct ad7091r_init_info {
 
 extern const struct iio_event_spec ad7091r_events[3];
 
-int ad7091r_probe(struct device *dev, const char *name,
-		const struct ad7091r_init_info *init_info,
-		struct regmap *map, int irq);
+int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
+		  int irq);
 
 bool ad7091r_volatile_reg(struct device *dev, unsigned int reg);
 bool ad7091r_writeable_reg(struct device *dev, unsigned int reg);
diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
index 69b436920e2d..2872b6a0bae7 100644
--- a/drivers/iio/adc/ad7091r5.c
+++ b/drivers/iio/adc/ad7091r5.c
@@ -64,14 +64,13 @@ static struct ad7091r_init_info ad7091r5_init_info = {
 
 static int ad7091r5_i2c_probe(struct i2c_client *i2c)
 {
-	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
 	const struct ad7091r_init_info *init_info;
 
 	init_info = i2c_get_match_data(i2c);
 	if (!init_info)
 		return -EINVAL;
 
-	return ad7091r_probe(&i2c->dev, id->name, init_info, NULL, i2c->irq);
+	return ad7091r_probe(&i2c->dev, init_info, i2c->irq);
 }
 
 static const struct of_device_id ad7091r5_dt_ids[] = {
-- 
2.42.0

[PATCH v4 08/15] iio: adc: ad7091r: Set device mode through chip_info callback

[Marcelo Schmitt]

AD7091R-5 devices have a few modes of operation (sample, command,
autocycle) which are set by writing to configuration register fields.
Follow up patches will add support for AD7091R-2/-4/-8 which don't have
those operation modes nor the register fields for setting them.
Make ad7091r_set_mode() a callback function of AD7091R chip_info struct
so the base driver can appropriately handle each design without having
to check which actual chip is connected.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 38 +---------------------------------
 drivers/iio/adc/ad7091r-base.h |  9 ++++++++
 drivers/iio/adc/ad7091r5.c     | 30 +++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index ed98729baf43..aead72ef55b6 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -19,14 +19,6 @@
 #define AD7091R_REG_RESULT_CH_ID(x)	    (((x) >> 13) & 0x3)
 #define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
 
-/* AD7091R_REG_CONF */
-#define AD7091R_REG_CONF_ALERT_EN   BIT(4)
-#define AD7091R_REG_CONF_AUTO   BIT(8)
-#define AD7091R_REG_CONF_CMD    BIT(10)
-
-#define AD7091R_REG_CONF_MODE_MASK  \
-	(AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
-
 const struct iio_event_spec ad7091r_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -48,34 +40,6 @@ const struct iio_event_spec ad7091r_events[] = {
 };
 EXPORT_SYMBOL_NS_GPL(ad7091r_events, IIO_AD7091R);
 
-static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
-{
-	int ret, conf;
-
-	switch (mode) {
-	case AD7091R_MODE_SAMPLE:
-		conf = 0;
-		break;
-	case AD7091R_MODE_COMMAND:
-		conf = AD7091R_REG_CONF_CMD;
-		break;
-	case AD7091R_MODE_AUTOCYCLE:
-		conf = AD7091R_REG_CONF_AUTO;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
-				 AD7091R_REG_CONF_MODE_MASK, conf);
-	if (ret)
-		return ret;
-
-	st->mode = mode;
-
-	return 0;
-}
-
 static int ad7091r_set_channel(struct ad7091r_state *st, unsigned int channel)
 {
 	unsigned int dummy;
@@ -264,7 +228,7 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
 	}
 
 	/* Use command mode by default to convert only desired channels*/
-	ret = ad7091r_set_mode(st, AD7091R_MODE_COMMAND);
+	ret = st->chip_info->set_mode(st, AD7091R_MODE_COMMAND);
 	if (ret)
 		return ret;
 
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 3b3c81904ac2..81b8a4bbb929 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -19,6 +19,14 @@
 #define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
 #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
 
+/* AD7091R_REG_CONF */
+#define AD7091R_REG_CONF_ALERT_EN	BIT(4)
+#define AD7091R_REG_CONF_AUTO		BIT(8)
+#define AD7091R_REG_CONF_CMD		BIT(10)
+
+#define AD7091R_REG_CONF_MODE_MASK  \
+	(AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
+
 #define AD7091R_CHANNEL(idx, bits, ev, num_ev) {			\
 	.type = IIO_VOLTAGE,						\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
@@ -53,6 +61,7 @@ struct ad7091r_chip_info {
 	unsigned int num_channels;
 	const struct iio_chan_spec *channels;
 	unsigned int vref_mV;
+	int (*set_mode)(struct ad7091r_state *st, enum ad7091r_mode mode);
 };
 
 struct ad7091r_init_info {
diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
index 2872b6a0bae7..08f4b9717329 100644
--- a/drivers/iio/adc/ad7091r5.c
+++ b/drivers/iio/adc/ad7091r5.c
@@ -26,11 +26,40 @@ static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
 	AD7091R_CHANNEL(3, 12, NULL, 0),
 };
 
+static int ad7091r5_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
+{
+	int ret, conf;
+
+	switch (mode) {
+	case AD7091R_MODE_SAMPLE:
+		conf = 0;
+		break;
+	case AD7091R_MODE_COMMAND:
+		conf = AD7091R_REG_CONF_CMD;
+		break;
+	case AD7091R_MODE_AUTOCYCLE:
+		conf = AD7091R_REG_CONF_AUTO;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
+				 AD7091R_REG_CONF_MODE_MASK, conf);
+	if (ret)
+		return ret;
+
+	st->mode = mode;
+
+	return 0;
+}
+
 static const struct ad7091r_chip_info ad7091r5_chip_info_irq = {
 	.name = "ad7091r-5",
 	.channels = ad7091r5_channels_irq,
 	.num_channels = ARRAY_SIZE(ad7091r5_channels_irq),
 	.vref_mV = 2500,
+	.set_mode = &ad7091r5_set_mode,
 };
 
 static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
@@ -38,6 +67,7 @@ static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
 	.channels = ad7091r5_channels_noirq,
 	.num_channels = ARRAY_SIZE(ad7091r5_channels_noirq),
 	.vref_mV = 2500,
+	.set_mode = &ad7091r5_set_mode,
 };
 
 static const struct regmap_config ad7091r_regmap_config = {
-- 
2.42.0

[PATCH v4 09/15] iio: adc: ad7091r: Enable internal vref if external vref is not supplied

[Marcelo Schmitt]

The ADC needs a voltage reference to work correctly.
Enable AD7091R internal voltage reference if no external vref is supplied.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 7 +++++++
 drivers/iio/adc/ad7091r-base.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index aead72ef55b6..9d0b489966f5 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -217,7 +217,14 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
 	if (IS_ERR(st->vref)) {
 		if (PTR_ERR(st->vref) == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
+
 		st->vref = NULL;
+		/* Enable internal vref */
+		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
+					 AD7091R_REG_CONF_INT_VREF, BIT(0));
+		if (ret)
+			return dev_err_probe(st->dev, ret,
+					     "Error on enable internal reference\n");
 	} else {
 		ret = regulator_enable(st->vref);
 		if (ret)
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 81b8a4bbb929..9cfb362a00a4 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -20,6 +20,7 @@
 #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
 
 /* AD7091R_REG_CONF */
+#define AD7091R_REG_CONF_INT_VREF	BIT(0)
 #define AD7091R_REG_CONF_ALERT_EN	BIT(4)
 #define AD7091R_REG_CONF_AUTO		BIT(8)
 #define AD7091R_REG_CONF_CMD		BIT(10)
-- 
2.42.0

Re: [PATCH v4 09/15] iio: adc: ad7091r: Enable internal vref if external vref is not supplied

[Jonathan Cameron]

On Sat, 16 Dec 2023 14:49:07 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> The ADC needs a voltage reference to work correctly.
> Enable AD7091R internal voltage reference if no external vref is supplied.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
This one sounds to me like it should have a fixes tag and be
much earlier in the set to perhaps simplify backports.

Jonathan

> ---
>  drivers/iio/adc/ad7091r-base.c | 7 +++++++
>  drivers/iio/adc/ad7091r-base.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index aead72ef55b6..9d0b489966f5 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -217,7 +217,14 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
>  	if (IS_ERR(st->vref)) {
>  		if (PTR_ERR(st->vref) == -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
> +
>  		st->vref = NULL;
> +		/* Enable internal vref */
> +		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> +					 AD7091R_REG_CONF_INT_VREF, BIT(0));
> +		if (ret)
> +			return dev_err_probe(st->dev, ret,
> +					     "Error on enable internal reference\n");
>  	} else {
>  		ret = regulator_enable(st->vref);
>  		if (ret)
> diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> index 81b8a4bbb929..9cfb362a00a4 100644
> --- a/drivers/iio/adc/ad7091r-base.h
> +++ b/drivers/iio/adc/ad7091r-base.h
> @@ -20,6 +20,7 @@
>  #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
>  
>  /* AD7091R_REG_CONF */
> +#define AD7091R_REG_CONF_INT_VREF	BIT(0)
>  #define AD7091R_REG_CONF_ALERT_EN	BIT(4)
>  #define AD7091R_REG_CONF_AUTO		BIT(8)
>  #define AD7091R_REG_CONF_CMD		BIT(10)

Re: [PATCH v4 09/15] iio: adc: ad7091r: Enable internal vref if external vref is not supplied

[Marcelo Schmitt]

On 12/17, Jonathan Cameron wrote:
> On Sat, 16 Dec 2023 14:49:07 -0300
> Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> 
> > The ADC needs a voltage reference to work correctly.
> > Enable AD7091R internal voltage reference if no external vref is supplied.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> This one sounds to me like it should have a fixes tag and be
> much earlier in the set to perhaps simplify backports.

Could be. If we stick to the fact that the dt-binding does not require a voltage
regulator then this can be seen as a fix.
Though, if users can provide an external reference this patch makes no
difference them.
I am using the internal reference for testing so having this one makes a
difference for me.

> 
> Jonathan
> 
> > ---
> >  drivers/iio/adc/ad7091r-base.c | 7 +++++++
> >  drivers/iio/adc/ad7091r-base.h | 1 +
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> > index aead72ef55b6..9d0b489966f5 100644
> > --- a/drivers/iio/adc/ad7091r-base.c
> > +++ b/drivers/iio/adc/ad7091r-base.c
> > @@ -217,7 +217,14 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
> >  	if (IS_ERR(st->vref)) {
> >  		if (PTR_ERR(st->vref) == -EPROBE_DEFER)
> >  			return -EPROBE_DEFER;
> > +
> >  		st->vref = NULL;
> > +		/* Enable internal vref */
> > +		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > +					 AD7091R_REG_CONF_INT_VREF, BIT(0));
> > +		if (ret)
> > +			return dev_err_probe(st->dev, ret,
> > +					     "Error on enable internal reference\n");
> >  	} else {
> >  		ret = regulator_enable(st->vref);
> >  		if (ret)
> > diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> > index 81b8a4bbb929..9cfb362a00a4 100644
> > --- a/drivers/iio/adc/ad7091r-base.h
> > +++ b/drivers/iio/adc/ad7091r-base.h
> > @@ -20,6 +20,7 @@
> >  #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> >  
> >  /* AD7091R_REG_CONF */
> > +#define AD7091R_REG_CONF_INT_VREF	BIT(0)
> >  #define AD7091R_REG_CONF_ALERT_EN	BIT(4)
> >  #define AD7091R_REG_CONF_AUTO		BIT(8)
> >  #define AD7091R_REG_CONF_CMD		BIT(10)
> 
> 

Re: [PATCH v4 09/15] iio: adc: ad7091r: Enable internal vref if external vref is not supplied

[Jonathan Cameron]

On Mon, 18 Dec 2023 14:35:27 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 12/17, Jonathan Cameron wrote:
> > On Sat, 16 Dec 2023 14:49:07 -0300
> > Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> >   
> > > The ADC needs a voltage reference to work correctly.
> > > Enable AD7091R internal voltage reference if no external vref is supplied.
> > > 
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>  
> > This one sounds to me like it should have a fixes tag and be
> > much earlier in the set to perhaps simplify backports.  
> 
> Could be. If we stick to the fact that the dt-binding does not require a voltage
> regulator then this can be seen as a fix.
> Though, if users can provide an external reference this patch makes no
> difference them.
> I am using the internal reference for testing so having this one makes a
> difference for me.
The binding has it as optional, though usually when not having an
external reference leads to use of an internal one, we call it out
in the description.  

Meh, can backport it as a fix if anyone asks for it.

Jonathan

> 
> > 
> > Jonathan
> >   
> > > ---
> > >  drivers/iio/adc/ad7091r-base.c | 7 +++++++
> > >  drivers/iio/adc/ad7091r-base.h | 1 +
> > >  2 files changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> > > index aead72ef55b6..9d0b489966f5 100644
> > > --- a/drivers/iio/adc/ad7091r-base.c
> > > +++ b/drivers/iio/adc/ad7091r-base.c
> > > @@ -217,7 +217,14 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
> > >  	if (IS_ERR(st->vref)) {
> > >  		if (PTR_ERR(st->vref) == -EPROBE_DEFER)
> > >  			return -EPROBE_DEFER;
> > > +
> > >  		st->vref = NULL;
> > > +		/* Enable internal vref */
> > > +		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > > +					 AD7091R_REG_CONF_INT_VREF, BIT(0));
> > > +		if (ret)
> > > +			return dev_err_probe(st->dev, ret,
> > > +					     "Error on enable internal reference\n");
> > >  	} else {
> > >  		ret = regulator_enable(st->vref);
> > >  		if (ret)
> > > diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> > > index 81b8a4bbb929..9cfb362a00a4 100644
> > > --- a/drivers/iio/adc/ad7091r-base.h
> > > +++ b/drivers/iio/adc/ad7091r-base.h
> > > @@ -20,6 +20,7 @@
> > >  #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> > >  
> > >  /* AD7091R_REG_CONF */
> > > +#define AD7091R_REG_CONF_INT_VREF	BIT(0)
> > >  #define AD7091R_REG_CONF_ALERT_EN	BIT(4)
> > >  #define AD7091R_REG_CONF_AUTO		BIT(8)
> > >  #define AD7091R_REG_CONF_CMD		BIT(10)  
> > 
> >   

Re: [PATCH v4 09/15] iio: adc: ad7091r: Enable internal vref if external vref is not supplied

[David Lechner]

On Sat, Dec 16, 2023 at 11:49 AM Marcelo Schmitt
<marcelo.schmitt@analog.com> wrote:
>
> The ADC needs a voltage reference to work correctly.
> Enable AD7091R internal voltage reference if no external vref is supplied.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
>  drivers/iio/adc/ad7091r-base.c | 7 +++++++
>  drivers/iio/adc/ad7091r-base.h | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index aead72ef55b6..9d0b489966f5 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -217,7 +217,14 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
>         if (IS_ERR(st->vref)) {
>                 if (PTR_ERR(st->vref) == -EPROBE_DEFER)
>                         return -EPROBE_DEFER;
> +
>                 st->vref = NULL;
> +               /* Enable internal vref */
> +               ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> +                                        AD7091R_REG_CONF_INT_VREF, BIT(0));

Can we use regmap_set_bits() here to avoid the BIT(0)?

The same comment applies to other patches in this series.

> +               if (ret)
> +                       return dev_err_probe(st->dev, ret,
> +                                            "Error on enable internal reference\n");
>         } else {
>                 ret = regulator_enable(st->vref);
>                 if (ret)
> diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> index 81b8a4bbb929..9cfb362a00a4 100644
> --- a/drivers/iio/adc/ad7091r-base.h
> +++ b/drivers/iio/adc/ad7091r-base.h
> @@ -20,6 +20,7 @@
>  #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
>
>  /* AD7091R_REG_CONF */
> +#define AD7091R_REG_CONF_INT_VREF      BIT(0)
>  #define AD7091R_REG_CONF_ALERT_EN      BIT(4)
>  #define AD7091R_REG_CONF_AUTO          BIT(8)
>  #define AD7091R_REG_CONF_CMD           BIT(10)
> --
> 2.42.0
>

Re: [PATCH v4 09/15] iio: adc: ad7091r: Enable internal vref if external vref is not supplied

[Marcelo Schmitt]

On 12/17, David Lechner wrote:
> On Sat, Dec 16, 2023 at 11:49 AM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> >
> > The ADC needs a voltage reference to work correctly.
> > Enable AD7091R internal voltage reference if no external vref is supplied.
> >
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> >  drivers/iio/adc/ad7091r-base.c | 7 +++++++
> >  drivers/iio/adc/ad7091r-base.h | 1 +
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> > index aead72ef55b6..9d0b489966f5 100644
> > --- a/drivers/iio/adc/ad7091r-base.c
> > +++ b/drivers/iio/adc/ad7091r-base.c
> > @@ -217,7 +217,14 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
> >         if (IS_ERR(st->vref)) {
> >                 if (PTR_ERR(st->vref) == -EPROBE_DEFER)
> >                         return -EPROBE_DEFER;
> > +
> >                 st->vref = NULL;
> > +               /* Enable internal vref */
> > +               ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > +                                        AD7091R_REG_CONF_INT_VREF, BIT(0));
> 
> Can we use regmap_set_bits() here to avoid the BIT(0)?
> 
> The same comment applies to other patches in this series.

Looks good, will do.

Thanks

> 
> > +               if (ret)
> > +                       return dev_err_probe(st->dev, ret,
> > +                                            "Error on enable internal reference\n");
> >         } else {
> >                 ret = regulator_enable(st->vref);
> >                 if (ret)
> > diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> > index 81b8a4bbb929..9cfb362a00a4 100644
> > --- a/drivers/iio/adc/ad7091r-base.h
> > +++ b/drivers/iio/adc/ad7091r-base.h
> > @@ -20,6 +20,7 @@
> >  #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> >
> >  /* AD7091R_REG_CONF */
> > +#define AD7091R_REG_CONF_INT_VREF      BIT(0)
> >  #define AD7091R_REG_CONF_ALERT_EN      BIT(4)
> >  #define AD7091R_REG_CONF_AUTO          BIT(8)
> >  #define AD7091R_REG_CONF_CMD           BIT(10)
> > --
> > 2.42.0
> >

[PATCH v4 10/15] iio: adc: ad7091r: Add chip_info callback to get conversion result channel

[Marcelo Schmitt]

AD7091R-5 and AD7091R-2/-4/-8 have slightly different register field
layout and due to that require different masks for getting the index of
the channel associated with each read.
Add a callback function so the base driver can get correct channel ID
for each chip variant.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/ad7091r-base.c | 6 +-----
 drivers/iio/adc/ad7091r-base.h | 6 ++++++
 drivers/iio/adc/ad7091r5.c     | 7 +++++++
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index 9d0b489966f5..a30dc587ce45 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -15,10 +15,6 @@
 
 #include "ad7091r-base.h"
 
-/* AD7091R_REG_RESULT */
-#define AD7091R_REG_RESULT_CH_ID(x)	    (((x) >> 13) & 0x3)
-#define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
-
 const struct iio_event_spec ad7091r_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -73,7 +69,7 @@ static int ad7091r_read_one(struct iio_dev *iio_dev,
 	if (ret)
 		return ret;
 
-	if (AD7091R_REG_RESULT_CH_ID(val) != channel)
+	if (st->chip_info->reg_result_chan_id(val) != channel)
 		return -EIO;
 
 	*read_val = AD7091R_REG_RESULT_CONV_RESULT(val);
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 9cfb362a00a4..2b4e25e766c8 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -19,6 +19,11 @@
 #define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
 #define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
 
+/* AD7091R_REG_RESULT */
+#define AD7091R5_REG_RESULT_CH_ID(x)	    (((x) >> 13) & 0x3)
+#define AD7091R8_REG_RESULT_CH_ID(x)	    (((x) >> 13) & 0x7)
+#define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
+
 /* AD7091R_REG_CONF */
 #define AD7091R_REG_CONF_INT_VREF	BIT(0)
 #define AD7091R_REG_CONF_ALERT_EN	BIT(4)
@@ -62,6 +67,7 @@ struct ad7091r_chip_info {
 	unsigned int num_channels;
 	const struct iio_chan_spec *channels;
 	unsigned int vref_mV;
+	unsigned int (*reg_result_chan_id)(unsigned int val);
 	int (*set_mode)(struct ad7091r_state *st, enum ad7091r_mode mode);
 };
 
diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
index 08f4b9717329..09d7cd6aeed7 100644
--- a/drivers/iio/adc/ad7091r5.c
+++ b/drivers/iio/adc/ad7091r5.c
@@ -54,11 +54,17 @@ static int ad7091r5_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
 	return 0;
 }
 
+static unsigned int ad7091r5_reg_result_chan_id(unsigned int val)
+{
+	return AD7091R5_REG_RESULT_CH_ID(val);
+}
+
 static const struct ad7091r_chip_info ad7091r5_chip_info_irq = {
 	.name = "ad7091r-5",
 	.channels = ad7091r5_channels_irq,
 	.num_channels = ARRAY_SIZE(ad7091r5_channels_irq),
 	.vref_mV = 2500,
+	.reg_result_chan_id = &ad7091r5_reg_result_chan_id,
 	.set_mode = &ad7091r5_set_mode,
 };
 
@@ -67,6 +73,7 @@ static const struct ad7091r_chip_info ad7091r5_chip_info_noirq = {
 	.channels = ad7091r5_channels_noirq,
 	.num_channels = ARRAY_SIZE(ad7091r5_channels_noirq),
 	.vref_mV = 2500,
+	.reg_result_chan_id = &ad7091r5_reg_result_chan_id,
 	.set_mode = &ad7091r5_set_mode,
 };
 
-- 
2.42.0

[PATCH v4 11/15] iio: adc: Split AD7091R-5 config symbol

[Marcelo Schmitt]

Split AD7091R-5 kconfig symbol into one symbol for the base AD7091R driver
and another one for the I2C interface AD7091R-5 driver.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/Kconfig  | 4 ++++
 drivers/iio/adc/Makefile | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 4eebd5161419..9d2d32d09166 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -36,9 +36,13 @@ config AD4130
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4130.
 
+config AD7091R
+	tristate
+
 config AD7091R5
 	tristate "Analog Devices AD7091R5 ADC Driver"
 	depends on I2C
+	select AD7091R
 	select REGMAP_I2C
 	help
 	  Say yes here to build support for Analog Devices AD7091R-5 ADC.
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index c0803383a7cc..1e289d674d4d 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -7,7 +7,8 @@
 obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD4130) += ad4130.o
-obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
+obj-$(CONFIG_AD7091R) += ad7091r-base.o
+obj-$(CONFIG_AD7091R5) += ad7091r5.o
 obj-$(CONFIG_AD7124) += ad7124.o
 obj-$(CONFIG_AD7192) += ad7192.o
 obj-$(CONFIG_AD7266) += ad7266.o
-- 
2.42.0

[PATCH v4 12/15] dt-bindings: iio: Add AD7091R-8

[Marcelo Schmitt]

Add device tree documentation for AD7091R-8.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 .../bindings/iio/adc/adi,ad7091r5.yaml        | 82 ++++++++++++++++++-
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml
index ce7ba634643c..ddec9747436c 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml
@@ -4,36 +4,92 @@
 $id: http://devicetree.org/schemas/iio/adc/adi,ad7091r5.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Analog Devices AD7091R5 4-Channel 12-Bit ADC
+title: Analog Devices AD7091R-2/-4/-5/-8 Multi-Channel 12-Bit ADCs
 
 maintainers:
   - Michael Hennerich <michael.hennerich@analog.com>
+  - Marcelo Schmitt <marcelo.schmitt@analog.com>
 
 description: |
-  Analog Devices AD7091R5 4-Channel 12-Bit ADC
+  Analog Devices AD7091R5 4-Channel 12-Bit ADC supporting I2C interface
   https://www.analog.com/media/en/technical-documentation/data-sheets/ad7091r-5.pdf
+  Analog Devices AD7091R-2/AD7091R-4/AD7091R-8 2-/4-/8-Channel 12-Bit ADCs
+  supporting SPI interface
+  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7091R-2_7091R-4_7091R-8.pdf
 
 properties:
   compatible:
     enum:
+      - adi,ad7091r2
+      - adi,ad7091r4
       - adi,ad7091r5
+      - adi,ad7091r8
 
   reg:
     maxItems: 1
 
+  vdd-supply:
+    description:
+      Provide VDD power to the sensor (VDD range is from 2.7V to 5.25V).
+
+  vdrive-supply:
+    description:
+      Determines the voltage level at which the interface logic will operate.
+      The V_drive voltage range is from 1.8V to 5.25V and must not exceed VDD by
+      more than 0.3V.
+
   vref-supply:
     description:
       Phandle to the vref power supply
 
-  interrupts:
+  convst-gpios:
+    description:
+      GPIO connected to the CONVST pin.
+      This logic input is used to initiate conversions on the analog
+      input channels.
     maxItems: 1
 
+  reset-gpios:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt for signaling when conversion results exceed the high limit for
+      ADC readings or fall below the low limit for them. Interrupt source must
+      be attached to ALERT/BUSY/GPO0 pin.
+    maxItems: 1
 
 required:
   - compatible
   - reg
 
-additionalProperties: false
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+  # AD7091R-2 does not have ALERT/BUSY/GPO pin
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7091r2
+    then:
+      properties:
+        interrupts: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7091r2
+              - adi,ad7091r4
+              - adi,ad7091r8
+    then:
+      required:
+        - convst-gpios
+
+unevaluatedProperties: false
 
 examples:
   - |
@@ -51,4 +107,22 @@ examples:
             interrupt-parent = <&gpio>;
         };
     };
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad7091r8";
+            reg = <0x0>;
+            spi-max-frequency = <1000000>;
+            vref-supply = <&adc_vref>;
+            convst-gpios = <&gpio 25 GPIO_ACTIVE_LOW>;
+            reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
+            interrupts = <22 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio>;
+        };
+    };
 ...
-- 
2.42.0

Re: [PATCH v4 12/15] dt-bindings: iio: Add AD7091R-8

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4269 bytes --]

On Sat, Dec 16, 2023 at 02:50:11PM -0300, Marcelo Schmitt wrote:
> Add device tree documentation for AD7091R-8.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> ---
>  .../bindings/iio/adc/adi,ad7091r5.yaml        | 82 ++++++++++++++++++-
>  1 file changed, 78 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml
> index ce7ba634643c..ddec9747436c 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml
> @@ -4,36 +4,92 @@
>  $id: http://devicetree.org/schemas/iio/adc/adi,ad7091r5.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Analog Devices AD7091R5 4-Channel 12-Bit ADC
> +title: Analog Devices AD7091R-2/-4/-5/-8 Multi-Channel 12-Bit ADCs
>  
>  maintainers:
>    - Michael Hennerich <michael.hennerich@analog.com>
> +  - Marcelo Schmitt <marcelo.schmitt@analog.com>
>  
>  description: |
> -  Analog Devices AD7091R5 4-Channel 12-Bit ADC
> +  Analog Devices AD7091R5 4-Channel 12-Bit ADC supporting I2C interface
>    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7091r-5.pdf
> +  Analog Devices AD7091R-2/AD7091R-4/AD7091R-8 2-/4-/8-Channel 12-Bit ADCs
> +  supporting SPI interface
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7091R-2_7091R-4_7091R-8.pdf
>  
>  properties:
>    compatible:
>      enum:
> +      - adi,ad7091r2
> +      - adi,ad7091r4
>        - adi,ad7091r5
> +      - adi,ad7091r8
>  
>    reg:
>      maxItems: 1
>  
> +  vdd-supply:
> +    description:
> +      Provide VDD power to the sensor (VDD range is from 2.7V to 5.25V).
> +
> +  vdrive-supply:
> +    description:
> +      Determines the voltage level at which the interface logic will operate.
> +      The V_drive voltage range is from 1.8V to 5.25V and must not exceed VDD by
> +      more than 0.3V.
> +
>    vref-supply:
>      description:
>        Phandle to the vref power supply
>  
> -  interrupts:
> +  convst-gpios:
> +    description:
> +      GPIO connected to the CONVST pin.
> +      This logic input is used to initiate conversions on the analog
> +      input channels.
>      maxItems: 1
>  
> +  reset-gpios:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Interrupt for signaling when conversion results exceed the high limit for
> +      ADC readings or fall below the low limit for them. Interrupt source must
> +      be attached to ALERT/BUSY/GPO0 pin.
> +    maxItems: 1
>  
>  required:
>    - compatible
>    - reg
>  
> -additionalProperties: false
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +  # AD7091R-2 does not have ALERT/BUSY/GPO pin
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad7091r2
> +    then:
> +      properties:
> +        interrupts: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad7091r2
> +              - adi,ad7091r4
> +              - adi,ad7091r8
> +    then:
> +      required:
> +        - convst-gpios
> +
> +unevaluatedProperties: false
>  
>  examples:
>    - |
> @@ -51,4 +107,22 @@ examples:
>              interrupt-parent = <&gpio>;
>          };
>      };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad7091r8";
> +            reg = <0x0>;
> +            spi-max-frequency = <1000000>;
> +            vref-supply = <&adc_vref>;
> +            convst-gpios = <&gpio 25 GPIO_ACTIVE_LOW>;
> +            reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> +            interrupts = <22 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-parent = <&gpio>;
> +        };
> +    };
>  ...
> -- 
> 2.42.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v4 13/15] iio: adc: Add support for AD7091R-8

[Marcelo Schmitt]

Add support for Analog Devices AD7091R-2, AD7091R-4, and AD7091R-8
low power 12-Bit SAR ADCs with SPI interface.
Extend ad7091r-base driver so it can be used by the AD7091R-8 driver.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 drivers/iio/adc/Kconfig        |  12 ++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/ad7091r-base.c |   7 +
 drivers/iio/adc/ad7091r-base.h |   7 +
 drivers/iio/adc/ad7091r8.c     | 257 +++++++++++++++++++++++++++++++++
 5 files changed, 284 insertions(+)
 create mode 100644 drivers/iio/adc/ad7091r8.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9d2d32d09166..3b73c509bd68 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -47,6 +47,18 @@ config AD7091R5
 	help
 	  Say yes here to build support for Analog Devices AD7091R-5 ADC.
 
+config AD7091R8
+	tristate "Analog Devices AD7091R8 ADC Driver"
+	depends on SPI
+	select AD7091R
+	select REGMAP_SPI
+	help
+	  Say yes here to build support for Analog Devices AD7091R-2, AD7091R-4,
+	  and AD7091R-8 ADC.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad7091r8.
+
 config AD7124
 	tristate "Analog Devices AD7124 and similar sigma-delta ADCs driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 1e289d674d4d..d2fda54a3259 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD7091R) += ad7091r-base.o
 obj-$(CONFIG_AD7091R5) += ad7091r5.o
+obj-$(CONFIG_AD7091R8) += ad7091r8.o
 obj-$(CONFIG_AD7124) += ad7124.o
 obj-$(CONFIG_AD7192) += ad7192.o
 obj-$(CONFIG_AD7266) += ad7266.o
diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index a30dc587ce45..57355ca157a1 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/bitfield.h>
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/interrupt.h>
@@ -187,6 +188,12 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
 	iio_dev->info = &ad7091r_info;
 	iio_dev->modes = INDIO_DIRECT_MODE;
 
+	if (init_info->setup) {
+		ret = init_info->setup(st);
+		if (ret < 0)
+			return ret;
+	}
+
 	if (irq) {
 		st->chip_info = &init_info->irq_info;
 		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
index 2b4e25e766c8..994505a740b3 100644
--- a/drivers/iio/adc/ad7091r-base.h
+++ b/drivers/iio/adc/ad7091r-base.h
@@ -45,6 +45,8 @@
 	.scan_type.realbits = bits,					\
 }
 
+#include <linux/gpio/consumer.h>
+
 struct device;
 
 enum ad7091r_mode {
@@ -56,10 +58,14 @@ enum ad7091r_mode {
 struct ad7091r_state {
 	struct device *dev;
 	struct regmap *map;
+	struct gpio_desc *convst_gpio;
+	struct gpio_desc *reset_gpio;
 	struct regulator *vref;
 	const struct ad7091r_chip_info *chip_info;
 	enum ad7091r_mode mode;
 	struct mutex lock; /*lock to prevent concurent reads */
+	__be16 tx_buf __aligned(IIO_DMA_MINALIGN);
+	__be16 rx_buf;
 };
 
 struct ad7091r_chip_info {
@@ -77,6 +83,7 @@ struct ad7091r_init_info {
 	const struct regmap_config *regmap_config;
 	void (*init_adc_regmap)(struct ad7091r_state *st,
 				const struct regmap_config *regmap_conf);
+	int (*setup)(struct ad7091r_state *st);
 };
 
 extern const struct iio_event_spec ad7091r_events[3];
diff --git a/drivers/iio/adc/ad7091r8.c b/drivers/iio/adc/ad7091r8.c
new file mode 100644
index 000000000000..0a6da47d89c0
--- /dev/null
+++ b/drivers/iio/adc/ad7091r8.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Analog Devices AD7091R8 12-bit SAR ADC driver
+ *
+ * Copyright 2023 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
+#include <linux/spi/spi.h>
+
+#include "ad7091r-base.h"
+
+#define AD7091R8_REG_ADDR_MSK				GENMASK(15, 11)
+#define AD7091R8_RD_WR_FLAG_MSK				BIT(10)
+#define AD7091R8_REG_DATA_MSK				GENMASK(9, 0)
+
+#define AD7091R_SPI_REGMAP_CONFIG(n) {					\
+	.reg_bits = 8,							\
+	.val_bits = 16,							\
+	.volatile_reg = ad7091r_volatile_reg,				\
+	.writeable_reg = ad7091r_writeable_reg,				\
+	.max_register = AD7091R_REG_CH_HYSTERESIS(n),			\
+}
+
+#define AD7091R_SPI_CHIP_INFO(_n, _name) {				\
+	.name = _name,							\
+	.channels = ad7091r##_n##_channels,				\
+	.num_channels = ARRAY_SIZE(ad7091r##_n##_channels),		\
+	.vref_mV = 2500,						\
+	.reg_result_chan_id = &ad7091r8_reg_result_chan_id,	\
+	.set_mode = &ad7091r8_set_mode,				\
+}
+
+#define AD7091R_SPI_CHIP_INFO_IRQ(_n, _name) {				\
+	.name = _name,							\
+	.channels = ad7091r##_n##_channels_irq,				\
+	.num_channels = ARRAY_SIZE(ad7091r##_n##_channels_irq),		\
+	.vref_mV = 2500,						\
+	.reg_result_chan_id = &ad7091r8_reg_result_chan_id,	\
+	.set_mode = &ad7091r8_set_mode,				\
+}
+
+static const struct iio_chan_spec ad7091r2_channels[] = {
+	AD7091R_CHANNEL(0, 12, NULL, 0),
+	AD7091R_CHANNEL(1, 12, NULL, 0),
+};
+
+static const struct iio_chan_spec ad7091r4_channels[] = {
+	AD7091R_CHANNEL(0, 12, NULL, 0),
+	AD7091R_CHANNEL(1, 12, NULL, 0),
+	AD7091R_CHANNEL(2, 12, NULL, 0),
+	AD7091R_CHANNEL(3, 12, NULL, 0),
+};
+
+static const struct iio_chan_spec ad7091r4_channels_irq[] = {
+	AD7091R_CHANNEL(0, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(1, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(2, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(3, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+};
+
+static const struct iio_chan_spec ad7091r8_channels[] = {
+	AD7091R_CHANNEL(0, 12, NULL, 0),
+	AD7091R_CHANNEL(1, 12, NULL, 0),
+	AD7091R_CHANNEL(2, 12, NULL, 0),
+	AD7091R_CHANNEL(3, 12, NULL, 0),
+	AD7091R_CHANNEL(4, 12, NULL, 0),
+	AD7091R_CHANNEL(5, 12, NULL, 0),
+	AD7091R_CHANNEL(6, 12, NULL, 0),
+	AD7091R_CHANNEL(7, 12, NULL, 0),
+};
+
+static const struct iio_chan_spec ad7091r8_channels_irq[] = {
+	AD7091R_CHANNEL(0, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(1, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(2, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(3, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(4, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(5, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(6, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+	AD7091R_CHANNEL(7, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
+};
+
+static void ad7091r_pulse_convst(struct ad7091r_state *st)
+{
+	gpiod_set_value_cansleep(st->convst_gpio, 1);
+	gpiod_set_value_cansleep(st->convst_gpio, 0);
+}
+
+static int ad7091r_regmap_bus_reg_read(void *context, unsigned int reg,
+				       unsigned int *val)
+{
+	struct ad7091r_state *st = context;
+	struct spi_device *spi = container_of(st->dev, struct spi_device, dev);
+	int ret;
+
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->tx_buf,
+			.len = 2,
+			.cs_change = 1,
+		}, {
+			.rx_buf = &st->rx_buf,
+			.len = 2,
+		}
+	};
+
+	if (reg == AD7091R_REG_RESULT)
+		ad7091r_pulse_convst(st);
+
+	st->tx_buf = cpu_to_be16(reg << 11);
+
+	ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	*val = be16_to_cpu(st->rx_buf);
+	return 0;
+}
+
+static int ad7091r_regmap_bus_reg_write(void *context, unsigned int reg,
+					unsigned int val)
+{
+	struct ad7091r_state *st = context;
+	struct spi_device *spi = container_of(st->dev, struct spi_device, dev);
+
+	/*
+	 * AD7091R-2/-4/-8 protocol (datasheet page 31) is to do a single SPI
+	 * transfer with reg address set in bits B15:B11 and value set in B9:B0.
+	 */
+	st->tx_buf = cpu_to_be16(FIELD_PREP(AD7091R8_REG_DATA_MSK, val) |
+				 FIELD_PREP(AD7091R8_RD_WR_FLAG_MSK, 1) |
+				 FIELD_PREP(AD7091R8_REG_ADDR_MSK, reg));
+
+	return spi_write(spi, &st->tx_buf, 2);
+}
+
+static struct regmap_bus ad7091r8_regmap_bus = {
+	.reg_read = ad7091r_regmap_bus_reg_read,
+	.reg_write = ad7091r_regmap_bus_reg_write,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
+static const struct regmap_config ad7091r2_reg_conf = AD7091R_SPI_REGMAP_CONFIG(2);
+static const struct regmap_config ad7091r4_reg_conf = AD7091R_SPI_REGMAP_CONFIG(4);
+static const struct regmap_config ad7091r8_reg_conf = AD7091R_SPI_REGMAP_CONFIG(8);
+
+static void ad7091r8_regmap_init(struct ad7091r_state *st,
+				 const struct regmap_config *regmap_conf)
+{
+	st->map = devm_regmap_init(st->dev, &ad7091r8_regmap_bus, st,
+				   regmap_conf);
+}
+
+static int ad7091r8_gpio_setup(struct ad7091r_state *st)
+{
+	st->convst_gpio = devm_gpiod_get(st->dev, "adi,conversion-start",
+					 GPIOD_OUT_LOW);
+	if (IS_ERR(st->convst_gpio))
+		return dev_err_probe(st->dev, PTR_ERR(st->convst_gpio),
+				     "Error getting convst GPIO\n");
+
+	st->reset_gpio =  devm_gpiod_get_optional(st->dev, "reset",
+						  GPIOD_OUT_HIGH);
+	if (IS_ERR(st->reset_gpio))
+		return dev_err_probe(st->dev, PTR_ERR(st->convst_gpio),
+				     "Error on requesting reset GPIO\n");
+
+	if (st->reset_gpio) {
+		fsleep(20);
+		gpiod_set_value_cansleep(st->reset_gpio, 0);
+	}
+
+	return 0;
+}
+
+static int ad7091r8_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
+{
+	/* AD7091R-2/-4/-8 don't set sample/command/autocycle mode in conf reg */
+	st->mode = mode;
+	return 0;
+}
+
+static unsigned int ad7091r8_reg_result_chan_id(unsigned int val)
+{
+	return AD7091R8_REG_RESULT_CH_ID(val);
+}
+
+static struct ad7091r_init_info ad7091r2_init_info = {
+	.info_no_irq = AD7091R_SPI_CHIP_INFO(2, "ad7091r-2"),
+	.regmap_config = &ad7091r2_reg_conf,
+	.init_adc_regmap = &ad7091r8_regmap_init,
+	.setup = &ad7091r8_gpio_setup
+};
+
+static struct ad7091r_init_info ad7091r4_init_info = {
+	.irq_info = AD7091R_SPI_CHIP_INFO_IRQ(4, "ad7091r-4"),
+	.info_no_irq = AD7091R_SPI_CHIP_INFO(4, "ad7091r-4"),
+	.regmap_config = &ad7091r4_reg_conf,
+	.init_adc_regmap = &ad7091r8_regmap_init,
+	.setup = &ad7091r8_gpio_setup
+};
+
+static struct ad7091r_init_info ad7091r8_init_info = {
+	.irq_info = AD7091R_SPI_CHIP_INFO_IRQ(8, "ad7091r-8"),
+	.info_no_irq = AD7091R_SPI_CHIP_INFO(8, "ad7091r-8"),
+	.regmap_config = &ad7091r8_reg_conf,
+	.init_adc_regmap = &ad7091r8_regmap_init,
+	.setup = &ad7091r8_gpio_setup
+};
+
+static int ad7091r8_spi_probe(struct spi_device *spi)
+{
+	const struct ad7091r_init_info *init_info;
+
+	init_info = spi_get_device_match_data(spi);
+	if (!init_info)
+		return -EINVAL;
+
+	return ad7091r_probe(&spi->dev, init_info, spi->irq);
+}
+
+static const struct of_device_id ad7091r8_of_match[] = {
+	{ .compatible = "adi,ad7091r2", .data = &ad7091r2_init_info },
+	{ .compatible = "adi,ad7091r4", .data = &ad7091r4_init_info },
+	{ .compatible = "adi,ad7091r8", .data = &ad7091r8_init_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad7091r8_of_match);
+
+static const struct spi_device_id ad7091r8_spi_id[] = {
+	{ "ad7091r2", (kernel_ulong_t)&ad7091r2_init_info },
+	{ "ad7091r4", (kernel_ulong_t)&ad7091r4_init_info },
+	{ "ad7091r8", (kernel_ulong_t)&ad7091r8_init_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad7091r8_spi_id);
+
+static struct spi_driver ad7091r8_driver = {
+	.driver = {
+		.name = "ad7091r8",
+		.of_match_table = ad7091r8_of_match,
+	},
+	.probe = ad7091r8_spi_probe,
+	.id_table = ad7091r8_spi_id,
+};
+module_spi_driver(ad7091r8_driver);
+
+MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD7091R8 ADC driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_AD7091R);
-- 
2.42.0

Re: [PATCH v4 13/15] iio: adc: Add support for AD7091R-8

[Jonathan Cameron]

On Sat, 16 Dec 2023 14:50:44 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Add support for Analog Devices AD7091R-2, AD7091R-4, and AD7091R-8
> low power 12-Bit SAR ADCs with SPI interface.
> Extend ad7091r-base driver so it can be used by the AD7091R-8 driver.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Hi Marcelo

A few trivial things from taking another look.

Jonathan

> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index a30dc587ce45..57355ca157a1 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/bitfield.h>

Not obvious what connection of this header to the code below is...
Wrong patch perhaps?
>  #include <linux/iio/events.h>
>  #include <linux/iio/iio.h>
>  #include <linux/interrupt.h>
> @@ -187,6 +188,12 @@ int ad7091r_probe(struct device *dev, const struct ad7091r_init_info *init_info,
>  	iio_dev->info = &ad7091r_info;
>  	iio_dev->modes = INDIO_DIRECT_MODE;
>  
> +	if (init_info->setup) {
> +		ret = init_info->setup(st);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	if (irq) {
>  		st->chip_info = &init_info->irq_info;
>  		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> index 2b4e25e766c8..994505a740b3 100644
> --- a/drivers/iio/adc/ad7091r-base.h
> +++ b/drivers/iio/adc/ad7091r-base.h
> @@ -45,6 +45,8 @@
>  	.scan_type.realbits = bits,					\
>  }
>  
> +#include <linux/gpio/consumer.h>
> +

struct gpio_desc;

and drop the include which only provides an opaque definition anyway.

>  struct device;
>  
>  enum ad7091r_mode {
> @@ -56,10 +58,14 @@ enum ad7091r_mode {
>  struct ad7091r_state {
>  	struct device *dev;
>  	struct regmap *map;
> +	struct gpio_desc *convst_gpio;
> +	struct gpio_desc *reset_gpio;
>  	struct regulator *vref;
>  	const struct ad7091r_chip_info *chip_info;
>  	enum ad7091r_mode mode;
>  	struct mutex lock; /*lock to prevent concurent reads */
> +	__be16 tx_buf __aligned(IIO_DMA_MINALIGN);
> +	__be16 rx_buf;
>  };
>  
>  struct ad7091r_chip_info {
> @@ -77,6 +83,7 @@ struct ad7091r_init_info {
>  	const struct regmap_config *regmap_config;
>  	void (*init_adc_regmap)(struct ad7091r_state *st,
>  				const struct regmap_config *regmap_conf);
> +	int (*setup)(struct ad7091r_state *st);
>  };
>  
>  extern const struct iio_event_spec ad7091r_events[3];
> diff --git a/drivers/iio/adc/ad7091r8.c b/drivers/iio/adc/ad7091r8.c
> new file mode 100644
> index 000000000000..0a6da47d89c0
> --- /dev/null
> +++ b/drivers/iio/adc/ad7091r8.c

> +static int ad7091r8_gpio_setup(struct ad7091r_state *st)
> +{
> +	st->convst_gpio = devm_gpiod_get(st->dev, "adi,conversion-start",
> +					 GPIOD_OUT_LOW);
> +	if (IS_ERR(st->convst_gpio))
> +		return dev_err_probe(st->dev, PTR_ERR(st->convst_gpio),
> +				     "Error getting convst GPIO\n");
> +
> +	st->reset_gpio =  devm_gpiod_get_optional(st->dev, "reset",

Trivial but looks like a bonus space after the =

> +						  GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->reset_gpio))
> +		return dev_err_probe(st->dev, PTR_ERR(st->convst_gpio),
> +				     "Error on requesting reset GPIO\n");
> +
> +	if (st->reset_gpio) {
> +		fsleep(20);
> +		gpiod_set_value_cansleep(st->reset_gpio, 0);
> +	}
> +
> +	return 0;
> +}

[PATCH v4 14/15] MAINTAINERS: Add MAINTAINERS entry for AD7091R

[Marcelo Schmitt]

The driver for AD7091R was added in
ca693001: iio: adc: Add support for AD7091R5 ADC
but no MAINTAINERS file entry was added for it since then.
Add a proper MAINTAINERS file entry for the AD7091R driver.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4eddc4212f2b..3473cfbac826 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1126,6 +1126,14 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
 F:	drivers/iio/adc/ad4130.c
 
+ANALOG DEVICES INC AD7091R DRIVER
+M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	http://ez.analog.com/community/linux-device-drivers
+F:	Documentation/devicetree/bindings/iio/adc/adi,ad7091r*
+F:	drivers/iio/adc/drivers/iio/adc/ad7091r*
+
 ANALOG DEVICES INC AD7192 DRIVER
 M:	Alexandru Tachici <alexandru.tachici@analog.com>
 L:	linux-iio@vger.kernel.org
-- 
2.42.0

[PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure device events

[Marcelo Schmitt]

Implement event configuration callbacks allowing users to read/write
event thresholds and enable/disable event generation.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
This is from a review suggestion David made on v3 [1].

Is this the case for a Suggested-by tag?

[1]: https://lore.kernel.org/linux-iio/CAMknhBFPbAqp4-AQdmbp+VRW-Ksk1PxaLCG+3n=Zk4gyStqhgw@mail.gmail.com/#t

 drivers/iio/adc/ad7091r-base.c | 117 +++++++++++++++++++++++++++++++--
 1 file changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
index 57355ca157a1..64e8baeff258 100644
--- a/drivers/iio/adc/ad7091r-base.c
+++ b/drivers/iio/adc/ad7091r-base.c
@@ -20,19 +20,18 @@ const struct iio_event_spec ad7091r_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
 		.dir = IIO_EV_DIR_RISING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-				 BIT(IIO_EV_INFO_ENABLE),
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
 	},
 	{
 		.type = IIO_EV_TYPE_THRESH,
 		.dir = IIO_EV_DIR_FALLING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-				 BIT(IIO_EV_INFO_ENABLE),
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
 	},
 	{
 		.type = IIO_EV_TYPE_THRESH,
 		.dir = IIO_EV_DIR_EITHER,
 		.mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
+		.mask_shared_by_all = BIT(IIO_EV_INFO_ENABLE),
 	},
 };
 EXPORT_SYMBOL_NS_GPL(ad7091r_events, IIO_AD7091R);
@@ -128,8 +127,118 @@ static int ad7091r_read_raw(struct iio_dev *iio_dev,
 	return ret;
 }
 
+static int ad7091r_read_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct ad7091r_state *st = iio_priv(indio_dev);
+	unsigned int alert;
+	int ret;
+
+	ret = regmap_read(st->map, AD7091R_REG_CONF, &alert);
+	if (ret)
+		return ret;
+
+	return !!(FIELD_GET(AD7091R_REG_CONF_ALERT_EN, alert));
+}
+
+static int ad7091r_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 ad7091r_state *st = iio_priv(indio_dev);
+
+	/*
+	 * Whenever alerts are enabled, every ADC conversion will generate
+	 * an alert if the conversion result for a particular channel falls
+	 * bellow the respective low limit register or above the respective
+	 * high limit register.
+	 * We can enable or disable all alerts by writing to the config reg.
+	 */
+	return regmap_update_bits(st->map, AD7091R_REG_CONF,
+				  AD7091R_REG_CONF_ALERT_EN, state << 4);
+}
+
+static int ad7091r_read_event_value(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 ad7091r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = regmap_read(st->map,
+					  AD7091R_REG_CH_HIGH_LIMIT(chan->channel),
+					  val);
+			if (ret)
+				return ret;
+			return IIO_VAL_INT;
+		case IIO_EV_DIR_FALLING:
+			ret = regmap_read(st->map,
+					  AD7091R_REG_CH_LOW_LIMIT(chan->channel),
+					  val);
+			if (ret)
+				return ret;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_INFO_HYSTERESIS:
+		ret = regmap_read(st->map,
+				  AD7091R_REG_CH_HYSTERESIS(chan->channel),
+				  val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7091r_write_event_value(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 ad7091r_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return regmap_write(st->map,
+					    AD7091R_REG_CH_HIGH_LIMIT(chan->channel),
+					    val);
+		case IIO_EV_DIR_FALLING:
+			return regmap_write(st->map,
+					    AD7091R_REG_CH_LOW_LIMIT(chan->channel),
+					    val);
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_INFO_HYSTERESIS:
+		return regmap_write(st->map,
+				    AD7091R_REG_CH_HYSTERESIS(chan->channel),
+				    val);
+	default:
+		return -EINVAL;
+	}
+}
+
 static const struct iio_info ad7091r_info = {
 	.read_raw = ad7091r_read_raw,
+	.read_event_config = &ad7091r_read_event_config,
+	.write_event_config = &ad7091r_write_event_config,
+	.read_event_value = &ad7091r_read_event_value,
+	.write_event_value = &ad7091r_write_event_value,
 };
 
 static irqreturn_t ad7091r_event_handler(int irq, void *private)
-- 
2.42.0

Re: [PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure device events

[Jonathan Cameron]

On Sat, 16 Dec 2023 14:51:50 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Implement event configuration callbacks allowing users to read/write
> event thresholds and enable/disable event generation.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> This is from a review suggestion David made on v3 [1].
> 
> Is this the case for a Suggested-by tag?
> 
> [1]: https://lore.kernel.org/linux-iio/CAMknhBFPbAqp4-AQdmbp+VRW-Ksk1PxaLCG+3n=Zk4gyStqhgw@mail.gmail.com/#t
> 
>  drivers/iio/adc/ad7091r-base.c | 117 +++++++++++++++++++++++++++++++--
>  1 file changed, 113 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index 57355ca157a1..64e8baeff258 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -20,19 +20,18 @@ const struct iio_event_spec ad7091r_events[] = {
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
>  		.dir = IIO_EV_DIR_RISING,
> -		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -				 BIT(IIO_EV_INFO_ENABLE),
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),

This is an ABI change.  So would need a really strong reason to make it...
mind you - it seems like this has been broken until now anyway so this change
may be fine.


>  	},
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
>  		.dir = IIO_EV_DIR_FALLING,
> -		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -				 BIT(IIO_EV_INFO_ENABLE),
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
>  	},
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
>  		.dir = IIO_EV_DIR_EITHER,
>  		.mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> +		.mask_shared_by_all = BIT(IIO_EV_INFO_ENABLE),
It's relatively unusual that you can't separate the two event directions with careful
control of the thresholds.  So I think you can implement the existing ABI by
just setting the thresholds to the either 0 or 2^12 - 1 as appropriate.
The docs seem to say it must exceed the value, or fall below the min so these values
should ensure it can't do either.

You can then enable the event generate if one of them is set.

Re: [PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure device events

[David Lechner]

On Sat, Dec 16, 2023 at 11:52 AM Marcelo Schmitt
<marcelo.schmitt@analog.com> wrote:
>
> Implement event configuration callbacks allowing users to read/write
> event thresholds and enable/disable event generation.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> This is from a review suggestion David made on v3 [1].
>
> Is this the case for a Suggested-by tag?
>
> [1]: https://lore.kernel.org/linux-iio/CAMknhBFPbAqp4-AQdmbp+VRW-Ksk1PxaLCG+3n=Zk4gyStqhgw@mail.gmail.com/#t
>

I suppose it fits the definition of Suggested-by well enough and would
be appreciated. Even more so on [PATCH v4 02/15] "iio: adc: ad7091r:
Pass iio_dev to event handler".

Re: [PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure device events

[David Lechner]

On Sun, Dec 17, 2023 at 5:58 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On Sat, Dec 16, 2023 at 11:52 AM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> >
> > Implement event configuration callbacks allowing users to read/write
> > event thresholds and enable/disable event generation.
> >
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > This is from a review suggestion David made on v3 [1].
> >
> > Is this the case for a Suggested-by tag?
> >
> > [1]: https://lore.kernel.org/linux-iio/CAMknhBFPbAqp4-AQdmbp+VRW-Ksk1PxaLCG+3n=Zk4gyStqhgw@mail.gmail.com/#t
> >
>
> I suppose it fits the definition of Suggested-by well enough and would
> be appreciated. Even more so on [PATCH v4 02/15] "iio: adc: ad7091r:
> Pass iio_dev to event handler".

And it seems like this should have the Fixes tag since this is fixing
a null pointer dereference. And the commit message should describe the
problem and that this as a fix, otherwise it sounds like we are just
adding a new feature here.

Re: [PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure device events

[David Lechner]

On Sat, Dec 16, 2023 at 11:52 AM Marcelo Schmitt
<marcelo.schmitt@analog.com> wrote:
>
> Implement event configuration callbacks allowing users to read/write
> event thresholds and enable/disable event generation.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> This is from a review suggestion David made on v3 [1].
>
> Is this the case for a Suggested-by tag?
>
> [1]: https://lore.kernel.org/linux-iio/CAMknhBFPbAqp4-AQdmbp+VRW-Ksk1PxaLCG+3n=Zk4gyStqhgw@mail.gmail.com/#t
>
>  drivers/iio/adc/ad7091r-base.c | 117 +++++++++++++++++++++++++++++++--
>  1 file changed, 113 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index 57355ca157a1..64e8baeff258 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -20,19 +20,18 @@ const struct iio_event_spec ad7091r_events[] = {
>         {
>                 .type = IIO_EV_TYPE_THRESH,
>                 .dir = IIO_EV_DIR_RISING,
> -               .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -                                BIT(IIO_EV_INFO_ENABLE),
> +               .mask_separate = BIT(IIO_EV_INFO_VALUE),
>         },
>         {
>                 .type = IIO_EV_TYPE_THRESH,
>                 .dir = IIO_EV_DIR_FALLING,
> -               .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -                                BIT(IIO_EV_INFO_ENABLE),
> +               .mask_separate = BIT(IIO_EV_INFO_VALUE),
>         },
>         {
>                 .type = IIO_EV_TYPE_THRESH,
>                 .dir = IIO_EV_DIR_EITHER,
>                 .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> +               .mask_shared_by_all = BIT(IIO_EV_INFO_ENABLE),
>         },
>  };
>  EXPORT_SYMBOL_NS_GPL(ad7091r_events, IIO_AD7091R);
> @@ -128,8 +127,118 @@ static int ad7091r_read_raw(struct iio_dev *iio_dev,
>         return ret;
>  }
>
> +static int ad7091r_read_event_config(struct iio_dev *indio_dev,
> +                                    const struct iio_chan_spec *chan,
> +                                    enum iio_event_type type,
> +                                    enum iio_event_direction dir)
> +{
> +       struct ad7091r_state *st = iio_priv(indio_dev);
> +       unsigned int alert;
> +       int ret;
> +
> +       ret = regmap_read(st->map, AD7091R_REG_CONF, &alert);
> +       if (ret)
> +               return ret;
> +
> +       return !!(FIELD_GET(AD7091R_REG_CONF_ALERT_EN, alert));
> +}

According to the datasheet, bit 4 of the config register is for
selecting the function of the ALERT/BUSY/GPO0 pin as either ALERT/BUSY
or GPIO, so this sounds like a pinmux function rather than an event
enable function.

context: `#define AD7091R_REG_CONF_ALERT_EN   BIT(4)` in a previous patch


> +
> +static int ad7091r_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 ad7091r_state *st = iio_priv(indio_dev);
> +
> +       /*
> +        * Whenever alerts are enabled, every ADC conversion will generate
> +        * an alert if the conversion result for a particular channel falls
> +        * bellow the respective low limit register or above the respective
> +        * high limit register.
> +        * We can enable or disable all alerts by writing to the config reg.
> +        */
> +       return regmap_update_bits(st->map, AD7091R_REG_CONF,
> +                                 AD7091R_REG_CONF_ALERT_EN, state << 4);
> +}
> +
> +static int ad7091r_read_event_value(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 ad7091r_state *st = iio_priv(indio_dev);
> +       int ret;
> +
> +       switch (info) {
> +       case IIO_EV_INFO_VALUE:
> +               switch (dir) {
> +               case IIO_EV_DIR_RISING:
> +                       ret = regmap_read(st->map,
> +                                         AD7091R_REG_CH_HIGH_LIMIT(chan->channel),
> +                                         val);
> +                       if (ret)
> +                               return ret;
> +                       return IIO_VAL_INT;
> +               case IIO_EV_DIR_FALLING:
> +                       ret = regmap_read(st->map,
> +                                         AD7091R_REG_CH_LOW_LIMIT(chan->channel),
> +                                         val);
> +                       if (ret)
> +                               return ret;
> +                       return IIO_VAL_INT;
> +               default:
> +                       return -EINVAL;
> +               }
> +       case IIO_EV_INFO_HYSTERESIS:
> +               ret = regmap_read(st->map,
> +                                 AD7091R_REG_CH_HYSTERESIS(chan->channel),
> +                                 val);
> +               if (ret)
> +                       return ret;
> +               return IIO_VAL_INT;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static int ad7091r_write_event_value(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 ad7091r_state *st = iio_priv(indio_dev);
> +
> +       switch (info) {
> +       case IIO_EV_INFO_VALUE:
> +               switch (dir) {
> +               case IIO_EV_DIR_RISING:
> +                       return regmap_write(st->map,
> +                                           AD7091R_REG_CH_HIGH_LIMIT(chan->channel),
> +                                           val);
> +               case IIO_EV_DIR_FALLING:
> +                       return regmap_write(st->map,
> +                                           AD7091R_REG_CH_LOW_LIMIT(chan->channel),
> +                                           val);

These registers are limited to 12-bit values. Do we need to check val
first and return -EINVAL if out of range?

> +               default:
> +                       return -EINVAL;
> +               }
> +       case IIO_EV_INFO_HYSTERESIS:
> +               return regmap_write(st->map,
> +                                   AD7091R_REG_CH_HYSTERESIS(chan->channel),
> +                                   val);
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
>  static const struct iio_info ad7091r_info = {
>         .read_raw = ad7091r_read_raw,
> +       .read_event_config = &ad7091r_read_event_config,
> +       .write_event_config = &ad7091r_write_event_config,
> +       .read_event_value = &ad7091r_read_event_value,
> +       .write_event_value = &ad7091r_write_event_value,
>  };
>
>  static irqreturn_t ad7091r_event_handler(int irq, void *private)
> --
> 2.42.0
>