[PATCH RESEND v2 0/3] MM8013 fg driver

[PATCH RESEND v2 0/3] MM8013 fg driver

[Konrad Dybcio]

Resending due to no responses for 2 months.

This series brings support for the Mitsumi MM8013 Li-Ion fuel gauge.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Changes in v2:
- Fix typo in patch 2 commit message
- Drop driver.owner in patch 3
- Add PRESENT psy property
- Pick up tags
- Link to v1: https://lore.kernel.org/r/20230621-topic-mm8013-v1-0-4407c6260053@linaro.org

---
Konrad Dybcio (3):
      dt-bindings: vendor-prefixes: Add Mitsumi Electric Co., Ltd.
      dt-bindings: power: supply: Document Mitsumi MM8013 fuel gauge
      power: supply: Introduce MM8013 fuel gauge driver

 .../bindings/power/supply/mitsumi,mm8013.yaml      |  35 +++
 .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
 MAINTAINERS                                        |   5 +
 drivers/power/supply/Kconfig                       |   9 +
 drivers/power/supply/Makefile                      |   1 +
 drivers/power/supply/mm8013.c                      | 283 +++++++++++++++++++++
 6 files changed, 335 insertions(+)
---
base-commit: 60e7c4a25da68cd826719b685babbd23e73b85b0
change-id: 20230621-topic-mm8013-376759e98b28

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>

[PATCH RESEND v2 1/3] dt-bindings: vendor-prefixes: Add Mitsumi Electric Co., Ltd.

[Konrad Dybcio]

Mitsumi was [1] a Japanese company making various electronics devices
and components. They've merged into a new entity, MinebeaMitsumi in 2017.

Their current main page is available at [2]

[1] https://en.wikipedia.org/wiki/Mitsumi_Electric
[2] https://product.minebeamitsumi.com/en/

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index af60bf1a6664..6142c40cf06e 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -865,6 +865,8 @@ patternProperties:
     description: MiraMEMS Sensing Technology Co., Ltd.
   "^mitsubishi,.*":
     description: Mitsubishi Electric Corporation
+  "^mitsumi,.*":
+    description: Mitsumi Electric Co., Ltd.
   "^mixel,.*":
     description: Mixel, Inc.
   "^miyoo,.*":

-- 
2.42.0

[PATCH RESEND v2 2/3] dt-bindings: power: supply: Document Mitsumi MM8013 fuel gauge

[Konrad Dybcio]

The Mitsumi MM8013 is an I2C fuel gauge for Li-Ion cells. The partial
datasheet is available at [1]. Add bindings for this chip.

[1] https://www.mitsumi.co.jp/latest-M/Catalog/pdf/battery_mm_8013_e.pdf

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 .../bindings/power/supply/mitsumi,mm8013.yaml      | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml b/Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml
new file mode 100644
index 000000000000..080fd44083ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/mitsumi,mm8013.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mitsumi MM8013 fuel gauge
+
+maintainers:
+  - Konrad Dybcio <konradybcio@kernel.org>
+
+properties:
+  compatible:
+    const: mitsumi,mm8013
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      fuel-gauge@55 {
+        compatible = "mitsumi,mm8013";
+        reg = <0x55>;
+      };
+    };

-- 
2.42.0

Re: [PATCH RESEND v2 2/3] dt-bindings: power: supply: Document Mitsumi MM8013 fuel gauge

[Sebastian Reichel]

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

Hi,

On Wed, Aug 23, 2023 at 04:36:14PM +0200, Konrad Dybcio wrote:
> The Mitsumi MM8013 is an I2C fuel gauge for Li-Ion cells. The partial
> datasheet is available at [1]. Add bindings for this chip.
> 
> [1] https://www.mitsumi.co.jp/latest-M/Catalog/pdf/battery_mm_8013_e.pdf
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  .../bindings/power/supply/mitsumi,mm8013.yaml      | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml b/Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml
> new file mode 100644
> index 000000000000..080fd44083ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/mitsumi,mm8013.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mitsumi MM8013 fuel gauge
> +
> +maintainers:
> +  - Konrad Dybcio <konradybcio@kernel.org>
> +

allOf:
  - $ref: power-supply.yaml#

-- Sebastian

> +properties:
> +  compatible:
> +    const: mitsumi,mm8013
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      fuel-gauge@55 {
> +        compatible = "mitsumi,mm8013";
> +        reg = <0x55>;
> +      };
> +    };
> 
> -- 
> 2.42.0
> 

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

[PATCH RESEND v2 3/3] power: supply: Introduce MM8013 fuel gauge driver

[Konrad Dybcio]

Add a driver for the Mitsumi MM8013 fuel gauge. The driver is a vastly
cleaned up and improved version of the one that shipped in some obscure
Lenovo downstream kernel [1], with some register definitions borrowed from
ChromeOS EC platform code [2].

[1] https://github.com/adazem009/kernel_lenovo_bengal/commit/b6b346427a871715709bd22aae449b9383f3b66b
[2] https://chromium.googlesource.com/chromiumos/platform/ec/+/master/driver/battery/mm8013.h
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 MAINTAINERS                   |   5 +
 drivers/power/supply/Kconfig  |   9 ++
 drivers/power/supply/Makefile |   1 +
 drivers/power/supply/mm8013.c | 283 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 298 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5ce188b58eaa..ba5f075a2ca8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14296,6 +14296,11 @@ W:	https://linuxtv.org
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/radio/radio-miropcm20*
 
+MITSUMI MM8013 FG DRIVER
+M:	Konrad Dybcio <konradybcio@kernel.org>
+F:	Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml
+F:	drivers/power/supply/mm8013.c
+
 MMP SUPPORT
 R:	Lubomir Rintel <lkundrak@v3.sk>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 663a1c423806..c19e8287d80f 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -951,4 +951,13 @@ config CHARGER_QCOM_SMB2
 	  adds support for the SMB2 switch mode battery charger found
 	  in PMI8998 and related PMICs.
 
+config FUEL_GAUGE_MM8013
+	tristate "Mitsumi MM8013 fuel gauge driver"
+	depends on I2C
+	help
+	  Say Y here to enable the Mitsumi MM8013 fuel gauge driver.
+	  It enables the monitoring of many battery parameters, including
+	  the state of charge, temperature, cycle count, actual and design
+	  capacity, etc.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index a8a9fa6de1e9..ba2c41f060be 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -111,3 +111,4 @@ obj-$(CONFIG_BATTERY_SURFACE)	+= surface_battery.o
 obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
 obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
 obj-$(CONFIG_CHARGER_QCOM_SMB2)	+= qcom_pmi8998_charger.o
+obj-$(CONFIG_FUEL_GAUGE_MM8013)	+= mm8013.o
diff --git a/drivers/power/supply/mm8013.c b/drivers/power/supply/mm8013.c
new file mode 100644
index 000000000000..ce20c6078116
--- /dev/null
+++ b/drivers/power/supply/mm8013.c
@@ -0,0 +1,283 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2019 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023, Linaro Limited
+ */
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/power_supply.h>
+
+#define REG_BATID			0x00 /* This one is very unclear */
+ #define BATID_101			0x0101 /* 107kOhm */
+ #define BATID_102			0x0102 /* 10kOhm */
+#define REG_TEMPERATURE			0x06
+#define REG_VOLTAGE			0x08
+#define REG_FLAGS			0x0a
+ #define MM8013_FLAG_OTC		BIT(15)
+ #define MM8013_FLAG_OTD		BIT(14)
+ #define MM8013_FLAG_BATHI		BIT(13)
+ #define MM8013_FLAG_FC			BIT(9)
+ #define MM8013_FLAG_CHG		BIT(8)
+ #define MM8013_FLAG_DSG		BIT(0)
+#define REG_FULL_CHARGE_CAPACITY	0x0e
+#define REG_AVERAGE_CURRENT		0x14
+#define REG_AVERAGE_TIME_TO_EMPTY	0x16
+#define REG_AVERAGE_TIME_TO_FULL	0x18
+#define REG_CYCLE_COUNT			0x2a
+#define REG_STATE_OF_CHARGE		0x2c
+#define REG_DESIGN_CAPACITY		0x3c
+/* TODO: 0x62-0x68 seem to contain 'MM8013C' in a length-prefixed, non-terminated string */
+
+#define DECIKELVIN_TO_DECIDEGC(t)	(t - 2731)
+
+struct mm8013_chip {
+	struct i2c_client *client;
+};
+
+static int mm8013_write_reg(struct i2c_client *client, u8 reg, u16 value)
+{
+	int ret;
+
+	ret = i2c_smbus_write_word_data(client, reg, value);
+	if (ret < 0)
+		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
+
+	usleep_range(4000, 5000);
+	return ret;
+}
+
+static int mm8013_read_reg(struct i2c_client *client, u8 reg)
+{
+	int ret;
+
+	ret = i2c_smbus_read_word_data(client, reg);
+	if (ret < 0)
+		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
+
+	usleep_range(4000, 5000);
+	return ret;
+}
+
+static int mm8013_checkdevice(struct mm8013_chip *chip)
+{
+	int battery_id, ret;
+
+	ret = mm8013_write_reg(chip->client, REG_BATID, 0x0008);
+	if (ret < 0)
+		return ret;
+
+	ret = mm8013_read_reg(chip->client, REG_BATID);
+	if (ret < 0)
+		return ret;
+
+	if (ret == BATID_102)
+		battery_id = 2;
+	else if (ret == BATID_101)
+		battery_id = 1;
+	else
+		return -EINVAL;
+
+	dev_dbg(&chip->client->dev, "battery_id: %d\n", battery_id);
+
+	return 0;
+}
+
+static enum power_supply_property mm8013_battery_props[] = {
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
+	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+};
+
+static int mm8013_get_property(struct power_supply *psy,
+			       enum power_supply_property psp,
+			       union power_supply_propval *val)
+{
+	struct mm8013_chip *chip = psy->drv_data;
+	struct i2c_client *client = chip->client;
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = mm8013_read_reg(client, REG_STATE_OF_CHARGE);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
+		ret = mm8013_read_reg(client, REG_FULL_CHARGE_CAPACITY);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		ret = mm8013_read_reg(client, REG_DESIGN_CAPACITY);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = mm8013_read_reg(client, REG_AVERAGE_CURRENT);
+		if (ret < 0)
+			return ret;
+
+		if (ret > S16_MAX)
+			val->intval -= (1 << 16);
+		else
+			val->intval = ret;
+
+		val->intval *= -1000;
+		break;
+	case POWER_SUPPLY_PROP_CYCLE_COUNT:
+		ret = mm8013_read_reg(client, REG_CYCLE_COUNT);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		ret = mm8013_read_reg(client, REG_FLAGS);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MM8013_FLAG_BATHI)
+			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		else if (ret & (MM8013_FLAG_OTD | MM8013_FLAG_OTC))
+			val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+		else
+			val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = mm8013_read_reg(client, REG_TEMPERATURE) > 0;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = mm8013_read_reg(client, REG_FLAGS);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MM8013_FLAG_DSG)
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		else if (ret & MM8013_FLAG_CHG)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else if (ret & MM8013_FLAG_FC)
+			val->intval = POWER_SUPPLY_STATUS_FULL;
+		else
+			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		ret = mm8013_read_reg(client, REG_TEMPERATURE);
+		if (ret < 0)
+			return ret;
+
+		val->intval = DECIKELVIN_TO_DECIDEGC(ret);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
+		ret = mm8013_read_reg(client, REG_AVERAGE_TIME_TO_EMPTY);
+		if (ret < 0)
+			return ret;
+
+		/* The estimation is not yet ready */
+		if (ret == U16_MAX)
+			return -ENODATA;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
+		ret = mm8013_read_reg(client, REG_AVERAGE_TIME_TO_FULL);
+		if (ret < 0)
+			return ret;
+
+		/* The estimation is not yet ready */
+		if (ret == U16_MAX)
+			return -ENODATA;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = mm8013_read_reg(client, REG_VOLTAGE);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct power_supply_desc mm8013_desc = {
+	.name			= "mm8013",
+	.type			= POWER_SUPPLY_TYPE_BATTERY,
+	.properties		= mm8013_battery_props,
+	.num_properties		= ARRAY_SIZE(mm8013_battery_props),
+	.get_property		= mm8013_get_property,
+};
+
+static int mm8013_probe(struct i2c_client *client)
+{
+	struct power_supply_config psy_cfg = {};
+	struct device *dev = &client->dev;
+	struct power_supply *psy;
+	struct mm8013_chip *chip;
+	int ret = 0;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
+		return dev_err_probe(dev, -EIO,
+				     "I2C_FUNC_SMBUS_WORD_DATA not supported\n");
+
+	chip = devm_kzalloc(dev, sizeof(struct mm8013_chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->client = client;
+
+	ret = mm8013_checkdevice(chip);
+	if (ret)
+		return dev_err_probe(dev, -ENODEV, "MM8013 not found\n");
+
+	psy_cfg.drv_data = chip;
+	psy_cfg.of_node = dev->of_node;
+
+	psy = devm_power_supply_register(dev, &mm8013_desc, &psy_cfg);
+	if (IS_ERR(psy))
+		return PTR_ERR(psy);
+
+	return 0;
+}
+
+static const struct i2c_device_id mm8013_id_table[] = {
+	{ "mm8013", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, mm8013_id_table);
+
+static const struct of_device_id mm8013_match_table[] = {
+	{ .compatible = "mitsumi,mm8013" },
+	{ },
+};
+
+static struct i2c_driver mm8013_i2c_driver = {
+	.probe = mm8013_probe,
+	.id_table = mm8013_id_table,
+	.driver = {
+		.name = "mm8013",
+		.of_match_table = mm8013_match_table,
+	},
+};
+module_i2c_driver(mm8013_i2c_driver);
+
+MODULE_DESCRIPTION("MM8013 fuel gauge driver");
+MODULE_LICENSE("GPL");

-- 
2.42.0

Re: [PATCH RESEND v2 3/3] power: supply: Introduce MM8013 fuel gauge driver

[Sebastian Reichel]

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

Hi,

On Wed, Aug 23, 2023 at 04:36:15PM +0200, Konrad Dybcio wrote:
> Add a driver for the Mitsumi MM8013 fuel gauge. The driver is a vastly
> cleaned up and improved version of the one that shipped in some obscure
> Lenovo downstream kernel [1], with some register definitions borrowed from
> ChromeOS EC platform code [2].
> 
> [1] https://github.com/adazem009/kernel_lenovo_bengal/commit/b6b346427a871715709bd22aae449b9383f3b66b
> [2] https://chromium.googlesource.com/chromiumos/platform/ec/+/master/driver/battery/mm8013.h
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  MAINTAINERS                   |   5 +
>  drivers/power/supply/Kconfig  |   9 ++
>  drivers/power/supply/Makefile |   1 +
>  drivers/power/supply/mm8013.c | 283 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 298 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5ce188b58eaa..ba5f075a2ca8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14296,6 +14296,11 @@ W:	https://linuxtv.org
>  T:	git git://linuxtv.org/media_tree.git
>  F:	drivers/media/radio/radio-miropcm20*
>  
> +MITSUMI MM8013 FG DRIVER
> +M:	Konrad Dybcio <konradybcio@kernel.org>
> +F:	Documentation/devicetree/bindings/power/supply/mitsumi,mm8013.yaml
> +F:	drivers/power/supply/mm8013.c
> +
>  MMP SUPPORT
>  R:	Lubomir Rintel <lkundrak@v3.sk>
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 663a1c423806..c19e8287d80f 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -951,4 +951,13 @@ config CHARGER_QCOM_SMB2
>  	  adds support for the SMB2 switch mode battery charger found
>  	  in PMI8998 and related PMICs.
>  
> +config FUEL_GAUGE_MM8013
> +	tristate "Mitsumi MM8013 fuel gauge driver"
> +	depends on I2C
> +	help
> +	  Say Y here to enable the Mitsumi MM8013 fuel gauge driver.
> +	  It enables the monitoring of many battery parameters, including
> +	  the state of charge, temperature, cycle count, actual and design
> +	  capacity, etc.
> +
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index a8a9fa6de1e9..ba2c41f060be 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -111,3 +111,4 @@ obj-$(CONFIG_BATTERY_SURFACE)	+= surface_battery.o
>  obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
>  obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
>  obj-$(CONFIG_CHARGER_QCOM_SMB2)	+= qcom_pmi8998_charger.o
> +obj-$(CONFIG_FUEL_GAUGE_MM8013)	+= mm8013.o
> diff --git a/drivers/power/supply/mm8013.c b/drivers/power/supply/mm8013.c
> new file mode 100644
> index 000000000000..ce20c6078116
> --- /dev/null
> +++ b/drivers/power/supply/mm8013.c
> @@ -0,0 +1,283 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2016-2019 The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023, Linaro Limited
> + */
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/power_supply.h>
> +
> +#define REG_BATID			0x00 /* This one is very unclear */
> + #define BATID_101			0x0101 /* 107kOhm */
> + #define BATID_102			0x0102 /* 10kOhm */
> +#define REG_TEMPERATURE			0x06
> +#define REG_VOLTAGE			0x08
> +#define REG_FLAGS			0x0a
> + #define MM8013_FLAG_OTC		BIT(15)
> + #define MM8013_FLAG_OTD		BIT(14)
> + #define MM8013_FLAG_BATHI		BIT(13)
> + #define MM8013_FLAG_FC			BIT(9)
> + #define MM8013_FLAG_CHG		BIT(8)
> + #define MM8013_FLAG_DSG		BIT(0)
> +#define REG_FULL_CHARGE_CAPACITY	0x0e
> +#define REG_AVERAGE_CURRENT		0x14
> +#define REG_AVERAGE_TIME_TO_EMPTY	0x16
> +#define REG_AVERAGE_TIME_TO_FULL	0x18
> +#define REG_CYCLE_COUNT			0x2a
> +#define REG_STATE_OF_CHARGE		0x2c
> +#define REG_DESIGN_CAPACITY		0x3c
> +/* TODO: 0x62-0x68 seem to contain 'MM8013C' in a length-prefixed, non-terminated string */
> +
> +#define DECIKELVIN_TO_DECIDEGC(t)	(t - 2731)
> +
> +struct mm8013_chip {
> +	struct i2c_client *client;
> +};
> +
> +static int mm8013_write_reg(struct i2c_client *client, u8 reg, u16 value)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_word_data(client, reg, value);
> +	if (ret < 0)
> +		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
> +
> +	usleep_range(4000, 5000);
> +	return ret;
> +}
> +
> +static int mm8013_read_reg(struct i2c_client *client, u8 reg)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_word_data(client, reg);
> +	if (ret < 0)
> +		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
> +
> +	usleep_range(4000, 5000);
> +	return ret;
> +}

Please use regmap.

> +static int mm8013_checkdevice(struct mm8013_chip *chip)
> +{
> +	int battery_id, ret;
> +
> +	ret = mm8013_write_reg(chip->client, REG_BATID, 0x0008);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = mm8013_read_reg(chip->client, REG_BATID);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == BATID_102)
> +		battery_id = 2;
> +	else if (ret == BATID_101)
> +		battery_id = 1;
> +	else
> +		return -EINVAL;
> +
> +	dev_dbg(&chip->client->dev, "battery_id: %d\n", battery_id);
> +
> +	return 0;
> +}
> +
> +static enum power_supply_property mm8013_battery_props[] = {
> +	POWER_SUPPLY_PROP_CAPACITY,
> +	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CYCLE_COUNT,
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
> +	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +};
> +
> +static int mm8013_get_property(struct power_supply *psy,
> +			       enum power_supply_property psp,
> +			       union power_supply_propval *val)
> +{
> +	struct mm8013_chip *chip = psy->drv_data;
> +	struct i2c_client *client = chip->client;
> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CAPACITY:

this is in %, while the next two are in uAh. So the fuel gauge does
not provide the current capacity in uAh
(POWER_SUPPLY_PROP_CHARGE_NOW)?

> +		ret = mm8013_read_reg(client, REG_STATE_OF_CHARGE);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL:
> +		ret = mm8013_read_reg(client, REG_FULL_CHARGE_CAPACITY);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> +		ret = mm8013_read_reg(client, REG_DESIGN_CAPACITY);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = mm8013_read_reg(client, REG_AVERAGE_CURRENT);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret > S16_MAX)
> +			val->intval -= (1 << 16);
> +		else
> +			val->intval = ret;

this is just 'val->intval = (s16) ret;'.

> +		val->intval *= -1000;

and this seems to be the only property, that properly scales its
values, assuming the hardware reports data in mA.

> +		break;
> +	case POWER_SUPPLY_PROP_CYCLE_COUNT:
> +		ret = mm8013_read_reg(client, REG_CYCLE_COUNT);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		ret = mm8013_read_reg(client, REG_FLAGS);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & MM8013_FLAG_BATHI)
> +			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +		else if (ret & (MM8013_FLAG_OTD | MM8013_FLAG_OTC))
> +			val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
> +		else
> +			val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +		break;
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = mm8013_read_reg(client, REG_TEMPERATURE) > 0;
> +		break;
> +	case POWER_SUPPLY_PROP_STATUS:
> +		ret = mm8013_read_reg(client, REG_FLAGS);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & MM8013_FLAG_DSG)
> +			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> +		else if (ret & MM8013_FLAG_CHG)
> +			val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +		else if (ret & MM8013_FLAG_FC)
> +			val->intval = POWER_SUPPLY_STATUS_FULL;
> +		else
> +			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> +		break;
> +	case POWER_SUPPLY_PROP_TEMP:
> +		ret = mm8013_read_reg(client, REG_TEMPERATURE);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = DECIKELVIN_TO_DECIDEGC(ret);
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
> +		ret = mm8013_read_reg(client, REG_AVERAGE_TIME_TO_EMPTY);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* The estimation is not yet ready */
> +		if (ret == U16_MAX)
> +			return -ENODATA;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
> +		ret = mm8013_read_reg(client, REG_AVERAGE_TIME_TO_FULL);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* The estimation is not yet ready */
> +		if (ret == U16_MAX)
> +			return -ENODATA;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:

that's in **micro** volts

> +		ret = mm8013_read_reg(client, REG_VOLTAGE);

this effectively does i2c_smbus_read_word_data() and thus the
maximum is is 65536. 65536uV = 65mV. I have serious doubts, that
this code does what you want. The same is true for a couple of
the other properties.

> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct power_supply_desc mm8013_desc = {
> +	.name			= "mm8013",
> +	.type			= POWER_SUPPLY_TYPE_BATTERY,
> +	.properties		= mm8013_battery_props,
> +	.num_properties		= ARRAY_SIZE(mm8013_battery_props),
> +	.get_property		= mm8013_get_property,
> +};
> +
> +static int mm8013_probe(struct i2c_client *client)
> +{
> +	struct power_supply_config psy_cfg = {};
> +	struct device *dev = &client->dev;
> +	struct power_supply *psy;
> +	struct mm8013_chip *chip;
> +	int ret = 0;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
> +		return dev_err_probe(dev, -EIO,
> +				     "I2C_FUNC_SMBUS_WORD_DATA not supported\n");
> +
> +	chip = devm_kzalloc(dev, sizeof(struct mm8013_chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	chip->client = client;
> +
> +	ret = mm8013_checkdevice(chip);
> +	if (ret)
> +		return dev_err_probe(dev, -ENODEV, "MM8013 not found\n");

dev_err_probe(dev, ret, ... ?

> +
> +	psy_cfg.drv_data = chip;
> +	psy_cfg.of_node = dev->of_node;
> +
> +	psy = devm_power_supply_register(dev, &mm8013_desc, &psy_cfg);
> +	if (IS_ERR(psy))
> +		return PTR_ERR(psy);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id mm8013_id_table[] = {
> +	{ "mm8013", 0 },
> +	{},

please remove the last ,

> +};
> +MODULE_DEVICE_TABLE(i2c, mm8013_id_table);
> +
> +static const struct of_device_id mm8013_match_table[] = {
> +	{ .compatible = "mitsumi,mm8013" },
> +	{ },

extra space in the Terminator, also terminator should not have a
comma at the end.

> +};
> +
> +static struct i2c_driver mm8013_i2c_driver = {
> +	.probe = mm8013_probe,
> +	.id_table = mm8013_id_table,
> +	.driver = {
> +		.name = "mm8013",
> +		.of_match_table = mm8013_match_table,
> +	},
> +};
> +module_i2c_driver(mm8013_i2c_driver);
> +
> +MODULE_DESCRIPTION("MM8013 fuel gauge driver");
> +MODULE_LICENSE("GPL");

With the next submission please include a dump of the uevent
in sysfs in the cover letter or below the fold, so that its
easy to validty check if the reported values look sensible.

Thanks and sorry for the slow processing,

-- Sebastian

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

Re: [PATCH RESEND v2 3/3] power: supply: Introduce MM8013 fuel gauge driver

[Konrad Dybcio]

On 13.09.2023 17:45, Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Aug 23, 2023 at 04:36:15PM +0200, Konrad Dybcio wrote:
>> Add a driver for the Mitsumi MM8013 fuel gauge. The driver is a vastly
>> cleaned up and improved version of the one that shipped in some obscure
>> Lenovo downstream kernel [1], with some register definitions borrowed from
>> ChromeOS EC platform code [2].
>>
>> [1] https://github.com/adazem009/kernel_lenovo_bengal/commit/b6b346427a871715709bd22aae449b9383f3b66b
>> [2] https://chromium.googlesource.com/chromiumos/platform/ec/+/master/driver/battery/mm8013.h
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
[...]

> 
> Please use regmap.
Ack

[...]

>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_CAPACITY:
> 
> this is in %, while the next two are in uAh. So the fuel gauge does
> not provide the current capacity in uAh
> (POWER_SUPPLY_PROP_CHARGE_NOW)?
Yes. Doesn't seem like raw values are supported.

[...]

> 
> this is just 'val->intval = (s16) ret;'.
> 
>> +		val->intval *= -1000;
Ack

> 
> and this seems to be the only property, that properly scales its
> values, assuming the hardware reports data in mA.
> 

[...]

> 
> that's in **micro** volts
Oh, I didn't read enough docs..

> 
>> +		ret = mm8013_read_reg(client, REG_VOLTAGE);
> 
> this effectively does i2c_smbus_read_word_data() and thus the
> maximum is is 65536. 65536uV = 65mV. I have serious doubts, that
> this code does what you want. The same is true for a couple of
> the other properties.
Ack

[...]

> With the next submission please include a dump of the uevent
> in sysfs in the cover letter or below the fold, so that its
> easy to validty check if the reported values look sensible.
State of what-will-be-sent in v(n+1), with additional fixups:

POWER_SUPPLY_NAME=mm8013
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_CAPACITY=100
POWER_SUPPLY_CHARGE_FULL=7124
POWER_SUPPLY_CHARGE_FULL_DESIGN=7500
POWER_SUPPLY_CURRENT_NOW=-122000
POWER_SUPPLY_CYCLE_COUNT=27
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_STATUS=Full
POWER_SUPPLY_TEMP=324
POWER_SUPPLY_VOLTAGE_NOW=4407000


> 
> Thanks and sorry for the slow processing,
No worries

Konrad

Re: [PATCH RESEND v2 3/3] power: supply: Introduce MM8013 fuel gauge driver

[Sebastian Reichel]

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

Hi,

On Thu, Sep 14, 2023 at 08:46:21PM +0200, Konrad Dybcio wrote:
> On 13.09.2023 17:45, Sebastian Reichel wrote:
> > On Wed, Aug 23, 2023 at 04:36:15PM +0200, Konrad Dybcio wrote:
> >> Add a driver for the Mitsumi MM8013 fuel gauge. The driver is a vastly
> >> cleaned up and improved version of the one that shipped in some obscure
> >> Lenovo downstream kernel [1], with some register definitions borrowed from
> >> ChromeOS EC platform code [2].
> >>
> >> [1] https://github.com/adazem009/kernel_lenovo_bengal/commit/b6b346427a871715709bd22aae449b9383f3b66b
> >> [2] https://chromium.googlesource.com/chromiumos/platform/ec/+/master/driver/battery/mm8013.h
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >> ---
> [...]

[...]

> >> +	switch (psp) {
> >> +	case POWER_SUPPLY_PROP_CAPACITY:
> > 
> > this is in %, while the next two are in uAh. So the fuel gauge does
> > not provide the current capacity in uAh
> > (POWER_SUPPLY_PROP_CHARGE_NOW)?
> Yes. Doesn't seem like raw values are supported.

Ok. (It's quite unusual for a chip to provide design and full
capacity separatly, but not providing the current capacity.)

[...]

> > With the next submission please include a dump of the uevent
> > in sysfs in the cover letter or below the fold, so that its
> > easy to validty check if the reported values look sensible.
> State of what-will-be-sent in v(n+1), with additional fixups:
> 
> POWER_SUPPLY_NAME=mm8013
> POWER_SUPPLY_TYPE=Battery
> POWER_SUPPLY_CAPACITY=100
> POWER_SUPPLY_CHARGE_FULL=7124
> POWER_SUPPLY_CHARGE_FULL_DESIGN=7500

The unit for the above two are uAh. So that would be 7.5 mAh. With
4.4V I expect you have something bigger than a coin cell, so that's
probably wrong :)

> POWER_SUPPLY_CURRENT_NOW=-122000
> POWER_SUPPLY_CYCLE_COUNT=27
> POWER_SUPPLY_HEALTH=Good
> POWER_SUPPLY_PRESENT=1
> POWER_SUPPLY_STATUS=Full
> POWER_SUPPLY_TEMP=324
> POWER_SUPPLY_VOLTAGE_NOW=4407000

Otherwise looks sensible.

Greetings,

-- Sebastian

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