Introduce IIO_VAL_INT_64 to read 64-bit value for
channel attribute. Val is used as lower 32 bits.
Add IIO_CHAN_INFO_RAW to the mask and implement corresponding
reading "raw" attribute in scmi_iio_read_raw.
Patches are based on v5.14.
Any comments are very welcome.
Thanks,
Andriy.
Andriy Tryshnivskyy (2):
iio: core: Introduce IIO_VAL_INT_64.
iio/scmi: Add reading "raw" attribute.
drivers/iio/common/scmi_sensors/scmi_iio.c | 57 +++++++++++++++++++++-
drivers/iio/industrialio-core.c | 3 ++
include/linux/iio/types.h | 1 +
3 files changed, 60 insertions(+), 1 deletion(-)
base-commit: 7d2a07b769330c34b4deabeed939325c77a7ec2f
--
2.17.1
Introduce IIO_VAL_INT_64 to read 64-bit value for
channel attribute. Val is used as lower 32 bits.
Signed-off-by: Andriy Tryshnivskyy <[email protected]>
---
drivers/iio/industrialio-core.c | 3 +++
include/linux/iio/types.h | 1 +
2 files changed, 4 insertions(+)
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..bb6578a5ee28 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 /* 64-bit data, val is lower 32 bits) */
#define IIO_VAL_FRACTIONAL 10
#define IIO_VAL_FRACTIONAL_LOG2 11
#define IIO_VAL_CHAR 12
--
2.17.1
Add IIO_CHAN_INFO_RAW to the mask and implement corresponding
reading "raw" attribute in scmi_iio_read_raw.
Signed-off-by: Andriy Tryshnivskyy <[email protected]>
---
Changes comparing v6 -> v7:
* split into two patches: one for changes in core functionality,
another - for changes in the driver
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
drivers/iio/common/scmi_sensors/scmi_iio.c | 57 +++++++++++++++++++++-
1 file changed, 56 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..d538bf3ab1ef 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 = lower_32_bits(readings[ch->scan_index].value);
+ *val2 = upper_32_bits(readings[ch->scan_index].value);
+
+ 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);
--
2.17.1
On Sun, 24 Oct 2021 12:16:26 +0300
Andriy Tryshnivskyy <[email protected]> wrote:
> Introduce IIO_VAL_INT_64 to read 64-bit value for
> channel attribute. Val is used as lower 32 bits.
>
> Signed-off-by: Andriy Tryshnivskyy <[email protected]>
> ---
> drivers/iio/industrialio-core.c | 3 +++
> include/linux/iio/types.h | 1 +
> 2 files changed, 4 insertions(+)
>
> 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..bb6578a5ee28 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 /* 64-bit data, val is lower 32 bits) */
I'm guessing the closing bracket is left over of some editing?
Otherwise fine and I can tidy that up whilst applying.
Note that this is almost certainly too late for this cycle (we are
about a week away from merge window subject to whatever Linus says
for rc7 and new stuff needs some time to soak in next), but I'll
plan to get it queued up early in the next one.
> #define IIO_VAL_FRACTIONAL 10
> #define IIO_VAL_FRACTIONAL_LOG2 11
> #define IIO_VAL_CHAR 12
On 24.10.21 19:10, 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 Sun, 24 Oct 2021 12:16:26 +0300
> Andriy Tryshnivskyy <[email protected]> wrote:
>
>> Introduce IIO_VAL_INT_64 to read 64-bit value for
>> channel attribute. Val is used as lower 32 bits.
>>
>> Signed-off-by: Andriy Tryshnivskyy <[email protected]>
>> ---
>> drivers/iio/industrialio-core.c | 3 +++
>> include/linux/iio/types.h | 1 +
>> 2 files changed, 4 insertions(+)
>>
>> 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..bb6578a5ee28 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 /* 64-bit data, val is lower 32 bits) */
> I'm guessing the closing bracket is left over of some editing?
>
> Otherwise fine and I can tidy that up whilst applying.
Yes, it's a typo. Please remove it while applying. Thanks!
> Note that this is almost certainly too late for this cycle (we are
> about a week away from merge window subject to whatever Linus says
> for rc7 and new stuff needs some time to soak in next), but I'll
> plan to get it queued up early in the next one.
>
Noted. Thanks a lot!
>> #define IIO_VAL_FRACTIONAL 10
>> #define IIO_VAL_FRACTIONAL_LOG2 11
>> #define IIO_VAL_CHAR 12
Best regards,
Andriy.
On Sun, 24 Oct 2021 12:16:27 +0300
Andriy Tryshnivskyy <[email protected]> wrote:
> Add IIO_CHAN_INFO_RAW to the mask and implement corresponding
> reading "raw" attribute in scmi_iio_read_raw.
>
> Signed-off-by: Andriy Tryshnivskyy <[email protected]>
@Jyoti,
looking for your Ack / or Reviewed-by tag for this one.
Thanks,
Jonathan
> ---
> Changes comparing v6 -> v7:
> * split into two patches: one for changes in core functionality,
> another - for changes in the driver
>
> 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
>
> drivers/iio/common/scmi_sensors/scmi_iio.c | 57 +++++++++++++++++++++-
> 1 file changed, 56 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..d538bf3ab1ef 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 = lower_32_bits(readings[ch->scan_index].value);
> + *val2 = upper_32_bits(readings[ch->scan_index].value);
> +
> + 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);
Acked-by: Jyoti Bhayana <[email protected]>
On Thu, Oct 28, 2021 at 7:04 AM Jonathan Cameron <[email protected]> wrote:
>
> On Sun, 24 Oct 2021 12:16:27 +0300
> Andriy Tryshnivskyy <[email protected]> wrote:
>
> > Add IIO_CHAN_INFO_RAW to the mask and implement corresponding
> > reading "raw" attribute in scmi_iio_read_raw.
> >
> > Signed-off-by: Andriy Tryshnivskyy <[email protected]>
>
> @Jyoti,
>
> looking for your Ack / or Reviewed-by tag for this one.
>
> Thanks,
>
> Jonathan
>
> > ---
> > Changes comparing v6 -> v7:
> > * split into two patches: one for changes in core functionality,
> > another - for changes in the driver
> >
> > 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
> >
> > drivers/iio/common/scmi_sensors/scmi_iio.c | 57 +++++++++++++++++++++-
> > 1 file changed, 56 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..d538bf3ab1ef 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 = lower_32_bits(readings[ch->scan_index].value);
> > + *val2 = upper_32_bits(readings[ch->scan_index].value);
> > +
> > + 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);
>
On Sun, 24 Oct 2021 19:58:52 +0300
Andriy Tryshnivskyy <[email protected]> wrote:
> On 24.10.21 19:10, 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.
Ah. One thing I forgot. Value formatting is the only bit of IIO where
we have self tests.
Would you mind writing some test cases in
drivers/iio/tests/iio-test-format.c ?
I'll pick this up in the meantime but definitely want to make
sure we don't forget the tests!
Jonathan
> >
> >
> > On Sun, 24 Oct 2021 12:16:26 +0300
> > Andriy Tryshnivskyy <[email protected]> wrote:
> >
> >> Introduce IIO_VAL_INT_64 to read 64-bit value for
> >> channel attribute. Val is used as lower 32 bits.
> >>
> >> Signed-off-by: Andriy Tryshnivskyy <[email protected]>
> >> ---
> >> drivers/iio/industrialio-core.c | 3 +++
> >> include/linux/iio/types.h | 1 +
> >> 2 files changed, 4 insertions(+)
> >>
> >> 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..bb6578a5ee28 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 /* 64-bit data, val is lower 32 bits) */
> > I'm guessing the closing bracket is left over of some editing?
> >
> > Otherwise fine and I can tidy that up whilst applying.
>
> Yes, it's a typo. Please remove it while applying. Thanks!
>
> > Note that this is almost certainly too late for this cycle (we are
> > about a week away from merge window subject to whatever Linus says
> > for rc7 and new stuff needs some time to soak in next), but I'll
> > plan to get it queued up early in the next one.
> >
> Noted. Thanks a lot!
>
> >> #define IIO_VAL_FRACTIONAL 10
> >> #define IIO_VAL_FRACTIONAL_LOG2 11
> >> #define IIO_VAL_CHAR 12
>
> Best regards,
> Andriy.
>
>
On Sun, 24 Oct 2021 12:16:25 +0300
Andriy Tryshnivskyy <[email protected]> wrote:
> Introduce IIO_VAL_INT_64 to read 64-bit value for
> channel attribute. Val is used as lower 32 bits.
> Add IIO_CHAN_INFO_RAW to the mask and implement corresponding
> reading "raw" attribute in scmi_iio_read_raw.
>
> Patches are based on v5.14.
>
> Any comments are very welcome.
>
> Thanks,
> Andriy.
>
> Andriy Tryshnivskyy (2):
> iio: core: Introduce IIO_VAL_INT_64.
> iio/scmi: Add reading "raw" attribute.
>
> drivers/iio/common/scmi_sensors/scmi_iio.c | 57 +++++++++++++++++++++-
> drivers/iio/industrialio-core.c | 3 ++
> include/linux/iio/types.h | 1 +
> 3 files changed, 60 insertions(+), 1 deletion(-)
>
>
> base-commit: 7d2a07b769330c34b4deabeed939325c77a7ec2f
Applied to the togreg branch of iio.git which is pushed out as testing for
0-day to poke at it and see what we missed.
Note this is next cycle material now given merge window is about to open.
As such I'll be rebasing my tree after rc1.
Thanks,
Jonathan
On 30.10.21 17:47, 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 Sun, 24 Oct 2021 19:58:52 +0300
> Andriy Tryshnivskyy <[email protected]> wrote:
>
>> On 24.10.21 19:10, 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.
> Ah. One thing I forgot. Value formatting is the only bit of IIO where
> we have self tests.
>
> Would you mind writing some test cases in
> drivers/iio/tests/iio-test-format.c ?
>
> I'll pick this up in the meantime but definitely want to make
> sure we don't forget the tests!
>
> Jonathan
Sure. I will add some tests.
Regards,
Andriy.
>
>>>
>>> On Sun, 24 Oct 2021 12:16:26 +0300
>>> Andriy Tryshnivskyy <[email protected]> wrote:
>>>
>>>> Introduce IIO_VAL_INT_64 to read 64-bit value for
>>>> channel attribute. Val is used as lower 32 bits.
>>>>
>>>> Signed-off-by: Andriy Tryshnivskyy <[email protected]>
>>>> ---
>>>> drivers/iio/industrialio-core.c | 3 +++
>>>> include/linux/iio/types.h | 1 +
>>>> 2 files changed, 4 insertions(+)
>>>>
>>>> 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..bb6578a5ee28 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 /* 64-bit data, val is lower 32 bits) */
>>> I'm guessing the closing bracket is left over of some editing?
>>>
>>> Otherwise fine and I can tidy that up whilst applying.
>> Yes, it's a typo. Please remove it while applying. Thanks!
>>
>>> Note that this is almost certainly too late for this cycle (we are
>>> about a week away from merge window subject to whatever Linus says
>>> for rc7 and new stuff needs some time to soak in next), but I'll
>>> plan to get it queued up early in the next one.
>>>
>> Noted. Thanks a lot!
>>
>>>> #define IIO_VAL_FRACTIONAL 10
>>>> #define IIO_VAL_FRACTIONAL_LOG2 11
>>>> #define IIO_VAL_CHAR 12
>> Best regards,
>> Andriy.
>>
>>
>
From 6e6a3661785584c6cc88370f78578810e67cb0e5 Mon Sep 17 00:00:00 2001
From: Andriy Tryshnivskyy <[email protected]>
Date: Mon, 1 Nov 2021 15:44:31 +0200
Subject: [PATCH] iio: test: Add test for IIO_VAL_INT_64.
Signed-off-by: Andriy Tryshnivskyy <[email protected]>
---
drivers/iio/test/iio-test-format.c | 51 ++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/drivers/iio/test/iio-test-format.c
b/drivers/iio/test/iio-test-format.c
index f1e951eddb43..f07945c2cf28 100644
--- a/drivers/iio/test/iio-test-format.c
+++ b/drivers/iio/test/iio-test-format.c
@@ -182,12 +182,63 @@ static void
iio_test_iio_format_value_multiple(struct kunit *test)
IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1 -2 3 -4 5 \n");
}
+static void iio_test_iio_format_value_integer_64(struct kunit *test)
+{
+ char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
+ s64 value;
+ int values[2];
+ int ret;
+
+ value = 24;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "24\n");
+
+ value = -24;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-24\n");
+
+ value = 0;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0\n");
+
+ value = 4294967295;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "4294967295\n");
+
+ value = -4294967295;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-4294967295\n");
+
+ value = LLONG_MAX;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9223372036854775807\n");
+
+ value = LLONG_MIN;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-9223372036854775808\n");
+}
+
static struct kunit_case iio_format_test_cases[] = {
KUNIT_CASE(iio_test_iio_format_value_integer),
KUNIT_CASE(iio_test_iio_format_value_fixedpoint),
KUNIT_CASE(iio_test_iio_format_value_fractional),
KUNIT_CASE(iio_test_iio_format_value_fractional_log2),
KUNIT_CASE(iio_test_iio_format_value_multiple),
+ KUNIT_CASE(iio_test_iio_format_value_integer_64),
{}
};
--
2.17.1
On 01.11.21 09:28, Andriy Tryshnivskyy wrote:
>
> On 30.10.21 17:47, 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 Sun, 24 Oct 2021 19:58:52 +0300
>> Andriy Tryshnivskyy <[email protected]> wrote:
>>
>>> On 24.10.21 19:10, 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.
>> Ah. One thing I forgot. Value formatting is the only bit of IIO where
>> we have self tests.
>>
>> Would you mind writing some test cases in
>> drivers/iio/tests/iio-test-format.c ?
>>
>> I'll pick this up in the meantime but definitely want to make
>> sure we don't forget the tests!
>>
>> Jonathan
>
> Sure. I will add some tests.
>
> Regards,
> Andriy.
>
>
>>
>>>>
>>>> On Sun, 24 Oct 2021 12:16:26 +0300
>>>> Andriy Tryshnivskyy <[email protected]> wrote:
>>>>
>>>>> Introduce IIO_VAL_INT_64 to read 64-bit value for
>>>>> channel attribute. Val is used as lower 32 bits.
>>>>>
>>>>> Signed-off-by: Andriy Tryshnivskyy
>>>>> <[email protected]>
>>>>> ---
>>>>> drivers/iio/industrialio-core.c | 3 +++
>>>>> include/linux/iio/types.h | 1 +
>>>>> 2 files changed, 4 insertions(+)
>>>>>
>>>>> 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..bb6578a5ee28 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 /* 64-bit data, val is lower 32 bits) */
>>>> I'm guessing the closing bracket is left over of some editing?
>>>>
>>>> Otherwise fine and I can tidy that up whilst applying.
>>> Yes, it's a typo. Please remove it while applying. Thanks!
>>>
>>>> Note that this is almost certainly too late for this cycle (we are
>>>> about a week away from merge window subject to whatever Linus says
>>>> for rc7 and new stuff needs some time to soak in next), but I'll
>>>> plan to get it queued up early in the next one.
>>>>
>>> Noted. Thanks a lot!
>>>
>>>>> #define IIO_VAL_FRACTIONAL 10
>>>>> #define IIO_VAL_FRACTIONAL_LOG2 11
>>>>> #define IIO_VAL_CHAR 12
>>> Best regards,
>>> Andriy.
>>>
>>>
>>
On Mon, Nov 1, 2021 at 3:54 PM Andriy Tryshnivskyy
<[email protected]> wrote:
>
> From 6e6a3661785584c6cc88370f78578810e67cb0e5 Mon Sep 17 00:00:00 2001
> From: Andriy Tryshnivskyy <[email protected]>
> Date: Mon, 1 Nov 2021 15:44:31 +0200
> Subject: [PATCH] iio: test: Add test for IIO_VAL_INT_64.
Something went wrong. Please, use the `git send-email ...` tool for the patches.
--
With Best Regards,
Andy Shevchenko
Signed-off-by: Andriy Tryshnivskyy <[email protected]>
---
drivers/iio/test/iio-test-format.c | 51 ++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/drivers/iio/test/iio-test-format.c b/drivers/iio/test/iio-test-format.c
index f1e951eddb43..f07945c2cf28 100644
--- a/drivers/iio/test/iio-test-format.c
+++ b/drivers/iio/test/iio-test-format.c
@@ -182,12 +182,63 @@ static void iio_test_iio_format_value_multiple(struct kunit *test)
IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1 -2 3 -4 5 \n");
}
+static void iio_test_iio_format_value_integer_64(struct kunit *test)
+{
+ char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
+ s64 value;
+ int values[2];
+ int ret;
+
+ value = 24;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "24\n");
+
+ value = -24;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-24\n");
+
+ value = 0;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0\n");
+
+ value = 4294967295;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "4294967295\n");
+
+ value = -4294967295;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-4294967295\n");
+
+ value = LLONG_MAX;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9223372036854775807\n");
+
+ value = LLONG_MIN;
+ values[0] = lower_32_bits(value);
+ values[1] = upper_32_bits(value);
+ ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
+ IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-9223372036854775808\n");
+}
+
static struct kunit_case iio_format_test_cases[] = {
KUNIT_CASE(iio_test_iio_format_value_integer),
KUNIT_CASE(iio_test_iio_format_value_fixedpoint),
KUNIT_CASE(iio_test_iio_format_value_fractional),
KUNIT_CASE(iio_test_iio_format_value_fractional_log2),
KUNIT_CASE(iio_test_iio_format_value_multiple),
+ KUNIT_CASE(iio_test_iio_format_value_integer_64),
{}
};
--
2.17.1
On Tue, Nov 2, 2021 at 9:33 AM Andriy Tryshnivskyy
<[email protected]> wrote:
>
Now it's good with format, but you have missed the commit message.
> Signed-off-by: Andriy Tryshnivskyy <[email protected]>
...
> +static void iio_test_iio_format_value_integer_64(struct kunit *test)
> +{
> + char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
Shouldn't this be checked against NULL?
> + s64 value;
> + int values[2];
> + int ret;
Reversed xmas tree ordering?
> + value = 24;
> + values[0] = lower_32_bits(value);
> + values[1] = upper_32_bits(value);
> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
ARRAY_SIZE()?
> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "24\n");
> +
> + value = -24;
> + values[0] = lower_32_bits(value);
> + values[1] = upper_32_bits(value);
> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-24\n");
> +
> + value = 0;
> + values[0] = lower_32_bits(value);
> + values[1] = upper_32_bits(value);
> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0\n");
> +
> + value = 4294967295;
Is this UINT_MAX?
> + values[0] = lower_32_bits(value);
> + values[1] = upper_32_bits(value);
> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "4294967295\n");
> + value = -4294967295;
Is this -UINT_MAX?
> + values[0] = lower_32_bits(value);
> + values[1] = upper_32_bits(value);
> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-4294967295\n");
> +
> + value = LLONG_MAX;
> + values[0] = lower_32_bits(value);
> + values[1] = upper_32_bits(value);
> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9223372036854775807\n");
> +
> + value = LLONG_MIN;
> + values[0] = lower_32_bits(value);
> + values[1] = upper_32_bits(value);
> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-9223372036854775808\n");
> +}
--
With Best Regards,
Andy Shevchenko
On 11/5/21 9:45 AM, Andriy Tryshnivskyy wrote:
> On 02.11.21 10:11, 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 Tue, Nov 2, 2021 at 9:33 AM Andriy Tryshnivskyy
>> <[email protected]> wrote:
>> Now it's good with format, but you have missed the commit message.
>
> Actually commit massage contains a header only (no body message), but
> I can add body message too.
> Thanks!
>
>>
>>> Signed-off-by: Andriy Tryshnivskyy
>>> <[email protected]>
>> ...
>>
>>> +static void iio_test_iio_format_value_integer_64(struct kunit *test)
>>> +{
>>> + char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>> Shouldn't this be checked against NULL?
>
> Good question. Truly speaking I've made new test similar to other. And
> no other tests has a check for NULL.
>
The other tests not having it is my fault. There should be a
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf) under the allocation.
On 05.11.21 10:50, Lars-Peter Clausen 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 11/5/21 9:45 AM, Andriy Tryshnivskyy wrote:
>> On 02.11.21 10:11, 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 Tue, Nov 2, 2021 at 9:33 AM Andriy Tryshnivskyy
>>> <[email protected]> wrote:
>>> Now it's good with format, but you have missed the commit message.
>>
>> Actually commit massage contains a header only (no body message), but
>> I can add body message too.
>> Thanks!
>>
>>>
>>>> Signed-off-by: Andriy Tryshnivskyy
>>>> <[email protected]>
>>> ...
>>>
>>>> +static void iio_test_iio_format_value_integer_64(struct kunit *test)
>>>> +{
>>>> + char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>>> Shouldn't this be checked against NULL?
>>
>> Good question. Truly speaking I've made new test similar to other. And
>> no other tests has a check for NULL.
>>
> The other tests not having it is my fault. There should be a
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf) under the allocation.
>
Understood. Then If you wouldn't mind I will add assert to other tests too.
On 02.11.21 10:11, 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 Tue, Nov 2, 2021 at 9:33 AM Andriy Tryshnivskyy
> <[email protected]> wrote:
> Now it's good with format, but you have missed the commit message.
Actually commit massage contains a header only (no body message), but I can add body message too.
Thanks!
>
>> Signed-off-by: Andriy Tryshnivskyy <[email protected]>
> ...
>
>> +static void iio_test_iio_format_value_integer_64(struct kunit *test)
>> +{
>> + char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
> Shouldn't this be checked against NULL?
Good question. Truly speaking I've made new test similar to other. And no other tests has a check for NULL.
>> + s64 value;
>> + int values[2];
>> + int ret;
> Reversed xmas tree ordering?
I will correct it. Thanks!
>> + value = 24;
>> + values[0] = lower_32_bits(value);
>> + values[1] = upper_32_bits(value);
>> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
> ARRAY_SIZE()?
Will use ARRAY_SIZE(). Thanks!
>> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "24\n");
>> +
>> + value = -24;
>> + values[0] = lower_32_bits(value);
>> + values[1] = upper_32_bits(value);
>> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
>> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-24\n");
>> +
>> + value = 0;
>> + values[0] = lower_32_bits(value);
>> + values[1] = upper_32_bits(value);
>> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
>> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0\n");
>> +
>> + value = 4294967295;
> Is this UINT_MAX?
Yes. It's UINT_MAX. I will use a constant. Thanks!
>> + values[0] = lower_32_bits(value);
>> + values[1] = upper_32_bits(value);
>> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
>> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "4294967295\n");
>> + value = -4294967295;
> Is this -UINT_MAX?
Yes. It's -UINT_MAX. I will use a constant. Thanks!
>> + values[0] = lower_32_bits(value);
>> + values[1] = upper_32_bits(value);
>> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
>> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-4294967295\n");
>> +
>> + value = LLONG_MAX;
>> + values[0] = lower_32_bits(value);
>> + values[1] = upper_32_bits(value);
>> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
>> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9223372036854775807\n");
>> +
>> + value = LLONG_MIN;
>> + values[0] = lower_32_bits(value);
>> + values[1] = upper_32_bits(value);
>> + ret = iio_format_value(buf, IIO_VAL_INT_64, 2, values);
>> + IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-9223372036854775808\n");
>> +}
> --
> With Best Regards,
> Andy Shevchenko
>
Thank you for review!
Regards,
Andriy.
On 11/5/21 9:55 AM, Andriy Tryshnivskyy wrote:
>
> On 05.11.21 10:50, Lars-Peter Clausen 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 11/5/21 9:45 AM, Andriy Tryshnivskyy wrote:
>>> On 02.11.21 10:11, 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 Tue, Nov 2, 2021 at 9:33 AM Andriy Tryshnivskyy
>>>> <[email protected]> wrote:
>>>> Now it's good with format, but you have missed the commit message.
>>>
>>> Actually commit massage contains a header only (no body message), but
>>> I can add body message too.
>>> Thanks!
>>>
>>>>
>>>>> Signed-off-by: Andriy Tryshnivskyy
>>>>> <[email protected]>
>>>> ...
>>>>
>>>>> +static void iio_test_iio_format_value_integer_64(struct kunit *test)
>>>>> +{
>>>>> + char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>>>> Shouldn't this be checked against NULL?
>>>
>>> Good question. Truly speaking I've made new test similar to other. And
>>> no other tests has a check for NULL.
>>>
>> The other tests not having it is my fault. There should be a
>> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf) under the allocation.
>>
> Understood. Then If you wouldn't mind I will add assert to other tests
> too.
Perfect, thanks!