linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] pwm: imx: Configure output to GPIO in disabled state
@ 2018-08-21 14:38 Michal Vokáč
  2018-08-21 14:38 ` [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO Michal Vokáč
  2018-08-21 14:38 ` [RFC PATCH 2/2] pmw: imx: Configure output to GPIO in disabled state Michal Vokáč
  0 siblings, 2 replies; 21+ messages in thread
From: Michal Vokáč @ 2018-08-21 14:38 UTC (permalink / raw)
  To: Thierry Reding, Rob Herring
  Cc: Mark Rutland, devicetree, linux-pwm, linux-kernel,
	Lukasz Majewski, Fabio Estevam, Lothar Waßmann,
	Michal Vokáč

This is an attempt to deal with i.MX SoC PWM HW limitation.
When a pad is configured as a PWM output the output level is always 0V
if the PWM block is disabled. This can cause problems when inverted PWM
signal is needed to drive the connected circuit. With inverted output
duty cycle = 0% corresponds to high output level and duty cycle = 100%
corresponds to low output level. This means that whenever the PWM block is
disabled the connected circuit is fed with 100% duty cycle.

This issue has been discussed in various threads about inverted PWM support
implementation. Finally this commit 326ed314fefe ("pwm: imx: Add polarity
inversion support to i.MX's PWMv2") from Lukasz was merged.

Later on Fabio came up with the same problem as I described.
His commit 1f6eefeb7cd4 ("pwm: imx: Let PWM be active during suspend")
solves the problem only in suspend state and not whenever PWM is disabled.

In the discussion Fabio also suggested a pinctrl solution though it was
still only for suspend [1].

I would like to bring attention to that pinctrl solution once again.
The code is basically a copy of the I2C recovery function [2].

The binding is totally optional and current users are not affected.
The idea is to use the "default" pinctrl state to set the pin to a safe
state. That is a GPIO function with pull-up. Then when a PWM signal is
needed on the output, select the "pwm" state.

Using the GPIO function as a default state assures that the pad is in
a safe state all the time from power-up and is switched to PWM only when
it is really needed.

It is also important to first change the muxing and then disable the PWM.
Otherwise you will get unwanted level changes on the output. And vice
versa when PWM output needs to be enabled. Enable PWM first, then change
the muxing.

I would like to know your opinion on this, thanks.

Michal

[1] https://patchwork.ozlabs.org/patch/839834/#1819865
[2] https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-imx.c#L989


Michal Vokáč (2):
  dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  pmw: imx: Configure output to GPIO in disabled state

 Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 ++++++++++++++++++
 drivers/pwm/pwm-imx.c                             | 56 +++++++++++++++++++++++
 2 files changed, 100 insertions(+)

-- 
2.1.4


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-21 14:38 [RFC PATCH 0/2] pwm: imx: Configure output to GPIO in disabled state Michal Vokáč
@ 2018-08-21 14:38 ` Michal Vokáč
  2018-08-22  6:14   ` Lothar Waßmann
                     ` (2 more replies)
  2018-08-21 14:38 ` [RFC PATCH 2/2] pmw: imx: Configure output to GPIO in disabled state Michal Vokáč
  1 sibling, 3 replies; 21+ messages in thread
From: Michal Vokáč @ 2018-08-21 14:38 UTC (permalink / raw)
  To: Thierry Reding, Rob Herring
  Cc: Mark Rutland, devicetree, linux-pwm, linux-kernel,
	Lukasz Majewski, Fabio Estevam, Lothar Waßmann,
	Michal Vokáč

Output of the PWM block of i.MX SoCs is always zero volts when the block
is disabled. This can caue issues when inverted PWM polarity is needed.
With inverted polarity a duty cycle = 0% corresponds to solid high level
on the output. If the PWM is dissabled its output instantly goes to solid
zero which corresponds to duty cycle = 100%.

To have a trully inverted PWM output configure the PWM pad as a GPIO
with pull-up. Then switch the pad to PWM output whenever non-zero
duty cycle is needed.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
 Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
index c61bdf8..3b1bc4c 100644
--- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
@@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
+  pin to gpio function.  It allows control over the pin output level when the
+  PWM block is disabled. This is meant to be used if inverted polarity of the
+  PWM signal is required. See "Inverted PWM output" section bellow.
+
 Example:
 
 pwm1: pwm@53fb4000 {
@@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
 	clock-names = "ipg", "per";
 	interrupts = <61>;
 };
+
+Inverted PWM output
+-------------------
+
+The i.MX SoC has such limitation that whenever a pad is configured as a PWM
+output, the output level is always zero volts when the PWM block is disabled.
+The zero 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 PWM output level in disabled state two pinctrl states
+can be used. The "default" state and the "pwm" state. In the default state the
+PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
+is configured as a PWM output. This setup assures that the PWM output is at
+the required level that corresponds to duty cycle = 0 when PWM is disabled.
+E.g. at boot.
+
+Example:
+
+&pwm1 {
+	pinctrl-names = "default", "pwm";
+	pinctrl-0 = <&pinctrl_backlight_gpio>;
+	pinctrl-1 = <&pinctrl_backlight_pwm>;
+}
+
+pinctrl_backlight_gpio: pwm1grp-gpio {
+	fsl,pins = <
+		/* GPIO with 22kOhm pull-up */
+		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
+	>;
+};
+
+pinctrl_backlight_pwm: pwm1grp-pwm {
+	fsl,pins = <
+		/* PWM output */
+		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
+	>;
+};
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH 2/2] pmw: imx: Configure output to GPIO in disabled state
  2018-08-21 14:38 [RFC PATCH 0/2] pwm: imx: Configure output to GPIO in disabled state Michal Vokáč
  2018-08-21 14:38 ` [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO Michal Vokáč
@ 2018-08-21 14:38 ` Michal Vokáč
  1 sibling, 0 replies; 21+ messages in thread
From: Michal Vokáč @ 2018-08-21 14:38 UTC (permalink / raw)
  To: Thierry Reding, Rob Herring
  Cc: Mark Rutland, devicetree, linux-pwm, linux-kernel,
	Lukasz Majewski, Fabio Estevam, Lothar Waßmann,
	Michal Vokáč

The PWM output is held at zero whenever the PWM block is disabled.
This can cause issues when inverted PWM signal polarity is needed.

Allow users to set a "default" pinctrl state that will be used when
PWM is disabled and a "pwm" pinctrl state that will be used when
PWM output is needed.

As an example, with this a PWM controlled backlight with inversed
signal polarity can be used in full brightness range. Without this
the backlight could not be turned off as brightness = 0 disables
the PWM and that in turn set output to 0V, that is full brightness.

When the pinctrl states are not correctly specifid in DT the logic
will work as before.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
 drivers/pwm/pwm-imx.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 1d5242c..6212c98 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -13,6 +13,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/pwm.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -52,10 +53,39 @@ struct imx_chip {
 	void __iomem	*mmio_base;
 
 	struct pwm_chip	chip;
+
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pinctrl_pins_default;
+	struct pinctrl_state *pinctrl_pins_pwm;
 };
 
+
 #define to_imx_chip(chip)	container_of(chip, struct imx_chip, chip)
 
+static int imx_pwm_init_pinctrl_info(struct imx_chip *imx_chip,
+		struct platform_device *pdev)
+{
+	imx_chip->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (!imx_chip->pinctrl || IS_ERR(imx_chip->pinctrl)) {
+		dev_info(&pdev->dev, "can not get pinctrl\n");
+		return PTR_ERR(imx_chip->pinctrl);
+	}
+
+	imx_chip->pinctrl_pins_default = pinctrl_lookup_state(imx_chip->pinctrl,
+			PINCTRL_STATE_DEFAULT);
+	imx_chip->pinctrl_pins_pwm = pinctrl_lookup_state(imx_chip->pinctrl,
+			"pwm");
+
+	if (IS_ERR(imx_chip->pinctrl_pins_default) ||
+	    IS_ERR(imx_chip->pinctrl_pins_pwm)) {
+		dev_info(&pdev->dev, "PWM output mux information incomplete\n");
+		devm_pinctrl_put(imx_chip->pinctrl);
+		imx_chip->pinctrl = NULL;
+	}
+
+	return 0;
+}
+
 static int imx_pwm_config_v1(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns)
 {
@@ -216,7 +246,29 @@ static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
 			cr |= MX3_PWMCR_POUTC;
 
 		writel(cr, imx->mmio_base + MX3_PWMCR);
+
+		/*
+		 * If we are in charge of pinctrl then switch output to
+		 * the PWM signal.
+		 */
+		if (imx->pinctrl)
+			pinctrl_select_state(imx->pinctrl,
+					imx->pinctrl_pins_pwm);
 	} else if (cstate.enabled) {
+		/*
+		 * PWM block will be disabled. Normally its output will go
+		 * to 0 no matter what polarity is set. In case reversed PWM
+		 * polarity is required reconfigure the output pin to GPIO to
+		 * keep the output at the same level as for duty-cycle = 0.
+		 *
+		 * First switch the muxing and then disable the PWM. In that
+		 * order we do not get unwanted logic level changes on the
+		 * output.
+		 */
+		if (imx->pinctrl)
+			pinctrl_select_state(imx->pinctrl,
+					imx->pinctrl_pins_default);
+
 		writel(0, imx->mmio_base + MX3_PWMCR);
 
 		clk_disable_unprepare(imx->clk_per);
@@ -294,6 +346,10 @@ static int imx_pwm_probe(struct platform_device *pdev)
 		imx->chip.of_pwm_n_cells = 3;
 	}
 
+	ret = imx_pwm_init_pinctrl_info(imx, pdev);
+	if (ret)
+		return ret;
+
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
 	if (IS_ERR(imx->mmio_base))
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-21 14:38 ` [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO Michal Vokáč
@ 2018-08-22  6:14   ` Lothar Waßmann
  2018-08-22  7:01     ` Michal Vokáč
  2018-08-23 10:40   ` Lukasz Majewski
  2018-08-31 12:18   ` Rob Herring
  2 siblings, 1 reply; 21+ messages in thread
From: Lothar Waßmann @ 2018-08-22  6:14 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

Michal Vokáč <michal.vokac@ysoft.com> wrote:

> Output of the PWM block of i.MX SoCs is always zero volts when the block
> is disabled. This can caue issues when inverted PWM polarity is needed.
> With inverted polarity a duty cycle = 0% corresponds to solid high level
> on the output. If the PWM is dissabled its output instantly goes to solid
> zero which corresponds to duty cycle = 100%.
> 
> To have a trully inverted PWM output configure the PWM pad as a GPIO
> with pull-up. Then switch the pad to PWM output whenever non-zero
> duty cycle is needed.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
>  Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> index c61bdf8..3b1bc4c 100644
> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
> +  pin to gpio function.  It allows control over the pin output level when the
> +  PWM block is disabled. This is meant to be used if inverted polarity of the
> +  PWM signal is required. See "Inverted PWM output" section bellow.
> +
>  Example:
>  
>  pwm1: pwm@53fb4000 {
> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>  	clock-names = "ipg", "per";
>  	interrupts = <61>;
>  };
> +
> +Inverted PWM output
> +-------------------
> +
> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
> +output, the output level is always zero volts when the PWM block is disabled.
> +The zero 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 PWM output level in disabled state two pinctrl states
> +can be used. The "default" state and the "pwm" state. In the default state the
>
The "default" function of a PWM is to deliver a PWM signal. So it is
more sensible to me to have the PWM function as "default" and a "gpio"
function as alternative state.

> +PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
> +is configured as a PWM output. This setup assures that the PWM output is at
> +the required level that corresponds to duty cycle = 0 when PWM is disabled.
> +E.g. at boot.
> +
> +Example:
> +
> +&pwm1 {
> +	pinctrl-names = "default", "pwm";
> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
> +}
> +
> +pinctrl_backlight_gpio: pwm1grp-gpio {
> +	fsl,pins = <
> +		/* GPIO with 22kOhm pull-up */
> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
> +	>;
> +};
> +
> +pinctrl_backlight_pwm: pwm1grp-pwm {
> +	fsl,pins = <
> +		/* PWM output */
> +		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
> +	>;
> +};



-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-22  6:14   ` Lothar Waßmann
@ 2018-08-22  7:01     ` Michal Vokáč
  2018-08-22 11:17       ` Lothar Waßmann
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Vokáč @ 2018-08-22  7:01 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

On 22.8.2018 08:14, Lothar Waßmann wrote:
> Michal Vokáč <michal.vokac@ysoft.com> wrote:
> 
>> Output of the PWM block of i.MX SoCs is always zero volts when the block
>> is disabled. This can caue issues when inverted PWM polarity is needed.
>> With inverted polarity a duty cycle = 0% corresponds to solid high level
>> on the output. If the PWM is dissabled its output instantly goes to solid
>> zero which corresponds to duty cycle = 100%.
>>
>> To have a trully inverted PWM output configure the PWM pad as a GPIO
>> with pull-up. Then switch the pad to PWM output whenever non-zero
>> duty cycle is needed.
>>
>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>> ---
>>   Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> index c61bdf8..3b1bc4c 100644
>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
>> +  pin to gpio function.  It allows control over the pin output level when the
>> +  PWM block is disabled. This is meant to be used if inverted polarity of the
>> +  PWM signal is required. See "Inverted PWM output" section bellow.
>> +
>>   Example:
>>   
>>   pwm1: pwm@53fb4000 {
>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>>   	clock-names = "ipg", "per";
>>   	interrupts = <61>;
>>   };
>> +
>> +Inverted PWM output
>> +-------------------
>> +
>> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
>> +output, the output level is always zero volts when the PWM block is disabled.
>> +The zero 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 PWM output level in disabled state two pinctrl states
>> +can be used. The "default" state and the "pwm" state. In the default state the
>>
> The "default" function of a PWM is to deliver a PWM signal. So it is
> more sensible to me to have the PWM function as "default" and a "gpio"
> function as alternative state.

Yes, I totally agree that using "default" for PWM and "gpio" as the
alternative function seems more sensible. That is actually how I started.
Then I realized that that way you end up with the PWM pad set to zero
until the first call of imx_pwm_apply_v2 where you can select the GPIO
function. On my system that first call is made by pwm-backlight more than
3s after pinctrl init.

I suggested to use the "default" state as a GPIO function as the only way
how to get a truly inverted PWM output all the time from power-up to
power-down.

In my opinion it is up to the DT author what pad configuration he uses for
each pinctrl function as he knows what the HW really needs. I see that this
approach is kind of controversial but I hope that with good documentation
this would not be a problem. And as I wrote in the intro, it is absolutely
optional. If you do not need it, you do not use it.

>> +PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
>> +is configured as a PWM output. This setup assures that the PWM output is at
>> +the required level that corresponds to duty cycle = 0 when PWM is disabled.
>> +E.g. at boot.
>> +
>> +Example:
>> +
>> +&pwm1 {
>> +	pinctrl-names = "default", "pwm";
>> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
>> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
>> +}
>> +
>> +pinctrl_backlight_gpio: pwm1grp-gpio {
>> +	fsl,pins = <
>> +		/* GPIO with 22kOhm pull-up */
>> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
>> +	>;
>> +};
>> +
>> +pinctrl_backlight_pwm: pwm1grp-pwm {
>> +	fsl,pins = <
>> +		/* PWM output */
>> +		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
>> +	>;
>> +};
> 
> 
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-22  7:01     ` Michal Vokáč
@ 2018-08-22 11:17       ` Lothar Waßmann
  2018-08-22 13:20         ` Michal Vokáč
  0 siblings, 1 reply; 21+ messages in thread
From: Lothar Waßmann @ 2018-08-22 11:17 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

Michal Vokáč <michal.vokac@ysoft.com> wrote:

> On 22.8.2018 08:14, Lothar Waßmann wrote:
> > Michal Vokáč <michal.vokac@ysoft.com> wrote:
> >   
> >> Output of the PWM block of i.MX SoCs is always zero volts when the block
> >> is disabled. This can caue issues when inverted PWM polarity is needed.
> >> With inverted polarity a duty cycle = 0% corresponds to solid high level
> >> on the output. If the PWM is dissabled its output instantly goes to solid
> >> zero which corresponds to duty cycle = 100%.
> >>
> >> To have a trully inverted PWM output configure the PWM pad as a GPIO
> >> with pull-up. Then switch the pad to PWM output whenever non-zero
> >> duty cycle is needed.
> >>
> >> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> >> ---
> >>   Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
> >>   1 file changed, 44 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >> index c61bdf8..3b1bc4c 100644
> >> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
> >> +  pin to gpio function.  It allows control over the pin output level when the
> >> +  PWM block is disabled. This is meant to be used if inverted polarity of the
> >> +  PWM signal is required. See "Inverted PWM output" section bellow.
> >> +
> >>   Example:
> >>   
> >>   pwm1: pwm@53fb4000 {
> >> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
> >>   	clock-names = "ipg", "per";
> >>   	interrupts = <61>;
> >>   };
> >> +
> >> +Inverted PWM output
> >> +-------------------
> >> +
> >> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
> >> +output, the output level is always zero volts when the PWM block is disabled.
> >> +The zero 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 PWM output level in disabled state two pinctrl states
> >> +can be used. The "default" state and the "pwm" state. In the default state the
> >>  
> > The "default" function of a PWM is to deliver a PWM signal. So it is
> > more sensible to me to have the PWM function as "default" and a "gpio"
> > function as alternative state.  
> 
> Yes, I totally agree that using "default" for PWM and "gpio" as the
> alternative function seems more sensible. That is actually how I started.
> Then I realized that that way you end up with the PWM pad set to zero
> until the first call of imx_pwm_apply_v2 where you can select the GPIO
> function. On my system that first call is made by pwm-backlight more than
> 3s after pinctrl init.
> 
> I suggested to use the "default" state as a GPIO function as the only way
> how to get a truly inverted PWM output all the time from power-up to
> power-down.
> 
> In my opinion it is up to the DT author what pad configuration he uses for
> each pinctrl function as he knows what the HW really needs. I see that this
> approach is kind of controversial but I hope that with good documentation
> this would not be a problem. And as I wrote in the intro, it is absolutely
> optional. If you do not need it, you do not use it.
> 
This is OK so far.
But the approach with the pin being driven high via the pullup
configuration has a fundamental flaw:
The pwm polarity is specified by the PWM client (e.g: the pwm-backlight
driver:
	pwms = <&pwm0 0 PWM_POLARITY_INVERTED>;
)
The pinconfig is defined in the pinctrl of the PWM driver.

If you have clients that may use the same PWM instance and require
different polarity, there is no way to set the pullup/-down
configuration in accordance with the clients needs.

IMO the PWM driver should actively set the pin to the 'INACTIVE' state
according to the polarity specified by the current client using the PWM.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-22 11:17       ` Lothar Waßmann
@ 2018-08-22 13:20         ` Michal Vokáč
  2018-08-22 14:10           ` Lothar Waßmann
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Vokáč @ 2018-08-22 13:20 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

On 22.8.2018 13:17, Lothar Waßmann wrote:
> Michal Vokáč <michal.vokac@ysoft.com> wrote:
> 
>> On 22.8.2018 08:14, Lothar Waßmann wrote:
>>> Michal Vokáč <michal.vokac@ysoft.com> wrote:
>>>    
>>>> Output of the PWM block of i.MX SoCs is always zero volts when the block
>>>> is disabled. This can caue issues when inverted PWM polarity is needed.
>>>> With inverted polarity a duty cycle = 0% corresponds to solid high level
>>>> on the output. If the PWM is dissabled its output instantly goes to solid
>>>> zero which corresponds to duty cycle = 100%.
>>>>
>>>> To have a trully inverted PWM output configure the PWM pad as a GPIO
>>>> with pull-up. Then switch the pad to PWM output whenever non-zero
>>>> duty cycle is needed.
>>>>
>>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
>>>>    1 file changed, 44 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>> index c61bdf8..3b1bc4c 100644
>>>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
>>>> +  pin to gpio function.  It allows control over the pin output level when the
>>>> +  PWM block is disabled. This is meant to be used if inverted polarity of the
>>>> +  PWM signal is required. See "Inverted PWM output" section bellow.
>>>> +
>>>>    Example:
>>>>    
>>>>    pwm1: pwm@53fb4000 {
>>>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>>>>    	clock-names = "ipg", "per";
>>>>    	interrupts = <61>;
>>>>    };
>>>> +
>>>> +Inverted PWM output
>>>> +-------------------
>>>> +
>>>> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
>>>> +output, the output level is always zero volts when the PWM block is disabled.
>>>> +The zero 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 PWM output level in disabled state two pinctrl states
>>>> +can be used. The "default" state and the "pwm" state. In the default state the
>>>>   
>>> The "default" function of a PWM is to deliver a PWM signal. So it is
>>> more sensible to me to have the PWM function as "default" and a "gpio"
>>> function as alternative state.
>>
>> Yes, I totally agree that using "default" for PWM and "gpio" as the
>> alternative function seems more sensible. That is actually how I started.
>> Then I realized that that way you end up with the PWM pad set to zero
>> until the first call of imx_pwm_apply_v2 where you can select the GPIO
>> function. On my system that first call is made by pwm-backlight more than
>> 3s after pinctrl init.
>>
>> I suggested to use the "default" state as a GPIO function as the only way
>> how to get a truly inverted PWM output all the time from power-up to
>> power-down.
>>
>> In my opinion it is up to the DT author what pad configuration he uses for
>> each pinctrl function as he knows what the HW really needs. I see that this
>> approach is kind of controversial but I hope that with good documentation
>> this would not be a problem. And as I wrote in the intro, it is absolutely
>> optional. If you do not need it, you do not use it.
>>
> This is OK so far.
> But the approach with the pin being driven high via the pullup
> configuration has a fundamental flaw:
> The pwm polarity is specified by the PWM client (e.g: the pwm-backlight
> driver:
> 	pwms = <&pwm0 0 PWM_POLARITY_INVERTED>;
> )
> The pinconfig is defined in the pinctrl of the PWM driver.
> 
> If you have clients that may use the same PWM instance and require
> different polarity, there is no way to set the pullup/-down
> configuration in accordance with the clients needs.

Hmm, I did not think about more than one PWM client. Is it even possible
to design such board? Do you have an example of such usage? It would mean
that the single PWM output would be routed to more than one circuit.
E.g. a LED driver and a FAN controller. Those would be two separate
clients of the same PWM controller. Each of the clients requires different
PWM polarity and different frequency. In this case do not you also need
additional "enable" GPIO to enable/disable a client to ignore the PWM
signal that is meant for the other client?

IMO you have any additional option how to disable PWM clients, then you do
not need to care about the state of the PWM output in disabled state. If
you do not have that option and the PWM signal is the only one to control
the circuit then you can have only one client. But I may misunderstood
your point.
> 
> IMO the PWM driver should actively set the pin to the 'INACTIVE' state
> according to the polarity specified by the current client using the PWM.

This at least allows me to do what is currently not possible - to actually
disable the circuit by disabling PWM from the client. So I can potentially
live with that.

What I find distracting is the fact that the logic changes some time
during boot. You start with the circuit disabled from bootloader. When
pinctrl driver is initialized the circuit is enabled because the PWM pad
default function is PWM output and its state is zero volts when PWM is
disabled. Some time later, when a PWM client driver is initialized, first
call to the PWM controller driver is made to disable the PWM. PWM is
actually already disabled. What now? You check that even though PWM is
already disabled, the client operates with inverted polarity and the
pinctrl is not set accordingly. So you switch the pad to the GPIO function
and just now you really disabled the circuit.

So for a single client this seems unnecessarily late and for more clients
you would need other means to disable them anyway. But I am sure I may be
missing some important things.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-22 13:20         ` Michal Vokáč
@ 2018-08-22 14:10           ` Lothar Waßmann
  2018-08-23  9:13             ` Michal Vokáč
  0 siblings, 1 reply; 21+ messages in thread
From: Lothar Waßmann @ 2018-08-22 14:10 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

Michal Vokáč <michal.vokac@ysoft.com> wrote:

> On 22.8.2018 13:17, Lothar Waßmann wrote:
> > Michal Vokáč <michal.vokac@ysoft.com> wrote:
> >   
> >> On 22.8.2018 08:14, Lothar Waßmann wrote:  
> >>> Michal Vokáč <michal.vokac@ysoft.com> wrote:
> >>>      
> >>>> Output of the PWM block of i.MX SoCs is always zero volts when the block
> >>>> is disabled. This can caue issues when inverted PWM polarity is needed.
> >>>> With inverted polarity a duty cycle = 0% corresponds to solid high level
> >>>> on the output. If the PWM is dissabled its output instantly goes to solid
> >>>> zero which corresponds to duty cycle = 100%.
> >>>>
> >>>> To have a trully inverted PWM output configure the PWM pad as a GPIO
> >>>> with pull-up. Then switch the pad to PWM output whenever non-zero
> >>>> duty cycle is needed.
> >>>>
> >>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> >>>> ---
> >>>>    Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
> >>>>    1 file changed, 44 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >>>> index c61bdf8..3b1bc4c 100644
> >>>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >>>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >>>> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
> >>>> +  pin to gpio function.  It allows control over the pin output level when the
> >>>> +  PWM block is disabled. This is meant to be used if inverted polarity of the
> >>>> +  PWM signal is required. See "Inverted PWM output" section bellow.
> >>>> +
> >>>>    Example:
> >>>>    
> >>>>    pwm1: pwm@53fb4000 {
> >>>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
> >>>>    	clock-names = "ipg", "per";
> >>>>    	interrupts = <61>;
> >>>>    };
> >>>> +
> >>>> +Inverted PWM output
> >>>> +-------------------
> >>>> +
> >>>> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
> >>>> +output, the output level is always zero volts when the PWM block is disabled.
> >>>> +The zero 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 PWM output level in disabled state two pinctrl states
> >>>> +can be used. The "default" state and the "pwm" state. In the default state the
> >>>>     
> >>> The "default" function of a PWM is to deliver a PWM signal. So it is
> >>> more sensible to me to have the PWM function as "default" and a "gpio"
> >>> function as alternative state.  
> >>
> >> Yes, I totally agree that using "default" for PWM and "gpio" as the
> >> alternative function seems more sensible. That is actually how I started.
> >> Then I realized that that way you end up with the PWM pad set to zero
> >> until the first call of imx_pwm_apply_v2 where you can select the GPIO
> >> function. On my system that first call is made by pwm-backlight more than
> >> 3s after pinctrl init.
> >>
> >> I suggested to use the "default" state as a GPIO function as the only way
> >> how to get a truly inverted PWM output all the time from power-up to
> >> power-down.
> >>
> >> In my opinion it is up to the DT author what pad configuration he uses for
> >> each pinctrl function as he knows what the HW really needs. I see that this
> >> approach is kind of controversial but I hope that with good documentation
> >> this would not be a problem. And as I wrote in the intro, it is absolutely
> >> optional. If you do not need it, you do not use it.
> >>  
> > This is OK so far.
> > But the approach with the pin being driven high via the pullup
> > configuration has a fundamental flaw:
> > The pwm polarity is specified by the PWM client (e.g: the pwm-backlight
> > driver:
> > 	pwms = <&pwm0 0 PWM_POLARITY_INVERTED>;
> > )
> > The pinconfig is defined in the pinctrl of the PWM driver.
> > 
> > If you have clients that may use the same PWM instance and require
> > different polarity, there is no way to set the pullup/-down
> > configuration in accordance with the clients needs.  
> 
> Hmm, I did not think about more than one PWM client. Is it even possible
> to design such board? Do you have an example of such usage? It would mean
>
My use case is attaching different displays to the same baseboard,
where some displays have the brightness control pin inverted with
respect to the others. It's easy to change the compatible string for
the simple-panel driver and the PWM polarity setting for the
pwm-backlight driver from U-Boot according to the display model, but
it's not so easy, to edit the pinctrl settings from pull-up to
pull-down or vice versa.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-22 14:10           ` Lothar Waßmann
@ 2018-08-23  9:13             ` Michal Vokáč
  2018-08-23 11:18               ` Lothar Waßmann
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Vokáč @ 2018-08-23  9:13 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

On 22.8.2018 16:10, Lothar Waßmann wrote:
> Michal Vokáč <michal.vokac@ysoft.com> wrote:
> 
>> On 22.8.2018 13:17, Lothar Waßmann wrote:
>>> Michal Vokáč <michal.vokac@ysoft.com> wrote:
>>>    
>>>> On 22.8.2018 08:14, Lothar Waßmann wrote:
>>>>> Michal Vokáč <michal.vokac@ysoft.com> wrote:
>>>>>       
>>>>>> Output of the PWM block of i.MX SoCs is always zero volts when the block
>>>>>> is disabled. This can caue issues when inverted PWM polarity is needed.
>>>>>> With inverted polarity a duty cycle = 0% corresponds to solid high level
>>>>>> on the output. If the PWM is dissabled its output instantly goes to solid
>>>>>> zero which corresponds to duty cycle = 100%.
>>>>>>
>>>>>> To have a trully inverted PWM output configure the PWM pad as a GPIO
>>>>>> with pull-up. Then switch the pad to PWM output whenever non-zero
>>>>>> duty cycle is needed.
>>>>>>
>>>>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>>>>> ---
>>>>>>     Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
>>>>>>     1 file changed, 44 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>>>> index c61bdf8..3b1bc4c 100644
>>>>>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>>>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>>>> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
>>>>>> +  pin to gpio function.  It allows control over the pin output level when the
>>>>>> +  PWM block is disabled. This is meant to be used if inverted polarity of the
>>>>>> +  PWM signal is required. See "Inverted PWM output" section bellow.
>>>>>> +
>>>>>>     Example:
>>>>>>     
>>>>>>     pwm1: pwm@53fb4000 {
>>>>>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>>>>>>     	clock-names = "ipg", "per";
>>>>>>     	interrupts = <61>;
>>>>>>     };
>>>>>> +
>>>>>> +Inverted PWM output
>>>>>> +-------------------
>>>>>> +
>>>>>> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
>>>>>> +output, the output level is always zero volts when the PWM block is disabled.
>>>>>> +The zero 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 PWM output level in disabled state two pinctrl states
>>>>>> +can be used. The "default" state and the "pwm" state. In the default state the
>>>>>>      
>>>>> The "default" function of a PWM is to deliver a PWM signal. So it is
>>>>> more sensible to me to have the PWM function as "default" and a "gpio"
>>>>> function as alternative state.
>>>>
>>>> Yes, I totally agree that using "default" for PWM and "gpio" as the
>>>> alternative function seems more sensible. That is actually how I started.
>>>> Then I realized that that way you end up with the PWM pad set to zero
>>>> until the first call of imx_pwm_apply_v2 where you can select the GPIO
>>>> function. On my system that first call is made by pwm-backlight more than
>>>> 3s after pinctrl init.
>>>>
>>>> I suggested to use the "default" state as a GPIO function as the only way
>>>> how to get a truly inverted PWM output all the time from power-up to
>>>> power-down.
>>>>
>>>> In my opinion it is up to the DT author what pad configuration he uses for
>>>> each pinctrl function as he knows what the HW really needs. I see that this
>>>> approach is kind of controversial but I hope that with good documentation
>>>> this would not be a problem. And as I wrote in the intro, it is absolutely
>>>> optional. If you do not need it, you do not use it.
>>>>   
>>> This is OK so far.
>>> But the approach with the pin being driven high via the pullup
>>> configuration has a fundamental flaw:
>>> The pwm polarity is specified by the PWM client (e.g: the pwm-backlight
>>> driver:
>>> 	pwms = <&pwm0 0 PWM_POLARITY_INVERTED>;
>>> )
>>> The pinconfig is defined in the pinctrl of the PWM driver.
>>>
>>> If you have clients that may use the same PWM instance and require
>>> different polarity, there is no way to set the pullup/-down
>>> configuration in accordance with the clients needs.
>>
>> Hmm, I did not think about more than one PWM client. Is it even possible
>> to design such board? Do you have an example of such usage? It would mean
>>
> My use case is attaching different displays to the same baseboard,
> where some displays have the brightness control pin inverted with
> respect to the others. It's easy to change the compatible string for
> the simple-panel driver and the PWM polarity setting for the
> pwm-backlight driver from U-Boot according to the display model, but
> it's not so easy, to edit the pinctrl settings from pull-up to
> pull-down or vice versa.

OK, I got it. Though that is something different than having two clients,
right?

You do not actually need to change the pinctrl pull-up/down configuration
in bootloader. You define the two pinctrl groups as I suggested in the
example. Or more precisely, you add a new pinctrl group where the PWM
output pad is configured as a GPIO with pull-up. You add this group to
all your common device trees. This does no harm as the group is not used
yet.

In bootloader you detect the type of the panel. Normal PWM polarity? OK,
do nothing and boot. Inverted PWM polarity? Set the pinctrl-names property
to "default", "pwm". Set the pinctrl-0 property to point to the GPIO group
and set pinctrl-1 property to point to the old PWM group.

E.g. something like:

=> fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-names default pwm
=> fdt get value gpio-phandle /soc/aips-bus@2000000/iomuxc@20e0000/pwm1grp-gpio phandle
=> fdt get value pwm-phandle /soc/aips-bus@2000000/iomuxc@20e0000/pwm1grp-pwm phandle
=> fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-0 ${gpio-phandle}
=> fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-1 ${pwm-phandle}

Will this work for you?

Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-21 14:38 ` [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO Michal Vokáč
  2018-08-22  6:14   ` Lothar Waßmann
@ 2018-08-23 10:40   ` Lukasz Majewski
  2018-08-23 11:19     ` Lothar Waßmann
  2018-08-23 11:21     ` Michal Vokáč
  2018-08-31 12:18   ` Rob Herring
  2 siblings, 2 replies; 21+ messages in thread
From: Lukasz Majewski @ 2018-08-23 10:40 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Fabio Estevam, Lothar Waßmann

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

Hi Michal,

> Output of the PWM block of i.MX SoCs is always zero volts when the
> block is disabled. This can caue issues when inverted PWM polarity is
> needed. With inverted polarity a duty cycle = 0% corresponds to solid
> high level on the output. If the PWM is dissabled its output
> instantly goes to solid zero which corresponds to duty cycle = 100%.
> 
> To have a trully inverted PWM output configure the PWM pad as a GPIO
> with pull-up. Then switch the pad to PWM output whenever non-zero
> duty cycle is needed.

Just to ask - Is your display equipped with power supply enable/disable
pin?

As fair as I remember the trick to avoid flickering the display
was to disable the display (enable-gpio property) and set the PWM PIN
as GPIO to high in u-boot.

> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
>  Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44
> +++++++++++++++++++++++ 1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt index
> c61bdf8..3b1bc4c 100644 ---
> a/Documentation/devicetree/bindings/pwm/imx-pwm.txt +++
> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt @@ -14,6 +14,12
> @@ 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. Add extra pinctrl to configure
> the PWM
> +  pin to gpio function.  It allows control over the pin output level
> when the
> +  PWM block is disabled. This is meant to be used if inverted
> polarity of the
> +  PWM signal is required. See "Inverted PWM output" section bellow.
> +
>  Example:
>  
>  pwm1: pwm@53fb4000 {
> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>  	clock-names = "ipg", "per";
>  	interrupts = <61>;
>  };
> +
> +Inverted PWM output
> +-------------------
> +
> +The i.MX SoC has such limitation that whenever a pad is configured
> as a PWM +output, the output level is always zero volts when the PWM
> block is disabled. +The zero 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 PWM output level in disabled state two
> pinctrl states +can be used. The "default" state and the "pwm" state.
> In the default state the +PWM output is configured as a GPIO with
> pull-up. In the "pwm" state the output +is configured as a PWM
> output. This setup assures that the PWM output is at +the required
> level that corresponds to duty cycle = 0 when PWM is disabled. +E.g.
> at boot. +
> +Example:
> +
> +&pwm1 {
> +	pinctrl-names = "default", "pwm";
> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
> +}
> +
> +pinctrl_backlight_gpio: pwm1grp-gpio {
> +	fsl,pins = <
> +		/* GPIO with 22kOhm pull-up */
> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
> +	>;
> +};
> +
> +pinctrl_backlight_pwm: pwm1grp-pwm {
> +	fsl,pins = <
> +		/* PWM output */
> +		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
> +	>;
> +};




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-23  9:13             ` Michal Vokáč
@ 2018-08-23 11:18               ` Lothar Waßmann
  2018-08-23 12:38                 ` Michal Vokáč
  0 siblings, 1 reply; 21+ messages in thread
From: Lothar Waßmann @ 2018-08-23 11:18 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

Michal Vokáč <michal.vokac@ysoft.com> wrote:
> On 22.8.2018 16:10, Lothar Waßmann wrote:
> > My use case is attaching different displays to the same baseboard,
> > where some displays have the brightness control pin inverted with
> > respect to the others. It's easy to change the compatible string for
> > the simple-panel driver and the PWM polarity setting for the
> > pwm-backlight driver from U-Boot according to the display model, but
> > it's not so easy, to edit the pinctrl settings from pull-up to
> > pull-down or vice versa.  
> 
> OK, I got it. Though that is something different than having two clients,
> right?
>
> You do not actually need to change the pinctrl pull-up/down configuration
> in bootloader. You define the two pinctrl groups as I suggested in the
> example. Or more precisely, you add a new pinctrl group where the PWM
> output pad is configured as a GPIO with pull-up. You add this group to
> all your common device trees. This does no harm as the group is not used
> yet.
> 
> In bootloader you detect the type of the panel. Normal PWM polarity? OK,
> do nothing and boot. Inverted PWM polarity? Set the pinctrl-names property
> to "default", "pwm". Set the pinctrl-0 property to point to the GPIO group
> and set pinctrl-1 property to point to the old PWM group.
> 
> E.g. something like:
> 
> => fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-names default pwm
> => fdt get value gpio-phandle /soc/aips-bus@2000000/iomuxc@20e0000/pwm1grp-gpio phandle
> => fdt get value pwm-phandle /soc/aips-bus@2000000/iomuxc@20e0000/pwm1grp-pwm phandle
> => fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-0 ${gpio-phandle}
> => fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-1 ${pwm-phandle}  
> 
> Will this work for you?
> 
This would probably work, but it's quite ugly.
I'd still prefer to set the pin output state to the desired level
rather than relying on the chip internal pullup/pulldown facility.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-23 10:40   ` Lukasz Majewski
@ 2018-08-23 11:19     ` Lothar Waßmann
  2018-08-23 11:21     ` Michal Vokáč
  1 sibling, 0 replies; 21+ messages in thread
From: Lothar Waßmann @ 2018-08-23 11:19 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Michal Vokáč,
	Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Fabio Estevam

Lukasz Majewski <lukma@denx.de> wrote:

> Hi Michal,
> 
> > Output of the PWM block of i.MX SoCs is always zero volts when the
> > block is disabled. This can caue issues when inverted PWM polarity is
> > needed. With inverted polarity a duty cycle = 0% corresponds to solid
> > high level on the output. If the PWM is dissabled its output
> > instantly goes to solid zero which corresponds to duty cycle = 100%.
> > 
> > To have a trully inverted PWM output configure the PWM pad as a GPIO
> > with pull-up. Then switch the pad to PWM output whenever non-zero
> > duty cycle is needed.  
> 
> Just to ask - Is your display equipped with power supply enable/disable
> pin?
> 
> As fair as I remember the trick to avoid flickering the display
> was to disable the display (enable-gpio property) and set the PWM PIN
> as GPIO to high in u-boot.
> 
Unfortunately there are both types of displays. Some with an ENABLE
pin, some without.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-23 10:40   ` Lukasz Majewski
  2018-08-23 11:19     ` Lothar Waßmann
@ 2018-08-23 11:21     ` Michal Vokáč
  2018-08-23 12:36       ` Lukasz Majewski
  1 sibling, 1 reply; 21+ messages in thread
From: Michal Vokáč @ 2018-08-23 11:21 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Fabio Estevam, Lothar Waßmann

On 23.8.2018 12:40, Lukasz Majewski wrote:

Hi Lukasz, thanks for the reply!
> Hi Michal,
> 
>> Output of the PWM block of i.MX SoCs is always zero volts when the
>> block is disabled. This can caue issues when inverted PWM polarity is
>> needed. With inverted polarity a duty cycle = 0% corresponds to solid
>> high level on the output. If the PWM is dissabled its output
>> instantly goes to solid zero which corresponds to duty cycle = 100%.
>>
>> To have a trully inverted PWM output configure the PWM pad as a GPIO
>> with pull-up. Then switch the pad to PWM output whenever non-zero
>> duty cycle is needed.
> 
> Just to ask - Is your display equipped with power supply enable/disable
> pin?

No it is not. The backlight on my display is just a bunch of serial-
parallel connected LEDs with separate GND and VCC pins on a separate flex
cable. And the display itself also does not have a reset or enable signal.
It is a PITA to use it I must say..
  
> As fair as I remember the trick to avoid flickering the display
> was to disable the display (enable-gpio property) and set the PWM PIN
> as GPIO to high in u-boot.

Yes, I know about that. I can not use this as the PWM output is the only
signal I have to control the backlight. I mentioned that somewhere in the
previous discussion with Lothar.

I also think this could be useful not only for backlight. Any circuit that
requires truly inverted PWM signal can use it. I see it as an enhancement
to what you with Lothar have already done ;)
  
>>
>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>> ---
>>   Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44
>> +++++++++++++++++++++++ 1 file changed, 44 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt index
>> c61bdf8..3b1bc4c 100644 ---
>> a/Documentation/devicetree/bindings/pwm/imx-pwm.txt +++
>> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt @@ -14,6 +14,12
>> @@ 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. Add extra pinctrl to configure
>> the PWM
>> +  pin to gpio function.  It allows control over the pin output level
>> when the
>> +  PWM block is disabled. This is meant to be used if inverted
>> polarity of the
>> +  PWM signal is required. See "Inverted PWM output" section bellow.
>> +
>>   Example:
>>   
>>   pwm1: pwm@53fb4000 {
>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>>   	clock-names = "ipg", "per";
>>   	interrupts = <61>;
>>   };
>> +
>> +Inverted PWM output
>> +-------------------
>> +
>> +The i.MX SoC has such limitation that whenever a pad is configured
>> as a PWM +output, the output level is always zero volts when the PWM
>> block is disabled. +The zero 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 PWM output level in disabled state two
>> pinctrl states +can be used. The "default" state and the "pwm" state.
>> In the default state the +PWM output is configured as a GPIO with
>> pull-up. In the "pwm" state the output +is configured as a PWM
>> output. This setup assures that the PWM output is at +the required
>> level that corresponds to duty cycle = 0 when PWM is disabled. +E.g.
>> at boot. +
>> +Example:
>> +
>> +&pwm1 {
>> +	pinctrl-names = "default", "pwm";
>> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
>> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
>> +}
>> +
>> +pinctrl_backlight_gpio: pwm1grp-gpio {
>> +	fsl,pins = <
>> +		/* GPIO with 22kOhm pull-up */
>> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
>> +	>;
>> +};
>> +
>> +pinctrl_backlight_pwm: pwm1grp-pwm {
>> +	fsl,pins = <
>> +		/* PWM output */
>> +		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
>> +	>;
>> +};

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-23 11:21     ` Michal Vokáč
@ 2018-08-23 12:36       ` Lukasz Majewski
  2018-10-09 11:03         ` Vokáč Michal
  0 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2018-08-23 12:36 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Fabio Estevam, Lothar Waßmann

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

Hi Michal,

> On 23.8.2018 12:40, Lukasz Majewski wrote:
> 
> Hi Lukasz, thanks for the reply!
> > Hi Michal,
> >   
> >> Output of the PWM block of i.MX SoCs is always zero volts when the
> >> block is disabled. This can caue issues when inverted PWM polarity
> >> is needed. With inverted polarity a duty cycle = 0% corresponds to
> >> solid high level on the output. If the PWM is dissabled its output
> >> instantly goes to solid zero which corresponds to duty cycle =
> >> 100%.
> >>
> >> To have a trully inverted PWM output configure the PWM pad as a
> >> GPIO with pull-up. Then switch the pad to PWM output whenever
> >> non-zero duty cycle is needed.  
> > 
> > Just to ask - Is your display equipped with power supply
> > enable/disable pin?  
> 
> No it is not. The backlight on my display is just a bunch of serial-
> parallel connected LEDs with separate GND and VCC pins on a separate
> flex cable. And the display itself also does not have a reset or
> enable signal. It is a PITA to use it I must say..

Yes, it seems so. I must have had more luck than you with the HW....

>   
> > As fair as I remember the trick to avoid flickering the display
> > was to disable the display (enable-gpio property) and set the PWM
> > PIN as GPIO to high in u-boot.  
> 
> Yes, I know about that. I can not use this as the PWM output is the
> only signal I have to control the backlight. I mentioned that
> somewhere in the previous discussion with Lothar.

Yes, I've read it. I also find the PWM pinctrl as "default" state more
natural.

One more idea - though.

In iMX6Q it was possible to specify the pinctrl PIN setup as 0x80000000
- this means that it goes untouched to the IP block (configured by
  bootloader).

Maybe it would work to:

1. Setup the PWM output as GPIO in u-boot (high)

2. In PWM IMX probe configure PWM to be 100% duty cycle. And switch
iomux to PWM function of the pin

3. Then latter in the code PWM gets configured and we can control it in
"normal" way ?

Or am I missing some important point?

> 
> I also think this could be useful not only for backlight. Any circuit
> that requires truly inverted PWM signal can use it. I see it as an
> enhancement to what you with Lothar have already done ;)
>   
> >>
> >> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> >> ---
> >>   Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44
> >> +++++++++++++++++++++++ 1 file changed, 44 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt index
> >> c61bdf8..3b1bc4c 100644 ---
> >> a/Documentation/devicetree/bindings/pwm/imx-pwm.txt +++
> >> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt @@ -14,6 +14,12
> >> @@ 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. Add extra pinctrl to
> >> configure the PWM
> >> +  pin to gpio function.  It allows control over the pin output
> >> level when the
> >> +  PWM block is disabled. This is meant to be used if inverted
> >> polarity of the
> >> +  PWM signal is required. See "Inverted PWM output" section
> >> bellow. +
> >>   Example:
> >>   
> >>   pwm1: pwm@53fb4000 {
> >> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
> >>   	clock-names = "ipg", "per";
> >>   	interrupts = <61>;
> >>   };
> >> +
> >> +Inverted PWM output
> >> +-------------------
> >> +
> >> +The i.MX SoC has such limitation that whenever a pad is configured
> >> as a PWM +output, the output level is always zero volts when the
> >> PWM block is disabled. +The zero 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 PWM output level in disabled state two
> >> pinctrl states +can be used. The "default" state and the "pwm"
> >> state. In the default state the +PWM output is configured as a
> >> GPIO with pull-up. In the "pwm" state the output +is configured as
> >> a PWM output. This setup assures that the PWM output is at +the
> >> required level that corresponds to duty cycle = 0 when PWM is
> >> disabled. +E.g. at boot. +
> >> +Example:
> >> +
> >> +&pwm1 {
> >> +	pinctrl-names = "default", "pwm";
> >> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
> >> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
> >> +}
> >> +
> >> +pinctrl_backlight_gpio: pwm1grp-gpio {
> >> +	fsl,pins = <
> >> +		/* GPIO with 22kOhm pull-up */
> >> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
> >> +	>;
> >> +};
> >> +
> >> +pinctrl_backlight_pwm: pwm1grp-pwm {
> >> +	fsl,pins = <
> >> +		/* PWM output */
> >> +		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
> >> +	>;
> >> +};  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-23 11:18               ` Lothar Waßmann
@ 2018-08-23 12:38                 ` Michal Vokáč
  0 siblings, 0 replies; 21+ messages in thread
From: Michal Vokáč @ 2018-08-23 12:38 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

On 23.8.2018 13:18, Lothar Waßmann wrote:
> Michal Vokáč <michal.vokac@ysoft.com> wrote:
>> On 22.8.2018 16:10, Lothar Waßmann wrote:
>>> My use case is attaching different displays to the same baseboard,
>>> where some displays have the brightness control pin inverted with
>>> respect to the others. It's easy to change the compatible string for
>>> the simple-panel driver and the PWM polarity setting for the
>>> pwm-backlight driver from U-Boot according to the display model, but
>>> it's not so easy, to edit the pinctrl settings from pull-up to
>>> pull-down or vice versa.
>>
>> OK, I got it. Though that is something different than having two clients,
>> right?
>>
>> You do not actually need to change the pinctrl pull-up/down configuration
>> in bootloader. You define the two pinctrl groups as I suggested in the
>> example. Or more precisely, you add a new pinctrl group where the PWM
>> output pad is configured as a GPIO with pull-up. You add this group to
>> all your common device trees. This does no harm as the group is not used
>> yet.
>>
>> In bootloader you detect the type of the panel. Normal PWM polarity? OK,
>> do nothing and boot. Inverted PWM polarity? Set the pinctrl-names property
>> to "default", "pwm". Set the pinctrl-0 property to point to the GPIO group
>> and set pinctrl-1 property to point to the old PWM group.
>>
>> E.g. something like:
>>
>> => fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-names default pwm
>> => fdt get value gpio-phandle /soc/aips-bus@2000000/iomuxc@20e0000/pwm1grp-gpio phandle
>> => fdt get value pwm-phandle /soc/aips-bus@2000000/iomuxc@20e0000/pwm1grp-pwm phandle
>> => fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-0 ${gpio-phandle}
>> => fdt set /soc/aips-bus@2000000/pwm@2080000 pinctrl-1 ${pwm-phandle}
>>
>> Will this work for you?
>>
> This would probably work, but it's quite ugly.
> I'd still prefer to set the pin output state to the desired level
> rather than relying on the chip internal pullup/pulldown facility.

You mean like actively setting the pin HIGH or LOW using
gpiod_set_value_cansleep()? I also used that in my very first prototype.
But even for that you still need to define the second GPIO pinctrl group
and select it before you can set the GPIO value. I think because this is
all meant to be used only with inverted PWM output you naturally configure
the GPIO with pull-up. So you do not need to actively drive the pin.
Is your concern that the 22k pull-up may not be strong enough for any
connected circuit? IMO the whole point of totally disabling the PWM is to
save some power. So I will go with just the pull-up.

In your case you also need additional xxxx-gpios property that describes
the GPIO. Actually here you have one more option to invert the logic as
you can specify the GPIO as GPIO_ACTIVE_LOW or GPIO_ACTIVE_HIGH.
For inverted PWM output you should use GPIO_ACIVE_LOW. In your case you
will need to tweak that in the bootloader also.

The final DT fow inverted PWM for backlight will look like this:

         backlight {
                 compatible = "pwm-backlight";
                 pwms = <&pwm1 0 500000 PWM_POLARITY_INVERTED>;
                 brightness-levels = <0 32 64 128 255>;
                 default-brightness-level = <32>;
                 num-interpolated-steps = <8>;
                 power-supply = <&sw2_reg>;
                 status = "okay";
         };

	&pwm1 {
		#pwm-cells = <3>;
		pinctrl-names = "default", "pwm";
		pinctrl-0 = <&pinctrl_backlight_gpio>;
		pinctrl-1 = <&pinctrl_backlight_pwm>;
		pwm-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
		status = "disabled";
	};

	&iomuxc {
		pinctrl_backlight_gpio: pwm1grp-gpio {
			fsl,pins = <
				MX6QDL_PAD_GPIO_9__GPIO1_IO09   0x8
			>;
		};

		pinctrl_backlight_pwm: pwm1grp-pwm {
			fsl,pins = <
				MX6QDL_PAD_GPIO_9__PWM1_OUT     0x8
			>;
		};
	};

I think that for further effective discussion we need to wait for some
comments from PWM maintainers.

The main questions still remain:

- Is it acceptable to use pinctrl from the PWM driver in the first place?
- If so, is it acceptable to allow to use the approach where GPIO is used
   as the default state and we switch to "pwm" when needed? This produces
   the cleanest inverted PWM signal.
- If not, we will use the "gpio" state as the second optional one and use
   it when clients disable PWM.
- In both cases, can we rely on internal the pull-up facility or do we
   need to actively drive the pin?

Thank you for your time!
Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-21 14:38 ` [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO Michal Vokáč
  2018-08-22  6:14   ` Lothar Waßmann
  2018-08-23 10:40   ` Lukasz Majewski
@ 2018-08-31 12:18   ` Rob Herring
  2018-08-31 12:45     ` Lothar Waßmann
  2 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2018-08-31 12:18 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Thierry Reding, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam,
	Lothar Waßmann

On Tue, Aug 21, 2018 at 04:38:52PM +0200, Michal Vokáč wrote:
> Output of the PWM block of i.MX SoCs is always zero volts when the block
> is disabled. This can caue issues when inverted PWM polarity is needed.
> With inverted polarity a duty cycle = 0% corresponds to solid high level
> on the output. If the PWM is dissabled its output instantly goes to solid
> zero which corresponds to duty cycle = 100%.
> 
> To have a trully inverted PWM output configure the PWM pad as a GPIO
> with pull-up. Then switch the pad to PWM output whenever non-zero
> duty cycle is needed.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
>  Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> index c61bdf8..3b1bc4c 100644
> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
> +  pin to gpio function.  It allows control over the pin output level when the
> +  PWM block is disabled. This is meant to be used if inverted polarity of the
> +  PWM signal is required. See "Inverted PWM output" section bellow.
> +
>  Example:
>  
>  pwm1: pwm@53fb4000 {
> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>  	clock-names = "ipg", "per";
>  	interrupts = <61>;
>  };
> +
> +Inverted PWM output
> +-------------------
> +
> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
> +output, the output level is always zero volts when the PWM block is disabled.
> +The zero 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 PWM output level in disabled state two pinctrl states
> +can be used. The "default" state and the "pwm" state. In the default state the
> +PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
> +is configured as a PWM output. This setup assures that the PWM output is at
> +the required level that corresponds to duty cycle = 0 when PWM is disabled.
> +E.g. at boot.
> +
> +Example:
> +
> +&pwm1 {
> +	pinctrl-names = "default", "pwm";
> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
> +}
> +
> +pinctrl_backlight_gpio: pwm1grp-gpio {
> +	fsl,pins = <
> +		/* GPIO with 22kOhm pull-up */
> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008

There's a slight problem here if I remember the i.MX pin muxing. In GPIO 
mode, doesn't the GPIO block control the direction and level if an 
output. I guess as long as unused GPIOs are all initialized to inputs it 
will be okay.

Rob

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-31 12:18   ` Rob Herring
@ 2018-08-31 12:45     ` Lothar Waßmann
  2018-08-31 13:17       ` Michal Vokáč
  0 siblings, 1 reply; 21+ messages in thread
From: Lothar Waßmann @ 2018-08-31 12:45 UTC (permalink / raw)
  To: Rob Herring
  Cc: Michal Vokáč,
	Thierry Reding, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

Rob Herring <robh@kernel.org> wrote:

> On Tue, Aug 21, 2018 at 04:38:52PM +0200, Michal Vokáč wrote:
> > Output of the PWM block of i.MX SoCs is always zero volts when the block
> > is disabled. This can caue issues when inverted PWM polarity is needed.
> > With inverted polarity a duty cycle = 0% corresponds to solid high level
> > on the output. If the PWM is dissabled its output instantly goes to solid
> > zero which corresponds to duty cycle = 100%.
> > 
> > To have a trully inverted PWM output configure the PWM pad as a GPIO
> > with pull-up. Then switch the pad to PWM output whenever non-zero
> > duty cycle is needed.
> > 
> > Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> > ---
> >  Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> > index c61bdf8..3b1bc4c 100644
> > --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> > +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> > @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
> > +  pin to gpio function.  It allows control over the pin output level when the
> > +  PWM block is disabled. This is meant to be used if inverted polarity of the
> > +  PWM signal is required. See "Inverted PWM output" section bellow.
> > +
> >  Example:
> >  
> >  pwm1: pwm@53fb4000 {
> > @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
> >  	clock-names = "ipg", "per";
> >  	interrupts = <61>;
> >  };
> > +
> > +Inverted PWM output
> > +-------------------
> > +
> > +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
> > +output, the output level is always zero volts when the PWM block is disabled.
> > +The zero 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 PWM output level in disabled state two pinctrl states
> > +can be used. The "default" state and the "pwm" state. In the default state the
> > +PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
> > +is configured as a PWM output. This setup assures that the PWM output is at
> > +the required level that corresponds to duty cycle = 0 when PWM is disabled.
> > +E.g. at boot.
> > +
> > +Example:
> > +
> > +&pwm1 {
> > +	pinctrl-names = "default", "pwm";
> > +	pinctrl-0 = <&pinctrl_backlight_gpio>;
> > +	pinctrl-1 = <&pinctrl_backlight_pwm>;
> > +}
> > +
> > +pinctrl_backlight_gpio: pwm1grp-gpio {
> > +	fsl,pins = <
> > +		/* GPIO with 22kOhm pull-up */
> > +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008  
> 
> There's a slight problem here if I remember the i.MX pin muxing. In GPIO 
> mode, doesn't the GPIO block control the direction and level if an 
> output. I guess as long as unused GPIOs are all initialized to inputs it 
> will be okay.
> 
One could set the pad_ctl DSE value to 0, so that the pin cannot be
driven even if configured as output:
		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF000


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-31 12:45     ` Lothar Waßmann
@ 2018-08-31 13:17       ` Michal Vokáč
  2018-08-31 13:30         ` Lothar Waßmann
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Vokáč @ 2018-08-31 13:17 UTC (permalink / raw)
  To: Lothar Waßmann, Rob Herring
  Cc: Thierry Reding, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

On 31.8.2018 14:45, Lothar Waßmann wrote:
> Rob Herring <robh@kernel.org> wrote:
> 
>> On Tue, Aug 21, 2018 at 04:38:52PM +0200, Michal Vokáč wrote:
>>> Output of the PWM block of i.MX SoCs is always zero volts when the block
>>> is disabled. This can caue issues when inverted PWM polarity is needed.
>>> With inverted polarity a duty cycle = 0% corresponds to solid high level
>>> on the output. If the PWM is dissabled its output instantly goes to solid
>>> zero which corresponds to duty cycle = 100%.
>>>
>>> To have a trully inverted PWM output configure the PWM pad as a GPIO
>>> with pull-up. Then switch the pad to PWM output whenever non-zero
>>> duty cycle is needed.
>>>
>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>> ---
>>>   Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
>>>   1 file changed, 44 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>> index c61bdf8..3b1bc4c 100644
>>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
>>> +  pin to gpio function.  It allows control over the pin output level when the
>>> +  PWM block is disabled. This is meant to be used if inverted polarity of the
>>> +  PWM signal is required. See "Inverted PWM output" section bellow.
>>> +
>>>   Example:
>>>   
>>>   pwm1: pwm@53fb4000 {
>>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>>>   	clock-names = "ipg", "per";
>>>   	interrupts = <61>;
>>>   };
>>> +
>>> +Inverted PWM output
>>> +-------------------
>>> +
>>> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
>>> +output, the output level is always zero volts when the PWM block is disabled.
>>> +The zero 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 PWM output level in disabled state two pinctrl states
>>> +can be used. The "default" state and the "pwm" state. In the default state the
>>> +PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
>>> +is configured as a PWM output. This setup assures that the PWM output is at
>>> +the required level that corresponds to duty cycle = 0 when PWM is disabled.
>>> +E.g. at boot.
>>> +
>>> +Example:
>>> +
>>> +&pwm1 {
>>> +	pinctrl-names = "default", "pwm";
>>> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
>>> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
>>> +}
>>> +
>>> +pinctrl_backlight_gpio: pwm1grp-gpio {
>>> +	fsl,pins = <
>>> +		/* GPIO with 22kOhm pull-up */
>>> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
>>
>> There's a slight problem here if I remember the i.MX pin muxing. In GPIO
>> mode, doesn't the GPIO block control the direction and level if an
>> output. I guess as long as unused GPIOs are all initialized to inputs it
>> will be okay.

I am not sure if I understand you correctly. Did you mean: "..doesn't the
GPIO block control the PULL-UP/DOWN and level if an output."? Yes, that is
true. And as you said, all GPIOs are configured as inputs after reset.

> One could set the pad_ctl DSE value to 0, so that the pin cannot be
> driven even if configured as output:
> 		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF000

Yes, it will make no harm to set the pin to high-Z if configured as
output. Though I am not sure that this makes sense.

In case we choose the pull-up to keep the level high the pin needs to stay
configured as input. And as the GPIO is reserved for us there is actually
no one else who could re-configure it.

In case we choose to actively drive the pin instead of relying on the
internal pull-up we need to use gpiod lib and configure the pin as output.
In that case DSE must be set non-zero.

Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-31 13:17       ` Michal Vokáč
@ 2018-08-31 13:30         ` Lothar Waßmann
  2018-10-09 10:30           ` Vokáč Michal
  0 siblings, 1 reply; 21+ messages in thread
From: Lothar Waßmann @ 2018-08-31 13:30 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Rob Herring, Thierry Reding, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

Michal Vokáč <michal.vokac@ysoft.com> wrote:

> On 31.8.2018 14:45, Lothar Waßmann wrote:
> > Rob Herring <robh@kernel.org> wrote:
> >   
> >> On Tue, Aug 21, 2018 at 04:38:52PM +0200, Michal Vokáč wrote:  
> >>> Output of the PWM block of i.MX SoCs is always zero volts when the block
> >>> is disabled. This can caue issues when inverted PWM polarity is needed.
> >>> With inverted polarity a duty cycle = 0% corresponds to solid high level
> >>> on the output. If the PWM is dissabled its output instantly goes to solid
> >>> zero which corresponds to duty cycle = 100%.
> >>>
> >>> To have a trully inverted PWM output configure the PWM pad as a GPIO
> >>> with pull-up. Then switch the pad to PWM output whenever non-zero
> >>> duty cycle is needed.
> >>>
> >>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> >>> ---
> >>>   Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
> >>>   1 file changed, 44 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >>> index c61bdf8..3b1bc4c 100644
> >>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> >>> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
> >>> +  pin to gpio function.  It allows control over the pin output level when the
> >>> +  PWM block is disabled. This is meant to be used if inverted polarity of the
> >>> +  PWM signal is required. See "Inverted PWM output" section bellow.
> >>> +
> >>>   Example:
> >>>   
> >>>   pwm1: pwm@53fb4000 {
> >>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
> >>>   	clock-names = "ipg", "per";
> >>>   	interrupts = <61>;
> >>>   };
> >>> +
> >>> +Inverted PWM output
> >>> +-------------------
> >>> +
> >>> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
> >>> +output, the output level is always zero volts when the PWM block is disabled.
> >>> +The zero 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 PWM output level in disabled state two pinctrl states
> >>> +can be used. The "default" state and the "pwm" state. In the default state the
> >>> +PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
> >>> +is configured as a PWM output. This setup assures that the PWM output is at
> >>> +the required level that corresponds to duty cycle = 0 when PWM is disabled.
> >>> +E.g. at boot.
> >>> +
> >>> +Example:
> >>> +
> >>> +&pwm1 {
> >>> +	pinctrl-names = "default", "pwm";
> >>> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
> >>> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
> >>> +}
> >>> +
> >>> +pinctrl_backlight_gpio: pwm1grp-gpio {
> >>> +	fsl,pins = <
> >>> +		/* GPIO with 22kOhm pull-up */
> >>> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008  
> >>
> >> There's a slight problem here if I remember the i.MX pin muxing. In GPIO
> >> mode, doesn't the GPIO block control the direction and level if an
> >> output. I guess as long as unused GPIOs are all initialized to inputs it
> >> will be okay.  
> 
> I am not sure if I understand you correctly. Did you mean: "..doesn't the
> GPIO block control the PULL-UP/DOWN and level if an output."? Yes, that is
> true. And as you said, all GPIOs are configured as inputs after reset.
> 
> > One could set the pad_ctl DSE value to 0, so that the pin cannot be
> > driven even if configured as output:
> > 		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF000  
> 
> Yes, it will make no harm to set the pin to high-Z if configured as
> output. Though I am not sure that this makes sense.
> 
If you want to rely on the function of the Pull resistors this is
exactly what you need.

> In case we choose the pull-up to keep the level high the pin needs to stay
> configured as input. And as the GPIO is reserved for us there is actually
> no one else who could re-configure it.
> 
U-Boot may have configured the PWM pin as output to enable the
backlight without brightness control.

> In case we choose to actively drive the pin instead of relying on the
> internal pull-up we need to use gpiod lib and configure the pin as output.
> In that case DSE must be set non-zero.
> 
That is my personal preference too.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-31 13:30         ` Lothar Waßmann
@ 2018-10-09 10:30           ` Vokáč Michal
  0 siblings, 0 replies; 21+ messages in thread
From: Vokáč Michal @ 2018-10-09 10:30 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Rob Herring, Thierry Reding, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Lukasz Majewski, Fabio Estevam

    On 31.8.2018 15:30, Lothar Waßmann wrote:
> Michal Vokáč <michal.vokac@ysoft.com> wrote:
> 
>> On 31.8.2018 14:45, Lothar Waßmann wrote:
>>> Rob Herring <robh@kernel.org> wrote:
>>>    
>>>> On Tue, Aug 21, 2018 at 04:38:52PM +0200, Michal Vokáč wrote:
>>>>> Output of the PWM block of i.MX SoCs is always zero volts when the block
>>>>> is disabled. This can caue issues when inverted PWM polarity is needed.
>>>>> With inverted polarity a duty cycle = 0% corresponds to solid high level
>>>>> on the output. If the PWM is dissabled its output instantly goes to solid
>>>>> zero which corresponds to duty cycle = 100%.
>>>>>
>>>>> To have a trully inverted PWM output configure the PWM pad as a GPIO
>>>>> with pull-up. Then switch the pad to PWM output whenever non-zero
>>>>> duty cycle is needed.

Ahoj,
Sorry for the long delay. I spent most of the time learning more about
the pinctr, pwm and clock subsystem internals. I have also done a lot
of experiments and measurements of various PWM setups and use-cases.

The number of possible use-cases is really huge and I doubt it is possible
to address all of them. So I would like to address at least some of them.

Cases I have on my mind fall into these two groups:

1) A single purpose HW that needs inverted PWM signal. With PWM client
in the kernel. Eg. a board with PWM controlled LCD backlight. The client
should be able to enable/disable the PWM with expected results.
Enabled = bright, disabled = dark.

2) A more generic HW that still needs inverted PWM signal. No PWM client
in the kernel. Eg. a board that controls some technology using only a PWM
signal. That technology needs to stay disabled unless some process from
user space enables the PWM.

Users that need normal, non-inverted PWM can happily use current binding.
The same for cases with totally generic PWM output with no specific usage.

More comments bellow.

>>>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>>>> ---
>>>>>    Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44 +++++++++++++++++++++++
>>>>>    1 file changed, 44 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>>> index c61bdf8..3b1bc4c 100644
>>>>> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>>> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>>> @@ -14,6 +14,12 @@ 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. Add extra pinctrl to configure the PWM
>>>>> +  pin to gpio function.  It allows control over the pin output level when the
>>>>> +  PWM block is disabled. This is meant to be used if inverted polarity of the
>>>>> +  PWM signal is required. See "Inverted PWM output" section bellow.
>>>>> +
>>>>>    Example:
>>>>>    
>>>>>    pwm1: pwm@53fb4000 {
>>>>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>>>>>    	clock-names = "ipg", "per";
>>>>>    	interrupts = <61>;
>>>>>    };
>>>>> +
>>>>> +Inverted PWM output
>>>>> +-------------------
>>>>> +
>>>>> +The i.MX SoC has such limitation that whenever a pad is configured as a PWM
>>>>> +output, the output level is always zero volts when the PWM block is disabled.
>>>>> +The zero 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 PWM output level in disabled state two pinctrl states
>>>>> +can be used. The "default" state and the "pwm" state. In the default state the
>>>>> +PWM output is configured as a GPIO with pull-up. In the "pwm" state the output
>>>>> +is configured as a PWM output. This setup assures that the PWM output is at
>>>>> +the required level that corresponds to duty cycle = 0 when PWM is disabled.
>>>>> +E.g. at boot.
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +&pwm1 {
>>>>> +	pinctrl-names = "default", "pwm";
>>>>> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
>>>>> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
>>>>> +}
>>>>> +
>>>>> +pinctrl_backlight_gpio: pwm1grp-gpio {
>>>>> +	fsl,pins = <
>>>>> +		/* GPIO with 22kOhm pull-up */
>>>>> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
>>>>
>>>> There's a slight problem here if I remember the i.MX pin muxing. In GPIO
>>>> mode, doesn't the GPIO block control the direction and level if an
>>>> output. I guess as long as unused GPIOs are all initialized to inputs it
>>>> will be okay.
>>
>> I am not sure if I understand you correctly. Did you mean: "..doesn't the
>> GPIO block control the PULL-UP/DOWN and level if an output."? Yes, that is
>> true. And as you said, all GPIOs are configured as inputs after reset.
>>
>>> One could set the pad_ctl DSE value to 0, so that the pin cannot be
>>> driven even if configured as output:
>>> 		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF000
>>
>> Yes, it will make no harm to set the pin to high-Z if configured as
>> output. Though I am not sure that this makes sense.
>>
> If you want to rely on the function of the Pull resistors this is
> exactly what you need.

I think I got your point. Just setting the pull resistors from DT does
not assure the output level. As you noted below - the bootloader may have
configured the pin as output. Then the pull resistor will have no effect.

>> In case we choose the pull-up to keep the level high the pin needs to stay
>> configured as input. And as the GPIO is reserved for us there is actually
>> no one else who could re-configure it.
>>
> U-Boot may have configured the PWM pin as output to enable the
> backlight without brightness control.

I see I used wrong assumption. The pin may not have its default (input,
100k pull-up) configuration when Linux is booting.

So once we request the GPIO from driver code (either as input or output)
we have full control over the output level. In input mode with the pull
resistors and in output mode we can set the output value.

>> In case we choose to actively drive the pin instead of relying on the
>> internal pull-up we need to use gpiod lib and configure the pin as output.
>> In that case DSE must be set non-zero.
>>
> That is my personal preference too.

OK, lets do it that way. It seems like the most reliable solution.

Also I came up with another idea to make the binding less confusing.
Lets not misuse the "default" pinctrl state for GPIO function. Instead we
can define two separate "pwm" and "gpio" states and switch between those.

Why I am not happy with just the "default" state for PWM and "gpio" state
for GPIO output is that the "default" state is automagically selected by
the pinctrl driver at pwm-imx driver probe. And that will cause some level
changes on the output.

I will send v2 with all the changes soon.

As a side effect I also find out that the current driver does not support
HW state readout. If you enable the PWM in bootloader, Linux does not know
about it. Hence the clock subsystem (imx-gate2) disables the PWM source
clock as it seems unused. As a result the PWM block is left enabled with
disabled source clock and the PWM output is stopped at random level. That
is not really good.

So in the meantime I submitted a series [1] that implement the get_state()
function.

[1] http://patchwork.ozlabs.org/project/linux-pwm/list/?series=68445

Best regards,
Michal


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO
  2018-08-23 12:36       ` Lukasz Majewski
@ 2018-10-09 11:03         ` Vokáč Michal
  0 siblings, 0 replies; 21+ messages in thread
From: Vokáč Michal @ 2018-10-09 11:03 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, Rob Herring, Mark Rutland, devicetree, linux-pwm,
	linux-kernel, Fabio Estevam, Lothar Waßmann

On 23.8.2018 14:36, Lukasz Majewski wrote:
> Hi Michal,
> 
>> On 23.8.2018 12:40, Lukasz Majewski wrote:
>>
>> Hi Lukasz, thanks for the reply!
>>> Hi Michal,
>>>    
>>>> Output of the PWM block of i.MX SoCs is always zero volts when the
>>>> block is disabled. This can caue issues when inverted PWM polarity
>>>> is needed. With inverted polarity a duty cycle = 0% corresponds to
>>>> solid high level on the output. If the PWM is dissabled its output
>>>> instantly goes to solid zero which corresponds to duty cycle =
>>>> 100%.
>>>>
>>>> To have a trully inverted PWM output configure the PWM pad as a
>>>> GPIO with pull-up. Then switch the pad to PWM output whenever
>>>> non-zero duty cycle is needed.
>>>
>>> Just to ask - Is your display equipped with power supply
>>> enable/disable pin?
>>
>> No it is not. The backlight on my display is just a bunch of serial-
>> parallel connected LEDs with separate GND and VCC pins on a separate
>> flex cable. And the display itself also does not have a reset or
>> enable signal. It is a PITA to use it I must say..
> 
> Yes, it seems so. I must have had more luck than you with the HW....
> 
>>    
>>> As fair as I remember the trick to avoid flickering the display
>>> was to disable the display (enable-gpio property) and set the PWM
>>> PIN as GPIO to high in u-boot.
>>
>> Yes, I know about that. I can not use this as the PWM output is the
>> only signal I have to control the backlight. I mentioned that
>> somewhere in the previous discussion with Lothar.
> 
> Yes, I've read it. I also find the PWM pinctrl as "default" state more
> natural.
> 
> One more idea - though.
> 
> In iMX6Q it was possible to specify the pinctrl PIN setup as 0x80000000
> - this means that it goes untouched to the IP block (configured by
>    bootloader).

Hi Lukasz,
Thank you for the ideas and sorry for the long delay.

 From reading a lot of replies from (not only) DT maintainers I got the
impression that using the 0x80000000 configuration value is generally
discouraged. And I support that idea.

> Maybe it would work to:
> 
> 1. Setup the PWM output as GPIO in u-boot (high)
> 
> 2. In PWM IMX probe configure PWM to be 100% duty cycle. And switch
> iomux to PWM function of the pin

The probe function should only prepare structures, not configure the HW.
Also you somehow need to know whether or not to enable the PWM/GPIO.
Normally it is the client (backlight, sysfs..) who calls the pwm_apply to
configure and enable/disable the PWM.

> 3. Then latter in the code PWM gets configured and we can control it in
> "normal" way ?
> 
> Or am I missing some important point?

Ideally, the final solution should allow all possible use-cases:

- inverted PWM, enabled from bootloader to userspace
- inverted PWM, disabled from bootloader to userspace

The same for non-inverted PWM.

What you are suggesting does not seem like it supports that idea.

I will send v2 with some proposed changes soon.

>>
>> I also think this could be useful not only for backlight. Any circuit
>> that requires truly inverted PWM signal can use it. I see it as an
>> enhancement to what you with Lothar have already done ;)
>>    
>>>>
>>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/pwm/imx-pwm.txt | 44
>>>> +++++++++++++++++++++++ 1 file changed, 44 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
>>>> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt index
>>>> c61bdf8..3b1bc4c 100644 ---
>>>> a/Documentation/devicetree/bindings/pwm/imx-pwm.txt +++
>>>> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt @@ -14,6 +14,12
>>>> @@ 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. Add extra pinctrl to
>>>> configure the PWM
>>>> +  pin to gpio function.  It allows control over the pin output
>>>> level when the
>>>> +  PWM block is disabled. This is meant to be used if inverted
>>>> polarity of the
>>>> +  PWM signal is required. See "Inverted PWM output" section
>>>> bellow. +
>>>>    Example:
>>>>    
>>>>    pwm1: pwm@53fb4000 {
>>>> @@ -25,3 +31,41 @@ pwm1: pwm@53fb4000 {
>>>>    	clock-names = "ipg", "per";
>>>>    	interrupts = <61>;
>>>>    };
>>>> +
>>>> +Inverted PWM output
>>>> +-------------------
>>>> +
>>>> +The i.MX SoC has such limitation that whenever a pad is configured
>>>> as a PWM +output, the output level is always zero volts when the
>>>> PWM block is disabled. +The zero 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 PWM output level in disabled state two
>>>> pinctrl states +can be used. The "default" state and the "pwm"
>>>> state. In the default state the +PWM output is configured as a
>>>> GPIO with pull-up. In the "pwm" state the output +is configured as
>>>> a PWM output. This setup assures that the PWM output is at +the
>>>> required level that corresponds to duty cycle = 0 when PWM is
>>>> disabled. +E.g. at boot. +
>>>> +Example:
>>>> +
>>>> +&pwm1 {
>>>> +	pinctrl-names = "default", "pwm";
>>>> +	pinctrl-0 = <&pinctrl_backlight_gpio>;
>>>> +	pinctrl-1 = <&pinctrl_backlight_pwm>;
>>>> +}
>>>> +
>>>> +pinctrl_backlight_gpio: pwm1grp-gpio {
>>>> +	fsl,pins = <
>>>> +		/* GPIO with 22kOhm pull-up */
>>>> +		MX6QDL_PAD_GPIO_9__GPIO1_IO09	0xF008
>>>> +	>;
>>>> +};
>>>> +
>>>> +pinctrl_backlight_pwm: pwm1grp-pwm {
>>>> +	fsl,pins = <
>>>> +		/* PWM output */
>>>> +		MX6QDL_PAD_GPIO_9__PWM1_OUT	0x8
>>>> +	>;
>>>> +};


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-10-09 11:04 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-21 14:38 [RFC PATCH 0/2] pwm: imx: Configure output to GPIO in disabled state Michal Vokáč
2018-08-21 14:38 ` [RFC PATCH 1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO Michal Vokáč
2018-08-22  6:14   ` Lothar Waßmann
2018-08-22  7:01     ` Michal Vokáč
2018-08-22 11:17       ` Lothar Waßmann
2018-08-22 13:20         ` Michal Vokáč
2018-08-22 14:10           ` Lothar Waßmann
2018-08-23  9:13             ` Michal Vokáč
2018-08-23 11:18               ` Lothar Waßmann
2018-08-23 12:38                 ` Michal Vokáč
2018-08-23 10:40   ` Lukasz Majewski
2018-08-23 11:19     ` Lothar Waßmann
2018-08-23 11:21     ` Michal Vokáč
2018-08-23 12:36       ` Lukasz Majewski
2018-10-09 11:03         ` Vokáč Michal
2018-08-31 12:18   ` Rob Herring
2018-08-31 12:45     ` Lothar Waßmann
2018-08-31 13:17       ` Michal Vokáč
2018-08-31 13:30         ` Lothar Waßmann
2018-10-09 10:30           ` Vokáč Michal
2018-08-21 14:38 ` [RFC PATCH 2/2] pmw: imx: Configure output to GPIO in disabled state Michal Vokáč

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).