| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 24 Oct 2021 11:49:19 +0300
From: Andriy Tryshnivskyy <andriy.tryshnivskyy@...nsynergy.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: "jbhayana@...gle.com" <jbhayana@...gle.com>,
"jic23@...nel.org" <jic23@...nel.org>,
"lars@...afoo.de" <lars@...afoo.de>,
"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Vasyl.Vavrychuk@...nsynergy.com" <Vasyl.Vavrychuk@...nsynergy.com>
Subject: Re: [PATCH v6 0/1] iio/scmi: Add reading "raw" attribute.
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@...nsynergy.com> 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@...nsynergy.com
> <mailto:andriy.tryshnivskyy@...nsynergy.com>>
> ---
> 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.
Powered by blists - more mailing lists