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 21:26:33 +0100
From:   "Graeme Gregory" <gg@...mlogic.co.uk>
To:     "H. Nikolaus Schaller" <hns@...delico.com>,
        "Mark Brown" <broonie@...nel.org>,
        "Tony Lindgren" <tony@...mide.com>
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



On Tue, 29 Jun 2021, at 9:21 PM, H. Nikolaus Schaller wrote:
> 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...
> 
Sorry after almost 10 years I have forgotten all about this driver.

Graeme