linux-kernel - Re: [PATCH] drm: panel: simple-panel: get the enable gpio as-is

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161107131736.GD12559@ulmo.ba.sec>
Date:   Mon, 7 Nov 2016 14:17:36 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Chen-Yu Tsai <wens@...e.org>
Cc:     Icenowy Zheng <icenowy@...c.xyz>, David Airlie <airlied@...ux.ie>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm: panel: simple-panel: get the enable gpio as-is

On Mon, Nov 07, 2016 at 06:12:43PM +0800, Chen-Yu Tsai wrote:
> On Sun, Nov 6, 2016 at 7:09 PM, Icenowy Zheng <icenowy@...c.xyz> wrote:
> > The enable gpio of simple-panel may be used by a simplefb or other
> > driver on the panel's display before the KMS driver get load.
> >
> > Get the GPIO as-is, so the panel won't be disabled, and the simplefb
> > can work.
> >
> > Signed-off-by: Icenowy Zheng <icenowy@...c.xyz>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index 113db3c..ccee4c1 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -312,7 +312,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
> >                 return PTR_ERR(panel->supply);
> >
> >         panel->enable_gpio = devm_gpiod_get_optional(dev, "enable",
> > -                                                    GPIOD_OUT_LOW);
> > +                                                    GPIOD_ASIS);
> 
> The GPIO requested as-is might be in input mode. You should change the
> gpiod_set_value calls to gpiod_direction_output calls. The later also
> allows you to give an initial value. Not sure if it checks for cansleep
> like the set_value calls though.

I'd prefer not to add gpiod_direction_output() calls outside of
->probe(). Instead, could we make this patch be smart about taking over
from an earlier user? Could it read the current direction and value of
the GPIO and not disable it if it had previously been enabled?

And even if we go this extra mile there's a possibility that the GPIO
was just left dangling by earlier software (or hardware) and leaving it
on would actually be worse than turning the panel off.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)