From: Benjamin Bara <[email protected]>
It can happen that monitors are activated before the kernel is started,
e.g. by bootloader or by OTP. If monitoring workarounds are active on a
regulator, the core shouldn't perform the state changes without applying
the workaround to the regulator. Therefore, warn the user already during
initialization that the device-tree should be adapted.
Warning can be fixed by enabling (or disabling) monitoring in the DT,
e.g.:
regulator-uv-protection-microvolt = <1>;
or
regulator-ov-error-microvolt = <0>;
Constraints regarding the monitoring of a regulator can usually be found
in the docu.
Signed-off-by: Benjamin Bara <[email protected]>
---
drivers/regulator/core.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ca5d6ba889dc..9bddab17450e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1564,6 +1564,47 @@ static int set_machine_constraints(struct regulator_dev *rdev)
}
}
+ /*
+ * When the core is in charge of handling monitoring workarounds,
+ * it is essential to know if a monitor is already active during
+ * initialization.
+ */
+ if (rdev->desc->mon_disable_reg_disabled ||
+ rdev->desc->mon_disable_reg_set_higher ||
+ rdev->desc->mon_disable_reg_set_lower ||
+ rdev->desc->mon_unsupported_reg_modes) {
+ unsigned int monitor_state = REGULATOR_MONITOR_NONE;
+
+ ret = ops->get_active_protections(rdev, &monitor_state);
+ if (ret)
+ return ret;
+
+ if (!rdev->constraints->over_voltage_detection &&
+ (monitor_state & REGULATOR_MONITOR_OVER_VOLTAGE)) {
+ rdev_err(rdev, "dt unaware of active %s monitor!\n",
+ "over-voltage");
+ return -EINVAL;
+ }
+ if (!rdev->constraints->under_voltage_detection &&
+ (monitor_state & REGULATOR_MONITOR_UNDER_VOLTAGE)) {
+ rdev_err(rdev, "dt unaware of active %s monitor!\n",
+ "under-voltage");
+ return -EINVAL;
+ }
+ if (!rdev->constraints->over_current_detection &&
+ (monitor_state & REGULATOR_MONITOR_OVER_CURRENT)) {
+ rdev_err(rdev, "dt unaware of active %s monitor!\n",
+ "over-current");
+ return -EINVAL;
+ }
+ if (!rdev->constraints->over_temp_detection &&
+ (monitor_state & REGULATOR_MONITOR_OVER_TEMPERATURE)) {
+ rdev_err(rdev, "dt unaware of active %s monitor!\n",
+ "over-temperature");
+ return -EINVAL;
+ }
+ }
+
if (rdev->constraints->over_current_detection)
ret = handle_notify_limits(rdev,
ops->set_over_current_protection,
--
2.34.1
On 6/20/23 23:03, Benjamin Bara wrote:
> From: Benjamin Bara <[email protected]>
>
> It can happen that monitors are activated before the kernel is started,
> e.g. by bootloader or by OTP. If monitoring workarounds are active on a
> regulator, the core shouldn't perform the state changes without applying
> the workaround to the regulator. Therefore, warn the user already during
> initialization that the device-tree should be adapted.
>
> Warning can be fixed by enabling (or disabling) monitoring in the DT,
> e.g.:
> regulator-uv-protection-microvolt = <1>;
> or
> regulator-ov-error-microvolt = <0>;
>
> Constraints regarding the monitoring of a regulator can usually be found
> in the docu.
I am not entirely sure if this is the right thing to do. Should we
expect the hardware state to be what is described in DT at Linux boot-up
- or, should we silently accept the fact that for example boot can alter
things.
From the 'code pov' I have no complaints though. I just can't say if
warning is the right idea. I'll leave this for bigger brains to decide :)
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
On Mon, Jun 26, 2023 at 04:56:21PM +0300, Matti Vaittinen wrote:
> On 6/20/23 23:03, Benjamin Bara wrote:
> > Warning can be fixed by enabling (or disabling) monitoring in the DT,
> > e.g.:
> > regulator-uv-protection-microvolt = <1>;
> > or
> > regulator-ov-error-microvolt = <0>;
> >
> > Constraints regarding the monitoring of a regulator can usually be found
> > in the docu.
> I am not entirely sure if this is the right thing to do. Should we expect
> the hardware state to be what is described in DT at Linux boot-up - or,
> should we silently accept the fact that for example boot can alter things.
> From the 'code pov' I have no complaints though. I just can't say if warning
> is the right idea. I'll leave this for bigger brains to decide :)
Yes, this isn't really the idiom we normally adopt - the default thing
is to just leave the hardware untouched, that should not usually be
regarded as a problem.
Hi Matti & Mark,
thank you for the feedback!
On Mon, 26 Jun 2023 at 18:49, Mark Brown <[email protected]> wrote:
> On Mon, Jun 26, 2023 at 04:56:21PM +0300, Matti Vaittinen wrote:
> > On 6/20/23 23:03, Benjamin Bara wrote:
> > > Warning can be fixed by enabling (or disabling) monitoring in the DT,
> > > e.g.:
> > > regulator-uv-protection-microvolt = <1>;
> > > or
> > > regulator-ov-error-microvolt = <0>;
> > >
> > > Constraints regarding the monitoring of a regulator can usually be found
> > > in the docu.
>
> > I am not entirely sure if this is the right thing to do. Should we expect
> > the hardware state to be what is described in DT at Linux boot-up - or,
> > should we silently accept the fact that for example boot can alter things.
>
> > From the 'code pov' I have no complaints though. I just can't say if warning
> > is the right idea. I'll leave this for bigger brains to decide :)
>
> Yes, this isn't really the idiom we normally adopt - the default thing
> is to just leave the hardware untouched, that should not usually be
> regarded as a problem.
Thanks for clarifying. I will now activate the constraint instead of erroring
out. This guarantees that the workaround will still be applied, so basically
similar to the current bd718x7 implementation. I would still keep the message as
a warn, or should I drop it too? My idea is to let the user know that there is
some kind of monitoring going on but the device-tree is not aware of it.
On Mon, Jul 03, 2023 at 08:43:47PM +0200, Benjamin Bara wrote:
> On Mon, 26 Jun 2023 at 18:49, Mark Brown <[email protected]> wrote:
> > Yes, this isn't really the idiom we normally adopt - the default thing
> > is to just leave the hardware untouched, that should not usually be
> > regarded as a problem.
> Thanks for clarifying. I will now activate the constraint instead of erroring
> out. This guarantees that the workaround will still be applied, so basically
> similar to the current bd718x7 implementation. I would still keep the message as
> a warn, or should I drop it too? My idea is to let the user know that there is
> some kind of monitoring going on but the device-tree is not aware of it.
I would leave the warning off, I'd say it's more unusual that it might
be possible to disable the montioring than that it's being enabled - a
lot of devices either have fixed limits or only allow the limit to be
configured without allowing it to be completely disabled.