Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
User-Agent: Cyrus-JMAP/3.5.0-alpha0-530-gd0c265785f-fm-20210616.002-gd0c26578
Mime-Version: 1.0
Message-Id: <4269dfb2-dd38-4b13-94e7-8c6afe0ab22e@www.fastmail.com>
In-Reply-To: <7B58B1BF-9D65-4CEC-B7D1-4EFDB2C0CB4E@goldelico.com>
References: <4ed67090bc048442567931ede8f1298a0b312b28.1624980242.git.hns@goldelico.com>
 <20210629155922.GD4613@sirena.org.uk>
 <2C7C3A47-4A5B-4052-98FC-7A96E2F138CA@goldelico.com>
 <20210629185638.GG4613@sirena.org.uk>
 <7B58B1BF-9D65-4CEC-B7D1-4EFDB2C0CB4E@goldelico.com>
Date:   Tue, 29 Jun 2021 21:26:33 +0100
From:   "Graeme Gregory" <gg@slimlogic.co.uk>
To:     "H. Nikolaus Schaller" <hns@goldelico.com>,
        "Mark Brown" <broonie@kernel.org>,
        "Tony Lindgren" <tony@atomide.com>
Cc:     "Liam Girdwood" <lgirdwood@gmail.com>,
        "Nishanth Menon" <nm@ti.com>,
        Linux-OMAP <linux-omap@vger.kernel.org>,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
        "Discussions about the Letux Kernel" <letux-kernel@openphoenux.org>,
        kernel@pyra-handheld.com,
        "Peter Ujfalusi" <peter.ujfalusi@gmail.com>
Subject: =?UTF-8?Q?Re:_[PATCH]_regulator:_palmas:_set_supply=5Fname_after_registe?=
 =?UTF-8?Q?ring_the_regulator?=
Content-Type: text/plain
Precedence: bulk


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