Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
David Miller writes: > From: Robert Jarzmik > Date: Fri, 18 Sep 2015 18:36:56 +0200 > >> Which brings me to wonder which is the more correct : >> (a) replace to reproduce the same calculation >> Previously mtt was compared to a difference of 76ns steps (as 307ns / 4 >> = >> 76ns): >> while ((sched_clock() - si->last_clk) * 76 < mtt) >> >> (b) change the calculation assuming mtt is in microseconds : >> while ((sched_clock() - si->last_clk) * 1000 < mtt) >> >> I have no IRDA protocol knowledge so unless someone points me to the correct >> calculation I'll try my luck with (b). > > "a" would be "safer" and less likely to break anything, although as > you say "b" might be more correct. Indeed. Well, let me try (b) then. I'll add in the commit message the timings change, and if anybody complains a regression pops out, I'll provide a patch to fallback to (a). Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
From: Robert Jarzmik Date: Fri, 18 Sep 2015 18:36:56 +0200 > Which brings me to wonder which is the more correct : > (a) replace to reproduce the same calculation > Previously mtt was compared to a difference of 76ns steps (as 307ns / 4 = > 76ns): > while ((sched_clock() - si->last_clk) * 76 < mtt) > > (b) change the calculation assuming mtt is in microseconds : > while ((sched_clock() - si->last_clk) * 1000 < mtt) > > I have no IRDA protocol knowledge so unless someone points me to the correct > calculation I'll try my luck with (b). "a" would be "safer" and less likely to break anything, although as you say "b" might be more correct. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
David Miller writes: >> My understanding is that the flow will be : >> sched_clock() >>rd->read_sched_clock() (cyc_to_ns() transformed for return) >> pxa_read_sched_clock() >>readl_relaxed(OSCR) >> >> I didn't see any timings issue, as the flow looks equivalent to the >> readl(OSCR), >> but I might have overlooked something. > > Of course it's different, because sched_clock() converts the value read > from OSCR into nanoseconds, which is obviously different from using the > OSCR register value directly. > > You're therefore feeding different values into this IRDA code. Ah yes, I see it. Which brings me to wonder which is the more correct : (a) replace to reproduce the same calculation Previously mtt was compared to a difference of 76ns steps (as 307ns / 4 = 76ns): while ((sched_clock() - si->last_clk) * 76 < mtt) (b) change the calculation assuming mtt is in microseconds : while ((sched_clock() - si->last_clk) * 1000 < mtt) I have no IRDA protocol knowledge so unless someone points me to the correct calculation I'll try my luck with (b). Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
From: Robert Jarzmik Date: Wed, 16 Sep 2015 11:34:01 +0200 > David Miller writes: > >> From: Robert Jarzmik >> Date: Sat, 12 Sep 2015 13:45:22 +0200 >> >>> Instead of using directly the OS timer through direct register access, >>> use the standard sched_clock(), which will end up in OSCR reading >>> anyway. >>> >>> This is a first step for direct access register removal and machine >>> specific code removal from this driver. >>> >>> Signed-off-by: Robert Jarzmik >> >> What is the granularity of the OSCR register? > It's 307ns (ie. 3.25MHz clock). > >> If it is not nanoseconds, then you need to adjust calculations >> such as this one: > Tell me if the 307ns requires something I should adjust. > > My understanding is that the flow will be : > sched_clock() >rd->read_sched_clock() (cyc_to_ns() transformed for return) > pxa_read_sched_clock() >readl_relaxed(OSCR) > > I didn't see any timings issue, as the flow looks equivalent to the > readl(OSCR), > but I might have overlooked something. Of course it's different, because sched_clock() converts the value read from OSCR into nanoseconds, which is obviously different from using the OSCR register value directly. You're therefore feeding different values into this IRDA code. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
David Miller writes: > From: Robert Jarzmik > Date: Sat, 12 Sep 2015 13:45:22 +0200 > >> Instead of using directly the OS timer through direct register access, >> use the standard sched_clock(), which will end up in OSCR reading >> anyway. >> >> This is a first step for direct access register removal and machine >> specific code removal from this driver. >> >> Signed-off-by: Robert Jarzmik > > What is the granularity of the OSCR register? It's 307ns (ie. 3.25MHz clock). > If it is not nanoseconds, then you need to adjust calculations > such as this one: Tell me if the 307ns requires something I should adjust. My understanding is that the flow will be : sched_clock() rd->read_sched_clock() (cyc_to_ns() transformed for return) pxa_read_sched_clock() readl_relaxed(OSCR) I didn't see any timings issue, as the flow looks equivalent to the readl(OSCR), but I might have overlooked something. Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
From: Robert Jarzmik Date: Sat, 12 Sep 2015 13:45:22 +0200 > Instead of using directly the OS timer through direct register access, > use the standard sched_clock(), which will end up in OSCR reading > anyway. > > This is a first step for direct access register removal and machine > specific code removal from this driver. > > Signed-off-by: Robert Jarzmik What is the granularity of the OSCR register? If it is not nanoseconds, then you need to adjust calculations such as this one: > @@ -549,7 +548,7 @@ static int pxa_irda_hard_xmit(struct sk_buff *skb, struct > net_device *dev) > skb_copy_from_linear_data(skb, si->dma_tx_buff, skb->len); > > if (mtt) > - while ((unsigned)(readl_relaxed(OSCR) - > si->last_oscr)/4 < mtt) > + while ((sched_clock() - si->last_clk) / 4 < mtt) > cpu_relax(); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
