Hi Richard,

On Tue, Feb 25, 2020 at 06:47:07PM -0800, Richard Cochran wrote:

On Tue, Feb 25, 2020 at 03:37:07PM -0800, Christopher S. Hall wrote:

quoted

On Sun, Feb 02, 2020 at 08:08:38PM -0800, Richard Cochran wrote:

quoted

The TGPIO input clock, the ART, is a free running counter, but you
want to support frequency adjustments.  Use a timecounter cyclecounter
pair.

I'm concerned about the complexity that the timecounter adds to
the driver. Specifically, the complexity of dealing with any rate mismatches
between the timecounter and the periodic output signal. The phase
error between the output and timecounter needs to be zero.

If I understood correctly, the device's outputs are generated from a
non-adjustable counter.  So, no matter what, you will have the problem
of changing the pulse period in concert with the user changing the
desired frequency.

quoted

This leaves the PHC API behavior as it is currently and uses the frequency
adjust API to adjust the output rate.

quoted

Let the user dial a periodic output signal in the normal way.

Let the user change the frequency in the normal way, and during this
call, adjust the counter values accordingly.

Yes to both of the above.

So, why then do you need this?

+#define PTP_EVENT_COUNT_TSTAMP2 \
+       _IOWR(PTP_CLK_MAGIC, 19, struct ptp_event_count_tstamp)

If you can make the device work with the existing user space API,

	ioctl(fd, PTP_PEROUT_REQUEST2, ...);
	while (1) {
		clock_adjtimex(FD_TO_CLOCKID(fd), ...);
	}

that would be ideal.  But I will push back on anything like the
following.

	ioctl(fd, PTP_PEROUT_REQUEST2, ...);
	while (1) {
		clock_adjtimex(FD_TO_CLOCKID(fd), ...);
		ioctl(fd, PTP_EVENT_COUNT_TSTAMP, ...);
	}

But maybe I misunderstood?

Thank you for the feedback, but Thomas wants to see this as
an extension of GPIO. I'll work on an RFC patch for that instead.

Thanks,
Richard

Thanks,
Christopher