| Message ID | 1427888858-29636-5-git-send-email-bhupesh.sharma@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 01, 2015 at 12:47:34PM +0100, Bhupesh Sharma wrote: > From: Wang Dongsheng <dongsheng.wang@freescale.com> > > Only FTM0 can be used as an alarm timer, so add a "fsl,ftm-alarm" > compatible string to describe FTM0 alarm mode. > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> > Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > --- > .../devicetree/bindings/timer/fsl,ftm-timer.txt | 46 ++++++++++++++------ > 1 file changed, 32 insertions(+), 14 deletions(-) Shouldn't there be a corresponding driver update? There wasn't one in this series. > > diff --git a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > index aa8c402..a372ed7 100644 > --- a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > +++ b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > @@ -2,12 +2,18 @@ Freescale FlexTimer Module (FTM) Timer > > Required properties: > > -- compatible : should be "fsl,ftm-timer" > +- compatible : should be "fsl,ftm-timer" & "fsl,ftm-alarm". > + (a) "fsl,ftm-timer", for FlexTimer compatible as normal timer. > + (b) "fsl,ftm-alarm", for FlexTimer compatible when FTM0 as an ALARM timer. Lets not be redundant here. This would be better as: - compatible: should contain one of: * "fsl,ftm-timer" for a FlexTimer usable as a normal timer. * "fsl,ftm-alarm" for a FlexTimer usable as an ALARM timer. I don't think FTM0 is useful to mention here; it sounds like that's just the name of an instance on a particular design. What exactly is the difference between a "normal" timer and an "alarm" timer? > + > - reg : Specifies base physical address and size of the register sets for the > clock event device and clock source device. > + > - interrupts : Should be the clock event device interrupt. > + > - clocks : The clocks provided by the SoC to drive the timer, must contain an > entry for each entry in clock-names. > + > - clock-names : Must include the following entries: > o "ftm-evt" > o "ftm-src" > @@ -16,16 +22,28 @@ Required properties: > - big-endian: One boolean property, the big endian mode will be in use if it is > present, or the little endian mode will be in use for all the device registers. > > -Example: > -ftm: ftm@400b8000 { > - compatible = "fsl,ftm-timer"; > - reg = <0x400b8000 0x1000 0x400b9000 0x1000>; > - interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>; > - clock-names = "ftm-evt", "ftm-src", > - "ftm-evt-counter-en", "ftm-src-counter-en"; > - clocks = <&clks VF610_CLK_FTM2>, > - <&clks VF610_CLK_FTM3>, > - <&clks VF610_CLK_FTM2_EXT_FIX_EN>, > - <&clks VF610_CLK_FTM3_EXT_FIX_EN>; > - big-endian; > -}; > +Example 1: In this example, The FlexTimer module (FTM) is a two-to-eight, > + channel timer that supports input capture, output compare, and > + the generation of PWM signals to control electric motor and power > + management applications. > + > + ftm: ftm@400b8000 { > + compatible = "fsl,ftm-timer"; > + reg = <0x400b8000 0x1000 0x400b9000 0x1000>; > + interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>; > + clock-names = "ftm-evt", "ftm-src", > + "ftm-evt-counter-en", "ftm-src-counter-en"; > + clocks = <&clks VF610_CLK_FTM2>, > + <&clks VF610_CLK_FTM3>, > + <&clks VF610_CLK_FTM2_EXT_FIX_EN>, > + <&clks VF610_CLK_FTM3_EXT_FIX_EN>; > + big-endian; > + }; > + > +Example 2: In this example, FTM0 only be used as an alarm timer. Do you mean that FTM0 is the only instance usable as an alarm timer, or that the FTM0 instance can only be used as an alarm timer? > + > + ftm0: ftm0@2800000 { > + compatible = "fsl,ftm-alarm"; > + reg = <0x0 0x2800000 0x0 0x10000>; > + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; > + }; Missing properties? Mark.
> -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Wednesday, April 01, 2015 9:24 PM > To: Sharma Bhupesh-B45370 > Cc: arnd@arndb.de; linux-arm-kernel@lists.infradead.org; Marc Zyngier; > bhupesh.linux@gmail.com; Catalin Marinas; Yoder Stuart-B08248; olof@lixom.net; > Will Deacon; Wang Dongsheng-B40534 > Subject: Re: [RESEND PATCH 4/8] layerscape/ftm: Add compatible string for FTM0 > be used as alarm timer. > > On Wed, Apr 01, 2015 at 12:47:34PM +0100, Bhupesh Sharma wrote: > > From: Wang Dongsheng <dongsheng.wang@freescale.com> > > > > Only FTM0 can be used as an alarm timer, so add a "fsl,ftm-alarm" > > compatible string to describe FTM0 alarm mode. > > > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> > > Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > > --- > > .../devicetree/bindings/timer/fsl,ftm-timer.txt | 46 ++++++++++++++----- > - > > 1 file changed, 32 insertions(+), 14 deletions(-) > > Shouldn't there be a corresponding driver update? > > There wasn't one in this series. > Hi Bhupesh, would you paste the driver link to here? Thanks very much. > > > > diff --git a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > > b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > > index aa8c402..a372ed7 100644 > > --- a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > > +++ b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt > > @@ -2,12 +2,18 @@ Freescale FlexTimer Module (FTM) Timer > > > > Required properties: > > > > -- compatible : should be "fsl,ftm-timer" > > +- compatible : should be "fsl,ftm-timer" & "fsl,ftm-alarm". > > + (a) "fsl,ftm-timer", for FlexTimer compatible as normal timer. > > + (b) "fsl,ftm-alarm", for FlexTimer compatible when FTM0 as an ALARM timer. > > > Lets not be redundant here. This would be better as: > > - compatible: should contain one of: > * "fsl,ftm-timer" for a FlexTimer usable as a normal timer. > * "fsl,ftm-alarm" for a FlexTimer usable as an ALARM timer. > Looks better, thanks. > I don't think FTM0 is useful to mention here; it sounds like that's just the > name of an instance on a particular design. > > What exactly is the difference between a "normal" timer and an "alarm" > timer? > Only FTM0 can be used as alarm timer. > > + > > - reg : Specifies base physical address and size of the register sets for the > > clock event device and clock source device. > > + > > - interrupts : Should be the clock event device interrupt. > > + > > - clocks : The clocks provided by the SoC to drive the timer, must contain an > > entry for each entry in clock-names. > > + > > - clock-names : Must include the following entries: > > o "ftm-evt" > > o "ftm-src" > > @@ -16,16 +22,28 @@ Required properties: > > - big-endian: One boolean property, the big endian mode will be in use if it > is > > present, or the little endian mode will be in use for all the device > registers. > > > > -Example: > > -ftm: ftm@400b8000 { > > - compatible = "fsl,ftm-timer"; > > - reg = <0x400b8000 0x1000 0x400b9000 0x1000>; > > - interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>; > > - clock-names = "ftm-evt", "ftm-src", > > - "ftm-evt-counter-en", "ftm-src-counter-en"; > > - clocks = <&clks VF610_CLK_FTM2>, > > - <&clks VF610_CLK_FTM3>, > > - <&clks VF610_CLK_FTM2_EXT_FIX_EN>, > > - <&clks VF610_CLK_FTM3_EXT_FIX_EN>; > > - big-endian; > > -}; > > +Example 1: In this example, The FlexTimer module (FTM) is a two-to-eight, > > + channel timer that supports input capture, output compare, and > > + the generation of PWM signals to control electric motor and power > > + management applications. > > + > > + ftm: ftm@400b8000 { > > + compatible = "fsl,ftm-timer"; > > + reg = <0x400b8000 0x1000 0x400b9000 0x1000>; > > + interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>; > > + clock-names = "ftm-evt", "ftm-src", > > + "ftm-evt-counter-en", "ftm-src-counter-en"; > > + clocks = <&clks VF610_CLK_FTM2>, > > + <&clks VF610_CLK_FTM3>, > > + <&clks VF610_CLK_FTM2_EXT_FIX_EN>, > > + <&clks VF610_CLK_FTM3_EXT_FIX_EN>; > > + big-endian; > > + }; > > + > > +Example 2: In this example, FTM0 only be used as an alarm timer. > > Do you mean that FTM0 is the only instance usable as an alarm timer, or that the > FTM0 instance can only be used as an alarm timer? > Ditto. > > + > > + ftm0: ftm0@2800000 { > > + compatible = "fsl,ftm-alarm"; > > + reg = <0x0 0x2800000 0x0 0x10000>; > > + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > Missing properties? > The FTM0 alarm timer only need "compatible", "reg" and "interrupts" properties. I miss something about the properties? Regards, -Dongsheng
> > > + ftm0: ftm0@2800000 { > > > + compatible = "fsl,ftm-alarm"; > > > + reg = <0x0 0x2800000 0x0 0x10000>; > > > + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; > > > + }; > > > > Missing properties? > > > > The FTM0 alarm timer only need "compatible", "reg" and "interrupts" properties. > I miss something about the properties? As far as I could tell from the original patch, clocks were also listed as required properties regardless. Mark.
> -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Tuesday, April 21, 2015 6:42 PM > To: Wang Dongsheng-B40534 > Cc: Sharma Bhupesh-B45370; arnd@arndb.de; linux-arm-kernel@lists.infradead.org; > Marc Zyngier; bhupesh.linux@gmail.com; Catalin Marinas; Yoder Stuart-B08248; > olof@lixom.net; Will Deacon > Subject: Re: [RESEND PATCH 4/8] layerscape/ftm: Add compatible string for FTM0 > be used as alarm timer. > > > > > + ftm0: ftm0@2800000 { > > > > + compatible = "fsl,ftm-alarm"; > > > > + reg = <0x0 0x2800000 0x0 0x10000>; > > > > + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; > > > > + }; > > > > > > Missing properties? > > > > > > > The FTM0 alarm timer only need "compatible", "reg" and "interrupts" properties. > > I miss something about the properties? > > As far as I could tell from the original patch, clocks were also listed as > required properties regardless. > Yes, But only one clock can be used for FTM0 as alarm timer. So this node not need to "clock" properties. :) Regards, -Dongsheng
On Tue, Apr 21, 2015 at 12:01:06PM +0100, Dongsheng.Wang@freescale.com wrote: > > > > -----Original Message----- > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > Sent: Tuesday, April 21, 2015 6:42 PM > > To: Wang Dongsheng-B40534 > > Cc: Sharma Bhupesh-B45370; arnd@arndb.de; linux-arm-kernel@lists.infradead.org; > > Marc Zyngier; bhupesh.linux@gmail.com; Catalin Marinas; Yoder Stuart-B08248; > > olof@lixom.net; Will Deacon > > Subject: Re: [RESEND PATCH 4/8] layerscape/ftm: Add compatible string for FTM0 > > be used as alarm timer. > > > > > > > + ftm0: ftm0@2800000 { > > > > > + compatible = "fsl,ftm-alarm"; > > > > > + reg = <0x0 0x2800000 0x0 0x10000>; > > > > > + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; > > > > > + }; > > > > > > > > Missing properties? > > > > > > > > > > The FTM0 alarm timer only need "compatible", "reg" and "interrupts" properties. > > > I miss something about the properties? > > > > As far as I could tell from the original patch, clocks were also listed as > > required properties regardless. > > > > Yes, But only one clock can be used for FTM0 as alarm timer. So this node not need to > "clock" properties. :) If the clock property is not necessary in some cases, please add those caveats in the clock property description. I'm not sure I follow why you don't need a clock in this case. Is there a clock internal to the unit? Or is it simply that there is only one possible external clock? For the latter it should still be described explicitly. Mark.
diff --git a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt index aa8c402..a372ed7 100644 --- a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt +++ b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt @@ -2,12 +2,18 @@ Freescale FlexTimer Module (FTM) Timer Required properties: -- compatible : should be "fsl,ftm-timer" +- compatible : should be "fsl,ftm-timer" & "fsl,ftm-alarm". + (a) "fsl,ftm-timer", for FlexTimer compatible as normal timer. + (b) "fsl,ftm-alarm", for FlexTimer compatible when FTM0 as an ALARM timer. + - reg : Specifies base physical address and size of the register sets for the clock event device and clock source device. + - interrupts : Should be the clock event device interrupt. + - clocks : The clocks provided by the SoC to drive the timer, must contain an entry for each entry in clock-names. + - clock-names : Must include the following entries: o "ftm-evt" o "ftm-src" @@ -16,16 +22,28 @@ Required properties: - big-endian: One boolean property, the big endian mode will be in use if it is present, or the little endian mode will be in use for all the device registers. -Example: -ftm: ftm@400b8000 { - compatible = "fsl,ftm-timer"; - reg = <0x400b8000 0x1000 0x400b9000 0x1000>; - interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>; - clock-names = "ftm-evt", "ftm-src", - "ftm-evt-counter-en", "ftm-src-counter-en"; - clocks = <&clks VF610_CLK_FTM2>, - <&clks VF610_CLK_FTM3>, - <&clks VF610_CLK_FTM2_EXT_FIX_EN>, - <&clks VF610_CLK_FTM3_EXT_FIX_EN>; - big-endian; -}; +Example 1: In this example, The FlexTimer module (FTM) is a two-to-eight, + channel timer that supports input capture, output compare, and + the generation of PWM signals to control electric motor and power + management applications. + + ftm: ftm@400b8000 { + compatible = "fsl,ftm-timer"; + reg = <0x400b8000 0x1000 0x400b9000 0x1000>; + interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>; + clock-names = "ftm-evt", "ftm-src", + "ftm-evt-counter-en", "ftm-src-counter-en"; + clocks = <&clks VF610_CLK_FTM2>, + <&clks VF610_CLK_FTM3>, + <&clks VF610_CLK_FTM2_EXT_FIX_EN>, + <&clks VF610_CLK_FTM3_EXT_FIX_EN>; + big-endian; + }; + +Example 2: In this example, FTM0 only be used as an alarm timer. + + ftm0: ftm0@2800000 { + compatible = "fsl,ftm-alarm"; + reg = <0x0 0x2800000 0x0 0x10000>; + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; + };