Date:   Wed, 30 Jun 2021 16:43:14 +0200
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Mark Brown <broonie@...nel.org>, Tony Lindgren <tony@...mide.com>
Cc:     Graeme Gregory <gg@...mlogic.co.uk>,
        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 30.06.2021 um 15:04 schrieb Mark Brown <broonie@...nel.org>:
> 
> On Wed, Jun 30, 2021 at 02:29:02PM +0200, H. Nikolaus Schaller wrote:
>>> Am 30.06.2021 um 14:13 schrieb Mark Brown <broonie@...nel.org>:
> 
>> I think it could be indirectly circular since ldo3_reg does not find smps3
>> registered. But I have to run more tests with printk inserted.
> 
> Why would LDO3 have a dependency on SPMS3 given what's written above and
> how would that be circular?

because both can only probe successfully in common or not at all. If either
fails the other is rewound.

> 
>>> The driver should just register all the DCDCs before the LDOs, then
>>> everything will sort itself out.
> 
>> Basically the driver code does it that way. But fails. Probing seems to defer
>> until deferral limits (AFAIR there is a timer or counter in the probe deferral
>> queue) an does not succeed.
> 
> Ah, I see - the issue is the intervening 1.8V regulator.  That's not a
> circularity, that's the callout to a separate device in the middle of
> the chain.

Ok, "circular" is maybe the wrong word. It is an unexpected dependency...

>  It's a super weird hardware design if the DT is accurate,

I get the impression that the vdds_1v8_main is in the DT (omap5-board-common.dtsi)
only as an alias for smps7. Maybe to get more flexibility in overwriting
in board files? I.e. replace the power controller without having a fixed
definition of smps7 elsewhere.

Or to separate DT node names defined by the power controller (smps1-5)
from their useage on the board (vmain, vsys, vdds_1v8, vmmcsd, ...).

And, vdds_1v8_main is the only fixed-regulator used as a wrapper.
Others have either no vin-supply or are real regulators with a control gpio.

Looking into the schematics of the OMAP5432EVM or the Pyra handheld does
not reveal a physical regulator. It is just that the output signal of
smps7 is called "VDDS_1v8_MAIN".

Therefore, a completely different approach could be to remove fixedregulator-vdds_1v8_main
and replace by smps7_reg.

I tried with

	#define vdds_1v8_main smps7_reg

and it compiles and boots successfully.

There are still messages from the new rule for supply_name && !supply, but this time
Palmas gets initialized (maybe the -EPROBE_DEFER is silently ignored somewhere).

But is changing the DT the right solution if the Palmas and Fixed regulator
drivers can't handle the untouched DT which is logically correct (not physically)?

> it's hard to see how it's not going to be hurting efficiency.

Well, I think the regulators are enabled only once during boot so nobody
notices an issue.

>  In any
> case simplest thing would be to have separate MFD subdevices in Palmas
> for the LDOs and DCDCs, that'll do the right thing.

Attached is some logging an additional rdev_info() if the new rule
in set_machine_constraints() if it triggers and returns -EPROBE_DEFER.

We can see that several regulators trigger on the condition but indeed
ldo3 fails (which I so far only deduced from DT as the potential disturbance).

So we know several ways to get the hardware running again:
* revert 98e48cd9283d
* my hack to set supply_name later (makes the new rule ignored but may have side-effects)
* modify DTS to make vdds_1v8_main == smps7_reg
* (untested) make SMPS and LDOs separate subdevices in Palmas drivers

@Tony, your comments are needed...

Maybe you didn't notice since you may have configured Palmas into the kernel
which changes probe sequence.

BR and thanks,
Nikolaus

[    2.017857] palmas-rtc 48070000.i2c:palmas@48:rtc: registered as rtc0
[    2.026122] palmas-rtc 48070000.i2c:palmas@48:rtc: setting system clock to 2000-01-01T00:00:00 UTC (946684800)
[    2.041192] smps123: supply_name: smps1-in supply: 00000000
[    2.047112] smps123: supplied by regulator-dummy
[    2.054376] smps45: supply_name: smps4-in supply: 00000000
[    2.060240] smps45: supplied by regulator-dummy
[    2.067193] smps6: supply_name: smps6-in supply: 00000000
[    2.072909] smps6: supplied by vsys_cobra
[    2.079644] smps7: supply_name: smps7-in supply: 00000000
[    2.085346] smps7: supplied by vsys_cobra
[    2.092034] smps8: supply_name: smps8-in supply: 00000000
[    2.097735] smps8: supplied by vsys_cobra
[    2.105096] smps9: Bringing 0uV into 2100000-2100000uV
[    2.113152] smps9: supplied by vsys_cobra
[    2.118035] smps10_out2: supply_name: smps10-in supply: 00000000
[    2.124429] smps10_out2: supplied by regulator-dummy
[    2.133172] smps10_out1: supplied by regulator-dummy
[    2.138787] ldo1: Bringing 0uV into 1800000-1800000uV
[    2.145928] ldo1: supplied by vsys_cobra
[    2.150994] ldo2: supplied by vsys_cobra
[    2.155633] ldo3: supply_name: ldo3-in supply: 00000000
[    2.161436] palmas-pmic 48070000.i2c:palmas@48:palmas_pmic: failed to register 48070000.i2c:palmas@48:palmas_pmic regulator
[    2.178796] omap_hsmmc 480ad000.mmc: allocated mmc-pwrseq
[    2.186431] mmc_pwrseq_simple_set_gpios_value: value=1
[    2.187507] smps123: supply_name: smps1-in supply: 00000000
[    2.197895] smps123: supplied by regulator-dummy
[    2.204583] smps45: supply_name: smps4-in supply: 00000000
[    2.210473] smps45: supplied by regulator-dummy
[    2.217019] smps6: supply_name: smps6-in supply: 00000000
[    2.223173] smps6: supplied by vsys_cobra
[    2.228906] smps7: supply_name: smps7-in supply: 00000000
[    2.234673] smps7: supplied by vsys_cobra
[    2.240930] smps8: supply_name: smps8-in supply: 00000000
[    2.246631] smps8: supplied by vsys_cobra
[    2.252745] smps9: Bringing 0uV into 2100000-2100000uV
[    2.259318] smps9: supplied by vsys_cobra
[    2.264469] smps10_out2: supply_name: smps10-in supply: 00000000
[    2.270932] smps10_out2: supplied by regulator-dummy
[    2.278222] smps10_out1: supplied by regulator-dummy
[    2.284324] ldo1: supplied by vsys_cobra
[    2.289422] ldo2: supplied by vsys_cobra
[    2.294270] ldo3: supply_name: ldo3-in supply: 00000000
[    2.299859] palmas-pmic 48070000.i2c:palmas@48:palmas_pmic: failed to register 48070000.i2c:palmas@48:palmas_pmic regulator
[    2.299905] mmc_pwrseq_simple_set_gpios_value: value=0
[    2.318572] input: user-buttons as /devices/platform/user-buttons/input/input0
[    2.329003] smps123: supply_name: smps1-in supply: 00000000
[    2.334916] input: pyra-game-buttons as /devices/platform/pyra-game-buttons/input/input1
[    2.336655] input: pyra-lid-wakeup as /devices/platform/pyra-lid-wakeup/input/input2
[    2.352845] smps123: supplied by regulator-dummy
[    2.359670] l3main2_cm:clk:0010:0: failed to disable
[    2.370659] l4sec_cm:clk:0038:0: failed to disable
[    2.376292] ALSA device list:
[    2.376739] smps45: supply_name: smps4-in supply: 00000000
[    2.379521]   No soundcards found.
[    2.389296] smps45: supplied by regulator-dummy
[    2.396203] smps6: supply_name: smps6-in supply: 00000000
[    2.401981] smps6: supplied by vsys_cobra
[    2.406732] omap_hsmmc 480ad000.mmc: card claims to support voltages below defined range
[    2.416790] smps7: supply_name: smps7-in supply: 00000000
[    2.422604] smps7: supplied by vsys_cobra
[    2.428674] smps8: supply_name: smps8-in supply: 00000000
[    2.434472] smps8: supplied by vsys_cobra
[    2.440728] smps9: Bringing 0uV into 2100000-2100000uV
[    2.447350] smps9: supplied by vsys_cobra
[    2.452508] smps10_out2: supply_name: smps10-in supply: 00000000
[    2.458917] smps10_out2: supplied by regulator-dummy
[    2.467645] mmc2: new high speed SDIO card at address 0001
[    2.474295] mmc_pwrseq_simple_set_gpios_value: value=1
[    2.480100] smps10_out1: supplied by regulator-dummy
[    2.486272] ldo1: supplied by vsys_cobra
[    2.492500] ldo2: supplied by vsys_cobra
[    2.497155] ldo3: supply_name: ldo3-in supply: 00000000
[    2.502709] palmas-pmic 48070000.i2c:palmas@48:palmas_pmic: failed to register 48070000.i2c:palmas@48:palmas_pmic regulator
[    2.522668] smps123: supply_name: smps1-in supply: 00000000
[    2.528588] smps123: supplied by regulator-dummy
[    2.535669] smps45: supply_name: smps4-in supply: 00000000
[    2.541711] smps45: supplied by regulator-dummy
[    2.548638] smps6: supply_name: smps6-in supply: 00000000
[    2.554498] smps6: supplied by vsys_cobra
[    2.560245] smps7: supply_name: smps7-in supply: 00000000
[    2.565968] smps7: supplied by vsys_cobra
[    2.572198] smps8: supply_name: smps8-in supply: 00000000
[    2.577897] smps8: supplied by vsys_cobra
[    2.584466] smps9: Bringing 0uV into 2100000-2100000uV
[    2.591631] smps9: supplied by vsys_cobra
[    2.596520] smps10_out2: supply_name: smps10-in supply: 00000000
[    2.603112] smps10_out2: supplied by regulator-dummy
[    2.610365] smps10_out1: supplied by regulator-dummy
[    2.616175] ldo1: supplied by vsys_cobra
[    2.621193] ldo2: supplied by vsys_cobra
[    2.625845] ldo3: supply_name: ldo3-in supply: 00000000
[    2.631402] palmas-pmic 48070000.i2c:palmas@48:palmas_pmic: failed to register 48070000.i2c:palmas@48:palmas_pmic regulator
[    2.646447] Waiting for root device PARTUUID=7c769003-02..

Powered by blists - more mailing lists