From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Stultz Subject: Re: [PATCH 0/2] Fix potential merge conflict for dw_apb_timer_of Date: Tue, 18 Jun 2013 11:40:47 -0700 Message-ID: <51C0A9AF.4030409@linaro.org> References: <1371514129-22801-1-git-send-email-dinguyen@altera.com> <20130618150244.GC18055@amd.pavel.ucw.cz> <201306181738.35671.heiko@sntech.de> <201306182028.50762.heiko@sntech.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <201306182028.50762.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: =?ISO-8859-1?Q?Heiko_St=FCbner?= Cc: dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Pavel Machek , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Jamie Iles , Thomas Gleixner List-Id: devicetree@vger.kernel.org On 06/18/2013 11:28 AM, Heiko St=FCbner wrote: > Am Dienstag, 18. Juni 2013, 17:38:35 schrieb Heiko St=FCbner: >> Hi Pavel, >> >> Am Dienstag, 18. Juni 2013, 17:02:44 schrieb Pavel Machek: >>> Hi! >>> >>>>> The following 2 patches will eliminate the need for the patch in John >>>>> Stultz's tree. If there is to be merge of the 2 trees, then the >>>>> patch: >>>>> >>>>> dw_apb_timer_of.c: Remove parts that were picoxcell-specific >>>>> >>>>> can be removed from John's tree to avoid a merge-conflict. >>>>> >>>>> Based on arm-soc/for-next: >>>>> >>>>> PATCH[1/2] - Rename "dw-apb-timer-osc" and "dw-apb-timer-sp" bindings >>>>> to just "dw-apb-timer" >>>>> PATCH[2/2] - Fix user/system reporting by fixing read_sched_clock() >>>> Pavel/Jamie: Can you take a look at these too and make sure these cover >>>> what you were doing. >>> [It seems like Heiko St=FCbner was not aware of patches in the clock >>> tree, so did pretty much equivalent patch.] >> Correct ... I was going after what was in linux-next and the tip.git [wh= ich >> I also only saw recently at all] does not seem to be part of it. >> >>> Dinh's changes look good to me, but >>> >>> [PATCH v2 4/4] clocksource: dw_apb_timer_of: use clocksource_of_init >>> >>> does not exactly look nice: (I'm sorry I did not see original series, >>> before it was merged to -soc.). The function counts number of times it >>> was called, and behaves differently in each case. It is not very >>> traditional kernel code at the very least. >>> >>> +static int num_called; >>> +static void __init dw_apb_timer_init(struct device_node *timer) >>> >>> { >>> >>> - struct device_node *event_timer, *source_timer; >>> - >>> - event_timer =3D of_find_matching_node(NULL, osctimer_ids); >>> - if (!event_timer) >>> - panic("No timer for clockevent"); >>> - add_clockevent(event_timer); >>> - >>> - source_timer =3D of_find_matching_node(event_timer, >>> osctimer_ids); >>> - if (!source_timer) >>> - panic("No timer for clocksource"); >>> - add_clocksource(source_timer); >>> - >>> - of_node_put(source_timer); >>> + switch (num_called) { >>> + case 0: >>> + pr_debug("%s: found clockevent timer\n", __func__); >>> + add_clockevent(timer); >>> + of_node_put(timer); >>> + break; >>> + case 1: >>> + pr_debug("%s: found clocksource timer\n", __func__); >>> + add_clocksource(timer); >>> + of_node_put(timer); >>> + init_sched_clock(); >>> + break; >>> + default: >>> + break; >>> + } >>> >>> - init_sched_clock(); >>> + num_called++; >>> >>> } >>> >>> Heiko, can you take a look at John Stultz tree? We modified this area >>> already... I understand you only have one timer on your silicon? > also it seems like not being able to use the apb_timer as sched_clock will > hurt my platform too. > > I've tried to use the arm_arch_timer, but when the arch_timer_get_cntfrq() > function gets called, I only get an "undefined instruction" Oops for the > executed asm in there. > > As there is no manual available for the SoC, I can only guess that it doe= sn't > contain such a component. This is fueled additionally by the PPI part of = the > gic only having 3 interrupt sources [there is small excerpt of the soc-ma= nual > floating around that contains this information], with one already being t= he > twd interrupt, while the arm_arch_timer seems to require 4 itself. > > Therefore it would cool, if we could keep the sched_clock functionality > (provided by the clocksource timer) around somehow. So whats the plan now? I'm feeling likely to revert "dw_apb_timer_of.c: Remove parts that were = picoxcell-specific" in my tree and leave the rest of the wreckage to the = arm-soc folks to sort out (either getting the fix in or reverting the = lot and trying again for 3.12), unless we get some agreed upon path = forward sorted quickly here. thanks -john