From 01fb70772ebb4c9b3d36f743c7bd54da72f82729 Mon Sep 17 00:00:00 2001 From: zhiayang Date: Wed, 21 Dec 2022 17:20:11 +0800 Subject: [PATCH] address PR comments --- app/drivers/sensor/max17048/Kconfig | 20 ++- app/drivers/sensor/max17048/max17048.c | 119 +++++++++++------- app/drivers/sensor/max17048/max17048.h | 5 +- .../dts/bindings/sensor/maxim,max17048.yml | 16 +++ 4 files changed, 107 insertions(+), 53 deletions(-) create mode 100644 app/module/dts/bindings/sensor/maxim,max17048.yml diff --git a/app/drivers/sensor/max17048/Kconfig b/app/drivers/sensor/max17048/Kconfig index 6d3de88b..4b7dd9c9 100644 --- a/app/drivers/sensor/max17048/Kconfig +++ b/app/drivers/sensor/max17048/Kconfig @@ -1,9 +1,23 @@ # Copyright (c) 2022 The ZMK Contributors # SPDX-License-Identifier: MIT -config MAX17048 - bool "MAX17048 Fuel Gauge" +DT_COMPAT_MAXIM_MAX17048 := maxim,max17048 + +menuconfig MAX17048 + bool "MAX17048/9 I2C-based Fuel Gauge" + default $(dt_compat_enabled,$(DT_COMPAT_MAXIM_MAX17048)) depends on I2C + select ZMK_BATTERY help - Enable I2C-based driver for MAX17048/9 Fuel Gauge. Supports measuring + Enable driver for MAX17048/9 I2C-based Fuel Gauge. Supports measuring battery voltage and state-of-charge. + +if MAX17048 + +config SENSOR_MAX17048_INIT_PRIORITY + int "Init priority" + default 75 + help + Device driver initialization priority. + +endif #MAX17048 diff --git a/app/drivers/sensor/max17048/max17048.c b/app/drivers/sensor/max17048/max17048.c index 6540873c..d9f90912 100644 --- a/app/drivers/sensor/max17048/max17048.c +++ b/app/drivers/sensor/max17048/max17048.c @@ -16,7 +16,7 @@ #include "max17048.h" -LOG_MODULE_REGISTER(MAX17048, CONFIG_SENSOR_LOG_LEVEL); +LOG_MODULE_REGISTER(sensor_max17048, CONFIG_SENSOR_LOG_LEVEL); static int read_register(const struct device *dev, uint8_t reg, uint16_t *value) { @@ -24,17 +24,17 @@ static int read_register(const struct device *dev, uint8_t reg, uint16_t *value) return -EWOULDBLOCK; } - struct max17048_drv_data *const drv_data = dev->data; - uint16_t dev_addr = ((struct max17048_config *)dev->config)->device_addr; + struct max17048_config *config = (struct max17048_config *)dev->config; - uint16_t data = 0; - int ret = i2c_burst_read(drv_data->i2c, dev_addr, reg, (uint8_t *)&data, sizeof(data)); + uint8_t data[2] = { 0 }; + int ret = i2c_burst_read_dt(&config->i2c_bus, reg, &data[0], sizeof(data)); if (ret != 0) { LOG_DBG("i2c_write_read FAIL %d\n", ret); return ret; } - *value = sys_le16_to_cpu(data); + // the register values are returned in big endian (MSB first) + *value = sys_get_be16(data); return 0; } @@ -44,37 +44,47 @@ static int write_register(const struct device *dev, uint8_t reg, uint16_t value) return -EWOULDBLOCK; } - struct max17048_drv_data *const drv_data = dev->data; + struct max17048_config *config = (struct max17048_config *)dev->config; - uint16_t data = sys_cpu_to_le16(value); - uint16_t dev_addr = ((struct max17048_config *)dev->config)->device_addr; + uint8_t data[2] = { 0 }; + sys_put_be16(value, &data[0]); - return i2c_burst_write(drv_data->i2c, dev_addr, reg, (uint8_t *)&data, sizeof(data)); + return i2c_burst_write_dt(&config->i2c_bus, reg, &data[0], sizeof(data)); } static int set_rcomp_value(const struct device *dev, uint8_t rcomp_value) { + struct max17048_drv_data *const drv_data = (struct max17048_drv_data *const)dev->data; + k_sem_take(&drv_data->lock, K_FOREVER); + uint16_t tmp = 0; int err = read_register(dev, REG_CONFIG, &tmp); if (err != 0) { - return err; + goto done; } tmp = ((uint16_t)rcomp_value << 8) | (tmp & 0xFF); err = write_register(dev, REG_CONFIG, tmp); if (err != 0) { - return err; + goto done; } LOG_DBG("set RCOMP to %d", rcomp_value); - return 0; + +done: + k_sem_give(&drv_data->lock); + return err; } static int set_sleep_enabled(const struct device *dev, bool sleep) { + + struct max17048_drv_data *const drv_data = (struct max17048_drv_data *const)dev->data; + k_sem_take(&drv_data->lock, K_FOREVER); + uint16_t tmp = 0; int err = read_register(dev, REG_CONFIG, &tmp); if (err != 0) { - return err; + goto done; } if (sleep) { @@ -85,40 +95,57 @@ static int set_sleep_enabled(const struct device *dev, bool sleep) { err = write_register(dev, REG_CONFIG, tmp); if (err != 0) { - return err; + goto done; } LOG_DBG("sleep mode %s", sleep ? "enabled" : "disabled"); - return 0; + +done: + k_sem_give(&drv_data->lock); + return err; } static int max17048_sample_fetch(const struct device *dev, enum sensor_channel chan) { - struct max17048_drv_data *const data = dev->data; if (chan != SENSOR_CHAN_GAUGE_VOLTAGE && chan != SENSOR_CHAN_GAUGE_STATE_OF_CHARGE) { LOG_DBG("unsupported channel %d", chan); return -ENOTSUP; } - int err = read_register(dev, REG_STATE_OF_CHARGE, &data->raw_state_of_charge); - if (err != 0) { - LOG_WRN("failed to read state-of-charge: %d", err); - return err; + struct max17048_drv_data *const drv_data = dev->data; + k_sem_take(&drv_data->lock, K_FOREVER); + + int err = 0; + + if (chan == SENSOR_CHAN_GAUGE_VOLTAGE) { + err = read_register(dev, REG_STATE_OF_CHARGE, &drv_data->raw_state_of_charge); + if (err != 0) { + LOG_WRN("failed to read state-of-charge: %d", err); + goto done; + } + LOG_DBG("read soc: %d", drv_data->raw_state_of_charge); + + } else if (chan == SENSOR_CHAN_GAUGE_STATE_OF_CHARGE) { + + err = read_register(dev, REG_VCELL, &drv_data->raw_vcell); + if (err != 0) { + LOG_WRN("failed to read vcell: %d", err); + goto done; + } + LOG_DBG("read vcell: %d", drv_data->raw_vcell); } - err = read_register(dev, REG_VCELL, &data->raw_vcell); - if (err != 0) { - LOG_WRN("failed to read vcell: %d", err); - return err; - } - - LOG_DBG("read values: soc=%d, vcell=%d", data->raw_state_of_charge, data->raw_vcell); - - return 0; +done: + k_sem_give(&drv_data->lock); + return err; } static int max17048_channel_get(const struct device *dev, enum sensor_channel chan, struct sensor_value *val) { + int err = 0; + + struct max17048_drv_data *const drv_data = dev->data; + k_sem_take(&drv_data->lock, K_FOREVER); struct max17048_drv_data *const data = dev->data; unsigned int tmp = 0; @@ -137,23 +164,19 @@ static int max17048_channel_get(const struct device *dev, enum sensor_channel ch break; default: - return -ENOTSUP; + err = -ENOTSUP; + break; } - return 0; + k_sem_give(&drv_data->lock); + return err; } static int max17048_init(const struct device *dev) { - struct max17048_drv_data *driver_data = dev->data; - const struct max17048_config *driver_config = dev->config; + struct max17048_drv_data *drv_data = dev->data; + const struct max17048_config *config = dev->config; - driver_data->i2c = device_get_binding((char *)driver_config->i2c_device_name); - if (!driver_data->i2c) { - LOG_DBG("Unable to get i2c device"); - return -ENODEV; - } - - if (!device_is_ready(driver_data->i2c)) { + if (!device_is_ready(config->i2c_bus.bus)) { LOG_WRN("i2c bus not ready!"); return -EINVAL; } @@ -171,8 +194,8 @@ static int max17048_init(const struct device *dev) { // set the default rcomp value -- 0x97, as stated in the datasheet set_rcomp_value(dev, 0x97); - LOG_INF("device initialised at 0x%x (i2c=%s) (version %d)", driver_config->device_addr, - driver_config->i2c_device_name, ic_version); + k_sem_init(&drv_data->lock, 1, 1); + LOG_INF("device initialised at 0x%x (version %d)", config->i2c_bus.addr, ic_version); return 0; } @@ -182,11 +205,13 @@ static const struct sensor_driver_api max17048_api_table = {.sample_fetch = max1 #define MAX17048_INIT(inst) \ static struct max17048_config max17048_##inst##_config = { \ - .i2c_device_name = DT_INST_BUS_LABEL(inst), \ - .device_addr = DT_INST_REG_ADDR(inst), \ - }; \ + .i2c_bus = I2C_DT_SPEC_INST_GET(inst)}; \ \ - static struct max17048_drv_data max17048_##inst##_drvdata = {}; \ + static struct max17048_drv_data max17048_##inst##_drvdata = { \ + .raw_state_of_charge = 0, \ + .raw_charge_rate = 0, \ + .raw_vcell = 0, \ + }; \ \ /* This has to init after SPI master */ \ DEVICE_DT_INST_DEFINE(inst, max17048_init, NULL, &max17048_##inst##_drvdata, \ diff --git a/app/drivers/sensor/max17048/max17048.h b/app/drivers/sensor/max17048/max17048.h index db728994..8bf99e31 100644 --- a/app/drivers/sensor/max17048/max17048.h +++ b/app/drivers/sensor/max17048/max17048.h @@ -25,12 +25,11 @@ extern "C" { #define REG_STATUS 0x1A struct max17048_config { - const char *i2c_device_name; - uint16_t device_addr; + struct i2c_dt_spec i2c_bus; }; struct max17048_drv_data { - const struct device *i2c; + struct k_sem lock; uint16_t raw_state_of_charge; uint16_t raw_charge_rate; diff --git a/app/module/dts/bindings/sensor/maxim,max17048.yml b/app/module/dts/bindings/sensor/maxim,max17048.yml new file mode 100644 index 00000000..5032c85f --- /dev/null +++ b/app/module/dts/bindings/sensor/maxim,max17048.yml @@ -0,0 +1,16 @@ +# +# Copyright (c) 2022 The ZMK Contributors +# +# SPDX-License-Identifier: MIT +# + +description: > + This is a representation of the Maxim max17048 I2C Fuel Gauge. + +compatible: "maxim,max17048" + +include: [i2c-device.yaml] + +properties: + label: + required: true