linux-kernel - Re: [PATCH] drm/bridge: fix anx6345 power up sequence

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: <CA+E=qVeX2aU0hiDMxLXzVk-YiMsqKKFKpm=cc=72joMhZmNV1g@mail.gmail.com>
Date:   Sun, 17 Apr 2022 11:52:14 -0700
From:   Vasily Khoruzhick <anarsoul@...il.com>
To:     Torsten Duwe <duwe@....de>
Cc:     Andrzej Hajda <a.hajda@...sung.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Robert Foss <robert.foss@...aro.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Thierry Reding <treding@...dia.com>,
        Lyude Paul <lyude@...hat.com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Harald Geyer <harald@...ib.org>, stable@...r.kernel.org
Subject: Re: [PATCH] drm/bridge: fix anx6345 power up sequence

On Sun, Apr 17, 2022 at 9:15 AM Torsten Duwe <duwe@....de> wrote:
>
> Align the power-up sequence with the known-good procedure documented in [1]:
> un-swap dvdd12 and dvdd25, and allow a little extra time for them to settle
> before de-asserting reset.

Hi Torsten,

Interesting find! I tried to fix the issue several times by playing
with the delays to no avail.

What's interesting, ANX6345 datasheet allows DVDD12 to come up either
earlier or later than DVDD25 with the delay of T1 (2ms typical)
between them, and actually bringing up DVDD12 first works fine in
u-boot.

The datasheet also requires reset to be deasserted no earlier than T2
(2-5ms) after all the rails are stable.

Another thing it mentions is that the system clock must be stable for
T3 (1-3ms) before reset is deasserted, T3 is already a part of T2,
however it cannot be gated on Pinebook, see [1], page 15

The change looks good to me, but I'll need some time to actually test
it. If you don't hear from me for longer than a week please ping me.

[1] https://files.pine64.org/doc/pinebook/pinebook_mainboard_schematic_3.0.pdf

Regards,
Vasily

> Fixes: 6aa192698089b ("drm/bridge: Add Analogix anx6345 support")
>
> [1] https://github.com/OLIMEX/DIY-LAPTOP/blob/master/
> HARDWARE/A64-TERES/TERES-PCB1-A64-MAIN/Rev.C/TERES_PCB1-A64-MAIN_Rev.C.pdf
> (page 5, blue comment down left)
>
> Reported-by: Harald Geyer <harald@...ib.org>
> Signed-off-by: Torsten Duwe <duwe@....de>
> Cc: stable@...r.kernel.org
> ---
>
> This fixes the problem that e.g. X screensaver turns the screen black,
> and it stays black until the next reboot; definitely on the Teres-I,
> probably on the pinebook64, too.

You should probably move this comment up to be included in the commit message.



>
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -309,27 +309,27 @@ static void anx6345_poweron(struct anx63
>         gpiod_set_value_cansleep(anx6345->gpiod_reset, 1);
>         usleep_range(1000, 2000);
>
> -       err = regulator_enable(anx6345->dvdd12);
> +       err = regulator_enable(anx6345->dvdd25);
>         if (err) {
> -               DRM_ERROR("Failed to enable dvdd12 regulator: %d\n",
> +               DRM_ERROR("Failed to enable dvdd25 regulator: %d\n",
>                           err);
>                 return;
>         }
>
> -       /* T1 - delay between VDD12 and VDD25 should be 0-2ms */
> +       /* T1 - delay between VDD25 and VDD12 should be 0-2ms */
>         usleep_range(1000, 2000);
>
> -       err = regulator_enable(anx6345->dvdd25);
> +       err = regulator_enable(anx6345->dvdd12);
>         if (err) {
> -               DRM_ERROR("Failed to enable dvdd25 regulator: %d\n",
> +               DRM_ERROR("Failed to enable dvdd12 regulator: %d\n",
>                           err);
>                 return;
>         }
>
>         /* T2 - delay between RESETN and all power rail stable,
> -        * should be 2-5ms
> +        * should be at least 2-5ms, 10ms to be safe.
>          */
> -       usleep_range(2000, 5000);
> +       usleep_range(9000, 11000);
>
>         gpiod_set_value_cansleep(anx6345->gpiod_reset, 0);
>