linux-kernel - Re: [PATCH] regulator: palmas: set supply

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Jun 2021 22:21:45 +0200
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Mark Brown <broonie@...nel.org>, Tony Lindgren <tony@...mide.com>,
        Graeme Gregory <gg@...mlogic.co.uk>
Cc:     Liam Girdwood <lgirdwood@...il.com>, Nishanth Menon <nm@...com>,
        Linux-OMAP <linux-omap@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Discussions about the Letux Kernel 
        <letux-kernel@...nphoenux.org>, kernel@...a-handheld.com,
        Peter Ujfalusi <peter.ujfalusi@...il.com>
Subject: Re: [PATCH] regulator: palmas: set supply_name after registering the
 regulator

Hi Mark,

> Am 29.06.2021 um 20:56 schrieb Mark Brown <broonie@...nel.org>:
> 
> On Tue, Jun 29, 2021 at 08:34:55PM +0200, H. Nikolaus Schaller wrote:
>>> Am 29.06.2021 um 17:59 schrieb Mark Brown <broonie@...nel.org>:
> 
> 
>> 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.

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

> 
>> Well, unless...
> 
>> ... 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.
> 
>> 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...

BR and thanks,
Nikolaus