On 2023-08-05 21:58, Svyatoslav Ryhel wrote:
5 серпня 2023 р. 15:49:32 GMT+03:00, Jonas Karlman jonas@kwiboo.se написав(-ла):
Hi,
On 2023-07-21 07:34, Svyatoslav Ryhel wrote:
чт, 20 лип. 2023 р. о 22:43 Simon Glass sjg@chromium.org пише:
Hi Svyatoslav,
On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel clamor95@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", ®) probe happens in i2c, pmic and regulator order.
I will check and re-test drivers I have ASAP.
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.
It is not a good decision in the long run and you should keep in mind that there is nothing more constant than a temporary solution.
Nor do I propose this to be a long-term solution, only that it is more reviewable to see changes in non-breaking smaller steps. In current state this patch breaks building U-Boot and requires the last patch to fix build again.
Anyway, I will be posting a similar patch for autoset as linked below as part of a series to add a Rockchip IO-domain driver shortly. In current state autoset cannot safely be called multiple times, and such patch should not have an impact on the direction of this series.
- 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.
uc_pdata is filled in pre_probe, while you are runing check in post_bind.
Please look closer at the code and not the snippet above, they are filled in post_bind or the code would not have worked :-)
Regards, Jonas
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/05db4dbcb8f908b417ed5cae8a7...
power: regulator: Perform regulator setup inside uclass https://github.com/Kwiboo/u-boot-rockchip/commit/489bd5d2c1a2a33824eac4f2d54...
Regards, Jonas
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com
drivers/power/regulator/regulator-uclass.c | 212 ++++++--------------- include/power/regulator.h | 119 ------------ 2 files changed, 62 insertions(+), 269 deletions(-)
Regards, SImon