From: Lakshmi Sowjanya D <[email protected]>
The goal of the PPS (Pulse Per Second) hardware/software is to generate a
signal from the system on a wire so that some third-party hardware can
observe that signal and judge how close the system's time is to another
system or piece of hardware.
Existing methods (like parallel ports) require software to flip a bit at
just the right time to create a PPS signal. Many things can prevent
software from doing this precisely. This (Timed I/O) method is better
because software only "arms" the hardware in advance and then depends on
the hardware to "fire" and flip the signal at just the right time.
To generate a PPS signal with this new hardware, the kernel wakes up
twice a second, once for 1->0 edge and other for the 0->1 edge. It does
this shortly (~10ms) before the actual change in the signal needs to be
made. It computes the TSC value at which edge will happen, convert to a
value hardware understands and program this value to Timed I/O hardware.
The actual edge transition happens without any further action from the
kernel.
The result here is a signal coming out of the system that is roughly
1,000 times more accurate than the old methods. If the system is heavily
loaded, the difference in accuracy is larger in old methods.
Application Interface:
The API to use Timed I/O is very simple. It is enabled and disabled by
writing a '1' or '0' value to the sysfs enable attribute associated with
the Timed I/O PPS device. Each Timed I/O pin is represented by a PPS
device. When enabled, a pulse-per-second (PPS) synchronized with the
system clock is continuously produced on the Timed I/O pin, otherwise it
is pulled low.
The Timed I/O signal on the motherboard is enabled in the BIOS setup.
Intel Advanced Menu -> PCH IO Configuration -> Timed I/O <Enable>
References:
https://en.wikipedia.org/wiki/Pulse-per-second_signal
https://drive.google.com/file/d/1vkBRRDuELmY8I3FlfOZaEBp-DxLW6t_V/view
https://youtu.be/JLUTT-lrDqw
Patch 1 adds base clock properties in clocksource structure
Patch 2 updates tsc, art values in the base clock structure
Patch 3 - 7 removes reference to convert_art_to_tsc function across
drivers
Patch 8 removes the convert art to tsc functions which are no longer
used
Patch 9 adds function to convert realtime to base clock
Patch 10 adds the pps(pulse per second) generator tio driver to the pps
subsystem.
Patch 11 documentation and usage of the pps tio generator module.
Patch 12 includes documentation for sysfs interface.
Please help to review the changes.
Thanks in advance,
Sowjanya
Changes from v2:
- Split patch 1 to remove the functions in later stages.
- Include required headers in pps_gen_tio.
Changes from v3:
- Corrections in Documentation.
- Introducing non-RFC version of the patch series.
Changes from v4:
- Setting id in ice_ptp
- Modified conversion logic in convert_base_to_cs.
- Included the usage of the APIs in the commit message of 2nd patch.
Changes from v5:
- Change nsecs variable to use_nsecs.
- Change order of 1&2 patches and modify the commit message.
- Add sysfs abi file entry in MAINTAINERS file.
- Add check to find if any event is missed and disable hardware
accordingly.
Changes from v6:
- Split patch 1 into 1&2 patches.
- Add check for overflow in convert_ns_to_cs().
- Refine commit messages.
Changes from v7:
- Split the if condition and return error if current time exceeds
expire time.
- Update kernel version and month in ABI file.
Lakshmi Sowjanya D (7):
timekeeping: Add base clock properties in clocksource structure
x86/tsc: Update tsc/art values in the base clock structure
x86/tsc: Remove art to tsc conversion functions which are obsolete
timekeeping: Add function to convert realtime to base clock
pps: generators: Add PPS Generator TIO Driver
Documentation: driver-api: pps: Add Intel Timed I/O PPS generator
ABI: pps: Add ABI documentation for Intel TIO
Thomas Gleixner (5):
e1000e: remove convert_art_to_tsc()
igc: remove convert_art_ns_to_tsc()
stmmac: intel: remove convert_art_to_tsc()
ALSA: hda: remove convert_art_to_tsc()
ice/ptp: remove convert_art_to_tsc()
.../ABI/testing/sysfs-platform-pps-tio | 7 +
Documentation/driver-api/pps.rst | 22 ++
MAINTAINERS | 1 +
arch/x86/include/asm/tsc.h | 3 -
arch/x86/kernel/tsc.c | 92 ++-----
drivers/net/ethernet/intel/e1000e/ptp.c | 3 +-
drivers/net/ethernet/intel/ice/ice_ptp.c | 3 +-
drivers/net/ethernet/intel/igc/igc_ptp.c | 6 +-
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 3 +-
drivers/pps/generators/Kconfig | 16 ++
drivers/pps/generators/Makefile | 1 +
drivers/pps/generators/pps_gen_tio.c | 259 ++++++++++++++++++
include/linux/clocksource.h | 27 ++
include/linux/clocksource_ids.h | 1 +
include/linux/timekeeping.h | 6 +
kernel/time/timekeeping.c | 124 ++++++++-
sound/pci/hda/hda_controller.c | 3 +-
17 files changed, 495 insertions(+), 82 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-platform-pps-tio
create mode 100644 drivers/pps/generators/pps_gen_tio.c
--
2.35.3
From: Thomas Gleixner <[email protected]>
The core code provides a mechanism to convert the ART base clock to the
corresponding TSC value without requiring an architecture specific
function.
All what is required is to store the ART clocksoure ID and the cycles
value in the provided system_counterval structure.
Replace the direct conversion via convert_art_to_tsc() by filling in the
required data.
No functional change intended.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
---
sound/pci/hda/hda_controller.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 206306a0eb82..6f648fae7a7b 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -463,7 +463,8 @@ static int azx_get_sync_time(ktime_t *device,
*device = ktime_add_ns(*device, (wallclk_cycles * NSEC_PER_SEC) /
((HDA_MAX_CYCLE_VALUE + 1) * runtime->rate));
- *system = convert_art_to_tsc(tsc_counter);
+ system->cycles = tsc_counter;
+ system->cs_id = CSID_X86_ART;
return 0;
}
--
2.35.3
From: Lakshmi Sowjanya D <[email protected]>
The convert_art_to_tsc() and convert_art_ns_to_tsc() interfaces are no
longer required. This conversion is internally done in
get_device_system_crosststamp() using convert_base_to_cs().
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
---
arch/x86/include/asm/tsc.h | 3 --
arch/x86/kernel/tsc.c | 60 --------------------------------------
2 files changed, 63 deletions(-)
diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 405efb3e4996..94408a784c8e 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -28,9 +28,6 @@ static inline cycles_t get_cycles(void)
}
#define get_cycles get_cycles
-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 void tsc_early_init(void);
extern void tsc_init(void);
extern void mark_tsc_unstable(char *reason);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 45bf2f6d0ffa..5f0bd441ed4d 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1297,66 +1297,6 @@ int unsynchronized_tsc(void)
return 0;
}
-/*
- * Convert ART to TSC given numerator/denominator found in detect_art()
- */
-struct system_counterval_t convert_art_to_tsc(u64 art)
-{
- u64 tmp, res, rem;
-
- rem = do_div(art, art_base_clk.denominator);
-
- res = art * art_base_clk.numerator;
- tmp = rem * art_base_clk.numerator;
-
- do_div(tmp, art_base_clk.denominator);
- res += tmp + art_base_clk.offset;
-
- return (struct system_counterval_t) {
- .cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
- .cycles = res,
- };
-}
-EXPORT_SYMBOL(convert_art_to_tsc);
-
-/**
- * convert_art_ns_to_tsc() - Convert ART in nanoseconds to TSC.
- * @art_ns: ART (Always Running Timer) in unit of nanoseconds
- *
- * PTM requires all timestamps to be in units of nanoseconds. When user
- * software requests a cross-timestamp, this function converts system timestamp
- * to TSC.
- *
- * 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 TSC is attempted.
- *
- * Return:
- * struct system_counterval_t - system counter value with the ID of the
- * corresponding clocksource:
- * cycles: System counter value
- * cs_id: The clocksource ID for validating comparability
- */
-
-struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns)
-{
- u64 tmp, res, rem;
-
- rem = do_div(art_ns, USEC_PER_SEC);
-
- res = art_ns * tsc_khz;
- tmp = rem * tsc_khz;
-
- do_div(tmp, USEC_PER_SEC);
- res += tmp;
-
- return (struct system_counterval_t) {
- .cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
- .cycles = res,
- };
-}
-EXPORT_SYMBOL(convert_art_ns_to_tsc);
-
static void tsc_refine_calibration_work(struct work_struct *work);
static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
/**
--
2.35.3
From: Thomas Gleixner <[email protected]>
The core code provides a mechanism to convert the ART base clock to the
corresponding TSC value without requiring an architecture specific
function.
All what is required is to store the ART clocksoure ID and the cycles
value in the provided system_counterval structure.
Replace the direct conversion via convert_art_to_tsc() by filling in the
required data.
No functional change intended.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index c11eba07283c..c416dd2e6622 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2116,7 +2116,8 @@ ice_ptp_get_syncdevicetime(ktime_t *device,
hh_ts_lo = rd32(hw, GLHH_ART_TIME_L);
hh_ts_hi = rd32(hw, GLHH_ART_TIME_H);
hh_ts = ((u64)hh_ts_hi << 32) | hh_ts_lo;
- *system = convert_art_ns_to_tsc(hh_ts);
+ system->cycles = hh_ts;
+ system->cs_id = CSID_X86_ART;
/* Read Device source clock time */
hh_ts_lo = rd32(hw, GLTSYN_HHTIME_L(tmr_idx));
hh_ts_hi = rd32(hw, GLTSYN_HHTIME_H(tmr_idx));
--
2.35.3
From: Lakshmi Sowjanya D <[email protected]>
The Intel Timed IO PPS generator driver outputs a PPS signal using
dedicated hardware that is more accurate than software actuated PPS.
The Timed IO hardware generates output events using the ART timer.
The ART timer period varies based on platform type, but is less than 100
nanoseconds for all current platforms. Timed IO output accuracy is
within 1 ART period.
PPS output is enabled by writing '1' the 'enable' sysfs attribute. The
driver uses hrtimers to schedule a wake-up 10 ms before each event
(edge) target time. At wakeup, the driver converts the target time in
terms of CLOCK_REALTIME to ART trigger time and writes this to the Timed
IO hardware. The Timed IO hardware generates an event precisely at the
requested system time without software involvement.
Co-developed-by: Christopher Hall <[email protected]>
Signed-off-by: Christopher Hall <[email protected]>
Co-developed-by: Pandith N <[email protected]>
Signed-off-by: Pandith N <[email protected]>
Co-developed-by: Thejesh Reddy T R <[email protected]>
Signed-off-by: Thejesh Reddy T R <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
Reviewed-by: Eddie Dong <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
---
drivers/pps/generators/Kconfig | 16 ++
drivers/pps/generators/Makefile | 1 +
drivers/pps/generators/pps_gen_tio.c | 259 +++++++++++++++++++++++++++
3 files changed, 276 insertions(+)
create mode 100644 drivers/pps/generators/pps_gen_tio.c
diff --git a/drivers/pps/generators/Kconfig b/drivers/pps/generators/Kconfig
index d615e640fcad..0f090932336f 100644
--- a/drivers/pps/generators/Kconfig
+++ b/drivers/pps/generators/Kconfig
@@ -12,3 +12,19 @@ config PPS_GENERATOR_PARPORT
If you say yes here you get support for a PPS signal generator which
utilizes STROBE pin of a parallel port to send PPS signals. It uses
parport abstraction layer and hrtimers to precisely control the signal.
+
+config PPS_GENERATOR_TIO
+ tristate "TIO PPS signal generator"
+ depends on X86 && CPU_SUP_INTEL
+ help
+ If you say yes here you get support for a PPS TIO signal generator
+ which generates a pulse at a prescribed time based on the system clock.
+ It uses time translation and hrtimers to precisely generate a pulse.
+ This hardware is present on 2019 and newer Intel CPUs. However, this
+ driver is not useful without adding highly specialized hardware outside
+ the Linux system to observe these pulses.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pps_gen_tio.
+
+ If unsure, say N.
diff --git a/drivers/pps/generators/Makefile b/drivers/pps/generators/Makefile
index 2589fd0f2481..714e847ae193 100644
--- a/drivers/pps/generators/Makefile
+++ b/drivers/pps/generators/Makefile
@@ -4,5 +4,6 @@
#
obj-$(CONFIG_PPS_GENERATOR_PARPORT) += pps_gen_parport.o
+obj-$(CONFIG_PPS_GENERATOR_TIO) += pps_gen_tio.o
ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG
diff --git a/drivers/pps/generators/pps_gen_tio.c b/drivers/pps/generators/pps_gen_tio.c
new file mode 100644
index 000000000000..c6b7f13ffeca
--- /dev/null
+++ b/drivers/pps/generators/pps_gen_tio.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel PPS signal Generator Driver
+ *
+ * Copyright (C) 2024 Intel Corporation
+ */
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/container_of.h>
+#include <linux/cpu.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/hrtimer.h>
+#include <linux/io-64-nonatomic-hi-lo.h>
+#include <linux/kstrtox.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/sysfs.h>
+#include <linux/timekeeping.h>
+#include <linux/types.h>
+
+#include <asm/cpu_device_id.h>
+
+#define TIOCTL 0x00
+#define TIOCOMPV 0x10
+#define TIOEC 0x30
+
+/* Control Register */
+#define TIOCTL_EN BIT(0)
+#define TIOCTL_DIR BIT(1)
+#define TIOCTL_EP GENMASK(3, 2)
+#define TIOCTL_EP_RISING_EDGE FIELD_PREP(TIOCTL_EP, 0)
+#define TIOCTL_EP_FALLING_EDGE FIELD_PREP(TIOCTL_EP, 1)
+#define TIOCTL_EP_TOGGLE_EDGE FIELD_PREP(TIOCTL_EP, 2)
+
+#define SAFE_TIME_NS (10 * NSEC_PER_MSEC) /* Safety time to set hrtimer early */
+#define MAGIC_CONST (NSEC_PER_SEC - SAFE_TIME_NS)
+#define ART_HW_DELAY_CYCLES 2
+
+struct pps_tio {
+ struct hrtimer timer;
+ struct device *dev;
+ spinlock_t lock;
+ struct attribute_group attrs;
+ void __iomem *base;
+ bool enabled;
+ u32 prev_count;
+};
+
+static inline u32 pps_tio_read(struct pps_tio *tio, u32 offset)
+{
+ return readl(tio->base + offset);
+}
+
+static inline void pps_ctl_write(struct pps_tio *tio, u32 value)
+{
+ writel(value, tio->base + TIOCTL);
+}
+
+/* For COMPV register, It's safer to write higher 32-bit followed by lower 32-bit */
+static inline void pps_compv_write(struct pps_tio *tio, u64 value)
+{
+ hi_lo_writeq(value, tio->base + TIOCOMPV);
+}
+
+static inline ktime_t first_event(struct pps_tio *tio)
+{
+ return ktime_set(ktime_get_real_seconds() + 1, MAGIC_CONST);
+}
+
+static u32 pps_tio_disable(struct pps_tio *tio)
+{
+ u32 ctrl;
+
+ ctrl = pps_tio_read(tio, TIOCTL);
+ pps_compv_write(tio, 0);
+
+ ctrl &= ~TIOCTL_EN;
+ pps_ctl_write(tio, ctrl);
+ tio->prev_count = 0;
+
+ return ctrl;
+}
+
+static void pps_tio_direction_output(struct pps_tio *tio)
+{
+ u32 ctrl;
+
+ ctrl = pps_tio_disable(tio);
+
+ /* We enable the device, be sure that the 'compare' value is invalid */
+ pps_compv_write(tio, 0);
+
+ ctrl &= ~(TIOCTL_DIR | TIOCTL_EP);
+ ctrl |= TIOCTL_EP_TOGGLE_EDGE;
+ pps_ctl_write(tio, ctrl);
+
+ ctrl |= TIOCTL_EN;
+ pps_ctl_write(tio, ctrl);
+}
+
+static bool pps_generate_next_pulse(struct pps_tio *tio, ktime_t expires)
+{
+ u64 art;
+
+ if (!ktime_real_to_base_clock(expires, CSID_X86_ART, &art)) {
+ pps_tio_disable(tio);
+ return false;
+ }
+
+ pps_compv_write(tio, art - ART_HW_DELAY_CYCLES);
+ return true;
+}
+
+static enum hrtimer_restart hrtimer_callback(struct hrtimer *timer)
+{
+ struct pps_tio *tio = container_of(timer, struct pps_tio, timer);
+ ktime_t expires, now;
+ u32 event_count;
+
+ guard(spinlock)(&tio->lock);
+
+ /* Check if any event is missed. If an event is missed, TIO will be disabled*/
+ event_count = pps_tio_read(tio, TIOEC);
+ if (tio->prev_count && tio->prev_count == event_count)
+ goto err;
+ tio->prev_count = event_count;
+ expires = hrtimer_get_expires(timer);
+ now = ktime_get_real();
+
+ if (now - expires >= SAFE_TIME_NS)
+ goto err;
+
+ if (!pps_generate_next_pulse(tio, expires + SAFE_TIME_NS))
+ return HRTIMER_NORESTART;
+
+ hrtimer_forward(timer, now, NSEC_PER_SEC / 2);
+ return HRTIMER_RESTART;
+err:
+ dev_err(tio->dev, "Event missed, Disabling Timed I/O");
+ pps_tio_disable(tio);
+ return HRTIMER_NORESTART;
+}
+
+static ssize_t enable_store(struct device *dev, struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct pps_tio *tio = dev_get_drvdata(dev);
+ bool enable;
+ int err;
+
+ err = kstrtobool(buf, &enable);
+ if (err)
+ return err;
+
+ guard(spinlock_irqsave)(&tio->lock);
+ if (enable && !tio->enabled) {
+ if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
+ dev_err(tio->dev, "PPS cannot be started as clock is not related to ART");
+ return -EPERM;
+ }
+ pps_tio_direction_output(tio);
+ hrtimer_start(&tio->timer, first_event(tio), HRTIMER_MODE_ABS);
+ tio->enabled = true;
+ } else if (!enable && tio->enabled) {
+ hrtimer_cancel(&tio->timer);
+ pps_tio_disable(tio);
+ tio->enabled = false;
+ }
+ return count;
+}
+
+static ssize_t enable_show(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+ struct pps_tio *tio = dev_get_drvdata(dev);
+ u32 ctrl;
+
+ ctrl = pps_tio_read(tio, TIOCTL);
+ ctrl &= TIOCTL_EN;
+
+ return sysfs_emit(buf, "%u\n", ctrl);
+}
+static DEVICE_ATTR_RW(enable);
+
+static struct attribute *pps_tio_attrs[] = {
+ &dev_attr_enable.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(pps_tio);
+
+static int pps_tio_probe(struct platform_device *pdev)
+{
+ struct pps_tio *tio;
+
+ if (!(cpu_feature_enabled(X86_FEATURE_TSC_KNOWN_FREQ) &&
+ cpu_feature_enabled(X86_FEATURE_ART))) {
+ dev_warn(&pdev->dev, "TSC/ART is not enabled");
+ return -ENODEV;
+ }
+
+ tio = devm_kzalloc(&pdev->dev, sizeof(*tio), GFP_KERNEL);
+ if (!tio)
+ return -ENOMEM;
+
+ tio->dev = &pdev->dev;
+ tio->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(tio->base))
+ return PTR_ERR(tio->base);
+
+ pps_tio_disable(tio);
+ hrtimer_init(&tio->timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
+ tio->timer.function = hrtimer_callback;
+ spin_lock_init(&tio->lock);
+ tio->enabled = false;
+ platform_set_drvdata(pdev, tio);
+
+ return 0;
+}
+
+static int pps_tio_remove(struct platform_device *pdev)
+{
+ struct pps_tio *tio = platform_get_drvdata(pdev);
+
+ hrtimer_cancel(&tio->timer);
+ pps_tio_disable(tio);
+
+ return 0;
+}
+
+static const struct acpi_device_id intel_pmc_tio_acpi_match[] = {
+ { "INTC1021" },
+ { "INTC1022" },
+ { "INTC1023" },
+ { "INTC1024" },
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, intel_pmc_tio_acpi_match);
+
+static struct platform_driver pps_tio_driver = {
+ .probe = pps_tio_probe,
+ .remove = pps_tio_remove,
+ .driver = {
+ .name = "intel-pps-generator",
+ .acpi_match_table = intel_pmc_tio_acpi_match,
+ .dev_groups = pps_tio_groups,
+ },
+};
+module_platform_driver(pps_tio_driver);
+
+MODULE_AUTHOR("Lakshmi Sowjanya D <[email protected]>");
+MODULE_AUTHOR("Christopher Hall <[email protected]>");
+MODULE_AUTHOR("Pandith N <[email protected]>");
+MODULE_AUTHOR("Thejesh Reddy T R <[email protected]>");
+MODULE_DESCRIPTION("Intel PMC Time-Aware IO Generator Driver");
+MODULE_LICENSE("GPL");
--
2.35.3
From: Lakshmi Sowjanya D <[email protected]>
Document sysfs interface for Intel Timed I/O PPS driver.
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
---
Documentation/ABI/testing/sysfs-platform-pps-tio | 7 +++++++
MAINTAINERS | 1 +
2 files changed, 8 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform-pps-tio
diff --git a/Documentation/ABI/testing/sysfs-platform-pps-tio b/Documentation/ABI/testing/sysfs-platform-pps-tio
new file mode 100644
index 000000000000..74f3244b55dc
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-pps-tio
@@ -0,0 +1,7 @@
+What: /sys/devices/platform/INTCxxxx/enable
+Date: June 2024
+KernelVersion: 6.11
+Contact: Lakshmi Sowjanya D <[email protected]>
+Description:
+ (RW) Enable or disable PPS TIO generator output, read to
+ see the status of hardware (Enabled/Disabled).
diff --git a/MAINTAINERS b/MAINTAINERS
index 28e20975c26f..97606c072e3f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17670,6 +17670,7 @@ M: Rodolfo Giometti <[email protected]>
L: [email protected] (subscribers-only)
S: Maintained
W: http://wiki.enneenne.com/index.php/LinuxPPS_support
+F: Documentation/ABI/testing/sysfs-platform-pps-tio
F: Documentation/ABI/testing/sysfs-pps
F: Documentation/devicetree/bindings/pps/pps-gpio.yaml
F: Documentation/driver-api/pps.rst
--
2.35.3
From: Thomas Gleixner <[email protected]>
The core code provides a mechanism to convert the ART base clock to the
corresponding TSC value without requiring an architecture specific
function.
All what is required is to store the ART clocksoure ID and the cycles
value in the provided system_counterval structure.
Replace the direct conversion via convert_art_ns_to_tsc() by filling in
the required data.
No functional change intended.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
---
drivers/net/ethernet/intel/igc/igc_ptp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 885faaa7b9de..39656489d73e 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -901,7 +901,11 @@ static bool igc_is_crosststamp_supported(struct igc_adapter *adapter)
static struct system_counterval_t igc_device_tstamp_to_system(u64 tstamp)
{
#if IS_ENABLED(CONFIG_X86_TSC) && !defined(CONFIG_UML)
- return convert_art_ns_to_tsc(tstamp);
+ return (struct system_counterval_t) {
+ .cs_id = CSID_X86_ART,
+ .cycles = tstamp,
+ .use_nsecs = true,
+ };
#else
return (struct system_counterval_t) { };
#endif
--
2.35.3
From: Lakshmi Sowjanya D <[email protected]>
Add Intel Timed I/O PPS usage instructions.
Co-developed-by: Pandith N <[email protected]>
Signed-off-by: Pandith N <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
---
Documentation/driver-api/pps.rst | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/Documentation/driver-api/pps.rst b/Documentation/driver-api/pps.rst
index 78dded03e5d8..52a6d5faf885 100644
--- a/Documentation/driver-api/pps.rst
+++ b/Documentation/driver-api/pps.rst
@@ -246,3 +246,25 @@ delay between assert and clear edge as small as possible to reduce system
latencies. But if it is too small slave won't be able to capture clear edge
transition. The default of 30us should be good enough in most situations.
The delay can be selected using 'delay' pps_gen_parport module parameter.
+
+
+Intel Timed I/O PPS signal generator
+------------------------------------
+
+Intel Timed I/O is a high precision device, present on 2019 and newer Intel
+CPUs, that can generate PPS signals.
+
+Timed I/O and system time are both driven by same hardware clock. The signal
+is generated with a precision of ~20 nanoseconds. The generated PPS signal
+is used to synchronize an external device with system clock. For example,
+share your clock with a device that receives PPS signal, generated by
+Timed I/O device. There are dedicated Timed I/O pins to deliver the PPS signal
+to an external device.
+
+Usage of Intel Timed I/O as PPS generator:
+
+Start generating PPS signal::
+ $echo 1 > /sys/devices/platform/INTCxxxx\:00/enable
+
+Stop generating PPS signal::
+ $echo 0 > /sys/devices/platform/INTCxxxx\:00/enable
--
2.35.3
On Mon, May 13, 2024 at 04:08:11PM +0530, [email protected] wrote:
> From: Lakshmi Sowjanya D <[email protected]>
>
> The Intel Timed IO PPS generator driver outputs a PPS signal using
> dedicated hardware that is more accurate than software actuated PPS.
> The Timed IO hardware generates output events using the ART timer.
> The ART timer period varies based on platform type, but is less than 100
> nanoseconds for all current platforms. Timed IO output accuracy is
> within 1 ART period.
>
> PPS output is enabled by writing '1' the 'enable' sysfs attribute. The
> driver uses hrtimers to schedule a wake-up 10 ms before each event
> (edge) target time. At wakeup, the driver converts the target time in
> terms of CLOCK_REALTIME to ART trigger time and writes this to the Timed
> IO hardware. The Timed IO hardware generates an event precisely at the
> requested system time without software involvement.
..
> +static ssize_t enable_store(struct device *dev, struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + struct pps_tio *tio = dev_get_drvdata(dev);
> + bool enable;
> + int err;
(1)
> + err = kstrtobool(buf, &enable);
> + if (err)
> + return err;
> +
> + guard(spinlock_irqsave)(&tio->lock);
> + if (enable && !tio->enabled) {
> + if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
> + dev_err(tio->dev, "PPS cannot be started as clock is not related to ART");
Why not simply dev_err(dev, ...)?
> + return -EPERM;
> + }
I'm wondering if we can move this check to (1) above.
Because currently it's a good question if we are able to stop PPS which was run
by somebody else without this check done.
I.o.w. this sounds too weird to me and reading the code doesn't give any hint
if it's even possible. And if it is, are we supposed to touch that since it was
definitely *not* us who ran it.
> + pps_tio_direction_output(tio);
> + hrtimer_start(&tio->timer, first_event(tio), HRTIMER_MODE_ABS);
> + tio->enabled = true;
> + } else if (!enable && tio->enabled) {
> + hrtimer_cancel(&tio->timer);
> + pps_tio_disable(tio);
> + tio->enabled = false;
> + }
> + return count;
> +}
..
> +static int pps_tio_probe(struct platform_device *pdev)
> +{
struct device *dev = &pdev->dev;
> + struct pps_tio *tio;
> +
> + if (!(cpu_feature_enabled(X86_FEATURE_TSC_KNOWN_FREQ) &&
> + cpu_feature_enabled(X86_FEATURE_ART))) {
> + dev_warn(&pdev->dev, "TSC/ART is not enabled");
dev_warn(dev, "TSC/ART is not enabled");
> + return -ENODEV;
> + }
> +
> + tio = devm_kzalloc(&pdev->dev, sizeof(*tio), GFP_KERNEL);
tio = devm_kzalloc(dev, sizeof(*tio), GFP_KERNEL);
> + if (!tio)
> + return -ENOMEM;
> +
> + tio->dev = &pdev->dev;
tio->dev = dev;
> + tio->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(tio->base))
> + return PTR_ERR(tio->base);
> + pps_tio_disable(tio);
This...
> + hrtimer_init(&tio->timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
> + tio->timer.function = hrtimer_callback;
> + spin_lock_init(&tio->lock);
> + tio->enabled = false;
..and this should go together, which makes me look at the enabled flag over
the code and it seems there are a few places where you missed to sync it with
the reality.
I would think of something like this:
pps_tio_direction_output() ==> true
pps_tio_disable(tio) ==> false
where "==> X" means assignment of enabled flag.
And perhaps this:
tio->enabled = pps_generate_next_pulse(tio, expires + SAFE_TIME_NS);
if (!tio->enabled)
...
But the above is just thinking out loudly, you may find the better approach(es).
> + platform_set_drvdata(pdev, tio);
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
On Mon, May 13, 2024 at 04:08:13PM +0530, [email protected] wrote:
> From: Lakshmi Sowjanya D <[email protected]>
>
> Document sysfs interface for Intel Timed I/O PPS driver.
..
> +Date: June 2024
Is this checked by phb?
"the v6.11 kernel predictions: merge window closes on Sunday, 2024-08-04 and
release on Sunday, 2024-09-29"
> +KernelVersion: 6.11
--
With Best Regards,
Andy Shevchenko
On Mon, May 13, 2024 at 02:18:49PM +0300, Andy Shevchenko wrote:
> On Mon, May 13, 2024 at 04:08:11PM +0530, [email protected] wrote:
> > + pps_tio_disable(tio);
>
> This...
> > + tio->enabled = false;
>
> ...and this should go together, which makes me look at the enabled flag over
> the code and it seems there are a few places where you missed to sync it with
> the reality.
>
> I would think of something like this:
>
> pps_tio_direction_output() ==> true
> pps_tio_disable(tio) ==> false
>
> where "==> X" means assignment of enabled flag.
>
> And perhaps this:
>
> tio->enabled = pps_generate_next_pulse(tio, expires + SAFE_TIME_NS);
> if (!tio->enabled)
> ...
>
> But the above is just thinking out loudly, you may find the better approach(es).
You might need to introduce pps_tio_enable() counterpart, in such case it would
be more natural to have enabled be assigned accordingly.
--
With Best Regards,
Andy Shevchenko
On Mon, May 13, 2024 at 04:08:12PM +0530, [email protected] wrote:
> +Timed I/O and system time are both driven by same hardware clock. The signal
> +is generated with a precision of ~20 nanoseconds. The generated PPS signal
> +is used to synchronize an external device with system clock. For example,
> +share your clock with a device that receives PPS signal, generated by
"... it can be used to share your clock ..."
> +Timed I/O device. There are dedicated Timed I/O pins to deliver the PPS signal
> +to an external device.
> +
That's it.
--
An old man doll... just what I always wanted! - Clara
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Monday, May 13, 2024 4:49 PM
> To: D, Lakshmi Sowjanya <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; intel-wired-
> [email protected]; Dong, Eddie <[email protected]>; Hall, Christopher
> S <[email protected]>; Brandeburg, Jesse
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Nguyen, Anthony L <[email protected]>;
> [email protected]; N, Pandith <[email protected]>; Mohan,
> Subramanian <[email protected]>; T R, Thejesh Reddy
> <[email protected]>
> Subject: Re: [PATCH v8 10/12] pps: generators: Add PPS Generator TIO Driver
>
> On Mon, May 13, 2024 at 04:08:11PM +0530, [email protected]
> wrote:
> > From: Lakshmi Sowjanya D <[email protected]>
> >
> > The Intel Timed IO PPS generator driver outputs a PPS signal using
> > dedicated hardware that is more accurate than software actuated PPS.
> > The Timed IO hardware generates output events using the ART timer.
> > The ART timer period varies based on platform type, but is less than
> > 100 nanoseconds for all current platforms. Timed IO output accuracy is
> > within 1 ART period.
> >
> > PPS output is enabled by writing '1' the 'enable' sysfs attribute. The
> > driver uses hrtimers to schedule a wake-up 10 ms before each event
> > (edge) target time. At wakeup, the driver converts the target time in
> > terms of CLOCK_REALTIME to ART trigger time and writes this to the
> > Timed IO hardware. The Timed IO hardware generates an event precisely
> > at the requested system time without software involvement.
>
> ...
>
> > +static ssize_t enable_store(struct device *dev, struct device_attribute
> *attr, const char *buf,
> > + size_t count)
> > +{
> > + struct pps_tio *tio = dev_get_drvdata(dev);
> > + bool enable;
> > + int err;
>
> (1)
>
> > + err = kstrtobool(buf, &enable);
> > + if (err)
> > + return err;
> > +
> > + guard(spinlock_irqsave)(&tio->lock);
> > + if (enable && !tio->enabled) {
>
> > + if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
> > + dev_err(tio->dev, "PPS cannot be started as clock is
> not related
> > +to ART");
>
> Why not simply dev_err(dev, ...)?
>
> > + return -EPERM;
> > + }
>
> I'm wondering if we can move this check to (1) above.
> Because currently it's a good question if we are able to stop PPS which was
> run by somebody else without this check done.
Do you mean can someone stop the signal without this check?
Yes, this check is not required to stop. So, I feel it need not be moved to (1).
Please, correct me if my understanding is wrong.
>
> I.o.w. this sounds too weird to me and reading the code doesn't give any hint
> if it's even possible. And if it is, are we supposed to touch that since it was
> definitely *not* us who ran it.
Yes, we are not restricting on who can stop/start the signal.
>
> > + pps_tio_direction_output(tio);
> > + hrtimer_start(&tio->timer, first_event(tio),
> HRTIMER_MODE_ABS);
> > + tio->enabled = true;
> > + } else if (!enable && tio->enabled) {
> > + hrtimer_cancel(&tio->timer);
> > + pps_tio_disable(tio);
> > + tio->enabled = false;
> > + }
> > + return count;
> > +}
>
> ...
>
> > +static int pps_tio_probe(struct platform_device *pdev) {
>
> struct device *dev = &pdev->dev;
>
> > + struct pps_tio *tio;
> > +
> > + if (!(cpu_feature_enabled(X86_FEATURE_TSC_KNOWN_FREQ) &&
> > + cpu_feature_enabled(X86_FEATURE_ART))) {
> > + dev_warn(&pdev->dev, "TSC/ART is not enabled");
>
> dev_warn(dev, "TSC/ART is not enabled");
>
> > + return -ENODEV;
> > + }
> > +
> > + tio = devm_kzalloc(&pdev->dev, sizeof(*tio), GFP_KERNEL);
>
> tio = devm_kzalloc(dev, sizeof(*tio), GFP_KERNEL);
>
>
> > + if (!tio)
> > + return -ENOMEM;
> > +
> > + tio->dev = &pdev->dev;
>
> tio->dev = dev;
>
> > + tio->base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(tio->base))
> > + return PTR_ERR(tio->base);
>
> > + pps_tio_disable(tio);
>
> This...
>
> > + hrtimer_init(&tio->timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
> > + tio->timer.function = hrtimer_callback;
> > + spin_lock_init(&tio->lock);
>
> > + tio->enabled = false;
>
> ...and this should go together, which makes me look at the enabled flag over
> the code and it seems there are a few places where you missed to sync it
> with the reality.
>
> I would think of something like this:
>
> pps_tio_direction_output() ==> true
> pps_tio_disable(tio) ==> false
>
> where "==> X" means assignment of enabled flag.
>
> And perhaps this:
>
> tio->enabled = pps_generate_next_pulse(tio, expires +
> SAFE_TIME_NS);
> if (!tio->enabled)
> ...
>
> But the above is just thinking out loudly, you may find the better
> approach(es).
Yeah, makes sense.
Will add enable counterpart.
Will update tio->enabled in disable and enable functions.
>
> > + platform_set_drvdata(pdev, tio);
> > +
> > + return 0;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
Regards,
Lakshmi Sowjanya
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Monday, May 13, 2024 4:52 PM
> To: D, Lakshmi Sowjanya <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; intel-wired-
> [email protected]; Dong, Eddie <[email protected]>; Hall, Christopher
> S <[email protected]>; Brandeburg, Jesse
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Nguyen, Anthony L <[email protected]>;
> [email protected]; N, Pandith <[email protected]>; Mohan,
> Subramanian <[email protected]>; T R, Thejesh Reddy
> <[email protected]>
> Subject: Re: [PATCH v8 12/12] ABI: pps: Add ABI documentation for Intel TIO
>
> On Mon, May 13, 2024 at 04:08:13PM +0530, [email protected]
> wrote:
> > From: Lakshmi Sowjanya D <[email protected]>
> >
> > Document sysfs interface for Intel Timed I/O PPS driver.
>
> ...
>
> > +Date: June 2024
>
> Is this checked by phb?
>
> "the v6.11 kernel predictions: merge window closes on Sunday, 2024-08-04
> and release on Sunday, 2024-09-29"
I have taken from phb but my understanding is that any probable month before merge window should be added.
I want to know if it should be the month when the merge window closes? (i.e in this case August)?
>
> > +KernelVersion: 6.11
>
> --
> With Best Regards,
> Andy Shevchenko
>
Regards,
Lakshmi Sowjanya
Mon, May 27, 2024 at 11:48:54AM +0000, D, Lakshmi Sowjanya kirjoitti:
> > -----Original Message-----
> > From: Andy Shevchenko <[email protected]>
> > Sent: Monday, May 13, 2024 4:49 PM
> > On Mon, May 13, 2024 at 04:08:11PM +0530, [email protected]
> > wrote:
..
> > > +static ssize_t enable_store(struct device *dev, struct device_attribute
> > *attr, const char *buf,
> > > + size_t count)
> > > +{
> > > + struct pps_tio *tio = dev_get_drvdata(dev);
> > > + bool enable;
> > > + int err;
> >
> > (1)
> >
> > > + err = kstrtobool(buf, &enable);
> > > + if (err)
> > > + return err;
> > > +
> > > + guard(spinlock_irqsave)(&tio->lock);
> > > + if (enable && !tio->enabled) {
> >
> > > + if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
> > > + dev_err(tio->dev, "PPS cannot be started as clock is
> > not related
> > > +to ART");
> >
> > Why not simply dev_err(dev, ...)?
> >
> > > + return -EPERM;
> > > + }
> >
> > I'm wondering if we can move this check to (1) above.
> > Because currently it's a good question if we are able to stop PPS which was
> > run by somebody else without this check done.
>
> Do you mean can someone stop the signal without this check?
> Yes, this check is not required to stop. So, I feel it need not be moved to (1).
>
> Please, correct me if my understanding is wrong.
So, there is a possibility to have a PPS being run (by somebody else) even if
there is no ART provided?
If "yes", your check is wrong to begin with. If "no", my suggestion is correct,
i.e. there is no need to stop something that can't be started at all.
> > I.o.w. this sounds too weird to me and reading the code doesn't give any hint
> > if it's even possible. And if it is, are we supposed to touch that since it was
> > definitely *not* us who ran it.
>
> Yes, we are not restricting on who can stop/start the signal.
See above. It's not about this kind of restriction.
> > > + pps_tio_direction_output(tio);
> > > + hrtimer_start(&tio->timer, first_event(tio),
> > HRTIMER_MODE_ABS);
> > > + tio->enabled = true;
> > > + } else if (!enable && tio->enabled) {
> > > + hrtimer_cancel(&tio->timer);
> > > + pps_tio_disable(tio);
> > > + tio->enabled = false;
> > > + }
> > > + return count;
> > > +}
--
With Best Regards,
Andy Shevchenko
Mon, May 27, 2024 at 11:53:07AM +0000, D, Lakshmi Sowjanya kirjoitti:
> > -----Original Message-----
> > From: Andy Shevchenko <[email protected]>
> > Sent: Monday, May 13, 2024 4:52 PM
> > On Mon, May 13, 2024 at 04:08:13PM +0530, [email protected]
> > wrote:
..
> > > +Date: June 2024
> >
> > Is this checked by phb?
> >
> > "the v6.11 kernel predictions: merge window closes on Sunday, 2024-08-04
> > and release on Sunday, 2024-09-29"
>
> I have taken from phb but my understanding is that any probable month before
> merge window should be added.
I didn't get this. You meant the merge window for the next cycle after your
changes are expected to land?
> I want to know if it should be the month when the merge window closes? (i.e
> in this case August)?
My common sense tells me that there will be no real users (except developers)
for any kernel that's marked as vX.Y-rcZ. Assuming that we announce the ABI in
the release, we should use date of the estimated relase. In this case I would
use September 2024.
> > > +KernelVersion: 6.11
--
With Best Regards,
Andy Shevchenko
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Monday, May 27, 2024 8:04 PM
> To: D, Lakshmi Sowjanya <[email protected]>
> Cc: Andy Shevchenko <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Dong, Eddie
> <[email protected]>; Hall, Christopher S <[email protected]>;
> Brandeburg, Jesse <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Nguyen, Anthony L <[email protected]>;
> [email protected]; N, Pandith <[email protected]>; Mohan,
> Subramanian <[email protected]>; T R, Thejesh Reddy
> <[email protected]>
> Subject: Re: [PATCH v8 10/12] pps: generators: Add PPS Generator TIO Driver
>
> Mon, May 27, 2024 at 11:48:54AM +0000, D, Lakshmi Sowjanya kirjoitti:
> > > -----Original Message-----
> > > From: Andy Shevchenko <[email protected]>
> > > Sent: Monday, May 13, 2024 4:49 PM
> > > On Mon, May 13, 2024 at 04:08:11PM +0530,
> > > [email protected]
> > > wrote:
>
> ...
>
> > > > +static ssize_t enable_store(struct device *dev, struct
> > > > +device_attribute
> > > *attr, const char *buf,
> > > > + size_t count)
> > > > +{
> > > > + struct pps_tio *tio = dev_get_drvdata(dev);
> > > > + bool enable;
> > > > + int err;
> > >
> > > (1)
> > >
> > > > + err = kstrtobool(buf, &enable);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + guard(spinlock_irqsave)(&tio->lock);
> > > > + if (enable && !tio->enabled) {
> > >
> > > > + if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
> > > > + dev_err(tio->dev, "PPS cannot be started as clock is
> > > not related
> > > > +to ART");
> > >
> > > Why not simply dev_err(dev, ...)?
> > >
> > > > + return -EPERM;
> > > > + }
> > >
> > > I'm wondering if we can move this check to (1) above.
> > > Because currently it's a good question if we are able to stop PPS
> > > which was run by somebody else without this check done.
> >
> > Do you mean can someone stop the signal without this check?
> > Yes, this check is not required to stop. So, I feel it need not be moved to (1).
> >
> > Please, correct me if my understanding is wrong.
>
> So, there is a possibility to have a PPS being run (by somebody else) even if there
> is no ART provided?
>
> If "yes", your check is wrong to begin with. If "no", my suggestion is correct, i.e.
> there is no need to stop something that can't be started at all.
It is a "no", PPS doesn't start without ART.
We can move the check to (1), but it would always be checking for ART even in case of disable.
Code readability is better with this approach.
struct pps_tio *tio = dev_get_drvdata(dev);
bool enable;
int err;
if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
dev_err(dev, "PPS cannot be started as clock is not related to ART");
return -EPERM;
}
err = kstrtobool(buf, &enable);
if (err)
return err;
>
> > > I.o.w. this sounds too weird to me and reading the code doesn't give
> > > any hint if it's even possible. And if it is, are we supposed to
> > > touch that since it was definitely *not* us who ran it.
> >
> > Yes, we are not restricting on who can stop/start the signal.
>
> See above. It's not about this kind of restriction.
>
> > > > + pps_tio_direction_output(tio);
> > > > + hrtimer_start(&tio->timer, first_event(tio),
> > > HRTIMER_MODE_ABS);
> > > > + tio->enabled = true;
> > > > + } else if (!enable && tio->enabled) {
> > > > + hrtimer_cancel(&tio->timer);
> > > > + pps_tio_disable(tio);
> > > > + tio->enabled = false;
> > > > + }
> > > > + return count;
> > > > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
Regards
Sowjanya
On Thu, May 30, 2024 at 8:52 AM D, Lakshmi Sowjanya
<[email protected]> wrote:
> > -----Original Message-----
> > From: Andy Shevchenko <[email protected]>
> > Sent: Monday, May 27, 2024 8:04 PM
> > Mon, May 27, 2024 at 11:48:54AM +0000, D, Lakshmi Sowjanya kirjoitti:
> > > > -----Original Message-----
> > > > From: Andy Shevchenko <[email protected]>
> > > > Sent: Monday, May 13, 2024 4:49 PM
> > > > On Mon, May 13, 2024 at 04:08:11PM +0530,
> > > > [email protected]
> > > > wrote:
..
> > > > > +static ssize_t enable_store(struct device *dev, struct
> > > > > +device_attribute
> > > > *attr, const char *buf,
> > > > > + size_t count)
> > > > > +{
> > > > > + struct pps_tio *tio = dev_get_drvdata(dev);
> > > > > + bool enable;
> > > > > + int err;
> > > >
> > > > (1)
> > > >
> > > > > + err = kstrtobool(buf, &enable);
> > > > > + if (err)
> > > > > + return err;
> > > > > +
> > > > > + guard(spinlock_irqsave)(&tio->lock);
> > > > > + if (enable && !tio->enabled) {
> > > >
> > > > > + if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
> > > > > + dev_err(tio->dev, "PPS cannot be started as clock is
> > > > not related
> > > > > +to ART");
> > > >
> > > > Why not simply dev_err(dev, ...)?
> > > >
> > > > > + return -EPERM;
> > > > > + }
> > > >
> > > > I'm wondering if we can move this check to (1) above.
> > > > Because currently it's a good question if we are able to stop PPS
> > > > which was run by somebody else without this check done.
> > >
> > > Do you mean can someone stop the signal without this check?
> > > Yes, this check is not required to stop. So, I feel it need not be moved to (1).
> > >
> > > Please, correct me if my understanding is wrong.
> >
> > So, there is a possibility to have a PPS being run (by somebody else) even if there
> > is no ART provided?
> >
> > If "yes", your check is wrong to begin with. If "no", my suggestion is correct, i.e.
> > there is no need to stop something that can't be started at all.
>
> It is a "no", PPS doesn't start without ART.
>
> We can move the check to (1), but it would always be checking for ART even in case of disable.
Please do,
> Code readability is better with this approach.
>
> struct pps_tio *tio = dev_get_drvdata(dev);
> bool enable;
> int err;
>
> if (!timekeeping_clocksource_has_base(CSID_X86_ART)) {
> dev_err(dev, "PPS cannot be started as clock is not related to ART");
started --> used
> return -EPERM;
> }
>
> err = kstrtobool(buf, &enable);
> if (err)
> return err;
>
> > > > I.o.w. this sounds too weird to me and reading the code doesn't give
> > > > any hint if it's even possible. And if it is, are we supposed to
> > > > touch that since it was definitely *not* us who ran it.
> > >
> > > Yes, we are not restricting on who can stop/start the signal.
> >
> > See above. It's not about this kind of restriction.
> >
> > > > > + pps_tio_direction_output(tio);
> > > > > + hrtimer_start(&tio->timer, first_event(tio),
> > > > HRTIMER_MODE_ABS);
> > > > > + tio->enabled = true;
> > > > > + } else if (!enable && tio->enabled) {
> > > > > + hrtimer_cancel(&tio->timer);
> > > > > + pps_tio_disable(tio);
> > > > > + tio->enabled = false;
> > > > > + }
> > > > > + return count;
> > > > > +}
--
With Best Regards,
Andy Shevchenko
The following commit has been merged into the timers/core branch of tip:
Commit-ID: fcb05911e5832364c5f154b519a471225b34855e
Gitweb: https://git.kernel.org/tip/fcb05911e5832364c5f154b519a471225b34855e
Author: Thomas Gleixner <[email protected]>
AuthorDate: Mon, 13 May 2024 16:08:05 +05:30
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 03 Jun 2024 11:18:50 +02:00
igc: Remove convert_art_ns_to_tsc()
The core code now provides a mechanism to convert the ART base clock to the
corresponding TSC value without requiring an architecture specific
function.
Replace the direct conversion by filling in the required data.
No functional change intended.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/net/ethernet/intel/igc/igc_ptp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 1bb0262..946edba 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -938,7 +938,11 @@ static bool igc_is_crosststamp_supported(struct igc_adapter *adapter)
static struct system_counterval_t igc_device_tstamp_to_system(u64 tstamp)
{
#if IS_ENABLED(CONFIG_X86_TSC) && !defined(CONFIG_UML)
- return convert_art_ns_to_tsc(tstamp);
+ return (struct system_counterval_t) {
+ .cs_id = CSID_X86_ART,
+ .cycles = tstamp,
+ .use_nsecs = true,
+ };
#else
return (struct system_counterval_t) { };
#endif
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 0f532a789f1b24258043d0f856409d2ab974fb64
Gitweb: https://git.kernel.org/tip/0f532a789f1b24258043d0f856409d2ab974fb64
Author: Lakshmi Sowjanya D <[email protected]>
AuthorDate: Mon, 13 May 2024 16:08:09 +05:30
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 03 Jun 2024 11:18:51 +02:00
x86/tsc: Remove obsolete ART to TSC conversion functions
convert_art_to_tsc() and convert_art_ns_to_tsc() interfaces are no
longer required. The conversion is now handled by the core code.
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/tsc.h | 3 +--
arch/x86/kernel/tsc.c | 60 +-------------------------------------
2 files changed, 63 deletions(-)
diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 405efb3..94408a7 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -28,9 +28,6 @@ static inline cycles_t get_cycles(void)
}
#define get_cycles get_cycles
-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 void tsc_early_init(void);
extern void tsc_init(void);
extern void mark_tsc_unstable(char *reason);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index d1888db..d4462fb 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1297,66 +1297,6 @@ int unsynchronized_tsc(void)
return 0;
}
-/*
- * Convert ART to TSC given numerator/denominator found in detect_art()
- */
-struct system_counterval_t convert_art_to_tsc(u64 art)
-{
- u64 tmp, res, rem;
-
- rem = do_div(art, art_base_clk.denominator);
-
- res = art * art_base_clk.numerator;
- tmp = rem * art_base_clk.numerator;
-
- do_div(tmp, art_base_clk.denominator);
- res += tmp + art_base_clk.offset;
-
- return (struct system_counterval_t) {
- .cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
- .cycles = res,
- };
-}
-EXPORT_SYMBOL(convert_art_to_tsc);
-
-/**
- * convert_art_ns_to_tsc() - Convert ART in nanoseconds to TSC.
- * @art_ns: ART (Always Running Timer) in unit of nanoseconds
- *
- * PTM requires all timestamps to be in units of nanoseconds. When user
- * software requests a cross-timestamp, this function converts system timestamp
- * to TSC.
- *
- * 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 TSC is attempted.
- *
- * Return:
- * struct system_counterval_t - system counter value with the ID of the
- * corresponding clocksource:
- * cycles: System counter value
- * cs_id: The clocksource ID for validating comparability
- */
-
-struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns)
-{
- u64 tmp, res, rem;
-
- rem = do_div(art_ns, USEC_PER_SEC);
-
- res = art_ns * tsc_khz;
- tmp = rem * tsc_khz;
-
- do_div(tmp, USEC_PER_SEC);
- res += tmp;
-
- return (struct system_counterval_t) {
- .cs_id = have_art ? CSID_X86_TSC : CSID_GENERIC,
- .cycles = res,
- };
-}
-EXPORT_SYMBOL(convert_art_ns_to_tsc);
-
static void tsc_refine_calibration_work(struct work_struct *work);
static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
/**
The following commit has been merged into the timers/core branch of tip:
Commit-ID: d4bea547ebb577a4b4c545a4a81d495cec7eefe1
Gitweb: https://git.kernel.org/tip/d4bea547ebb577a4b4c545a4a81d495cec7eefe1
Author: Thomas Gleixner <[email protected]>
AuthorDate: Mon, 13 May 2024 16:08:08 +05:30
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 03 Jun 2024 11:18:51 +02:00
ice/ptp: Remove convert_art_to_tsc()
The core code now provides a mechanism to convert the ART base clock to the
corresponding TSC value without requiring an architecture specific
function.
Replace the direct conversion by filling in the required data.
No functional change intended.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 0f17fc1..15ca440 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2091,7 +2091,8 @@ ice_ptp_get_syncdevicetime(ktime_t *device,
hh_ts_lo = rd32(hw, GLHH_ART_TIME_L);
hh_ts_hi = rd32(hw, GLHH_ART_TIME_H);
hh_ts = ((u64)hh_ts_hi << 32) | hh_ts_lo;
- *system = convert_art_ns_to_tsc(hh_ts);
+ system->cycles = hh_ts;
+ system->cs_id = CSID_X86_ART;
/* Read Device source clock time */
hh_ts_lo = rd32(hw, GLTSYN_HHTIME_L(tmr_idx));
hh_ts_hi = rd32(hw, GLTSYN_HHTIME_H(tmr_idx));
The following commit has been merged into the timers/core branch of tip:
Commit-ID: b3266ed85f77047a9674100f0da8058750e5bc62
Gitweb: https://git.kernel.org/tip/b3266ed85f77047a9674100f0da8058750e5bc62
Author: Thomas Gleixner <[email protected]>
AuthorDate: Mon, 13 May 2024 16:08:07 +05:30
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 03 Jun 2024 11:18:50 +02:00
ALSA: hda: Remove convert_art_to_tsc()
The core code now provides a mechanism to convert the ART base clock to the
corresponding TSC value without requiring an architecture specific
function.
Replace the direct conversion by filling in the required data.
No functional change intended.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Lakshmi Sowjanya D <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
sound/pci/hda/hda_controller.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 766734d..5d86e5a 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -463,7 +463,8 @@ static int azx_get_sync_time(ktime_t *device,
*device = ktime_add_ns(*device, (wallclk_cycles * NSEC_PER_SEC) /
((HDA_MAX_CYCLE_VALUE + 1) * runtime->rate));
- *system = convert_art_to_tsc(tsc_counter);
+ system->cycles = tsc_counter;
+ system->cs_id = CSID_X86_ART;
return 0;
}