On 7/31/23 23:34, Doug Anderson wrote:
Hi,
On Mon, Jul 31, 2023 at 2:15 PM Marek Vasut <marex@xxxxxxx> wrote:
On 7/31/23 21:50, Doug Anderson wrote:
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.
I just measured that with a scope on actual hardware .
reg_fixed_voltage_probe() is the code which turns the regulator OFF,
specifically in the part gpiod_get_optional(...GPIOD_OUT_LOW);
Great, at least we're on the same page here now.
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).
Yes, that's well possible.
...so assuming we agree that this is _not_ a regression introduced by
e5e30dfcf3db ("drm: panel: simple: Defer unprepare delay till next
prepare to shorten it"), that means that all other preexisting users
of panel-simple were fine with the old behavior
No, it does not mean that all the previous users were fine with that
behavior. It means the driver operates panels out of spec, we cannot
know which ones, but we do know it does. Whether users noticed that
defect or not is another question, which we cannot answer.
where the unprepare
delay was only enforced when the panel driver itself turned things off
and then back on and no extra delay was needed at probe. The one board
we know about that has a problem with this behavior is the one you're
reproducing on, which we think only worked previously by chance.
There is at least one device now which shows a problem with the current
state of driver.
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.
No, the driver must respect the unprepare delay, that is what is
currently not happening. Trying to somehow work around that by adding
random changes to DT is not going to fix the fact that panel-simple is
not respecting its own internal built-in constraints.
Well, except that the panel _isn't_ actually unpreparing the panel.
The constant you're talking about is a delay between unpreparing the
panel and then preparing it again and we're not doing that here. Here,
you are trying to account for an implicit unprepare that happened
outside the context of the panel driver (in the regulator framework).
The regulator framework is the one disabling the regulator on its own
and without the knowledge of the panel driver.
I agree until this point.
The problem should be
addressed at the regulator framework, which might involve the
off-on-delay.
I disagree with this point.
I would even go further and say that, perhaps, when the regulator
framework turns this regulator off at bootup then you might be
violating the power sequencing requirements in the panel anyway. If
the bootloader left the panel on and _also_ left an enable GPIO on
then it's entirely possible that you've got a power leak through the
enable GPIO where you're backpowering the panel's logic when the
regulator framework turns things off. This is something that would be
impossible for the panel driver to solve because the panel driver
hasn't even probed yet.
I agree with this part as well.
In any case, at this point it seems unlikely that I'll convince you or
that you'll convince me. I wonder if it's time for someone else on
this thread to provide an opinion.
I agree with this part as well.
