From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752234AbbKBV4j (ORCPT ); Mon, 2 Nov 2015 16:56:39 -0500 Received: from mout.kundenserver.de ([212.227.17.10]:58484 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751875AbbKBV4h (ORCPT ); Mon, 2 Nov 2015 16:56:37 -0500 From: Arnd Bergmann To: Jisheng Zhang Cc: Daniel Lezcano , linux-arm-kernel@lists.infradead.org, tglx@linutronix.de, linux-kernel@vger.kernel.org Subject: Re: [PATCH] clocksource: dw_apb_timer_of: support timer-based delay Date: Mon, 02 Nov 2015 22:56:02 +0100 Message-ID: <5909853.Bs72yAP0HH@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20151102110334.049b5439@xhacker> References: <1446193659-1698-1-git-send-email-jszhang@marvell.com> <3950525.pKSJGoymu4@wuerfel> <20151102110334.049b5439@xhacker> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:iAnkw4KVZ630nGoB2CD3XU9wApO5qoY1CXcBo9VMH93YG9AWdU1 8JDL5mQ21Ki8+XB4PHRwEwV4CSZZw/Hw3Xg/N7KV4u/kq4IghYVtm2o6ph/+1t6mTuMD3v8 mcpGir2vyhN8FB0feGFV9q0qBd2iVNXxcM+O4fuId0bhHIh/rRN1JHPNHLAVRf4KnfxAAW2 C1ZK5XoMxtg3pvZnMykQw== X-UI-Out-Filterresults: notjunk:1;V01:K0:uki1S/KIho8=:1dIWFJ849j6KTNeIUWTQYo T34h9TmURepsEyhAKItzpKHgwoWeOaQxXYeZEBK0WI9Zl/HSl/AC7c5hc+8+Mq9QT0Lry5Ohd Ypbag2j6N5P6v9PyMjpWH3ETd0Q0tS2s0Si1n7lxlCK6t96wAXfIIqVEC0/E0dmhxhwSyuUEd WvuTAXdta/4XQiJphWr0my8jQ5MvePkzUhjcSPRSxxR0EBb+l/O0YWK3nKw7Hpue+QrCkAOQO Qf9oMP9WM3VR0vJpG6Z6L1EfkYQggFfq286rqx8FYnYw2BZk419tnMCpo86aQuD9wd8BqWDzY WZAzV6alTZuUbxr04c+nq3cW1+mMzYOkcy7ZZdIVyUzDFuMvO2F4I7l+KzwtuDq4Kiw5ydaWr eFJyQK+pHCEnOfpXIoU36Apcb7Xeo9TaORmIsU6b7hNgeukrQOjh2w5ATjKiRZUSYIIQHAie3 1c1sMEnurxwYcdk9sKj1CK5m/MvsUVhxXSOiXO9gBvbEIqqrkgrVFvqkDAtMBtK6nAqXi1X3g 0wIqglyMRmE75FBvtAhpiCGJi9ILKBhBF70WmVfEr77wdbDJHZis2um7Q+UpyssbM5Lzb7a5I UN5YbUwCV6Wr4NMTBlwWQo/ndTl4F5H1hzh8QAbFVVoZGTjiOhCxtCo/E4RXAHPU8u3KiVYwg nM/GMI7qB4sBx4asfcuUAhjPM4jpj46BvaD2ot5ttVi4Z/oTlpPojZCOtzy3qDVWOkBkqt5JR CAuvJu5WNmDcZoBV Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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. Arnd