linux-kernel - Re: [RESEND PATCH v5 8/9] pwm: add documentation for pwm push-pull mode

Open Source and information security mailing list archives

Message-ID: <f4a1f9e3-b3a8-2847-f13e-bb07dab400e6@microchip.com>
Date:   Fri, 12 Oct 2018 13:05:50 +0000
From:   <Claudiu.Beznea@...rochip.com>
To:     <thierry.reding@...il.com>
CC:     <mark.rutland@....com>, <linux-pwm@...r.kernel.org>,
        <alexandre.belloni@...tlin.com>, <shc_work@...l.ru>,
        <corbet@....net>, <linux-doc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <robh+dt@...nel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RESEND PATCH v5 8/9] pwm: add documentation for pwm push-pull
 mode

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@...rochip.com>
>> Reviewed-by: Rob Herring <robh@...nel.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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives