Messages in this thread | | | Date | Wed, 10 Apr 2024 09:56:05 +0200 | From | Sebastian Reichel <> | Subject | Re: [PATCH v3 2/2] power: supply: ltc3350-charger: Add driver |
| |
Hi,
On Tue, Apr 09, 2024 at 03:54:41PM +0200, Mike Looijmans wrote: > The LTC3350 is a backup power controller that can charge and monitor > a series stack of one to four supercapacitors. > > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> > > ---
please share output of ./tools/testing/selftests/power_supply/test_power_supply_properties.sh below the fold with your next submission. It's useful for verifying, that you got the unit scaling correct for the standard properties :)
> (no changes since v2) > > Changes in v2: > Duplicate "vin_ov" and "vin_uv" attributes > > drivers/power/supply/Kconfig | 10 + > drivers/power/supply/Makefile | 1 + > drivers/power/supply/ltc3350-charger.c | 718 +++++++++++++++++++++++++ > 3 files changed, 729 insertions(+) > create mode 100644 drivers/power/supply/ltc3350-charger.c > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index 3e31375491d5..7cb1a66e522d 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -514,6 +514,16 @@ config CHARGER_LT3651 > Say Y to include support for the Analog Devices (Linear Technology) > LT3651 battery charger which reports its status via GPIO lines. > > +config CHARGER_LTC3350 > + tristate "LTC3350 Supercapacitor Backup Controller and System Monitor" > + depends on I2C > + select REGMAP_I2C > + select I2C_SMBUS > + help > + Say Y to include support for the Analog Devices (Linear Technology) > + LTC3350 Supercapacitor Backup Controller and System Monitor connected > + to I2C. > + > config CHARGER_LTC4162L > tristate "LTC4162-L charger" > depends on I2C > diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile > index 58b567278034..a8d618e4ac91 100644 > --- a/drivers/power/supply/Makefile > +++ b/drivers/power/supply/Makefile > @@ -72,6 +72,7 @@ obj-$(CONFIG_CHARGER_LP8788) += lp8788-charger.o > obj-$(CONFIG_CHARGER_GPIO) += gpio-charger.o > obj-$(CONFIG_CHARGER_MANAGER) += charger-manager.o > obj-$(CONFIG_CHARGER_LT3651) += lt3651-charger.o > +obj-$(CONFIG_CHARGER_LTC3350) += ltc3350-charger.o > obj-$(CONFIG_CHARGER_LTC4162L) += ltc4162-l-charger.o > obj-$(CONFIG_CHARGER_MAX14577) += max14577_charger.o > obj-$(CONFIG_CHARGER_DETECTOR_MAX14656) += max14656_charger_detector.o > diff --git a/drivers/power/supply/ltc3350-charger.c b/drivers/power/supply/ltc3350-charger.c > new file mode 100644 > index 000000000000..84c7a3ca914e > --- /dev/null > +++ b/drivers/power/supply/ltc3350-charger.c > @@ -0,0 +1,718 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Driver for Analog Devices (Linear Technology) LTC3350 > + * High Current Supercapacitor Backup Controller and System Monitor > + * Copyright (C) 2024, Topic Embedded Products > + */ > + > +#include <linux/module.h> > +#include <linux/delay.h> > +#include <linux/of_device.h>
replace of_device.h with mod_devicetable.h (which defines of_device_id).
> +#include <linux/pm_runtime.h> > +#include <linux/power_supply.h>
add
#include <linux/property.h>
since you are using device_property_*
> +#include <linux/i2c.h> > +#include <linux/regmap.h> > + > +/* Registers (names based on what datasheet uses) */ > +#define LTC3350_REG_CLR_ALARMS 0x00 > +#define LTC3350_REG_MSK_ALARMS 0x01 > +#define LTC3350_REG_MSK_MON_STATUS 0x02 > +#define LTC3350_REG_CAP_ESR_PER 0x04 > +#define LTC3350_REG_VCAPFB_DAC 0x05 > +#define LTC3350_REG_VSHUNT 0x06 > +#define LTC3350_REG_CAP_UV_LVL 0x07 > +#define LTC3350_REG_CAP_OV_LVL 0x08 > +#define LTC3350_REG_GPI_UV_LVL 0x09 > +#define LTC3350_REG_GPI_OV_LVL 0x0A > +#define LTC3350_REG_VIN_UV_LVL 0x0B > +#define LTC3350_REG_VIN_OV_LVL 0x0C > +#define LTC3350_REG_VCAP_UV_LVL 0x0D > +#define LTC3350_REG_VCAP_OV_LVL 0x0E > +#define LTC3350_REG_VOUT_UV_LVL 0x0F > +#define LTC3350_REG_VOUT_OV_LVL 0x10 > +#define LTC3350_REG_IIN_OC_LVL 0x11 > +#define LTC3350_REG_ICHG_UC_LVL 0x12 > +#define LTC3350_REG_DTEMP_COLD_LVL 0x13 > +#define LTC3350_REG_DTEMP_HOT_LVL 0x14 > +#define LTC3350_REG_ESR_HI_LVL 0x15 > +#define LTC3350_REG_CAP_LO_LVL 0x16 > +#define LTC3350_REG_CTL_REG 0x17 > +#define LTC3350_REG_NUM_CAPS 0x1A > +#define LTC3350_REG_CHRG_STATUS 0x1B > +#define LTC3350_REG_MON_STATUS 0x1C > +#define LTC3350_REG_ALARM_REG 0x1D > +#define LTC3350_REG_MEAS_CAP 0x1E > +#define LTC3350_REG_MEAS_ESR 0x1F > +#define LTC3350_REG_MEAS_VCAP1 0x20 > +#define LTC3350_REG_MEAS_VCAP2 0x21 > +#define LTC3350_REG_MEAS_VCAP3 0x22 > +#define LTC3350_REG_MEAS_VCAP4 0x23 > +#define LTC3350_REG_MEAS_GPI 0x24 > +#define LTC3350_REG_MEAS_VIN 0x25 > +#define LTC3350_REG_MEAS_VCAP 0x26 > +#define LTC3350_REG_MEAS_VOUT 0x27 > +#define LTC3350_REG_MEAS_IIN 0x28 > +#define LTC3350_REG_MEAS_ICHG 0x29 > +#define LTC3350_REG_MEAS_DTEMP 0x2A > + > +/* LTC3350_REG_CLR_ALARMS, LTC3350_REG_MASK_ALARMS, LTC3350_REG_ALARM_REG */ > +#define LTC3350_MSK_CAP_UV BIT(0) /* capacitor undervoltage alarm */ > +#define LTC3350_MSK_CAP_OV BIT(1) /* capacitor overvoltage alarm */ > +#define LTC3350_MSK_GPI_UV BIT(2) /* GPI undervoltage alarm */ > +#define LTC3350_MSK_GPI_OV BIT(3) /* GPI overvoltage alarm */ > +#define LTC3350_MSK_VIN_UV BIT(4) /* VIN undervoltage alarm */ > +#define LTC3350_MSK_VIN_OV BIT(5) /* VIN overvoltage alarm */ > +#define LTC3350_MSK_VCAP_UV BIT(6) /* VCAP undervoltage alarm */ > +#define LTC3350_MSK_VCAP_OV BIT(7) /* VCAP overvoltage alarm */ > +#define LTC3350_MSK_VOUT_UV BIT(8) /* VOUT undervoltage alarm */ > +#define LTC3350_MSK_VOUT_OV BIT(9) /* VOUT overvoltage alarm */ > +#define LTC3350_MSK_IIN_OC BIT(10) /* input overcurrent alarm */ > +#define LTC3350_MSK_ICHG_UC BIT(11) /* charge undercurrent alarm */ > +#define LTC3350_MSK_DTEMP_COLD BIT(12) /* die temperature cold alarm */ > +#define LTC3350_MSK_DTEMP_HOT BIT(13) /* die temperature hot alarm */ > +#define LTC3350_MSK_ESR_HI BIT(14) /* ESR high alarm */ > +#define LTC3350_MSK_CAP_LO BIT(15) /* capacitance low alarm */ > + > +/* LTC3350_REG_MSK_MON_STATUS masks */ > +#define LTC3350_MSK_MON_CAPESR_ACTIVE BIT(0) > +#define LTC3350_MSK_MON_CAPESR_SCHEDULED BIT(1) > +#define LTC3350_MSK_MON_CAPESR_PENDING BIT(2) > +#define LTC3350_MSK_MON_CAP_DONE BIT(3) > +#define LTC3350_MSK_MON_ESR_DONE BIT(4) > +#define LTC3350_MSK_MON_CAP_FAILED BIT(5) > +#define LTC3350_MSK_MON_ESR_FAILED BIT(6) > +#define LTC3350_MSK_MON_POWER_FAILED BIT(8) > +#define LTC3350_MSK_MON_POWER_RETURNED BIT(9) > + > +/* LTC3350_REG_CTL_REG */ > +/* Begin a capacitance and ESR measurement when possible */ > +#define LTC3350_CTL_STRT_CAPESR BIT(0) > +/* A one in this bit location enables the input buffer on the GPI pin */ > +#define LTC3350_CTL_GPI_BUFFER_EN BIT(1) > +/* Stops an active capacitance/ESR measurement */ > +#define LTC3350_CTL_STOP_CAPESR BIT(2) > +/* Increases capacitor measurement resolution by 100x for smaller capacitors */ > +#define LTC3350_CTL_CAP_SCALE BIT(3) > + > +/* LTC3350_REG_CHRG_STATUS */ > +#define LTC3350_CHRG_STEPDOWN BIT(0) /* Synchronous controller in step-down mode (charging) */ > +#define LTC3350_CHRG_STEPUP BIT(1) /* Synchronous controller in step-up mode (backup) */ > +#define LTC3350_CHRG_CV BIT(2) /* The charger is in constant voltage mode */ > +#define LTC3350_CHRG_UVLO BIT(3) /* The charger is in undervoltage lockout */ > +#define LTC3350_CHRG_INPUT_ILIM BIT(4) /* The charger is in input current limit */ > +#define LTC3350_CHRG_CAPPG BIT(5) /* The capacitor voltage is above power good threshold */ > +#define LTC3350_CHRG_SHNT BIT(6) /* The capacitor manager is shunting */ > +#define LTC3350_CHRG_BAL BIT(7) /* The capacitor manager is balancing */ > +#define LTC3350_CHRG_DIS BIT(8) /* Charger disabled for capacitance measurement */ > +#define LTC3350_CHRG_CI BIT(9) /* The charger is in constant current mode */ > +#define LTC3350_CHRG_PFO BIT(11) /* Input voltage is below PFI threshold */ > + > +/* LTC3350_REG_MON_STATUS */ > +#define LTC3350_MON_CAPESR_ACTIVE BIT(0) /* Capacitance/ESR measurement in progress */ > +#define LTC3350_MON_CAPESR_SCHEDULED BIT(1) /* Waiting programmed time */ > +#define LTC3350_MON_CAPESR_PENDING BIT(2) /* Waiting for satisfactory conditions */ > +#define LTC3350_MON_CAP_DONE BIT(3) /* Capacitance measurement has completed */ > +#define LTC3350_MON_ESR_DONE BIT(4) /* ESR Measurement has completed */ > +#define LTC3350_MON_CAP_FAILED BIT(5) /* Last capacitance measurement failed */ > +#define LTC3350_MON_ESR_FAILED BIT(6) /* Last ESR measurement failed */ > +#define LTC3350_MON_POWER_FAILED BIT(8) /* Unable to charge */ > +#define LTC3350_MON_POWER_RETURNED BIT(9) /* Able to charge */ > + > + > +struct ltc3350_info { > + struct i2c_client *client; > + struct regmap *regmap; > + struct power_supply *charger; > + u32 rsnsc; /* Series resistor that sets charge current, microOhm */ > + u32 rsnsi; /* Series resistor to measure input current, microOhm */ > +}; > + > +/* > + * About LTC3350 "alarm" functions: Setting a bit in the LTC3350_REG_MSK_ALARMS > + * register enables the alarm. The alarm will trigger an SMBALERT only once. > + * To reset the alarm, write a "1" bit to LTC3350_REG_CLR_ALARMS. Then the alarm > + * will trigger another SMBALERT when conditions are met (may be immediately). > + * After writing to one of the corresponding level registers, enable the alarm, > + * so that a UEVENT triggers when the alarm goes off. > + */ > +static void ltc3350_enable_alarm(struct ltc3350_info *info, unsigned int reg) > +{ > + unsigned int mask; > + > + /* Register locations correspond to alarm mask bits */ > + mask = BIT(reg - LTC3350_REG_CAP_UV_LVL); > + /* Clear the alarm bit so it may trigger again */ > + regmap_write(info->regmap, LTC3350_REG_CLR_ALARMS, mask); > + /* Enable the alarm */ > + regmap_update_bits(info->regmap, LTC3350_REG_MSK_ALARMS, mask, mask); > +} > + > +/* Convert enum value to POWER_SUPPLY_STATUS value */ > +static int ltc3350_state_decode(unsigned int value) > +{ > + if (value & LTC3350_CHRG_STEPUP) > + return POWER_SUPPLY_STATUS_DISCHARGING; /* running on backup */ > + > + if (value & LTC3350_CHRG_PFO) > + return POWER_SUPPLY_STATUS_NOT_CHARGING; > + > + if (value & LTC3350_CHRG_STEPDOWN) { > + /* The chip remains in CV mode indefinitely, hence "full" */ > + if (value & LTC3350_CHRG_CV) > + return POWER_SUPPLY_STATUS_FULL; > + > + return POWER_SUPPLY_STATUS_CHARGING; > + } > + > + /* Not in step down? Must be full then (never seen this) */ > + return POWER_SUPPLY_STATUS_FULL; > +}; > + > +static int ltc3350_get_status(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_CHRG_STATUS, ®val); > + if (ret) > + return ret; > + > + val->intval = ltc3350_state_decode(regval); > + > + return 0; > +} > + > +static int ltc3350_charge_status_decode(unsigned int value) > +{ > + if (!(value & LTC3350_CHRG_STEPDOWN)) > + return POWER_SUPPLY_CHARGE_TYPE_NONE; > + > + /* constant voltage is "topping off" */ > + if (value & LTC3350_CHRG_CV) > + return POWER_SUPPLY_CHARGE_TYPE_TRICKLE; > + > + /* input limiter */ > + if (value & LTC3350_CHRG_INPUT_ILIM) > + return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE; > + > + /* constant current is "fast" */ > + if (value & LTC3350_CHRG_CI) > + return POWER_SUPPLY_CHARGE_TYPE_FAST; > + > + return POWER_SUPPLY_CHARGE_TYPE_UNKNOWN; > +} > + > +static int ltc3350_get_charge_type(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_CHRG_STATUS, ®val); > + if (ret) > + return ret; > + > + val->intval = ltc3350_charge_status_decode(regval); > + > + return 0; > +} > + > +static int ltc3350_get_online(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_MON_STATUS, ®val); > + if (ret) > + return ret; > + > + /* indicates if input voltage is sufficient to charge */ > + val->intval = !!(regval & LTC3350_MON_POWER_RETURNED); > + > + return 0; > +} > + > +static int ltc3350_get_input_voltage(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_MEAS_VIN, ®val); > + if (ret) > + return ret; > + > + /* 2.21mV/LSB */ > + val->intval = regval * 2210; > + > + return 0; > +} > + > +static int ltc3350_get_input_current(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_MEAS_IIN, ®val); > + if (ret) > + return ret; > + > + /* 1.983µV/RSNSI amperes per LSB */ > + ret = regval * 19830; > + ret /= info->rsnsi; > + ret *= 100; > + > + val->intval = ret; > + > + return 0; > +} > + > +static int ltc3350_get_icharge(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_MEAS_ICHG, ®val); > + if (ret) > + return ret; > + > + /* 1.983µV/RSNSC amperes per LSB */ > + ret = regval * 19830; > + ret /= info->rsnsc; > + ret *= 100; > + > + val->intval = ret; > + > + return 0; > +} > + > +static int ltc3350_get_icharge_max(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + /* I_CHG(MAX) = 32mV / RSNSC (Ampere) */ > + val->intval = 3200000000U / (info->rsnsc / 10); > + > + return 0; > +} > + > +static int ltc3350_get_iin_max(struct ltc3350_info *info, union power_supply_propval *val) > +{ > + /* I_IN(MAX) = 32mV / RSNSI (Ampere) */ > + val->intval = 3200000000U / (info->rsnsi / 10); > + > + return 0; > +} > + > + > +static int ltc3350_get_die_temp(struct ltc3350_info *info, unsigned int reg, > + union power_supply_propval *val) > +{ > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, reg, ®val); > + if (ret) > + return ret; > + > + /* 0.028°C per LSB – 251.4°C */ > + ret = 280 * regval; > + ret /= 100; /* Centidegrees scale */ > + ret -= 25140; > + val->intval = ret; > + > + return 0; > +} > + > +static int ltc3350_set_die_temp(struct ltc3350_info *info, unsigned int reg, int val) > +{ > + unsigned int regval; > + int ret; > + > + /* 0.028°C per LSB – 251.4°C */ > + regval = val + 25140; > + regval *= 100; > + regval /= 280; > + > + ret = regmap_write(info->regmap, reg, regval); > + if (ret) > + return ret; > + > + ltc3350_enable_alarm(info, reg); > + return 0; > +} > + > +/* Custom properties */ > + > +static ssize_t ltc3350_attr_show(struct device *dev, > + struct device_attribute *attr, char *buf, > + unsigned int reg, unsigned int scale) > +{ > + struct power_supply *psy = to_power_supply(dev); > + struct ltc3350_info *info = power_supply_get_drvdata(psy); > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, reg, ®val); > + if (ret) > + return ret; > + > + regval *= scale; /* Scale is in 10 μV units */
please keep custom uAPI consistent with the general uAPI and use µV.
> + regval /= 10; > + > + return sprintf(buf, "%u\n", regval); > +} > + > +static ssize_t ltc3350_attr_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count, > + unsigned int reg, unsigned int scale) > +{ > + struct power_supply *psy = to_power_supply(dev); > + struct ltc3350_info *info = power_supply_get_drvdata(psy); > + unsigned long val; > + int ret; > + > + ret = kstrtoul(buf, 10, &val); > + if (ret) > + return ret; > + > + val *= 10; > + val = DIV_ROUND_CLOSEST(val, scale); /* Scale is in 10 μV units */
obviously also applied to writing.
> + > + ret = regmap_write(info->regmap, reg, val); > + if (ret) > + return ret; > + > + /* When writing to one of the LVL registers, update the alarm mask */ > + if (reg >= LTC3350_REG_CAP_UV_LVL && reg <= LTC3350_REG_CAP_LO_LVL) > + ltc3350_enable_alarm(info, reg); > + > + return count; > +} > + > +#define LTC3350_DEVICE_ATTR_RO(_name, _reg, _scale) \ > +static ssize_t _name##_show(struct device *dev, struct device_attribute *attr, \ > + char *buf) \ > +{ \ > + return ltc3350_attr_show(dev, attr, buf, _reg, _scale); \ > +} \ > +static DEVICE_ATTR_RO(_name) > + > +#define LTC3350_DEVICE_ATTR_RW(_name, _reg, _scale) \ > +static ssize_t _name##_show(struct device *dev, struct device_attribute *attr, \ > + char *buf) \ > +{ \ > + return ltc3350_attr_show(dev, attr, buf, _reg, _scale); \ > +} \ > +static ssize_t _name##_store(struct device *dev, struct device_attribute *attr, \ > + const char *buf, size_t count) \ > +{ \ > + return ltc3350_attr_store(dev, attr, buf, count, _reg, _scale); \ > +} \ > +static DEVICE_ATTR_RW(_name) > + > +/* Shunt voltage, 183.5μV per LSB */ > +LTC3350_DEVICE_ATTR_RW(vshunt, LTC3350_REG_VSHUNT, 1835); > + > +/* Single capacitor voltages, 183.5μV per LSB */ > +LTC3350_DEVICE_ATTR_RO(vcap1, LTC3350_REG_MEAS_VCAP1, 1835); > +LTC3350_DEVICE_ATTR_RO(vcap2, LTC3350_REG_MEAS_VCAP2, 1835); > +LTC3350_DEVICE_ATTR_RO(vcap3, LTC3350_REG_MEAS_VCAP3, 1835); > +LTC3350_DEVICE_ATTR_RO(vcap4, LTC3350_REG_MEAS_VCAP4, 1835); > +LTC3350_DEVICE_ATTR_RW(cap_uv, LTC3350_REG_CAP_UV_LVL, 1835); > +LTC3350_DEVICE_ATTR_RW(cap_ov, LTC3350_REG_CAP_OV_LVL, 1835); > + > +/* General purpose input, 183.5μV per LSB */ > +LTC3350_DEVICE_ATTR_RO(gpi, LTC3350_REG_MEAS_GPI, 1835); > +LTC3350_DEVICE_ATTR_RW(gpi_uv, LTC3350_REG_GPI_UV_LVL, 1835); > +LTC3350_DEVICE_ATTR_RW(gpi_ov, LTC3350_REG_GPI_OV_LVL, 1835); > + > +/* Input voltage, 2.21mV per LSB */ > +LTC3350_DEVICE_ATTR_RO(vin, LTC3350_REG_MEAS_VIN, 22100); > +LTC3350_DEVICE_ATTR_RW(vin_uv, LTC3350_REG_VIN_UV_LVL, 22100); > +LTC3350_DEVICE_ATTR_RW(vin_ov, LTC3350_REG_VIN_OV_LVL, 22100); > + > +/* Capacitor stack voltage, 1.476 mV per LSB */ > +LTC3350_DEVICE_ATTR_RO(vcap, LTC3350_REG_MEAS_VCAP, 14760); > +LTC3350_DEVICE_ATTR_RW(vcap_uv, LTC3350_REG_VCAP_UV_LVL, 14760); > +LTC3350_DEVICE_ATTR_RW(vcap_ov, LTC3350_REG_VCAP_OV_LVL, 14760);
I suppose it would be sensible to expose this as a second power_supply device of type battery with ltc3350_config.supplied_to set to this second power_supply device.
Then you can map
LTC3350_REG_MEAS_VCAP -> VOLTAGE_NOW LTC3350_REG_VCAP_UV_LVL -> VOLTAGE_MIN LTC3350_REG_VCAP_OV_LVL -> VOLTAGE_MAX LTC3350_REG_VSHUNT -> CURRENT_NOW TECHNOLOGY = POWER_SUPPLY_TECHNOLOGY_CAPACITOR (new)
Also the single capacitor voltages are similar to per-cell voltage information, so I think we should create generic properties for that:
LTC3350_REG_NUM_CAPS -> NUMBER_OF_CELLS (new) LTC3350_REG_MEAS_VCAP1 -> CELL1_VOLTAGE_NOW (new) LTC3350_REG_MEAS_VCAP2 -> CELL2_VOLTAGE_NOW (new) LTC3350_REG_MEAS_VCAP3 -> CELL3_VOLTAGE_NOW (new) LTC3350_REG_MEAS_VCAP4 -> CELL4_VOLTAGE_NOW (new) LTC3350_REG_CAP_UV_LVL -> CELL_VOLTAGE_MIN (new) LTC3350_REG_CAP_OV_LVL -> CELL_VOLTAGE_MAX (new)
> +/* Output, 2.21mV per LSB */ > +LTC3350_DEVICE_ATTR_RO(vout, LTC3350_REG_MEAS_VOUT, 22100); > +LTC3350_DEVICE_ATTR_RW(vout_uv, LTC3350_REG_VOUT_UV_LVL, 22100); > +LTC3350_DEVICE_ATTR_RW(vout_ov, LTC3350_REG_VOUT_OV_LVL, 22100); > + > +static ssize_t num_caps_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct power_supply *psy = to_power_supply(dev); > + struct ltc3350_info *info = power_supply_get_drvdata(psy); > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_NUM_CAPS, ®val); > + if (ret) > + return ret; > + > + return sprintf(buf, "%u\n", regval + 1); > +} > +static DEVICE_ATTR_RO(num_caps); > + > +static ssize_t charge_status_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct power_supply *psy = to_power_supply(dev); > + struct ltc3350_info *info = power_supply_get_drvdata(psy); > + unsigned int regval; > + int ret; > + > + ret = regmap_read(info->regmap, LTC3350_REG_CHRG_STATUS, ®val); > + if (ret) > + return ret; > + > + return sprintf(buf, "0x%x\n", regval); > +} > +static DEVICE_ATTR_RO(charge_status); > + > +static struct attribute *ltc3350_sysfs_entries[] = { > + &dev_attr_vshunt.attr, > + &dev_attr_vcap1.attr, > + &dev_attr_vcap2.attr, > + &dev_attr_vcap3.attr, > + &dev_attr_vcap4.attr, > + &dev_attr_cap_uv.attr, > + &dev_attr_cap_ov.attr, > + &dev_attr_gpi.attr, > + &dev_attr_gpi_uv.attr, > + &dev_attr_gpi_ov.attr, > + &dev_attr_vin.attr, > + &dev_attr_vin_uv.attr, > + &dev_attr_vin_ov.attr, > + &dev_attr_vcap.attr, > + &dev_attr_vcap_uv.attr, > + &dev_attr_vcap_ov.attr, > + &dev_attr_vout.attr, > + &dev_attr_vout_uv.attr, > + &dev_attr_vout_ov.attr, > + &dev_attr_num_caps.attr, > + &dev_attr_charge_status.attr, > + NULL, > +};
Exposing these to sysfs makes them ABI and ABI needs to be documented in Documentation/ABI, see for example
Documentation/ABI/testing/sysfs-class-power-rt9467
> +static const struct attribute_group ltc3350_attr_group = { > + .name = NULL, /* put in device directory */ > + .attrs = ltc3350_sysfs_entries, > +}; > + > +static const struct attribute_group *ltc3350_attr_groups[] = { > + <c3350_attr_group, > + NULL, > +}; > + > +static int ltc3350_get_property(struct power_supply *psy, enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct ltc3350_info *info = power_supply_get_drvdata(psy); > + > + switch (psp) { > + case POWER_SUPPLY_PROP_STATUS: > + return ltc3350_get_status(info, val); > + case POWER_SUPPLY_PROP_CHARGE_TYPE: > + return ltc3350_get_charge_type(info, val); > + case POWER_SUPPLY_PROP_ONLINE: > + return ltc3350_get_online(info, val); > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + return ltc3350_get_input_voltage(info, val); > + case POWER_SUPPLY_PROP_CURRENT_NOW: > + return ltc3350_get_input_current(info, val); > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT: > + return ltc3350_get_icharge(info, val); > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: > + return ltc3350_get_icharge_max(info, val); > + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT: > + return ltc3350_get_iin_max(info, val); > + case POWER_SUPPLY_PROP_TEMP: > + return ltc3350_get_die_temp(info, LTC3350_REG_MEAS_DTEMP, val); > + case POWER_SUPPLY_PROP_TEMP_ALERT_MIN: > + return ltc3350_get_die_temp(info, LTC3350_REG_DTEMP_COLD_LVL, val); > + case POWER_SUPPLY_PROP_TEMP_ALERT_MAX: > + return ltc3350_get_die_temp(info, LTC3350_REG_DTEMP_HOT_LVL, val); > + default: > + return -EINVAL; > + } > +} > + > +static int ltc3350_set_property(struct power_supply *psy, enum power_supply_property psp, > + const union power_supply_propval *val) > +{ > + struct ltc3350_info *info = power_supply_get_drvdata(psy); > + > + switch (psp) { > + case POWER_SUPPLY_PROP_TEMP_ALERT_MIN: > + return ltc3350_set_die_temp(info, LTC3350_REG_DTEMP_COLD_LVL, val->intval); > + case POWER_SUPPLY_PROP_TEMP_ALERT_MAX: > + return ltc3350_set_die_temp(info, LTC3350_REG_DTEMP_HOT_LVL, val->intval); > + default: > + return -EINVAL; > + } > +} > + > +static int ltc3350_property_is_writeable(struct power_supply *psy, enum power_supply_property psp) > +{ > + switch (psp) { > + case POWER_SUPPLY_PROP_TEMP_ALERT_MIN: > + case POWER_SUPPLY_PROP_TEMP_ALERT_MAX: > + return 1; > + default: > + return 0; > + } > +} > + > +/* Charger power supply property routines */ > +static enum power_supply_property ltc3350_properties[] = { > + POWER_SUPPLY_PROP_STATUS, > + POWER_SUPPLY_PROP_CHARGE_TYPE, > + POWER_SUPPLY_PROP_ONLINE, > + POWER_SUPPLY_PROP_VOLTAGE_NOW, > + POWER_SUPPLY_PROP_CURRENT_NOW, > + POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT, > + POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX, > + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, > + POWER_SUPPLY_PROP_TEMP, > + POWER_SUPPLY_PROP_TEMP_ALERT_MIN, > + POWER_SUPPLY_PROP_TEMP_ALERT_MAX, > +}; > + > +static const struct power_supply_desc ltc3350_desc = { > + .name = "ltc3350", > + .type = POWER_SUPPLY_TYPE_MAINS, > + .properties = ltc3350_properties, > + .num_properties = ARRAY_SIZE(ltc3350_properties), > + .get_property = ltc3350_get_property, > + .set_property = ltc3350_set_property, > + .property_is_writeable = ltc3350_property_is_writeable, > +}; > + > +static bool ltc3350_is_writeable_reg(struct device *dev, unsigned int reg) > +{ > + /* all registers up to this one are writeable */ > + return reg < LTC3350_REG_NUM_CAPS; > +} > + > +static bool ltc3350_is_volatile_reg(struct device *dev, unsigned int reg) > +{ > + /* read-only status registers and self-clearing register */ > + return reg > LTC3350_REG_NUM_CAPS || reg == LTC3350_REG_CLR_ALARMS; > +} > + > +static const struct regmap_config ltc3350_regmap_config = { > + .reg_bits = 8, > + .val_bits = 16, > + .val_format_endian = REGMAP_ENDIAN_LITTLE, > + .writeable_reg = ltc3350_is_writeable_reg, > + .volatile_reg = ltc3350_is_volatile_reg, > + .max_register = LTC3350_REG_MEAS_DTEMP, > + .cache_type = REGCACHE_MAPLE, > +}; > + > +static int ltc3350_probe(struct i2c_client *client) > +{ > + struct i2c_adapter *adapter = client->adapter; > + struct device *dev = &client->dev; > + struct ltc3350_info *info; > + struct power_supply_config ltc3350_config = {}; > + int ret; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) { > + dev_err(dev, "No support for SMBUS_WORD_DATA\n"); > + return -ENODEV; > + }
return dev_err_probe(dev, -ENODEV, "No support for SMBUS_WORD_DATA\n");
But I think this can just be dropped. devm_regmap_init_i2c() should generate an error, if the i2c adapter requirements are not met.
> + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL); > + if (!info) > + return -ENOMEM; > + > + info->client = client; > + i2c_set_clientdata(client, info); > + > + info->regmap = devm_regmap_init_i2c(client, <c3350_regmap_config); > + if (IS_ERR(info->regmap)) { > + dev_err(dev, "Failed to initialize register map\n"); > + return PTR_ERR(info->regmap); > + }
dev_err_probe()
> + > + ret = device_property_read_u32(dev, "lltc,rsnsc-micro-ohms", > + &info->rsnsc); > + if (ret) { > + dev_err(dev, "Missing lltc,rsnsc-micro-ohms property\n"); > + return ret; > + }
dev_err_probe()
> + if (!info->rsnsc) > + return -EINVAL; > + > + ret = device_property_read_u32(dev, "lltc,rsnsi-micro-ohms", > + &info->rsnsi); > + if (ret) { > + dev_err(dev, "Missing lltc,rsnsi-micro-ohms property\n"); > + return ret; > + }
dev_err_probe()
> + if (!info->rsnsi) > + return -EINVAL; > + > + /* Clear and disable all interrupt sources*/ > + ret = regmap_write(info->regmap, LTC3350_REG_CLR_ALARMS, 0xFFFF); > + if (ret) { > + dev_err(dev, "Failed to write configuration\n"); > + return ret; > + }
dev_err_probe()
> + regmap_write(info->regmap, LTC3350_REG_MSK_ALARMS, 0); > + regmap_write(info->regmap, LTC3350_REG_MSK_MON_STATUS, 0); > + > + ltc3350_config.of_node = dev->of_node;
replace with
ltc3350_config.fwnode = dev_fwnode(dev);
> + ltc3350_config.drv_data = info; > + ltc3350_config.attr_grp = ltc3350_attr_groups; > + > + info->charger = devm_power_supply_register(dev, <c3350_desc, > + <c3350_config); > + if (IS_ERR(info->charger)) { > + dev_err(dev, "Failed to register charger\n"); > + return PTR_ERR(info->charger); > + }
dev_err_probe()
> + > + /* Enable interrupts on status changes */ > + regmap_write(info->regmap, LTC3350_REG_MSK_MON_STATUS, > + LTC3350_MON_POWER_FAILED | LTC3350_MON_POWER_RETURNED); > + > + return 0; > +} > + > +static void ltc3350_alert(struct i2c_client *client, enum i2c_alert_protocol type, > + unsigned int flag) > +{ > + struct ltc3350_info *info = i2c_get_clientdata(client); > + > + if (type != I2C_PROTOCOL_SMBUS_ALERT) > + return; > + > + power_supply_changed(info->charger); > +} > + > +static const struct i2c_device_id ltc3350_i2c_id_table[] = { > + { "ltc3350", 0 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(i2c, ltc3350_i2c_id_table); > + > +static const struct of_device_id ltc3350_of_match[] = { > + { .compatible = "lltc,ltc3350", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, ltc3350_of_match); > + > +static struct i2c_driver ltc3350_driver = { > + .probe = ltc3350_probe, > + .alert = ltc3350_alert, > + .id_table = ltc3350_i2c_id_table, > + .driver = { > + .name = "ltc3350-charger", > + .of_match_table = of_match_ptr(ltc3350_of_match),
Please check what of_match_ptr() does and then drop it :)
> + }, > +}; > +module_i2c_driver(ltc3350_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Mike Looijmans <mike.looijmans@topic.nl>"); > +MODULE_DESCRIPTION("LTC3350 charger driver");
Thanks for your patch,
-- Sebastian [unhandled content-type:application/pgp-signature] | |