linux-kernel - Re: [PATCH] drm/bridge: ti-sn65dsi86: fix REFCLK setting

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: <CAD=FV=W8RNcZvg5zgL1wDRmaH_eXrc79YQsMr9Be5HVtOWwwcw@mail.gmail.com>
Date: Thu, 12 Jun 2025 10:52:21 -0700
From: Doug Anderson <dianders@...omium.org>
To: Jayesh Choudhary <j-choudhary@...com>
Cc: Michael Walle <mwalle@...nel.org>, Andrzej Hajda <andrzej.hajda@...el.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, 
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>, 
	Jernej Skrabec <jernej.skrabec@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/bridge: ti-sn65dsi86: fix REFCLK setting

Hi,

On Thu, Jun 12, 2025 at 12:35 AM Jayesh Choudhary <j-choudhary@...com> wrote:
>
> >> If refclk is described in devicetree node, then I see that
> >> the driver modifies it in every resume call based solely on the
> >> clock value in dts.
> >
> > Exactly. But that is racy with what the chip itself is doing. I.e.
> > if you don't have that usleep() above, the chip will win the race
> > and the refclk frequency setting will be set according to the
> > external GPIOs (which is poorly described in the datasheet, btw),
> > regardless what the linux driver is setting (because that I2C write
> > happens too early).
>
> I am a little confused here.
> Won't it be opposite?
> If we have this delay here, GPIO will stabilize and set the register
> accordingly?
>
> In the driver, I came across the case when we do not have refclk.
> (My platform does have a refclk, I am just removing the property from
> the dts node to check the affect of GPIO[3:1] in question because clock
> is not a required property for the bridge as per the bindings)
>
> In the ti_sn65dsi86_probe(), before we read SN_DEVICE_ID_REGS,
> when we go to resume(), we do not do enable_comms() that calls
> ti_sn_bridge_set_refclk_freq() to set SN_DPPLL_SRC_REG.
> I see that register read for SN_DEVICE_ID_REGS fails in that case.
>
> Adding this delay fixes that issue. This made me think that we need
> the delay for GPIO to stabilize and set the refclk.

FWIW, it's been on my plate for a while to delete the "no refclk"
support. The chip is really hard to use properly without a refclk and
I'm not at all convinced that the current code actually works properly
without a refclk. I'm not aware of any current hardware working this
way. I know we had some very early prototype hardware ages ago that
tried it and we got it limping along at one point, but the driver
looked _very_ different then. I believe someone on the lists once
mentioned trying to do something without a refclk and it didn't work
and I strongly encouraged them to add a refclk.

-Doug