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. -Doug