linux-kernel - Re: [PATCHv4 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20130917103301.GC30088@pengutronix.de>
Date:	Tue, 17 Sep 2013 12:33:01 +0200
From:	Sascha Hauer <s.hauer@...gutronix.de>
To:	Xiubo Li <Li.Xiubo@...escale.com>
Cc:	r65073@...escale.com, thierry.reding@...il.com,
	swarren@...dotorg.org, t.figa@...sung.com, grant.likely@...aro.org,
	linux@....linux.org.uk, rob@...dley.net, ian.campbell@...rix.com,
	mark.rutland@....com, pawel.moll@....com, rob.herring@...xeda.com,
	linux-arm-kernel@...ts.infradead.org, linux-pwm@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org
Subject: Re: [PATCHv4 4/4] Documentation: Add device tree bindings for
 Freescale FTM PWM.

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@...escale.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

-- 
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 |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/