[PATCH v3 03/11] clocksource: sp804: add device tree support
Pawel Moll
pawel.moll at arm.com
Wed Mar 13 10:42:37 EDT 2013
On Wed, 2013-03-13 at 14:17 +0000, Rob Herring wrote:
> 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.
I was rather thinking about using the "interrupt-names" property and
naming them explicitly, eg:
interrupt-names = "timint1", "timint2";
interrupts = <1>, <2>;
interrupt-names = "timint1";
interrupts = <1>;
interrupt-names = "timint2";
interrupts = <2>;
interrupt-names = "timintc";
interrupts = <3>;
But now I see that of_amba_device_create() doesn't do anything about it
(platform device would use them as resource name so we could use
platform_get_resource_byname), so I'm not sure any more...
> >
> >> +- 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.
Sorry, wrong wording. I meant to say "even not mentioning the
APBCLK" (because it's required anyway by the primecell binding). VE
SP804 node obviously has it defined:
v2m_timer01: timer at 110000 {
compatible = "arm,sp804", "arm,primecell";
reg = <0x110000 0x1000>;
interrupts = <2>;
clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>;
clock-names = "timclken1", "timclken2", "apb_pclk";
};
> > 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.
I merely pointed out that it's missing from the binding description.
I couldn't agree more that both should not be there in the first place.
Paweł
More information about the linux-arm-kernel
mailing list