* [PATCH 1/3] iio: Move attach/detach of the poll func to the core
@ 2020-05-22 10:46 Alexandru Ardelean
2020-05-22 10:46 ` [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks Alexandru Ardelean
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Alexandru Ardelean @ 2020-05-22 10:46 UTC (permalink / raw)
To: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32
Cc: jic23, shawnguo, s.hauer, mcoquelin.stm32, alexandre.torgue,
linus.walleij, lorenzo.bianconi83, songqiang1304521,
Lars-Peter Clausen, Alexandru Ardelean
From: Lars-Peter Clausen <lars@metafoo.de>
All devices using a triggered buffer need to attach and detach the trigger
to the device in order to properly work. Instead of doing this in each and
every driver by hand move this into the core.
At this point in time, all drivers should have been resolved to
attach/detach the poll-function in the same order.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../buffer/industrialio-triggered-buffer.c | 10 +--------
drivers/iio/iio_core_trigger.h | 17 ++++++++++++++
drivers/iio/industrialio-buffer.c | 13 +++++++++++
drivers/iio/industrialio-trigger.c | 22 ++++---------------
include/linux/iio/trigger_consumer.h | 7 ------
5 files changed, 35 insertions(+), 34 deletions(-)
diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
index e8046c1ecd6b..6c20a83f887e 100644
--- a/drivers/iio/buffer/industrialio-triggered-buffer.c
+++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
@@ -13,11 +13,6 @@
#include <linux/iio/triggered_buffer.h>
#include <linux/iio/trigger_consumer.h>
-static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
- .postenable = &iio_triggered_buffer_postenable,
- .predisable = &iio_triggered_buffer_predisable,
-};
-
/**
* iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc
* @indio_dev: IIO device structure
@@ -67,10 +62,7 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
}
/* Ring buffer functions - here trigger setup related */
- if (setup_ops)
- indio_dev->setup_ops = setup_ops;
- else
- indio_dev->setup_ops = &iio_triggered_buffer_setup_ops;
+ indio_dev->setup_ops = setup_ops;
/* Flag that polled ring buffering is possible */
indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
diff --git a/drivers/iio/iio_core_trigger.h b/drivers/iio/iio_core_trigger.h
index e59fe2f36bbb..9d1a92cc6480 100644
--- a/drivers/iio/iio_core_trigger.h
+++ b/drivers/iio/iio_core_trigger.h
@@ -18,6 +18,12 @@ void iio_device_register_trigger_consumer(struct iio_dev *indio_dev);
**/
void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev);
+
+int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf);
+int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf);
+
#else
/**
@@ -37,4 +43,15 @@ static void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
{
}
+static inline int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf)
+{
+ return 0;
+}
+static inline int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf)
+{
+ return 0;
+}
+
#endif /* CONFIG_TRIGGER_CONSUMER */
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index ec4f531994fa..88d756107fb2 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -20,6 +20,7 @@
#include <linux/iio/iio.h>
#include "iio_core.h"
+#include "iio_core_trigger.h"
#include <linux/iio/sysfs.h>
#include <linux/iio/buffer.h>
#include <linux/iio/buffer_impl.h>
@@ -972,6 +973,13 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
}
}
+ if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
+ ret = iio_trigger_attach_poll_func(indio_dev->trig,
+ indio_dev->pollfunc);
+ if (ret)
+ goto err_disable_buffers;
+ }
+
return 0;
err_disable_buffers:
@@ -998,6 +1006,11 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
if (list_empty(&indio_dev->buffer_list))
return 0;
+ if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
+ iio_trigger_detach_poll_func(indio_dev->trig,
+ indio_dev->pollfunc);
+ }
+
/*
* If things go wrong at some step in disable we still need to continue
* to perform the other steps, otherwise we leave the device in a
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 53d1931f6be8..6f16357fd732 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -239,8 +239,8 @@ static void iio_trigger_put_irq(struct iio_trigger *trig, int irq)
* the relevant function is in there may be the best option.
*/
/* Worth protecting against double additions? */
-static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
- struct iio_poll_func *pf)
+int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf)
{
int ret = 0;
bool notinuse
@@ -290,8 +290,8 @@ static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
return ret;
}
-static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
- struct iio_poll_func *pf)
+int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf)
{
int ret = 0;
bool no_other_users
@@ -705,17 +705,3 @@ void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
if (indio_dev->trig)
iio_trigger_put(indio_dev->trig);
}
-
-int iio_triggered_buffer_postenable(struct iio_dev *indio_dev)
-{
- return iio_trigger_attach_poll_func(indio_dev->trig,
- indio_dev->pollfunc);
-}
-EXPORT_SYMBOL(iio_triggered_buffer_postenable);
-
-int iio_triggered_buffer_predisable(struct iio_dev *indio_dev)
-{
- return iio_trigger_detach_poll_func(indio_dev->trig,
- indio_dev->pollfunc);
-}
-EXPORT_SYMBOL(iio_triggered_buffer_predisable);
diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
index c3c6ba5ec423..3aa2f132dd67 100644
--- a/include/linux/iio/trigger_consumer.h
+++ b/include/linux/iio/trigger_consumer.h
@@ -50,11 +50,4 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
void iio_trigger_notify_done(struct iio_trigger *trig);
-/*
- * Two functions for common case where all that happens is a pollfunc
- * is attached and detached from a trigger
- */
-int iio_triggered_buffer_postenable(struct iio_dev *indio_dev);
-int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
-
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks
2020-05-22 10:46 [PATCH 1/3] iio: Move attach/detach of the poll func to the core Alexandru Ardelean
@ 2020-05-22 10:46 ` Alexandru Ardelean
2020-05-24 13:54 ` Jonathan Cameron
2020-05-22 10:46 ` [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable() Alexandru Ardelean
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Alexandru Ardelean @ 2020-05-22 10:46 UTC (permalink / raw)
To: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32
Cc: jic23, shawnguo, s.hauer, mcoquelin.stm32, alexandre.torgue,
linus.walleij, lorenzo.bianconi83, songqiang1304521,
Alexandru Ardelean
This should be squashed into the first patch, but it's the more peculiar of
the changes.
I am not sure whether this is correct. The touchscreen channels shouldn't
be enabled by the IIO framework. So, we may need a different way to handle
those if needed.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/at91-sama5d2_adc.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 9abbbdcc7420..f71071096392 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -937,14 +937,6 @@ static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
return 0;
}
-static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
-{
- if (at91_adc_current_chan_is_touch(indio_dev))
- return 0;
-
- return iio_triggered_buffer_postenable(indio_dev);
-}
-
static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
{
struct at91_adc_state *st = iio_priv(indio_dev);
@@ -995,19 +987,9 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
return 0;
}
-static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
-{
- if (at91_adc_current_chan_is_touch(indio_dev))
- return 0;
-
- return iio_triggered_buffer_predisable(indio_dev);
-}
-
static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
.preenable = &at91_adc_buffer_preenable,
.postdisable = &at91_adc_buffer_postdisable,
- .postenable = &at91_adc_buffer_postenable,
- .predisable = &at91_adc_buffer_predisable,
};
static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable()
2020-05-22 10:46 [PATCH 1/3] iio: Move attach/detach of the poll func to the core Alexandru Ardelean
2020-05-22 10:46 ` [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks Alexandru Ardelean
@ 2020-05-22 10:46 ` Alexandru Ardelean
2020-05-24 13:38 ` Jonathan Cameron
2020-05-24 13:41 ` [PATCH 1/3] iio: Move attach/detach of the poll func to the core Jonathan Cameron
2020-07-14 14:57 ` Ardelean, Alexandru
3 siblings, 1 reply; 9+ messages in thread
From: Alexandru Ardelean @ 2020-05-22 10:46 UTC (permalink / raw)
To: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32
Cc: jic23, shawnguo, s.hauer, mcoquelin.stm32, alexandre.torgue,
linus.walleij, lorenzo.bianconi83, songqiang1304521,
Lars-Peter Clausen, Alexandru Ardelean
From: Lars-Peter Clausen <lars@metafoo.de>
This patch should be squashed into the first one, as the first one is
breaking the build (intentionally) to make the IIO core files easier to
review.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/accel/adxl372.c | 20 +++--------
drivers/iio/accel/bmc150-accel-core.c | 4 +--
drivers/iio/accel/kxcjk-1013.c | 2 --
drivers/iio/accel/kxsd9.c | 2 --
drivers/iio/accel/st_accel_buffer.c | 16 +++------
drivers/iio/accel/stk8312.c | 2 --
drivers/iio/accel/stk8ba50.c | 2 --
drivers/iio/adc/ad7266.c | 2 --
drivers/iio/adc/ad7606.c | 3 +-
drivers/iio/adc/ad7766.c | 2 --
drivers/iio/adc/ad7768-1.c | 8 +----
drivers/iio/adc/ad7887.c | 2 --
drivers/iio/adc/ad_sigma_delta.c | 5 ---
drivers/iio/adc/dln2-adc.c | 12 +------
drivers/iio/adc/mxs-lradc-adc.c | 2 --
drivers/iio/adc/stm32-adc.c | 36 +++----------------
drivers/iio/adc/stm32-dfsdm-adc.c | 39 +++------------------
drivers/iio/adc/ti-adc084s021.c | 2 --
drivers/iio/adc/ti-ads1015.c | 2 --
drivers/iio/adc/vf610_adc.c | 7 +---
drivers/iio/adc/xilinx-xadc-core.c | 2 --
drivers/iio/chemical/atlas-sensor.c | 6 +---
drivers/iio/dummy/iio_simple_dummy_buffer.c | 14 --------
drivers/iio/gyro/bmg160_core.c | 2 --
drivers/iio/gyro/mpu3050-core.c | 2 --
drivers/iio/gyro/st_gyro_buffer.c | 15 ++------
drivers/iio/humidity/hdc100x.c | 12 +------
drivers/iio/humidity/hts221_buffer.c | 2 --
drivers/iio/light/gp2ap020a00f.c | 10 ------
drivers/iio/light/isl29125.c | 20 ++---------
drivers/iio/light/rpr0521.c | 2 --
drivers/iio/light/si1145.c | 2 --
drivers/iio/light/st_uvis25_core.c | 2 --
drivers/iio/light/tcs3414.c | 20 ++---------
drivers/iio/light/vcnl4000.c | 29 +++++----------
drivers/iio/magnetometer/bmc150_magn.c | 2 --
drivers/iio/magnetometer/rm3100-core.c | 2 --
drivers/iio/magnetometer/st_magn_buffer.c | 26 ++------------
drivers/iio/potentiostat/lmp91000.c | 13 ++-----
drivers/iio/pressure/st_pressure_buffer.c | 26 ++------------
drivers/iio/pressure/zpa2326.c | 18 +++-------
drivers/iio/proximity/sx9310.c | 2 --
drivers/iio/proximity/sx9500.c | 9 -----
43 files changed, 50 insertions(+), 358 deletions(-)
diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 60daf04ce188..53697ca09a22 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -795,13 +795,9 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
unsigned int mask;
int i, ret;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret < 0)
- return ret;
-
ret = adxl372_set_interrupts(st, ADXL372_INT1_MAP_FIFO_FULL_MSK, 0);
if (ret < 0)
- goto err;
+ return ret;
mask = *indio_dev->active_scan_mask;
@@ -810,10 +806,8 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
break;
}
- if (i == ARRAY_SIZE(adxl372_axis_lookup_table)) {
- ret = -EINVAL;
- goto err;
- }
+ if (i == ARRAY_SIZE(adxl372_axis_lookup_table))
+ return -EINVAL;
st->fifo_format = adxl372_axis_lookup_table[i].fifo_format;
st->fifo_set_size = bitmap_weight(indio_dev->active_scan_mask,
@@ -833,14 +827,10 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
if (ret < 0) {
st->fifo_mode = ADXL372_FIFO_BYPASSED;
adxl372_set_interrupts(st, 0, 0);
- goto err;
+ return ret;
}
return 0;
-
-err:
- iio_triggered_buffer_predisable(indio_dev);
- return ret;
}
static int adxl372_buffer_predisable(struct iio_dev *indio_dev)
@@ -851,7 +841,7 @@ static int adxl372_buffer_predisable(struct iio_dev *indio_dev)
st->fifo_mode = ADXL372_FIFO_BYPASSED;
adxl372_configure_fifo(st);
- return iio_triggered_buffer_predisable(indio_dev);
+ return 0;
}
static const struct iio_buffer_setup_ops adxl372_buffer_ops = {
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 121b4e89f038..a2ff7033202d 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1411,7 +1411,7 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
int ret = 0;
if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
- return iio_triggered_buffer_postenable(indio_dev);
+ return 0;
mutex_lock(&data->mutex);
@@ -1443,7 +1443,7 @@ static int bmc150_accel_buffer_predisable(struct iio_dev *indio_dev)
struct bmc150_accel_data *data = iio_priv(indio_dev);
if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
- return iio_triggered_buffer_predisable(indio_dev);
+ return 0;
mutex_lock(&data->mutex);
diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index c9924a65c32a..209b0e189fd4 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1027,9 +1027,7 @@ static const struct iio_chan_spec kxcjk1013_channels[] = {
static const struct iio_buffer_setup_ops kxcjk1013_buffer_setup_ops = {
.preenable = kxcjk1013_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
.postdisable = kxcjk1013_buffer_postdisable,
- .predisable = iio_triggered_buffer_predisable,
};
static const struct iio_info kxcjk1013_info = {
diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 0b876b2dc5bd..39da8a090822 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -252,8 +252,6 @@ static int kxsd9_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops kxsd9_buffer_setup_ops = {
.preenable = kxsd9_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = kxsd9_buffer_postdisable,
};
diff --git a/drivers/iio/accel/st_accel_buffer.c b/drivers/iio/accel/st_accel_buffer.c
index b5c814ef1637..c87f9a7d2453 100644
--- a/drivers/iio/accel/st_accel_buffer.c
+++ b/drivers/iio/accel/st_accel_buffer.c
@@ -33,13 +33,9 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
{
int err;
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err < 0)
- return err;
-
err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
if (err < 0)
- goto st_accel_buffer_predisable;
+ return err;
err = st_sensors_set_enable(indio_dev, true);
if (err < 0)
@@ -49,8 +45,6 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
st_accel_buffer_enable_all_axis:
st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
-st_accel_buffer_predisable:
- iio_triggered_buffer_predisable(indio_dev);
return err;
}
@@ -60,12 +54,10 @@ static int st_accel_buffer_predisable(struct iio_dev *indio_dev)
err = st_sensors_set_enable(indio_dev, false);
if (err < 0)
- goto st_accel_buffer_predisable;
-
- err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
+ return err;
-st_accel_buffer_predisable:
- err2 = iio_triggered_buffer_predisable(indio_dev);
+ err2 = st_sensors_set_axis_enable(indio_dev,
+ ST_SENSORS_ENABLE_ALL_AXIS);
if (!err)
err = err2;
diff --git a/drivers/iio/accel/stk8312.c b/drivers/iio/accel/stk8312.c
index 58c160ccdee7..a7684545f12a 100644
--- a/drivers/iio/accel/stk8312.c
+++ b/drivers/iio/accel/stk8312.c
@@ -492,8 +492,6 @@ static int stk8312_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops stk8312_buffer_setup_ops = {
.preenable = stk8312_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = stk8312_buffer_postdisable,
};
diff --git a/drivers/iio/accel/stk8ba50.c b/drivers/iio/accel/stk8ba50.c
index c70ddec29eb4..1eec8dfceefe 100644
--- a/drivers/iio/accel/stk8ba50.c
+++ b/drivers/iio/accel/stk8ba50.c
@@ -376,8 +376,6 @@ static int stk8ba50_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops stk8ba50_buffer_setup_ops = {
.preenable = stk8ba50_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = stk8ba50_buffer_postdisable,
};
diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
index c8524f098883..d365c89d34ab 100644
--- a/drivers/iio/adc/ad7266.c
+++ b/drivers/iio/adc/ad7266.c
@@ -74,8 +74,6 @@ static int ad7266_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
.preenable = &ad7266_preenable,
- .postenable = &iio_triggered_buffer_postenable,
- .predisable = &iio_triggered_buffer_predisable,
.postdisable = &ad7266_postdisable,
};
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index e4683a68522a..a88e07869a69 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -499,7 +499,6 @@ static int ad7606_buffer_postenable(struct iio_dev *indio_dev)
{
struct ad7606_state *st = iio_priv(indio_dev);
- iio_triggered_buffer_postenable(indio_dev);
gpiod_set_value(st->gpio_convst, 1);
return 0;
@@ -511,7 +510,7 @@ static int ad7606_buffer_predisable(struct iio_dev *indio_dev)
gpiod_set_value(st->gpio_convst, 0);
- return iio_triggered_buffer_predisable(indio_dev);
+ return 0;
}
static const struct iio_buffer_setup_ops ad7606_buffer_ops = {
diff --git a/drivers/iio/adc/ad7766.c b/drivers/iio/adc/ad7766.c
index bc388ea41754..0b6d27cb5f81 100644
--- a/drivers/iio/adc/ad7766.c
+++ b/drivers/iio/adc/ad7766.c
@@ -178,8 +178,6 @@ static const struct ad7766_chip_info ad7766_chip_info[] = {
static const struct iio_buffer_setup_ops ad7766_buffer_setup_ops = {
.preenable = &ad7766_preenable,
- .postenable = &iio_triggered_buffer_postenable,
- .predisable = &iio_triggered_buffer_predisable,
.postdisable = &ad7766_postdisable,
};
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 0d132708c429..addd802ed5d7 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -490,7 +490,6 @@ static int ad7768_buffer_postenable(struct iio_dev *indio_dev)
{
struct ad7768_state *st = iio_priv(indio_dev);
- iio_triggered_buffer_postenable(indio_dev);
/*
* Write a 1 to the LSB of the INTERFACE_FORMAT register to enter
* continuous read mode. Subsequent data reads do not require an
@@ -502,17 +501,12 @@ static int ad7768_buffer_postenable(struct iio_dev *indio_dev)
static int ad7768_buffer_predisable(struct iio_dev *indio_dev)
{
struct ad7768_state *st = iio_priv(indio_dev);
- int ret;
/*
* To exit continuous read mode, perform a single read of the ADC_DATA
* reg (0x2C), which allows further configuration of the device.
*/
- ret = ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
- if (ret < 0)
- return ret;
-
- return iio_triggered_buffer_predisable(indio_dev);
+ return ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
}
static const struct iio_buffer_setup_ops ad7768_buffer_ops = {
diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c
index c6a3428e950a..7ce665601ce4 100644
--- a/drivers/iio/adc/ad7887.c
+++ b/drivers/iio/adc/ad7887.c
@@ -136,8 +136,6 @@ static irqreturn_t ad7887_trigger_handler(int irq, void *p)
static const struct iio_buffer_setup_ops ad7887_ring_setup_ops = {
.preenable = &ad7887_ring_preenable,
- .postenable = &iio_triggered_buffer_postenable,
- .predisable = &iio_triggered_buffer_predisable,
.postdisable = &ad7887_ring_postdisable,
};
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index dd3d54b3bc8b..3554ee6ee099 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -345,10 +345,6 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
unsigned int channel;
int ret;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret < 0)
- return ret;
-
channel = find_first_bit(indio_dev->active_scan_mask,
indio_dev->masklength);
ret = ad_sigma_delta_set_channel(sigma_delta,
@@ -441,7 +437,6 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p)
static const struct iio_buffer_setup_ops ad_sd_buffer_setup_ops = {
.postenable = &ad_sd_buffer_postenable,
- .predisable = &iio_triggered_buffer_predisable,
.postdisable = &ad_sd_buffer_postdisable,
.validate_scan_mask = &iio_validate_scan_mask_onehot,
};
diff --git a/drivers/iio/adc/dln2-adc.c b/drivers/iio/adc/dln2-adc.c
index 65c7c9329b1c..e4616b4a790a 100644
--- a/drivers/iio/adc/dln2-adc.c
+++ b/drivers/iio/adc/dln2-adc.c
@@ -524,10 +524,6 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
u16 conflict;
unsigned int trigger_chan;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret)
- return ret;
-
mutex_lock(&dln2->mutex);
/* Enable ADC */
@@ -541,7 +537,6 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
(int)conflict);
ret = -EBUSY;
}
- iio_triggered_buffer_predisable(indio_dev);
return ret;
}
@@ -555,7 +550,6 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
mutex_unlock(&dln2->mutex);
if (ret < 0) {
dev_dbg(&dln2->pdev->dev, "Problem in %s\n", __func__);
- iio_triggered_buffer_predisable(indio_dev);
return ret;
}
} else {
@@ -568,7 +562,7 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
static int dln2_adc_triggered_buffer_predisable(struct iio_dev *indio_dev)
{
- int ret, ret2;
+ int ret;
struct dln2_adc *dln2 = iio_priv(indio_dev);
mutex_lock(&dln2->mutex);
@@ -586,10 +580,6 @@ static int dln2_adc_triggered_buffer_predisable(struct iio_dev *indio_dev)
if (ret < 0)
dev_dbg(&dln2->pdev->dev, "Problem in %s\n", __func__);
- ret2 = iio_triggered_buffer_predisable(indio_dev);
- if (ret == 0)
- ret = ret2;
-
return ret;
}
diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c
index 9d2f74c2489a..7f49586944a9 100644
--- a/drivers/iio/adc/mxs-lradc-adc.c
+++ b/drivers/iio/adc/mxs-lradc-adc.c
@@ -568,8 +568,6 @@ static bool mxs_lradc_adc_validate_scan_mask(struct iio_dev *iio,
static const struct iio_buffer_setup_ops mxs_lradc_adc_buffer_ops = {
.preenable = &mxs_lradc_adc_buffer_preenable,
- .postenable = &iio_triggered_buffer_postenable,
- .predisable = &iio_triggered_buffer_predisable,
.postdisable = &mxs_lradc_adc_buffer_postdisable,
.validate_scan_mask = &mxs_lradc_adc_validate_scan_mask,
};
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index ae622ee6d08c..cbc7f0467ae8 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -1482,7 +1482,7 @@ static int stm32_adc_dma_start(struct iio_dev *indio_dev)
return 0;
}
-static int __stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
+static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
{
struct stm32_adc *adc = iio_priv(indio_dev);
struct device *dev = indio_dev->dev.parent;
@@ -1527,22 +1527,7 @@ static int __stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
return ret;
}
-static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
-{
- int ret;
-
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret < 0)
- return ret;
-
- ret = __stm32_adc_buffer_postenable(indio_dev);
- if (ret < 0)
- iio_triggered_buffer_predisable(indio_dev);
-
- return ret;
-}
-
-static void __stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
+static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
{
struct stm32_adc *adc = iio_priv(indio_dev);
struct device *dev = indio_dev->dev.parent;
@@ -1561,19 +1546,8 @@ static void __stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
-}
-
-static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
-{
- int ret;
-
- __stm32_adc_buffer_predisable(indio_dev);
-
- ret = iio_triggered_buffer_predisable(indio_dev);
- if (ret < 0)
- dev_err(&indio_dev->dev, "predisable failed\n");
- return ret;
+ return 0;
}
static const struct iio_buffer_setup_ops stm32_adc_buffer_setup_ops = {
@@ -2016,7 +1990,7 @@ static int stm32_adc_suspend(struct device *dev)
struct iio_dev *indio_dev = iio_priv_to_dev(adc);
if (iio_buffer_enabled(indio_dev))
- __stm32_adc_buffer_predisable(indio_dev);
+ stm32_adc_buffer_predisable(indio_dev);
return pm_runtime_force_suspend(dev);
}
@@ -2039,7 +2013,7 @@ static int stm32_adc_resume(struct device *dev)
if (ret < 0)
return ret;
- return __stm32_adc_buffer_postenable(indio_dev);
+ return stm32_adc_buffer_postenable(indio_dev);
}
#endif
diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index 76a60d93fe23..0f652165d781 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -993,7 +993,7 @@ static int stm32_dfsdm_update_scan_mode(struct iio_dev *indio_dev,
return 0;
}
-static int __stm32_dfsdm_postenable(struct iio_dev *indio_dev)
+static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
int ret;
@@ -1036,30 +1036,7 @@ static int __stm32_dfsdm_postenable(struct iio_dev *indio_dev)
return ret;
}
-static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
-{
- int ret;
-
- if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret < 0)
- return ret;
- }
-
- ret = __stm32_dfsdm_postenable(indio_dev);
- if (ret < 0)
- goto err_predisable;
-
- return 0;
-
-err_predisable:
- if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
- iio_triggered_buffer_predisable(indio_dev);
-
- return ret;
-}
-
-static void __stm32_dfsdm_predisable(struct iio_dev *indio_dev)
+static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
{
struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
@@ -1071,14 +1048,6 @@ static void __stm32_dfsdm_predisable(struct iio_dev *indio_dev)
if (adc->hwc)
iio_hw_consumer_disable(adc->hwc);
-}
-
-static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
-{
- __stm32_dfsdm_predisable(indio_dev);
-
- if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
- iio_triggered_buffer_predisable(indio_dev);
return 0;
}
@@ -1667,7 +1636,7 @@ static int __maybe_unused stm32_dfsdm_adc_suspend(struct device *dev)
struct iio_dev *indio_dev = iio_priv_to_dev(adc);
if (iio_buffer_enabled(indio_dev))
- __stm32_dfsdm_predisable(indio_dev);
+ stm32_dfsdm_predisable(indio_dev);
return 0;
}
@@ -1690,7 +1659,7 @@ static int __maybe_unused stm32_dfsdm_adc_resume(struct device *dev)
}
if (iio_buffer_enabled(indio_dev))
- __stm32_dfsdm_postenable(indio_dev);
+ stm32_dfsdm_postenable(indio_dev);
return 0;
}
diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
index bdedf456ee05..75ec8a21d28e 100644
--- a/drivers/iio/adc/ti-adc084s021.c
+++ b/drivers/iio/adc/ti-adc084s021.c
@@ -187,8 +187,6 @@ static const struct iio_info adc084s021_info = {
static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
.preenable = adc084s021_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = adc084s021_buffer_postdisable,
};
diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 5ea4f45d6bad..efb55c2bccf4 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -788,8 +788,6 @@ static int ads1015_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
.preenable = ads1015_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = ads1015_buffer_postdisable,
.validate_scan_mask = &iio_validate_scan_mask_onehot,
};
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index cb7380bf07ca..72b363ea4074 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -724,13 +724,8 @@ static int vf610_adc_buffer_postenable(struct iio_dev *indio_dev)
{
struct vf610_adc *info = iio_priv(indio_dev);
unsigned int channel;
- int ret;
int val;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret)
- return ret;
-
val = readl(info->regs + VF610_REG_ADC_GC);
val |= VF610_ADC_ADCON;
writel(val, info->regs + VF610_REG_ADC_GC);
@@ -761,7 +756,7 @@ static int vf610_adc_buffer_predisable(struct iio_dev *indio_dev)
writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
- return iio_triggered_buffer_predisable(indio_dev);
+ return 0;
}
static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index d7fecab9252e..2e81aa390978 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -839,8 +839,6 @@ static int xadc_preenable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops xadc_buffer_ops = {
.preenable = &xadc_preenable,
- .postenable = &iio_triggered_buffer_postenable,
- .predisable = &iio_triggered_buffer_predisable,
.postdisable = &xadc_postdisable,
};
diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
index a6d996ab9a66..362cf766a79a 100644
--- a/drivers/iio/chemical/atlas-sensor.c
+++ b/drivers/iio/chemical/atlas-sensor.c
@@ -398,10 +398,6 @@ static int atlas_buffer_postenable(struct iio_dev *indio_dev)
struct atlas_data *data = iio_priv(indio_dev);
int ret;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret)
- return ret;
-
ret = pm_runtime_get_sync(&data->client->dev);
if (ret < 0) {
pm_runtime_put_noidle(&data->client->dev);
@@ -425,7 +421,7 @@ static int atlas_buffer_predisable(struct iio_dev *indio_dev)
if (ret)
return ret;
- return iio_triggered_buffer_predisable(indio_dev);
+ return 0;
}
static const struct iio_trigger_ops atlas_interrupt_trigger_ops = {
diff --git a/drivers/iio/dummy/iio_simple_dummy_buffer.c b/drivers/iio/dummy/iio_simple_dummy_buffer.c
index 17606eca42b4..8e13c53d4360 100644
--- a/drivers/iio/dummy/iio_simple_dummy_buffer.c
+++ b/drivers/iio/dummy/iio_simple_dummy_buffer.c
@@ -99,20 +99,6 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
}
static const struct iio_buffer_setup_ops iio_simple_dummy_buffer_setup_ops = {
- /*
- * iio_triggered_buffer_postenable:
- * Generic function that simply attaches the pollfunc to the trigger.
- * Replace this to mess with hardware state before we attach the
- * trigger.
- */
- .postenable = &iio_triggered_buffer_postenable,
- /*
- * iio_triggered_buffer_predisable:
- * Generic function that simple detaches the pollfunc from the trigger.
- * Replace this to put hardware state back again after the trigger is
- * detached but before userspace knows we have disabled the ring.
- */
- .predisable = &iio_triggered_buffer_predisable,
};
int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 428ddfc13acb..0d0bfdd6036e 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -1051,8 +1051,6 @@ static int bmg160_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops bmg160_buffer_setup_ops = {
.preenable = bmg160_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = bmg160_buffer_postdisable,
};
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index 8e908a749f95..de1cd17d57f0 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -662,8 +662,6 @@ static int mpu3050_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops mpu3050_buffer_setup_ops = {
.preenable = mpu3050_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = mpu3050_buffer_postdisable,
};
diff --git a/drivers/iio/gyro/st_gyro_buffer.c b/drivers/iio/gyro/st_gyro_buffer.c
index 9c92ff7a82be..7b86502d5da3 100644
--- a/drivers/iio/gyro/st_gyro_buffer.c
+++ b/drivers/iio/gyro/st_gyro_buffer.c
@@ -33,13 +33,9 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
{
int err;
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err < 0)
- return err;
-
err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
if (err < 0)
- goto st_gyro_buffer_predisable;
+ return err;
err = st_sensors_set_enable(indio_dev, true);
if (err < 0)
@@ -49,8 +45,6 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
st_gyro_buffer_enable_all_axis:
st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
-st_gyro_buffer_predisable:
- iio_triggered_buffer_predisable(indio_dev);
return err;
}
@@ -59,13 +53,8 @@ static int st_gyro_buffer_predisable(struct iio_dev *indio_dev)
int err, err2;
err = st_sensors_set_enable(indio_dev, false);
- if (err < 0)
- goto st_gyro_buffer_predisable;
-
- err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
-st_gyro_buffer_predisable:
- err2 = iio_triggered_buffer_predisable(indio_dev);
+ err2 = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
if (!err)
err = err2;
diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index 7ecd2ffa3132..9243f8546cfb 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -278,17 +278,11 @@ static int hdc100x_buffer_postenable(struct iio_dev *indio_dev)
struct hdc100x_data *data = iio_priv(indio_dev);
int ret;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret)
- return ret;
-
/* Buffer is enabled. First set ACQ Mode, then attach poll func */
mutex_lock(&data->lock);
ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE,
HDC100X_REG_CONFIG_ACQ_MODE);
mutex_unlock(&data->lock);
- if (ret)
- iio_triggered_buffer_predisable(indio_dev);
return ret;
}
@@ -296,16 +290,12 @@ static int hdc100x_buffer_postenable(struct iio_dev *indio_dev)
static int hdc100x_buffer_predisable(struct iio_dev *indio_dev)
{
struct hdc100x_data *data = iio_priv(indio_dev);
- int ret, ret2;
+ int ret;
mutex_lock(&data->lock);
ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, 0);
mutex_unlock(&data->lock);
- ret2 = iio_triggered_buffer_predisable(indio_dev);
- if (ret == 0)
- ret = ret2;
-
return ret;
}
diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index 9fb3f33614d4..d18b39f30ea7 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -153,8 +153,6 @@ static int hts221_buffer_postdisable(struct iio_dev *iio_dev)
static const struct iio_buffer_setup_ops hts221_buffer_ops = {
.preenable = hts221_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = hts221_buffer_postdisable,
};
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 070d4cd0cf54..29d7af33efa1 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1390,12 +1390,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
mutex_lock(&data->lock);
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err < 0) {
- mutex_unlock(&data->lock);
- return err;
- }
-
/*
* Enable triggers according to the scan_mask. Enabling either
* LIGHT_CLEAR or LIGHT_IR scan mode results in enabling ALS
@@ -1430,8 +1424,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
err = -ENOMEM;
error_unlock:
- if (err < 0)
- iio_triggered_buffer_predisable(indio_dev);
mutex_unlock(&data->lock);
return err;
@@ -1465,8 +1457,6 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
if (err == 0)
kfree(data->buffer);
- iio_triggered_buffer_predisable(indio_dev);
-
mutex_unlock(&data->lock);
return err;
diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
index 95611f5eff01..76065433a967 100644
--- a/drivers/iio/light/isl29125.c
+++ b/drivers/iio/light/isl29125.c
@@ -216,36 +216,20 @@ static const struct iio_info isl29125_info = {
static int isl29125_buffer_postenable(struct iio_dev *indio_dev)
{
struct isl29125_data *data = iio_priv(indio_dev);
- int err;
-
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err)
- return err;
data->conf1 |= ISL29125_MODE_RGB;
- err = i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
+ return i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
data->conf1);
- if (err) {
- iio_triggered_buffer_predisable(indio_dev);
- return err;
- }
-
- return 0;
}
static int isl29125_buffer_predisable(struct iio_dev *indio_dev)
{
struct isl29125_data *data = iio_priv(indio_dev);
- int ret;
data->conf1 &= ~ISL29125_MODE_MASK;
data->conf1 |= ISL29125_MODE_PD;
- ret = i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
+ return i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
data->conf1);
-
- iio_triggered_buffer_predisable(indio_dev);
-
- return ret;
}
static const struct iio_buffer_setup_ops isl29125_buffer_setup_ops = {
diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
index a0a7aeae5a82..e43c050cc766 100644
--- a/drivers/iio/light/rpr0521.c
+++ b/drivers/iio/light/rpr0521.c
@@ -570,8 +570,6 @@ static int rpr0521_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops rpr0521_buffer_setup_ops = {
.preenable = rpr0521_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = rpr0521_buffer_postdisable,
};
diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
index 0476c2bc8138..d409389ba38e 100644
--- a/drivers/iio/light/si1145.c
+++ b/drivers/iio/light/si1145.c
@@ -1171,8 +1171,6 @@ static bool si1145_validate_scan_mask(struct iio_dev *indio_dev,
static const struct iio_buffer_setup_ops si1145_buffer_setup_ops = {
.preenable = si1145_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.validate_scan_mask = si1145_validate_scan_mask,
};
diff --git a/drivers/iio/light/st_uvis25_core.c b/drivers/iio/light/st_uvis25_core.c
index d262c254b895..0951dbeb5d04 100644
--- a/drivers/iio/light/st_uvis25_core.c
+++ b/drivers/iio/light/st_uvis25_core.c
@@ -227,8 +227,6 @@ static int st_uvis25_buffer_postdisable(struct iio_dev *iio_dev)
static const struct iio_buffer_setup_ops st_uvis25_buffer_ops = {
.preenable = st_uvis25_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = st_uvis25_buffer_postdisable,
};
diff --git a/drivers/iio/light/tcs3414.c b/drivers/iio/light/tcs3414.c
index b542e5619ead..2dc3d5db5d08 100644
--- a/drivers/iio/light/tcs3414.c
+++ b/drivers/iio/light/tcs3414.c
@@ -243,35 +243,19 @@ static const struct iio_info tcs3414_info = {
static int tcs3414_buffer_postenable(struct iio_dev *indio_dev)
{
struct tcs3414_data *data = iio_priv(indio_dev);
- int ret;
-
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret)
- return ret;
data->control |= TCS3414_CONTROL_ADC_EN;
- ret = i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
+ return i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
data->control);
- if (ret)
- iio_triggered_buffer_predisable(indio_dev);
-
- return ret;
}
static int tcs3414_buffer_predisable(struct iio_dev *indio_dev)
{
struct tcs3414_data *data = iio_priv(indio_dev);
- int ret, ret2;
data->control &= ~TCS3414_CONTROL_ADC_EN;
- ret = i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
+ return i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
data->control);
-
- ret2 = iio_triggered_buffer_predisable(indio_dev);
- if (!ret)
- ret = ret2;
-
- return ret;
}
static const struct iio_buffer_setup_ops tcs3414_buffer_setup_ops = {
diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 2a4b3d331055..0fee767af026 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -957,29 +957,20 @@ static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
int ret;
int cmd;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret)
- return ret;
-
/* Do not enable the buffer if we are already capturing events. */
- if (vcnl4010_is_in_periodic_mode(data)) {
- ret = -EBUSY;
- goto end;
- }
+ if (vcnl4010_is_in_periodic_mode(data))
+ return -EBUSY;
ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL,
VCNL4010_INT_PROX_EN);
if (ret < 0)
- goto end;
+ return ret;
cmd = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
+
ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
if (ret < 0)
- goto end;
-
- return 0;
-end:
- iio_triggered_buffer_predisable(indio_dev);
+ i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
return ret;
}
@@ -987,18 +978,14 @@ static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
static int vcnl4010_buffer_predisable(struct iio_dev *indio_dev)
{
struct vcnl4000_data *data = iio_priv(indio_dev);
- int ret, ret_disable;
+ int ret, ret2;
ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
- if (ret < 0)
- goto end;
- ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
+ ret2 = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
-end:
- ret_disable = iio_triggered_buffer_predisable(indio_dev);
if (ret == 0)
- ret = ret_disable;
+ ret = ret2;
return ret;
}
diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index d4de16750b10..1422da8db803 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -836,8 +836,6 @@ static int bmc150_magn_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops bmc150_magn_buffer_setup_ops = {
.preenable = bmc150_magn_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = bmc150_magn_buffer_postdisable,
};
diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index 43a2e420c9c4..96ad92a3c5d0 100644
--- a/drivers/iio/magnetometer/rm3100-core.c
+++ b/drivers/iio/magnetometer/rm3100-core.c
@@ -463,8 +463,6 @@ static int rm3100_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops rm3100_buffer_ops = {
.preenable = rm3100_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = rm3100_buffer_postdisable,
};
diff --git a/drivers/iio/magnetometer/st_magn_buffer.c b/drivers/iio/magnetometer/st_magn_buffer.c
index bb425c167a96..4917721fa2e5 100644
--- a/drivers/iio/magnetometer/st_magn_buffer.c
+++ b/drivers/iio/magnetometer/st_magn_buffer.c
@@ -31,34 +31,12 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
{
- int err;
-
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err < 0)
- return err;
-
- err = st_sensors_set_enable(indio_dev, true);
- if (err < 0)
- goto st_magn_buffer_predisable;
-
- return 0;
-
-st_magn_buffer_predisable:
- iio_triggered_buffer_predisable(indio_dev);
- return err;
+ return st_sensors_set_enable(indio_dev, true);
}
static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
{
- int err, err2;
-
- err = st_sensors_set_enable(indio_dev, false);
-
- err2 = iio_triggered_buffer_predisable(indio_dev);
- if (!err)
- err = err2;
-
- return err;
+ return st_sensors_set_enable(indio_dev, false);
}
static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index 2cb11da18e0f..5eac2dfae747 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -278,17 +278,8 @@ static const struct iio_trigger_ops lmp91000_trigger_ops = {
static int lmp91000_buffer_postenable(struct iio_dev *indio_dev)
{
struct lmp91000_data *data = iio_priv(indio_dev);
- int err;
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err)
- return err;
-
- err = iio_channel_start_all_cb(data->cb_buffer);
- if (err)
- iio_triggered_buffer_predisable(indio_dev);
-
- return err;
+ return iio_channel_start_all_cb(data->cb_buffer);
}
static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
@@ -297,7 +288,7 @@ static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
iio_channel_stop_all_cb(data->cb_buffer);
- return iio_triggered_buffer_predisable(indio_dev);
+ return 0;
}
static const struct iio_buffer_setup_ops lmp91000_buffer_setup_ops = {
diff --git a/drivers/iio/pressure/st_pressure_buffer.c b/drivers/iio/pressure/st_pressure_buffer.c
index 418dbf9e6e1e..7cf6f06797e1 100644
--- a/drivers/iio/pressure/st_pressure_buffer.c
+++ b/drivers/iio/pressure/st_pressure_buffer.c
@@ -31,34 +31,12 @@ int st_press_trig_set_state(struct iio_trigger *trig, bool state)
static int st_press_buffer_postenable(struct iio_dev *indio_dev)
{
- int err;
-
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err < 0)
- return err;
-
- err = st_sensors_set_enable(indio_dev, true);
- if (err < 0)
- goto st_press_buffer_predisable;
-
- return 0;
-
-st_press_buffer_predisable:
- iio_triggered_buffer_predisable(indio_dev);
- return err;
+ return st_sensors_set_enable(indio_dev, true);
}
static int st_press_buffer_predisable(struct iio_dev *indio_dev)
{
- int err, err2;
-
- err = st_sensors_set_enable(indio_dev, false);
-
- err2 = iio_triggered_buffer_predisable(indio_dev);
- if (!err)
- err = err2;
-
- return err;
+ return st_sensors_set_enable(indio_dev, false);
}
static const struct iio_buffer_setup_ops st_press_buffer_setup_ops = {
diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c
index 37fe851f89af..e082ad007b22 100644
--- a/drivers/iio/pressure/zpa2326.c
+++ b/drivers/iio/pressure/zpa2326.c
@@ -1240,12 +1240,7 @@ static int zpa2326_preenable_buffer(struct iio_dev *indio_dev)
static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
{
const struct zpa2326_private *priv = iio_priv(indio_dev);
- int err;
-
- /* Plug our own trigger event handler. */
- err = iio_triggered_buffer_postenable(indio_dev);
- if (err)
- goto err;
+ int err = 0;
if (!priv->waken) {
/*
@@ -1254,7 +1249,7 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
*/
err = zpa2326_clear_fifo(indio_dev, 0);
if (err)
- goto err_buffer_predisable;
+ goto out;
}
if (!iio_trigger_using_own(indio_dev) && priv->waken) {
@@ -1264,14 +1259,10 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
*/
err = zpa2326_config_oneshot(indio_dev, priv->irq);
if (err)
- goto err_buffer_predisable;
+ goto out;
}
- return 0;
-
-err_buffer_predisable:
- iio_triggered_buffer_predisable(indio_dev);
-err:
+out:
zpa2326_err(indio_dev, "failed to enable buffering (%d)", err);
return err;
@@ -1287,7 +1278,6 @@ static int zpa2326_postdisable_buffer(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops zpa2326_buffer_setup_ops = {
.preenable = zpa2326_preenable_buffer,
.postenable = zpa2326_postenable_buffer,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = zpa2326_postdisable_buffer
};
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index d161f3061e35..764b147c8bde 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -736,8 +736,6 @@ static int sx9310_buffer_postdisable(struct iio_dev *indio_dev)
static const struct iio_buffer_setup_ops sx9310_buffer_setup_ops = {
.preenable = sx9310_buffer_preenable,
- .postenable = iio_triggered_buffer_postenable,
- .predisable = iio_triggered_buffer_predisable,
.postdisable = sx9310_buffer_postdisable,
};
diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c
index 287d288e40c2..129c14e9a359 100644
--- a/drivers/iio/proximity/sx9500.c
+++ b/drivers/iio/proximity/sx9500.c
@@ -680,10 +680,6 @@ static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
struct sx9500_data *data = iio_priv(indio_dev);
int ret = 0, i;
- ret = iio_triggered_buffer_postenable(indio_dev);
- if (ret)
- return ret;
-
mutex_lock(&data->mutex);
for (i = 0; i < SX9500_NUM_CHANNELS; i++)
@@ -700,9 +696,6 @@ static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
mutex_unlock(&data->mutex);
- if (ret)
- iio_triggered_buffer_predisable(indio_dev);
-
return ret;
}
@@ -727,8 +720,6 @@ static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
mutex_unlock(&data->mutex);
- iio_triggered_buffer_predisable(indio_dev);
-
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable()
2020-05-22 10:46 ` [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable() Alexandru Ardelean
@ 2020-05-24 13:38 ` Jonathan Cameron
2020-05-25 11:30 ` Ardelean, Alexandru
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2020-05-24 13:38 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32, shawnguo,
s.hauer, mcoquelin.stm32, alexandre.torgue, linus.walleij,
lorenzo.bianconi83, songqiang1304521, Lars-Peter Clausen
On Fri, 22 May 2020 13:46:32 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
>
> This patch should be squashed into the first one, as the first one is
> breaking the build (intentionally) to make the IIO core files easier to
> review.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Yeah! Didn't realise you'd finally gotten to the end of your mammoth rework
leading to this.
A few really minor things inline to tidy up.
Thanks,
Jonathan
> diff --git a/drivers/iio/accel/st_accel_buffer.c b/drivers/iio/accel/st_accel_buffer.c
> index b5c814ef1637..c87f9a7d2453 100644
> --- a/drivers/iio/accel/st_accel_buffer.c
> +++ b/drivers/iio/accel/st_accel_buffer.c
> @@ -33,13 +33,9 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
> {
> int err;
>
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err < 0)
> - return err;
> -
> err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
> if (err < 0)
> - goto st_accel_buffer_predisable;
> + return err;
>
> err = st_sensors_set_enable(indio_dev, true);
> if (err < 0)
> @@ -49,8 +45,6 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
>
> st_accel_buffer_enable_all_axis:
> st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> -st_accel_buffer_predisable:
> - iio_triggered_buffer_predisable(indio_dev);
> return err;
> }
>
> @@ -60,12 +54,10 @@ static int st_accel_buffer_predisable(struct iio_dev *indio_dev)
>
> err = st_sensors_set_enable(indio_dev, false);
> if (err < 0)
> - goto st_accel_buffer_predisable;
> -
> - err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> + return err;
>
> -st_accel_buffer_predisable:
> - err2 = iio_triggered_buffer_predisable(indio_dev);
> + err2 = st_sensors_set_axis_enable(indio_dev,
> + ST_SENSORS_ENABLE_ALL_AXIS);
> if (!err)
I don't think you can get here with err set.
> err = err2;
>
...
> diff --git a/drivers/iio/gyro/st_gyro_buffer.c b/drivers/iio/gyro/st_gyro_buffer.c
> index 9c92ff7a82be..7b86502d5da3 100644
> --- a/drivers/iio/gyro/st_gyro_buffer.c
> +++ b/drivers/iio/gyro/st_gyro_buffer.c
> @@ -33,13 +33,9 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
> {
> int err;
>
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err < 0)
> - return err;
> -
> err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
> if (err < 0)
> - goto st_gyro_buffer_predisable;
> + return err;
>
> err = st_sensors_set_enable(indio_dev, true);
> if (err < 0)
> @@ -49,8 +45,6 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
>
> st_gyro_buffer_enable_all_axis:
> st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> -st_gyro_buffer_predisable:
> - iio_triggered_buffer_predisable(indio_dev);
> return err;
> }
>
> @@ -59,13 +53,8 @@ static int st_gyro_buffer_predisable(struct iio_dev *indio_dev)
> int err, err2;
>
> err = st_sensors_set_enable(indio_dev, false);
> - if (err < 0)
> - goto st_gyro_buffer_predisable;
Previously we didn't bother trying to carry on if this failed. I don't think we
should start doing so now.
> -
> - err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
>
> -st_gyro_buffer_predisable:
> - err2 = iio_triggered_buffer_predisable(indio_dev);
> + err2 = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> if (!err)
> err = err2;
>
...
> diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
> index 070d4cd0cf54..29d7af33efa1 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -1390,12 +1390,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
>
> mutex_lock(&data->lock);
I guess it doesn't matter, but no idea why this was ever under the local lock!
>
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err < 0) {
> - mutex_unlock(&data->lock);
> - return err;
> - }
> -
> /*
> * Enable triggers according to the scan_mask. Enabling either
> * LIGHT_CLEAR or LIGHT_IR scan mode results in enabling ALS
> @@ -1430,8 +1424,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
> err = -ENOMEM;
>
> error_unlock:
> - if (err < 0)
> - iio_triggered_buffer_predisable(indio_dev);
> mutex_unlock(&data->lock);
>
> return err;
> @@ -1465,8 +1457,6 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
> if (err == 0)
> kfree(data->buffer);
>
> - iio_triggered_buffer_predisable(indio_dev);
> -
> mutex_unlock(&data->lock);
>
> return err;
...
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 2a4b3d331055..0fee767af026 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -957,29 +957,20 @@ static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
> int ret;
> int cmd;
>
> - ret = iio_triggered_buffer_postenable(indio_dev);
> - if (ret)
> - return ret;
> -
> /* Do not enable the buffer if we are already capturing events. */
> - if (vcnl4010_is_in_periodic_mode(data)) {
> - ret = -EBUSY;
> - goto end;
> - }
> + if (vcnl4010_is_in_periodic_mode(data))
> + return -EBUSY;
>
> ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL,
> VCNL4010_INT_PROX_EN);
> if (ret < 0)
> - goto end;
> + return ret;
>
> cmd = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
> +
> ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
> if (ret < 0)
> - goto end;
> -
> - return 0;
> -end:
> - iio_triggered_buffer_predisable(indio_dev);
> + i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
>
> return ret;
> }
> @@ -987,18 +978,14 @@ static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
> static int vcnl4010_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct vcnl4000_data *data = iio_priv(indio_dev);
> - int ret, ret_disable;
> + int ret, ret2;
>
> ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
> - if (ret < 0)
> - goto end;
>
> - ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
> + ret2 = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
hmm. This does change the flow a tiny bit. I wonder if we really
care about carrying on if we get an error on the first write?
We are device not responding territory at that point. Maybe just return
immediately and avoid the dance with the two ret variables?
>
> -end:
> - ret_disable = iio_triggered_buffer_predisable(indio_dev);
> if (ret == 0)
> - ret = ret_disable;
> + ret = ret2;
>
> return ret;
> }
...
> static const struct iio_buffer_setup_ops st_press_buffer_setup_ops = {
> diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c
> index 37fe851f89af..e082ad007b22 100644
> --- a/drivers/iio/pressure/zpa2326.c
> +++ b/drivers/iio/pressure/zpa2326.c
> @@ -1240,12 +1240,7 @@ static int zpa2326_preenable_buffer(struct iio_dev *indio_dev)
> static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
> {
> const struct zpa2326_private *priv = iio_priv(indio_dev);
> - int err;
> -
> - /* Plug our own trigger event handler. */
> - err = iio_triggered_buffer_postenable(indio_dev);
> - if (err)
> - goto err;
> + int err = 0;
>
> if (!priv->waken) {
> /*
> @@ -1254,7 +1249,7 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
> */
> err = zpa2326_clear_fifo(indio_dev, 0);
> if (err)
> - goto err_buffer_predisable;
> + goto out;
> }
>
> if (!iio_trigger_using_own(indio_dev) && priv->waken) {
> @@ -1264,14 +1259,10 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
> */
> err = zpa2326_config_oneshot(indio_dev, priv->irq);
> if (err)
> - goto err_buffer_predisable;
> + goto out;
> }
>
> - return 0;
> -
> -err_buffer_predisable:
> - iio_triggered_buffer_predisable(indio_dev);
> -err:
> +out:
> zpa2326_err(indio_dev, "failed to enable buffering (%d)", err);
Doesn't this now print the error in the good path?
Probably still want the return 0. It's a bit messier but I'd
just move the prints into the error paths and return directly from
each. Will be cleaner code that this.
>
> return err;
> @@ -1287,7 +1278,6 @@ static int zpa2326_postdisable_buffer(struct iio_dev *indio_dev)
> static const struct iio_buffer_setup_ops zpa2326_buffer_setup_ops = {
> .preenable = zpa2326_preenable_buffer,
> .postenable = zpa2326_postenable_buffer,
> - .predisable = iio_triggered_buffer_predisable,
> .postdisable = zpa2326_postdisable_buffer
> };
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] iio: Move attach/detach of the poll func to the core
2020-05-22 10:46 [PATCH 1/3] iio: Move attach/detach of the poll func to the core Alexandru Ardelean
2020-05-22 10:46 ` [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks Alexandru Ardelean
2020-05-22 10:46 ` [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable() Alexandru Ardelean
@ 2020-05-24 13:41 ` Jonathan Cameron
2020-07-14 14:57 ` Ardelean, Alexandru
3 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2020-05-24 13:41 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32, shawnguo,
s.hauer, mcoquelin.stm32, alexandre.torgue, linus.walleij,
lorenzo.bianconi83, songqiang1304521, Lars-Peter Clausen
On Fri, 22 May 2020 13:46:30 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
>
> All devices using a triggered buffer need to attach and detach the trigger
> to the device in order to properly work. Instead of doing this in each and
> every driver by hand move this into the core.
>
> At this point in time, all drivers should have been resolved to
> attach/detach the poll-function in the same order.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Looks good to me.
Jonathan
> ---
> .../buffer/industrialio-triggered-buffer.c | 10 +--------
> drivers/iio/iio_core_trigger.h | 17 ++++++++++++++
> drivers/iio/industrialio-buffer.c | 13 +++++++++++
> drivers/iio/industrialio-trigger.c | 22 ++++---------------
> include/linux/iio/trigger_consumer.h | 7 ------
> 5 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
> index e8046c1ecd6b..6c20a83f887e 100644
> --- a/drivers/iio/buffer/industrialio-triggered-buffer.c
> +++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
> @@ -13,11 +13,6 @@
> #include <linux/iio/triggered_buffer.h>
> #include <linux/iio/trigger_consumer.h>
>
> -static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
> - .postenable = &iio_triggered_buffer_postenable,
> - .predisable = &iio_triggered_buffer_predisable,
> -};
> -
> /**
> * iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc
> * @indio_dev: IIO device structure
> @@ -67,10 +62,7 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
> }
>
> /* Ring buffer functions - here trigger setup related */
> - if (setup_ops)
> - indio_dev->setup_ops = setup_ops;
> - else
> - indio_dev->setup_ops = &iio_triggered_buffer_setup_ops;
> + indio_dev->setup_ops = setup_ops;
>
> /* Flag that polled ring buffering is possible */
> indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
> diff --git a/drivers/iio/iio_core_trigger.h b/drivers/iio/iio_core_trigger.h
> index e59fe2f36bbb..9d1a92cc6480 100644
> --- a/drivers/iio/iio_core_trigger.h
> +++ b/drivers/iio/iio_core_trigger.h
> @@ -18,6 +18,12 @@ void iio_device_register_trigger_consumer(struct iio_dev *indio_dev);
> **/
> void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev);
>
> +
> +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf);
> +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf);
> +
> #else
>
> /**
> @@ -37,4 +43,15 @@ static void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
> {
> }
>
> +static inline int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> +{
> + return 0;
> +}
> +static inline int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_TRIGGER_CONSUMER */
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index ec4f531994fa..88d756107fb2 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -20,6 +20,7 @@
>
> #include <linux/iio/iio.h>
> #include "iio_core.h"
> +#include "iio_core_trigger.h"
> #include <linux/iio/sysfs.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/buffer_impl.h>
> @@ -972,6 +973,13 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
> }
> }
>
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> + ret = iio_trigger_attach_poll_func(indio_dev->trig,
> + indio_dev->pollfunc);
> + if (ret)
> + goto err_disable_buffers;
> + }
> +
> return 0;
>
> err_disable_buffers:
> @@ -998,6 +1006,11 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
> if (list_empty(&indio_dev->buffer_list))
> return 0;
>
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> + iio_trigger_detach_poll_func(indio_dev->trig,
> + indio_dev->pollfunc);
> + }
> +
> /*
> * If things go wrong at some step in disable we still need to continue
> * to perform the other steps, otherwise we leave the device in a
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 53d1931f6be8..6f16357fd732 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -239,8 +239,8 @@ static void iio_trigger_put_irq(struct iio_trigger *trig, int irq)
> * the relevant function is in there may be the best option.
> */
> /* Worth protecting against double additions? */
> -static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> - struct iio_poll_func *pf)
> +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> {
> int ret = 0;
> bool notinuse
> @@ -290,8 +290,8 @@ static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> return ret;
> }
>
> -static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> - struct iio_poll_func *pf)
> +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> {
> int ret = 0;
> bool no_other_users
> @@ -705,17 +705,3 @@ void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
> if (indio_dev->trig)
> iio_trigger_put(indio_dev->trig);
> }
> -
> -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev)
> -{
> - return iio_trigger_attach_poll_func(indio_dev->trig,
> - indio_dev->pollfunc);
> -}
> -EXPORT_SYMBOL(iio_triggered_buffer_postenable);
> -
> -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev)
> -{
> - return iio_trigger_detach_poll_func(indio_dev->trig,
> - indio_dev->pollfunc);
> -}
> -EXPORT_SYMBOL(iio_triggered_buffer_predisable);
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index c3c6ba5ec423..3aa2f132dd67 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -50,11 +50,4 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
>
> void iio_trigger_notify_done(struct iio_trigger *trig);
>
> -/*
> - * Two functions for common case where all that happens is a pollfunc
> - * is attached and detached from a trigger
> - */
> -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev);
> -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
> -
> #endif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks
2020-05-22 10:46 ` [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks Alexandru Ardelean
@ 2020-05-24 13:54 ` Jonathan Cameron
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2020-05-24 13:54 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32, shawnguo,
s.hauer, mcoquelin.stm32, alexandre.torgue, linus.walleij,
lorenzo.bianconi83, songqiang1304521
On Fri, 22 May 2020 13:46:31 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This should be squashed into the first patch, but it's the more peculiar of
> the changes.
> I am not sure whether this is correct. The touchscreen channels shouldn't
> be enabled by the IIO framework. So, we may need a different way to handle
> those if needed.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Hmm. Unfortunately I can't remember exactly what is going on here.
From a quick look my suspicion is we can handle this using the same
'is it a triggered buffer' test as you now have in the core code.
The touchscreen path operates as a non triggered buffer (I think...)
I'm definitely looking for an ack and preferably a tested-by for this
one. You are right - it's non obvious!
Jonathan
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 9abbbdcc7420..f71071096392 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -937,14 +937,6 @@ static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
> return 0;
> }
>
> -static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> -{
> - if (at91_adc_current_chan_is_touch(indio_dev))
> - return 0;
> -
> - return iio_triggered_buffer_postenable(indio_dev);
> -}
> -
> static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
> {
> struct at91_adc_state *st = iio_priv(indio_dev);
> @@ -995,19 +987,9 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
> return 0;
> }
>
> -static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> -{
> - if (at91_adc_current_chan_is_touch(indio_dev))
> - return 0;
> -
> - return iio_triggered_buffer_predisable(indio_dev);
> -}
> -
> static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
> .preenable = &at91_adc_buffer_preenable,
> .postdisable = &at91_adc_buffer_postdisable,
> - .postenable = &at91_adc_buffer_postenable,
> - .predisable = &at91_adc_buffer_predisable,
> };
>
> static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable()
2020-05-24 13:38 ` Jonathan Cameron
@ 2020-05-25 11:30 ` Ardelean, Alexandru
0 siblings, 0 replies; 9+ messages in thread
From: Ardelean, Alexandru @ 2020-05-25 11:30 UTC (permalink / raw)
To: jic23
Cc: lars, linux-stm32, alexandre.torgue, linus.walleij, linux-kernel,
mcoquelin.stm32, linux-arm-kernel, linux-iio, lorenzo.bianconi83,
songqiang1304521, shawnguo, s.hauer
On Sun, 2020-05-24 at 14:38 +0100, Jonathan Cameron wrote:
> [External]
>
> On Fri, 22 May 2020 13:46:32 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > From: Lars-Peter Clausen <lars@metafoo.de>
> >
> > This patch should be squashed into the first one, as the first one is
> > breaking the build (intentionally) to make the IIO core files easier to
> > review.
> >
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> Yeah! Didn't realise you'd finally gotten to the end of your mammoth rework
> leading to this.
>
> A few really minor things inline to tidy up.
Will fix these and send a V2.
>
> Thanks,
>
> Jonathan
>
>
> > diff --git a/drivers/iio/accel/st_accel_buffer.c
> > b/drivers/iio/accel/st_accel_buffer.c
> > index b5c814ef1637..c87f9a7d2453 100644
> > --- a/drivers/iio/accel/st_accel_buffer.c
> > +++ b/drivers/iio/accel/st_accel_buffer.c
> > @@ -33,13 +33,9 @@ static int st_accel_buffer_postenable(struct iio_dev
> > *indio_dev)
> > {
> > int err;
> >
> > - err = iio_triggered_buffer_postenable(indio_dev);
> > - if (err < 0)
> > - return err;
> > -
> > err = st_sensors_set_axis_enable(indio_dev, indio_dev-
> > >active_scan_mask[0]);
> > if (err < 0)
> > - goto st_accel_buffer_predisable;
> > + return err;
> >
> > err = st_sensors_set_enable(indio_dev, true);
> > if (err < 0)
> > @@ -49,8 +45,6 @@ static int st_accel_buffer_postenable(struct iio_dev
> > *indio_dev)
> >
> > st_accel_buffer_enable_all_axis:
> > st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> > -st_accel_buffer_predisable:
> > - iio_triggered_buffer_predisable(indio_dev);
> > return err;
> > }
> >
> > @@ -60,12 +54,10 @@ static int st_accel_buffer_predisable(struct iio_dev
> > *indio_dev)
> >
> > err = st_sensors_set_enable(indio_dev, false);
> > if (err < 0)
> > - goto st_accel_buffer_predisable;
> > -
> > - err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> > + return err;
> >
> > -st_accel_buffer_predisable:
> > - err2 = iio_triggered_buffer_predisable(indio_dev);
> > + err2 = st_sensors_set_axis_enable(indio_dev,
> > + ST_SENSORS_ENABLE_ALL_AXIS);
> > if (!err)
> I don't think you can get here with err set.
> > err = err2;
> >
>
> ...
>
> > diff --git a/drivers/iio/gyro/st_gyro_buffer.c
> > b/drivers/iio/gyro/st_gyro_buffer.c
> > index 9c92ff7a82be..7b86502d5da3 100644
> > --- a/drivers/iio/gyro/st_gyro_buffer.c
> > +++ b/drivers/iio/gyro/st_gyro_buffer.c
> > @@ -33,13 +33,9 @@ static int st_gyro_buffer_postenable(struct iio_dev
> > *indio_dev)
> > {
> > int err;
> >
> > - err = iio_triggered_buffer_postenable(indio_dev);
> > - if (err < 0)
> > - return err;
> > -
> > err = st_sensors_set_axis_enable(indio_dev, indio_dev-
> > >active_scan_mask[0]);
> > if (err < 0)
> > - goto st_gyro_buffer_predisable;
> > + return err;
> >
> > err = st_sensors_set_enable(indio_dev, true);
> > if (err < 0)
> > @@ -49,8 +45,6 @@ static int st_gyro_buffer_postenable(struct iio_dev
> > *indio_dev)
> >
> > st_gyro_buffer_enable_all_axis:
> > st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> > -st_gyro_buffer_predisable:
> > - iio_triggered_buffer_predisable(indio_dev);
> > return err;
> > }
> >
> > @@ -59,13 +53,8 @@ static int st_gyro_buffer_predisable(struct iio_dev
> > *indio_dev)
> > int err, err2;
> >
> > err = st_sensors_set_enable(indio_dev, false);
> > - if (err < 0)
> > - goto st_gyro_buffer_predisable;
>
> Previously we didn't bother trying to carry on if this failed. I don't think
> we
> should start doing so now.
>
> > -
> > - err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> >
> > -st_gyro_buffer_predisable:
> > - err2 = iio_triggered_buffer_predisable(indio_dev);
> > + err2 = st_sensors_set_axis_enable(indio_dev,
> > ST_SENSORS_ENABLE_ALL_AXIS);
> > if (!err)
> > err = err2;
> >
>
> ...
>
> > diff --git a/drivers/iio/light/gp2ap020a00f.c
> > b/drivers/iio/light/gp2ap020a00f.c
> > index 070d4cd0cf54..29d7af33efa1 100644
> > --- a/drivers/iio/light/gp2ap020a00f.c
> > +++ b/drivers/iio/light/gp2ap020a00f.c
> > @@ -1390,12 +1390,6 @@ static int gp2ap020a00f_buffer_postenable(struct
> > iio_dev *indio_dev)
> >
> > mutex_lock(&data->lock);
> I guess it doesn't matter, but no idea why this was ever under the local lock!
>
> >
> > - err = iio_triggered_buffer_postenable(indio_dev);
> > - if (err < 0) {
> > - mutex_unlock(&data->lock);
> > - return err;
> > - }
> > -
> > /*
> > * Enable triggers according to the scan_mask. Enabling either
> > * LIGHT_CLEAR or LIGHT_IR scan mode results in enabling ALS
> > @@ -1430,8 +1424,6 @@ static int gp2ap020a00f_buffer_postenable(struct
> > iio_dev *indio_dev)
> > err = -ENOMEM;
> >
> > error_unlock:
> > - if (err < 0)
> > - iio_triggered_buffer_predisable(indio_dev);
> > mutex_unlock(&data->lock);
> >
> > return err;
> > @@ -1465,8 +1457,6 @@ static int gp2ap020a00f_buffer_predisable(struct
> > iio_dev *indio_dev)
> > if (err == 0)
> > kfree(data->buffer);
> >
> > - iio_triggered_buffer_predisable(indio_dev);
> > -
> > mutex_unlock(&data->lock);
> >
> > return err;
>
> ...
>
> > diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> > index 2a4b3d331055..0fee767af026 100644
> > --- a/drivers/iio/light/vcnl4000.c
> > +++ b/drivers/iio/light/vcnl4000.c
> > @@ -957,29 +957,20 @@ static int vcnl4010_buffer_postenable(struct iio_dev
> > *indio_dev)
> > int ret;
> > int cmd;
> >
> > - ret = iio_triggered_buffer_postenable(indio_dev);
> > - if (ret)
> > - return ret;
> > -
> > /* Do not enable the buffer if we are already capturing events. */
> > - if (vcnl4010_is_in_periodic_mode(data)) {
> > - ret = -EBUSY;
> > - goto end;
> > - }
> > + if (vcnl4010_is_in_periodic_mode(data))
> > + return -EBUSY;
> >
> > ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL,
> > VCNL4010_INT_PROX_EN);
> > if (ret < 0)
> > - goto end;
> > + return ret;
> >
> > cmd = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
> > +
> > ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
> > if (ret < 0)
> > - goto end;
> > -
> > - return 0;
> > -end:
> > - iio_triggered_buffer_predisable(indio_dev);
> > + i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
> >
> > return ret;
> > }
> > @@ -987,18 +978,14 @@ static int vcnl4010_buffer_postenable(struct iio_dev
> > *indio_dev)
> > static int vcnl4010_buffer_predisable(struct iio_dev *indio_dev)
> > {
> > struct vcnl4000_data *data = iio_priv(indio_dev);
> > - int ret, ret_disable;
> > + int ret, ret2;
> >
> > ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
> > - if (ret < 0)
> > - goto end;
> >
> > - ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
> > + ret2 = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
>
> hmm. This does change the flow a tiny bit. I wonder if we really
> care about carrying on if we get an error on the first write?
> We are device not responding territory at that point. Maybe just return
> immediately and avoid the dance with the two ret variables?
>
> >
> > -end:
> > - ret_disable = iio_triggered_buffer_predisable(indio_dev);
> > if (ret == 0)
> > - ret = ret_disable;
> > + ret = ret2;
> >
> > return ret;
> > }
>
> ...
>
> > static const struct iio_buffer_setup_ops st_press_buffer_setup_ops = {
> > diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c
> > index 37fe851f89af..e082ad007b22 100644
> > --- a/drivers/iio/pressure/zpa2326.c
> > +++ b/drivers/iio/pressure/zpa2326.c
> > @@ -1240,12 +1240,7 @@ static int zpa2326_preenable_buffer(struct iio_dev
> > *indio_dev)
> > static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
> > {
> > const struct zpa2326_private *priv = iio_priv(indio_dev);
> > - int err;
> > -
> > - /* Plug our own trigger event handler. */
> > - err = iio_triggered_buffer_postenable(indio_dev);
> > - if (err)
> > - goto err;
> > + int err = 0;
> >
> > if (!priv->waken) {
> > /*
> > @@ -1254,7 +1249,7 @@ static int zpa2326_postenable_buffer(struct iio_dev
> > *indio_dev)
> > */
> > err = zpa2326_clear_fifo(indio_dev, 0);
> > if (err)
> > - goto err_buffer_predisable;
> > + goto out;
> > }
> >
> > if (!iio_trigger_using_own(indio_dev) && priv->waken) {
> > @@ -1264,14 +1259,10 @@ static int zpa2326_postenable_buffer(struct iio_dev
> > *indio_dev)
> > */
> > err = zpa2326_config_oneshot(indio_dev, priv->irq);
> > if (err)
> > - goto err_buffer_predisable;
> > + goto out;
> > }
> >
> > - return 0;
> > -
> > -err_buffer_predisable:
> > - iio_triggered_buffer_predisable(indio_dev);
> > -err:
> > +out:
> > zpa2326_err(indio_dev, "failed to enable buffering (%d)", err);
>
> Doesn't this now print the error in the good path?
>
> Probably still want the return 0. It's a bit messier but I'd
> just move the prints into the error paths and return directly from
> each. Will be cleaner code that this.
>
>
> >
> > return err;
> > @@ -1287,7 +1278,6 @@ static int zpa2326_postdisable_buffer(struct iio_dev
> > *indio_dev)
> > static const struct iio_buffer_setup_ops zpa2326_buffer_setup_ops = {
> > .preenable = zpa2326_preenable_buffer,
> > .postenable = zpa2326_postenable_buffer,
> > - .predisable = iio_triggered_buffer_predisable,
> > .postdisable = zpa2326_postdisable_buffer
> > };
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] iio: Move attach/detach of the poll func to the core
2020-05-22 10:46 [PATCH 1/3] iio: Move attach/detach of the poll func to the core Alexandru Ardelean
` (2 preceding siblings ...)
2020-05-24 13:41 ` [PATCH 1/3] iio: Move attach/detach of the poll func to the core Jonathan Cameron
@ 2020-07-14 14:57 ` Ardelean, Alexandru
2020-07-14 17:25 ` Jonathan Cameron
3 siblings, 1 reply; 9+ messages in thread
From: Ardelean, Alexandru @ 2020-07-14 14:57 UTC (permalink / raw)
To: linux-kernel, linux-iio; +Cc: jic23, lars
On Fri, 2020-05-22 at 13:46 +0300, Alexandru Ardelean wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
>
> All devices using a triggered buffer need to attach and detach the
> trigger
> to the device in order to properly work. Instead of doing this in each
> and
> every driver by hand move this into the core.
>
> At this point in time, all drivers should have been resolved to
> attach/detach the poll-function in the same order.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../buffer/industrialio-triggered-buffer.c | 10 +--------
> drivers/iio/iio_core_trigger.h | 17 ++++++++++++++
> drivers/iio/industrialio-buffer.c | 13 +++++++++++
> drivers/iio/industrialio-trigger.c | 22 ++++---------------
> include/linux/iio/trigger_consumer.h | 7 ------
> 5 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c
> b/drivers/iio/buffer/industrialio-triggered-buffer.c
> index e8046c1ecd6b..6c20a83f887e 100644
> --- a/drivers/iio/buffer/industrialio-triggered-buffer.c
> +++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
> @@ -13,11 +13,6 @@
> #include <linux/iio/triggered_buffer.h>
> #include <linux/iio/trigger_consumer.h>
>
> -static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops
> = {
> - .postenable = &iio_triggered_buffer_postenable,
> - .predisable = &iio_triggered_buffer_predisable,
> -};
> -
> /**
> * iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc
> * @indio_dev: IIO device structure
> @@ -67,10 +62,7 @@ int iio_triggered_buffer_setup(struct iio_dev
> *indio_dev,
> }
>
> /* Ring buffer functions - here trigger setup related */
> - if (setup_ops)
> - indio_dev->setup_ops = setup_ops;
> - else
> - indio_dev->setup_ops = &iio_triggered_buffer_setup_ops;
> + indio_dev->setup_ops = setup_ops;
>
> /* Flag that polled ring buffering is possible */
> indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
> diff --git a/drivers/iio/iio_core_trigger.h
> b/drivers/iio/iio_core_trigger.h
> index e59fe2f36bbb..9d1a92cc6480 100644
> --- a/drivers/iio/iio_core_trigger.h
> +++ b/drivers/iio/iio_core_trigger.h
> @@ -18,6 +18,12 @@ void iio_device_register_trigger_consumer(struct
> iio_dev *indio_dev);
> **/
> void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev);
>
> +
> +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf);
> +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf);
> +
> #else
>
> /**
> @@ -37,4 +43,15 @@ static void
> iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
> {
> }
>
> +static inline int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> +{
> + return 0;
> +}
> +static inline int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_TRIGGER_CONSUMER */
> diff --git a/drivers/iio/industrialio-buffer.c
> b/drivers/iio/industrialio-buffer.c
> index ec4f531994fa..88d756107fb2 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -20,6 +20,7 @@
>
> #include <linux/iio/iio.h>
> #include "iio_core.h"
> +#include "iio_core_trigger.h"
> #include <linux/iio/sysfs.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/buffer_impl.h>
> @@ -972,6 +973,13 @@ static int iio_enable_buffers(struct iio_dev
> *indio_dev,
> }
> }
>
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> + ret = iio_trigger_attach_poll_func(indio_dev->trig,
> + indio_dev->pollfunc);
> + if (ret)
> + goto err_disable_buffers;
> + }
> +
I'm wondering what happened here, or whether I was on some other planet, or
some other universe where this was correct, but this part looks wrong.
This should be called before the "indio_dev->setup_ops->postenable" call
And similarly iio_trigger_detach_poll_func() should be called after the
"indio_dev->setup_ops->predisable" call.
This looks clearly like my fault.
And it looks like I sent it like this from the start....
At this point, what's the way to fix this?
Re-send or send a fix?
I noticed this while sync-ing the ADI tree with upstream and comparing and
spent a bit looking at this.
> return 0;
>
> err_disable_buffers:
> @@ -998,6 +1006,11 @@ static int iio_disable_buffers(struct iio_dev
> *indio_dev)
> if (list_empty(&indio_dev->buffer_list))
> return 0;
>
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> + iio_trigger_detach_poll_func(indio_dev->trig,
> + indio_dev->pollfunc);
> + }
> +
> /*
> * If things go wrong at some step in disable we still need to
> continue
> * to perform the other steps, otherwise we leave the device in a
> diff --git a/drivers/iio/industrialio-trigger.c
> b/drivers/iio/industrialio-trigger.c
> index 53d1931f6be8..6f16357fd732 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -239,8 +239,8 @@ static void iio_trigger_put_irq(struct iio_trigger
> *trig, int irq)
> * the relevant function is in there may be the best option.
> */
> /* Worth protecting against double additions? */
> -static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> - struct iio_poll_func *pf)
> +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> {
> int ret = 0;
> bool notinuse
> @@ -290,8 +290,8 @@ static int iio_trigger_attach_poll_func(struct
> iio_trigger *trig,
> return ret;
> }
>
> -static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> - struct iio_poll_func *pf)
> +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> + struct iio_poll_func *pf)
> {
> int ret = 0;
> bool no_other_users
> @@ -705,17 +705,3 @@ void iio_device_unregister_trigger_consumer(struct
> iio_dev *indio_dev)
> if (indio_dev->trig)
> iio_trigger_put(indio_dev->trig);
> }
> -
> -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev)
> -{
> - return iio_trigger_attach_poll_func(indio_dev->trig,
> - indio_dev->pollfunc);
> -}
> -EXPORT_SYMBOL(iio_triggered_buffer_postenable);
> -
> -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev)
> -{
> - return iio_trigger_detach_poll_func(indio_dev->trig,
> - indio_dev->pollfunc);
> -}
> -EXPORT_SYMBOL(iio_triggered_buffer_predisable);
> diff --git a/include/linux/iio/trigger_consumer.h
> b/include/linux/iio/trigger_consumer.h
> index c3c6ba5ec423..3aa2f132dd67 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -50,11 +50,4 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
>
> void iio_trigger_notify_done(struct iio_trigger *trig);
>
> -/*
> - * Two functions for common case where all that happens is a pollfunc
> - * is attached and detached from a trigger
> - */
> -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev);
> -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
> -
> #endif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] iio: Move attach/detach of the poll func to the core
2020-07-14 14:57 ` Ardelean, Alexandru
@ 2020-07-14 17:25 ` Jonathan Cameron
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2020-07-14 17:25 UTC (permalink / raw)
To: Ardelean, Alexandru; +Cc: linux-kernel, linux-iio, jic23, lars
On Tue, 14 Jul 2020 14:57:36 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> On Fri, 2020-05-22 at 13:46 +0300, Alexandru Ardelean wrote:
> > From: Lars-Peter Clausen <lars@metafoo.de>
> >
> > All devices using a triggered buffer need to attach and detach the
> > trigger
> > to the device in order to properly work. Instead of doing this in each
> > and
> > every driver by hand move this into the core.
> >
> > At this point in time, all drivers should have been resolved to
> > attach/detach the poll-function in the same order.
> >
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > .../buffer/industrialio-triggered-buffer.c | 10 +--------
> > drivers/iio/iio_core_trigger.h | 17 ++++++++++++++
> > drivers/iio/industrialio-buffer.c | 13 +++++++++++
> > drivers/iio/industrialio-trigger.c | 22 ++++---------------
> > include/linux/iio/trigger_consumer.h | 7 ------
> > 5 files changed, 35 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c
> > b/drivers/iio/buffer/industrialio-triggered-buffer.c
> > index e8046c1ecd6b..6c20a83f887e 100644
> > --- a/drivers/iio/buffer/industrialio-triggered-buffer.c
> > +++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
> > @@ -13,11 +13,6 @@
> > #include <linux/iio/triggered_buffer.h>
> > #include <linux/iio/trigger_consumer.h>
> >
> > -static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops
> > = {
> > - .postenable = &iio_triggered_buffer_postenable,
> > - .predisable = &iio_triggered_buffer_predisable,
> > -};
> > -
> > /**
> > * iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc
> > * @indio_dev: IIO device structure
> > @@ -67,10 +62,7 @@ int iio_triggered_buffer_setup(struct iio_dev
> > *indio_dev,
> > }
> >
> > /* Ring buffer functions - here trigger setup related */
> > - if (setup_ops)
> > - indio_dev->setup_ops = setup_ops;
> > - else
> > - indio_dev->setup_ops = &iio_triggered_buffer_setup_ops;
> > + indio_dev->setup_ops = setup_ops;
> >
> > /* Flag that polled ring buffering is possible */
> > indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
> > diff --git a/drivers/iio/iio_core_trigger.h
> > b/drivers/iio/iio_core_trigger.h
> > index e59fe2f36bbb..9d1a92cc6480 100644
> > --- a/drivers/iio/iio_core_trigger.h
> > +++ b/drivers/iio/iio_core_trigger.h
> > @@ -18,6 +18,12 @@ void iio_device_register_trigger_consumer(struct
> > iio_dev *indio_dev);
> > **/
> > void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev);
> >
> > +
> > +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> > + struct iio_poll_func *pf);
> > +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> > + struct iio_poll_func *pf);
> > +
> > #else
> >
> > /**
> > @@ -37,4 +43,15 @@ static void
> > iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
> > {
> > }
> >
> > +static inline int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> > + struct iio_poll_func *pf)
> > +{
> > + return 0;
> > +}
> > +static inline int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> > + struct iio_poll_func *pf)
> > +{
> > + return 0;
> > +}
> > +
> > #endif /* CONFIG_TRIGGER_CONSUMER */
> > diff --git a/drivers/iio/industrialio-buffer.c
> > b/drivers/iio/industrialio-buffer.c
> > index ec4f531994fa..88d756107fb2 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -20,6 +20,7 @@
> >
> > #include <linux/iio/iio.h>
> > #include "iio_core.h"
> > +#include "iio_core_trigger.h"
> > #include <linux/iio/sysfs.h>
> > #include <linux/iio/buffer.h>
> > #include <linux/iio/buffer_impl.h>
> > @@ -972,6 +973,13 @@ static int iio_enable_buffers(struct iio_dev
> > *indio_dev,
> > }
> > }
> >
> > + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> > + ret = iio_trigger_attach_poll_func(indio_dev->trig,
> > + indio_dev->pollfunc);
> > + if (ret)
> > + goto err_disable_buffers;
> > + }
> > +
>
> I'm wondering what happened here, or whether I was on some other planet, or
> some other universe where this was correct, but this part looks wrong.
> This should be called before the "indio_dev->setup_ops->postenable" call
>
> And similarly iio_trigger_detach_poll_func() should be called after the
> "indio_dev->setup_ops->predisable" call.
>
> This looks clearly like my fault.
> And it looks like I sent it like this from the start....
>
> At this point, what's the way to fix this?
> Re-send or send a fix?
Gah. I missed it as well. At this stage, a fix definitely. It will all
be in the same pull request, but I'd really rather not rebase the tree
again.
I guess this happened because of that debate a long time back on which
order made sense. We concluded the one that you morphed all the drivers
into, but I guess we forgot the final patch.
I'll hold that pull request a bit longer.
Jonathan
>
> I noticed this while sync-ing the ADI tree with upstream and comparing and
> spent a bit looking at this.
>
>
> > return 0;
> >
> > err_disable_buffers:
> > @@ -998,6 +1006,11 @@ static int iio_disable_buffers(struct iio_dev
> > *indio_dev)
> > if (list_empty(&indio_dev->buffer_list))
> > return 0;
> >
> > + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> > + iio_trigger_detach_poll_func(indio_dev->trig,
> > + indio_dev->pollfunc);
> > + }
> > +
> > /*
> > * If things go wrong at some step in disable we still need to
> > continue
> > * to perform the other steps, otherwise we leave the device in a
> > diff --git a/drivers/iio/industrialio-trigger.c
> > b/drivers/iio/industrialio-trigger.c
> > index 53d1931f6be8..6f16357fd732 100644
> > --- a/drivers/iio/industrialio-trigger.c
> > +++ b/drivers/iio/industrialio-trigger.c
> > @@ -239,8 +239,8 @@ static void iio_trigger_put_irq(struct iio_trigger
> > *trig, int irq)
> > * the relevant function is in there may be the best option.
> > */
> > /* Worth protecting against double additions? */
> > -static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> > - struct iio_poll_func *pf)
> > +int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> > + struct iio_poll_func *pf)
> > {
> > int ret = 0;
> > bool notinuse
> > @@ -290,8 +290,8 @@ static int iio_trigger_attach_poll_func(struct
> > iio_trigger *trig,
> > return ret;
> > }
> >
> > -static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> > - struct iio_poll_func *pf)
> > +int iio_trigger_detach_poll_func(struct iio_trigger *trig,
> > + struct iio_poll_func *pf)
> > {
> > int ret = 0;
> > bool no_other_users
> > @@ -705,17 +705,3 @@ void iio_device_unregister_trigger_consumer(struct
> > iio_dev *indio_dev)
> > if (indio_dev->trig)
> > iio_trigger_put(indio_dev->trig);
> > }
> > -
> > -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev)
> > -{
> > - return iio_trigger_attach_poll_func(indio_dev->trig,
> > - indio_dev->pollfunc);
> > -}
> > -EXPORT_SYMBOL(iio_triggered_buffer_postenable);
> > -
> > -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev)
> > -{
> > - return iio_trigger_detach_poll_func(indio_dev->trig,
> > - indio_dev->pollfunc);
> > -}
> > -EXPORT_SYMBOL(iio_triggered_buffer_predisable);
> > diff --git a/include/linux/iio/trigger_consumer.h
> > b/include/linux/iio/trigger_consumer.h
> > index c3c6ba5ec423..3aa2f132dd67 100644
> > --- a/include/linux/iio/trigger_consumer.h
> > +++ b/include/linux/iio/trigger_consumer.h
> > @@ -50,11 +50,4 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
> >
> > void iio_trigger_notify_done(struct iio_trigger *trig);
> >
> > -/*
> > - * Two functions for common case where all that happens is a pollfunc
> > - * is attached and detached from a trigger
> > - */
> > -int iio_triggered_buffer_postenable(struct iio_dev *indio_dev);
> > -int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
> > -
> > #endif
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-07-14 17:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 10:46 [PATCH 1/3] iio: Move attach/detach of the poll func to the core Alexandru Ardelean
2020-05-22 10:46 ` [PATCH 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks Alexandru Ardelean
2020-05-24 13:54 ` Jonathan Cameron
2020-05-22 10:46 ` [PATCH 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable() Alexandru Ardelean
2020-05-24 13:38 ` Jonathan Cameron
2020-05-25 11:30 ` Ardelean, Alexandru
2020-05-24 13:41 ` [PATCH 1/3] iio: Move attach/detach of the poll func to the core Jonathan Cameron
2020-07-14 14:57 ` Ardelean, Alexandru
2020-07-14 17:25 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).