From mboxrd@z Thu Jan 1 00:00:00 1970 From: robherring2@gmail.com (Rob Herring) Date: Wed, 13 Mar 2013 09:17:21 -0500 Subject: [PATCH v3 03/11] clocksource: sp804: add device tree support In-Reply-To: <1363172730.3100.17.camel@hornet> References: <1363151142-32162-1-git-send-email-haojian.zhuang@linaro.org> <1363151142-32162-4-git-send-email-haojian.zhuang@linaro.org> <1363172730.3100.17.camel@hornet> Message-ID: <51408A71.9090501@gmail.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 03/13/2013 06:05 AM, Pawel Moll wrote: > On Wed, 2013-03-13 at 05:05 +0000, Haojian Zhuang wrote: >> +++ b/Documentation/devicetree/bindings/timer/arm,sp804.txt >> @@ -0,0 +1,27 @@ >> +ARM sp804 Dual Timers >> +--------------------------------------- >> + >> +Required properties: >> +- compatible: Should be "arm,sp804" & "arm,primecell" >> +- interrupts: Should contain the list of Dual Timer interrupts >> + interrupts = <0 0 4>, <0 1 4>; > > SP804 has three interrupt outputs: TIMINT1 (interrupt generated by the > first timer), TIMINT2 (the second timer) and TIMINTC (combined - logical > sum of the two former ones). You may want to describe this somehow to > make the choice possible later (eg. VE has the combined interrupt wired > while - if I understand what Rob is saying correctly - Highbank uses > only the TIMINT1). How about: 1 irq - TIMINT1 2 irqs w/ same source # - TIMINTC 2 irqs w/ different source # - TIMINT1 and TIMINT2 I'm not completely sure if Linux and the irq domain code handles the same interrupt source repeated. It should because that is basically a shared irq line. If we ever see only TIMINT2 connected we can add a property for that, but I think that case is unlikely. > >> +- reg: Should contain location and length for dual timer register. >> +- clocks: clock driving the dual timer hardware >> + clocks = <&timclk0 &timclk1>; > > Again, there are three "clock" inputs (even ignoring the APBCLK): We should not ignore that clock as we may want this to be a proper amba_device someday. > TIMCLK, TIMCLKEN1, TIMCLKEN2. The real clocking rate for the timer > depends on the TIMCLK and respective TIMCLKENx - see > http://infocenter.arm.com/help/topic/com.arm.doc.ddi0271d/CJABHCIG.html > > The driver than could make educated choice based on this information. > > You may choose to ignore this fact (and require only a clock > representing the effective rate). I did it for the VE DTS, but it still > doesn't seem completely right > >> +Optional properties: >> +- arm,sp804-clocksource: Should contain the register offset of TIMER1 or >> + TIMER2 in Dual Timer Controller. >> + arm,sp804-clocksource = <0x20>; > > You seem to be missing the "arm,sp804-clockevent" one here. These should stay as aliases or go away as I suggested. Rob