linux-kernel - Re: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

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]

Date:   Wed, 9 Sep 2020 13:38:25 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Paul Cercueil <paul@...pouillou.net>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Sam Ravnborg <sam@...nborg.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Rob Herring <robh+dt@...nel.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Noralf Tronnes <noralf@...nnes.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        od@...c.me,
        "open list:DRM PANEL DRIVERS" <dri-devel@...ts.freedesktop.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

Hi Paul,

just a drive-by comment:

On Sat, Aug 22, 2020 at 6:33 PM Paul Cercueil <paul@...pouillou.net> wrote:

> +       gpiod_set_value_cansleep(priv->reset_gpiod, 0);
> +       usleep_range(20, 1000);
> +       gpiod_set_value_cansleep(priv->reset_gpiod, 1);

This implies that the reset line is active low.

I would specify in the DT GPIO handle that it is active low
and invert the above.

So:

reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;

gpiod_set_value_cansleep(priv->reset_gpiod, 1);
usleep_range(20, 1000);
gpiod_set_value_cansleep(priv->reset_gpiod, 0);

> +       priv->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(priv->reset_gpiod)) {
> +               dev_err(dev, "Couldn't get our reset GPIO\n");
> +               return PTR_ERR(priv->reset_gpiod);
> +       }

This would then fetch the GPIO as asserted (device in reset)
unless changed, but that may be the right thing to do actually.

> +static const struct ili9341_pdata yx240qv29_pdata = {
> +       .mode = { DRM_SIMPLE_MODE(240, 320, 37, 49) },
> +       .width_mm = 0, // TODO
> +       .height_mm = 0, // TODO

When nothing else works and data sheets are incomplete I
just take out a ruler and measure on the actual device.

Yours,
Linus Walleij