Hi, On Mon, Jul 31, 2023 at 11:03 AM Marek Vasut <marex@xxxxxxx> wrote: > > On 7/24/23 15:49, Doug Anderson wrote: > > Hi, > > [...] > > >> Maybe the EPROBE_DEFER actually happens and triggers the failure ? > > > > I could certainly believe that EPROBE_DEFER is involved. > > So no, it is not. It is difficult to set this up and access the signals, > but so I did. > > What happens is this: > panel_simple_probe() calls devm_regulator_get() > -> If the regulator was ENABLED, then it is now DISABLED As per my previous response, I don't think this is true. This was the part where I referred to `Documentation/power/regulator/consumer.rst` which said: NOTE: The supply may already be enabled before regulator_enabled() is called. This may happen if the consumer shares the regulator or the regulator has been previously enabled by bootloader or kernel board initialization code. My belief is that what's actually happening is that when the regulator _probes_ that the regulator turns off. In Linux GPIO regulators cannot read the state of the regulator at bootup. That means that when the regulator itself probes that Linux has to decide on the default state of the regulator itself. If the device tree has "regulator-boot-on" then Linux will turn the regulator on (even without any clients). In this case the regulator will stay on until some client enables and then disables the regulator, or until the regulator boot timeout happens and all unused regulators are powered off. If the device tree doesn't have "regulator-boot-on" then Linux will turn the regulator off. > -> For regulator-fixed, this means the regulator GPIO goes HIGH->LOW > > panel_simple_prepare() triggers panel_simple_resume() > -> If this occurs too soon after devm_regulator_get() turned the > regulator OFF and thus regulator GPIO low, then unprepare time is > not respected => FAIL > > Since there is no way to find out in which state the regulator was when > devm_regulator_get() was called, we have to wait the full unprepare time > before re-enabling that regulator in panel_simple_resume(). So just as a point of order, do you agree that prior to the commit that you are "Fixing" that we _weren't_ doing the full delay at probe time? That means that if things worked before they were working by some amount of luck, right? The old code used to do a delay after turning _off_ the regulator (at unprepare time). I will also continue to assert that trying to fix the problem via a delay in the probe of the panel code is the wrong place to fix the code. The problem is that you need a board-level constraint on this regulator (off-on-delay-us) preventing it from turning on again within a certain amount of time after it is turned off. This allows the regulator framework, which is what decided to turn this rail off during the regulator probe, to enforce this constraint. -Doug
