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

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

 




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.



[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