From mboxrd@z Thu Jan 1 00:00:00 1970 From: Carsten Emde Subject: Re: [PATCH 1/1] Clocksource: Add sched_clock to Atmel TCB clocksource Date: Tue, 02 Jun 2015 23:30:23 +0200 Message-ID: <556E206F.5080600@osadl.org> References: <20150602134631.969367833@osadl.org> <556E1ABC.8090907@osadl.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: Sebastian Andrzej Siewior , RT-users To: Thomas Gleixner Return-path: Received: from toro.web-alm.net ([62.245.132.31]:55658 "EHLO toro.web-alm.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752444AbbFBVah (ORCPT ); Tue, 2 Jun 2015 17:30:37 -0400 In-Reply-To: Sender: linux-rt-users-owner@vger.kernel.org List-ID: Hi Thomas, >>>> static void __iomem *tcaddr; >>>> +static cycle_t (*do_sched_clock_get_cycles)(struct clocksource *cs); >>> >>> what's that for? Indirection for indirection sake? >> The code apparently provides two different functions to read the current clock >> - depending on whether the clock is setup in single-channel mode or not. >> >> The default function is predefined in the struct clocksource clksrc: >> .read = tc_get_cycles >> This structure element may later on be overwritten, if the counter is >> configured in single-channel mode: >> clksrc.read = tc_get_cycles32; >> >> This is why my code sets do_sched_clock_get_cycles to clksrc.read before >> sched_clock is registered to make sure the appropriate read function will be >> used. > return clksrc.read(NULL); > then? Much better, of course, V2 will have it. >>>> + local_irq_save(flags); >>> What's the purpose of this? >> The sched_clock_register() function contains >> WARN_ON(!irqs_disabled()); >> which I did not want to trigger. > That code is run in the early setup with interrupts disabled unless > I'm missing something ... Maybe, something else is wrong, and interrupts are enabled too early on this system. I added the lines around sched_clock_register(), because WARN_ON triggered. Will investigate further. Thanks, -Carsten.