[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <11ae473f-cee1-241b-174c-915dc46209b1@mentor.com>
Date: Tue, 3 Dec 2019 11:21:26 +0530
From: Harish Jenny K N <harish_kandiga@...tor.com>
To: Geert Uytterhoeven <geert+renesas@...der.be>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
Jonathan Corbet <corbet@....net>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Eugeniu Rosca <erosca@...adit-jv.com>
CC: Alexander Graf <graf@...zon.com>,
Peter Maydell <peter.maydell@...aro.org>,
Paolo Bonzini <pbonzini@...hat.com>,
Phil Reid <preid@...ctromag.com.au>,
Marc Zyngier <marc.zyngier@....com>,
Christoffer Dall <christoffer.dall@....com>,
Magnus Damm <magnus.damm@...il.com>,
<linux-gpio@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-renesas-soc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <qemu-devel@...gnu.org>
Subject: Re: [PATCH v3 4/7] dt-bindings: gpio: Add gpio-repeater bindings
On 27/11/19 2:12 PM, Geert Uytterhoeven wrote:
> Add Device Tree bindings for a GPIO repeater, with optional translation
> of physical signal properties. This is useful for describing explicitly
> the presence of e.g. an inverter on a GPIO line, and was inspired by the
> non-YAML gpio-inverter bindings by Harish Jenny K N
> <harish_kandiga@...tor.com>[1].
>
> Note that this is different from a GPIO Nexus Node[2], which cannot do
> physical signal property translation.
>
> While an inverter can be described implicitly by exchanging the
> GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags, this has its limitations.
> Each GPIO line has only a single GPIO_ACTIVE_* flag, but applies to both
> th provider and consumer sides:
> 1. The GPIO provider (controller) looks at the flags to know the
> polarity, so it can translate between logical (active/not active)
> and physical (high/low) signal levels.
> 2. While the signal polarity is usually fixed on the GPIO consumer
> side (e.g. an LED is tied to either the supply voltage or GND),
> it may be configurable on some devices, and both sides need to
> agree. Hence the GPIO_ACTIVE_* flag as seen by the consumer must
> match the actual polarity.
> There exists a similar issue with interrupt flags, where both the
> interrupt controller and the device generating the interrupt need
> to agree, which breaks in the presence of a physical inverter not
> described in DT (see e.g. [3]).
>
> [1] "[PATCH V4 2/2] gpio: inverter: document the inverter bindings"
> https://lore.kernel.org/linux-gpio/1561699236-18620-3-git-send-email-harish_kandiga@mentor.com/
>
> [2] Devicetree Specification v0.3-rc2, Section 2.5
> https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.3-rc2
>
> [3] "[PATCH] wlcore/wl18xx: Add invert-irq OF property for physically
> inverted IRQ"
> https://lore.kernel.org/linux-renesas-soc/20190607172958.20745-1-erosca@de.adit-jv.com/
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> ---
> v3:
> - New.
> ---
> .../bindings/gpio/gpio-repeater.yaml | 53 +++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-repeater.yaml b/Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
> new file mode 100644
> index 0000000000000000..efdee0c3be43f731
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-repeater.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO Repeater
> +
> +maintainers:
> + - Harish Jenny K N <harish_kandiga@...tor.com>
> + - Geert Uytterhoeven <geert+renesas@...der.be>
> +
> +description:
> + This represents a repeater for one or more GPIOs, possibly including physical
> + signal property translation (e.g. polarity inversion).
> +
> +properties:
> + compatible:
> + const: gpio-repeater
> +
> + "#gpio-cells":
> + const: 2
> +
> + gpio-controller: true
> +
> + gpios:
> + description:
> + Phandle and specifier, one for each repeated GPIO.
> +
> + gpio-line-names:
> + description:
> + Strings defining the names of the GPIO lines going out of the GPIO
> + controller.
> +
> +required:
> + - compatible
> + - "#gpio-cells"
> + - gpio-controller
> + - gpios
> +
> +additionalProperties: false
> +
> +examples:
> + # Device node describing a polarity inverter for a single GPIO
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + inverter: gpio-repeater {
> + compatible = "gpio-repeater";
> + #gpio-cells = <2>;
> + gpio-controller;
> + gpios = <&gpio 95 GPIO_ACTIVE_LOW>;
> + };
just a suggestion: giving a gpio-line-names in the example would look useful.
Powered by blists - more mailing lists