2013-10-28 09:48:34

by Xiubo Li

[permalink] [raw]
Subject: [RFC][PATCHv5 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.

The RFC patch only added "See ../clock/clock-bindings.txt for details of
the property values."



This adds the Document for Freescale FTM PWM driver under
Documentation/devicetree/bindings/pwm/.

Signed-off-by: Xiubo Li <[email protected]>
---
.../devicetree/bindings/pwm/pwm-fsl-ftm.txt | 34 ++++++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt

diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
new file mode 100644
index 0000000..175b762
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
@@ -0,0 +1,34 @@
+Freescale FTM PWM controller
+
+Required properties:
+- compatible: Should be "fsl,vf610-ftm-pwm"
+- reg: Physical base address and length of the controller's registers
+- #pwm-cells: Should be 3. See pwm.txt in this directory for a description of
+ the cells format.
+- clock-names : Includes the following module clock source entries:
+ "ftm0" (system clock),
+ "ftm0_fix_sel" (fixed frequency clock),
+ "ftm0_ext_sel" (external clock)
+- clocks : Must contain a clock specifier for each entry in clock-names,
+ See ../clock/clock-bindings.txt for details of the property values.
+- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be one of the
+ entries in clock-names.
+- pinctrl-names: must contain a "default" entry.
+- pinctrl-NNN: One property must exist for each entry in pinctrl-names.
+ See ../pinctrl/pinctrl-bindings.txt for details of the property values.
+
+
+Example:
+
+pwm0: pwm@40038000 {
+ compatible = "fsl,vf610-ftm-pwm";
+ reg = <0x40038000 0x1000>;
+ #pwm-cells = <3>;
+ clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel";
+ clocks = <&clks VF610_CLK_FTM0>,
+ <&clks VF610_CLK_FTM0_FIX_SEL>,
+ <&clks VF610_CLK_FTM0_EXT_SEL>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_1>;
+ fsl,pwm-counter-clk = "ftm0_ext_sel";
+};
--
1.8.4


2013-10-28 10:59:47

by Kumar Gala

[permalink] [raw]
Subject: Re: [RFC][PATCHv5 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.


On Oct 28, 2013, at 4:05 AM, Xiubo Li wrote:

> The RFC patch only added "See ../clock/clock-bindings.txt for details of
> the property values."
>
>
>
> This adds the Document for Freescale FTM PWM driver under
> Documentation/devicetree/bindings/pwm/.
>
> Signed-off-by: Xiubo Li <[email protected]>
> ---
> .../devicetree/bindings/pwm/pwm-fsl-ftm.txt | 34 ++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> new file mode 100644
> index 0000000..175b762
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> @@ -0,0 +1,34 @@
> +Freescale FTM PWM controller

What does FTM stand for, and can we spell out PWM at least once early on.

> +
> +Required properties:
> +- compatible: Should be "fsl,vf610-ftm-pwm"
> +- reg: Physical base address and length of the controller's registers
> +- #pwm-cells: Should be 3. See pwm.txt in this directory for a description of
> + the cells format.
> +- clock-names : Includes the following module clock source entries:
> + "ftm0" (system clock),
> + "ftm0_fix_sel" (fixed frequency clock),
> + "ftm0_ext_sel" (external clock)
> +- clocks : Must contain a clock specifier for each entry in clock-names,
> + See ../clock/clock-bindings.txt for details of the property values.
> +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be one of the
> + entries in clock-names.

Why do we need this, why not just have only the clock-names/clocks reference the clk that is actually used?

> +- pinctrl-names: must contain a "default" entry.
> +- pinctrl-NNN: One property must exist for each entry in pinctrl-names.
> + See ../pinctrl/pinctrl-bindings.txt for details of the property values.

let's drop the .. when making directory references.

> +
> +
> +Example:
> +
> +pwm0: pwm@40038000 {
> + compatible = "fsl,vf610-ftm-pwm";
> + reg = <0x40038000 0x1000>;
> + #pwm-cells = <3>;
> + clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel";
> + clocks = <&clks VF610_CLK_FTM0>,
> + <&clks VF610_CLK_FTM0_FIX_SEL>,
> + <&clks VF610_CLK_FTM0_EXT_SEL>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pwm0_1>;
> + fsl,pwm-counter-clk = "ftm0_ext_sel";
> +};
> --
> 1.8.4

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2013-10-29 03:49:09

by Xiubo Li-B47053

[permalink] [raw]
Subject: RE: [RFC][PATCHv5 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.

> > This adds the Document for Freescale FTM PWM driver under
> > Documentation/devicetree/bindings/pwm/.
> >
> > Signed-off-by: Xiubo Li <[email protected]>
> > ---
> > .../devicetree/bindings/pwm/pwm-fsl-ftm.txt | 34
> ++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> > b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> > new file mode 100644
> > index 0000000..175b762
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> > @@ -0,0 +1,34 @@
> > +Freescale FTM PWM controller
>
> What does FTM stand for, and can we spell out PWM at least once early on.
>

"FTM" is for short of "FlexTimer Module", I'll use the full name then.

> > +
> > +Required properties:
> > +- compatible: Should be "fsl,vf610-ftm-pwm"
> > +- reg: Physical base address and length of the controller's registers
> > +- #pwm-cells: Should be 3. See pwm.txt in this directory for a
> > +description of
> > + the cells format.
> > +- clock-names : Includes the following module clock source entries:
> > + "ftm0" (system clock),
> > + "ftm0_fix_sel" (fixed frequency clock),
> > + "ftm0_ext_sel" (external clock)
> > +- clocks : Must contain a clock specifier for each entry in
> > +clock-names,
> > + See ../clock/clock-bindings.txt for details of the property values.
> > +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be
> > +one of the
> > + entries in clock-names.
>
> Why do we need this, why not just have only the clock-names/clocks
> reference the clk that is actually used?
>

As I have replied before, the FTM has two clock sources: the module clock and the counter clock.

The counter clock source is selectable depends on different board and also the hardware design:
+++++
* FTM source clock is selectable
* Source clock can be the system clock, the fixed frequency clock, or an external
clock
* Fixed frequency clock is an additional clock input to allow the selection of an on
chip clock source other than the system clock
* Selecting external clock connects FTM clock to a chip level input pin therefore
allowing to synchronize the FTM counter with an off chip clock source
-----
>From the above description we can see that the external clock source can allow to synchronize the
FTM counter with an off chip clock source.

As the chip spec permits the counter clock source be selectable, so the different board maybe has different
implementation, if the driver do not support, this will be a bug.


> > +- pinctrl-names: must contain a "default" entry.
> > +- pinctrl-NNN: One property must exist for each entry in pinctrl-names.
> > + See ../pinctrl/pinctrl-bindings.txt for details of the property
> values.
>
> let's drop the .. when making directory references.
>

Is absolute path " Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt " or just "pinctrl/pinctrl-bindings.txt " ?


--
Xiubo


2013-10-29 08:09:59

by Kumar Gala

[permalink] [raw]
Subject: Re: [RFC][PATCHv5 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.


On Oct 28, 2013, at 10:48 PM, Xiubo Li-B47053 wrote:

>>> This adds the Document for Freescale FTM PWM driver under
>>> Documentation/devicetree/bindings/pwm/.
>>>
>>> Signed-off-by: Xiubo Li <[email protected]>
>>> ---
>>> .../devicetree/bindings/pwm/pwm-fsl-ftm.txt | 34
>> ++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
>>> b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
>>> new file mode 100644
>>> index 0000000..175b762
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
>>> @@ -0,0 +1,34 @@
>>> +Freescale FTM PWM controller
>>
>> What does FTM stand for, and can we spell out PWM at least once early on.
>>
>
> "FTM" is for short of "FlexTimer Module", I'll use the full name then.
>
>>> +
>>> +Required properties:
>>> +- compatible: Should be "fsl,vf610-ftm-pwm"
>>> +- reg: Physical base address and length of the controller's registers
>>> +- #pwm-cells: Should be 3. See pwm.txt in this directory for a
>>> +description of
>>> + the cells format.
>>> +- clock-names : Includes the following module clock source entries:
>>> + "ftm0" (system clock),
>>> + "ftm0_fix_sel" (fixed frequency clock),
>>> + "ftm0_ext_sel" (external clock)
>>> +- clocks : Must contain a clock specifier for each entry in
>>> +clock-names,
>>> + See ../clock/clock-bindings.txt for details of the property values.
>>> +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be
>>> +one of the
>>> + entries in clock-names.
>>
>> Why do we need this, why not just have only the clock-names/clocks
>> reference the clk that is actually used?
>>
>
> As I have replied before, the FTM has two clock sources: the module clock and the counter clock.
>
> The counter clock source is selectable depends on different board and also the hardware design:
> +++++
> * FTM source clock is selectable
> * Source clock can be the system clock, the fixed frequency clock, or an external
> clock
> * Fixed frequency clock is an additional clock input to allow the selection of an on
> chip clock source other than the system clock
> * Selecting external clock connects FTM clock to a chip level input pin therefore
> allowing to synchronize the FTM counter with an off chip clock source
> -----
> From the above description we can see that the external clock source can allow to synchronize the
> FTM counter with an off chip clock source.
>
> As the chip spec permits the counter clock source be selectable, so the different board maybe has different
> implementation, if the driver do not support, this will be a bug.

The binding specs the HW so what a driver does or doesn't support isn't relevant. Since I assume only one of these clocks makes sense for a given system I don't see a reason the binding should not just state that a given dts will have the one the system specified rather than all 3.

>>> +- pinctrl-names: must contain a "default" entry.
>>> +- pinctrl-NNN: One property must exist for each entry in pinctrl-names.
>>> + See ../pinctrl/pinctrl-bindings.txt for details of the property
>> values.
>>
>> let's drop the .. when making directory references.
>>
>
> Is absolute path " Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt " or just "pinctrl/pinctrl-bindings.txt " ?

The later.

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation