Re: [PATCH v5 1/1] iio/scmi: Add reading "raw" attribute.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 17.10.21 14:51, Jonathan Cameron 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 Fri,  8 Oct 2021 21:28:26 +0300
Andriy Tryshnivskyy <andriy.tryshnivskyy@xxxxxxxxxxxxxxx> wrote:

Add scmi_iio_get_raw() to read "raw" attribute.

Signed-off-by: Andriy Tryshnivskyy <andriy.tryshnivskyy@xxxxxxxxxxxxxxx>
---
For a single patch series, it is better to put a change log in the patch (here)
Whilst I can see why you would use this approach rather than the read_raw callback
there are significant disadvantages in doing so.  The channel can't be used
by in kernel users such as iio-hwmon.   That may cause you more trouble than
it is worth in the long run.

Note that you could also define a new IIO_VAL type to still use the two
(possibly) 32 bit values and return a 64 bit value. That way, with appropriate
additions in the consumer drivers the channel could still be used.

IIO_VAL_INT_64 perhaps with val as the lower 32 bits and val2 as the upper
with appropriate care around the sign.

Hi Jonathan,

In the past I also was thinking about introducing a new type to return 64 bits value - IIO_VAL_INT_LONG.
However since it requires changes in

    include/linux/iio/types.h
    drivers/iio/industrialio-core.c

I thought it will be harder to get approval for this approach.
But now it seems there is no other way. So I will prepare a new patch version soon which introduces IIO_VAL_INT_LONG.

Thank you for your review!


  drivers/iio/common/scmi_sensors/scmi_iio.c | 61 ++++++++++++++++++++++
  1 file changed, 61 insertions(+)

diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c b/drivers/iio/common/scmi_sensors/scmi_iio.c
index 7cf2bf282cef..691cbbd61e3a 100644
--- a/drivers/iio/common/scmi_sensors/scmi_iio.c
+++ b/drivers/iio/common/scmi_sensors/scmi_iio.c
@@ -311,6 +311,62 @@ static const struct iio_info scmi_iio_info = {
       .write_raw = scmi_iio_write_raw,
  };

+static ssize_t scmi_iio_get_raw(struct iio_dev *iio_dev, uintptr_t private,
+                             const struct iio_chan_spec *chan, char *buf)
+{
+     struct scmi_iio_priv *sensor = iio_priv(iio_dev);
+     int err;
+     u32 sensor_config;
+     struct scmi_sensor_reading readings[SCMI_IIO_NUM_OF_AXIS];
+     int len = 0;
+
+     err = iio_device_claim_direct_mode(iio_dev);
+     if (err) {
+             dev_err(&iio_dev->dev,
+                     "Error in claiming direct mode for sensor %s err %d",
+                     sensor->sensor_info->name, err);
It's not an error, it just means the device is busy, so at most dev_info()
or just rely on userspace correctly interpreting EBUSY.

Agree. Will use dev_info().
Thank you!

+             goto err_release;
+     }
+
+     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);
+             goto err_release;
+     }
+
+     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);
+             goto err_release;
+     }
+
+     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);
+             goto err_release;
+     }
+
+     len = scnprintf(buf, PAGE_SIZE, "%lld\n",
+                     readings[chan->scan_index].value);
+
+err_release:
+     iio_device_release_direct_mode(iio_dev);
+
+     return len;
+}
+
  static ssize_t scmi_iio_get_raw_available(struct iio_dev *iio_dev,
                                         uintptr_t private,
                                         const struct iio_chan_spec *chan,
@@ -355,6 +411,11 @@ static ssize_t scmi_iio_get_raw_available(struct iio_dev *iio_dev,
  }

  static const struct iio_chan_spec_ext_info scmi_iio_ext_info[] = {
+     {
+             .name = "raw",
+             .read = scmi_iio_get_raw,
+             .shared = IIO_SEPARATE,
+     },
       {
               .name = "raw_available",
               .read = scmi_iio_get_raw_available,




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux