Re: [PATCH v6 0/1] iio/scmi: Add reading "raw" attribute.
From: Andriy Tryshnivskyy
Date: Sun Oct 24 2021 - 04:49:31 EST
On 24.10.21 11:33, Andy Shevchenko wrote:
CAUTION: This email originated from outside of the organization.
Do not click links or open attachments unless you recognize the sender
and know the content is safe.
On Tuesday, October 19, 2021, Andriy Tryshnivskyy
<andriy.tryshnivskyy@xxxxxxxxxxxxxxx> wrote:
Add IIO_CHAN_INFO_RAW to the mask and implement corresponding
reading "raw" attribute in scmi_iio_read_raw.
Introduce new type IIO_VAL_INT_64 to read 64-bit value
for "raw" attribute.
Changes comparing v5 -> v6:
* revert v5 changes since with scmi_iio_read_raw() the channel
can't be used by in kernel users (iio-hwmon)
* returned to v3 with direct mode
* introduce new type IIO_VAL_INT_64 to read 64-bit value
Changes comparing v4 -> v5:
* call iio_device_release_direct_mode() on error
* code cleanup, fix typo
Changes comparing v3 -> v4:
* do not use scmi_iio_get_raw() for reading raw attribute due to
32-bit
return value limitation (actually I reverted the previous v3)
* introduce scmi_iio_read_raw to scmi_iio_ext_info[] which can return
64-bit value
* enabling/disabling and reading raw attribute is done in direct mode
Changes comparing v2 -> v3:
* adaptation for changes in structure scmi_iio_priv (no member
named 'handle')
Changes comparing v0 -> v2:
* added an error return when the error happened during config_set
* removed redundant cast for "readings"
* added check if raw value fits 32 bits
Signed-off-by: Andriy Tryshnivskyy
<andriy.tryshnivskyy@xxxxxxxxxxxxxxx
<mailto:andriy.tryshnivskyy@xxxxxxxxxxxxxxx>>
---
drivers/iio/common/scmi_sensors/scmi_iio.c | 57
+++++++++++++++++++++-
drivers/iio/industrialio-core.c | 3 ++
include/linux/iio/types.h | 1 +
Usually we separate changes for core and for individual drivers for
the sake of bisecting and, if requested, reverting.
Noted. Thanks!
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c
b/drivers/iio/common/scmi_sensors/scmi_iio.c
index 7cf2bf282cef..2c1aec0fd5ff 100644
--- a/drivers/iio/common/scmi_sensors/scmi_iio.c
+++ b/drivers/iio/common/scmi_sensors/scmi_iio.c
@@ -279,6 +279,52 @@ static int scmi_iio_get_odr_val(struct
iio_dev *iio_dev, int *val, int *val2)
return 0;
}
+static int scmi_iio_read_channel_data(struct iio_dev *iio_dev,
+ struct iio_chan_spec const *ch, int
*val, int *val2)
+{
+ struct scmi_iio_priv *sensor = iio_priv(iio_dev);
+ u32 sensor_config;
+ struct scmi_sensor_reading readings[SCMI_IIO_NUM_OF_AXIS];
+ int err;
+
+ sensor_config = FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK,
+ SCMI_SENS_CFG_SENSOR_ENABLE);
+ err = sensor->sensor_ops->config_set(
+ sensor->ph, sensor->sensor_info->id, sensor_config);
+ if (err) {
+ dev_err(&iio_dev->dev,
+ "Error in enabling sensor %s err %d",
+ sensor->sensor_info->name, err);
+ return err;
+ }
+
+ err = sensor->sensor_ops->reading_get_timestamped(
+ sensor->ph, sensor->sensor_info->id,
+ sensor->sensor_info->num_axis, readings);
+ if (err) {
+ dev_err(&iio_dev->dev,
+ "Error in reading raw attribute for sensor
%s err %d",
+ sensor->sensor_info->name, err);
+ return err;
+ }
+
+ sensor_config = FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK,
+ SCMI_SENS_CFG_SENSOR_DISABLE);
+ err = sensor->sensor_ops->config_set(
+ sensor->ph, sensor->sensor_info->id, sensor_config);
+ if (err) {
+ dev_err(&iio_dev->dev,
+ "Error in disabling sensor %s err %d",
+ sensor->sensor_info->name, err);
+ return err;
+ }
+ *val = (u32)readings[ch->scan_index].value;
+ *val2 = (u32)(readings[ch->scan_index].value >> 32);
We have upper_32_bits() and similar for low part.
I will use upper_32_bits() and lower_32_bits() then.
Thank you for review!
+
+ return IIO_VAL_INT_64;
+}
+
static int scmi_iio_read_raw(struct iio_dev *iio_dev,
struct iio_chan_spec const *ch, int *val,
int *val2, long mask)
@@ -300,6 +346,14 @@ static int scmi_iio_read_raw(struct iio_dev
*iio_dev,
case IIO_CHAN_INFO_SAMP_FREQ:
ret = scmi_iio_get_odr_val(iio_dev, val, val2);
return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(iio_dev);
+ if (ret)
+ return ret;
+
+ ret = scmi_iio_read_channel_data(iio_dev, ch, val,
val2);
+ iio_device_release_direct_mode(iio_dev);
+ return ret;
default:
return -EINVAL;
}
@@ -381,7 +435,8 @@ static void scmi_iio_set_data_channel(struct
iio_chan_spec *iio_chan,
iio_chan->type = type;
iio_chan->modified = 1;
iio_chan->channel2 = mod;
- iio_chan->info_mask_separate = BIT(IIO_CHAN_INFO_SCALE);
+ iio_chan->info_mask_separate =
+ BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_RAW);
iio_chan->info_mask_shared_by_type =
BIT(IIO_CHAN_INFO_SAMP_FREQ);
iio_chan->info_mask_shared_by_type_available =
BIT(IIO_CHAN_INFO_SAMP_FREQ);
diff --git a/drivers/iio/industrialio-core.c
b/drivers/iio/industrialio-core.c
index 6d2175eb7af2..49e42d04ea16 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -702,6 +702,9 @@ static ssize_t __iio_format_value(char *buf,
size_t offset, unsigned int type,
}
case IIO_VAL_CHAR:
return sysfs_emit_at(buf, offset, "%c",
(char)vals[0]);
+ case IIO_VAL_INT_64:
+ tmp2 = (s64)((((u64)vals[1]) << 32) | (u32)vals[0]);
+ return sysfs_emit_at(buf, offset, "%lld", tmp2);
default:
return 0;
}
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 84b3f8175cc6..e148fe11a3dc 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -24,6 +24,7 @@ enum iio_event_info {
#define IIO_VAL_INT_PLUS_NANO 3
#define IIO_VAL_INT_PLUS_MICRO_DB 4
#define IIO_VAL_INT_MULTIPLE 5
+#define IIO_VAL_INT_64 6
#define IIO_VAL_FRACTIONAL 10
#define IIO_VAL_FRACTIONAL_LOG2 11
#define IIO_VAL_CHAR 12
--
2.17.1
--
With Best Regards,
Andy Shevchenko
Thanks,
Andriy.