Re: [PATCH v3] pwm: imx: Let PWM be active during suspend

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

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

On Mon, Dec 11, 2017 at 08:54:19AM -0200, Fabio Estevam wrote:
> Hi Thierry,
> 
> On Mon, Dec 11, 2017 at 7:16 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> 
> > How does it break? Note that you have to change the polarity of the PWM
> > signal (in the pwms property) and then remove the "active-low" property
> > from the leds device tree node to avoid double inversion.
> 
> This is what I tried as per your suggestion:
> 
> --- a/arch/arm/boot/dts/imx6qdl-cubox-i.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-cubox-i.dtsi
> @@ -42,6 +42,7 @@
>  #include "imx6qdl-microsom-ar8035.dtsi"
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pwm/pwm.h>
> 
>  / {
>         ir_recv: ir-receiver {
> @@ -57,10 +58,9 @@
>                 pinctrl-0 = <&pinctrl_cubox_i_pwm1>;
> 
>                 front {
> -                       active-low;
>                         label = "imx6:red:front";
>                         max-brightness = <248>;
> -                       pwms = <&pwm1 0 50000>;
> +                       pwms = <&pwm1 0 50000 PWM_POLARITY_INVERTED>;
>                 };
>         };
> 
> @@ -231,6 +231,7 @@
>  };
> 
>  &pwm1 {
> +       #pwm-cells = <3>;
>         status = "okay";
>  };
> 
> The problem I see is that it seems to be an off-by-one calculation in
> the pwm-imx driver for the inverted polarity case.
> 
> For example:
> 
> # cd /sys/class/leds/imx6\:red\:front/
> # echo 0 > brightness (This keeps the LED on with its maximum brightness)
> # echo 1 > brightness (This keeps the LED off)
> # echo 248 > brightness (This keeps the LED on with its maximum brightness)
> 
> When I go to suspend:
> 
> # echo enabled > /sys/class/tty/ttymxc0/power/wakeup
> # echo mem > /sys/power/state
> 
> Then LED goes on with its maximum brightness.
> 
> So the issue now is that I need to tell the pwm-imx driver that '0'
> means to turn the LED off.

Yeah, my suggestion was to make setting the STOPEN bit dependent on the
polarity inversion setting of the PWM. That way you don't have to hard-
code it, which might not be the correct setting for non-inverted PWMs.

Thierry

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