From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755357AbbJHJZB (ORCPT ); Thu, 8 Oct 2015 05:25:01 -0400 Received: from unicorn.mansr.com ([81.2.72.234]:34723 "EHLO unicorn.mansr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755167AbbJHJY6 convert rfc822-to-8bit (ORCPT ); Thu, 8 Oct 2015 05:24:58 -0400 From: =?iso-8859-1?Q?M=E5ns_Rullg=E5rd?= To: Rob Herring Cc: Mark Rutland , Rob Herring , Pawel Moll , Ian Campbell , Kumar Gala , "devicetree\@vger.kernel.org" , "linux-kernel\@vger.kernel.org" Subject: Re: [PATCH v2 1/2] devicetree: add binding for generic mmio clocksource References: <1444232234-2133-1-git-send-email-mans@mansr.com> <20151007154727.GC28981@leverpostej> Date: Thu, 08 Oct 2015 10:24:57 +0100 In-Reply-To: (Rob Herring's message of "Wed, 7 Oct 2015 14:40:53 -0500") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Rob Herring writes: > On Wed, Oct 7, 2015 at 11:47 AM, Måns Rullgård wrote: >> Mark Rutland writes: >> >>> On Wed, Oct 07, 2015 at 04:37:13PM +0100, Mans Rullgard wrote: >>>> This adds a DT binding for a generic mmio clocksource as implemented >>>> by clocksource_mmio_init(). >>>> >>>> Signed-off-by: Mans Rullgard >>>> --- >>>> Changed in v2: >>>> - added sched_clock support >>>> --- >>>> .../devicetree/bindings/timer/clocksource-mmio.txt | 28 ++++++++++++++++++++++ >>>> 1 file changed, 28 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/timer/clocksource-mmio.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/timer/clocksource-mmio.txt b/Documentation/devicetree/bindings/timer/clocksource-mmio.txt >>>> new file mode 100644 >>>> index 0000000..cfb3601 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/timer/clocksource-mmio.txt >>>> @@ -0,0 +1,28 @@ >>>> +Generic MMIO clocksource >>>> + >>>> +Required properties: >>>> + >>>> +- compatible: should be "clocksource-mmio" > > I'm doubtful this matches any h/w. This would assume there is no other > control like enabling, reset, clock dividers, etc. I know it matches real hardware. > Who is your user of this? Various Sigma Designs chips have such counters. I figured others might as well, and it seemed silly to tie such generic functionality to a specific chip family. >> I had a bad feeling about this. How would you suggest determining a >> suitable value from actual hardware parameters? > > I wouldn't. I think there is general agreement that rating is broken. So hardcoding something like 300 would be OK? >>>> +- linux,sched-clock: boolean, register clocksource as sched_clock >>> >>> Likewise, this property doesn't belong in the DT for the same reasons as >>> clocksource-rating. >> >> What would be a proper way to select a sched_clock source? I realise >> it's a Linux-specific thing and DT is supposed to be generic, but the >> information must be provided somehow. > > The kernel already has some logic to do this. Most number of bits > followed by highest frequency will be the winning sched_clock. You > might also want to look at things like always on or not. The problem is that sched_clock_register() doesn't take a pointer to be passed back to the read_sched_clock callback like most interfaces of this type do. This means the callback must use global variables set up before the register call, but at that time there's no way of knowing which one will be used. If there were a way of getting a pointer to the callback, it would be a simple matter of registering all instances and letting the kernel choose which to use. -- Måns Rullgård mans@mansr.com