On Tue, 28 May 2024, Dmitry Baryshkov wrote: > On the Lenovo Yoga C630 WOS laptop the EC provides access to the adapter > and battery status. Add the driver to read power supply status on the > laptop. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > --- > drivers/power/supply/Kconfig | 9 + > drivers/power/supply/Makefile | 1 + > drivers/power/supply/lenovo_yoga_c630_battery.c | 479 ++++++++++++++++++++++++ > 3 files changed, 489 insertions(+) > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index 3e31375491d5..55ab8e90747d 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -167,6 +167,15 @@ config BATTERY_LEGO_EV3 > help > Say Y here to enable support for the LEGO MINDSTORMS EV3 battery. > > +config BATTERY_LENOVO_YOGA_C630 > + tristate "Lenovo Yoga C630 battery" > + depends on OF && EC_LENOVO_YOGA_C630 > + help > + This driver enables battery support on the Lenovo Yoga C630 laptop. > + > + To compile the driver as a module, choose M here: the module will be > + called lenovo_yoga_c630_battery. > + > config BATTERY_PMU > tristate "Apple PMU battery" > depends on PPC32 && ADB_PMU > diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile > index 58b567278034..8ebbdcf92dac 100644 > --- a/drivers/power/supply/Makefile > +++ b/drivers/power/supply/Makefile > @@ -32,6 +32,7 @@ obj-$(CONFIG_BATTERY_DS2782) += ds2782_battery.o > obj-$(CONFIG_BATTERY_GAUGE_LTC2941) += ltc2941-battery-gauge.o > obj-$(CONFIG_BATTERY_GOLDFISH) += goldfish_battery.o > obj-$(CONFIG_BATTERY_LEGO_EV3) += lego_ev3_battery.o > +obj-$(CONFIG_BATTERY_LENOVO_YOGA_C630) += lenovo_yoga_c630_battery.o > obj-$(CONFIG_BATTERY_PMU) += pmu_battery.o > obj-$(CONFIG_BATTERY_QCOM_BATTMGR) += qcom_battmgr.o > obj-$(CONFIG_BATTERY_OLPC) += olpc_battery.o > diff --git a/drivers/power/supply/lenovo_yoga_c630_battery.c b/drivers/power/supply/lenovo_yoga_c630_battery.c > new file mode 100644 > index 000000000000..76152ad38d46 > --- /dev/null > +++ b/drivers/power/supply/lenovo_yoga_c630_battery.c > @@ -0,0 +1,479 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022-2024, Linaro Ltd > + * Authors: > + * Bjorn Andersson > + * Dmitry Baryshkov > + */ > +#include <linux/auxiliary_bus.h> > +#include <linux/delay.h> > +#include <linux/module.h> > +#include <linux/platform_data/lenovo-yoga-c630.h> > +#include <linux/power_supply.h> > + > +struct yoga_c630_psy { > + struct yoga_c630_ec *ec; > + struct device *dev; > + struct device_node *of_node; > + struct notifier_block nb; > + struct mutex lock; Missing a few includes, please check them here as well. > + struct power_supply *adp_psy; > + struct power_supply *bat_psy; > + > + unsigned long last_status_update; > + > + bool adapter_online; > + > + bool unit_mA; > + > + bool bat_present; > + unsigned int bat_status; > + unsigned int design_capacity; > + unsigned int design_voltage; > + unsigned int full_charge_capacity; > + > + unsigned int capacity_now; > + unsigned int voltage_now; > + > + int current_now; > + int rate_now; > +}; > + > +#define LENOVO_EC_CACHE_TIME (10 * HZ) > + > +#define LENOVO_EC_ADPT_STATUS 0xa3 > +#define LENOVO_EC_ADPT_PRESENT BIT(7) Add include for BIT() > +#define LENOVO_EC_BAT_ATTRIBUTES 0xc0 > +#define LENOVO_EC_BAT_ATTR_UNIT_IS_MA BIT(1) > +#define LENOVO_EC_BAT_STATUS 0xc1 > +#define LENOVO_EC_BAT_REMAIN_CAPACITY 0xc2 > +#define LENOVO_EC_BAT_VOLTAGE 0xc6 > +#define LENOVO_EC_BAT_DESIGN_VOLTAGE 0xc8 > +#define LENOVO_EC_BAT_DESIGN_CAPACITY 0xca > +#define LENOVO_EC_BAT_FULL_CAPACITY 0xcc > +#define LENOVO_EC_BAT_CURRENT 0xd2 > +#define LENOVO_EC_BAT_FULL_FACTORY 0xd6 > +#define LENOVO_EC_BAT_PRESENT 0xda > +#define LENOVO_EC_BAT_FULL_REGISTER 0xdb > +#define LENOVO_EC_BAT_FULL_IS_FACTORY BIT(0) > + > +/* the mutex should already be locked */ > +static int yoga_c630_psy_update_bat_info(struct yoga_c630_psy *ecbat) > +{ > + struct yoga_c630_ec *ec = ecbat->ec; > + int val; > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_PRESENT); > + if (val < 0) > + return val; > + ecbat->bat_present = !!(val & BIT(0)); Name the bit with a define. > + if (!ecbat->bat_present) > + return val; > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > + if (val < 0) > + return val; > + ecbat->unit_mA = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_DESIGN_CAPACITY); > + if (val < 0) > + return val; > + ecbat->design_capacity = val * 1000; Check linux/units.h if some WATT related one matches to that literal 1000. > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_DESIGN_VOLTAGE); > + if (val < 0) > + return val; > + ecbat->design_voltage = val; > + > + msleep(50); > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_FULL_REGISTER); > + if (val < 0) > + return val; > + if (val & LENOVO_EC_BAT_FULL_IS_FACTORY) > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_FULL_FACTORY); > + else > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_FULL_CAPACITY); You could consider doing this instead of branching: val = yoga_c630_ec_read16(ec, val & LENOVO_EC_BAT_FULL_IS_FACTORY ? LENOVO_EC_BAT_FULL_FACTORY : LENOVO_EC_BAT_FULL_CAPACITY); > + if (val < 0) > + return val; > + > + ecbat->full_charge_capacity = val * 1000; Something from linux/units.h ? > + if (!ecbat->unit_mA) { > + ecbat->design_capacity *= 10; > + ecbat->full_charge_capacity *= 10; > + } > + > + return 0; > +} > + > +/* the mutex should already be locked */ > +static int yoga_c630_psy_maybe_update_bat_status(struct yoga_c630_psy *ecbat) > +{ > + struct yoga_c630_ec *ec = ecbat->ec; > + int current_mA; > + int val; > + > + if (time_before(jiffies, ecbat->last_status_update + LENOVO_EC_CACHE_TIME)) > + return 0; > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_STATUS); > + if (val < 0) > + return val; > + ecbat->bat_status = val; > + > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_REMAIN_CAPACITY); > + if (val < 0) > + return val; > + ecbat->capacity_now = val * 1000; units.h ? > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_VOLTAGE); > + if (val < 0) > + return val; > + ecbat->voltage_now = val * 1000; Ditto. > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_CURRENT); > + if (val < 0) > + return val; > + current_mA = sign_extend32(val, 15); > + ecbat->current_now = current_mA * 1000; > + ecbat->rate_now = current_mA * (ecbat->voltage_now / 1000); Ditto. > + msleep(50); > + > + if (!ecbat->unit_mA) > + ecbat->capacity_now *= 10; > + > + ecbat->last_status_update = jiffies; Add jiffies.h > + return 0; > +} > + > +static int yoga_c630_psy_update_adapter_status(struct yoga_c630_psy *ecbat) > +{ > + struct yoga_c630_ec *ec = ecbat->ec; > + int val; > + > + mutex_lock(&ecbat->lock); This kind of functions coul use guard to take the mutex so unlock will be handled for you by the cleanup automation. > + val = yoga_c630_ec_read8(ec, LENOVO_EC_ADPT_STATUS); > + if (val > 0) > + ecbat->adapter_online = FIELD_GET(LENOVO_EC_ADPT_PRESENT, val); > + > + mutex_unlock(&ecbat->lock); > + > + return val; > +} > + > +static bool yoga_c630_psy_is_charged(struct yoga_c630_psy *ecbat) > +{ > + if (ecbat->bat_status != 0) > + return false; > + > + if (ecbat->full_charge_capacity <= ecbat->capacity_now) > + return true; > + > + if (ecbat->design_capacity <= ecbat->capacity_now) > + return true; > + > + return false; > +} > + > +static int yoga_c630_psy_bat_get_property(struct power_supply *psy, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct yoga_c630_psy *ecbat = power_supply_get_drvdata(psy); > + int rc = 0; > + > + if (!ecbat->bat_present && > + psp != POWER_SUPPLY_PROP_PRESENT) Fits to one line. > + return -ENODEV; > + > + mutex_lock(&ecbat->lock); > + rc = yoga_c630_psy_maybe_update_bat_status(ecbat); > + mutex_unlock(&ecbat->lock); > + > + if (rc) Remove empty line in between since this is the error handling. > + return rc; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_STATUS: > + if (ecbat->bat_status & BIT(0)) Name bits with defines. > + val->intval = POWER_SUPPLY_STATUS_DISCHARGING; > + else if (ecbat->bat_status & BIT(1)) Ditto. > + val->intval = POWER_SUPPLY_STATUS_CHARGING; > + else if (yoga_c630_psy_is_charged(ecbat)) > + val->intval = POWER_SUPPLY_STATUS_FULL; > + else > + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; > + break; > + case POWER_SUPPLY_PROP_PRESENT: > + val->intval = ecbat->bat_present; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN: > + val->intval = ecbat->design_voltage; > + break; > + case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: > + case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN: > + val->intval = ecbat->design_capacity; > + break; > + case POWER_SUPPLY_PROP_CHARGE_FULL: > + case POWER_SUPPLY_PROP_ENERGY_FULL: > + val->intval = ecbat->full_charge_capacity; > + break; > + case POWER_SUPPLY_PROP_CHARGE_NOW: > + case POWER_SUPPLY_PROP_ENERGY_NOW: > + val->intval = ecbat->capacity_now; > + break; > + case POWER_SUPPLY_PROP_CURRENT_NOW: > + val->intval = ecbat->current_now; > + break; > + case POWER_SUPPLY_PROP_POWER_NOW: > + val->intval = ecbat->rate_now; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + val->intval = ecbat->voltage_now; > + break; > + case POWER_SUPPLY_PROP_TECHNOLOGY: > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION; > + break; > + case POWER_SUPPLY_PROP_MODEL_NAME: > + val->strval = "PABAS0241231"; > + break; > + case POWER_SUPPLY_PROP_MANUFACTURER: > + val->strval = "Compal"; > + break; > + default: > + rc = -EINVAL; > + break; > + } > + > + return rc; > +} > +static int yoga_c630_psy_register_bat_psy(struct yoga_c630_psy *ecbat) > +{ > + struct power_supply_config bat_cfg = {}; > + > + bat_cfg.drv_data = ecbat; > + bat_cfg.of_node = ecbat->of_node; > + if (ecbat->unit_mA) > + ecbat->bat_psy = power_supply_register_no_ws(ecbat->dev, &yoga_c630_psy_bat_psy_desc_mA, &bat_cfg); > + else > + ecbat->bat_psy = power_supply_register_no_ws(ecbat->dev, &yoga_c630_psy_bat_psy_desc_mWh, &bat_cfg); Again, it might be easier to see what's different in here if the relevant parameter just uses ?: instead of full blown if/else. > + if (IS_ERR(ecbat->bat_psy)) { > + dev_err(ecbat->dev, "failed to register battery supply\n"); > + return PTR_ERR(ecbat->bat_psy); > + } > + > + return 0; > +} -- i.