[PATCH 0/5] Add driver for Qualcomm QGauge (qcom_qg) fuel gauge

[PATCH 0/5] Add driver for Qualcomm QGauge (qcom_qg) fuel gauge

[Val Packett]

Hi o/

This series adds a driver for the "qgauge" battery fuel gauge
present in Qualcomm PMICs like PMI632, PM6150, PM7250B, PM2250.
The PMI632 is used in SDM632, SM6115/SM4250, SM6125 based phones.

This series was tested on sm6115-motorola-{cebu,guamp}, whose basic
dts has been submitted as [1].

There have been two attempts at making this driver, the first one
"WIP! power: supply: Add rude driver for QGauge" as found in
various development forks [2] was rather infamous for wild capacity%
swings due to load variation :D

The second one was submitted to msm8953-mainline at [3] by
Marc Lainez, and it behaved much better thanks to the use of
power_supply_batinfo_ocv2cap.

This is a continuation of that work, carrying his From/S-o-b on
modified patches with explicit permission. I have added the
SDAM read via NVMEM API, fixed the batt_info leak and did some
general cleanup.

With the SMB5 work also going on right now, we'll finally have
full battery support on these awkward pre-battmgr generations!

[1]: https://lore.kernel.org/all/20261109221453.658316-1-val@packett.cool/
[2]: https://github.com/sm7150-mainline/linux/commit/76879bf7a71f4f8f66b9bcd7175069ca7bbf4274
[3]: https://github.com/msm8953-mainline/linux/pull/242

Thanks,
~val

---

Marc Lainez (2):
  dt-bindings: power: supply: add Qualcomm QGauge fuel gauge
  power: supply: add Qualcomm QGauge (qcom_qg) fuel gauge driver

Val Packett (3):
  iio: adc: qcom-spmi-adc5: Add ADC5_BAT_THERM_100K_PU
  arm64: dts: qcom: pmi632: Add missing bat_therm ADC channel
  arm64: dts: qcom: pmi632: Add battery fuel gauge node

 .../bindings/power/supply/qcom,qg.yaml        | 105 +++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/qcom/pmi632.dtsi          |  41 +
 drivers/iio/adc/qcom-spmi-adc5.c              |   2 +
 drivers/power/supply/Kconfig                  |  13 +
 drivers/power/supply/Makefile                 |   1 +
 drivers/power/supply/qcom_qg.c                | 807 ++++++++++++++++++
 7 files changed, 976 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom,qg.yaml
 create mode 100644 drivers/power/supply/qcom_qg.c

-- 
2.56.0

[PATCH 1/5] dt-bindings: power: supply: add Qualcomm QGauge fuel gauge

[Val Packett]

From: Marc Lainez <marc.lainez@gmail.com>

Document the QGauge fuel-gauge block handled by the qcom_qg driver, with
compatibles for the PMI632 and PM6150 PMICs.

Signed-off-by: Marc Lainez <marc.lainez@gmail.com>
Signed-off-by: Val Packett <val@packett.cool>
---
 .../bindings/power/supply/qcom,qg.yaml        | 105 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 2 files changed, 111 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/qcom,qg.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/qcom,qg.yaml b/Documentation/devicetree/bindings/power/supply/qcom,qg.yaml
new file mode 100644
index 000000000000..79dd18d87879
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/qcom,qg.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/qcom,qg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QGauge Fuel Gauge
+
+maintainers:
+  - Val Packett <val@packett.cool>
+
+description:
+  The QGauge (QG) block found in some Qualcomm PMICs (PM6150, PMI632)
+  provides battery fuel gauging through hardware voltage and current
+  FIFO sampling, open circuit voltage (OCV) measurement, and coulomb
+  counting. The hardware has a multi-state FSM (S1-S7) that automatically
+  transitions between active sampling and low-power sleep states.
+
+allOf:
+  - $ref: /schemas/power/supply/power-supply.yaml#
+  - $ref: /schemas/nvmem/nvmem-consumer.yaml#
+
+properties:
+  compatible:
+    enum:
+      - qcom,pm6150-qg
+      - qcom,pmi632-qg
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Battery missing interrupt
+      - description: Battery voltage low interrupt
+      - description: Battery voltage empty interrupt
+      - description: FIFO data update done interrupt
+      - description: Good OCV measured interrupt
+      - description: FSM state change interrupt
+      - description: QG event interrupt
+
+  interrupt-names:
+    items:
+      - const: batt-missing
+      - const: vbat-low
+      - const: vbat-empty
+      - const: fifo-done
+      - const: good-ocv
+      - const: fsm-state-chg
+      - const: event
+
+  io-channels:
+    items:
+      - description: ADC channel for battery thermistor temperature
+
+  io-channel-names:
+    items:
+      - const: batt-therm
+
+  monitored-battery: true
+
+  nvmem: true
+
+  power-supplies: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - monitored-battery
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    pmic {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      fuel-gauge@4800 {
+        compatible = "qcom,pmi632-qg";
+        reg = <0x4800>;
+
+        interrupts = <0x2 0x48 0x0 IRQ_TYPE_EDGE_BOTH>,
+                     <0x2 0x48 0x1 IRQ_TYPE_EDGE_BOTH>,
+                     <0x2 0x48 0x2 IRQ_TYPE_EDGE_RISING>,
+                     <0x2 0x48 0x3 IRQ_TYPE_EDGE_RISING>,
+                     <0x2 0x48 0x4 IRQ_TYPE_EDGE_RISING>,
+                     <0x2 0x48 0x5 IRQ_TYPE_EDGE_RISING>,
+                     <0x2 0x48 0x6 IRQ_TYPE_EDGE_RISING>;
+        interrupt-names = "batt-missing",
+                          "vbat-low",
+                          "vbat-empty",
+                          "fifo-done",
+                          "good-ocv",
+                          "fsm-state-chg",
+                          "event";
+
+        nvmem = <&pmi632_sdam_2>;
+
+        monitored-battery = <&battery>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index b253b10dd436..7b76b07ed66f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23328,6 +23328,12 @@ F:	drivers/firmware/qcom/qcom_pas.h
 F:	drivers/firmware/qcom/qcom_pas_tee.c
 F:	include/linux/firmware/qcom/qcom_pas.h
 
+QUALCOMM QGAUGE FUEL GAUGE DRIVER
+M:	Val Packett <val@packett.cool>
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/power/supply/qcom,qg.yaml
+
 QUALCOMM SMB CHARGER DRIVER
 M:	Casey Connolly <casey.connolly@linaro.org>
 L:	linux-arm-msm@vger.kernel.org
-- 
2.56.0

Re: [PATCH 1/5] dt-bindings: power: supply: add Qualcomm QGauge fuel gauge

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] dt-bindings: power: supply: qcom,qg: unconstrained nvmem and
  schema refs
--

commit 3e127648f3c0c46268041b3bde4ad7d6e88f27b3
Author: Marc Lainez <marc.lainez@gmail.com>

dt-bindings: power: supply: add Qualcomm QGauge fuel gauge

This commit adds the Device Tree binding schema for the Qualcomm QGauge fuel
gauge block on PMI632 and PM6150 PMICs and adds a corresponding MAINTAINERS
entry.

> diff --git a/Documentation/devicetree/bindings/power/supply/qcom,qg.yaml b/Documentation/devicetree/bindings/power/supply/qcom,qg.yaml
> new file mode 100644
> index 0000000000000..79dd18d878793
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/qcom,qg.yaml
> @@ -0,0 +1,105 @@
[ ... ]
> +allOf:
> +  - $ref: /schemas/power/supply/power-supply.yaml#
> +  - $ref: /schemas/nvmem/nvmem-consumer.yaml#
[ ... ]
> +  monitored-battery: true
> +
> +  nvmem: true
> +
> +  power-supplies: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - monitored-battery
> +
> +additionalProperties: false

[Severity: Low]
Should nvmem specify maxItems: 1 instead of true, and should this binding
use unevaluatedProperties: false without referencing nvmem-consumer.yaml#
in allOf?

nvmem-consumer.yaml is a base consumer schema with select: true that
dtschema applies automatically, and it only constrains each nvmem entry to
a single phandle while leaving the outer array length to the device
binding.  Dropping nvmem-consumer.yaml# from allOf and switching
additionalProperties: false to unevaluatedProperties: false also avoids
having to redeclare monitored-battery and power-supplies from
power-supply.yaml#.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112170841.2189322-1-val@packett.cool?part=1

[PATCH 2/5] power: supply: add Qualcomm QGauge (qcom_qg) fuel gauge driver

[Val Packett]

From: Marc Lainez <marc.lainez@gmail.com>

Add a driver for the QGauge (QG) fuel-gauge block found in Qualcomm SPMI
PMICs such as PMI632 and PM6150. The QG hardware provides FIFO-based
voltage/current sampling, sleep-time open-circuit-voltage measurement and
coulomb counting; state-of-charge is computed in-kernel from the
monitored-battery OCV-to-capacity tables. Coulomb counting is not
implemented yet.

The driver exposes present/status/health/technology, instantaneous and
running-average voltage and current, OCV, capacity, temperature,
and battery design limits via the power_supply class.

Signed-off-by: Marc Lainez <marc.lainez@gmail.com>
[val: cleanup, implement SDAM full charge, fix batt_info leak]
Signed-off-by: Val Packett <val@packett.cool>
---
Assisted-by: LLM
(as declared by Marc in the GitHub thread)
---
 MAINTAINERS                    |   1 +
 drivers/power/supply/Kconfig   |  13 +
 drivers/power/supply/Makefile  |   1 +
 drivers/power/supply/qcom_qg.c | 807 +++++++++++++++++++++++++++++++++
 4 files changed, 822 insertions(+)
 create mode 100644 drivers/power/supply/qcom_qg.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7b76b07ed66f..111e56340be8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23333,6 +23333,7 @@ M:	Val Packett <val@packett.cool>
 L:	linux-arm-msm@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/power/supply/qcom,qg.yaml
+F:	drivers/power/supply/qcom_qg.c
 
 QUALCOMM SMB CHARGER DRIVER
 M:	Casey Connolly <casey.connolly@linaro.org>
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 16d0a38b41b7..ad235a9057d1 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -228,6 +228,19 @@ config BATTERY_QCOM_BATTMGR
 	  which is used on modern Qualcomm platforms to provide battery and
 	  power supply information.
 
+config BATTERY_QCOM_QG
+	tristate "Qualcomm QGauge fuel gauge support"
+	depends on MFD_SPMI_PMIC || COMPILE_TEST
+	depends on IIO
+	help
+	  Say Y to add support for the QGauge (QG) fuel gauge block found
+	  in Qualcomm PMICs such as PMI632 and PM6150. This hardware provides
+	  battery voltage and current FIFO sampling, open circuit voltage
+	  (OCV) measurement, and coulomb counting for battery state estimation.
+
+	  To compile this driver as module, choose M here: the
+	  module will be called qcom_qg.
+
 config BATTERY_OLPC
 	tristate "One Laptop Per Child battery"
 	depends on OLPC_EC
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index cd3640f4b748..07296f3f7914 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -134,6 +134,7 @@ obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
 obj-$(CONFIG_BATTERY_CHARGER_SURFACE_RT) += surface-rt-ec.o
 obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
 obj-$(CONFIG_CHARGER_QCOM_SMB2)	+= qcom_smbx.o
+obj-$(CONFIG_BATTERY_QCOM_QG)	+= qcom_qg.o
 obj-$(CONFIG_FUEL_GAUGE_MM8013)	+= mm8013.o
 obj-$(CONFIG_MACSMC_POWER)	+= macsmc-power.o
 obj-$(CONFIG_CHARGER_MAX77759)	+= max77759_charger.o
diff --git a/drivers/power/supply/qcom_qg.c b/drivers/power/supply/qcom_qg.c
new file mode 100644
index 000000000000..071e29d4c299
--- /dev/null
+++ b/drivers/power/supply/qcom_qg.c
@@ -0,0 +1,807 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2025, Marc Lainez <marc.lainez@gmail.com>
+ * Copyright (c) 2026, Val Packett <val@packett.cool>
+ *
+ * Qualcomm QGauge (QG) fuel gauge driver (PMI632, PM6150, ...).
+ *
+ * The QG hardware block provides battery voltage and current measurement
+ * via FIFO-based sampling, open circuit voltage (OCV) measurement during
+ * sleep states, and coulomb counting through V/I accumulators.
+ *
+ * This driver exposes battery state via the power_supply subsystem.
+ * SOC is computed in-kernel using OCV-to-capacity lookup tables from
+ * the monitored-battery simple-battery node.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/errno.h>
+#include <linux/iio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/math64.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/timekeeping.h>
+
+/* Peripheral type register */
+#define QG_PERPH_TYPE			0x04
+#define QG_PERPH_TYPE_VALUE		0x0D
+
+/* Status registers */
+#define QG_STATUS1			0x08
+#define  QG_STATUS1_BATT_PRESENT	BIT(0)
+
+#define QG_STATUS2			0x09
+#define  QG_STATUS2_GOOD_OCV		BIT(1)
+
+#define QG_STATUS3			0x0A
+#define  QG_STATUS3_FIFO_RT_COUNT	GENMASK(3, 0)
+
+/* Data control */
+#define QG_DATA_CTL1			0x41
+#define  QG_DATA_CTL1_MASTER_HOLD	BIT(0)
+
+#define QG_DATA_CTL2			0x42
+#define  QG_DATA_CTL2_BURST_AVG_HOLD	BIT(0)
+
+/* OCV data registers (16-bit little-endian) */
+#define QG_S7_PON_OCV_V		0x70
+#define QG_S3_GOOD_OCV_V		0x74
+
+/* S2 (normal-mode) running-average data registers */
+#define QG_S2_NORMAL_AVG_V		0x80
+#define QG_S2_NORMAL_AVG_I		0x82
+
+/* FIFO data registers */
+#define QG_V_FIFO0			0x90
+#define QG_I_FIFO0			0xA0
+
+/* Last ADC data registers */
+#define QG_LAST_ADC_V			0xC0
+#define QG_LAST_ADC_I			0xC2
+#define QG_LAST_BURST_AVG_I		0xC6
+
+/* Sentinel value indicating FIFO entry is not yet written */
+#define QG_FIFO_RESET_VAL		0x8000
+
+/* Maximum hardware FIFO depth */
+#define QG_MAX_FIFO_LENGTH		8
+
+/* SDAM (shared direct access memory) offsets */
+#define QG_SDAM_VALID_OFFSET		0x46	/* 1-byte validity marker */
+#define QG_SDAM_LEARNED_CAPACITY_OFFSET	0x68	/* 2-byte, mAh */
+
+#define QG_OCV_STALE_SECS		180
+
+struct qcom_qg {
+	struct device *dev;
+	struct regmap *regmap;
+	unsigned int base;
+
+	struct power_supply *psy;
+	struct power_supply_battery_info *batt_info;
+
+	struct iio_channel *batt_therm;
+	struct nvmem_device *sdam;
+
+	struct mutex lock; /* protects the FIFO/burst hold register sequences */
+
+	/* Cached measurements (updated from IRQs and on-demand reads) */
+	unsigned int vbat_uv;
+	int ibat_ua;
+	unsigned int ocv_uv;
+	time64_t ocv_time;
+	bool batt_present;
+};
+
+static inline unsigned int qg_raw_to_uv(u16 raw)
+{
+	return div_u64(194637ULL * raw, 1000);
+}
+
+static inline int qg_raw_to_ua(s16 raw)
+{
+	/*
+	 * Hardware convention: positive = discharge, negative = charge.
+	 * Power supply class convention (ABI): positive = charge.
+	 */
+	return -div_s64(152588LL * raw, 1000);
+}
+
+static int qg_read16(struct qcom_qg *qg, unsigned int reg, u16 *val)
+{
+	int ret;
+	__le16 raw;
+
+	ret = regmap_bulk_read(qg->regmap, qg->base + reg, &raw, sizeof(raw));
+	if (ret)
+		return ret;
+
+	*val = le16_to_cpu(raw);
+	return 0;
+}
+
+static int qg_master_hold(struct qcom_qg *qg, bool hold)
+{
+	int ret;
+
+	/* Clear first (required for 0->1 transition to latch) */
+	ret = regmap_update_bits(qg->regmap, qg->base + QG_DATA_CTL1,
+				 QG_DATA_CTL1_MASTER_HOLD, 0);
+	if (ret)
+		return ret;
+
+	if (hold)
+		ret = regmap_update_bits(qg->regmap, qg->base + QG_DATA_CTL1,
+					 QG_DATA_CTL1_MASTER_HOLD,
+					 QG_DATA_CTL1_MASTER_HOLD);
+
+	return ret;
+}
+
+static int qg_read_battery_present(struct qcom_qg *qg, bool *present)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(qg->regmap, qg->base + QG_STATUS1, &val);
+	if (ret)
+		return ret;
+
+	*present = !!(val & QG_STATUS1_BATT_PRESENT);
+	return 0;
+}
+
+static int qg_read_vbat(struct qcom_qg *qg, unsigned int *vbat_uv)
+{
+	u16 raw;
+	int ret;
+
+	ret = qg_read16(qg, QG_LAST_ADC_V, &raw);
+	if (ret)
+		return ret;
+
+	*vbat_uv = qg_raw_to_uv(raw);
+	return 0;
+}
+
+static int qg_read_ibat(struct qcom_qg *qg, int *ibat_ua)
+{
+	u16 raw;
+	int ret;
+
+	guard(mutex)(&qg->lock);
+
+	/* Hold the burst average register to get a coherent reading */
+	ret = regmap_update_bits(qg->regmap, qg->base + QG_DATA_CTL2,
+				 QG_DATA_CTL2_BURST_AVG_HOLD,
+				 QG_DATA_CTL2_BURST_AVG_HOLD);
+	if (ret)
+		return ret;
+
+	ret = qg_read16(qg, QG_LAST_BURST_AVG_I, &raw);
+
+	/* Always release the hold, even if the read failed */
+	regmap_update_bits(qg->regmap, qg->base + QG_DATA_CTL2,
+			   QG_DATA_CTL2_BURST_AVG_HOLD, 0);
+
+	if (ret)
+		return ret;
+
+	*ibat_ua = qg_raw_to_ua((s16)raw);
+	return 0;
+}
+
+static int qg_read_ocv(struct qcom_qg *qg, unsigned int reg,
+		       unsigned int *ocv_uv)
+{
+	u16 raw;
+	int ret;
+
+	ret = qg_read16(qg, reg, &raw);
+	if (ret)
+		return ret;
+
+	*ocv_uv = qg_raw_to_uv(raw);
+	return 0;
+}
+
+static int qg_read_temperature(struct qcom_qg *qg, int *temp_decidegc)
+{
+	int ret, val;
+
+	lockdep_assert_not_held(&qg->lock);
+
+	if (!qg->batt_therm)
+		return -ENODEV;
+
+	ret = iio_read_channel_processed(qg->batt_therm, &val);
+	if (ret < 0)
+		return ret;
+
+	/* SPMI ADC5 bat_therm channel returns millidegrees */
+	*temp_decidegc = DIV_ROUND_CLOSEST(val, 100);
+
+	return 0;
+}
+
+static bool qg_ocv_valid(struct qcom_qg *qg)
+{
+	return qg->ocv_uv &&
+	       ktime_get_seconds() - qg->ocv_time <= QG_OCV_STALE_SECS;
+}
+
+static int qg_read_fifo_vbat(struct qcom_qg *qg)
+{
+	unsigned int count_reg;
+	unsigned int fifo_count;
+	u64 vbat_sum = 0;
+	unsigned int valid = 0;
+	u16 raw;
+	int ret, i;
+
+	guard(mutex)(&qg->lock);
+
+	ret = qg_master_hold(qg, true);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(qg->regmap, qg->base + QG_STATUS3, &count_reg);
+	if (ret)
+		goto release;
+
+	fifo_count = FIELD_GET(QG_STATUS3_FIFO_RT_COUNT, count_reg);
+	fifo_count = min_t(unsigned int, fifo_count, QG_MAX_FIFO_LENGTH);
+
+	for (i = 0; i < fifo_count; i++) {
+		ret = qg_read16(qg, QG_V_FIFO0 + i * 2, &raw);
+		if (ret)
+			goto release;
+
+		if (raw == QG_FIFO_RESET_VAL)
+			continue;
+
+		vbat_sum += qg_raw_to_uv(raw);
+		valid++;
+	}
+
+	if (valid)
+		qg->vbat_uv = div_u64(vbat_sum, valid);
+
+release:
+	qg_master_hold(qg, false);
+	return ret;
+}
+
+static int qg_update_ocv(struct qcom_qg *qg)
+{
+	unsigned int status2;
+	unsigned int ocv_uv;
+	int ret;
+
+	lockdep_assert_held(&qg->lock);
+
+	ret = regmap_read(qg->regmap, qg->base + QG_STATUS2, &status2);
+	if (ret)
+		return ret;
+
+	/* Clear the sticky register by writing back */
+	regmap_write(qg->regmap, qg->base + QG_STATUS2, 0);
+
+	if (!(status2 & QG_STATUS2_GOOD_OCV))
+		return 0;
+
+	ret = qg_read_ocv(qg, QG_S3_GOOD_OCV_V, &ocv_uv);
+	if (ret)
+		return ret;
+
+	qg->ocv_uv = ocv_uv;
+	qg->ocv_time = ktime_get_seconds();
+
+	return 0;
+}
+
+static int qg_get_capacity(struct qcom_qg *qg, int *capacity)
+{
+	int temp_decidegc;
+	int cap, ret;
+
+	if (!qg->batt_info)
+		return -ENODATA;
+
+	/*
+	 * Without OCV, it's impossible to calculate state-of-charge.
+	 * Well, you could try, but you'd get *massive* load swings.
+	 */
+	if (!qg->ocv_uv)
+		return -ENODATA;
+
+	ret = qg_read_temperature(qg, &temp_decidegc);
+	if (ret)
+		temp_decidegc = 250; /* default 25.0 degC */
+
+	cap = power_supply_batinfo_ocv2cap(qg->batt_info, qg->ocv_uv,
+					   temp_decidegc);
+	if (cap < 0)
+		return cap;
+
+	*capacity = clamp(cap, 0, 100);
+
+	return 0;
+}
+
+static int qg_get_charge_full(struct qcom_qg *qg, int *charge_uah)
+{
+	u8 valid;
+	__le16 raw;
+	int ret;
+
+	if (!qg->sdam)
+		return -ENODATA;
+
+	ret = nvmem_device_read(qg->sdam, QG_SDAM_VALID_OFFSET, 1, &valid);
+	if (ret < 0)
+		return ret;
+	if (ret != 1 || !valid)
+		return -ENODATA;
+
+	ret = nvmem_device_read(qg->sdam, QG_SDAM_LEARNED_CAPACITY_OFFSET,
+				sizeof(raw), &raw);
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(raw))
+		return -EIO;
+
+	/* 0 mAh means no full-charge learning cycle has completed yet */
+	if (!le16_to_cpu(raw))
+		return -ENODATA;
+
+	*charge_uah = le16_to_cpu(raw) * 1000;
+
+	return 0;
+}
+
+static int qg_get_health(struct qcom_qg *qg, int *health)
+{
+	struct power_supply_battery_info *info = qg->batt_info;
+	int temp_decidegc;
+	int ret;
+
+	if (!qg->batt_present) {
+		*health = POWER_SUPPLY_HEALTH_UNKNOWN;
+		return 0;
+	}
+
+	if (info) {
+		ret = qg_read_temperature(qg, &temp_decidegc);
+
+		/* No thermistor is not a health fault */
+		if (ret && ret != -ENODEV)
+			return ret;
+
+		if (!ret) {
+			/* Unset thresholds stay 0; 0 degC is a valid reading. */
+			if (info->temp_alert_max > 0 &&
+			    temp_decidegc >= info->temp_alert_max) {
+				*health = POWER_SUPPLY_HEALTH_HOT;
+				return 0;
+			}
+			if (info->temp_alert_min > 0 &&
+			    temp_decidegc <= info->temp_alert_min) {
+				*health = POWER_SUPPLY_HEALTH_COLD;
+				return 0;
+			}
+			if (info->temp_max > 0 &&
+			    temp_decidegc >= info->temp_max) {
+				*health = POWER_SUPPLY_HEALTH_WARM;
+				return 0;
+			}
+			if (info->temp_min > 0 &&
+			    temp_decidegc <= info->temp_min) {
+				*health = POWER_SUPPLY_HEALTH_COOL;
+				return 0;
+			}
+		}
+	}
+
+	*health = POWER_SUPPLY_HEALTH_GOOD;
+
+	if (info && qg->vbat_uv) {
+		if (qg->vbat_uv < info->voltage_min_design_uv)
+			*health = POWER_SUPPLY_HEALTH_DEAD;
+		else if (qg->vbat_uv > info->voltage_max_design_uv)
+			*health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	}
+
+	return 0;
+}
+
+static irqreturn_t qg_fifo_done_irq(int irq, void *data)
+{
+	struct qcom_qg *qg = data;
+	int ret;
+
+	ret = qg_read_fifo_vbat(qg);
+	if (ret)
+		dev_warn_once(qg->dev, "Failed to read FIFO: %d\n", ret);
+
+	power_supply_changed(qg->psy);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t qg_good_ocv_irq(int irq, void *data)
+{
+	struct qcom_qg *qg = data;
+
+	scoped_guard(mutex, &qg->lock)
+		qg_update_ocv(qg);
+
+	power_supply_changed(qg->psy);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t qg_batt_missing_irq(int irq, void *data)
+{
+	struct qcom_qg *qg = data;
+	bool present;
+
+	if (!qg_read_battery_present(qg, &present))
+		qg->batt_present = present;
+
+	power_supply_changed(qg->psy);
+
+	return IRQ_HANDLED;
+}
+
+static int qg_get_property(struct power_supply *psy,
+			   enum power_supply_property psp,
+			   union power_supply_propval *val)
+{
+	struct qcom_qg *qg = power_supply_get_drvdata(psy);
+	int ret;
+
+	/*
+	 * Not locking the mutex as we can't hold locks across the
+	 * temperature ADC read. No writes here, should be fine.
+	 */
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = qg->batt_present;
+		return 0;
+
+	case POWER_SUPPLY_PROP_STATUS:
+		if (!qg->batt_present) {
+			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+			return 0;
+		}
+		ret = power_supply_am_i_supplied(psy);
+		if (ret < 0 && ret != -ENODEV)
+			return ret;
+		if (ret > 0)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		return 0;
+
+	case POWER_SUPPLY_PROP_HEALTH:
+		return qg_get_health(qg, &val->intval);
+
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		if (!qg->batt_present)
+			return -ENODATA;
+		/* Refresh from last ADC register */
+		ret = qg_read_vbat(qg, &qg->vbat_uv);
+		if (ret)
+			return ret;
+		val->intval = qg->vbat_uv;
+		return 0;
+
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		if (!qg->batt_present)
+			return -ENODATA;
+		ret = qg_read_ibat(qg, &qg->ibat_ua);
+		if (ret)
+			return ret;
+		val->intval = qg->ibat_ua;
+		return 0;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_OCV:
+		if (!qg->batt_present)
+			return -ENODATA;
+		if (!qg_ocv_valid(qg))
+			return -ENODATA;
+		val->intval = qg->ocv_uv;
+		return 0;
+
+	case POWER_SUPPLY_PROP_CAPACITY:
+		if (!qg->batt_present)
+			return -ENODATA;
+		return qg_get_capacity(qg, &val->intval);
+
+	case POWER_SUPPLY_PROP_TEMP:
+		if (!qg->batt_present)
+			return -ENODATA;
+		return qg_read_temperature(qg, &val->intval);
+
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+		return 0;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		if (!qg->batt_info)
+			return -ENODATA;
+		val->intval = qg->batt_info->voltage_max_design_uv;
+		return 0;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		if (!qg->batt_info)
+			return -ENODATA;
+		val->intval = qg->batt_info->voltage_min_design_uv;
+		return 0;
+
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		if (!qg->batt_info ||
+		    qg->batt_info->charge_full_design_uah < 0)
+			return -ENODATA;
+		val->intval = qg->batt_info->charge_full_design_uah;
+		return 0;
+
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
+		return qg_get_charge_full(qg, &val->intval);
+
+	case POWER_SUPPLY_PROP_VOLTAGE_AVG: {
+		u16 raw;
+
+		if (!qg->batt_present)
+			return -ENODATA;
+		ret = qg_read16(qg, QG_S2_NORMAL_AVG_V, &raw);
+		if (ret)
+			return ret;
+		val->intval = qg_raw_to_uv(raw);
+		return 0;
+	}
+
+	case POWER_SUPPLY_PROP_CURRENT_AVG: {
+		u16 raw;
+
+		if (!qg->batt_present)
+			return -ENODATA;
+		ret = qg_read16(qg, QG_S2_NORMAL_AVG_I, &raw);
+		if (ret)
+			return ret;
+		val->intval = qg_raw_to_ua((s16)raw);
+		return 0;
+	}
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static enum power_supply_property qg_properties[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_AVG,
+	POWER_SUPPLY_PROP_VOLTAGE_OCV,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_TEMP,
+};
+
+static const struct power_supply_desc qg_psy_desc = {
+	.name = "qcom-qg",
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.properties = qg_properties,
+	.num_properties = ARRAY_SIZE(qg_properties),
+	.get_property = qg_get_property,
+};
+
+static int qg_get_optional_irq(struct qcom_qg *qg, struct platform_device *pdev,
+			       const char *name, irq_handler_t handler)
+{
+	int irq, ret;
+
+	irq = platform_get_irq_byname_optional(pdev, name);
+	if (irq < 0)
+		return irq;
+
+	ret = devm_request_threaded_irq(qg->dev, irq, NULL, handler,
+					IRQF_ONESHOT, name, qg);
+	if (ret)
+		return dev_err_probe(qg->dev, ret,
+				     "Failed to request %s IRQ", name);
+
+	return 0;
+}
+
+static void qg_batt_info_cleanup(void *data)
+{
+	struct qcom_qg *qg = data;
+
+	power_supply_put_battery_info(qg->psy, qg->batt_info);
+}
+
+static int qcom_qg_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct qcom_qg *qg;
+	struct power_supply_config psy_cfg = {};
+	unsigned int perph_type;
+	int ret;
+
+	qg = devm_kzalloc_obj(dev, struct qcom_qg);
+	if (!qg)
+		return -ENOMEM;
+
+	qg->dev = dev;
+	mutex_init(&qg->lock);
+
+	qg->regmap = dev_get_regmap(dev->parent, NULL);
+	if (!qg->regmap)
+		return dev_err_probe(dev, -ENODEV,
+				     "Failed to get parent regmap");
+
+	ret = device_property_read_u32(dev, "reg", &qg->base);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read reg property");
+
+	ret = regmap_read(qg->regmap, qg->base + QG_PERPH_TYPE, &perph_type);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to read peripheral type");
+
+	if (perph_type != QG_PERPH_TYPE_VALUE)
+		return dev_err_probe(dev, -ENODEV,
+				     "Unexpected peripheral type: 0x%02x",
+				     perph_type);
+
+	ret = qg_read_battery_present(qg, &qg->batt_present);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to read battery presence");
+
+	ret = qg_read_ocv(qg, QG_S7_PON_OCV_V, &qg->ocv_uv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read PON OCV");
+
+	qg->ocv_time = ktime_get_seconds();
+
+	ret = qg_read_vbat(qg, &qg->vbat_uv);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to read battery voltage");
+
+	qg->batt_therm = devm_iio_channel_get(dev, "batt-therm");
+	if (IS_ERR(qg->batt_therm)) {
+		ret = PTR_ERR(qg->batt_therm);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		dev_dbg(dev, "No batt-therm IIO channel: %d\n", ret);
+		qg->batt_therm = NULL;
+	}
+
+	qg->sdam = devm_nvmem_device_get(dev, NULL);
+	if (IS_ERR(qg->sdam)) {
+		ret = PTR_ERR(qg->sdam);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		dev_dbg(dev, "No SDAM nvmem device: %d\n", ret);
+		qg->sdam = NULL;
+	}
+
+	psy_cfg.drv_data = qg;
+	psy_cfg.fwnode = dev_fwnode(dev);
+
+	qg->psy = devm_power_supply_register(dev, &qg_psy_desc, &psy_cfg);
+	if (IS_ERR(qg->psy))
+		return dev_err_probe(dev, PTR_ERR(qg->psy),
+				     "Failed to register power supply");
+
+	ret = power_supply_get_battery_info(qg->psy, &qg->batt_info);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get battery info");
+
+	ret = devm_add_action_or_reset(dev, qg_batt_info_cleanup, qg);
+	if (ret)
+		return ret;
+
+	ret = qg_get_optional_irq(qg, pdev, "fifo-done", qg_fifo_done_irq);
+	if (ret && ret != -ENXIO)
+		return ret;
+
+	ret = qg_get_optional_irq(qg, pdev, "good-ocv", qg_good_ocv_irq);
+	if (ret && ret != -ENXIO)
+		return ret;
+
+	ret = qg_get_optional_irq(qg, pdev, "batt-missing",
+				  qg_batt_missing_irq);
+	if (ret && ret != -ENXIO)
+		return ret;
+
+	platform_set_drvdata(pdev, qg);
+
+	dev_dbg(dev, "QG: batt=%s OCV=%u uV VBAT=%u uV\n",
+		qg->batt_present ? "present" : "absent",
+		qg->ocv_uv, qg->vbat_uv);
+
+	return 0;
+}
+
+static int qcom_qg_suspend(struct device *dev)
+{
+	struct qcom_qg *qg = dev_get_drvdata(dev);
+
+	/*
+	 * The QG hardware FSM will automatically transition to sleep
+	 * states (S3) and measure OCV. Nothing special is needed here
+	 * beyond ensuring we pick up the new OCV on resume.
+	 */
+	dev_dbg(qg->dev, "Suspending, last OCV=%u uV\n", qg->ocv_uv);
+
+	return 0;
+}
+
+static int qcom_qg_resume(struct device *dev)
+{
+	struct qcom_qg *qg = dev_get_drvdata(dev);
+
+	/*
+	 * After resume, the hardware may have measured a good OCV during
+	 * the sleep state. Check for it and update our cached value.
+	 * Also refresh the battery voltage.
+	 */
+	scoped_guard(mutex, &qg->lock)
+		qg_update_ocv(qg);
+
+	qg_read_vbat(qg, &qg->vbat_uv);
+
+	dev_dbg(qg->dev, "Resumed, OCV=%u uV VBAT=%u uV\n",
+		qg->ocv_uv, qg->vbat_uv);
+
+	power_supply_changed(qg->psy);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(qcom_qg_pm_ops,
+				qcom_qg_suspend, qcom_qg_resume);
+
+static const struct of_device_id qcom_qg_of_match[] = {
+	{ .compatible = "qcom,pm6150-qg" },
+	{ .compatible = "qcom,pmi632-qg" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_qg_of_match);
+
+static struct platform_driver qcom_qg_driver = {
+	.driver = {
+		.name = "qcom-qg",
+		.of_match_table = qcom_qg_of_match,
+		.pm = pm_sleep_ptr(&qcom_qg_pm_ops),
+	},
+	.probe = qcom_qg_probe,
+};
+module_platform_driver(qcom_qg_driver);
+
+MODULE_DESCRIPTION("Qualcomm QGauge fuel gauge driver");
+MODULE_AUTHOR("Marc Lainez <marc.lainez@gmail.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_CONSUMER");
-- 
2.56.0

Re: [PATCH 2/5] power: supply: add Qualcomm QGauge (qcom_qg) fuel gauge driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] power: supply: qcom_qg: UAF of batt_info in qcom_qg_probe()
  teardown
- [Medium] power: supply: qcom_qg: temperature unit mismatch in
  qg_get_capacity()
- [Medium] power: supply: qcom_qg: broken temperature threshold checks
  in qg_get_health()
- [Medium] power: supply: qcom_qg: unhandled -EINVAL design voltage
  limits in health/props
--

commit 23cbeedd29303e2809a8e496524a3641be8ea9bc
Author: Marc Lainez <marc.lainez@gmail.com>

power: supply: add Qualcomm QGauge (qcom_qg) fuel gauge driver

This commit adds a power_supply driver for the Qualcomm QGauge (QG) fuel
gauge hardware block found in SPMI PMICs such as PMI632 and PM6150.

> diff --git a/drivers/power/supply/qcom_qg.c b/drivers/power/supply/qcom_qg.c
> new file mode 100644
> index 0000000000000..071e29d4c2991
> --- /dev/null
> +++ b/drivers/power/supply/qcom_qg.c

[ ... ]

> +static int qg_get_capacity(struct qcom_qg *qg, int *capacity)
> +{
> +	int temp_decidegc;
> +	int cap, ret;
> +
> +	if (!qg->batt_info)
> +		return -ENODATA;
> +
> +	/*
> +	 * Without OCV, it's impossible to calculate state-of-charge.
> +	 * Well, you could try, but you'd get *massive* load swings.
> +	 */
> +	if (!qg->ocv_uv)
> +		return -ENODATA;
> +
> +	ret = qg_read_temperature(qg, &temp_decidegc);
> +	if (ret)
> +		temp_decidegc = 250; /* default 25.0 degC */
> +
> +	cap = power_supply_batinfo_ocv2cap(qg->batt_info, qg->ocv_uv,
> +					   temp_decidegc);

[Severity: Medium]
Does passing temp_decidegc to power_supply_batinfo_ocv2cap() use the wrong
temperature unit?

qg_read_temperature() and the fallback value return tenths of a degree
Celsius (e.g. 250 for 25.0 degC), whereas power_supply_get_battery_info()
populates info->ocv_temp[] from ocv-capacity-celsius in whole degrees
Celsius.  When multiple temperature OCV tables are defined,
power_supply_find_ocv2cap_table() compares decidegrees against degrees and
selects the wrong table.

[ ... ]

> +static int qg_get_health(struct qcom_qg *qg, int *health)
> +{
> +	struct power_supply_battery_info *info = qg->batt_info;
> +	int temp_decidegc;
> +	int ret;
> +
> +	if (!qg->batt_present) {
> +		*health = POWER_SUPPLY_HEALTH_UNKNOWN;
> +		return 0;
> +	}
> +
> +	if (info) {
> +		ret = qg_read_temperature(qg, &temp_decidegc);
> +
> +		/* No thermistor is not a health fault */
> +		if (ret && ret != -ENODEV)
> +			return ret;
> +
> +		if (!ret) {
> +			/* Unset thresholds stay 0; 0 degC is a valid reading. */
> +			if (info->temp_alert_max > 0 &&
> +			    temp_decidegc >= info->temp_alert_max) {
> +				*health = POWER_SUPPLY_HEALTH_HOT;
> +				return 0;
> +			}
> +			if (info->temp_alert_min > 0 &&
> +			    temp_decidegc <= info->temp_alert_min) {
> +				*health = POWER_SUPPLY_HEALTH_COLD;
> +				return 0;
> +			}
> +			if (info->temp_max > 0 &&
> +			    temp_decidegc >= info->temp_max) {
> +				*health = POWER_SUPPLY_HEALTH_WARM;
> +				return 0;
> +			}
> +			if (info->temp_min > 0 &&
> +			    temp_decidegc <= info->temp_min) {
> +				*health = POWER_SUPPLY_HEALTH_COOL;
> +				return 0;
> +			}
> +		}
> +	}

[Severity: Medium]
Are the temperature threshold checks in qg_get_health() comparing
decidegrees against whole degrees and using the wrong sentinel values?

power_supply_get_battery_info() initializes unset info->temp_alert_min and
info->temp_min to INT_MIN and unset info->temp_alert_max and info->temp_max
to INT_MAX, and populates them from alert-celsius and
operating-range-celsius in whole degrees Celsius.

Because temp_decidegc is in tenths of a degree Celsius and unset max limits
are INT_MAX (which satisfies > 0), any room-temperature reading (e.g. 250
for 25.0 degC) is compared directly against a whole-degree limit (e.g. 45)
and falsely reports POWER_SUPPLY_HEALTH_HOT.  Checking > 0 on
temp_alert_min and temp_min also ignores any cold threshold at or below
0 degC.  In addition, temp_min and temp_max represent the outer operating
range (COLD/HOT) while temp_alert_min and temp_alert_max are the inner
alert thresholds (COOL/WARM), so their health states are swapped here.

> +
> +	*health = POWER_SUPPLY_HEALTH_GOOD;
> +
> +	if (info && qg->vbat_uv) {
> +		if (qg->vbat_uv < info->voltage_min_design_uv)
> +			*health = POWER_SUPPLY_HEALTH_DEAD;
> +		else if (qg->vbat_uv > info->voltage_max_design_uv)
> +			*health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +	}
> +
> +	return 0;
> +}

[ ... ]

> +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> +		if (!qg->batt_info)
> +			return -ENODATA;
> +		val->intval = qg->batt_info->voltage_max_design_uv;
> +		return 0;
> +
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> +		if (!qg->batt_info)
> +			return -ENODATA;
> +		val->intval = qg->batt_info->voltage_min_design_uv;
> +		return 0;

[Severity: Medium]
What happens in qg_get_health() and qg_get_property() when
voltage-min-design-microvolt or voltage-max-design-microvolt is omitted from
the battery node?

power_supply_get_battery_info() initializes info->voltage_min_design_uv and
info->voltage_max_design_uv to -EINVAL.  In qg_get_health(), comparing
unsigned int qg->vbat_uv against signed int info->voltage_min_design_uv
promotes -EINVAL to UINT_MAX - 21, so qg->vbat_uv <
info->voltage_min_design_uv is always true and reports
POWER_SUPPLY_HEALTH_DEAD.  In qg_get_property(),
POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN and
POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN return 0 with val->intval = -22 instead
of returning -ENODATA when those fields are negative (unlike
POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN just below).

[ ... ]

> +static void qg_batt_info_cleanup(void *data)
> +{
> +	struct qcom_qg *qg = data;
> +
> +	power_supply_put_battery_info(qg->psy, qg->batt_info);
> +}
> +
> +static int qcom_qg_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	qg->psy = devm_power_supply_register(dev, &qg_psy_desc, &psy_cfg);
> +	if (IS_ERR(qg->psy))
> +		return dev_err_probe(dev, PTR_ERR(qg->psy),
> +				     "Failed to register power supply");
> +
> +	ret = power_supply_get_battery_info(qg->psy, &qg->batt_info);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get battery info");
> +
> +	ret = devm_add_action_or_reset(dev, qg_batt_info_cleanup, qg);
> +	if (ret)
> +		return ret;

[Severity: High]
Can qg_get_property() dereference qg->batt_info after it has been freed
during driver unbind or probe unwind?

devm_add_action_or_reset(dev, qg_batt_info_cleanup, qg) is registered on
dev after devm_power_supply_register().  Because devres releases resources
in reverse order, on driver unbind or if any of the subsequent
qg_get_optional_irq() calls fails, qg_batt_info_cleanup() frees
qg->batt_info and its ocv_table[] arrays while qg->psy is still registered
and qg->batt_info remains non-NULL.  Any sysfs read or pending uevent work
flushed during power_supply_unregister() then calls qg_get_property() and
reads freed memory.  Calling power_supply_get_battery_info() after
devm_power_supply_register() also leaves qg->batt_info NULL while qg->psy
is first exposed to userspace and thermal/hwmon consumers.

Note that because qg_psy_desc.type is POWER_SUPPLY_TYPE_BATTERY,
__power_supply_register() already populates psy->battery_info before
device_add() and frees it when psy->dev is unregistered, or if
qg->batt_info is kept, its cleanup action only runs safely if unregistered
after qg->psy (or if qg->batt_info is allocated on &qg->psy->dev without a
separate dev action that runs before power_supply_unregister()).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112170841.2189322-1-val@packett.cool?part=2

[PATCH 3/5] iio: adc: qcom-spmi-adc5: Add ADC5_BAT_THERM_100K_PU

[Val Packett]

Even though it existed in bindings for the longest time,
ADC5_BAT_THERM_100K_PU was never assigned in the driver. Do so.

Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/iio/adc/qcom-spmi-adc5.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
index d57ba8526e0c..c27ff89429f5 100644
--- a/drivers/iio/adc/qcom-spmi-adc5.c
+++ b/drivers/iio/adc/qcom-spmi-adc5.c
@@ -542,6 +542,8 @@ static const struct adc5_channels adc5_chans_pmic[ADC5_MAX_CHANNEL] = {
 					SCALE_HW_CALIB_DEFAULT)
 	[ADC5_XO_THERM_100K_PU]	= ADC5_CHAN_TEMP("xo_therm", 0,
 					SCALE_HW_CALIB_XOTHERM)
+	[ADC5_BAT_THERM_100K_PU]	= ADC5_CHAN_TEMP("bat_therm", 0,
+					SCALE_HW_CALIB_THERM_100K_PULLUP)
 	[ADC5_BAT_ID_100K_PU]	= ADC5_CHAN_TEMP("bat_id", 0,
 					SCALE_HW_CALIB_DEFAULT)
 	[ADC5_AMUX_THM1_100K_PU] = ADC5_CHAN_TEMP("amux_thm1_100k_pu", 0,
-- 
2.56.0

[PATCH 4/5] arm64: dts: qcom: pmi632: Add missing bat_therm ADC channel

[Val Packett]

Add the temperature channel that is used with the battery fuel gauge.

Signed-off-by: Val Packett <val@packett.cool>
---
 arch/arm64/boot/dts/qcom/pmi632.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pmi632.dtsi b/arch/arm64/boot/dts/qcom/pmi632.dtsi
index ab850050eb22..82b5d7b7bb5b 100644
--- a/arch/arm64/boot/dts/qcom/pmi632.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmi632.dtsi
@@ -124,6 +124,14 @@ channel@9 {
 				label = "chg_temp";
 			};
 
+			channel@4a {
+				reg = <ADC5_BAT_THERM_100K_PU>;
+				qcom,hw-settle-time = <200>;
+				qcom,pre-scaling = <1 1>;
+				qcom,ratiometric;
+				label = "bat_therm";
+			};
+
 			channel@4b {
 				reg = <ADC5_BAT_ID_100K_PU>;
 				qcom,hw-settle-time = <200>;
-- 
2.56.0

[PATCH 5/5] arm64: dts: qcom: pmi632: Add battery fuel gauge node

[Val Packett]

Add a node for the "qgauge" battery fuel gauge at 0x4800, and for the
SDAM region at 0xb100 which contains the full charge capacity.

Signed-off-by: Val Packett <val@packett.cool>
---
 arch/arm64/boot/dts/qcom/pmi632.dtsi | 33 ++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pmi632.dtsi b/arch/arm64/boot/dts/qcom/pmi632.dtsi
index 82b5d7b7bb5b..5e9e82563884 100644
--- a/arch/arm64/boot/dts/qcom/pmi632.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmi632.dtsi
@@ -163,11 +163,44 @@ pmi632_adc_tm: adc-tm@3500 {
 			status = "disabled";
 		};
 
+		pmi632_qgauge: fuel-gauge@4800 {
+			compatible = "qcom,pmi632-qg";
+
+			reg = <0x4800>;
+
+			interrupts = <0x2 0x48 0x0 IRQ_TYPE_EDGE_BOTH>,
+				     <0x2 0x48 0x1 IRQ_TYPE_EDGE_BOTH>,
+				     <0x2 0x48 0x2 IRQ_TYPE_EDGE_RISING>,
+				     <0x2 0x48 0x3 IRQ_TYPE_EDGE_RISING>,
+				     <0x2 0x48 0x4 IRQ_TYPE_EDGE_RISING>,
+				     <0x2 0x48 0x5 IRQ_TYPE_EDGE_RISING>,
+				     <0x2 0x48 0x6 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "batt-missing",
+					  "vbat-low",
+					  "vbat-empty",
+					  "fifo-done",
+					  "good-ocv",
+					  "fsm-state-chg",
+					  "event";
+
+			nvmem = <&pmi632_sdam_2>;
+
+			status = "disabled";
+		};
+
 		pmi632_pbs_client3: pbs@7400 {
 			compatible = "qcom,pmi632-pbs", "qcom,pbs";
 			reg = <0x7400>;
 		};
 
+		pmi632_sdam_2: nvram@b100 {
+			compatible = "qcom,spmi-sdam";
+			reg = <0xb100>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 0xb100 0x100>;
+		};
+
 		pmi632_sdam_7: nvram@b600 {
 			compatible = "qcom,spmi-sdam";
 			reg = <0xb600>;
-- 
2.56.0

Re: [PATCH 5/5] arm64: dts: qcom: pmi632: Add battery fuel gauge node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] arm64: dts: qcom: pmi632: fuel-gauge@4800 violates
  qcom,spmi-pmic.yaml schema
- [Low] arm64: dts: qcom: pmi632: missing io-channels in pmi632_qgauge
  node
--

commit 15ccef39df06d430c96a1c58092319c5e86db8a5
Author: Val Packett <val@packett.cool>

arm64: dts: qcom: pmi632: Add battery fuel gauge node

This commit adds devicetree nodes for the PMI632 qgauge battery fuel gauge
at 0x4800 and the SDAM region at 0xb100.

> diff --git a/arch/arm64/boot/dts/qcom/pmi632.dtsi b/arch/arm64/boot/dts/qcom/pmi632.dtsi
> index 85cef103e00dc..f04d65fb7aabb 100644
> --- a/arch/arm64/boot/dts/qcom/pmi632.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pmi632.dtsi
> @@ -163,11 +163,44 @@ pmi632_adc_tm: adc-tm@3500 {
>  			status = "disabled";
>  		};
>  
> +		pmi632_qgauge: fuel-gauge@4800 {
> +			compatible = "qcom,pmi632-qg";

[Severity: Low]
Does adding fuel-gauge@4800 under pmic@2 break dtbs_check against
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml?

That schema matches "qcom,pmi632" and sets additionalProperties: false,
but its patternProperties do not list "^fuel-gauge@[0-9a-f]+$" referencing
/schemas/power/supply/qcom,qg.yaml#.

> +
> +			reg = <0x4800>;
> +
> +			interrupts = <0x2 0x48 0x0 IRQ_TYPE_EDGE_BOTH>,
> +				     <0x2 0x48 0x1 IRQ_TYPE_EDGE_BOTH>,
> +				     <0x2 0x48 0x2 IRQ_TYPE_EDGE_RISING>,
> +				     <0x2 0x48 0x3 IRQ_TYPE_EDGE_RISING>,
> +				     <0x2 0x48 0x4 IRQ_TYPE_EDGE_RISING>,
> +				     <0x2 0x48 0x5 IRQ_TYPE_EDGE_RISING>,
> +				     <0x2 0x48 0x6 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-names = "batt-missing",
> +					  "vbat-low",
> +					  "vbat-empty",
> +					  "fifo-done",
> +					  "good-ocv",
> +					  "fsm-state-chg",
> +					  "event";
> +
> +			nvmem = <&pmi632_sdam_2>;
> +
> +			status = "disabled";
> +		};

[Severity: Low]
Should pmi632_qgauge also wire up io-channels = <&pmi632_adc
ADC5_BAT_THERM_100K_PU> and io-channel-names = "batt-therm" here?

Earlier in this series, commit "arm64: dts: qcom: pmi632: Add missing
bat_therm ADC channel" added channel@4a to pmi632_adc for the fuel gauge.
Without io-channels on pmi632_qgauge (or in every board DTS),
qcom_qg_probe() leaves qg->batt_therm NULL, causing temperature reads to
fail with -ENODEV, qg_get_capacity() to fall back to a hardcoded 25 C, and
qg_get_health() to skip temperature fault checks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112170841.2189322-1-val@packett.cool?part=5