| Message ID | 20161128230428.6872-4-grygorii.strashko@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Mon, Nov 28, 2016 at 05:04:25PM -0600, Grygorii Strashko wrote: > +/* HW TS */ > +static int cpts_extts_enable(struct cpts *cpts, u32 index, int on) > +{ > + unsigned long flags; > + u32 v; > + > + if (index >= cpts->info.n_ext_ts) > + return -ENXIO; > + > + if (((cpts->hw_ts_enable & BIT(index)) >> index) == on) > + return 0; > + > + spin_lock_irqsave(&cpts->lock, flags); > + > + v = cpts_read32(cpts, control); > + if (on) { > + v |= BIT(8 + index); > + cpts->hw_ts_enable |= BIT(index); > + } else { > + v &= ~BIT(8 + index); > + cpts->hw_ts_enable &= ~BIT(index); > + } > + cpts_write32(cpts, v, control); > + > + spin_unlock_irqrestore(&cpts->lock, flags); > + > + if (cpts->hw_ts_enable) > + /* poll for events faster - evry 200 ms */ every > + cpts->ov_check_period = > + msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT); Bad indentation. Use braces {} to contain the comment and assignment statement. > + else > + cpts->ov_check_period = cpts->ov_check_period_slow; > + > + mod_delayed_work(system_wq, &cpts->overflow_work, > + cpts->ov_check_period); > + > + return 0; > +} Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mo, 2016-11-28 at 17:04 -0600, Grygorii Strashko wrote: > This patch adds support of the CPTS HW_TS_PUSH events which are generated > by external low frequency time stamp channels on TI's OMAP CPSW and > Keystone 2 platforms. It supports up to 8 external time stamp channels for > HW_TS_PUSH input pins (the number of supported channel is different for > different SoCs and CPTS versions, check corresponding Data maual before > enabling it). Therefore, new DT property "cpts-ext-ts-inputs" is introduced > for specifying number of available external timestamp channels. If this only depends on SoC and CTPS, it should be possible to derive the correct value from the compatible value and possibly a CPTS version register? If the existing compatible strings are not specific enough, possible a new one should be added. Regards, Jan
On 11/30/2016 05:08 AM, Jan Lübbe wrote: > On Mo, 2016-11-28 at 17:04 -0600, Grygorii Strashko wrote: >> This patch adds support of the CPTS HW_TS_PUSH events which are generated >> by external low frequency time stamp channels on TI's OMAP CPSW and >> Keystone 2 platforms. It supports up to 8 external time stamp channels for >> HW_TS_PUSH input pins (the number of supported channel is different for >> different SoCs and CPTS versions, check corresponding Data maual before >> enabling it). Therefore, new DT property "cpts-ext-ts-inputs" is introduced >> for specifying number of available external timestamp channels. > > If this only depends on SoC and CTPS, it should be possible to derive > the correct value from the compatible value and possibly a CPTS version > register? If the existing compatible strings are not specific enough, > possible a new one should be added. > In general, I can try to add and use new compat strings "ti,netcp-k2hk" "ti,netcp-k2l" "ti,netcp-k2e" "ti,netcp-k2g" for determining CPTS capabilities. CPTS version is not the choice due to very poor documentation which do not allow identify relations between CPTS ver and supported features :( Murali, what do you think?
On Mon, Nov 28, 2016 at 05:04:25PM -0600, Grygorii Strashko wrote: > This also change overflow polling period when HW_TS_PUSH feature is > enabled - overflow check work will be scheduled more often (every > 200ms) for proper HW_TS_PUSH events reporting. For proper reporting, you should make use of the interrupt. The small fifo (16 iirc) could very well overflow in 200 ms. The interrupt handler should read out the entire fifo at each interrupt. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/03/2016 05:21 PM, Richard Cochran wrote: > On Mon, Nov 28, 2016 at 05:04:25PM -0600, Grygorii Strashko wrote: >> This also change overflow polling period when HW_TS_PUSH feature is >> enabled - overflow check work will be scheduled more often (every >> 200ms) for proper HW_TS_PUSH events reporting. > > For proper reporting, you should make use of the interrupt. The small > fifo (16 iirc) could very well overflow in 200 ms. The interrupt > handler should read out the entire fifo at each interrupt. > huh. Seems this is not really good idea, because MISC Irq will be triggered for *any* CPTS event and there is no way to enable it just for HW_TS_PUSH. So, this doesn't work will with current code for RX/TX timestamping (which uses polling mode). + runtime overhead in net RX/TX caused by triggering more interrupts. May be, overflow check/polling timeout can be made configurable (module parameter).
On Thu, Dec 08, 2016 at 01:04:11PM -0600, Grygorii Strashko wrote: > huh. Seems this is not really good idea, because MISC Irq will be > triggered for *any* CPTS event and there is no way to enable it just for > HW_TS_PUSH. So what? That is not a problem. > So, this doesn't work will with current code for RX/TX timestamping > (which uses polling mode). Why doesn't it work? > + runtime overhead in net RX/TX caused by > triggering more interrupts. This is not relevant. Without HW_TS_PUSH, there is no need for enabling the interrupt simply because we don't need it. Now, with HW_TS_PUSH, we do need it. > May be, overflow check/polling timeout can be made configurable (module parameter). No, it should just work without any user space fiddling. I getting a bit tired of your half-baked implementations of the ancillary clock functions. Either do it right, or just leave it unsupported. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt index ebda7c9..86a2f61 100644 --- a/Documentation/devicetree/bindings/net/cpsw.txt +++ b/Documentation/devicetree/bindings/net/cpsw.txt @@ -38,6 +38,10 @@ Optional properties: Mult and shift will be calculated basing on CPTS rftclk frequency if both cpts_clock_shift and cpts_clock_mult properties are not provided. +- cpts-ext-ts-inputs : The number of external time stamp channels. + The different CPTS versions might support up 8 + external time stamp channels. + if absent - unsupported. Slave Properties: Required properties: diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt index ec4a241..1c805319 100644 --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt @@ -123,6 +123,10 @@ Optional properties: Defaults: clock_mult, clock_shift = calculated from CPTS refclk + - cpts-ext-ts-inputs: + The number of external time stamp channels. + The different CPTS versions might support up 8 + external time stamp channels. if absent - unsupported. NetCP interface properties: Interface specification for NetCP sub-modules. Required properties: diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c index 9c5b835..2f7641a 100644 --- a/drivers/net/ethernet/ti/cpts.c +++ b/drivers/net/ethernet/ti/cpts.c @@ -34,6 +34,11 @@ #define cpts_read32(c, r) readl_relaxed(&c->reg->r) #define cpts_write32(c, v, r) writel_relaxed(v, &c->reg->r) +static int cpts_event_port(struct cpts_event *event) +{ + return (event->high >> PORT_NUMBER_SHIFT) & PORT_NUMBER_MASK; +} + static int event_expired(struct cpts_event *event) { return time_after(jiffies, event->tmo); @@ -96,11 +101,15 @@ static int cpts_fifo_read(struct cpts *cpts, int match) } event = list_first_entry(&cpts->pool, struct cpts_event, list); - event->tmo = jiffies + 2; + event->tmo = jiffies + + msecs_to_jiffies(CPTS_EVENT_RX_TX_TIMEOUT); event->high = hi; event->low = lo; type = event_type(event); switch (type) { + case CPTS_EV_HW: + event->tmo += + msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT); case CPTS_EV_PUSH: case CPTS_EV_RX: case CPTS_EV_TX: @@ -109,7 +118,6 @@ static int cpts_fifo_read(struct cpts *cpts, int match) break; case CPTS_EV_ROLL: case CPTS_EV_HALF: - case CPTS_EV_HW: break; default: pr_err("cpts: unknown event type\n"); @@ -218,9 +226,83 @@ static int cpts_ptp_settime(struct ptp_clock_info *ptp, return 0; } +static int cpts_report_ts_events(struct cpts *cpts) +{ + struct list_head *this, *next; + struct ptp_clock_event pevent; + struct cpts_event *event; + int reported = 0, ev; + + list_for_each_safe(this, next, &cpts->events) { + event = list_entry(this, struct cpts_event, list); + ev = event_type(event); + if (ev == CPTS_EV_HW) { + list_del_init(&event->list); + list_add(&event->list, &cpts->pool); + /* report the event */ + pevent.timestamp = + timecounter_cyc2time(&cpts->tc, event->low); + pevent.type = PTP_CLOCK_EXTTS; + pevent.index = cpts_event_port(event) - 1; + ptp_clock_event(cpts->clock, &pevent); + ++reported; + continue; + } + } + return reported; +} + +/* HW TS */ +static int cpts_extts_enable(struct cpts *cpts, u32 index, int on) +{ + unsigned long flags; + u32 v; + + if (index >= cpts->info.n_ext_ts) + return -ENXIO; + + if (((cpts->hw_ts_enable & BIT(index)) >> index) == on) + return 0; + + spin_lock_irqsave(&cpts->lock, flags); + + v = cpts_read32(cpts, control); + if (on) { + v |= BIT(8 + index); + cpts->hw_ts_enable |= BIT(index); + } else { + v &= ~BIT(8 + index); + cpts->hw_ts_enable &= ~BIT(index); + } + cpts_write32(cpts, v, control); + + spin_unlock_irqrestore(&cpts->lock, flags); + + if (cpts->hw_ts_enable) + /* poll for events faster - evry 200 ms */ + cpts->ov_check_period = + msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT); + else + cpts->ov_check_period = cpts->ov_check_period_slow; + + mod_delayed_work(system_wq, &cpts->overflow_work, + cpts->ov_check_period); + + return 0; +} + static int cpts_ptp_enable(struct ptp_clock_info *ptp, struct ptp_clock_request *rq, int on) { + struct cpts *cpts = container_of(ptp, struct cpts, info); + + switch (rq->type) { + case PTP_CLK_REQ_EXTTS: + return cpts_extts_enable(cpts, rq->extts.index, on); + default: + break; + } + return -EOPNOTSUPP; } @@ -240,10 +322,16 @@ static struct ptp_clock_info cpts_info = { static void cpts_overflow_check(struct work_struct *work) { - struct timespec64 ts; struct cpts *cpts = container_of(work, struct cpts, overflow_work.work); + struct timespec64 ts; + unsigned long flags; - cpts_ptp_gettime(&cpts->info, &ts); + spin_lock_irqsave(&cpts->lock, flags); + ts = ns_to_timespec64(timecounter_read(&cpts->tc)); + spin_unlock_irqrestore(&cpts->lock, flags); + + if (cpts->hw_ts_enable) + cpts_report_ts_events(cpts); pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec); schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period); } @@ -422,6 +510,8 @@ static void cpts_calc_mult_shift(struct cpts *cpts) /* Calc overflow check period (maxsec / 2) */ cpts->ov_check_period = (HZ * maxsec) / 2; + cpts->ov_check_period_slow = cpts->ov_check_period; + dev_info(cpts->dev, "cpts: overflow check period %lu (jiffies)\n", cpts->ov_check_period); @@ -468,6 +558,9 @@ static int cpts_of_parse(struct cpts *cpts, struct device_node *node) cpts->rftclk_sel = prop & CPTS_RFTCLK_SEL_MASK; } + if (!of_property_read_u32(node, "cpts-ext-ts-inputs", &prop)) + cpts->ext_ts_inputs = prop; + return 0; of_error: @@ -512,6 +605,9 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs, cpts->cc.mask = CLOCKSOURCE_MASK(32); cpts->info = cpts_info; + if (cpts->ext_ts_inputs) + cpts->info.n_ext_ts = cpts->ext_ts_inputs; + cpts_calc_mult_shift(cpts); return cpts; diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h index c934b61..ad80c95 100644 --- a/drivers/net/ethernet/ti/cpts.h +++ b/drivers/net/ethernet/ti/cpts.h @@ -102,6 +102,9 @@ enum { #define CPTS_FIFO_DEPTH 16 #define CPTS_MAX_EVENTS 32 +#define CPTS_EVENT_RX_TX_TIMEOUT 20 /* ms */ +#define CPTS_EVENT_HWSTAMP_TIMEOUT 200 /* ms */ + struct cpts_event { struct list_head list; unsigned long tmo; @@ -129,7 +132,10 @@ struct cpts { struct list_head pool; struct cpts_event pool_data[CPTS_MAX_EVENTS]; unsigned long ov_check_period; + unsigned long ov_check_period_slow; u32 rftclk_sel; + u32 ext_ts_inputs; + u32 hw_ts_enable; u32 caps; };