| Message ID | 1379051922-4930-5-git-send-email-Li.Xiubo@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/12/2013 11:58 PM, Xiubo Li wrote: > This adds the Document for Freescale FTM PWM driver under > Documentation/devicetree/bindings/pwm/. > diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > +Required properties: > +- 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 an entry list for entries in clock-names. s/an entry list/a clock specifier/ s/for entries/for each entry/ > +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be one of the > + entries in clock-names. So the IP block has 3 input clocks, and also a mux to select which one to use? That sounds slightly unusual, but possible. If there is really only 1 clock input to the IP block, and the mux is part of some other module, then this binding should have only 1 entry in clocks. > +- For each channel's pinctrl, the "chN-active" and "chN-idle" states should be > + implemented at the same time. I still don't believe that multiple pinctrl states active at once is something that the pinctrl bindings conceptually support. CC+=LinusW, do we want to allow this? Assuming this is allowed, you'd want to write something more like the following: pinctrl-names: Must include "chN-active" and "chN-idle" for each channel ID N in range 0..7. 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 = "ch0-active", "ch0-idle", "ch1-active", "ch1-idle", > + ....; > + pinctrl-0 = <&pinctrl_pwm0_ch0_active>; > + pinctrl-1 = <&pinctrl_pwm0_ch0_idle>; > + pinctrl-2 = <&pinctrl_pwm0_ch1_active>; > + pinctrl-3 = <&pinctrl_pwm0_ch1_idle>; > + ... > + fsl,pwm-counter-clk = "ftm0_ext_sel"; > +};
> > +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be > > +one of the > > + entries in clock-names. > > So the IP block has 3 input clocks, and also a mux to select which one to > use? That sounds slightly unusual, but possible. > > If there is really only 1 clock input to the IP block, and the mux is > part of some other module, then this binding should have only 1 entry in > clocks. > Yes, there are 3 input clocks that can be selectable, and the mux is inside the FTM IP block. Thanks. -- Best Regard, Xiubo
On Fri, Sep 13, 2013 at 01:58:42PM +0800, Xiubo Li wrote: > This adds the Document for Freescale FTM PWM driver under > Documentation/devicetree/bindings/pwm/. > > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com> > --- > .../devicetree/bindings/pwm/pwm-fsl-ftm.txt | 36 ++++++++++++++++++++++ > 1 file changed, 36 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..e736806 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > @@ -0,0 +1,36 @@ > +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 an entry list for entries in clock-names. > +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be one of the > + entries in clock-names. > +- For each channel's pinctrl, the "chN-active" and "chN-idle" states should be > + implemented at the same time. I thought about this active/idle pinctrl stuff again. It should be removed from the PWM driver. The above is used to control the PWM output state when the PWM is disabled, so that for example a backlight stays disabled after a call to pwm_disable. IMO this is wrong. Instead, the pwm client drivers (pwm_backlight) shouldn't assume a particular output state of the PWM after a pwm disable. Instead they should simply set the PWM to a duty cycle of 0% (or 100% for inverted) to effectively disable it. If then a PWM driver can optimize this to actually disabling the PWM completely, then fine, but that's an optimization. If the handoff of PWM pins from bootloader state to regular state causes problems like flashing backlights or LEDs due to the pinctrl framework configuring the iomux before the PWM driver takes over, then the pinctrl should be done from the client drivers (pwm_backlight, pwm_led), not from the pwm driver and not from the pinctrl framework before initializing the clients. So NACK to this complex pinctrl setup in this pwm driver. Sascha
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..e736806 --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt @@ -0,0 +1,36 @@ +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 an entry list for entries in clock-names. +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be one of the + entries in clock-names. +- For each channel's pinctrl, the "chN-active" and "chN-idle" states should be + implemented at the same time. + +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 = "ch0-active", "ch0-idle", "ch1-active", "ch1-idle", + ....; + pinctrl-0 = <&pinctrl_pwm0_ch0_active>; + pinctrl-1 = <&pinctrl_pwm0_ch0_idle>; + pinctrl-2 = <&pinctrl_pwm0_ch1_active>; + pinctrl-3 = <&pinctrl_pwm0_ch1_idle>; + ... + fsl,pwm-counter-clk = "ftm0_ext_sel"; +};
This adds the Document for Freescale FTM PWM driver under Documentation/devicetree/bindings/pwm/. Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com> --- .../devicetree/bindings/pwm/pwm-fsl-ftm.txt | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt