2017-04-29 14:34:46

by Alex Mihaylov

[permalink] [raw]
Subject: [PATCH 0/2] Add support for MAX1721x fuel gauge chips

Maxim Semiconductor MAX17211/MAX17215 is M5 Fuel Gauge Monitor with
OneWire (W1) interface.

This patch provide W1 family, MAX17211x slave device and MAX17211x
battery power_supply class device.

Alex A. Mihaylov (2):
Add support for OneWire (W1) devices family 0x26 (MAX17211/MAX17215)
Add driver for MAX17211/MAX17215 fuel gauge

drivers/power/supply/Kconfig | 8 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/max1721x_battery.c | 303 ++++++++++++++++++++++++++++++++
drivers/w1/slaves/Kconfig | 12 ++
drivers/w1/slaves/Makefile | 1 +
drivers/w1/slaves/w1_max1721x.c | 121 +++++++++++++
drivers/w1/slaves/w1_max1721x.h | 101 +++++++++++
drivers/w1/w1_family.h | 1 +
8 files changed, 548 insertions(+)
create mode 100644 drivers/power/supply/max1721x_battery.c
create mode 100644 drivers/w1/slaves/w1_max1721x.c
create mode 100644 drivers/w1/slaves/w1_max1721x.h

--
2.8.4 (Apple Git-73)


2017-04-29 14:35:01

by Alex Mihaylov

[permalink] [raw]
Subject: [PATCH 2/2] Add driver for MAX17211/MAX17215 fuel gauge

Maxim Semiconductor MAX17211/MAX17215 single/multi-cell fuel gauge
monitor with M5 Fuel Gauge algorithm

This driver provide userspace access to MAX17211/MAX17215 data with
power_supply class drivers.
---
drivers/power/supply/Kconfig | 8 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/max1721x_battery.c | 303 ++++++++++++++++++++++++++++++++
3 files changed, 312 insertions(+)
create mode 100644 drivers/power/supply/max1721x_battery.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 76806a0..25fe441 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -117,6 +117,14 @@ config BATTERY_DS2782
Say Y here to enable support for the DS2782/DS2786 standalone battery
gas-gauge.

+config BATTERY_MAX1721X
+ tristate "MAX17211/MAX17215 standalone gas-gauge"
+ depends on W1
+ select W1_SLAVE_MAX1721X
+ help
+ Say Y here to enable support for the MAX17211/MAX17215 standalone
+ battery gas-gauge.
+
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 36c599d..1e5546d 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
obj-$(CONFIG_BATTERY_DS2782) += ds2782_battery.o
+obj-$(CONFIG_BATTERY_MAX1721X) += max1721x_battery.o
obj-$(CONFIG_BATTERY_GAUGE_LTC2941) += ltc2941-battery-gauge.o
obj-$(CONFIG_BATTERY_GOLDFISH) += goldfish_battery.o
obj-$(CONFIG_BATTERY_PMU) += pmu_battery.o
diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
new file mode 100644
index 0000000..ff77d3f
--- /dev/null
+++ b/drivers/power/supply/max1721x_battery.c
@@ -0,0 +1,303 @@
+/*
+ * 1-wire client/driver for the Maxim MAX17211/MAX17215 Fuel Gauge IC
+ *
+ * Author: Alex A. Mihaylov <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/param.h>
+#include <linux/pm.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/idr.h>
+
+#include "../../w1/w1.h"
+#include "../../w1/slaves/w1_max1721x.h"
+
+#define DEF_DEV_NAME_MAX17211 "MAX17211"
+#define DEF_DEV_NAME_MAX17215 "MAX17215"
+#define DEF_DEV_NAME_UNKNOWN "UNKNOWN"
+#define DEF_MFG_NAME "MAXIM"
+
+struct max17211_device_info {
+ struct device *dev;
+ struct power_supply *bat;
+ struct power_supply_desc bat_desc;
+ struct device *w1_dev;
+ /* battery design format */
+ uint16_t rsense; /* in tenths uOhm */
+ char DeviceName[2 * MAX1721X_REG_DEV_NUMB + 1];
+ char ManufacturerName[2 * MAX1721X_REG_MFG_NUMB + 1];
+ char SerialNumber[13]; /* see get_sn_str() later for comment */
+};
+
+static inline struct max17211_device_info *
+to_device_info(struct power_supply *psy)
+{
+ return power_supply_get_drvdata(psy);
+}
+
+static int max1721x_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct max17211_device_info *info = to_device_info(psy);
+ uint16_t reg;
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ /*
+ * POWER_SUPPLY_PROP_PRESENT will always readable via
+ * sysfs interface. Value return 0 if battery not
+ * present or unaccesable via W1.
+ */
+ val->intval =
+ w1_max1721x_reg_get(info->w1_dev, MAX172XX_REG_STATUS,
+ &reg) ? 0 : !(reg & MAX172XX_BAT_PRESENT);
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_REPSOC, &reg);
+ val->intval = max172xx_percent_to_ps(reg);
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_BATT, &reg);
+ val->intval = max172xx_voltage_to_ps(reg);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_DESIGNCAP, &reg);
+ val->intval = max172xx_capacity_to_ps(reg);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_AVG:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_REPCAP, &reg);
+ val->intval = max172xx_capacity_to_ps(reg);
+ break;
+ case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_TTE, &reg);
+ val->intval = max172xx_time_to_ps(reg);
+ break;
+ case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_TTF, &reg);
+ val->intval = max172xx_time_to_ps(reg);
+ break;
+ case POWER_SUPPLY_PROP_TEMP:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_TEMP, &reg);
+ val->intval = max172xx_temperature_to_ps(reg);
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_CURRENT, &reg);
+ val->intval = max172xx_current_to_voltage(reg) / info->rsense;
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_AVG:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_AVGCURRENT, &reg);
+ val->intval = max172xx_current_to_voltage(reg) / info->rsense;
+ break;
+ /*
+ * Strings already received and inited by probe.
+ * We do dummy read for check battery still available.
+ */
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX1721X_REG_DEV_STR, &reg);
+ val->strval = info->DeviceName;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX1721X_REG_MFG_STR, &reg);
+ val->strval = info->ManufacturerName;
+ break;
+ case POWER_SUPPLY_PROP_SERIAL_NUMBER:
+ ret = w1_max1721x_reg_get(info->w1_dev,
+ MAX1721X_REG_SER_HEX, &reg);
+ val->strval = info->SerialNumber;
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+static enum power_supply_property max1721x_battery_props[] = {
+ /* int */
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+ POWER_SUPPLY_PROP_CHARGE_AVG,
+ POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
+ POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+ POWER_SUPPLY_PROP_TEMP,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
+ POWER_SUPPLY_PROP_CURRENT_AVG,
+ /* strings */
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+ POWER_SUPPLY_PROP_SERIAL_NUMBER,
+};
+
+static int get_string(struct device *dev, uint16_t reg, uint8_t nr, char *str)
+{
+ uint16_t val;
+
+ if (!str || !(reg == MAX1721X_REG_MFG_STR ||
+ reg == MAX1721X_REG_DEV_STR))
+ return -EFAULT;
+
+ while (nr--) {
+ if (w1_max1721x_reg_get(dev, reg++, &val))
+ return -EFAULT;
+ *str++ = val>>8 & 0x00FF;
+ *str++ = val & 0x00FF;
+ }
+ return 0;
+}
+
+/* Maxim say: Serial number is a hex string up to 12 hex characters */
+static int get_sn_string(struct device *dev, char *str)
+{
+ uint16_t val[3];
+
+ if (!str)
+ return -EFAULT;
+
+ if (w1_max1721x_reg_get(dev, MAX1721X_REG_SER_HEX, &val[0]))
+ return -EFAULT;
+ if (w1_max1721x_reg_get(dev, MAX1721X_REG_SER_HEX + 1, &val[1]))
+ return -EFAULT;
+ if (w1_max1721x_reg_get(dev, MAX1721X_REG_SER_HEX + 2, &val[2]))
+ return -EFAULT;
+
+ snprintf(str, 13, "%04X%04X%04X", val[0], val[1], val[2]);
+ return 0;
+}
+
+static int max1721x_battery_probe(struct platform_device *pdev)
+{
+ struct power_supply_config psy_cfg = {};
+ struct max17211_device_info *info;
+
+ info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, info);
+
+ info->dev = &pdev->dev;
+ info->w1_dev = pdev->dev.parent;
+ info->bat_desc.name = dev_name(&pdev->dev);
+ info->bat_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ info->bat_desc.properties = max1721x_battery_props;
+ info->bat_desc.num_properties = ARRAY_SIZE(max1721x_battery_props);
+ info->bat_desc.get_property = max1721x_battery_get_property;
+ /*
+ * FixMe:
+ * Device without no_thermal = true not register (err -22)
+ * Len of platform device name "max17211-battery.X.auto"
+ * more than 20 chars limit in THERMAL_NAME_LENGTH from
+ * include/uapi/linux/thermal.h
+ */
+ info->bat_desc.no_thermal = true;
+ psy_cfg.drv_data = info;
+
+ if (w1_max1721x_reg_get(info->w1_dev,
+ MAX1721X_REG_NRSENSE, &info->rsense))
+ return -ENODEV;
+
+ if (!info->rsense) {
+ dev_warn(info->dev, "RSenese not calibrated, set 10 mOhms!\n");
+ info->rsense = 1000; /* in regs in 10^-5 */
+ }
+ dev_dbg(info->dev, "RSense: %d mOhms.\n", info->rsense / 100);
+
+ if (get_string(info->w1_dev, MAX1721X_REG_MFG_STR,
+ MAX1721X_REG_MFG_NUMB, info->ManufacturerName)) {
+ dev_err(info->dev, "Can't read manufacturer. Hardware error.\n");
+ return -ENODEV;
+ }
+
+ if (!info->ManufacturerName[0])
+ strncpy(info->ManufacturerName, DEF_MFG_NAME,
+ 2 * MAX1721X_REG_MFG_NUMB);
+
+ if (get_string(info->w1_dev, MAX1721X_REG_DEV_STR,
+ MAX1721X_REG_DEV_NUMB, info->DeviceName)) {
+ dev_err(info->dev, "Can't read device. Hardware error.\n");
+ return -ENODEV;
+ }
+ if (!info->DeviceName[0]) {
+ uint16_t dev_name;
+
+ if (w1_max1721x_reg_get(info->w1_dev,
+ MAX172XX_REG_DEVNAME, &dev_name)) {
+ dev_err(info->w1_dev, "Can't read device name reg.\n");
+ return -ENODEV;
+ }
+
+ switch (dev_name & MAX172XX_DEV_MASK) {
+ case MAX172X1_DEV:
+ strncpy(info->DeviceName, DEF_DEV_NAME_MAX17211,
+ 2 * MAX1721X_REG_DEV_NUMB);
+ break;
+ case MAX172X5_DEV:
+ strncpy(info->DeviceName, DEF_DEV_NAME_MAX17215,
+ 2 * MAX1721X_REG_DEV_NUMB);
+ break;
+ default:
+ strncpy(info->DeviceName, DEF_DEV_NAME_UNKNOWN,
+ 2 * MAX1721X_REG_DEV_NUMB);
+ }
+ }
+
+ if (get_sn_string(info->w1_dev, info->SerialNumber)) {
+ dev_err(info->dev, "Can't read serial. Hardware error.\n");
+ return -ENODEV;
+ }
+
+ info->bat = power_supply_register(&pdev->dev, &info->bat_desc,
+ &psy_cfg);
+ if (IS_ERR(info->bat)) {
+ dev_err(info->dev, "failed to register battery\n");
+ return PTR_ERR(info->bat);
+ }
+
+ return 0;
+}
+
+static int max1721x_battery_remove(struct platform_device *pdev)
+{
+ struct max17211_device_info *info = platform_get_drvdata(pdev);
+
+ power_supply_unregister(info->bat);
+
+ return 0;
+}
+
+static struct platform_driver max1721x_battery_driver = {
+ .driver = {
+ .name = "max1721x-battery",
+ },
+ .probe = max1721x_battery_probe,
+ .remove = max1721x_battery_remove,
+};
+module_platform_driver(max1721x_battery_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alex A. Mihaylov <[email protected]>");
+MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
+MODULE_ALIAS("platform:max1721x-battery");
--
2.8.4 (Apple Git-73)

2017-04-29 14:34:53

by Alex Mihaylov

[permalink] [raw]
Subject: [PATCH 1/2] Add support for OneWire (W1) devices family 0x26 (MAX17211/MAX17215)

Maxim Semiconductor MAX17211/MAX17215 single/multi-cell fuel gauge
monitor with M5 Fuel Gauge algorithm

Slave device provide software layer for access to internal registers
MAX17211/MAX17215 chip.
---
drivers/w1/slaves/Kconfig | 12 ++++
drivers/w1/slaves/Makefile | 1 +
drivers/w1/slaves/w1_max1721x.c | 121 ++++++++++++++++++++++++++++++++++++++++
drivers/w1/slaves/w1_max1721x.h | 101 +++++++++++++++++++++++++++++++++
drivers/w1/w1_family.h | 1 +
5 files changed, 236 insertions(+)
create mode 100644 drivers/w1/slaves/w1_max1721x.c
create mode 100644 drivers/w1/slaves/w1_max1721x.h

diff --git a/drivers/w1/slaves/Kconfig b/drivers/w1/slaves/Kconfig
index cfe74d0..05467da 100644
--- a/drivers/w1/slaves/Kconfig
+++ b/drivers/w1/slaves/Kconfig
@@ -78,6 +78,18 @@ config W1_SLAVE_DS2433_CRC
Each block has 30 bytes of data and a two byte CRC16.
Full block writes are only allowed if the CRC is valid.

+config W1_SLAVE_MAX1721X
+ tristate "Maxim MAX17211/MAX17215 battery monitor chip"
+ help
+ If you enable this you will have the MAX17211/MAX17215 battery
+ monitor chip support.
+
+ The battery monitor chip is used in many batteries/devices
+ as the one who is responsible for charging/discharging/monitoring
+ Li+ batteries.
+
+ If you are unsure, say N.
+
config W1_SLAVE_DS2760
tristate "Dallas 2760 battery monitor chip (HP iPAQ & others)"
help
diff --git a/drivers/w1/slaves/Makefile b/drivers/w1/slaves/Makefile
index 1e9989a..1e9f942 100644
--- a/drivers/w1/slaves/Makefile
+++ b/drivers/w1/slaves/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_W1_SLAVE_DS2406) += w1_ds2406.o
obj-$(CONFIG_W1_SLAVE_DS2423) += w1_ds2423.o
obj-$(CONFIG_W1_SLAVE_DS2431) += w1_ds2431.o
obj-$(CONFIG_W1_SLAVE_DS2433) += w1_ds2433.o
+obj-$(CONFIG_W1_SLAVE_MAX1721X) += w1_max1721x.o
obj-$(CONFIG_W1_SLAVE_DS2760) += w1_ds2760.o
obj-$(CONFIG_W1_SLAVE_DS2780) += w1_ds2780.o
obj-$(CONFIG_W1_SLAVE_DS2781) += w1_ds2781.o
diff --git a/drivers/w1/slaves/w1_max1721x.c b/drivers/w1/slaves/w1_max1721x.c
new file mode 100644
index 0000000..bb73e90
--- /dev/null
+++ b/drivers/w1/slaves/w1_max1721x.c
@@ -0,0 +1,121 @@
+/*
+ * 1-Wire implementation for the max17211 chip
+ *
+ * Copyright © 2017, Alex A. Mihaylov <[email protected]>
+ *
+ * Use consistent with the GNU GPL is permitted,
+ * provided that this copyright notice is
+ * preserved in its entirety in all copies and derived works.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/types.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <linux/idr.h>
+#include <linux/gfp.h>
+
+#include "../w1.h"
+#include "../w1_int.h"
+#include "../w1_family.h"
+#include "w1_max1721x.h"
+
+int w1_max1721x_reg_get(struct device *dev, uint16_t addr, uint16_t *val)
+{
+ struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+ int ret = -ENODEV;
+
+ if (addr > MAX1721X_MAX_REG_NR || addr < 0 || !val || !dev)
+ return -EFAULT;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (!w1_reset_select_slave(sl)) {
+ w1_write_8(sl->master, W1_MAX1721X_READ_DATA);
+ w1_write_8(sl->master, addr & 0x00FF);
+ w1_write_8(sl->master, addr>>8 & 0x00FF);
+ *val = w1_read_8(sl->master);
+ *val |= w1_read_8(sl->master)<<8;
+ ret = 0;
+ }
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(w1_max1721x_reg_get);
+
+int w1_max1721x_reg_set(struct device *dev, uint16_t addr, uint16_t val)
+{
+ struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+ int ret = -ENODEV;
+
+ if (addr > MAX1721X_MAX_REG_NR || addr < 0 || !dev)
+ return -EFAULT;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (!w1_reset_select_slave(sl)) {
+ w1_write_8(sl->master, W1_MAX1721X_READ_DATA);
+ w1_write_8(sl->master, addr & 0x00FF);
+ w1_write_8(sl->master, addr>>8 & 0x00FF);
+ w1_write_8(sl->master, val & 0x00FF);
+ w1_write_8(sl->master, val>>8 & 0x00FF);
+ ret = 0;
+ }
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(w1_max1721x_reg_set);
+
+static int w1_max17211_add_device(struct w1_slave *sl)
+{
+ int ret;
+ struct platform_device *pdev;
+
+ pdev = platform_device_alloc("max1721x-battery", PLATFORM_DEVID_AUTO);
+ if (!pdev)
+ return -ENOMEM;
+ pdev->dev.parent = &sl->dev;
+
+ ret = platform_device_add(pdev);
+ if (ret)
+ goto pdev_add_failed;
+
+ dev_set_drvdata(&sl->dev, pdev);
+
+ return 0;
+
+pdev_add_failed:
+ platform_device_put(pdev);
+
+ return ret;
+}
+
+static void w1_max17211_remove_device(struct w1_slave *sl)
+{
+ struct platform_device *pdev = dev_get_drvdata(&sl->dev);
+
+ platform_device_unregister(pdev);
+}
+
+static struct w1_family_ops w1_max17211_fops = {
+ .add_slave = w1_max17211_add_device,
+ .remove_slave = w1_max17211_remove_device,
+};
+
+static struct w1_family w1_max17211_family = {
+ .fid = W1_FAMILY_MAX17211,
+ .fops = &w1_max17211_fops,
+};
+module_w1_family(w1_max17211_family);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alex A. Mihaylov <[email protected]>");
+MODULE_DESCRIPTION("1-wire Driver for MAX17211/MAX17215 battery monitor");
+MODULE_ALIAS("w1-family-" __stringify(W1_FAMILY_MAX17211));
diff --git a/drivers/w1/slaves/w1_max1721x.h b/drivers/w1/slaves/w1_max1721x.h
new file mode 100644
index 0000000..1deb3d3
--- /dev/null
+++ b/drivers/w1/slaves/w1_max1721x.h
@@ -0,0 +1,101 @@
+/*
+ * 1-Wire implementation for the max17211 chip
+ *
+ * Copyright © 2017, Alex A. Mihaylov <[email protected]>
+ *
+ * Use consistent with the GNU GPL is permitted,
+ * provided that this copyright notice is
+ * preserved in its entirety in all copies and derived works.
+ *
+ */
+
+#ifndef __w1_max17211_h__
+#define __w1_max17211_h__
+
+/* Known commands to the MAX1721X chip */
+#define W1_MAX1721X_READ_DATA 0x69
+#define W1_MAX1721X_WRITE_DATA 0x6C
+
+/* Factory settings (nonvilatile registers) (W1 specific) */
+
+#define MAX1721X_REG_NRSENSE 0x1CF /* RSense in 10^-5 Ohm */
+/* Strings */
+#define MAX1721X_REG_MFG_STR 0x1CC
+#define MAX1721X_REG_MFG_NUMB 3
+#define MAX1721X_REG_DEV_STR 0x1DB
+#define MAX1721X_REG_DEV_NUMB 5
+/* HEX Strings */
+#define MAX1721X_REG_SER_HEX 0x1D8
+
+/* Number of valid register addresses */
+#define MAX1721X_MAX_REG_NR (0x1EF)
+
+/* MAX1721X/MAX17215 Output Registers for I2C and W1 chips */
+
+#define MAX172XX_REG_STATUS 0x000 /* status reg */
+#define MAX172XX_BAT_PRESENT (1<<4) /* battery connected bit */
+#define MAX172XX_REG_DEVNAME 0x021 /* chip config */
+#define MAX172XX_DEV_MASK 0x000F /* chip type mask */
+#define MAX172X1_DEV 0x0001
+#define MAX172X5_DEV 0x0005
+#define MAX172XX_REG_TEMP 0x008 /* Temperature */
+#define MAX172XX_REG_BATT 0x0DA /* Battery voltage */
+#define MAX172XX_REG_CURRENT 0x00A /* Actual current */
+#define MAX172XX_REG_AVGCURRENT 0x00B /* Average current */
+#define MAX172XX_REG_REPSOC 0x006 /* Percentage of charge */
+#define MAX172XX_REG_DESIGNCAP 0x018 /* Design capacity */
+#define MAX172XX_REG_REPCAP 0x005 /* Average capacity */
+#define MAX172XX_REG_TTE 0x011 /* Time to empty */
+#define MAX172XX_REG_TTF 0x020 /* Time to full */
+
+/* Convert regs value to power_supply units */
+
+static inline int max172xx_time_to_ps(uint16_t reg)
+{
+ return reg * 5625 / 1000; /* in sec. */
+}
+
+static inline int max172xx_percent_to_ps(uint16_t reg)
+{
+ return reg / 256; /* in percent from 0 to 100 */
+}
+
+static inline int max172xx_voltage_to_ps(uint16_t reg)
+{
+ return reg * 1250; /* in uV */
+}
+
+static inline int max172xx_capacity_to_ps(uint16_t reg)
+{
+ return reg * 500; /* in uAh */
+}
+
+/*
+ * Current and temperature is signed values, so unsigned regs
+ * value must be converted to signed type
+ */
+
+static inline int max172xx_temperature_to_ps(uint16_t reg)
+{
+ return (int16_t)reg * 10 / 256; /* in tenths of deg. C */
+}
+
+/*
+ * Calculating current registers resolution:
+ *
+ * RSense stored in 10^-5 Ohm, so mesaurment voltage must be
+ * in 10^-11 Volts for get current in uA.
+ * 16 bit current reg fullscale +/-51.2mV is 102400 uV.
+ * So: 102400 / 65535 * 10^5 = 156252
+ */
+static inline int max172xx_current_to_voltage(uint16_t reg)
+{
+ return (int16_t)reg * 156252;
+}
+
+extern int w1_max1721x_reg_get(struct device *dev, uint16_t addr,
+ uint16_t *val);
+extern int w1_max1721x_reg_set(struct device *dev, uint16_t addr,
+ uint16_t val);
+
+#endif /* !__w1_max17211_h__ */
diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h
index 10a7a07..797c162 100644
--- a/drivers/w1/w1_family.h
+++ b/drivers/w1/w1_family.h
@@ -35,6 +35,7 @@
#define W1_COUNTER_DS2423 0x1D
#define W1_THERM_DS1822 0x22
#define W1_EEPROM_DS2433 0x23
+#define W1_FAMILY_MAX17211 0x26
#define W1_THERM_DS18B20 0x28
#define W1_FAMILY_DS2408 0x29
#define W1_EEPROM_DS2431 0x2D
--
2.8.4 (Apple Git-73)

2017-04-29 15:59:27

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add support for OneWire (W1) devices family 0x26 (MAX17211/MAX17215)

Hi,

On Sat, Apr 29, 2017 at 05:34:28PM +0300, Alex A. Mihaylov wrote:
> Maxim Semiconductor MAX17211/MAX17215 single/multi-cell fuel gauge
> monitor with M5 Fuel Gauge algorithm
>
> Slave device provide software layer for access to internal registers
> MAX17211/MAX17215 chip.

Please convert this to regmap. There should be lots of docs and
examples around, for example:

http://elinux.org/images/a/a3/Regmap-_The_Power_of_Subsystems_and_Abstractions.pdf

There is no generic w1 handler, but you can provide custom
read/write functions. Also it would be nice to have this
based on https://lkml.org/lkml/2017/3/16/604. Then everything
could go into the power-supply driver.

-- Sebastian


Attachments:
(No filename) (690.00 B)
signature.asc (833.00 B)
Download all attachments

2017-04-29 16:41:10

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 2/2] Add driver for MAX17211/MAX17215 fuel gauge

Hi,

Please make sure you Cc the relevant people / mailing lists. You
can use ./scripts/get_maintainer.pl to find out who should receive
the patches.

On Sat, Apr 29, 2017 at 05:34:29PM +0300, Alex A. Mihaylov wrote:
> Maxim Semiconductor MAX17211/MAX17215 single/multi-cell fuel gauge
> monitor with M5 Fuel Gauge algorithm
>
> This driver provide userspace access to MAX17211/MAX17215 data with
> power_supply class drivers.

> ---
> drivers/power/supply/Kconfig | 8 +
> drivers/power/supply/Makefile | 1 +

Please move the entries in both files before CONFIG_BATTERY_MAX17040.

> [...]
>
> + * FixMe:
> + * Device without no_thermal = true not register (err -22)
> + * Len of platform device name "max17211-battery.X.auto"
> + * more than 20 chars limit in THERMAL_NAME_LENGTH from
> + * include/uapi/linux/thermal.h
> + */

IIRC we already had problems with small THERMAL_NAME_LENGTH before.
I suggest to add another patch, that increases THERMAL_NAME_LENGTH
(don't forget to Cc/To the thermal subsystem people).

> + info->bat_desc.no_thermal = true;
> + psy_cfg.drv_data = info;
> +
> + if (w1_max1721x_reg_get(info->w1_dev,
> + MAX1721X_REG_NRSENSE, &info->rsense))
> + return -ENODEV;
> +
> + if (!info->rsense) {
> + dev_warn(info->dev, "RSenese not calibrated, set 10 mOhms!\n");
> + info->rsense = 1000; /* in regs in 10^-5 */
> + }
> + dev_dbg(info->dev, "RSense: %d mOhms.\n", info->rsense / 100);
> +
> + if (get_string(info->w1_dev, MAX1721X_REG_MFG_STR,
> + MAX1721X_REG_MFG_NUMB, info->ManufacturerName)) {
> + dev_err(info->dev, "Can't read manufacturer. Hardware error.\n");
> + return -ENODEV;
> + }
> +
> + if (!info->ManufacturerName[0])
> + strncpy(info->ManufacturerName, DEF_MFG_NAME,
> + 2 * MAX1721X_REG_MFG_NUMB);
> +
> + if (get_string(info->w1_dev, MAX1721X_REG_DEV_STR,
> + MAX1721X_REG_DEV_NUMB, info->DeviceName)) {
> + dev_err(info->dev, "Can't read device. Hardware error.\n");
> + return -ENODEV;
> + }
> + if (!info->DeviceName[0]) {
> + uint16_t dev_name;
> +
> + if (w1_max1721x_reg_get(info->w1_dev,
> + MAX172XX_REG_DEVNAME, &dev_name)) {
> + dev_err(info->w1_dev, "Can't read device name reg.\n");
> + return -ENODEV;
> + }
> +
> + switch (dev_name & MAX172XX_DEV_MASK) {
> + case MAX172X1_DEV:
> + strncpy(info->DeviceName, DEF_DEV_NAME_MAX17211,
> + 2 * MAX1721X_REG_DEV_NUMB);
> + break;
> + case MAX172X5_DEV:
> + strncpy(info->DeviceName, DEF_DEV_NAME_MAX17215,
> + 2 * MAX1721X_REG_DEV_NUMB);
> + break;
> + default:
> + strncpy(info->DeviceName, DEF_DEV_NAME_UNKNOWN,
> + 2 * MAX1721X_REG_DEV_NUMB);
> + }
> + }
> +
> + if (get_sn_string(info->w1_dev, info->SerialNumber)) {
> + dev_err(info->dev, "Can't read serial. Hardware error.\n");
> + return -ENODEV;
> + }
> +
> + info->bat = power_supply_register(&pdev->dev, &info->bat_desc,
> + &psy_cfg);
> + if (IS_ERR(info->bat)) {
> + dev_err(info->dev, "failed to register battery\n");
> + return PTR_ERR(info->bat);
> + }

Please use devm_power_supply_register() and drop the remove
function.

> + return 0;
> +}
> +
> +static int max1721x_battery_remove(struct platform_device *pdev)
> +{
> + struct max17211_device_info *info = platform_get_drvdata(pdev);
> +
> + power_supply_unregister(info->bat);
> +
> + return 0;
> +}
> +
> +static struct platform_driver max1721x_battery_driver = {
> + .driver = {
> + .name = "max1721x-battery",
> + },
> + .probe = max1721x_battery_probe,
> + .remove = max1721x_battery_remove,
> +};
> +module_platform_driver(max1721x_battery_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alex A. Mihaylov <[email protected]>");
> +MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
> +MODULE_ALIAS("platform:max1721x-battery");

-- Sebastian


Attachments:
(No filename) (3.71 kB)
signature.asc (833.00 B)
Download all attachments

2017-04-30 05:22:03

by Alex Mihaylov

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add support for OneWire (W1) devices family 0x26 (MAX17211/MAX17215)

Hi,


>> Slave device provide software layer for access to internal registers
>> MAX17211/MAX17215 chip.
> Please convert this to regmap.There is no generic w1 handler, but you can provide custom
> read/write functions.
I think regmap be overkill for this driver. Here we need access to a
small number of registers. Registers are extremely simple on the
internal device. As a result, the software layer of regmap will only
complicate the readability and understanding of the code. And also
increase the size and time of code execution, and even complicate the
perception.

> Also it would be nice to have this
> based on https://lkml.org/lkml/2017/3/16/604. Then everything
> could go into the power-supply driver.
>
No problems. Once this set of patches will be accepted in the mainline.

Alex.

2017-04-30 17:32:22

by Alex Mihaylov

[permalink] [raw]
Subject: Re: [PATCH 2/2] Add driver for MAX17211/MAX17215 fuel gauge

Hi!


>> [...]
>>
>> + * FixMe:
>> + * Device without no_thermal = true not register (err -22)
>> + * Len of platform device name "max17211-battery.X.auto"
>> + * more than 20 chars limit in THERMAL_NAME_LENGTH from
>> + * include/uapi/linux/thermal.h
>> + */
> IIRC we already had problems with small THERMAL_NAME_LENGTH before.
> I suggest to add another patch, that increases THERMAL_NAME_LENGTH
> (don't forget to Cc/To the thermal subsystem people).
May be rename "max17211-battery" to "max17211" and remove no_thermal =
true? This case thermal zone will work again. In current (mainline)
kernel all drivers "xxxxxx-battery" (like ds2780-battery or
ds2760-battery) compiled, but not working (not registered). They can
start working again without thermal zone by adding no_thermal = true or
by remove "-battery" from platform device name. Alternative limit on
THERMAL_NAME_LENGTH may be extended.
>> + info->bat = power_supply_register(&pdev->dev, &info->bat_desc,
>> + &psy_cfg);
>> + if (IS_ERR(info->bat)) {
>> + dev_err(info->dev, "failed to register battery\n");
>> + return PTR_ERR(info->bat);
>> + }
> Please use devm_power_supply_register() and drop the remove
> function.
Khm... probe/remove paired functions. I can use
devm_power_supply_register in my code. But all other fuel gauge drivers
use classic probe/remove pair. Which decision will be more correct?

2017-04-30 20:44:22

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 2/2] Add driver for MAX17211/MAX17215 fuel gauge

Hi,

On Sun, Apr 30, 2017 at 08:32:10PM +0300, Михайлов Алексей Анатольевич wrote:
> > IIRC we already had problems with small THERMAL_NAME_LENGTH before.
> > I suggest to add another patch, that increases THERMAL_NAME_LENGTH
> > (don't forget to Cc/To the thermal subsystem people).
> May be rename "max17211-battery" to "max17211" and remove no_thermal = true?
> This case thermal zone will work again. In current (mainline) kernel all
> drivers "xxxxxx-battery" (like ds2780-battery or ds2760-battery) compiled,
> but not working (not registered). They can start working again without
> thermal zone by adding no_thermal = true or by remove "-battery" from
> platform device name. Alternative limit on THERMAL_NAME_LENGTH may be
> extended.

I think increasing THERMAL_NAME_LENGTH is the right way.

> > > + info->bat = power_supply_register(&pdev->dev, &info->bat_desc,
> > > + &psy_cfg);
> > > + if (IS_ERR(info->bat)) {
> > > + dev_err(info->dev, "failed to register battery\n");
> > > + return PTR_ERR(info->bat);
> > > + }
> > Please use devm_power_supply_register() and drop the remove
> > function.
> Khm... probe/remove paired functions. I can use devm_power_supply_register
> in my code. But all other fuel gauge drivers use classic probe/remove pair.
> Which decision will be more correct?

Most drivers are older than devm_power_supply_register. We already
have a few fuel gauge drivers, which use it, though:

$ git grep devm_power_supply_register | grep "battery"
88pm860x_battery.c: info->battery = devm_power_supply_register(&pdev->dev,
da9150-fg.c: fg->battery = devm_power_supply_register(dev, &fg_desc, NULL);
max17042_battery.c: chip->battery = devm_power_supply_register(&client->dev, max17042_desc,
max8997_charger.c: charger->battery = devm_power_supply_register(&pdev->dev,
max8998_charger.c: max8998->battery = devm_power_supply_register(max8998->dev,
sbs-battery.c: chip->power_supply = devm_power_supply_register(&client->dev, sbs_desc,

-- Sebastian


Attachments:
(No filename) (1.97 kB)
signature.asc (833.00 B)
Download all attachments

2017-04-30 20:53:52

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add support for OneWire (W1) devices family 0x26 (MAX17211/MAX17215)

Hi,

On Sun, Apr 30, 2017 at 08:21:51AM +0300, Alex A. Mihaylov wrote:
> > > Slave device provide software layer for access to internal registers
> > > MAX17211/MAX17215 chip.
> > Please convert this to regmap.There is no generic w1 handler, but you can provide custom
> > read/write functions.
> I think regmap be overkill for this driver. Here we need access to a small
> number of registers. Registers are extremely simple on the internal device.
> As a result, the software layer of regmap will only complicate the
> readability and understanding of the code. And also increase the size and
> time of code execution, and even complicate the perception.

I did not ask for full usage of all regmap features. Just register
the read/write handler as regmap handler and use regmap_read/write
to get values.

-- Sebastian


Attachments:
(No filename) (822.00 B)
signature.asc (833.00 B)
Download all attachments