linux-kernel - Re: [PATCH v2 1/8] drm/bridge: rgb-to-vga: Support an enable GPIO

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20161031155628.GD1041@n2100.armlinux.org.uk>
Date:   Mon, 31 Oct 2016 15:56:28 +0000
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc:     Rob Herring <robh@...nel.org>, Mark Rutland <mark.rutland@....com>,
        devicetree@...r.kernel.org, David Airlie <airlied@...ux.ie>,
        linux-sunxi@...glegroups.com, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, Chen-Yu Tsai <wens@...e.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/8] drm/bridge: rgb-to-vga: Support an enable GPIO

On Thu, Oct 27, 2016 at 12:17:24AM +0200, Maxime Ripard wrote:
> Hi Rob,
> 
> On Wed, Oct 26, 2016 at 05:13:46PM -0500, Rob Herring wrote:
> > On Thu, Oct 20, 2016 at 11:43:37AM +0800, Chen-Yu Tsai wrote:
> > > Some rgb-to-vga bridges have an enable GPIO, either directly tied to
> > > an enable pin on the bridge IC, or indirectly controlling a power
> > > switch.
> > > 
> > > Add support for it.
> > > 
> > > Signed-off-by: Chen-Yu Tsai <wens@...e.org>
> > > ---
> > >  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
> > >  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 28 ++++++++++++++++++++++
> > >  2 files changed, 30 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > > index 003bc246a270..d3484822bf77 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > > +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> > > @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> > >  - Video port 0 for RGB input
> > >  - Video port 1 for VGA output
> > >  
> > > +Optional properties:
> > > +- enable-gpios: GPIO pin to enable or disable the bridge
> > 
> > This should also define the active state.
> > 
> > > +static void dumb_vga_enable(struct drm_bridge *bridge)
> > > +{
> > > +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> > > +
> > > +	if (vga->enable_gpio)
> > > +		gpiod_set_value_cansleep(vga->enable_gpio, 1);
> > 
> > So the driver should allow either active high or low.
> 
> You mean like having a enable-active-high property? Isn't that
> redundant with the GPIO flags?

Correct - the gpiod APIs remove the need for drivers to know the
polarity of the signal, handling it inside the GPIO subsystem
instead, controlled either from the gpiod lookup tables in legacy
board files, or the DT specification for the GPIO.

So, in drivers, gpiod_set_value*(, 1) means "set gpio to active
level" and gpiod_set_value*(, 0) means "set gpio to inactive level".

Far nicer than all the bugs we've had with the legacy GPIO interfaces
with random different drivers implementing random different ways to
invert the signal, with all the pain that brings with it when a
platform comes along with a different inversion state.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.