Sebastian

On 1/2/21 7:26 PM, Sebastian Reichel wrote:
Hi Ricardo,

On Wed, Dec 30, 2020 at 05:01:16PM -0600, Ricardo Rivera-Matos wrote:
The BQ256XX family of devices are highly integrated buck chargers
for single cell batteries.

Signed-off-by: Ricardo Rivera-Matos <r-rivera-ma...@ti.com>

v5 - adds power_supply_put_battery_info() and devm_add_action_or_rest() calls

v6 - implements bq256xx_remove function

v7 - applies various fixes

    - implements clamp() API

    - implements memcmp() API

    - changes cache_type to REGACHE_FLAT

    - changes bq256xx_probe to properly unregister device

Signed-off-by: Ricardo Rivera-Matos <r-rivera-ma...@ti.com>
---
Thanks, looks mostly good now.
Cool :)

  drivers/power/supply/Kconfig           |   11 +
  drivers/power/supply/Makefile          |    1 +
  drivers/power/supply/bq256xx_charger.c | 1747 ++++++++++++++++++++++++
  3 files changed, 1759 insertions(+)
  create mode 100644 drivers/power/supply/bq256xx_charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 44d3c8512fb8..87d852914bc2 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -618,6 +618,17 @@ config CHARGER_BQ25890
        help
          Say Y to enable support for the TI BQ25890 battery charger.
+config CHARGER_BQ256XX
+       tristate "TI BQ256XX battery charger driver"
+       depends on I2C
+       depends on GPIOLIB || COMPILE_TEST
+       select REGMAP_I2C
+       help
+         Say Y to enable support for the TI BQ256XX battery chargers. The
+         BQ256XX family of devices are highly-integrated, switch-mode battery
+         charge management and system power path management devices for single
+         cell Li-ion and Li-polymer batteries.
+
  config CHARGER_SMB347
        tristate "Summit Microelectronics SMB347 Battery Charger"
        depends on I2C
Please rebase to current power-supply for-next branch, Kconfig and
Makefile diff does not apply because of one additional BQ device.
ACK

[...]
+static void bq256xx_usb_work(struct work_struct *data)
+{
+       struct bq256xx_device *bq =
+                       container_of(data, struct bq256xx_device, usb_work);
+
+       switch (bq->usb_event) {
+       case USB_EVENT_ID:
+               break;
+
spurious newline, please remove!
ACK

+       case USB_EVENT_NONE:
+               power_supply_changed(bq->charger);
+               break;
+       default:
+               dev_err(bq->dev, "Error switching to charger mode.\n");
+               break;
+       }
+}
+
[...]
+static int bq256xx_hw_init(struct bq256xx_device *bq)
+{
+       struct power_supply_battery_info bat_info = { };
+       int wd_reg_val = BQ256XX_WATCHDOG_DIS;
+       int ret = 0;
+       int i;
+
+       for (i = 0; i < BQ256XX_NUM_WD_VAL; i++) {
+               if (bq->watchdog_timer > bq256xx_watchdog_time[i] &&
+                   bq->watchdog_timer < bq256xx_watchdog_time[i + 1])
+                       wd_reg_val = i;
+       }
+       ret = regmap_update_bits(bq->regmap, BQ256XX_CHARGER_CONTROL_1,
+                                BQ256XX_WATCHDOG_MASK, wd_reg_val <<
+                                               BQ256XX_WDT_BIT_SHIFT);
+
+       ret = power_supply_get_battery_info(bq->charger, &bat_info);
+       if (ret) {
+               dev_warn(bq->dev, "battery info missing, default values will be 
applied\n");
+
+               bat_info.constant_charge_current_max_ua =
+                               bq->chip_info->bq256xx_def_ichg;
+
+               bat_info.constant_charge_voltage_max_uv =
+                               bq->chip_info->bq256xx_def_vbatreg;
+
+               bat_info.precharge_current_ua =
+                               bq->chip_info->bq256xx_def_iprechg;
+
+               bat_info.charge_term_current_ua =
+                               bq->chip_info->bq256xx_def_iterm;
+
+               bq->init_data.ichg_max =
+                               bq->chip_info->bq256xx_max_ichg;
+
+               bq->init_data.vbatreg_max =
+                               bq->chip_info->bq256xx_max_vbatreg;
+       } else {
+               bq->init_data.ichg_max =
+                       bat_info.constant_charge_current_max_ua;
+
+               bq->init_data.vbatreg_max =
+                       bat_info.constant_charge_voltage_max_uv;
+       }
+
+       ret = bq->chip_info->bq256xx_set_vindpm(bq, bq->init_data.vindpm);
+       if (ret)
+               goto err_out;
+
+       ret = bq->chip_info->bq256xx_set_iindpm(bq, bq->init_data.iindpm);
+       if (ret)
+               goto err_out;
+
+       ret = bq->chip_info->bq256xx_set_ichg(bq,
+                               bat_info.constant_charge_current_max_ua);
+       if (ret)
+               goto err_out;
+
+       ret = bq->chip_info->bq256xx_set_iprechg(bq,
+                               bat_info.precharge_current_ua);
+       if (ret)
+               goto err_out;
+
+       ret = bq->chip_info->bq256xx_set_vbatreg(bq,
+                               bat_info.constant_charge_voltage_max_uv);
+       if (ret)
+               goto err_out;
+
+       ret = bq->chip_info->bq256xx_set_iterm(bq,
+                               bat_info.charge_term_current_ua);
+       if (ret)
+               goto err_out;
+
+       power_supply_put_battery_info(bq->charger, &bat_info);
+
+       return 0;
+
+err_out:
+       return ret;
please return error code directly instead of adding this useless
goto.
ACK

[...]
-- Sebastian
Ricardo

Reply via email to