Thread (13 messages) 13 messages, 3 authors, 2021-06-30

Re: [PATCH] regulator: palmas: set supply_name after registering the regulator

From: Graeme Gregory <hidden>
Date: 2021-06-29 20:27:00
Also in: lkml


On Tue, 29 Jun 2021, at 9:21 PM, H. Nikolaus Schaller wrote:
Hi Mark,
quoted
Am 29.06.2021 um 20:56 schrieb Mark Brown [off-list ref]:

On Tue, Jun 29, 2021 at 08:34:55PM +0200, H. Nikolaus Schaller wrote:
quoted
quoted
Am 29.06.2021 um 17:59 schrieb Mark Brown [off-list ref]:
quoted
So it was working fine without having the supplying regulator resolved. AFAIK they
just serve as fixed regulators in the device tree and have no physical equivalent.
No, not at all - it's representing whatever provides input power to the
regulator.  There may be no physical control of it at runtime on your
system but that may not be true on other systems.  It's quite common for
there to be a chain of regulators (eg, DCDCs supplying LDOs) and then
they all need to get get power managed appropriately and you don't end
up thinking a regulator is enabled when the input regulator is disabled.  
Yes, that is how it is chained in other cases.
quoted
quoted
My proposal just moves setting the supply_name behind devm_regulator_register() and
by that restores the old behaviour.
This means that we won't actually map the supply and any system that
relies on software handling the supply regulator will be broken.
Well, it seems as if the supply regulators are only vsys_cobra and 
vdds_1v8_main.
At least in omap5-board-common.dtsi which is what I can test.

Both are fixed regulators where calling enable or not doesn't become
physically visible. The hardware still supplies the 5V and 1.8V to the palmas
chip.

Maybe the new rule by commit 98e48cd9283dreveals a design issue inside of
the Palmas driver which assumes that there is no need to control its supply
regulators. And does not handle probe deferral.

Then of course my patch is just a work-around for a bug but not a solution.
quoted
quoted
Well, unless...
quoted
... devm_regulator_register() does something differently if desc->supply_name
is not set before and changed afterwards. It may miss that change.
We resolve supplies during regulator registration, this would
effectively just skip mapping of the supply.
quoted
So I hope for guidance if my approach is good or needs a different solution.
What I would expect to happen here would be that once vsys_cobra is
registered the regulators supplied by it can register and then all their
consumers would in turn be able to register.  You should look into why
that supply regulator isn't appearing and resolve that, or if a consumer
isn't handling deferral then that would need to be addressed.
Ah, I think I get an idea what may be going wrong.

There seems to be a deadlock in probing:

	e.g. ldo3_reg depends on vdds_1v8_main supply
	vdds_1v8_main depends on smps7_reg supply
	smps7_reg depends on vsys_cobra supply
	vsys_cobra depends on nothing

This would normally lead to a simple chain as you described above. But
ldo3_reg and smps7_reg share the same driver and can probe successfully or
fail only in common.

Now if ldo3_reg defers probe through the new rule, smps7_reg is never
probed successfully because it is the same driver. Hence vdds_1v8_main can
not become available. And the Palmas continues to run in boot initialization
until some driver (MMC) wants to switch voltages or whatever.

Maybe Tony or Graeme has an idea how to do it right...
Sorry after almost 10 years I have forgotten all about this driver.

Graeme
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help