linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: richardcochran@gmail.com (Richard Cochran)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] misc: Add initial Digital Timing Engine (DTE) driver for cygnus
Date: Wed, 13 May 2015 17:35:44 +0200	[thread overview]
Message-ID: <20150513153544.GC2065@localhost.localdomain> (raw)
In-Reply-To: <554D1649.2030106@broadcom.com>

On Fri, May 08, 2015 at 01:02:17PM -0700, Jonathan Richardson wrote:
> For the clock functions I think we can use the existing framework
> unchanged with one exception: ptp_clock_adjtime() doesn't allow negative
> time adjustments and we would like to allow this.

???

/**
 * struct ptp_clock_info - decribes a PTP hardware clock
   ...

 * @adjtime:  Shifts the time of the hardware clock.
 *            parameter delta: Desired change in nanoseconds.
   ...

	int (*adjtime)(struct ptp_clock_info *ptp, s64 delta);

That s64 is 's' as in "signed".
 
> IRQ interval: I mentioned before that we may be able to calculate the
> isochronous interrupt rate automatically but this isn't possible because
> the DTE doesn't know the frequency of the clients. One solution is to
> use the 'PTP_PEROUT_REQUEST' ioctl to set a periodic timer frequency.
> Not really a timer but good enough for our purposes.

As I said in my other reply, I don't understand what the problem is.
 
> Set divider: There is no ability to set a frequency or do anything to a
> channel. We could re-use the PTP_EXTTS_REQUEST ioctl but extend 'struct
> ptp_extts_request' by using the reserved field rsv to allow setting an
> integer value representing either a frequency or divider value in our
> case - some value to configure a channel. A new flag would have to be
> added to the existing PTP_ENABLE_FEATURE, RISING and FALLING EDGE.

I don't get this, either.
 
> Get timestamp: This is a bit more complicated. Currently the PTP driver
> does list management for timestamps from external timestamp channels.
> Timestamps from all channels go into the same list. In our driver we
> have a s/w FIFO for each client and it closely matches the h/w FIFO and
> handles any overflow. We would like to keep it this way because it also
> allows multiple user space processes to only fetch timestamps for the
> client it's handling. 

But having many readers is less efficient and more complex.

Also, we can adjust the buffer if needed to prevent HW FIFO overflows.

> We could add a new ioctl to get a timestamp from
> the driver instead of doing it through ptp_read() but it would be nice
> if we could let ptp_read() allow the driver to do timestamp management
> instead of PTP. Maybe provide an option to obtain the timestamps from a
> container in the driver instead of the one managed by PTP. I like being
> able to use read/poll to obtain data instead of polling the kernel with
> ioctls as we are currently doing.

The PTP interface supports poll/read just fine already.

> Also, avoiding the kmalloc in ptp_read
> would be nice because this of the frequency it would be called at. Do
> you have any preference on how to handle this?

Originally I had the buffer on the stack, but DaveM didn't like it,
saying performance is no excuse for not doing it "the right way".

Thanks,
Richard

  parent reply	other threads:[~2015-05-13 15:35 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-22 23:22 [PATCH 0/2] Add DTE driver for Cygnus Jonathan Richardson
2015-04-22 23:22 ` [PATCH 1/2] misc: Add DT binding for cygnus Digital Timing Engine (DTE) driver Jonathan Richardson
2015-04-22 23:22 ` [PATCH 2/2] misc: Add initial Digital Timing Engine (DTE) driver for cygnus Jonathan Richardson
2015-04-23  8:04   ` Arnd Bergmann
2015-04-23 18:07     ` Jonathan Richardson
2015-05-01 19:01     ` Jonathan Richardson
2015-05-01 19:30       ` One Thousand Gnomes
2015-05-01 19:40         ` Arnd Bergmann
2015-05-08 20:02           ` Jonathan Richardson
2015-05-13  1:03             ` Jonathan Richardson
2015-05-13 12:19             ` Arnd Bergmann
2015-05-13 14:37               ` Richard Cochran
2015-05-13 14:51                 ` Richard Cochran
2015-05-13 15:35             ` Richard Cochran [this message]
2015-05-13 19:50               ` Jonathan Richardson
2015-05-13 20:27                 ` Richard Cochran
2015-05-13 23:25                   ` Jonathan Richardson
2015-05-14 11:30                     ` Richard Cochran
2015-05-20 23:38                       ` Jonathan Richardson
2015-05-21  6:33                         ` Richard Cochran
2015-05-21 17:48                           ` Jonathan Richardson
2015-05-13 15:21       ` Richard Cochran
2015-05-13 19:38         ` Jonathan Richardson
2015-05-13 19:42           ` Richard Cochran
2015-05-13 19:39         ` Jonathan Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150513153544.GC2065@localhost.localdomain \
    --to=richardcochran@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).