All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Cochran <richardcochran@gmail.com>
To: Shawn Bohrer <shawn.bohrer@gmail.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>,
	Amir Vadai <amirv@mellanox.com>,
	netdev@vger.kernel.org, tomk@rgmadvisors.com,
	Shawn Bohrer <sbohrer@rgmadvisors.com>
Subject: Re: [PATCH RFC] mlx4_en: Add PTP hardware clock
Date: Wed, 11 Dec 2013 19:54:39 +0100	[thread overview]
Message-ID: <20131211185438.GC4371@netboy> (raw)
In-Reply-To: <1386786265-6322-1-git-send-email-shawn.bohrer@gmail.com>

On Wed, Dec 11, 2013 at 12:24:25PM -0600, Shawn Bohrer wrote:
> From: Shawn Bohrer <sbohrer@rgmadvisors.com>
> 
> This adds a PHC to the mlx4_en driver.  This is largely based off of the
> e1000e driver (drivers/net/ethernet/intel/e1000e/ptp.c) which seemed
> very similar.  One thing I am unsure about is that in the e1000e driver
> they protect the timecounter code with a spinlock because the hardware
> reports the time in two 32bit registers.  The Mellanox code looks
> similar however the existing timecounter code in the mlx4_en driver
> wasn't protected with a lock so I left the lock out here as well.

Before, there were only three call sites,

grep -nH -e timecounter_ *
en_clock.c:108:	nsec = timecounter_cyc2time(&mdev->clock, timestamp);
en_clock.c:131:	timecounter_init(&mdev->clock, &mdev->cycles,
en_clock.c:148:		timecounter_read(&mdev->clock);

and so perhaps the locking was unnecessary (but maybe not).
In any case, the code that you added definitely needs locks.

> Additionally here the mlx4_en_phc_adjfreq() method simply returns
> -EOPNOTSUPP because I don't have the relevant hardware documentation on
> how to do this.  I'm hoping one of the Mellanox developers can either
> provide this documentation or provide a patch to implement that
> function.

Since the code already uses timecounter_read to convert clock ticks
into nanoseconds, why not simply adjust the 'mult' as other drivers
do?

[ If the clock is easily adjustable in hardware, then, by all means,
  do it that way. ]
 
> This is minimally tested at this point but Documentation/ptp/testptp
> appears to work on a Mellanox ConnectX-3 card.

Once you have the adjustment in place, then you can try it with
linuxptp.

Thanks,
Richard

  reply	other threads:[~2013-12-11 18:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-11 18:24 [PATCH RFC] mlx4_en: Add PTP hardware clock Shawn Bohrer
2013-12-11 18:54 ` Richard Cochran [this message]
2013-12-11 21:28   ` Shawn Bohrer
2013-12-12  9:50     ` Hadar Hen Zion
2013-12-12 15:28       ` Shawn Bohrer
2013-12-16 23:34         ` mlx4_en SIOCSHWTSTAMP cycles the link Shawn Bohrer
2013-12-17 11:26           ` Richard Cochran
2013-12-17 19:54             ` Shawn Bohrer
2013-12-17 11:44           ` Richard Cochran
2013-12-17 12:06             ` tedheadster
2013-12-11 20:47 ` [PATCH RFC] mlx4_en: Add PTP hardware clock Or Gerlitz
2013-12-11 21:23   ` Shawn Bohrer
2013-12-11 21:35 ` Ben Hutchings

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=20131211185438.GC4371@netboy \
    --to=richardcochran@gmail.com \
    --cc=amirv@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=ogerlitz@mellanox.com \
    --cc=sbohrer@rgmadvisors.com \
    --cc=shawn.bohrer@gmail.com \
    --cc=tomk@rgmadvisors.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.