All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC
@ 2021-08-24 16:47 lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 01/20] gpio: Add basic GPIO driver for Intel PMC Timed I/O device lakshmi.sowjanya.d
                   ` (20 more replies)
  0 siblings, 21 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Starting with Intel(R) Tiger Lake and Elkhart Lake platforms the PMC
hardware adds the Timed I/O hardware interface.

The Timed I/O hardware implements some functionality similar to GPIO
with added timing logic that is driven by the Always Running Timer
(ART).

The Timed I/O Hardware implement 3 basic functions:
  * Input Timestamping
  * Single Shot Timed Output
  * Periodic Timed Output

 Please help to review the changes.

Thanks in advance,
Sowjanya 

Christopher Hall (9):
  gpio: Add basic GPIO driver for Intel PMC Timed I/O device
  arch: x86: Add ART support function to tsc code
  gpio: Add input code to Intel PMC Timed I/O Driver
  tools: gpio: Add additional polling support to gpio-event-mon
  kernel: time: Add system time to system counter translation
  tools: gpio: Add GPIO output generation user application
  arch/x86: Add ART nanosecond to ART conversion
  pwm: Add capability for PWM Driver managed state
  tools: gpio: Add PWM monitor application

Lakshmi Sowjanya D (11):
  gpio: Add GPIO polling interface to GPIO lib
  gpio: Add set_input and polling interface to GPIO lib code
  gpio: Add output event generation method to GPIOLIB and PMC Driver
  arch: x86: Add TSC to ART translation
  gpio: Add event count interface to gpiolib
  gpio: Add event count to Intel(R) PMC Timed I/O driver
  tools: gpio: Add event count capability to event monitor application
  gpio: Add PWM capabilities to Intel(R) PMC TIO driver
  pwm: Add second alignment to the core PWM interface
  gpio: Add PWM alignment support to the Intel(R) PMC Timed I/O driver
  gpio: Add GPIO monitor line to Intel(R) Timed I/O Driver

 MAINTAINERS                       |   7 +
 arch/x86/include/asm/tsc.h        |   5 +
 arch/x86/kernel/tsc.c             |  73 +++
 drivers/gpio/Kconfig              |  11 +
 drivers/gpio/Makefile             |   1 +
 drivers/gpio/gpio-intel-tio-pmc.c | 834 ++++++++++++++++++++++++++++++
 drivers/gpio/gpiolib-cdev.c       | 145 +++++-
 drivers/pwm/core.c                |  10 +-
 drivers/pwm/sysfs.c               |  37 ++
 include/linux/gpio/driver.h       |  29 ++
 include/linux/pwm.h               |   3 +
 include/linux/timekeeping.h       |   3 +
 include/uapi/linux/gpio.h         |  21 +
 kernel/time/timekeeping.c         |  63 +++
 tools/gpio/.gitignore             |   1 +
 tools/gpio/Build                  |   2 +
 tools/gpio/Makefile               |  21 +-
 tools/gpio/gpio-event-gen.c       | 191 +++++++
 tools/gpio/gpio-event-mon.c       |  58 ++-
 tools/gpio/gpio-pwm-mon.c         | 431 +++++++++++++++
 20 files changed, 1916 insertions(+), 30 deletions(-)
 create mode 100644 drivers/gpio/gpio-intel-tio-pmc.c
 create mode 100644 tools/gpio/gpio-event-gen.c
 create mode 100644 tools/gpio/gpio-pwm-mon.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 01/20] gpio: Add basic GPIO driver for Intel PMC Timed I/O device
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib lakshmi.sowjanya.d
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

The Intel PMC Timed I/O device provides GPIO-like functionality to
generate and capture I/O timestamped using ART

Several additional GPIO interfaces are needed to enable the device.
Add basic driver initialization, acpi_match_tables and register accessors.

Add entry for INTEL GPIO PMC TIO driver and the test drivers
gpio-event-gen.c and gpio-pwm-mon.c in the MAINTAINERS file.

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Co-developed-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 MAINTAINERS                       |   7 ++
 drivers/gpio/Kconfig              |  11 ++
 drivers/gpio/Makefile             |   1 +
 drivers/gpio/gpio-intel-tio-pmc.c | 182 ++++++++++++++++++++++++++++++
 4 files changed, 201 insertions(+)
 create mode 100644 drivers/gpio/gpio-intel-tio-pmc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c6b8a720c0bc..1b61e80deeae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9311,6 +9311,13 @@ F:	drivers/gpio/gpio-pch.c
 F:	drivers/gpio/gpio-sch.c
 F:	drivers/gpio/gpio-sodaville.c
 
+INTEL GPIO PMC TIO
+M:	Tamal Saha <tamal.saha@intel.com>
+S:	Supported
+F:	drivers/gpio/gpio-intel-tio-pmc.c
+F:	tools/gpio/gpio-event-gen.c
+F:	tools/gpio/gpio-pwm-mon.c
+
 INTEL GVT-g DRIVERS (Intel GPU Virtualization)
 M:	Zhenyu Wang <zhenyuw@linux.intel.com>
 M:	Zhi Wang <zhi.a.wang@intel.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index fab571016adf..962a92db09ba 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -794,6 +794,17 @@ config GPIO_IDT3243X
 	  To compile this driver as a module, choose M here: the module will
 	  be called gpio-idt3243x.
 
+config GPIO_INTEL_PMC_TIO
+	tristate "Intel PMC Time-Aware GPIO"
+	depends on X86
+	select GPIO_GENERIC
+	help
+	  This driver adds support for Intel PMC Timed-Aware GPIO (TGPIO)
+	  controller. The device clock used to drive TGPIO logic is the
+	  Always Running Timer (ART).
+
+	  Say yes here to build support for PMC TGPIO Driver.
+
 endmenu
 
 menu "Port-mapped I/O GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 32a32659866a..59bb4e756382 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -183,3 +183,4 @@ obj-$(CONFIG_GPIO_XRA1403)		+= gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)		+= gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)		+= gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)			+= gpio-zynq.o
+obj-$(CONFIG_GPIO_INTEL_PMC_TIO)	+= gpio-intel-tio-pmc.o
diff --git a/drivers/gpio/gpio-intel-tio-pmc.c b/drivers/gpio/gpio-intel-tio-pmc.c
new file mode 100644
index 000000000000..e6436b56ebea
--- /dev/null
+++ b/drivers/gpio/gpio-intel-tio-pmc.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel Time-Aware GPIO Controller Driver
+ * Copyright (C) 2021 Intel Corporation
+ */
+
+#include <linux/acpi.h>
+#include <linux/debugfs.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <uapi/linux/gpio.h>
+
+#define TGPIOCTL		0x00
+#define TGPIOCOMPV31_0		0x10
+#define TGPIOCOMPV63_32		0x14
+#define TGPIOPIV31_0		0x18
+#define TGPIOPIV63_32		0x1c
+#define TGPIOTCV31_0		0x20
+#define TGPIOTCV63_32		0x24 /* Not used */
+#define TGPIOECCV31_0		0x28
+#define TGPIOECCV63_32		0x2c
+#define TGPIOEC31_0		0x30
+#define TGPIOEC63_32		0x34
+
+/* Control Register */
+#define TGPIOCTL_EN			BIT(0)
+#define TGPIOCTL_DIR			BIT(1)
+#define TGPIOCTL_EP			GENMASK(3, 2)
+#define TGPIOCTL_EP_RISING_EDGE		(0 << 2)
+#define TGPIOCTL_EP_FALLING_EDGE	BIT(2)
+#define TGPIOCTL_EP_TOGGLE_EDGE		BIT(3)
+#define TGPIOCTL_PM			BIT(4)
+
+#define DRIVER_NAME		"intel-pmc-tio"
+
+struct intel_pmc_tio_chip {
+	struct gpio_chip gch;
+	struct platform_device *pdev;
+	struct dentry *root;
+	struct debugfs_regset32 *regset;
+	void __iomem *base;
+};
+
+static const struct debugfs_reg32 intel_pmc_tio_regs[] = {
+	{
+		.name = "TGPIOCTL",
+		.offset = TGPIOCTL
+	},
+	{
+		.name = "TGPIOCOMPV31_0",
+		.offset = TGPIOCOMPV31_0
+	},
+	{
+		.name = "TGPIOCOMPV63_32",
+		.offset = TGPIOCOMPV63_32
+	},
+	{
+		.name = "TGPIOPIV31_0",
+		.offset = TGPIOPIV31_0
+	},
+	{
+		.name = "TGPIOPIV63_32",
+		.offset = TGPIOPIV63_32
+	},
+	{
+		.name = "TGPIOECCV31_0",
+		.offset = TGPIOECCV31_0
+	},
+	{
+		.name = "TGPIOECCV63_32",
+		.offset = TGPIOECCV63_32
+	},
+	{
+		.name = "TGPIOEC31_0",
+		.offset = TGPIOEC31_0
+	},
+	{
+		.name = "TGPIOEC63_32",
+		.offset = TGPIOEC63_32
+	},
+};
+
+static int intel_pmc_tio_probe(struct platform_device *pdev)
+{
+	struct intel_pmc_tio_chip *tio;
+	int err;
+
+	tio = devm_kzalloc(&pdev->dev, sizeof(*tio), GFP_KERNEL);
+	if (!tio)
+		return -ENOMEM;
+	tio->pdev = pdev;
+
+	tio->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(tio->base))
+		return PTR_ERR(tio->base);
+
+	tio->regset = devm_kzalloc
+		(&pdev->dev, sizeof(*tio->regset), GFP_KERNEL);
+	if (!tio->regset)
+		return -ENOMEM;
+
+	tio->regset->regs = intel_pmc_tio_regs;
+	tio->regset->nregs = ARRAY_SIZE(intel_pmc_tio_regs);
+	tio->regset->base = tio->base;
+
+	tio->root = debugfs_create_dir(pdev->name, NULL);
+	if (IS_ERR(tio->root))
+		return PTR_ERR(tio->root);
+
+	debugfs_create_regset32("regdump", 0444, tio->root, tio->regset);
+
+	tio->gch.label = pdev->name;
+	tio->gch.ngpio = 0;
+	tio->gch.base = -1;
+
+	platform_set_drvdata(pdev, tio);
+
+	err = devm_gpiochip_add_data(&pdev->dev, &tio->gch, tio);
+	if (err < 0)
+		goto out_recurse_remove_tio_root;
+
+	return 0;
+
+out_recurse_remove_tio_root:
+	debugfs_remove_recursive(tio->root);
+
+	return err;
+}
+
+static int intel_pmc_tio_remove(struct platform_device *pdev)
+{
+	struct intel_pmc_tio_chip *tio;
+
+	tio = platform_get_drvdata(pdev);
+	if (!tio)
+		return -ENODEV;
+
+	debugfs_remove_recursive(tio->root);
+
+	return 0;
+}
+
+static const struct acpi_device_id intel_pmc_tio_acpi_match[] = {
+	{ "INTC1021", 0 }, /* EHL */
+	{ "INTC1022", 0 }, /* EHL */
+	{ "INTC1023", 0 }, /* TGL */
+	{ "INTC1024", 0 }, /* TGL */
+	{  }
+};
+
+static struct platform_driver intel_pmc_tio_driver = {
+	.probe          = intel_pmc_tio_probe,
+	.remove         = intel_pmc_tio_remove,
+	.driver         = {
+		.name                   = DRIVER_NAME,
+		.acpi_match_table       = intel_pmc_tio_acpi_match,
+	},
+};
+
+static int intel_pmc_tio_init(void)
+{
+	/* To ensure ART to TSC conversion is correct */
+	if (!boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ))
+		return -ENXIO;
+
+	return platform_driver_register(&intel_pmc_tio_driver);
+}
+
+static void intel_pmc_tio_exit(void)
+{
+	platform_driver_unregister(&intel_pmc_tio_driver);
+}
+
+module_init(intel_pmc_tio_init);
+module_exit(intel_pmc_tio_exit);
+
+MODULE_AUTHOR("Christopher Hall <christopher.s.hall@intel.com>");
+MODULE_AUTHOR("Tamal Saha <tamal.saha@intel.com>");
+MODULE_AUTHOR("Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>");
+MODULE_DESCRIPTION("Intel PMC Time-Aware GPIO Controller Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 01/20] gpio: Add basic GPIO driver for Intel PMC Timed I/O device lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-09-22 10:03   ` Bartosz Golaszewski
  2021-08-24 16:47 ` [RFC PATCH v1 03/20] arch: x86: Add ART support function to tsc code lakshmi.sowjanya.d
                   ` (18 subsequent siblings)
  20 siblings, 1 reply; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Some Intel Timed I/O devices do not implement IRQ functionality. Augment
read() interface to allow polling.

Add two GPIO device methods: setup_poll() and poll():
- setup_poll() configures the GPIO interface e.g. capture rising edges
- poll() checks for events on the interface

To implement polling, the driver must implement the two functions above
and should either leave to_irq() method NULL or return irq 0.

setup_poll() should configure the hardware to 'listen' for input events.

poll() driver implementation must return the realtime timestamp
corresponding to the event and -EAGAIN if no data is available.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpiolib-cdev.c | 28 ++++++++++++++++++++++++++--
 include/linux/gpio/driver.h | 19 +++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index c7b5446d01fd..4741bf34750b 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1227,13 +1227,34 @@ static ssize_t linereq_read(struct file *file,
 			    loff_t *f_ps)
 {
 	struct linereq *lr = file->private_data;
+	struct gpioevent_poll_data poll_data;
 	struct gpio_v2_line_event le;
 	ssize_t bytes_read = 0;
-	int ret;
+	int ret, offset;
 
 	if (count < sizeof(le))
 		return -EINVAL;
 
+	/* Without an IRQ, we can only poll */
+	offset = gpio_chip_hwgpio(lr->gdev->descs);
+	if (lr->lines[offset].irq == 0) {
+		struct gpio_v2_line_event *event;
+
+		if (!(file->f_flags & O_NONBLOCK))
+			return -ENODEV;
+
+		ret = lr->gdev->chip->do_poll(lr->gdev->chip, offset,
+					      lr->lines[offset].eflags, &poll_data);
+		if (ret)
+			return ret;
+		event = kzalloc(sizeof(*event), GFP_KERNEL);
+		event->timestamp_ns = poll_data.timestamp;
+		event->id = poll_data.id;
+		if (copy_to_user(buf, (void *)&event, sizeof(event)))
+			return -EFAULT;
+		return sizeof(event);
+	}
+
 	do {
 		spin_lock(&lr->wait.lock);
 		if (kfifo_is_empty(&lr->events)) {
@@ -1314,6 +1335,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 {
 	struct gpio_v2_line_request ulr;
 	struct gpio_v2_line_config *lc;
+	unsigned int file_flags;
 	struct linereq *lr;
 	struct file *file;
 	u64 flags;
@@ -1411,6 +1433,8 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 				goto out_free_linereq;
 		}
 
+		file_flags = O_RDONLY | O_CLOEXEC;
+
 		blocking_notifier_call_chain(&desc->gdev->notifier,
 					     GPIO_V2_LINE_CHANGED_REQUESTED, desc);
 
@@ -1425,7 +1449,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 	}
 
 	file = anon_inode_getfile("gpio-line", &line_fileops, lr,
-				  O_RDONLY | O_CLOEXEC);
+				  file_flags);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
 		goto out_put_unused_fd;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 3a268781fcec..f5b971ad40bc 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -17,6 +17,7 @@ struct device_node;
 struct seq_file;
 struct gpio_device;
 struct module;
+struct gpioevent_poll_data;
 enum gpiod_flags;
 enum gpio_lookup_flags;
 
@@ -304,6 +305,11 @@ struct gpio_irq_chip {
  * @add_pin_ranges: optional routine to initialize pin ranges, to be used when
  *	requires special mapping of the pins that provides GPIO functionality.
  *	It is called after adding GPIO chip and before adding IRQ chip.
+ * @setup_poll: optional routine for devices that don't support interrupts.
+ *	Takes flags argument as in/out parameter, where caller requests
+ *	event flags and driver returns accepted flags.
+ * @do_poll: optional routine for devices that don't support interrupts.
+ *	Returns event specification in data parameter.
  * @base: identifies the first GPIO number handled by this chip;
  *	or, if negative during registration, requests dynamic ID allocation.
  *	DEPRECATION: providing anything non-negative and nailing the base
@@ -396,6 +402,14 @@ struct gpio_chip {
 
 	int			(*add_pin_ranges)(struct gpio_chip *gc);
 
+	int                     (*setup_poll)(struct gpio_chip *chip,
+					      unsigned int offset,
+					      u32 *eflags);
+
+	int                     (*do_poll)(struct gpio_chip *chip,
+					   unsigned int offset, u32 eflags,
+					   struct gpioevent_poll_data *data);
+
 	int			base;
 	u16			ngpio;
 	const char		*const *names;
@@ -471,6 +485,11 @@ struct gpio_chip {
 #endif /* CONFIG_OF_GPIO */
 };
 
+struct gpioevent_poll_data {
+	__u64 timestamp;
+	__u32 id;
+};
+
 extern const char *gpiochip_is_requested(struct gpio_chip *gc,
 			unsigned int offset);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 03/20] arch: x86: Add ART support function to tsc code
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 01/20] gpio: Add basic GPIO driver for Intel PMC Timed I/O device lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 04/20] gpio: Add input code to Intel PMC Timed I/O Driver lakshmi.sowjanya.d
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

Add a function to 'read' the ART(Always Running Timer) value using
TSC(Time Stamp Capture) conversion.

The Intel PMC Timed I/O driver requires the current ART value to
test for rollover.

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Co-developed-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 arch/x86/include/asm/tsc.h |  1 +
 arch/x86/kernel/tsc.c      | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 01a300a9700b..a50b0102e5c1 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -28,6 +28,7 @@ static inline cycles_t get_cycles(void)
 	return rdtsc();
 }
 
+extern u64 read_art_time(void);
 extern struct system_counterval_t convert_art_to_tsc(u64 art);
 extern struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns);
 
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 2e076a459a0c..bbab6cf1a73b 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1230,6 +1230,26 @@ int unsynchronized_tsc(void)
 	return 0;
 }
 
+/*
+ * Converts the current TSC to the current ART value using conversion
+ * factors discovered by detect_art()
+ */
+u64 read_art_time(void)
+{
+	u64 tsc, tmp, res, rem;
+
+	tsc = read_tsc(NULL) - art_to_tsc_offset;
+	rem = do_div(tsc, art_to_tsc_numerator);
+
+	res = tsc * art_to_tsc_denominator;
+	tmp = rem * art_to_tsc_denominator;
+
+	do_div(tmp, art_to_tsc_numerator);
+
+	return res + tmp;
+}
+EXPORT_SYMBOL(read_art_time);
+
 /*
  * Convert ART to TSC given numerator/denominator found in detect_art()
  */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 04/20] gpio: Add input code to Intel PMC Timed I/O Driver
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (2 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 03/20] arch: x86: Add ART support function to tsc code lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 05/20] tools: gpio: Add additional polling support to gpio-event-mon lakshmi.sowjanya.d
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

Implement poll() and setup_poll() methods in the PMC Timed I/O Driver
for added GPIO lib functionality.

The setup_poll() code configures the hardware to listen for events on
the Timed I/O interface.

The poll() interface returns the timestamp of the last event or
-EAGAIN if no events are available.

Use timekeeping event get_device_system_crosststamp() interface to
translate ART/TSC to CLOCK_REALTIME. The poll operation is driven from user
space and may not occur within the same timekeeping interval as the actual
event, necessiating the use of the get_device_system_crosststamp() with
an inteepolation window.

Uses snapshotting interface to extend the translation/interpolation
window beyond the current timekeeping interval because, poll operation
is driven from user space and may not occur within the same timekeeping
interval as the actual event. . Necessitating use of the
get_device_system_crosststamp() with an interpolation window.

The ktime snapshot is guaranteed to be updated at least every 1/8 second
using a work queue item minimizing the interpolation window.

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Co-developed-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpio-intel-tio-pmc.c | 220 +++++++++++++++++++++++++++++-
 1 file changed, 219 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-intel-tio-pmc.c b/drivers/gpio/gpio-intel-tio-pmc.c
index e6436b56ebea..7e5e61054dea 100644
--- a/drivers/gpio/gpio-intel-tio-pmc.c
+++ b/drivers/gpio/gpio-intel-tio-pmc.c
@@ -8,6 +8,7 @@
 #include <linux/debugfs.h>
 #include <linux/gpio/driver.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <uapi/linux/gpio.h>
 
@@ -33,6 +34,9 @@
 #define TGPIOCTL_PM			BIT(4)
 
 #define DRIVER_NAME		"intel-pmc-tio"
+#define GPIO_COUNT		1
+#define INPUT_SNAPSHOT_FREQ	8
+#define INPUT_SNAPSHOT_COUNT	3
 
 struct intel_pmc_tio_chip {
 	struct gpio_chip gch;
@@ -40,8 +44,29 @@ struct intel_pmc_tio_chip {
 	struct dentry *root;
 	struct debugfs_regset32 *regset;
 	void __iomem *base;
+	struct mutex lock;		/* Protects 'ctrl', time */
+	struct delayed_work input_work;
+	bool input_work_running;
+	bool systime_valid;
+	unsigned int systime_index;
+	struct system_time_snapshot systime_snapshot[INPUT_SNAPSHOT_COUNT];
+	u64 last_event_count;
+	u64 last_art_timestamp;
 };
 
+struct intel_pmc_tio_get_time_arg {
+	struct intel_pmc_tio_chip *tio;
+	u32 eflags;
+	u32 event_id;
+	u64 abs_event_count;
+};
+
+#define gch_to_intel_pmc_tio(i)					\
+	(container_of((i), struct intel_pmc_tio_chip, gch))
+
+#define inws_to_intel_pmc_tio(i)					\
+	(container_of((i), struct intel_pmc_tio_chip, input_work.work))
+
 static const struct debugfs_reg32 intel_pmc_tio_regs[] = {
 	{
 		.name = "TGPIOCTL",
@@ -81,6 +106,193 @@ static const struct debugfs_reg32 intel_pmc_tio_regs[] = {
 	},
 };
 
+static inline u32 intel_pmc_tio_readl(struct intel_pmc_tio_chip *tio,
+				      u32 offset)
+{
+	return readl(tio->base + offset);
+}
+
+static inline void intel_pmc_tio_writel(struct intel_pmc_tio_chip *tio,
+					u32 offset, u32 value)
+{
+	writel(value, tio->base + offset);
+}
+
+#define INTEL_PMC_TIO_RD_REG(offset)(			\
+		intel_pmc_tio_readl((tio), (offset)))
+#define INTEL_PMC_TIO_WR_REG(offset, value)(			\
+		intel_pmc_tio_writel((tio), (offset), (value)))
+
+static void intel_pmc_tio_enable_input(struct intel_pmc_tio_chip *tio,
+				       u32 eflags)
+{
+	bool rising, falling;
+	u32 ctrl;
+
+	/* Disable */
+	ctrl = INTEL_PMC_TIO_RD_REG(TGPIOCTL);
+	ctrl &= ~TGPIOCTL_EN;
+	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
+
+	tio->last_event_count = 0;
+
+	/* Configure Input */
+	ctrl |= TGPIOCTL_DIR;
+	ctrl &= ~TGPIOCTL_EP;
+
+	rising = eflags & GPIO_V2_LINE_FLAG_EDGE_RISING;
+	falling = eflags & GPIO_V2_LINE_FLAG_EDGE_FALLING;
+	if (rising && falling)
+		ctrl |= TGPIOCTL_EP_TOGGLE_EDGE;
+	else if (rising)
+		ctrl |= TGPIOCTL_EP_RISING_EDGE;
+	else
+		ctrl |= TGPIOCTL_EP_FALLING_EDGE;
+
+	/* Enable */
+	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
+	ctrl |= TGPIOCTL_EN;
+	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
+}
+
+static void intel_pmc_tio_input_work(struct work_struct *input_work)
+{
+	struct intel_pmc_tio_chip *tio = inws_to_intel_pmc_tio(input_work);
+
+	mutex_lock(&tio->lock);
+
+	tio->systime_index = (tio->systime_index + 1) % INPUT_SNAPSHOT_COUNT;
+	if (tio->systime_index == INPUT_SNAPSHOT_COUNT - 1)
+		tio->systime_valid = true;
+	ktime_get_snapshot(&tio->systime_snapshot[tio->systime_index]);
+	schedule_delayed_work(&tio->input_work, HZ / INPUT_SNAPSHOT_FREQ);
+
+	mutex_unlock(&tio->lock);
+}
+
+static void intel_pmc_tio_start_input_work(struct intel_pmc_tio_chip *tio)
+{
+	if (tio->input_work_running)
+		return;
+
+	tio->systime_index = 0;
+	tio->systime_valid = false;
+	ktime_get_snapshot(&tio->systime_snapshot[tio->systime_index]);
+
+	schedule_delayed_work(&tio->input_work, HZ / INPUT_SNAPSHOT_FREQ);
+	tio->input_work_running = true;
+}
+
+static void intel_pmc_tio_stop_input_work(struct intel_pmc_tio_chip *tio)
+{
+	if (!tio->input_work_running)
+		return;
+
+	cancel_delayed_work_sync(&tio->input_work);
+	tio->input_work_running = false;
+}
+
+static int intel_pmc_tio_setup_poll(struct gpio_chip *chip, unsigned int offset,
+				    u32 *eflags)
+{
+	struct intel_pmc_tio_chip *tio;
+
+	if (offset != 0)
+		return -EINVAL;
+
+	tio = gch_to_intel_pmc_tio(chip);
+
+	mutex_lock(&tio->lock);
+	intel_pmc_tio_start_input_work(tio);
+	intel_pmc_tio_enable_input(tio, *eflags);
+	mutex_unlock(&tio->lock);
+
+	return 0;
+}
+
+static int intel_pmc_tio_get_time(ktime_t *device_time,
+				  struct system_counterval_t *system_counterval,
+				  void *ctx)
+{
+	struct intel_pmc_tio_get_time_arg *arg = (typeof(arg))ctx;
+	struct intel_pmc_tio_chip *tio = arg->tio;
+	u32 flags = arg->eflags;
+	u64 abs_event_count;
+	u32 rel_event_count;
+	u64 art_timestamp;
+	u32 dt_hi_s;
+	u32 dt_hi_e;
+	int err = 0;
+	u32 dt_lo;
+
+	/* Upper 64 bits of TCV are unlocked, don't use */
+	dt_hi_s = read_art_time() >> 32;
+	dt_lo = INTEL_PMC_TIO_RD_REG(TGPIOTCV31_0);
+	abs_event_count = INTEL_PMC_TIO_RD_REG(TGPIOECCV63_32);
+	abs_event_count <<= 32;
+	abs_event_count |= INTEL_PMC_TIO_RD_REG(TGPIOECCV31_0);
+	dt_hi_e = read_art_time() >> 32;
+
+	art_timestamp = ((dt_hi_e != dt_hi_s) && !(dt_lo & 0x80000000)) ?
+			 dt_hi_e : dt_hi_s;
+	art_timestamp <<= 32;
+	art_timestamp |= dt_lo;
+
+	rel_event_count = abs_event_count - tio->last_event_count;
+	if (rel_event_count == 0 || art_timestamp == tio->last_art_timestamp) {
+		err = -EAGAIN;
+		goto out;
+	}
+
+	tio->last_art_timestamp = art_timestamp;
+
+	*system_counterval = convert_art_to_tsc(art_timestamp);
+	arg->abs_event_count = abs_event_count;
+	arg->event_id = 0;
+	arg->event_id |= (flags & GPIO_V2_LINE_FLAG_EDGE_RISING) ?
+		GPIO_V2_LINE_EVENT_RISING_EDGE : 0;
+	arg->event_id |= (flags & GPIO_V2_LINE_FLAG_EDGE_FALLING) ?
+		GPIO_V2_LINE_EVENT_FALLING_EDGE : 0;
+
+out:
+	return err;
+}
+
+static int intel_pmc_tio_do_poll(struct gpio_chip *chip, unsigned int offset,
+				 u32 eflags, struct gpioevent_poll_data *data)
+{
+	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
+	struct intel_pmc_tio_get_time_arg arg = {
+		.eflags = eflags, .tio = tio };
+	struct system_device_crosststamp xtstamp;
+	unsigned int i, stop;
+	int err = -EAGAIN;
+
+	mutex_lock(&tio->lock);
+
+	i = tio->systime_index;
+	stop = tio->systime_valid ?
+		tio->systime_index : INPUT_SNAPSHOT_COUNT - 1;
+	do {
+		err = get_device_system_crosststamp(intel_pmc_tio_get_time,
+						    &arg,
+						    &tio->systime_snapshot[i],
+						    &xtstamp);
+		if (!err) {
+			data->timestamp = ktime_to_ns(xtstamp.sys_realtime);
+			data->id = arg.event_id;
+			tio->last_event_count = arg.abs_event_count;
+		}
+		if (!err || err == -EAGAIN)
+			break;
+		i = (i + (INPUT_SNAPSHOT_COUNT - 1)) % INPUT_SNAPSHOT_COUNT;
+	} while (i != stop);
+
+	mutex_unlock(&tio->lock);
+
+	return err;
+}
+
 static int intel_pmc_tio_probe(struct platform_device *pdev)
 {
 	struct intel_pmc_tio_chip *tio;
@@ -111,10 +323,14 @@ static int intel_pmc_tio_probe(struct platform_device *pdev)
 	debugfs_create_regset32("regdump", 0444, tio->root, tio->regset);
 
 	tio->gch.label = pdev->name;
-	tio->gch.ngpio = 0;
+	tio->gch.ngpio = GPIO_COUNT;
 	tio->gch.base = -1;
+	tio->gch.setup_poll = intel_pmc_tio_setup_poll;
+	tio->gch.do_poll = intel_pmc_tio_do_poll;
 
 	platform_set_drvdata(pdev, tio);
+	mutex_init(&tio->lock);
+	INIT_DELAYED_WORK(&tio->input_work, intel_pmc_tio_input_work);
 
 	err = devm_gpiochip_add_data(&pdev->dev, &tio->gch, tio);
 	if (err < 0)
@@ -136,6 +352,8 @@ static int intel_pmc_tio_remove(struct platform_device *pdev)
 	if (!tio)
 		return -ENODEV;
 
+	intel_pmc_tio_stop_input_work(tio);
+	mutex_destroy(&tio->lock);
 	debugfs_remove_recursive(tio->root);
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 05/20] tools: gpio: Add additional polling support to gpio-event-mon
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (3 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 04/20] gpio: Add input code to Intel PMC Timed I/O Driver lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 06/20] gpio: Add set_input and polling interface to GPIO lib code lakshmi.sowjanya.d
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

Intel Timed I/O hardware doesn't support reading the current levels,
allow application to continue if this fails.

The Timed I/O hardware aggregates muliple events, but doesn't distinguish
between rising and falling edges *if* both types are selected. Add
output 'verbiage' for unknown event type.

Add verbosity parameter to suppress printing of "nothing available" poll
result. This can be re-enabled at runtime with "-vv" parameter.

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Co-developed-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 include/uapi/linux/gpio.h   |  1 +
 tools/gpio/gpio-event-mon.c | 42 ++++++++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index eaaea3d8e6b4..ed84805baee8 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -267,6 +267,7 @@ struct gpio_v2_line_info_changed {
 enum gpio_v2_line_event_id {
 	GPIO_V2_LINE_EVENT_RISING_EDGE	= 1,
 	GPIO_V2_LINE_EVENT_FALLING_EDGE	= 2,
+	GPIO_V2_LINE_EVENT_UNKNOWN_EDGE = 3,
 };
 
 /**
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index a2b233fdb572..d8f0bbf78728 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -29,7 +29,8 @@ int monitor_device(const char *device_name,
 		   unsigned int *lines,
 		   unsigned int num_lines,
 		   struct gpio_v2_line_config *config,
-		   unsigned int loops)
+		   unsigned int loops,
+		   int verbosity)
 {
 	struct gpio_v2_line_values values;
 	char *chrdev_name;
@@ -62,16 +63,23 @@ int monitor_device(const char *device_name,
 		gpiotools_set_bit(&values.mask, i);
 	ret = gpiotools_get_values(lfd, &values);
 	if (ret < 0) {
-		fprintf(stderr,
-			"Failed to issue GPIO LINE GET VALUES IOCTL (%d)\n",
-			ret);
-		goto exit_line_close;
+		if (errno == EIO) {
+			fprintf(stdout,
+				"Failed to get line values. Function unimplemented, continuing\n");
+		} else {
+			ret = -errno;
+			fprintf(stderr,
+				"Failed to issue GPIO LINE GET VALUES IOCTL (%d)\n",
+				ret);
+			goto exit_line_close;
+		}
 	}
 
 	if (num_lines == 1) {
 		fprintf(stdout, "Monitoring line %d on %s\n", lines[0], device_name);
-		fprintf(stdout, "Initial line value: %d\n",
-			gpiotools_test_bit(values.bits, 0));
+		if (ret != -1)
+			fprintf(stdout, "Initial line value: %d\n",
+				gpiotools_test_bit(values.bits, 0));
 	} else {
 		fprintf(stdout, "Monitoring lines %d", lines[0]);
 		for (i = 1; i < num_lines - 1; i++)
@@ -91,8 +99,9 @@ int monitor_device(const char *device_name,
 
 		ret = read(lfd, &event, sizeof(event));
 		if (ret == -1) {
-			if (errno == -EAGAIN) {
-				fprintf(stderr, "nothing available\n");
+			if (errno == EAGAIN) {
+				if (verbosity >= 2)
+					fprintf(stdout, "nothing available\n");
 				continue;
 			} else {
 				ret = -errno;
@@ -117,8 +126,11 @@ int monitor_device(const char *device_name,
 		case GPIO_V2_LINE_EVENT_FALLING_EDGE:
 			fprintf(stdout, "falling edge");
 			break;
+		case GPIO_V2_LINE_EVENT_UNKNOWN_EDGE:
+			fprintf(stdout, "rising/falling edge");
+			break;
 		default:
-			fprintf(stdout, "unknown event");
+			fprintf(stdout, "unknown event spec: %x", event.id);
 		}
 		fprintf(stdout, "\n");
 
@@ -150,6 +162,7 @@ void print_usage(void)
 		"  -f         Listen for falling edges\n"
 		"  -w         Report the wall-clock time for events\n"
 		"  -b <n>     Debounce the line with period n microseconds\n"
+		"  -v	      Verbosity\n"
 		" [-c <n>]    Do <n> loops (optional, infinite loop if not stated)\n"
 		"  -?         This helptext\n"
 		"\n"
@@ -169,12 +182,13 @@ int main(int argc, char **argv)
 	unsigned int num_lines = 0;
 	unsigned int loops = 0;
 	struct gpio_v2_line_config config;
+	int verbosity = 0;
 	int c, attr, i;
 	unsigned long debounce_period_us = 0;
 
 	memset(&config, 0, sizeof(config));
 	config.flags = GPIO_V2_LINE_FLAG_INPUT;
-	while ((c = getopt(argc, argv, "c:n:o:b:dsrfw?")) != -1) {
+	while ((c = getopt(argc, argv, "c:n:o:b:dsrfwv?")) != -1) {
 		switch (c) {
 		case 'c':
 			loops = strtoul(optarg, NULL, 10);
@@ -208,6 +222,9 @@ int main(int argc, char **argv)
 		case 'w':
 			config.flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME;
 			break;
+		case 'v':
+			++verbosity;
+			break;
 		case '?':
 			print_usage();
 			return -1;
@@ -232,5 +249,6 @@ int main(int argc, char **argv)
 		       "falling edges\n");
 		config.flags |= EDGE_FLAGS;
 	}
-	return monitor_device(device_name, lines, num_lines, &config, loops);
+	return monitor_device(device_name, lines, num_lines, &config, loops,
+			      verbosity);
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 06/20] gpio: Add set_input and polling interface to GPIO lib code
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (4 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 05/20] tools: gpio: Add additional polling support to gpio-event-mon lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver lakshmi.sowjanya.d
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Some Intel Timed I/O devices do not implement IRQ functionality. Augment
read() interface to allow polling.

Move input setup functionality to a separate function simplifying
linereq_create() to add output code in subsequent commit.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpiolib-cdev.c | 52 +++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 4741bf34750b..cb6b9155884c 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1331,6 +1331,52 @@ static const struct file_operations line_fileops = {
 #endif
 };
 
+static int setup_input(struct linereq *lr, struct gpio_v2_line_config *lc,
+		       u32 line_no, unsigned int offset, u32 lflags)
+{
+	int err, ret;
+
+	/* Only one bias flag can be set. */
+	if (((lflags & GPIO_V2_LINE_FLAG_BIAS_DISABLED) &&
+	     (lflags & (GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN |
+			GPIO_V2_LINE_FLAG_BIAS_PULL_UP))) ||
+	    ((lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN) &&
+	     (lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP)))
+		return -EINVAL;
+
+	if (lflags & GPIO_V2_LINE_FLAG_ACTIVE_LOW)
+		set_bit(FLAG_ACTIVE_LOW, &lr->lines[line_no].desc->flags);
+	if (lflags & GPIO_V2_LINE_FLAG_BIAS_DISABLED)
+		set_bit(FLAG_BIAS_DISABLE, &lr->lines[line_no].desc->flags);
+	if (lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN)
+		set_bit(FLAG_PULL_DOWN, &lr->lines[line_no].desc->flags);
+	if (lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP)
+		set_bit(FLAG_PULL_UP, &lr->lines[line_no].desc->flags);
+
+	err = gpiod_direction_input(lr->lines[line_no].desc);
+	if (err)
+		return err;
+
+	ret = edge_detector_setup(&lr->lines[line_no], lc, line_no,
+				  lflags & GPIO_V2_LINE_EDGE_FLAGS);
+	if (ret < 0) {
+		if (ret != -ENXIO) {
+			if (lr->gdev->chip->setup_poll &&
+			    lr->gdev->chip->setup_poll(lr->gdev->chip, offset,
+						       &lflags) == 0 &&
+			    lr->gdev->chip->do_poll)
+				ret = 0;
+			else
+				return -ENODEV;
+		} else {
+			return -ENODEV;
+		}
+	}
+
+	lr->lines[line_no].eflags = lflags;
+	return ret;
+}
+
 static int linereq_create(struct gpio_device *gdev, void __user *ip)
 {
 	struct gpio_v2_line_request ulr;
@@ -1423,12 +1469,8 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 			if (ret)
 				goto out_free_linereq;
 		} else if (flags & GPIO_V2_LINE_FLAG_INPUT) {
-			ret = gpiod_direction_input(desc);
-			if (ret)
-				goto out_free_linereq;
+			ret = setup_input(lr, lc, i, offset, flags);
 
-			ret = edge_detector_setup(&lr->lines[i], lc, i,
-					flags & GPIO_V2_LINE_EDGE_FLAGS);
 			if (ret)
 				goto out_free_linereq;
 		}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (5 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 06/20] gpio: Add set_input and polling interface to GPIO lib code lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-09-16 21:42   ` Linus Walleij
  2021-08-24 16:47 ` [RFC PATCH v1 08/20] kernel: time: Add system time to system counter translation lakshmi.sowjanya.d
                   ` (13 subsequent siblings)
  20 siblings, 1 reply; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Intel Timed I/O hardware supports output scheduled in hardware. Enable
this functionality using GPIOlib

Adds GPIOlib generate_output() hook into the driver. The driver is
supplied with a timestamp in terms of realtime system clock (the same
used for input timestamping). The driver must know how to translate this
into a timebase meaningful for the hardware.

Adds userspace write() interface. Output can be selected using the line
event create ioctl. The write() interface takes a single timestamp
event request parameter. An output edge rising or falling is generated
for each event request.

The user application supplies a trigger time in terms of the realtime
clock the driver converts this into the corresponding ART clock value
that is used to 'arm' the output.

Work around device quirk that doesn't allow the output to be explicitly
set. Instead, count the output edges and insert an additional edge as
needed to reset the output to zero.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpio-intel-tio-pmc.c | 154 ++++++++++++++++++++++++++++--
 drivers/gpio/gpiolib-cdev.c       |  49 +++++++++-
 include/linux/gpio/driver.h       |   9 ++
 include/uapi/linux/gpio.h         |   8 ++
 4 files changed, 210 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-intel-tio-pmc.c b/drivers/gpio/gpio-intel-tio-pmc.c
index 7e5e61054dea..f57f521edc40 100644
--- a/drivers/gpio/gpio-intel-tio-pmc.c
+++ b/drivers/gpio/gpio-intel-tio-pmc.c
@@ -6,6 +6,7 @@
 
 #include <linux/acpi.h>
 #include <linux/debugfs.h>
+#include <linux/delay.h>
 #include <linux/gpio/driver.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -48,6 +49,7 @@ struct intel_pmc_tio_chip {
 	struct delayed_work input_work;
 	bool input_work_running;
 	bool systime_valid;
+	bool output_high;
 	unsigned int systime_index;
 	struct system_time_snapshot systime_snapshot[INPUT_SNAPSHOT_COUNT];
 	u64 last_event_count;
@@ -123,6 +125,38 @@ static inline void intel_pmc_tio_writel(struct intel_pmc_tio_chip *tio,
 #define INTEL_PMC_TIO_WR_REG(offset, value)(			\
 		intel_pmc_tio_writel((tio), (offset), (value)))
 
+/* Must hold mutex */
+static u32 intel_pmc_tio_disable(struct intel_pmc_tio_chip *tio)
+{
+	u32 ctrl;
+	u64 art;
+
+	ctrl = INTEL_PMC_TIO_RD_REG(TGPIOCTL);
+	if (!(ctrl & TGPIOCTL_DIR) && ctrl & TGPIOCTL_EN) {
+		/* 'compare' value is invalid */
+		art = read_art_time();
+		--art;
+		INTEL_PMC_TIO_WR_REG(TGPIOCOMPV31_0, art & 0xFFFFFFFF);
+		INTEL_PMC_TIO_WR_REG(TGPIOCOMPV63_32, art >> 32);
+		udelay(1);
+		tio->output_high = (INTEL_PMC_TIO_RD_REG(TGPIOEC31_0) & 0x1);
+	}
+
+	if (ctrl & TGPIOCTL_EN) {
+		ctrl &= ~TGPIOCTL_EN;
+		INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
+	}
+
+	return ctrl;
+}
+
+static void intel_pmc_tio_enable(struct intel_pmc_tio_chip *tio, u32 ctrl)
+{
+	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
+	ctrl |= TGPIOCTL_EN;
+	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
+}
+
 static void intel_pmc_tio_enable_input(struct intel_pmc_tio_chip *tio,
 				       u32 eflags)
 {
@@ -131,10 +165,6 @@ static void intel_pmc_tio_enable_input(struct intel_pmc_tio_chip *tio,
 
 	/* Disable */
 	ctrl = INTEL_PMC_TIO_RD_REG(TGPIOCTL);
-	ctrl &= ~TGPIOCTL_EN;
-	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
-
-	tio->last_event_count = 0;
 
 	/* Configure Input */
 	ctrl |= TGPIOCTL_DIR;
@@ -150,9 +180,7 @@ static void intel_pmc_tio_enable_input(struct intel_pmc_tio_chip *tio,
 		ctrl |= TGPIOCTL_EP_FALLING_EDGE;
 
 	/* Enable */
-	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
-	ctrl |= TGPIOCTL_EN;
-	INTEL_PMC_TIO_WR_REG(TGPIOCTL, ctrl);
+	intel_pmc_tio_enable(tio, ctrl);
 }
 
 static void intel_pmc_tio_input_work(struct work_struct *input_work)
@@ -293,6 +321,115 @@ static int intel_pmc_tio_do_poll(struct gpio_chip *chip, unsigned int offset,
 	return err;
 }
 
+static int intel_pmc_tio_insert_edge(struct intel_pmc_tio_chip *tio, u32 *ctrl)
+{
+	struct system_counterval_t sys_counter;
+	ktime_t trigger;
+	int err;
+	u64 art;
+
+	trigger = ktime_get_real();
+	trigger = ktime_add_ns(trigger, NSEC_PER_SEC / 20);
+
+	err = ktime_convert_real_to_system_counter(trigger, &sys_counter);
+	if (err)
+		return err;
+
+	err = convert_tsc_to_art(&sys_counter, &art);
+	if (err)
+		return err;
+
+	/* In disabled state */
+	*ctrl &= ~(TGPIOCTL_DIR | TGPIOCTL_PM);
+	*ctrl &= ~TGPIOCTL_EP;
+	*ctrl |= TGPIOCTL_EP_TOGGLE_EDGE;
+
+	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV31_0, art & 0xFFFFFFFF);
+	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV63_32, art >> 32);
+
+	intel_pmc_tio_enable(tio, *ctrl);
+
+	/* sleep for 100 milli-second */
+	msleep(2 * (MSEC_PER_SEC / 20));
+
+	*ctrl = intel_pmc_tio_disable(tio);
+
+	return 0;
+}
+
+static int intel_pmc_tio_direction_output(struct gpio_chip *chip, unsigned int offset,
+					  int value)
+{
+	struct intel_pmc_tio_chip *tio;
+	int err = 0;
+	u32 ctrl;
+	u64 art;
+
+	if (value)
+		return -EINVAL;
+
+	tio = gch_to_intel_pmc_tio(chip);
+
+	mutex_lock(&tio->lock);
+	ctrl = intel_pmc_tio_disable(tio);
+
+	/*
+	 * Make sure the output is zero'ed by inserting an edge as needed
+	 * Only need to worry about this when restarting output
+	 */
+	if (tio->output_high) {
+		err = intel_pmc_tio_insert_edge(tio, &ctrl);
+		if (err)
+			goto out;
+		tio->output_high = false;
+	}
+
+	/* Enable the device, be sure that the 'compare(COMPV)' value is invalid */
+	art = read_art_time();
+	--art;
+	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV31_0, art & 0xFFFFFFFF);
+	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV63_32, art >> 32);
+
+	ctrl &= ~(TGPIOCTL_DIR | TGPIOCTL_PM);
+	ctrl &= ~TGPIOCTL_EP;
+	ctrl |= TGPIOCTL_EP_TOGGLE_EDGE;
+
+	intel_pmc_tio_enable(tio, ctrl);
+
+out:
+	mutex_unlock(&tio->lock);
+
+	return err;
+}
+
+static int intel_pmc_tio_generate_output(struct gpio_chip *chip,
+					 unsigned int offset,
+					 struct gpio_output_event_data *data)
+{
+	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
+	ktime_t sys_realtime = ns_to_ktime(data->timestamp);
+	struct system_counterval_t sys_counter;
+	u64 art_timestamp;
+	int err;
+
+	err = ktime_convert_real_to_system_counter(sys_realtime, &sys_counter);
+	if (err)
+		return err;
+
+	err = convert_tsc_to_art(&sys_counter, &art_timestamp);
+	if (err)
+		return err;
+
+	mutex_lock(&tio->lock);
+
+	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV63_32, art_timestamp >> 32);
+	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV31_0, art_timestamp);
+
+	mutex_unlock(&tio->lock);
+
+	return 0;
+}
+
 static int intel_pmc_tio_probe(struct platform_device *pdev)
 {
 	struct intel_pmc_tio_chip *tio;
@@ -327,10 +464,13 @@ static int intel_pmc_tio_probe(struct platform_device *pdev)
 	tio->gch.base = -1;
 	tio->gch.setup_poll = intel_pmc_tio_setup_poll;
 	tio->gch.do_poll = intel_pmc_tio_do_poll;
+	tio->gch.generate_output = intel_pmc_tio_generate_output;
+	tio->gch.direction_output = intel_pmc_tio_direction_output;
 
 	platform_set_drvdata(pdev, tio);
 	mutex_init(&tio->lock);
 	INIT_DELAYED_WORK(&tio->input_work, intel_pmc_tio_input_work);
+	tio->output_high = false;
 
 	err = devm_gpiochip_add_data(&pdev->dev, &tio->gch, tio);
 	if (err < 0)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index cb6b9155884c..1df28a71f88b 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1221,6 +1221,29 @@ static __poll_t linereq_poll(struct file *file,
 	return events;
 }
 
+static ssize_t linereq_write(struct file *filep, const char __user *buf,
+			     size_t count, loff_t *f_ps)
+{
+	struct linereq *lr = filep->private_data;
+	struct gpio_output_event out_event;
+	struct gpio_output_event_data out_data;
+	int offset, err;
+
+	if (count < sizeof(struct gpio_output_event))
+		return -EINVAL;
+
+	if (copy_from_user(&out_event, buf, sizeof(out_event)))
+		return -EFAULT;
+
+	out_data.timestamp = out_event.timestamp;
+	offset = gpio_chip_hwgpio(lr->lines[0].desc);
+	err = lr->gdev->chip->generate_output(lr->gdev->chip, offset, &out_data);
+	if (err)
+		return err;
+
+	return sizeof(struct gpio_output_event);
+}
+
 static ssize_t linereq_read(struct file *file,
 			    char __user *buf,
 			    size_t count,
@@ -1302,6 +1325,8 @@ static void linereq_free(struct linereq *lr)
 
 	for (i = 0; i < lr->num_lines; i++) {
 		edge_detector_stop(&lr->lines[i]);
+		if (lr->lines[i].irq)
+			free_irq(lr->lines[i].irq, lr);
 		if (lr->lines[i].desc)
 			gpiod_free(lr->lines[i].desc);
 	}
@@ -1319,7 +1344,18 @@ static int linereq_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static const struct file_operations line_fileops = {
+static const struct file_operations line_output_fileops = {
+	.release = linereq_release,
+	.write = linereq_write,
+	.owner = THIS_MODULE,
+	.llseek = noop_llseek,
+	.unlocked_ioctl = linereq_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = linereq_ioctl_compat,
+#endif
+};
+
+static const struct file_operations line_input_fileops = {
 	.release = linereq_release,
 	.read = linereq_read,
 	.poll = linereq_poll,
@@ -1382,6 +1418,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 	struct gpio_v2_line_request ulr;
 	struct gpio_v2_line_config *lc;
 	unsigned int file_flags;
+	bool output = false;
 	struct linereq *lr;
 	struct file *file;
 	u64 flags;
@@ -1458,11 +1495,12 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 		if (ret < 0)
 			goto out_free_linereq;
 
+		output = flags & GPIO_V2_LINE_FLAG_OUTPUT;
 		/*
 		 * Lines have to be requested explicitly for input
 		 * or output, else the line will be treated "as is".
 		 */
-		if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
+		if (output) {
 			int val = gpio_v2_line_config_output_value(lc, i);
 
 			ret = gpiod_direction_output(desc, val);
@@ -1476,6 +1514,8 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 		}
 
 		file_flags = O_RDONLY | O_CLOEXEC;
+		file_flags |= output ? O_WRONLY : O_RDONLY;
+		file_flags |= (!output && !lr->lines[i].irq) ? O_NONBLOCK : 0;
 
 		blocking_notifier_call_chain(&desc->gdev->notifier,
 					     GPIO_V2_LINE_CHANGED_REQUESTED, desc);
@@ -1490,7 +1530,10 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 		goto out_free_linereq;
 	}
 
-	file = anon_inode_getfile("gpio-line", &line_fileops, lr,
+	file = anon_inode_getfile("gpio-line",
+				  output ? &line_output_fileops :
+				  &line_input_fileops,
+				  lr,
 				  file_flags);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index f5b971ad40bc..561e289434aa 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -18,6 +18,7 @@ struct seq_file;
 struct gpio_device;
 struct module;
 struct gpioevent_poll_data;
+struct gpio_output_event_data;
 enum gpiod_flags;
 enum gpio_lookup_flags;
 
@@ -310,6 +311,7 @@ struct gpio_irq_chip {
  *	event flags and driver returns accepted flags.
  * @do_poll: optional routine for devices that don't support interrupts.
  *	Returns event specification in data parameter.
+ * @generate_output: generate out event. Takes timestamp as input.
  * @base: identifies the first GPIO number handled by this chip;
  *	or, if negative during registration, requests dynamic ID allocation.
  *	DEPRECATION: providing anything non-negative and nailing the base
@@ -409,6 +411,9 @@ struct gpio_chip {
 	int                     (*do_poll)(struct gpio_chip *chip,
 					   unsigned int offset, u32 eflags,
 					   struct gpioevent_poll_data *data);
+	int                     (*generate_output)(struct gpio_chip *chip,
+						   unsigned int offset,
+						   struct gpio_output_event_data *data);
 
 	int			base;
 	u16			ngpio;
@@ -490,6 +495,10 @@ struct gpioevent_poll_data {
 	__u32 id;
 };
 
+struct gpio_output_event_data {
+	__u64 timestamp;
+};
+
 extern const char *gpiochip_is_requested(struct gpio_chip *gc,
 			unsigned int offset);
 
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index ed84805baee8..c39efc459b9f 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -298,6 +298,14 @@ struct gpio_v2_line_event {
 	__u32 padding[6];
 };
 
+/**
+ * struct gpio_output_event - Output event request
+ * @timestamp: When the time should occur
+ */
+struct gpio_output_event {
+	__u64 timestamp;
+};
+
 /*
  * ABI v1
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 08/20] kernel: time: Add system time to system counter translation
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (6 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-09-16 21:48   ` Linus Walleij
  2021-08-24 16:47 ` [RFC PATCH v1 09/20] arch: x86: Add TSC to ART translation lakshmi.sowjanya.d
                   ` (12 subsequent siblings)
  20 siblings, 1 reply; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

The GPIOlib event generation interface supplies a time in terms of the
realtime system clock. The driver must translate the system clock to
something meaningful to the hardware.

The Intel(R) PMC Timed I/O hardware uses ART to trigger events. For
most Intel(R) platforms that use TSC for timekeeping this added
function translates from system clock to TSC. The relation between TSC
and ART is easily derived from CPUID[15H].

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 include/linux/timekeeping.h |  3 ++
 kernel/time/timekeeping.c   | 63 +++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 78a98bdff76d..46ee524ca1a0 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -277,6 +277,9 @@ struct system_counterval_t {
 	struct clocksource	*cs;
 };
 
+extern int ktime_convert_real_to_system_counter(ktime_t sys_realtime,
+						struct system_counterval_t *ret);
+
 /*
  * Get cross timestamp between system clock and device clock
  */
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 8a364aa9881a..69e4be8f1bfb 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -369,6 +369,31 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 
 /* Timekeeper helper functions. */
 
+#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
+static u32 default_arch_gettimeoffset(void) { return 0; }
+u32 (*arch_gettimeoffset)(void) = default_arch_gettimeoffset;
+#else
+static inline u32 arch_gettimeoffset(void) { return 0; }
+#endif
+
+static inline u64 timekeeping_ns_delta_to_cycles(const struct tk_read_base *tkr,
+						 u64 nsec)
+{
+	u64 cycles;
+
+	/* If arch requires, subtract get_arch_timeoffset() */
+	cycles = nsec - arch_gettimeoffset();
+
+	if (fls64(cycles) + tkr->shift > sizeof(cycles) * 8)
+		return (typeof(cycles))-1;
+
+	cycles <<= tkr->shift;
+	cycles -= tkr->xtime_nsec;
+	do_div(cycles, tkr->mult);
+
+	return cycles;
+}
+
 static inline u64 timekeeping_delta_to_ns(const struct tk_read_base *tkr, u64 delta)
 {
 	u64 nsec;
@@ -1284,6 +1309,44 @@ int get_device_system_crosststamp(int (*get_time_fn)
 }
 EXPORT_SYMBOL_GPL(get_device_system_crosststamp);
 
+/**
+ * ktime_convert_real_to_system_counter - Convert system time to counter value
+ * @sys_realtime:	REALTIME clock value to convert
+ * @ret:		Computed system counter value with clocksource pointer
+ *
+ * Converts a supplied, future REALTIME clock value to the corresponding
+ *	system counter value. Returns current clock source in 'ret'.
+ */
+int ktime_convert_real_to_system_counter(ktime_t sys_realtime,
+					 struct system_counterval_t *ret)
+{
+	struct timekeeper *tk = &tk_core.timekeeper;
+	u64 ns_delta;
+	ktime_t base_real;
+	unsigned int seq;
+
+	do {
+		seq = read_seqcount_begin(&tk_core.seq);
+
+		base_real = ktime_add(tk->tkr_mono.base,
+				      tk_core.timekeeper.offs_real);
+		if (ktime_compare(sys_realtime, base_real) < 0)
+			return -EINVAL;
+
+		ret->cs = tk->tkr_mono.clock;
+		ns_delta = ktime_to_ns(ktime_sub(sys_realtime, base_real));
+		ret->cycles = timekeeping_ns_delta_to_cycles(&tk->tkr_mono,
+							     ns_delta);
+		if (ret->cycles == (typeof(ret->cycles))-1)
+			return -ERANGE;
+
+		ret->cycles += tk->tkr_mono.cycle_last;
+	} while (read_seqcount_retry(&tk_core.seq, seq));
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ktime_convert_real_to_system_counter);
+
 /**
  * do_settimeofday64 - Sets the time of day.
  * @ts:     pointer to the timespec64 variable containing the new time
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 09/20] arch: x86: Add TSC to ART translation
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (7 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 08/20] kernel: time: Add system time to system counter translation lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 10/20] tools: gpio: Add GPIO output generation user application lakshmi.sowjanya.d
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Add a function to convert TSC(Time Stamp Capture) time to ART.

The Intel(R) PMC Timed I/O device uses ART to trigger output events.

The TSC to ART translation converts the TSC value yielded by
ktime_convert_real_to_system_counter() to ART. The conversion is
required to program the COMPV register that will be compared against
the art time to generate output events.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 arch/x86/include/asm/tsc.h |  3 +++
 arch/x86/kernel/tsc.c      | 35 ++++++++++++++++++++++++++++-------
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index a50b0102e5c1..f1de7ede3ec9 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -8,6 +8,8 @@
 #include <asm/processor.h>
 #include <asm/cpufeature.h>
 
+struct system_counterval_t;
+
 /*
  * Standard way to access the cycle counter.
  */
@@ -29,6 +31,7 @@ static inline cycles_t get_cycles(void)
 }
 
 extern u64 read_art_time(void);
+extern int convert_tsc_to_art(const struct system_counterval_t *tsc, u64 *art);
 extern struct system_counterval_t convert_art_to_tsc(u64 art);
 extern struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns);
 
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index bbab6cf1a73b..040109228100 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1231,22 +1231,43 @@ int unsynchronized_tsc(void)
 }
 
 /*
- * Converts the current TSC to the current ART value using conversion
+ * Converts input TSC to the corresponding ART value using conversion
  * factors discovered by detect_art()
  */
-u64 read_art_time(void)
+int convert_tsc_to_art(const struct system_counterval_t *system_counter, u64 *art)
 {
-	u64 tsc, tmp, res, rem;
+	u64 tmp, res, rem;
+
+	if (system_counter->cs != art_related_clocksource)
+		return -EINVAL;
 
-	tsc = read_tsc(NULL) - art_to_tsc_offset;
-	rem = do_div(tsc, art_to_tsc_numerator);
+	res = system_counter->cycles - art_to_tsc_offset;
+	rem = do_div(res, art_to_tsc_numerator);
 
-	res = tsc * art_to_tsc_denominator;
+	*art = res * art_to_tsc_denominator;
 	tmp = rem * art_to_tsc_denominator;
 
 	do_div(tmp, art_to_tsc_numerator);
+	*art += tmp;
+
+	return 0;
+}
+EXPORT_SYMBOL(convert_tsc_to_art);
+
+/*
+ * Converts the current TSC to the current ART value using conversion
+ * factors discovered by detect_art()
+ */
+u64 read_art_time(void)
+{
+	struct system_counterval_t tsc;
+	u64 art = 0;
+
+	tsc.cs = art_related_clocksource;
+	tsc.cycles = read_tsc(NULL);
+	convert_tsc_to_art(&tsc, &art);
 
-	return res + tmp;
+	return art;
 }
 EXPORT_SYMBOL(read_art_time);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 10/20] tools: gpio: Add GPIO output generation user application
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (8 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 09/20] arch: x86: Add TSC to ART translation lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-09-16 21:52   ` Linus Walleij
  2021-08-24 16:47 ` [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib lakshmi.sowjanya.d
                   ` (10 subsequent siblings)
  20 siblings, 1 reply; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

Add GPIO user application - gpio_event_gen - to generate output using
output methods added to GPIO lib. The output produced is 1 Hz clock
aligned to the system clock using singly scheduled edges.

gpio_event_gen accepts similar arguments to gpio-event-mon.

Example output:
	$ gpio-event-gen -n gpiochip0 -o 0 -c 3
	Generating events on line 0 on gpiochip1
	clock realtime : 1612453529996832765
	GPIO EVENT TRIGGER: 1612453531000000000
	clock realtime 2 2 : 1612453531500000000
	GPIO EVENT TRIGGER: 1612453531500000000
	clock realtime 2 2 : 1612453532000000000
	GPIO EVENT TRIGGER: 1612453532000000000
	clock realtime 2 2 : 1612453532500000000

Produces 3 events of 1 Hz output on line 0 of chip/device 0.

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Co-developed-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 tools/gpio/.gitignore       |   1 +
 tools/gpio/Build            |   1 +
 tools/gpio/Makefile         |  11 ++-
 tools/gpio/gpio-event-gen.c | 191 ++++++++++++++++++++++++++++++++++++
 4 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100644 tools/gpio/gpio-event-gen.c

diff --git a/tools/gpio/.gitignore b/tools/gpio/.gitignore
index a00d604027a2..d5685cd0eb51 100644
--- a/tools/gpio/.gitignore
+++ b/tools/gpio/.gitignore
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 gpio-event-mon
+gpio-event-gen
 gpio-hammer
 gpio-watch
 lsgpio
diff --git a/tools/gpio/Build b/tools/gpio/Build
index 67c7b7f6a717..dc6a178c195a 100644
--- a/tools/gpio/Build
+++ b/tools/gpio/Build
@@ -2,4 +2,5 @@ gpio-utils-y += gpio-utils.o
 lsgpio-y += lsgpio.o gpio-utils.o
 gpio-hammer-y += gpio-hammer.o gpio-utils.o
 gpio-event-mon-y += gpio-event-mon.o gpio-utils.o
+gpio-event-gen-y += gpio-event-gen.o gpio-utils.o
 gpio-watch-y += gpio-watch.o
diff --git a/tools/gpio/Makefile b/tools/gpio/Makefile
index 440434027557..c9efaee76f28 100644
--- a/tools/gpio/Makefile
+++ b/tools/gpio/Makefile
@@ -18,7 +18,7 @@ MAKEFLAGS += -r
 
 override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include
 
-ALL_TARGETS := lsgpio gpio-hammer gpio-event-mon gpio-watch
+ALL_TARGETS := lsgpio gpio-hammer gpio-event-mon gpio-event-gen gpio-watch
 ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
 
 all: $(ALL_PROGRAMS)
@@ -66,6 +66,15 @@ $(GPIO_EVENT_MON_IN): prepare FORCE $(OUTPUT)gpio-utils-in.o
 $(OUTPUT)gpio-event-mon: $(GPIO_EVENT_MON_IN)
 	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
 
+#
+# gpio-event-gen
+#
+GPIO_EVENT_GEN_IN := $(OUTPUT)gpio-event-gen-in.o
+$(GPIO_EVENT_GEN_IN): prepare FORCE $(OUTPUT)gpio-utils-in.o
+	$(Q)$(MAKE) $(build)=gpio-event-gen
+$(OUTPUT)gpio-event-gen: $(GPIO_EVENT_GEN_IN)
+	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
+
 #
 # gpio-watch
 #
diff --git a/tools/gpio/gpio-event-gen.c b/tools/gpio/gpio-event-gen.c
new file mode 100644
index 000000000000..3d5ef47d79d0
--- /dev/null
+++ b/tools/gpio/gpio-event-gen.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * gpio-event-gen - generate GPIO line events from userspace
+ *
+ * Copyright (C) 2020 Intel Corporation
+ * Author: Christopher S Hall <christopher.s.hall@intel.com>
+ *
+ * Adapted from gpio-event-mon.c
+ * Copyright (C) 2016 Linus Walleij
+ *
+ * Usage:
+ *	gpio-event-gen -n <device-name> -o <offset>
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <inttypes.h>
+#include <linux/gpio.h>
+#include <poll.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <time.h>
+#include <unistd.h>
+
+#define NSEC_PER_SEC (1000000000ULL)
+#define TIMESPEC_TO_U64(x) (((uint64_t)(x).tv_sec) * NSEC_PER_SEC + (x).tv_nsec)
+#define U64_TO_TIMESPEC(x)						\
+	((struct timespec){	.tv_sec = (x) / NSEC_PER_SEC,		\
+				.tv_nsec = (x) % NSEC_PER_SEC})
+
+int sleep_until(uint64_t systime)
+{
+	struct timespec wait_until;
+
+	wait_until = U64_TO_TIMESPEC(systime);
+	return clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &wait_until, NULL);
+}
+
+int generate_events(const char *device_name,
+		    unsigned int line[],
+		    unsigned int num_lines,
+		    uint32_t flags,
+		    unsigned int loops)
+{
+	struct gpio_v2_line_request req;
+	struct gpio_v2_line_config config;
+	uint64_t trigger_time;
+	struct timespec now;
+	char *chrdev_name;
+	int ret, fd;
+	int i = 0;
+
+	ret = asprintf(&chrdev_name, "/dev/%s", device_name);
+	if (ret < 0)
+		return -ENOMEM;
+
+	fd = open(chrdev_name, 0);
+	if (fd == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to open %s\n", chrdev_name);
+		goto exit_close_error;
+	}
+
+	memset(&config, 0, sizeof(config));
+	config.flags = flags;
+
+	memset(&req, 0, sizeof(req));
+
+	for (i = 0; i < num_lines; i++)
+		req.offsets[i] = line[i];
+	req.num_lines = num_lines;
+
+	req.config = config;
+	strcpy(req.consumer, "gpio-event-gen");
+
+	ret = ioctl(fd, GPIO_V2_GET_LINE_IOCTL, &req);
+	if (ret == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to issue GET EVENT IOCTL (%d)\n", ret);
+		goto exit_close_error;
+	}
+
+	if (req.num_lines == 1) {
+		fprintf(stdout, "Generating events on line %u on %s\n",
+			line[0], device_name);
+	} else {
+		fprintf(stdout, "Generating events on %s for line %u",
+			device_name, line[0]);
+		for (i = 1; i < num_lines; i++)
+			fprintf(stdout, " line %u", line[i]);
+	}
+
+	clock_gettime(CLOCK_REALTIME, &now);
+	trigger_time = TIMESPEC_TO_U64(now);
+	trigger_time -= trigger_time % NSEC_PER_SEC;
+	trigger_time += 2 * NSEC_PER_SEC;
+	i = 0;
+	while (1) {
+		struct gpio_output_event out_event;
+
+		out_event.timestamp = trigger_time;
+		printf("GPIO EVENT TRIGGER: %llu\n", trigger_time);
+		ret = write(req.fd, &out_event, sizeof(out_event));
+		if (ret == -1) {
+			ret = -errno;
+			fprintf(stderr, "Failed to write event spec(%s)\n",
+				strerror(-ret));
+			break;
+		}
+
+		if (ret != sizeof(out_event)) {
+			fprintf(stderr, "Writing event spec failed\n");
+			ret = -EIO;
+			break;
+		}
+
+		sleep_until(trigger_time + NSEC_PER_SEC / 10);
+		trigger_time += NSEC_PER_SEC / 2;
+
+		i++;
+		if (i == loops)
+			break;
+	}
+
+exit_close_error:
+	if (close(fd) == -1)
+		perror("Failed to close GPIO character device file");
+	free(chrdev_name);
+	return ret;
+}
+
+void print_usage(void)
+{
+	fprintf(stderr, "Usage: gpio-event-gen [options]...;"
+		"Listen to events on GPIO lines, 0->1 1->0;"
+		"  -n <name>  Listen on GPIOs on a named device;"
+		"(must be stated);"
+		"  -o <n>     Offset to monitor;"
+		" [-c <n>]    Do <n> loops;"
+		"(optional, infinite loop if not stated);"
+		"  -?         This helptext;"
+		"Example: gpio-event-gen -n gpiochip0 -o 0"
+	);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t flags = GPIO_V2_LINE_FLAG_OUTPUT;
+	const char *device_name = NULL;
+	unsigned int lines[GPIO_V2_LINES_MAX];
+	unsigned int loops = 0;
+	int num_lines = 0;
+	int c;
+
+	while ((c = getopt(argc, argv, "c:n:o:dsrf?")) != -1) {
+		switch (c) {
+		case 'c':
+			loops = strtoul(optarg, NULL, 10);
+			break;
+		case 'n':
+			device_name = optarg;
+			break;
+		case 'o':
+			if (num_lines >= GPIO_V2_LINES_MAX) {
+				print_usage();
+				return -1;
+			}
+			lines[num_lines] = strtoul(optarg, NULL, 10);
+			num_lines++;
+			break;
+		case '?':
+			print_usage();
+			return -1;
+		}
+	}
+
+	if (!device_name || num_lines == -1) {
+		print_usage();
+		return -1;
+	}
+
+	return generate_events(device_name, lines, num_lines,
+			       flags, loops);
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (9 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 10/20] tools: gpio: Add GPIO output generation user application lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-09-22  9:53   ` Bartosz Golaszewski
  2021-08-24 16:47 ` [RFC PATCH v1 12/20] gpio: Add event count to Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
                   ` (9 subsequent siblings)
  20 siblings, 1 reply; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Add a flag for event count and an extended structure to capture the event
count when the flag is enabled.

Intel(R) PMC Timed I/O device has an event count register counting
the number of missed input edges. The register interface captures the
event count and timestamp of the last event. For an event rate
exceeding the rate that software can read events, the software can use
the missed event count to calculate average event rates.

The application requests one or both rising and falling edges when
initializing the interface. The count of the selected edge type is
optionally selected with an added event type flag. The event count is
returned in an extended buffer using the read() interface.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpiolib-cdev.c | 28 +++++++++++++++++++---------
 include/linux/gpio/driver.h |  1 +
 include/uapi/linux/gpio.h   | 12 ++++++++++++
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 1df28a71f88b..3b5719d5e2dc 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -518,7 +518,8 @@ struct linereq {
 	 GPIO_V2_LINE_DRIVE_FLAGS | \
 	 GPIO_V2_LINE_EDGE_FLAGS | \
 	 GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
-	 GPIO_V2_LINE_BIAS_FLAGS)
+	 GPIO_V2_LINE_BIAS_FLAGS | \
+	 GPIO_V2_LINE_FLAG_EVENT_COUNT)
 
 static void linereq_put_event(struct linereq *lr,
 			      struct gpio_v2_line_event *le)
@@ -1252,10 +1253,14 @@ static ssize_t linereq_read(struct file *file,
 	struct linereq *lr = file->private_data;
 	struct gpioevent_poll_data poll_data;
 	struct gpio_v2_line_event le;
+	size_t min_userbuf_size;
 	ssize_t bytes_read = 0;
 	int ret, offset;
 
-	if (count < sizeof(le))
+	min_userbuf_size = sizeof(le);
+	min_userbuf_size += lr->lines[0].eflags & GPIO_V2_LINE_FLAG_EVENT_COUNT ?
+					sizeof(struct gpio_v2_line_event_ext) : 0;
+	if (count < min_userbuf_size)
 		return -EINVAL;
 
 	/* Without an IRQ, we can only poll */
@@ -1270,12 +1275,17 @@ static ssize_t linereq_read(struct file *file,
 					      lr->lines[offset].eflags, &poll_data);
 		if (ret)
 			return ret;
-		event = kzalloc(sizeof(*event), GFP_KERNEL);
+		event = kzalloc(min_userbuf_size, GFP_KERNEL);
 		event->timestamp_ns = poll_data.timestamp;
 		event->id = poll_data.id;
-		if (copy_to_user(buf, (void *)&event, sizeof(event)))
-			return -EFAULT;
-		return sizeof(event);
+		if (lr->lines[offset].eflags & GPIO_V2_LINE_FLAG_EVENT_COUNT)
+			event->ext[0].event_count = poll_data.event_count;
+
+		ret = copy_to_user(buf, (void *)event, min_userbuf_size);
+		if (ret)
+			ret = -EFAULT;
+		kfree(event);
+		return ret ? ret : min_userbuf_size;
 	}
 
 	do {
@@ -1396,7 +1406,7 @@ static int setup_input(struct linereq *lr, struct gpio_v2_line_config *lc,
 	ret = edge_detector_setup(&lr->lines[line_no], lc, line_no,
 				  lflags & GPIO_V2_LINE_EDGE_FLAGS);
 	if (ret < 0) {
-		if (ret != -ENXIO) {
+		if (ret == -ENXIO) {
 			if (lr->gdev->chip->setup_poll &&
 			    lr->gdev->chip->setup_poll(lr->gdev->chip, offset,
 						       &lflags) == 0 &&
@@ -1513,7 +1523,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 				goto out_free_linereq;
 		}
 
-		file_flags = O_RDONLY | O_CLOEXEC;
+		file_flags = O_CLOEXEC;
 		file_flags |= output ? O_WRONLY : O_RDONLY;
 		file_flags |= (!output && !lr->lines[i].irq) ? O_NONBLOCK : 0;
 
@@ -1524,7 +1534,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 			offset);
 	}
 
-	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+	fd = get_unused_fd_flags(file_flags);
 	if (fd < 0) {
 		ret = fd;
 		goto out_free_linereq;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 561e289434aa..09637fcbfd52 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -493,6 +493,7 @@ struct gpio_chip {
 struct gpioevent_poll_data {
 	__u64 timestamp;
 	__u32 id;
+	__u32 event_count;
 };
 
 struct gpio_output_event_data {
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index c39efc459b9f..e7fff2a205ec 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -80,6 +80,7 @@ enum gpio_v2_line_flag {
 	GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN	= _BITULL(9),
 	GPIO_V2_LINE_FLAG_BIAS_DISABLED		= _BITULL(10),
 	GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME	= _BITULL(11),
+	GPIO_V2_LINE_FLAG_EVENT_COUNT           = _BITULL(12),
 };
 
 /**
@@ -270,6 +271,15 @@ enum gpio_v2_line_event_id {
 	GPIO_V2_LINE_EVENT_UNKNOWN_EDGE = 3,
 };
 
+/**
+ * struct gpio_v2_line_event_ext - Extended gpio line event
+ * @event_count: count of events
+ */
+struct gpio_v2_line_event_ext {
+	__u32 event_count;
+	__u32 reserved[3];
+};
+
 /**
  * struct gpio_v2_line_event - The actual event being pushed to userspace
  * @timestamp_ns: best estimate of time of event occurrence, in nanoseconds.
@@ -280,6 +290,7 @@ enum gpio_v2_line_event_id {
  * @line_seqno: the sequence number for this event in the sequence of
  * events on this particular line
  * @padding: reserved for future use
+ * @gpio_v2_line_event_ext: Extended gpio line event
  *
  * By default the @timestamp_ns is read from %CLOCK_MONOTONIC and is
  * intended to allow the accurate measurement of the time between events.
@@ -296,6 +307,7 @@ struct gpio_v2_line_event {
 	__u32 line_seqno;
 	/* Space reserved for future use. */
 	__u32 padding[6];
+	struct gpio_v2_line_event_ext ext[];
 };
 
 /**
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 12/20] gpio: Add event count to Intel(R) PMC Timed I/O driver
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (10 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 13/20] tools: gpio: Add event count capability to event monitor application lakshmi.sowjanya.d
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Implement event count interface added to GPIOlib in Intel PMC
Timed I/O device

The Intel(R) PMC Timed I/O device has an event count register counting
the number of missed input edges. The register interface captures the
event count and timestamp of the last event. For an event rate
exceeding the rate that software can read events, the software can use
the missed event count to calculate average event rates.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpio-intel-tio-pmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpio-intel-tio-pmc.c b/drivers/gpio/gpio-intel-tio-pmc.c
index f57f521edc40..7c4dd5c2661c 100644
--- a/drivers/gpio/gpio-intel-tio-pmc.c
+++ b/drivers/gpio/gpio-intel-tio-pmc.c
@@ -60,6 +60,7 @@ struct intel_pmc_tio_get_time_arg {
 	struct intel_pmc_tio_chip *tio;
 	u32 eflags;
 	u32 event_id;
+	u32 event_count;
 	u64 abs_event_count;
 };
 
@@ -276,6 +277,7 @@ static int intel_pmc_tio_get_time(ktime_t *device_time,
 
 	*system_counterval = convert_art_to_tsc(art_timestamp);
 	arg->abs_event_count = abs_event_count;
+	arg->event_count = rel_event_count;
 	arg->event_id = 0;
 	arg->event_id |= (flags & GPIO_V2_LINE_FLAG_EDGE_RISING) ?
 		GPIO_V2_LINE_EVENT_RISING_EDGE : 0;
@@ -310,6 +312,7 @@ static int intel_pmc_tio_do_poll(struct gpio_chip *chip, unsigned int offset,
 			data->timestamp = ktime_to_ns(xtstamp.sys_realtime);
 			data->id = arg.event_id;
 			tio->last_event_count = arg.abs_event_count;
+			data->event_count = arg.event_count;
 		}
 		if (!err || err == -EAGAIN)
 			break;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 13/20] tools: gpio: Add event count capability to event monitor application
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (11 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 12/20] gpio: Add event count to Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-09-16 21:57   ` Linus Walleij
  2021-08-24 16:47 ` [RFC PATCH v1 14/20] arch/x86: Add ART nanosecond to ART conversion lakshmi.sowjanya.d
                   ` (7 subsequent siblings)
  20 siblings, 1 reply; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Add -t command line flag requesting event count to the gpio-event-mon
application. If event count is unsupported an invalid argument error is
returned by GPIOlib and the application exits with an error. The event
count is printed with the event type and timestamp.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 tools/gpio/gpio-event-mon.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index d8f0bbf78728..4bdd6b3d6ad8 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -33,6 +33,9 @@ int monitor_device(const char *device_name,
 		   int verbosity)
 {
 	struct gpio_v2_line_values values;
+	struct gpio_v2_line_event *event;
+	bool req_event_count;
+	size_t event_size;
 	char *chrdev_name;
 	int cfd, lfd;
 	int ret;
@@ -55,6 +58,10 @@ int monitor_device(const char *device_name,
 		goto exit_device_close;
 	else
 		lfd = ret;
+	req_event_count = config->flags & GPIO_V2_LINE_FLAG_EVENT_COUNT;
+	event_size = sizeof(*event);
+	event_size += req_event_count ? sizeof(event->ext[0]) : 0;
+	event = alloca(event_size);
 
 	/* Read initial states */
 	values.mask = 0;
@@ -111,7 +118,7 @@ int monitor_device(const char *device_name,
 			}
 		}
 
-		if (ret != sizeof(event)) {
+		if (ret != event_size) {
 			fprintf(stderr, "Reading event failed\n");
 			ret = -EIO;
 			break;
@@ -133,6 +140,9 @@ int monitor_device(const char *device_name,
 			fprintf(stdout, "unknown event spec: %x", event.id);
 		}
 		fprintf(stdout, "\n");
+		if (req_event_count)
+			fprintf(stdout, "Event count: %u\n",
+				event.ext[0].event_count);
 
 		i++;
 		if (i == loops)
@@ -163,6 +173,7 @@ void print_usage(void)
 		"  -w         Report the wall-clock time for events\n"
 		"  -b <n>     Debounce the line with period n microseconds\n"
 		"  -v	      Verbosity\n"
+		"  -t         Request event count\n"
 		" [-c <n>]    Do <n> loops (optional, infinite loop if not stated)\n"
 		"  -?         This helptext\n"
 		"\n"
@@ -188,7 +199,7 @@ int main(int argc, char **argv)
 
 	memset(&config, 0, sizeof(config));
 	config.flags = GPIO_V2_LINE_FLAG_INPUT;
-	while ((c = getopt(argc, argv, "c:n:o:b:dsrfwv?")) != -1) {
+	while ((c = getopt(argc, argv, "c:n:o:b:dsrfwvt?")) != -1) {
 		switch (c) {
 		case 'c':
 			loops = strtoul(optarg, NULL, 10);
@@ -225,6 +236,9 @@ int main(int argc, char **argv)
 		case 'v':
 			++verbosity;
 			break;
+		case 't':
+			config.flags |= GPIO_V2_LINE_FLAG_EVENT_COUNT;
+			break;
 		case '?':
 			print_usage();
 			return -1;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 14/20] arch/x86: Add ART nanosecond to ART conversion
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (12 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 13/20] tools: gpio: Add event count capability to event monitor application lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 15/20] pwm: Add capability for PWM Driver managed state lakshmi.sowjanya.d
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

The PWM 'apply' interface uses units of nominal nanoseconds for period and
the Intel(R) PMC Timed I/O hardware requires a period in terms of ART
cycles. Add a function using ART conversion coefficients determined at boot
time by the TSC initialization code to convert from ART in nominal
nanoseconds to ART cycles.

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 arch/x86/include/asm/tsc.h |  1 +
 arch/x86/kernel/tsc.c      | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index f1de7ede3ec9..e53507df92e1 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -34,6 +34,7 @@ extern u64 read_art_time(void);
 extern int convert_tsc_to_art(const struct system_counterval_t *tsc, u64 *art);
 extern struct system_counterval_t convert_art_to_tsc(u64 art);
 extern struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns);
+extern u64 convert_art_ns_to_art(u64 art_ns);
 
 extern void tsc_early_init(void);
 extern void tsc_init(void);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 040109228100..381d15894dd2 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1329,6 +1329,38 @@ struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns)
 }
 EXPORT_SYMBOL(convert_art_ns_to_tsc);
 
+/*
+ * convert_art_ns_to_art() - Convert ART in nanoseconds to ART
+ * @art_ns: ART (Always Running Timer) in nominal nanoseconds
+ *
+ * Computes the ART cycles given the duration in nominal nanoseconds
+ *
+ * This is valid when CPU feature flag X86_FEATURE_TSC_KNOWN_FREQ is set
+ * indicating the tsc_khz is derived from CPUID[15H]. Drivers should check
+ * that this flag is set before conversion to ART is attempted.
+ *
+ * Return:
+ *	u64 ART value rounded to nearest cycle corresponding to nanosecond
+ *	duration input
+ */
+u64 convert_art_ns_to_art(u64 art_ns)
+{
+	u64 tmp, res, rem;
+	u32 crystal_khz;
+
+	crystal_khz = (tsc_khz * art_to_tsc_denominator) /
+		art_to_tsc_numerator;
+
+	rem = do_div(art_ns, USEC_PER_SEC);
+	res = art_ns * crystal_khz;
+	tmp = rem * crystal_khz;
+
+	rem = do_div(tmp, USEC_PER_SEC);
+	res += rem < USEC_PER_SEC / 2 ? tmp : tmp + 1;
+
+	return res;
+}
+EXPORT_SYMBOL(convert_art_ns_to_art);
 
 static void tsc_refine_calibration_work(struct work_struct *work);
 static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 15/20] pwm: Add capability for PWM Driver managed state
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (13 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 14/20] arch/x86: Add ART nanosecond to ART conversion lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-09-16 22:00   ` Linus Walleij
  2021-08-24 16:47 ` [RFC PATCH v1 16/20] gpio: Add PWM capabilities to Intel(R) PMC TIO driver lakshmi.sowjanya.d
                   ` (5 subsequent siblings)
  20 siblings, 1 reply; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

Add additional flag that can be set by drivers to indicate that the
driver will manage its own PWM state. When calling pwm_ops.apply the
driver applies the requested state change to the pwm_device reconciling,
if possible, any conflicting requests.

Intel(R) Timed I/O devices support very limited PWM capabilities. The
duty cycle must always be 50% of the period. When changing one parameter
at a time through the sysfs interface, it isn't possible for the user or
the PWM subsystem to maintain this relation.

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/pwm/core.c  | 3 ++-
 include/linux/pwm.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 35e894f4a379..c658585ac3bc 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -554,7 +554,8 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 
 		trace_pwm_apply(pwm, state);
 
-		pwm->state = *state;
+		if (!test_bit(PWMF_DRIVERMANAGEDSTATE, &pwm->flags))
+			pwm->state = *state;
 
 		/*
 		 * only do this after pwm->state was applied as some
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index a0b7e43049d5..d805fee81e2c 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -46,6 +46,7 @@ struct pwm_args {
 enum {
 	PWMF_REQUESTED = 1 << 0,
 	PWMF_EXPORTED = 1 << 1,
+	PWMF_DRIVERMANAGEDSTATE = 1 << 2,
 };
 
 /*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 16/20] gpio: Add PWM capabilities to Intel(R) PMC TIO driver
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (14 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 15/20] pwm: Add capability for PWM Driver managed state lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 17/20] pwm: Add second alignment to the core PWM interface lakshmi.sowjanya.d
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Add minimal PWM capabilities to the Intel Timed I/O driver. Requires the
extended flags interface allowing the driver to manage PWM state
because only 50% duty cycle is supported. The PWM function is primarily
used to export a clock using this device.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpio-intel-tio-pmc.c | 249 +++++++++++++++++++++++++++---
 1 file changed, 230 insertions(+), 19 deletions(-)

diff --git a/drivers/gpio/gpio-intel-tio-pmc.c b/drivers/gpio/gpio-intel-tio-pmc.c
index 7c4dd5c2661c..f8981e1e92a4 100644
--- a/drivers/gpio/gpio-intel-tio-pmc.c
+++ b/drivers/gpio/gpio-intel-tio-pmc.c
@@ -7,10 +7,15 @@
 #include <linux/acpi.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
+
+#include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
+#include <linux/gpio/machine.h>
+
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/pwm.h>
 #include <uapi/linux/gpio.h>
 
 #define TGPIOCTL		0x00
@@ -54,6 +59,14 @@ struct intel_pmc_tio_chip {
 	struct system_time_snapshot systime_snapshot[INPUT_SNAPSHOT_COUNT];
 	u64 last_event_count;
 	u64 last_art_timestamp;
+	u64 last_art_period;
+	u32 half_period;
+};
+
+struct intel_pmc_tio_pwm {
+	struct pwm_chip pch;
+	struct intel_pmc_tio_chip *tio;
+	struct gpio_desc *gpiod;
 };
 
 struct intel_pmc_tio_get_time_arg {
@@ -64,6 +77,9 @@ struct intel_pmc_tio_get_time_arg {
 	u64 abs_event_count;
 };
 
+#define pch_to_intel_pmc_tio_pwm(i) \
+	(container_of((i), struct intel_pmc_tio_pwm, pch))
+
 #define gch_to_intel_pmc_tio(i)					\
 	(container_of((i), struct intel_pmc_tio_chip, gch))
 
@@ -360,20 +376,17 @@ static int intel_pmc_tio_insert_edge(struct intel_pmc_tio_chip *tio, u32 *ctrl)
 	return 0;
 }
 
-static int intel_pmc_tio_direction_output(struct gpio_chip *chip, unsigned int offset,
-					  int value)
+static int _intel_pmc_tio_direction_output(struct intel_pmc_tio_chip *tio,
+					   u32 offset, int value,
+					   u64 period)
 {
-	struct intel_pmc_tio_chip *tio;
-	int err = 0;
 	u32 ctrl;
+	int err;
 	u64 art;
 
 	if (value)
 		return -EINVAL;
 
-	tio = gch_to_intel_pmc_tio(chip);
-
-	mutex_lock(&tio->lock);
 	ctrl = intel_pmc_tio_disable(tio);
 
 	/*
@@ -383,7 +396,7 @@ static int intel_pmc_tio_direction_output(struct gpio_chip *chip, unsigned int o
 	if (tio->output_high) {
 		err = intel_pmc_tio_insert_edge(tio, &ctrl);
 		if (err)
-			goto out;
+			return err;
 		tio->output_high = false;
 	}
 
@@ -394,27 +407,48 @@ static int intel_pmc_tio_direction_output(struct gpio_chip *chip, unsigned int o
 	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV63_32, art >> 32);
 
 	ctrl &= ~(TGPIOCTL_DIR | TGPIOCTL_PM);
+	if (period != 0) {
+		ctrl |= TGPIOCTL_PM;
+		INTEL_PMC_TIO_WR_REG(TGPIOPIV31_0, period & 0xFFFFFFFF);
+		INTEL_PMC_TIO_WR_REG(TGPIOPIV63_32, period >> 32);
+	}
+
 	ctrl &= ~TGPIOCTL_EP;
 	ctrl |= TGPIOCTL_EP_TOGGLE_EDGE;
 
 	intel_pmc_tio_enable(tio, ctrl);
 
-out:
+	return 0;
+}
+
+static int intel_pmc_tio_direction_output(struct gpio_chip *chip,
+					  unsigned int offset, int value)
+{
+	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
+	int ret;
+
+	mutex_lock(&tio->lock);
+	ret =  _intel_pmc_tio_direction_output(tio, offset, value, 0);
 	mutex_unlock(&tio->lock);
 
-	return err;
+	return ret;
 }
 
-static int intel_pmc_tio_generate_output(struct gpio_chip *chip,
-					 unsigned int offset,
-					 struct gpio_output_event_data *data)
+static int _intel_pmc_tio_generate_output(struct intel_pmc_tio_chip *tio,
+					  unsigned int offset, u64 timestamp)
 {
-	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
-	ktime_t sys_realtime = ns_to_ktime(data->timestamp);
 	struct system_counterval_t sys_counter;
+	ktime_t sys_realtime;
 	u64 art_timestamp;
 	int err;
 
+	if (timestamp != 0) {
+		sys_realtime = ns_to_ktime(timestamp);
+	} else {
+		sys_realtime = ktime_get_real();
+		sys_realtime = ktime_add_ns(sys_realtime, NSEC_PER_SEC / 20);
+	}
+
 	err = ktime_convert_real_to_system_counter(sys_realtime, &sys_counter);
 	if (err)
 		return err;
@@ -423,18 +457,177 @@ static int intel_pmc_tio_generate_output(struct gpio_chip *chip,
 	if (err)
 		return err;
 
-	mutex_lock(&tio->lock);
-
 	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV63_32, art_timestamp >> 32);
 	INTEL_PMC_TIO_WR_REG(TGPIOCOMPV31_0, art_timestamp);
 
+	return 0;
+}
+
+static int intel_pmc_tio_generate_output(struct gpio_chip *chip,
+					 unsigned int offset,
+					 struct gpio_output_event_data *output_data)
+{
+	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
+	int ret;
+
+	mutex_lock(&tio->lock);
+	ret =  _intel_pmc_tio_generate_output
+		(tio, offset, output_data->timestamp);
 	mutex_unlock(&tio->lock);
 
-	return 0;
+	return ret;
+}
+
+static int intel_pmc_tio_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct intel_pmc_tio_pwm *tio_pwm = pch_to_intel_pmc_tio_pwm(chip);
+	struct intel_pmc_tio_chip *tio = tio_pwm->tio;
+	int ret = 0;
+
+	mutex_lock(&tio->lock);
+
+	if (tio_pwm->gpiod) {
+		ret = -EBUSY;
+	} else {
+		struct gpio_desc *gpiod;
+
+		gpiod = gpiochip_request_own_desc
+			(&tio->gch, pwm->hwpwm, "intel-pmc-tio-pwm", 0, 0);
+		if (IS_ERR(gpiod)) {
+			ret = PTR_ERR(gpiod);
+			goto out;
+		}
+
+		tio_pwm->gpiod = gpiod;
+	}
+
+out:
+	mutex_unlock(&tio->lock);
+	return ret;
 }
 
+#define MIN_ART_PERIOD (3)
+
+static int intel_pmc_tio_pwm_apply(struct pwm_chip *chip,
+				   struct pwm_device *pwm,
+				   const struct pwm_state *state)
+{
+	struct intel_pmc_tio_pwm *tio_pwm = pch_to_intel_pmc_tio_pwm(chip);
+	struct intel_pmc_tio_chip *tio = tio_pwm->tio;
+	bool start_output, change_period;
+	u64 art_period;
+	int ret = 0;
+
+	/* Only support 'normal' polarity */
+	if (state->polarity != PWM_POLARITY_NORMAL)
+		return -EINVAL;
+
+	mutex_lock(&tio->lock);
+
+	if (!state->enabled) {
+		if (pwm->state.enabled) {
+			intel_pmc_tio_disable(tio);
+			pwm->state.enabled = false;
+		}
+	}
+
+	/* 50% duty cycle only */
+	if (pwm->state.period != state->period &&
+	    pwm->state.duty_cycle != state->duty_cycle &&
+	    state->duty_cycle != state->period / 2) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	change_period = state->period != pwm->state.period ||
+		state->duty_cycle != pwm->state.duty_cycle ? state->enabled : false;
+
+	if (pwm->state.period != state->period) {
+		pwm->state.period = state->period;
+		pwm->state.duty_cycle = state->period / 2;
+	} else if (pwm->state.duty_cycle != state->duty_cycle) {
+		pwm->state.duty_cycle = state->duty_cycle;
+		pwm->state.period = state->duty_cycle * 2;
+	}
+
+	start_output = state->enabled && !pwm->state.enabled;
+	if (start_output || change_period) {
+		art_period = convert_art_ns_to_art(pwm->state.duty_cycle);
+		if (art_period < MIN_ART_PERIOD) {
+			ret = -EINVAL;
+			goto out;
+		}
+		tio->half_period = pwm->state.duty_cycle;
+	}
+
+	if (start_output) {
+		u64 start_time;
+		u32 nsec;
+
+		pwm->state.enabled = true;
+		start_time = ktime_get_real_ns();
+		div_u64_rem(start_time, NSEC_PER_SEC, &nsec);
+		start_time -= nsec;
+		start_time += 2 * NSEC_PER_SEC;
+		_intel_pmc_tio_direction_output(tio, pwm->hwpwm, 0, art_period);
+		ret = _intel_pmc_tio_generate_output(tio, pwm->hwpwm,
+						     start_time);
+		if (ret)
+			goto out;
+	} else if (change_period && tio->last_art_period != art_period) {
+		INTEL_PMC_TIO_WR_REG(TGPIOPIV31_0, art_period & 0xFFFFFFFF);
+		INTEL_PMC_TIO_WR_REG(TGPIOPIV63_32, art_period >> 32);
+		tio->last_art_period = art_period;
+	}
+
+out:
+	mutex_unlock(&tio->lock);
+
+	return ret;
+}
+
+/* Get initial state */
+static void intel_pmc_tio_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+					struct pwm_state *state)
+{
+	struct intel_pmc_tio_pwm *tio_pwm = pch_to_intel_pmc_tio_pwm(chip);
+	struct intel_pmc_tio_chip *tio = tio_pwm->tio;
+	u32 ctrl;
+
+	mutex_lock(&tio->lock);
+
+	ctrl = INTEL_PMC_TIO_RD_REG(TGPIOCTL);
+	state->enabled = ctrl & TGPIOCTL_EN && ctrl & TGPIOCTL_PM &&
+			!(ctrl & TGPIOCTL_DIR) ? true : false;
+
+	state->duty_cycle = tio->half_period;
+	state->period = state->duty_cycle * 2;
+
+	mutex_unlock(&tio->lock);
+}
+
+static void intel_pmc_tio_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct intel_pmc_tio_pwm *tio_pwm = pch_to_intel_pmc_tio_pwm(chip);
+	struct intel_pmc_tio_chip *tio = tio_pwm->tio;
+
+	tio->half_period = pwm->state.duty_cycle;
+
+	gpiochip_free_own_desc(tio_pwm->gpiod);
+	tio_pwm->gpiod = NULL;
+}
+
+static const struct pwm_ops intel_pmc_tio_pwm_ops = {
+	.request = intel_pmc_tio_pwm_request,
+	.free = intel_pmc_tio_pwm_free,
+	.apply = intel_pmc_tio_pwm_apply,
+	.get_state = intel_pmc_tio_pwm_get_state,
+	.owner = THIS_MODULE,
+};
+
 static int intel_pmc_tio_probe(struct platform_device *pdev)
 {
+	struct intel_pmc_tio_pwm *tio_pwm;
 	struct intel_pmc_tio_chip *tio;
 	int err;
 
@@ -479,11 +672,29 @@ static int intel_pmc_tio_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto out_recurse_remove_tio_root;
 
+	tio_pwm = devm_kzalloc(&pdev->dev, sizeof(*tio_pwm), GFP_KERNEL);
+	if (!tio_pwm) {
+		err = -ENOMEM;
+		goto out_recurse_remove_tio_root;
+	}
+
+	tio_pwm->tio = tio;
+	tio_pwm->pch.dev = &pdev->dev;
+	tio_pwm->pch.ops = &intel_pmc_tio_pwm_ops;
+	tio_pwm->pch.npwm = GPIO_COUNT;
+	tio_pwm->pch.base = -1;
+
+	err = pwmchip_add(&tio_pwm->pch);
+	if (err)
+		goto out_recurse_remove_tio_root;
+
+	/* Make sure tio and device state are sync'd to a reasonable value */
+	tio->half_period = NSEC_PER_SEC / 2;
+
 	return 0;
 
 out_recurse_remove_tio_root:
 	debugfs_remove_recursive(tio->root);
-
 	return err;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 17/20] pwm: Add second alignment to the core PWM interface
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (15 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 16/20] gpio: Add PWM capabilities to Intel(R) PMC TIO driver lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:47 ` [RFC PATCH v1 18/20] gpio: Add PWM alignment support to the Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

The Intel(R) PMC Timed I/O driver uses the PWM interface to export a
clock from the platform. Normally, PWM output is not phase aligned to
any clock. To remedy this: add an additional PWM state parameter
specifying alignment in nanoseconds within the output period.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/pwm/core.c  |  7 +++++--
 drivers/pwm/sysfs.c | 37 +++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h |  2 ++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index c658585ac3bc..5d6c64916e51 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -271,6 +271,7 @@ int pwmchip_add(struct pwm_chip *chip)
 		pwm->chip = chip;
 		pwm->pwm = chip->base + i;
 		pwm->hwpwm = i;
+		pwm->state.alignment = 0;
 
 		radix_tree_insert(&pwm_tree, pwm->pwm, pwm);
 	}
@@ -535,7 +536,8 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 	int err;
 
 	if (!pwm || !state || !state->period ||
-	    state->duty_cycle > state->period)
+	    state->duty_cycle > state->period ||
+	    state->alignment >= state->period)
 		return -EINVAL;
 
 	chip = pwm->chip;
@@ -544,7 +546,8 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 	    state->duty_cycle == pwm->state.duty_cycle &&
 	    state->polarity == pwm->state.polarity &&
 	    state->enabled == pwm->state.enabled &&
-	    state->usage_power == pwm->state.usage_power)
+	    state->usage_power == pwm->state.usage_power &&
+	    state->alignment == pwm->state.alignment)
 		return 0;
 
 	if (chip->ops->apply) {
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 9903c3a7eced..5d020618d8ba 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -33,6 +33,41 @@ static struct pwm_device *child_to_pwm_device(struct device *child)
 	return export->pwm;
 }
 
+static ssize_t alignment_show(struct device *child,
+			      struct device_attribute *attr,
+			      char *buf)
+{
+	const struct pwm_device *pwm = child_to_pwm_device(child);
+	struct pwm_state state;
+
+	pwm_get_state(pwm, &state);
+
+	return sprintf(buf, "%llu\n", state.alignment);
+}
+
+static ssize_t alignment_store(struct device *child,
+			       struct device_attribute *attr,
+			       const char *buf, size_t size)
+{
+	struct pwm_export *export = child_to_pwm_export(child);
+	struct pwm_device *pwm = export->pwm;
+	struct pwm_state state;
+	unsigned int val;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	mutex_lock(&export->lock);
+	pwm_get_state(pwm, &state);
+	state.alignment = val;
+	ret = pwm_apply_state(pwm, &state);
+	mutex_unlock(&export->lock);
+
+	return ret ? : size;
+}
+
 static ssize_t period_show(struct device *child,
 			   struct device_attribute *attr,
 			   char *buf)
@@ -219,6 +254,7 @@ static DEVICE_ATTR_RW(period);
 static DEVICE_ATTR_RW(duty_cycle);
 static DEVICE_ATTR_RW(enable);
 static DEVICE_ATTR_RW(polarity);
+static DEVICE_ATTR_RW(alignment);
 static DEVICE_ATTR_RO(capture);
 
 static struct attribute *pwm_attrs[] = {
@@ -226,6 +262,7 @@ static struct attribute *pwm_attrs[] = {
 	&dev_attr_duty_cycle.attr,
 	&dev_attr_enable.attr,
 	&dev_attr_polarity.attr,
+	&dev_attr_alignment.attr,
 	&dev_attr_capture.attr,
 	NULL
 };
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index d805fee81e2c..17ca1ea7ba94 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -59,6 +59,7 @@ enum {
  *               output but has more freedom regarding signal form.
  *               If supported, the signal can be optimized, for example to
  *               improve EMI by phase shifting individual channels.
+ * @alignment: offset in ns to device clock second
  */
 struct pwm_state {
 	u64 period;
@@ -66,6 +67,7 @@ struct pwm_state {
 	enum pwm_polarity polarity;
 	bool enabled;
 	bool usage_power;
+	u64 alignment;
 };
 
 /**
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 18/20] gpio: Add PWM alignment support to the Intel(R) PMC Timed I/O driver
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (16 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 17/20] pwm: Add second alignment to the core PWM interface lakshmi.sowjanya.d
@ 2021-08-24 16:47 ` lakshmi.sowjanya.d
  2021-08-24 16:48 ` [RFC PATCH v1 19/20] gpio: Add GPIO monitor line to Intel(R) Timed I/O Driver lakshmi.sowjanya.d
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:47 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Add capability to align PWM outbput with the realtime system clock by
adding an 'alignment' paramerter.  The realtime system clock is used
because it is already used for timestamping in GPIOlib and is easily
relatable to ART which drives the logic.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpio-intel-tio-pmc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-intel-tio-pmc.c b/drivers/gpio/gpio-intel-tio-pmc.c
index f8981e1e92a4..1b0eea7b3b2f 100644
--- a/drivers/gpio/gpio-intel-tio-pmc.c
+++ b/drivers/gpio/gpio-intel-tio-pmc.c
@@ -56,11 +56,12 @@ struct intel_pmc_tio_chip {
 	bool systime_valid;
 	bool output_high;
 	unsigned int systime_index;
+	u32 half_period;
+	u32 alignment;
 	struct system_time_snapshot systime_snapshot[INPUT_SNAPSHOT_COUNT];
 	u64 last_event_count;
 	u64 last_art_timestamp;
 	u64 last_art_period;
-	u32 half_period;
 };
 
 struct intel_pmc_tio_pwm {
@@ -550,6 +551,7 @@ static int intel_pmc_tio_pwm_apply(struct pwm_chip *chip,
 		pwm->state.period = state->duty_cycle * 2;
 	}
 
+	pwm->state.alignment = state->alignment;
 	start_output = state->enabled && !pwm->state.enabled;
 	if (start_output || change_period) {
 		art_period = convert_art_ns_to_art(pwm->state.duty_cycle);
@@ -566,8 +568,9 @@ static int intel_pmc_tio_pwm_apply(struct pwm_chip *chip,
 
 		pwm->state.enabled = true;
 		start_time = ktime_get_real_ns();
-		div_u64_rem(start_time, NSEC_PER_SEC, &nsec);
+		div_u64_rem(start_time, pwm->state.period, &nsec);
 		start_time -= nsec;
+		start_time += pwm->state.alignment;
 		start_time += 2 * NSEC_PER_SEC;
 		_intel_pmc_tio_direction_output(tio, pwm->hwpwm, 0, art_period);
 		ret = _intel_pmc_tio_generate_output(tio, pwm->hwpwm,
@@ -602,6 +605,7 @@ static void intel_pmc_tio_pwm_get_state(struct pwm_chip *chip, struct pwm_device
 
 	state->duty_cycle = tio->half_period;
 	state->period = state->duty_cycle * 2;
+	state->alignment = tio->alignment;
 
 	mutex_unlock(&tio->lock);
 }
@@ -612,6 +616,7 @@ static void intel_pmc_tio_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm
 	struct intel_pmc_tio_chip *tio = tio_pwm->tio;
 
 	tio->half_period = pwm->state.duty_cycle;
+	tio->alignment = pwm->state.alignment;
 
 	gpiochip_free_own_desc(tio_pwm->gpiod);
 	tio_pwm->gpiod = NULL;
@@ -689,6 +694,7 @@ static int intel_pmc_tio_probe(struct platform_device *pdev)
 		goto out_recurse_remove_tio_root;
 
 	/* Make sure tio and device state are sync'd to a reasonable value */
+	tio->alignment = 0;
 	tio->half_period = NSEC_PER_SEC / 2;
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 19/20] gpio: Add GPIO monitor line to Intel(R) Timed I/O Driver
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (17 preceding siblings ...)
  2021-08-24 16:47 ` [RFC PATCH v1 18/20] gpio: Add PWM alignment support to the Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
@ 2021-08-24 16:48 ` lakshmi.sowjanya.d
  2021-08-24 16:48 ` [RFC PATCH v1 20/20] tools: gpio: Add PWM monitor application lakshmi.sowjanya.d
  2021-09-16 21:21 ` [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC Linus Walleij
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:48 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

The device's PWM output is initially aligned with the system clock using
the 'alignment' state parameter. Small differences between
the system clock rate and nominal ART rate cause the Timed I/O clock to
drift over time.

To solve this: add a secondary input only 'virtual' GPIO line to monitor
the PWM output. This is possible because the I/O lines in the Intel PMC
Timed I/O driver can simultaneously generate output and capture input.
By timestamping the PWM output using the monitor line, the output can be
adjusted using a PI controller to maintain alignment with the system
clock (or any related clock).

NOTE: It is not possible to capture either rising or falling edges
using the 'monitor' interface. Requesting anything other than any/all
edges results in an error.

Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 drivers/gpio/gpio-intel-tio-pmc.c | 82 +++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-intel-tio-pmc.c b/drivers/gpio/gpio-intel-tio-pmc.c
index 1b0eea7b3b2f..b0db6f545ec6 100644
--- a/drivers/gpio/gpio-intel-tio-pmc.c
+++ b/drivers/gpio/gpio-intel-tio-pmc.c
@@ -40,10 +40,14 @@
 #define TGPIOCTL_PM			BIT(4)
 
 #define DRIVER_NAME		"intel-pmc-tio"
-#define GPIO_COUNT		1
+#define GPIO_COUNT		2
 #define INPUT_SNAPSHOT_FREQ	8
 #define INPUT_SNAPSHOT_COUNT	3
 
+#define EDGE_FLAGS \
+	(GPIO_V2_LINE_FLAG_EDGE_RISING | \
+	 GPIO_V2_LINE_FLAG_EDGE_FALLING)
+
 struct intel_pmc_tio_chip {
 	struct gpio_chip gch;
 	struct platform_device *pdev;
@@ -54,7 +58,9 @@ struct intel_pmc_tio_chip {
 	struct delayed_work input_work;
 	bool input_work_running;
 	bool systime_valid;
+	bool input;
 	bool output_high;
+	bool monitor;
 	unsigned int systime_index;
 	u32 half_period;
 	u32 alignment;
@@ -243,14 +249,25 @@ static int intel_pmc_tio_setup_poll(struct gpio_chip *chip, unsigned int offset,
 {
 	struct intel_pmc_tio_chip *tio;
 
-	if (offset != 0)
+	if (offset > 1)
+		return -EINVAL;
+
+	/* The monitor line inherits the configuration from the output line */
+	if (offset == 1 && (*eflags & EDGE_FLAGS) != EDGE_FLAGS)
 		return -EINVAL;
 
 	tio = gch_to_intel_pmc_tio(chip);
 
 	mutex_lock(&tio->lock);
+	if (!tio->monitor && !tio->input) {
+		mutex_unlock(&tio->lock);
+		return -EINVAL;
+	}
+
 	intel_pmc_tio_start_input_work(tio);
-	intel_pmc_tio_enable_input(tio, *eflags);
+	if (offset == 0)
+		intel_pmc_tio_enable_input(tio, *eflags);
+
 	mutex_unlock(&tio->lock);
 
 	return 0;
@@ -341,6 +358,54 @@ static int intel_pmc_tio_do_poll(struct gpio_chip *chip, unsigned int offset,
 	return err;
 }
 
+static int intel_pcm_tio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	return -EIO;
+}
+
+static int intel_pcm_tio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
+	int ret = 0;
+
+	mutex_lock(&tio->lock);
+
+	if (offset == 0) {
+		if (!tio->monitor)
+			tio->input = true;
+		else
+			ret = -EBUSY;
+	} else { /* offset = 1 */
+		if (!tio->input)
+			tio->monitor = true;
+		else
+			ret = -EBUSY;
+	}
+
+	if (!ret)
+		tio->last_event_count = 0;
+
+	mutex_unlock(&tio->lock);
+
+	return ret;
+}
+
+static void intel_pmc_tio_gpio_free(struct gpio_chip *chip, unsigned int offset)
+{
+	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
+
+	if (offset == 0 && tio->input) {
+		tio->input = false;
+		intel_pmc_tio_disable(tio);
+	}
+
+	if (offset == 1)
+		tio->monitor = false;
+
+	if (!tio->monitor && !tio->input)
+		intel_pmc_tio_stop_input_work(tio);
+}
+
 static int intel_pmc_tio_insert_edge(struct intel_pmc_tio_chip *tio, u32 *ctrl)
 {
 	struct system_counterval_t sys_counter;
@@ -428,6 +493,9 @@ static int intel_pmc_tio_direction_output(struct gpio_chip *chip,
 	struct intel_pmc_tio_chip *tio = gch_to_intel_pmc_tio(chip);
 	int ret;
 
+	if (offset != 0)
+		return -ENODEV;
+
 	mutex_lock(&tio->lock);
 	ret =  _intel_pmc_tio_direction_output(tio, offset, value, 0);
 	mutex_unlock(&tio->lock);
@@ -443,6 +511,9 @@ static int _intel_pmc_tio_generate_output(struct intel_pmc_tio_chip *tio,
 	u64 art_timestamp;
 	int err;
 
+	if (offset != 0)
+		return -ENODEV;
+
 	if (timestamp != 0) {
 		sys_realtime = ns_to_ktime(timestamp);
 	} else {
@@ -667,6 +738,9 @@ static int intel_pmc_tio_probe(struct platform_device *pdev)
 	tio->gch.do_poll = intel_pmc_tio_do_poll;
 	tio->gch.generate_output = intel_pmc_tio_generate_output;
 	tio->gch.direction_output = intel_pmc_tio_direction_output;
+	tio->gch.free = intel_pmc_tio_gpio_free;
+	tio->gch.direction_input = intel_pcm_tio_direction_input;
+	tio->gch.get = intel_pcm_tio_get;
 
 	platform_set_drvdata(pdev, tio);
 	mutex_init(&tio->lock);
@@ -686,7 +760,7 @@ static int intel_pmc_tio_probe(struct platform_device *pdev)
 	tio_pwm->tio = tio;
 	tio_pwm->pch.dev = &pdev->dev;
 	tio_pwm->pch.ops = &intel_pmc_tio_pwm_ops;
-	tio_pwm->pch.npwm = GPIO_COUNT;
+	tio_pwm->pch.npwm = GPIO_COUNT - 1;
 	tio_pwm->pch.base = -1;
 
 	err = pwmchip_add(&tio_pwm->pch);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v1 20/20] tools: gpio: Add PWM monitor application
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (18 preceding siblings ...)
  2021-08-24 16:48 ` [RFC PATCH v1 19/20] gpio: Add GPIO monitor line to Intel(R) Timed I/O Driver lakshmi.sowjanya.d
@ 2021-08-24 16:48 ` lakshmi.sowjanya.d
  2021-09-16 21:21 ` [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC Linus Walleij
  20 siblings, 0 replies; 35+ messages in thread
From: lakshmi.sowjanya.d @ 2021-08-24 16:48 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, linux-kernel, mgross,
	andriy.shevchenko, tamal.saha, bala.senthil, lakshmi.sowjanya.d

From: Christopher Hall <christopher.s.hall@intel.com>

The Intel(R) Timed I/O driver produces periodic output using the PMW
interface. The output alignment is set initially with respect to the system
clock using the PWM 'alignment' parameter. There will always be a small
frequency difference between the system clock and the hardware ART clock
used to drive the PWM output especially if the system clock is adjusted by
the PTP/NTP time sync daemon.

A GPIO line is used to 'monitor' the PWM output's drift. Small changes are
made to the output frequency, using a PI controller, to compensate for the
drift. The PWM monitor application (gpio-pwm-mon) implements the PI
controller. The application takes four arguments: GPIO device, PWM device,
output alignment in nanoseconds (ns), and period in ns. The first two
arguments are mandatory, the last two are optional defaulting to a one
second period aligned to 0 ns. Example usage is:

       gpio-pwm-mon -g gpiochip0 -p pwmchip0 -a <alignment> -r <period>
-o <num_of_lines>

Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Tamal Saha <tamal.saha@intel.com>
Co-developed-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Reviewed-by: Mark Gross <mgross@linux.intel.com>
---
 tools/gpio/Build          |   1 +
 tools/gpio/Makefile       |  12 +-
 tools/gpio/gpio-pwm-mon.c | 431 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 443 insertions(+), 1 deletion(-)
 create mode 100644 tools/gpio/gpio-pwm-mon.c

diff --git a/tools/gpio/Build b/tools/gpio/Build
index dc6a178c195a..1d4fc7bb9d7d 100644
--- a/tools/gpio/Build
+++ b/tools/gpio/Build
@@ -3,4 +3,5 @@ lsgpio-y += lsgpio.o gpio-utils.o
 gpio-hammer-y += gpio-hammer.o gpio-utils.o
 gpio-event-mon-y += gpio-event-mon.o gpio-utils.o
 gpio-event-gen-y += gpio-event-gen.o gpio-utils.o
+gpio-pwm-mon-y += gpio-pwm-mon.o gpio-utils.o
 gpio-watch-y += gpio-watch.o
diff --git a/tools/gpio/Makefile b/tools/gpio/Makefile
index c9efaee76f28..63e158ef8f7f 100644
--- a/tools/gpio/Makefile
+++ b/tools/gpio/Makefile
@@ -18,7 +18,8 @@ MAKEFLAGS += -r
 
 override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include
 
-ALL_TARGETS := lsgpio gpio-hammer gpio-event-mon gpio-event-gen gpio-watch
+ALL_TARGETS := lsgpio gpio-hammer gpio-event-mon gpio-event-gen gpio-pwm-mon \
+	gpio-watch
 ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
 
 all: $(ALL_PROGRAMS)
@@ -75,6 +76,15 @@ $(GPIO_EVENT_GEN_IN): prepare FORCE $(OUTPUT)gpio-utils-in.o
 $(OUTPUT)gpio-event-gen: $(GPIO_EVENT_GEN_IN)
 	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
 
+#
+# gpio-pwm-mon
+#
+GPIO_EVENT_MON_IN := $(OUTPUT)gpio-pwm-mon-in.o
+$(GPIO_EVENT_MON_IN): prepare FORCE $(OUTPUT)gpio-utils-in.o
+	$(Q)$(MAKE) $(build)=gpio-pwm-mon
+$(OUTPUT)gpio-pwm-mon: $(GPIO_EVENT_MON_IN)
+	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -lpthread -pthread $< -o $@
+
 #
 # gpio-watch
 #
diff --git a/tools/gpio/gpio-pwm-mon.c b/tools/gpio/gpio-pwm-mon.c
new file mode 100644
index 000000000000..71e02aca8b27
--- /dev/null
+++ b/tools/gpio/gpio-pwm-mon.c
@@ -0,0 +1,431 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * gpio-pwm-mon - Use 'virtual' GPIO input line to monitor PWM output and
+ *	adjust period to align the clock with the system clock
+ *
+ * Copyright (C) 2020 Intel Corporation
+ * Author: christopher.s.hall@intel.com
+ *
+ * Usage:
+ *      gpio-pwm-mon -g gpiochip0 -p pwmchip0
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <pthread.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <linux/gpio.h>
+#include <sys/ioctl.h>
+
+#define PWM_PATH "/sys/class/pwm"
+
+#define NSEC_PER_SEC	(1000000000U)
+#define MAX_PERIOD	(4000000000U /*ns*/)
+#define DEFAULT_PERIOD	(125000000U /*ns*/)
+
+#define PWM_ENABLE	(1)
+#define PWM_DISABLE	(0)
+#define GPIO_MONITOR	(1)
+#define PWM_LINE	(0)
+
+#define KPROP_DEFAULT	(1.0)
+#define KINT_DEFAULT	(0.25)
+
+#define EVENT_SIZE	(sizeof(struct gpio_v2_line_event) + sizeof(struct gpio_v2_line_event_ext))
+
+void print_usage(void)
+{
+	fprintf(stdout, "Usage: gpio-pwm-mon [options]...\n"
+		"Listen to events on virtual GPIO lines, adjust PWM\n"
+		"\t -g <name>\t Listen on the GPIO device (required)\n"
+		"\t -p <name>\t Generate output on the PWM device (required)\n"
+		"\t -a <ns>\t Output alignment (ns) to the second\n"
+		"\t -r <ns>\t Output period (ns) (default: %u, maximum: %u)\n"
+		"\t -?\t\t This helptext\n"
+		"\n"
+		"Example:\n"
+		"gpio-pwm-mon -g gpiochip0 -p pwmchip0\n",
+		DEFAULT_PERIOD, MAX_PERIOD
+		);
+}
+
+int write_unsigned_int_to_file(unsigned int val, char *file)
+{
+	char *buf;
+	int fd;
+	int ret = 0;
+
+	fd = open(file, O_WRONLY);
+	if (fd == -1)
+		return -errno;
+
+	ret = asprintf(&buf, "%u", val);
+	if (ret == -1) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = write(fd, buf, strlen(buf));
+	if (ret == -1)
+		ret = -errno;
+	else if (ret != strlen(buf))
+		ret = -EIO;
+	else
+		ret = 0;
+
+	free(buf);
+out:
+	close(fd);
+
+	return ret;
+}
+
+int start_pwm(const char *pwm_name, unsigned int pwm_number,
+	      unsigned int period, unsigned int alignment)
+{
+	char *pwm_path;
+	int ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/export", PWM_PATH, pwm_name);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(pwm_number, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/pwm%u/period", PWM_PATH, pwm_name,
+		       pwm_number);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(MAX_PERIOD, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/pwm%u/duty_cycle", PWM_PATH, pwm_name,
+		       pwm_number);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(period / 2, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/pwm%u/period", PWM_PATH, pwm_name,
+		       pwm_number);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(period, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/pwm%u/alignment", PWM_PATH, pwm_name,
+		       pwm_number);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(alignment, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/pwm%u/enable", PWM_PATH, pwm_name,
+		       pwm_number);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(PWM_ENABLE, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+int set_period_pwm(const char *pwm_name, unsigned int pwm_number,
+		   unsigned int period)
+{
+	char *pwm_path;
+	int ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/pwm%u/period", PWM_PATH, pwm_name,
+		       pwm_number);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(period, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+int stop_pwm(const char *pwm_name, unsigned int pwm_number)
+{
+	char *pwm_path;
+	int ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/pwm%u/enable", PWM_PATH, pwm_name,
+		       pwm_number);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(PWM_DISABLE, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	ret = asprintf(&pwm_path, "%s/%s/unexport", PWM_PATH, pwm_name);
+	if (ret == -1)
+		return -errno;
+	ret = write_unsigned_int_to_file(pwm_number, pwm_path);
+	free(pwm_path);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+int adjust_pwm_loop(const char *gpio_name, const char *pwm_name,
+		    const char *consumer, unsigned int period,
+		    unsigned int align, double kprop, double kint,
+		    volatile bool *exit, int *lines, int num_lines)
+{
+	unsigned int adjusted_period = period;
+	struct gpio_v2_line_config config;
+	struct gpio_v2_line_event event[EVENT_SIZE];
+	struct gpio_v2_line_request req;
+	uint64_t last_event_timestamp;
+	uint64_t total_event_count;
+	uint64_t last_event_count;
+	uint64_t start_time = 0;
+	char *chrdev_name;
+	size_t event_size;
+	int  i = 0;
+	int ret;
+	int fd;
+
+	ret = asprintf(&chrdev_name, "/dev/%s", gpio_name);
+	if (ret < 0)
+		return -ENOMEM;
+
+	fd = open(chrdev_name, 0);
+	if (fd == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to open %s\n", chrdev_name);
+		goto exit_close_error;
+	}
+
+	memset(&config, 0, sizeof(config));
+	config.flags = GPIO_V2_LINE_FLAG_INPUT | GPIO_V2_LINE_FLAG_EDGE_FALLING |
+		       GPIO_V2_LINE_FLAG_EDGE_RISING | GPIO_V2_LINE_FLAG_EVENT_COUNT;
+	memset(&req, 0, sizeof(req));
+	for (i = 0; i < num_lines; i++)
+		req.offsets[i] = lines[i];
+	req.num_lines = num_lines;
+	req.config = config;
+	req.config.flags = config.flags;
+	strcpy(req.consumer, consumer);
+
+	ret = ioctl(fd, GPIO_V2_GET_LINE_IOCTL, &req);
+	if (ret == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to issue GET EVENT IOCTL (%d)\n", ret);
+		goto exit_close_error;
+	}
+
+	while (!*exit) {
+		ret = read(req.fd, event, event_size);
+		if (ret == -1) {
+			if (errno == EAGAIN) {
+				continue;
+			} else {
+				ret = -errno;
+				fprintf(stderr, "Failed to read event (%d)\n",
+					ret);
+				break;
+			}
+		}
+
+		if (ret != event_size) {
+			ret = -EIO;
+			fprintf(stderr, "Reading event failed (%d)\n", ret);
+			break;
+		}
+
+		if (start_time == 0) {
+			uint32_t res;
+			bool round_up;
+
+			start_time = event[0].timestamp_ns;
+			if (event[0].ext[0].event_count > 1) {
+				ret = -EINTR;
+				fprintf(stderr, "Lost start event\n");
+				break;
+			}			start_time -= align;
+			res = start_time % NSEC_PER_SEC;
+			round_up = res > NSEC_PER_SEC / 2;
+			start_time -= res;
+			start_time += round_up ? NSEC_PER_SEC : 0;
+			start_time += align;
+			total_event_count = event[0].ext[0].event_count - 1;
+			last_event_timestamp = start_time;
+			last_event_count = total_event_count;
+		} else {
+			uint64_t duration;
+			int int_error;
+			double prop_error;
+			double adjust;
+
+			total_event_count += event[0].ext[0].event_count;
+			if (total_event_count % 2 == 1)
+				continue;
+			duration = event[0].timestamp_ns - start_time;
+			int_error = duration > total_event_count / 2 * period ?
+				-(duration - total_event_count / 2 * period) :
+				total_event_count / 2 * period - duration;
+			prop_error = event[0].timestamp_ns - last_event_timestamp;
+			prop_error *= -1;
+			prop_error /= (total_event_count - last_event_count) / 2;
+			prop_error += period;
+			adjust = prop_error * kprop + int_error * kint;
+			adjusted_period += adjust;
+			set_period_pwm(pwm_name, PWM_LINE, adjusted_period);
+			last_event_count = total_event_count;
+			last_event_timestamp = event[0].timestamp_ns;
+		}
+		printf("Event %04llu timestamp: %llu\n", last_event_count,
+		       last_event_timestamp);
+	}
+
+exit_close_error:
+	if (close(fd) == -1)
+		perror("Failed to close GPIO character device file");
+	free(chrdev_name);
+	return ret;
+}
+
+struct wait_arg {
+	volatile bool *exit;
+	sigset_t *sigint;
+};
+
+void *wait_for_interrupt(void *arg)
+{
+	volatile bool *exit = ((struct wait_arg *)arg)->exit;
+	sigset_t *sigint = ((struct wait_arg *)arg)->sigint;
+	siginfo_t info;
+	int ret;
+
+	do {
+		ret = sigwaitinfo(sigint, &info);
+	} while (ret == -1 && errno == EINTR);
+
+	if (ret != -1) {
+		printf("Received %s\n", strsignal(info.si_signo));
+		*exit = true;
+	}
+
+	return NULL;
+}
+
+int main(int argc, char **argv)
+{
+	unsigned int period = DEFAULT_PERIOD;
+	const char *gpio_name = NULL;
+	double kprop = KPROP_DEFAULT;
+	const char *pwm_name = NULL;
+	double kint = KINT_DEFAULT;
+	unsigned int alignment = 0;
+	volatile bool exit = false;
+	struct wait_arg wait_arg;
+	pthread_t int_thread;
+	sigset_t sigint;
+	int num_lines = 0;
+	int lines[10];
+	int err;
+	int c;
+
+	while ((c = getopt(argc, argv, "g:p:r:a:t:n:o:?")) != -1) {
+		switch (c) {
+		case 'g':
+			gpio_name = optarg;
+			break;
+		case 'p':
+			pwm_name = optarg;
+			break;
+		case 'r':
+			period = strtoul(optarg, NULL, 10);
+			break;
+		case 'a':
+			alignment = strtoul(optarg, NULL, 10);
+			break;
+		case 't':
+			kprop = strtod(optarg, NULL);
+			break;
+		case 'n':
+			kint = strtod(optarg, NULL);
+			break;
+		case 'o':
+			if (num_lines >= GPIO_V2_LINES_MAX) {
+				print_usage();
+				return -1;
+			}
+			lines[num_lines] = strtoul(optarg, NULL, 10);
+			num_lines++;
+			break;
+
+		case '?':
+			print_usage();
+			return 0;
+		}
+	}
+
+	if (!pwm_name || !gpio_name || period > MAX_PERIOD) {
+		print_usage();
+		return -1;
+	}
+
+	err = start_pwm(pwm_name, PWM_LINE, period, alignment);
+	if (err) {
+		printf("Failed to start PWM: %s (%s)\n", pwm_name, strerror(-err));
+		return -1;
+	}
+
+	sigemptyset(&sigint);
+	sigaddset(&sigint, SIGINT);
+	err = pthread_sigmask(SIG_BLOCK, &sigint, NULL);
+	if (err) {
+		printf("Failed to block interrupt signals: %s\n", strerror(err));
+		goto cleanup_pwm;
+	}
+	wait_arg.sigint = &sigint;
+	wait_arg.exit = &exit;
+	err = pthread_create(&int_thread, NULL, wait_for_interrupt, &wait_arg);
+	if (err) {
+		printf("Failed to listen on user interrupt: %s\n", strerror(err));
+		goto cleanup_pwm;
+	}
+
+	err = adjust_pwm_loop(gpio_name, pwm_name, argv[0], period, alignment,
+			      kprop, kint, &exit, lines, num_lines);
+	if (err) {
+		printf("Failed to monitor PWM: %s\n", strerror(-err));
+		pthread_kill(int_thread, SIGINT);
+	}
+
+	pthread_join(int_thread, NULL);
+
+cleanup_pwm:
+	err = stop_pwm(pwm_name, PWM_LINE);
+	if (err)
+		printf("Failed to stop PWM: %s (%s)\n", pwm_name, strerror(-err));
+
+	return err ? -1 : 0;
+}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC
  2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
                   ` (19 preceding siblings ...)
  2021-08-24 16:48 ` [RFC PATCH v1 20/20] tools: gpio: Add PWM monitor application lakshmi.sowjanya.d
@ 2021-09-16 21:21 ` Linus Walleij
  2021-10-11 21:14   ` Dipen Patel
  20 siblings, 1 reply; 35+ messages in thread
From: Linus Walleij @ 2021-09-16 21:21 UTC (permalink / raw)
  To: D, Lakshmi Sowjanya, Dipen Patel
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-kernel,
	Mark Gross, Andy Shevchenko, Saha, Tamal, bala.senthil

Hi Sowjanya,

thanks for your patches!

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:

> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
>
> Starting with Intel(R) Tiger Lake and Elkhart Lake platforms the PMC
> hardware adds the Timed I/O hardware interface.
>
> The Timed I/O hardware implements some functionality similar to GPIO
> with added timing logic that is driven by the Always Running Timer
> (ART).
>
> The Timed I/O Hardware implement 3 basic functions:
>   * Input Timestamping
>   * Single Shot Timed Output
>   * Periodic Timed Output
>
>  Please help to review the changes.

This looks very similar to the usecase proposed for the HTE
Hardware Timestamping Engine, proposed by Dipen Patel
for the nVidia 194 and which is currently in RFC:
https://lore.kernel.org/linux-gpio/20210625235532.19575-1-dipenp@nvidia.com/

Please review this new subsystem and see if you can just
make a slot-in driver using Dipen's patches instead.

Dipen: please have a look at Sowjanya's patches to see
if this hardware is similar to yours.

Sometimes several vendors come up with similar hardware
around the same time, because of industry trends, so I would
not be surprised if these two hardwares address the very
same usecase.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver
  2021-08-24 16:47 ` [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver lakshmi.sowjanya.d
@ 2021-09-16 21:42   ` Linus Walleij
  2021-09-17  7:27     ` Uwe Kleine-König
  0 siblings, 1 reply; 35+ messages in thread
From: Linus Walleij @ 2021-09-16 21:42 UTC (permalink / raw)
  To: D, Lakshmi Sowjanya, thierry.reding, Uwe Kleine-König,
	Lee Jones, open list:PWM SUBSYSTEM
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-kernel,
	Mark Gross, Andy Shevchenko, Saha, Tamal, bala.senthil,
	Dipen Patel

Hi Lakshmi,

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:

> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
>
> Intel Timed I/O hardware supports output scheduled in hardware. Enable
> this functionality using GPIOlib
>
> Adds GPIOlib generate_output() hook into the driver. The driver is
> supplied with a timestamp in terms of realtime system clock (the same
> used for input timestamping). The driver must know how to translate this
> into a timebase meaningful for the hardware.
>
> Adds userspace write() interface. Output can be selected using the line
> event create ioctl. The write() interface takes a single timestamp
> event request parameter. An output edge rising or falling is generated
> for each event request.
>
> The user application supplies a trigger time in terms of the realtime
> clock the driver converts this into the corresponding ART clock value
> that is used to 'arm' the output.
>
> Work around device quirk that doesn't allow the output to be explicitly
> set. Instead, count the output edges and insert an additional edge as
> needed to reset the output to zero.
>
> Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>

So this is some street organ machine that generates sequences
with determined timing between positive and negative edges
right?

I can't see how this hardware is different from a PWM, or well
I do to some extent, you can control the period of several
subsequent waves, but that is really just an elaborate version
of PWM in my book.

It seems to me that this part of the functionality belongs in the
PWM subsystem which already has interfaces for similar
things, and you should probably extend PWM to handle
random waveforms rather than trying to shoehorn this
into the GPIO subsystem.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 08/20] kernel: time: Add system time to system counter translation
  2021-08-24 16:47 ` [RFC PATCH v1 08/20] kernel: time: Add system time to system counter translation lakshmi.sowjanya.d
@ 2021-09-16 21:48   ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2021-09-16 21:48 UTC (permalink / raw)
  To: D, Lakshmi Sowjanya, Thomas Gleixner, John Stultz, Stephen Boyd
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-kernel,
	Mark Gross, Andy Shevchenko, Saha, Tamal, bala.senthil,
	Dipen Patel

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:

> From: Christopher Hall <christopher.s.hall@intel.com>
>
> The GPIOlib event generation interface supplies a time in terms of the
> realtime system clock. The driver must translate the system clock to
> something meaningful to the hardware.
>
> The Intel(R) PMC Timed I/O hardware uses ART to trigger events. For
> most Intel(R) platforms that use TSC for timekeeping this added
> function translates from system clock to TSC. The relation between TSC
> and ART is easily derived from CPUID[15H].
>
> Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>

I don't quite understand what this patch does, but it looks very generic.
I think the exact usecase needs to be in the commit message.

In either case, this is a patch TGLX et al needs to look at, we can't
merge timekeeping code without their consent.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 10/20] tools: gpio: Add GPIO output generation user application
  2021-08-24 16:47 ` [RFC PATCH v1 10/20] tools: gpio: Add GPIO output generation user application lakshmi.sowjanya.d
@ 2021-09-16 21:52   ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2021-09-16 21:52 UTC (permalink / raw)
  To: D, Lakshmi Sowjanya, thierry.reding, Uwe Kleine-König, Lee Jones
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-kernel,
	Mark Gross, Andy Shevchenko, Saha, Tamal, bala.senthil,
	Dipen Patel

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:

> From: Christopher Hall <christopher.s.hall@intel.com>
>
> Add GPIO user application - gpio_event_gen - to generate output using
> output methods added to GPIO lib. The output produced is 1 Hz clock
> aligned to the system clock using singly scheduled edges.
>
> gpio_event_gen accepts similar arguments to gpio-event-mon.
>
> Example output:
>         $ gpio-event-gen -n gpiochip0 -o 0 -c 3
>         Generating events on line 0 on gpiochip1
>         clock realtime : 1612453529996832765
>         GPIO EVENT TRIGGER: 1612453531000000000
>         clock realtime 2 2 : 1612453531500000000
>         GPIO EVENT TRIGGER: 1612453531500000000
>         clock realtime 2 2 : 1612453532000000000
>         GPIO EVENT TRIGGER: 1612453532000000000
>         clock realtime 2 2 : 1612453532500000000
>
> Produces 3 events of 1 Hz output on line 0 of chip/device 0.
>
> Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> Co-developed-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>

To me this looks as very similar to what the PWM subsystem
is doing, just with a restricted number of periods.
Especially with that command line example.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 13/20] tools: gpio: Add event count capability to event monitor application
  2021-08-24 16:47 ` [RFC PATCH v1 13/20] tools: gpio: Add event count capability to event monitor application lakshmi.sowjanya.d
@ 2021-09-16 21:57   ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2021-09-16 21:57 UTC (permalink / raw)
  To: D, Lakshmi Sowjanya
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-kernel,
	Mark Gross, Andy Shevchenko, Saha, Tamal, bala.senthil

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:

> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
>
> Add -t command line flag requesting event count to the gpio-event-mon
> application. If event count is unsupported an invalid argument error is
> returned by GPIOlib and the application exits with an error. The event
> count is printed with the event type and timestamp.
>
> Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>

This looks wrong.

If this hardware can report timestamped events, which I believe
it does, each event should be reported to userspace uniquely,
ideally using the native timestamp, and using the in-progress HTE
subsystem.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 15/20] pwm: Add capability for PWM Driver managed state
  2021-08-24 16:47 ` [RFC PATCH v1 15/20] pwm: Add capability for PWM Driver managed state lakshmi.sowjanya.d
@ 2021-09-16 22:00   ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2021-09-16 22:00 UTC (permalink / raw)
  To: D, Lakshmi Sowjanya, thierry.reding, Uwe Kleine-König,
	Lee Jones, open list:PWM SUBSYSTEM
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-kernel,
	Mark Gross, Andy Shevchenko, Saha, Tamal, bala.senthil

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:

> From: Christopher Hall <christopher.s.hall@intel.com>
>
> Add additional flag that can be set by drivers to indicate that the
> driver will manage its own PWM state. When calling pwm_ops.apply the
> driver applies the requested state change to the pwm_device reconciling,
> if possible, any conflicting requests.
>
> Intel(R) Timed I/O devices support very limited PWM capabilities. The
> duty cycle must always be 50% of the period. When changing one parameter
> at a time through the sysfs interface, it isn't possible for the user or
> the PWM subsystem to maintain this relation.
>
> Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>

These PWM changes clearly need to be reviewed by the PWM
subsystem maintainers, and I think all of the output generation
portions of this really need to go into the PWM subsystem.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver
  2021-09-16 21:42   ` Linus Walleij
@ 2021-09-17  7:27     ` Uwe Kleine-König
  2021-09-19 19:38       ` Linus Walleij
  0 siblings, 1 reply; 35+ messages in thread
From: Uwe Kleine-König @ 2021-09-17  7:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: D, Lakshmi Sowjanya, thierry.reding, Lee Jones,
	open list:PWM SUBSYSTEM, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, linux-kernel, Mark Gross, Andy Shevchenko,
	Saha, Tamal, bala.senthil, Dipen Patel

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

Hello,

On Thu, Sep 16, 2021 at 11:42:04PM +0200, Linus Walleij wrote:
> Hi Lakshmi,
> 
> On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
> 
> > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> >
> > Intel Timed I/O hardware supports output scheduled in hardware. Enable
> > this functionality using GPIOlib
> >
> > Adds GPIOlib generate_output() hook into the driver. The driver is
> > supplied with a timestamp in terms of realtime system clock (the same
> > used for input timestamping). The driver must know how to translate this
> > into a timebase meaningful for the hardware.
> >
> > Adds userspace write() interface. Output can be selected using the line
> > event create ioctl. The write() interface takes a single timestamp
> > event request parameter. An output edge rising or falling is generated
> > for each event request.
> >
> > The user application supplies a trigger time in terms of the realtime
> > clock the driver converts this into the corresponding ART clock value
> > that is used to 'arm' the output.
> >
> > Work around device quirk that doesn't allow the output to be explicitly
> > set. Instead, count the output edges and insert an additional edge as
> > needed to reset the output to zero.
> >
> > Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> > Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> > Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> 
> So this is some street organ machine that generates sequences
> with determined timing between positive and negative edges
> right?
> 
> I can't see how this hardware is different from a PWM, or well
> I do to some extent, you can control the period of several
> subsequent waves, but that is really just an elaborate version
> of PWM in my book.

From looking in the patch I think this is more versatile than the PWM
framework abstracts. I wonder if there is a usecase for the
functionality that cannot be expressed using pwm_apply_state?!

I remember we had approaches before that implemented repeating patterns
(something like: active for 5ms, inactive for 10 ms, active for 30 ms,
inactive for 10 ms, repeat) and limiting the number of periods
(something like: .duty_cycle = 5ms, .period = 20ms, after 5 periods go
into inactive state). These were considered to be too special to be
abstracted in drivers/pwm.

> It seems to me that this part of the functionality belongs in the
> PWM subsystem which already has interfaces for similar
> things, and you should probably extend PWM to handle
> random waveforms rather than trying to shoehorn this
> into the GPIO subsystem.

I agree that GPIO is a worse candidate than PWM to abstract that. But
I'm not convinced (yet?) that it's a good idea to extend PWM
accordingly.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver
  2021-09-17  7:27     ` Uwe Kleine-König
@ 2021-09-19 19:38       ` Linus Walleij
  2021-09-19 21:21         ` Clemens Gruber
  0 siblings, 1 reply; 35+ messages in thread
From: Linus Walleij @ 2021-09-19 19:38 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: D, Lakshmi Sowjanya, thierry.reding, Lee Jones,
	open list:PWM SUBSYSTEM, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, linux-kernel, Mark Gross, Andy Shevchenko,
	Saha, Tamal, bala.senthil, Dipen Patel

On Fri, Sep 17, 2021 at 9:27 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Thu, Sep 16, 2021 at 11:42:04PM +0200, Linus Walleij wrote:
> > On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
> >
> > > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > >
> > > Intel Timed I/O hardware supports output scheduled in hardware. Enable
> > > this functionality using GPIOlib
> > >
> > > Adds GPIOlib generate_output() hook into the driver. The driver is
> > > supplied with a timestamp in terms of realtime system clock (the same
> > > used for input timestamping). The driver must know how to translate this
> > > into a timebase meaningful for the hardware.
> > >
> > > Adds userspace write() interface. Output can be selected using the line
> > > event create ioctl. The write() interface takes a single timestamp
> > > event request parameter. An output edge rising or falling is generated
> > > for each event request.
> > >
> > > The user application supplies a trigger time in terms of the realtime
> > > clock the driver converts this into the corresponding ART clock value
> > > that is used to 'arm' the output.
> > >
> > > Work around device quirk that doesn't allow the output to be explicitly
> > > set. Instead, count the output edges and insert an additional edge as
> > > needed to reset the output to zero.
> > >
> > > Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> > > Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> > > Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> > > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> >
> > So this is some street organ machine that generates sequences
> > with determined timing between positive and negative edges
> > right?
> >
> > I can't see how this hardware is different from a PWM, or well
> > I do to some extent, you can control the period of several
> > subsequent waves, but that is really just an elaborate version
> > of PWM in my book.
>
> From looking in the patch I think this is more versatile than the PWM
> framework abstracts. I wonder if there is a usecase for the
> functionality that cannot be expressed using pwm_apply_state?!
>
> I remember we had approaches before that implemented repeating patterns
> (something like: active for 5ms, inactive for 10 ms, active for 30 ms,
> inactive for 10 ms, repeat) and limiting the number of periods
> (something like: .duty_cycle = 5ms, .period = 20ms, after 5 periods go
> into inactive state). These were considered to be too special to be
> abstracted in drivers/pwm.
>
> > It seems to me that this part of the functionality belongs in the
> > PWM subsystem which already has interfaces for similar
> > things, and you should probably extend PWM to handle
> > random waveforms rather than trying to shoehorn this
> > into the GPIO subsystem.
>
> I agree that GPIO is a worse candidate than PWM to abstract that. But
> I'm not convinced (yet?) that it's a good idea to extend PWM
> accordingly.

Yeah it is a bit unfortunate.

I think we need to fully understand the intended usecase before
we can deal with this: exactly what was this hardware constructed
to handle? Sound? Robotic stepper motors? It must be something
and apparently there are users.

Maybe even a new subsystem is needed, like a
drivers/gpio-patterns or drivers/stepper-motor or whatever this
is supposed to drive.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver
  2021-09-19 19:38       ` Linus Walleij
@ 2021-09-19 21:21         ` Clemens Gruber
  2021-09-20  7:14           ` Uwe Kleine-König
  0 siblings, 1 reply; 35+ messages in thread
From: Clemens Gruber @ 2021-09-19 21:21 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Uwe Kleine-König, D, Lakshmi Sowjanya, thierry.reding,
	Lee Jones, open list:PWM SUBSYSTEM, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, linux-kernel, Mark Gross, Andy Shevchenko,
	Saha, Tamal, bala.senthil, Dipen Patel

On Sun, Sep 19, 2021 at 09:38:58PM +0200, Linus Walleij wrote:
> On Fri, Sep 17, 2021 at 9:27 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Thu, Sep 16, 2021 at 11:42:04PM +0200, Linus Walleij wrote:
> > > On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
> > >
> > > > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > > >
> > > > Intel Timed I/O hardware supports output scheduled in hardware. Enable
> > > > this functionality using GPIOlib
> > > >
> > > > Adds GPIOlib generate_output() hook into the driver. The driver is
> > > > supplied with a timestamp in terms of realtime system clock (the same
> > > > used for input timestamping). The driver must know how to translate this
> > > > into a timebase meaningful for the hardware.
> > > >
> > > > Adds userspace write() interface. Output can be selected using the line
> > > > event create ioctl. The write() interface takes a single timestamp
> > > > event request parameter. An output edge rising or falling is generated
> > > > for each event request.
> > > >
> > > > The user application supplies a trigger time in terms of the realtime
> > > > clock the driver converts this into the corresponding ART clock value
> > > > that is used to 'arm' the output.
> > > >
> > > > Work around device quirk that doesn't allow the output to be explicitly
> > > > set. Instead, count the output edges and insert an additional edge as
> > > > needed to reset the output to zero.
> > > >
> > > > Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> > > > Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> > > > Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> > > > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > > > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> > >
> > > So this is some street organ machine that generates sequences
> > > with determined timing between positive and negative edges
> > > right?
> > >
> > > I can't see how this hardware is different from a PWM, or well
> > > I do to some extent, you can control the period of several
> > > subsequent waves, but that is really just an elaborate version
> > > of PWM in my book.
> >
> > From looking in the patch I think this is more versatile than the PWM
> > framework abstracts. I wonder if there is a usecase for the
> > functionality that cannot be expressed using pwm_apply_state?!
> >
> > I remember we had approaches before that implemented repeating patterns
> > (something like: active for 5ms, inactive for 10 ms, active for 30 ms,
> > inactive for 10 ms, repeat) and limiting the number of periods
> > (something like: .duty_cycle = 5ms, .period = 20ms, after 5 periods go
> > into inactive state). These were considered to be too special to be
> > abstracted in drivers/pwm.
> >
> > > It seems to me that this part of the functionality belongs in the
> > > PWM subsystem which already has interfaces for similar
> > > things, and you should probably extend PWM to handle
> > > random waveforms rather than trying to shoehorn this
> > > into the GPIO subsystem.
> >
> > I agree that GPIO is a worse candidate than PWM to abstract that. But
> > I'm not convinced (yet?) that it's a good idea to extend PWM
> > accordingly.
> 
> Yeah it is a bit unfortunate.
> 
> I think we need to fully understand the intended usecase before
> we can deal with this: exactly what was this hardware constructed
> to handle? Sound? Robotic stepper motors? It must be something
> and apparently there are users.
> 
> Maybe even a new subsystem is needed, like a
> drivers/gpio-patterns or drivers/stepper-motor or whatever this
> is supposed to drive.

This would be interesting. Maybe even more abstract, not just supporting
GPIO patterns but also PWM patterns.

E.g. Set gpiochip1 line 2 to 1, wait 5ms, set it to 0
Or set pwmchip1 pwm 2 to 100%, wait 250ms, set it back to 50% duty cycle

This subsystem could then implement the patterns with hrtimers and be
usable with every GPIO or PWM device supported in Linux, and for
special hardware like the Intel Timed I/O, it could configure it to
output the pattern itself.

One usecase besides stepper motors and Robotics would be solenoid
valves: You often have different sequences for opening, closing and
maintenance. E.g. for liquid valves, especially if the liquid is
viscuous, you have to first use 100% duty cycle PWM for e.g. 250ms to
get it open and then dial back to 50% to keep it open without
overheating it.

Of course this can be done in userspace.. but it may also be useful to
have some kind of pattern generator in the kernel. What do you think?

Clemens

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver
  2021-09-19 21:21         ` Clemens Gruber
@ 2021-09-20  7:14           ` Uwe Kleine-König
  0 siblings, 0 replies; 35+ messages in thread
From: Uwe Kleine-König @ 2021-09-20  7:14 UTC (permalink / raw)
  To: Clemens Gruber
  Cc: Linus Walleij, D, Lakshmi Sowjanya, thierry.reding, Lee Jones,
	open list:PWM SUBSYSTEM, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, linux-kernel, Mark Gross, Andy Shevchenko,
	Saha, Tamal, bala.senthil, Dipen Patel

[-- Attachment #1: Type: text/plain, Size: 6398 bytes --]

On Sun, Sep 19, 2021 at 11:21:22PM +0200, Clemens Gruber wrote:
> On Sun, Sep 19, 2021 at 09:38:58PM +0200, Linus Walleij wrote:
> > On Fri, Sep 17, 2021 at 9:27 AM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > > On Thu, Sep 16, 2021 at 11:42:04PM +0200, Linus Walleij wrote:
> > > > On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
> > > >
> > > > > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > > > >
> > > > > Intel Timed I/O hardware supports output scheduled in hardware. Enable
> > > > > this functionality using GPIOlib
> > > > >
> > > > > Adds GPIOlib generate_output() hook into the driver. The driver is
> > > > > supplied with a timestamp in terms of realtime system clock (the same
> > > > > used for input timestamping). The driver must know how to translate this
> > > > > into a timebase meaningful for the hardware.
> > > > >
> > > > > Adds userspace write() interface. Output can be selected using the line
> > > > > event create ioctl. The write() interface takes a single timestamp
> > > > > event request parameter. An output edge rising or falling is generated
> > > > > for each event request.
> > > > >
> > > > > The user application supplies a trigger time in terms of the realtime
> > > > > clock the driver converts this into the corresponding ART clock value
> > > > > that is used to 'arm' the output.
> > > > >
> > > > > Work around device quirk that doesn't allow the output to be explicitly
> > > > > set. Instead, count the output edges and insert an additional edge as
> > > > > needed to reset the output to zero.
> > > > >
> > > > > Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> > > > > Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> > > > > Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> > > > > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > > > > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> > > >
> > > > So this is some street organ machine that generates sequences
> > > > with determined timing between positive and negative edges
> > > > right?
> > > >
> > > > I can't see how this hardware is different from a PWM, or well
> > > > I do to some extent, you can control the period of several
> > > > subsequent waves, but that is really just an elaborate version
> > > > of PWM in my book.
> > >
> > > From looking in the patch I think this is more versatile than the PWM
> > > framework abstracts. I wonder if there is a usecase for the
> > > functionality that cannot be expressed using pwm_apply_state?!
> > >
> > > I remember we had approaches before that implemented repeating patterns
> > > (something like: active for 5ms, inactive for 10 ms, active for 30 ms,
> > > inactive for 10 ms, repeat) and limiting the number of periods
> > > (something like: .duty_cycle = 5ms, .period = 20ms, after 5 periods go
> > > into inactive state). These were considered to be too special to be
> > > abstracted in drivers/pwm.
> > >
> > > > It seems to me that this part of the functionality belongs in the
> > > > PWM subsystem which already has interfaces for similar
> > > > things, and you should probably extend PWM to handle
> > > > random waveforms rather than trying to shoehorn this
> > > > into the GPIO subsystem.
> > >
> > > I agree that GPIO is a worse candidate than PWM to abstract that. But
> > > I'm not convinced (yet?) that it's a good idea to extend PWM
> > > accordingly.
> > 
> > Yeah it is a bit unfortunate.
> > 
> > I think we need to fully understand the intended usecase before
> > we can deal with this: exactly what was this hardware constructed
> > to handle? Sound? Robotic stepper motors? It must be something
> > and apparently there are users.
> > 
> > Maybe even a new subsystem is needed, like a
> > drivers/gpio-patterns or drivers/stepper-motor or whatever this
> > is supposed to drive.
> 
> This would be interesting. Maybe even more abstract, not just supporting
> GPIO patterns but also PWM patterns.
> 
> E.g. Set gpiochip1 line 2 to 1, wait 5ms, set it to 0
> Or set pwmchip1 pwm 2 to 100%, wait 250ms, set it back to 50% duty cycle

Note that adding support to drive PWMs in this "GPIO command sequence"
framework would not increase its expressiveness assuming this framework
has loop support. That's because the sequence "set pwmchip1 pwm 2 to
100% (with a period of X), wait 250ms, set it back to 50% duty cycle
with a period of Y" can be expressed using a GPIO as:

	set GPIO to active
	wait 250 ms + Y/2
	while True:
	    toggle GPIO
	    wait Y/2

That's because this framework could provide a PWM from a GPIO.

(Also note that the original command sequence has some problems. That's
because (depending on the PWM in use and X) doing

	pwm_apply_state(mypwm, &(struct pwm_state){ .period = X, .duty_cycle = X, .enabled = 1 });

and then 250ms later

	pwm_apply_state(mypwm, &(struct pwm_state){ .period = Y, .duty_cycle = Y / 2, .enabled = 1 });

might give you an initially active phase that is considerably longer
than 250 ms + Y/2 because the PWM cannot implement .period = X exactly
and completes the period that is currently running at time 250 ms.

> This subsystem could then implement the patterns with hrtimers and be
> usable with every GPIO or PWM device supported in Linux, and for
> special hardware like the Intel Timed I/O, it could configure it to
> output the pattern itself.
> 
> One usecase besides stepper motors and Robotics would be solenoid
> valves: You often have different sequences for opening, closing and
> maintenance. E.g. for liquid valves, especially if the liquid is
> viscuous, you have to first use 100% duty cycle PWM for e.g. 250ms to
> get it open and then dial back to 50% to keep it open without
> overheating it.
> 
> Of course this can be done in userspace.. but it may also be useful to
> have some kind of pattern generator in the kernel. What do you think?

Without hearing the usecases of the original idea my feeling is:
Implement it in userspace, than the sequences can even contain network
interaction and access to SPI eeproms.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib
  2021-08-24 16:47 ` [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib lakshmi.sowjanya.d
@ 2021-09-22  9:53   ` Bartosz Golaszewski
  0 siblings, 0 replies; 35+ messages in thread
From: Bartosz Golaszewski @ 2021-09-22  9:53 UTC (permalink / raw)
  To: lakshmi.sowjanya.d
  Cc: Linus Walleij, linux-gpio, LKML, mgross, Andy Shevchenko,
	tamal.saha, bala.senthil

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
>
> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
>
> Add a flag for event count and an extended structure to capture the event
> count when the flag is enabled.
>
> Intel(R) PMC Timed I/O device has an event count register counting
> the number of missed input edges. The register interface captures the
> event count and timestamp of the last event. For an event rate
> exceeding the rate that software can read events, the software can use
> the missed event count to calculate average event rates.
>
> The application requests one or both rising and falling edges when
> initializing the interface. The count of the selected edge type is
> optionally selected with an added event type flag. The event count is
> returned in an extended buffer using the read() interface.
>
> Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>
> ---
>  drivers/gpio/gpiolib-cdev.c | 28 +++++++++++++++++++---------
>  include/linux/gpio/driver.h |  1 +
>  include/uapi/linux/gpio.h   | 12 ++++++++++++
>  3 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 1df28a71f88b..3b5719d5e2dc 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -518,7 +518,8 @@ struct linereq {
>          GPIO_V2_LINE_DRIVE_FLAGS | \
>          GPIO_V2_LINE_EDGE_FLAGS | \
>          GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
> -        GPIO_V2_LINE_BIAS_FLAGS)
> +        GPIO_V2_LINE_BIAS_FLAGS | \
> +        GPIO_V2_LINE_FLAG_EVENT_COUNT)
>
>  static void linereq_put_event(struct linereq *lr,
>                               struct gpio_v2_line_event *le)
> @@ -1252,10 +1253,14 @@ static ssize_t linereq_read(struct file *file,
>         struct linereq *lr = file->private_data;
>         struct gpioevent_poll_data poll_data;
>         struct gpio_v2_line_event le;
> +       size_t min_userbuf_size;
>         ssize_t bytes_read = 0;
>         int ret, offset;
>
> -       if (count < sizeof(le))
> +       min_userbuf_size = sizeof(le);
> +       min_userbuf_size += lr->lines[0].eflags & GPIO_V2_LINE_FLAG_EVENT_COUNT ?
> +                                       sizeof(struct gpio_v2_line_event_ext) : 0;
> +       if (count < min_userbuf_size)
>                 return -EINVAL;
>
>         /* Without an IRQ, we can only poll */
> @@ -1270,12 +1275,17 @@ static ssize_t linereq_read(struct file *file,
>                                               lr->lines[offset].eflags, &poll_data);
>                 if (ret)
>                         return ret;
> -               event = kzalloc(sizeof(*event), GFP_KERNEL);
> +               event = kzalloc(min_userbuf_size, GFP_KERNEL);
>                 event->timestamp_ns = poll_data.timestamp;
>                 event->id = poll_data.id;
> -               if (copy_to_user(buf, (void *)&event, sizeof(event)))
> -                       return -EFAULT;
> -               return sizeof(event);
> +               if (lr->lines[offset].eflags & GPIO_V2_LINE_FLAG_EVENT_COUNT)
> +                       event->ext[0].event_count = poll_data.event_count;
> +
> +               ret = copy_to_user(buf, (void *)event, min_userbuf_size);
> +               if (ret)
> +                       ret = -EFAULT;
> +               kfree(event);
> +               return ret ? ret : min_userbuf_size;
>         }
>
>         do {
> @@ -1396,7 +1406,7 @@ static int setup_input(struct linereq *lr, struct gpio_v2_line_config *lc,
>         ret = edge_detector_setup(&lr->lines[line_no], lc, line_no,
>                                   lflags & GPIO_V2_LINE_EDGE_FLAGS);
>         if (ret < 0) {
> -               if (ret != -ENXIO) {
> +               if (ret == -ENXIO) {
>                         if (lr->gdev->chip->setup_poll &&
>                             lr->gdev->chip->setup_poll(lr->gdev->chip, offset,
>                                                        &lflags) == 0 &&
> @@ -1513,7 +1523,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>                                 goto out_free_linereq;
>                 }
>
> -               file_flags = O_RDONLY | O_CLOEXEC;
> +               file_flags = O_CLOEXEC;
>                 file_flags |= output ? O_WRONLY : O_RDONLY;
>                 file_flags |= (!output && !lr->lines[i].irq) ? O_NONBLOCK : 0;
>
> @@ -1524,7 +1534,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>                         offset);
>         }
>
> -       fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
> +       fd = get_unused_fd_flags(file_flags);
>         if (fd < 0) {
>                 ret = fd;
>                 goto out_free_linereq;
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 561e289434aa..09637fcbfd52 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -493,6 +493,7 @@ struct gpio_chip {
>  struct gpioevent_poll_data {
>         __u64 timestamp;
>         __u32 id;
> +       __u32 event_count;
>  };
>
>  struct gpio_output_event_data {
> diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
> index c39efc459b9f..e7fff2a205ec 100644
> --- a/include/uapi/linux/gpio.h
> +++ b/include/uapi/linux/gpio.h
> @@ -80,6 +80,7 @@ enum gpio_v2_line_flag {
>         GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN        = _BITULL(9),
>         GPIO_V2_LINE_FLAG_BIAS_DISABLED         = _BITULL(10),
>         GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME  = _BITULL(11),
> +       GPIO_V2_LINE_FLAG_EVENT_COUNT           = _BITULL(12),
>  };
>
>  /**
> @@ -270,6 +271,15 @@ enum gpio_v2_line_event_id {
>         GPIO_V2_LINE_EVENT_UNKNOWN_EDGE = 3,
>  };
>
> +/**
> + * struct gpio_v2_line_event_ext - Extended gpio line event
> + * @event_count: count of events
> + */
> +struct gpio_v2_line_event_ext {
> +       __u32 event_count;
> +       __u32 reserved[3];
> +};
> +
>  /**
>   * struct gpio_v2_line_event - The actual event being pushed to userspace
>   * @timestamp_ns: best estimate of time of event occurrence, in nanoseconds.
> @@ -280,6 +290,7 @@ enum gpio_v2_line_event_id {
>   * @line_seqno: the sequence number for this event in the sequence of
>   * events on this particular line
>   * @padding: reserved for future use
> + * @gpio_v2_line_event_ext: Extended gpio line event
>   *
>   * By default the @timestamp_ns is read from %CLOCK_MONOTONIC and is
>   * intended to allow the accurate measurement of the time between events.
> @@ -296,6 +307,7 @@ struct gpio_v2_line_event {
>         __u32 line_seqno;
>         /* Space reserved for future use. */
>         __u32 padding[6];
> +       struct gpio_v2_line_event_ext ext[];

This bit is an instant NAK as it breaks the ABI.

While I understand this is a bit of a different problem than the one
handled by the seqno fields, I think you should just think about
adding a field called something like "real_seqno" for those hardware
counted sequence numbers.

Bart

>  };
>
>  /**
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib
  2021-08-24 16:47 ` [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib lakshmi.sowjanya.d
@ 2021-09-22 10:03   ` Bartosz Golaszewski
  2021-10-07  2:14     ` Kent Gibson
  0 siblings, 1 reply; 35+ messages in thread
From: Bartosz Golaszewski @ 2021-09-22 10:03 UTC (permalink / raw)
  To: lakshmi.sowjanya.d
  Cc: Linus Walleij, linux-gpio, LKML, mgross, Andy Shevchenko,
	tamal.saha, bala.senthil, Kent Gibson

On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
>
> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
>
> Some Intel Timed I/O devices do not implement IRQ functionality. Augment
> read() interface to allow polling.
>
> Add two GPIO device methods: setup_poll() and poll():
> - setup_poll() configures the GPIO interface e.g. capture rising edges
> - poll() checks for events on the interface
>
> To implement polling, the driver must implement the two functions above
> and should either leave to_irq() method NULL or return irq 0.
>
> setup_poll() should configure the hardware to 'listen' for input events.
>
> poll() driver implementation must return the realtime timestamp
> corresponding to the event and -EAGAIN if no data is available.
>
> Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Reviewed-by: Mark Gross <mgross@linux.intel.com>
> ---

Interesting. So the idea is to allow user-space to read line events as
if they were generated by interrupts handled in the kernel. While this
whole series has a long way to go and this patch looks wrong to me in
several places at first glance, I find the idea interesting. Cc'ing
Kent who's the author of most of this code - Kent: what do you think?

Bart

>  drivers/gpio/gpiolib-cdev.c | 28 ++++++++++++++++++++++++++--
>  include/linux/gpio/driver.h | 19 +++++++++++++++++++
>  2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index c7b5446d01fd..4741bf34750b 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -1227,13 +1227,34 @@ static ssize_t linereq_read(struct file *file,
>                             loff_t *f_ps)

Why would you do this in linereq_read()? Userspace ends up in
linereq_poll() when it calls poll().

>  {
>         struct linereq *lr = file->private_data;
> +       struct gpioevent_poll_data poll_data;
>         struct gpio_v2_line_event le;
>         ssize_t bytes_read = 0;
> -       int ret;
> +       int ret, offset;
>
>         if (count < sizeof(le))
>                 return -EINVAL;
>
> +       /* Without an IRQ, we can only poll */
> +       offset = gpio_chip_hwgpio(lr->gdev->descs);
> +       if (lr->lines[offset].irq == 0) {
> +               struct gpio_v2_line_event *event;
> +
> +               if (!(file->f_flags & O_NONBLOCK))
> +                       return -ENODEV;
> +
> +               ret = lr->gdev->chip->do_poll(lr->gdev->chip, offset,
> +                                             lr->lines[offset].eflags, &poll_data);

What if the driver doesn't implement do_poll()?

> +               if (ret)
> +                       return ret;
> +               event = kzalloc(sizeof(*event), GFP_KERNEL);
> +               event->timestamp_ns = poll_data.timestamp;
> +               event->id = poll_data.id;
> +               if (copy_to_user(buf, (void *)&event, sizeof(event)))
> +                       return -EFAULT;
> +               return sizeof(event);
> +       }
> +
>         do {
>                 spin_lock(&lr->wait.lock);
>                 if (kfifo_is_empty(&lr->events)) {
> @@ -1314,6 +1335,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>  {
>         struct gpio_v2_line_request ulr;
>         struct gpio_v2_line_config *lc;
> +       unsigned int file_flags;
>         struct linereq *lr;
>         struct file *file;
>         u64 flags;
> @@ -1411,6 +1433,8 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>                                 goto out_free_linereq;
>                 }
>
> +               file_flags = O_RDONLY | O_CLOEXEC;
> +
>                 blocking_notifier_call_chain(&desc->gdev->notifier,
>                                              GPIO_V2_LINE_CHANGED_REQUESTED, desc);
>
> @@ -1425,7 +1449,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>         }
>
>         file = anon_inode_getfile("gpio-line", &line_fileops, lr,
> -                                 O_RDONLY | O_CLOEXEC);
> +                                 file_flags);
>         if (IS_ERR(file)) {
>                 ret = PTR_ERR(file);
>                 goto out_put_unused_fd;
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 3a268781fcec..f5b971ad40bc 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -17,6 +17,7 @@ struct device_node;
>  struct seq_file;
>  struct gpio_device;
>  struct module;
> +struct gpioevent_poll_data;
>  enum gpiod_flags;
>  enum gpio_lookup_flags;
>
> @@ -304,6 +305,11 @@ struct gpio_irq_chip {
>   * @add_pin_ranges: optional routine to initialize pin ranges, to be used when
>   *     requires special mapping of the pins that provides GPIO functionality.
>   *     It is called after adding GPIO chip and before adding IRQ chip.
> + * @setup_poll: optional routine for devices that don't support interrupts.
> + *     Takes flags argument as in/out parameter, where caller requests
> + *     event flags and driver returns accepted flags.
> + * @do_poll: optional routine for devices that don't support interrupts.
> + *     Returns event specification in data parameter.
>   * @base: identifies the first GPIO number handled by this chip;
>   *     or, if negative during registration, requests dynamic ID allocation.
>   *     DEPRECATION: providing anything non-negative and nailing the base
> @@ -396,6 +402,14 @@ struct gpio_chip {
>
>         int                     (*add_pin_ranges)(struct gpio_chip *gc);
>
> +       int                     (*setup_poll)(struct gpio_chip *chip,
> +                                             unsigned int offset,
> +                                             u32 *eflags);

Does anyone even call this?

> +
> +       int                     (*do_poll)(struct gpio_chip *chip,
> +                                          unsigned int offset, u32 eflags,
> +                                          struct gpioevent_poll_data *data);
> +
>         int                     base;
>         u16                     ngpio;
>         const char              *const *names;
> @@ -471,6 +485,11 @@ struct gpio_chip {
>  #endif /* CONFIG_OF_GPIO */
>  };
>
> +struct gpioevent_poll_data {
> +       __u64 timestamp;
> +       __u32 id;
> +};
> +
>  extern const char *gpiochip_is_requested(struct gpio_chip *gc,
>                         unsigned int offset);
>
> --
> 2.17.1
>

This patch doesn't look good - or even tested - but as I said - the
idea itself sounds reasonable in general.

Bart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib
  2021-09-22 10:03   ` Bartosz Golaszewski
@ 2021-10-07  2:14     ` Kent Gibson
  0 siblings, 0 replies; 35+ messages in thread
From: Kent Gibson @ 2021-10-07  2:14 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: lakshmi.sowjanya.d, Linus Walleij, linux-gpio, LKML, mgross,
	Andy Shevchenko, tamal.saha, bala.senthil

On Wed, Sep 22, 2021 at 12:03:53PM +0200, Bartosz Golaszewski wrote:
> On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
> >
> > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> >
> > Some Intel Timed I/O devices do not implement IRQ functionality. Augment
> > read() interface to allow polling.
> >
> > Add two GPIO device methods: setup_poll() and poll():
> > - setup_poll() configures the GPIO interface e.g. capture rising edges
> > - poll() checks for events on the interface
> >
> > To implement polling, the driver must implement the two functions above
> > and should either leave to_irq() method NULL or return irq 0.
> >
> > setup_poll() should configure the hardware to 'listen' for input events.
> >
> > poll() driver implementation must return the realtime timestamp
> > corresponding to the event and -EAGAIN if no data is available.
> >
> > Co-developed-by: Christopher Hall <christopher.s.hall@intel.com>
> > Signed-off-by: Christopher Hall <christopher.s.hall@intel.com>
> > Signed-off-by: Tamal Saha <tamal.saha@intel.com>
> > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> > ---
> 
> Interesting. So the idea is to allow user-space to read line events as
> if they were generated by interrupts handled in the kernel. While this
> whole series has a long way to go and this patch looks wrong to me in
> several places at first glance, I find the idea interesting. Cc'ing
> Kent who's the author of most of this code - Kent: what do you think?
> 

It is interesting that we're seeing more hardware that provides more
detailed edge info than we get from irq.  The hte patch series can also
provide hardware timestamps, but the Timed I/O could even provide the
sequence numbers.
It might be worth abstracting the edge detection so edge events could be
more easily driven by subsystems other than irq, without festooning cdev
with special cases.

I'm not a fan of the polling here though, particularly from userspace.
If polling can't be avoided (why did they not provide an irq??) then I
would do the polling in kernel and place any resulting events in the
cdev kfifo for userspace to read as per the existing events.

Of course that is without knowing a whole lot about the hardware or use
cases.  The Intel datasheet doesn't provide much in the way of data :|.

Cheers,
Kent.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC
  2021-09-16 21:21 ` [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC Linus Walleij
@ 2021-10-11 21:14   ` Dipen Patel
  0 siblings, 0 replies; 35+ messages in thread
From: Dipen Patel @ 2021-10-11 21:14 UTC (permalink / raw)
  To: Linus Walleij, D, Lakshmi Sowjanya
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-kernel,
	Mark Gross, Andy Shevchenko, Saha, Tamal, bala.senthil

Thanks Linus for referring. I will take a look.

@Sowjanya: I have recently sent out the RFC v2 of the HTE (https://patchwork.ozlabs.org/project/linux-tegra/list/?series=264896).

Please have a look and see if you can add TIO as one of the provider. That patch has necessary GPIOLIB and GPIO-CDEV changes

which can help userspace and in kernel driver retrieve GPIO realtime timestamps.


Best,

Dipen Patel

On 9/16/21 2:21 PM, Linus Walleij wrote:
> Hi Sowjanya,
>
> thanks for your patches!
>
> On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@intel.com> wrote:
>
>> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
>>
>> Starting with Intel(R) Tiger Lake and Elkhart Lake platforms the PMC
>> hardware adds the Timed I/O hardware interface.
>>
>> The Timed I/O hardware implements some functionality similar to GPIO
>> with added timing logic that is driven by the Always Running Timer
>> (ART).
>>
>> The Timed I/O Hardware implement 3 basic functions:
>>   * Input Timestamping
>>   * Single Shot Timed Output
>>   * Periodic Timed Output
>>
>>  Please help to review the changes.
> This looks very similar to the usecase proposed for the HTE
> Hardware Timestamping Engine, proposed by Dipen Patel
> for the nVidia 194 and which is currently in RFC:
> https://lore.kernel.org/linux-gpio/20210625235532.19575-1-dipenp@nvidia.com/
>
> Please review this new subsystem and see if you can just
> make a slot-in driver using Dipen's patches instead.
>
> Dipen: please have a look at Sowjanya's patches to see
> if this hardware is similar to yours.
>
> Sometimes several vendors come up with similar hardware
> around the same time, because of industry trends, so I would
> not be surprised if these two hardwares address the very
> same usecase.
>
> Yours,
> Linus Walleij

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2021-10-11 21:14 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 16:47 [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 01/20] gpio: Add basic GPIO driver for Intel PMC Timed I/O device lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 02/20] gpio: Add GPIO polling interface to GPIO lib lakshmi.sowjanya.d
2021-09-22 10:03   ` Bartosz Golaszewski
2021-10-07  2:14     ` Kent Gibson
2021-08-24 16:47 ` [RFC PATCH v1 03/20] arch: x86: Add ART support function to tsc code lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 04/20] gpio: Add input code to Intel PMC Timed I/O Driver lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 05/20] tools: gpio: Add additional polling support to gpio-event-mon lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 06/20] gpio: Add set_input and polling interface to GPIO lib code lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 07/20] gpio: Add output event generation method to GPIOLIB and PMC Driver lakshmi.sowjanya.d
2021-09-16 21:42   ` Linus Walleij
2021-09-17  7:27     ` Uwe Kleine-König
2021-09-19 19:38       ` Linus Walleij
2021-09-19 21:21         ` Clemens Gruber
2021-09-20  7:14           ` Uwe Kleine-König
2021-08-24 16:47 ` [RFC PATCH v1 08/20] kernel: time: Add system time to system counter translation lakshmi.sowjanya.d
2021-09-16 21:48   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 09/20] arch: x86: Add TSC to ART translation lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 10/20] tools: gpio: Add GPIO output generation user application lakshmi.sowjanya.d
2021-09-16 21:52   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib lakshmi.sowjanya.d
2021-09-22  9:53   ` Bartosz Golaszewski
2021-08-24 16:47 ` [RFC PATCH v1 12/20] gpio: Add event count to Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 13/20] tools: gpio: Add event count capability to event monitor application lakshmi.sowjanya.d
2021-09-16 21:57   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 14/20] arch/x86: Add ART nanosecond to ART conversion lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 15/20] pwm: Add capability for PWM Driver managed state lakshmi.sowjanya.d
2021-09-16 22:00   ` Linus Walleij
2021-08-24 16:47 ` [RFC PATCH v1 16/20] gpio: Add PWM capabilities to Intel(R) PMC TIO driver lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 17/20] pwm: Add second alignment to the core PWM interface lakshmi.sowjanya.d
2021-08-24 16:47 ` [RFC PATCH v1 18/20] gpio: Add PWM alignment support to the Intel(R) PMC Timed I/O driver lakshmi.sowjanya.d
2021-08-24 16:48 ` [RFC PATCH v1 19/20] gpio: Add GPIO monitor line to Intel(R) Timed I/O Driver lakshmi.sowjanya.d
2021-08-24 16:48 ` [RFC PATCH v1 20/20] tools: gpio: Add PWM monitor application lakshmi.sowjanya.d
2021-09-16 21:21 ` [RFC PATCH v1 00/20] Review Request: Add support for Intel PMC Linus Walleij
2021-10-11 21:14   ` Dipen Patel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.