netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Cochran <richardcochran@gmail.com>
To: "Mintz, Yuval" <Yuval.Mintz@cavium.com>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Kalluru, Sudarsana" <Sudarsana.Kalluru@cavium.com>
Subject: Re: [PATCH net-next v4 1/2] qed: Add infrastructure for PTP support.
Date: Sun, 12 Feb 2017 20:50:27 +0100	[thread overview]
Message-ID: <20170212195027.GA2696@localhost.localdomain> (raw)
In-Reply-To: <BL2PR07MB2306751ABE3FC347E40302B18D460@BL2PR07MB2306.namprd07.prod.outlook.com>

On Sun, Feb 12, 2017 at 03:07:34PM +0000, Mintz, Yuval wrote:
> Your algorithm ignores the HW limitation. Consider (ppb == 1): 
> your logic would output N == 7, *M == 7000000000,
>    Which has perfect accuracy [N / *M is 1 / 10^9].
> But the solution for
>    'period' * 16 + 8 == 7 * 10^9
> isn't a whole number, so this result doesn't really reflect the actual
> approximation error since we couldn't configure it to HW.

Ok, so change my code to read:

		/*truncate to HW resolution*/
		reg = (m - 8) / 16;
		m = reg * 16 + 8;

Your HW will happyly accept the value of 'reg', right?

> The original would return val == 1, period == 62499999; While this
> does have some error [val / (period * 16 + 8) is slightly bigger
> than 1 / 10^9, error at 18[?] digit after dot], it's the best we can
> configure for the HW.

That is *not* the best you can do:

Perfect:  1 / 1000000000 = .000000001
Yours:    1 / 999999992  = .000000001000000008
Mine*:    7 / 6999999992 = .00000000100000000114

[ * revised with the above change ]

Not a huge difference, but yours is not "the best we can".

Let's try another:

ppb = 40831

Perfect:  40831 / 1000000000 = .000040831
Yours:    4 / 97960          = .00004083299305839118
Mine:     5 / 122456         = .00004083099235643823

See the difference?

Please, try the two algorithms and plot the RMS error over the
interval ppb = 1 ... 100000.  The result may surprise you.

> No. In an ideal world, I would have liked optimizing everything.
> But in this world if I do find time to spend on optimizations 
> I rather do that for the stuff that matters. I.e., datapath.

As the PTP maintainer, I look after about the PTP drivers.  They
should be as good as we can make them (even when the HW is a broken as
yours is).  That is why I bothered to review and to spend time
thinking about your problem.  I especially care about having good
examples in the tree, since this stuff will inevitably get copied by
new driver authors.  It is wonderful that your data path is so very
optimized, but that is no excuse for poor PTP code.

Thanks,
Richard

  parent reply	other threads:[~2017-02-12 19:50 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-08  6:43 [PATCH net-next v4 0/2] qed*: Add support for PTP Sudarsana Kalluru
2017-02-08  6:43 ` [PATCH net-next v4 1/2] qed: Add infrastructure for PTP support Sudarsana Kalluru
2017-02-11  8:58   ` Richard Cochran
2017-02-11 11:16     ` Richard Cochran
2017-02-12 11:27       ` Mintz, Yuval
2017-02-12 11:52         ` Mintz, Yuval
2017-02-12 18:47           ` Richard Cochran
2017-02-12 14:02         ` Richard Cochran
2017-02-12 15:07           ` Mintz, Yuval
2017-02-12 15:53             ` Mintz, Yuval
2017-02-12 19:50             ` Richard Cochran [this message]
2017-02-13  9:48               ` FW: " Mintz, Yuval
2017-02-08  6:43 ` [PATCH net-next v4 2/2] qede: Add driver support for PTP Sudarsana Kalluru

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=20170212195027.GA2696@localhost.localdomain \
    --to=richardcochran@gmail.com \
    --cc=Sudarsana.Kalluru@cavium.com \
    --cc=Yuval.Mintz@cavium.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.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).