* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
[not found] ` <Y+3m/PpzkBN9kxJY@localhost>
@ 2023-02-16 17:54 ` Matt Corallo
2023-02-16 22:54 ` Richard Cochran
0 siblings, 1 reply; 26+ messages in thread
From: Matt Corallo @ 2023-02-16 17:54 UTC (permalink / raw)
To: chrony-dev, Miroslav Lichvar; +Cc: Richard Cochran, netdev
On 2/16/23 12:19 AM, Miroslav Lichvar wrote:
> On Wed, Feb 15, 2023 at 10:27:15PM -0800, Matt Corallo wrote:
>> My naive solution from skimming the code would be to shove
>> formerly-discarded samples into a global limited queue and check for
>> available timestamps in `phc_poll`. However, I have no idea if the time
>> difference between when the sample was taken by the hardware and when the
>> `HCL_CookTime` call is done would impact accuracy (or maybe the opposite,
>> since we'd then be cooking time with the hardware clock right after taking
>> the HCL sample rather than when the PHC timestamp happens), or if such a
>> patch would simply be rejected as a dirty, dirty hack rather than unifying
>> the PHC read sockets across the devices into one socket (via some global
>> tracking the device -> socket mapping?) and passing the samples out
>> appropriately. Let me know what makes the most sense here.
>
> My first thought is that this should be addressed in the kernel, so
> even different processes having open the PHC device can receive all
> extts samples. If it turns out it's too difficult to do for the
> character device (I'm not very familiar with that subsystem), maybe it
> could be done at least in sysfs (/sys/class/ptp/ptp*/fifo or a new
> file showing the last event like the PPS assert and clear).
I mean my first thought seeing an ioctl on a socket that gives an explicit channel and then receives
crap from other channels on the same socket was "wtf" so I went and read the kernel to figure out
why first to see if its a driver bug. I can't seem to find *any* documentation for how these ioctls
are supposed to work, but it seems the "request" here is kinda misnomer, its really a "configure
hardware" request, and is unrelated to future reads on the socket, or really the specific socket at all.
As for duplicating the output across sockets, ptp_chardev.c's `ptp_read` is pretty trivial - just
pop the next sample off the queue and return it. Tweaking that to copy the sample into every reader
is probably above my paygrade (and has a whole host of leak risk I'd probably screw up).
`extts_fifo_show` appears to be functionally identical.
I've CC'd the MAINTAINERs for ptp to see what they think about this, though it won't let chrony
support this without a kernel upgrade - not sure if that's an issue for chrony or not.
Matt
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-16 17:54 ` [chrony-dev] Support for Multiple PPS Inputs on single PHC Matt Corallo
@ 2023-02-16 22:54 ` Richard Cochran
2023-02-17 0:58 ` Matt Corallo
2023-02-20 10:08 ` Miroslav Lichvar
0 siblings, 2 replies; 26+ messages in thread
From: Richard Cochran @ 2023-02-16 22:54 UTC (permalink / raw)
To: Matt Corallo; +Cc: chrony-dev, Miroslav Lichvar, netdev
On Thu, Feb 16, 2023 at 09:54:56AM -0800, Matt Corallo wrote:
>
>
> On 2/16/23 12:19 AM, Miroslav Lichvar wrote:
> > My first thought is that this should be addressed in the kernel, so
> > even different processes having open the PHC device can receive all
> > extts samples. If it turns out it's too difficult to do for the
> > character device (I'm not very familiar with that subsystem), maybe it
> > could be done at least in sysfs (/sys/class/ptp/ptp*/fifo or a new
> > file showing the last event like the PPS assert and clear).
The PPS thing has a race, and so I'd rather not copy that!
> As for duplicating the output across sockets, ptp_chardev.c's `ptp_read` is
> pretty trivial - just pop the next sample off the queue and return it.
> Tweaking that to copy the sample into every reader is probably above my
> paygrade (and has a whole host of leak risk I'd probably screw up).
> `extts_fifo_show` appears to be functionally identical.
Each extts in the fifo is delivered only once. If there are multiple
readers, each reader will receive only some of the data. This is
similar to how a pipe behaves.
HTH,
Richard
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-16 22:54 ` Richard Cochran
@ 2023-02-17 0:58 ` Matt Corallo
2023-02-20 10:08 ` Miroslav Lichvar
1 sibling, 0 replies; 26+ messages in thread
From: Matt Corallo @ 2023-02-17 0:58 UTC (permalink / raw)
To: Richard Cochran; +Cc: chrony-dev, Miroslav Lichvar, netdev
On 2/16/23 2:54 PM, Richard Cochran wrote:
> On Thu, Feb 16, 2023 at 09:54:56AM -0800, Matt Corallo wrote:
>> As for duplicating the output across sockets, ptp_chardev.c's `ptp_read` is
>> pretty trivial - just pop the next sample off the queue and return it.
>> Tweaking that to copy the sample into every reader is probably above my
>> paygrade (and has a whole host of leak risk I'd probably screw up).
>> `extts_fifo_show` appears to be functionally identical.
>
> Each extts in the fifo is delivered only once. If there are multiple
> readers, each reader will receive only some of the data. This is
> similar to how a pipe behaves.
Right, sorry if the context wasn't clear, I only realized part of the message was removed in the
first reply after sending. The question from Miroslav was, basically, "would kernel accept something
to only get notified of extts pulses on a given channel, and, if so, how would we go about doing that".
The "we get pulses from all extts channels on the same socket" thing is a bit annoying to munge into
chrony - it has the concept of "refclocks" which are a single clock, in this case a single pps pulse
generator. If you have two of them on the same PTP clock but coming in on different pins/channels it
doesn't have a way to express that outside of two refclocks. While we could take pulses from both
refclocks on one socket and shove them into some queue and have the refclocks pick that data up its
a bunch of complexity on the client side and not super clean in the current codebase.
Thanks,
Matt
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-16 22:54 ` Richard Cochran
2023-02-17 0:58 ` Matt Corallo
@ 2023-02-20 10:08 ` Miroslav Lichvar
2023-02-20 15:24 ` Richard Cochran
1 sibling, 1 reply; 26+ messages in thread
From: Miroslav Lichvar @ 2023-02-20 10:08 UTC (permalink / raw)
To: Richard Cochran; +Cc: Matt Corallo, chrony-dev, netdev
On Thu, Feb 16, 2023 at 02:54:29PM -0800, Richard Cochran wrote:
> Each extts in the fifo is delivered only once. If there are multiple
> readers, each reader will receive only some of the data. This is
> similar to how a pipe behaves.
Does it need to be that way? It seems strange for the kernel to
support enabling PPS on multiple channels at the same time, but not
allow multiple applications to receive all samples from their channel.
--
Miroslav Lichvar
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-20 10:08 ` Miroslav Lichvar
@ 2023-02-20 15:24 ` Richard Cochran
2023-02-23 20:56 ` Matt Corallo
0 siblings, 1 reply; 26+ messages in thread
From: Richard Cochran @ 2023-02-20 15:24 UTC (permalink / raw)
To: Miroslav Lichvar; +Cc: Matt Corallo, chrony-dev, netdev
On Mon, Feb 20, 2023 at 11:08:23AM +0100, Miroslav Lichvar wrote:
> Does it need to be that way? It seems strange for the kernel to
> support enabling PPS on multiple channels at the same time, but not
> allow multiple applications to receive all samples from their channel.
It does not need to be that way, but nobody ever wanted multiple
readers before.
Implementing this would make the kernel side much more complex, as the
code would need per-reader tracking of the buffered time stamps, or
per-reader fifo buffers, etc.
Thanks,
Richard
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-20 15:24 ` Richard Cochran
@ 2023-02-23 20:56 ` Matt Corallo
2023-02-24 0:19 ` Richard Cochran
0 siblings, 1 reply; 26+ messages in thread
From: Matt Corallo @ 2023-02-23 20:56 UTC (permalink / raw)
To: Richard Cochran, Miroslav Lichvar; +Cc: chrony-dev, netdev
On 2/20/23 7:24 AM, Richard Cochran wrote:
> On Mon, Feb 20, 2023 at 11:08:23AM +0100, Miroslav Lichvar wrote:
>> Does it need to be that way? It seems strange for the kernel to
>> support enabling PPS on multiple channels at the same time, but not
>> allow multiple applications to receive all samples from their channel.
>
> It does not need to be that way, but nobody ever wanted multiple
> readers before.
>
> Implementing this would make the kernel side much more complex, as the
> code would need per-reader tracking of the buffered time stamps, or
> per-reader fifo buffers, etc.
There's two separate questions here - multiple readers receiving the same data, and multiple readers
receiving data exclusively about one channel.
I'd imagine the second is (much?) easier to implement, whereas the first is a bunch of complexity.
At least personally I'm okay with the second, rather than the first, and that fixes the issue for
chrony, though it doesn't allow one to, say, get raw samples in one program while having another
handle them.
Matt
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-23 20:56 ` Matt Corallo
@ 2023-02-24 0:19 ` Richard Cochran
2023-02-24 1:18 ` Matt Corallo
0 siblings, 1 reply; 26+ messages in thread
From: Richard Cochran @ 2023-02-24 0:19 UTC (permalink / raw)
To: Matt Corallo; +Cc: Miroslav Lichvar, chrony-dev, netdev
On Thu, Feb 23, 2023 at 12:56:34PM -0800, Matt Corallo wrote:
> There's two separate questions here - multiple readers receiving the same
> data, and multiple readers receiving data exclusively about one channel.
>
> I'd imagine the second is (much?) easier to implement, whereas the first is a bunch of complexity.
This second idea would require a new API, so that user could select a
particular channel.
First idea would only change kernel behavior without changing the API.
Thanks,
Richard
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-24 0:19 ` Richard Cochran
@ 2023-02-24 1:18 ` Matt Corallo
2023-02-24 5:07 ` Richard Cochran
0 siblings, 1 reply; 26+ messages in thread
From: Matt Corallo @ 2023-02-24 1:18 UTC (permalink / raw)
To: Richard Cochran; +Cc: Miroslav Lichvar, chrony-dev, netdev
On 2/23/23 4:19 PM, Richard Cochran wrote:
> On Thu, Feb 23, 2023 at 12:56:34PM -0800, Matt Corallo wrote:
>
>> There's two separate questions here - multiple readers receiving the same
>> data, and multiple readers receiving data exclusively about one channel.
>>
>> I'd imagine the second is (much?) easier to implement, whereas the first is a bunch of complexity.
>
> This second idea would require a new API, so that user could select a
> particular channel.
>
> First idea would only change kernel behavior without changing the API.
Fair point. I figured a new IOCTL to filter was a lighter lift, even if a bunch of boilerplate to
define it.
I'm happy to take a crack at something to get the ball rolling, though not this week. I'm sure I
could copy+paste to make a new IOCTL work, but adding relevant queue limiting means I have to go
read much more kernel code to figure out which datastructures already exist there :).
It sounds like I should go replace the extts queue with a circular buffer, have every reader socket
store an index in the buffer, and new sockets read only futures pulses? I assume a new pulse already
wakes all select()ers on the sockets so nothing would need to change there. Is there some existing
code somewhere I should crib off of or just run and see where I get?
Thanks,
Matt
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-24 1:18 ` Matt Corallo
@ 2023-02-24 5:07 ` Richard Cochran
2023-08-29 11:47 ` Xabier Marquiegui
0 siblings, 1 reply; 26+ messages in thread
From: Richard Cochran @ 2023-02-24 5:07 UTC (permalink / raw)
To: Matt Corallo; +Cc: Miroslav Lichvar, chrony-dev, netdev
On Thu, Feb 23, 2023 at 05:18:06PM -0800, Matt Corallo wrote:
> It sounds like I should go replace the extts queue with a circular buffer,
> have every reader socket store an index in the buffer, and new sockets read
> only futures pulses?
Single circular buffer with multiple heads will be complex.
It might be simpler to allocate one queue per reader.
If there are few readers, cost of allocation and en-queuing won't matter.
Thanks,
Richard
^ permalink raw reply [flat|nested] 26+ messages in thread
* [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-02-24 5:07 ` Richard Cochran
@ 2023-08-29 11:47 ` Xabier Marquiegui
2023-08-29 11:47 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
0 siblings, 1 reply; 26+ messages in thread
From: Xabier Marquiegui @ 2023-08-29 11:47 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
I am interested in moving forward this conversation. I have found myself
in a situation where I need to use multiple external timestamp channels
with different clients. In my specific application, I use channel 0 with
ts2phc to synchronize a PPS clock reference, and I want to use channel 1
to timestamp external events. Since ts2phc is consuming from the fifo,
if I add another consumer everything starts to go wrong.
I would like to share my idea on how to solve this. Like Richard
mentions, I just create a separate work queue per channel, and add a
sysfs interface to dynamically be able to move queues from the
shared/multiplexed queue to individual queues.
This is the first time I try to contribute something using this mailing
list, so please forgive me if I'm doing something wrong.
Let me know what you thing about my code (the patch should be on the
next message). Thanks.
Xabier.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] ptp: Demultiplexed timestamp channels
2023-08-29 11:47 ` Xabier Marquiegui
@ 2023-08-29 11:47 ` Xabier Marquiegui
2023-08-29 14:07 ` Richard Cochran
0 siblings, 1 reply; 26+ messages in thread
From: Xabier Marquiegui @ 2023-08-29 11:47 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
Add the posibility to demultiplex the timestamp channels for
external timestamp event channels.
In some applications it can be necessary to have different
consumers for different timestamp channels. For example,
synchronize to an external pps source with linuxptp ts2phc
while timestmping external events with another application.
This change proposes the dynamic creation of one char-device
per timestamp channel only if the user requests the demuxing
of timestamp channels. It allows for on-the-fly demuxing of
specific channels.
The operation can be controlled via sysfs. See file
Documentation/ABI/testing/sysfs-ptp for more details.
---
Documentation/ABI/testing/sysfs-ptp | 16 +++
MAINTAINERS | 5 +
drivers/ptp/Makefile | 2 +-
drivers/ptp/ptp_chardev.c | 2 -
drivers/ptp/ptp_clock.c | 22 ++-
drivers/ptp/ptp_demuxtschan.c | 211 ++++++++++++++++++++++++++++
drivers/ptp/ptp_private.h | 25 ++++
drivers/ptp/ptp_sysfs.c | 113 +++++++++++++++
8 files changed, 392 insertions(+), 4 deletions(-)
create mode 100644 drivers/ptp/ptp_demuxtschan.c
diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
index 9c317ac7c47a..9d9875e7c56a 100644
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -81,6 +81,22 @@ Description:
switches the physical clock back to normal, adjustable
operation.
+What: /sys/class/ptp/ptp<N>/dmtsc_en_flags
+Date: August 2023
+Contact: Xabier Marquiegui <reibax@gmail.com>
+Description:
+ This read/write file controls the de-multiplexing of
+ external timestamp channel fifos. In write more, you
+ can de-multiplex by enabling a channel or re-multiplex
+ it by disabling it. Write mode can be done in channel mode
+ for single channel control "c 0 1" or mask mode for
+ multi-channel "m 0x3 1". Syntax is:
+ <mode (m/c)> <value> <enable (0/1)>
+ In read mode you get the enable mask for all channels in
+ hex.
+ See function dmtsc_en_flags_store on
+ drivers/ptp/ptp_sysfs.c for more details.
+
What: /sys/class/ptp/ptp<N>/pins
Date: March 2014
Contact: Richard Cochran <richardcochran@gmail.com>
diff --git a/MAINTAINERS b/MAINTAINERS
index 4cc6bf79fdd8..8d9a24039d67 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17171,6 +17171,11 @@ S: Maintained
F: drivers/ptp/ptp_vclock.c
F: net/ethtool/phc_vclocks.c
+PTP DEMUXED TS CHANEL FIFOS
+M: Xabier Marquiegui <reibax@gmail.com>
+S: Maintained
+F: drivers/ptp/ptp_demuxtschan.c
+
PTRACE SUPPORT
M: Oleg Nesterov <oleg@redhat.com>
S: Maintained
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index 553f18bf3c83..6dbae0b9aa7c 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -3,7 +3,7 @@
# Makefile for PTP 1588 clock support.
#
-ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o ptp_vclock.o
+ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o ptp_vclock.o ptp_demuxtschan.o
ptp_kvm-$(CONFIG_X86) := ptp_kvm_x86.o ptp_kvm_common.o
ptp_kvm-$(CONFIG_HAVE_ARM_SMCCC) := ptp_kvm_arm.o ptp_kvm_common.o
obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 362bf756e6b7..0c900042c389 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -441,8 +441,6 @@ __poll_t ptp_poll(struct posix_clock *pc, struct file *fp, poll_table *wait)
return queue_cnt(&ptp->tsevq) ? EPOLLIN : 0;
}
-#define EXTTS_BUFSIZE (PTP_BUF_TIMESTAMPS * sizeof(struct ptp_extts_event))
-
ssize_t ptp_read(struct posix_clock *pc,
uint rdflags, char __user *buf, size_t cnt)
{
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 80f74e38c2da..383508c269a2 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -172,6 +172,8 @@ static void ptp_clock_release(struct device *dev)
ptp_cleanup_pin_groups(ptp);
kfree(ptp->vclock_index);
+ mutex_destroy(&ptp->dmtsc_devs.dmtsc_devs_mux);
+ mutex_destroy(&ptp->dmtsc_sysfs_mux);
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
@@ -232,7 +234,13 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
mutex_init(&ptp->tsevq_mux);
mutex_init(&ptp->pincfg_mux);
mutex_init(&ptp->n_vclocks_mux);
+ mutex_init(&ptp->dmtsc_sysfs_mux);
+ mutex_init(&ptp->dmtsc_devs.dmtsc_devs_mux);
init_waitqueue_head(&ptp->tsev_wq);
+ ptp->dmtsc_en_flags = 0x0;
+ ptp->dmtscevq = NULL;
+ ptp->dmtsc_devs.readers = 0;
+ ptp->dmtsc_devs.clean_request = false;
if (ptp->info->getcycles64 || ptp->info->getcyclesx64) {
ptp->has_cycles = true;
@@ -330,6 +338,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (ptp->kworker)
kthread_destroy_worker(ptp->kworker);
kworker_err:
+ mutex_destroy(&ptp->dmtsc_devs.dmtsc_devs_mux);
+ mutex_destroy(&ptp->dmtsc_sysfs_mux);
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
@@ -351,6 +361,8 @@ static int unregister_vclock(struct device *dev, void *data)
int ptp_clock_unregister(struct ptp_clock *ptp)
{
+ ptp_dmtsc_dev_uregister(ptp);
+
if (ptp_vclock_in_use(ptp)) {
device_for_each_child(&ptp->dev, NULL, unregister_vclock);
}
@@ -383,7 +395,15 @@ void ptp_clock_event(struct ptp_clock *ptp, struct ptp_clock_event *event)
break;
case PTP_CLOCK_EXTTS:
- enqueue_external_timestamp(&ptp->tsevq, event);
+ /* If event index demuxed queue mask is enabled send to dedicated fifo */
+ if (ptp->dmtsc_en_flags & (0x1 << event->index)) {
+ enqueue_external_timestamp(&ptp->dmtscevq[event->index], event);
+ }
+ else
+ {
+ enqueue_external_timestamp(&ptp->tsevq, event);
+ }
+
wake_up_interruptible(&ptp->tsev_wq);
break;
diff --git a/drivers/ptp/ptp_demuxtschan.c b/drivers/ptp/ptp_demuxtschan.c
new file mode 100644
index 000000000000..516a1dabe8dd
--- /dev/null
+++ b/drivers/ptp/ptp_demuxtschan.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * PTP exploded timestamp event queue driver
+ *
+ * Copyright 2023 Aingura IIoT
+ */
+
+#include <linux/slab.h>
+#include <linux/cdev.h>
+#include <linux/fs.h>
+#include "ptp_private.h"
+
+static int ptp_dmtsc_open(struct inode *inode, struct file *file)
+{
+ struct ptp_dmtsc_cdev_info *cdev = container_of(inode->i_cdev,
+ struct ptp_dmtsc_cdev_info, dmtsc_cdev);
+
+ file->private_data = cdev;
+
+ if (mutex_lock_interruptible(&cdev->pclock->dmtsc_devs.dmtsc_devs_mux))
+ return -ERESTARTSYS;
+ cdev->pclock->dmtsc_devs.readers++;
+ mutex_unlock(&cdev->pclock->dmtsc_devs.dmtsc_devs_mux);
+
+ return stream_open(inode, file);
+}
+
+int ptp_dmtsc_release (struct inode *inode, struct file *file)
+{
+ struct ptp_dmtsc_cdev_info *cdev = file->private_data;
+
+ if (mutex_lock_interruptible(&cdev->pclock->dmtsc_devs.dmtsc_devs_mux))
+ return -ERESTARTSYS;
+ cdev->pclock->dmtsc_devs.readers--;
+
+ if ((cdev->pclock->dmtsc_devs.readers == 0) &&
+ cdev->pclock->dmtsc_devs.clean_request) {
+ mutex_unlock(&cdev->pclock->dmtsc_devs.dmtsc_devs_mux);
+ ptp_dmtsc_dev_uregister(cdev->pclock);
+ }
+ mutex_unlock(&cdev->pclock->dmtsc_devs.dmtsc_devs_mux);
+ return 0;
+}
+
+ssize_t ptp_dmtsc_read(struct file *file, char __user *buf,
+ size_t cnt, loff_t *offset)
+{
+ struct ptp_dmtsc_cdev_info *cdev = file->private_data;
+ struct timestamp_event_queue *queue = &cdev->pclock->dmtscevq[cdev->minor];
+ struct mutex *dmtsceq_mux = &cdev->pclock->dmtsc_devs.cdev_info[cdev->minor].dmtsceq_mux;
+ struct ptp_extts_event *event;
+ unsigned long flags;
+ size_t qcnt, i;
+ int result;
+
+ if (cnt % sizeof(struct ptp_extts_event) != 0)
+ return -EINVAL;
+
+ if (cnt > EXTTS_BUFSIZE)
+ cnt = EXTTS_BUFSIZE;
+
+ cnt = cnt / sizeof(struct ptp_extts_event);
+
+ if (mutex_lock_interruptible(dmtsceq_mux))
+ return -ERESTARTSYS;
+
+ if (wait_event_interruptible(cdev->pclock->tsev_wq,
+ cdev->pclock->defunct || queue_cnt(queue))) {
+ mutex_unlock(dmtsceq_mux);
+ return -ERESTARTSYS;
+ }
+
+ if (cdev->pclock->defunct) {
+ mutex_unlock(dmtsceq_mux);
+ return -ENODEV;
+ }
+
+ event = kmalloc(EXTTS_BUFSIZE, GFP_KERNEL);
+ if (!event) {
+ mutex_unlock(dmtsceq_mux);
+ return -ENOMEM;
+ }
+
+ spin_lock_irqsave(&queue->lock, flags);
+
+ qcnt = queue_cnt(queue);
+
+ if (cnt > qcnt)
+ cnt = qcnt;
+
+ for (i = 0; i < cnt; i++) {
+ event[i] = queue->buf[queue->head];
+ queue->head = (queue->head + 1) % PTP_MAX_TIMESTAMPS;
+ }
+
+ spin_unlock_irqrestore(&queue->lock, flags);
+
+ cnt = cnt * sizeof(struct ptp_extts_event);
+
+ mutex_unlock(dmtsceq_mux);
+
+ result = cnt;
+ if (copy_to_user(buf, event, cnt))
+ result = -EFAULT;
+
+ kfree(event);
+ return result;
+}
+
+struct file_operations fops = {
+ .owner = THIS_MODULE,
+ .open = ptp_dmtsc_open,
+ .read = ptp_dmtsc_read,
+ .release = ptp_dmtsc_release
+};
+
+void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
+{
+ int idx, major;
+ dev_t device;
+
+ major = MAJOR(ptp->dmtsc_devs.devid);
+ for (idx = 0; idx < ptp->info->n_ext_ts ; idx++) {
+ if (ptp->dmtsc_devs.cdev_info[idx].minor >= 0) {
+ device = MKDEV(major, idx);
+ device_destroy(ptp->dmtsc_devs.dmtsc_class, device);
+ cdev_del(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev);
+ ptp->dmtsc_devs.cdev_info[idx].minor = -1;
+ }
+ }
+ class_destroy(ptp->dmtsc_devs.dmtsc_class);
+ unregister_chrdev_region(ptp->dmtsc_devs.devid, ptp->info->n_ext_ts);
+ mutex_destroy(&ptp->dmtsc_devs.cdev_info[idx].dmtsceq_mux);
+}
+
+int ptp_dmtsc_dev_register(struct ptp_clock *ptp)
+{
+ int err, idx, major;
+ dev_t device;
+ struct device *dev;
+
+ // Create fifos for all channels. The mask will control which of them get fed
+ ptp->dmtscevq = kcalloc(ptp->info->n_ext_ts, sizeof(*ptp->dmtscevq), GFP_KERNEL);
+ if (!ptp->dmtscevq) {
+ err = -EFAULT;
+ goto err;
+ }
+ ptp->dmtsc_devs.cdev_info = kcalloc(ptp->info->n_ext_ts,
+ sizeof(*ptp->dmtsc_devs.cdev_info), GFP_KERNEL);
+ if (!ptp->dmtsc_devs.cdev_info) {
+ err = -ENODEV;
+ goto fifo_clean;
+ }
+ for (idx = 0; idx < ptp->info->n_ext_ts ; idx++) {
+ ptp->dmtsc_devs.cdev_info[idx].minor = -1;
+ }
+ // Create devices for all channels. The mask will control which of them get fed
+ err = alloc_chrdev_region(&ptp->dmtsc_devs.devid, 0, ptp->info->n_ext_ts, "ptptsevqch");
+ if (!err) {
+ major = MAJOR(ptp->dmtsc_devs.devid);
+ ptp->dmtsc_devs.dmtsc_class = class_create(THIS_MODULE, "ptptsevqch_class");
+ for (idx = 0; idx < ptp->info->n_ext_ts ; idx++) {
+ mutex_init(&ptp->dmtsc_devs.cdev_info[idx].dmtsceq_mux);
+ device = MKDEV(major, idx);
+ ptp->dmtsc_devs.cdev_info[idx].pclock = ptp;
+ cdev_init(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev, &fops);
+ err = cdev_add(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev, device, 1);
+ if (err) {
+ goto cdev_clean;
+ } else {
+ ptp->dmtsc_devs.cdev_info[idx].minor = idx;
+ dev = device_create(ptp->dmtsc_devs.dmtsc_class, &ptp->dev, device, NULL, "ptp%dch%d", ptp->index, idx);
+ if (IS_ERR(dev)) {
+ err = PTR_ERR(dev);
+ goto cdev_clean;
+ }
+ }
+ }
+ } else {
+ goto dev_clean;
+ }
+ return 0;
+
+cdev_clean:
+ ptp_dmtsc_cdev_clean(ptp);
+dev_clean:
+ kfree(ptp->dmtsc_devs.cdev_info);
+ ptp->dmtsc_devs.cdev_info = NULL;
+fifo_clean:
+ kfree(ptp->dmtscevq);
+ ptp->dmtscevq = NULL;
+err:
+ return err;
+}
+
+void ptp_dmtsc_dev_uregister(struct ptp_clock *ptp)
+{
+ if (mutex_lock_interruptible(&ptp->dmtsc_devs.dmtsc_devs_mux))
+ return;
+ if (ptp->dmtsc_devs.readers > 0) {
+ ptp->dmtsc_devs.clean_request = true;
+ mutex_unlock(&ptp->dmtsc_devs.dmtsc_devs_mux);
+ return;
+ }
+ mutex_unlock(&ptp->dmtsc_devs.dmtsc_devs_mux);
+ ptp_dmtsc_cdev_clean(ptp);
+ kfree(ptp->dmtsc_devs.cdev_info);
+ ptp->dmtsc_devs.cdev_info = NULL;
+ kfree(ptp->dmtscevq);
+ ptp->dmtscevq = NULL;
+}
\ No newline at end of file
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 75f58fc468a7..9fc7b6ec6517 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -20,6 +20,8 @@
#define PTP_BUF_TIMESTAMPS 30
#define PTP_DEFAULT_MAX_VCLOCKS 20
+#define EXTTS_BUFSIZE (PTP_BUF_TIMESTAMPS * sizeof(struct ptp_extts_event))
+
struct timestamp_event_queue {
struct ptp_extts_event buf[PTP_MAX_TIMESTAMPS];
int head;
@@ -27,6 +29,22 @@ struct timestamp_event_queue {
spinlock_t lock;
};
+struct ptp_dmtsc_cdev_info {
+ struct cdev dmtsc_cdev; /* Demuxed event device chardev */
+ int minor; /* Demuxed event queue chardev device minor */
+ struct ptp_clock *pclock; /* Direct access to parent clock device */
+ struct mutex dmtsceq_mux; /* Protect access to demuxed event queue */
+};
+
+struct ptp_dmtsc_dev_info {
+ dev_t devid;
+ int readers; /* Amount of users with chardev open */
+ bool clean_request; /* Signal userspace open chardev preventing safe device removal */
+ struct mutex dmtsc_devs_mux; /* Protect access to device management */
+ struct class *dmtsc_class;
+ struct ptp_dmtsc_cdev_info *cdev_info;
+};
+
struct ptp_clock {
struct posix_clock clock;
struct device dev;
@@ -36,6 +54,10 @@ struct ptp_clock {
struct pps_device *pps_source;
long dialed_frequency; /* remembers the frequency adjustment */
struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
+ u32 dmtsc_en_flags; /* Demultiplexed timestamp channels enable flags */
+ struct mutex dmtsc_sysfs_mux; /* Demultiplexed timestamp channels sysfs mutex */
+ struct timestamp_event_queue *dmtscevq; /* Demultiplexed timestamp channel fifos */
+ struct ptp_dmtsc_dev_info dmtsc_devs; /* Demultiplexed timestamp channel access character devices */
struct mutex tsevq_mux; /* one process at a time reading the fifo */
struct mutex pincfg_mux; /* protect concurrent info->pin_config access */
wait_queue_head_t tsev_wq;
@@ -139,4 +161,7 @@ void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
void ptp_vclock_unregister(struct ptp_vclock *vclock);
+
+int ptp_dmtsc_dev_register(struct ptp_clock *ptp);
+void ptp_dmtsc_dev_uregister(struct ptp_clock *ptp);
#endif
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 6e4d5456a885..fc85aa1d3d23 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -316,6 +316,117 @@ static ssize_t max_vclocks_store(struct device *dev,
}
static DEVICE_ATTR_RW(max_vclocks);
+static ssize_t dmtsc_en_flags_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ ssize_t size;
+
+ size = snprintf(page, PAGE_SIZE - 1, "0x%X\n", ptp->dmtsc_en_flags);
+
+ return size;
+}
+
+static ssize_t dmtsc_en_flags_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ int err = -EINVAL;
+ char mode, val[7];
+ int enable, channel, cnt;
+ u32 req_mask, new_mask;
+
+ /* Read mode, value and enable
+ - mode: "m" - Channel mask mode
+ "c" - Single channel mode
+ - value: channel mask in hex or channel number in decimal
+ lsb = channel 0
+ - enable: '1' - enable
+ '0' - disable
+ */
+ cnt = sscanf(buf, "%c %s %d", &mode, val, &enable);
+ if (cnt != 3)
+ return err;
+
+ if (mutex_lock_interruptible(&ptp->dmtsc_sysfs_mux))
+ return -ERESTARTSYS;
+
+ switch (mode) {
+ case 'm':
+ if (kstrtou32(val, 0, &req_mask))
+ {
+ dev_info(dev, "dmtscevq invalid arguments");
+ goto out;
+ }
+ break;
+ case 'c':
+ if (kstrtoint(val, 0, &channel)) {
+ dev_info(dev, "dmtscevq invalid channel number");
+ goto out;
+ }
+ if ((channel < 0) || (channel > 31))
+ {
+ dev_info(dev, "dmtscevq channel number out of range");
+ goto out;
+ }
+ req_mask = (0x1 << channel);
+ break;
+ default:
+ goto out;
+ break;
+ }
+
+ switch (enable) {
+ case 0:
+ new_mask = ptp->dmtsc_en_flags & ~req_mask;
+ break;
+ case 1:
+ new_mask = ptp->dmtsc_en_flags | req_mask;
+ break;
+ default:
+ dev_info(dev, "dmtscevq invalid enable value");
+ break;
+ }
+
+ if (new_mask == 0x0) {
+ // All queues disabled. Remove all character devices.
+ if (ptp->dmtscevq != NULL) {
+ ptp_dmtsc_dev_uregister(ptp);
+ } else {
+ if (ptp->dmtsc_en_flags != 0x0) {
+ dev_info(dev, "dmtscevq. Unexpected error: TSEVQ exploded buffers presumed unitialized. Skipping.");
+ }
+ }
+ } else {
+ // At least une queue enabled. Create all character devices.
+ // The mask will feed the selected character device and keep others inactive.
+ ptp->dmtsc_devs.readers = 0;
+ ptp->dmtsc_devs.clean_request = false;
+ if (ptp->dmtscevq == NULL) {
+ err = ptp_dmtsc_dev_register(ptp);
+ if (err != 0) {
+ dev_info(dev, "dmtscevq. Error while trying to register exploded queues");
+ goto out;
+ }
+ } else {
+ if (ptp->dmtsc_en_flags == 0x0) {
+ dev_info(dev, "dmtscevq. Unexpected error: TSEVQ exploded buffers already initialized, skipping initialization.");
+ }
+ }
+
+ }
+
+ ptp->dmtsc_en_flags = new_mask;
+
+ mutex_unlock(&ptp->dmtsc_sysfs_mux);
+ return count;
+out:
+ mutex_unlock(&ptp->dmtsc_sysfs_mux);
+ return err;
+}
+static DEVICE_ATTR_RW(dmtsc_en_flags);
+
static struct attribute *ptp_attrs[] = {
&dev_attr_clock_name.attr,
@@ -333,6 +444,8 @@ static struct attribute *ptp_attrs[] = {
&dev_attr_pps_enable.attr,
&dev_attr_n_vclocks.attr,
&dev_attr_max_vclocks.attr,
+
+ &dev_attr_dmtsc_en_flags.attr,
NULL
};
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] ptp: Demultiplexed timestamp channels
2023-08-29 11:47 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
@ 2023-08-29 14:07 ` Richard Cochran
2023-08-29 14:15 ` Richard Cochran
2023-08-30 21:41 ` [chrony-dev] Support for Multiple PPS Inputs on single PHC Xabier Marquiegui
0 siblings, 2 replies; 26+ messages in thread
From: Richard Cochran @ 2023-08-29 14:07 UTC (permalink / raw)
To: Xabier Marquiegui; +Cc: chrony-dev, mlichvar, netdev, ntp-lists
On Tue, Aug 29, 2023 at 01:47:52PM +0200, Xabier Marquiegui wrote:
> Add the posibility to demultiplex the timestamp channels for
> external timestamp event channels.
>
> In some applications it can be necessary to have different
> consumers for different timestamp channels. For example,
> synchronize to an external pps source with linuxptp ts2phc
> while timestmping external events with another application.
>
> This change proposes the dynamic creation of one char-device
> per timestamp channel only if the user requests the demuxing
> of timestamp channels. It allows for on-the-fly demuxing of
> specific channels.
No need to make complex configuration to enable this. Just make one
queue per open character device, and one for sysfs.
> The operation can be controlled via sysfs. See file
> Documentation/ABI/testing/sysfs-ptp for more details.
No need for new sysfs hooks.
> ---
> Documentation/ABI/testing/sysfs-ptp | 16 +++
> MAINTAINERS | 5 +
> drivers/ptp/Makefile | 2 +-
> drivers/ptp/ptp_chardev.c | 2 -
> drivers/ptp/ptp_clock.c | 22 ++-
> drivers/ptp/ptp_demuxtschan.c | 211 ++++++++++++++++++++++++++++
No need to add a second char dev implementation.
Just change the existing one to have a per-file queue.
General comment: Lots of coding style violations here.
See CodingStyle and use scripts/checkpatch.pl
Thanks,
Richard
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ptp: Demultiplexed timestamp channels
2023-08-29 14:07 ` Richard Cochran
@ 2023-08-29 14:15 ` Richard Cochran
2023-08-30 21:41 ` [chrony-dev] Support for Multiple PPS Inputs on single PHC Xabier Marquiegui
1 sibling, 0 replies; 26+ messages in thread
From: Richard Cochran @ 2023-08-29 14:15 UTC (permalink / raw)
To: Xabier Marquiegui; +Cc: chrony-dev, mlichvar, netdev, ntp-lists
On Tue, Aug 29, 2023 at 07:07:57AM -0700, Richard Cochran wrote:
> > The operation can be controlled via sysfs. See file
> > Documentation/ABI/testing/sysfs-ptp for more details.
>
> No need for new sysfs hooks.
Just add a ioctl that adds a filter to an open file.
The filter can be a simple bit mask of say, 1024 bits.
Thanks,
Richard
^ permalink raw reply [flat|nested] 26+ messages in thread
* [chrony-dev] Support for Multiple PPS Inputs on single PHC
2023-08-29 14:07 ` Richard Cochran
2023-08-29 14:15 ` Richard Cochran
@ 2023-08-30 21:41 ` Xabier Marquiegui
2023-08-30 21:41 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
1 sibling, 1 reply; 26+ messages in thread
From: Xabier Marquiegui @ 2023-08-30 21:41 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
Thank you very much for your prompt and kind answer. I appreciate you
taking the time to help me learn and improve my contributions.
I have now tweaked my proposal. You are absolutely right that the whole
thing is simpler and more intuitive without the sysfs control structure.
I have also removed redundant code. I hope it's better now. Let me know
what you think.
Thank you very much,
Xabier.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] ptp: Demultiplexed timestamp channels
2023-08-30 21:41 ` [chrony-dev] Support for Multiple PPS Inputs on single PHC Xabier Marquiegui
@ 2023-08-30 21:41 ` Xabier Marquiegui
2023-08-30 22:01 ` Richard Cochran
` (3 more replies)
0 siblings, 4 replies; 26+ messages in thread
From: Xabier Marquiegui @ 2023-08-30 21:41 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
Add the posibility to demultiplex the timestamp channels for
external timestamp event channels.
In some applications it can be necessary to have different
consumers for different timestamp channels. For example,
synchronize to an external pps source with linuxptp ts2phc
while timestmping external events with another application.
This commit maintains the original multiplexed queue and adds an
individual queue per external timestamp channel. All enabled channels
will be directed to the multiplexed queue by default. On file open, a
specific channel will be redirected to the dedicated char device, and on
close it will automatically go back to the multiplexed queue.
Signed-off-by: Xabier Marquiegui <reibax@gmail.com>
---
drivers/ptp/ptp_chardev.c | 167 +++++++++++++++++++++++++++++++++++---
drivers/ptp/ptp_clock.c | 43 +++++++---
drivers/ptp/ptp_private.h | 22 +++++
3 files changed, 210 insertions(+), 22 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 362bf756e6b7..c31cfc5b0907 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -10,11 +10,14 @@
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/timekeeping.h>
-
+#include <linux/cdev.h>
+#include <linux/fs.h>
#include <linux/nospec.h>
#include "ptp_private.h"
+#define DMTSC_NOT -1
+
static int ptp_disable_pinfunc(struct ptp_clock_info *ops,
enum ptp_pin_function func, unsigned int chan)
{
@@ -443,16 +446,24 @@ __poll_t ptp_poll(struct posix_clock *pc, struct file *fp, poll_table *wait)
#define EXTTS_BUFSIZE (PTP_BUF_TIMESTAMPS * sizeof(struct ptp_extts_event))
-ssize_t ptp_read(struct posix_clock *pc,
- uint rdflags, char __user *buf, size_t cnt)
+ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
+ int dmtsc)
{
- struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
- struct timestamp_event_queue *queue = &ptp->tsevq;
+ struct timestamp_event_queue *queue;
+ struct mutex *tsevq_mux;
struct ptp_extts_event *event;
unsigned long flags;
size_t qcnt, i;
int result;
+ if (dmtsc < 0) {
+ queue = &ptp->tsevq;
+ tsevq_mux = &ptp->tsevq_mux;
+ } else {
+ queue = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq;
+ tsevq_mux = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq_mux;
+ }
+
if (cnt % sizeof(struct ptp_extts_event) != 0)
return -EINVAL;
@@ -461,23 +472,23 @@ ssize_t ptp_read(struct posix_clock *pc,
cnt = cnt / sizeof(struct ptp_extts_event);
- if (mutex_lock_interruptible(&ptp->tsevq_mux))
+ if (mutex_lock_interruptible(tsevq_mux))
return -ERESTARTSYS;
if (wait_event_interruptible(ptp->tsev_wq,
ptp->defunct || queue_cnt(queue))) {
- mutex_unlock(&ptp->tsevq_mux);
+ mutex_unlock(tsevq_mux);
return -ERESTARTSYS;
}
if (ptp->defunct) {
- mutex_unlock(&ptp->tsevq_mux);
+ mutex_unlock(tsevq_mux);
return -ENODEV;
}
event = kmalloc(EXTTS_BUFSIZE, GFP_KERNEL);
if (!event) {
- mutex_unlock(&ptp->tsevq_mux);
+ mutex_unlock(tsevq_mux);
return -ENOMEM;
}
@@ -497,7 +508,7 @@ ssize_t ptp_read(struct posix_clock *pc,
cnt = cnt * sizeof(struct ptp_extts_event);
- mutex_unlock(&ptp->tsevq_mux);
+ mutex_unlock(tsevq_mux);
result = cnt;
if (copy_to_user(buf, event, cnt))
@@ -506,3 +517,139 @@ ssize_t ptp_read(struct posix_clock *pc,
kfree(event);
return result;
}
+
+ssize_t ptp_read(struct posix_clock *pc, uint rdflags, char __user *buf,
+ size_t cnt)
+{
+ struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+
+ return ptp_queue_read(ptp, buf, cnt, DMTSC_NOT);
+}
+
+static int ptp_dmtsc_open(struct inode *inode, struct file *file)
+{
+ struct ptp_dmtsc_cdev_info *cdev = container_of(
+ inode->i_cdev, struct ptp_dmtsc_cdev_info, dmtsc_cdev);
+
+ file->private_data = cdev;
+
+ if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
+ return -ERESTARTSYS;
+ cdev->pclock->dmtsc_en_flags |= (0x1 << (cdev->minor));
+ mutex_unlock(&cdev->pclock->dmtsc_en_mux);
+
+ return stream_open(inode, file);
+}
+
+int ptp_dmtsc_release(struct inode *inode, struct file *file)
+{
+ struct ptp_dmtsc_cdev_info *cdev = file->private_data;
+
+ if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
+ return -ERESTARTSYS;
+ cdev->pclock->dmtsc_en_flags &= ~(0x1 << (cdev->minor));
+ mutex_unlock(&cdev->pclock->dmtsc_en_mux);
+
+ return 0;
+}
+
+ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
+ loff_t *offset)
+{
+ struct ptp_dmtsc_cdev_info *cdev = file->private_data;
+
+ return ptp_queue_read(cdev->pclock, buf, cnt, cdev->minor);
+}
+
+static const struct file_operations fops = {
+ .owner = THIS_MODULE,
+ .open = ptp_dmtsc_open,
+ .read = ptp_dmtsc_read,
+ .release = ptp_dmtsc_release
+ };
+
+void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
+{
+ int idx, major;
+ dev_t device;
+
+ major = MAJOR(ptp->dmtsc_devs.devid);
+ for (idx = 0; idx < ptp->info->n_ext_ts; idx++) {
+ if (ptp->dmtsc_devs.cdev_info[idx].minor >= 0) {
+ device = MKDEV(major, idx);
+ device_destroy(ptp->dmtsc_devs.dmtsc_class, device);
+ cdev_del(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev);
+ ptp->dmtsc_devs.cdev_info[idx].minor = -1;
+ }
+ }
+ class_destroy(ptp->dmtsc_devs.dmtsc_class);
+ unregister_chrdev_region(ptp->dmtsc_devs.devid, ptp->info->n_ext_ts);
+ mutex_destroy(&ptp->dmtsc_devs.cdev_info[idx].tsevq_mux);
+}
+
+int ptp_dmtsc_dev_register(struct ptp_clock *ptp)
+{
+ int err, idx, major;
+ dev_t device;
+ struct device *dev;
+
+ /* Allocate memory for demuxed device management */
+ ptp->dmtsc_devs.cdev_info = kcalloc(ptp->info->n_ext_ts,
+ sizeof(*ptp->dmtsc_devs.cdev_info),
+ GFP_KERNEL);
+ if (!ptp->dmtsc_devs.cdev_info) {
+ err = -ENODEV;
+ goto err;
+ }
+ for (idx = 0; idx < ptp->info->n_ext_ts; idx++)
+ ptp->dmtsc_devs.cdev_info[idx].minor = -1;
+ /* Create devices for all channels. The mask will control which of them get fed */
+ err = alloc_chrdev_region(&ptp->dmtsc_devs.devid, 0,
+ ptp->info->n_ext_ts, "ptptsevqch");
+ if (!err) {
+ major = MAJOR(ptp->dmtsc_devs.devid);
+ ptp->dmtsc_devs.dmtsc_class =
+ class_create(THIS_MODULE, "ptptsevqch_class");
+ for (idx = 0; idx < ptp->info->n_ext_ts; idx++) {
+ mutex_init(&ptp->dmtsc_devs.cdev_info[idx].tsevq_mux);
+ device = MKDEV(major, idx);
+ ptp->dmtsc_devs.cdev_info[idx].pclock = ptp;
+ cdev_init(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev,
+ &fops);
+ err = cdev_add(
+ &ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev,
+ device, 1);
+ if (err) {
+ goto cdev_clean;
+ } else {
+ ptp->dmtsc_devs.cdev_info[idx].minor = idx;
+ dev = device_create(ptp->dmtsc_devs.dmtsc_class,
+ &ptp->dev, device, NULL,
+ "ptp%dch%d", ptp->index,
+ idx);
+ if (IS_ERR(dev)) {
+ err = PTR_ERR(dev);
+ goto cdev_clean;
+ }
+ }
+ }
+ } else {
+ goto dev_clean;
+ }
+ return 0;
+
+cdev_clean:
+ ptp_dmtsc_cdev_clean(ptp);
+dev_clean:
+ kfree(ptp->dmtsc_devs.cdev_info);
+ ptp->dmtsc_devs.cdev_info = NULL;
+err:
+ return err;
+}
+
+void ptp_dmtsc_dev_uregister(struct ptp_clock *ptp)
+{
+ ptp_dmtsc_cdev_clean(ptp);
+ kfree(ptp->dmtsc_devs.cdev_info);
+ ptp->dmtsc_devs.cdev_info = NULL;
+}
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 80f74e38c2da..0a42c27c3514 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -172,6 +172,7 @@ static void ptp_clock_release(struct device *dev)
ptp_cleanup_pin_groups(ptp);
kfree(ptp->vclock_index);
+ mutex_destroy(&ptp->dmtsc_en_mux);
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
@@ -232,7 +233,9 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
mutex_init(&ptp->tsevq_mux);
mutex_init(&ptp->pincfg_mux);
mutex_init(&ptp->n_vclocks_mux);
+ mutex_init(&ptp->dmtsc_en_mux);
init_waitqueue_head(&ptp->tsev_wq);
+ ptp->dmtsc_en_flags = 0x0;
if (ptp->info->getcycles64 || ptp->info->getcyclesx64) {
ptp->has_cycles = true;
@@ -307,21 +310,27 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
/* Create a posix clock and link it to the device. */
err = posix_clock_register(&ptp->clock, &ptp->dev);
- if (err) {
- if (ptp->pps_source)
- pps_unregister_source(ptp->pps_source);
+ if (err)
+ goto reg_err;
- if (ptp->kworker)
- kthread_destroy_worker(ptp->kworker);
+ /* Create chardevs for demuxed external timestamp channels */
+ if (ptp_dmtsc_dev_register(ptp))
+ goto reg_err;
- put_device(&ptp->dev);
+ return ptp;
- pr_err("failed to create posix clock\n");
- return ERR_PTR(err);
- }
+reg_err:
+ ptp_dmtsc_dev_uregister(ptp);
+ if (ptp->pps_source)
+ pps_unregister_source(ptp->pps_source);
- return ptp;
+ if (ptp->kworker)
+ kthread_destroy_worker(ptp->kworker);
+
+ put_device(&ptp->dev);
+ pr_err("failed to create posix clock\n");
+ return ERR_PTR(err);
no_pps:
ptp_cleanup_pin_groups(ptp);
no_pin_groups:
@@ -330,6 +339,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (ptp->kworker)
kthread_destroy_worker(ptp->kworker);
kworker_err:
+ mutex_destroy(&ptp->dmtsc_en_mux);
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
@@ -367,6 +377,8 @@ int ptp_clock_unregister(struct ptp_clock *ptp)
if (ptp->pps_source)
pps_unregister_source(ptp->pps_source);
+ ptp_dmtsc_dev_uregister(ptp);
+
posix_clock_unregister(&ptp->clock);
return 0;
@@ -378,12 +390,19 @@ void ptp_clock_event(struct ptp_clock *ptp, struct ptp_clock_event *event)
struct pps_event_time evt;
switch (event->type) {
-
case PTP_CLOCK_ALARM:
break;
case PTP_CLOCK_EXTTS:
- enqueue_external_timestamp(&ptp->tsevq, event);
+ /* If event index demuxed queue mask is enabled send to dedicated fifo */
+ if (ptp->dmtsc_en_flags & (0x1 << event->index)) {
+ enqueue_external_timestamp(
+ &ptp->dmtsc_devs.cdev_info[event->index].tsevq,
+ event);
+ } else {
+ enqueue_external_timestamp(&ptp->tsevq, event);
+ }
+
wake_up_interruptible(&ptp->tsev_wq);
break;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 75f58fc468a7..c473ef75d8d7 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -27,6 +27,20 @@ struct timestamp_event_queue {
spinlock_t lock;
};
+struct ptp_dmtsc_cdev_info {
+ struct cdev dmtsc_cdev; /* Demuxed event device chardev */
+ int minor; /* Demuxed event queue chardev device minor */
+ struct ptp_clock *pclock; /* Direct access to parent clock device */
+ struct mutex tsevq_mux; /* Protect access to device management */
+ struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
+};
+
+struct ptp_dmtsc_dev_info {
+ dev_t devid;
+ struct class *dmtsc_class;
+ struct ptp_dmtsc_cdev_info *cdev_info;
+};
+
struct ptp_clock {
struct posix_clock clock;
struct device dev;
@@ -36,6 +50,11 @@ struct ptp_clock {
struct pps_device *pps_source;
long dialed_frequency; /* remembers the frequency adjustment */
struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
+ u32 dmtsc_en_flags; /* Demultiplexed timestamp channels enable flags */
+ struct mutex
+ dmtsc_en_mux; /* Demultiplexed timestamp channels sysfs mutex */
+ struct ptp_dmtsc_dev_info
+ dmtsc_devs; /* Demultiplexed timestamp channel access character devices */
struct mutex tsevq_mux; /* one process at a time reading the fifo */
struct mutex pincfg_mux; /* protect concurrent info->pin_config access */
wait_queue_head_t tsev_wq;
@@ -139,4 +158,7 @@ void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
void ptp_vclock_unregister(struct ptp_vclock *vclock);
+
+int ptp_dmtsc_dev_register(struct ptp_clock *ptp);
+void ptp_dmtsc_dev_uregister(struct ptp_clock *ptp);
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] ptp: Demultiplexed timestamp channels
2023-08-30 21:41 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
@ 2023-08-30 22:01 ` Richard Cochran
2023-09-06 10:47 ` Xabier Marquiegui
2023-08-31 0:29 ` [PATCH] ptp: Demultiplexed timestamp channels kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 26+ messages in thread
From: Richard Cochran @ 2023-08-30 22:01 UTC (permalink / raw)
To: Xabier Marquiegui; +Cc: chrony-dev, mlichvar, netdev, ntp-lists
On Wed, Aug 30, 2023 at 11:41:01PM +0200, Xabier Marquiegui wrote:
> +ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
> + loff_t *offset)
> +{
> + struct ptp_dmtsc_cdev_info *cdev = file->private_data;
> +
> + return ptp_queue_read(cdev->pclock, buf, cnt, cdev->minor);
> +}
> +
> +static const struct file_operations fops = {
> + .owner = THIS_MODULE,
> + .open = ptp_dmtsc_open,
> + .read = ptp_dmtsc_read,
> + .release = ptp_dmtsc_release
> + };
I'll say it again... There is no need to add a new char dev!
You will need a patch series of at least three patches, step by step:
1. Delete ptp_clock.tsevq
Replace it will a linked list of `struct timestamp_event_queue`
Populate the list with ONE queue in ptp_clock_register()
This will be your first patch.
It will have the same functionality as before.
2. Allocate a new `struct timestamp_event_queue` on posix_clock_operations.open()
Add it into the list.
Remove it again on posix_clock_operations.release()
This will introduce new functionality, as all events will go to all
consumers.
3. Introduce a new ioctl that filters events based on user preference.
Hm?
Thanks,
Richard
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ptp: Demultiplexed timestamp channels
2023-08-30 21:41 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
2023-08-30 22:01 ` Richard Cochran
@ 2023-08-31 0:29 ` kernel test robot
2023-08-31 13:28 ` kernel test robot
2023-08-31 16:20 ` kernel test robot
3 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2023-08-31 0:29 UTC (permalink / raw)
To: Xabier Marquiegui, richardcochran
Cc: oe-kbuild-all, chrony-dev, mlichvar, netdev, ntp-lists, reibax
Hi Xabier,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
[also build test WARNING on net-next/main linus/master horms-ipvs/master v6.5 next-20230830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Xabier-Marquiegui/ptp-Demultiplexed-timestamp-channels/20230831-054428
base: net/main
patch link: https://lore.kernel.org/r/20230830214101.509086-2-reibax%40gmail.com
patch subject: [PATCH] ptp: Demultiplexed timestamp channels
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230831/202308310809.wcvhamyw-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230831/202308310809.wcvhamyw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308310809.wcvhamyw-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/ptp/ptp_chardev.c:449:9: warning: no previous prototype for 'ptp_queue_read' [-Wmissing-prototypes]
449 | ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
| ^~~~~~~~~~~~~~
>> drivers/ptp/ptp_chardev.c:544:5: warning: no previous prototype for 'ptp_dmtsc_release' [-Wmissing-prototypes]
544 | int ptp_dmtsc_release(struct inode *inode, struct file *file)
| ^~~~~~~~~~~~~~~~~
>> drivers/ptp/ptp_chardev.c:556:9: warning: no previous prototype for 'ptp_dmtsc_read' [-Wmissing-prototypes]
556 | ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
| ^~~~~~~~~~~~~~
>> drivers/ptp/ptp_chardev.c:571:6: warning: no previous prototype for 'ptp_dmtsc_cdev_clean' [-Wmissing-prototypes]
571 | void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
| ^~~~~~~~~~~~~~~~~~~~
In file included from include/linux/linkage.h:7,
from include/linux/preempt.h:10,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:17,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:7,
from include/linux/mm_types_task.h:13,
from include/linux/mm_types.h:5,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/ptp/ptp_chardev.c:7:
drivers/ptp/ptp_chardev.c: In function 'ptp_dmtsc_dev_register':
include/linux/export.h:31:21: error: passing argument 1 of 'class_create' from incompatible pointer type [-Werror=incompatible-pointer-types]
31 | #define THIS_MODULE ((struct module *)0)
| ^~~~~~~~~~~~~~~~~~~~
| |
| struct module *
drivers/ptp/ptp_chardev.c:612:38: note: in expansion of macro 'THIS_MODULE'
612 | class_create(THIS_MODULE, "ptptsevqch_class");
| ^~~~~~~~~~~
In file included from include/linux/device.h:31,
from include/linux/cdev.h:8,
from include/linux/posix-clock.h:10,
from drivers/ptp/ptp_chardev.c:8:
include/linux/device/class.h:230:54: note: expected 'const char *' but argument is of type 'struct module *'
230 | struct class * __must_check class_create(const char *name);
| ~~~~~~~~~~~~^~~~
drivers/ptp/ptp_chardev.c:612:25: error: too many arguments to function 'class_create'
612 | class_create(THIS_MODULE, "ptptsevqch_class");
| ^~~~~~~~~~~~
include/linux/device/class.h:230:29: note: declared here
230 | struct class * __must_check class_create(const char *name);
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/ptp_queue_read +449 drivers/ptp/ptp_chardev.c
448
> 449 ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
450 int dmtsc)
451 {
452 struct timestamp_event_queue *queue;
453 struct mutex *tsevq_mux;
454 struct ptp_extts_event *event;
455 unsigned long flags;
456 size_t qcnt, i;
457 int result;
458
459 if (dmtsc < 0) {
460 queue = &ptp->tsevq;
461 tsevq_mux = &ptp->tsevq_mux;
462 } else {
463 queue = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq;
464 tsevq_mux = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq_mux;
465 }
466
467 if (cnt % sizeof(struct ptp_extts_event) != 0)
468 return -EINVAL;
469
470 if (cnt > EXTTS_BUFSIZE)
471 cnt = EXTTS_BUFSIZE;
472
473 cnt = cnt / sizeof(struct ptp_extts_event);
474
475 if (mutex_lock_interruptible(tsevq_mux))
476 return -ERESTARTSYS;
477
478 if (wait_event_interruptible(ptp->tsev_wq,
479 ptp->defunct || queue_cnt(queue))) {
480 mutex_unlock(tsevq_mux);
481 return -ERESTARTSYS;
482 }
483
484 if (ptp->defunct) {
485 mutex_unlock(tsevq_mux);
486 return -ENODEV;
487 }
488
489 event = kmalloc(EXTTS_BUFSIZE, GFP_KERNEL);
490 if (!event) {
491 mutex_unlock(tsevq_mux);
492 return -ENOMEM;
493 }
494
495 spin_lock_irqsave(&queue->lock, flags);
496
497 qcnt = queue_cnt(queue);
498
499 if (cnt > qcnt)
500 cnt = qcnt;
501
502 for (i = 0; i < cnt; i++) {
503 event[i] = queue->buf[queue->head];
504 queue->head = (queue->head + 1) % PTP_MAX_TIMESTAMPS;
505 }
506
507 spin_unlock_irqrestore(&queue->lock, flags);
508
509 cnt = cnt * sizeof(struct ptp_extts_event);
510
511 mutex_unlock(tsevq_mux);
512
513 result = cnt;
514 if (copy_to_user(buf, event, cnt))
515 result = -EFAULT;
516
517 kfree(event);
518 return result;
519 }
520
521 ssize_t ptp_read(struct posix_clock *pc, uint rdflags, char __user *buf,
522 size_t cnt)
523 {
524 struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
525
526 return ptp_queue_read(ptp, buf, cnt, DMTSC_NOT);
527 }
528
529 static int ptp_dmtsc_open(struct inode *inode, struct file *file)
530 {
531 struct ptp_dmtsc_cdev_info *cdev = container_of(
532 inode->i_cdev, struct ptp_dmtsc_cdev_info, dmtsc_cdev);
533
534 file->private_data = cdev;
535
536 if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
537 return -ERESTARTSYS;
538 cdev->pclock->dmtsc_en_flags |= (0x1 << (cdev->minor));
539 mutex_unlock(&cdev->pclock->dmtsc_en_mux);
540
541 return stream_open(inode, file);
542 }
543
> 544 int ptp_dmtsc_release(struct inode *inode, struct file *file)
545 {
546 struct ptp_dmtsc_cdev_info *cdev = file->private_data;
547
548 if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
549 return -ERESTARTSYS;
550 cdev->pclock->dmtsc_en_flags &= ~(0x1 << (cdev->minor));
551 mutex_unlock(&cdev->pclock->dmtsc_en_mux);
552
553 return 0;
554 }
555
> 556 ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
557 loff_t *offset)
558 {
559 struct ptp_dmtsc_cdev_info *cdev = file->private_data;
560
561 return ptp_queue_read(cdev->pclock, buf, cnt, cdev->minor);
562 }
563
564 static const struct file_operations fops = {
565 .owner = THIS_MODULE,
566 .open = ptp_dmtsc_open,
567 .read = ptp_dmtsc_read,
568 .release = ptp_dmtsc_release
569 };
570
> 571 void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
572 {
573 int idx, major;
574 dev_t device;
575
576 major = MAJOR(ptp->dmtsc_devs.devid);
577 for (idx = 0; idx < ptp->info->n_ext_ts; idx++) {
578 if (ptp->dmtsc_devs.cdev_info[idx].minor >= 0) {
579 device = MKDEV(major, idx);
580 device_destroy(ptp->dmtsc_devs.dmtsc_class, device);
581 cdev_del(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev);
582 ptp->dmtsc_devs.cdev_info[idx].minor = -1;
583 }
584 }
585 class_destroy(ptp->dmtsc_devs.dmtsc_class);
586 unregister_chrdev_region(ptp->dmtsc_devs.devid, ptp->info->n_ext_ts);
587 mutex_destroy(&ptp->dmtsc_devs.cdev_info[idx].tsevq_mux);
588 }
589
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ptp: Demultiplexed timestamp channels
2023-08-30 21:41 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
2023-08-30 22:01 ` Richard Cochran
2023-08-31 0:29 ` [PATCH] ptp: Demultiplexed timestamp channels kernel test robot
@ 2023-08-31 13:28 ` kernel test robot
2023-08-31 16:20 ` kernel test robot
3 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2023-08-31 13:28 UTC (permalink / raw)
To: Xabier Marquiegui, richardcochran
Cc: llvm, oe-kbuild-all, chrony-dev, mlichvar, netdev, ntp-lists, reibax
Hi Xabier,
kernel test robot noticed the following build errors:
[auto build test ERROR on net/main]
[also build test ERROR on net-next/main linus/master horms-ipvs/master v6.5 next-20230831]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Xabier-Marquiegui/ptp-Demultiplexed-timestamp-channels/20230831-054428
base: net/main
patch link: https://lore.kernel.org/r/20230830214101.509086-2-reibax%40gmail.com
patch subject: [PATCH] ptp: Demultiplexed timestamp channels
config: powerpc64-randconfig-r026-20230831 (https://download.01.org/0day-ci/archive/20230831/202308312155.cA1uQaGm-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230831/202308312155.cA1uQaGm-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308312155.cA1uQaGm-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> drivers/ptp/ptp_chardev.c:449:9: warning: no previous prototype for function 'ptp_queue_read' [-Wmissing-prototypes]
449 | ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
| ^
drivers/ptp/ptp_chardev.c:449:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
449 | ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
| ^
| static
>> drivers/ptp/ptp_chardev.c:544:5: warning: no previous prototype for function 'ptp_dmtsc_release' [-Wmissing-prototypes]
544 | int ptp_dmtsc_release(struct inode *inode, struct file *file)
| ^
drivers/ptp/ptp_chardev.c:544:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
544 | int ptp_dmtsc_release(struct inode *inode, struct file *file)
| ^
| static
>> drivers/ptp/ptp_chardev.c:556:9: warning: no previous prototype for function 'ptp_dmtsc_read' [-Wmissing-prototypes]
556 | ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
| ^
drivers/ptp/ptp_chardev.c:556:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
556 | ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
| ^
| static
>> drivers/ptp/ptp_chardev.c:571:6: warning: no previous prototype for function 'ptp_dmtsc_cdev_clean' [-Wmissing-prototypes]
571 | void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
| ^
drivers/ptp/ptp_chardev.c:571:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
571 | void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
| ^
| static
>> drivers/ptp/ptp_chardev.c:612:30: error: too many arguments to function call, expected single argument 'name', have 2 arguments
612 | class_create(THIS_MODULE, "ptptsevqch_class");
| ~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~
include/linux/device/class.h:230:29: note: 'class_create' declared here
230 | struct class * __must_check class_create(const char *name);
| ^
4 warnings and 1 error generated.
vim +/name +612 drivers/ptp/ptp_chardev.c
448
> 449 ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
450 int dmtsc)
451 {
452 struct timestamp_event_queue *queue;
453 struct mutex *tsevq_mux;
454 struct ptp_extts_event *event;
455 unsigned long flags;
456 size_t qcnt, i;
457 int result;
458
459 if (dmtsc < 0) {
460 queue = &ptp->tsevq;
461 tsevq_mux = &ptp->tsevq_mux;
462 } else {
463 queue = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq;
464 tsevq_mux = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq_mux;
465 }
466
467 if (cnt % sizeof(struct ptp_extts_event) != 0)
468 return -EINVAL;
469
470 if (cnt > EXTTS_BUFSIZE)
471 cnt = EXTTS_BUFSIZE;
472
473 cnt = cnt / sizeof(struct ptp_extts_event);
474
475 if (mutex_lock_interruptible(tsevq_mux))
476 return -ERESTARTSYS;
477
478 if (wait_event_interruptible(ptp->tsev_wq,
479 ptp->defunct || queue_cnt(queue))) {
480 mutex_unlock(tsevq_mux);
481 return -ERESTARTSYS;
482 }
483
484 if (ptp->defunct) {
485 mutex_unlock(tsevq_mux);
486 return -ENODEV;
487 }
488
489 event = kmalloc(EXTTS_BUFSIZE, GFP_KERNEL);
490 if (!event) {
491 mutex_unlock(tsevq_mux);
492 return -ENOMEM;
493 }
494
495 spin_lock_irqsave(&queue->lock, flags);
496
497 qcnt = queue_cnt(queue);
498
499 if (cnt > qcnt)
500 cnt = qcnt;
501
502 for (i = 0; i < cnt; i++) {
503 event[i] = queue->buf[queue->head];
504 queue->head = (queue->head + 1) % PTP_MAX_TIMESTAMPS;
505 }
506
507 spin_unlock_irqrestore(&queue->lock, flags);
508
509 cnt = cnt * sizeof(struct ptp_extts_event);
510
511 mutex_unlock(tsevq_mux);
512
513 result = cnt;
514 if (copy_to_user(buf, event, cnt))
515 result = -EFAULT;
516
517 kfree(event);
518 return result;
519 }
520
521 ssize_t ptp_read(struct posix_clock *pc, uint rdflags, char __user *buf,
522 size_t cnt)
523 {
524 struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
525
526 return ptp_queue_read(ptp, buf, cnt, DMTSC_NOT);
527 }
528
529 static int ptp_dmtsc_open(struct inode *inode, struct file *file)
530 {
531 struct ptp_dmtsc_cdev_info *cdev = container_of(
532 inode->i_cdev, struct ptp_dmtsc_cdev_info, dmtsc_cdev);
533
534 file->private_data = cdev;
535
536 if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
537 return -ERESTARTSYS;
538 cdev->pclock->dmtsc_en_flags |= (0x1 << (cdev->minor));
539 mutex_unlock(&cdev->pclock->dmtsc_en_mux);
540
541 return stream_open(inode, file);
542 }
543
> 544 int ptp_dmtsc_release(struct inode *inode, struct file *file)
545 {
546 struct ptp_dmtsc_cdev_info *cdev = file->private_data;
547
548 if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
549 return -ERESTARTSYS;
550 cdev->pclock->dmtsc_en_flags &= ~(0x1 << (cdev->minor));
551 mutex_unlock(&cdev->pclock->dmtsc_en_mux);
552
553 return 0;
554 }
555
> 556 ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
557 loff_t *offset)
558 {
559 struct ptp_dmtsc_cdev_info *cdev = file->private_data;
560
561 return ptp_queue_read(cdev->pclock, buf, cnt, cdev->minor);
562 }
563
564 static const struct file_operations fops = {
565 .owner = THIS_MODULE,
566 .open = ptp_dmtsc_open,
567 .read = ptp_dmtsc_read,
568 .release = ptp_dmtsc_release
569 };
570
> 571 void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
572 {
573 int idx, major;
574 dev_t device;
575
576 major = MAJOR(ptp->dmtsc_devs.devid);
577 for (idx = 0; idx < ptp->info->n_ext_ts; idx++) {
578 if (ptp->dmtsc_devs.cdev_info[idx].minor >= 0) {
579 device = MKDEV(major, idx);
580 device_destroy(ptp->dmtsc_devs.dmtsc_class, device);
581 cdev_del(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev);
582 ptp->dmtsc_devs.cdev_info[idx].minor = -1;
583 }
584 }
585 class_destroy(ptp->dmtsc_devs.dmtsc_class);
586 unregister_chrdev_region(ptp->dmtsc_devs.devid, ptp->info->n_ext_ts);
587 mutex_destroy(&ptp->dmtsc_devs.cdev_info[idx].tsevq_mux);
588 }
589
590 int ptp_dmtsc_dev_register(struct ptp_clock *ptp)
591 {
592 int err, idx, major;
593 dev_t device;
594 struct device *dev;
595
596 /* Allocate memory for demuxed device management */
597 ptp->dmtsc_devs.cdev_info = kcalloc(ptp->info->n_ext_ts,
598 sizeof(*ptp->dmtsc_devs.cdev_info),
599 GFP_KERNEL);
600 if (!ptp->dmtsc_devs.cdev_info) {
601 err = -ENODEV;
602 goto err;
603 }
604 for (idx = 0; idx < ptp->info->n_ext_ts; idx++)
605 ptp->dmtsc_devs.cdev_info[idx].minor = -1;
606 /* Create devices for all channels. The mask will control which of them get fed */
607 err = alloc_chrdev_region(&ptp->dmtsc_devs.devid, 0,
608 ptp->info->n_ext_ts, "ptptsevqch");
609 if (!err) {
610 major = MAJOR(ptp->dmtsc_devs.devid);
611 ptp->dmtsc_devs.dmtsc_class =
> 612 class_create(THIS_MODULE, "ptptsevqch_class");
613 for (idx = 0; idx < ptp->info->n_ext_ts; idx++) {
614 mutex_init(&ptp->dmtsc_devs.cdev_info[idx].tsevq_mux);
615 device = MKDEV(major, idx);
616 ptp->dmtsc_devs.cdev_info[idx].pclock = ptp;
617 cdev_init(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev,
618 &fops);
619 err = cdev_add(
620 &ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev,
621 device, 1);
622 if (err) {
623 goto cdev_clean;
624 } else {
625 ptp->dmtsc_devs.cdev_info[idx].minor = idx;
626 dev = device_create(ptp->dmtsc_devs.dmtsc_class,
627 &ptp->dev, device, NULL,
628 "ptp%dch%d", ptp->index,
629 idx);
630 if (IS_ERR(dev)) {
631 err = PTR_ERR(dev);
632 goto cdev_clean;
633 }
634 }
635 }
636 } else {
637 goto dev_clean;
638 }
639 return 0;
640
641 cdev_clean:
642 ptp_dmtsc_cdev_clean(ptp);
643 dev_clean:
644 kfree(ptp->dmtsc_devs.cdev_info);
645 ptp->dmtsc_devs.cdev_info = NULL;
646 err:
647 return err;
648 }
649
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] ptp: Demultiplexed timestamp channels
2023-08-30 21:41 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
` (2 preceding siblings ...)
2023-08-31 13:28 ` kernel test robot
@ 2023-08-31 16:20 ` kernel test robot
3 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2023-08-31 16:20 UTC (permalink / raw)
To: Xabier Marquiegui, richardcochran
Cc: oe-kbuild-all, chrony-dev, mlichvar, netdev, ntp-lists, reibax
Hi Xabier,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
[also build test WARNING on net-next/main linus/master horms-ipvs/master v6.5 next-20230831]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Xabier-Marquiegui/ptp-Demultiplexed-timestamp-channels/20230831-054428
base: net/main
patch link: https://lore.kernel.org/r/20230830214101.509086-2-reibax%40gmail.com
patch subject: [PATCH] ptp: Demultiplexed timestamp channels
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20230901/202309010030.U9g5VVWf-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230901/202309010030.U9g5VVWf-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309010030.U9g5VVWf-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/ptp/ptp_chardev.c:449:9: warning: no previous declaration for 'ptp_queue_read' [-Wmissing-declarations]
ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
^~~~~~~~~~~~~~
>> drivers/ptp/ptp_chardev.c:544:5: warning: no previous declaration for 'ptp_dmtsc_release' [-Wmissing-declarations]
int ptp_dmtsc_release(struct inode *inode, struct file *file)
^~~~~~~~~~~~~~~~~
>> drivers/ptp/ptp_chardev.c:556:9: warning: no previous declaration for 'ptp_dmtsc_read' [-Wmissing-declarations]
ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
^~~~~~~~~~~~~~
>> drivers/ptp/ptp_chardev.c:571:6: warning: no previous declaration for 'ptp_dmtsc_cdev_clean' [-Wmissing-declarations]
void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
^~~~~~~~~~~~~~~~~~~~
In file included from include/linux/linkage.h:7:0,
from include/linux/kernel.h:17,
from include/linux/cpumask.h:10,
from include/linux/mm_types_task.h:14,
from include/linux/mm_types.h:5,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/ptp/ptp_chardev.c:7:
drivers/ptp/ptp_chardev.c: In function 'ptp_dmtsc_dev_register':
include/linux/export.h:31:21: error: passing argument 1 of 'class_create' from incompatible pointer type [-Werror=incompatible-pointer-types]
#define THIS_MODULE ((struct module *)0)
^
drivers/ptp/ptp_chardev.c:612:17: note: in expansion of macro 'THIS_MODULE'
class_create(THIS_MODULE, "ptptsevqch_class");
^~~~~~~~~~~
In file included from include/linux/device.h:31:0,
from include/linux/cdev.h:8,
from include/linux/posix-clock.h:10,
from drivers/ptp/ptp_chardev.c:8:
include/linux/device/class.h:230:29: note: expected 'const char *' but argument is of type 'struct module *'
struct class * __must_check class_create(const char *name);
^~~~~~~~~~~~
drivers/ptp/ptp_chardev.c:612:4: error: too many arguments to function 'class_create'
class_create(THIS_MODULE, "ptptsevqch_class");
^~~~~~~~~~~~
In file included from include/linux/device.h:31:0,
from include/linux/cdev.h:8,
from include/linux/posix-clock.h:10,
from drivers/ptp/ptp_chardev.c:8:
include/linux/device/class.h:230:29: note: declared here
struct class * __must_check class_create(const char *name);
^~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/ptp_queue_read +449 drivers/ptp/ptp_chardev.c
448
> 449 ssize_t ptp_queue_read(struct ptp_clock *ptp, char __user *buf, size_t cnt,
450 int dmtsc)
451 {
452 struct timestamp_event_queue *queue;
453 struct mutex *tsevq_mux;
454 struct ptp_extts_event *event;
455 unsigned long flags;
456 size_t qcnt, i;
457 int result;
458
459 if (dmtsc < 0) {
460 queue = &ptp->tsevq;
461 tsevq_mux = &ptp->tsevq_mux;
462 } else {
463 queue = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq;
464 tsevq_mux = &ptp->dmtsc_devs.cdev_info[dmtsc].tsevq_mux;
465 }
466
467 if (cnt % sizeof(struct ptp_extts_event) != 0)
468 return -EINVAL;
469
470 if (cnt > EXTTS_BUFSIZE)
471 cnt = EXTTS_BUFSIZE;
472
473 cnt = cnt / sizeof(struct ptp_extts_event);
474
475 if (mutex_lock_interruptible(tsevq_mux))
476 return -ERESTARTSYS;
477
478 if (wait_event_interruptible(ptp->tsev_wq,
479 ptp->defunct || queue_cnt(queue))) {
480 mutex_unlock(tsevq_mux);
481 return -ERESTARTSYS;
482 }
483
484 if (ptp->defunct) {
485 mutex_unlock(tsevq_mux);
486 return -ENODEV;
487 }
488
489 event = kmalloc(EXTTS_BUFSIZE, GFP_KERNEL);
490 if (!event) {
491 mutex_unlock(tsevq_mux);
492 return -ENOMEM;
493 }
494
495 spin_lock_irqsave(&queue->lock, flags);
496
497 qcnt = queue_cnt(queue);
498
499 if (cnt > qcnt)
500 cnt = qcnt;
501
502 for (i = 0; i < cnt; i++) {
503 event[i] = queue->buf[queue->head];
504 queue->head = (queue->head + 1) % PTP_MAX_TIMESTAMPS;
505 }
506
507 spin_unlock_irqrestore(&queue->lock, flags);
508
509 cnt = cnt * sizeof(struct ptp_extts_event);
510
511 mutex_unlock(tsevq_mux);
512
513 result = cnt;
514 if (copy_to_user(buf, event, cnt))
515 result = -EFAULT;
516
517 kfree(event);
518 return result;
519 }
520
521 ssize_t ptp_read(struct posix_clock *pc, uint rdflags, char __user *buf,
522 size_t cnt)
523 {
524 struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
525
526 return ptp_queue_read(ptp, buf, cnt, DMTSC_NOT);
527 }
528
529 static int ptp_dmtsc_open(struct inode *inode, struct file *file)
530 {
531 struct ptp_dmtsc_cdev_info *cdev = container_of(
532 inode->i_cdev, struct ptp_dmtsc_cdev_info, dmtsc_cdev);
533
534 file->private_data = cdev;
535
536 if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
537 return -ERESTARTSYS;
538 cdev->pclock->dmtsc_en_flags |= (0x1 << (cdev->minor));
539 mutex_unlock(&cdev->pclock->dmtsc_en_mux);
540
541 return stream_open(inode, file);
542 }
543
> 544 int ptp_dmtsc_release(struct inode *inode, struct file *file)
545 {
546 struct ptp_dmtsc_cdev_info *cdev = file->private_data;
547
548 if (mutex_lock_interruptible(&cdev->pclock->dmtsc_en_mux))
549 return -ERESTARTSYS;
550 cdev->pclock->dmtsc_en_flags &= ~(0x1 << (cdev->minor));
551 mutex_unlock(&cdev->pclock->dmtsc_en_mux);
552
553 return 0;
554 }
555
> 556 ssize_t ptp_dmtsc_read(struct file *file, char __user *buf, size_t cnt,
557 loff_t *offset)
558 {
559 struct ptp_dmtsc_cdev_info *cdev = file->private_data;
560
561 return ptp_queue_read(cdev->pclock, buf, cnt, cdev->minor);
562 }
563
564 static const struct file_operations fops = {
565 .owner = THIS_MODULE,
566 .open = ptp_dmtsc_open,
567 .read = ptp_dmtsc_read,
568 .release = ptp_dmtsc_release
569 };
570
> 571 void ptp_dmtsc_cdev_clean(struct ptp_clock *ptp)
572 {
573 int idx, major;
574 dev_t device;
575
576 major = MAJOR(ptp->dmtsc_devs.devid);
577 for (idx = 0; idx < ptp->info->n_ext_ts; idx++) {
578 if (ptp->dmtsc_devs.cdev_info[idx].minor >= 0) {
579 device = MKDEV(major, idx);
580 device_destroy(ptp->dmtsc_devs.dmtsc_class, device);
581 cdev_del(&ptp->dmtsc_devs.cdev_info[idx].dmtsc_cdev);
582 ptp->dmtsc_devs.cdev_info[idx].minor = -1;
583 }
584 }
585 class_destroy(ptp->dmtsc_devs.dmtsc_class);
586 unregister_chrdev_region(ptp->dmtsc_devs.devid, ptp->info->n_ext_ts);
587 mutex_destroy(&ptp->dmtsc_devs.cdev_info[idx].tsevq_mux);
588 }
589
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
* (no subject)
2023-08-30 22:01 ` Richard Cochran
@ 2023-09-06 10:47 ` Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 1/3] ptp: Replace timestamp event queue with linked list Xabier Marquiegui
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Xabier Marquiegui @ 2023-09-06 10:47 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
Thank you for your patience Richard, and thank you for guiding me. Let's
see if I got closer to what we want this time.
Let me know what you thing about these patches, please.
Thanks,
Xabier
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/3] ptp: Replace timestamp event queue with linked list
2023-09-06 10:47 ` Xabier Marquiegui
@ 2023-09-06 10:47 ` Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 2/3] ptp: support multiple timestamp event readers Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 3/3] ptp: support event queue reader channel masks Xabier Marquiegui
2 siblings, 0 replies; 26+ messages in thread
From: Xabier Marquiegui @ 2023-09-06 10:47 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
This is the first of a set of patches to introduce linked lists to the
timestamp event queue. The final goal is to be able to have multiple
readers for the timestamp queue.
On this one we maintain the original feature set, and we just introduce
the linked lists to the data structure.
Signed-off-by: Xabier Marquiegui <reibax@gmail.com>
---
drivers/ptp/ptp_chardev.c | 18 ++++++++++++++++--
drivers/ptp/ptp_clock.c | 30 ++++++++++++++++++++++++++++--
drivers/ptp/ptp_private.h | 4 +++-
drivers/ptp/ptp_sysfs.c | 6 +++++-
4 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 362bf756e6b7..1ea11f864abb 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -435,10 +435,17 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
__poll_t ptp_poll(struct posix_clock *pc, struct file *fp, poll_table *wait)
{
struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+ struct timestamp_event_queue *queue;
poll_wait(fp, &ptp->tsev_wq, wait);
- return queue_cnt(&ptp->tsevq) ? EPOLLIN : 0;
+ /*
+ * Extract only the first element in the queue list
+ * TODO: Identify the relevant queue
+ */
+ queue = list_entry(&ptp->tsevqs, struct timestamp_event_queue, qlist);
+
+ return queue_cnt(queue) ? EPOLLIN : 0;
}
#define EXTTS_BUFSIZE (PTP_BUF_TIMESTAMPS * sizeof(struct ptp_extts_event))
@@ -447,12 +454,19 @@ ssize_t ptp_read(struct posix_clock *pc,
uint rdflags, char __user *buf, size_t cnt)
{
struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
- struct timestamp_event_queue *queue = &ptp->tsevq;
+ struct timestamp_event_queue *queue;
struct ptp_extts_event *event;
unsigned long flags;
size_t qcnt, i;
int result;
+ /*
+ * Extract only the first element in the queue list
+ * TODO: Identify the relevant queue
+ */
+ queue = list_first_entry(&ptp->tsevqs, struct timestamp_event_queue,
+ qlist);
+
if (cnt % sizeof(struct ptp_extts_event) != 0)
return -EINVAL;
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 80f74e38c2da..dd48b9f41535 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -166,6 +166,18 @@ static struct posix_clock_operations ptp_clock_ops = {
.read = ptp_read,
};
+static void ptp_clean_queue_list(struct ptp_clock *ptp)
+{
+ struct list_head *pos;
+ struct timestamp_event_queue *element;
+
+ list_for_each(pos, &ptp->tsevqs) {
+ element = list_entry(pos, struct timestamp_event_queue, qlist);
+ list_del(pos);
+ kfree(element);
+ }
+}
+
static void ptp_clock_release(struct device *dev)
{
struct ptp_clock *ptp = container_of(dev, struct ptp_clock, dev);
@@ -175,6 +187,7 @@ static void ptp_clock_release(struct device *dev)
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
+ ptp_clean_queue_list(ptp);
ida_free(&ptp_clocks_map, ptp->index);
kfree(ptp);
}
@@ -206,6 +219,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
struct device *parent)
{
struct ptp_clock *ptp;
+ struct timestamp_event_queue *queue = NULL;
int err = 0, index, major = MAJOR(ptp_devt);
size_t size;
@@ -228,7 +242,13 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
ptp->info = info;
ptp->devid = MKDEV(major, index);
ptp->index = index;
- spin_lock_init(&ptp->tsevq.lock);
+ INIT_LIST_HEAD(&ptp->tsevqs);
+ queue = kzalloc(sizeof(struct timestamp_event_queue), GFP_KERNEL);
+ if (queue == NULL)
+ goto no_memory_queue;
+ spin_lock_init(&queue->lock);
+ list_add_tail(&queue->qlist, &ptp->tsevqs);
+ /* TODO - Transform or delete this mutex */
mutex_init(&ptp->tsevq_mux);
mutex_init(&ptp->pincfg_mux);
mutex_init(&ptp->n_vclocks_mux);
@@ -333,6 +353,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
+ ptp_clean_queue_list(ptp);
+no_memory_queue:
ida_free(&ptp_clocks_map, index);
no_slot:
kfree(ptp);
@@ -376,6 +398,7 @@ EXPORT_SYMBOL(ptp_clock_unregister);
void ptp_clock_event(struct ptp_clock *ptp, struct ptp_clock_event *event)
{
struct pps_event_time evt;
+ struct timestamp_event_queue *tsevq, *tsevq_alt;
switch (event->type) {
@@ -383,7 +406,10 @@ void ptp_clock_event(struct ptp_clock *ptp, struct ptp_clock_event *event)
break;
case PTP_CLOCK_EXTTS:
- enqueue_external_timestamp(&ptp->tsevq, event);
+ /* Enqueue timestamp on all other queues */
+ list_for_each_entry_safe(tsevq, tsevq_alt, &ptp->tsevqs, qlist) {
+ enqueue_external_timestamp(tsevq, event);
+ }
wake_up_interruptible(&ptp->tsev_wq);
break;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 75f58fc468a7..014293255677 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -15,6 +15,7 @@
#include <linux/ptp_clock.h>
#include <linux/ptp_clock_kernel.h>
#include <linux/time.h>
+#include <linux/list.h>
#define PTP_MAX_TIMESTAMPS 128
#define PTP_BUF_TIMESTAMPS 30
@@ -25,6 +26,7 @@ struct timestamp_event_queue {
int head;
int tail;
spinlock_t lock;
+ struct list_head qlist; /* Link to other queues */
};
struct ptp_clock {
@@ -35,7 +37,7 @@ struct ptp_clock {
int index; /* index into clocks.map */
struct pps_device *pps_source;
long dialed_frequency; /* remembers the frequency adjustment */
- struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
+ struct list_head tsevqs; /* timestamp fifo list */
struct mutex tsevq_mux; /* one process at a time reading the fifo */
struct mutex pincfg_mux; /* protect concurrent info->pin_config access */
wait_queue_head_t tsev_wq;
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 6e4d5456a885..2675f383cd0a 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -75,12 +75,16 @@ static ssize_t extts_fifo_show(struct device *dev,
struct device_attribute *attr, char *page)
{
struct ptp_clock *ptp = dev_get_drvdata(dev);
- struct timestamp_event_queue *queue = &ptp->tsevq;
+ struct timestamp_event_queue *queue;
struct ptp_extts_event event;
unsigned long flags;
size_t qcnt;
int cnt = 0;
+ /* The sysfs fifo will always draw from the fist queue */
+ queue = list_first_entry(&ptp->tsevqs, struct timestamp_event_queue,
+ qlist);
+
memset(&event, 0, sizeof(event));
if (mutex_lock_interruptible(&ptp->tsevq_mux))
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/3] ptp: support multiple timestamp event readers
2023-09-06 10:47 ` Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 1/3] ptp: Replace timestamp event queue with linked list Xabier Marquiegui
@ 2023-09-06 10:47 ` Xabier Marquiegui
2023-09-06 18:13 ` Simon Horman
2023-09-06 22:13 ` kernel test robot
2023-09-06 10:47 ` [PATCH 3/3] ptp: support event queue reader channel masks Xabier Marquiegui
2 siblings, 2 replies; 26+ messages in thread
From: Xabier Marquiegui @ 2023-09-06 10:47 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
Use linked lists to create one event queue per open file. This enables
simultaneous readers for timestamp event queues.
Signed-off-by: Xabier Marquiegui <reibax@gmail.com>
---
drivers/ptp/ptp_chardev.c | 95 ++++++++++++++++++++++++++++++---------
drivers/ptp/ptp_clock.c | 6 +--
drivers/ptp/ptp_private.h | 4 +-
drivers/ptp/ptp_sysfs.c | 4 --
4 files changed, 80 insertions(+), 29 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 1ea11f864abb..c65dc6fefaa6 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -103,9 +103,39 @@ int ptp_set_pinfunc(struct ptp_clock *ptp, unsigned int pin,
int ptp_open(struct posix_clock *pc, fmode_t fmode)
{
+ struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+ struct timestamp_event_queue *queue;
+
+ queue = kzalloc(sizeof(struct timestamp_event_queue), GFP_KERNEL);
+ if (queue == NULL)
+ return -EINVAL;
+ queue->reader_pid = task_pid_nr(current);
+ list_add_tail(&queue->qlist, &ptp->tsevqs);
+
return 0;
}
+int ptp_release(struct posix_clock *pc)
+{
+ struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+ struct list_head *pos, *n;
+ struct timestamp_event_queue *element;
+ int found = -1;
+ pid_t reader_pid = task_pid_nr(current);
+
+ list_for_each_safe(pos, n, &ptp->tsevqs) {
+ element = list_entry(pos, struct timestamp_event_queue, qlist);
+ if (element->reader_pid == reader_pid) {
+ list_del(pos);
+ kfree(element);
+ found = 0;
+ return found;
+ }
+ }
+
+ return found;
+}
+
long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
{
struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
@@ -436,14 +466,25 @@ __poll_t ptp_poll(struct posix_clock *pc, struct file *fp, poll_table *wait)
{
struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
struct timestamp_event_queue *queue;
+ struct list_head *pos, *n;
+ bool found = false;
+ pid_t reader_pid = task_pid_nr(current);
poll_wait(fp, &ptp->tsev_wq, wait);
/*
- * Extract only the first element in the queue list
- * TODO: Identify the relevant queue
+ * Extract only the desired element in the queue list
*/
- queue = list_entry(&ptp->tsevqs, struct timestamp_event_queue, qlist);
+ list_for_each_safe(pos, n, &ptp->tsevqs) {
+ queue = list_entry(pos, struct timestamp_event_queue, qlist);
+ if (queue->reader_pid == reader_pid) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ return -EINVAL;
return queue_cnt(queue) ? EPOLLIN : 0;
}
@@ -459,40 +500,50 @@ ssize_t ptp_read(struct posix_clock *pc,
unsigned long flags;
size_t qcnt, i;
int result;
+ struct list_head *pos, *n;
+ bool found = false;
+ pid_t reader_pid = task_pid_nr(current);
/*
- * Extract only the first element in the queue list
- * TODO: Identify the relevant queue
+ * Extract only the desired element in the queue list
*/
- queue = list_first_entry(&ptp->tsevqs, struct timestamp_event_queue,
- qlist);
+ list_for_each_safe(pos, n, &ptp->tsevqs) {
+ queue = list_entry(pos, struct timestamp_event_queue, qlist);
+ if (queue->reader_pid == reader_pid) {
+ found = true;
+ break;
+ }
+ }
- if (cnt % sizeof(struct ptp_extts_event) != 0)
- return -EINVAL;
+ if (!found) {
+ result = -EINVAL;
+ goto exit;
+ }
+
+ if (cnt % sizeof(struct ptp_extts_event) != 0) {
+ result = -EINVAL;
+ goto exit;
+ }
if (cnt > EXTTS_BUFSIZE)
cnt = EXTTS_BUFSIZE;
cnt = cnt / sizeof(struct ptp_extts_event);
- if (mutex_lock_interruptible(&ptp->tsevq_mux))
- return -ERESTARTSYS;
-
if (wait_event_interruptible(ptp->tsev_wq,
ptp->defunct || queue_cnt(queue))) {
- mutex_unlock(&ptp->tsevq_mux);
return -ERESTARTSYS;
}
if (ptp->defunct) {
- mutex_unlock(&ptp->tsevq_mux);
- return -ENODEV;
+ result = -ENODEV;
+ goto exit;
}
event = kmalloc(EXTTS_BUFSIZE, GFP_KERNEL);
if (!event) {
- mutex_unlock(&ptp->tsevq_mux);
- return -ENOMEM;
+ result = -ENOMEM;
+ goto exit;
}
spin_lock_irqsave(&queue->lock, flags);
@@ -511,12 +562,16 @@ ssize_t ptp_read(struct posix_clock *pc,
cnt = cnt * sizeof(struct ptp_extts_event);
- mutex_unlock(&ptp->tsevq_mux);
-
result = cnt;
- if (copy_to_user(buf, event, cnt))
+ if (copy_to_user(buf, event, cnt)) {
result = -EFAULT;
+ goto free_event;
+ }
+free_event:
kfree(event);
+exit:
+ if (result < 0)
+ ptp_release(pc);
return result;
}
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index dd48b9f41535..dc2f045cacbd 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -162,6 +162,7 @@ static struct posix_clock_operations ptp_clock_ops = {
.clock_settime = ptp_clock_settime,
.ioctl = ptp_ioctl,
.open = ptp_open,
+ .release = ptp_release,
.poll = ptp_poll,
.read = ptp_read,
};
@@ -184,7 +185,6 @@ static void ptp_clock_release(struct device *dev)
ptp_cleanup_pin_groups(ptp);
kfree(ptp->vclock_index);
- mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
ptp_clean_queue_list(ptp);
@@ -246,10 +246,9 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
queue = kzalloc(sizeof(struct timestamp_event_queue), GFP_KERNEL);
if (queue == NULL)
goto no_memory_queue;
+ queue->reader_pid = 0;
spin_lock_init(&queue->lock);
list_add_tail(&queue->qlist, &ptp->tsevqs);
- /* TODO - Transform or delete this mutex */
- mutex_init(&ptp->tsevq_mux);
mutex_init(&ptp->pincfg_mux);
mutex_init(&ptp->n_vclocks_mux);
init_waitqueue_head(&ptp->tsev_wq);
@@ -350,7 +349,6 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (ptp->kworker)
kthread_destroy_worker(ptp->kworker);
kworker_err:
- mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
ptp_clean_queue_list(ptp);
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 014293255677..56b0c9df188d 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -27,6 +27,7 @@ struct timestamp_event_queue {
int tail;
spinlock_t lock;
struct list_head qlist; /* Link to other queues */
+ pid_t reader_pid;
};
struct ptp_clock {
@@ -38,7 +39,6 @@ struct ptp_clock {
struct pps_device *pps_source;
long dialed_frequency; /* remembers the frequency adjustment */
struct list_head tsevqs; /* timestamp fifo list */
- struct mutex tsevq_mux; /* one process at a time reading the fifo */
struct mutex pincfg_mux; /* protect concurrent info->pin_config access */
wait_queue_head_t tsev_wq;
int defunct; /* tells readers to go away when clock is being removed */
@@ -124,6 +124,8 @@ long ptp_ioctl(struct posix_clock *pc,
int ptp_open(struct posix_clock *pc, fmode_t fmode);
+int ptp_release(struct posix_clock *pc);
+
ssize_t ptp_read(struct posix_clock *pc,
uint flags, char __user *buf, size_t cnt);
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 2675f383cd0a..512b0164ef18 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -87,9 +87,6 @@ static ssize_t extts_fifo_show(struct device *dev,
memset(&event, 0, sizeof(event));
- if (mutex_lock_interruptible(&ptp->tsevq_mux))
- return -ERESTARTSYS;
-
spin_lock_irqsave(&queue->lock, flags);
qcnt = queue_cnt(queue);
if (qcnt) {
@@ -104,7 +101,6 @@ static ssize_t extts_fifo_show(struct device *dev,
cnt = snprintf(page, PAGE_SIZE, "%u %lld %u\n",
event.index, event.t.sec, event.t.nsec);
out:
- mutex_unlock(&ptp->tsevq_mux);
return cnt;
}
static DEVICE_ATTR(fifo, 0444, extts_fifo_show, NULL);
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/3] ptp: support event queue reader channel masks
2023-09-06 10:47 ` Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 1/3] ptp: Replace timestamp event queue with linked list Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 2/3] ptp: support multiple timestamp event readers Xabier Marquiegui
@ 2023-09-06 10:47 ` Xabier Marquiegui
2023-09-06 18:18 ` kernel test robot
2 siblings, 1 reply; 26+ messages in thread
From: Xabier Marquiegui @ 2023-09-06 10:47 UTC (permalink / raw)
To: richardcochran; +Cc: chrony-dev, mlichvar, netdev, ntp-lists, reibax
Implement ioctl to support filtering of external timestamp event queue
channels per reader based on the process PID accessing the timestamp
queue.
Can be tested using testptp test binary. Use lsof to figure out readers
of the DUT.
Signed-off-by: Xabier Marquiegui <reibax@gmail.com>
---
drivers/ptp/ptp_chardev.c | 17 +++++++++++++++++
drivers/ptp/ptp_clock.c | 4 +++-
drivers/ptp/ptp_private.h | 1 +
include/uapi/linux/ptp_clock.h | 7 +++++++
tools/testing/selftests/ptp/testptp.c | 26 +++++++++++++++++++++++++-
5 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index c65dc6fefaa6..72697189ac59 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -109,6 +109,7 @@ int ptp_open(struct posix_clock *pc, fmode_t fmode)
queue = kzalloc(sizeof(struct timestamp_event_queue), GFP_KERNEL);
if (queue == NULL)
return -EINVAL;
+ queue->mask = 0xFFFFFFFF;
queue->reader_pid = task_pid_nr(current);
list_add_tail(&queue->qlist, &ptp->tsevqs);
@@ -139,9 +140,11 @@ int ptp_release(struct posix_clock *pc)
long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
{
struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+ struct timestamp_event_queue *tsevq, *tsevq_alt;
struct ptp_sys_offset_extended *extoff = NULL;
struct ptp_sys_offset_precise precise_offset;
struct system_device_crosststamp xtstamp;
+ struct ptp_tsfilter_request tsfilter_req;
struct ptp_clock_info *ops = ptp->info;
struct ptp_sys_offset *sysoff = NULL;
struct ptp_system_timestamp sts;
@@ -451,6 +454,20 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
mutex_unlock(&ptp->pincfg_mux);
break;
+ case PTP_FILTERTS_REQUEST:
+ if (copy_from_user(&tsfilter_req, (void __user *)arg,
+ sizeof(tsfilter_req))) {
+ err = -EFAULT;
+ break;
+ }
+ list_for_each_entry_safe(tsevq, tsevq_alt, &ptp->tsevqs, qlist) {
+ if (tsevq->reader_pid == tsfilter_req.reader_pid) {
+ tsevq->mask = tsfilter_req.mask;
+ break;
+ }
+ }
+ break;
+
default:
err = -ENOTTY;
break;
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index dc2f045cacbd..360bd5f9d759 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -247,6 +247,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (queue == NULL)
goto no_memory_queue;
queue->reader_pid = 0;
+ queue->mask = 0xFFFFFFFF;
spin_lock_init(&queue->lock);
list_add_tail(&queue->qlist, &ptp->tsevqs);
mutex_init(&ptp->pincfg_mux);
@@ -406,7 +407,8 @@ void ptp_clock_event(struct ptp_clock *ptp, struct ptp_clock_event *event)
case PTP_CLOCK_EXTTS:
/* Enqueue timestamp on all other queues */
list_for_each_entry_safe(tsevq, tsevq_alt, &ptp->tsevqs, qlist) {
- enqueue_external_timestamp(tsevq, event);
+ if (tsevq->mask & (0x1 << event->index))
+ enqueue_external_timestamp(tsevq, event);
}
wake_up_interruptible(&ptp->tsev_wq);
break;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 56b0c9df188d..07f9e6b64e99 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -28,6 +28,7 @@ struct timestamp_event_queue {
spinlock_t lock;
struct list_head qlist; /* Link to other queues */
pid_t reader_pid;
+ int mask;
};
struct ptp_clock {
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 05cc35fc94ac..372d0a1dc6f7 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -105,6 +105,11 @@ struct ptp_extts_request {
unsigned int rsv[2]; /* Reserved for future use. */
};
+struct ptp_tsfilter_request {
+ pid_t reader_pid; /* PID of process reading the timestamp event queue */
+ unsigned int mask; /* Channel mask. LSB = channel 0 */
+};
+
struct ptp_perout_request {
union {
/*
@@ -224,6 +229,8 @@ struct ptp_pin_desc {
_IOWR(PTP_CLK_MAGIC, 17, struct ptp_sys_offset_precise)
#define PTP_SYS_OFFSET_EXTENDED2 \
_IOWR(PTP_CLK_MAGIC, 18, struct ptp_sys_offset_extended)
+#define PTP_FILTERTS_REQUEST \
+ _IOW(PTP_CLK_MAGIC, 19, struct ptp_tsfilter_request)
struct ptp_extts_event {
struct ptp_clock_time t; /* Time event occured. */
diff --git a/tools/testing/selftests/ptp/testptp.c b/tools/testing/selftests/ptp/testptp.c
index c9f6cca4feb4..8c4d40c16cdf 100644
--- a/tools/testing/selftests/ptp/testptp.c
+++ b/tools/testing/selftests/ptp/testptp.c
@@ -121,6 +121,7 @@ static void usage(char *progname)
" -d name device to open\n"
" -e val read 'val' external time stamp events\n"
" -f val adjust the ptp clock frequency by 'val' ppb\n"
+ " -F pid,msk apply ts channel mask to queue open by pid\n"
" -g get the ptp clock time\n"
" -h prints this message\n"
" -i val index for event/trigger\n"
@@ -162,6 +163,7 @@ int main(int argc, char *argv[])
struct ptp_sys_offset *sysoff;
struct ptp_sys_offset_extended *soe;
struct ptp_sys_offset_precise *xts;
+ struct ptp_tsfilter_request tsfilter_req;
char *progname;
unsigned int i;
@@ -194,9 +196,14 @@ int main(int argc, char *argv[])
int64_t pulsewidth = -1;
int64_t perout = -1;
+ tsfilter_req.reader_pid = 0;
+ tsfilter_req.mask = 0xFFFFFFFF;
+
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
- while (EOF != (c = getopt(argc, argv, "cd:e:f:ghH:i:k:lL:n:o:p:P:sSt:T:w:x:Xz"))) {
+ while (EOF !=
+ (c = getopt(argc, argv,
+ "cd:e:f:F:ghH:i:k:lL:n:o:p:P:sSt:T:w:x:Xz"))) {
switch (c) {
case 'c':
capabilities = 1;
@@ -210,6 +217,14 @@ int main(int argc, char *argv[])
case 'f':
adjfreq = atoi(optarg);
break;
+ case 'F':
+ cnt = sscanf(optarg, "%d,%X", &tsfilter_req.reader_pid,
+ &tsfilter_req.mask);
+ if (cnt != 2) {
+ usage(progname);
+ return -1;
+ }
+ break;
case 'g':
gettime = 1;
break;
@@ -604,6 +619,15 @@ int main(int argc, char *argv[])
free(xts);
}
+ if (tsfilter_req.reader_pid != 0) {
+ if (ioctl(fd, PTP_FILTERTS_REQUEST, &tsfilter_req)) {
+ perror("PTP_FILTERTS_REQUEST");
+ } else {
+ printf("Timestamp event queue mask 0x%X applied to reader with PID: %d\n",
+ (int)tsfilter_req.mask, tsfilter_req.reader_pid);
+ }
+ }
+
close(fd);
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] ptp: support multiple timestamp event readers
2023-09-06 10:47 ` [PATCH 2/3] ptp: support multiple timestamp event readers Xabier Marquiegui
@ 2023-09-06 18:13 ` Simon Horman
2023-09-06 22:13 ` kernel test robot
1 sibling, 0 replies; 26+ messages in thread
From: Simon Horman @ 2023-09-06 18:13 UTC (permalink / raw)
To: Xabier Marquiegui; +Cc: richardcochran, chrony-dev, mlichvar, netdev, ntp-lists
On Wed, Sep 06, 2023 at 12:47:53PM +0200, Xabier Marquiegui wrote:
> Use linked lists to create one event queue per open file. This enables
> simultaneous readers for timestamp event queues.
>
> Signed-off-by: Xabier Marquiegui <reibax@gmail.com>
Hi Xabier,
some minor feedback from my side
...
> diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
> index 1ea11f864abb..c65dc6fefaa6 100644
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -103,9 +103,39 @@ int ptp_set_pinfunc(struct ptp_clock *ptp, unsigned int pin,
>
> int ptp_open(struct posix_clock *pc, fmode_t fmode)
> {
> + struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
> + struct timestamp_event_queue *queue;
> +
> + queue = kzalloc(sizeof(struct timestamp_event_queue), GFP_KERNEL);
> + if (queue == NULL)
nit: this could be: if (!queue)
> + return -EINVAL;
> + queue->reader_pid = task_pid_nr(current);
> + list_add_tail(&queue->qlist, &ptp->tsevqs);
> +
> return 0;
> }
...
> @@ -436,14 +466,25 @@ __poll_t ptp_poll(struct posix_clock *pc, struct file *fp, poll_table *wait)
> {
> struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
> struct timestamp_event_queue *queue;
> + struct list_head *pos, *n;
> + bool found = false;
> + pid_t reader_pid = task_pid_nr(current);
>
> poll_wait(fp, &ptp->tsev_wq, wait);
>
> /*
> - * Extract only the first element in the queue list
> - * TODO: Identify the relevant queue
> + * Extract only the desired element in the queue list
> */
> - queue = list_entry(&ptp->tsevqs, struct timestamp_event_queue, qlist);
> + list_for_each_safe(pos, n, &ptp->tsevqs) {
> + queue = list_entry(pos, struct timestamp_event_queue, qlist);
> + if (queue->reader_pid == reader_pid) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found)
> + return -EINVAL;
Sparse complains that the return type for this function is __poll_t,
but here an int is returned. Perhaps returning EPOLLERR is appropriate here?
>
> return queue_cnt(queue) ? EPOLLIN : 0;
> }
...
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] ptp: support event queue reader channel masks
2023-09-06 10:47 ` [PATCH 3/3] ptp: support event queue reader channel masks Xabier Marquiegui
@ 2023-09-06 18:18 ` kernel test robot
0 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2023-09-06 18:18 UTC (permalink / raw)
To: Xabier Marquiegui, richardcochran
Cc: oe-kbuild-all, chrony-dev, mlichvar, netdev, ntp-lists, reibax
Hi Xabier,
kernel test robot noticed the following build errors:
[auto build test ERROR on net/main]
[also build test ERROR on net-next/main linus/master next-20230906]
[cannot apply to shuah-kselftest/next shuah-kselftest/fixes horms-ipvs/master v6.5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Xabier-Marquiegui/ptp-support-multiple-timestamp-event-readers/20230906-194848
base: net/main
patch link: https://lore.kernel.org/r/20230906104754.1324412-4-reibax%40gmail.com
patch subject: [PATCH 3/3] ptp: support event queue reader channel masks
config: i386-randconfig-005-20230906 (https://download.01.org/0day-ci/archive/20230907/202309070203.1o2AVeeS-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230907/202309070203.1o2AVeeS-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309070203.1o2AVeeS-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from <command-line>:32:
>> ./usr/include/linux/ptp_clock.h:109:2: error: unknown type name 'pid_t'
109 | pid_t reader_pid; /* PID of process reading the timestamp event queue */
| ^~~~~
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] ptp: support multiple timestamp event readers
2023-09-06 10:47 ` [PATCH 2/3] ptp: support multiple timestamp event readers Xabier Marquiegui
2023-09-06 18:13 ` Simon Horman
@ 2023-09-06 22:13 ` kernel test robot
1 sibling, 0 replies; 26+ messages in thread
From: kernel test robot @ 2023-09-06 22:13 UTC (permalink / raw)
To: Xabier Marquiegui, richardcochran
Cc: oe-kbuild-all, chrony-dev, mlichvar, netdev, ntp-lists, reibax
Hi Xabier,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
[also build test WARNING on net-next/main linus/master horms-ipvs/master v6.5 next-20230906]
[cannot apply to shuah-kselftest/next shuah-kselftest/fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Xabier-Marquiegui/ptp-support-multiple-timestamp-event-readers/20230906-194848
base: net/main
patch link: https://lore.kernel.org/r/20230906104754.1324412-3-reibax%40gmail.com
patch subject: [PATCH 2/3] ptp: support multiple timestamp event readers
config: i386-randconfig-061-20230906 (https://download.01.org/0day-ci/archive/20230907/202309070650.eysYMNnu-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230907/202309070650.eysYMNnu-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309070650.eysYMNnu-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/ptp/ptp_chardev.c:487:24: sparse: sparse: incorrect type in return expression (different base types) @@ expected restricted __poll_t @@ got int @@
drivers/ptp/ptp_chardev.c:487:24: sparse: expected restricted __poll_t
drivers/ptp/ptp_chardev.c:487:24: sparse: got int
vim +487 drivers/ptp/ptp_chardev.c
464
465 __poll_t ptp_poll(struct posix_clock *pc, struct file *fp, poll_table *wait)
466 {
467 struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
468 struct timestamp_event_queue *queue;
469 struct list_head *pos, *n;
470 bool found = false;
471 pid_t reader_pid = task_pid_nr(current);
472
473 poll_wait(fp, &ptp->tsev_wq, wait);
474
475 /*
476 * Extract only the desired element in the queue list
477 */
478 list_for_each_safe(pos, n, &ptp->tsevqs) {
479 queue = list_entry(pos, struct timestamp_event_queue, qlist);
480 if (queue->reader_pid == reader_pid) {
481 found = true;
482 break;
483 }
484 }
485
486 if (!found)
> 487 return -EINVAL;
488
489 return queue_cnt(queue) ? EPOLLIN : 0;
490 }
491
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2023-09-06 22:14 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <72ac9741-27f5-36a5-f64c-7d81008eebbc@bluematt.me>
[not found] ` <Y+3m/PpzkBN9kxJY@localhost>
2023-02-16 17:54 ` [chrony-dev] Support for Multiple PPS Inputs on single PHC Matt Corallo
2023-02-16 22:54 ` Richard Cochran
2023-02-17 0:58 ` Matt Corallo
2023-02-20 10:08 ` Miroslav Lichvar
2023-02-20 15:24 ` Richard Cochran
2023-02-23 20:56 ` Matt Corallo
2023-02-24 0:19 ` Richard Cochran
2023-02-24 1:18 ` Matt Corallo
2023-02-24 5:07 ` Richard Cochran
2023-08-29 11:47 ` Xabier Marquiegui
2023-08-29 11:47 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
2023-08-29 14:07 ` Richard Cochran
2023-08-29 14:15 ` Richard Cochran
2023-08-30 21:41 ` [chrony-dev] Support for Multiple PPS Inputs on single PHC Xabier Marquiegui
2023-08-30 21:41 ` [PATCH] ptp: Demultiplexed timestamp channels Xabier Marquiegui
2023-08-30 22:01 ` Richard Cochran
2023-09-06 10:47 ` Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 1/3] ptp: Replace timestamp event queue with linked list Xabier Marquiegui
2023-09-06 10:47 ` [PATCH 2/3] ptp: support multiple timestamp event readers Xabier Marquiegui
2023-09-06 18:13 ` Simon Horman
2023-09-06 22:13 ` kernel test robot
2023-09-06 10:47 ` [PATCH 3/3] ptp: support event queue reader channel masks Xabier Marquiegui
2023-09-06 18:18 ` kernel test robot
2023-08-31 0:29 ` [PATCH] ptp: Demultiplexed timestamp channels kernel test robot
2023-08-31 13:28 ` kernel test robot
2023-08-31 16:20 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).