On Sun, 3 May 2026, Armin Wolf wrote: > Am 30.04.26 um 14:53 schrieb Ilpo Järvinen: > > On Fri, 17 Apr 2026, Armin Wolf wrote: > > > > > The EC might initialize the charge threshold with 0 to signal that > > > said threshold is uninitialized. Detect this and replace said value > > > with 100 to signal the EC that we want to take control of battery > > > charging. Also set the threshold to 100 if the EC-provided value > > > is invalid. > > > > > > Fixes: d050479693bb ("platform/x86: Add Uniwill laptop driver") > > > Reviewed-by: Werner Sembach > > > Signed-off-by: Armin Wolf > > > --- > > > drivers/platform/x86/uniwill/uniwill-acpi.c | 28 ++++++++++++++++++++- > > > 1 file changed, 27 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c > > > b/drivers/platform/x86/uniwill/uniwill-acpi.c > > > index faade4cf08be..8f16c94221aa 100644 > > > --- a/drivers/platform/x86/uniwill/uniwill-acpi.c > > > +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c > > > @@ -1404,7 +1404,12 @@ static int uniwill_get_property(struct power_supply > > > *psy, const struct power_sup > > > if (ret < 0) > > > return ret; > > > - val->intval = clamp_val(FIELD_GET(CHARGE_CTRL_MASK, regval), > > > 0, 100); > > > + regval = FIELD_GET(CHARGE_CTRL_MASK, regval); > > > + if (!regval) > > > + val->intval = 100; > > > + else > > > + val->intval = min(regval, 100); > > > > ... > > > > > + /* > > > + * The charge control threshold might be initialized with 0 by > > > + * the EC to signal that said threshold is uninitialized. We thus > > > + * need to replace this value with 100 to signal that we want to > > > + * take control of battery charging. For the sake of completeness > > > + * we also set the charging threshold to 100 if the EC-provided > > > + * value is invalid. > > > + */ > > > + threshold = FIELD_GET(CHARGE_CTRL_MASK, value); > > > + if (threshold == 0 || threshold > 100) { > > > + FIELD_MODIFY(CHARGE_CTRL_MASK, &value, 100); > > > > AFAICT, this does exactly the same thing as the other code above (but > > looks very different on surface). Wouldn't it make sense to have them > > share code? > > I do not think that this would be a good idea. The two call sides are two > different, creating a helper function for both would likely be very > difficult. Both seem to be sanitizing the charge control threshold (AFAICT, both map 0 and out-of-range values to 100) isn't that the case? Why cannot we have uniwill_charge_ctrl_thres_sanitize() or something along those lines? -- i.