* [PATCH v4 0/4] Add IIO backend support for AD7779
@ 2025-08-20 12:02 Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 1/4] iio: adc: adi-axi-adc: add axi_adc_num_lanes_set Ioana Risteiu
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Ioana Risteiu @ 2025-08-20 12:02 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
Cc: Ioana Risteiu
- Add axi_adc_num_lanes_set in the adi_axi_adc_ops structure to support
setting number of lanes used by AXI ADC.
- Add the generic io-backends property to the AD7779 binding to enable
support for the IIO backend framework.
- Add the adi,num-lanes property to set the number of lanes used by
AD7779.
- Move the initialization specific to communication without iio-backend
into a separate setup function.
- Add a new functionality to ad7779 driver that streams data through data
output interface using IIO backend interface.
Ioana Risteiu (4):
iio: adc: adi-axi-adc: add axi_adc_num_lanes_set
dt-bindings: iio: adc: add IIO backend support
iio: adc: extract setup function without backend
iio: adc: update ad7779 to use IIO backend
.../bindings/iio/adc/adi,ad7779.yaml | 44 +++-
drivers/iio/adc/ad7779.c | 197 ++++++++++++++----
drivers/iio/adc/adi-axi-adc.c | 1 +
3 files changed, 203 insertions(+), 39 deletions(-)
--
2.47.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/4] iio: adc: adi-axi-adc: add axi_adc_num_lanes_set
2025-08-20 12:02 [PATCH v4 0/4] Add IIO backend support for AD7779 Ioana Risteiu
@ 2025-08-20 12:02 ` Ioana Risteiu
2025-08-20 14:14 ` Andy Shevchenko
2025-08-20 12:02 ` [PATCH v4 2/4] dt-bindings: iio: adc: add IIO backend support Ioana Risteiu
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Ioana Risteiu @ 2025-08-20 12:02 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
Cc: Ioana Risteiu
Add axi_adc_num_lanes_set in the adi_axi_adc_ops structure to support
setting number of lanes used by AXI ADC. This operation is included in
the generic structure because the number of lanes is a configurable
parameter of the generic AXI ADC IP core, not specific to a device.
Signed-off-by: Ioana Risteiu <Ioana.Risteiu@analog.com>
---
changes in v4:
- improved patch description
drivers/iio/adc/adi-axi-adc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 2d86bb0e08a7..20bb59c24acf 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -617,6 +617,7 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
.chan_status = axi_adc_chan_status,
.interface_type_get = axi_adc_interface_type_get,
.oversampling_ratio_set = axi_adc_oversampling_ratio_set,
+ .num_lanes_set = axi_adc_num_lanes_set,
.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
};
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/4] dt-bindings: iio: adc: add IIO backend support
2025-08-20 12:02 [PATCH v4 0/4] Add IIO backend support for AD7779 Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 1/4] iio: adc: adi-axi-adc: add axi_adc_num_lanes_set Ioana Risteiu
@ 2025-08-20 12:02 ` Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 3/4] iio: adc: extract setup function without backend Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend Ioana Risteiu
3 siblings, 0 replies; 9+ messages in thread
From: Ioana Risteiu @ 2025-08-20 12:02 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
Cc: Ioana Risteiu, Krzysztof Kozlowski
Add the generic io-backends property to the AD7779 binding to enable
support for the IIO backend framework.
Also add the adi,num-lanes property to set the number of lanes used by
AD7779.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Ioana Risteiu <Ioana.Risteiu@analog.com>
---
changes in v4:
- included previously omitted reviewers from earlier patches
- modified the logic for handling adi,num-lanes in devicetree
.../bindings/iio/adc/adi,ad7779.yaml | 44 ++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
index 044f92f39cfa..ba3f7b2bd6cf 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7779.yaml
@@ -80,11 +80,36 @@ properties:
reset-gpios:
maxItems: 1
+ io-backends:
+ maxItems: 1
+
+ adi,num-lanes:
+ description:
+ Number of lanes on which the data is sent on the output when the data
+ output interface is used.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2, 4]
+ default: 4
+
required:
- compatible
- reg
- clocks
- - interrupts
+
+allOf:
+ - if:
+ not:
+ required:
+ - io-backends
+ then:
+ properties:
+ adi,num-lanes: false
+
+oneOf:
+ - required:
+ - interrupts
+ - required:
+ - io-backends
unevaluatedProperties: false
@@ -107,4 +132,21 @@ examples:
clocks = <&adc_clk>;
};
};
+
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad7779";
+ reg = <0>;
+ start-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
+ reset-gpios = <&gpio0 93 GPIO_ACTIVE_LOW>;
+ clocks = <&adc_clk>;
+ io-backends = <&iio_backend>;
+ adi,num-lanes = <4>;
+ };
+ };
...
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/4] iio: adc: extract setup function without backend
2025-08-20 12:02 [PATCH v4 0/4] Add IIO backend support for AD7779 Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 1/4] iio: adc: adi-axi-adc: add axi_adc_num_lanes_set Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 2/4] dt-bindings: iio: adc: add IIO backend support Ioana Risteiu
@ 2025-08-20 12:02 ` Ioana Risteiu
2025-08-20 14:13 ` Andy Shevchenko
2025-08-20 12:02 ` [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend Ioana Risteiu
3 siblings, 1 reply; 9+ messages in thread
From: Ioana Risteiu @ 2025-08-20 12:02 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
Cc: Ioana Risteiu
Refactor probe function by moving the initialization specific to
communication without iio-backend into a separate setup function.
The purpose of this modification is better code organization. No
functional changes intended.
Signed-off-by: Ioana Risteiu <Ioana.Risteiu@analog.com>
---
no changes in v4.
drivers/iio/adc/ad7779.c | 83 ++++++++++++++++++++++------------------
1 file changed, 45 insertions(+), 38 deletions(-)
diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
index 845adc510239..f7e681c0e8c0 100644
--- a/drivers/iio/adc/ad7779.c
+++ b/drivers/iio/adc/ad7779.c
@@ -752,6 +752,50 @@ static int ad7779_conf(struct ad7779_state *st, struct gpio_desc *start_gpio)
return 0;
}
+static int ad7779_setup_without_backend(struct ad7779_state *st, struct iio_dev *indio_dev)
+{
+ int ret;
+ struct device *dev = &st->spi->dev;
+
+ indio_dev->info = &ad7779_info;
+ indio_dev->channels = st->chip_info->channels;
+
+ st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
+ iio_device_id(indio_dev));
+ if (!st->trig)
+ return -ENOMEM;
+
+ st->trig->ops = &ad7779_trigger_ops;
+
+ iio_trigger_set_drvdata(st->trig, st);
+
+ ret = devm_request_irq(dev, st->spi->irq, iio_trigger_generic_data_rdy_poll,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN, indio_dev->name,
+ st->trig);
+ if (ret)
+ return dev_err_probe(dev, ret, "request IRQ %d failed\n",
+ st->spi->irq);
+
+ ret = devm_iio_trigger_register(dev, st->trig);
+ if (ret)
+ return ret;
+
+ indio_dev->trig = iio_trigger_get(st->trig);
+
+ init_completion(&st->completion);
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+ &iio_pollfunc_store_time,
+ &ad7779_trigger_handler,
+ &ad7779_buffer_setup_ops);
+ if (ret)
+ return ret;
+
+ return ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
+ AD7779_DCLK_CLK_DIV_MSK,
+ FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
+}
+
static int ad7779_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
@@ -760,9 +804,6 @@ static int ad7779_probe(struct spi_device *spi)
struct device *dev = &spi->dev;
int ret = -EINVAL;
- if (!spi->irq)
- return dev_err_probe(dev, ret, "DRDY irq not present\n");
-
indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
@@ -804,45 +845,11 @@ static int ad7779_probe(struct spi_device *spi)
return ret;
indio_dev->name = st->chip_info->name;
- indio_dev->info = &ad7779_info;
indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->channels = st->chip_info->channels;
indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
- st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
- iio_device_id(indio_dev));
- if (!st->trig)
- return -ENOMEM;
-
- st->trig->ops = &ad7779_trigger_ops;
-
- iio_trigger_set_drvdata(st->trig, st);
-
- ret = devm_request_irq(dev, spi->irq, iio_trigger_generic_data_rdy_poll,
- IRQF_ONESHOT | IRQF_NO_AUTOEN, indio_dev->name,
- st->trig);
- if (ret)
- return dev_err_probe(dev, ret, "request IRQ %d failed\n",
- st->spi->irq);
-
- ret = devm_iio_trigger_register(dev, st->trig);
- if (ret)
- return ret;
-
- indio_dev->trig = iio_trigger_get(st->trig);
-
- init_completion(&st->completion);
-
- ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
- &iio_pollfunc_store_time,
- &ad7779_trigger_handler,
- &ad7779_buffer_setup_ops);
- if (ret)
- return ret;
+ ret = ad7779_setup_without_backend(st, indio_dev);
- ret = ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
- AD7779_DCLK_CLK_DIV_MSK,
- FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
if (ret)
return ret;
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend
2025-08-20 12:02 [PATCH v4 0/4] Add IIO backend support for AD7779 Ioana Risteiu
` (2 preceding siblings ...)
2025-08-20 12:02 ` [PATCH v4 3/4] iio: adc: extract setup function without backend Ioana Risteiu
@ 2025-08-20 12:02 ` Ioana Risteiu
2025-08-20 14:18 ` Andy Shevchenko
3 siblings, 1 reply; 9+ messages in thread
From: Ioana Risteiu @ 2025-08-20 12:02 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
Cc: Ioana Risteiu
Add a new functionality to ad7779 driver that streams data through data
output interface using IIO backend interface.
Signed-off-by: Ioana Risteiu <Ioana.Risteiu@analog.com>
---
changes in v4:
- grouped includes in alphabetical order
- reordered fields in struct ad7779_state
- modified logic of setting the number of lanes
- validating devicetree value
drivers/iio/adc/ad7779.c | 116 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 115 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/ad7779.c b/drivers/iio/adc/ad7779.c
index f7e681c0e8c0..adca490061c9 100644
--- a/drivers/iio/adc/ad7779.c
+++ b/drivers/iio/adc/ad7779.c
@@ -25,6 +25,7 @@
#include <linux/units.h>
#include <linux/iio/iio.h>
+#include <linux/iio/backend.h>
#include <linux/iio/buffer.h>
#include <linux/iio/sysfs.h>
#include <linux/iio/trigger.h>
@@ -116,6 +117,12 @@
#define AD7779_CRC8_POLY 0x07
DECLARE_CRC8_TABLE(ad7779_crc8_table);
+enum ad7779_data_lines {
+ AD7779_1LINE = 1,
+ AD7779_2LINES = 2,
+ AD7779_4LINES = 4,
+};
+
enum ad7779_filter {
AD7779_SINC3,
AD7779_SINC5,
@@ -145,6 +152,7 @@ struct ad7779_state {
struct completion completion;
unsigned int sampling_freq;
enum ad7779_filter filter_enabled;
+ struct iio_backend *back;
/*
* DMA (thus cache coherency maintenance) requires the
* transfer buffers to live in their own cache lines.
@@ -630,12 +638,38 @@ static int ad7779_reset(struct iio_dev *indio_dev, struct gpio_desc *reset_gpio)
return ret;
}
+static int ad7779_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+ unsigned int c;
+ int ret;
+
+ for (c = 0; c < AD7779_NUM_CHANNELS; c++) {
+ if (test_bit(c, scan_mask))
+ ret = iio_backend_chan_enable(st->back, c);
+ else
+ ret = iio_backend_chan_disable(st->back, c);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct iio_info ad7779_info = {
.read_raw = ad7779_read_raw,
.write_raw = ad7779_write_raw,
.debugfs_reg_access = &ad7779_reg_access,
};
+static const struct iio_info ad7779_info_data = {
+ .read_raw = ad7779_read_raw,
+ .write_raw = ad7779_write_raw,
+ .debugfs_reg_access = &ad7779_reg_access,
+ .update_scan_mode = &ad7779_update_scan_mode,
+};
+
static const struct iio_enum ad7779_filter_enum = {
.items = ad7779_filter_type,
.num_items = ARRAY_SIZE(ad7779_filter_type),
@@ -752,6 +786,49 @@ static int ad7779_conf(struct ad7779_state *st, struct gpio_desc *start_gpio)
return 0;
}
+static int ad7779_set_data_lines(struct iio_dev *indio_dev,
+ unsigned int num_lanes)
+{
+ struct ad7779_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ if (num_lanes != AD7779_1LINE &&
+ num_lanes != AD7779_2LINES &&
+ num_lanes != AD7779_4LINES)
+ return ret;
+
+ ret = ad7779_set_sampling_frequency(st, num_lanes * AD7779_DEFAULT_SAMPLING_1LINE);
+ if (ret)
+ return ret;
+
+ ret = iio_backend_num_lanes_set(st->back, num_lanes);
+ if (ret)
+ return ret;
+
+ return ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
+ AD7779_DOUT_FORMAT_MSK,
+ FIELD_PREP(AD7779_DOUT_FORMAT_MSK, 2 - ilog2(num_lanes)));
+}
+
+static int ad7779_setup_channels(struct iio_dev *indio_dev, const struct ad7779_state *st)
+{
+ struct iio_chan_spec *channels;
+ struct device *dev = &st->spi->dev;
+
+ channels = devm_kmemdup_array(dev, st->chip_info->channels,
+ ARRAY_SIZE(ad7779_channels),
+ sizeof(*channels), GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
+ for (int i = 0; i < ARRAY_SIZE(ad7779_channels); i++)
+ channels[i].scan_type.endianness = IIO_CPU;
+
+ indio_dev->channels = channels;
+
+ return 0;
+}
+
static int ad7779_setup_without_backend(struct ad7779_state *st, struct iio_dev *indio_dev)
{
int ret;
@@ -796,6 +873,39 @@ static int ad7779_setup_without_backend(struct ad7779_state *st, struct iio_dev
FIELD_PREP(AD7779_DCLK_CLK_DIV_MSK, 7));
}
+static int ad7779_setup_backend(struct ad7779_state *st, struct iio_dev *indio_dev)
+{
+ struct device *dev = &st->spi->dev;
+ int ret = -EINVAL;
+ int num_lanes;
+
+ indio_dev->info = &ad7779_info_data;
+
+ ret = ad7779_setup_channels(indio_dev, st);
+ if (ret)
+ return ret;
+
+ st->back = devm_iio_backend_get(dev, NULL);
+ if (IS_ERR(st->back))
+ return dev_err_probe(dev, PTR_ERR(st->back),
+ "failed to get iio backend");
+
+ ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_backend_enable(dev, st->back);
+ if (ret)
+ return ret;
+
+ num_lanes = 4;
+ ret = device_property_read_u32(dev, "adi,num-lanes", &num_lanes);
+ if (ret && ret != -EINVAL)
+ return ret;
+
+ return ad7779_set_data_lines(indio_dev, num_lanes);
+}
+
static int ad7779_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
@@ -848,7 +958,10 @@ static int ad7779_probe(struct spi_device *spi)
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->num_channels = ARRAY_SIZE(ad7779_channels);
- ret = ad7779_setup_without_backend(st, indio_dev);
+ if (device_property_present(dev, "io-backends"))
+ ret = ad7779_setup_backend(st, indio_dev);
+ else
+ ret = ad7779_setup_without_backend(st, indio_dev);
if (ret)
return ret;
@@ -943,3 +1056,4 @@ module_spi_driver(ad7779_driver);
MODULE_AUTHOR("Ramona Alexandra Nechita <ramona.nechita@analog.com>");
MODULE_DESCRIPTION("Analog Devices AD7779 ADC");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_BACKEND");
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 3/4] iio: adc: extract setup function without backend
2025-08-20 12:02 ` [PATCH v4 3/4] iio: adc: extract setup function without backend Ioana Risteiu
@ 2025-08-20 14:13 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2025-08-20 14:13 UTC (permalink / raw)
To: Ioana Risteiu
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
On Wed, Aug 20, 2025 at 03:02:44PM +0300, Ioana Risteiu wrote:
> Refactor probe function by moving the initialization specific to
> communication without iio-backend into a separate setup function.
>
> The purpose of this modification is better code organization. No
> functional changes intended.
...
> + if (ret)
> + return dev_err_probe(dev, ret, "request IRQ %d failed\n",
> + st->spi->irq);
Something went wrong with the indentation.
...
> + ret = ad7779_setup_without_backend(st, indio_dev);
>
Now we have redundant blank line here, please remove it.
> if (ret)
> return ret;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/4] iio: adc: adi-axi-adc: add axi_adc_num_lanes_set
2025-08-20 12:02 ` [PATCH v4 1/4] iio: adc: adi-axi-adc: add axi_adc_num_lanes_set Ioana Risteiu
@ 2025-08-20 14:14 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2025-08-20 14:14 UTC (permalink / raw)
To: Ioana Risteiu
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
On Wed, Aug 20, 2025 at 03:02:42PM +0300, Ioana Risteiu wrote:
> Add axi_adc_num_lanes_set in the adi_axi_adc_ops structure to support
axi_adc_num_lanes_set()
or I think even better to rephrase
"Assign num_lanes_set in the adi_axi_adc_ops to axi_adc_num_lanes_set()..."
> setting number of lanes used by AXI ADC. This operation is included in
> the generic structure because the number of lanes is a configurable
> parameter of the generic AXI ADC IP core, not specific to a device.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend
2025-08-20 12:02 ` [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend Ioana Risteiu
@ 2025-08-20 14:18 ` Andy Shevchenko
2025-08-25 10:15 ` Jonathan Cameron
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2025-08-20 14:18 UTC (permalink / raw)
To: Ioana Risteiu
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
On Wed, Aug 20, 2025 at 03:02:45PM +0300, Ioana Risteiu wrote:
> Add a new functionality to ad7779 driver that streams data through data
> output interface using IIO backend interface.
...
> +static int ad7779_set_data_lines(struct iio_dev *indio_dev,
> + unsigned int num_lanes)
> +{
> + struct ad7779_state *st = iio_priv(indio_dev);
> + int ret = -EINVAL;
In general the split assignment is easier to maintain and less prone to subtle
errors. In this case it's even worse as it's not needed...
> + if (num_lanes != AD7779_1LINE &&
> + num_lanes != AD7779_2LINES &&
> + num_lanes != AD7779_4LINES)
> + return ret;
...just return the error code directly here.
> + ret = ad7779_set_sampling_frequency(st, num_lanes * AD7779_DEFAULT_SAMPLING_1LINE);
> + if (ret)
> + return ret;
> +
> + ret = iio_backend_num_lanes_set(st->back, num_lanes);
> + if (ret)
> + return ret;
> +
> + return ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
> + AD7779_DOUT_FORMAT_MSK,
> + FIELD_PREP(AD7779_DOUT_FORMAT_MSK, 2 - ilog2(num_lanes)));
> +}
...
> +static int ad7779_setup_channels(struct iio_dev *indio_dev, const struct ad7779_state *st)
> +{
> + struct iio_chan_spec *channels;
> + struct device *dev = &st->spi->dev;
> +
> + channels = devm_kmemdup_array(dev, st->chip_info->channels,
> + ARRAY_SIZE(ad7779_channels),
> + sizeof(*channels), GFP_KERNEL);
Indentation...
> + if (!channels)
> + return -ENOMEM;
> +
> + for (int i = 0; i < ARRAY_SIZE(ad7779_channels); i++)
Why signed iterator?
> + channels[i].scan_type.endianness = IIO_CPU;
> +
> + indio_dev->channels = channels;
> +
> + return 0;
> +}
...
> +static int ad7779_setup_backend(struct ad7779_state *st, struct iio_dev *indio_dev)
> +{
> + struct device *dev = &st->spi->dev;
> + int ret = -EINVAL;
Why?!
> + int num_lanes;
Can it be negatie?
> + indio_dev->info = &ad7779_info_data;
> +
> + ret = ad7779_setup_channels(indio_dev, st);
> + if (ret)
> + return ret;
> +
> + st->back = devm_iio_backend_get(dev, NULL);
> + if (IS_ERR(st->back))
> + return dev_err_probe(dev, PTR_ERR(st->back),
> + "failed to get iio backend");
> +
> + ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = devm_iio_backend_enable(dev, st->back);
> + if (ret)
> + return ret;
> +
> + num_lanes = 4;
> + ret = device_property_read_u32(dev, "adi,num-lanes", &num_lanes);
> + if (ret && ret != -EINVAL)
> + return ret;
> +
> + return ad7779_set_data_lines(indio_dev, num_lanes);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend
2025-08-20 14:18 ` Andy Shevchenko
@ 2025-08-25 10:15 ` Jonathan Cameron
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2025-08-25 10:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ioana Risteiu, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ramona Nechita, linux-iio,
devicetree, linux-kernel
On Wed, 20 Aug 2025 17:18:05 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Wed, Aug 20, 2025 at 03:02:45PM +0300, Ioana Risteiu wrote:
> > Add a new functionality to ad7779 driver that streams data through data
> > output interface using IIO backend interface.
A few follow ups from me.
Jonathan
> ...
>
> > +static int ad7779_set_data_lines(struct iio_dev *indio_dev,
> > + unsigned int num_lanes)
> > +{
> > + struct ad7779_state *st = iio_priv(indio_dev);
>
> > + int ret = -EINVAL;
>
> In general the split assignment is easier to maintain and less prone to subtle
> errors. In this case it's even worse as it's not needed...
>
> > + if (num_lanes != AD7779_1LINE &&
> > + num_lanes != AD7779_2LINES &&
> > + num_lanes != AD7779_4LINES)
> > + return ret;
>
> ...just return the error code directly here.
Those enum values in general seem like a bad idea. Just use numbers.
If the enum was used as a type it would be a different situation but here it
is used as a glorified way to give the 1 lane to the value 1 etc which just
makes it harder to read.
>
> > + ret = ad7779_set_sampling_frequency(st, num_lanes * AD7779_DEFAULT_SAMPLING_1LINE);
> > + if (ret)
> > + return ret;
> > +
> > + ret = iio_backend_num_lanes_set(st->back, num_lanes);
> > + if (ret)
> > + return ret;
> > +
> > + return ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
> > + AD7779_DOUT_FORMAT_MSK,
> > + FIELD_PREP(AD7779_DOUT_FORMAT_MSK, 2 - ilog2(num_lanes)));
> > +}
>
> ...
>
> > +static int ad7779_setup_channels(struct iio_dev *indio_dev, const struct ad7779_state *st)
> > +{
> > + struct iio_chan_spec *channels;
> > + struct device *dev = &st->spi->dev;
> > +
> > + channels = devm_kmemdup_array(dev, st->chip_info->channels,
> > + ARRAY_SIZE(ad7779_channels),
> > + sizeof(*channels), GFP_KERNEL);
>
> Indentation...
>
> > + if (!channels)
> > + return -ENOMEM;
> > +
> > + for (int i = 0; i < ARRAY_SIZE(ad7779_channels); i++)
>
> Why signed iterator?
>
> > + channels[i].scan_type.endianness = IIO_CPU;
> > +
> > + indio_dev->channels = channels;
> > +
> > + return 0;
> > +}
>
> ...
>
> > +static int ad7779_setup_backend(struct ad7779_state *st, struct iio_dev *indio_dev)
> > +{
> > + struct device *dev = &st->spi->dev;
>
> > + int ret = -EINVAL;
>
> Why?!
>
> > + int num_lanes;
>
> Can it be negatie?
Good spot.
Even beyond sign it needs to be a u32 given how it is used.
>
> > + indio_dev->info = &ad7779_info_data;
> > +
> > + ret = ad7779_setup_channels(indio_dev, st);
> > + if (ret)
> > + return ret;
> > +
> > + st->back = devm_iio_backend_get(dev, NULL);
> > + if (IS_ERR(st->back))
> > + return dev_err_probe(dev, PTR_ERR(st->back),
> > + "failed to get iio backend");
> > +
> > + ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_iio_backend_enable(dev, st->back);
> > + if (ret)
> > + return ret;
> > +
> > + num_lanes = 4;
> > + ret = device_property_read_u32(dev, "adi,num-lanes", &num_lanes);
> > + if (ret && ret != -EINVAL)
> > + return ret;
> > +
> > + return ad7779_set_data_lines(indio_dev, num_lanes);
> > +}
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-08-25 10:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 12:02 [PATCH v4 0/4] Add IIO backend support for AD7779 Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 1/4] iio: adc: adi-axi-adc: add axi_adc_num_lanes_set Ioana Risteiu
2025-08-20 14:14 ` Andy Shevchenko
2025-08-20 12:02 ` [PATCH v4 2/4] dt-bindings: iio: adc: add IIO backend support Ioana Risteiu
2025-08-20 12:02 ` [PATCH v4 3/4] iio: adc: extract setup function without backend Ioana Risteiu
2025-08-20 14:13 ` Andy Shevchenko
2025-08-20 12:02 ` [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend Ioana Risteiu
2025-08-20 14:18 ` Andy Shevchenko
2025-08-25 10:15 ` 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).