* [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc
@ 2020-09-29 12:59 Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 1/9] iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs() Alexandru Ardelean
` (9 more replies)
0 siblings, 10 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This is a v2 & v3 for [1]:
https://lore.kernel.org/linux-iio/20200925083743.46469-1-alexandru.ardelean@analog.com/
It also includes a at91-sama5d2_adc cleanup patch in this series:
https://lore.kernel.org/linux-iio/20200924102902.136169-1-alexandru.ardelean@analog.com/
This patch is required, in order to make the removal of
iio_buffer_set_attrs() a bit cleaner in the at91-sama5d2_adc driver.
Following the discussion from [1], this patchset implements the
following:
Changelog v2 -> v3:
* in patch 'iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs()'
- minor stylistic change;
Changelog v1 -> v2:
* rename '{devm_}iio_triggered_buffer_setup()' ->
'{devm_}iio_triggered_buffer_setup_ext()'
- wrap with macros the new ext functions to preserve backwards
compatibility
- add a new parameter to the ext functions, which are the
buffer->attrs
* split into separate patches the removal [from each driver] of
iio_buffer_set_attrs() and the switch to a
{devm_}iio_triggered_buffer_setup_ext variant
* add patch to remove iio_buffer_set_attrs() from DMAEngine IIO buffer
* remove the iio_buffer_set_attrs() helper in a final/separate patch
* add 'at91-sama5d2_adc: merge buffer & trigger' patch in this series,
so that the removal of iio_buffer_set_attrs() is easier to view
Alexandru Ardelean (9):
iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs()
iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a
function
iio: triggered-buffer: add {devm_}iio_triggered_buffer_setup_ext
variants
iio: accel: adxl372: use devm_iio_triggered_buffer_setup_ext()
iio: accel: bmc150: use iio_triggered_buffer_setup_ext()
iio: adc: at91-sama5d2_adc: use devm_iio_triggered_buffer_setup_ext()
iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
iio: hid-sensors: use iio_triggered_buffer_setup_ext()
iio: buffer: remove iio_buffer_set_attrs() helper
drivers/iio/accel/adxl372.c | 11 ++-
drivers/iio/accel/bmc150-accel-core.c | 25 +++---
drivers/iio/adc/at91-sama5d2_adc.c | 82 +++++++++----------
.../buffer/industrialio-buffer-dmaengine.c | 3 +-
.../buffer/industrialio-triggered-buffer.c | 31 ++++---
.../cros_ec_sensors/cros_ec_sensors_core.c | 15 ++--
.../common/hid-sensors/hid-sensor-trigger.c | 22 ++---
drivers/iio/industrialio-buffer.c | 12 ---
include/linux/iio/buffer.h | 3 -
include/linux/iio/triggered_buffer.h | 23 ++++--
10 files changed, 113 insertions(+), 114 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/9] iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs()
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 2/9] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function Alexandru Ardelean
` (8 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
The iio_buffer_set_attrs() helper will be removed in this series. So, just
assign the attributes of the DMAEngine buffer logic directly.
This is IIO buffer core context, so there is direct access to the
buffer->attrs object.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
Changelog v2 -> v3:
* minor stylistic change
drivers/iio/buffer/industrialio-buffer-dmaengine.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 93b4e9e6bb55..b0cb9a35f5cd 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -200,9 +200,8 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
iio_dma_buffer_init(&dmaengine_buffer->queue, chan->device->dev,
&iio_dmaengine_default_ops);
- iio_buffer_set_attrs(&dmaengine_buffer->queue.buffer,
- iio_dmaengine_buffer_attrs);
+ dmaengine_buffer->queue.buffer.attrs = iio_dmaengine_buffer_attrs;
dmaengine_buffer->queue.buffer.access = &iio_dmaengine_buffer_ops;
return &dmaengine_buffer->queue.buffer;
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/9] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 1/9] iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs() Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 3/9] iio: triggered-buffer: add {devm_}iio_triggered_buffer_setup_ext variants Alexandru Ardelean
` (7 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This change is mostly cosmetic, but it's also a pre-cursor to the
the change for 'iio_buffer_set_attrs()', where the helper gets updated to
better support multiple IIO buffers for 1 IIO device.
The only functional change is that the error message for the trigger alloc
failure is bound to the parent device vs the IIO device object.
Also, the new at91_adc_buffer_and_trigger_init() function was moved after
the definition of the 'at91_adc_fifo_attributes'.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/at91-sama5d2_adc.c | 78 ++++++++++++++----------------
1 file changed, 36 insertions(+), 42 deletions(-)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index ad7d9819f83c..b9c3cc6d5913 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -1014,21 +1014,6 @@ static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
return trig;
}
-
-static int at91_adc_trigger_init(struct iio_dev *indio)
-{
- struct at91_adc_state *st = iio_priv(indio);
-
- st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
- if (IS_ERR(st->trig)) {
- dev_err(&indio->dev,
- "could not allocate trigger\n");
- return PTR_ERR(st->trig);
- }
-
- return 0;
-}
-
static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
struct iio_poll_func *pf)
{
@@ -1156,13 +1141,6 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
return IRQ_HANDLED;
}
-static int at91_adc_buffer_init(struct iio_dev *indio)
-{
- return devm_iio_triggered_buffer_setup(&indio->dev, indio,
- &iio_pollfunc_store_time,
- &at91_adc_trigger_handler, &at91_buffer_setup_ops);
-}
-
static unsigned at91_adc_startup_time(unsigned startup_time_min,
unsigned adc_clk_khz)
{
@@ -1683,6 +1661,40 @@ static const struct iio_info at91_adc_info = {
.hwfifo_set_watermark = &at91_adc_set_watermark,
};
+static int at91_adc_buffer_and_trigger_init(struct device *dev,
+ struct iio_dev *indio)
+{
+ struct at91_adc_state *st = iio_priv(indio);
+ int ret;
+
+ ret = devm_iio_triggered_buffer_setup(&indio->dev, indio,
+ &iio_pollfunc_store_time,
+ &at91_adc_trigger_handler, &at91_buffer_setup_ops);
+ if (ret < 0) {
+ dev_err(dev, "couldn't initialize the buffer.\n");
+ return ret;
+ }
+
+ if (!st->selected_trig->hw_trig)
+ return 0;
+
+ iio_buffer_set_attrs(indio->buffer, at91_adc_fifo_attributes);
+
+ st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
+ if (IS_ERR(st->trig)) {
+ dev_err(dev, "could not allocate trigger\n");
+ return PTR_ERR(st->trig);
+ }
+
+ /*
+ * Initially the iio buffer has a length of 2 and
+ * a watermark of 1
+ */
+ st->dma_st.watermark = 1;
+
+ return 0;
+}
+
static int at91_adc_probe(struct platform_device *pdev)
{
struct iio_dev *indio_dev;
@@ -1818,27 +1830,9 @@ static int at91_adc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, indio_dev);
- ret = at91_adc_buffer_init(indio_dev);
- if (ret < 0) {
- dev_err(&pdev->dev, "couldn't initialize the buffer.\n");
+ ret = at91_adc_buffer_and_trigger_init(&pdev->dev, indio_dev);
+ if (ret < 0)
goto per_clk_disable_unprepare;
- }
-
- if (st->selected_trig->hw_trig) {
- ret = at91_adc_trigger_init(indio_dev);
- if (ret < 0) {
- dev_err(&pdev->dev, "couldn't setup the triggers.\n");
- goto per_clk_disable_unprepare;
- }
- /*
- * Initially the iio buffer has a length of 2 and
- * a watermark of 1
- */
- st->dma_st.watermark = 1;
-
- iio_buffer_set_attrs(indio_dev->buffer,
- at91_adc_fifo_attributes);
- }
if (dma_coerce_mask_and_coherent(&indio_dev->dev, DMA_BIT_MASK(32)))
dev_info(&pdev->dev, "cannot set DMA mask to 32-bit\n");
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 3/9] iio: triggered-buffer: add {devm_}iio_triggered_buffer_setup_ext variants
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 1/9] iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs() Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 2/9] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 4/9] iio: accel: adxl372: use devm_iio_triggered_buffer_setup_ext() Alexandru Ardelean
` (6 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This change adds a parameter to the {devm_}iio_triggered_buffer_setup()
functions to assign the extra sysfs buffer attributes that are typically
assigned via iio_buffer_set_attrs().
The functions also get renamed to iio_triggered_buffer_setup_ext() &
devm_iio_triggered_buffer_setup_ext().
For backwards compatibility the old {devm_}iio_triggered_buffer_setup()
functions are now macros wrap the new (renamed) functions with NULL for the
buffer attrs.
The aim is to remove iio_buffer_set_attrs(), so in the
iio_triggered_buffer_setup_ext() function the attributes are assigned
directly to 'buffer->attrs'.
When adding multiple IIO buffers per IIO device, it can be pretty
cumbersome to first allocate a set of buffers, then to dig them out of IIO
to assign extra attributes (with iio_buffer_set_attrs()).
Naturally, the best way would be to provide them at allocation time, which
is what this change does.
At this moment, buffers allocated with {devm_}iio_triggered_buffer_setup()
are the only ones in mainline IIO to call iio_buffer_set_attrs().
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../buffer/industrialio-triggered-buffer.c | 31 ++++++++++++-------
include/linux/iio/triggered_buffer.h | 23 +++++++++-----
2 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
index 6c20a83f887e..92b8aea3e063 100644
--- a/drivers/iio/buffer/industrialio-triggered-buffer.c
+++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
@@ -9,18 +9,20 @@
#include <linux/module.h>
#include <linux/iio/iio.h>
#include <linux/iio/buffer.h>
+#include <linux/iio/buffer_impl.h>
#include <linux/iio/kfifo_buf.h>
#include <linux/iio/triggered_buffer.h>
#include <linux/iio/trigger_consumer.h>
/**
- * iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc
+ * iio_triggered_buffer_setup_ext() - Setup triggered buffer and pollfunc
* @indio_dev: IIO device structure
* @h: Function which will be used as pollfunc top half
* @thread: Function which will be used as pollfunc bottom half
* @setup_ops: Buffer setup functions to use for this device.
* If NULL the default setup functions for triggered
* buffers will be used.
+ * @buffer_attrs: Extra sysfs buffer attributes for this IIO buffer
*
* This function combines some common tasks which will normally be performed
* when setting up a triggered buffer. It will allocate the buffer and the
@@ -33,10 +35,11 @@
* To free the resources allocated by this function call
* iio_triggered_buffer_cleanup().
*/
-int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
+int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev,
irqreturn_t (*h)(int irq, void *p),
irqreturn_t (*thread)(int irq, void *p),
- const struct iio_buffer_setup_ops *setup_ops)
+ const struct iio_buffer_setup_ops *setup_ops,
+ const struct attribute **buffer_attrs)
{
struct iio_buffer *buffer;
int ret;
@@ -67,6 +70,8 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
/* Flag that polled ring buffering is possible */
indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
+ buffer->attrs = buffer_attrs;
+
return 0;
error_kfifo_free:
@@ -74,10 +79,10 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
error_ret:
return ret;
}
-EXPORT_SYMBOL(iio_triggered_buffer_setup);
+EXPORT_SYMBOL(iio_triggered_buffer_setup_ext);
/**
- * iio_triggered_buffer_cleanup() - Free resources allocated by iio_triggered_buffer_setup()
+ * iio_triggered_buffer_cleanup() - Free resources allocated by iio_triggered_buffer_setup_ext()
* @indio_dev: IIO device structure
*/
void iio_triggered_buffer_cleanup(struct iio_dev *indio_dev)
@@ -92,11 +97,12 @@ static void devm_iio_triggered_buffer_clean(struct device *dev, void *res)
iio_triggered_buffer_cleanup(*(struct iio_dev **)res);
}
-int devm_iio_triggered_buffer_setup(struct device *dev,
- struct iio_dev *indio_dev,
- irqreturn_t (*h)(int irq, void *p),
- irqreturn_t (*thread)(int irq, void *p),
- const struct iio_buffer_setup_ops *ops)
+int devm_iio_triggered_buffer_setup_ext(struct device *dev,
+ struct iio_dev *indio_dev,
+ irqreturn_t (*h)(int irq, void *p),
+ irqreturn_t (*thread)(int irq, void *p),
+ const struct iio_buffer_setup_ops *ops,
+ const struct attribute **buffer_attrs)
{
struct iio_dev **ptr;
int ret;
@@ -108,7 +114,8 @@ int devm_iio_triggered_buffer_setup(struct device *dev,
*ptr = indio_dev;
- ret = iio_triggered_buffer_setup(indio_dev, h, thread, ops);
+ ret = iio_triggered_buffer_setup_ext(indio_dev, h, thread, ops,
+ buffer_attrs);
if (!ret)
devres_add(dev, ptr);
else
@@ -116,7 +123,7 @@ int devm_iio_triggered_buffer_setup(struct device *dev,
return ret;
}
-EXPORT_SYMBOL_GPL(devm_iio_triggered_buffer_setup);
+EXPORT_SYMBOL_GPL(devm_iio_triggered_buffer_setup_ext);
MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
MODULE_DESCRIPTION("IIO helper functions for setting up triggered buffers");
diff --git a/include/linux/iio/triggered_buffer.h b/include/linux/iio/triggered_buffer.h
index e99c91799359..7f154d1f8739 100644
--- a/include/linux/iio/triggered_buffer.h
+++ b/include/linux/iio/triggered_buffer.h
@@ -4,19 +4,28 @@
#include <linux/interrupt.h>
+struct attribute;
struct iio_dev;
struct iio_buffer_setup_ops;
-int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
+int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev,
irqreturn_t (*h)(int irq, void *p),
irqreturn_t (*thread)(int irq, void *p),
- const struct iio_buffer_setup_ops *setup_ops);
+ const struct iio_buffer_setup_ops *setup_ops,
+ const struct attribute **buffer_attrs);
void iio_triggered_buffer_cleanup(struct iio_dev *indio_dev);
-int devm_iio_triggered_buffer_setup(struct device *dev,
- struct iio_dev *indio_dev,
- irqreturn_t (*h)(int irq, void *p),
- irqreturn_t (*thread)(int irq, void *p),
- const struct iio_buffer_setup_ops *ops);
+#define iio_triggered_buffer_setup(indio_dev, h, thread, setup_ops) \
+ iio_triggered_buffer_setup_ext((indio_dev), (h), (thread), (setup_ops), NULL)
+
+int devm_iio_triggered_buffer_setup_ext(struct device *dev,
+ struct iio_dev *indio_dev,
+ irqreturn_t (*h)(int irq, void *p),
+ irqreturn_t (*thread)(int irq, void *p),
+ const struct iio_buffer_setup_ops *ops,
+ const struct attribute **buffer_attrs);
+
+#define devm_iio_triggered_buffer_setup(dev, indio_dev, h, thread, setup_ops) \
+ devm_iio_triggered_buffer_setup_ext((dev), (indio_dev), (h), (thread), (setup_ops), NULL)
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 4/9] iio: accel: adxl372: use devm_iio_triggered_buffer_setup_ext()
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
` (2 preceding siblings ...)
2020-09-29 12:59 ` [PATCH v3 3/9] iio: triggered-buffer: add {devm_}iio_triggered_buffer_setup_ext variants Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 5/9] iio: accel: bmc150: use iio_triggered_buffer_setup_ext() Alexandru Ardelean
` (5 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This change switches to the new devm_iio_triggered_buffer_setup_ext()
function and removes the iio_buffer_set_attrs() call, for assigning the
HW FIFO attributes to the buffer.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/accel/adxl372.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index aed2a4930fb0..8ba1453b8dbf 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -1211,15 +1211,14 @@ int adxl372_probe(struct device *dev, struct regmap *regmap,
return ret;
}
- ret = devm_iio_triggered_buffer_setup(dev,
- indio_dev, NULL,
- adxl372_trigger_handler,
- &adxl372_buffer_ops);
+ ret = devm_iio_triggered_buffer_setup_ext(dev,
+ indio_dev, NULL,
+ adxl372_trigger_handler,
+ &adxl372_buffer_ops,
+ adxl372_fifo_attributes);
if (ret < 0)
return ret;
- iio_buffer_set_attrs(indio_dev->buffer, adxl372_fifo_attributes);
-
if (st->irq) {
st->dready_trig = devm_iio_trigger_alloc(dev,
"%s-dev%d",
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 5/9] iio: accel: bmc150: use iio_triggered_buffer_setup_ext()
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
` (3 preceding siblings ...)
2020-09-29 12:59 ` [PATCH v3 4/9] iio: accel: adxl372: use devm_iio_triggered_buffer_setup_ext() Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 6/9] iio: adc: at91-sama5d2_adc: use devm_iio_triggered_buffer_setup_ext() Alexandru Ardelean
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This change switches to the new iio_triggered_buffer_setup_ext()
function and removes the iio_buffer_set_attrs() call, for assigning the
HW FIFO attributes to the buffer.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/accel/bmc150-accel-core.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 48435865fdaf..c641ee552038 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1558,6 +1558,7 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
const char *name, bool block_supported)
{
+ const struct attribute **fifo_attrs;
struct bmc150_accel_data *data;
struct iio_dev *indio_dev;
int ret;
@@ -1590,10 +1591,19 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &bmc150_accel_info;
- ret = iio_triggered_buffer_setup(indio_dev,
- &iio_pollfunc_store_time,
- bmc150_accel_trigger_handler,
- &bmc150_accel_buffer_ops);
+ if (block_supported) {
+ indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
+ indio_dev->info = &bmc150_accel_info_fifo;
+ fifo_attrs = bmc150_accel_fifo_attributes;
+ } else {
+ fifo_attrs = NULL;
+ }
+
+ ret = iio_triggered_buffer_setup_ext(indio_dev,
+ &iio_pollfunc_store_time,
+ bmc150_accel_trigger_handler,
+ &bmc150_accel_buffer_ops,
+ fifo_attrs);
if (ret < 0) {
dev_err(dev, "Failed: iio triggered buffer setup\n");
return ret;
@@ -1628,13 +1638,6 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
ret = bmc150_accel_triggers_setup(indio_dev, data);
if (ret)
goto err_buffer_cleanup;
-
- if (block_supported) {
- indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
- indio_dev->info = &bmc150_accel_info_fifo;
- iio_buffer_set_attrs(indio_dev->buffer,
- bmc150_accel_fifo_attributes);
- }
}
ret = pm_runtime_set_active(dev);
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 6/9] iio: adc: at91-sama5d2_adc: use devm_iio_triggered_buffer_setup_ext()
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
` (4 preceding siblings ...)
2020-09-29 12:59 ` [PATCH v3 5/9] iio: accel: bmc150: use iio_triggered_buffer_setup_ext() Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 7/9] iio: cros_ec: " Alexandru Ardelean
` (3 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This change switches to the new devm_iio_triggered_buffer_setup_ext()
function and removes the iio_buffer_set_attrs() call, for assigning the
HW FIFO attributes to the buffer.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/at91-sama5d2_adc.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index b9c3cc6d5913..b012ce766f91 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -1665,11 +1665,17 @@ static int at91_adc_buffer_and_trigger_init(struct device *dev,
struct iio_dev *indio)
{
struct at91_adc_state *st = iio_priv(indio);
+ const struct attribute **fifo_attrs;
int ret;
- ret = devm_iio_triggered_buffer_setup(&indio->dev, indio,
+ if (st->selected_trig->hw_trig)
+ fifo_attrs = at91_adc_fifo_attributes;
+ else
+ fifo_attrs = NULL;
+
+ ret = devm_iio_triggered_buffer_setup_ext(&indio->dev, indio,
&iio_pollfunc_store_time,
- &at91_adc_trigger_handler, &at91_buffer_setup_ops);
+ &at91_adc_trigger_handler, &at91_buffer_setup_ops, fifo_attrs);
if (ret < 0) {
dev_err(dev, "couldn't initialize the buffer.\n");
return ret;
@@ -1678,8 +1684,6 @@ static int at91_adc_buffer_and_trigger_init(struct device *dev,
if (!st->selected_trig->hw_trig)
return 0;
- iio_buffer_set_attrs(indio->buffer, at91_adc_fifo_attributes);
-
st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
if (IS_ERR(st->trig)) {
dev_err(dev, "could not allocate trigger\n");
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
` (5 preceding siblings ...)
2020-09-29 12:59 ` [PATCH v3 6/9] iio: adc: at91-sama5d2_adc: use devm_iio_triggered_buffer_setup_ext() Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 13:08 ` Andy Shevchenko
2020-09-29 12:59 ` [PATCH v3 8/9] iio: hid-sensors: use iio_triggered_buffer_setup_ext() Alexandru Ardelean
` (2 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This change switches to the new devm_iio_triggered_buffer_setup_ext()
function and removes the iio_buffer_set_attrs() call, for assigning the
HW FIFO attributes to the buffer.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index c62cacc04672..1eafcf04ad69 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
if (ret)
return ret;
} else {
+ const struct attribute **fifo_attrs;
+
+ if (has_hw_fifo)
+ fifo_attrs = cros_ec_sensor_fifo_attributes;
+ else
+ fifo_attrs = NULL;
+
/*
* The only way to get samples in buffer is to set a
* software trigger (systrig, hrtimer).
*/
- ret = devm_iio_triggered_buffer_setup(
+ ret = devm_iio_triggered_buffer_setup_ext(
dev, indio_dev, NULL, trigger_capture,
- NULL);
+ NULL, fifo_attrs);
if (ret)
return ret;
-
- if (has_hw_fifo)
- iio_buffer_set_attrs(indio_dev->buffer,
- cros_ec_sensor_fifo_attributes);
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 8/9] iio: hid-sensors: use iio_triggered_buffer_setup_ext()
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
` (6 preceding siblings ...)
2020-09-29 12:59 ` [PATCH v3 7/9] iio: cros_ec: " Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 9/9] iio: buffer: remove iio_buffer_set_attrs() helper Alexandru Ardelean
2020-09-29 13:09 ` [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Andy Shevchenko
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
This change switches to the new iio_triggered_buffer_setup_ext()
function and removes the iio_buffer_set_attrs() call, for assigning the
HW FIFO attributes to the buffer.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../common/hid-sensors/hid-sensor-trigger.c | 22 ++++++++-----------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index ff375790b7e8..064c32bec9c7 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -84,15 +84,6 @@ static const struct attribute *hid_sensor_fifo_attributes[] = {
NULL,
};
-static void hid_sensor_setup_batch_mode(struct iio_dev *indio_dev,
- struct hid_sensor_common *st)
-{
- if (!hid_sensor_batch_mode_supported(st))
- return;
-
- iio_buffer_set_attrs(indio_dev->buffer, hid_sensor_fifo_attributes);
-}
-
static int _hid_sensor_power_state(struct hid_sensor_common *st, bool state)
{
int state_val;
@@ -247,11 +238,18 @@ static const struct iio_trigger_ops hid_sensor_trigger_ops = {
int hid_sensor_setup_trigger(struct iio_dev *indio_dev, const char *name,
struct hid_sensor_common *attrb)
{
+ const struct attribute **fifo_attrs;
int ret;
struct iio_trigger *trig;
- ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
- NULL, NULL);
+ if (hid_sensor_batch_mode_supported(attrb))
+ fifo_attrs = hid_sensor_fifo_attributes;
+ else
+ fifo_attrs = NULL;
+
+ ret = iio_triggered_buffer_setup_ext(indio_dev,
+ &iio_pollfunc_store_time,
+ NULL, NULL, fifo_attrs);
if (ret) {
dev_err(&indio_dev->dev, "Triggered Buffer Setup Failed\n");
return ret;
@@ -276,8 +274,6 @@ int hid_sensor_setup_trigger(struct iio_dev *indio_dev, const char *name,
attrb->trigger = trig;
indio_dev->trig = iio_trigger_get(trig);
- hid_sensor_setup_batch_mode(indio_dev, attrb);
-
ret = pm_runtime_set_active(&indio_dev->dev);
if (ret)
goto error_unreg_trigger;
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 9/9] iio: buffer: remove iio_buffer_set_attrs() helper
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
` (7 preceding siblings ...)
2020-09-29 12:59 ` [PATCH v3 8/9] iio: hid-sensors: use iio_triggered_buffer_setup_ext() Alexandru Ardelean
@ 2020-09-29 12:59 ` Alexandru Ardelean
2020-09-29 13:09 ` [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Andy Shevchenko
9 siblings, 0 replies; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 12:59 UTC (permalink / raw)
To: linux-iio, linux-kernel
Cc: jic23, eugen.hristev, nicolas.ferre, ludovic.desroches, bleung,
enric.balletbo, groeck, srinivas.pandruvada, andy.shevchenko,
gwendal, Alexandru Ardelean
The iio_buffer_set_attrs() is no longer used in the drivers, so it can be
removed now.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/industrialio-buffer.c | 12 ------------
include/linux/iio/buffer.h | 3 ---
2 files changed, 15 deletions(-)
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index a4f6bb96d4f4..9663dec3dcf3 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -210,18 +210,6 @@ void iio_buffer_init(struct iio_buffer *buffer)
}
EXPORT_SYMBOL(iio_buffer_init);
-/**
- * iio_buffer_set_attrs - Set buffer specific attributes
- * @buffer: The buffer for which we are setting attributes
- * @attrs: Pointer to a null terminated list of pointers to attributes
- */
-void iio_buffer_set_attrs(struct iio_buffer *buffer,
- const struct attribute **attrs)
-{
- buffer->attrs = attrs;
-}
-EXPORT_SYMBOL_GPL(iio_buffer_set_attrs);
-
static ssize_t iio_show_scan_index(struct device *dev,
struct device_attribute *attr,
char *buf)
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index fbba4093f06c..8febc23f5f26 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -11,9 +11,6 @@
struct iio_buffer;
-void iio_buffer_set_attrs(struct iio_buffer *buffer,
- const struct attribute **attrs);
-
int iio_push_to_buffers(struct iio_dev *indio_dev, const void *data);
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-09-29 12:59 ` [PATCH v3 7/9] iio: cros_ec: " Alexandru Ardelean
@ 2020-09-29 13:08 ` Andy Shevchenko
2020-09-29 14:31 ` Alexandru Ardelean
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2020-09-29 13:08 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, Linux Kernel Mailing List, Jonathan Cameron,
Eugen Hristev, Nicolas Ferre, Ludovic Desroches, Benson Leung,
Enric Balletbo i Serra, groeck, Srinivas Pandruvada,
Gwendal Grignou
On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
> This change switches to the new devm_iio_triggered_buffer_setup_ext()
> function and removes the iio_buffer_set_attrs() call, for assigning the
> HW FIFO attributes to the buffer.
Sorry, you were too fast with the version, below one nit.
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index c62cacc04672..1eafcf04ad69 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> if (ret)
> return ret;
> } else {
> + const struct attribute **fifo_attrs;
> +
> + if (has_hw_fifo)
> + fifo_attrs = cros_ec_sensor_fifo_attributes;
> + else
> + fifo_attrs = NULL;
> +
> /*
> * The only way to get samples in buffer is to set a
> * software trigger (systrig, hrtimer).
> */
> - ret = devm_iio_triggered_buffer_setup(
> + ret = devm_iio_triggered_buffer_setup_ext(
> dev, indio_dev, NULL, trigger_capture,
> - NULL);
> + NULL, fifo_attrs);
Perhaps it's time to reformat a bit, i.e. move dev to the first line
and do the rest accordingly?
> if (ret)
> return ret;
> -
> - if (has_hw_fifo)
> - iio_buffer_set_attrs(indio_dev->buffer,
> - cros_ec_sensor_fifo_attributes);
> }
> }
>
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
` (8 preceding siblings ...)
2020-09-29 12:59 ` [PATCH v3 9/9] iio: buffer: remove iio_buffer_set_attrs() helper Alexandru Ardelean
@ 2020-09-29 13:09 ` Andy Shevchenko
2020-09-29 13:09 ` Andy Shevchenko
9 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2020-09-29 13:09 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, Linux Kernel Mailing List, Jonathan Cameron,
Eugen Hristev, Nicolas Ferre, Ludovic Desroches, Benson Leung,
Enric Balletbo i Serra, groeck, Srinivas Pandruvada,
Gwendal Grignou
On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> This is a v2 & v3 for [1]:
> https://lore.kernel.org/linux-iio/20200925083743.46469-1-alexandru.ardelean@analog.com/
>
> It also includes a at91-sama5d2_adc cleanup patch in this series:
> https://lore.kernel.org/linux-iio/20200924102902.136169-1-alexandru.ardelean@analog.com/
> This patch is required, in order to make the removal of
> iio_buffer_set_attrs() a bit cleaner in the at91-sama5d2_adc driver.
>
> Following the discussion from [1], this patchset implements the
> following:
Makes sense to me, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenjko>
>
> Changelog v2 -> v3:
> * in patch 'iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs()'
> - minor stylistic change;
>
> Changelog v1 -> v2:
> * rename '{devm_}iio_triggered_buffer_setup()' ->
> '{devm_}iio_triggered_buffer_setup_ext()'
> - wrap with macros the new ext functions to preserve backwards
> compatibility
> - add a new parameter to the ext functions, which are the
> buffer->attrs
> * split into separate patches the removal [from each driver] of
> iio_buffer_set_attrs() and the switch to a
> {devm_}iio_triggered_buffer_setup_ext variant
> * add patch to remove iio_buffer_set_attrs() from DMAEngine IIO buffer
> * remove the iio_buffer_set_attrs() helper in a final/separate patch
> * add 'at91-sama5d2_adc: merge buffer & trigger' patch in this series,
> so that the removal of iio_buffer_set_attrs() is easier to view
>
> Alexandru Ardelean (9):
> iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs()
> iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a
> function
> iio: triggered-buffer: add {devm_}iio_triggered_buffer_setup_ext
> variants
> iio: accel: adxl372: use devm_iio_triggered_buffer_setup_ext()
> iio: accel: bmc150: use iio_triggered_buffer_setup_ext()
> iio: adc: at91-sama5d2_adc: use devm_iio_triggered_buffer_setup_ext()
> iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
> iio: hid-sensors: use iio_triggered_buffer_setup_ext()
> iio: buffer: remove iio_buffer_set_attrs() helper
>
> drivers/iio/accel/adxl372.c | 11 ++-
> drivers/iio/accel/bmc150-accel-core.c | 25 +++---
> drivers/iio/adc/at91-sama5d2_adc.c | 82 +++++++++----------
> .../buffer/industrialio-buffer-dmaengine.c | 3 +-
> .../buffer/industrialio-triggered-buffer.c | 31 ++++---
> .../cros_ec_sensors/cros_ec_sensors_core.c | 15 ++--
> .../common/hid-sensors/hid-sensor-trigger.c | 22 ++---
> drivers/iio/industrialio-buffer.c | 12 ---
> include/linux/iio/buffer.h | 3 -
> include/linux/iio/triggered_buffer.h | 23 ++++--
> 10 files changed, 113 insertions(+), 114 deletions(-)
>
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc
2020-09-29 13:09 ` [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Andy Shevchenko
@ 2020-09-29 13:09 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2020-09-29 13:09 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, Linux Kernel Mailing List, Jonathan Cameron,
Eugen Hristev, Nicolas Ferre, Ludovic Desroches, Benson Leung,
Enric Balletbo i Serra, groeck, Srinivas Pandruvada,
Gwendal Grignou
On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> >
> > This is a v2 & v3 for [1]:
> > https://lore.kernel.org/linux-iio/20200925083743.46469-1-alexandru.ardelean@analog.com/
> >
> > It also includes a at91-sama5d2_adc cleanup patch in this series:
> > https://lore.kernel.org/linux-iio/20200924102902.136169-1-alexandru.ardelean@analog.com/
> > This patch is required, in order to make the removal of
> > iio_buffer_set_attrs() a bit cleaner in the at91-sama5d2_adc driver.
> >
> > Following the discussion from [1], this patchset implements the
> > following:
>
> Makes sense to me, FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenjko>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-09-29 13:08 ` Andy Shevchenko
@ 2020-09-29 14:31 ` Alexandru Ardelean
2020-09-29 15:40 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Alexandru Ardelean @ 2020-09-29 14:31 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Alexandru Ardelean, linux-iio, Linux Kernel Mailing List,
Jonathan Cameron, Eugen Hristev, Nicolas Ferre,
Ludovic Desroches, Benson Leung, Enric Balletbo i Serra, groeck,
Srinivas Pandruvada, Gwendal Grignou
On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
>
> > This change switches to the new devm_iio_triggered_buffer_setup_ext()
> > function and removes the iio_buffer_set_attrs() call, for assigning the
> > HW FIFO attributes to the buffer.
>
> Sorry, you were too fast with the version, below one nit.
>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > index c62cacc04672..1eafcf04ad69 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > if (ret)
> > return ret;
> > } else {
> > + const struct attribute **fifo_attrs;
> > +
> > + if (has_hw_fifo)
> > + fifo_attrs = cros_ec_sensor_fifo_attributes;
> > + else
> > + fifo_attrs = NULL;
> > +
> > /*
> > * The only way to get samples in buffer is to set a
> > * software trigger (systrig, hrtimer).
> > */
> > - ret = devm_iio_triggered_buffer_setup(
>
> > + ret = devm_iio_triggered_buffer_setup_ext(
> > dev, indio_dev, NULL, trigger_capture,
> > - NULL);
> > + NULL, fifo_attrs);
>
> Perhaps it's time to reformat a bit, i.e. move dev to the first line
> and do the rest accordingly?
this feels like a mix of preferences here;
for once, the patch here [as-is], is the minimal form for this change
[in terms of patch-noise];
so, some people would choose the least noisiest patch;
also, this indentation was chosen [as-is here] from the start [for
this code block];
not sure if it was preferred; i'd suspect it was due to the old 80-col limit;
i'd leave it as-is [for now], or defer the decision to a maintainer to
decide [either IIO or chromium];
>
> > if (ret)
> > return ret;
> > -
> > - if (has_hw_fifo)
> > - iio_buffer_set_attrs(indio_dev->buffer,
> > - cros_ec_sensor_fifo_attributes);
> > }
> > }
> >
> > --
> > 2.17.1
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-09-29 14:31 ` Alexandru Ardelean
@ 2020-09-29 15:40 ` Jonathan Cameron
2020-11-18 10:35 ` Alexandru Ardelean
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2020-09-29 15:40 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: Andy Shevchenko, Alexandru Ardelean, linux-iio,
Linux Kernel Mailing List, Eugen Hristev, Nicolas Ferre,
Ludovic Desroches, Benson Leung, Enric Balletbo i Serra, groeck,
Srinivas Pandruvada, Gwendal Grignou
On Tue, 29 Sep 2020 17:31:55 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
> > <alexandru.ardelean@analog.com> wrote:
> >
> > > This change switches to the new devm_iio_triggered_buffer_setup_ext()
> > > function and removes the iio_buffer_set_attrs() call, for assigning the
> > > HW FIFO attributes to the buffer.
> >
> > Sorry, you were too fast with the version, below one nit.
> >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > ---
> > > .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
> > > 1 file changed, 9 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > index c62cacc04672..1eafcf04ad69 100644
> > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > if (ret)
> > > return ret;
> > > } else {
> > > + const struct attribute **fifo_attrs;
> > > +
> > > + if (has_hw_fifo)
> > > + fifo_attrs = cros_ec_sensor_fifo_attributes;
> > > + else
> > > + fifo_attrs = NULL;
> > > +
> > > /*
> > > * The only way to get samples in buffer is to set a
> > > * software trigger (systrig, hrtimer).
> > > */
> > > - ret = devm_iio_triggered_buffer_setup(
> >
> > > + ret = devm_iio_triggered_buffer_setup_ext(
> > > dev, indio_dev, NULL, trigger_capture,
> > > - NULL);
> > > + NULL, fifo_attrs);
> >
> > Perhaps it's time to reformat a bit, i.e. move dev to the first line
> > and do the rest accordingly?
>
> this feels like a mix of preferences here;
> for once, the patch here [as-is], is the minimal form for this change
> [in terms of patch-noise];
> so, some people would choose the least noisiest patch;
>
> also, this indentation was chosen [as-is here] from the start [for
> this code block];
> not sure if it was preferred; i'd suspect it was due to the old 80-col limit;
>
> i'd leave it as-is [for now], or defer the decision to a maintainer to
> decide [either IIO or chromium];
The indenting of this whole code block is a bit too deep.
Looks to me like we should flip the sense of the outer if statement
if (!physical_device)
return 0;
That would lead to a whole bunch of reformatting around here including
picking up this.
For now I can just shuffle it a bit whilst applying.
This set isn't likely to make the merge window anyway now as I'd like
it to sit on list a little longer just because it touches several
drivers with active maintainers and I'd like time for them to sanity
check.
Jonathan
>
> >
> > > if (ret)
> > > return ret;
> > > -
> > > - if (has_hw_fifo)
> > > - iio_buffer_set_attrs(indio_dev->buffer,
> > > - cros_ec_sensor_fifo_attributes);
> > > }
> > > }
> > >
> > > --
> > > 2.17.1
> > >
> >
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-09-29 15:40 ` Jonathan Cameron
@ 2020-11-18 10:35 ` Alexandru Ardelean
2020-11-21 14:47 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Alexandru Ardelean @ 2020-11-18 10:35 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Alexandru Ardelean, linux-iio,
Linux Kernel Mailing List, Eugen Hristev, Nicolas Ferre,
Ludovic Desroches, Benson Leung, Enric Balletbo i Serra, groeck,
Srinivas Pandruvada, Gwendal Grignou
On Tue, Sep 29, 2020 at 6:40 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 29 Sep 2020 17:31:55 +0300
> Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
>
> > On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
> > > <alexandru.ardelean@analog.com> wrote:
> > >
> > > > This change switches to the new devm_iio_triggered_buffer_setup_ext()
> > > > function and removes the iio_buffer_set_attrs() call, for assigning the
> > > > HW FIFO attributes to the buffer.
> > >
> > > Sorry, you were too fast with the version, below one nit.
> > >
> > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > ---
> > > > .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
> > > > 1 file changed, 9 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > index c62cacc04672..1eafcf04ad69 100644
> > > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > > if (ret)
> > > > return ret;
> > > > } else {
> > > > + const struct attribute **fifo_attrs;
> > > > +
> > > > + if (has_hw_fifo)
> > > > + fifo_attrs = cros_ec_sensor_fifo_attributes;
> > > > + else
> > > > + fifo_attrs = NULL;
> > > > +
> > > > /*
> > > > * The only way to get samples in buffer is to set a
> > > > * software trigger (systrig, hrtimer).
> > > > */
> > > > - ret = devm_iio_triggered_buffer_setup(
> > >
> > > > + ret = devm_iio_triggered_buffer_setup_ext(
> > > > dev, indio_dev, NULL, trigger_capture,
> > > > - NULL);
> > > > + NULL, fifo_attrs);
> > >
> > > Perhaps it's time to reformat a bit, i.e. move dev to the first line
> > > and do the rest accordingly?
> >
> > this feels like a mix of preferences here;
> > for once, the patch here [as-is], is the minimal form for this change
> > [in terms of patch-noise];
> > so, some people would choose the least noisiest patch;
> >
> > also, this indentation was chosen [as-is here] from the start [for
> > this code block];
> > not sure if it was preferred; i'd suspect it was due to the old 80-col limit;
> >
> > i'd leave it as-is [for now], or defer the decision to a maintainer to
> > decide [either IIO or chromium];
>
> The indenting of this whole code block is a bit too deep.
>
> Looks to me like we should flip the sense of the outer if statement
>
> if (!physical_device)
> return 0;
>
> That would lead to a whole bunch of reformatting around here including
> picking up this.
>
> For now I can just shuffle it a bit whilst applying.
>
> This set isn't likely to make the merge window anyway now as I'd like
> it to sit on list a little longer just because it touches several
> drivers with active maintainers and I'd like time for them to sanity
> check.
>
ping on this;
should i do a V4 for this?
this is related to the multiple IIO buffer support:
https://lore.kernel.org/linux-iio/20201117162340.43924-1-alexandru.ardelean@analog.com/T/#t
it's one of the patchsets i could split away on it's own;
> Jonathan
>
>
> >
> > >
> > > > if (ret)
> > > > return ret;
> > > > -
> > > > - if (has_hw_fifo)
> > > > - iio_buffer_set_attrs(indio_dev->buffer,
> > > > - cros_ec_sensor_fifo_attributes);
> > > > }
> > > > }
> > > >
> > > > --
> > > > 2.17.1
> > > >
> > >
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-11-18 10:35 ` Alexandru Ardelean
@ 2020-11-21 14:47 ` Jonathan Cameron
2020-11-21 14:55 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2020-11-21 14:47 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: Andy Shevchenko, Alexandru Ardelean, linux-iio,
Linux Kernel Mailing List, Eugen Hristev, Nicolas Ferre,
Ludovic Desroches, Benson Leung, Enric Balletbo i Serra, groeck,
Srinivas Pandruvada, Gwendal Grignou
On Wed, 18 Nov 2020 12:35:16 +0200
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> On Tue, Sep 29, 2020 at 6:40 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Tue, 29 Sep 2020 17:31:55 +0300
> > Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> >
> > > On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > >
> > > > On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
> > > > <alexandru.ardelean@analog.com> wrote:
> > > >
> > > > > This change switches to the new devm_iio_triggered_buffer_setup_ext()
> > > > > function and removes the iio_buffer_set_attrs() call, for assigning the
> > > > > HW FIFO attributes to the buffer.
> > > >
> > > > Sorry, you were too fast with the version, below one nit.
> > > >
> > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > > ---
> > > > > .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
> > > > > 1 file changed, 9 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > index c62cacc04672..1eafcf04ad69 100644
> > > > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > > > if (ret)
> > > > > return ret;
> > > > > } else {
> > > > > + const struct attribute **fifo_attrs;
> > > > > +
> > > > > + if (has_hw_fifo)
> > > > > + fifo_attrs = cros_ec_sensor_fifo_attributes;
> > > > > + else
> > > > > + fifo_attrs = NULL;
> > > > > +
> > > > > /*
> > > > > * The only way to get samples in buffer is to set a
> > > > > * software trigger (systrig, hrtimer).
> > > > > */
> > > > > - ret = devm_iio_triggered_buffer_setup(
> > > >
> > > > > + ret = devm_iio_triggered_buffer_setup_ext(
> > > > > dev, indio_dev, NULL, trigger_capture,
> > > > > - NULL);
> > > > > + NULL, fifo_attrs);
> > > >
> > > > Perhaps it's time to reformat a bit, i.e. move dev to the first line
> > > > and do the rest accordingly?
> > >
> > > this feels like a mix of preferences here;
> > > for once, the patch here [as-is], is the minimal form for this change
> > > [in terms of patch-noise];
> > > so, some people would choose the least noisiest patch;
> > >
> > > also, this indentation was chosen [as-is here] from the start [for
> > > this code block];
> > > not sure if it was preferred; i'd suspect it was due to the old 80-col limit;
> > >
> > > i'd leave it as-is [for now], or defer the decision to a maintainer to
> > > decide [either IIO or chromium];
> >
> > The indenting of this whole code block is a bit too deep.
> >
> > Looks to me like we should flip the sense of the outer if statement
> >
> > if (!physical_device)
> > return 0;
> >
> > That would lead to a whole bunch of reformatting around here including
> > picking up this.
> >
> > For now I can just shuffle it a bit whilst applying.
> >
> > This set isn't likely to make the merge window anyway now as I'd like
> > it to sit on list a little longer just because it touches several
> > drivers with active maintainers and I'd like time for them to sanity
> > check.
> >
>
> ping on this;
> should i do a V4 for this?
Yes, probably worth sending out again. I'd like to see a few more acks
on the individual drivers ideally and a v4 will get this to the
top of peoples' inboxes.
If we don't get them I won't let it block this series, but it's nice
to try at least!
Thanks,
Jonathan
>
> this is related to the multiple IIO buffer support:
> https://lore.kernel.org/linux-iio/20201117162340.43924-1-alexandru.ardelean@analog.com/T/#t
>
> it's one of the patchsets i could split away on it's own;
>
> > Jonathan
> >
> >
> > >
> > > >
> > > > > if (ret)
> > > > > return ret;
> > > > > -
> > > > > - if (has_hw_fifo)
> > > > > - iio_buffer_set_attrs(indio_dev->buffer,
> > > > > - cros_ec_sensor_fifo_attributes);
> > > > > }
> > > > > }
> > > > >
> > > > > --
> > > > > 2.17.1
> > > > >
> > > >
> > > >
> > > > --
> > > > With Best Regards,
> > > > Andy Shevchenko
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-11-21 14:47 ` Jonathan Cameron
@ 2020-11-21 14:55 ` Jonathan Cameron
2020-11-21 14:56 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2020-11-21 14:55 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: Andy Shevchenko, Alexandru Ardelean, linux-iio,
Linux Kernel Mailing List, Eugen Hristev, Nicolas Ferre,
Ludovic Desroches, Benson Leung, Enric Balletbo i Serra, groeck,
Srinivas Pandruvada, Gwendal Grignou
On Sat, 21 Nov 2020 14:47:39 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 18 Nov 2020 12:35:16 +0200
> Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
>
> > On Tue, Sep 29, 2020 at 6:40 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Tue, 29 Sep 2020 17:31:55 +0300
> > > Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> > >
> > > > On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > >
> > > > > On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
> > > > > <alexandru.ardelean@analog.com> wrote:
> > > > >
> > > > > > This change switches to the new devm_iio_triggered_buffer_setup_ext()
> > > > > > function and removes the iio_buffer_set_attrs() call, for assigning the
> > > > > > HW FIFO attributes to the buffer.
> > > > >
> > > > > Sorry, you were too fast with the version, below one nit.
> > > > >
> > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > > > ---
> > > > > > .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
> > > > > > 1 file changed, 9 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > > index c62cacc04672..1eafcf04ad69 100644
> > > > > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > > @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > > > > if (ret)
> > > > > > return ret;
> > > > > > } else {
> > > > > > + const struct attribute **fifo_attrs;
> > > > > > +
> > > > > > + if (has_hw_fifo)
> > > > > > + fifo_attrs = cros_ec_sensor_fifo_attributes;
> > > > > > + else
> > > > > > + fifo_attrs = NULL;
> > > > > > +
> > > > > > /*
> > > > > > * The only way to get samples in buffer is to set a
> > > > > > * software trigger (systrig, hrtimer).
> > > > > > */
> > > > > > - ret = devm_iio_triggered_buffer_setup(
> > > > >
> > > > > > + ret = devm_iio_triggered_buffer_setup_ext(
> > > > > > dev, indio_dev, NULL, trigger_capture,
> > > > > > - NULL);
> > > > > > + NULL, fifo_attrs);
> > > > >
> > > > > Perhaps it's time to reformat a bit, i.e. move dev to the first line
> > > > > and do the rest accordingly?
> > > >
> > > > this feels like a mix of preferences here;
> > > > for once, the patch here [as-is], is the minimal form for this change
> > > > [in terms of patch-noise];
> > > > so, some people would choose the least noisiest patch;
> > > >
> > > > also, this indentation was chosen [as-is here] from the start [for
> > > > this code block];
> > > > not sure if it was preferred; i'd suspect it was due to the old 80-col limit;
> > > >
> > > > i'd leave it as-is [for now], or defer the decision to a maintainer to
> > > > decide [either IIO or chromium];
> > >
> > > The indenting of this whole code block is a bit too deep.
> > >
> > > Looks to me like we should flip the sense of the outer if statement
> > >
> > > if (!physical_device)
> > > return 0;
> > >
> > > That would lead to a whole bunch of reformatting around here including
> > > picking up this.
> > >
> > > For now I can just shuffle it a bit whilst applying.
> > >
> > > This set isn't likely to make the merge window anyway now as I'd like
> > > it to sit on list a little longer just because it touches several
> > > drivers with active maintainers and I'd like time for them to sanity
> > > check.
> > >
> >
> > ping on this;
> > should i do a V4 for this?
> Yes, probably worth sending out again. I'd like to see a few more acks
> on the individual drivers ideally and a v4 will get this to the
> top of peoples' inboxes.
>
> If we don't get them I won't let it block this series, but it's nice
> to try at least!
Actually scratch that this is old now and should be fine to check by
inspection.
I'll have a go at applying it and if nothing odd happens we should
be good.
I was too lazy to fix the alignment Andy pointed out but would like
to see the more major refactoring as discussed for that patch if one
of us gets round to it sometime :)
Hmm. b4 doesn't deal with accidental half typed email addresses so
just went through and fixed all those.
Thanks,
Jonathan
>
> Thanks,
>
> Jonathan
>
> >
> > this is related to the multiple IIO buffer support:
> > https://lore.kernel.org/linux-iio/20201117162340.43924-1-alexandru.ardelean@analog.com/T/#t
> >
> > it's one of the patchsets i could split away on it's own;
> >
> > > Jonathan
> > >
> > >
> > > >
> > > > >
> > > > > > if (ret)
> > > > > > return ret;
> > > > > > -
> > > > > > - if (has_hw_fifo)
> > > > > > - iio_buffer_set_attrs(indio_dev->buffer,
> > > > > > - cros_ec_sensor_fifo_attributes);
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > --
> > > > > > 2.17.1
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > With Best Regards,
> > > > > Andy Shevchenko
> > >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] iio: cros_ec: use devm_iio_triggered_buffer_setup_ext()
2020-11-21 14:55 ` Jonathan Cameron
@ 2020-11-21 14:56 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2020-11-21 14:56 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: Andy Shevchenko, Alexandru Ardelean, linux-iio,
Linux Kernel Mailing List, Eugen Hristev, Nicolas Ferre,
Ludovic Desroches, Benson Leung, Enric Balletbo i Serra, groeck,
Srinivas Pandruvada, Gwendal Grignou
On Sat, 21 Nov 2020 14:47:39 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 18 Nov 2020 12:35:16 +0200
> Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
>
> > On Tue, Sep 29, 2020 at 6:40 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Tue, 29 Sep 2020 17:31:55 +0300
> > > Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> > >
> > > > On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > >
> > > > > On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean
> > > > > <alexandru.ardelean@analog.com> wrote:
> > > > >
> > > > > > This change switches to the new devm_iio_triggered_buffer_setup_ext()
> > > > > > function and removes the iio_buffer_set_attrs() call, for assigning the
> > > > > > HW FIFO attributes to the buffer.
> > > > >
> > > > > Sorry, you were too fast with the version, below one nit.
> > > > >
> > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > > > ---
> > > > > > .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 +++++++++------
> > > > > > 1 file changed, 9 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > > index c62cacc04672..1eafcf04ad69 100644
> > > > > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> > > > > > @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> > > > > > if (ret)
> > > > > > return ret;
> > > > > > } else {
> > > > > > + const struct attribute **fifo_attrs;
> > > > > > +
> > > > > > + if (has_hw_fifo)
> > > > > > + fifo_attrs = cros_ec_sensor_fifo_attributes;
> > > > > > + else
> > > > > > + fifo_attrs = NULL;
> > > > > > +
> > > > > > /*
> > > > > > * The only way to get samples in buffer is to set a
> > > > > > * software trigger (systrig, hrtimer).
> > > > > > */
> > > > > > - ret = devm_iio_triggered_buffer_setup(
> > > > >
> > > > > > + ret = devm_iio_triggered_buffer_setup_ext(
> > > > > > dev, indio_dev, NULL, trigger_capture,
> > > > > > - NULL);
> > > > > > + NULL, fifo_attrs);
> > > > >
> > > > > Perhaps it's time to reformat a bit, i.e. move dev to the first line
> > > > > and do the rest accordingly?
> > > >
> > > > this feels like a mix of preferences here;
> > > > for once, the patch here [as-is], is the minimal form for this change
> > > > [in terms of patch-noise];
> > > > so, some people would choose the least noisiest patch;
> > > >
> > > > also, this indentation was chosen [as-is here] from the start [for
> > > > this code block];
> > > > not sure if it was preferred; i'd suspect it was due to the old 80-col limit;
> > > >
> > > > i'd leave it as-is [for now], or defer the decision to a maintainer to
> > > > decide [either IIO or chromium];
> > >
> > > The indenting of this whole code block is a bit too deep.
> > >
> > > Looks to me like we should flip the sense of the outer if statement
> > >
> > > if (!physical_device)
> > > return 0;
> > >
> > > That would lead to a whole bunch of reformatting around here including
> > > picking up this.
> > >
> > > For now I can just shuffle it a bit whilst applying.
> > >
> > > This set isn't likely to make the merge window anyway now as I'd like
> > > it to sit on list a little longer just because it touches several
> > > drivers with active maintainers and I'd like time for them to sanity
> > > check.
> > >
> >
> > ping on this;
> > should i do a V4 for this?
> Yes, probably worth sending out again. I'd like to see a few more acks
> on the individual drivers ideally and a v4 will get this to the
> top of peoples' inboxes.
>
> If we don't get them I won't let it block this series, but it's nice
> to try at least!
Actually scratch that this is old now and should be fine to check by
inspection.
I'll have a go at applying it and if nothing odd happens we should
be good.
I was too lazy to fix the alignment Andy pointed out but would like
to see the more major refactoring as discussed for that patch if one
of us gets round to it sometime :)
Hmm. b4 doesn't deal with accidental half typed email addresses so
just went through and fixed all those.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to poke at it.
Thanks,
Jonathan
>
> Thanks,
>
> Jonathan
>
> >
> > this is related to the multiple IIO buffer support:
> > https://lore.kernel.org/linux-iio/20201117162340.43924-1-alexandru.ardelean@analog.com/T/#t
> >
> > it's one of the patchsets i could split away on it's own;
> >
> > > Jonathan
> > >
> > >
> > > >
> > > > >
> > > > > > if (ret)
> > > > > > return ret;
> > > > > > -
> > > > > > - if (has_hw_fifo)
> > > > > > - iio_buffer_set_attrs(indio_dev->buffer,
> > > > > > - cros_ec_sensor_fifo_attributes);
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > --
> > > > > > 2.17.1
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > With Best Regards,
> > > > > Andy Shevchenko
> > >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-11-21 14:56 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 12:59 [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 1/9] iio: buffer: dmaengine: unwrap the use of iio_buffer_set_attrs() Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 2/9] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 3/9] iio: triggered-buffer: add {devm_}iio_triggered_buffer_setup_ext variants Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 4/9] iio: accel: adxl372: use devm_iio_triggered_buffer_setup_ext() Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 5/9] iio: accel: bmc150: use iio_triggered_buffer_setup_ext() Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 6/9] iio: adc: at91-sama5d2_adc: use devm_iio_triggered_buffer_setup_ext() Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 7/9] iio: cros_ec: " Alexandru Ardelean
2020-09-29 13:08 ` Andy Shevchenko
2020-09-29 14:31 ` Alexandru Ardelean
2020-09-29 15:40 ` Jonathan Cameron
2020-11-18 10:35 ` Alexandru Ardelean
2020-11-21 14:47 ` Jonathan Cameron
2020-11-21 14:55 ` Jonathan Cameron
2020-11-21 14:56 ` Jonathan Cameron
2020-09-29 12:59 ` [PATCH v3 8/9] iio: hid-sensors: use iio_triggered_buffer_setup_ext() Alexandru Ardelean
2020-09-29 12:59 ` [PATCH v3 9/9] iio: buffer: remove iio_buffer_set_attrs() helper Alexandru Ardelean
2020-09-29 13:09 ` [PATCH v3 0/9] iio: remove iio_buffer_set_attrs() and assign buffer attrs during alloc Andy Shevchenko
2020-09-29 13:09 ` Andy Shevchenko
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).