diff mbox series

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

Message ID 1534862333-27950-2-git-send-email-michal.vokac@ysoft.com
State Changes Requested, archived
Headers show
Series [RFC,1/2] dt-bindings: pwm: imx: Allow switching PWM output between PWM and GPIO | expand

Commit Message

Michal Vokáč Aug. 21, 2018, 2:38 p.m. UTC
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(+)

Comments

Lothar Waßmann Aug. 22, 2018, 6:14 a.m. UTC | #1
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
> +	>;
> +};
Michal Vokáč Aug. 22, 2018, 7:01 a.m. UTC | #2
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
>> +	>;
>> +};
> 
> 
>
Lothar Waßmann Aug. 22, 2018, 11:17 a.m. UTC | #3
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
Michal Vokáč Aug. 22, 2018, 1:20 p.m. UTC | #4
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
Lothar Waßmann Aug. 22, 2018, 2:10 p.m. UTC | #5
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
Michal Vokáč Aug. 23, 2018, 9:13 a.m. UTC | #6
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
Lukasz Majewski Aug. 23, 2018, 10:40 a.m. UTC | #7
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
Lothar Waßmann Aug. 23, 2018, 11:18 a.m. UTC | #8
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
Lothar Waßmann Aug. 23, 2018, 11:19 a.m. UTC | #9
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
Michal Vokáč Aug. 23, 2018, 11:21 a.m. UTC | #10
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
>> +	>;
>> +};
Lukasz Majewski Aug. 23, 2018, 12:36 p.m. UTC | #11
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
Michal Vokáč Aug. 23, 2018, 12:38 p.m. UTC | #12
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
Rob Herring Aug. 31, 2018, 12:18 p.m. UTC | #13
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
Lothar Waßmann Aug. 31, 2018, 12:45 p.m. UTC | #14
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
Michal Vokáč Aug. 31, 2018, 1:17 p.m. UTC | #15
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
Lothar Waßmann Aug. 31, 2018, 1:30 p.m. UTC | #16
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
Michal Vokáč Oct. 9, 2018, 10:30 a.m. UTC | #17
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
Michal Vokáč Oct. 9, 2018, 11:03 a.m. UTC | #18
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
>>>> +	>;
>>>> +};
diff mbox series

Patch

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
+	>;
+};