| Message ID | 1363151142-32162-7-git-send-email-haojian.zhuang@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2013-03-13 at 05:05 +0000, Haojian Zhuang wrote: > Remove all code to parse sp804. Use clocksource_of_init() instead since > all these code are implemented in sp804 driver already. > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > --- > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 2 ++ > arch/arm/boot/dts/vexpress-v2m.dtsi | 2 ++ > arch/arm/boot/dts/vexpress-v2p-ca9.dts | 2 ++ > arch/arm/mach-vexpress/Kconfig | 1 + > arch/arm/mach-vexpress/v2m.c | 13 ++----------- > 5 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi > index ac870fb..3fa798f 100644 > --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi > +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi > @@ -183,6 +183,8 @@ > interrupts = <2>; > clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>; > clock-names = "timclken1", "timclken2", "apb_pclk"; > + arm,sp804-clocksource = <0x20>; > + arm,sp804-clockevent = <0>; > }; > > v2m_timer23: timer@120000 { > diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi > index f142036..86e4046 100644 > --- a/arch/arm/boot/dts/vexpress-v2m.dtsi > +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi > @@ -182,6 +182,8 @@ > interrupts = <2>; > clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>; > clock-names = "timclken1", "timclken2", "apb_pclk"; > + arm,sp804-clocksource = <0x20>; > + arm,sp804-clockevent = <0>; > }; > > v2m_timer23: timer@12000 { > diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > index 1420bb1..a2eba95 100644 > --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts > +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts > @@ -98,6 +98,8 @@ > <0 49 4>; > clocks = <&oscclk2>, <&oscclk2>; > clock-names = "timclk", "apb_pclk"; > + arm,sp804-clocksource = <0x20>; > + arm,sp804-clockevent = <0>; > }; > > watchdog@100e5000 { I love the possibility of removing the SP804 code from the VE machine (thanks for your work!), but I'd like to avoid it being replaced by cryptic properties in the tree. Now, to get this right: I'm not saying that there should be completly no OS-specific (but neutral) properties for the clocking system. I'm saying that in "my" (the VE) case, there are four, effectively identical timers (each pair sharing an interrupt though). Every single one of them is perfectly capable of serving as clocksource or clockevent. Therefore I believe the driver/framework/OS should make its own decision. Cheers! Paweł
On 13 March 2013 19:10, Pawel Moll <pawel.moll@arm.com> wrote: > On Wed, 2013-03-13 at 05:05 +0000, Haojian Zhuang wrote: >> Remove all code to parse sp804. Use clocksource_of_init() instead since >> all these code are implemented in sp804 driver already. >> >> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> >> --- >> arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 2 ++ >> arch/arm/boot/dts/vexpress-v2m.dtsi | 2 ++ >> arch/arm/boot/dts/vexpress-v2p-ca9.dts | 2 ++ >> arch/arm/mach-vexpress/Kconfig | 1 + >> arch/arm/mach-vexpress/v2m.c | 13 ++----------- >> 5 files changed, 9 insertions(+), 11 deletions(-) >> >> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi >> index ac870fb..3fa798f 100644 >> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi >> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi >> @@ -183,6 +183,8 @@ >> interrupts = <2>; >> clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>; >> clock-names = "timclken1", "timclken2", "apb_pclk"; >> + arm,sp804-clocksource = <0x20>; >> + arm,sp804-clockevent = <0>; >> }; >> >> v2m_timer23: timer@120000 { >> diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi >> index f142036..86e4046 100644 >> --- a/arch/arm/boot/dts/vexpress-v2m.dtsi >> +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi >> @@ -182,6 +182,8 @@ >> interrupts = <2>; >> clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>; >> clock-names = "timclken1", "timclken2", "apb_pclk"; >> + arm,sp804-clocksource = <0x20>; >> + arm,sp804-clockevent = <0>; >> }; >> >> v2m_timer23: timer@12000 { >> diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts >> index 1420bb1..a2eba95 100644 >> --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts >> +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts >> @@ -98,6 +98,8 @@ >> <0 49 4>; >> clocks = <&oscclk2>, <&oscclk2>; >> clock-names = "timclk", "apb_pclk"; >> + arm,sp804-clocksource = <0x20>; >> + arm,sp804-clockevent = <0>; >> }; >> >> watchdog@100e5000 { > > I love the possibility of removing the SP804 code from the VE machine > (thanks for your work!), but I'd like to avoid it being replaced by > cryptic properties in the tree. > > Now, to get this right: I'm not saying that there should be completly no > OS-specific (but neutral) properties for the clocking system. > > I'm saying that in "my" (the VE) case, there are four, effectively > identical timers (each pair sharing an interrupt though). Every single > one of them is perfectly capable of serving as clocksource or > clockevent. Therefore I believe the driver/framework/OS should make its > own decision. > I can move those properties into dts file instead. If clocksource uses alias to configure source or event. There's still a lot of work. At least we need to split dual timer into two timer devices in DTS.
On Wed, 2013-03-13 at 11:42 +0000, Haojian Zhuang wrote: > > I love the possibility of removing the SP804 code from the VE machine > > (thanks for your work!), but I'd like to avoid it being replaced by > > cryptic properties in the tree. > > > > Now, to get this right: I'm not saying that there should be completly no > > OS-specific (but neutral) properties for the clocking system. > > > > I'm saying that in "my" (the VE) case, there are four, effectively > > identical timers (each pair sharing an interrupt though). Every single > > one of them is perfectly capable of serving as clocksource or > > clockevent. Therefore I believe the driver/framework/OS should make its > > own decision. > > > I can move those properties into dts file instead. I'm not sure I understand you here... > If clocksource uses alias to configure source or event. There's still a lot > of work. At least we need to split dual timer into two timer devices in DTS. My point is: at least for VE there is _no need_ for any alias or special property in the tree (DTS). All timers are equal. The system can (and should) pick any one it wants. Paweł
On 13 March 2013 19:46, Pawel Moll <pawel.moll@arm.com> wrote: > On Wed, 2013-03-13 at 11:42 +0000, Haojian Zhuang wrote: >> > I love the possibility of removing the SP804 code from the VE machine >> > (thanks for your work!), but I'd like to avoid it being replaced by >> > cryptic properties in the tree. >> > >> > Now, to get this right: I'm not saying that there should be completly no >> > OS-specific (but neutral) properties for the clocking system. >> > >> > I'm saying that in "my" (the VE) case, there are four, effectively >> > identical timers (each pair sharing an interrupt though). Every single >> > one of them is perfectly capable of serving as clocksource or >> > clockevent. Therefore I believe the driver/framework/OS should make its >> > own decision. >> > >> I can move those properties into dts file instead. > > I'm not sure I understand you here... > >> If clocksource uses alias to configure source or event. There's still a lot >> of work. At least we need to split dual timer into two timer devices in DTS. > > My point is: at least for VE there is _no need_ for any alias or special > property in the tree (DTS). All timers are equal. The system can (and > should) pick any one it wants. > > Paweł > Do you mean that system wants to pick any one randomly? It's impossible. Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller. Could system still pick any one it wants? I think that clocksource or clockevent must be specified in DTS. Whatever we're using alias or property.
On Wed, 2013-03-13 at 12:21 +0000, Haojian Zhuang wrote: > Do you mean that system wants to pick any one randomly? It's impossible. It is possible in case of the VE platform. Therefore it is not completely impossible. QED :-) > Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller. > Could system still pick any one it wants? If you tell the system that only TIMINT1 is wired, it shouldn't even try to use the second timer for anything that would require the interrupt. Therefore, describing the hardware sufficiently, you're giving the system enough information to make the right decisions. > I think that clocksource or clockevent must be specified in DTS. Whatever > we're using alias or property. I'm convinced it does not *must* be there. I think it *can* be there, for platforms where the available timers are not equal (for whatever reason). My point is: why are you trying to force me to put some information into my hardware description which doesn't have any meaning for me? Paweł
On 13 March 2013 22:48, Pawel Moll <pawel.moll@arm.com> wrote: > On Wed, 2013-03-13 at 12:21 +0000, Haojian Zhuang wrote: >> Do you mean that system wants to pick any one randomly? It's impossible. > > It is possible in case of the VE platform. Therefore it is not > completely impossible. QED :-) > Yes, it's possible. Since only clocksource / clockevent is using SP804 timer on the VE platform. If user app or something else is using other SP804 timer, it would be mess because of resource conflict. >> Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller. >> Could system still pick any one it wants? > Then we need another new property. I try to find using minim properties. > If you tell the system that only TIMINT1 is wired, it shouldn't even try > to use the second timer for anything that would require the interrupt. > Therefore, describing the hardware sufficiently, you're giving the > system enough information to make the right decisions. > >> I think that clocksource or clockevent must be specified in DTS. Whatever >> we're using alias or property. > > I'm convinced it does not *must* be there. I think it *can* be there, > for platforms where the available timers are not equal (for whatever > reason). My point is: why are you trying to force me to put some > information into my hardware description which doesn't have any meaning > for me? > As I mentioned above, system must know the behavior first. The upper layer software could know which timer is used in clocksource or clockevent. For example, your Android system could be shared on multiple platforms. It could parse hardware information from DTS. (Of course, it hasn't this feature yet). Then it could arrange apps to use other timers. It's like you define console on specified UART even you have multiples UART port.
On Wed, 2013-03-13 at 15:01 +0000, Haojian Zhuang wrote: > Yes, it's possible. Since only clocksource / clockevent is using SP804 timer > on the VE platform. If user app or something else is using other SP804 > timer, it would be mess because of resource conflict. How would you expose this device to the app or something else? It's an amba_bus device, so it must be bound with an amba_driver. Otherwise you're abusing the device model and, well, you're on your own. The devices are managed by the system. The device tree is merely a description of the available hardware. > >> Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller. > >> Could system still pick any one it wants? > > > Then we need another new property. I try to find using minim properties. No, we don't. See the discussion with Rob. He has an idea (using the interrupt property only), I have another one (using already defined interrupt-names property). I see you've just came with another one (TIMINTC treated as TIMINT1), which is fine with me. Either way you've got what you need to make the right decision. > As I mentioned above, system must know the behavior first. The upper layer > software could know which timer is used in clocksource or clockevent. I claim otherwise. The upper layer software should know what does the hardware offer and based on this knowledge use it in a way it wants to. > For example, your Android system could be shared on multiple platforms. It > could parse hardware information from DTS. (Of course, it hasn't this feature > yet). Then it could arrange apps to use other timers. I honestly don't know what are you suggesting here other than it looks like an abuse to the device model. > It's like you define console on specified UART even you have multiples UART > port. Where do you do this? In the Device Tree? No. In the kernel command line, which is a runtime argument (the fact it is actually being passed through the tree is irrelevant here) You have the serialX aliases, which - again - simply describe the hardware ("this node is connected to a connector labeled as serial X"). The tree doesn't tell that the user connected his serial cable to serial1 so it should be used as the console. Paweł
On 13 March 2013 23:19, Pawel Moll <pawel.moll@arm.com> wrote: > On Wed, 2013-03-13 at 15:01 +0000, Haojian Zhuang wrote: >> Yes, it's possible. Since only clocksource / clockevent is using SP804 timer >> on the VE platform. If user app or something else is using other SP804 >> timer, it would be mess because of resource conflict. > > How would you expose this device to the app or something else? It's an > amba_bus device, so it must be bound with an amba_driver. Otherwise > you're abusing the device model and, well, you're on your own. > > The devices are managed by the system. The device tree is merely a > description of the available hardware. > Maybe I didn't express my idea well. My point is that timer could be clock source /clock event. It could also just be a timer that user app or other device driver may want to use it directly. There're a lot of reason that user app or some driver wants to use hardware timer. For example, the specific timer may keep running even system is in low power mode. Although there's so much hardware timers available, system must choose one for the usage of clock source / clock event / other hardware timer usage. >> >> Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller. >> >> Could system still pick any one it wants? >> > >> Then we need another new property. I try to find using minim properties. > > No, we don't. See the discussion with Rob. He has an idea (using the > interrupt property only), I have another one (using already defined > interrupt-names property). I see you've just came with another one > (TIMINTC treated as TIMINT1), which is fine with me. Either way you've > got what you need to make the right decision. > After considered as second time, I think that we still need a property to meet alias usage. Excuse me expand it in this topic. arm,sp804-has-irq = <offset>; The offset means TIMER register offset. If it's not zero, TIMER2 is using irq. And it could be configured as clock event. Otherwise, TIMER1 is clock event. We must make sure that only one of all sp804 timers is using this property. alias: linux,clocksource = <&sp804-A> linux,clockevent = <&sp804-A> Although both of them points to timerA, only TIMER1 or TIMER2 of sp804 is using irq. So one is clock source, and the other is clock event. alias: linux,clocksource = <&sp804-A> linux,clockevent = <&sp804-B> The issue comes since system don't know whether TIMER1 or TIMER2 could be configured as clock source. We still need a new property to claim it. It seems that we can't drop "arm,sp804-clocksource", although everybody don't like it. What's your opinion? > > Where do you do this? In the Device Tree? No. In the kernel command > line Yes, command line could also be filled in DTS.
On Wed, 2013-03-13 at 15:59 +0000, Haojian Zhuang wrote: > Maybe I didn't express my idea well. My point is that timer could be > clock source > /clock event. It could also just be a timer that user app or other device driver > may want to use it directly. There're a lot of reason that user app or some > driver wants to use hardware timer. For example, the specific timer may > keep running even system is in low power mode. That's all fair enough. My point is in any case the *system* will take care of setting this all up. Therefore it can manage the available resources to avoid the conflict. > Although there's so much hardware timers available, system must choose > one for the usage of clock source / clock event / other hardware timer usage. By all means. I'm just claiming that in normal situation it can choose on its convenience. In other words - if there are no limitations in hardware, it can pick any of them, being responsible for making the right decision. > >> >> Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller. > >> >> Could system still pick any one it wants? > >> > > >> Then we need another new property. I try to find using minim properties. > > > > No, we don't. See the discussion with Rob. He has an idea (using the > > interrupt property only), I have another one (using already defined > > interrupt-names property). I see you've just came with another one > > (TIMINTC treated as TIMINT1), which is fine with me. Either way you've > > got what you need to make the right decision. > > > After considered as second time, I think that we still need a property to meet > alias usage. Excuse me expand it in this topic. > > arm,sp804-has-irq = <offset>; > The offset means TIMER register offset. If it's not zero, TIMER2 is using > irq. And it could be configured as clock event. Otherwise, TIMER1 is clock > event. > We must make sure that only one of all sp804 timers is using this property. > > alias: > linux,clocksource = <&sp804-A> > linux,clockevent = <&sp804-A> > > Although both of them points to timerA, only TIMER1 or TIMER2 of sp804 > is using irq. So one is clock source, and the other is clock event. > > alias: > linux,clocksource = <&sp804-A> > linux,clockevent = <&sp804-B> > The issue comes since system don't know whether TIMER1 or TIMER2 could > be configured as clock source. > > We still need a new property to claim it. It seems that we can't drop > "arm,sp804-clocksource", although everybody don't like it. > > What's your opinion? Let me repeat myself - I'm not saying that there must be no such thing like the alias (or your property). I'm sure there will be a use case for them. What I am saying is that they should not be obligatory. Shortly speaking - I don't want to have them in VE DTS files. For VE platform the driver should pick one of the timers and use it as clocksource (I don't care which one), another one as clockevent (again, it's up to you) and the rest for whatever it wants. Could you please provide a solution for such a "generic" case, when there are no aliases defined in the tree? I see no problem with checking if the aliases exist and if they don't picking the first SP804 you can get hold of. Just give me that and you won't hear me complaining further :-) > > Where do you do this? In the Device Tree? No. In the kernel command > > line > Yes, command line could also be filled in DTS. Which is a very bad practice... There's a reason why "chosen" node is named like this ;-) Paweł
On 03/13/2013 10:59 AM, Haojian Zhuang wrote: > On 13 March 2013 23:19, Pawel Moll <pawel.moll@arm.com> wrote: >> On Wed, 2013-03-13 at 15:01 +0000, Haojian Zhuang wrote: >>> Yes, it's possible. Since only clocksource / clockevent is using SP804 timer >>> on the VE platform. If user app or something else is using other SP804 >>> timer, it would be mess because of resource conflict. >> >> How would you expose this device to the app or something else? It's an >> amba_bus device, so it must be bound with an amba_driver. Otherwise >> you're abusing the device model and, well, you're on your own. >> >> The devices are managed by the system. The device tree is merely a >> description of the available hardware. >> > Maybe I didn't express my idea well. My point is that timer could be > clock source > /clock event. It could also just be a timer that user app or other device driver > may want to use it directly. There're a lot of reason that user app or some > driver wants to use hardware timer. For example, the specific timer may > keep running even system is in low power mode. > > Although there's so much hardware timers available, system must choose > one for the usage of clock source / clock event / other hardware timer usage. > >>>>> Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller. >>>>> Could system still pick any one it wants? >>>> >>> Then we need another new property. I try to find using minim properties. >> >> No, we don't. See the discussion with Rob. He has an idea (using the >> interrupt property only), I have another one (using already defined >> interrupt-names property). I see you've just came with another one >> (TIMINTC treated as TIMINT1), which is fine with me. Either way you've >> got what you need to make the right decision. >> > After considered as second time, I think that we still need a property to meet > alias usage. Excuse me expand it in this topic. > > arm,sp804-has-irq = <offset>; Interrupt/timer number not offset please. The offset is implied by the fact that the h/w is an sp804. The Integrator AP is not actually an sp804, so it's init should be handled differently with a different compatible property. > The offset means TIMER register offset. If it's not zero, TIMER2 is using > irq. And it could be configured as clock event. Otherwise, TIMER1 is clock > event. > We must make sure that only one of all sp804 timers is using this property. > > alias: > linux,clocksource = <&sp804-A> > linux,clockevent = <&sp804-A> > > Although both of them points to timerA, only TIMER1 or TIMER2 of sp804 > is using irq. So one is clock source, and the other is clock event. > > alias: > linux,clocksource = <&sp804-A> > linux,clockevent = <&sp804-B> > The issue comes since system don't know whether TIMER1 or TIMER2 could > be configured as clock source. If you have 2 sp804's, why do you care which one is used by the OS? If they are identical, then it doesn't matter. If they are not, then describe how they are not identical. The realview and versatile boards currently do this, but there is no good reason that I see and I intend to change this. > We still need a new property to claim it. It seems that we can't drop > "arm,sp804-clocksource", although everybody don't like it. > > What's your opinion? As previously stated, I don't think we need this or the alias based on current in tree users. There is an issue that you don't know about all available timers so you can select the one with the minimum required features leaving others with more features (i.e. an irq) free. If you need more control like this then you can do custom setup before or instead of calling clocksource_of_init. None of the current platforms have this issue nor do they use sp804 for anything beyond clksrc and clkevt. I would not expect lots of new users of sp804 as I expect most new platforms will use architected timers. So let's not solve imaginary problems now. Rob
On Wed, Mar 13, 2013 at 11:32:01AM -0500, Rob Herring wrote: > If you have 2 sp804's, why do you care which one is used by the OS? If > they are identical, then it doesn't matter. If they are not, then > describe how they are not identical. > > The realview and versatile boards currently do this, but there is no > good reason that I see and I intend to change this. It is highly likely that the timer choice on these boards was made by independent people, who came up with different random choices. Nothing more than that - I believe it to be entirely arbitary. I do think discussion is getting too bogged down on this point. So, we could make the decision about which timer to use for the clock event based on whether an interrupt is specified, the known clock rate and size of the timer register. Same can be done for the clock source, but preferring a timer without an interrupt in preference to one with an interrupt. We do have this nice rating system for clocksources and clockevents - and it could be more flexible if it didn't separate clocksources from clockevents - allowing just a single "timer" and marking it as being suitable as a clocksource or a clockevent, or indeed both. However, for SP804 selection, there are cases where it matters, such as on Versatile Express where you have to select the right set of SP804 to have a working system (because ARM didn't fully document the SP804 setup there) and we have no way to switch some of those timers from their 32kHz clock source. But then we could just omit the clock source for the "bad" SP804s which would prevent them being used.
On Fri, 2013-03-15 at 12:34 +0000, Russell King - ARM Linux wrote: > However, for SP804 selection, there are cases where it matters, such as > on Versatile Express where you have to select the right set of SP804 to > have a working system (because ARM didn't fully document the SP804 setup > there) and we have no way to switch some of those timers from their > 32kHz clock source. But then we could just omit the clock source for > the "bad" SP804s which would prevent them being used. Both VE motherboard's SP804s TIMCLKENx inputs are fed by the SP810's outputs: v2m_timer01: timer@11000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x11000 0x1000>; interrupts = <2>; clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>; clock-names = "timclken1", "timclken2", "apb_pclk"; }; v2m_timer23: timer@12000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x12000 0x1000>; interrupts = <3>; clocks = <&v2m_sysctl 2>, <&v2m_sysctl 3>, <&smbclk>; clock-names = "timclken1", "timclken2", "apb_pclk"; }; TIMCLKs are wired to the SMB_CLK, so the effective rate depends on the TIMCLKENx only. The SP810 takes the 32kHz and 1MHz reference clocks: v2m_sysctl: sysctl@01000 { compatible = "arm,sp810", "arm,primecell"; reg = <0x01000 0x1000>; clocks = <&v2m_refclk32khz>, <&v2m_refclk1mhz>, <&smbclk>; clock-names = "refclk", "timclk", "apb_pclk"; #clock-cells = <1>; clock-output-names = "timerclken0", "timerclken1", "timerclken2", "timerclken3"; }; and the code in drivers/clk/versatile/clk-vexpress.c picks up the faster one as the parent. The V2P-CA9 SP804's TIMCLK is wired to one of the main clock generators: timer@100e4000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x100e4000 0x1000>; interrupts = <0 48 4>, <0 49 4>; clocks = <&oscclk2>, <&oscclk2>; clock-names = "timclk", "apb_pclk"; }; Both TIMCLKEN0 & 1 are pulled up to 1 so they have no effect on the timer rates. Overall, the description of the VE clocking in the Device Tree is rather complete. Paweł
On Fri, Mar 15, 2013 at 12:58:32PM +0000, Pawel Moll wrote: > On Fri, 2013-03-15 at 12:34 +0000, Russell King - ARM Linux wrote: > > However, for SP804 selection, there are cases where it matters, such as > > on Versatile Express where you have to select the right set of SP804 to > > have a working system (because ARM didn't fully document the SP804 setup > > there) and we have no way to switch some of those timers from their > > 32kHz clock source. But then we could just omit the clock source for > > the "bad" SP804s which would prevent them being used. > > Both VE motherboard's SP804s TIMCLKENx inputs are fed by the SP810's > outputs: Sigh. You do remember who (re)wrote the Versatile Express support that's in mainline... I know that the motherboard SP804s are clocked from the SP810 (which has no published documentation) and these are selectable between 1MHz and 32kHz. What I was referring to is the SP804s on the CT9x4 tile, which seem to be clocked at 32kHz with no way to change them, and it's that which has been totally and utterly undocumented (believe me, when I did that work above, I searched the available documentation many times.) It may be that as a result of my complaints to ARM about that, the documentation has since been fixed. Whether it makes sense to use the CT9x4 SP804s over the motherboards, I've no idea because I've been forced out of the loop on Versatile Express.
diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi index ac870fb..3fa798f 100644 --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi @@ -183,6 +183,8 @@ interrupts = <2>; clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>; clock-names = "timclken1", "timclken2", "apb_pclk"; + arm,sp804-clocksource = <0x20>; + arm,sp804-clockevent = <0>; }; v2m_timer23: timer@120000 { diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi index f142036..86e4046 100644 --- a/arch/arm/boot/dts/vexpress-v2m.dtsi +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi @@ -182,6 +182,8 @@ interrupts = <2>; clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>, <&smbclk>; clock-names = "timclken1", "timclken2", "apb_pclk"; + arm,sp804-clocksource = <0x20>; + arm,sp804-clockevent = <0>; }; v2m_timer23: timer@12000 { diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts index 1420bb1..a2eba95 100644 --- a/arch/arm/boot/dts/vexpress-v2p-ca9.dts +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts @@ -98,6 +98,8 @@ <0 49 4>; clocks = <&oscclk2>, <&oscclk2>; clock-names = "timclk", "apb_pclk"; + arm,sp804-clocksource = <0x20>; + arm,sp804-clockevent = <0>; }; watchdog@100e5000 { diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig index 52d315b..ff78c15 100644 --- a/arch/arm/mach-vexpress/Kconfig +++ b/arch/arm/mach-vexpress/Kconfig @@ -5,6 +5,7 @@ config ARCH_VEXPRESS select ARM_GIC select ARM_TIMER_SP804 select CLKDEV_LOOKUP + select CLKSRC_OF select COMMON_CLK select COMMON_CLK_VERSATILE select CPU_V7 diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c index 51e8701..bb599d3 100644 --- a/arch/arm/mach-vexpress/v2m.c +++ b/arch/arm/mach-vexpress/v2m.c @@ -21,6 +21,7 @@ #include <linux/regulator/fixed.h> #include <linux/regulator/machine.h> #include <linux/vexpress.h> +#include <linux/clocksource.h> #include <clocksource/arm_timer.h> #include <clocksource/timer-sp.h> @@ -431,19 +432,9 @@ void __init v2m_dt_init_early(void) static void __init v2m_dt_timer_init(void) { - struct device_node *node = NULL; - vexpress_clk_of_init(); - do { - node = of_find_compatible_node(node, NULL, "arm,sp804"); - } while (node && vexpress_get_site_by_node(node) != VEXPRESS_SITE_MB); - if (node) { - pr_info("Using SP804 '%s' as a clock & events source\n", - node->full_name); - v2m_sp804_init(of_iomap(node, 0), - irq_of_parse_and_map(node, 0)); - } + clocksource_of_init(); if (arch_timer_of_register() != 0) twd_local_timer_of_register();
Remove all code to parse sp804. Use clocksource_of_init() instead since all these code are implemented in sp804 driver already. Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> --- arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 2 ++ arch/arm/boot/dts/vexpress-v2m.dtsi | 2 ++ arch/arm/boot/dts/vexpress-v2p-ca9.dts | 2 ++ arch/arm/mach-vexpress/Kconfig | 1 + arch/arm/mach-vexpress/v2m.c | 13 ++----------- 5 files changed, 9 insertions(+), 11 deletions(-)