2023-09-22 00:43:55

by David Lechner

[permalink] [raw]
Subject: [v2 00/19] iio: resolver: move ad2s1210 out of staging

Changes since v1:
* Address initial device tree patch feedback
* Drop "iio: sysfs: add IIO_DEVICE_ATTR_NAMED_RW macro" (related cleanups
also dropped for now, will address in a future series if needed)
* Apply improvements as a series as patches to the staging driver. It is not
quite ready for the move out of staging patch yet.

This series has been tested on actual hardware using a EVAL-AD2S1210 evaluation
board. (Note: not all device tree features have been implemented in the driver
since the eval board doesn't support them out of the box. We plan to add them
later if needed.)

One thing left over from the staging driver that probably needs more attention
still is the fault handling (both the fault threshold attributes and how
userspace gets notified of fault conditions). We considered adding these as
events, but the fault conditions are related to internal measurements in the
chip that aren't available as channels.

Since the chip is designed to read the fault register each time we read the
data registers for one of the two channels it seems like faults should be
associated with channels one way or another. Would it make sense to add extra
channels for the internal signals that only have fault events (mostly with
IIO_EV_TYPE_THRESH)? Or would it make sense to add a new "flags" channel type
where the "raw" value is bit flags? Or something else?

Here is the table of available faults for context. Sine/cosine inputs are
internal signals.

| Bit | Description
+-----+------------
| D7 | Sine/cosine inputs clipped
| D6 | Sine/cosine inputs below LOS threshold
| D5 | Sine/cosine inputs exceed DOS overrange threshold
| D4 | Sine/cosine inputs exceed DOS mismatch threshold
| D3 | Tracking error exceeds LOT threshold
| D2 | Velocity exceeds maximum tracking rate
| D1 | Phase error exceeds phase lock range
| D0 | Configuration parity error

David Lechner (19):
dt-bindings: iio: resolver: add devicetree bindings for ad2s1210
staging: iio: Documentation: document IIO resolver AD2S1210 sysfs
attributes
staging: iio: resolver: ad2s1210: fix ad2s1210_show_fault
staging: iio: resolver: ad2s1210: fix not restoring sample gpio in
channel read
staging: iio: resolver: ad2s1210: fix probe
staging: iio: resolver: ad2s1210: always use 16-bit value for raw read
staging: iio: resolver: ad2s1210: implement IIO_CHAN_INFO_SCALE
staging: iio: resolver: ad2s1210: use devicetree to get fclkin
staging: iio: resolver: ad2s1210: use regmap for config registers
staging: iio: resolver: ad2s1210: add debugfs reg access
staging: iio: resolver: ad2s1210: remove config attribute
staging: iio: resolver: ad2s1210: rework gpios
staging: iio: resolver: ad2s1210: implement hysteresis as channel attr
staging: iio: resolver: ad2s1210: refactor setting excitation
frequency
staging: iio: resolver: ad2s1210: read excitation frequency from
control register
staging: iio: resolver: ad2s1210: rename fexcit attribute
staging: iio: resolver: ad2s1210: convert resolution to devicetree
property
staging: iio: resolver: ad2s1210: add phase_lock_range attributes
staging: iio: resolver: ad2s1210: add triggered buffer support

.../bindings/iio/resolver/adi,ad2s1210.yaml | 150 +++
.../sysfs-bus-iio-resolver-ad2s1210 | 109 ++
drivers/staging/iio/resolver/Kconfig | 1 +
drivers/staging/iio/resolver/ad2s1210.c | 948 +++++++++++-------
4 files changed, 857 insertions(+), 351 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/resolver/adi,ad2s1210.yaml
create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-resolver-ad2s1210

--
2.34.1


2023-09-22 00:52:04

by David Lechner

[permalink] [raw]
Subject: [v2 08/19] staging: iio: resolver: ad2s1210: use devicetree to get fclkin

This removes the fclkin sysfs attribute and replaces it with getting
the fclkin clock rate using the clk subsystem (i.e. from the
devicetree).

The fclkin clock comes from an external oscillator that is connected
directly to the AD2S1210 chip, so users of the sysfs attributes should
not need to be concerned with this.

Also sort includes while we are touching this.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/Kconfig | 1 +
drivers/staging/iio/resolver/ad2s1210.c | 76 +++++++++----------------
2 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/drivers/staging/iio/resolver/Kconfig b/drivers/staging/iio/resolver/Kconfig
index 6d1e2622e0b0..bebb35822c9e 100644
--- a/drivers/staging/iio/resolver/Kconfig
+++ b/drivers/staging/iio/resolver/Kconfig
@@ -7,6 +7,7 @@ menu "Resolver to digital converters"
config AD2S1210
tristate "Analog Devices ad2s1210 driver"
depends on SPI
+ depends on COMMON_CLK
depends on GPIOLIB || COMPILE_TEST
help
Say yes here to build support for Analog Devices spi resolver
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 95d43b241a75..153ac7704ad7 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -5,16 +5,17 @@
* Copyright (c) 2010-2010 Analog Devices Inc.
* Copyright (C) 2023 BayLibre, SAS
*/
-#include <linux/types.h>
-#include <linux/mutex.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/of.h>
-#include <linux/spi/spi.h>
#include <linux/slab.h>
+#include <linux/spi/spi.h>
#include <linux/sysfs.h>
-#include <linux/delay.h>
-#include <linux/gpio/consumer.h>
-#include <linux/module.h>
+#include <linux/types.h>

#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
@@ -91,7 +92,8 @@ struct ad2s1210_state {
struct mutex lock;
struct spi_device *sdev;
struct gpio_desc *gpios[5];
- unsigned int fclkin;
+ /** The external oscillator frequency in Hz. */
+ unsigned long fclkin;
unsigned int fexcit;
bool hysteresis;
u8 resolution;
@@ -198,45 +200,6 @@ static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
return ad2s1210_config_write(st, 0x0);
}

-static ssize_t ad2s1210_show_fclkin(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
-
- return sprintf(buf, "%u\n", st->fclkin);
-}
-
-static ssize_t ad2s1210_store_fclkin(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
- unsigned int fclkin;
- int ret;
-
- ret = kstrtouint(buf, 10, &fclkin);
- if (ret)
- return ret;
- if (fclkin < AD2S1210_MIN_CLKIN || fclkin > AD2S1210_MAX_CLKIN) {
- dev_err(dev, "ad2s1210: fclkin out of range\n");
- return -EINVAL;
- }
-
- mutex_lock(&st->lock);
- st->fclkin = fclkin;
-
- ret = ad2s1210_update_frequency_control_word(st);
- if (ret < 0)
- goto error_ret;
- ret = ad2s1210_soft_reset(st);
-error_ret:
- mutex_unlock(&st->lock);
-
- return ret < 0 ? ret : len;
-}
-
static ssize_t ad2s1210_show_fexcit(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -546,8 +509,6 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
return ret;
}

-static IIO_DEVICE_ATTR(fclkin, 0644,
- ad2s1210_show_fclkin, ad2s1210_store_fclkin, 0);
static IIO_DEVICE_ATTR(fexcit, 0644,
ad2s1210_show_fexcit, ad2s1210_store_fexcit, 0);
static IIO_DEVICE_ATTR(control, 0644,
@@ -596,7 +557,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
};

static struct attribute *ad2s1210_attributes[] = {
- &iio_dev_attr_fclkin.dev_attr.attr,
&iio_dev_attr_fexcit.dev_attr.attr,
&iio_dev_attr_control.dev_attr.attr,
&iio_dev_attr_bits.dev_attr.attr,
@@ -654,6 +614,24 @@ static const struct iio_info ad2s1210_info = {
.attrs = &ad2s1210_attribute_group,
};

+static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
+{
+ struct device *dev = &st->sdev->dev;
+ struct clk *clk;
+
+ clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
+
+ st->fclkin = clk_get_rate(clk);
+ if (st->fclkin < AD2S1210_MIN_CLKIN || st->fclkin > AD2S1210_MAX_CLKIN)
+ return dev_err_probe(dev, -EINVAL,
+ "clock frequency out of range: %lu\n",
+ st->fclkin);
+
+ return 0;
+}
+
static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
{
struct spi_device *spi = st->sdev;
--
2.34.1

2023-09-22 01:43:36

by David Lechner

[permalink] [raw]
Subject: [v2 14/19] staging: iio: resolver: ad2s1210: refactor setting excitation frequency

This combines the ad2s1210_update_frequency_control_word() and
ad2s1210_soft_reset() functions into a single function since they
both have to be called together.

Also clean up a few things while touching this:
- move AD2S1210_DEF_EXCIT macro with similar macros
- remove unnecessary dev_err() calls

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 63 ++++++++++++-------------
1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index fe413759deb9..f1ffee34ebbc 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -53,12 +53,11 @@
#define AD2S1210_MIN_CLKIN 6144000
#define AD2S1210_MAX_CLKIN 10240000
#define AD2S1210_MIN_EXCIT 2000
+#define AD2S1210_DEF_EXCIT 10000
#define AD2S1210_MAX_EXCIT 20000
#define AD2S1210_MIN_FCW 0x4
#define AD2S1210_MAX_FCW 0x50

-#define AD2S1210_DEF_EXCIT 10000
-
enum ad2s1210_mode {
MOD_POS = 0b00,
MOD_VEL = 0b01,
@@ -184,18 +183,29 @@ static int ad2s1210_regmap_reg_read(void *context, unsigned int reg,
return 0;
}

-static inline
-int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
+/*
+ * Sets the excitation frequency and performs software reset.
+ *
+ * Must be called with lock held.
+ */
+static int ad2s1210_set_excitation_frequency(struct ad2s1210_state *st,
+ u16 fexcit)
{
- unsigned char fcw;
+ int ret;
+ u8 fcw;

- fcw = (unsigned char)(st->fexcit * (1 << 15) / st->fclkin);
- if (fcw < AD2S1210_MIN_FCW || fcw > AD2S1210_MAX_FCW) {
- dev_err(&st->sdev->dev, "ad2s1210: FCW out of range\n");
+ fcw = fexcit * (1 << 15) / st->fclkin;
+ if (fcw < AD2S1210_MIN_FCW || fcw > AD2S1210_MAX_FCW)
return -ERANGE;
- }

- return regmap_write(st->regmap, AD2S1210_REG_EXCIT_FREQ, fcw);
+ ret = regmap_write(st->regmap, AD2S1210_REG_EXCIT_FREQ, fcw);
+ if (ret < 0)
+ return ret;
+
+ st->fexcit = fexcit;
+
+ /* software reset reinitializes the excitation frequency output */
+ return regmap_write(st->regmap, AD2S1210_REG_SOFT_RESET, 0);
}

static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st,
@@ -210,11 +220,6 @@ static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st,
bitmap);
}

-static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
-{
- return regmap_write(st->regmap, AD2S1210_REG_SOFT_RESET, 0);
-}
-
static ssize_t ad2s1210_show_fexcit(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -229,27 +234,24 @@ static ssize_t ad2s1210_store_fexcit(struct device *dev,
const char *buf, size_t len)
{
struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
- unsigned int fexcit;
+ u16 fexcit;
int ret;

- ret = kstrtouint(buf, 10, &fexcit);
- if (ret < 0)
- return ret;
- if (fexcit < AD2S1210_MIN_EXCIT || fexcit > AD2S1210_MAX_EXCIT) {
- dev_err(dev,
- "ad2s1210: excitation frequency out of range\n");
+ ret = kstrtou16(buf, 10, &fexcit);
+ if (ret < 0 || fexcit < AD2S1210_MIN_EXCIT || fexcit > AD2S1210_MAX_EXCIT)
return -EINVAL;
- }
+
mutex_lock(&st->lock);
- st->fexcit = fexcit;
- ret = ad2s1210_update_frequency_control_word(st);
+ ret = ad2s1210_set_excitation_frequency(st, fexcit);
if (ret < 0)
goto error_ret;
- ret = ad2s1210_soft_reset(st);
+
+ ret = len;
+
error_ret:
mutex_unlock(&st->lock);

- return ret < 0 ? ret : len;
+ return ret;
}

static ssize_t ad2s1210_show_resolution(struct device *dev,
@@ -624,10 +626,8 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
if (ret < 0)
goto error_ret;

- ret = ad2s1210_update_frequency_control_word(st);
- if (ret < 0)
- goto error_ret;
- ret = ad2s1210_soft_reset(st);
+ ret = ad2s1210_set_excitation_frequency(st, AD2S1210_DEF_EXCIT);
+
error_ret:
mutex_unlock(&st->lock);
return ret;
@@ -773,7 +773,6 @@ static int ad2s1210_probe(struct spi_device *spi)
mutex_init(&st->lock);
st->sdev = spi;
st->resolution = 12;
- st->fexcit = AD2S1210_DEF_EXCIT;

ret = ad2s1210_setup_clocks(st);
if (ret < 0)
--
2.34.1

2023-09-22 01:50:03

by David Lechner

[permalink] [raw]
Subject: [v2 18/19] staging: iio: resolver: ad2s1210: add phase_lock_range attributes

This adds new phase_lock_range and phase_lock_range_available attributes
to the ad2s1210 resolver driver. These attributes allow the user to set
the phase lock range bit in the control register to modify the behavior
of the resolver to digital converter.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 58 +++++++++++++++++++++++++
1 file changed, 58 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 71f0913b7e2e..f5b8b290e860 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -259,6 +259,60 @@ static ssize_t excitation_frequency_store(struct device *dev,
return ret;
}

+static ssize_t phase_lock_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
+ int ret;
+
+ mutex_lock(&st->lock);
+ ret = regmap_test_bits(st->regmap, AD2S1210_REG_CONTROL,
+ AD2S1210_PHASE_LOCK_RANGE_44);
+ if (ret < 0)
+ goto error_ret;
+
+ ret = sprintf(buf, "%d\n", ret ? 44 : 360);
+
+error_ret:
+ mutex_unlock(&st->lock);
+ return ret;
+}
+
+static ssize_t phase_lock_range_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
+ u16 udata;
+ int ret;
+
+ ret = kstrtou16(buf, 10, &udata);
+ if (ret < 0 || (udata != 44 && udata != 360))
+ return -EINVAL;
+
+ mutex_lock(&st->lock);
+
+ ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
+ AD2S1210_PHASE_LOCK_RANGE_44,
+ udata == 44 ? AD2S1210_PHASE_LOCK_RANGE_44 : 0);
+ if (ret < 0)
+ goto error_ret;
+
+ ret = len;
+
+error_ret:
+ mutex_unlock(&st->lock);
+ return ret;
+}
+
+static ssize_t phase_lock_range_available_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "44 360\n");
+}
+
/* read the fault register since last sample */
static ssize_t ad2s1210_show_fault(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -506,6 +560,8 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
}

static IIO_DEVICE_ATTR_RW(excitation_frequency, 0);
+static IIO_DEVICE_ATTR_RW(phase_lock_range, 0);
+static IIO_DEVICE_ATTR_RO(phase_lock_range_available, 0);
static IIO_DEVICE_ATTR(fault, 0644,
ad2s1210_show_fault, ad2s1210_clear_fault, 0);

@@ -552,6 +608,8 @@ static const struct iio_chan_spec ad2s1210_channels[] = {

static struct attribute *ad2s1210_attributes[] = {
&iio_dev_attr_excitation_frequency.dev_attr.attr,
+ &iio_dev_attr_phase_lock_range.dev_attr.attr,
+ &iio_dev_attr_phase_lock_range_available.dev_attr.attr,
&iio_dev_attr_fault.dev_attr.attr,
&iio_dev_attr_los_thrd.dev_attr.attr,
&iio_dev_attr_dos_ovr_thrd.dev_attr.attr,
--
2.34.1

2023-09-22 03:33:51

by David Lechner

[permalink] [raw]
Subject: [v2 10/19] staging: iio: resolver: ad2s1210: add debugfs reg access

This add an implementation of debugfs_reg_access for the AD2S1210
driver.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 3c81ee61b897..b99928394e3f 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -606,9 +606,29 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
return ret;
}

+static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
+ unsigned int reg, unsigned int writeval,
+ unsigned int *readval)
+{
+ struct ad2s1210_state *st = iio_priv(indio_dev);
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ if (readval)
+ ret = regmap_read(st->regmap, reg, readval);
+ else
+ ret = regmap_write(st->regmap, reg, writeval);
+
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
static const struct iio_info ad2s1210_info = {
.read_raw = ad2s1210_read_raw,
.attrs = &ad2s1210_attribute_group,
+ .debugfs_reg_access = &ad2s1210_debugfs_reg_access,
};

static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
--
2.34.1

2023-09-22 04:05:34

by David Lechner

[permalink] [raw]
Subject: [v2 03/19] staging: iio: resolver: ad2s1210: fix ad2s1210_show_fault

When reading the fault attribute, an empty string was printed if the
fault register value was non-zero.

This is fixed by checking that the return value is less than zero
instead of not zero.

Also always print two hex digits while we are touching this line.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 06de5823eb8e..84743e31261a 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -393,7 +393,7 @@ static ssize_t ad2s1210_show_fault(struct device *dev,
ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
mutex_unlock(&st->lock);

- return ret ? ret : sprintf(buf, "0x%x\n", ret);
+ return (ret < 0) ? ret : sprintf(buf, "0x%02x\n", ret);
}

static ssize_t ad2s1210_clear_fault(struct device *dev,
--
2.34.1

2023-09-22 04:40:45

by David Lechner

[permalink] [raw]
Subject: [v2 07/19] staging: iio: resolver: ad2s1210: implement IIO_CHAN_INFO_SCALE

This adds an implementation of IIO_CHAN_INFO_SCALE to the ad2s1210
resolver driver. This allows userspace to get the scale factor for the
raw values.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 107 ++++++++++++++++--------
1 file changed, 72 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 985b8fecd65a..95d43b241a75 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -458,56 +458,91 @@ static ssize_t ad2s1210_store_reg(struct device *dev,
return ret < 0 ? ret : len;
}

+static const int ad2s1210_velocity_scale[] = {
+ 17089132, /* 8.192MHz / (2*pi * 2500 / 2^15) */
+ 42722830, /* 8.192MHz / (2*pi * 1000 / 2^15) */
+ 85445659, /* 8.192MHz / (2*pi * 500 / 2^15) */
+ 341782638, /* 8.192MHz / (2*pi * 125 / 2^15) */
+};
+
static int ad2s1210_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val,
int *val2,
- long m)
+ long mask)
{
struct ad2s1210_state *st = iio_priv(indio_dev);
int ret = 0;

- mutex_lock(&st->lock);
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
- /* delay (6 * tck + 20) nano seconds */
- udelay(1);
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(&st->lock);
+ gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
+ /* delay (6 * tck + 20) nano seconds */
+ udelay(1);
+
+ switch (chan->type) {
+ case IIO_ANGL:
+ ad2s1210_set_mode(MOD_POS, st);
+ break;
+ case IIO_ANGL_VEL:
+ ad2s1210_set_mode(MOD_VEL, st);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ if (ret < 0)
+ goto error_info_raw;
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0)
+ goto error_info_raw;
+
+ switch (chan->type) {
+ case IIO_ANGL:
+ *val = be16_to_cpup((__be16 *)st->rx);
+ ret = IIO_VAL_INT;
+ break;
+ case IIO_ANGL_VEL:
+ *val = (s16)be16_to_cpup((__be16 *)st->rx);
+ ret = IIO_VAL_INT;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }

- switch (chan->type) {
- case IIO_ANGL:
- ad2s1210_set_mode(MOD_POS, st);
- break;
- case IIO_ANGL_VEL:
- ad2s1210_set_mode(MOD_VEL, st);
- break;
- default:
- ret = -EINVAL;
+error_info_raw:
+ gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
+ /* delay (2 * tck + 20) nano seconds */
+ udelay(1);
+ mutex_unlock(&st->lock);
break;
- }
- if (ret < 0)
- goto error_ret;
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0)
- goto error_ret;

- switch (chan->type) {
- case IIO_ANGL:
- *val = be16_to_cpup((__be16 *)st->rx);
- ret = IIO_VAL_INT;
- break;
- case IIO_ANGL_VEL:
- *val = (s16)be16_to_cpup((__be16 *)st->rx);
- ret = IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_ANGL:
+ /* approx 0.3 arc min converted to radians */
+ *val = 0;
+ *val2 = 95874;
+ ret = IIO_VAL_INT_PLUS_NANO;
+ break;
+ case IIO_ANGL_VEL:
+ *val = st->fclkin;
+ *val2 = ad2s1210_velocity_scale[st->resolution];
+ ret = IIO_VAL_FRACTIONAL;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
break;
+
default:
ret = -EINVAL;
break;
}

-error_ret:
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
- /* delay (2 * tck + 20) nano seconds */
- udelay(1);
- mutex_unlock(&st->lock);
return ret;
}

@@ -549,12 +584,14 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
}, {
.type = IIO_ANGL_VEL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
}
};

--
2.34.1

2023-09-22 05:33:51

by David Lechner

[permalink] [raw]
Subject: [v2 13/19] staging: iio: resolver: ad2s1210: implement hysteresis as channel attr

The AD2S1210 resolver has a hysteresis feature that can be used to
prevent flicker in the LSB of the position register. This can be either
enabled or disabled. Disabling hysteresis is useful for increasing
precision by oversampling.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 88 ++++++++++++++++++++++++-
1 file changed, 85 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 7a1069d948eb..fe413759deb9 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -80,7 +80,6 @@ struct ad2s1210_state {
/** The external oscillator frequency in Hz. */
unsigned long fclkin;
unsigned int fexcit;
- bool hysteresis;
u8 resolution;
u8 rx[2] __aligned(IIO_DMA_MINALIGN);
u8 tx[2];
@@ -456,6 +455,27 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
}
break;

+ case IIO_CHAN_INFO_HYSTERESIS:
+ switch (chan->type) {
+ case IIO_ANGL:
+ mutex_lock(&st->lock);
+ ret = regmap_test_bits(st->regmap, AD2S1210_REG_CONTROL,
+ AD2S1210_ENABLE_HYSTERESIS);
+ if (ret < 0)
+ goto error_info_hysteresis;
+
+ *val = !!ret;
+ ret = IIO_VAL_INT;
+
+error_info_hysteresis:
+ mutex_unlock(&st->lock);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ break;
+
default:
ret = -EINVAL;
break;
@@ -464,6 +484,64 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
return ret;
}

+static int ad2s1210_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type,
+ int *length, long mask)
+{
+ static const int available[] = { 0, 1 };
+ int ret = -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_HYSTERESIS:
+ switch (chan->type) {
+ case IIO_ANGL:
+ *vals = available;
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(available);
+ ret = IIO_AVAIL_LIST;
+ break;
+ default:
+ break;
+ }
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static int ad2s1210_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct ad2s1210_state *st = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_HYSTERESIS:
+ switch (chan->type) {
+ case IIO_ANGL:
+ mutex_lock(&st->lock);
+ ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
+ AD2S1210_ENABLE_HYSTERESIS,
+ val ? AD2S1210_ENABLE_HYSTERESIS
+ : 0);
+ mutex_unlock(&st->lock);
+ break;
+
+ default:
+ break;
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ return ret;
+}
+
static IIO_DEVICE_ATTR(fexcit, 0644,
ad2s1210_show_fexcit, ad2s1210_store_fexcit, 0);
static IIO_DEVICE_ATTR(bits, 0644,
@@ -499,7 +577,10 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
.indexed = 1,
.channel = 0,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
- BIT(IIO_CHAN_INFO_SCALE),
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS),
+ .info_mask_separate_available =
+ BIT(IIO_CHAN_INFO_HYSTERESIS),
}, {
.type = IIO_ANGL_VEL,
.indexed = 1,
@@ -573,6 +654,8 @@ static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,

static const struct iio_info ad2s1210_info = {
.read_raw = ad2s1210_read_raw,
+ .read_avail = ad2s1210_read_avail,
+ .write_raw = ad2s1210_write_raw,
.attrs = &ad2s1210_attribute_group,
.debugfs_reg_access = &ad2s1210_debugfs_reg_access,
};
@@ -689,7 +772,6 @@ static int ad2s1210_probe(struct spi_device *spi)

mutex_init(&st->lock);
st->sdev = spi;
- st->hysteresis = true;
st->resolution = 12;
st->fexcit = AD2S1210_DEF_EXCIT;

--
2.34.1

2023-09-22 08:27:00

by David Lechner

[permalink] [raw]
Subject: [v2 19/19] staging: iio: resolver: ad2s1210: add triggered buffer support

This adds support for triggered buffers to the AD2S1210 resolver driver.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 84 ++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index f5b8b290e860..44a2ecaeeeff 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -19,8 +19,11 @@
#include <linux/sysfs.h>
#include <linux/types.h>

+#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>

#define DRV_NAME "ad2s1210"

@@ -85,6 +88,12 @@ struct ad2s1210_state {
unsigned long fclkin;
/** The selected resolution */
enum ad2s1210_resolution resolution;
+ /** Scan buffer */
+ struct {
+ __be16 chan[2];
+ /* Ensure timestamp is naturally aligned. */
+ s64 timestamp __aligned(8);
+ } scan;
u8 rx[2] __aligned(IIO_DMA_MINALIGN);
u8 tx[2];
};
@@ -592,18 +601,35 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 16,
+ .storagebits = 16,
+ .endianness = IIO_BE,
+ },
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_HYSTERESIS),
.info_mask_separate_available =
BIT(IIO_CHAN_INFO_HYSTERESIS),
+ .datasheet_name = "position",
}, {
.type = IIO_ANGL_VEL,
.indexed = 1,
.channel = 0,
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 16,
+ .storagebits = 16,
+ .endianness = IIO_BE,
+ },
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE),
- }
+ .datasheet_name = "velocity",
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(2),
};

static struct attribute *ad2s1210_attributes[] = {
@@ -665,6 +691,55 @@ static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
return ret;
}

+static irqreturn_t ad2s1210_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad2s1210_state *st = iio_priv(indio_dev);
+ size_t chan = 0;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ memset(&st->scan, 0, sizeof(st->scan));
+ gpiod_set_value(st->sample_gpio, 1);
+
+ if (test_bit(0, indio_dev->active_scan_mask)) {
+ ret = ad2s1210_set_mode(st, MOD_POS);
+ if (ret < 0)
+ goto error_ret;
+
+ /* REVIST: we can read 3 bytes here and also get fault flags */
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0)
+ goto error_ret;
+
+ memcpy(&st->scan.chan[chan++], st->rx, 2);
+ }
+
+ if (test_bit(1, indio_dev->active_scan_mask)) {
+ ret = ad2s1210_set_mode(st, MOD_VEL);
+ if (ret < 0)
+ goto error_ret;
+
+ /* REVIST: we can read 3 bytes here and also get fault flags */
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0)
+ goto error_ret;
+
+ memcpy(&st->scan.chan[chan++], st->rx, 2);
+ }
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &st->scan, pf->timestamp);
+
+error_ret:
+ gpiod_set_value(st->sample_gpio, 0);
+ mutex_unlock(&st->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
static const struct iio_info ad2s1210_info = {
.read_raw = ad2s1210_read_raw,
.read_avail = ad2s1210_read_avail,
@@ -850,6 +925,13 @@ static int ad2s1210_probe(struct spi_device *spi)
indio_dev->num_channels = ARRAY_SIZE(ad2s1210_channels);
indio_dev->name = spi_get_device_id(spi)->name;

+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+ &iio_pollfunc_store_time,
+ &ad2s1210_trigger_handler, NULL);
+ if (ret < 0)
+ return dev_err_probe(&spi->dev, ret,
+ "iio triggered buffer setup failed\n");
+
return devm_iio_device_register(&spi->dev, indio_dev);
}

--
2.34.1

2023-09-22 13:06:11

by David Lechner

[permalink] [raw]
Subject: [v2 17/19] staging: iio: resolver: ad2s1210: convert resolution to devicetree property

Selecting the resolution was implemented as the `bits` sysfs attribute.
However, the selection of the resolution depends on how the hardware
is wired and the specific application, so this is rather a job for
devicetree to describe.

A new devicetree property `adi,resolution` to specify the resolution
required for each chip is added and the `bits` sysfs attribute is
removed.

Since the resolution is now supplied by a devicetree property, the
resolution-gpios are now optional and we can allow for the case where
the resolution pins on the AD2S1210 are hard-wired instead of requiring
them to be connected to gpios.

Signed-off-by: David Lechner <[email protected]>
---
drivers/staging/iio/resolver/ad2s1210.c | 136 +++++++++++-------------
1 file changed, 61 insertions(+), 75 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 14bec2b20939..71f0913b7e2e 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -65,6 +65,13 @@ enum ad2s1210_mode {
MOD_CONFIG = 0b11,
};

+enum ad2s1210_resolution {
+ AD2S1210_RES_10 = 0b00,
+ AD2S1210_RES_12 = 0b01,
+ AD2S1210_RES_14 = 0b10,
+ AD2S1210_RES_16 = 0b11,
+};
+
struct ad2s1210_state {
struct mutex lock;
struct spi_device *sdev;
@@ -72,13 +79,12 @@ struct ad2s1210_state {
struct gpio_desc *sample_gpio;
/** GPIO pins connected to A0 and A1 lines. */
struct gpio_descs *mode_gpios;
- /** GPIO pins connected to RES0 and RES1 lines. */
- struct gpio_descs *resolution_gpios;
/** Used to access config registers. */
struct regmap *regmap;
/** The external oscillator frequency in Hz. */
unsigned long fclkin;
- u8 resolution;
+ /** The selected resolution */
+ enum ad2s1210_resolution resolution;
u8 rx[2] __aligned(IIO_DMA_MINALIGN);
u8 tx[2];
};
@@ -205,18 +211,6 @@ static int ad2s1210_set_excitation_frequency(struct ad2s1210_state *st,
return regmap_write(st->regmap, AD2S1210_REG_SOFT_RESET, 0);
}

-static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st,
- u8 resolution)
-{
- struct gpio_descs *gpios = st->resolution_gpios;
- DECLARE_BITMAP(bitmap, 2);
-
- bitmap[0] = (resolution - 10) >> 1;
-
- return gpiod_set_array_value(gpios->ndescs, gpios->desc, gpios->info,
- bitmap);
-}
-
static ssize_t excitation_frequency_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -265,50 +259,6 @@ static ssize_t excitation_frequency_store(struct device *dev,
return ret;
}

-static ssize_t ad2s1210_show_resolution(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
-
- return sprintf(buf, "%d\n", st->resolution);
-}
-
-static ssize_t ad2s1210_store_resolution(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t len)
-{
- struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
- unsigned char data;
- unsigned char udata;
- int ret;
-
- ret = kstrtou8(buf, 10, &udata);
- if (ret || udata < 10 || udata > 16) {
- dev_err(dev, "ad2s1210: resolution out of range\n");
- return -EINVAL;
- }
-
- data = (udata - 10) >> 1;
-
- mutex_lock(&st->lock);
- ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
- AD2S1210_SET_RES, data);
- if (ret < 0)
- goto error_ret;
-
- ret = ad2s1210_set_resolution_gpios(st, udata);
- if (ret < 0)
- goto error_ret;
-
- st->resolution = udata;
- ret = len;
-
-error_ret:
- mutex_unlock(&st->lock);
- return ret;
-}
-
/* read the fault register since last sample */
static ssize_t ad2s1210_show_fault(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -556,8 +506,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
}

static IIO_DEVICE_ATTR_RW(excitation_frequency, 0);
-static IIO_DEVICE_ATTR(bits, 0644,
- ad2s1210_show_resolution, ad2s1210_store_resolution, 0);
static IIO_DEVICE_ATTR(fault, 0644,
ad2s1210_show_fault, ad2s1210_clear_fault, 0);

@@ -604,7 +552,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = {

static struct attribute *ad2s1210_attributes[] = {
&iio_dev_attr_excitation_frequency.dev_attr.attr,
- &iio_dev_attr_bits.dev_attr.attr,
&iio_dev_attr_fault.dev_attr.attr,
&iio_dev_attr_los_thrd.dev_attr.attr,
&iio_dev_attr_dos_ovr_thrd.dev_attr.attr,
@@ -626,12 +573,10 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
int ret;

mutex_lock(&st->lock);
- ret = ad2s1210_set_resolution_gpios(st, st->resolution);
- if (ret < 0)
- return ret;

data = AD2S1210_DEF_CONTROL & ~AD2S1210_SET_RES;
- data |= (st->resolution - 10) >> 1;
+ data |= st->resolution;
+
ret = regmap_write(st->regmap, AD2S1210_REG_CONTROL, data);
if (ret < 0)
goto error_ret;
@@ -670,6 +615,26 @@ static const struct iio_info ad2s1210_info = {
.debugfs_reg_access = &ad2s1210_debugfs_reg_access,
};

+static int ad2s1210_setup_properties(struct ad2s1210_state *st)
+{
+ struct device *dev = &st->sdev->dev;
+ u32 val;
+ int ret;
+
+ ret = device_property_read_u32(dev, "assigned-resolution-bits", &val);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "failed to read assigned-resolution-bits property\n");
+
+ if (val < 10 || val > 16)
+ return dev_err_probe(dev, -EINVAL,
+ "resolution out of range: %u\n", val);
+
+ st->resolution = (val - 10) >> 1;
+
+ return 0;
+}
+
static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
{
struct device *dev = &st->sdev->dev;
@@ -691,6 +656,9 @@ static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
{
struct device *dev = &st->sdev->dev;
+ struct gpio_descs *resolution_gpios;
+ DECLARE_BITMAP(bitmap, 2);
+ int ret;

/* should not be sampling on startup */
st->sample_gpio = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
@@ -708,16 +676,31 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
return dev_err_probe(dev, -EINVAL,
"requires exactly 2 mode-gpios\n");

- /* both pins high means that we start with 16-bit resolution */
- st->resolution_gpios = devm_gpiod_get_array(dev, "resolution",
- GPIOD_OUT_HIGH);
- if (IS_ERR(st->resolution_gpios))
- return dev_err_probe(dev, PTR_ERR(st->resolution_gpios),
+ /* If resolution gpios are provided, they get set to the required
+ * resolution, otherwise it is assumed the RES0 and RES1 pins are
+ * hard-wired to match the resolution indicated in the devicetree.
+ */
+ resolution_gpios = devm_gpiod_get_array_optional(dev, "resolution",
+ GPIOD_ASIS);
+ if (IS_ERR(resolution_gpios))
+ return dev_err_probe(dev, PTR_ERR(resolution_gpios),
"failed to request resolution GPIOs\n");

- if (st->resolution_gpios->ndescs != 2)
- return dev_err_probe(dev, -EINVAL,
- "requires exactly 2 resolution-gpios\n");
+ if (resolution_gpios) {
+ if (resolution_gpios->ndescs != 2)
+ return dev_err_probe(dev, -EINVAL,
+ "requires exactly 2 resolution-gpios\n");
+
+ bitmap[0] = st->resolution;
+
+ ret = gpiod_set_array_value(resolution_gpios->ndescs,
+ resolution_gpios->desc,
+ resolution_gpios->info,
+ bitmap);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "failed to set resolution gpios\n");
+ }

return 0;
}
@@ -782,7 +765,10 @@ static int ad2s1210_probe(struct spi_device *spi)

mutex_init(&st->lock);
st->sdev = spi;
- st->resolution = 12;
+
+ ret = ad2s1210_setup_properties(st);
+ if (ret < 0)
+ return ret;

ret = ad2s1210_setup_clocks(st);
if (ret < 0)
--
2.34.1