From: Yassine Oudjana <y.oudjana@protonmail.com>

Samsung's S2MU005 PMIC, which contains battery charger functionality
also includes a battery fuel gauge device, which is separate from the
PMIC itself, and typically connected to an I2C bus. Add a generic driver
to support said device.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Co-developed-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/power/supply/Kconfig           |   9 ++
 drivers/power/supply/Makefile          |   1 +
 drivers/power/supply/s2mu005-battery.c | 234 +++++++++++++++++++++++++++++++++
 3 files changed, 244 insertions(+)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 92f9f7aae92f2..a5777309b1f62 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -229,6 +229,15 @@ config BATTERY_SAMSUNG_SDI
 	  Say Y to enable support for Samsung SDI battery data.
 	  These batteries are used in Samsung mobile phones.
 
+config BATTERY_S2MU005
+	tristate "Samsung S2MU005 PMIC fuel gauge driver"
+	help
+	  Say Y to enable support for the Samsung S2MU005 PMIC integrated
+	  fuel gauge, which works indepenently of the PMIC battery charger
+	  counterpart, and reports battery metrics.
+
+	  This driver, if built as a module, will be called s2mu005-fuel-gauge.
+
 config BATTERY_COLLIE
 	tristate "Sharp SL-5500 (collie) battery"
 	depends on SA1100_COLLIE && MCP_UCB1200
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 4b79d5abc49a7..cd061887c1727 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
 obj-$(CONFIG_BATTERY_QCOM_BATTMGR)	+= qcom_battmgr.o
 obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_SAMSUNG_SDI)	+= samsung-sdi-battery.o
+obj-$(CONFIG_BATTERY_S2MU005)	+= s2mu005-battery.o
 obj-$(CONFIG_BATTERY_COLLIE)	+= collie_battery.o
 obj-$(CONFIG_BATTERY_INGENIC)	+= ingenic-battery.o
 obj-$(CONFIG_BATTERY_INTEL_DC_TI) += intel_dc_ti_battery.o
diff --git a/drivers/power/supply/s2mu005-battery.c b/drivers/power/supply/s2mu005-battery.c
new file mode 100644
index 0000000000000..914308e82683b
--- /dev/null
+++ b/drivers/power/supply/s2mu005-battery.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Battery Fuel Gauge Driver for Samsung S2MU005 PMIC.
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ * Copyright (C) 2023 Yassine Oudjana <y.oudjana@protonmail.com>
+ * Copyright (C) 2025 Kaustabh Chakraborty <kauschluss@disroot.org>
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+#define S2MU005_FG_REG_STATUS		0x00
+#define S2MU005_FG_REG_IRQ		0x02
+#define S2MU005_FG_REG_RVBAT		0x04
+#define S2MU005_FG_REG_RCUR_CC		0x06
+#define S2MU005_FG_REG_RSOC		0x08
+#define S2MU005_FG_REG_MONOUT		0x0a
+#define S2MU005_FG_REG_MONOUT_SEL	0x0c
+#define S2MU005_FG_REG_RBATCAP		0x0e
+#define S2MU005_FG_REG_RZADJ		0x12
+#define S2MU005_FG_REG_RBATZ0		0x16
+#define S2MU005_FG_REG_RBATZ1		0x18
+#define S2MU005_FG_REG_IRQ_LVL		0x1a
+#define S2MU005_FG_REG_START		0x1e
+
+struct s2mu005_fg {
+	struct device *dev;
+	struct regmap *regmap;
+	struct power_supply *psy;
+};
+
+static const struct regmap_config s2mu005_fg_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static irqreturn_t s2mu005_handle_irq(int irq, void *data)
+{
+	struct s2mu005_fg *priv = data;
+
+	msleep(100);
+	power_supply_changed(priv->psy);
+
+	return IRQ_HANDLED;
+}
+
+static int s2mu005_fg_get_voltage_now(struct s2mu005_fg *priv, int *value)
+{
+	struct regmap *regmap = priv->regmap;
+	u16 reg;
+	int ret;
+
+	ret = regmap_raw_read(regmap, S2MU005_FG_REG_RVBAT, &reg, sizeof(reg));
+	if (ret < 0) {
+		dev_err(priv->dev, "failed to read voltage register (%d)\n", ret);
+		return ret;
+	}
+
+	*value = ((unsigned long)reg * 1000000) >> 13; /* uV */
+
+	return 0;
+}
+
+static int s2mu005_fg_get_current_now(struct s2mu005_fg *priv, int *value)
+{
+	struct regmap *regmap = priv->regmap;
+	s16 reg;
+	int ret;
+
+	ret = regmap_raw_read(regmap, S2MU005_FG_REG_RCUR_CC, &reg, sizeof(reg));
+	if (ret < 0) {
+		dev_err(priv->dev, "failed to read current register (%d)\n", ret);
+		return ret;
+	}
+
+	*value = -((long)reg * 1000000) >> 12; /* uA */
+
+	return 0;
+}
+
+static int s2mu005_fg_get_capacity(struct s2mu005_fg *priv, int *value)
+{
+	struct regmap *regmap = priv->regmap;
+	s16 reg;
+	int ret;
+
+	ret = regmap_raw_read(regmap, S2MU005_FG_REG_RSOC, &reg, sizeof(reg));
+	if (ret < 0) {
+		dev_err(priv->dev, "failed to read capacity register (%d)\n", ret);
+		return ret;
+	}
+
+	*value = (reg * 100) >> 14; /* percentage */
+
+	return 0;
+}
+
+static int s2mu005_fg_get_status(struct s2mu005_fg *priv, int *value)
+{
+	int current_now;
+	int capacity;
+	int ret;
+
+	ret = s2mu005_fg_get_current_now(priv, &current_now);
+	if (ret)
+		return ret;
+
+	if (current_now <= 0) {
+		*value = POWER_SUPPLY_STATUS_DISCHARGING;
+		return 0;
+	}
+
+	ret = s2mu005_fg_get_capacity(priv, &capacity);
+	if (ret)
+		return ret;
+
+	if (capacity < 90)
+		*value = POWER_SUPPLY_STATUS_CHARGING;
+	else
+		*value = POWER_SUPPLY_STATUS_FULL;
+
+	return 0;
+}
+
+static const enum power_supply_property s2mu005_fg_properties[] = {
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_STATUS,
+};
+
+static int s2mu005_fg_get_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct s2mu005_fg *priv = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = s2mu005_fg_get_voltage_now(priv, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = s2mu005_fg_get_current_now(priv, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = s2mu005_fg_get_capacity(priv, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = s2mu005_fg_get_status(priv, &val->intval);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct power_supply_desc s2mu005_fg_desc = {
+	.name = "s2mu005-fuel-gauge",
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.properties = s2mu005_fg_properties,
+	.num_properties = ARRAY_SIZE(s2mu005_fg_properties),
+	.get_property = s2mu005_fg_get_property,
+};
+
+static int s2mu005_fg_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct s2mu005_fg *priv;
+	struct power_supply_config psy_cfg = {};
+	const struct power_supply_desc *psy_desc;
+	int flags;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return dev_err_probe(dev, -ENOMEM,
+				     "failed to allocate driver private\n");
+
+	dev_set_drvdata(dev, priv);
+	priv->dev = dev;
+
+	priv->regmap = devm_regmap_init_i2c(client, &s2mu005_fg_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->regmap),
+				     "failed to initialize regmap\n");
+
+	psy_desc = device_get_match_data(dev);
+
+	psy_cfg.drv_data = priv;
+	priv->psy = devm_power_supply_register(priv->dev, psy_desc, &psy_cfg);
+	if (IS_ERR(priv->psy))
+		return dev_err_probe(dev, PTR_ERR(priv->psy),
+				     "failed to register power supply subsystem\n");
+
+	flags = irq_get_trigger_type(client->irq);
+
+	ret = devm_request_threaded_irq(priv->dev, client->irq, NULL,
+					s2mu005_handle_irq, IRQF_ONESHOT | flags,
+					psy_desc->name, priv);
+	if (ret)
+		dev_err_probe(dev, ret, "failed to request IRQ\n");
+
+	return 0;
+}
+
+static const struct of_device_id s2mu005_fg_of_match_table[] = {
+	{
+		.compatible = "samsung,s2mu005-fuel-gauge",
+		.data = &s2mu005_fg_desc,
+	}, { },
+};
+MODULE_DEVICE_TABLE(of, s2mu005_fg_of_match_table);
+
+static struct i2c_driver s2mu005_fg_i2c_driver = {
+	.probe = s2mu005_fg_i2c_probe,
+	.driver = {
+		.name = "s2mu005-fuel-gauge",
+		.of_match_table = of_match_ptr(s2mu005_fg_of_match_table),
+	},
+};
+module_i2c_driver(s2mu005_fg_i2c_driver);
+
+MODULE_DESCRIPTION("Samsung S2MU005 PMIC Battery Fuel Gauge Driver");
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
+MODULE_LICENSE("GPL");

-- 
2.52.0

Hi,

On Mon, Jan 26, 2026 at 09:09:49PM +0530, Kaustabh Chakraborty wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Samsung's S2MU005 PMIC, which contains battery charger functionality
> also includes a battery fuel gauge device, which is separate from the
> PMIC itself, and typically connected to an I2C bus. Add a generic driver
> to support said device.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> Co-developed-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  drivers/power/supply/Kconfig           |   9 ++
>  drivers/power/supply/Makefile          |   1 +
>  drivers/power/supply/s2mu005-battery.c | 234 +++++++++++++++++++++++++++++++++
>  3 files changed, 244 insertions(+)
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 92f9f7aae92f2..a5777309b1f62 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -229,6 +229,15 @@ config BATTERY_SAMSUNG_SDI
>  	  Say Y to enable support for Samsung SDI battery data.
>  	  These batteries are used in Samsung mobile phones.
>  
> +config BATTERY_S2MU005
> +	tristate "Samsung S2MU005 PMIC fuel gauge driver"
> +	help
> +	  Say Y to enable support for the Samsung S2MU005 PMIC integrated
> +	  fuel gauge, which works indepenently of the PMIC battery charger
> +	  counterpart, and reports battery metrics.
> +
> +	  This driver, if built as a module, will be called s2mu005-fuel-gauge.
> +
>  config BATTERY_COLLIE
>  	tristate "Sharp SL-5500 (collie) battery"
>  	depends on SA1100_COLLIE && MCP_UCB1200
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 4b79d5abc49a7..cd061887c1727 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
>  obj-$(CONFIG_BATTERY_QCOM_BATTMGR)	+= qcom_battmgr.o
>  obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
>  obj-$(CONFIG_BATTERY_SAMSUNG_SDI)	+= samsung-sdi-battery.o
> +obj-$(CONFIG_BATTERY_S2MU005)	+= s2mu005-battery.o
>  obj-$(CONFIG_BATTERY_COLLIE)	+= collie_battery.o
>  obj-$(CONFIG_BATTERY_INGENIC)	+= ingenic-battery.o
>  obj-$(CONFIG_BATTERY_INTEL_DC_TI) += intel_dc_ti_battery.o
> diff --git a/drivers/power/supply/s2mu005-battery.c b/drivers/power/supply/s2mu005-battery.c
> new file mode 100644
> index 0000000000000..914308e82683b
> --- /dev/null
> +++ b/drivers/power/supply/s2mu005-battery.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Battery Fuel Gauge Driver for Samsung S2MU005 PMIC.
> + *
> + * Copyright (C) 2015 Samsung Electronics
> + * Copyright (C) 2023 Yassine Oudjana <y.oudjana@protonmail.com>
> + * Copyright (C) 2025 Kaustabh Chakraborty <kauschluss@disroot.org>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/of.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +#define S2MU005_FG_REG_STATUS		0x00
> +#define S2MU005_FG_REG_IRQ		0x02
> +#define S2MU005_FG_REG_RVBAT		0x04
> +#define S2MU005_FG_REG_RCUR_CC		0x06
> +#define S2MU005_FG_REG_RSOC		0x08
> +#define S2MU005_FG_REG_MONOUT		0x0a
> +#define S2MU005_FG_REG_MONOUT_SEL	0x0c
> +#define S2MU005_FG_REG_RBATCAP		0x0e
> +#define S2MU005_FG_REG_RZADJ		0x12
> +#define S2MU005_FG_REG_RBATZ0		0x16
> +#define S2MU005_FG_REG_RBATZ1		0x18
> +#define S2MU005_FG_REG_IRQ_LVL		0x1a
> +#define S2MU005_FG_REG_START		0x1e
> +
> +struct s2mu005_fg {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct power_supply *psy;
> +};
> +
> +static const struct regmap_config s2mu005_fg_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};

Looks like all register addresses are 2 byte aligned and you are
always using regmap_raw_read to get 16bit values. So just use
.val_bits = 16 here?

> +static irqreturn_t s2mu005_handle_irq(int irq, void *data)
> +{
> +	struct s2mu005_fg *priv = data;
> +
> +	msleep(100);
> +	power_supply_changed(priv->psy);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int s2mu005_fg_get_voltage_now(struct s2mu005_fg *priv, int *value)
> +{
> +	struct regmap *regmap = priv->regmap;
> +	u16 reg;
> +	int ret;
> +
> +	ret = regmap_raw_read(regmap, S2MU005_FG_REG_RVBAT, &reg, sizeof(reg));
> +	if (ret < 0) {
> +		dev_err(priv->dev, "failed to read voltage register (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	*value = ((unsigned long)reg * 1000000) >> 13; /* uV */
> +
> +	return 0;
> +}
> +
> +static int s2mu005_fg_get_current_now(struct s2mu005_fg *priv, int *value)
> +{
> +	struct regmap *regmap = priv->regmap;
> +	s16 reg;
> +	int ret;
> +
> +	ret = regmap_raw_read(regmap, S2MU005_FG_REG_RCUR_CC, &reg, sizeof(reg));
> +	if (ret < 0) {
> +		dev_err(priv->dev, "failed to read current register (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	*value = -((long)reg * 1000000) >> 12; /* uA */
> +
> +	return 0;
> +}
> +
> +static int s2mu005_fg_get_capacity(struct s2mu005_fg *priv, int *value)
> +{
> +	struct regmap *regmap = priv->regmap;
> +	s16 reg;
> +	int ret;
> +
> +	ret = regmap_raw_read(regmap, S2MU005_FG_REG_RSOC, &reg, sizeof(reg));
> +	if (ret < 0) {
> +		dev_err(priv->dev, "failed to read capacity register (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	*value = (reg * 100) >> 14; /* percentage */
> +
> +	return 0;
> +}
> +
> +static int s2mu005_fg_get_status(struct s2mu005_fg *priv, int *value)
> +{
> +	int current_now;
> +	int capacity;
> +	int ret;
> +
> +	ret = s2mu005_fg_get_current_now(priv, &current_now);
> +	if (ret)
> +		return ret;
> +
> +	if (current_now <= 0) {
> +		*value = POWER_SUPPLY_STATUS_DISCHARGING;
> +		return 0;
> +	}
> +
> +	ret = s2mu005_fg_get_capacity(priv, &capacity);
> +	if (ret)
> +		return ret;
> +
> +	if (capacity < 90)
> +		*value = POWER_SUPPLY_STATUS_CHARGING;
> +	else
> +		*value = POWER_SUPPLY_STATUS_FULL;

Usually there is some kind of hysteresis that stops charging
when the battery is full and then restarts charging once the
battery drops under a certain capacity. As this code first
checks the current to determine if the battery is discharging
and only then checks if the battery is full - does your code
toggle between FULL and DISCHARGING?

> +	return 0;
> +}
> +
> +static const enum power_supply_property s2mu005_fg_properties[] = {
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +	POWER_SUPPLY_PROP_STATUS,
> +};
> +
> +static int s2mu005_fg_get_property(struct power_supply *psy,
> +				   enum power_supply_property psp,
> +				   union power_supply_propval *val)
> +{
> +	struct s2mu005_fg *priv = power_supply_get_drvdata(psy);
> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = s2mu005_fg_get_voltage_now(priv, &val->intval);
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = s2mu005_fg_get_current_now(priv, &val->intval);
> +		break;
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		ret = s2mu005_fg_get_capacity(priv, &val->intval);
> +		break;
> +	case POWER_SUPPLY_PROP_STATUS:
> +		ret = s2mu005_fg_get_status(priv, &val->intval);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct power_supply_desc s2mu005_fg_desc = {
> +	.name = "s2mu005-fuel-gauge",
> +	.type = POWER_SUPPLY_TYPE_BATTERY,
> +	.properties = s2mu005_fg_properties,
> +	.num_properties = ARRAY_SIZE(s2mu005_fg_properties),
> +	.get_property = s2mu005_fg_get_property,
> +};
> +
> +static int s2mu005_fg_i2c_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct s2mu005_fg *priv;
> +	struct power_supply_config psy_cfg = {};
> +	const struct power_supply_desc *psy_desc;
> +	int flags;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return dev_err_probe(dev, -ENOMEM,
> +				     "failed to allocate driver private\n");

Do not print error messages for -ENOMEM.

> +
> +	dev_set_drvdata(dev, priv);
> +	priv->dev = dev;
> +
> +	priv->regmap = devm_regmap_init_i2c(client, &s2mu005_fg_regmap_config);
> +	if (IS_ERR(priv->regmap))
> +		return dev_err_probe(dev, PTR_ERR(priv->regmap),
> +				     "failed to initialize regmap\n");
> +
> +	psy_desc = device_get_match_data(dev);
> +
> +	psy_cfg.drv_data = priv;
> +	priv->psy = devm_power_supply_register(priv->dev, psy_desc, &psy_cfg);
> +	if (IS_ERR(priv->psy))
> +		return dev_err_probe(dev, PTR_ERR(priv->psy),
> +				     "failed to register power supply subsystem\n");
> +
> +	flags = irq_get_trigger_type(client->irq);

This is not needed. By not specifying the trigger type the irq core
code will do this internally.

> +	ret = devm_request_threaded_irq(priv->dev, client->irq, NULL,
> +					s2mu005_handle_irq, IRQF_ONESHOT | flags,
> +					psy_desc->name, priv);
> +	if (ret)
> +		dev_err_probe(dev, ret, "failed to request IRQ\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id s2mu005_fg_of_match_table[] = {
> +	{
> +		.compatible = "samsung,s2mu005-fuel-gauge",
> +		.data = &s2mu005_fg_desc,
> +	}, { },

Please put the terminator into its own line and remove the final ,

> +};
> +MODULE_DEVICE_TABLE(of, s2mu005_fg_of_match_table);
> +
> +static struct i2c_driver s2mu005_fg_i2c_driver = {
> +	.probe = s2mu005_fg_i2c_probe,
> +	.driver = {
> +		.name = "s2mu005-fuel-gauge",
> +		.of_match_table = of_match_ptr(s2mu005_fg_of_match_table),

Remove of_match_ptr().

> +	},
> +};
> +module_i2c_driver(s2mu005_fg_i2c_driver);
> +
> +MODULE_DESCRIPTION("Samsung S2MU005 PMIC Battery Fuel Gauge Driver");
> +MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
> +MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
> +MODULE_LICENSE("GPL");

Greetings,

-- Sebastian

On 2026-01-29 02:59 +01:00, Sebastian Reichel wrote:
> Hi,
>
> On Mon, Jan 26, 2026 at 09:09:49PM +0530, Kaustabh Chakraborty wrote:
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>> 
>> Samsung's S2MU005 PMIC, which contains battery charger functionality
>> also includes a battery fuel gauge device, which is separate from the
>> PMIC itself, and typically connected to an I2C bus. Add a generic driver
>> to support said device.
>> 
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> Co-developed-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> ---

[...]

>> +
>> +static const struct regmap_config s2mu005_fg_regmap_config = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +};
>
> Looks like all register addresses are 2 byte aligned and you are
> always using regmap_raw_read to get 16bit values. So just use
> .val_bits = 16 here?

Sure.

>> +static int s2mu005_fg_get_status(struct s2mu005_fg *priv, int *value)
>> +{
>> +	int current_now;
>> +	int capacity;
>> +	int ret;
>> +
>> +	ret = s2mu005_fg_get_current_now(priv, &current_now);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (current_now <= 0) {
>> +		*value = POWER_SUPPLY_STATUS_DISCHARGING;
>> +		return 0;
>> +	}
>> +
>> +	ret = s2mu005_fg_get_capacity(priv, &capacity);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (capacity < 90)
>> +		*value = POWER_SUPPLY_STATUS_CHARGING;
>> +	else
>> +		*value = POWER_SUPPLY_STATUS_FULL;
>
> Usually there is some kind of hysteresis that stops charging
> when the battery is full and then restarts charging once the
> battery drops under a certain capacity. As this code first
> checks the current to determine if the battery is discharging
> and only then checks if the battery is full - does your code
> toggle between FULL and DISCHARGING?

(Perhaps I misunderstood your query, let me know if I did)

It has been a while since I last tested this - but as far as I can
recall, after a certain threshold, the current would constantly bounce
between -ve and +ve. I believe it was somewhere around 90% to 95% and
above. If that's what 'hysteresis' is, then yes.

An older revision of this driver (I don't have it anymore) used to add
up the consecutive values of current in order to reduce the effect of
this inconsistency, but it was still unreliable.

Moreover, I do not possess any documentation for this device, so it's
not possible for me to know what or how.

Hi,

On Sat, Jan 31, 2026 at 04:44:36PM +0530, Kaustabh Chakraborty wrote:
> On 2026-01-29 02:59 +01:00, Sebastian Reichel wrote:
> >> +static int s2mu005_fg_get_status(struct s2mu005_fg *priv, int *value)
> >> +{
> >> +	int current_now;
> >> +	int capacity;
> >> +	int ret;
> >> +
> >> +	ret = s2mu005_fg_get_current_now(priv, &current_now);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	if (current_now <= 0) {
> >> +		*value = POWER_SUPPLY_STATUS_DISCHARGING;
> >> +		return 0;
> >> +	}
> >> +
> >> +	ret = s2mu005_fg_get_capacity(priv, &capacity);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	if (capacity < 90)
> >> +		*value = POWER_SUPPLY_STATUS_CHARGING;
> >> +	else
> >> +		*value = POWER_SUPPLY_STATUS_FULL;
> >
> > Usually there is some kind of hysteresis that stops charging
> > when the battery is full and then restarts charging once the
> > battery drops under a certain capacity. As this code first
> > checks the current to determine if the battery is discharging
> > and only then checks if the battery is full - does your code
> > toggle between FULL and DISCHARGING?
> 
> (Perhaps I misunderstood your query, let me know if I did)
> 
> It has been a while since I last tested this - but as far as I can
> recall, after a certain threshold, the current would constantly bounce
> between -ve and +ve. I believe it was somewhere around 90% to 95% and
> above. If that's what 'hysteresis' is, then yes.

A typical charging setup is:

1. charge to 100%
2. stop charging
3. wait for charge to drop under 95% (or some other treshold)
4. restart charging
5. go to 1

> An older revision of this driver (I don't have it anymore) used to add
> up the consecutive values of current in order to reduce the effect of
> this inconsistency, but it was still unreliable.
> 
> Moreover, I do not possess any documentation for this device, so it's
> not possible for me to know what or how.

For the setup I described above, you consider everything above the
treshold as POWER_SUPPLY_STATUS_FULL independent of the current
direction. So you need to reorder:

if (capacity >= 90)
    return POWER_SUPPLY_STATUS_FULL;

if (current_now < 0)
    return POWER_SUPPLY_STATUS_DISCHARGING;
else if (current_now == 0)
    return POWER_SUPPLY_STATUS_NOT_CHARGING;
else
    return POWER_SUPPLY_STATUS_CHARGING;

Greetings,

-- Sebastian

On 2026-02-01 12:14 +01:00, Sebastian Reichel wrote:
> Hi,
>
> On Sat, Jan 31, 2026 at 04:44:36PM +0530, Kaustabh Chakraborty wrote:
>> On 2026-01-29 02:59 +01:00, Sebastian Reichel wrote:
>> >> +static int s2mu005_fg_get_status(struct s2mu005_fg *priv, int *value)
>> >> +{
>> >> +	int current_now;
>> >> +	int capacity;
>> >> +	int ret;
>> >> +
>> >> +	ret = s2mu005_fg_get_current_now(priv, &current_now);
>> >> +	if (ret)
>> >> +		return ret;
>> >> +
>> >> +	if (current_now <= 0) {
>> >> +		*value = POWER_SUPPLY_STATUS_DISCHARGING;
>> >> +		return 0;
>> >> +	}
>> >> +
>> >> +	ret = s2mu005_fg_get_capacity(priv, &capacity);
>> >> +	if (ret)
>> >> +		return ret;
>> >> +
>> >> +	if (capacity < 90)
>> >> +		*value = POWER_SUPPLY_STATUS_CHARGING;
>> >> +	else
>> >> +		*value = POWER_SUPPLY_STATUS_FULL;

[...]

>> An older revision of this driver (I don't have it anymore) used to add
>> up the consecutive values of current in order to reduce the effect of
>> this inconsistency, but it was still unreliable.
>> 
>> Moreover, I do not possess any documentation for this device, so it's
>> not possible for me to know what or how.
>
> For the setup I described above, you consider everything above the
> treshold as POWER_SUPPLY_STATUS_FULL independent of the current
> direction. So you need to reorder:
>
> if (capacity >= 90)
>     return POWER_SUPPLY_STATUS_FULL;

So I have tested this. Should it not be discharging when power is not
connected (current_now > 0). Doing this shows 'fully-charged' in upower,
with the charging symbol in GNOME even if the charger is not connected.

In such case, the logic in the current revision of the driver is
correct, where it assumes the battery to be full ONLY if it is charging.

>
> if (current_now < 0)
>     return POWER_SUPPLY_STATUS_DISCHARGING;
> else if (current_now == 0)
>     return POWER_SUPPLY_STATUS_NOT_CHARGING;
> else
>     return POWER_SUPPLY_STATUS_CHARGING;
>
> Greetings,
>
> -- Sebastian

On 2026-02-01 12:14 +01:00, Sebastian Reichel wrote:
> Hi,
>
> On Sat, Jan 31, 2026 at 04:44:36PM +0530, Kaustabh Chakraborty wrote:
>> On 2026-01-29 02:59 +01:00, Sebastian Reichel wrote:
>> >> +static int s2mu005_fg_get_status(struct s2mu005_fg *priv, int *value)
>> >> +{
>> >> +	int current_now;
>> >> +	int capacity;
>> >> +	int ret;
>> >> +
>> >> +	ret = s2mu005_fg_get_current_now(priv, &current_now);
>> >> +	if (ret)
>> >> +		return ret;
>> >> +
>> >> +	if (current_now <= 0) {
>> >> +		*value = POWER_SUPPLY_STATUS_DISCHARGING;
>> >> +		return 0;
>> >> +	}
>> >> +
>> >> +	ret = s2mu005_fg_get_capacity(priv, &capacity);
>> >> +	if (ret)
>> >> +		return ret;
>> >> +
>> >> +	if (capacity < 90)
>> >> +		*value = POWER_SUPPLY_STATUS_CHARGING;
>> >> +	else
>> >> +		*value = POWER_SUPPLY_STATUS_FULL;
>> >
>> > Usually there is some kind of hysteresis that stops charging
>> > when the battery is full and then restarts charging once the
>> > battery drops under a certain capacity. As this code first
>> > checks the current to determine if the battery is discharging
>> > and only then checks if the battery is full - does your code
>> > toggle between FULL and DISCHARGING?
>> 
>> (Perhaps I misunderstood your query, let me know if I did)
>> 
>> It has been a while since I last tested this - but as far as I can
>> recall, after a certain threshold, the current would constantly bounce
>> between -ve and +ve. I believe it was somewhere around 90% to 95% and
>> above. If that's what 'hysteresis' is, then yes.
>
> A typical charging setup is:
>
> 1. charge to 100%
> 2. stop charging
> 3. wait for charge to drop under 95% (or some other treshold)
> 4. restart charging
> 5. go to 1

Yes, I believe that's what's happening internally. I just tested it and
the threshold seems to be much higher, at ~97%. I will update it in the
next revision then.

>> An older revision of this driver (I don't have it anymore) used to add
>> up the consecutive values of current in order to reduce the effect of
>> this inconsistency, but it was still unreliable.
>> 
>> Moreover, I do not possess any documentation for this device, so it's
>> not possible for me to know what or how.
>
> For the setup I described above, you consider everything above the
> treshold as POWER_SUPPLY_STATUS_FULL independent of the current
> direction. So you need to reorder:
>
> if (capacity >= 90)
>     return POWER_SUPPLY_STATUS_FULL;
>
> if (current_now < 0)
>     return POWER_SUPPLY_STATUS_DISCHARGING;
> else if (current_now == 0)
>     return POWER_SUPPLY_STATUS_NOT_CHARGING;
> else
>     return POWER_SUPPLY_STATUS_CHARGING;

Understood. I will re-implement it in this order then.

>
> Greetings,
>
> -- Sebastian