[PATCH 0/3] add support for cherryview gpio controller driver

[PATCH 0/3] add support for cherryview gpio controller driver

[Hongzhan Chen]

1. move out of OF config conditional compilation so that non-OF platform
  call same API to remove rtdm gpio chip device.
2. Introduce helper to find gpiochip as referring to pair of 
   rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
3. Add Intel Cherryview pinctrl driver based on on 1 and 2.

I also did following tests with this patchset:
1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
   to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
   setting GPIO_RTIOC_TS
2. apply following patch, and rerun 1.

index f83d7689f..50afbd418 100644
--- a/testsuite/gpiobench/gpiobench.c
+++ b/testsuite/gpiobench/gpiobench.c
@@ -619,7 +619,7 @@ int main(int argc, char **argv)
                        goto out;
                }

-               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
+               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
                if (ret) {
                        printf("ioctl gpio port ts, failed\n");
                        goto out;

Hardware env:
1. Rock PI X V1.4.
2. GPIO loopback connection between GPIO 334 and 335.
 

Hongzhan Chen (3):
  drivers/gpio: core: move out of OF config conditional compilation
  drivers/gpio: core: Introduce helper to find gpiochip
  driver/gpio: Add Intel Cherryview pinctrl driver

 include/cobalt/kernel/rtdm/gpio.h     | 10 ++++--
 kernel/drivers/gpio/Kconfig           |  7 ++++
 kernel/drivers/gpio/Makefile          |  2 ++
 kernel/drivers/gpio/gpio-cherryview.c | 42 ++++++++++++++++++++++++
 kernel/drivers/gpio/gpio-core.c       | 46 +++++++++++++++++++++++++--
 5 files changed, 103 insertions(+), 4 deletions(-)
 create mode 100644 kernel/drivers/gpio/gpio-cherryview.c

-- 
2.17.1

[PATCH 1/3] drivers/gpio: core: move out of OF config conditional compilation

[Hongzhan Chen]

The api would be called by both non-OF and OF platform to remove rtdm
gpiochip device.

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
---
 include/cobalt/kernel/rtdm/gpio.h | 3 +--
 kernel/drivers/gpio/gpio-core.c   | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/cobalt/kernel/rtdm/gpio.h b/include/cobalt/kernel/rtdm/gpio.h
index 72cc3a035..c307b1572 100644
--- a/include/cobalt/kernel/rtdm/gpio.h
+++ b/include/cobalt/kernel/rtdm/gpio.h
@@ -70,9 +70,8 @@ int rtdm_gpiochip_scan_of(struct device_node *from,
 int rtdm_gpiochip_scan_array_of(struct device_node *from,
 				const char *compat[],
 				int nentries, int type);
+#endif
 
 void rtdm_gpiochip_remove_of(int type);
 
-#endif
-
 #endif /* !_COBALT_RTDM_GPIO_H */
diff --git a/kernel/drivers/gpio/gpio-core.c b/kernel/drivers/gpio/gpio-core.c
index 06a19b33a..ddda62b24 100644
--- a/kernel/drivers/gpio/gpio-core.c
+++ b/kernel/drivers/gpio/gpio-core.c
@@ -626,6 +626,8 @@ int rtdm_gpiochip_scan_array_of(struct device_node *from,
 }
 EXPORT_SYMBOL_GPL(rtdm_gpiochip_scan_array_of);
 
+#endif /* CONFIG_OF */
+
 void rtdm_gpiochip_remove_of(int type)
 {
 	struct rtdm_gpio_chip *rgc, *n;
@@ -644,5 +646,3 @@ void rtdm_gpiochip_remove_of(int type)
 	mutex_unlock(&chip_lock);
 }
 EXPORT_SYMBOL_GPL(rtdm_gpiochip_remove_of);
-
-#endif /* CONFIG_OF */
-- 
2.17.1

[PATCH 2/3] drivers/gpio: core: Introduce helper to find gpiochip

[Hongzhan Chen]

To find gpiochip for non-OF platforms like x86

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
---
 include/cobalt/kernel/rtdm/gpio.h |  7 ++++++
 kernel/drivers/gpio/gpio-core.c   | 42 +++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/cobalt/kernel/rtdm/gpio.h b/include/cobalt/kernel/rtdm/gpio.h
index c307b1572..07093e0be 100644
--- a/include/cobalt/kernel/rtdm/gpio.h
+++ b/include/cobalt/kernel/rtdm/gpio.h
@@ -62,6 +62,13 @@ int rtdm_gpiochip_add_by_name(struct rtdm_gpio_chip *rgc,
 int rtdm_gpiochip_post_event(struct rtdm_gpio_chip *rgc,
 			     unsigned int offset);
 
+int rtdm_gpiochip_find(struct device_node *from,
+			  const char *label, int type);
+
+int rtdm_gpiochip_array_find(struct device_node *from,
+				const char *compat[],
+				int nentries, int type);
+
 #ifdef CONFIG_OF
 
 int rtdm_gpiochip_scan_of(struct device_node *from,
diff --git a/kernel/drivers/gpio/gpio-core.c b/kernel/drivers/gpio/gpio-core.c
index ddda62b24..6bcf04ea3 100644
--- a/kernel/drivers/gpio/gpio-core.c
+++ b/kernel/drivers/gpio/gpio-core.c
@@ -528,6 +528,48 @@ int rtdm_gpiochip_add_by_name(struct rtdm_gpio_chip *rgc,
 }
 EXPORT_SYMBOL_GPL(rtdm_gpiochip_add_by_name);
 
+int rtdm_gpiochip_find(struct device_node *from, const char *label,
+			  int type)
+{
+	struct rtdm_gpio_chip *rgc;
+	struct gpio_chip *chip;
+	int ret = -ENODEV;
+
+	if (!rtdm_available())
+		return -ENOSYS;
+
+	chip = find_chip_by_name(label);
+	if (chip == NULL)
+		return ret;
+
+	ret = 0;
+	rgc = rtdm_gpiochip_alloc(chip, type);
+	if (IS_ERR(rgc))
+		ret = PTR_ERR(rgc);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rtdm_gpiochip_find);
+
+int rtdm_gpiochip_array_find(struct device_node *from,
+				const char *compat[],
+				int nentries, int type)
+{
+	int ret = -ENODEV, _ret, n;
+
+	for (n = 0; n < nentries; n++) {
+		_ret = rtdm_gpiochip_find(from, compat[n], type);
+		if (_ret) {
+			if (_ret != -ENODEV)
+				return _ret;
+		} else
+			ret = 0;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rtdm_gpiochip_array_find);
+
 #ifdef CONFIG_OF
 
 #include <linux/of_platform.h>
-- 
2.17.1

[PATCH 3/3] driver/gpio: Add Intel Cherryview pinctrl driver

[Hongzhan Chen]

Add support for Intel Cherryview pin controller driver

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
---
 kernel/drivers/gpio/Kconfig           |  7 +++++
 kernel/drivers/gpio/Makefile          |  2 ++
 kernel/drivers/gpio/gpio-cherryview.c | 42 +++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)
 create mode 100644 kernel/drivers/gpio/gpio-cherryview.c

diff --git a/kernel/drivers/gpio/Kconfig b/kernel/drivers/gpio/Kconfig
index 44f41688c..c257444f0 100644
--- a/kernel/drivers/gpio/Kconfig
+++ b/kernel/drivers/gpio/Kconfig
@@ -57,6 +57,13 @@ config XENO_DRIVERS_GPIO_OMAP
 	Enables support for the GPIO controller available from
 	OMAP family SOC.
 
+config XENO_DRIVERS_GPIO_CHERRYVIEW
+	depends on PINCTRL_CHERRYVIEW
+	tristate "Support for Cherryview GPIOs"
+	help
+
+	Enables support for the Intel Cherryview GPIO controller
+
 config XENO_DRIVERS_GPIO_DEBUG
        bool "Enable GPIO core debugging features"
 
diff --git a/kernel/drivers/gpio/Makefile b/kernel/drivers/gpio/Makefile
index 7bcf59949..e534eab93 100644
--- a/kernel/drivers/gpio/Makefile
+++ b/kernel/drivers/gpio/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_XENO_DRIVERS_GPIO_SUN8I_H3) += xeno-gpio-sun8i-h3.o
 obj-$(CONFIG_XENO_DRIVERS_GPIO_ZYNQ7000) += xeno-gpio-zynq7000.o
 obj-$(CONFIG_XENO_DRIVERS_GPIO_XILINX) += xeno-gpio-xilinx.o
 obj-$(CONFIG_XENO_DRIVERS_GPIO_OMAP) += xeno-gpio-omap.o
+obj-$(CONFIG_XENO_DRIVERS_GPIO_CHERRYVIEW) += xeno-gpio-cherryview.o
 obj-$(CONFIG_XENO_DRIVERS_GPIO) += gpio-core.o
 
 xeno-gpio-bcm2835-y := gpio-bcm2835.o
@@ -14,3 +15,4 @@ xeno-gpio-sun8i-h3-y := gpio-sun8i-h3.o
 xeno-gpio-zynq7000-y := gpio-zynq7000.o
 xeno-gpio-xilinx-y := gpio-xilinx.o
 xeno-gpio-omap-y := gpio-omap.o
+xeno-gpio-cherryview-y := gpio-cherryview.o
diff --git a/kernel/drivers/gpio/gpio-cherryview.c b/kernel/drivers/gpio/gpio-cherryview.c
new file mode 100644
index 000000000..ac7136325
--- /dev/null
+++ b/kernel/drivers/gpio/gpio-cherryview.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * @note Copyright (C) 2021 Hongzhan Chen <hongzhan.chen@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/module.h>
+#include <rtdm/gpio.h>
+
+#define RTDM_SUBCLASS_CHERRYVIEW  7
+
+static const char *compat_array[] = {
+	"INT33FF:00",
+	"INT33FF:01",
+	"INT33FF:02",
+	"INT33FF:03",
+};
+
+static int __init cherryview_gpio_init(void)
+{
+	return rtdm_gpiochip_array_find(NULL, compat_array,
+					ARRAY_SIZE(compat_array),
+					RTDM_SUBCLASS_CHERRYVIEW);
+}
+module_init(cherryview_gpio_init);
+
+static void __exit cherryview_gpio_exit(void)
+{
+	rtdm_gpiochip_remove_of(RTDM_SUBCLASS_CHERRYVIEW);
+}
+module_exit(cherryview_gpio_exit);
+
+MODULE_LICENSE("GPL");
-- 
2.17.1

Re: [PATCH 1/3] drivers/gpio: core: move out of OF config conditional compilation

[Jan Kiszka]

On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
> The api would be called by both non-OF and OF platform to remove rtdm
> gpiochip device.
> 
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
> ---
>  include/cobalt/kernel/rtdm/gpio.h | 3 +--
>  kernel/drivers/gpio/gpio-core.c   | 4 ++--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/include/cobalt/kernel/rtdm/gpio.h b/include/cobalt/kernel/rtdm/gpio.h
> index 72cc3a035..c307b1572 100644
> --- a/include/cobalt/kernel/rtdm/gpio.h
> +++ b/include/cobalt/kernel/rtdm/gpio.h
> @@ -70,9 +70,8 @@ int rtdm_gpiochip_scan_of(struct device_node *from,
>  int rtdm_gpiochip_scan_array_of(struct device_node *from,
>  				const char *compat[],
>  				int nentries, int type);
> +#endif
>  
>  void rtdm_gpiochip_remove_of(int type);
>  

But, when we call this for ACPI platforms as well, is _of still the
right name?

> -#endif
> -
>  #endif /* !_COBALT_RTDM_GPIO_H */
> diff --git a/kernel/drivers/gpio/gpio-core.c b/kernel/drivers/gpio/gpio-core.c
> index 06a19b33a..ddda62b24 100644
> --- a/kernel/drivers/gpio/gpio-core.c
> +++ b/kernel/drivers/gpio/gpio-core.c
> @@ -626,6 +626,8 @@ int rtdm_gpiochip_scan_array_of(struct device_node *from,
>  }
>  EXPORT_SYMBOL_GPL(rtdm_gpiochip_scan_array_of);
>  
> +#endif /* CONFIG_OF */
> +
>  void rtdm_gpiochip_remove_of(int type)
>  {
>  	struct rtdm_gpio_chip *rgc, *n;
> @@ -644,5 +646,3 @@ void rtdm_gpiochip_remove_of(int type)
>  	mutex_unlock(&chip_lock);
>  }
>  EXPORT_SYMBOL_GPL(rtdm_gpiochip_remove_of);
> -
> -#endif /* CONFIG_OF */
> 

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] add support for cherryview gpio controller driver

[Jan Kiszka]

On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
> 1. move out of OF config conditional compilation so that non-OF platform
>   call same API to remove rtdm gpio chip device.
> 2. Introduce helper to find gpiochip as referring to pair of 
>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
> 

Looks good, except for that naming issue.

> I also did following tests with this patchset:
> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>    setting GPIO_RTIOC_TS
> 2. apply following patch, and rerun 1.
> 
> index f83d7689f..50afbd418 100644
> --- a/testsuite/gpiobench/gpiobench.c
> +++ b/testsuite/gpiobench/gpiobench.c
> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>                         goto out;
>                 }
> 
> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>                 if (ret) {
>                         printf("ioctl gpio port ts, failed\n");
>                         goto out;
> 
> Hardware env:
> 1. Rock PI X V1.4.
> 2. GPIO loopback connection between GPIO 334 and 335.
>  

Did you check if timestamps are as expected (different)?

Thanks,
Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

RE: [PATCH 0/3] add support for cherryview gpio controller driver

[Chen, Hongzhan]

>
>
>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Monday, August 30, 2021 3:37 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>
>On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>> 1. move out of OF config conditional compilation so that non-OF platform
>>   call same API to remove rtdm gpio chip device.
>> 2. Introduce helper to find gpiochip as referring to pair of 
>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>> 
>
>Looks good, except for that naming issue.
>
>> I also did following tests with this patchset:
>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>    setting GPIO_RTIOC_TS
>> 2. apply following patch, and rerun 1.
>> 
>> index f83d7689f..50afbd418 100644
>> --- a/testsuite/gpiobench/gpiobench.c
>> +++ b/testsuite/gpiobench/gpiobench.c
>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>                         goto out;
>>                 }
>> 
>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>                 if (ret) {
>>                         printf("ioctl gpio port ts, failed\n");
>>                         goto out;
>> 
>> Hardware env:
>> 1. Rock PI X V1.4.
>> 2. GPIO loopback connection between GPIO 334 and 335.
>>  
>
>Did you check if timestamps are as expected (different)?

According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
But it is only about  16.879997 when we set REALTIME. 

Regards

Hongzhan Chen
>
>Thanks,
>Jan
>
>-- 
>Siemens AG, T RDA IOT
>Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] add support for cherryview gpio controller driver

[Jan Kiszka]

On 31.08.21 03:35, Chen, Hongzhan wrote:
>>
>>
>> -----Original Message-----
>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>> Sent: Monday, August 30, 2021 3:37 PM
>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>
>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>   call same API to remove rtdm gpio chip device.
>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>
>>
>> Looks good, except for that naming issue.
>>
>>> I also did following tests with this patchset:
>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>    setting GPIO_RTIOC_TS
>>> 2. apply following patch, and rerun 1.
>>>
>>> index f83d7689f..50afbd418 100644
>>> --- a/testsuite/gpiobench/gpiobench.c
>>> +++ b/testsuite/gpiobench/gpiobench.c
>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>                         goto out;
>>>                 }
>>>
>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>                 if (ret) {
>>>                         printf("ioctl gpio port ts, failed\n");
>>>                         goto out;
>>>
>>> Hardware env:
>>> 1. Rock PI X V1.4.
>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>  
>>
>> Did you check if timestamps are as expected (different)?
> 
> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
> But it is only about  16.879997 when we set REALTIME. 
> 

gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
calls)). If you compare them to REALTIME, that will give an offset. But
you report just the opposite, this confuses me now...

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

RE: [PATCH 0/3] add support for cherryview gpio controller driver

[Chen, Hongzhan]

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Tuesday, August 31, 2021 1:59 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>
>On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>> Sent: Monday, August 30, 2021 3:37 PM
>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>
>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>   call same API to remove rtdm gpio chip device.
>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>
>>>
>>> Looks good, except for that naming issue.
>>>
>>>> I also did following tests with this patchset:
>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>    setting GPIO_RTIOC_TS
>>>> 2. apply following patch, and rerun 1.
>>>>
>>>> index f83d7689f..50afbd418 100644
>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>                         goto out;
>>>>                 }
>>>>
>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>                 if (ret) {
>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>                         goto out;
>>>>
>>>> Hardware env:
>>>> 1. Rock PI X V1.4.
>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>  
>>>
>>> Did you check if timestamps are as expected (different)?
>> 
>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>> But it is only about  16.879997 when we set REALTIME. 
>> 
>
>gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>calls)). If you compare them to REALTIME, that will give an offset. But
>you report just the opposite, this confuses me now...

The test is running on dovetail based 5.10.

>
>Jan
>
>-- 
>Siemens AG, T RDA IOT
>Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] add support for cherryview gpio controller driver

[Jan Kiszka]

On 31.08.21 08:14, Chen, Hongzhan wrote:
> 
> 
>> -----Original Message-----
>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>> Sent: Tuesday, August 31, 2021 1:59 PM
>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>
>> On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>> Sent: Monday, August 30, 2021 3:37 PM
>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>
>>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>>   call same API to remove rtdm gpio chip device.
>>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>>
>>>>
>>>> Looks good, except for that naming issue.
>>>>
>>>>> I also did following tests with this patchset:
>>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>>    setting GPIO_RTIOC_TS
>>>>> 2. apply following patch, and rerun 1.
>>>>>
>>>>> index f83d7689f..50afbd418 100644
>>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>>                         goto out;
>>>>>                 }
>>>>>
>>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>>                 if (ret) {
>>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>>                         goto out;
>>>>>
>>>>> Hardware env:
>>>>> 1. Rock PI X V1.4.
>>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>>  
>>>>
>>>> Did you check if timestamps are as expected (different)?
>>>
>>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>>> But it is only about  16.879997 when we set REALTIME. 
>>>
>>
>> gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>> calls)). If you compare them to REALTIME, that will give an offset. But
>> you report just the opposite, this confuses me now...
> 
> The test is running on dovetail based 5.10.
> 

I missed one case in gpio_pin_interrupt where I need to check
monotonic_timestamp - but, again, mono is what should remain fine,
GPIO_RTIOC_TS_REAL (or GPIO_RTIOC_TS, the default) should give a delta.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

RE: [PATCH 0/3] add support for cherryview gpio controller driver

[Chen, Hongzhan]

>> -----Original Message-----
>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>> Sent: Tuesday, August 31, 2021 1:59 PM
>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>
>>> On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>> Sent: Monday, August 30, 2021 3:37 PM
>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>
>>>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>>>   call same API to remove rtdm gpio chip device.
>>>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>>>
>>>>>
>>>>> Looks good, except for that naming issue.
>>>>>
>>>>>> I also did following tests with this patchset:
>>>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>>>    setting GPIO_RTIOC_TS
>>>>>> 2. apply following patch, and rerun 1.
>>>>>>
>>>>>> index f83d7689f..50afbd418 100644
>>>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>>>                         goto out;
>>>>>>                 }
>>>>>>
>>>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>>>                 if (ret) {
>>>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>>>                         goto out;
>>>>>>
>>>>>> Hardware env:
>>>>>> 1. Rock PI X V1.4.
>>>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>>>  
>>>>>
>>>>> Did you check if timestamps are as expected (different)?
>>>>
>>>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>>>> But it is only about  16.879997 when we set REALTIME. 
>>>>
>>>
>>> gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>>> calls)). If you compare them to REALTIME, that will give an offset. But
>>> you report just the opposite, this confuses me now...
>> 
>> The test is running on dovetail based 5.10.
>> 
>
>I missed one case in gpio_pin_interrupt where I need to check
>monotonic_timestamp - but, again, mono is what should remain fine,
>GPIO_RTIOC_TS_REAL (or GPIO_RTIOC_TS, the default) should give a delta.

I found there is one obvious gap for my comparison test.

For REALTEIM , it totally run 999999 like [1] till test normally exit. But for MONO, I force to quit during running
because it may take several hours to finish and I can not wait like [2]. Let me rerun test and wait till it normally exit and 
get result to check.

[1]:
# Total: 000999999
# Min Latencies: 00013
# Avg Latencies: 16.879997
# Max Latencies: 00043
[2]:
# Total: 000083988
# Min Latencies: 00012
# Avg Latencies: 2814377.000000
# Max Latencies: 00041



>
>Jan
>
>-- 
>Siemens AG, T RDA IOT
>Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] add support for cherryview gpio controller driver

[Jan Kiszka]

On 31.08.21 08:36, Chen, Hongzhan wrote:
>>> -----Original Message-----
>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>> Sent: Tuesday, August 31, 2021 1:59 PM
>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>
>>>> On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>> Sent: Monday, August 30, 2021 3:37 PM
>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>
>>>>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>>>>   call same API to remove rtdm gpio chip device.
>>>>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>>>>
>>>>>>
>>>>>> Looks good, except for that naming issue.
>>>>>>
>>>>>>> I also did following tests with this patchset:
>>>>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>>>>    setting GPIO_RTIOC_TS
>>>>>>> 2. apply following patch, and rerun 1.
>>>>>>>
>>>>>>> index f83d7689f..50afbd418 100644
>>>>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>>>>                         goto out;
>>>>>>>                 }
>>>>>>>
>>>>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>>>>                 if (ret) {
>>>>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>>>>                         goto out;
>>>>>>>
>>>>>>> Hardware env:
>>>>>>> 1. Rock PI X V1.4.
>>>>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>>>>  
>>>>>>
>>>>>> Did you check if timestamps are as expected (different)?
>>>>>
>>>>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>>>>> But it is only about  16.879997 when we set REALTIME. 
>>>>>
>>>>
>>>> gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>>>> calls)). If you compare them to REALTIME, that will give an offset. But
>>>> you report just the opposite, this confuses me now...
>>>
>>> The test is running on dovetail based 5.10.
>>>
>>
>> I missed one case in gpio_pin_interrupt where I need to check
>> monotonic_timestamp - but, again, mono is what should remain fine,
>> GPIO_RTIOC_TS_REAL (or GPIO_RTIOC_TS, the default) should give a delta.
> 
> I found there is one obvious gap for my comparison test.
> 
> For REALTEIM , it totally run 999999 like [1] till test normally exit. But for MONO, I force to quit during running
> because it may take several hours to finish and I can not wait like [2]. Let me rerun test and wait till it normally exit and 
> get result to check.
> 
> [1]:
> # Total: 000999999
> # Min Latencies: 00013
> # Avg Latencies: 16.879997
> # Max Latencies: 00043
> [2]:
> # Total: 000083988
> # Min Latencies: 00012
> # Avg Latencies: 2814377.000000
> # Max Latencies: 00041
> 

Were you able to resolve this mystery over latest next?

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

RE: [PATCH 0/3] add support for cherryview gpio controller driver

[Chen, Hongzhan]

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Wednesday, September 1, 2021 10:27 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>
>On 31.08.21 08:36, Chen, Hongzhan wrote:
>>>> -----Original Message-----
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>> Sent: Tuesday, August 31, 2021 1:59 PM
>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>
>>>>> On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>>>>>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>>> Sent: Monday, August 30, 2021 3:37 PM
>>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>>
>>>>>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>>>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>>>>>   call same API to remove rtdm gpio chip device.
>>>>>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>>>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>>>>>
>>>>>>>
>>>>>>> Looks good, except for that naming issue.
>>>>>>>
>>>>>>>> I also did following tests with this patchset:
>>>>>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>>>>>    setting GPIO_RTIOC_TS
>>>>>>>> 2. apply following patch, and rerun 1.
>>>>>>>>
>>>>>>>> index f83d7689f..50afbd418 100644
>>>>>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>>>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>>>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>>>>>                         goto out;
>>>>>>>>                 }
>>>>>>>>
>>>>>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>>>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>>>>>                 if (ret) {
>>>>>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>>>>>                         goto out;
>>>>>>>>
>>>>>>>> Hardware env:
>>>>>>>> 1. Rock PI X V1.4.
>>>>>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>>>>>  
>>>>>>>
>>>>>>> Did you check if timestamps are as expected (different)?
>>>>>>
>>>>>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>>>>>> But it is only about  16.879997 when we set REALTIME. 
>>>>>>
>>>>>
>>>>> gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>>>>> calls)). If you compare them to REALTIME, that will give an offset. But
>>>>> you report just the opposite, this confuses me now...
>>>>
>>>> The test is running on dovetail based 5.10.
>>>>
>>>
>>> I missed one case in gpio_pin_interrupt where I need to check
>>> monotonic_timestamp - but, again, mono is what should remain fine,
>>> GPIO_RTIOC_TS_REAL (or GPIO_RTIOC_TS, the default) should give a delta.
>> 
>> I found there is one obvious gap for my comparison test.
>> 
>> For REALTEIM , it totally run 999999 like [1] till test normally exit. But for MONO, I force to quit during running
>> because it may take several hours to finish and I can not wait like [2]. Let me rerun test and wait till it normally exit and 
>> get result to check.
>> 
>> [1]:
>> # Total: 000999999
>> # Min Latencies: 00013
>> # Avg Latencies: 16.879997
>> # Max Latencies: 00043
>> [2]:
>> # Total: 000083988
>> # Min Latencies: 00012
>> # Avg Latencies: 2814377.000000
>> # Max Latencies: 00041
>> 
>
>Were you able to resolve this mystery over latest next?

Yes. Firstly , I am doing complete test with passing MONO or REALTIME to check if this mystery large AVG latency 
still happen till gpiobench normally exit and do comparison test based on updated next code. And then figure out 
why it happen when gpiobench is interrupted during running and fix it if it is problem caused by timestamp.

Regards

Hongzhan Chen
>
>Jan
>
>-- 
>Siemens AG, T RDA IOT
>Corporate Competence Center Embedded Linux

Re: [PATCH 0/3] add support for cherryview gpio controller driver

[Jan Kiszka]

On 02.09.21 02:42, Chen, Hongzhan wrote:
> 
> 
>> -----Original Message-----
>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>> Sent: Wednesday, September 1, 2021 10:27 PM
>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>
>> On 31.08.21 08:36, Chen, Hongzhan wrote:
>>>>> -----Original Message-----
>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>> Sent: Tuesday, August 31, 2021 1:59 PM
>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>
>>>>>> On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>>>> Sent: Monday, August 30, 2021 3:37 PM
>>>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>>>
>>>>>>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>>>>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>>>>>>   call same API to remove rtdm gpio chip device.
>>>>>>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>>>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>>>>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Looks good, except for that naming issue.
>>>>>>>>
>>>>>>>>> I also did following tests with this patchset:
>>>>>>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>>>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>>>>>>    setting GPIO_RTIOC_TS
>>>>>>>>> 2. apply following patch, and rerun 1.
>>>>>>>>>
>>>>>>>>> index f83d7689f..50afbd418 100644
>>>>>>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>>>>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>>>>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>>>>>>                         goto out;
>>>>>>>>>                 }
>>>>>>>>>
>>>>>>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>>>>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>>>>>>                 if (ret) {
>>>>>>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>>>>>>                         goto out;
>>>>>>>>>
>>>>>>>>> Hardware env:
>>>>>>>>> 1. Rock PI X V1.4.
>>>>>>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>>>>>>  
>>>>>>>>
>>>>>>>> Did you check if timestamps are as expected (different)?
>>>>>>>
>>>>>>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>>>>>>> But it is only about  16.879997 when we set REALTIME. 
>>>>>>>
>>>>>>
>>>>>> gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>>>>>> calls)). If you compare them to REALTIME, that will give an offset. But
>>>>>> you report just the opposite, this confuses me now...
>>>>>
>>>>> The test is running on dovetail based 5.10.
>>>>>
>>>>
>>>> I missed one case in gpio_pin_interrupt where I need to check
>>>> monotonic_timestamp - but, again, mono is what should remain fine,
>>>> GPIO_RTIOC_TS_REAL (or GPIO_RTIOC_TS, the default) should give a delta.
>>>
>>> I found there is one obvious gap for my comparison test.
>>>
>>> For REALTEIM , it totally run 999999 like [1] till test normally exit. But for MONO, I force to quit during running
>>> because it may take several hours to finish and I can not wait like [2]. Let me rerun test and wait till it normally exit and 
>>> get result to check.
>>>
>>> [1]:
>>> # Total: 000999999
>>> # Min Latencies: 00013
>>> # Avg Latencies: 16.879997
>>> # Max Latencies: 00043
>>> [2]:
>>> # Total: 000083988
>>> # Min Latencies: 00012
>>> # Avg Latencies: 2814377.000000
>>> # Max Latencies: 00041
>>>
>>
>> Were you able to resolve this mystery over latest next?
> 
> Yes. Firstly , I am doing complete test with passing MONO or REALTIME to check if this mystery large AVG latency 
> still happen till gpiobench normally exit and do comparison test based on updated next code. And then figure out 
> why it happen when gpiobench is interrupted during running and fix it if it is problem caused by timestamp.
> 

So, you WILL do this, right? I'm asking because this is blocking -rc1,
and I can't test here.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

RE: [PATCH 0/3] add support for cherryview gpio controller driver

[Chen, Hongzhan]

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Thursday, September 2, 2021 2:11 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>
>On 02.09.21 02:42, Chen, Hongzhan wrote:
>> 
>> 
>>> -----Original Message-----
>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>> Sent: Wednesday, September 1, 2021 10:27 PM
>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>
>>> On 31.08.21 08:36, Chen, Hongzhan wrote:
>>>>>> -----Original Message-----
>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>>> Sent: Tuesday, August 31, 2021 1:59 PM
>>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>>
>>>>>>> On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>>>>> Sent: Monday, August 30, 2021 3:37 PM
>>>>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>>>>
>>>>>>>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>>>>>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>>>>>>>   call same API to remove rtdm gpio chip device.
>>>>>>>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>>>>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>>>>>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Looks good, except for that naming issue.
>>>>>>>>>
>>>>>>>>>> I also did following tests with this patchset:
>>>>>>>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>>>>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>>>>>>>    setting GPIO_RTIOC_TS
>>>>>>>>>> 2. apply following patch, and rerun 1.
>>>>>>>>>>
>>>>>>>>>> index f83d7689f..50afbd418 100644
>>>>>>>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>>>>>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>>>>>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>>>>>>>                         goto out;
>>>>>>>>>>                 }
>>>>>>>>>>
>>>>>>>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>>>>>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>>>>>>>                 if (ret) {
>>>>>>>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>>>>>>>                         goto out;
>>>>>>>>>>
>>>>>>>>>> Hardware env:
>>>>>>>>>> 1. Rock PI X V1.4.
>>>>>>>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>>>>>>>  
>>>>>>>>>
>>>>>>>>> Did you check if timestamps are as expected (different)?
>>>>>>>>
>>>>>>>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>>>>>>>> But it is only about  16.879997 when we set REALTIME. 
>>>>>>>>
>>>>>>>
>>>>>>> gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>>>>>>> calls)). If you compare them to REALTIME, that will give an offset. But
>>>>>>> you report just the opposite, this confuses me now...
>>>>>>
>>>>>> The test is running on dovetail based 5.10.
>>>>>>
>>>>>
>>>>> I missed one case in gpio_pin_interrupt where I need to check
>>>>> monotonic_timestamp - but, again, mono is what should remain fine,
>>>>> GPIO_RTIOC_TS_REAL (or GPIO_RTIOC_TS, the default) should give a delta.
>>>>
>>>> I found there is one obvious gap for my comparison test.
>>>>
>>>> For REALTEIM , it totally run 999999 like [1] till test normally exit. But for MONO, I force to quit during running
>>>> because it may take several hours to finish and I can not wait like [2]. Let me rerun test and wait till it normally exit and 
>>>> get result to check.
>>>>
>>>> [1]:
>>>> # Total: 000999999
>>>> # Min Latencies: 00013
>>>> # Avg Latencies: 16.879997
>>>> # Max Latencies: 00043
>>>> [2]:
>>>> # Total: 000083988
>>>> # Min Latencies: 00012
>>>> # Avg Latencies: 2814377.000000
>>>> # Max Latencies: 00041
>>>>
>>>
>>> Were you able to resolve this mystery over latest next?
>> 
>> Yes. Firstly , I am doing complete test with passing MONO or REALTIME to check if this mystery large AVG latency 
>> still happen till gpiobench normally exit and do comparison test based on updated next code. And then figure out 
>> why it happen when gpiobench is interrupted during running and fix it if it is problem caused by timestamp.
>> 
>
>So, you WILL do this, right? I'm asking because this is blocking -rc1,
>and I can't test here.

Yes. I already found the root cause. It is caused by gpiobench itself. Because 
It do not execute "ti.ts.inner_avg /= (ti.total_cycles * 2) when I use Ctrl+C to force the gpiobench to quit and inner_avg keep
all sum of latency so that it is very large number. I will submit patch to review after I validate it.

Regards

Hongzhan Chen

>
>Jan
>
>-- 
>Siemens AG, T RDA IOT
>Corporate Competence Center Embedded Linux
>

Re: [PATCH 0/3] add support for cherryview gpio controller driver

[Jan Kiszka]

On 02.09.21 08:24, Chen, Hongzhan wrote:
> 
> 
>> -----Original Message-----
>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>> Sent: Thursday, September 2, 2021 2:11 PM
>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>
>> On 02.09.21 02:42, Chen, Hongzhan wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>> Sent: Wednesday, September 1, 2021 10:27 PM
>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>
>>>> On 31.08.21 08:36, Chen, Hongzhan wrote:
>>>>>>> -----Original Message-----
>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>>>> Sent: Tuesday, August 31, 2021 1:59 PM
>>>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>>>
>>>>>>>> On 31.08.21 03:35, Chen, Hongzhan wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>>>>>>> Sent: Monday, August 30, 2021 3:37 PM
>>>>>>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>>>>>>> Subject: Re: [PATCH 0/3] add support for cherryview gpio controller driver
>>>>>>>>>>
>>>>>>>>>> On 30.08.21 08:45, Hongzhan Chen via Xenomai wrote:
>>>>>>>>>>> 1. move out of OF config conditional compilation so that non-OF platform
>>>>>>>>>>>   call same API to remove rtdm gpio chip device.
>>>>>>>>>>> 2. Introduce helper to find gpiochip as referring to pair of 
>>>>>>>>>>>    rtdm_gpiochip_scan_of and rtdm_gpiochip_scan_array_of. 
>>>>>>>>>>> 3. Add Intel Cherryview pinctrl driver based on on 1 and 2.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Looks good, except for that naming issue.
>>>>>>>>>>
>>>>>>>>>>> I also did following tests with this patchset:
>>>>>>>>>>> 1. run /usr/lib/xenomai/testsuite/gpiobench -i 334 -i 335 -c INT33FF:02
>>>>>>>>>>>    to validate patch 9afea5ff2d7ba97db96b22a005a9a7fcf5f2d892 when
>>>>>>>>>>>    setting GPIO_RTIOC_TS
>>>>>>>>>>> 2. apply following patch, and rerun 1.
>>>>>>>>>>>
>>>>>>>>>>> index f83d7689f..50afbd418 100644
>>>>>>>>>>> --- a/testsuite/gpiobench/gpiobench.c
>>>>>>>>>>> +++ b/testsuite/gpiobench/gpiobench.c
>>>>>>>>>>> @@ -619,7 +619,7 @@ int main(int argc, char **argv)
>>>>>>>>>>>                         goto out;
>>>>>>>>>>>                 }
>>>>>>>>>>>
>>>>>>>>>>> -               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS, &value);
>>>>>>>>>>> +               ret = ioctl(ti.fd_dev_intr, GPIO_RTIOC_TS_MONO, &value);
>>>>>>>>>>>                 if (ret) {
>>>>>>>>>>>                         printf("ioctl gpio port ts, failed\n");
>>>>>>>>>>>                         goto out;
>>>>>>>>>>>
>>>>>>>>>>> Hardware env:
>>>>>>>>>>> 1. Rock PI X V1.4.
>>>>>>>>>>> 2. GPIO loopback connection between GPIO 334 and 335.
>>>>>>>>>>>  
>>>>>>>>>>
>>>>>>>>>> Did you check if timestamps are as expected (different)?
>>>>>>>>>
>>>>>>>>> According to output of gpiobench, the inner Avg Latency would be vary large like ( 2814377.00000) after switch to MONO.
>>>>>>>>> But it is only about  16.879997 when we set REALTIME. 
>>>>>>>>>
>>>>>>>>
>>>>>>>> gpiobench takes timestamps from CLOCK_MONOTONIC (see clock_gettime
>>>>>>>> calls)). If you compare them to REALTIME, that will give an offset. But
>>>>>>>> you report just the opposite, this confuses me now...
>>>>>>>
>>>>>>> The test is running on dovetail based 5.10.
>>>>>>>
>>>>>>
>>>>>> I missed one case in gpio_pin_interrupt where I need to check
>>>>>> monotonic_timestamp - but, again, mono is what should remain fine,
>>>>>> GPIO_RTIOC_TS_REAL (or GPIO_RTIOC_TS, the default) should give a delta.
>>>>>
>>>>> I found there is one obvious gap for my comparison test.
>>>>>
>>>>> For REALTEIM , it totally run 999999 like [1] till test normally exit. But for MONO, I force to quit during running
>>>>> because it may take several hours to finish and I can not wait like [2]. Let me rerun test and wait till it normally exit and 
>>>>> get result to check.
>>>>>
>>>>> [1]:
>>>>> # Total: 000999999
>>>>> # Min Latencies: 00013
>>>>> # Avg Latencies: 16.879997
>>>>> # Max Latencies: 00043
>>>>> [2]:
>>>>> # Total: 000083988
>>>>> # Min Latencies: 00012
>>>>> # Avg Latencies: 2814377.000000
>>>>> # Max Latencies: 00041
>>>>>
>>>>
>>>> Were you able to resolve this mystery over latest next?
>>>
>>> Yes. Firstly , I am doing complete test with passing MONO or REALTIME to check if this mystery large AVG latency 
>>> still happen till gpiobench normally exit and do comparison test based on updated next code. And then figure out 
>>> why it happen when gpiobench is interrupted during running and fix it if it is problem caused by timestamp.
>>>
>>
>> So, you WILL do this, right? I'm asking because this is blocking -rc1,
>> and I can't test here.
> 
> Yes. I already found the root cause. It is caused by gpiobench itself. Because 
> It do not execute "ti.ts.inner_avg /= (ti.total_cycles * 2) when I use Ctrl+C to force the gpiobench to quit and inner_avg keep
> all sum of latency so that it is very large number. I will submit patch to review after I validate it.
> 

Ah, perfect, TIA!

Jan


-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux