[PATCH v2 01/10] arm: zynq: Use standard timer binding
Steffen Trumtrar
s.trumtrar at pengutronix.de
Wed Mar 27 05:04:34 EDT 2013
On Wed, Mar 27, 2013 at 08:40:36AM +0100, Michal Simek wrote:
> 2013/3/26 Steffen Trumtrar <s.trumtrar at pengutronix.de>:
> > On Tue, Mar 26, 2013 at 06:43:33PM +0100, Michal Simek wrote:
> >> Use cdns,ttc because this driver is Cadence Rev06
> >> Triple Timer Counter and everybody can use it
> >> without xilinx specific function name or probing.
> >>
> >> Also use standard dt description for timer
> >> and also prepare for moving to clocksource
> >> initialization.
> >>
> >> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> >> ---
> >> v2: Update year in copyright
> >> Fix typo fault
> >> Remove all zynq specific names
> >>
> >> Steffen: I have done this change based on our discussion.
> >> Moving to generic location will be done in the next patch.
> >>
> >> Josh: We have talked about this change at ELC.
> >> Using standard dt binding as other timers.
> >>
> >> I have also discussed this with Rob some time ago.
> >> https://patchwork.kernel.org/patch/2112791/
> >> ---
> >> arch/arm/boot/dts/zynq-7000.dtsi | 45 +------
> >> arch/arm/boot/dts/zynq-zc702.dts | 10 --
> >> arch/arm/mach-zynq/common.c | 1 +
> >> arch/arm/mach-zynq/timer.c | 261 +++++++++++++++++++++++++++-----------
> >> 4 files changed, 195 insertions(+), 122 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> >> index 5914b56..51243db 100644
> >> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> >> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> >> @@ -111,56 +111,23 @@
> >> };
> >>
> >> ttc0: ttc0 at f8001000 {
> >> - #address-cells = <1>;
> >> - #size-cells = <0>;
> >> - compatible = "xlnx,ttc";
> >> + interrupt-parent = <&intc>;
> >> + interrupts = < 0 10 4 0 11 4 0 12 4 >;
> >> + compatible = "cdns,ttc";
> >> reg = <0xF8001000 0x1000>;
> >> clocks = <&cpu_clk 3>;
> >> clock-names = "cpu_1x";
> >> clock-ranges;
> >> -
> >> - ttc0_0: ttc0.0 {
> >> - status = "disabled";
> >> - reg = <0>;
> >> - interrupts = <0 10 4>;
> >> - };
> >> - ttc0_1: ttc0.1 {
> >> - status = "disabled";
> >> - reg = <1>;
> >> - interrupts = <0 11 4>;
> >> - };
> >> - ttc0_2: ttc0.2 {
> >> - status = "disabled";
> >> - reg = <2>;
> >> - interrupts = <0 12 4>;
> >> - };
> >> };
> >>
> >> ttc1: ttc1 at f8002000 {
> >> - #interrupt-parent = <&intc>;
> >> - #address-cells = <1>;
> >> - #size-cells = <0>;
> >> - compatible = "xlnx,ttc";
> >> + interrupt-parent = <&intc>;
> >> + interrupts = < 0 37 4 0 38 4 0 39 4 >;
> >> + compatible = "cdns,ttc";
> >> reg = <0xF8002000 0x1000>;
> >> clocks = <&cpu_clk 3>;
> >> clock-names = "cpu_1x";
> >> clock-ranges;
> >> -
> >> - ttc1_0: ttc1.0 {
> >> - status = "disabled";
> >> - reg = <0>;
> >> - interrupts = <0 37 4>;
> >> - };
> >> - ttc1_1: ttc1.1 {
> >> - status = "disabled";
> >> - reg = <1>;
> >> - interrupts = <0 38 4>;
> >> - };
> >> - ttc1_2: ttc1.2 {
> >> - status = "disabled";
> >> - reg = <2>;
> >> - interrupts = <0 39 4>;
> >> - };
> >> };
> >> };
> >> };
> >
> > Hi Michal!
> >
> > Thought about this a little more. I'm not seeing the improvment this gives us.
> > The ttcs give us 6 counters that could be used however one likes. Linux wants
> > a clocksource and clockevent provider, but I could use the Cortex timer as
> > clocksource and would only have to sacrifice one of the 6 counters.
> > With this patch I have to sacrifice 3 counters and would only use 2 of them.
> > Then there is pinmuxing. That can be handled by one driver, so I think that is
> > doable with both versions and I think I'm okay with that.
> > So what am I missing? Why would I want it like this and not the way it is right
> > now?
>
> There is a big move because previous DT description was incorrect.
> Device-tree should describe hardware and there is no something like
> ttc-counter-clocksource or ttc-counter-clockevent compatible driver.
> Every ttc counter can be used as clocksource or clockevent.
> Changing/Setting compatible properties for linux purpose is not correct.
>
That is correct. DT describes the HW and the TTC includes three counters that
could be used in any way a HW vendor likes. How would you decide that with
just the driver?
The compatible property is uncommon and might even be wrong. How about adding
for example a mode-property like usb does for otg,peripheral,host?
Suppose I have some obscure board with a Zynq on it, that has ttc1_0 setup to
count some external signal and ttc1_1 counts some signal from the PL. I mean
it is possible to configure the SoC in that way, no?!
I would than have a board-specific dts where I would somehow have to describe
this setup and mark the different ttcs accordingly.
What I'm going for is, what is wrong in having the subnodes in ttc and
then iterate over these subnodes in the driver and registering the first two
ttcs that I find and that support being used as clocksource/event?
> Just read this thread about.
> https://patchwork.kernel.org/patch/2112791/
>
Already did.
> Utilization of the core or using different timers in the system for clock
> is different discussion. We can just try to utilize timers in the arm core
> or add bunch of them to PL to see how kernel behaves.
>
No problem there. I use the smp_twd on the ZedBoard and the kernel just does the
right thing.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
More information about the linux-arm-kernel
mailing list