Adds support for IIO Events. Optionally, gp0 is assigned as Threshold
Either signal, if not present, fallback to an I3C IBI with the same
role.
Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
drivers/iio/adc/ad4062.c | 351 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 347 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
index 40b7c10b8ce7145b010bb11e8e4698baacb6b3d3..b5b12f81c71b52f244600ed23dad11253290b868 100644
--- a/drivers/iio/adc/ad4062.c
+++ b/drivers/iio/adc/ad4062.c
@@ -13,6 +13,7 @@
#include <linux/i3c/device.h>
#include <linux/i3c/master.h>
#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
#include <linux/iio/trigger.h>
@@ -172,6 +173,8 @@ struct ad4062_state {
struct i3c_device *i3cdev;
struct regmap *regmap;
u16 sampling_frequency;
+ u16 events_frequency;
+ bool wait_event;
int vref_uv;
u8 raw[4] __aligned(IIO_DMA_MINALIGN);
};
@@ -202,6 +205,26 @@ static const struct regmap_access_table ad4062_regmap_wr_table = {
.n_yes_ranges = ARRAY_SIZE(ad4062_regmap_wr_ranges),
};
+static const struct iio_event_spec ad4062_events[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_shared_by_all = BIT(IIO_EV_INFO_ENABLE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_shared_by_all = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_HYSTERESIS),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_shared_by_all = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_HYSTERESIS),
+ },
+};
+
static const char *const ad4062_conversion_freqs[] = {
"2000000", "1000000", "300000", "100000", /* 0 - 3 */
"33300", "10000", "3000", "500", /* 4 - 7 */
@@ -263,6 +286,8 @@ AD4062_EXT_INFO(AD4062_2MSPS);
.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
.indexed = 1, \
.channel = 0, \
+ .event_spec = ad4062_events, \
+ .num_event_specs = ARRAY_SIZE(ad4062_events), \
.has_ext_scan_type = 1, \
.ext_scan_type = ad4062_scan_type_##bits##_s, \
.num_ext_scan_type = ARRAY_SIZE(ad4062_scan_type_##bits##_s), \
@@ -285,6 +310,82 @@ static const struct ad4062_chip_info ad4062_chip_info = {
.grade = AD4062_2MSPS,
};
+/**
+ * A register access will cause the device to drop from monitor mode
+ * into configuration mode, update the state to reflect that.
+ */
+static void ad4062_exit_monitor_mode(struct ad4062_state *st)
+{
+ if (st->wait_event) {
+ pm_runtime_mark_last_busy(&st->i3cdev->dev);
+ pm_runtime_put_autosuspend(&st->i3cdev->dev);
+ st->wait_event = 0;
+ }
+}
+
+static ssize_t ad4062_events_frequency_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ad4062_state *st = iio_priv(dev_to_iio_dev(dev));
+
+ return sysfs_emit(buf, "%s\n", ad4062_conversion_freqs[st->events_frequency]);
+}
+
+static ssize_t ad4062_events_frequency_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct ad4062_state *st = iio_priv(indio_dev);
+ int ret;
+
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+ ad4062_exit_monitor_mode(st);
+
+ ret = __sysfs_match_string(AD4062_FS(st->chip->grade),
+ AD4062_FS_LEN(st->chip->grade), buf);
+ if (ret < 0)
+ goto out_release;
+
+ st->events_frequency = ret;
+
+out_release:
+ iio_device_release_direct(indio_dev);
+ return ret ? ret : len;
+}
+
+static IIO_DEVICE_ATTR(sampling_frequency, 0644, ad4062_events_frequency_show,
+ ad4062_events_frequency_store, 0);
+
+static ssize_t sampling_frequency_available_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ad4062_state *st = iio_priv(dev_to_iio_dev(dev));
+ int ret = 0;
+
+ for (u8 i = AD4062_FS_OFFSET(st->chip->grade);
+ i < AD4062_FS_LEN(st->chip->grade); i++)
+ ret += sysfs_emit_at(buf, ret, "%s ", ad4062_conversion_freqs[i]);
+
+ ret += sysfs_emit_at(buf, ret, "\n");
+ return ret;
+}
+
+static IIO_DEVICE_ATTR_RO(sampling_frequency_available, 0);
+
+static struct attribute *ad4062_event_attributes[] = {
+ &iio_dev_attr_sampling_frequency.dev_attr.attr,
+ &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group ad4062_event_attribute_group = {
+ .attrs = ad4062_event_attributes,
+};
+
static int ad4062_set_oversampling_ratio(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
unsigned int val)
@@ -431,6 +532,19 @@ static int ad4062_setup(struct iio_dev *indio_dev, struct iio_chan_spec const *c
val);
}
+static irqreturn_t ad4062_irq_handler_thresh(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, 0,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_EITHER),
+ iio_get_time_ns(indio_dev));
+
+ return IRQ_HANDLED;
+}
+
static irqreturn_t ad4062_irq_handler_drdy(int irq, void *private)
{
struct iio_dev *indio_dev = private;
@@ -449,10 +563,18 @@ static void ad4062_ibi_handler(struct i3c_device *i3cdev,
{
struct ad4062_state *st = i3cdev_get_drvdata(i3cdev);
- if (iio_buffer_enabled(st->indio_dev))
- iio_trigger_poll(st->trigger);
- else
- complete(&st->completion);
+ if (st->wait_event) {
+ iio_push_event(st->indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, 0,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_EITHER),
+ iio_get_time_ns(st->indio_dev));
+ } else {
+ if (iio_buffer_enabled(st->indio_dev))
+ iio_trigger_poll(st->trigger);
+ else
+ complete(&st->completion);
+ }
}
static irqreturn_t ad4062_poll_handler(int irq, void *p)
@@ -523,6 +645,24 @@ static int ad4062_request_irq(struct iio_dev *indio_dev)
struct device *dev = &st->i3cdev->dev;
int ret;
+ ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp0");
+ if (ret >= 0) {
+ ret = devm_request_threaded_irq(dev, ret, NULL,
+ ad4062_irq_handler_thresh,
+ IRQF_ONESHOT, indio_dev->name,
+ indio_dev);
+ if (ret)
+ return ret;
+ } else if (ret != -EPROBE_DEFER) {
+ ret = regmap_update_bits(st->regmap, AD4062_REG_ADC_IBI_EN,
+ AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN,
+ AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN);
+ if (ret)
+ return ret;
+ } else {
+ return ret;
+ }
+
ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp1");
if (ret >= 0) {
ret = devm_request_threaded_irq(dev, ret, NULL,
@@ -734,6 +874,7 @@ static int ad4062_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val,
int *val2, long info)
{
+ struct ad4062_state *st = iio_priv(indio_dev);
int ret;
if (info == IIO_CHAN_INFO_SCALE)
@@ -741,6 +882,7 @@ static int ad4062_read_raw(struct iio_dev *indio_dev,
if (!iio_device_claim_direct(indio_dev))
return -EBUSY;
+ ad4062_exit_monitor_mode(st);
ret = ad4062_read_raw_dispatch(indio_dev, chan, val, val2, info);
@@ -768,10 +910,12 @@ static int ad4062_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int val,
int val2, long info)
{
+ struct ad4062_state *st = iio_priv(indio_dev);
int ret;
if (!iio_device_claim_direct(indio_dev))
return -EBUSY;
+ ad4062_exit_monitor_mode(st);
ret = ad4062_write_raw_dispatch(indio_dev, chan, val, val2, info);
@@ -779,6 +923,196 @@ static int ad4062_write_raw(struct iio_dev *indio_dev,
return ret;
}
+static int ad4062_monitor_mode_enable(struct ad4062_state *st, bool enable)
+{
+ int ret = 0;
+
+ if (!enable)
+ goto out_suspend;
+
+ ret = pm_runtime_resume_and_get(&st->i3cdev->dev);
+ if (ret)
+ return ret;
+
+ ret = ad4062_conversion_frequency_set(st, st->events_frequency);
+ if (ret)
+ goto out_suspend;
+
+ ret = ad4062_set_operation_mode(st, AD4062_MONITOR_MODE);
+ if (ret)
+ goto out_suspend;
+
+ return ret;
+out_suspend:
+ pm_runtime_put_autosuspend(&st->i3cdev->dev);
+ return ret;
+}
+
+static int ad4062_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 ad4062_state *st = iio_priv(indio_dev);
+
+ return st->wait_event;
+}
+
+static int ad4062_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ bool state)
+{
+ struct ad4062_state *st = iio_priv(indio_dev);
+ int ret;
+
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+ if (st->wait_event == state) {
+ ret = 0;
+ goto out_release;
+ }
+
+ ret = ad4062_monitor_mode_enable(st, state);
+ if (!ret)
+ st->wait_event = state;
+
+out_release:
+ iio_device_release_direct(indio_dev);
+ return ret;
+}
+
+static int __ad4062_read_event_info_value(struct ad4062_state *st,
+ enum iio_event_direction dir, int *val)
+{
+ int ret;
+ u8 reg;
+
+ if (dir == IIO_EV_DIR_RISING)
+ reg = AD4062_REG_MAX_LIMIT;
+ else
+ reg = AD4062_REG_MIN_LIMIT;
+
+ ret = regmap_bulk_read(st->regmap, reg, &st->raw, 2);
+ if (ret)
+ return ret;
+
+ *val = sign_extend32(get_unaligned_be16(st->raw), 11);
+
+ return 0;
+}
+
+static int __ad4062_read_event_info_hysteresis(struct ad4062_state *st,
+ enum iio_event_direction dir, int *val)
+{
+ u8 reg;
+
+ if (dir == IIO_EV_DIR_RISING)
+ reg = AD4062_REG_MAX_HYST;
+ else
+ reg = AD4062_REG_MIN_HYST;
+ return regmap_read(st->regmap, reg, val);
+}
+
+static int ad4062_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 ad4062_state *st = iio_priv(indio_dev);
+ int ret;
+
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+ ad4062_exit_monitor_mode(st);
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ ret = __ad4062_read_event_info_value(st, dir, val);
+ break;
+ case IIO_EV_INFO_HYSTERESIS:
+ ret = __ad4062_read_event_info_hysteresis(st, dir, val);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ iio_device_release_direct(indio_dev);
+ return ret ? ret : IIO_VAL_INT;
+}
+
+static int __ad4062_write_event_info_value(struct ad4062_state *st,
+ enum iio_event_direction dir, int val)
+{
+ u8 reg;
+
+ if (val > 2047 || val < -2048)
+ return -EINVAL;
+ if (dir == IIO_EV_DIR_RISING)
+ reg = AD4062_REG_MAX_LIMIT;
+ else
+ reg = AD4062_REG_MIN_LIMIT;
+ put_unaligned_be16(val, &st->raw);
+
+ return regmap_bulk_write(st->regmap, reg, &st->raw, 2);
+}
+
+static int __ad4062_write_event_info_hysteresis(struct ad4062_state *st,
+ enum iio_event_direction dir, int val)
+{
+ u8 reg;
+
+ if (val >= BIT(7))
+ return -EINVAL;
+ if (dir == IIO_EV_DIR_RISING)
+ reg = AD4062_REG_MAX_HYST;
+ else
+ reg = AD4062_REG_MIN_HYST;
+
+ return regmap_write(st->regmap, reg, val);
+}
+
+static int ad4062_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 ad4062_state *st = iio_priv(indio_dev);
+ int ret;
+
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+ ad4062_exit_monitor_mode(st);
+
+ switch (type) {
+ case IIO_EV_TYPE_THRESH:
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ ret = __ad4062_write_event_info_value(st, dir, val);
+ break;
+ case IIO_EV_INFO_HYSTERESIS:
+ ret = __ad4062_write_event_info_hysteresis(st, dir, val);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ iio_device_release_direct(indio_dev);
+ return ret;
+}
+
static int ad4062_triggered_buffer_postenable(struct iio_dev *indio_dev)
{
struct ad4062_state *st = iio_priv(indio_dev);
@@ -788,6 +1122,7 @@ static int ad4062_triggered_buffer_postenable(struct iio_dev *indio_dev)
ret = pm_runtime_resume_and_get(&st->i3cdev->dev);
if (ret)
return ret;
+ ad4062_exit_monitor_mode(st);
ret = ad4062_set_operation_mode(st, st->mode);
if (ret)
@@ -833,6 +1168,7 @@ static int ad4062_debugfs_reg_access(struct iio_dev *indio_dev, unsigned int reg
if (!iio_device_claim_direct(indio_dev))
return -EBUSY;
+ ad4062_exit_monitor_mode(st);
if (readval)
ret = regmap_read(st->regmap, reg, readval);
@@ -857,6 +1193,11 @@ static const struct iio_info ad4062_info = {
.read_raw = ad4062_read_raw,
.write_raw = ad4062_write_raw,
.read_avail = ad4062_read_avail,
+ .read_event_config = &ad4062_read_event_config,
+ .write_event_config = &ad4062_write_event_config,
+ .read_event_value = &ad4062_read_event_value,
+ .write_event_value = &ad4062_write_event_value,
+ .event_attrs = &ad4062_event_attribute_group,
.get_current_scan_type = &ad4062_get_current_scan_type,
.debugfs_reg_access = &ad4062_debugfs_reg_access,
};
@@ -932,8 +1273,10 @@ static int ad4062_probe(struct i3c_device *i3cdev)
"Failed to initialize regmap\n");
st->mode = AD4062_SAMPLE_MODE;
+ st->wait_event = false;
st->chip = chip;
st->sampling_frequency = AD4062_FS_OFFSET(st->chip->grade);
+ st->events_frequency = AD4062_FS_OFFSET(st->chip->grade);
st->indio_dev = indio_dev;
indio_dev->modes = INDIO_DIRECT_MODE;
--
2.49.0On Mon, 13 Oct 2025 09:28:05 +0200
Jorge Marques <jorge.marques@analog.com> wrote:
> Adds support for IIO Events. Optionally, gp0 is assigned as Threshold
> Either signal, if not present, fallback to an I3C IBI with the same
> role.
>
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The one bit of this that I'm not sure on is the apparent dropping out
of monitor mode on most userspace interactions that cause register accesses.
That seems like a fairly unintuitive ABI. It might be better to block the access
until the events are turned off. Perhaps I missed something?
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad4062.c | 351 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 347 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
> index 40b7c10b8ce7145b010bb11e8e4698baacb6b3d3..b5b12f81c71b52f244600ed23dad11253290b868 100644
> --- a/drivers/iio/adc/ad4062.c
> +++ b/drivers/iio/adc/ad4062.c
> @@ -13,6 +13,7 @@
> +/**
> + * A register access will cause the device to drop from monitor mode
> + * into configuration mode, update the state to reflect that.
> + */
> +static void ad4062_exit_monitor_mode(struct ad4062_state *st)
> +{
> + if (st->wait_event) {
> + pm_runtime_mark_last_busy(&st->i3cdev->dev);
> + pm_runtime_put_autosuspend(&st->i3cdev->dev);
As elsewhere, no longer need to have the mark_last_busy() call here.
> + st->wait_event = 0;
> + }
> +}
> +static ssize_t sampling_frequency_available_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ad4062_state *st = iio_priv(dev_to_iio_dev(dev));
> + int ret = 0;
> +
> + for (u8 i = AD4062_FS_OFFSET(st->chip->grade);
> + i < AD4062_FS_LEN(st->chip->grade); i++)
> + ret += sysfs_emit_at(buf, ret, "%s ", ad4062_conversion_freqs[i]);
> +
> + ret += sysfs_emit_at(buf, ret, "\n");
> + return ret;
Has slightly ugly format of " \n" at end rather than "\n"
There are various ways to handle this perhaps easiest is something like
for (u8 i = AD4062_FS_OFFSET(st->chip->grade);
i < AD4062_FS_LEN(st->chip->grade); i++)
ret += sysfs_emit_at(buf, ret, "%s%c", ad4062_conversion_freqs[i],
i != (AD4062_FS_LEN(st->chip_grade) - 1) ? "\n", " ");
> +}
> static irqreturn_t ad4062_poll_handler(int irq, void *p)
> @@ -523,6 +645,24 @@ static int ad4062_request_irq(struct iio_dev *indio_dev)
> struct device *dev = &st->i3cdev->dev;
> int ret;
>
> + ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp0");
> + if (ret >= 0) {
> + ret = devm_request_threaded_irq(dev, ret, NULL,
> + ad4062_irq_handler_thresh,
> + IRQF_ONESHOT, indio_dev->name,
> + indio_dev);
> + if (ret)
> + return ret;
> + } else if (ret != -EPROBE_DEFER) {
> + ret = regmap_update_bits(st->regmap, AD4062_REG_ADC_IBI_EN,
> + AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN,
> + AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN);
> + if (ret)
> + return ret;
> + } else {
> + return ret;
As before. I'd prefer error cases handled first. The earlier code suggestion
doesn't quite work but something along those lines should be doable.
> + }
> +
> ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp1");
> if (ret >= 0) {
> ret = devm_request_threaded_irq(dev, ret, NULL,
> @@ -779,6 +923,196 @@ static int ad4062_write_raw(struct iio_dev *indio_dev,
> return ret;
> }
>
> +static int ad4062_monitor_mode_enable(struct ad4062_state *st, bool enable)
> +{
> + int ret = 0;
> +
> + if (!enable)
> + goto out_suspend;
> +
> + ret = pm_runtime_resume_and_get(&st->i3cdev->dev);
> + if (ret)
> + return ret;
> +
> + ret = ad4062_conversion_frequency_set(st, st->events_frequency);
> + if (ret)
> + goto out_suspend;
> +
> + ret = ad4062_set_operation_mode(st, AD4062_MONITOR_MODE);
> + if (ret)
> + goto out_suspend;
> +
> + return ret;
return 0;
> +out_suspend:
> + pm_runtime_put_autosuspend(&st->i3cdev->dev);
> + return ret;
> +}
> static int ad4062_triggered_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct ad4062_state *st = iio_priv(indio_dev);
> @@ -788,6 +1122,7 @@ static int ad4062_triggered_buffer_postenable(struct iio_dev *indio_dev)
> ret = pm_runtime_resume_and_get(&st->i3cdev->dev);
> if (ret)
> return ret;
> + ad4062_exit_monitor_mode(st);
Hmm. So you always exist monitor mode if we enable the buffer. I assume that doesn't
change detection of events because the buffered mode also allows that?
Do we not need something to turn monitor mode on again once we disable buffered capture?
>
> ret = ad4062_set_operation_mode(st, st->mode);
> if (ret)
> @@ -833,6 +1168,7 @@ static int ad4062_debugfs_reg_access(struct iio_dev *indio_dev, unsigned int reg
>
> if (!iio_device_claim_direct(indio_dev))
> return -EBUSY;
> + ad4062_exit_monitor_mode(st);
This probably needs a comment. Not obvious to me how you end up in with it enabled
again after the debugfs read / write finishes.
>
> if (readval)
> ret = regmap_read(st->regmap, reg, readval);
On Sat, Oct 18, 2025 at 05:26:25PM +0100, Jonathan Cameron wrote:
> On Mon, 13 Oct 2025 09:28:05 +0200
> Jorge Marques <jorge.marques@analog.com> wrote:
>
> > Adds support for IIO Events. Optionally, gp0 is assigned as Threshold
> > Either signal, if not present, fallback to an I3C IBI with the same
> > role.
> >
> > Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Hi Jonathan,
> The one bit of this that I'm not sure on is the apparent dropping out
> of monitor mode on most userspace interactions that cause register accesses.
> That seems like a fairly unintuitive ABI. It might be better to block the access
> until the events are turned off. Perhaps I missed something?
>
I will change the behaviour to block access until monitor mode is disabled:
if (!iio_device_claim_direct(indio_dev))
return -EBUSY;
if (st->wait_event) {
ret = -EBUSY;
goto out_release;
}
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/iio/adc/ad4062.c | 351 ++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 347 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
> > index 40b7c10b8ce7145b010bb11e8e4698baacb6b3d3..b5b12f81c71b52f244600ed23dad11253290b868 100644
> > --- a/drivers/iio/adc/ad4062.c
> > +++ b/drivers/iio/adc/ad4062.c
> > @@ -13,6 +13,7 @@
>
> > +/**
> > + * A register access will cause the device to drop from monitor mode
> > + * into configuration mode, update the state to reflect that.
> > + */
> > +static void ad4062_exit_monitor_mode(struct ad4062_state *st)
> > +{
> > + if (st->wait_event) {
> > + pm_runtime_mark_last_busy(&st->i3cdev->dev);
>
> > + pm_runtime_put_autosuspend(&st->i3cdev->dev);
> As elsewhere, no longer need to have the mark_last_busy() call here.
>
Ack.
> > + st->wait_event = 0;
> > + }
> > +}
>
> > +static ssize_t sampling_frequency_available_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct ad4062_state *st = iio_priv(dev_to_iio_dev(dev));
> > + int ret = 0;
> > +
> > + for (u8 i = AD4062_FS_OFFSET(st->chip->grade);
> > + i < AD4062_FS_LEN(st->chip->grade); i++)
> > + ret += sysfs_emit_at(buf, ret, "%s ", ad4062_conversion_freqs[i]);
> > +
> > + ret += sysfs_emit_at(buf, ret, "\n");
> > + return ret;
>
> Has slightly ugly format of " \n" at end rather than "\n"
> There are various ways to handle this perhaps easiest is something like
> for (u8 i = AD4062_FS_OFFSET(st->chip->grade);
> i < AD4062_FS_LEN(st->chip->grade); i++)
> ret += sysfs_emit_at(buf, ret, "%s%c", ad4062_conversion_freqs[i],
> i != (AD4062_FS_LEN(st->chip_grade) - 1) ? "\n", " ");
Thanks for the suggestion, I will use.
>
>
> > +}
>
> > static irqreturn_t ad4062_poll_handler(int irq, void *p)
> > @@ -523,6 +645,24 @@ static int ad4062_request_irq(struct iio_dev *indio_dev)
> > struct device *dev = &st->i3cdev->dev;
> > int ret;
> >
> > + ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp0");
> > + if (ret >= 0) {
> > + ret = devm_request_threaded_irq(dev, ret, NULL,
> > + ad4062_irq_handler_thresh,
> > + IRQF_ONESHOT, indio_dev->name,
> > + indio_dev);
> > + if (ret)
> > + return ret;
> > + } else if (ret != -EPROBE_DEFER) {
> > + ret = regmap_update_bits(st->regmap, AD4062_REG_ADC_IBI_EN,
> > + AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN,
> > + AD4062_REG_ADC_IBI_EN_MAX | AD4062_REG_ADC_IBI_EN_MIN);
> > + if (ret)
> > + return ret;
> > + } else {
> > + return ret;
>
> As before. I'd prefer error cases handled first. The earlier code suggestion
> doesn't quite work but something along those lines should be doable.
>
Ack.
> > + }
> > +
> > ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp1");
> > if (ret >= 0) {
> > ret = devm_request_threaded_irq(dev, ret, NULL,
>
> > @@ -779,6 +923,196 @@ static int ad4062_write_raw(struct iio_dev *indio_dev,
> > return ret;
> > }
> >
> > +static int ad4062_monitor_mode_enable(struct ad4062_state *st, bool enable)
> > +{
> > + int ret = 0;
> > +
> > + if (!enable)
> > + goto out_suspend;
> > +
> > + ret = pm_runtime_resume_and_get(&st->i3cdev->dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ad4062_conversion_frequency_set(st, st->events_frequency);
> > + if (ret)
> > + goto out_suspend;
> > +
> > + ret = ad4062_set_operation_mode(st, AD4062_MONITOR_MODE);
> > + if (ret)
> > + goto out_suspend;
> > +
> > + return ret;
> return 0;
>
Ack.
> > +out_suspend:
> > + pm_runtime_put_autosuspend(&st->i3cdev->dev);
> > + return ret;
> > +}
>
> > static int ad4062_triggered_buffer_postenable(struct iio_dev *indio_dev)
> > {
> > struct ad4062_state *st = iio_priv(indio_dev);
> > @@ -788,6 +1122,7 @@ static int ad4062_triggered_buffer_postenable(struct iio_dev *indio_dev)
> > ret = pm_runtime_resume_and_get(&st->i3cdev->dev);
> > if (ret)
> > return ret;
> > + ad4062_exit_monitor_mode(st);
> Hmm. So you always exist monitor mode if we enable the buffer. I assume that doesn't
> change detection of events because the buffered mode also allows that?
>
> Do we not need something to turn monitor mode on again once we disable buffered capture?
No, the buffer mode does not work alongside monitor mode, I will add the
lock
if (st->wait_event)
return -EBUSY;
> >
> > ret = ad4062_set_operation_mode(st, st->mode);
> > if (ret)
> > @@ -833,6 +1168,7 @@ static int ad4062_debugfs_reg_access(struct iio_dev *indio_dev, unsigned int reg
> >
> > if (!iio_device_claim_direct(indio_dev))
> > return -EBUSY;
> > + ad4062_exit_monitor_mode(st);
>
> This probably needs a comment. Not obvious to me how you end up in with it enabled
> again after the debugfs read / write finishes.
>
You don't. Once a register access is done, it will drop of monitor mode
or sample/buffer mode. But since it is a debug interface, that may be
alright?
> >
> > if (readval)
> > ret = regmap_read(st->regmap, reg, readval);
Best regards,
Jorge
© 2016 - 2025 Red Hat, Inc.