[PATCH v2 01/10] arm: zynq: Use standard timer binding
Steffen Trumtrar
s.trumtrar at pengutronix.de
Wed Mar 27 05:37:14 EDT 2013
On Wed, Mar 27, 2013 at 10:25:30AM +0100, Michal Simek wrote:
> 2013/3/27 Steffen Trumtrar <s.trumtrar at pengutronix.de>:
> > 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?
>
> I don't think that this is right thing to do.
> Rob: What do you think?
>
> > 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?!
>
> If you are on ttc1 then it should be fine because ttc0 should be probed first
> and ttc1 is not allocated but look below.
>
> > I would than have a board-specific dts where I would somehow have to describe
> > this setup and mark the different ttcs accordingly.
>
> I understand your setup and I am not aware if there is any standard
> way how to handles
> other timers in the system to be able to work with them via standard api.
> What do you use? UIO for ttc setup and reading it back?
>
Oh, I don't have this setup. I'm just imagining things and how we could then
handle this setup, if it ever occurs. When this binding is fixed it is fixed.
At the moment it might be okay to rework this, as current mainline only works
under certain circumstances and there is probably no one really using it as of yet.
> > 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?
>
> I understand your concern but the question is how to describe this in
> dts in a proper way.
> Compatible property - no
Agreed.
> Based on aliases order - possible but based on my discussion with Rob
> probably no
Agreed.
> Additional mode options - it is also suggesting Linux specific usage.
>
Hm, it would specify the capabilities of the ttc. Other users of the DT
might be interested in this info, too. I'm not saying it is the best/only
solution.
> What next?
>
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