This series is working towards moving the ad2s1210 resolver driver out of
staging (after 13 years!). It involves a bunch of fixes and improvements
to make proper device tree bindings and use standard IIO sysfs attributes.
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.)
---
v4 changes:
* Dropped applied patches:
* "dt-bindings: iio: resolver: add devicetree bindings for ad2s1210"
* "staging: iio: resolver: ad2s1210: read excitation frequency from
control register"
* "staging: iio: resolver: ad2s1210: refactor setting excitation
frequency"
* "staging: iio: resolver: ad2s1210: rework gpios"
* "staging: iio: resolver: ad2s1210: remove config attribute"
* "staging: iio: resolver: ad2s1210: add debugfs reg access"
* "staging: iio: resolver: ad2s1210: use regmap for config registers"
* "staging: iio: resolver: ad2s1210: use devicetree to get CLKIN rate"
* "staging: iio: resolver: ad2s1210: implement IIO_CHAN_INFO_SCALE"
* "staging: iio: resolver: ad2s1210: always use 16-bit value for raw
read"
* "staging: iio: resolver: ad2s1210: sort imports"
* "staging: iio: resolver: ad2s1210: remove spi_set_drvdata()"
* "staging: iio: resolver: ad2s1210: check return of ad2s1210_initial()"
* "staging: iio: resolver: ad2s1210: remove call to spi_setup()"
* "staging: iio: resolver: ad2s1210: fix use before initialization"
* Added new patches:
* "staging: iio: resolver: ad2s1210: do not use fault register for
dummy read"
* "iio: event: add optional event label support"
* "staging: iio: resolver: ad2s1210: add register/fault support summary"
* "staging: iio: resolver: ad2s1210: remove fault attribute"
* "staging: iio: resolver: ad2s1210: simplify code with guard(mutex)"
* Fixed DT property name in commit description of "staging: iio:
resolver: ad2s1210: convert resolution to devicetree property"
* Fixed compile error in "staging: iio: resolver: ad2s1210: implement
fault events".
* Fixed angl0 hysteresis raw values when assigned-resolution-bits != 16.
* Fixed missing word in "staging: iio: resolver: ad2s1210: convert DOS
overrange threshold to event attr" commit description.
* Fixed missing static qualifier on event attribute definitions.
* Dropped used of X/Y modifiers on sine/cosine channels.
* Changed type/direction on some events.
* Added event *_label attributes.
Link to v3: https://lore.kernel.org/r/[email protected]
v3 changes:
* Dropped applied patches:
* "staging: iio: resolver: ad2s1210: fix ad2s1210_show_fault"
* "iio: adc: MCP3564: fix the static checker warning"
* Dropped "staging: iio: Documentation: document IIO resolver AD2S1210
sysfs attributes". We will attempt to use existing ABI for faults/
thresholds in a future series.
* Added description of A0/A1 lines in DT bindings.
* Added power supply regulators to DT bindings.
* Moved sorting imports to separate patch.
* Renamed fclkin to clkin_hz.
* Added __be16 sample field to state struct for reading raw samples.
* Split out new function ad2s1210_single_conversion() from
ad2s1210_read_raw().
* Split out new ad2s1210_get_hysteresis() and ad2s1210_set_hysteresis()
functions.
* Fixed multi-line comment style.
* Added notes about soft reset not resetting config registers.
* Made use of FIELD_PREP() macro.
* Added more explanation to regmap commit message.
* Removed datasheet names from channel specs.
* Replaced "staging: iio: resolver: ad2s1210: rename fexcit attribute"
with "staging: iio: resolver: ad2s1210: convert fexcit to channel
attribute".
* Replaced "staging: iio: resolver: ad2s1210: add phase_lock_range
attributes" with "staging: iio: resolver: ad2s1210: add phase lock
range support"
* Added additional patches to convert custom device attributes to event
attributes.
* Added patch for to add label attributes.
Link to v2: https://lore.kernel.org/r/[email protected]
v2 changes:
* 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 of patches to the staging driver. It is
not quite ready for the move out of staging patch yet.
---
David Lechner (17):
staging: iio: resolver: ad2s1210: do not use fault register for dummy read
staging: iio: resolver: ad2s1210: implement hysteresis as channel attr
staging: iio: resolver: ad2s1210: convert fexcit to channel attribute
staging: iio: resolver: ad2s1210: convert resolution to devicetree property
staging: iio: resolver: ad2s1210: add phase lock range support
staging: iio: resolver: ad2s1210: add triggered buffer support
staging: iio: resolver: ad2s1210: convert LOT threshold attrs to event attrs
staging: iio: resolver: ad2s1210: convert LOS threshold to event attr
staging: iio: resolver: ad2s1210: convert DOS overrange threshold to event attr
staging: iio: resolver: ad2s1210: convert DOS mismatch threshold to event attr
staging: iio: resolver: ad2s1210: rename DOS reset min/max attrs
iio: event: add optional event label support
staging: iio: resolver: ad2s1210: implement fault events
staging: iio: resolver: ad2s1210: add register/fault support summary
staging: iio: resolver: ad2s1210: add label attribute support
staging: iio: resolver: ad2s1210: remove fault attribute
staging: iio: resolver: ad2s1210: simplify code with guard(mutex)
Documentation/ABI/testing/sysfs-bus-iio | 15 +
drivers/iio/industrialio-event.c | 55 +
.../Documentation/sysfs-bus-iio-resolver-ad2s1210 | 27 +
drivers/staging/iio/resolver/ad2s1210.c | 1194 ++++++++++++++++----
include/linux/iio/iio.h | 8 +
5 files changed, 1061 insertions(+), 238 deletions(-)
---
base-commit: 2d3dff577dd0ea8fe9637a13822f7603c4a881c8
change-id: 20230925-ad2s1210-mainline-2791ef75e386
When reading registers on the AD2S1210 chip, we have to supply a "dummy"
address for the second SPI tx byte so that we don't accidentally write
to a register. This register will be read and the value discarded on the
next regmap read or write call.
Reading the fault register has a side-effect of clearing the faults
so we should not use this register for the dummy read.
Signed-off-by: David Lechner <[email protected]>
---
v4 changes: New patch
(this probably should have been done before "staging: iio: resolver:
ad2s1210: use regmap for config registers" but was overlooked until now)
drivers/staging/iio/resolver/ad2s1210.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 67d8af0dd7ae..8fbde9517fe9 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -166,9 +166,10 @@ static int ad2s1210_regmap_reg_read(void *context, unsigned int reg,
st->tx[0] = reg;
/*
* Must be valid register address here otherwise this could write data.
- * It doesn't matter which one.
+ * It doesn't matter which one as long as reading doesn't have side-
+ * effects.
*/
- st->tx[1] = AD2S1210_REG_FAULT;
+ st->tx[1] = AD2S1210_REG_CONTROL;
ret = spi_sync_transfer(st->sdev, xfers, ARRAY_SIZE(xfers));
if (ret < 0)
--
2.42.0
The AD2S1210 chip has a phase lock range feature that allows selecting
the allowable phase difference between the excitation output and the
sine and cosine inputs. This can be set to either 44 degrees (default)
or 360 degrees.
This patch adds a new phase channel with a phase0_mag_rising event that
can be used to configure the phase lock range. Actually emitting the
event will be added in a subsequent patch.
Signed-off-by: David Lechner <[email protected]>
---
v4 changes:
* Changed event direction from none to rising.
* Fixed missing static qualifier on attribute definition.
v3 changes:
* This is a new patch to replace "staging: iio: resolver: ad2s1210: add
phase_lock_range attributes"
drivers/staging/iio/resolver/ad2s1210.c | 125 ++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 66ef35fbb6fe..83f6ac890dbc 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -56,6 +56,13 @@
#define AD2S1210_MIN_FCW 0x4
#define AD2S1210_MAX_FCW 0x50
+/* 44 degrees ~= 0.767945 radians */
+#define PHASE_44_DEG_TO_RAD_INT 0
+#define PHASE_44_DEG_TO_RAD_MICRO 767945
+/* 360 degrees ~= 6.283185 radians */
+#define PHASE_360_DEG_TO_RAD_INT 6
+#define PHASE_360_DEG_TO_RAD_MICRO 283185
+
enum ad2s1210_mode {
MOD_POS = 0b00,
MOD_VEL = 0b01,
@@ -379,6 +386,54 @@ static int ad2s1210_set_hysteresis(struct ad2s1210_state *st, int val)
return ret;
}
+static int ad2s1210_get_phase_lock_range(struct ad2s1210_state *st,
+ int *val, int *val2)
+{
+ int ret;
+
+ mutex_lock(&st->lock);
+ ret = regmap_test_bits(st->regmap, AD2S1210_REG_CONTROL,
+ AD2S1210_PHASE_LOCK_RANGE_44);
+ mutex_unlock(&st->lock);
+
+ if (ret < 0)
+ return ret;
+
+ if (ret) {
+ /* 44 degrees as radians */
+ *val = PHASE_44_DEG_TO_RAD_INT;
+ *val2 = PHASE_44_DEG_TO_RAD_MICRO;
+ } else {
+ /* 360 degrees as radians */
+ *val = PHASE_360_DEG_TO_RAD_INT;
+ *val2 = PHASE_360_DEG_TO_RAD_MICRO;
+ }
+
+ return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int ad2s1210_set_phase_lock_range(struct ad2s1210_state *st,
+ int val, int val2)
+{
+ int deg, ret;
+
+ /* convert radians to degrees - only two allowable values */
+ if (val == PHASE_44_DEG_TO_RAD_INT && val2 == PHASE_44_DEG_TO_RAD_MICRO)
+ deg = 44;
+ else if (val == PHASE_360_DEG_TO_RAD_INT &&
+ val2 == PHASE_360_DEG_TO_RAD_MICRO)
+ deg = 360;
+ else
+ return -EINVAL;
+
+ mutex_lock(&st->lock);
+ ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
+ AD2S1210_PHASE_LOCK_RANGE_44,
+ deg == 44 ? AD2S1210_PHASE_LOCK_RANGE_44 : 0);
+ mutex_unlock(&st->lock);
+ return ret;
+}
+
static int ad2s1210_get_excitation_frequency(struct ad2s1210_state *st, int *val)
{
unsigned int reg_val;
@@ -551,6 +606,16 @@ static IIO_DEVICE_ATTR(lot_low_thrd, 0644,
ad2s1210_show_reg, ad2s1210_store_reg,
AD2S1210_REG_LOT_LOW_THRD);
+static const struct iio_event_spec ad2s1210_phase_event_spec[] = {
+ {
+ /* Phase error fault. */
+ .type = IIO_EV_TYPE_MAG,
+ .dir = IIO_EV_DIR_RISING,
+ /* Phase lock range. */
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
+};
+
static const struct iio_chan_spec ad2s1210_channels[] = {
{
.type = IIO_ANGL,
@@ -567,6 +632,14 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
.channel = 0,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE),
+ }, {
+ /* used to configure phase lock range and get phase lock error */
+ .type = IIO_PHASE,
+ .indexed = 1,
+ .channel = 0,
+ .scan_index = -1,
+ .event_spec = ad2s1210_phase_event_spec,
+ .num_event_specs = ARRAY_SIZE(ad2s1210_phase_event_spec),
}, {
/* excitation frequency output */
.type = IIO_ALTVOLTAGE,
@@ -595,6 +668,21 @@ static const struct attribute_group ad2s1210_attribute_group = {
.attrs = ad2s1210_attributes,
};
+static IIO_CONST_ATTR(in_phase0_mag_rising_value_available,
+ __stringify(PHASE_44_DEG_TO_RAD_INT) "."
+ __stringify(PHASE_44_DEG_TO_RAD_MICRO) " "
+ __stringify(PHASE_360_DEG_TO_RAD_INT) "."
+ __stringify(PHASE_360_DEG_TO_RAD_MICRO));
+
+static struct attribute *ad2s1210_event_attributes[] = {
+ &iio_const_attr_in_phase0_mag_rising_value_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group ad2s1210_event_attribute_group = {
+ .attrs = ad2s1210_event_attributes,
+};
+
static int ad2s1210_initial(struct ad2s1210_state *st)
{
unsigned char data;
@@ -619,6 +707,40 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
return ret;
}
+static int ad2s1210_read_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int *val, int *val2)
+{
+ struct ad2s1210_state *st = iio_priv(indio_dev);
+
+ switch (chan->type) {
+ case IIO_PHASE:
+ return ad2s1210_get_phase_lock_range(st, val, val2);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad2s1210_write_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int val, int val2)
+{
+ struct ad2s1210_state *st = iio_priv(indio_dev);
+
+ switch (chan->type) {
+ case IIO_PHASE:
+ return ad2s1210_set_phase_lock_range(st, val, val2);
+ default:
+ return -EINVAL;
+ }
+}
+
static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
unsigned int reg, unsigned int writeval,
unsigned int *readval)
@@ -639,10 +761,13 @@ static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
}
static const struct iio_info ad2s1210_info = {
+ .event_attrs = &ad2s1210_event_attribute_group,
.read_raw = ad2s1210_read_raw,
.read_avail = ad2s1210_read_avail,
.write_raw = ad2s1210_write_raw,
.attrs = &ad2s1210_attribute_group,
+ .read_event_value = ad2s1210_read_event_value,
+ .write_event_value = ad2s1210_write_event_value,
.debugfs_reg_access = &ad2s1210_debugfs_reg_access,
};
--
2.42.0
The AD2S1210 has a programmable threshold for the loss of signal (LOS)
fault. This fault is triggered when either the sine or cosine input
falls below the threshold voltage.
This patch converts the custom device LOS threshold attribute to an
event falling edge threshold attribute on a new monitor signal channel.
The monitor signal is an internal signal that combines the amplitudes
of the sine and cosine inputs as well as the current angle and position
output. This signal is used to detect faults in the input signals.
The attribute now uses millivolts instead of the raw register value in
accordance with the IIO ABI.
Emitting the event will be implemented in a later patch.
Signed-off-by: David Lechner <[email protected]>
---
v4 changes:
* Fixed missing static qualifier on attribute definition.
v3 changes: This is a new patch in v3
drivers/staging/iio/resolver/ad2s1210.c | 76 +++++++++++++++++++++++++++++++--
1 file changed, 72 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 12437f697f79..d52aed30ca66 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -66,6 +66,11 @@
#define PHASE_360_DEG_TO_RAD_INT 6
#define PHASE_360_DEG_TO_RAD_MICRO 283185
+/* Threshold voltage registers have 1 LSB == 38 mV */
+#define THRESHOLD_MILLIVOLT_PER_LSB 38
+/* max voltage for threshold registers is 0x7F * 38 mV */
+#define THRESHOLD_RANGE_STR "[0 38 4826]"
+
enum ad2s1210_mode {
MOD_POS = 0b00,
MOD_VEL = 0b01,
@@ -451,6 +456,38 @@ static const int ad2s1210_lot_threshold_urad_per_lsb[] = {
1237, /* 16-bit: same as 14-bit */
};
+static int ad2s1210_get_voltage_threshold(struct ad2s1210_state *st,
+ unsigned int reg, int *val)
+{
+ unsigned int reg_val;
+ int ret;
+
+ mutex_lock(&st->lock);
+ ret = regmap_read(st->regmap, reg, ®_val);
+ mutex_unlock(&st->lock);
+
+ if (ret < 0)
+ return ret;
+
+ *val = reg_val * THRESHOLD_MILLIVOLT_PER_LSB;
+ return IIO_VAL_INT;
+}
+
+static int ad2s1210_set_voltage_threshold(struct ad2s1210_state *st,
+ unsigned int reg, int val)
+{
+ unsigned int reg_val;
+ int ret;
+
+ reg_val = val / THRESHOLD_MILLIVOLT_PER_LSB;
+
+ mutex_lock(&st->lock);
+ ret = regmap_write(st->regmap, reg, reg_val);
+ mutex_unlock(&st->lock);
+
+ return ret;
+}
+
static int ad2s1210_get_lot_high_threshold(struct ad2s1210_state *st,
int *val, int *val2)
{
@@ -710,9 +747,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
static IIO_DEVICE_ATTR(fault, 0644,
ad2s1210_show_fault, ad2s1210_clear_fault, 0);
-static IIO_DEVICE_ATTR(los_thrd, 0644,
- ad2s1210_show_reg, ad2s1210_store_reg,
- AD2S1210_REG_LOS_THRD);
static IIO_DEVICE_ATTR(dos_ovr_thrd, 0644,
ad2s1210_show_reg, ad2s1210_store_reg,
AD2S1210_REG_DOS_OVR_THRD);
@@ -749,6 +783,16 @@ static const struct iio_event_spec ad2s1210_phase_event_spec[] = {
},
};
+static const struct iio_event_spec ad2s1210_monitor_signal_event_spec[] = {
+ {
+ /* Sine/cosine below LOS threshold fault. */
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_FALLING,
+ /* Loss of signal threshold. */
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
+};
+
static const struct iio_chan_spec ad2s1210_channels[] = {
{
.type = IIO_ANGL,
@@ -807,12 +851,19 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
.scan_index = -1,
.info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY),
.info_mask_separate_available = BIT(IIO_CHAN_INFO_FREQUENCY),
+ }, {
+ /* monitor signal */
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .channel = 0,
+ .scan_index = -1,
+ .event_spec = ad2s1210_monitor_signal_event_spec,
+ .num_event_specs = ARRAY_SIZE(ad2s1210_monitor_signal_event_spec),
},
};
static struct attribute *ad2s1210_attributes[] = {
&iio_dev_attr_fault.dev_attr.attr,
- &iio_dev_attr_los_thrd.dev_attr.attr,
&iio_dev_attr_dos_ovr_thrd.dev_attr.attr,
&iio_dev_attr_dos_mis_thrd.dev_attr.attr,
&iio_dev_attr_dos_rst_max_thrd.dev_attr.attr,
@@ -851,11 +902,14 @@ static IIO_CONST_ATTR(in_phase0_mag_rising_value_available,
__stringify(PHASE_44_DEG_TO_RAD_MICRO) " "
__stringify(PHASE_360_DEG_TO_RAD_INT) "."
__stringify(PHASE_360_DEG_TO_RAD_MICRO));
+static IIO_CONST_ATTR(in_altvoltage0_thresh_falling_value_available,
+ THRESHOLD_RANGE_STR);
static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_value_available, 0);
static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_hysteresis_available, 0);
static struct attribute *ad2s1210_event_attributes[] = {
&iio_const_attr_in_phase0_mag_rising_value_available.dev_attr.attr,
+ &iio_const_attr_in_altvoltage0_thresh_falling_value_available.dev_attr.attr,
&iio_dev_attr_in_angl1_thresh_rising_value_available.dev_attr.attr,
&iio_dev_attr_in_angl1_thresh_rising_hysteresis_available.dev_attr.attr,
NULL,
@@ -908,6 +962,13 @@ static int ad2s1210_read_event_value(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
+ case IIO_ALTVOLTAGE:
+ if (chan->output)
+ return -EINVAL;
+ if (type == IIO_EV_TYPE_THRESH && dir == IIO_EV_DIR_FALLING)
+ return ad2s1210_get_voltage_threshold(st,
+ AD2S1210_REG_LOS_THRD, val);
+ return -EINVAL;
case IIO_PHASE:
return ad2s1210_get_phase_lock_range(st, val, val2);
default:
@@ -934,6 +995,13 @@ static int ad2s1210_write_event_value(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
+ case IIO_ALTVOLTAGE:
+ if (chan->output)
+ return -EINVAL;
+ if (type == IIO_EV_TYPE_THRESH && dir == IIO_EV_DIR_FALLING)
+ return ad2s1210_set_voltage_threshold(st,
+ AD2S1210_REG_LOS_THRD, val);
+ return -EINVAL;
case IIO_PHASE:
return ad2s1210_set_phase_lock_range(st, val, val2);
default:
--
2.42.0
The AD2S1210 has a programmable threshold for the degradation of signal
(DOS) mismatch fault. This fault is triggered when the difference in
amplitude between the sine and cosine inputs exceeds the threshold.
The DOS reset min/max registers on the chip provide initial values
for internal tracking of the min/max of the monitor signal after the
fault register is cleared.
This patch converts the custom device DOS reset min/max threshold
attributes custom event attributes on the monitor signal channel.
The attributes now use millivolts instead of the raw register value in
accordance with the IIO ABI.
Signed-off-by: David Lechner <[email protected]>
---
v4 changes:
* Fixed name of attributes in sysfs docs.
* Changed event direction from none to rising.
* Fixed missing static qualifier on attribute definition.
v3 changes: This is a new patch in v3
.../Documentation/sysfs-bus-iio-resolver-ad2s1210 | 27 ++++++
drivers/staging/iio/resolver/ad2s1210.c | 99 ++++++++++++----------
2 files changed, 82 insertions(+), 44 deletions(-)
diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-resolver-ad2s1210 b/drivers/staging/iio/Documentation/sysfs-bus-iio-resolver-ad2s1210
new file mode 100644
index 000000000000..f92c79342b93
--- /dev/null
+++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-resolver-ad2s1210
@@ -0,0 +1,27 @@
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0_mag_rising_reset_max
+KernelVersion: 6.7
+Contact: [email protected]
+Description:
+ Reading returns the current Degradation of Signal Reset Maximum
+ Threshold value in millivolts. Writing sets the value.
+
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0_mag_rising_reset_max_available
+KernelVersion: 6.7
+Contact: [email protected]
+Description:
+ Reading returns the allowable voltage range for
+ in_altvoltage0_mag_rising_reset_max.
+
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0_mag_rising_reset_min
+KernelVersion: 6.7
+Contact: [email protected]
+Description:
+ Reading returns the current Degradation of Signal Reset Minimum
+ Threshold value in millivolts. Writing sets the value.
+
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0_mag_rising_reset_min_available
+KernelVersion: 6.7
+Contact: [email protected]
+Description:
+ Reading returns the allowable voltage range for
+ in_altvoltage0_mag_rising_reset_min.
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 870c4a9a6214..9fac806c2a5f 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -286,41 +286,6 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
return ret < 0 ? ret : len;
}
-static ssize_t ad2s1210_show_reg(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
- struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
- unsigned int value;
- int ret;
-
- mutex_lock(&st->lock);
- ret = regmap_read(st->regmap, iattr->address, &value);
- mutex_unlock(&st->lock);
-
- return ret < 0 ? ret : sprintf(buf, "%d\n", value);
-}
-
-static ssize_t ad2s1210_store_reg(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;
- int ret;
- struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
-
- ret = kstrtou8(buf, 10, &data);
- if (ret)
- return -EINVAL;
-
- mutex_lock(&st->lock);
- ret = regmap_write(st->regmap, iattr->address, data);
- mutex_unlock(&st->lock);
- return ret < 0 ? ret : len;
-}
-
static int ad2s1210_single_conversion(struct ad2s1210_state *st,
struct iio_chan_spec const *chan,
int *val)
@@ -747,13 +712,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
static IIO_DEVICE_ATTR(fault, 0644,
ad2s1210_show_fault, ad2s1210_clear_fault, 0);
-static IIO_DEVICE_ATTR(dos_rst_max_thrd, 0644,
- ad2s1210_show_reg, ad2s1210_store_reg,
- AD2S1210_REG_DOS_RST_MAX_THRD);
-static IIO_DEVICE_ATTR(dos_rst_min_thrd, 0644,
- ad2s1210_show_reg, ad2s1210_store_reg,
- AD2S1210_REG_DOS_RST_MIN_THRD);
-
static const struct iio_event_spec ad2s1210_position_event_spec[] = {
{
/* Tracking error exceeds LOT threshold fault. */
@@ -871,8 +829,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
static struct attribute *ad2s1210_attributes[] = {
&iio_dev_attr_fault.dev_attr.attr,
- &iio_dev_attr_dos_rst_max_thrd.dev_attr.attr,
- &iio_dev_attr_dos_rst_min_thrd.dev_attr.attr,
NULL,
};
@@ -880,6 +836,49 @@ static const struct attribute_group ad2s1210_attribute_group = {
.attrs = ad2s1210_attributes,
};
+static ssize_t event_attr_voltage_reg_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
+ struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
+ unsigned int value;
+ int ret;
+
+ mutex_lock(&st->lock);
+ ret = regmap_read(st->regmap, iattr->address, &value);
+ mutex_unlock(&st->lock);
+
+ if (ret < 0)
+ return ret;
+
+ return sprintf(buf, "%d\n", value * THRESHOLD_MILLIVOLT_PER_LSB);
+}
+
+static ssize_t event_attr_voltage_reg_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));
+ struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
+ u16 data;
+ int ret;
+
+ ret = kstrtou16(buf, 10, &data);
+ if (ret)
+ return -EINVAL;
+
+ mutex_lock(&st->lock);
+ ret = regmap_write(st->regmap, iattr->address,
+ data / THRESHOLD_MILLIVOLT_PER_LSB);
+ mutex_unlock(&st->lock);
+
+ if (ret < 0)
+ return ret;
+
+ return len;
+}
+
static ssize_t
in_angl1_thresh_rising_value_available_show(struct device *dev,
struct device_attribute *attr,
@@ -913,6 +912,14 @@ static IIO_CONST_ATTR(in_altvoltage0_thresh_rising_value_available,
THRESHOLD_RANGE_STR);
static IIO_CONST_ATTR(in_altvoltage0_mag_rising_value_available,
THRESHOLD_RANGE_STR);
+static IIO_DEVICE_ATTR(in_altvoltage0_mag_rising_reset_max, 0644,
+ event_attr_voltage_reg_show, event_attr_voltage_reg_store,
+ AD2S1210_REG_DOS_RST_MAX_THRD);
+static IIO_CONST_ATTR(in_altvoltage0_mag_rising_reset_max_available, THRESHOLD_RANGE_STR);
+static IIO_DEVICE_ATTR(in_altvoltage0_mag_rising_reset_min, 0644,
+ event_attr_voltage_reg_show, event_attr_voltage_reg_store,
+ AD2S1210_REG_DOS_RST_MIN_THRD);
+static IIO_CONST_ATTR(in_altvoltage0_mag_rising_reset_min_available, THRESHOLD_RANGE_STR);
static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_value_available, 0);
static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_hysteresis_available, 0);
@@ -921,6 +928,10 @@ static struct attribute *ad2s1210_event_attributes[] = {
&iio_const_attr_in_altvoltage0_thresh_falling_value_available.dev_attr.attr,
&iio_const_attr_in_altvoltage0_thresh_rising_value_available.dev_attr.attr,
&iio_const_attr_in_altvoltage0_mag_rising_value_available.dev_attr.attr,
+ &iio_dev_attr_in_altvoltage0_mag_rising_reset_max.dev_attr.attr,
+ &iio_const_attr_in_altvoltage0_mag_rising_reset_max_available.dev_attr.attr,
+ &iio_dev_attr_in_altvoltage0_mag_rising_reset_min.dev_attr.attr,
+ &iio_const_attr_in_altvoltage0_mag_rising_reset_min_available.dev_attr.attr,
&iio_dev_attr_in_angl1_thresh_rising_value_available.dev_attr.attr,
&iio_dev_attr_in_angl1_thresh_rising_hysteresis_available.dev_attr.attr,
NULL,
--
2.42.0
The AD2S1210 has a programmable threshold for the degradation of signal
(DOS) mismatch fault. This fault is triggered when the difference in
voltage between the sine and cosine inputs exceeds the threshold. In
other words, when the magnitude of sine and cosine inputs are equal,
the AC component of the monitor signal is zero and when the magnitudes
of the sine and cosine inputs are not equal, the AC component of the
monitor signal is the difference between the sine and cosine inputs.
So the fault occurs when the magnitude of the AC component of the
monitor signal exceeds the DOS mismatch threshold voltage.
This patch converts the custom device DOS mismatch threshold attribute
to an event magnitude attribute on the monitor signal channel.
The attribute now uses millivolts instead of the raw register value in
accordance with the IIO ABI.
Emitting the event will be implemented in a later patch.
Signed-off-by: David Lechner <[email protected]>
---
v4 changes:
* Changed event direction from none to rising.
* Fixed missing static qualifier on attribute definition.
v3 changes: This is a new patch in v3
drivers/staging/iio/resolver/ad2s1210.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 3c224bbeae17..870c4a9a6214 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -747,9 +747,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
static IIO_DEVICE_ATTR(fault, 0644,
ad2s1210_show_fault, ad2s1210_clear_fault, 0);
-static IIO_DEVICE_ATTR(dos_mis_thrd, 0644,
- ad2s1210_show_reg, ad2s1210_store_reg,
- AD2S1210_REG_DOS_MIS_THRD);
static IIO_DEVICE_ATTR(dos_rst_max_thrd, 0644,
ad2s1210_show_reg, ad2s1210_store_reg,
AD2S1210_REG_DOS_RST_MAX_THRD);
@@ -795,6 +792,12 @@ static const struct iio_event_spec ad2s1210_monitor_signal_event_spec[] = {
/* Degredation of signal overrange threshold. */
.mask_separate = BIT(IIO_EV_INFO_VALUE),
},
+ {
+ /* Sine/cosine DOS mismatch fault.*/
+ .type = IIO_EV_TYPE_MAG,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
};
static const struct iio_chan_spec ad2s1210_channels[] = {
@@ -868,7 +871,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
static struct attribute *ad2s1210_attributes[] = {
&iio_dev_attr_fault.dev_attr.attr,
- &iio_dev_attr_dos_mis_thrd.dev_attr.attr,
&iio_dev_attr_dos_rst_max_thrd.dev_attr.attr,
&iio_dev_attr_dos_rst_min_thrd.dev_attr.attr,
NULL,
@@ -909,6 +911,8 @@ static IIO_CONST_ATTR(in_altvoltage0_thresh_falling_value_available,
THRESHOLD_RANGE_STR);
static IIO_CONST_ATTR(in_altvoltage0_thresh_rising_value_available,
THRESHOLD_RANGE_STR);
+static IIO_CONST_ATTR(in_altvoltage0_mag_rising_value_available,
+ THRESHOLD_RANGE_STR);
static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_value_available, 0);
static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_hysteresis_available, 0);
@@ -916,6 +920,7 @@ static struct attribute *ad2s1210_event_attributes[] = {
&iio_const_attr_in_phase0_mag_rising_value_available.dev_attr.attr,
&iio_const_attr_in_altvoltage0_thresh_falling_value_available.dev_attr.attr,
&iio_const_attr_in_altvoltage0_thresh_rising_value_available.dev_attr.attr,
+ &iio_const_attr_in_altvoltage0_mag_rising_value_available.dev_attr.attr,
&iio_dev_attr_in_angl1_thresh_rising_value_available.dev_attr.attr,
&iio_dev_attr_in_angl1_thresh_rising_hysteresis_available.dev_attr.attr,
NULL,
@@ -977,6 +982,9 @@ static int ad2s1210_read_event_value(struct iio_dev *indio_dev,
if (type == IIO_EV_TYPE_THRESH && dir == IIO_EV_DIR_RISING)
return ad2s1210_get_voltage_threshold(st,
AD2S1210_REG_DOS_OVR_THRD, val);
+ if (type == IIO_EV_TYPE_MAG)
+ return ad2s1210_get_voltage_threshold(st,
+ AD2S1210_REG_DOS_MIS_THRD, val);
return -EINVAL;
case IIO_PHASE:
return ad2s1210_get_phase_lock_range(st, val, val2);
@@ -1013,6 +1021,9 @@ static int ad2s1210_write_event_value(struct iio_dev *indio_dev,
if (type == IIO_EV_TYPE_THRESH && dir == IIO_EV_DIR_RISING)
return ad2s1210_set_voltage_threshold(st,
AD2S1210_REG_DOS_OVR_THRD, val);
+ if (type == IIO_EV_TYPE_MAG)
+ return ad2s1210_set_voltage_threshold(st,
+ AD2S1210_REG_DOS_MIS_THRD, val);
return -EINVAL;
case IIO_PHASE:
return ad2s1210_set_phase_lock_range(st, val, val2);
--
2.42.0
When reading the position and velocity on the AD2S1210, there is also a
3rd byte following the two data bytes that contains the fault flag bits.
This patch adds support for reading this byte and generating events when
faults occur.
The faults are mapped to various channels and event type in order to
have a unique event for each fault.
Signed-off-by: David Lechner <[email protected]>
---
v4 changes:
* Added sysfs docs for *_label attributes.
* Added implementation of read_event_label.
* Dropped use of IIO_MOD_X_OR_Y.
* Tweaked event type/direction as in previous patches.
* Fixed build error due to st->rx[2].
v3 changes: This is a new patch in v3
Documentation/ABI/testing/sysfs-bus-iio | 15 +++
drivers/staging/iio/resolver/ad2s1210.c | 211 +++++++++++++++++++++++++++++---
2 files changed, 212 insertions(+), 14 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 8e13642bbe23..19cde14f3869 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2239,3 +2239,18 @@ Contact: [email protected]
Description:
The x and y light color coordinate on the CIE 1931 chromaticity
diagram.
+
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltageY_mag_either_label
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltageY_mag_rising_label
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltageY_thresh_falling_label
+What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltageY_thresh_rising_label
+What: /sys/bus/iio/devices/iio:deviceX/events/in_anglvelY_mag_rising_label
+What: /sys/bus/iio/devices/iio:deviceX/events/in_anglY_thresh_rising_label
+What: /sys/bus/iio/devices/iio:deviceX/events/in_phaseY_mag_rising_label
+KernelVersion: 6.7
+Contact: [email protected]
+Description:
+ Optional symbolic label to a device channel event.
+ If a label is defined for this event add that to the event
+ specific attributes. This is useful for userspace to be able to
+ better identify an individual event.
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 9fac806c2a5f..d9d51bbbade8 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -21,6 +21,7 @@
#include <linux/types.h>
#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
#include <linux/iio/trigger_consumer.h>
@@ -35,6 +36,16 @@
#define AD2S1210_SET_ENRES GENMASK(3, 2)
#define AD2S1210_SET_RES GENMASK(1, 0)
+/* fault register flags */
+#define AD2S1210_FAULT_CLIP BIT(7)
+#define AD2S1210_FAULT_LOS BIT(6)
+#define AD2S1210_FAULT_DOS_OVR BIT(5)
+#define AD2S1210_FAULT_DOS_MIS BIT(4)
+#define AD2S1210_FAULT_LOT BIT(3)
+#define AD2S1210_FAULT_VELOCITY BIT(2)
+#define AD2S1210_FAULT_PHASE BIT(1)
+#define AD2S1210_FAULT_CONFIG_PARITY BIT(0)
+
#define AD2S1210_REG_POSITION_MSB 0x80
#define AD2S1210_REG_POSITION_LSB 0x81
#define AD2S1210_REG_VELOCITY_MSB 0x82
@@ -71,6 +82,8 @@
/* max voltage for threshold registers is 0x7F * 38 mV */
#define THRESHOLD_RANGE_STR "[0 38 4826]"
+#define FAULT_ONESHOT(bit, new, old) (new & bit && !(old & bit))
+
enum ad2s1210_mode {
MOD_POS = 0b00,
MOD_VEL = 0b01,
@@ -100,8 +113,13 @@ struct ad2s1210_state {
int hysteresis_available[2];
/** The selected resolution */
enum ad2s1210_resolution resolution;
+ /** Copy of fault register from the previous read. */
+ u8 prev_fault_flags;
/** For reading raw sample value via SPI. */
- __be16 sample __aligned(IIO_DMA_MINALIGN);
+ struct {
+ __be16 raw;
+ u8 fault;
+ } sample __aligned(IIO_DMA_MINALIGN);
/** Scan buffer */
struct {
__be16 chan[2];
@@ -160,7 +178,15 @@ static int ad2s1210_regmap_reg_write(void *context, unsigned int reg,
if (ret < 0)
return ret;
- return spi_sync_transfer(st->sdev, xfers, ARRAY_SIZE(xfers));
+ ret = spi_sync_transfer(st->sdev, xfers, ARRAY_SIZE(xfers));
+ if (ret < 0)
+ return ret;
+
+ /* soft reset also clears the fault register */
+ if (reg == AD2S1210_REG_SOFT_RESET)
+ st->prev_fault_flags = 0;
+
+ return 0;
}
/*
@@ -203,6 +229,10 @@ static int ad2s1210_regmap_reg_read(void *context, unsigned int reg,
if (ret < 0)
return ret;
+ /* reading the fault register also clears it */
+ if (reg == AD2S1210_REG_FAULT)
+ st->prev_fault_flags = 0;
+
/*
* If the D7 bit is set on any read/write register, it indicates a
* parity error. The fault register is read-only and the D7 bit means
@@ -286,14 +316,101 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
return ret < 0 ? ret : len;
}
-static int ad2s1210_single_conversion(struct ad2s1210_state *st,
+static void ad2s1210_push_events(struct iio_dev *indio_dev,
+ u8 flags, s64 timestamp)
+{
+ struct ad2s1210_state *st = iio_priv(indio_dev);
+
+ /* Sine/cosine inputs clipped */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_CLIP, flags, st->prev_fault_flags)) {
+ /*
+ * The chip does not differentiate between fault on sine vs.
+ * cosine channel so we just send an event on both channels.
+ */
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_ALTVOLTAGE, 1,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_EITHER),
+ timestamp);
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_ALTVOLTAGE, 2,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_EITHER),
+ timestamp);
+ }
+
+ /* Sine/cosine inputs below LOS threshold */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_LOS, flags, st->prev_fault_flags))
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_ALTVOLTAGE, 0,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_FALLING),
+ timestamp);
+
+ /* Sine/cosine inputs exceed DOS overrange threshold */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_DOS_OVR, flags, st->prev_fault_flags))
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_ALTVOLTAGE, 0,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ timestamp);
+
+ /* Sine/cosine inputs exceed DOS mismatch threshold */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_DOS_MIS, flags, st->prev_fault_flags))
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_ALTVOLTAGE, 0,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_RISING),
+ timestamp);
+
+ /* Tracking error exceeds LOT threshold */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_LOT, flags, st->prev_fault_flags))
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_ANGL, 1,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ timestamp);
+
+ /* Velocity exceeds maximum tracking rate */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_VELOCITY, flags, st->prev_fault_flags))
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_ANGL_VEL, 0,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ timestamp);
+
+ /* Phase error exceeds phase lock range */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_PHASE, flags, st->prev_fault_flags))
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_PHASE, 0,
+ IIO_EV_TYPE_MAG,
+ IIO_EV_DIR_RISING),
+ timestamp);
+
+ /* Configuration parity error */
+ if (FAULT_ONESHOT(AD2S1210_FAULT_CONFIG_PARITY, flags,
+ st->prev_fault_flags))
+ /*
+ * Userspace should also get notified of this via error return
+ * when trying to write to any attribute that writes a register.
+ */
+ dev_err_ratelimited(&indio_dev->dev,
+ "Configuration parity error\n");
+
+ st->prev_fault_flags = flags;
+}
+
+static int ad2s1210_single_conversion(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val)
{
+ struct ad2s1210_state *st = iio_priv(indio_dev);
+ s64 timestamp;
int ret;
mutex_lock(&st->lock);
gpiod_set_value(st->sample_gpio, 1);
+ timestamp = iio_get_time_ns(indio_dev);
/* delay (6 * tck + 20) nano seconds */
udelay(1);
@@ -310,17 +427,17 @@ static int ad2s1210_single_conversion(struct ad2s1210_state *st,
}
if (ret < 0)
goto error_ret;
- ret = spi_read(st->sdev, &st->sample, 2);
+ ret = spi_read(st->sdev, &st->sample, 3);
if (ret < 0)
goto error_ret;
switch (chan->type) {
case IIO_ANGL:
- *val = be16_to_cpu(st->sample);
+ *val = be16_to_cpu(st->sample.raw);
ret = IIO_VAL_INT;
break;
case IIO_ANGL_VEL:
- *val = (s16)be16_to_cpu(st->sample);
+ *val = (s16)be16_to_cpu(st->sample.raw);
ret = IIO_VAL_INT;
break;
default:
@@ -328,6 +445,8 @@ static int ad2s1210_single_conversion(struct ad2s1210_state *st,
break;
}
+ ad2s1210_push_events(indio_dev, st->sample.fault, timestamp);
+
error_ret:
gpiod_set_value(st->sample_gpio, 0);
/* delay (2 * tck + 20) nano seconds */
@@ -611,7 +730,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
- return ad2s1210_single_conversion(st, chan, val);
+ return ad2s1210_single_conversion(indio_dev, chan, val);
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
case IIO_ANGL:
@@ -725,6 +844,14 @@ static const struct iio_event_spec ad2s1210_position_event_spec[] = {
},
};
+static const struct iio_event_spec ad2s1210_velocity_event_spec[] = {
+ {
+ /* Velocity exceeds maximum tracking rate fault. */
+ .type = IIO_EV_TYPE_MAG,
+ .dir = IIO_EV_DIR_RISING,
+ },
+};
+
static const struct iio_event_spec ad2s1210_phase_event_spec[] = {
{
/* Phase error fault. */
@@ -758,6 +885,14 @@ static const struct iio_event_spec ad2s1210_monitor_signal_event_spec[] = {
},
};
+static const struct iio_event_spec ad2s1210_sin_cos_event_spec[] = {
+ {
+ /* Sine/cosine clipping fault. */
+ .type = IIO_EV_TYPE_MAG,
+ .dir = IIO_EV_DIR_EITHER,
+ },
+};
+
static const struct iio_chan_spec ad2s1210_channels[] = {
{
.type = IIO_ANGL,
@@ -788,6 +923,8 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
},
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE),
+ .event_spec = ad2s1210_velocity_event_spec,
+ .num_event_specs = ARRAY_SIZE(ad2s1210_velocity_event_spec),
},
IIO_CHAN_SOFT_TIMESTAMP(2),
{
@@ -824,6 +961,22 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
.scan_index = -1,
.event_spec = ad2s1210_monitor_signal_event_spec,
.num_event_specs = ARRAY_SIZE(ad2s1210_monitor_signal_event_spec),
+ }, {
+ /* sine input */
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .channel = 1,
+ .scan_index = -1,
+ .event_spec = ad2s1210_sin_cos_event_spec,
+ .num_event_specs = ARRAY_SIZE(ad2s1210_sin_cos_event_spec),
+ }, {
+ /* cosine input */
+ .type = IIO_ALTVOLTAGE,
+ .indexed = 1,
+ .channel = 2,
+ .scan_index = -1,
+ .event_spec = ad2s1210_sin_cos_event_spec,
+ .num_event_specs = ARRAY_SIZE(ad2s1210_sin_cos_event_spec),
},
};
@@ -943,7 +1096,7 @@ static const struct attribute_group ad2s1210_event_attribute_group = {
static int ad2s1210_initial(struct ad2s1210_state *st)
{
- unsigned char data;
+ unsigned int data;
int ret;
mutex_lock(&st->lock);
@@ -1043,6 +1196,36 @@ static int ad2s1210_write_event_value(struct iio_dev *indio_dev,
}
}
+static int ad2s1210_read_event_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ char *label)
+{
+ if (chan->type == IIO_ANGL)
+ return sprintf(label, "LOT\n");
+ if (chan->type == IIO_ANGL_VEL)
+ return sprintf(label, "max tracking rate\n");
+ if (chan->type == IIO_PHASE)
+ return sprintf(label, "phase lock\n");
+ if (chan->type == IIO_ALTVOLTAGE) {
+ if (chan->channel == 0) {
+ if (type == IIO_EV_TYPE_THRESH &&
+ dir == IIO_EV_DIR_FALLING)
+ return sprintf(label, "LOS\n");
+ if (type == IIO_EV_TYPE_THRESH &&
+ dir == IIO_EV_DIR_RISING)
+ return sprintf(label, "DOS overrange\n");
+ if (type == IIO_EV_TYPE_MAG)
+ return sprintf(label, "DOS mismatch\n");
+ }
+ if (chan->channel == 1 || chan->channel == 2)
+ return sprintf(label, "clipped\n");
+ }
+
+ return -EINVAL;
+}
+
static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
unsigned int reg, unsigned int writeval,
unsigned int *readval)
@@ -1080,12 +1263,11 @@ static irqreturn_t ad2s1210_trigger_handler(int irq, void *p)
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);
+ ret = spi_read(st->sdev, &st->sample, 3);
if (ret < 0)
goto error_ret;
- memcpy(&st->scan.chan[chan++], st->rx, 2);
+ memcpy(&st->scan.chan[chan++], &st->sample.raw, 2);
}
if (test_bit(1, indio_dev->active_scan_mask)) {
@@ -1093,14 +1275,14 @@ static irqreturn_t ad2s1210_trigger_handler(int irq, void *p)
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);
+ ret = spi_read(st->sdev, &st->sample, 3);
if (ret < 0)
goto error_ret;
- memcpy(&st->scan.chan[chan++], st->rx, 2);
+ memcpy(&st->scan.chan[chan++], &st->sample.raw, 2);
}
+ ad2s1210_push_events(indio_dev, st->sample.fault, pf->timestamp);
iio_push_to_buffers_with_timestamp(indio_dev, &st->scan, pf->timestamp);
error_ret:
@@ -1119,6 +1301,7 @@ static const struct iio_info ad2s1210_info = {
.attrs = &ad2s1210_attribute_group,
.read_event_value = ad2s1210_read_event_value,
.write_event_value = ad2s1210_write_event_value,
+ .read_event_label = ad2s1210_read_event_label,
.debugfs_reg_access = &ad2s1210_debugfs_reg_access,
};
--
2.42.0
The ad2s1210 driver shoe-horns the register and fault support into IIO
events. The mapping between the registers/faults and the events is not
obvious. To save users from having to read the entire driver to figure
out how to use it, add a summary of the register/fault support to the
top of the file.
Signed-off-by: David Lechner <[email protected]>
---
v4 changes: New patch in v4.
drivers/staging/iio/resolver/ad2s1210.c | 40 +++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index d9d51bbbade8..51490fea1647 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -4,7 +4,47 @@
*
* Copyright (c) 2010-2010 Analog Devices Inc.
* Copyright (c) 2023 BayLibre, SAS
+ *
+ * Device register to IIO ABI mapping:
+ *
+ * Register | Addr | IIO ABI (sysfs)
+ * ----------------------------|------|-------------------------------------------
+ * DOS Overrange Threshold | 0x89 | events/in_altvoltage0_thresh_rising_value
+ * DOS Mismatch Threshold | 0x8A | events/in_altvoltage0_mag_rising_value
+ * DOS Reset Maximum Threshold | 0x8B | events/in_altvoltage0_mag_rising_reset_max
+ * DOS Reset Minimum Threshold | 0x8C | events/in_altvoltage0_mag_rising_reset_min
+ * LOT High Threshold | 0x8D | events/in_angl1_thresh_rising_value
+ * LOT Low Threshold [1] | 0x8E | events/in_angl1_thresh_rising_hysteresis
+ * Excitation Frequency | 0x91 | out_altvoltage0_frequency
+ * Control | 0x92 | *as bit fields*
+ * Phase lock range | D5 | events/in_phase0_mag_rising_value
+ * Hysteresis | D4 | in_angl0_hysteresis
+ * Encoder resolution | D3:2 | *not implemented*
+ * Resolution | D1:0 | *device tree: assigned-resolution-bits*
+ * Soft Reset | 0xF0 | [2]
+ * Fault | 0xFF | *not implemented*
+ *
+ * [1]: The value written to the LOT low register is high value minus the
+ * hysteresis.
+ * [2]: Soft reset is performed when `out_altvoltage0_frequency` is written.
+ *
+ * Fault to event mapping:
+ *
+ * Fault | | Channel | Type | Direction
+ * ----------------------------------------|----|---------------------------------
+ * Sine/cosine inputs clipped [3] | D7 | altvoltage1 | mag | either
+ * Sine/cosine inputs below LOS | D6 | altvoltage0 | thresh | falling
+ * Sine/cosine inputs exceed DOS overrange | D5 | altvoltage0 | thresh | rising
+ * Sine/cosine inputs exceed DOS mismatch | D4 | altvoltage0 | mag | rising
+ * Tracking error exceeds LOT | D3 | angl1 | thresh | rising
+ * Velocity exceeds maximum tracking rate | D2 | anglvel0 | mag | rising
+ * Phase error exceeds phase lock range | D1 | phase0 | mag | rising
+ * Configuration parity error | D0 | *writes to kernel log*
+ *
+ * [3]: The chip does not differentiate between fault on sine vs. cosine so
+ * there will also be an event on the altvoltage2 channel.
*/
+
#include <linux/bitfield.h>
#include <linux/bits.h>
#include <linux/clk.h>
--
2.42.0
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 `assigned-resolution-bits` 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]>
---
v4 changes:
* Fixed devicetree property name in commit message.
* Reworked handling of hysteresis_available to handle
assigned-resolution-bits != 16.
v3 changes:
* Fixed multiline comment style.
drivers/staging/iio/resolver/ad2s1210.c | 150 +++++++++++++++-----------------
1 file changed, 71 insertions(+), 79 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 0c7772725330..66ef35fbb6fe 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -63,6 +63,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;
@@ -70,15 +77,14 @@ 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 clkin_hz;
/** Available raw hysteresis values based on resolution. */
int hysteresis_available[2];
- u8 resolution;
+ /** The selected resolution */
+ enum ad2s1210_resolution resolution;
/** For reading raw sample value via SPI. */
__be16 sample __aligned(IIO_DMA_MINALIGN);
/** SPI transmit buffer. */
@@ -215,63 +221,6 @@ static int ad2s1210_reinit_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 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;
- st->hysteresis_available[1] = 1 << (16 - st->resolution);
- 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)
@@ -413,7 +362,7 @@ static int ad2s1210_get_hysteresis(struct ad2s1210_state *st, int *val)
if (ret < 0)
return ret;
- *val = ret << (16 - st->resolution);
+ *val = ret << (2 * (AD2S1210_RES_16 - st->resolution));
return IIO_VAL_INT;
}
@@ -577,8 +526,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
}
}
-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);
@@ -633,7 +580,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
};
static struct attribute *ad2s1210_attributes[] = {
- &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,
@@ -655,15 +601,12 @@ 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)
- goto error_ret;
/* Use default config register value plus resolution from devicetree. */
data = FIELD_PREP(AD2S1210_PHASE_LOCK_RANGE_44, 1);
data |= FIELD_PREP(AD2S1210_ENABLE_HYSTERESIS, 1);
data |= FIELD_PREP(AD2S1210_SET_ENRES, 0x3);
- data |= FIELD_PREP(AD2S1210_SET_RES, (st->resolution - 10) >> 1);
+ data |= FIELD_PREP(AD2S1210_SET_RES, st->resolution);
ret = regmap_write(st->regmap, AD2S1210_REG_CONTROL, data);
if (ret < 0)
@@ -703,6 +646,35 @@ 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;
+ /*
+ * These are values that correlate to the hysteresis bit in the Control
+ * register. 0 = disabled, 1 = enabled. When enabled, the actual
+ * hysteresis is +/- 1 LSB of the raw position value. Which bit is the
+ * LSB depends on the specified resolution.
+ */
+ st->hysteresis_available[0] = 0;
+ st->hysteresis_available[1] = 1 << (2 * (AD2S1210_RES_16 -
+ st->resolution));
+
+ return 0;
+}
+
static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
{
struct device *dev = &st->sdev->dev;
@@ -724,6 +696,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);
@@ -741,16 +716,32 @@ 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;
}
@@ -814,9 +805,10 @@ static int ad2s1210_probe(struct spi_device *spi)
mutex_init(&st->lock);
st->sdev = spi;
- st->resolution = 12;
- st->hysteresis_available[0] = 0;
- st->hysteresis_available[1] = 1 << (16 - st->resolution);
+
+ ret = ad2s1210_setup_properties(st);
+ if (ret < 0)
+ return ret;
ret = ad2s1210_setup_clocks(st);
if (ret < 0)
--
2.42.0
On Thu, 5 Oct 2023 19:50:18 -0500
David Lechner <[email protected]> wrote:
> When reading registers on the AD2S1210 chip, we have to supply a "dummy"
> address for the second SPI tx byte so that we don't accidentally write
> to a register. This register will be read and the value discarded on the
> next regmap read or write call.
>
> Reading the fault register has a side-effect of clearing the faults
> so we should not use this register for the dummy read.
>
> Signed-off-by: David Lechner <[email protected]>
ouch.
Applied to the togreg branch of iio.git and pushed out as testing for 0day
to take a look at it.
Thanks,
Jonathan
> ---
>
> v4 changes: New patch
>
> (this probably should have been done before "staging: iio: resolver:
> ad2s1210: use regmap for config registers" but was overlooked until now)
>
> drivers/staging/iio/resolver/ad2s1210.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index 67d8af0dd7ae..8fbde9517fe9 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -166,9 +166,10 @@ static int ad2s1210_regmap_reg_read(void *context, unsigned int reg,
> st->tx[0] = reg;
> /*
> * Must be valid register address here otherwise this could write data.
> - * It doesn't matter which one.
> + * It doesn't matter which one as long as reading doesn't have side-
> + * effects.
> */
> - st->tx[1] = AD2S1210_REG_FAULT;
> + st->tx[1] = AD2S1210_REG_CONTROL;
>
> ret = spi_sync_transfer(st->sdev, xfers, ARRAY_SIZE(xfers));
> if (ret < 0)
>
On Thu, 5 Oct 2023 19:50:21 -0500
David Lechner <[email protected]> wrote:
> 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 `assigned-resolution-bits` 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]>
Applied
> ---
>
> v4 changes:
> * Fixed devicetree property name in commit message.
> * Reworked handling of hysteresis_available to handle
> assigned-resolution-bits != 16.
>
> v3 changes:
> * Fixed multiline comment style.
>
> drivers/staging/iio/resolver/ad2s1210.c | 150 +++++++++++++++-----------------
> 1 file changed, 71 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index 0c7772725330..66ef35fbb6fe 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -63,6 +63,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;
> @@ -70,15 +77,14 @@ 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 clkin_hz;
> /** Available raw hysteresis values based on resolution. */
> int hysteresis_available[2];
> - u8 resolution;
> + /** The selected resolution */
> + enum ad2s1210_resolution resolution;
> /** For reading raw sample value via SPI. */
> __be16 sample __aligned(IIO_DMA_MINALIGN);
> /** SPI transmit buffer. */
> @@ -215,63 +221,6 @@ static int ad2s1210_reinit_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 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;
> - st->hysteresis_available[1] = 1 << (16 - st->resolution);
> - 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)
> @@ -413,7 +362,7 @@ static int ad2s1210_get_hysteresis(struct ad2s1210_state *st, int *val)
> if (ret < 0)
> return ret;
>
> - *val = ret << (16 - st->resolution);
> + *val = ret << (2 * (AD2S1210_RES_16 - st->resolution));
> return IIO_VAL_INT;
> }
>
> @@ -577,8 +526,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> -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);
>
> @@ -633,7 +580,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
> };
>
> static struct attribute *ad2s1210_attributes[] = {
> - &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,
> @@ -655,15 +601,12 @@ 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)
> - goto error_ret;
>
> /* Use default config register value plus resolution from devicetree. */
> data = FIELD_PREP(AD2S1210_PHASE_LOCK_RANGE_44, 1);
> data |= FIELD_PREP(AD2S1210_ENABLE_HYSTERESIS, 1);
> data |= FIELD_PREP(AD2S1210_SET_ENRES, 0x3);
> - data |= FIELD_PREP(AD2S1210_SET_RES, (st->resolution - 10) >> 1);
> + data |= FIELD_PREP(AD2S1210_SET_RES, st->resolution);
>
> ret = regmap_write(st->regmap, AD2S1210_REG_CONTROL, data);
> if (ret < 0)
> @@ -703,6 +646,35 @@ 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;
> + /*
> + * These are values that correlate to the hysteresis bit in the Control
> + * register. 0 = disabled, 1 = enabled. When enabled, the actual
> + * hysteresis is +/- 1 LSB of the raw position value. Which bit is the
> + * LSB depends on the specified resolution.
> + */
> + st->hysteresis_available[0] = 0;
> + st->hysteresis_available[1] = 1 << (2 * (AD2S1210_RES_16 -
> + st->resolution));
> +
> + return 0;
> +}
> +
> static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
> {
> struct device *dev = &st->sdev->dev;
> @@ -724,6 +696,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);
> @@ -741,16 +716,32 @@ 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;
> }
> @@ -814,9 +805,10 @@ static int ad2s1210_probe(struct spi_device *spi)
>
> mutex_init(&st->lock);
> st->sdev = spi;
> - st->resolution = 12;
> - st->hysteresis_available[0] = 0;
> - st->hysteresis_available[1] = 1 << (16 - st->resolution);
> +
> + ret = ad2s1210_setup_properties(st);
> + if (ret < 0)
> + return ret;
>
> ret = ad2s1210_setup_clocks(st);
> if (ret < 0)
>
On Thu, 5 Oct 2023 19:50:22 -0500
David Lechner <[email protected]> wrote:
> The AD2S1210 chip has a phase lock range feature that allows selecting
> the allowable phase difference between the excitation output and the
> sine and cosine inputs. This can be set to either 44 degrees (default)
> or 360 degrees.
>
> This patch adds a new phase channel with a phase0_mag_rising event that
> can be used to configure the phase lock range. Actually emitting the
> event will be added in a subsequent patch.
>
> Signed-off-by: David Lechner <[email protected]>
Whilst I'm not feeling totally happy with some of the error condition
handling we have here, I think it's the best we can do without inventing
a whole knew path for error records. Whilst I have some thoughts on that
and using the tracepoint approach used for RAS event handling for servers
etc and the heavier weight userspace software that entails, we can always
bolt that on top later and it doesn't solve the control element anyway..
So applied,
Thanks,
Jonathan
> ---
>
> v4 changes:
> * Changed event direction from none to rising.
> * Fixed missing static qualifier on attribute definition.
>
> v3 changes:
> * This is a new patch to replace "staging: iio: resolver: ad2s1210: add
> phase_lock_range attributes"
>
>
> drivers/staging/iio/resolver/ad2s1210.c | 125 ++++++++++++++++++++++++++++++++
> 1 file changed, 125 insertions(+)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index 66ef35fbb6fe..83f6ac890dbc 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -56,6 +56,13 @@
> #define AD2S1210_MIN_FCW 0x4
> #define AD2S1210_MAX_FCW 0x50
>
> +/* 44 degrees ~= 0.767945 radians */
> +#define PHASE_44_DEG_TO_RAD_INT 0
> +#define PHASE_44_DEG_TO_RAD_MICRO 767945
> +/* 360 degrees ~= 6.283185 radians */
> +#define PHASE_360_DEG_TO_RAD_INT 6
> +#define PHASE_360_DEG_TO_RAD_MICRO 283185
> +
> enum ad2s1210_mode {
> MOD_POS = 0b00,
> MOD_VEL = 0b01,
> @@ -379,6 +386,54 @@ static int ad2s1210_set_hysteresis(struct ad2s1210_state *st, int val)
> return ret;
> }
>
> +static int ad2s1210_get_phase_lock_range(struct ad2s1210_state *st,
> + int *val, int *val2)
> +{
> + int ret;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_test_bits(st->regmap, AD2S1210_REG_CONTROL,
> + AD2S1210_PHASE_LOCK_RANGE_44);
> + mutex_unlock(&st->lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + if (ret) {
> + /* 44 degrees as radians */
> + *val = PHASE_44_DEG_TO_RAD_INT;
> + *val2 = PHASE_44_DEG_TO_RAD_MICRO;
> + } else {
> + /* 360 degrees as radians */
> + *val = PHASE_360_DEG_TO_RAD_INT;
> + *val2 = PHASE_360_DEG_TO_RAD_MICRO;
> + }
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int ad2s1210_set_phase_lock_range(struct ad2s1210_state *st,
> + int val, int val2)
> +{
> + int deg, ret;
> +
> + /* convert radians to degrees - only two allowable values */
> + if (val == PHASE_44_DEG_TO_RAD_INT && val2 == PHASE_44_DEG_TO_RAD_MICRO)
> + deg = 44;
> + else if (val == PHASE_360_DEG_TO_RAD_INT &&
> + val2 == PHASE_360_DEG_TO_RAD_MICRO)
> + deg = 360;
> + else
> + return -EINVAL;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
> + AD2S1210_PHASE_LOCK_RANGE_44,
> + deg == 44 ? AD2S1210_PHASE_LOCK_RANGE_44 : 0);
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> static int ad2s1210_get_excitation_frequency(struct ad2s1210_state *st, int *val)
> {
> unsigned int reg_val;
> @@ -551,6 +606,16 @@ static IIO_DEVICE_ATTR(lot_low_thrd, 0644,
> ad2s1210_show_reg, ad2s1210_store_reg,
> AD2S1210_REG_LOT_LOW_THRD);
>
> +static const struct iio_event_spec ad2s1210_phase_event_spec[] = {
> + {
> + /* Phase error fault. */
> + .type = IIO_EV_TYPE_MAG,
> + .dir = IIO_EV_DIR_RISING,
> + /* Phase lock range. */
> + .mask_separate = BIT(IIO_EV_INFO_VALUE),
> + },
> +};
> +
> static const struct iio_chan_spec ad2s1210_channels[] = {
> {
> .type = IIO_ANGL,
> @@ -567,6 +632,14 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
> .channel = 0,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> BIT(IIO_CHAN_INFO_SCALE),
> + }, {
> + /* used to configure phase lock range and get phase lock error */
> + .type = IIO_PHASE,
> + .indexed = 1,
> + .channel = 0,
> + .scan_index = -1,
> + .event_spec = ad2s1210_phase_event_spec,
> + .num_event_specs = ARRAY_SIZE(ad2s1210_phase_event_spec),
> }, {
> /* excitation frequency output */
> .type = IIO_ALTVOLTAGE,
> @@ -595,6 +668,21 @@ static const struct attribute_group ad2s1210_attribute_group = {
> .attrs = ad2s1210_attributes,
> };
>
> +static IIO_CONST_ATTR(in_phase0_mag_rising_value_available,
> + __stringify(PHASE_44_DEG_TO_RAD_INT) "."
> + __stringify(PHASE_44_DEG_TO_RAD_MICRO) " "
> + __stringify(PHASE_360_DEG_TO_RAD_INT) "."
> + __stringify(PHASE_360_DEG_TO_RAD_MICRO));
> +
> +static struct attribute *ad2s1210_event_attributes[] = {
> + &iio_const_attr_in_phase0_mag_rising_value_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group ad2s1210_event_attribute_group = {
> + .attrs = ad2s1210_event_attributes,
> +};
> +
> static int ad2s1210_initial(struct ad2s1210_state *st)
> {
> unsigned char data;
> @@ -619,6 +707,40 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
> return ret;
> }
>
> +static int ad2s1210_read_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int *val, int *val2)
> +{
> + struct ad2s1210_state *st = iio_priv(indio_dev);
> +
> + switch (chan->type) {
> + case IIO_PHASE:
> + return ad2s1210_get_phase_lock_range(st, val, val2);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad2s1210_write_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> +{
> + struct ad2s1210_state *st = iio_priv(indio_dev);
> +
> + switch (chan->type) {
> + case IIO_PHASE:
> + return ad2s1210_set_phase_lock_range(st, val, val2);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
> unsigned int reg, unsigned int writeval,
> unsigned int *readval)
> @@ -639,10 +761,13 @@ static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
> }
>
> static const struct iio_info ad2s1210_info = {
> + .event_attrs = &ad2s1210_event_attribute_group,
> .read_raw = ad2s1210_read_raw,
> .read_avail = ad2s1210_read_avail,
> .write_raw = ad2s1210_write_raw,
> .attrs = &ad2s1210_attribute_group,
> + .read_event_value = ad2s1210_read_event_value,
> + .write_event_value = ad2s1210_write_event_value,
> .debugfs_reg_access = &ad2s1210_debugfs_reg_access,
> };
>
>
On Thu, 5 Oct 2023 19:50:25 -0500
David Lechner <[email protected]> wrote:
> The AD2S1210 has a programmable threshold for the loss of signal (LOS)
> fault. This fault is triggered when either the sine or cosine input
> falls below the threshold voltage.
>
> This patch converts the custom device LOS threshold attribute to an
> event falling edge threshold attribute on a new monitor signal channel.
> The monitor signal is an internal signal that combines the amplitudes
> of the sine and cosine inputs as well as the current angle and position
> output. This signal is used to detect faults in the input signals.
>
> The attribute now uses millivolts instead of the raw register value in
> accordance with the IIO ABI.
Hmm. The ABI is a bit ambigious on this front. When we have 'normal'
device attributes we tend to use the presence of _scale attributes
to scale both raw channels and the events associated with them.
I don't think we talk about what to do if that isn't available or
what happens for 'processed' channels - i.e. the ones already in
base units because those tend not to have associated events (as their
is normally nasty maths involved that is only sometimes reversible.)
We may want to clarify the ABI on this, but this is definitely
a valid option given there is no _scale for the channel.
Hence in interests of moving forwards I'm going to apply it.
Applied to the togreg branch of iio.git and pushed out as testing for
0-day to poke at it.
Thanks,
Jonathan
>
> Emitting the event will be implemented in a later patch.
>
> Signed-off-by: David Lechner <[email protected]>
> ---
>
> v4 changes:
> * Fixed missing static qualifier on attribute definition.
>
> v3 changes: This is a new patch in v3
>
> drivers/staging/iio/resolver/ad2s1210.c | 76 +++++++++++++++++++++++++++++++--
> 1 file changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index 12437f697f79..d52aed30ca66 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -66,6 +66,11 @@
> #define PHASE_360_DEG_TO_RAD_INT 6
> #define PHASE_360_DEG_TO_RAD_MICRO 283185
>
> +/* Threshold voltage registers have 1 LSB == 38 mV */
> +#define THRESHOLD_MILLIVOLT_PER_LSB 38
> +/* max voltage for threshold registers is 0x7F * 38 mV */
> +#define THRESHOLD_RANGE_STR "[0 38 4826]"
> +
> enum ad2s1210_mode {
> MOD_POS = 0b00,
> MOD_VEL = 0b01,
> @@ -451,6 +456,38 @@ static const int ad2s1210_lot_threshold_urad_per_lsb[] = {
> 1237, /* 16-bit: same as 14-bit */
> };
>
> +static int ad2s1210_get_voltage_threshold(struct ad2s1210_state *st,
> + unsigned int reg, int *val)
> +{
> + unsigned int reg_val;
> + int ret;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_read(st->regmap, reg, ®_val);
> + mutex_unlock(&st->lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + *val = reg_val * THRESHOLD_MILLIVOLT_PER_LSB;
> + return IIO_VAL_INT;
> +}
> +
> +static int ad2s1210_set_voltage_threshold(struct ad2s1210_state *st,
> + unsigned int reg, int val)
> +{
> + unsigned int reg_val;
> + int ret;
> +
> + reg_val = val / THRESHOLD_MILLIVOLT_PER_LSB;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_write(st->regmap, reg, reg_val);
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> static int ad2s1210_get_lot_high_threshold(struct ad2s1210_state *st,
> int *val, int *val2)
> {
> @@ -710,9 +747,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
> static IIO_DEVICE_ATTR(fault, 0644,
> ad2s1210_show_fault, ad2s1210_clear_fault, 0);
>
> -static IIO_DEVICE_ATTR(los_thrd, 0644,
> - ad2s1210_show_reg, ad2s1210_store_reg,
> - AD2S1210_REG_LOS_THRD);
> static IIO_DEVICE_ATTR(dos_ovr_thrd, 0644,
> ad2s1210_show_reg, ad2s1210_store_reg,
> AD2S1210_REG_DOS_OVR_THRD);
> @@ -749,6 +783,16 @@ static const struct iio_event_spec ad2s1210_phase_event_spec[] = {
> },
> };
>
> +static const struct iio_event_spec ad2s1210_monitor_signal_event_spec[] = {
> + {
> + /* Sine/cosine below LOS threshold fault. */
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_FALLING,
> + /* Loss of signal threshold. */
> + .mask_separate = BIT(IIO_EV_INFO_VALUE),
> + },
> +};
> +
> static const struct iio_chan_spec ad2s1210_channels[] = {
> {
> .type = IIO_ANGL,
> @@ -807,12 +851,19 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
> .scan_index = -1,
> .info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY),
> .info_mask_separate_available = BIT(IIO_CHAN_INFO_FREQUENCY),
> + }, {
> + /* monitor signal */
> + .type = IIO_ALTVOLTAGE,
> + .indexed = 1,
> + .channel = 0,
> + .scan_index = -1,
> + .event_spec = ad2s1210_monitor_signal_event_spec,
> + .num_event_specs = ARRAY_SIZE(ad2s1210_monitor_signal_event_spec),
> },
> };
>
> static struct attribute *ad2s1210_attributes[] = {
> &iio_dev_attr_fault.dev_attr.attr,
> - &iio_dev_attr_los_thrd.dev_attr.attr,
> &iio_dev_attr_dos_ovr_thrd.dev_attr.attr,
> &iio_dev_attr_dos_mis_thrd.dev_attr.attr,
> &iio_dev_attr_dos_rst_max_thrd.dev_attr.attr,
> @@ -851,11 +902,14 @@ static IIO_CONST_ATTR(in_phase0_mag_rising_value_available,
> __stringify(PHASE_44_DEG_TO_RAD_MICRO) " "
> __stringify(PHASE_360_DEG_TO_RAD_INT) "."
> __stringify(PHASE_360_DEG_TO_RAD_MICRO));
> +static IIO_CONST_ATTR(in_altvoltage0_thresh_falling_value_available,
> + THRESHOLD_RANGE_STR);
> static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_value_available, 0);
> static IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_hysteresis_available, 0);
>
> static struct attribute *ad2s1210_event_attributes[] = {
> &iio_const_attr_in_phase0_mag_rising_value_available.dev_attr.attr,
> + &iio_const_attr_in_altvoltage0_thresh_falling_value_available.dev_attr.attr,
> &iio_dev_attr_in_angl1_thresh_rising_value_available.dev_attr.attr,
> &iio_dev_attr_in_angl1_thresh_rising_hysteresis_available.dev_attr.attr,
> NULL,
> @@ -908,6 +962,13 @@ static int ad2s1210_read_event_value(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> + case IIO_ALTVOLTAGE:
> + if (chan->output)
> + return -EINVAL;
> + if (type == IIO_EV_TYPE_THRESH && dir == IIO_EV_DIR_FALLING)
> + return ad2s1210_get_voltage_threshold(st,
> + AD2S1210_REG_LOS_THRD, val);
> + return -EINVAL;
> case IIO_PHASE:
> return ad2s1210_get_phase_lock_range(st, val, val2);
> default:
> @@ -934,6 +995,13 @@ static int ad2s1210_write_event_value(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> + case IIO_ALTVOLTAGE:
> + if (chan->output)
> + return -EINVAL;
> + if (type == IIO_EV_TYPE_THRESH && dir == IIO_EV_DIR_FALLING)
> + return ad2s1210_set_voltage_threshold(st,
> + AD2S1210_REG_LOS_THRD, val);
> + return -EINVAL;
> case IIO_PHASE:
> return ad2s1210_set_phase_lock_range(st, val, val2);
> default:
>
On Thu, 5 Oct 2023 19:50:27 -0500
David Lechner <[email protected]> wrote:
> The AD2S1210 has a programmable threshold for the degradation of signal
> (DOS) mismatch fault. This fault is triggered when the difference in
> voltage between the sine and cosine inputs exceeds the threshold. In
> other words, when the magnitude of sine and cosine inputs are equal,
> the AC component of the monitor signal is zero and when the magnitudes
> of the sine and cosine inputs are not equal, the AC component of the
> monitor signal is the difference between the sine and cosine inputs.
> So the fault occurs when the magnitude of the AC component of the
> monitor signal exceeds the DOS mismatch threshold voltage.
>
> This patch converts the custom device DOS mismatch threshold attribute
> to an event magnitude attribute on the monitor signal channel.
>
> The attribute now uses millivolts instead of the raw register value in
> accordance with the IIO ABI.
>
> Emitting the event will be implemented in a later patch.
>
> Signed-off-by: David Lechner <[email protected]>
Applied,
Thanks,
Jonathan
On Thu, 5 Oct 2023 19:50:28 -0500
David Lechner <[email protected]> wrote:
> The AD2S1210 has a programmable threshold for the degradation of signal
> (DOS) mismatch fault. This fault is triggered when the difference in
> amplitude between the sine and cosine inputs exceeds the threshold.
>
> The DOS reset min/max registers on the chip provide initial values
> for internal tracking of the min/max of the monitor signal after the
> fault register is cleared.
>
> This patch converts the custom device DOS reset min/max threshold
> attributes custom event attributes on the monitor signal channel.
>
> The attributes now use millivolts instead of the raw register value in
> accordance with the IIO ABI.
>
> Signed-off-by: David Lechner <[email protected]>
Applied.
On Thu, 5 Oct 2023 19:50:30 -0500
David Lechner <[email protected]> wrote:
> When reading the position and velocity on the AD2S1210, there is also a
> 3rd byte following the two data bytes that contains the fault flag bits.
> This patch adds support for reading this byte and generating events when
> faults occur.
>
> The faults are mapped to various channels and event type in order to
> have a unique event for each fault.
>
> Signed-off-by: David Lechner <[email protected]>
Applied.
On Thu, 5 Oct 2023 19:50:31 -0500
David Lechner <[email protected]> wrote:
> The ad2s1210 driver shoe-horns the register and fault support into IIO
> events. The mapping between the registers/faults and the events is not
> obvious. To save users from having to read the entire driver to figure
> out how to use it, add a summary of the register/fault support to the
> top of the file.
>
> Signed-off-by: David Lechner <[email protected]>
This is indeed useful to have.
Applied.
Thanks,
Jonathan
> ---
>
> v4 changes: New patch in v4.
>
> drivers/staging/iio/resolver/ad2s1210.c | 40 +++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index d9d51bbbade8..51490fea1647 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -4,7 +4,47 @@
> *
> * Copyright (c) 2010-2010 Analog Devices Inc.
> * Copyright (c) 2023 BayLibre, SAS
> + *
> + * Device register to IIO ABI mapping:
> + *
> + * Register | Addr | IIO ABI (sysfs)
> + * ----------------------------|------|-------------------------------------------
> + * DOS Overrange Threshold | 0x89 | events/in_altvoltage0_thresh_rising_value
> + * DOS Mismatch Threshold | 0x8A | events/in_altvoltage0_mag_rising_value
> + * DOS Reset Maximum Threshold | 0x8B | events/in_altvoltage0_mag_rising_reset_max
> + * DOS Reset Minimum Threshold | 0x8C | events/in_altvoltage0_mag_rising_reset_min
> + * LOT High Threshold | 0x8D | events/in_angl1_thresh_rising_value
> + * LOT Low Threshold [1] | 0x8E | events/in_angl1_thresh_rising_hysteresis
> + * Excitation Frequency | 0x91 | out_altvoltage0_frequency
> + * Control | 0x92 | *as bit fields*
> + * Phase lock range | D5 | events/in_phase0_mag_rising_value
> + * Hysteresis | D4 | in_angl0_hysteresis
> + * Encoder resolution | D3:2 | *not implemented*
> + * Resolution | D1:0 | *device tree: assigned-resolution-bits*
> + * Soft Reset | 0xF0 | [2]
> + * Fault | 0xFF | *not implemented*
> + *
> + * [1]: The value written to the LOT low register is high value minus the
> + * hysteresis.
> + * [2]: Soft reset is performed when `out_altvoltage0_frequency` is written.
> + *
> + * Fault to event mapping:
> + *
> + * Fault | | Channel | Type | Direction
> + * ----------------------------------------|----|---------------------------------
> + * Sine/cosine inputs clipped [3] | D7 | altvoltage1 | mag | either
> + * Sine/cosine inputs below LOS | D6 | altvoltage0 | thresh | falling
> + * Sine/cosine inputs exceed DOS overrange | D5 | altvoltage0 | thresh | rising
> + * Sine/cosine inputs exceed DOS mismatch | D4 | altvoltage0 | mag | rising
> + * Tracking error exceeds LOT | D3 | angl1 | thresh | rising
> + * Velocity exceeds maximum tracking rate | D2 | anglvel0 | mag | rising
> + * Phase error exceeds phase lock range | D1 | phase0 | mag | rising
> + * Configuration parity error | D0 | *writes to kernel log*
> + *
> + * [3]: The chip does not differentiate between fault on sine vs. cosine so
> + * there will also be an event on the altvoltage2 channel.
> */
> +
> #include <linux/bitfield.h>
> #include <linux/bits.h>
> #include <linux/clk.h>
>