From: Richard Cochran <richardcochran@gmail.com>
To: Mugunthan V N <mugunthanvnm@ti.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net
Subject: Re: [PATCH 5/6] drivers: net: ethernet: cpts: implement cpts hardware clock
Date: Sat, 20 Oct 2012 15:43:31 +0200 [thread overview]
Message-ID: <20121020134331.GA5444@netboy.at.omicron.at> (raw)
In-Reply-To: <1350427518-7230-6-git-send-email-mugunthanvnm@ti.com>
I did a careful read of this driver, and I found a number of issues,
but I am not giving a line by line review, for reasons explained
below. For the most part, your driver is very similar to mine, and
your driver would probably work fine too, when given a bit of review
and revision. However, in the end, it would end up looking nearly
identical to mine.
The issues about which you have voiced concern (counter roll over,
changing input clock frequency) are by no means unique to the CPTS.
The existing PHC drivers all handle these issues in a way consistent
with each other. New drivers should not invent their own solutions to
such problems.
If I had never written a CPTS driver and had seen your present
submission first, then I would be willing to help you get your driver
merged. But in my view, this work is already done, and so I am favoring
my own driver at this point.
Here is my suggestion for moving forward with support the CPTS:
- It is too early to support the CPTS v1 found on the DM81xx, since
the CPSW support (including DT) is totally lacking. Judging from
your present driver and the TRM, the differences are minimal and
will be easy to add later on. For now, we can simply check the
version register for v2.
- If you can get the patches needed to get the CPSW working merged
during the present v3.7-rc interval, that would form a basis for
getting the CPTS into v3.8. I tried to post the patches that I had
collected, but I failed to get them all accepted. I really don't
know the issues surrounding those patches at all, and so I would
appreciate if you would take the lead and get this support merged.
- I will add a version check for CPTS v2 and integrate your CPSW
multicast patch (and anything else you think is missing) into my own
patch series. Once your background patches have appeared, I will
post the CPTS series for net-next, in time for v3.8.
- Looking ahead, once you get a patch series to fully support the
DM81xx CPSW, we can patch the CPTS too.
Thanks,
Richard
next prev parent reply other threads:[~2012-10-20 13:43 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-16 22:45 [PATCH 0/6] Add CPTS PTP driver support Mugunthan V N
2012-10-16 22:45 ` [PATCH 1/6] drivers: net: ethernet: cpsw: add support for CPSW register offset changes in different IP version Mugunthan V N
2012-10-18 2:45 ` Richard Cochran
2012-10-22 10:39 ` N, Mugunthan V
2012-10-22 11:23 ` Richard Cochran
2012-10-22 12:19 ` N, Mugunthan V
2012-10-22 12:25 ` Richard Cochran
2012-10-16 22:45 ` [PATCH 2/6] drivers: net: ethernet: davinci_cpdma: add clear api for statistics interrupt Mugunthan V N
2012-10-18 2:48 ` Richard Cochran
2012-10-22 10:42 ` N, Mugunthan V
2012-10-22 11:31 ` Richard Cochran
2012-10-16 22:45 ` [PATCH 3/6] drivers: net: ethernet: cpsw: add multicast address to ALE table Mugunthan V N
2012-10-18 2:49 ` Richard Cochran
2012-10-21 11:26 ` Richard Cochran
2012-10-22 10:46 ` N, Mugunthan V
2012-10-16 22:45 ` [PATCH 4/6] ptp: add api to get ptp seq id and event type from skb Mugunthan V N
2012-10-16 23:10 ` Ben Hutchings
2012-10-17 13:48 ` N, Mugunthan V
2012-10-18 2:55 ` Richard Cochran
2012-10-22 10:46 ` N, Mugunthan V
2012-10-22 11:36 ` Richard Cochran
2012-10-22 12:32 ` N, Mugunthan V
2012-10-22 12:37 ` Richard Cochran
2012-10-16 22:45 ` [PATCH 5/6] drivers: net: ethernet: cpts: implement cpts hardware clock Mugunthan V N
2012-10-20 13:43 ` Richard Cochran [this message]
2012-10-22 10:55 ` N, Mugunthan V
2012-10-22 11:41 ` Richard Cochran
2012-10-22 11:44 ` Richard Cochran
2012-10-22 12:38 ` N, Mugunthan V
2012-10-22 11:46 ` Richard Cochran
2012-10-22 12:42 ` N, Mugunthan V
2012-10-16 22:45 ` [PATCH 6/6] drivers: net: ethernet: cpsw: implement timestamping capabilities in cpsw Mugunthan V N
2012-10-21 18:11 ` Richard Cochran
2012-10-21 18:46 ` [PATCH 0/6] Add CPTS PTP driver support Richard Cochran
2012-10-22 10:51 ` N, Mugunthan V
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=20121020134331.GA5444@netboy.at.omicron.at \
--to=richardcochran@gmail.com \
--cc=davem@davemloft.net \
--cc=mugunthanvnm@ti.com \
--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 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.