[RCF PATCH,v2,1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO

Message ID 1539163920-9442-2-git-send-email-michal.vokac@ysoft.com
State Changes Requested
Headers show
Series
  • pwm: imx: Configure output to GPIO in disabled state
Related show

Commit Message

Vokáč Michal Oct. 10, 2018, 9:33 a.m.
Output of the PWM block on i.MX SoCs is always low when the block is
disabled. This can cause issues when inverted PWM polarity is needed.
With inverted polarity a duty cycle = 0% corresponds to high level on
the output. Now, when PWM is disabled its output instantly goes low
which corresponds to duty cycle = 100%.

To get a truly inverted PWM output two pinctrl states of the PWM pin
can be used. Configure the pin to GPIO function when PWM is disabled
and switch back to PWM function whenever non-zero duty cycle is needed.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
Changes in v2:
 - Do not use the "default" pinctrl state for GPIO.
 - Use two new "pwm" and "gpio" pinctrl states.
 - Add a new pwm-gpios signal.

 Documentation/devicetree/bindings/pwm/imx-pwm.txt | 51 +++++++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Thierry Reding Oct. 10, 2018, 1:39 p.m. | #1
On Wed, Oct 10, 2018 at 09:33:25AM +0000, Vokáč Michal wrote:
> Output of the PWM block on i.MX SoCs is always low when the block is
> disabled. This can cause issues when inverted PWM polarity is needed.
> With inverted polarity a duty cycle = 0% corresponds to high level on
> the output. Now, when PWM is disabled its output instantly goes low
> which corresponds to duty cycle = 100%.
> 
> To get a truly inverted PWM output two pinctrl states of the PWM pin
> can be used. Configure the pin to GPIO function when PWM is disabled
> and switch back to PWM function whenever non-zero duty cycle is needed.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
> Changes in v2:
>  - Do not use the "default" pinctrl state for GPIO.
>  - Use two new "pwm" and "gpio" pinctrl states.
>  - Add a new pwm-gpios signal.
> 
>  Documentation/devicetree/bindings/pwm/imx-pwm.txt | 51 +++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> index c61bdf8..bd5b6bd 100644
> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> @@ -14,6 +14,17 @@ See the clock consumer binding,
>  	Documentation/devicetree/bindings/clock/clock-bindings.txt
>  - interrupts: The interrupt for the pwm controller
>  
> +Optional properties:
> +- pinctrl: For i.MX27 and newer SoCs. Use "pwm" and "gpio" specific pinctrls
> +  instead of the "default" to configure the PWM pin to GPIO and PWM function.
> +  It allows control over the pin output level when the PWM block is disabled.
> +  This is useful if you use the PWM for single purpose and you need inverted
> +  polarity of the PWM signal. See "Inverted PWM output" section bellow.
> +- pwm-gpios: Specify the GPIO pin that will act as the PWM output. This should
> +  be the same pin as is used for normal PWM output. Define the pin as
> +  GPIO_ACTIVE_LOW to get HIGH level on the output when PWM is disabled. See
> +  "Inverted PWM output" section bellow.

It's somewhat unfortunate that we have to specify this in DT. For one
thing, we don't really want to use the pin as GPIO, we really only care
about whether it is "active" or "inactive". We also already specify the
GPIO in the pinctrl nodes, albeit via a different name. So we're
effectively duplicating information here. It'd be nice to avoid that.
Two possibilities that I could think about are:

  1) Do not explicitly rely on driving the GPIO as output: I know this
     was discussed before and it sounds like this is not an option for
     PWM because the GPIO may be configured as output by the firmware,
     and hence switching to GPIO mode may not give the expected result.
     I suppose one way to solve this is by using a gpio-hog entry for
     the PWM GPIO so that it will automatically get configured as an
     input and at the same time marked as busy so that nobody can go
     and just request it again (via sysfs for example).

  2) Derive the GPIO from the pin. I'm not sure there's anything in the
     pinctrl framework to do that. The reverse (GPIO -> pin) can be
     done, so perhaps this is something that could be added?

Other than than I think this looks very nice.

Thierry
Vokáč Michal Oct. 29, 2018, 3:52 p.m. | #2
On 10.10.2018 15:39, Thierry Reding wrote:
> On Wed, Oct 10, 2018 at 09:33:25AM +0000, Vokáč Michal wrote:
>> Output of the PWM block on i.MX SoCs is always low when the block is
>> disabled. This can cause issues when inverted PWM polarity is needed.
>> With inverted polarity a duty cycle = 0% corresponds to high level on
>> the output. Now, when PWM is disabled its output instantly goes low
>> which corresponds to duty cycle = 100%.
>>
>> To get a truly inverted PWM output two pinctrl states of the PWM pin
>> can be used. Configure the pin to GPIO function when PWM is disabled
>> and switch back to PWM function whenever non-zero duty cycle is needed.
>>
>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>> ---
>> Changes in v2:
>>   - Do not use the "default" pinctrl state for GPIO.
>>   - Use two new "pwm" and "gpio" pinctrl states.
>>   - Add a new pwm-gpios signal.
>>
>>   Documentation/devicetree/bindings/pwm/imx-pwm.txt | 51 +++++++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> index c61bdf8..bd5b6bd 100644
>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> @@ -14,6 +14,17 @@ See the clock consumer binding,
>>   	Documentation/devicetree/bindings/clock/clock-bindings.txt
>>   - interrupts: The interrupt for the pwm controller
>>   
>> +Optional properties:
>> +- pinctrl: For i.MX27 and newer SoCs. Use "pwm" and "gpio" specific pinctrls
>> +  instead of the "default" to configure the PWM pin to GPIO and PWM function.
>> +  It allows control over the pin output level when the PWM block is disabled.
>> +  This is useful if you use the PWM for single purpose and you need inverted
>> +  polarity of the PWM signal. See "Inverted PWM output" section bellow.
>> +- pwm-gpios: Specify the GPIO pin that will act as the PWM output. This should
>> +  be the same pin as is used for normal PWM output. Define the pin as
>> +  GPIO_ACTIVE_LOW to get HIGH level on the output when PWM is disabled. See
>> +  "Inverted PWM output" section bellow.
> 
> It's somewhat unfortunate that we have to specify this in DT. For one
> thing, we don't really want to use the pin as GPIO, we really only care
> about whether it is "active" or "inactive". We also already specify the
> GPIO in the pinctrl nodes, albeit via a different name. So we're
> effectively duplicating information here. It'd be nice to avoid that.

 From reading the pinctrl binding doc I understand that you can specify
as much pinctrl states as you need to configure a pin appropriately for
different "use cases". Eg. init, sleep, default, tx, rx ... as all might
need different pull-up/down and DSE configuration.

So I did not think this might be considered "duplicating information".

> Two possibilities that I could think about are:
> 
>    1) Do not explicitly rely on driving the GPIO as output: I know this
>       was discussed before and it sounds like this is not an option for
>       PWM because the GPIO may be configured as output by the firmware,
>       and hence switching to GPIO mode may not give the expected result.

I think it is actually possible. Previously I only saw two extremes.
No GPIO at all (only the pinctrl states) or full GPIO control including
driving the output. But once we request the GPIO as an input with

	devm_gpiod_get_optional(&pdev->dev, "pwm", GPIOD_IN);

it will be configured as input regardless what state it was left in from
bootloader. And hence the pull-up/down setting from DTS will be applied.

>       I suppose one way to solve this is by using a gpio-hog entry for
>       the PWM GPIO so that it will automatically get configured as an
>       input and at the same time marked as busy so that nobody can go
>       and just request it again (via sysfs for example).

I think this is still valid in the case I described above - nobody else
can request that pin once we acquire it in pwm-imx.

>    2) Derive the GPIO from the pin. I'm not sure there's anything in the
>       pinctrl framework to do that. The reverse (GPIO -> pin) can be
>       done, so perhaps this is something that could be added?

I can look at that option if you really want but I would like to avoid
doing that.

> Other than than I think this looks very nice.

Thanks. Would you prefer RFC v3 or non-RFC v1 when I respin?

Best regards,
Michal

Patch

diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
index c61bdf8..bd5b6bd 100644
--- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
@@ -14,6 +14,17 @@  See the clock consumer binding,
 	Documentation/devicetree/bindings/clock/clock-bindings.txt
 - interrupts: The interrupt for the pwm controller
 
+Optional properties:
+- pinctrl: For i.MX27 and newer SoCs. Use "pwm" and "gpio" specific pinctrls
+  instead of the "default" to configure the PWM pin to GPIO and PWM function.
+  It allows control over the pin output level when the PWM block is disabled.
+  This is useful if you use the PWM for single purpose and you need inverted
+  polarity of the PWM signal. See "Inverted PWM output" section bellow.
+- pwm-gpios: Specify the GPIO pin that will act as the PWM output. This should
+  be the same pin as is used for normal PWM output. Define the pin as
+  GPIO_ACTIVE_LOW to get HIGH level on the output when PWM is disabled. See
+  "Inverted PWM output" section bellow.
+
 Example:
 
 pwm1: pwm@53fb4000 {
@@ -25,3 +36,43 @@  pwm1: pwm@53fb4000 {
 	clock-names = "ipg", "per";
 	interrupts = <61>;
 };
+
+Inverted PWM output
+-------------------
+
+The i.MX SoC has such limitation that whenever a pin is configured as a PWM
+output, the output level on the pin is always low when the PWM block is
+disabled. The low output level is actively driven by the output stage of the
+PWM block and can not be overridden by pull-up. It also does not matter what
+PWM polarity a PWM client (e.g. backlight) requested.
+
+To gain control of the output level in PWM disabled state two pinctrl states
+can be used. A "gpio" state and a "pwm" state. In the gpio state the pin is
+configured as a GPIO. In the pwm state the pin is configured as a normal PWM
+output. In the gpio state the output level can be controlled by actively
+driving the output by the pwm-gpios signal. This setup assures that the PWM
+output is at the required level that corresponds to duty cycle = 0 when PWM
+is disabled.
+
+Example:
+
+&pwm1 {
+	pinctrl-names = "pwm", "gpio";
+	pinctrl-0 = <&pinctrl_backlight_pwm>;
+	pinctrl-1 = <&pinctrl_backlight_gpio>;
+	pwm-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>
+}
+
+pinctrl_backlight_gpio: pwm1grp-gpio {
+	fsl,pins = <
+		/* GPIO with 100kOhm pull-up */
+		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xb000
+	>;
+};
+
+pinctrl_backlight_pwm: pwm1grp-pwm {
+	fsl,pins = <
+		/* PWM output */
+		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
+	>;
+};