[net-next,v3,03/11] net: ethernet: ti: cpts: move tc mult update in cpts_fifo_read()
diff mbox series

Message ID 20200320194244.4703-4-grygorii.strashko@ti.com
State New
Headers show
Series
  • net: ethernet: ti: cpts: add irq and HW_TS_PUSH events
Related show

Commit Message

Grygorii Strashko March 20, 2020, 7:42 p.m. UTC
Now CPTS driver .adjfreq() generates request to read CPTS current time
(CPTS_EV_PUSH) with intention to process all pending event using previous
frequency adjustment values before switching to the new ones. So
CPTS_EV_PUSH works as a marker to switch to the new frequency adjustment
values. Current code assumes that all job is done in .adjfreq(), but after
enabling IRQ this will not be true any more.

Hence save new frequency adjustment values (mult) and perform actual freq
adjustment in cpts_fifo_read() immediately after CPTS_EV_PUSH is received.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpts.c | 8 ++++++--
 drivers/net/ethernet/ti/cpts.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

Comments

Richard Cochran March 26, 2020, 2:20 p.m. UTC | #1
On Fri, Mar 20, 2020 at 09:42:36PM +0200, Grygorii Strashko wrote:
> Now CPTS driver .adjfreq() generates request to read CPTS current time
> (CPTS_EV_PUSH) with intention to process all pending event using previous
> frequency adjustment values before switching to the new ones. So
> CPTS_EV_PUSH works as a marker to switch to the new frequency adjustment
> values. Current code assumes that all job is done in .adjfreq(), but after
> enabling IRQ this will not be true any more.
> 
> Hence save new frequency adjustment values (mult) and perform actual freq
> adjustment in cpts_fifo_read() immediately after CPTS_EV_PUSH is received.

Now THIS comment is much better!  The explanation here really should
be in the previous patch, to help poor reviewers like me.

Thanks,
Richard
Grygorii Strashko March 26, 2020, 8:18 p.m. UTC | #2
On 26/03/2020 16:20, Richard Cochran wrote:
> On Fri, Mar 20, 2020 at 09:42:36PM +0200, Grygorii Strashko wrote:
>> Now CPTS driver .adjfreq() generates request to read CPTS current time
>> (CPTS_EV_PUSH) with intention to process all pending event using previous
>> frequency adjustment values before switching to the new ones. So
>> CPTS_EV_PUSH works as a marker to switch to the new frequency adjustment
>> values. Current code assumes that all job is done in .adjfreq(), but after
>> enabling IRQ this will not be true any more.
>>
>> Hence save new frequency adjustment values (mult) and perform actual freq
>> adjustment in cpts_fifo_read() immediately after CPTS_EV_PUSH is received.
> 
> Now THIS comment is much better!  The explanation here really should
> be in the previous patch, to help poor reviewers like me.

I've been thinking to squash them. What's your opinion.

Thank you.
Richard Cochran March 27, 2020, 1:26 a.m. UTC | #3
On Thu, Mar 26, 2020 at 10:18:18PM +0200, Grygorii Strashko wrote:
> I've been thinking to squash them. What's your opinion.

I favor small, incremental patches.  Just the motivation for the first
patch was missing, that's all.

Thanks,
Richard

Patch
diff mbox series

diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 6a1844cd23ff..e6a8ccae711c 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -165,6 +165,10 @@  static int cpts_fifo_read(struct cpts *cpts, int match)
 		case CPTS_EV_PUSH:
 			WRITE_ONCE(cpts->cur_timestamp, lo);
 			timecounter_read(&cpts->tc);
+			if (cpts->mult_new) {
+				cpts->cc.mult = cpts->mult_new;
+				cpts->mult_new = 0;
+			}
 			break;
 		case CPTS_EV_TX:
 			if (cpts_match_tx_ts(cpts, event)) {
@@ -228,9 +232,9 @@  static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 
 	spin_lock_irqsave(&cpts->lock, flags);
 
-	cpts_update_cur_time(cpts, CPTS_EV_PUSH);
+	cpts->mult_new = neg_adj ? mult - diff : mult + diff;
 
-	cpts->cc.mult = neg_adj ? mult - diff : mult + diff;
+	cpts_update_cur_time(cpts, CPTS_EV_PUSH);
 
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index 32ecd1ce4d3b..421630049ee7 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -116,6 +116,7 @@  struct cpts {
 	unsigned long ov_check_period;
 	struct sk_buff_head txq;
 	u64 cur_timestamp;
+	u32 mult_new;
 };
 
 void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);