Regulator: drivers that need to know their supply
Hi,
I am working on a driver for the tps22993 load switch.
This chip has configurable slew rates but no voltage setting (as it's just a switch).
To implement the .enable_time() method I need the voltage (which is the supply's voltage).
The problem is that, when my regulator is configured as always-on the supply is not known when .enable_time() is called from regulator_register() (it does work if the regulator is not always-on)
This is similar to the problem fixed by
5e3ca2b349b1 regulator: Try to resolve regulators supplies on
registration
and the follow up deadlock fix.
I tried adding a new function:
struct regulator *rdev_get_supply(struct regulator_dev *rdev)
{
int rc;
rc = regulator_resolve_supply(rdev);
if (rc)
return ERR_PTR(rc);
return rdev->supply;
}EXPORT_SYMBOL_GPL(rdev_get_supply);
Unfortunately that doesn't work.
First of all supply_name is not set at that point
(since it is set in regulator_register() AFTER set_machine_constraints())
Even after swapping the call order in regulator_register() it still doesn't work for my configuration since the parent supply is probed later so rdev_get_supply() returns -EDEFER which just results in
a warning message from _regulator_do_enable() when it propagates back from .enable_time()
3V3_LCD: enable_time() failed: -517
And even without that problem there is a risk of another deadlock since the driver callbacks are called from regulator_register() with the regulator_list mutex held in the always_on case.
Any ideas how to do this?
Regards,
Martin
Quoted 2 lines · Martin Fuzzey
To implement the .enable_time() method I need the voltage (which is the supply's voltage).
No, this is not sensible. You should be telling the framework about the slew rate and letting the framework work out how long it's going to take to transition. Your driver shouldn't be peering around inside other regulators, it should be telling the framework what it does itself and any handling of interrelationships should be in the framework.
Thanks for your reply.
Quoted 9 lines · Mark Brown
Quoted 2 lines · Martin Fuzzey
To implement the .enable_time() method I need the voltage (which is the supply's voltage).
No, this is not sensible. You should be telling the framework about the slew rate and letting the framework work out how long it's going to take to transition. Your driver shouldn't be peering around inside other regulators, it should be telling the framework what it does itself and any handling of interrelationships should be in the framework.
Ok, but I fail to see any way to do that (at least with the framework as is).
The framework can tell the driver what slew rate to use (via .set_ramp_delay) And it can ask the driver what the turn on time is (via .enable_time) It doesn't actullay know the slew rate (as a driver may not have configurable slew rates or the driver may have rounded the slew rate to something it supports)
Or are you saying I should extend the framework to add a .get_ramp_delay driver callback and make the framework do the calculation itself?
Happy to do that if it's the best way but your reply makes it sound like i'm not using an existing framework functionality and I don't think that is the case unless I'm overlooking something.
I'm just trying to do what rc5t583 already does in rc5t583_regulator_enable_time() except that in my case I don't know the voltage (since it comes from my supply).
While moving it to the framework will avoid the driver knowing about other regulators it won't fix the underlying issues I mentionned.
For a regulator configured as always-on the framework doesn't lookup and enable the parent regulators until regulator_register_resolve_supply() is caused right at the end of regulator_register().
This means that, when the always-on register is enabled it has no supply assigned, even at the framework level.
It also means that currently there is a difference in the switch on order depending if the regulator is configured as always-on:
Suppose A is the supply for B.
If B is NOT always-on, calling regulator_enable(B) will first enable A then B However if B is always-on, regulator_register(B) will first enable B then A
I'm not sure this actually matters in practice but A then B seems better from an inrush current point of view.
Regards,
Martin
Quoted 10 lines · Martin Fuzzey
Quoted 5 lines · Mark Brown
No, this is not sensible. You should be telling the framework about the slew rate and letting the framework work out how long it's going to take to transition. Your driver shouldn't be peering around inside other regulators, it should be telling the framework what it does itself and any handling of interrelationships should be in the framework.
Ok, but I fail to see any way to do that (at least with the framework as is).
Then send patches for the framework. One of the great things about working on Linux is that the whole OS is free software so you don't have to work around things in your driver! I've not looked at the code for this specific case.
Quoted 2 lines
Or are you saying I should extend the framework to add a .get_ramp_delay driver callback and make the framework do the calculation itself?
Yes.
Quoted 9 lines
While moving it to the framework will avoid the driver knowing about other regulators it won't fix the underlying issues I mentionned.
For a regulator configured as always-on the framework doesn't lookup and enable the parent regulators until
regulator_register_resolve_supply()is caused right at the end ofregulator_register().This means that, when the always-on register is enabled it has no supply assigned, even at the framework level.
You might find this changing very shortly but in any case the answer is still the same, change the framework if it needs changing.