Re: [PATCH 1/2] pwm: document the pin state after pwm_disable() to be undefined

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

On Mon, Aug 20, 2018 at 04:43:56PM +0200, Uwe Kleine-König wrote:
> Contrarily to the common assumption that pwm_disable() stops the
> output at the state where it just happens to be, this isn't what the
> hardware does on some machines. For example on i.MX6 the pin goes to
> 0 level instead.
> 
> Note this doesn't reduce the expressiveness of the pwm-API because if
> you want the PWM-Pin to stay at a given state, you are supposed to
> configure it to 0% or 100% duty cycle without calling pwm_disable().
> The backend driver is free to implement potential power savings
> already when the duty cycle changes to one of these two cases so this
> doesn't come at an increased runtime power cost (once the respective
> drivers are fixed).
> 
> In return this allows to adhere to the API more easily for drivers that
> currently cannot know if it's ok to disable the hardware on
> pwm_disable() because the caller might or might not rely on the pin
> stopping at a certain level.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  Documentation/pwm.txt | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

I don't think this is correct. An API whose result is an undefined state
is useless in my opinion. So either we properly define what the state
should be after a disable operation, or we might as well just remove the
disable operation altogether.

From an API point of view I think it makes more sense to define the
state after disable to be inactive, that is high for normal PWM and low
for inversed PWM. The reason for this is that the disabled state is
effectively what the reset state of the PWM would be. If the PWM is
inverted, typically the board design has a pull-up somewhere to avoid a
backlight or LED from getting enabled by default. In all other cases a
PWM pin is likely just going to default to low when disabled.

This also matches what all other drivers, and users, assume.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]