| Message ID | 20221009192136.106859-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: supply: bq25890: Add support for setting max CC current and voltage | expand |
Hi, On 10/9/22 21:21, Marek Vasut wrote: > Let user set battery charge current and voltage limit via sysfs. This is > useful in case the user space needs to reduce charge current to keep the > system within thermal limits. The maximum charge current and voltage are > still limited to "ti,charge-current" and "ti,battery-regulation-voltage" > values to avoid damaging the hardware in case too high values are set by > user space. As I have tried to explain in my replies to your other patch-series, the currently used battery charge current and voltage limits belong in the POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT resp. POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE properties (which may also be written to set these). Where as the _MAX variants of these properties should give the maximum allowed values as read from devicetree in the "ti,charge-current" and "ti,battery-regulation-voltage" properties. Making this work as it should will require first fixing the abuse of at least POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE which I mentioned in the other thread. In case it is not clear, this is a nack for this patch. Sorry that you have to cleanup this mess, but IMHO it is better to fix the mess now then to try and hack around it making an even bigger mess. Regards, Hans > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Sebastian Reichel <sebastian.reichel@collabora.com> > To: linux-pm@vger.kernel.org > --- > drivers/power/supply/bq25890_charger.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c > index c4c830247e0e0..d48c147c8be81 100644 > --- a/drivers/power/supply/bq25890_charger.c > +++ b/drivers/power/supply/bq25890_charger.c > @@ -531,7 +531,11 @@ static int bq25890_power_supply_get_property(struct power_supply *psy, > break; > > case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: > - val->intval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG); > + ret = bq25890_field_read(bq, F_ICHG); > + if (ret < 0) > + return ret; > + > + val->intval = bq25890_find_val(ret, TBL_ICHG); > > /* When temperature is too low, charge current is decreased */ > if (bq->state.ntc_fault == NTC_FAULT_COOL) { > @@ -561,7 +565,11 @@ static int bq25890_power_supply_get_property(struct power_supply *psy, > break; > > case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX: > - val->intval = bq25890_find_val(bq->init_data.vreg, TBL_VREG); > + ret = bq25890_field_read(bq, F_VREG); > + if (ret < 0) > + return ret; > + > + val->intval = bq25890_find_val(ret, TBL_VREG); > break; > > case POWER_SUPPLY_PROP_PRECHARGE_CURRENT: > @@ -619,9 +627,18 @@ static int bq25890_power_supply_set_property(struct power_supply *psy, > const union power_supply_propval *val) > { > struct bq25890_device *bq = power_supply_get_drvdata(psy); > + int maxval; > u8 lval; > > switch (psp) { > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: > + maxval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG); > + lval = bq25890_find_idx(min(val->intval, maxval), TBL_ICHG); > + return bq25890_field_write(bq, F_ICHG, lval); > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX: > + maxval = bq25890_find_val(bq->init_data.vreg, TBL_VREG); > + lval = bq25890_find_idx(min(val->intval, maxval), TBL_VREG); > + return bq25890_field_write(bq, F_VREG, lval); > case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT: > lval = bq25890_find_idx(val->intval, TBL_IINLIM); > return bq25890_field_write(bq, F_IINLIM, lval); > @@ -634,6 +651,8 @@ static int bq25890_power_supply_property_is_writeable(struct power_supply *psy, > enum power_supply_property psp) > { > switch (psp) { > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX: > case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT: > return true; > default:
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c index c4c830247e0e0..d48c147c8be81 100644 --- a/drivers/power/supply/bq25890_charger.c +++ b/drivers/power/supply/bq25890_charger.c @@ -531,7 +531,11 @@ static int bq25890_power_supply_get_property(struct power_supply *psy, break; case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: - val->intval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG); + ret = bq25890_field_read(bq, F_ICHG); + if (ret < 0) + return ret; + + val->intval = bq25890_find_val(ret, TBL_ICHG); /* When temperature is too low, charge current is decreased */ if (bq->state.ntc_fault == NTC_FAULT_COOL) { @@ -561,7 +565,11 @@ static int bq25890_power_supply_get_property(struct power_supply *psy, break; case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX: - val->intval = bq25890_find_val(bq->init_data.vreg, TBL_VREG); + ret = bq25890_field_read(bq, F_VREG); + if (ret < 0) + return ret; + + val->intval = bq25890_find_val(ret, TBL_VREG); break; case POWER_SUPPLY_PROP_PRECHARGE_CURRENT: @@ -619,9 +627,18 @@ static int bq25890_power_supply_set_property(struct power_supply *psy, const union power_supply_propval *val) { struct bq25890_device *bq = power_supply_get_drvdata(psy); + int maxval; u8 lval; switch (psp) { + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: + maxval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG); + lval = bq25890_find_idx(min(val->intval, maxval), TBL_ICHG); + return bq25890_field_write(bq, F_ICHG, lval); + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX: + maxval = bq25890_find_val(bq->init_data.vreg, TBL_VREG); + lval = bq25890_find_idx(min(val->intval, maxval), TBL_VREG); + return bq25890_field_write(bq, F_VREG, lval); case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT: lval = bq25890_find_idx(val->intval, TBL_IINLIM); return bq25890_field_write(bq, F_IINLIM, lval); @@ -634,6 +651,8 @@ static int bq25890_power_supply_property_is_writeable(struct power_supply *psy, enum power_supply_property psp) { switch (psp) { + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX: case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT: return true; default:
Let user set battery charge current and voltage limit via sysfs. This is useful in case the user space needs to reduce charge current to keep the system within thermal limits. The maximum charge current and voltage are still limited to "ti,charge-current" and "ti,battery-regulation-voltage" values to avoid damaging the hardware in case too high values are set by user space. Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Hans de Goede <hdegoede@redhat.com> Cc: Sebastian Reichel <sebastian.reichel@collabora.com> To: linux-pm@vger.kernel.org --- drivers/power/supply/bq25890_charger.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)