Hi, On Thu, Jun 12, 2025 at 10:52 AM Doug Anderson <diand...@chromium.org> wrote: > > Hi, > > On Thu, Jun 12, 2025 at 12:35 AM Jayesh Choudhary <j-choudh...@ti.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.
Actually, I may have to eat my words here. I double-checked the dts and I see there's at least two mainline users ("meson-g12b-bananapi-cm4-mnt-reform2.dts" and "/imx8mq-mnt-reform2.dts") that don't seem to be specifying a `refclk` to `ti,sn65dsi86`. Neil / Lucas: is that correct? ...and it actually works? -Doug