From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751484AbdFHFmx convert rfc822-to-8bit (ORCPT ); Thu, 8 Jun 2017 01:42:53 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:38106 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750752AbdFHFmw (ORCPT ); Thu, 8 Jun 2017 01:42:52 -0400 Date: Thu, 8 Jun 2017 07:42:36 +0200 From: Boris Brezillon To: Alexandre Belloni , Rob Herring Cc: Daniel Lezcano , Nicolas Ferre , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Thomas Gleixner , "devicetree@vger.kernel.org" Subject: Re: [PATCH 46/58] clocksource/drivers: Add a new driver for the Atmel ARM TC blocks Message-ID: <20170608074236.62924f01@bbrezillon> In-Reply-To: <20170607231715.ns2vcxza2eexnzjs@piout.net> References: <20170530215139.9983-1-alexandre.belloni@free-electrons.com> <20170530215139.9983-47-alexandre.belloni@free-electrons.com> <20170606152104.GC2345@mai> <20170606180559.pkrr7ux2qqnmsd6y@piout.net> <20170607141735.GH2345@mai> <20170607152750.tksmyf5p3oajbsac@piout.net> <20170607210848.GJ2345@mai> <20170607231715.ns2vcxza2eexnzjs@piout.net> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le Thu, 8 Jun 2017 01:17:15 +0200, Alexandre Belloni a écrit : > On 07/06/2017 at 23:08:48 +0200, Daniel Lezcano wrote: > > > I was going to agree but this is not flexible enough because the > > > quadrature decoder always uses the first two channels. So on some > > > products, we may have: > > > - TCB0: > > > o channels 0,1: qdec > > > o channel 2: clocksource > > > > > > - TCB1: > > > o channels 0,1: qdec > > > o channel 2: clockevent > > > > > > This avoids wasting TCB channels. > > > > Ok. In this case you can check if the interrupt is specified for the node, if > > yes, then it is a clockevent. > > > > But currently it is always specified in the SoC's dtsi. I don't find > that too practical to push that to the board's dts. Also, lying by > omission (the IRQ is always wired) in the DT is not different from > having a property selecting which timer is the clocksource and which is > the clockevent. > I agree with Alexandre here. Really, there's not much we can do to detect which timer should be used as a clockevent and which one should be used as a clocksource except explicitly specifying it in the DT. Having an interrupt defined in one case (clockevent) and undefined in the other case (clocksource), is just as hack-ish as the detection logic Alexandre developed to avoid explicitly specifying the function assigned to a specific timer. Can we please find a solution that makes everyone happy (DT, clocksoure/clockevent and at91 maintainers)? How about adding a linux,timer-function property to specify which function this timer is providing? Something like that for example: tcb0: timer@fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk"; timer@0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; linux,timer-function = "clocksource"; }; timer@2 { compatible = "atmel,tcb-timer"; reg = <2>; linux,timer-function = "clockevent"; }; }; Alternatively, we could have a property or a node in chosen describing which timer should be used: chosen { clockevent { timer = <&timer2>; }; clocksource { timer = <&timer0>; }; /* * or * * clockevent = <&timer2>; * clocksource = <&timer0>; * * but I think the clocksource/clockevent node approach * is more future proof in case we need to add extra * information like the expected resolution/precision or * anything that could be tweakable. */ }; tcb0: timer@fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk"; timer0: timer@0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; }; timer2: timer@2 { compatible = "atmel,tcb-timer"; reg = <2>; }; }; From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH 46/58] clocksource/drivers: Add a new driver for the Atmel ARM TC blocks Date: Thu, 8 Jun 2017 07:42:36 +0200 Message-ID: <20170608074236.62924f01@bbrezillon> References: <20170530215139.9983-1-alexandre.belloni@free-electrons.com> <20170530215139.9983-47-alexandre.belloni@free-electrons.com> <20170606152104.GC2345@mai> <20170606180559.pkrr7ux2qqnmsd6y@piout.net> <20170607141735.GH2345@mai> <20170607152750.tksmyf5p3oajbsac@piout.net> <20170607210848.GJ2345@mai> <20170607231715.ns2vcxza2eexnzjs@piout.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <20170607231715.ns2vcxza2eexnzjs-m++hUPXGwpdeoWH0uzbU5w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Alexandre Belloni , Rob Herring Cc: Daniel Lezcano , Nicolas Ferre , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Thomas Gleixner , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: devicetree@vger.kernel.org Le Thu, 8 Jun 2017 01:17:15 +0200, Alexandre Belloni a écrit : > On 07/06/2017 at 23:08:48 +0200, Daniel Lezcano wrote: > > > I was going to agree but this is not flexible enough because the > > > quadrature decoder always uses the first two channels. So on some > > > products, we may have: > > > - TCB0: > > > o channels 0,1: qdec > > > o channel 2: clocksource > > > > > > - TCB1: > > > o channels 0,1: qdec > > > o channel 2: clockevent > > > > > > This avoids wasting TCB channels. > > > > Ok. In this case you can check if the interrupt is specified for the node, if > > yes, then it is a clockevent. > > > > But currently it is always specified in the SoC's dtsi. I don't find > that too practical to push that to the board's dts. Also, lying by > omission (the IRQ is always wired) in the DT is not different from > having a property selecting which timer is the clocksource and which is > the clockevent. > I agree with Alexandre here. Really, there's not much we can do to detect which timer should be used as a clockevent and which one should be used as a clocksource except explicitly specifying it in the DT. Having an interrupt defined in one case (clockevent) and undefined in the other case (clocksource), is just as hack-ish as the detection logic Alexandre developed to avoid explicitly specifying the function assigned to a specific timer. Can we please find a solution that makes everyone happy (DT, clocksoure/clockevent and at91 maintainers)? How about adding a linux,timer-function property to specify which function this timer is providing? Something like that for example: tcb0: timer@fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk"; timer@0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; linux,timer-function = "clocksource"; }; timer@2 { compatible = "atmel,tcb-timer"; reg = <2>; linux,timer-function = "clockevent"; }; }; Alternatively, we could have a property or a node in chosen describing which timer should be used: chosen { clockevent { timer = <&timer2>; }; clocksource { timer = <&timer0>; }; /* * or * * clockevent = <&timer2>; * clocksource = <&timer0>; * * but I think the clocksource/clockevent node approach * is more future proof in case we need to add extra * information like the expected resolution/precision or * anything that could be tweakable. */ }; tcb0: timer@fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk"; timer0: timer@0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; }; timer2: timer@2 { compatible = "atmel,tcb-timer"; reg = <2>; }; }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: boris.brezillon@free-electrons.com (Boris Brezillon) Date: Thu, 8 Jun 2017 07:42:36 +0200 Subject: [PATCH 46/58] clocksource/drivers: Add a new driver for the Atmel ARM TC blocks In-Reply-To: <20170607231715.ns2vcxza2eexnzjs@piout.net> References: <20170530215139.9983-1-alexandre.belloni@free-electrons.com> <20170530215139.9983-47-alexandre.belloni@free-electrons.com> <20170606152104.GC2345@mai> <20170606180559.pkrr7ux2qqnmsd6y@piout.net> <20170607141735.GH2345@mai> <20170607152750.tksmyf5p3oajbsac@piout.net> <20170607210848.GJ2345@mai> <20170607231715.ns2vcxza2eexnzjs@piout.net> Message-ID: <20170608074236.62924f01@bbrezillon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Le Thu, 8 Jun 2017 01:17:15 +0200, Alexandre Belloni a ?crit : > On 07/06/2017 at 23:08:48 +0200, Daniel Lezcano wrote: > > > I was going to agree but this is not flexible enough because the > > > quadrature decoder always uses the first two channels. So on some > > > products, we may have: > > > - TCB0: > > > o channels 0,1: qdec > > > o channel 2: clocksource > > > > > > - TCB1: > > > o channels 0,1: qdec > > > o channel 2: clockevent > > > > > > This avoids wasting TCB channels. > > > > Ok. In this case you can check if the interrupt is specified for the node, if > > yes, then it is a clockevent. > > > > But currently it is always specified in the SoC's dtsi. I don't find > that too practical to push that to the board's dts. Also, lying by > omission (the IRQ is always wired) in the DT is not different from > having a property selecting which timer is the clocksource and which is > the clockevent. > I agree with Alexandre here. Really, there's not much we can do to detect which timer should be used as a clockevent and which one should be used as a clocksource except explicitly specifying it in the DT. Having an interrupt defined in one case (clockevent) and undefined in the other case (clocksource), is just as hack-ish as the detection logic Alexandre developed to avoid explicitly specifying the function assigned to a specific timer. Can we please find a solution that makes everyone happy (DT, clocksoure/clockevent and at91 maintainers)? How about adding a linux,timer-function property to specify which function this timer is providing? Something like that for example: tcb0: timer at fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk"; timer at 0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; linux,timer-function = "clocksource"; }; timer at 2 { compatible = "atmel,tcb-timer"; reg = <2>; linux,timer-function = "clockevent"; }; }; Alternatively, we could have a property or a node in chosen describing which timer should be used: chosen { clockevent { timer = <&timer2>; }; clocksource { timer = <&timer0>; }; /* * or * * clockevent = <&timer2>; * clocksource = <&timer0>; * * but I think the clocksource/clockevent node approach * is more future proof in case we need to add extra * information like the expected resolution/precision or * anything that could be tweakable. */ }; tcb0: timer at fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk"; timer0: timer at 0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; }; timer2: timer at 2 { compatible = "atmel,tcb-timer"; reg = <2>; }; };