[RESEND,v5,8/9] pwm: add documentation for pwm push-pull mode

Message ID 1535461286-12308-9-git-send-email-claudiu.beznea@microchip.com
State New
Headers show
Series
  • extend PWM framework to support PWM modes
Related show

Commit Message

Claudiu Beznea Aug. 28, 2018, 1:01 p.m.
Add documentation for PWM push-pull mode.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/pwm/pwm.txt |  2 ++
 Documentation/pwm.txt                         | 16 ++++++++++++++++
 include/dt-bindings/pwm/pwm.h                 |  1 +
 3 files changed, 19 insertions(+)

Comments

Thierry Reding Oct. 12, 2018, 12:15 p.m. | #1
On Tue, Aug 28, 2018 at 04:01:25PM +0300, Claudiu Beznea wrote:
> Add documentation for PWM push-pull mode.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/pwm/pwm.txt |  2 ++
>  Documentation/pwm.txt                         | 16 ++++++++++++++++
>  include/dt-bindings/pwm/pwm.h                 |  1 +
>  3 files changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
> index 7c8aaac43f92..6a60c0fca112 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
> @@ -49,6 +49,8 @@ Optionally, the pwm-specifier can encode a number of flags (defined in
>  - PWM_MODE_COMPLEMENTARY: PWM complementary working mode (for PWM channels
>  with two outputs); if not specified, the default for PWM channel will be
>  used
> +- PWM_MODE_PUSH_PULL: PWM push-pull working modes (for PWM channels with
> +two outputs); if not specified the default for PWM channel will be used

What if somebody has this in the DT:

	PWM_MODE_COMPLEMENTARY | PWM_MODE_PUSH_PULL

which one takes precedence, or do we reject it?

Wouldn't it be preferable to either move the modes into an extra field
within the flags field, or perhaps even add another field?

I guess since Rob's already acked this, that concern may be unfounded.

Thierry
Claudiu Beznea Oct. 12, 2018, 1:05 p.m. | #2
On 12.10.2018 15:15, Thierry Reding wrote:
> On Tue, Aug 28, 2018 at 04:01:25PM +0300, Claudiu Beznea wrote:
>> Add documentation for PWM push-pull mode.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>>  Documentation/devicetree/bindings/pwm/pwm.txt |  2 ++
>>  Documentation/pwm.txt                         | 16 ++++++++++++++++
>>  include/dt-bindings/pwm/pwm.h                 |  1 +
>>  3 files changed, 19 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
>> index 7c8aaac43f92..6a60c0fca112 100644
>> --- a/Documentation/devicetree/bindings/pwm/pwm.txt
>> +++ b/Documentation/devicetree/bindings/pwm/pwm.txt
>> @@ -49,6 +49,8 @@ Optionally, the pwm-specifier can encode a number of flags (defined in
>>  - PWM_MODE_COMPLEMENTARY: PWM complementary working mode (for PWM channels
>>  with two outputs); if not specified, the default for PWM channel will be
>>  used
>> +- PWM_MODE_PUSH_PULL: PWM push-pull working modes (for PWM channels with
>> +two outputs); if not specified the default for PWM channel will be used
> 
> What if somebody has this in the DT:
> 
> 	PWM_MODE_COMPLEMENTARY | PWM_MODE_PUSH_PULL
> 
> which one takes precedence, or do we reject it?

The first valid one will be selected.

In patch 1/1 from this series, changes added to of_pwm_xlate_with_flags()
function, there is this code:

+		for (modebit = PWMC_MODE_COMPLEMENTARY_BIT;
+		     modebit < PWMC_MODE_CNT; modebit++) {
+			unsigned long mode = BIT(modebit);
+
+			if ((args->args[2] & mode) &&
+			    pwm_mode_valid(pwm, mode)) {
+				pwm->args.mode = mode;
+				break;
+			}
+		}

And since the modes bits are defined as follows:

enum {
	PWMC_MODE_NORMAL_BIT,
	PWMC_MODE_COMPLEMENTARY_BIT,
	PWMC_MODE_PUSH_PULL_BIT,
	PWMC_MODE_CNT,
};

in your proposed scenario: PWM_MODE_COMPLEMENTARY | PWM_MODE_PUSH_PULL
the PWM_MODE_COMPLEMENTARY mode will be selected since it is the first
valid one.

> 
> Wouldn't it be preferable to either move the modes into an extra field
> within the flags field, or perhaps even add another field?

This approach was proposed in version 2 of this series and based on the
discussions I had with Rob Herring [1] I decided to use the remaining space
from cell specific to PWM flags.

Thank you,
Claudiu Beznea

[1] https://lkml.org/lkml/2018/1/22/655

> 
> I guess since Rob's already acked this, that concern may be unfounded.
> 
> Thierry
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Patch

diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
index 7c8aaac43f92..6a60c0fca112 100644
--- a/Documentation/devicetree/bindings/pwm/pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm.txt
@@ -49,6 +49,8 @@  Optionally, the pwm-specifier can encode a number of flags (defined in
 - PWM_MODE_COMPLEMENTARY: PWM complementary working mode (for PWM channels
 with two outputs); if not specified, the default for PWM channel will be
 used
+- PWM_MODE_PUSH_PULL: PWM push-pull working modes (for PWM channels with
+two outputs); if not specified the default for PWM channel will be used
 
 Example with optional PWM specifier for inverse polarity and complementary
 mode:
diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
index 912c43da8b48..675f9351fed1 100644
--- a/Documentation/pwm.txt
+++ b/Documentation/pwm.txt
@@ -128,6 +128,22 @@  channel that was exported. The following properties will then be available:
     PWML1   |__|  |__|  |__|  |__|
             <--T-->
 
+    Push-pull mode - for PWM channels with two outputs; output waveforms
+        for a PWM channel in push-pull mode, with normal polarity looks like
+        this:
+            __          __
+    PWMH __|  |________|  |________
+                  __          __
+    PWML ________|  |________|  |__
+           <--T-->
+
+    If polarity is inversed:
+         __    ________    ________
+    PWMH   |__|        |__|
+         ________    ________    __
+    PWML         |__|        |__|
+           <--T-->
+
     Where T is the signal period.
 
 Implementing a PWM driver
diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
index b82279cc1787..cd4265bce968 100644
--- a/include/dt-bindings/pwm/pwm.h
+++ b/include/dt-bindings/pwm/pwm.h
@@ -12,5 +12,6 @@ 
 
 #define PWM_POLARITY_INVERTED			(1 << 0)
 #define PWM_MODE_COMPLEMENTARY			(1 << 1)
+#define PWM_MODE_PUSH_PULL			(1 << 2)
 
 #endif