Re: [PATCH v7 1/2] iio: core: Introduce IIO_VAL_INT_64.

From: Andriy Tryshnivskyy <andriy.tryshnivskyy@opensynergy.com>
To: Jonathan Cameron <jic23@kernel.org>, linux-kernel@vger.kernel.org
Cc: jbhayana@google.com, lars@metafoo.de, linux-iio@vger.kernel.org,
	Vasyl.Vavrychuk@opensynergy.com, andy.shevchenko@gmail.com
Subject: Re: [PATCH v7 1/2] iio: core: Introduce IIO_VAL_INT_64.
Date: Mon, 1 Nov 2021 15:54:37 +0200	[thread overview]
Message-ID: <4b53b3da-38b5-5c22-60bc-9f0731d6550c@opensynergy.com> (raw)
In-Reply-To: <7efacdde-0c3a-36e0-bfc7-ef30c14cbf13@opensynergy.com>

 From 6e6a3661785584c6cc88370f78578810e67cb0e5 Mon Sep 17 00:00:00 2001
From: Andriy Tryshnivskyy <andriy.tryshnivskyy@opensynergy.com>
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 <andriy.tryshnivskyy@opensynergy.com>
---
  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 <andriy.tryshnivskyy@opensynergy.com> 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 <andriy.tryshnivskyy@opensynergy.com> 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 
>>>>> <andriy.tryshnivskyy@opensynergy.com>
>>>>> ---
>>>>>    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.
>>>
>>>
>>
