Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200221041631.10960-1-chris.packham@alliedtelesis.co.nz> <20200221041631.10960-4-chris.packham@alliedtelesis.co.nz>
In-Reply-To: <20200221041631.10960-4-chris.packham@alliedtelesis.co.nz>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Fri, 21 Feb 2020 09:40:00 -0600
Message-ID: <CAL_JsqK+jKyRr98_YX1GGwk-rHrLMOq_v7Z_57dQWYYQPuLS7A@mail.gmail.com>
Subject: Re: [PATCH v4 3/5] dt-bindings: hwmon: Document adt7475 invert-pwm property
To:     Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc:     Jean Delvare <jdelvare@suse.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Mark Rutland <mark.rutland@arm.com>,
        Linux HWMON List <linux-hwmon@vger.kernel.org>,
        devicetree@vger.kernel.org,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Logan Shaw <logan.shaw@alliedtelesis.co.nz>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Feb 20, 2020 at 10:16 PM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
>
> Add binding information for the invert-pwm property.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>
> Notes:
>     Changes in v4:
>     - use $ref uint32 and enum
>     - add adi vendor prefix
>
>     Cahnges in v3:
>     - new
>
>  Documentation/devicetree/bindings/hwmon/adt7475.yaml | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> index e40612ee075f..6a358b30586c 100644
> --- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
> @@ -50,6 +50,17 @@ patternProperties:
>       - $ref: /schemas/types.yaml#/definitions/uint32
>       - enum: [0, 1]
>
> +  "^adi,invert-pwm[1-3]$":
> +    description: |
> +      Configures the pwm output to use inverted logic. If set to 1
> +      the pwm uses a logic low output for 100% duty cycle. If set
> +      to 0 the pwm uses a logic high output for 100% duty cycle.
> +      If the property is absent the pwm retains it's configuration
> +      from the bios/bootloader.

I believe we already have an inverted flag for consumers. That doesn't
work if you don't have a consumer described in DT, but then the
question is should you? Or is this something the user will want to
configure from userspace.

The problem with 'invert' properties is they assume you know what the
not inverted state is. I would also make this an array:

adi,pwm-active-state = <1 0 0 1>; // PWM1 and PWM2 active low

And not present means <1 1 1 1>.

Rob