Re: [PATCH] clocksource: dw_apb_timer_of: support timer-based delay

[Jisheng Zhang <jszhang@marvell.com>]

Dear Arnd,

On Mon, 2 Nov 2015 22:56:02 +0100
Arnd Bergmann wrote:

> On Monday 02 November 2015 11:03:34 Jisheng Zhang wrote:
> > On Fri, 30 Oct 2015 13:42:01 +0100 Arnd Bergmann wrote:  
> > > 
> > > This is not ideal from an overall maintenance perspective. We want to
> > > be able to have a kernel with all drivers enabled that gives us the
> > > best behavior on all platforms.
> > > 
> > > The current behavior appears to be that we override all previous
> > > registrations as long as the new one is higher resolution. Is that
> > > the case here? I.e. does the arch timer have a lower resultion than
> > > the dw-apb timer but have some other advantages?  
> > 
> > Take one Marvell Berlin platform for example, the arch timer freq is 25MHZ,
> > whose resolution is lower than the dw apb timer at 100MHZ. But dw apb timer
> > is on the APB bus while arch timer sits in CPU, so I guess the cost of
> > accessing the apb timer is higher than arch timer.   
> 
> Ok, I see.
> 
> > I have a solution for this case: in platforms with arch timer, I can mark
> > the dw apb timer as "disabled" in the dts even though the timer sits there.
> > Then I could make DW_APB_TIMER_BASED_DELAY non-optional but selected by the
> > the ARCH_XYZ. Is this acceptable?  
> 
> That would do the right thing, but doesn't look ideal: The DW_APB timer
> on those platforms is fully functional, and a future Linux version or
> another OS might decide to use both timers for one reason or another.
> 
> I'd be happier with a solution that keeps the DT describing the hardware
> and not the way we expect Linux to use it, and instead has some heuristic
> in the selection of the delay timer. At the moment, we purely base this
> on the frequency, which as you say is suboptimal.
> 
> One possible way to improve this would be to add an optional 'latency'
> property to the DT nodes (or the driver), and use a combination of latency
> and resolution to make the decision.

Got it. Thanks for the suggestions. The 'latency' here seems a 'rating'
similar as the one in clocksource. I will cook a series for review:

patch 1 to make register_current_timer_delay() aware of 'rating'

patch 2 to set rating of arch timer as 400

patch 3 to add timer based delay support to dw_apb_timer whose rating is 300

Thanks a lot,
Jisheng

> A simpler way would be to always prefer the arch timer on ARM if that
> is present, even if another timer has a higher resolution. This should
> be only a few additional lines in register_current_timer_delay(), or
> possibly an additional function argument.
>