[PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass

[Jonas Karlman]

Hi,

On 2023-07-21 07:34, Svyatoslav Ryhel wrote:
> чт, 20 лип. 2023 р. о 22:43 Simon Glass <sjg at chromium.org> пише:
>>
>> Hi Svyatoslav,
>>
>> On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>>>
>>> Regulators initial setup was previously dependent on board call.
>>> To move from this behaviour were introduced two steps. First is
>>> that all individual regulators will be probed just after binding
>>
>> We must not probe devices automatically when bound. The i2c interface
>> may not be available, etc. Do a probe afterwards.
>>
>> Perhaps I am misunderstanding this, iwc please reword this commit message.
> 
> After bound. If the regulator is a self-sufficient i2c device then it
> will be bound
> after i2c is available by code design so i2c interface should be
> available at that
> moment. At least led and gpio uclasses perform this for initial setup
> of devices.
> 
> Platform regulators (aka fixed/gpio regulators) work perfectly fine. I
> have no i2c
> regulators to test deeply.
> 
> As for now only one uclass is not compatible with this system - PMIC which has
> strong dependency between regulator and main mfd. This is why probing after
> bind for pmic regulators is disabled and pmic regulators are probed on pmic core
> post_probe.

This sounds more like a possible bug of some dependency not being
probed in correct order or not leaving the device in a ready state
after probe.

If pmic regulators are bind in pmic bind with the pmic as parent, then
at regulator probe the driver core ensure that pmic has probed before
regulator use.

This works perfect fine with the RK8xx I2C PMIC and its regulators.
Wich a call to device_get_supply_regulator(dev, "test-supply", &reg)
probe happens in i2c, pmic and regulator order.

> 
>>> which ensures that regulator pdata is filled and second is setting
>>> up regulator in post probe which enseres that correct regulator
>>> state according to pdata is reached.

I think that the approach in this patch is trying to change too much all
at once.

Have made some adjustments that I think should help with transition.
- Added a flag to protect regulator_autoset from being called more then
  once for each regulator, in a separate patch.
- Changed to only probe-after-bind on regulators marked as
  always-on/boot-on.

And it works something like this:

static int regulator_post_bind(struct udevice *dev)
{
	[...]

	if (uc_pdata->always_on || uc_pdata->boot_on)
		dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
}

static int regulator_post_probe(struct udevice *dev)
{
	ret = regulator_autoset(dev);
}

With that any always-on/boot-on regulator would automatically probe and
autoset after bind, remaining would only probe and autoset if they are
used.

This approach should also prevent having to change existing code that
may call autoset, and cleanup can be done in follow-up patches/series.

Probe-after-bind and call to autoset could also be protected with a new
Kconfig to help with transition.

See following for a working example using a new driver that depends
on that regulators have been autoset prior to regulator_get_value.
https://github.com/Kwiboo/u-boot-rockchip/compare/master...rk3568-io-domain

Or the two commits separate:

power: regulator: Only run autoset once for each regulator
https://github.com/Kwiboo/u-boot-rockchip/commit/05db4dbcb8f908b417ed5cae8a7a325c82112d75

power: regulator: Perform regulator setup inside uclass
https://github.com/Kwiboo/u-boot-rockchip/commit/489bd5d2c1a2a33824eac4f2d54399ef5dff4fdf

Regards,
Jonas

>>>
>>> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
>>> ---
>>>  drivers/power/regulator/regulator-uclass.c | 212 ++++++---------------
>>>  include/power/regulator.h                  | 119 ------------
>>>  2 files changed, 62 insertions(+), 269 deletions(-)
>>
>> Regards,
>> SImon