Hi Nuno,
On 6/24/24 17:22, Nuno Sá wrote:
Hi Olivier,
On Mon, 2024-06-24 at 14:43 +0200, Olivier MOYSAN wrote:
Hi Jonathan,
On 6/23/24 17:11, Jonathan Cameron wrote:
On Tue, 18 Jun 2024 18:08:33 +0200
Olivier Moysan <olivier.moysan@xxxxxxxxxxx> wrote:
Add a generic driver to support sigma delta modulators.
Typically, this device is a hardware connected to an IIO device
in charge of the conversion. The device is exposed as an IIO backend
device. This backend device and the associated conversion device
can be seen as an aggregate device from IIO framework.
Signed-off-by: Olivier Moysan <olivier.moysan@xxxxxxxxxxx>
Trivial comments inline.
diff --git a/drivers/iio/adc/sd_adc_backend.c
b/drivers/iio/adc/sd_adc_backend.c
new file mode 100644
index 000000000000..556a49dc537b
--- /dev/null
+++ b/drivers/iio/adc/sd_adc_backend.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic sigma delta modulator IIO backend
+ *
+ * Copyright (C) 2024, STMicroelectronics - All Rights Reserved
+ */
+
+#include <linux/iio/backend.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct iio_sd_backend_priv {
+ struct regulator *vref;
+ int vref_mv;
+};
+
+static int sd_backend_enable(struct iio_backend *backend)
+{
+ struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+ return regulator_enable(priv->vref);
+};
+
+static void sd_backend_disable(struct iio_backend *backend)
+{
+ struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+ regulator_disable(priv->vref);
+};
+
+static int sd_backend_read(struct iio_backend *backend, int *val, int *val2,
long mask)
Nothing to do with this patch as such:
One day I'm going to bit the bullet and fix that naming.
Long long ago when the Earth was young it actually was a bitmap which
I miscalled a mask - it only had one bit ever set, which was a dumb
bit of API. It's not been true for a long time.
Anyhow, one more instances isn't too much of a problem I guess.
I changed the callback .read_raw to .ext_info_get to take Nuno's comment
about iio_backend_read_raw() API, into account.
So, I changed this function to
static int sd_backend_ext_info_get(struct iio_backend *back, uintptr_t
private, const struct iio_chan_spec *chan, char *buf)
for v2 version.
Maybe I'm missing something but I think I did not explained myself very well. What I
had in mind was that since you're calling .read_raw() from IIO_CHAN_INFO_SCALE and
IIO_CHAN_INFO_OFFSET, it could make sense to have more dedicated API's. Meaning:
iio_backend_read_scale(...)
iio_backend_read_offset(...)
The iio_backend_read_raw() may make sense when frontends call
iio_backend_extend_chan_spec() and have no idea what the backend may have added to
the channel. So, in those cases something like this could make sense:
switch (mask)
IIO_CHAN_INFO_RAW:
...
default:
return iio_backend_read_raw();
but like I said maybe this is me over-complicating and a simple
iio_backend_read_raw() is sufficient. But I think I never mentioned something like
.read_raw -> .ext_info_get.
Thanks for clarification. Your previous message was actually clear
enough regarding iio_backend_read_raw() API.
However, your comment about extend_chan_spec(), let me think that I
could maybe spare a new API, and just re-use iio_backend_ext_info_get()
callback.
Nevertheless, this API cannot be used directly, as it can be used only
for a frontend associated to a single backend. There is a comment in
iio_backend_ext_info_get() about the need of another API for such case.
So I considered introducing this new API (instead of read_raw):
ssize_t iio_backend_ext_info_get_from_backend(struct iio_backend *back,
uintptr_t private, const struct iio_chan_spec *chan, char *buf)
(I'm not sure this name is the most relevant).
But if you don't like this alternative too much, I will keep the initial
"catch all" iio_backend_read_raw() API.
BRs
Olivier
The other thing I mentioned was to instead of having:
if (child) {
ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OFFSET);
}
You could use iio_backend_extend_chan_spec() and have the backend set the SCALE and
OFFSET bits for you as it seems these functionality depends on the backend.
But none of the above were critical or things that I feel to strong about.
Anyways, just wanted to give some clarification as it seems there were some
misunderstandings (I think).
> - Nuno Sá
