Messages in this thread | | | Date | Tue, 10 Oct 2023 14:14:46 +0200 | Subject | Re: [PATCH RFC 1/2] regulator: core: Disable unused regulators with unknown status | From | Konrad Dybcio <> |
| |
On 10/9/23 22:21, Stephan Gerhold wrote: > On Fri, Oct 06, 2023 at 11:11:48PM +0200, Konrad Dybcio wrote: >> On 4.10.2023 16:17, Stephan Gerhold wrote: >>> Some regulator drivers do not provide a way to check if the regulator is >>> currently enabled or not. That does not necessarily mean that the >>> regulator is always-on. For example, the regulators managed by the RPM >>> firmware on Qualcomm platforms can be either on or off during boot but >>> the initial state is not known. To sync the state the regulator should >>> get either explicitly enabled or explicitly disabled. >>> >>> Enabling all regulators unconditionally is not safe, because we might >>> not know which voltages are safe. The devices supplied by those >>> regulators might also require a special power-up sequence where the >>> regulators are turned on in a certain order or with specific delay. >>> >>> Disabling all unused regulators is safer. If the regulator is already >>> off it will just stay that way. If the regulator is on, disabling it >>> explicitly allows the firmware to turn it off for reduced power >>> consumption. >>> >>> The regulator core already has functionality for disabling unused >>> regulators. However, at the moment it assumes that all regulators where >>> the .is_enabled() callback fails are actually off. There is no way to >>> return a special value for the "unknown" state to explicitly ask for >>> disabling those regulators. >>> >>> Some drivers (e.g. qcom-rpmh-regulator.c) return -EINVAL for the case >>> where the initial status is unknown. Use that return code to assume the >>> initial status is unknown and try to explicitly disable the regulator >>> in that case. >>> >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> >>> --- >>> Instead of -EINVAL we could also use a different return code to indicate >>> the initial status is unknown. Or maybe there is some other option that >>> would be easier? This is working for me but I'm sending it as RFC to get >>> more feedback. :) >> >> -EOPNOTSUPP for "doesn't support getting is_enabled state"? >> > > The way it is implemented right now the Qualcomm SMD RPM regulator does > actually support getting the .is_enabled() state. It is only unable to > determine the initial state during boot. Once the regulator has been > enabled by some consumer for the first time the .is_enabled() callback > starts returning the expected results. > > Typically -EOPNOTSUPP is used when the driver callback (or similar) is > not implemented at all. I'm not sure if using -EOPNOTSUPP for the > "temporarily unable to determine state" purpose would be misleading. I'd say EOPNOTSUPP is fair here because calling is_enabled in that context is not supported, but I guess it's up to Mark.
Konrad
| |