All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Battery monitor MAX1721x with w1-regmap and w1-slave (resend)
@ 2017-06-02  7:06 Alex A. Mihaylov
  2017-06-02  7:06 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Alex A. Mihaylov @ 2017-06-02  7:06 UTC (permalink / raw)
  To: Mark Brown, Greg Kroah-Hartman, Sebastian Reichel,
	Evgeniy Polyakov, linux-kernel, linux-pm
  Cc: Alex A. Mihaylov

Codding style fix. Maillist message subject fix.
Soryy for all. It's hard to be stuppid :-(

Alex A. Mihaylov (3):
  regmap: Add 1-Wire bus support
  w1: MAX1721x Stanalone Fuel Gauge - add 1-Wire slave drivers
  power: supply: Add support MAX1721x battery monitor

 drivers/base/regmap/Kconfig             |   6 +-
 drivers/base/regmap/Makefile            |   1 +
 drivers/base/regmap/regmap-w1.c         | 245 ++++++++++++++++++++++
 drivers/power/supply/Kconfig            |  14 ++
 drivers/power/supply/Makefile           |   1 +
 drivers/power/supply/max1721x_battery.c | 357 ++++++++++++++++++++++++++++++++
 drivers/w1/slaves/Kconfig               |  12 ++
 drivers/w1/slaves/Makefile              |   1 +
 drivers/w1/slaves/w1_max1721x.c         |  73 +++++++
 drivers/w1/slaves/w1_max1721x.h         | 102 +++++++++
 drivers/w1/w1_family.h                  |   1 +
 include/linux/regmap.h                  |  34 +++
 12 files changed, 846 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-w1.c
 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.13.0

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] regmap: Add 1-Wire bus support
  2017-06-02  7:06 [PATCH 0/3] Battery monitor MAX1721x with w1-regmap and w1-slave (resend) Alex A. Mihaylov
@ 2017-06-02  7:06 ` Alex A. Mihaylov
  2017-06-06 18:50   ` Mark Brown
  2017-06-08 12:53   ` Sebastian Reichel
  2017-06-02  7:06 ` [PATCH 2/3] w1: MAX1721x Stanalone Fuel Gauge - add 1-Wire slave drivers Alex A. Mihaylov
  2017-06-02  7:06 ` [PATCH 3/3] power: supply: Add support MAX1721x battery monitor Alex A. Mihaylov
  2 siblings, 2 replies; 16+ messages in thread
From: Alex A. Mihaylov @ 2017-06-02  7:06 UTC (permalink / raw)
  To: Mark Brown, Greg Kroah-Hartman, Sebastian Reichel,
	Evgeniy Polyakov, linux-kernel, linux-pm
  Cc: Alex A. Mihaylov

Add basic support regmap (register map access) API for 1-Wire bus
---
 drivers/base/regmap/Kconfig     |   6 +-
 drivers/base/regmap/Makefile    |   1 +
 drivers/base/regmap/regmap-w1.c | 245 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h          |  34 ++++++
 4 files changed, 285 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-w1.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index db9d00c36a..413af5f940 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -3,7 +3,7 @@
 # subsystems should select the appropriate symbols.
 
 config REGMAP
-	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
+	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 	select IRQ_DOMAIN if REGMAP_IRQ
@@ -24,6 +24,10 @@ config REGMAP_SPMI
 	tristate
 	depends on SPMI
 
+config REGMAP_W1
+	tristate
+	depends on W1
+
 config REGMAP_MMIO
 	tristate
 
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 609e4c84f4..17741ae14e 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
 obj-$(CONFIG_REGMAP_SPMI) += regmap-spmi.o
 obj-$(CONFIG_REGMAP_MMIO) += regmap-mmio.o
 obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
+obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
diff --git a/drivers/base/regmap/regmap-w1.c b/drivers/base/regmap/regmap-w1.c
new file mode 100644
index 0000000000..5f04e7bf06
--- /dev/null
+++ b/drivers/base/regmap/regmap-w1.c
@@ -0,0 +1,245 @@
+/*
+ * Register map access API - W1 (1-Wire) support
+ *
+ * Copyright (C) 2017 OAO Radioavionica
+ * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
+ *
+ * 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/regmap.h>
+#include <linux/module.h>
+#include "../../w1/w1.h"
+
+#include "internal.h"
+
+#define W1_CMD_READ_DATA	0x69
+#define W1_CMD_WRITE_DATA	0x6C
+
+/*
+ * 1-Wire slaves registers with addess 8 bit and data 8 bit
+ */
+
+static int w1_reg_a8_v8_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_READ_DATA);
+		w1_write_8(sl->master, reg);
+		*val = w1_read_8(sl->master);
+	} else {
+		ret = -ENODEV;
+	}
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+static int w1_reg_a8_v8_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_WRITE_DATA);
+		w1_write_8(sl->master, reg);
+		w1_write_8(sl->master, val);
+	} else {
+		ret = -ENODEV;
+	}
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+/*
+ * 1-Wire slaves registers with addess 8 bit and data 16 bit
+ */
+
+static int w1_reg_a8_v16_read(void *context, unsigned int reg,
+				unsigned int *val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_READ_DATA);
+		w1_write_8(sl->master, reg);
+		*val = w1_read_8(sl->master);
+		*val |= w1_read_8(sl->master)<<8;
+	} else {
+		ret = -ENODEV;
+	}
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+static int w1_reg_a8_v16_write(void *context, unsigned int reg,
+				unsigned int val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_WRITE_DATA);
+		w1_write_8(sl->master, reg);
+		w1_write_8(sl->master, val & 0x00FF);
+		w1_write_8(sl->master, val>>8 & 0x00FF);
+	} else {
+		ret = -ENODEV;
+	}
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+/*
+ * 1-Wire slaves registers with addess 16 bit and data 16 bit
+ */
+
+static int w1_reg_a16_v16_read(void *context, unsigned int reg,
+				unsigned int *val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 65535)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_READ_DATA);
+		w1_write_8(sl->master, reg & 0x00FF);
+		w1_write_8(sl->master, reg>>8 & 0x00FF);
+		*val = w1_read_8(sl->master);
+		*val |= w1_read_8(sl->master)<<8;
+	} else {
+		ret = -ENODEV;
+	}
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+static int w1_reg_a16_v16_write(void *context, unsigned int reg,
+				unsigned int val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 65535)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_WRITE_DATA);
+		w1_write_8(sl->master, reg & 0x00FF);
+		w1_write_8(sl->master, reg>>8 & 0x00FF);
+		w1_write_8(sl->master, val & 0x00FF);
+		w1_write_8(sl->master, val>>8 & 0x00FF);
+	} else {
+		ret = -ENODEV;
+	}
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+/*
+ * Various types of supported bus addressing
+ */
+
+static struct regmap_bus regmap_w1_bus_a8_v8 = {
+	.reg_read = w1_reg_a8_v8_read,
+	.reg_write = w1_reg_a8_v8_write,
+};
+
+static struct regmap_bus regmap_w1_bus_a8_v16 = {
+	.reg_read = w1_reg_a8_v16_read,
+	.reg_write = w1_reg_a8_v16_write,
+};
+
+static struct regmap_bus regmap_w1_bus_a16_v16 = {
+	.reg_read = w1_reg_a16_v16_read,
+	.reg_write = w1_reg_a16_v16_write,
+};
+
+static const struct regmap_bus *regmap_get_w1_bus(struct device *w1_dev,
+					const struct regmap_config *config)
+{
+	if (config->reg_bits == 8 && config->val_bits == 8)
+		return &regmap_w1_bus_a8_v8;
+
+	if (config->reg_bits == 8 && config->val_bits == 16)
+		return &regmap_w1_bus_a8_v16;
+
+	if (config->reg_bits == 16 && config->val_bits == 16)
+		return &regmap_w1_bus_a16_v16;
+
+	return ERR_PTR(-ENOTSUPP);
+}
+
+struct regmap *__regmap_init_w1(struct device *w1_dev,
+				 const struct regmap_config *config,
+				 struct lock_class_key *lock_key,
+				 const char *lock_name)
+{
+
+	const struct regmap_bus *bus = regmap_get_w1_bus(w1_dev, config);
+
+	if (IS_ERR(bus))
+		return ERR_CAST(bus);
+
+	return __regmap_init(w1_dev, bus, w1_dev, config,
+			 lock_key, lock_name);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(__regmap_init_w1);
+
+struct regmap *__devm_regmap_init_w1(struct device *w1_dev,
+				 const struct regmap_config *config,
+				 struct lock_class_key *lock_key,
+				 const char *lock_name)
+{
+
+	const struct regmap_bus *bus = regmap_get_w1_bus(w1_dev, config);
+
+	if (IS_ERR(bus))
+		return ERR_CAST(bus);
+
+	return __devm_regmap_init(w1_dev, bus, w1_dev, config,
+				 lock_key, lock_name);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_w1);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index e88649225a..86eeacc142 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -461,6 +461,10 @@ struct regmap *__regmap_init_spmi_ext(struct spmi_device *dev,
 				      const struct regmap_config *config,
 				      struct lock_class_key *lock_key,
 				      const char *lock_name);
+struct regmap *__regmap_init_w1(struct device *w1_dev,
+				 const struct regmap_config *config,
+				 struct lock_class_key *lock_key,
+				 const char *lock_name);
 struct regmap *__regmap_init_mmio_clk(struct device *dev, const char *clk_id,
 				      void __iomem *regs,
 				      const struct regmap_config *config,
@@ -493,6 +497,10 @@ struct regmap *__devm_regmap_init_spmi_ext(struct spmi_device *dev,
 					   const struct regmap_config *config,
 					   struct lock_class_key *lock_key,
 					   const char *lock_name);
+struct regmap *__devm_regmap_init_w1(struct device *w1_dev,
+				      const struct regmap_config *config,
+				      struct lock_class_key *lock_key,
+				      const char *lock_name);
 struct regmap *__devm_regmap_init_mmio_clk(struct device *dev,
 					   const char *clk_id,
 					   void __iomem *regs,
@@ -597,6 +605,19 @@ int regmap_attach_dev(struct device *dev, struct regmap *map,
 				dev, config)
 
 /**
+ * regmap_init_w1() - Initialise register map
+ *
+ * @w1_dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer to
+ * a struct regmap.
+ */
+#define regmap_init_w1(w1_dev, config)					\
+	__regmap_lockdep_wrapper(__regmap_init_w1, #config,		\
+				w1_dev, config)
+
+/**
  * regmap_init_mmio_clk() - Initialise register map with register clock
  *
  * @dev: Device that will be interacted with
@@ -712,6 +733,19 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
 				dev, config)
 
 /**
+ * devm_regmap_init_w1() - Initialise managed register map
+ *
+ * @w1_dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.  The regmap will be automatically freed by the
+ * device management code.
+ */
+#define devm_regmap_init_w1(w1_dev, config)				\
+	__regmap_lockdep_wrapper(__devm_regmap_init_w1, #config,	\
+				w1_dev, config)
+/**
  * devm_regmap_init_mmio_clk() - Initialise managed register map with clock
  *
  * @dev: Device that will be interacted with
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] w1: MAX1721x Stanalone Fuel Gauge - add 1-Wire slave drivers
  2017-06-02  7:06 [PATCH 0/3] Battery monitor MAX1721x with w1-regmap and w1-slave (resend) Alex A. Mihaylov
  2017-06-02  7:06 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
@ 2017-06-02  7:06 ` Alex A. Mihaylov
  2017-06-08 12:27   ` Sebastian Reichel
  2017-06-02  7:06 ` [PATCH 3/3] power: supply: Add support MAX1721x battery monitor Alex A. Mihaylov
  2 siblings, 1 reply; 16+ messages in thread
From: Alex A. Mihaylov @ 2017-06-02  7:06 UTC (permalink / raw)
  To: Mark Brown, Greg Kroah-Hartman, Sebastian Reichel,
	Evgeniy Polyakov, linux-kernel, linux-pm
  Cc: Alex A. Mihaylov

Add support for Maxim Semiconductor MAX17211/MAX17215
Standlone Fuel Gauge chips (1-Wire family 0x26)
---
 drivers/w1/slaves/Kconfig       |  12 +++++
 drivers/w1/slaves/Makefile      |   1 +
 drivers/w1/slaves/w1_max1721x.c |  73 ++++++++++++++++++++++++++++
 drivers/w1/slaves/w1_max1721x.h | 102 ++++++++++++++++++++++++++++++++++++++++
 drivers/w1/w1_family.h          |   1 +
 5 files changed, 189 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 0ef9f2663d..06a63a1021 100644
--- a/drivers/w1/slaves/Kconfig
+++ b/drivers/w1/slaves/Kconfig
@@ -86,6 +86,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 b4a358955e..967329a4b8 100644
--- a/drivers/w1/slaves/Makefile
+++ b/drivers/w1/slaves/Makefile
@@ -11,6 +11,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 0000000000..017ec3e159
--- /dev/null
+++ b/drivers/w1/slaves/w1_max1721x.c
@@ -0,0 +1,73 @@
+/*
+ * 1-Wire implementation for Maxim Semiconductor
+ * MAX17211/MAX17215 standalone fuel gauge chip
+ *
+ * Copyright (C) 2017 OAO Radioavionica
+ * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
+ *
+ * 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"
+
+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 <minimumlaw@rambler.ru>");
+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 0000000000..47694d91dc
--- /dev/null
+++ b/drivers/w1/slaves/w1_max1721x.h
@@ -0,0 +1,102 @@
+/*
+ * 1-Wire implementation for Maxim Semiconductor
+ * MAX7211/MAX17215 stanalone fuel gauge chip
+ *
+ * Copyright (C) 2017 OAO Radioavionica
+ * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
+ *
+ * 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
+
+/* 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 */
+
+/* Number of valid register addresses */
+#define MAX1721X_MAX_REG_NR	0x1EF
+
+/* Convert regs value to power_supply units */
+
+static inline int max172xx_time_to_ps(unsigned int reg)
+{
+	return reg * 5625 / 1000;	/* in sec. */
+}
+
+static inline int max172xx_percent_to_ps(unsigned int reg)
+{
+	return reg / 256;	/* in percent from 0 to 100 */
+}
+
+static inline int max172xx_voltage_to_ps(unsigned int reg)
+{
+	return reg * 1250;	/* in uV */
+}
+
+static inline int max172xx_capacity_to_ps(unsigned int 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(unsigned int reg)
+{
+	int val = (int16_t)(reg);
+
+	return val * 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(unsigned int reg)
+{
+	int val = (int16_t)(reg);
+
+	return val * 156252;
+}
+
+#endif /* !__w1_max17211_h__ */
diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h
index c4a6b257a3..398adc9973 100644
--- a/drivers/w1/w1_family.h
+++ b/drivers/w1/w1_family.h
@@ -29,6 +29,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.13.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] power: supply: Add support MAX1721x battery monitor
  2017-06-02  7:06 [PATCH 0/3] Battery monitor MAX1721x with w1-regmap and w1-slave (resend) Alex A. Mihaylov
  2017-06-02  7:06 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
  2017-06-02  7:06 ` [PATCH 2/3] w1: MAX1721x Stanalone Fuel Gauge - add 1-Wire slave drivers Alex A. Mihaylov
@ 2017-06-02  7:06 ` Alex A. Mihaylov
  2017-06-08 12:48   ` Sebastian Reichel
  2 siblings, 1 reply; 16+ messages in thread
From: Alex A. Mihaylov @ 2017-06-02  7:06 UTC (permalink / raw)
  To: Mark Brown, Greg Kroah-Hartman, Sebastian Reichel,
	Evgeniy Polyakov, linux-kernel, linux-pm
  Cc: Alex A. Mihaylov

Add support for battery monitor MAX1721x (power_supply class).
Maxim Semiconductor MAX1721x Standalone Fuel Gauge battery monitor.
MAX17211 used for singlecell, MAX17215 for multicell batteryes.
---
 drivers/power/supply/Kconfig            |  14 ++
 drivers/power/supply/Makefile           |   1 +
 drivers/power/supply/max1721x_battery.c | 357 ++++++++++++++++++++++++++++++++
 3 files changed, 372 insertions(+)
 create mode 100644 drivers/power/supply/max1721x_battery.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index da54ac88f0..d431d6f8aa 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -268,6 +268,20 @@ config BATTERY_MAX17042
 	  with MAX17042. This driver also supports max17047/50 chips which are
 	  improved version of max17042.
 
+config BATTERY_MAX1721X
+	tristate "MAX17211/MAX17215 standalone gas-gauge"
+	depends on W1
+	select W1_SLAVE_MAX1721X
+	select REGMAP_W1
+	help
+	  MAX1721x is fuel-gauge systems for lithium-ion (Li+) batteries
+	  in handheld and portable equipment. MAX17211 used with single cell
+	  battery. MAX17215 designed for muticell battery. Both them have
+	  1-Wire (W1) host interface.
+
+	  Say Y here to enable support for the MAX17211/MAX17215 standalone
+	  battery gas-gauge.
+
 config BATTERY_Z2
 	tristate "Z2 battery driver"
 	depends on I2C && MACH_ZIPIT2
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 3789a2c06f..c785d05e02 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_CHARGER_DA9150)	+= da9150-charger.o
 obj-$(CONFIG_BATTERY_DA9150)	+= da9150-fg.o
 obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
 obj-$(CONFIG_BATTERY_MAX17042)	+= max17042_battery.o
+obj-$(CONFIG_BATTERY_MAX1721X)	+= max1721x_battery.o
 obj-$(CONFIG_BATTERY_Z2)	+= z2_battery.o
 obj-$(CONFIG_BATTERY_RT5033)	+= rt5033_battery.o
 obj-$(CONFIG_CHARGER_RT9455)	+= rt9455_charger.o
diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
new file mode 100644
index 0000000000..aa0effec3d
--- /dev/null
+++ b/drivers/power/supply/max1721x_battery.c
@@ -0,0 +1,357 @@
+/*
+ * 1-wire client/driver for the Maxim Semicondactor
+ * MAX17211/MAX17215 Standalone Fuel Gauge IC
+ *
+ * Copyright (C) 2017 OAO Radioavionica
+ * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
+ *
+ * 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/regmap.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;
+	struct regmap *regmap;
+	/* battery design format */
+	unsigned int 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);
+	unsigned int reg = 0;
+	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 =
+			regmap_read(info->regmap, MAX172XX_REG_STATUS,
+			&reg) ? 0 : !(reg & MAX172XX_BAT_PRESENT);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = regmap_read(info->regmap, MAX172XX_REG_REPSOC, &reg);
+		val->intval = max172xx_percent_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = regmap_read(info->regmap, MAX172XX_REG_BATT, &reg);
+		val->intval = max172xx_voltage_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		ret = regmap_read(info->regmap, MAX172XX_REG_DESIGNCAP, &reg);
+		val->intval = max172xx_capacity_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_REPCAP, &reg);
+		val->intval = max172xx_capacity_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_TTE, &reg);
+		val->intval = max172xx_time_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_TTF, &reg);
+		val->intval = max172xx_time_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		ret = regmap_read(info->regmap, MAX172XX_REG_TEMP, &reg);
+		val->intval = max172xx_temperature_to_ps(reg);
+		break;
+	/* We need signed current, so must cast info->rsense to signed type */
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = regmap_read(info->regmap, MAX172XX_REG_CURRENT, &reg);
+		val->intval =
+			max172xx_current_to_voltage(reg) / (int)info->rsense;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_AVGCURRENT, &reg);
+		val->intval =
+			max172xx_current_to_voltage(reg) / (int)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 = regmap_read(info->regmap, MAX1721X_REG_DEV_STR, &reg);
+		val->strval = info->DeviceName;
+		break;
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		ret = regmap_read(info->regmap, MAX1721X_REG_MFG_STR, &reg);
+		val->strval = info->ManufacturerName;
+		break;
+	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
+		ret = regmap_read(info->regmap, 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 max17211_device_info *info,
+			uint16_t reg, uint8_t nr, char *str)
+{
+	unsigned int val;
+
+	if (!str || !(reg == MAX1721X_REG_MFG_STR ||
+			reg == MAX1721X_REG_DEV_STR))
+		return -EFAULT;
+
+	while (nr--) {
+		if (regmap_read(info->regmap, 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 max17211_device_info *info, char *str)
+{
+	unsigned int val[3];
+
+	if (!str)
+		return -EFAULT;
+
+	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX, &val[0]))
+		return -EFAULT;
+	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 1, &val[1]))
+		return -EFAULT;
+	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 2, &val[2]))
+		return -EFAULT;
+
+	snprintf(str, 13, "%04X%04X%04X", val[0], val[1], val[2]);
+	return 0;
+}
+
+/* Model Gauge M5 Register Memory Map access */
+static const struct regmap_range max1721x_regs_allow[] = {
+	/* M5 Model Gauge Algorithm area */
+	regmap_reg_range(0x00, 0x23),
+	regmap_reg_range(0x27, 0x2F),
+	regmap_reg_range(0x32, 0x32),
+	regmap_reg_range(0x35, 0x36),
+	regmap_reg_range(0x38, 0x3A),
+	regmap_reg_range(0x3D, 0x3F),
+	regmap_reg_range(0x42, 0x42),
+	regmap_reg_range(0x45, 0x46),
+	regmap_reg_range(0x4A, 0x4A),
+	regmap_reg_range(0x4D, 0x4D),
+	regmap_reg_range(0xB0, 0xB0),
+	regmap_reg_range(0xB4, 0xB4),
+	regmap_reg_range(0xB8, 0xBE),
+	regmap_reg_range(0xD1, 0xDA),
+	regmap_reg_range(0xDC, 0xDF),
+	/* Factory settins area */
+	regmap_reg_range(0x180, 0x1DF),
+	{ }
+};
+
+static const struct regmap_range max1721x_regs_deny[] = {
+	regmap_reg_range(0x24, 0x26),
+	regmap_reg_range(0x30, 0x31),
+	regmap_reg_range(0x33, 0x34),
+	regmap_reg_range(0x37, 0x37),
+	regmap_reg_range(0x3B, 0x3C),
+	regmap_reg_range(0x40, 0x41),
+	regmap_reg_range(0x43, 0x44),
+	regmap_reg_range(0x47, 0x49),
+	regmap_reg_range(0x4B, 0x4C),
+	regmap_reg_range(0x4E, 0xAF),
+	regmap_reg_range(0xB1, 0xB3),
+	regmap_reg_range(0xB5, 0xB7),
+	regmap_reg_range(0xBF, 0xD0),
+	regmap_reg_range(0xDB, 0xDB),
+	regmap_reg_range(0xE0, 0x17F),
+	{ }
+};
+
+static const struct regmap_access_table max1721x_regs = {
+	.yes_ranges	= max1721x_regs_allow,
+	.n_yes_ranges	= ARRAY_SIZE(max1721x_regs_allow),
+	.no_ranges	= max1721x_regs_deny,
+	.n_no_ranges	= ARRAY_SIZE(max1721x_regs_deny),
+};
+
+/* W1 regmap config */
+static const struct regmap_config max1721x_regmap_w1_config = {
+	.reg_bits = 16,
+	.val_bits = 16,
+	.volatile_table = &max1721x_regs,
+	.max_register = MAX1721X_MAX_REG_NR,
+};
+
+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;
+
+	/* regmap init */
+	info->regmap = devm_regmap_init_w1(info->w1_dev,
+					&max1721x_regmap_w1_config);
+	if (IS_ERR(info->regmap)) {
+		int err = PTR_ERR(info->regmap);
+
+		dev_err(info->dev, "Failed to allocate register map: %d\n",
+			err);
+		return err;
+	}
+
+	/* rsense init */
+	info->rsense = 0;
+	if (regmap_read(info->regmap, MAX1721X_REG_NRSENSE, &info->rsense))
+
+	if (!info->rsense) {
+		dev_warn(info->dev, "RSenese not calibrated, set 10 mOhms!\n");
+		info->rsense = 1000; /* in regs in 10^-5 */
+	}
+	dev_info(info->dev, "RSense: %d mOhms.\n", info->rsense / 100);
+
+	if (get_string(info, 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, 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]) {
+		unsigned int dev_name;
+
+		if (regmap_read(info->regmap,
+				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, info->SerialNumber)) {
+		dev_err(info->dev, "Can't read serial. Hardware error.\n");
+		return -ENODEV;
+	}
+
+	info->bat = devm_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 struct platform_driver max1721x_battery_driver = {
+	.driver = {
+		.name = "max1721x-battery",
+	},
+	.probe	  = max1721x_battery_probe,
+};
+module_platform_driver(max1721x_battery_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alex A. Mihaylov <minimumlaw@rambler.ru>");
+MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
+MODULE_ALIAS("platform:max1721x-battery");
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] regmap: Add 1-Wire bus support
  2017-06-02  7:06 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
@ 2017-06-06 18:50   ` Mark Brown
  2017-06-08 12:53   ` Sebastian Reichel
  1 sibling, 0 replies; 16+ messages in thread
From: Mark Brown @ 2017-06-06 18:50 UTC (permalink / raw)
  To: Alex A. Mihaylov
  Cc: Greg Kroah-Hartman, Sebastian Reichel, Evgeniy Polyakov,
	linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

On Fri, Jun 02, 2017 at 10:06:27AM +0300, Alex A. Mihaylov wrote:
> Add basic support regmap (register map access) API for 1-Wire bus

Applied, thanks.  I created a signed tag for the other two patches to
use:

The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:

  Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git regmap-1wire-support

for you to fetch changes up to cc5d0db390b0ff0f5da95b643a2b070da15a9c3e:

  regmap: Add 1-Wire bus support (2017-06-06 19:47:28 +0100)

----------------------------------------------------------------
regmap: Add support for 1wire bus

This tag contains a patch adding support for the 1wire bus to regmap.

----------------------------------------------------------------
Alex A. Mihaylov (1):
      regmap: Add 1-Wire bus support

 drivers/base/regmap/Kconfig     |   6 +-
 drivers/base/regmap/Makefile    |   1 +
 drivers/base/regmap/regmap-w1.c | 245 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h          |  34 ++++++
 4 files changed, 285 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-w1.c

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] w1: MAX1721x Stanalone Fuel Gauge - add 1-Wire slave drivers
  2017-06-02  7:06 ` [PATCH 2/3] w1: MAX1721x Stanalone Fuel Gauge - add 1-Wire slave drivers Alex A. Mihaylov
@ 2017-06-08 12:27   ` Sebastian Reichel
  0 siblings, 0 replies; 16+ messages in thread
From: Sebastian Reichel @ 2017-06-08 12:27 UTC (permalink / raw)
  To: Alex A. Mihaylov
  Cc: Mark Brown, Greg Kroah-Hartman, Evgeniy Polyakov, linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 8674 bytes --]

Hi,

On Fri, Jun 02, 2017 at 10:06:28AM +0300, Alex A. Mihaylov wrote:
> Add support for Maxim Semiconductor MAX17211/MAX17215
> Standlone Fuel Gauge chips (1-Wire family 0x26)

I think this is pretty pointless after the w1 subsystem
is available publically [0]. We can just instantiate the w1
device in power-supply and skip registration of another
platform-device.

The registers and helper functions should also be added directly
to the power-supply driver.

[0] https://www.spinics.net/lists/kernel/msg2524566.html

-- Sebastian

> ---
>  drivers/w1/slaves/Kconfig       |  12 +++++
>  drivers/w1/slaves/Makefile      |   1 +
>  drivers/w1/slaves/w1_max1721x.c |  73 ++++++++++++++++++++++++++++
>  drivers/w1/slaves/w1_max1721x.h | 102 ++++++++++++++++++++++++++++++++++++++++
>  drivers/w1/w1_family.h          |   1 +
>  5 files changed, 189 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 0ef9f2663d..06a63a1021 100644
> --- a/drivers/w1/slaves/Kconfig
> +++ b/drivers/w1/slaves/Kconfig
> @@ -86,6 +86,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 b4a358955e..967329a4b8 100644
> --- a/drivers/w1/slaves/Makefile
> +++ b/drivers/w1/slaves/Makefile
> @@ -11,6 +11,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 0000000000..017ec3e159
> --- /dev/null
> +++ b/drivers/w1/slaves/w1_max1721x.c
> @@ -0,0 +1,73 @@
> +/*
> + * 1-Wire implementation for Maxim Semiconductor
> + * MAX17211/MAX17215 standalone fuel gauge chip
> + *
> + * Copyright (C) 2017 OAO Radioavionica
> + * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
> + *
> + * 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"
> +
> +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 <minimumlaw@rambler.ru>");
> +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 0000000000..47694d91dc
> --- /dev/null
> +++ b/drivers/w1/slaves/w1_max1721x.h
> @@ -0,0 +1,102 @@
> +/*
> + * 1-Wire implementation for Maxim Semiconductor
> + * MAX7211/MAX17215 stanalone fuel gauge chip
> + *
> + * Copyright (C) 2017 OAO Radioavionica
> + * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
> + *
> + * 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
> +
> +/* 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 */
> +
> +/* Number of valid register addresses */
> +#define MAX1721X_MAX_REG_NR	0x1EF
> +
> +/* Convert regs value to power_supply units */
> +
> +static inline int max172xx_time_to_ps(unsigned int reg)
> +{
> +	return reg * 5625 / 1000;	/* in sec. */
> +}
> +
> +static inline int max172xx_percent_to_ps(unsigned int reg)
> +{
> +	return reg / 256;	/* in percent from 0 to 100 */
> +}
> +
> +static inline int max172xx_voltage_to_ps(unsigned int reg)
> +{
> +	return reg * 1250;	/* in uV */
> +}
> +
> +static inline int max172xx_capacity_to_ps(unsigned int 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(unsigned int reg)
> +{
> +	int val = (int16_t)(reg);
> +
> +	return val * 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(unsigned int reg)
> +{
> +	int val = (int16_t)(reg);
> +
> +	return val * 156252;
> +}
> +
> +#endif /* !__w1_max17211_h__ */
> diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h
> index c4a6b257a3..398adc9973 100644
> --- a/drivers/w1/w1_family.h
> +++ b/drivers/w1/w1_family.h
> @@ -29,6 +29,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.13.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] power: supply: Add support MAX1721x battery monitor
  2017-06-02  7:06 ` [PATCH 3/3] power: supply: Add support MAX1721x battery monitor Alex A. Mihaylov
@ 2017-06-08 12:48   ` Sebastian Reichel
  2017-06-08 17:44     ` Alex A. Mihaylov
  0 siblings, 1 reply; 16+ messages in thread
From: Sebastian Reichel @ 2017-06-08 12:48 UTC (permalink / raw)
  To: Alex A. Mihaylov
  Cc: Mark Brown, Greg Kroah-Hartman, Evgeniy Polyakov, linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 14812 bytes --]

Hi Alex,

On Fri, Jun 02, 2017 at 10:06:29AM +0300, Alex A. Mihaylov wrote:
> Add support for battery monitor MAX1721x (power_supply class).
> Maxim Semiconductor MAX1721x Standalone Fuel Gauge battery monitor.
> MAX17211 used for singlecell, MAX17215 for multicell batteryes.
> ---
>  drivers/power/supply/Kconfig            |  14 ++
>  drivers/power/supply/Makefile           |   1 +
>  drivers/power/supply/max1721x_battery.c | 357 ++++++++++++++++++++++++++++++++
>  3 files changed, 372 insertions(+)
>  create mode 100644 drivers/power/supply/max1721x_battery.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index da54ac88f0..d431d6f8aa 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -268,6 +268,20 @@ config BATTERY_MAX17042
>  	  with MAX17042. This driver also supports max17047/50 chips which are
>  	  improved version of max17042.
>  
> +config BATTERY_MAX1721X
> +	tristate "MAX17211/MAX17215 standalone gas-gauge"
> +	depends on W1
> +	select W1_SLAVE_MAX1721X
> +	select REGMAP_W1
> +	help
> +	  MAX1721x is fuel-gauge systems for lithium-ion (Li+) batteries
> +	  in handheld and portable equipment. MAX17211 used with single cell
> +	  battery. MAX17215 designed for muticell battery. Both them have
> +	  1-Wire (W1) host interface.
> +
> +	  Say Y here to enable support for the MAX17211/MAX17215 standalone
> +	  battery gas-gauge.
> +
>  config BATTERY_Z2
>  	tristate "Z2 battery driver"
>  	depends on I2C && MACH_ZIPIT2
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 3789a2c06f..c785d05e02 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -41,6 +41,7 @@ obj-$(CONFIG_CHARGER_DA9150)	+= da9150-charger.o
>  obj-$(CONFIG_BATTERY_DA9150)	+= da9150-fg.o
>  obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
>  obj-$(CONFIG_BATTERY_MAX17042)	+= max17042_battery.o
> +obj-$(CONFIG_BATTERY_MAX1721X)	+= max1721x_battery.o
>  obj-$(CONFIG_BATTERY_Z2)	+= z2_battery.o
>  obj-$(CONFIG_BATTERY_RT5033)	+= rt5033_battery.o
>  obj-$(CONFIG_CHARGER_RT9455)	+= rt9455_charger.o
> diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
> new file mode 100644
> index 0000000000..aa0effec3d
> --- /dev/null
> +++ b/drivers/power/supply/max1721x_battery.c
> @@ -0,0 +1,357 @@
> +/*
> + * 1-wire client/driver for the Maxim Semicondactor
> + * MAX17211/MAX17215 Standalone Fuel Gauge IC
> + *
> + * Copyright (C) 2017 OAO Radioavionica
> + * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
> + *
> + * 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>

param?

> +#include <linux/pm.h>
> +#include <linux/regmap.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/idr.h>
> +
> +#include "../../w1/w1.h"

This will conflict with public w1 interface patch
https://www.spinics.net/lists/kernel/msg2524566.html

This patch should be on top of that patch.

> +#include "../../w1/slaves/w1_max1721x.h"

Let's merge those defines into the driver. They
are not used anywhere else.

> +#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;
> +	struct regmap *regmap;
> +	/* battery design format */
> +	unsigned int 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);
> +	unsigned int reg = 0;
> +	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 =
> +			regmap_read(info->regmap, MAX172XX_REG_STATUS,
> +			&reg) ? 0 : !(reg & MAX172XX_BAT_PRESENT);
> +		break;
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_REPSOC, &reg);
> +		val->intval = max172xx_percent_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_BATT, &reg);
> +		val->intval = max172xx_voltage_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_DESIGNCAP, &reg);
> +		val->intval = max172xx_capacity_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_REPCAP, &reg);
> +		val->intval = max172xx_capacity_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_TTE, &reg);
> +		val->intval = max172xx_time_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_TTF, &reg);
> +		val->intval = max172xx_time_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_TEMP:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_TEMP, &reg);
> +		val->intval = max172xx_temperature_to_ps(reg);
> +		break;
> +	/* We need signed current, so must cast info->rsense to signed type */
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_CURRENT, &reg);
> +		val->intval =
> +			max172xx_current_to_voltage(reg) / (int)info->rsense;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_AVGCURRENT, &reg);
> +		val->intval =
> +			max172xx_current_to_voltage(reg) / (int)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 = regmap_read(info->regmap, MAX1721X_REG_DEV_STR, &reg);
> +		val->strval = info->DeviceName;
> +		break;
> +	case POWER_SUPPLY_PROP_MANUFACTURER:
> +		ret = regmap_read(info->regmap, MAX1721X_REG_MFG_STR, &reg);
> +		val->strval = info->ManufacturerName;
> +		break;
> +	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
> +		ret = regmap_read(info->regmap, 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 max17211_device_info *info,
> +			uint16_t reg, uint8_t nr, char *str)
> +{
> +	unsigned int val;
> +
> +	if (!str || !(reg == MAX1721X_REG_MFG_STR ||
> +			reg == MAX1721X_REG_DEV_STR))
> +		return -EFAULT;
> +
> +	while (nr--) {
> +		if (regmap_read(info->regmap, 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 max17211_device_info *info, char *str)
> +{
> +	unsigned int val[3];
> +
> +	if (!str)
> +		return -EFAULT;
> +
> +	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX, &val[0]))
> +		return -EFAULT;
> +	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 1, &val[1]))
> +		return -EFAULT;
> +	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 2, &val[2]))
> +		return -EFAULT;
> +
> +	snprintf(str, 13, "%04X%04X%04X", val[0], val[1], val[2]);
> +	return 0;
> +}
> +
> +/* Model Gauge M5 Register Memory Map access */
> +static const struct regmap_range max1721x_regs_allow[] = {
> +	/* M5 Model Gauge Algorithm area */
> +	regmap_reg_range(0x00, 0x23),
> +	regmap_reg_range(0x27, 0x2F),
> +	regmap_reg_range(0x32, 0x32),
> +	regmap_reg_range(0x35, 0x36),
> +	regmap_reg_range(0x38, 0x3A),
> +	regmap_reg_range(0x3D, 0x3F),
> +	regmap_reg_range(0x42, 0x42),
> +	regmap_reg_range(0x45, 0x46),
> +	regmap_reg_range(0x4A, 0x4A),
> +	regmap_reg_range(0x4D, 0x4D),
> +	regmap_reg_range(0xB0, 0xB0),
> +	regmap_reg_range(0xB4, 0xB4),
> +	regmap_reg_range(0xB8, 0xBE),
> +	regmap_reg_range(0xD1, 0xDA),
> +	regmap_reg_range(0xDC, 0xDF),
> +	/* Factory settins area */
> +	regmap_reg_range(0x180, 0x1DF),
> +	{ }
> +};
> +
> +static const struct regmap_range max1721x_regs_deny[] = {
> +	regmap_reg_range(0x24, 0x26),
> +	regmap_reg_range(0x30, 0x31),
> +	regmap_reg_range(0x33, 0x34),
> +	regmap_reg_range(0x37, 0x37),
> +	regmap_reg_range(0x3B, 0x3C),
> +	regmap_reg_range(0x40, 0x41),
> +	regmap_reg_range(0x43, 0x44),
> +	regmap_reg_range(0x47, 0x49),
> +	regmap_reg_range(0x4B, 0x4C),
> +	regmap_reg_range(0x4E, 0xAF),
> +	regmap_reg_range(0xB1, 0xB3),
> +	regmap_reg_range(0xB5, 0xB7),
> +	regmap_reg_range(0xBF, 0xD0),
> +	regmap_reg_range(0xDB, 0xDB),
> +	regmap_reg_range(0xE0, 0x17F),
> +	{ }
> +};
> +
> +static const struct regmap_access_table max1721x_regs = {
> +	.yes_ranges	= max1721x_regs_allow,
> +	.n_yes_ranges	= ARRAY_SIZE(max1721x_regs_allow),
> +	.no_ranges	= max1721x_regs_deny,
> +	.n_no_ranges	= ARRAY_SIZE(max1721x_regs_deny),
> +};

It should be enough to specify the yes_range. Unspecified
values will be no implicitly.

> +/* W1 regmap config */
> +static const struct regmap_config max1721x_regmap_w1_config = {
> +	.reg_bits = 16,
> +	.val_bits = 16,
> +	.volatile_table = &max1721x_regs,
> +	.max_register = MAX1721X_MAX_REG_NR,
> +};

Are the non-volatile registers missing? Then you probably also
want to specify .rd_table with the same access table, so that
dumping registers via debugfs work correctly. Did you try to
cat /sys/kernel/debug/regmap/<your-device>/registers?

> +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;
> +
> +	/* regmap init */
> +	info->regmap = devm_regmap_init_w1(info->w1_dev,
> +					&max1721x_regmap_w1_config);
> +	if (IS_ERR(info->regmap)) {
> +		int err = PTR_ERR(info->regmap);
> +
> +		dev_err(info->dev, "Failed to allocate register map: %d\n",
> +			err);
> +		return err;
> +	}
> +
> +	/* rsense init */
> +	info->rsense = 0;
> +	if (regmap_read(info->regmap, MAX1721X_REG_NRSENSE, &info->rsense))
> +
> +	if (!info->rsense) {
> +		dev_warn(info->dev, "RSenese not calibrated, set 10 mOhms!\n");
> +		info->rsense = 1000; /* in regs in 10^-5 */
> +	}
> +	dev_info(info->dev, "RSense: %d mOhms.\n", info->rsense / 100);
> +
> +	if (get_string(info, 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, 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]) {
> +		unsigned int dev_name;
> +
> +		if (regmap_read(info->regmap,
> +				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, info->SerialNumber)) {
> +		dev_err(info->dev, "Can't read serial. Hardware error.\n");
> +		return -ENODEV;
> +	}
> +
> +	info->bat = devm_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 struct platform_driver max1721x_battery_driver = {
> +	.driver = {
> +		.name = "max1721x-battery",
> +	},
> +	.probe	  = max1721x_battery_probe,
> +};
> +module_platform_driver(max1721x_battery_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alex A. Mihaylov <minimumlaw@rambler.ru>");
> +MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
> +MODULE_ALIAS("platform:max1721x-battery");

Otherwise looks good.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] regmap: Add 1-Wire bus support
  2017-06-02  7:06 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
  2017-06-06 18:50   ` Mark Brown
@ 2017-06-08 12:53   ` Sebastian Reichel
  2017-06-08 12:57     ` Mark Brown
  1 sibling, 1 reply; 16+ messages in thread
From: Sebastian Reichel @ 2017-06-08 12:53 UTC (permalink / raw)
  To: Alex A. Mihaylov
  Cc: Mark Brown, Greg Kroah-Hartman, Evgeniy Polyakov, linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

Hi,

On Fri, Jun 02, 2017 at 10:06:27AM +0300, Alex A. Mihaylov wrote:
> Add basic support regmap (register map access) API for 1-Wire bus

Thanks :)

> diff --git a/drivers/base/regmap/regmap-w1.c b/drivers/base/regmap/regmap-w1.c
> new file mode 100644
> index 0000000000..5f04e7bf06
> --- /dev/null
> +++ b/drivers/base/regmap/regmap-w1.c
> @@ -0,0 +1,245 @@
> +/*
> + * Register map access API - W1 (1-Wire) support
> + *
> + * Copyright (C) 2017 OAO Radioavionica
> + * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
> + *
> + * 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/regmap.h>
> +#include <linux/module.h>
> +#include "../../w1/w1.h"

This conflicts with the following patch:

https://www.spinics.net/lists/kernel/msg2524566.html

I guess Greg could pull your immutable branch and above
patch can be augumented to also update regmap-w1.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] regmap: Add 1-Wire bus support
  2017-06-08 12:53   ` Sebastian Reichel
@ 2017-06-08 12:57     ` Mark Brown
  2017-06-08 13:13       ` Sebastian Reichel
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2017-06-08 12:57 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Alex A. Mihaylov, Greg Kroah-Hartman, Evgeniy Polyakov,
	linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

On Thu, Jun 08, 2017 at 02:53:50PM +0200, Sebastian Reichel wrote:
> On Fri, Jun 02, 2017 at 10:06:27AM +0300, Alex A. Mihaylov wrote:
> > Add basic support regmap (register map access) API for 1-Wire bus

> > +#include <linux/module.h>
> > +#include "../../w1/w1.h"

> This conflicts with the following patch:

> https://www.spinics.net/lists/kernel/msg2524566.html

Not in -next yet?

> I guess Greg could pull your immutable branch and above
> patch can be augumented to also update regmap-w1.

Yes, or I can pull in a tag and apply an integration fix if needed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] regmap: Add 1-Wire bus support
  2017-06-08 12:57     ` Mark Brown
@ 2017-06-08 13:13       ` Sebastian Reichel
  0 siblings, 0 replies; 16+ messages in thread
From: Sebastian Reichel @ 2017-06-08 13:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Alex A. Mihaylov, Greg Kroah-Hartman, Evgeniy Polyakov,
	linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

Hi,

On Thu, Jun 08, 2017 at 01:57:04PM +0100, Mark Brown wrote:
> On Thu, Jun 08, 2017 at 02:53:50PM +0200, Sebastian Reichel wrote:
> > On Fri, Jun 02, 2017 at 10:06:27AM +0300, Alex A. Mihaylov wrote:
> > > Add basic support regmap (register map access) API for 1-Wire bus
> 
> > > +#include <linux/module.h>
> > > +#include "../../w1/w1.h"
> 
> > This conflicts with the following patch:
> 
> > https://www.spinics.net/lists/kernel/msg2524566.html
> 
> Not in -next yet?

I think Greg did not yet apply it. It already has been
Acked by w1 maintainer, though. This was just a heads
up for upcoming trouble :)

> > I guess Greg could pull your immutable branch and above
> > patch can be augumented to also update regmap-w1.
> 
> Yes, or I can pull in a tag and apply an integration fix if needed.

Sure.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] power: supply: Add support MAX1721x battery monitor
  2017-06-08 12:48   ` Sebastian Reichel
@ 2017-06-08 17:44     ` Alex A. Mihaylov
  2017-06-08 19:17       ` Sebastian Reichel
  0 siblings, 1 reply; 16+ messages in thread
From: Alex A. Mihaylov @ 2017-06-08 17:44 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Mark Brown, Greg Kroah-Hartman, Evgeniy Polyakov, linux-kernel, linux-pm


08.06.17 15:48, Sebastian Reichel wrote:
> On Fri, Jun 02, 2017 at 10:06:29AM +0300, Alex A. Mihaylov wrote:
>> diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
>> new file mode 100644
>> index 0000000000..aa0effec3d
>> --- /dev/null
>> +++ b/drivers/power/supply/max1721x_battery.c
>> @@ -0,0 +1,357 @@
>> +/*
>> + * 1-wire client/driver for the Maxim Semicondactor
>> + * MAX17211/MAX17215 Standalone Fuel Gauge IC
>> + *
>> + * Copyright (C) 2017 OAO Radioavionica
>> + * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
>> + *
>> + * 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>
> param?
Ok, sorry. This really not need. I remove this in next revision.
>> +#include <linux/pm.h>
>> +#include <linux/regmap.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/idr.h>
>> +
>> +#include "../../w1/w1.h"
> This will conflict with public w1 interface patch
> https://www.spinics.net/lists/kernel/msg2524566.html
>
> This patch should be on top of that patch.
Ok. No problem. I can fix this here. I can fix this in regmap-w1. Just 
tell me which of the patches will be applied first. If the one to which 
you refer, I will resend the patches immediately after it appears at 
least in -rc.
>> +#include "../../w1/slaves/w1_max1721x.h"
> Let's merge those defines into the driver. They
> are not used anywhere else.
Theory, Maxim integrated have MAX17201/MAX17205 with I2C interface. This 
may required for feature i2c driver.
>> +
>> +/* Model Gauge M5 Register Memory Map access */
>> +static const struct regmap_range max1721x_regs_allow[] = {
>> +	/* M5 Model Gauge Algorithm area */
>> +	regmap_reg_range(0x00, 0x23),
>> +	regmap_reg_range(0x27, 0x2F),
>> +	regmap_reg_range(0x32, 0x32),
>> +	regmap_reg_range(0x35, 0x36),
>> +	regmap_reg_range(0x38, 0x3A),
>> +	regmap_reg_range(0x3D, 0x3F),
>> +	regmap_reg_range(0x42, 0x42),
>> +	regmap_reg_range(0x45, 0x46),
>> +	regmap_reg_range(0x4A, 0x4A),
>> +	regmap_reg_range(0x4D, 0x4D),
>> +	regmap_reg_range(0xB0, 0xB0),
>> +	regmap_reg_range(0xB4, 0xB4),
>> +	regmap_reg_range(0xB8, 0xBE),
>> +	regmap_reg_range(0xD1, 0xDA),
>> +	regmap_reg_range(0xDC, 0xDF),
>> +	/* Factory settins area */
>> +	regmap_reg_range(0x180, 0x1DF),
>> +	{ }
>> +};
>> +
>> +static const struct regmap_range max1721x_regs_deny[] = {
>> +	regmap_reg_range(0x24, 0x26),
>> +	regmap_reg_range(0x30, 0x31),
>> +	regmap_reg_range(0x33, 0x34),
>> +	regmap_reg_range(0x37, 0x37),
>> +	regmap_reg_range(0x3B, 0x3C),
>> +	regmap_reg_range(0x40, 0x41),
>> +	regmap_reg_range(0x43, 0x44),
>> +	regmap_reg_range(0x47, 0x49),
>> +	regmap_reg_range(0x4B, 0x4C),
>> +	regmap_reg_range(0x4E, 0xAF),
>> +	regmap_reg_range(0xB1, 0xB3),
>> +	regmap_reg_range(0xB5, 0xB7),
>> +	regmap_reg_range(0xBF, 0xD0),
>> +	regmap_reg_range(0xDB, 0xDB),
>> +	regmap_reg_range(0xE0, 0x17F),
>> +	{ }
>> +};
>> +
>> +static const struct regmap_access_table max1721x_regs = {
>> +	.yes_ranges	= max1721x_regs_allow,
>> +	.n_yes_ranges	= ARRAY_SIZE(max1721x_regs_allow),
>> +	.no_ranges	= max1721x_regs_deny,
>> +	.n_no_ranges	= ARRAY_SIZE(max1721x_regs_deny),
>> +};
> It should be enough to specify the yes_range. Unspecified
> values will be no implicitly.
I can remove this. I just desribe all registers hole described in 
datasheet. I hope this reduce memory in regmap infrastructure.
>> +/* W1 regmap config */
>> +static const struct regmap_config max1721x_regmap_w1_config = {
>> +	.reg_bits = 16,
>> +	.val_bits = 16,
>> +	.volatile_table = &max1721x_regs,
>> +	.max_register = MAX1721X_MAX_REG_NR,
>> +};
> Are the non-volatile registers missing? Then you probably also
> want to specify .rd_table with the same access table, so that
> dumping registers via debugfs work correctly. Did you try to
> cat /sys/kernel/debug/regmap/<your-device>/registers?
Ok, I try this. Non-volatile registers present (Rsense, manufaturer, 
device name, serial number). I not read this register until probe step, 
so I not put them into nonvolatile regmap table. But I can do this. May 
be it's more correctly, than desribe registers hole.
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Alex A. Mihaylov <minimumlaw@rambler.ru>");
>> +MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
>> +MODULE_ALIAS("platform:max1721x-battery");
> Otherwise looks good.
BTW. I try send RFC with alternative realisation of this driver into 
linux-pm:
[1] http://marc.info/?l=linux-pm&m=149422406914440
[2] http://marc.info/?l=linux-pm&m=149422407014444

This code maped to thermal zones, not used platform device and put 
max172xx-battery.h into include/linux/power. All know troubel in [1].

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] power: supply: Add support MAX1721x battery monitor
  2017-06-08 17:44     ` Alex A. Mihaylov
@ 2017-06-08 19:17       ` Sebastian Reichel
  2017-06-13 16:27         ` [PATCH] power_supply: add max1721x_battery driver Alex A. Mihaylov
  0 siblings, 1 reply; 16+ messages in thread
From: Sebastian Reichel @ 2017-06-08 19:17 UTC (permalink / raw)
  To: Alex A. Mihaylov
  Cc: Mark Brown, Greg Kroah-Hartman, Evgeniy Polyakov, linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 8249 bytes --]

Hi Alex,

On Thu, Jun 08, 2017 at 08:44:35PM +0300, Alex A. Mihaylov wrote:
> 08.06.17 15:48, Sebastian Reichel wrote:
> > On Fri, Jun 02, 2017 at 10:06:29AM +0300, Alex A. Mihaylov wrote:
> > > diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
> > > new file mode 100644
> > > index 0000000000..aa0effec3d
> > > --- /dev/null
> > > +++ b/drivers/power/supply/max1721x_battery.c
> > > @@ -0,0 +1,357 @@
> > > +/*
> > > + * 1-wire client/driver for the Maxim Semicondactor
> > > + * MAX17211/MAX17215 Standalone Fuel Gauge IC
> > > + *
> > > + * Copyright (C) 2017 OAO Radioavionica
> > > + * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
> > > + *
> > > + * 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>
> > param?
> Ok, sorry. This really not need. I remove this in next revision.
> > > +#include <linux/pm.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/power_supply.h>
> > > +#include <linux/idr.h>
> > > +
> > > +#include "../../w1/w1.h"
> > This will conflict with public w1 interface patch
> > https://www.spinics.net/lists/kernel/msg2524566.html
> > 
> > This patch should be on top of that patch.
> Ok. No problem. I can fix this here. I can fix this in regmap-w1. Just tell
> me which of the patches will be applied first. If the one to which you
> refer, I will resend the patches immediately after it appears at least in
> -rc.
> > > +#include "../../w1/slaves/w1_max1721x.h"
> > Let's merge those defines into the driver. They
> > are not used anywhere else.
> Theory, Maxim integrated have MAX17201/MAX17205 with I2C interface. This may
> required for feature i2c driver.

This would end up in the same driver with only probe function being
different.

> > > +
> > > +/* Model Gauge M5 Register Memory Map access */
> > > +static const struct regmap_range max1721x_regs_allow[] = {
> > > +	/* M5 Model Gauge Algorithm area */
> > > +	regmap_reg_range(0x00, 0x23),
> > > +	regmap_reg_range(0x27, 0x2F),
> > > +	regmap_reg_range(0x32, 0x32),
> > > +	regmap_reg_range(0x35, 0x36),
> > > +	regmap_reg_range(0x38, 0x3A),
> > > +	regmap_reg_range(0x3D, 0x3F),
> > > +	regmap_reg_range(0x42, 0x42),
> > > +	regmap_reg_range(0x45, 0x46),
> > > +	regmap_reg_range(0x4A, 0x4A),
> > > +	regmap_reg_range(0x4D, 0x4D),
> > > +	regmap_reg_range(0xB0, 0xB0),
> > > +	regmap_reg_range(0xB4, 0xB4),
> > > +	regmap_reg_range(0xB8, 0xBE),
> > > +	regmap_reg_range(0xD1, 0xDA),
> > > +	regmap_reg_range(0xDC, 0xDF),
> > > +	/* Factory settins area */
> > > +	regmap_reg_range(0x180, 0x1DF),
> > > +	{ }
> > > +};
> > > +
> > > +static const struct regmap_range max1721x_regs_deny[] = {
> > > +	regmap_reg_range(0x24, 0x26),
> > > +	regmap_reg_range(0x30, 0x31),
> > > +	regmap_reg_range(0x33, 0x34),
> > > +	regmap_reg_range(0x37, 0x37),
> > > +	regmap_reg_range(0x3B, 0x3C),
> > > +	regmap_reg_range(0x40, 0x41),
> > > +	regmap_reg_range(0x43, 0x44),
> > > +	regmap_reg_range(0x47, 0x49),
> > > +	regmap_reg_range(0x4B, 0x4C),
> > > +	regmap_reg_range(0x4E, 0xAF),
> > > +	regmap_reg_range(0xB1, 0xB3),
> > > +	regmap_reg_range(0xB5, 0xB7),
> > > +	regmap_reg_range(0xBF, 0xD0),
> > > +	regmap_reg_range(0xDB, 0xDB),
> > > +	regmap_reg_range(0xE0, 0x17F),
> > > +	{ }
> > > +};
> > > +
> > > +static const struct regmap_access_table max1721x_regs = {
> > > +	.yes_ranges	= max1721x_regs_allow,
> > > +	.n_yes_ranges	= ARRAY_SIZE(max1721x_regs_allow),
> > > +	.no_ranges	= max1721x_regs_deny,
> > > +	.n_no_ranges	= ARRAY_SIZE(max1721x_regs_deny),
> > > +};
> > It should be enough to specify the yes_range. Unspecified
> > values will be no implicitly.
> I can remove this. I just desribe all registers hole described in datasheet.
> I hope this reduce memory in regmap infrastructure.

That's what I suspected.

> > > +/* W1 regmap config */
> > > +static const struct regmap_config max1721x_regmap_w1_config = {
> > > +	.reg_bits = 16,
> > > +	.val_bits = 16,
> > > +	.volatile_table = &max1721x_regs,
> > > +	.max_register = MAX1721X_MAX_REG_NR,
> > > +};
> > Are the non-volatile registers missing? Then you probably also
> > want to specify .rd_table with the same access table, so that
> > dumping registers via debugfs work correctly. Did you try to
> > cat /sys/kernel/debug/regmap/<your-device>/registers?
> Ok, I try this. Non-volatile registers present (Rsense, manufaturer, device
> name, serial number). I not read this register until probe step, so I not
> put them into nonvolatile regmap table. But I can do this. May be it's more
> correctly, than desribe registers hole.

Register hole table should be used for the rd_table. You can skip
configuration of the volatile_table, if you do not enable caching
via max1721x_regmap_w1_config.cache_type. Enabling the caching is
only sensible, if you do not mark all registers as volatile ;)

> > > +
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_AUTHOR("Alex A. Mihaylov <minimumlaw@rambler.ru>");
> > > +MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
> > > +MODULE_ALIAS("platform:max1721x-battery");
> > Otherwise looks good.
> BTW. I try send RFC with alternative realisation of this driver into
> linux-pm:
> [1] http://marc.info/?l=linux-pm&m=149422406914440
> [2] http://marc.info/?l=linux-pm&m=149422407014444

Ah I skipped this one, since there was a newer revision.
Yes, this is what I had in mind when I talked about merging
the w1 driver into the power-supply driver.

> This code maped to thermal zones, not used platform device and put
> max172xx-battery.h into include/linux/power. All know troubel in [1].

So here are answers to your questions

> 1. Endian for W1-regmap
> Code writen for use nativ endian for host (master) device. But waiting for
> little-endian slave divice. I don't know W1 slave device with big-endian
> addrees or data format. May be regmap_bus or regmap_config will be checked
> for host/device endian requirements?

regmap_bus.reg_format_endian_default provides a default endianess for
the bus, which can be overwritten via regmap_config.val_format_endian.
Since Mark queued your patch everything seems to be alright (I did not
review it).

> 2. W1 Family/device infrastructure
> All present power_supply class drivers in vanilla kernel use w1-slave devices.
> All of them create platform_device with name "chip-battery.X.auto", and 
> power_supply class driver use this platform device. Why used this way? I
> write code, allocated power_supply at W1 slave (family) device register.
> This work as expected.

Historical reasons. The patch [0], which I already mentioned is the
first step to move bq27xxx w1 driver into the power-supply subsystem.

> 3. Device names
> All W1 device have unical 64 bit ID (8-bit family, 48-bit serial number,
> 8 bit CRC). W1 infrastructure show 56 bits (family-serial_nimber) as w1
> slave device name. I use this (unical) device name as power_supply battery
> name. This work. But in /sys/class/power_supply is placed machine readabe 
> subdir "26-HexDigString", instead of human-readable "chip-battery.X.auto".
> But in /sys/class/power_supply/26-HexDigString/type file still content
> human-readable type "Battery".
> Thermal zone working, but in /sys/class/thermal/thermal_zoneX/type also
> placed "26-HexDigString", instead of "chip-battery.X.auto".
> I can rename "26-HexDigString" to "battery-26-HexDigString" or 
> "26-HexDigString-battery", but this exceeded 20 chars thermal zone device
> name. So, thermal zone will have to be disabled.

I think max1721x-<serial> would be better. If the name is too long, just
disable the thermal zone for now. We have more w1/psy drivers, that have problems
due to the thermal zone device name length limitation.

[0] https://www.spinics.net/lists/kernel/msg2524566.html

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] power_supply: add max1721x_battery driver
  2017-06-08 19:17       ` Sebastian Reichel
@ 2017-06-13 16:27         ` Alex A. Mihaylov
  2017-07-03 16:36           ` Sebastian Reichel
  0 siblings, 1 reply; 16+ messages in thread
From: Alex A. Mihaylov @ 2017-06-13 16:27 UTC (permalink / raw)
  To: Sebastian Reichel, linux-kernel, linux-pm; +Cc: Alex A. Mihaylov

Maxim Integrated MAX17211/MAX17215 Standalone Fuel Gauge IC with
1-Wire interface. MAX17215 sinle cell version, MAX17211 multicell
with cell ballancing.

This driver use regmap-w1 infrastucture. Waiting for other patches
for 1-Wire. This time for review only.

Signed-off-by: Alex A. Mihaylov <minimumlaw@rambler.ru>
---
 drivers/power/supply/Kconfig            |  13 ++
 drivers/power/supply/Makefile           |   1 +
 drivers/power/supply/max1721x_battery.c | 377 ++++++++++++++++++++++++++++++++
 include/linux/power/max172xx-battery.h  | 100 +++++++++
 4 files changed, 491 insertions(+)
 create mode 100644 drivers/power/supply/max1721x_battery.c
 create mode 100644 include/linux/power/max172xx-battery.h

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index da54ac8..deacd80 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -268,6 +268,19 @@ config BATTERY_MAX17042
 	  with MAX17042. This driver also supports max17047/50 chips which are
 	  improved version of max17042.
 
+config BATTERY_MAX1721X
+	tristate "MAX17211/MAX17215 standalone gas-gauge"
+	depends on W1
+	select REGMAP_W1
+	help
+	  MAX1721x is fuel-gauge systems for lithium-ion (Li+) batteries
+	  in handheld and portable equipment. MAX17211 used with single cell
+	  battery. MAX17215 designed for muticell battery. Both them have
+	  OneWire (W1) host interface.
+
+	  Say Y here to enable support for the MAX17211/MAX17215 standalone
+	  battery gas-gauge.
+
 config BATTERY_Z2
 	tristate "Z2 battery driver"
 	depends on I2C && MACH_ZIPIT2
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 3789a2c..c785d05 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_CHARGER_DA9150)	+= da9150-charger.o
 obj-$(CONFIG_BATTERY_DA9150)	+= da9150-fg.o
 obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
 obj-$(CONFIG_BATTERY_MAX17042)	+= max17042_battery.o
+obj-$(CONFIG_BATTERY_MAX1721X)	+= max1721x_battery.o
 obj-$(CONFIG_BATTERY_Z2)	+= z2_battery.o
 obj-$(CONFIG_BATTERY_RT5033)	+= rt5033_battery.o
 obj-$(CONFIG_CHARGER_RT9455)	+= rt9455_charger.o
diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
new file mode 100644
index 0000000..d3e7435
--- /dev/null
+++ b/drivers/power/supply/max1721x_battery.c
@@ -0,0 +1,377 @@
+/*
+ * 1-wire client/driver for the Maxim Semicondactor
+ * MAX17211/MAX17215 Standalone Fuel Gauge IC
+ *
+ * Copyright (C) 2017 OAO Radioavionica
+ * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
+ *
+ * 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/regmap.h>
+#include <linux/power_supply.h>
+
+#include "../../w1/w1.h"
+#include "linux/power/max172xx-battery.h"
+
+#define DEF_DEV_NAME_MAX17211	"MAX17211"
+#define DEF_DEV_NAME_MAX17215	"MAX17215"
+#define DEF_DEV_NAME_UNKNOWN	"UNKNOWN"
+#define DEF_MFG_NAME		"MAXIM"
+
+#define PSY_MAX_NAME_LEN	32
+
+struct max17211_device_info {
+	char name[PSY_MAX_NAME_LEN];
+	struct power_supply *bat;
+	struct power_supply_desc bat_desc;
+	struct device *w1_dev;
+	struct regmap *regmap;
+	/* battery design format */
+	unsigned int 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);
+	unsigned int reg = 0;
+	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 =
+			regmap_read(info->regmap, MAX172XX_REG_STATUS,
+			&reg) ? 0 : !(reg & MAX172XX_BAT_PRESENT);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = regmap_read(info->regmap, MAX172XX_REG_REPSOC, &reg);
+		val->intval = max172xx_percent_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = regmap_read(info->regmap, MAX172XX_REG_BATT, &reg);
+		val->intval = max172xx_voltage_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		ret = regmap_read(info->regmap, MAX172XX_REG_DESIGNCAP, &reg);
+		val->intval = max172xx_capacity_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_REPCAP, &reg);
+		val->intval = max172xx_capacity_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_TTE, &reg);
+		val->intval = max172xx_time_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_TTF, &reg);
+		val->intval = max172xx_time_to_ps(reg);
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		ret = regmap_read(info->regmap, MAX172XX_REG_TEMP, &reg);
+		val->intval = max172xx_temperature_to_ps(reg);
+		break;
+	/* We need signed current, so must cast info->rsense to signed type */
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = regmap_read(info->regmap, MAX172XX_REG_CURRENT, &reg);
+		val->intval =
+			max172xx_current_to_voltage(reg) / (int)info->rsense;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REG_AVGCURRENT, &reg);
+		val->intval =
+			max172xx_current_to_voltage(reg) / (int)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 = regmap_read(info->regmap, MAX1721X_REG_DEV_STR, &reg);
+		val->strval = info->DeviceName;
+		break;
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		ret = regmap_read(info->regmap, MAX1721X_REG_MFG_STR, &reg);
+		val->strval = info->ManufacturerName;
+		break;
+	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
+		ret = regmap_read(info->regmap, 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 max17211_device_info *info,
+			uint16_t reg, uint8_t nr, char *str)
+{
+	unsigned int val;
+
+	if (!str || !(reg == MAX1721X_REG_MFG_STR ||
+			reg == MAX1721X_REG_DEV_STR))
+		return -EFAULT;
+
+	while (nr--) {
+		if (regmap_read(info->regmap, 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 max17211_device_info *info, char *str)
+{
+	unsigned int val[3];
+
+	if (!str)
+		return -EFAULT;
+
+	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX, &val[0]))
+		return -EFAULT;
+	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 1, &val[1]))
+		return -EFAULT;
+	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 2, &val[2]))
+		return -EFAULT;
+
+	snprintf(str, 13, "%04X%04X%04X", val[0], val[1], val[2]);
+	return 0;
+}
+
+/*
+ * MAX1721x registers description for w1-regmap
+ */
+static const struct regmap_range max1721x_allow_range[] = {
+	regmap_reg_range(0, 0xDF),	/* volatile data */
+	regmap_reg_range(0x180, 0x1DF),	/* non-volatile memory */
+	regmap_reg_range(0x1E0, 0x1EF),	/* non-volatile history (unused) */
+};
+
+static const struct regmap_range max1721x_deny_range[] = {
+	/* volatile data unused registers */
+	regmap_reg_range(0x24, 0x26),
+	regmap_reg_range(0x30, 0x31),
+	regmap_reg_range(0x33, 0x34),
+	regmap_reg_range(0x37, 0x37),
+	regmap_reg_range(0x3B, 0x3C),
+	regmap_reg_range(0x40, 0x41),
+	regmap_reg_range(0x43, 0x44),
+	regmap_reg_range(0x47, 0x49),
+	regmap_reg_range(0x4B, 0x4C),
+	regmap_reg_range(0x4E, 0xAF),
+	regmap_reg_range(0xB1, 0xB3),
+	regmap_reg_range(0xB5, 0xB7),
+	regmap_reg_range(0xBF, 0xD0),
+	regmap_reg_range(0xDB, 0xDB),
+	/* hole between volatile and non-volatile registers */
+	regmap_reg_range(0xE0, 0x17F),
+};
+
+static const struct regmap_access_table max1721x_regs = {
+	.yes_ranges	= max1721x_allow_range,
+	.n_yes_ranges	= ARRAY_SIZE(max1721x_allow_range),
+	.no_ranges	= max1721x_deny_range,
+	.n_no_ranges	= ARRAY_SIZE(max1721x_deny_range),
+};
+
+/*
+ * Model Gauge M5 Algorithm output register
+ * Volatile data (must not be cached)
+ */
+static const struct regmap_range max1721x_volatile_allow[] = {
+	regmap_reg_range(0, 0xDF),
+};
+
+static const struct regmap_access_table max1721x_volatile_regs = {
+	.yes_ranges	= max1721x_volatile_allow,
+	.n_yes_ranges	= ARRAY_SIZE(max1721x_volatile_allow),
+};
+
+/*
+ * W1-regmap config
+ */
+static const struct regmap_config max1721x_regmap_w1_config = {
+	.reg_bits = 16,
+	.val_bits = 16,
+	.rd_table = &max1721x_regs,
+	.volatile_table = &max1721x_volatile_regs,
+	.max_register = MAX1721X_MAX_REG_NR,
+};
+
+static int w1_max1721x_add_device(struct w1_slave *sl)
+{
+	struct power_supply_config psy_cfg = {};
+	struct max17211_device_info *info;
+
+	info = devm_kzalloc(&sl->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	sl->family_data = (void *)info;
+	info->w1_dev = &sl->dev;
+
+	/*
+	 * power_supply class battery name translated from W1 slave device
+	 * unical ID (look like 26-0123456789AB) to "max1721x-0123456789AB\0"
+	 * so, 26 (device family) correcpondent to max1721x devices.
+	 * Device name still unical for any numbers connected devices.
+	 */
+	snprintf(info->name, sizeof(info->name),
+		"max1721x-%012X", (unsigned int)sl->reg_num.id);
+	info->bat_desc.name = info->name;
+
+	/*
+	 * FixMe: battery device name exceed max len for thermal_zone device
+	 * name and translation to thermal_zone must be disabled.
+	 */
+	info->bat_desc.no_thermal = true;
+	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;
+	psy_cfg.drv_data = info;
+
+	/* regmap init */
+	info->regmap = regmap_init_w1(info->w1_dev,
+					&max1721x_regmap_w1_config);
+	if (IS_ERR(info->regmap)) {
+		int err = PTR_ERR(info->regmap);
+
+		dev_err(info->w1_dev, "Failed to allocate register map: %d\n",
+			err);
+		return err;
+	}
+
+	/* rsense init */
+	info->rsense = 0;
+	if (regmap_read(info->regmap, MAX1721X_REG_NRSENSE, &info->rsense))
+
+	if (!info->rsense) {
+		dev_warn(info->w1_dev, "RSenese not calibrated, set 10 mOhms!\n");
+		info->rsense = 1000; /* in regs in 10^-5 */
+	}
+	dev_info(info->w1_dev, "RSense: %d mOhms.\n", info->rsense / 100);
+
+	if (get_string(info, MAX1721X_REG_MFG_STR,
+			MAX1721X_REG_MFG_NUMB, info->ManufacturerName)) {
+		dev_err(info->w1_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, MAX1721X_REG_DEV_STR,
+			MAX1721X_REG_DEV_NUMB, info->DeviceName)) {
+		dev_err(info->w1_dev, "Can't read device. Hardware error.\n");
+		return -ENODEV;
+	}
+	if (!info->DeviceName[0]) {
+		unsigned int dev_name;
+
+		if (regmap_read(info->regmap,
+				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, info->SerialNumber)) {
+		dev_err(info->w1_dev, "Can't read serial. Hardware error.\n");
+		return -ENODEV;
+	}
+
+	info->bat = power_supply_register(&sl->dev, &info->bat_desc,
+						&psy_cfg);
+	if (IS_ERR(info->bat)) {
+		dev_err(info->w1_dev, "failed to register battery\n");
+		return PTR_ERR(info->bat);
+	}
+
+	return 0;
+}
+
+static void w1_max1721x_remove_device(struct w1_slave *sl)
+{
+	struct max17211_device_info *info =
+		(struct max17211_device_info *)sl->family_data;
+
+	power_supply_unregister(info->bat);
+	regmap_exit(info->regmap);
+	kfree(info);
+}
+
+static struct w1_family_ops w1_max1721x_fops = {
+	.add_slave = w1_max1721x_add_device,
+	.remove_slave = w1_max1721x_remove_device,
+};
+
+static struct w1_family w1_max1721x_family = {
+	.fid = W1_MAX1721X_FAMILY_ID,
+	.fops = &w1_max1721x_fops,
+};
+
+module_w1_family(w1_max1721x_family);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alex A. Mihaylov <minimumlaw@rambler.ru>");
+MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
+MODULE_ALIAS("w1-family-" __stringify(W1_MAX1721X_FAMILY_ID));
diff --git a/include/linux/power/max172xx-battery.h b/include/linux/power/max172xx-battery.h
new file mode 100644
index 0000000..c46ac56
--- /dev/null
+++ b/include/linux/power/max172xx-battery.h
@@ -0,0 +1,100 @@
+/*
+ * 1-Wire implementation for Maxim Semiconductor
+ * MAX7211/MAX17215 stanalone fuel gauge chip
+ *
+ * Copyright (C) 2017 OAO Radioavionica
+ * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
+ *
+ * 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_max172xx_h__
+#define __w1_max172xx_h__
+
+/* Known commands to the MAX1721X chip */
+#define W1_MAX1721X_FAMILY_ID		0x26
+
+/* 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
+
+/* MAX172XX Output Registers for 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 */
+
+/* Number of valid register addresses in W1 mode */
+#define MAX1721X_MAX_REG_NR	0x1EF
+
+/* Convert regs value to power_supply units */
+
+static inline int max172xx_time_to_ps(unsigned int reg)
+{
+	return reg * 5625 / 1000;	/* in sec. */
+}
+
+static inline int max172xx_percent_to_ps(unsigned int reg)
+{
+	return reg / 256;	/* in percent from 0 to 100 */
+}
+
+static inline int max172xx_voltage_to_ps(unsigned int reg)
+{
+	return reg * 1250;	/* in uV */
+}
+
+static inline int max172xx_capacity_to_ps(unsigned int 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(unsigned int reg)
+{
+	int val = (int16_t)(reg);
+
+	return val * 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(unsigned int reg)
+{
+	int val = (int16_t)(reg);
+
+	return val * 156252;
+}
+
+#endif /* !__w1_max172xx_h__ */
-- 
2.8.4 (Apple Git-73)

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] power_supply: add max1721x_battery driver
  2017-06-13 16:27         ` [PATCH] power_supply: add max1721x_battery driver Alex A. Mihaylov
@ 2017-07-03 16:36           ` Sebastian Reichel
  0 siblings, 0 replies; 16+ messages in thread
From: Sebastian Reichel @ 2017-07-03 16:36 UTC (permalink / raw)
  To: Alex A. Mihaylov; +Cc: linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 15970 bytes --]

Hi,

On Tue, Jun 13, 2017 at 07:27:53PM +0300, Alex A. Mihaylov wrote:
> Maxim Integrated MAX17211/MAX17215 Standalone Fuel Gauge IC with
> 1-Wire interface. MAX17215 sinle cell version, MAX17211 multicell
> with cell ballancing.
> 
> This driver use regmap-w1 infrastucture. Waiting for other patches
> for 1-Wire. This time for review only.
> 
> Signed-off-by: Alex A. Mihaylov <minimumlaw@rambler.ru>

This looks mostly fine now. I have a few more small change requests.

> ---
>  drivers/power/supply/Kconfig            |  13 ++
>  drivers/power/supply/Makefile           |   1 +
>  drivers/power/supply/max1721x_battery.c | 377 ++++++++++++++++++++++++++++++++
>  include/linux/power/max172xx-battery.h  | 100 +++++++++
>  4 files changed, 491 insertions(+)
>  create mode 100644 drivers/power/supply/max1721x_battery.c
>  create mode 100644 include/linux/power/max172xx-battery.h
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index da54ac8..deacd80 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -268,6 +268,19 @@ config BATTERY_MAX17042
>  	  with MAX17042. This driver also supports max17047/50 chips which are
>  	  improved version of max17042.
>  
> +config BATTERY_MAX1721X
> +	tristate "MAX17211/MAX17215 standalone gas-gauge"
> +	depends on W1
> +	select REGMAP_W1
> +	help
> +	  MAX1721x is fuel-gauge systems for lithium-ion (Li+) batteries
> +	  in handheld and portable equipment. MAX17211 used with single cell
> +	  battery. MAX17215 designed for muticell battery. Both them have
> +	  OneWire (W1) host interface.
> +
> +	  Say Y here to enable support for the MAX17211/MAX17215 standalone
> +	  battery gas-gauge.
> +
>  config BATTERY_Z2
>  	tristate "Z2 battery driver"
>  	depends on I2C && MACH_ZIPIT2
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 3789a2c..c785d05 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -41,6 +41,7 @@ obj-$(CONFIG_CHARGER_DA9150)	+= da9150-charger.o
>  obj-$(CONFIG_BATTERY_DA9150)	+= da9150-fg.o
>  obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
>  obj-$(CONFIG_BATTERY_MAX17042)	+= max17042_battery.o
> +obj-$(CONFIG_BATTERY_MAX1721X)	+= max1721x_battery.o
>  obj-$(CONFIG_BATTERY_Z2)	+= z2_battery.o
>  obj-$(CONFIG_BATTERY_RT5033)	+= rt5033_battery.o
>  obj-$(CONFIG_CHARGER_RT9455)	+= rt9455_charger.o
> diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
> new file mode 100644
> index 0000000..d3e7435
> --- /dev/null
> +++ b/drivers/power/supply/max1721x_battery.c
> @@ -0,0 +1,377 @@
> +/*
> + * 1-wire client/driver for the Maxim Semicondactor
> + * MAX17211/MAX17215 Standalone Fuel Gauge IC
> + *
> + * Copyright (C) 2017 OAO Radioavionica
> + * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
> + *
> + * 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/regmap.h>
> +#include <linux/power_supply.h>
> +
> +#include "../../w1/w1.h"

Please base your next version on linux-next (or v4.13-rc1, if its
out before you sent it).

> +#include "linux/power/max172xx-battery.h"
> +
> +#define DEF_DEV_NAME_MAX17211	"MAX17211"
> +#define DEF_DEV_NAME_MAX17215	"MAX17215"
> +#define DEF_DEV_NAME_UNKNOWN	"UNKNOWN"
> +#define DEF_MFG_NAME		"MAXIM"
> +
> +#define PSY_MAX_NAME_LEN	32
> +
> +struct max17211_device_info {
> +	char name[PSY_MAX_NAME_LEN];
> +	struct power_supply *bat;
> +	struct power_supply_desc bat_desc;
> +	struct device *w1_dev;
> +	struct regmap *regmap;
> +	/* battery design format */
> +	unsigned int 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);
> +	unsigned int reg = 0;
> +	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 =
> +			regmap_read(info->regmap, MAX172XX_REG_STATUS,
> +			&reg) ? 0 : !(reg & MAX172XX_BAT_PRESENT);
> +		break;
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_REPSOC, &reg);
> +		val->intval = max172xx_percent_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_BATT, &reg);
> +		val->intval = max172xx_voltage_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_DESIGNCAP, &reg);
> +		val->intval = max172xx_capacity_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_REPCAP, &reg);
> +		val->intval = max172xx_capacity_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_TTE, &reg);
> +		val->intval = max172xx_time_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_TTF, &reg);
> +		val->intval = max172xx_time_to_ps(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_TEMP:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_TEMP, &reg);
> +		val->intval = max172xx_temperature_to_ps(reg);
> +		break;
> +	/* We need signed current, so must cast info->rsense to signed type */
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_CURRENT, &reg);
> +		val->intval =
> +			max172xx_current_to_voltage(reg) / (int)info->rsense;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REG_AVGCURRENT, &reg);
> +		val->intval =
> +			max172xx_current_to_voltage(reg) / (int)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 = regmap_read(info->regmap, MAX1721X_REG_DEV_STR, &reg);
> +		val->strval = info->DeviceName;
> +		break;
> +	case POWER_SUPPLY_PROP_MANUFACTURER:
> +		ret = regmap_read(info->regmap, MAX1721X_REG_MFG_STR, &reg);
> +		val->strval = info->ManufacturerName;
> +		break;
> +	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
> +		ret = regmap_read(info->regmap, 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 max17211_device_info *info,
> +			uint16_t reg, uint8_t nr, char *str)
> +{
> +	unsigned int val;
> +
> +	if (!str || !(reg == MAX1721X_REG_MFG_STR ||
> +			reg == MAX1721X_REG_DEV_STR))
> +		return -EFAULT;
> +
> +	while (nr--) {
> +		if (regmap_read(info->regmap, 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 max17211_device_info *info, char *str)
> +{
> +	unsigned int val[3];
> +
> +	if (!str)
> +		return -EFAULT;
> +
> +	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX, &val[0]))
> +		return -EFAULT;
> +	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 1, &val[1]))
> +		return -EFAULT;
> +	if (regmap_read(info->regmap, MAX1721X_REG_SER_HEX + 2, &val[2]))
> +		return -EFAULT;
> +
> +	snprintf(str, 13, "%04X%04X%04X", val[0], val[1], val[2]);
> +	return 0;
> +}
> +
> +/*
> + * MAX1721x registers description for w1-regmap
> + */
> +static const struct regmap_range max1721x_allow_range[] = {
> +	regmap_reg_range(0, 0xDF),	/* volatile data */
> +	regmap_reg_range(0x180, 0x1DF),	/* non-volatile memory */
> +	regmap_reg_range(0x1E0, 0x1EF),	/* non-volatile history (unused) */
> +};
> +
> +static const struct regmap_range max1721x_deny_range[] = {
> +	/* volatile data unused registers */
> +	regmap_reg_range(0x24, 0x26),
> +	regmap_reg_range(0x30, 0x31),
> +	regmap_reg_range(0x33, 0x34),
> +	regmap_reg_range(0x37, 0x37),
> +	regmap_reg_range(0x3B, 0x3C),
> +	regmap_reg_range(0x40, 0x41),
> +	regmap_reg_range(0x43, 0x44),
> +	regmap_reg_range(0x47, 0x49),
> +	regmap_reg_range(0x4B, 0x4C),
> +	regmap_reg_range(0x4E, 0xAF),
> +	regmap_reg_range(0xB1, 0xB3),
> +	regmap_reg_range(0xB5, 0xB7),
> +	regmap_reg_range(0xBF, 0xD0),
> +	regmap_reg_range(0xDB, 0xDB),
> +	/* hole between volatile and non-volatile registers */
> +	regmap_reg_range(0xE0, 0x17F),
> +};
> +
> +static const struct regmap_access_table max1721x_regs = {
> +	.yes_ranges	= max1721x_allow_range,
> +	.n_yes_ranges	= ARRAY_SIZE(max1721x_allow_range),
> +	.no_ranges	= max1721x_deny_range,
> +	.n_no_ranges	= ARRAY_SIZE(max1721x_deny_range),
> +};
> +
> +/*
> + * Model Gauge M5 Algorithm output register
> + * Volatile data (must not be cached)
> + */
> +static const struct regmap_range max1721x_volatile_allow[] = {
> +	regmap_reg_range(0, 0xDF),
> +};
> +
> +static const struct regmap_access_table max1721x_volatile_regs = {
> +	.yes_ranges	= max1721x_volatile_allow,
> +	.n_yes_ranges	= ARRAY_SIZE(max1721x_volatile_allow),
> +};
> +
> +/*
> + * W1-regmap config
> + */
> +static const struct regmap_config max1721x_regmap_w1_config = {
> +	.reg_bits = 16,
> +	.val_bits = 16,
> +	.rd_table = &max1721x_regs,
> +	.volatile_table = &max1721x_volatile_regs,
> +	.max_register = MAX1721X_MAX_REG_NR,
> +};
> +
> +static int w1_max1721x_add_device(struct w1_slave *sl)
> +{
> +	struct power_supply_config psy_cfg = {};
> +	struct max17211_device_info *info;
> +
> +	info = devm_kzalloc(&sl->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	sl->family_data = (void *)info;
> +	info->w1_dev = &sl->dev;
> +
> +	/*
> +	 * power_supply class battery name translated from W1 slave device
> +	 * unical ID (look like 26-0123456789AB) to "max1721x-0123456789AB\0"
> +	 * so, 26 (device family) correcpondent to max1721x devices.
> +	 * Device name still unical for any numbers connected devices.
> +	 */
> +	snprintf(info->name, sizeof(info->name),
> +		"max1721x-%012X", (unsigned int)sl->reg_num.id);
> +	info->bat_desc.name = info->name;
> +
> +	/*
> +	 * FixMe: battery device name exceed max len for thermal_zone device
> +	 * name and translation to thermal_zone must be disabled.
> +	 */
> +	info->bat_desc.no_thermal = true;
> +	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;
> +	psy_cfg.drv_data = info;
> +
> +	/* regmap init */
> +	info->regmap = regmap_init_w1(info->w1_dev,
> +					&max1721x_regmap_w1_config);
> +	if (IS_ERR(info->regmap)) {
> +		int err = PTR_ERR(info->regmap);
> +
> +		dev_err(info->w1_dev, "Failed to allocate register map: %d\n",
> +			err);
> +		return err;
> +	}

You also added devm_regmap_init_w1(), so use it! :)

> +	/* rsense init */
> +	info->rsense = 0;
> +	if (regmap_read(info->regmap, MAX1721X_REG_NRSENSE, &info->rsense))
> +
> +	if (!info->rsense) {
> +		dev_warn(info->w1_dev, "RSenese not calibrated, set 10 mOhms!\n");
> +		info->rsense = 1000; /* in regs in 10^-5 */
> +	}
> +	dev_info(info->w1_dev, "RSense: %d mOhms.\n", info->rsense / 100);
> +
> +	if (get_string(info, MAX1721X_REG_MFG_STR,
> +			MAX1721X_REG_MFG_NUMB, info->ManufacturerName)) {
> +		dev_err(info->w1_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, MAX1721X_REG_DEV_STR,
> +			MAX1721X_REG_DEV_NUMB, info->DeviceName)) {
> +		dev_err(info->w1_dev, "Can't read device. Hardware error.\n");
> +		return -ENODEV;
> +	}
> +	if (!info->DeviceName[0]) {
> +		unsigned int dev_name;
> +
> +		if (regmap_read(info->regmap,
> +				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, info->SerialNumber)) {
> +		dev_err(info->w1_dev, "Can't read serial. Hardware error.\n");
> +		return -ENODEV;
> +	}
> +
> +	info->bat = power_supply_register(&sl->dev, &info->bat_desc,
> +						&psy_cfg);
> +	if (IS_ERR(info->bat)) {
> +		dev_err(info->w1_dev, "failed to register battery\n");
> +		return PTR_ERR(info->bat);
> +	}

There is devm_power_supply_register, that can be used once
you use devm_regmap_init_w1().

> +	return 0;
> +}
> +
> +static void w1_max1721x_remove_device(struct w1_slave *sl)
> +{
> +	struct max17211_device_info *info =
> +		(struct max17211_device_info *)sl->family_data;
> +
> +	power_supply_unregister(info->bat);
> +	regmap_exit(info->regmap);

Both of these can go once you use the devm_ based register
in probe().

> +	kfree(info);

This free is wrong. info will be free'd automatically, since
it has been requested with devm.

> +}
> +
> +static struct w1_family_ops w1_max1721x_fops = {
> +	.add_slave = w1_max1721x_add_device,
> +	.remove_slave = w1_max1721x_remove_device,

The remove function can be dropped completly once the devm_
conversion happened.

> +};
> +
> +static struct w1_family w1_max1721x_family = {
> +	.fid = W1_MAX1721X_FAMILY_ID,
> +	.fops = &w1_max1721x_fops,
> +};
> +
> +module_w1_family(w1_max1721x_family);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alex A. Mihaylov <minimumlaw@rambler.ru>");
> +MODULE_DESCRIPTION("Maxim MAX17211/MAX17215 Fuel Gauage IC driver");
> +MODULE_ALIAS("w1-family-" __stringify(W1_MAX1721X_FAMILY_ID));
> diff --git a/include/linux/power/max172xx-battery.h b/include/linux/power/max172xx-battery.h
> new file mode 100644
> index 0000000..c46ac56
> --- /dev/null
> +++ b/include/linux/power/max172xx-battery.h
> [...]

Move the whole content of the header into the main driver file.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] regmap: Add 1-Wire bus support
  2017-05-30 17:57 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
@ 2017-05-31 18:28   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2017-05-31 18:28 UTC (permalink / raw)
  To: Alex A. Mihaylov
  Cc: Greg Kroah-Hartman, Sebastian Reichel, Evgeniy Polyakov,
	linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

On Tue, May 30, 2017 at 08:57:33PM +0300, Alex A. Mihaylov wrote:

> +	if (!w1_reset_select_slave(sl)) {
> +		w1_write_8(sl->master, W1_CMD_READ_DATA);
> +		w1_write_8(sl->master, reg);
> +		*val = w1_read_8(sl->master);
> +	} else
> +		ret = -ENODEV;

CodingStyle says { } on both sides of an if statement if they're on one.
Otherwise this looks good.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] regmap: Add 1-Wire bus support
  2017-05-30 17:57 [PATCH 0/3] Battery monitor MAX1721x with w1-regmap and w1-slave (resend) Alex A. Mihaylov
@ 2017-05-30 17:57 ` Alex A. Mihaylov
  2017-05-31 18:28   ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Alex A. Mihaylov @ 2017-05-30 17:57 UTC (permalink / raw)
  To: Mark Brown, Greg Kroah-Hartman, Sebastian Reichel,
	Evgeniy Polyakov, linux-kernel, linux-pm
  Cc: Alex A. Mihaylov

Add basic support regmap (register map access) API for 1-Wire bus

Signed-off-by: Alex A. Mihaylov <minimumlaw@rambler.ru>
---
 drivers/base/regmap/Kconfig     |   6 +-
 drivers/base/regmap/Makefile    |   1 +
 drivers/base/regmap/regmap-w1.c | 239 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h          |  34 ++++++
 4 files changed, 279 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-w1.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index db9d00c..413af5f 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -3,7 +3,7 @@
 # subsystems should select the appropriate symbols.
 
 config REGMAP
-	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
+	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 	select IRQ_DOMAIN if REGMAP_IRQ
@@ -24,6 +24,10 @@ config REGMAP_SPMI
 	tristate
 	depends on SPMI
 
+config REGMAP_W1
+	tristate
+	depends on W1
+
 config REGMAP_MMIO
 	tristate
 
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 609e4c8..17741ae 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
 obj-$(CONFIG_REGMAP_SPMI) += regmap-spmi.o
 obj-$(CONFIG_REGMAP_MMIO) += regmap-mmio.o
 obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
+obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
diff --git a/drivers/base/regmap/regmap-w1.c b/drivers/base/regmap/regmap-w1.c
new file mode 100644
index 0000000..84abe13
--- /dev/null
+++ b/drivers/base/regmap/regmap-w1.c
@@ -0,0 +1,239 @@
+/*
+ * Register map access API - W1 (1-Wire) support
+ *
+ * Copyright (C) 2017 OAO Radioavionica
+ * Author: Alex A. Mihaylov <minimumlaw@rambler.ru>
+ *
+ * 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/regmap.h>
+#include <linux/module.h>
+#include "../../w1/w1.h"
+
+#include "internal.h"
+
+#define W1_CMD_READ_DATA	0x69
+#define W1_CMD_WRITE_DATA	0x6C
+
+/*
+ * 1-Wire slaves registers with addess 8 bit and data 8 bit
+ */
+
+static int w1_reg_a8_v8_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_READ_DATA);
+		w1_write_8(sl->master, reg);
+		*val = w1_read_8(sl->master);
+	} else
+		ret = -ENODEV;
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+static int w1_reg_a8_v8_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret=0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_WRITE_DATA);
+		w1_write_8(sl->master, reg);
+		w1_write_8(sl->master, val);
+	} else
+		ret = -ENODEV;
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+/*
+ * 1-Wire slaves registers with addess 8 bit and data 16 bit
+ */
+
+static int w1_reg_a8_v16_read(void *context, unsigned int reg,
+				unsigned int *val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_READ_DATA);
+		w1_write_8(sl->master, reg);
+		*val = w1_read_8(sl->master);
+		*val |= w1_read_8(sl->master)<<8;
+	} else
+		ret = -ENODEV;
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+static int w1_reg_a8_v16_write(void *context, unsigned int reg,
+				unsigned int val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 255)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_WRITE_DATA);
+		w1_write_8(sl->master, reg);
+		w1_write_8(sl->master, val & 0x00FF);
+		w1_write_8(sl->master, val>>8 & 0x00FF);
+	} else
+		ret = -ENODEV;
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+/*
+ * 1-Wire slaves registers with addess 16 bit and data 16 bit
+ */
+
+static int w1_reg_a16_v16_read(void *context, unsigned int reg,
+				unsigned int *val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 65535)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_READ_DATA);
+		w1_write_8(sl->master, reg & 0x00FF);
+		w1_write_8(sl->master, reg>>8 & 0x00FF);
+		*val = w1_read_8(sl->master);
+		*val |= w1_read_8(sl->master)<<8;
+	} else
+		ret = -ENODEV;
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+static int w1_reg_a16_v16_write(void *context, unsigned int reg,
+				unsigned int val)
+{
+	struct device *dev = context;
+	struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int ret = 0;
+
+	if (reg > 65535)
+		return -EINVAL;
+
+	mutex_lock(&sl->master->bus_mutex);
+	if (!w1_reset_select_slave(sl)) {
+		w1_write_8(sl->master, W1_CMD_WRITE_DATA);
+		w1_write_8(sl->master, reg & 0x00FF);
+		w1_write_8(sl->master, reg>>8 & 0x00FF);
+		w1_write_8(sl->master, val & 0x00FF);
+		w1_write_8(sl->master, val>>8 & 0x00FF);
+	} else
+		ret = -ENODEV;
+	mutex_unlock(&sl->master->bus_mutex);
+
+	return ret;
+}
+
+/*
+ * Various types of supported bus addressing types
+ */
+
+static struct regmap_bus regmap_w1_bus_a8_v8 = {
+	.reg_read = w1_reg_a8_v8_read,
+	.reg_write = w1_reg_a8_v8_write,
+};
+
+static struct regmap_bus regmap_w1_bus_a8_v16 = {
+	.reg_read = w1_reg_a8_v16_read,
+	.reg_write = w1_reg_a8_v16_write,
+};
+
+static struct regmap_bus regmap_w1_bus_a16_v16 = {
+	.reg_read = w1_reg_a16_v16_read,
+	.reg_write = w1_reg_a16_v16_write,
+};
+
+static const struct regmap_bus *regmap_get_w1_bus(struct device *w1_dev,
+					const struct regmap_config *config)
+{
+	if (config->reg_bits == 8 && config->val_bits == 8)
+		return &regmap_w1_bus_a8_v8;
+
+	if (config->reg_bits == 8 && config->val_bits == 16)
+		return &regmap_w1_bus_a8_v16;
+
+	if (config->reg_bits == 16 && config->val_bits == 16)
+		return &regmap_w1_bus_a16_v16;
+
+	return ERR_PTR(-ENOTSUPP);
+}
+
+struct regmap *__regmap_init_w1(struct device *w1_dev,
+				 const struct regmap_config *config,
+				 struct lock_class_key *lock_key,
+				 const char *lock_name)
+{
+
+	const struct regmap_bus *bus = regmap_get_w1_bus(w1_dev, config);
+
+	if (IS_ERR(bus))
+		return ERR_CAST(bus);
+
+	return __regmap_init(w1_dev, bus, w1_dev, config,
+			 lock_key, lock_name);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(__regmap_init_w1);
+
+struct regmap *__devm_regmap_init_w1(struct device *w1_dev,
+				 const struct regmap_config *config,
+				 struct lock_class_key *lock_key,
+				 const char *lock_name)
+{
+
+	const struct regmap_bus *bus = regmap_get_w1_bus(w1_dev, config);
+
+	if (IS_ERR(bus))
+		return ERR_CAST(bus);
+
+	return __devm_regmap_init(w1_dev, bus, w1_dev, config,
+				 lock_key, lock_name);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_w1);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index e886492..86eeacc 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -461,6 +461,10 @@ struct regmap *__regmap_init_spmi_ext(struct spmi_device *dev,
 				      const struct regmap_config *config,
 				      struct lock_class_key *lock_key,
 				      const char *lock_name);
+struct regmap *__regmap_init_w1(struct device *w1_dev,
+				 const struct regmap_config *config,
+				 struct lock_class_key *lock_key,
+				 const char *lock_name);
 struct regmap *__regmap_init_mmio_clk(struct device *dev, const char *clk_id,
 				      void __iomem *regs,
 				      const struct regmap_config *config,
@@ -493,6 +497,10 @@ struct regmap *__devm_regmap_init_spmi_ext(struct spmi_device *dev,
 					   const struct regmap_config *config,
 					   struct lock_class_key *lock_key,
 					   const char *lock_name);
+struct regmap *__devm_regmap_init_w1(struct device *w1_dev,
+				      const struct regmap_config *config,
+				      struct lock_class_key *lock_key,
+				      const char *lock_name);
 struct regmap *__devm_regmap_init_mmio_clk(struct device *dev,
 					   const char *clk_id,
 					   void __iomem *regs,
@@ -597,6 +605,19 @@ int regmap_attach_dev(struct device *dev, struct regmap *map,
 				dev, config)
 
 /**
+ * regmap_init_w1() - Initialise register map
+ *
+ * @w1_dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer to
+ * a struct regmap.
+ */
+#define regmap_init_w1(w1_dev, config)					\
+	__regmap_lockdep_wrapper(__regmap_init_w1, #config,		\
+				w1_dev, config)
+
+/**
  * regmap_init_mmio_clk() - Initialise register map with register clock
  *
  * @dev: Device that will be interacted with
@@ -712,6 +733,19 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
 				dev, config)
 
 /**
+ * devm_regmap_init_w1() - Initialise managed register map
+ *
+ * @w1_dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.  The regmap will be automatically freed by the
+ * device management code.
+ */
+#define devm_regmap_init_w1(w1_dev, config)				\
+	__regmap_lockdep_wrapper(__devm_regmap_init_w1, #config,	\
+				w1_dev, config)
+/**
  * devm_regmap_init_mmio_clk() - Initialise managed register map with clock
  *
  * @dev: Device that will be interacted with
-- 
2.8.4 (Apple Git-73)

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-07-03 16:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-02  7:06 [PATCH 0/3] Battery monitor MAX1721x with w1-regmap and w1-slave (resend) Alex A. Mihaylov
2017-06-02  7:06 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
2017-06-06 18:50   ` Mark Brown
2017-06-08 12:53   ` Sebastian Reichel
2017-06-08 12:57     ` Mark Brown
2017-06-08 13:13       ` Sebastian Reichel
2017-06-02  7:06 ` [PATCH 2/3] w1: MAX1721x Stanalone Fuel Gauge - add 1-Wire slave drivers Alex A. Mihaylov
2017-06-08 12:27   ` Sebastian Reichel
2017-06-02  7:06 ` [PATCH 3/3] power: supply: Add support MAX1721x battery monitor Alex A. Mihaylov
2017-06-08 12:48   ` Sebastian Reichel
2017-06-08 17:44     ` Alex A. Mihaylov
2017-06-08 19:17       ` Sebastian Reichel
2017-06-13 16:27         ` [PATCH] power_supply: add max1721x_battery driver Alex A. Mihaylov
2017-07-03 16:36           ` Sebastian Reichel
  -- strict thread matches above, loose matches on Subject: below --
2017-05-30 17:57 [PATCH 0/3] Battery monitor MAX1721x with w1-regmap and w1-slave (resend) Alex A. Mihaylov
2017-05-30 17:57 ` [PATCH 1/3] regmap: Add 1-Wire bus support Alex A. Mihaylov
2017-05-31 18:28   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.