[PATCH v2 0/8] power: supply: Add several features support in qcom-battmgr driver

[PATCH v2 0/8] power: supply: Add several features support in qcom-battmgr driver

[Fenglin Wu via B4 Relay]

Add following features in qcom-battmgr drivers as the battery management
firmware has provided such capabilities:
 - Add resistance power supply property in core driver and qcom-battmgr
   driver to get battery resistance
 - Add state_of_health power supply property in core driver and
   qcom-battmgr driver to get battery health percentage
 - Add charge control start/end threshold control by using
   charge_control_start_threshold and charge_control_end_threshold power
   supply properties

The changes have been tested on QRD8650 and X1E80100-CRD devices based on
qcom/linux.git for-next commit f8d04825b12f42ec8198dee1ab4654792f9ac231.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
Changes in v2:
- Corrected "qcom-battmgr" to "qcom_battmgr" in the commit subject of
  patch 4/5.
- Added charge control support for X1E80100 platform in patch 5.
- X1E80100 is no longer a fallback of SM8550 in pmic-glink battmgr support,
  hence added patch 6 in the pmic-glink binding to move X1E80100 out of the
  fallbacks.
- Added patch 7 in glink-ucsi driver to include UCSI quirk for X1E80100
  platform
- Added patch 8 to remove "qcom,sm8550-pmic-glink" compatible string in
  x1* board files.
- Rebased the changes on qcom/linux.git for-next commit 44ef9ab4baaf496d227ab98d368016700f0b9300.
- Link to v1: https://lore.kernel.org/r/20250523-qcom_battmgr_update-v1-0-2bb6d4e0a56e@oss.qualcomm.com

---
Fenglin Wu (8):
      power: supply: core: Add resistance power supply property
      power: supply: core: Add state_of_health power supply property
      power: supply: qcom_battmgr: Add resistance power supply property
      power: supply: qcom_battmgr: Add state_of_health property
      power: supply: qcom_battmgr: Add charge control support
      dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks
      usb: typec: ucsi_glink: Add UCSI quirk for X1E80100 platform
      arm64: dts: qcom: x1*: Remove qcom,sm8550-pmic-glink fallback

 Documentation/ABI/testing/sysfs-class-power        |  20 ++
 .../bindings/soc/qcom/qcom,pmic-glink.yaml         |   4 +-
 arch/arm64/boot/dts/qcom/x1-crd.dtsi               |   1 -
 arch/arm64/boot/dts/qcom/x1e001de-devkit.dts       |   1 -
 .../dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi    |   1 -
 .../boot/dts/qcom/x1e80100-asus-vivobook-s15.dts   |   1 -
 .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts     |   1 -
 .../boot/dts/qcom/x1e80100-hp-omnibook-x14.dts     |   1 -
 .../boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts  |   1 -
 .../boot/dts/qcom/x1e80100-microsoft-romulus.dtsi  |   1 -
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts          |   1 -
 drivers/power/supply/power_supply_sysfs.c          |   2 +
 drivers/power/supply/qcom_battmgr.c                | 275 ++++++++++++++++++++-
 drivers/usb/typec/ucsi/ucsi_glink.c                |   1 +
 include/linux/power_supply.h                       |   2 +
 15 files changed, 295 insertions(+), 18 deletions(-)
---
base-commit: abbf1025002e4966bfcbf8a069234e485d49edf1
change-id: 20250520-qcom_battmgr_update-3561dc526c05

Best regards,
-- 
Fenglin Wu <fenglin.wu@oss.qualcomm.com>

[PATCH v2 1/8] power: supply: core: Add resistance power supply property

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Some battery drivers provide the ability to export resistance as a
parameter. Add resistance power supply property for that purpose.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 Documentation/ABI/testing/sysfs-class-power | 10 ++++++++++
 drivers/power/supply/power_supply_sysfs.c   |  1 +
 include/linux/power_supply.h                |  1 +
 3 files changed, 12 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 560124cc31770cde03bcdbbba0d85a5bd78b15a0..22a565a6a1c509461b8c483e12975295765121d6 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -552,6 +552,16 @@ Description:
 			Integer > 0: representing full cycles
 			Integer = 0: cycle_count info is not available
 
+What:		/sys/class/power_supply/<supply_name>/resistance
+Date:		May 2025
+Contact:	linux-arm-msm@vger.kernel.org
+Description:
+		Reports the resistance of the battery power supply.
+
+		Access: Read
+
+		Valid values: Represented in microohms
+
 **USB Properties**
 
 What:		/sys/class/power_supply/<supply_name>/input_current_limit
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index a438f7983d4f6a832e9d479184c7c35453e1757c..dd829148eb6fda5dcd7eab53fc70f99081763714 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -220,6 +220,7 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
 	POWER_SUPPLY_ATTR(MANUFACTURE_YEAR),
 	POWER_SUPPLY_ATTR(MANUFACTURE_MONTH),
 	POWER_SUPPLY_ATTR(MANUFACTURE_DAY),
+	POWER_SUPPLY_ATTR(RESISTANCE),
 	/* Properties of type `const char *' */
 	POWER_SUPPLY_ATTR(MODEL_NAME),
 	POWER_SUPPLY_ATTR(MANUFACTURER),
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index c4cb854971f53a244ba7742a15ce7a5515da6199..de3e88810e322546470b21258913abc7707c86a7 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -174,6 +174,7 @@ enum power_supply_property {
 	POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
 	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
 	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
+	POWER_SUPPLY_PROP_RESISTANCE,
 	/* Properties of type `const char *' */
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_MANUFACTURER,

-- 
2.34.1

[PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Add state_of_health power supply property to represent battery
health percentage.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 Documentation/ABI/testing/sysfs-class-power | 10 ++++++++++
 drivers/power/supply/power_supply_sysfs.c   |  1 +
 include/linux/power_supply.h                |  1 +
 3 files changed, 12 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 22a565a6a1c509461b8c483e12975295765121d6..74e0d4d67467500c3cd62da3ae0b2e4a67e77680 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -562,6 +562,16 @@ Description:
 
 		Valid values: Represented in microohms
 
+What:		/sys/class/power_supply/<supply_name>/state_of_health
+Date:		May 2025
+Contact:	linux-arm-msm@vger.kernel.org
+Description:
+		Reports battery power supply state of health in percentage.
+
+		Access: Read
+
+		Valid values: 0 - 100 (percent)
+
 **USB Properties**
 
 What:		/sys/class/power_supply/<supply_name>/input_current_limit
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index dd829148eb6fda5dcd7eab53fc70f99081763714..12af0d0398822ff23d8970f6bdc8e3ef68081a1d 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -221,6 +221,7 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
 	POWER_SUPPLY_ATTR(MANUFACTURE_MONTH),
 	POWER_SUPPLY_ATTR(MANUFACTURE_DAY),
 	POWER_SUPPLY_ATTR(RESISTANCE),
+	POWER_SUPPLY_ATTR(STATE_OF_HEALTH),
 	/* Properties of type `const char *' */
 	POWER_SUPPLY_ATTR(MODEL_NAME),
 	POWER_SUPPLY_ATTR(MANUFACTURER),
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index de3e88810e322546470b21258913abc7707c86a7..dd0108940231352ac6c6f0fa962d1ea904d81c7a 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -175,6 +175,7 @@ enum power_supply_property {
 	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
 	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
 	POWER_SUPPLY_PROP_RESISTANCE,
+	POWER_SUPPLY_PROP_STATE_OF_HEALTH,
 	/* Properties of type `const char *' */
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_MANUFACTURER,

-- 
2.34.1

[PATCH v2 3/8] power: supply: qcom_battmgr: Add resistance power supply property

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Add power supply property to get battery resistance from the battery
management firmware.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 drivers/power/supply/qcom_battmgr.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
index fe27676fbc7cd12292caa6fb3b5b46a18c426e6d..bc521f60f67fa427cc03b51c44adaeb46ae746f5 100644
--- a/drivers/power/supply/qcom_battmgr.c
+++ b/drivers/power/supply/qcom_battmgr.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved.
  * Copyright (c) 2022, Linaro Ltd
+ * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 #include <linux/auxiliary_bus.h>
 #include <linux/module.h>
@@ -254,6 +255,7 @@ struct qcom_battmgr_status {
 	unsigned int voltage_now;
 	unsigned int voltage_ocv;
 	unsigned int temperature;
+	unsigned int resistance;
 
 	unsigned int discharge_time;
 	unsigned int charge_time;
@@ -418,6 +420,7 @@ static const u8 sm8350_bat_prop_map[] = {
 	[POWER_SUPPLY_PROP_MODEL_NAME] = BATT_MODEL_NAME,
 	[POWER_SUPPLY_PROP_TIME_TO_FULL_AVG] = BATT_TTF_AVG,
 	[POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG] = BATT_TTE_AVG,
+	[POWER_SUPPLY_PROP_RESISTANCE] = BATT_RESISTANCE,
 	[POWER_SUPPLY_PROP_POWER_NOW] = BATT_POWER_NOW,
 };
 
@@ -582,6 +585,9 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_TEMP:
 		val->intval = battmgr->status.temperature;
 		break;
+	case POWER_SUPPLY_PROP_RESISTANCE:
+		val->intval = battmgr->status.resistance;
+		break;
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
 		val->intval = battmgr->status.discharge_time;
 		break;
@@ -665,6 +671,7 @@ static const enum power_supply_property sm8350_bat_props[] = {
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
 	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
+	POWER_SUPPLY_PROP_RESISTANCE,
 	POWER_SUPPLY_PROP_POWER_NOW,
 };
 
@@ -1174,6 +1181,9 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
 		case BATT_TTE_AVG:
 			battmgr->status.discharge_time = le32_to_cpu(resp->intval.value);
 			break;
+		case BATT_RESISTANCE:
+			battmgr->status.resistance = le32_to_cpu(resp->intval.value);
+			break;
 		case BATT_POWER_NOW:
 			battmgr->status.power_now = le32_to_cpu(resp->intval.value);
 			break;

-- 
2.34.1

[PATCH v2 4/8] power: supply: qcom_battmgr: Add state_of_health property

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Add state_of_health property to read battery health percentage from
battery management firmware.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 drivers/power/supply/qcom_battmgr.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
index bc521f60f67fa427cc03b51c44adaeb46ae746f5..d5d0200b92bdc3d9a22f44159ad45b152efe8be0 100644
--- a/drivers/power/supply/qcom_battmgr.c
+++ b/drivers/power/supply/qcom_battmgr.c
@@ -256,6 +256,7 @@ struct qcom_battmgr_status {
 	unsigned int voltage_ocv;
 	unsigned int temperature;
 	unsigned int resistance;
+	unsigned int soh_percent;
 
 	unsigned int discharge_time;
 	unsigned int charge_time;
@@ -421,6 +422,7 @@ static const u8 sm8350_bat_prop_map[] = {
 	[POWER_SUPPLY_PROP_TIME_TO_FULL_AVG] = BATT_TTF_AVG,
 	[POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG] = BATT_TTE_AVG,
 	[POWER_SUPPLY_PROP_RESISTANCE] = BATT_RESISTANCE,
+	[POWER_SUPPLY_PROP_STATE_OF_HEALTH] = BATT_SOH,
 	[POWER_SUPPLY_PROP_POWER_NOW] = BATT_POWER_NOW,
 };
 
@@ -588,6 +590,9 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_RESISTANCE:
 		val->intval = battmgr->status.resistance;
 		break;
+	case POWER_SUPPLY_PROP_STATE_OF_HEALTH:
+		val->intval = battmgr->status.soh_percent;
+		break;
 	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
 		val->intval = battmgr->status.discharge_time;
 		break;
@@ -672,6 +677,7 @@ static const enum power_supply_property sm8350_bat_props[] = {
 	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
 	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
 	POWER_SUPPLY_PROP_RESISTANCE,
+	POWER_SUPPLY_PROP_STATE_OF_HEALTH,
 	POWER_SUPPLY_PROP_POWER_NOW,
 };
 
@@ -1141,6 +1147,9 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
 		case BATT_CAPACITY:
 			battmgr->status.percent = le32_to_cpu(resp->intval.value) / 100;
 			break;
+		case BATT_SOH:
+			battmgr->status.soh_percent = le32_to_cpu(resp->intval.value);
+			break;
 		case BATT_VOLT_OCV:
 			battmgr->status.voltage_ocv = le32_to_cpu(resp->intval.value);
 			break;

-- 
2.34.1

[PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Add charge control support for SM8550 and X1E80100. It's supported
with below two power supply properties:

charge_control_end_threshold: SOC threshold at which the charging
should be terminated.

charge_control_start_threshold: SOC threshold at which the charging
should be resumed.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 drivers/power/supply/qcom_battmgr.c | 256 ++++++++++++++++++++++++++++++++++--
 1 file changed, 248 insertions(+), 8 deletions(-)

diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
index d5d0200b92bdc3d9a22f44159ad45b152efe8be0..39009415f4bfcd76b010305179d3c8fb847254a6 100644
--- a/drivers/power/supply/qcom_battmgr.c
+++ b/drivers/power/supply/qcom_battmgr.c
@@ -21,6 +21,8 @@
 enum qcom_battmgr_variant {
 	QCOM_BATTMGR_SM8350,
 	QCOM_BATTMGR_SC8280XP,
+	QCOM_BATTMGR_X1E80100,
+	QCOM_BATTMGR_SM8550,
 };
 
 #define BATTMGR_BAT_STATUS		0x1
@@ -66,6 +68,9 @@ enum qcom_battmgr_variant {
 #define BATT_RESISTANCE			21
 #define BATT_POWER_NOW			22
 #define BATT_POWER_AVG			23
+#define BATT_CHG_CTRL_EN		24
+#define BATT_CHG_CTRL_START_THR		25
+#define BATT_CHG_CTRL_END_THR		26
 
 #define BATTMGR_USB_PROPERTY_GET	0x32
 #define BATTMGR_USB_PROPERTY_SET	0x33
@@ -90,6 +95,13 @@ enum qcom_battmgr_variant {
 #define WLS_TYPE			5
 #define WLS_BOOST_EN			6
 
+#define BATTMGR_CHG_CTRL_LIMIT_EN	0x48
+#define CHARGE_CTRL_START_THR_MIN	50
+#define CHARGE_CTRL_START_THR_MAX	95
+#define CHARGE_CTRL_END_THR_MIN		55
+#define CHARGE_CTRL_END_THR_MAX		100
+#define CHARGE_CTRL_DELTA_SOC		5
+
 struct qcom_battmgr_enable_request {
 	struct pmic_glink_hdr hdr;
 	__le32 battery_id;
@@ -124,6 +136,13 @@ struct qcom_battmgr_discharge_time_request {
 	__le32 reserved;
 };
 
+struct qcom_battmgr_charge_ctrl_request {
+	struct pmic_glink_hdr hdr;
+	__le32 enable;
+	__le32 target_soc;
+	__le32 delta_soc;
+};
+
 struct qcom_battmgr_message {
 	struct pmic_glink_hdr hdr;
 	union {
@@ -236,6 +255,8 @@ struct qcom_battmgr_info {
 	unsigned int capacity_warning;
 	unsigned int cycle_count;
 	unsigned int charge_count;
+	unsigned int charge_ctrl_start;
+	unsigned int charge_ctrl_end;
 	char model_number[BATTMGR_STRING_LEN];
 	char serial_number[BATTMGR_STRING_LEN];
 	char oem_info[BATTMGR_STRING_LEN];
@@ -424,6 +445,8 @@ static const u8 sm8350_bat_prop_map[] = {
 	[POWER_SUPPLY_PROP_RESISTANCE] = BATT_RESISTANCE,
 	[POWER_SUPPLY_PROP_STATE_OF_HEALTH] = BATT_SOH,
 	[POWER_SUPPLY_PROP_POWER_NOW] = BATT_POWER_NOW,
+	[POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD] = BATT_CHG_CTRL_START_THR,
+	[POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD] = BATT_CHG_CTRL_END_THR,
 };
 
 static int qcom_battmgr_bat_sm8350_update(struct qcom_battmgr *battmgr,
@@ -494,7 +517,8 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
 	if (!battmgr->service_up)
 		return -EAGAIN;
 
-	if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)
 		ret = qcom_battmgr_bat_sc8280xp_update(battmgr, psp);
 	else
 		ret = qcom_battmgr_bat_sm8350_update(battmgr, psp);
@@ -599,6 +623,12 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
 		val->intval = battmgr->status.charge_time;
 		break;
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
+		val->intval = battmgr->info.charge_ctrl_start;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+		val->intval = battmgr->info.charge_ctrl_end;
+		break;
 	case POWER_SUPPLY_PROP_MANUFACTURE_YEAR:
 		val->intval = battmgr->info.year;
 		break;
@@ -624,6 +654,120 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
 	return 0;
 }
 
+static int qcom_battmgr_set_charge_control(struct qcom_battmgr *battmgr,
+					   u32 target_soc, u32 delta_soc)
+{
+	struct qcom_battmgr_charge_ctrl_request request = {
+		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_BATTMGR),
+		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
+		.hdr.opcode = cpu_to_le32(BATTMGR_CHG_CTRL_LIMIT_EN),
+		.enable = cpu_to_le32(1),
+		.target_soc = cpu_to_le32(target_soc),
+		.delta_soc = cpu_to_le32(delta_soc),
+	};
+
+	return qcom_battmgr_request(battmgr, &request, sizeof(request));
+}
+
+static int qcom_battmgr_set_charge_start_threshold(struct qcom_battmgr *battmgr, int soc)
+{
+	u32 target_soc, delta_soc;
+	int ret;
+
+	if (soc < CHARGE_CTRL_START_THR_MIN ||
+			soc > CHARGE_CTRL_START_THR_MAX) {
+		dev_err(battmgr->dev, "charge control start threshold exceed range: [%u - %u]\n",
+				CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
+		return -EINVAL;
+	}
+
+	/*
+	 * If the new start threshold is larger than the old end threshold,
+	 * move the end threshold one step (DELTA_SOC) after the new start
+	 * threshold.
+	 */
+	if (soc > battmgr->info.charge_ctrl_end) {
+		target_soc = soc + CHARGE_CTRL_DELTA_SOC;
+		target_soc = min_t(u32, target_soc, CHARGE_CTRL_END_THR_MAX);
+		delta_soc = target_soc - soc;
+		delta_soc = min_t(u32, delta_soc, CHARGE_CTRL_DELTA_SOC);
+	} else {
+		target_soc =  battmgr->info.charge_ctrl_end;
+		delta_soc = battmgr->info.charge_ctrl_end - soc;
+	}
+
+	mutex_lock(&battmgr->lock);
+	ret = qcom_battmgr_set_charge_control(battmgr, target_soc, delta_soc);
+	mutex_unlock(&battmgr->lock);
+	if (!ret) {
+		battmgr->info.charge_ctrl_start = soc;
+		battmgr->info.charge_ctrl_end = target_soc;
+	}
+
+	return 0;
+}
+
+static int qcom_battmgr_set_charge_end_threshold(struct qcom_battmgr *battmgr, int soc)
+{
+	u32 delta_soc = CHARGE_CTRL_DELTA_SOC;
+	int ret;
+
+	if (soc < CHARGE_CTRL_END_THR_MIN ||
+			soc > CHARGE_CTRL_END_THR_MAX) {
+		dev_err(battmgr->dev, "charge control end threshold exceed range: [%u - %u]\n",
+				CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);
+		return -EINVAL;
+	}
+
+	if (battmgr->info.charge_ctrl_start && soc > battmgr->info.charge_ctrl_start)
+		delta_soc = soc - battmgr->info.charge_ctrl_start;
+
+	mutex_lock(&battmgr->lock);
+	ret = qcom_battmgr_set_charge_control(battmgr, soc, delta_soc);
+	mutex_unlock(&battmgr->lock);
+	if (!ret) {
+		battmgr->info.charge_ctrl_start = soc - delta_soc;
+		battmgr->info.charge_ctrl_end = soc;
+	}
+
+	return 0;
+}
+
+static int qcom_battmgr_bat_is_writeable(struct power_supply *psy,
+				enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+		return 1;
+	default:
+		return 0;
+	}
+
+	return 0;
+}
+
+static int qcom_battmgr_bat_set_property(struct power_supply *psy,
+			enum power_supply_property psp,
+			const union power_supply_propval *pval)
+{
+	struct qcom_battmgr *battmgr = power_supply_get_drvdata(psy);
+
+	if (!battmgr->service_up)
+		return -EAGAIN;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
+		return qcom_battmgr_set_charge_start_threshold(battmgr, pval->intval);
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+		return qcom_battmgr_set_charge_end_threshold(battmgr, pval->intval);
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const enum power_supply_property sc8280xp_bat_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
@@ -657,6 +801,43 @@ static const struct power_supply_desc sc8280xp_bat_psy_desc = {
 	.get_property = qcom_battmgr_bat_get_property,
 };
 
+static const enum power_supply_property x1e80100_bat_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_POWER_NOW,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_CHARGE_EMPTY,
+	POWER_SUPPLY_PROP_CHARGE_NOW,
+	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
+	POWER_SUPPLY_PROP_ENERGY_FULL,
+	POWER_SUPPLY_PROP_ENERGY_EMPTY,
+	POWER_SUPPLY_PROP_ENERGY_NOW,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
+	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
+	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_MANUFACTURER,
+	POWER_SUPPLY_PROP_SERIAL_NUMBER,
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
+};
+
+static const struct power_supply_desc x1e80100_bat_psy_desc = {
+	.name = "qcom-battmgr-bat",
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.properties = x1e80100_bat_props,
+	.num_properties = ARRAY_SIZE(x1e80100_bat_props),
+	.get_property = qcom_battmgr_bat_get_property,
+	.set_property = qcom_battmgr_bat_set_property,
+	.property_is_writeable = qcom_battmgr_bat_is_writeable,
+};
+
 static const enum power_supply_property sm8350_bat_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_HEALTH,
@@ -689,6 +870,42 @@ static const struct power_supply_desc sm8350_bat_psy_desc = {
 	.get_property = qcom_battmgr_bat_get_property,
 };
 
+static const enum power_supply_property sm8550_bat_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_VOLTAGE_OCV,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CHARGE_COUNTER,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
+	POWER_SUPPLY_PROP_RESISTANCE,
+	POWER_SUPPLY_PROP_STATE_OF_HEALTH,
+	POWER_SUPPLY_PROP_POWER_NOW,
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
+};
+
+static const struct power_supply_desc sm8550_bat_psy_desc = {
+	.name = "qcom-battmgr-bat",
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.properties = sm8550_bat_props,
+	.num_properties = ARRAY_SIZE(sm8550_bat_props),
+	.get_property = qcom_battmgr_bat_get_property,
+	.set_property = qcom_battmgr_bat_set_property,
+	.property_is_writeable = qcom_battmgr_bat_is_writeable,
+};
+
 static int qcom_battmgr_ac_get_property(struct power_supply *psy,
 					enum power_supply_property psp,
 					union power_supply_propval *val)
@@ -764,7 +981,8 @@ static int qcom_battmgr_usb_get_property(struct power_supply *psy,
 	if (!battmgr->service_up)
 		return -EAGAIN;
 
-	if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)
 		ret = qcom_battmgr_bat_sc8280xp_update(battmgr, psp);
 	else
 		ret = qcom_battmgr_usb_sm8350_update(battmgr, psp);
@@ -886,7 +1104,8 @@ static int qcom_battmgr_wls_get_property(struct power_supply *psy,
 	if (!battmgr->service_up)
 		return -EAGAIN;
 
-	if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)
 		ret = qcom_battmgr_bat_sc8280xp_update(battmgr, psp);
 	else
 		ret = qcom_battmgr_wls_sm8350_update(battmgr, psp);
@@ -1196,6 +1415,12 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
 		case BATT_POWER_NOW:
 			battmgr->status.power_now = le32_to_cpu(resp->intval.value);
 			break;
+		case BATT_CHG_CTRL_START_THR:
+			battmgr->info.charge_ctrl_start = le32_to_cpu(resp->intval.value);
+			break;
+		case BATT_CHG_CTRL_END_THR:
+			battmgr->info.charge_ctrl_end = le32_to_cpu(resp->intval.value);
+			break;
 		default:
 			dev_warn(battmgr->dev, "unknown property %#x\n", property);
 			break;
@@ -1278,6 +1503,7 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
 		}
 		break;
 	case BATTMGR_REQUEST_NOTIFICATION:
+	case BATTMGR_CHG_CTRL_LIMIT_EN:
 		battmgr->error = 0;
 		break;
 	default:
@@ -1297,7 +1523,8 @@ static void qcom_battmgr_callback(const void *data, size_t len, void *priv)
 
 	if (opcode == BATTMGR_NOTIFICATION)
 		qcom_battmgr_notification(battmgr, data, len);
-	else if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
+	else if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)
 		qcom_battmgr_sc8280xp_callback(battmgr, data, len);
 	else
 		qcom_battmgr_sm8350_callback(battmgr, data, len);
@@ -1333,7 +1560,8 @@ static void qcom_battmgr_pdr_notify(void *priv, int state)
 static const struct of_device_id qcom_battmgr_of_variants[] = {
 	{ .compatible = "qcom,sc8180x-pmic-glink", .data = (void *)QCOM_BATTMGR_SC8280XP },
 	{ .compatible = "qcom,sc8280xp-pmic-glink", .data = (void *)QCOM_BATTMGR_SC8280XP },
-	{ .compatible = "qcom,x1e80100-pmic-glink", .data = (void *)QCOM_BATTMGR_SC8280XP },
+	{ .compatible = "qcom,x1e80100-pmic-glink", .data = (void *)QCOM_BATTMGR_X1E80100 },
+	{ .compatible = "qcom,sm8550-pmic-glink", .data = (void *)QCOM_BATTMGR_SM8550 },
 	/* Unmatched devices falls back to QCOM_BATTMGR_SM8350 */
 	{}
 };
@@ -1343,6 +1571,7 @@ static char *qcom_battmgr_battery[] = { "battery" };
 static int qcom_battmgr_probe(struct auxiliary_device *adev,
 			      const struct auxiliary_device_id *id)
 {
+	const struct power_supply_desc *psy_desc;
 	struct power_supply_config psy_cfg_supply = {};
 	struct power_supply_config psy_cfg = {};
 	const struct of_device_id *match;
@@ -1373,8 +1602,14 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
 	else
 		battmgr->variant = QCOM_BATTMGR_SM8350;
 
-	if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
-		battmgr->bat_psy = devm_power_supply_register(dev, &sc8280xp_bat_psy_desc, &psy_cfg);
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100) {
+		if (battmgr->variant == QCOM_BATTMGR_X1E80100)
+			psy_desc = &x1e80100_bat_psy_desc;
+		else
+			psy_desc = &sc8280xp_bat_psy_desc;
+
+		battmgr->bat_psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
 		if (IS_ERR(battmgr->bat_psy))
 			return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
 					     "failed to register battery power supply\n");
@@ -1394,7 +1629,12 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
 			return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
 					     "failed to register wireless charing power supply\n");
 	} else {
-		battmgr->bat_psy = devm_power_supply_register(dev, &sm8350_bat_psy_desc, &psy_cfg);
+		if (battmgr->variant == QCOM_BATTMGR_SM8550)
+			psy_desc = &sm8550_bat_psy_desc;
+		else
+			psy_desc = &sm8350_bat_psy_desc;
+
+		battmgr->bat_psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
 		if (IS_ERR(battmgr->bat_psy))
 			return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
 					     "failed to register battery power supply\n");

-- 
2.34.1

[PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
index 4c9e78f29523e3d77aacb4299f64ab96f9b1a831..972bec151118f2e20e1f3b4e0c0a8fbbbea7ab90 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
@@ -39,9 +39,11 @@ properties:
           - enum:
               - qcom,sm8650-pmic-glink
               - qcom,sm8750-pmic-glink
-              - qcom,x1e80100-pmic-glink
           - const: qcom,sm8550-pmic-glink
           - const: qcom,pmic-glink
+      - items:
+          - const: qcom,x1e80100-pmic-glink
+          - const: qcom,pmic-glink
 
   '#address-cells':
     const: 1

-- 
2.34.1

[PATCH v2 7/8] usb: typec: ucsi_glink: Add UCSI quirk for X1E80100 platform

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Currently, the Qualcomm X1E80100 is treated as a fallback of SM8550
in pmic-glink support. However, the battmgr driver, which uses the
same pmic-glink compatible strings, has implemented charge control
functionality differently between SM8550 and X1E80100. As a result,
X1E80100 is no longer a fallback of SM8550 in pmic-glink support.

Therefore, add match data for X1E80100 separately in ucsi_glink driver
but keep the UCSI quirk the same as SM8550.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 drivers/usb/typec/ucsi/ucsi_glink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index 8af79101a2fc75ae17be1d119b25a9c862e010b3..b10c3161fd577f672d15602023c1aa71a1ab4fe6 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -319,6 +319,7 @@ static const struct of_device_id pmic_glink_ucsi_of_quirks[] = {
 	{ .compatible = "qcom,sm8350-pmic-glink", .data = &quirk_sc8180x, },
 	{ .compatible = "qcom,sm8450-pmic-glink", .data = &quirk_sm8450, },
 	{ .compatible = "qcom,sm8550-pmic-glink", .data = &quirk_sm8450, },
+	{ .compatible = "qcom,x1e80100-pmic-glink", .data = &quirk_sm8450, },
 	{}
 };
 

-- 
2.34.1

[PATCH v2 8/8] arm64: dts: qcom: x1*: Remove qcom,sm8550-pmic-glink fallback

[Fenglin Wu via B4 Relay]

From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

The "qcom,x1e80100-pmic-glink" is not longer a fallback compatible
string of "qcom,sm8550-pmic-glink", so remove "qcom,sm8550-pmic-glink"
in x1* platform pmic-glink device nodes.

Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/x1-crd.dtsi                        | 1 -
 arch/arm64/boot/dts/qcom/x1e001de-devkit.dts                | 1 -
 arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi | 1 -
 arch/arm64/boot/dts/qcom/x1e80100-asus-vivobook-s15.dts     | 1 -
 arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts       | 1 -
 arch/arm64/boot/dts/qcom/x1e80100-hp-omnibook-x14.dts       | 1 -
 arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts    | 1 -
 arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi    | 1 -
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts                   | 1 -
 9 files changed, 9 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
index c9f0d505267081af66b0973fe6c1e33832a2c86b..33d908c8011abe7bbbaca539bb9724f12c679c68 100644
--- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
@@ -74,7 +74,6 @@ switch-lid {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/qcom/x1e001de-devkit.dts b/arch/arm64/boot/dts/qcom/x1e001de-devkit.dts
index 2d9627e6c7983daedba87619ba01074ee22b43c9..d6ad762b8f30cc586761fc75ba95608301b3f599 100644
--- a/arch/arm64/boot/dts/qcom/x1e001de-devkit.dts
+++ b/arch/arm64/boot/dts/qcom/x1e001de-devkit.dts
@@ -51,7 +51,6 @@ chosen {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
index ac1dddf27da30e6a9f7e1d1ecbd5192bf2d0671e..00e6009e3e4e89e4ca45c2d1b1f20e8caaa85bbf 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
@@ -64,7 +64,6 @@ switch-lid {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		orientation-gpios = <&tlmm 121 GPIO_ACTIVE_HIGH>,
 				    <&tlmm 123 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-asus-vivobook-s15.dts b/arch/arm64/boot/dts/qcom/x1e80100-asus-vivobook-s15.dts
index 71b2cc6c392fef9edd19477e4aab6e28699e1eb7..e278997c98e99b1791eb2e0a9dd25ec01b40563b 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-asus-vivobook-s15.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-asus-vivobook-s15.dts
@@ -39,7 +39,6 @@ switch-lid {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		orientation-gpios = <&tlmm 121 GPIO_ACTIVE_HIGH>,
 				    <&tlmm 123 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
index 967f6dba0878b51a985fd7c9570b8c4e71afe57d..e6f1f72505a8dff5b1ffed5f93614973f649e275 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
@@ -59,7 +59,6 @@ led-camera-indicator {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		orientation-gpios = <&tlmm 121 GPIO_ACTIVE_HIGH>,
 				    <&tlmm 123 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-hp-omnibook-x14.dts b/arch/arm64/boot/dts/qcom/x1e80100-hp-omnibook-x14.dts
index 10b3af5e79fb6493cd6b6c661de6a801e40092f7..8057a5dadabcbf16426ba0088a13eb9c35ffff61 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-hp-omnibook-x14.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-hp-omnibook-x14.dts
@@ -83,7 +83,6 @@ switch-lid {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		orientation-gpios = <&tlmm 121 GPIO_ACTIVE_HIGH>,
 				    <&tlmm 123 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
index dad0f11e8e8583df6fd8aeec5be2af86739d85fb..aee38ead38a94ddca525b55004d8b8655e8484df 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
@@ -41,7 +41,6 @@ switch-lid {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi b/arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi
index 0fd8516580b2679ee425438cb73fd4078cb20581..d2bce79c4a4146c57602cf48fbb42446004f48e2 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi
@@ -94,7 +94,6 @@ led-camera-indicator {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
index 4dfba835af6a064dbc5ad65671cb8a6e4df79758..2845a8929f80f9f9921568fb76cba79e60ebcd42 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
@@ -52,7 +52,6 @@ chosen {
 
 	pmic-glink {
 		compatible = "qcom,x1e80100-pmic-glink",
-			     "qcom,sm8550-pmic-glink",
 			     "qcom,pmic-glink";
 		#address-cells = <1>;
 		#size-cells = <0>;

-- 
2.34.1

Re: [PATCH v2 7/8] usb: typec: ucsi_glink: Add UCSI quirk for X1E80100 platform

[Bryan O'Donoghue]

On 30/05/2025 08:35, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> Currently, the Qualcomm X1E80100 is treated as a fallback of SM8550
> in pmic-glink support. However, the battmgr driver, which uses the
> same pmic-glink compatible strings, has implemented charge control
> functionality differently between SM8550 and X1E80100. As a result,
> X1E80100 is no longer a fallback of SM8550 in pmic-glink support.
> 
> Therefore, add match data for X1E80100 separately in ucsi_glink driver
> but keep the UCSI quirk the same as SM8550.
> 
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

Small suggestion for your commit log.

Call out _which_ commit makes that change.

---
bod

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Bryan O'Donoghue]

On 30/05/2025 08:35, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> Add charge control support for SM8550 and X1E80100. It's supported
> with below two power supply properties:
> 
> charge_control_end_threshold: SOC threshold at which the charging
> should be terminated.
> 
> charge_control_start_threshold: SOC threshold at which the charging
> should be resumed.

Maybe this is very obvious to battery charger experts but what does SOC 
mean here ?

Reading your patch you pass a "int soc" and compare it to a threshold 
value, without 'soc' having an obvious meaning.

Its a threshold right ? Why not just call it threshold ?

> 
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
>   drivers/power/supply/qcom_battmgr.c | 256 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 248 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c
> index d5d0200b92bdc3d9a22f44159ad45b152efe8be0..39009415f4bfcd76b010305179d3c8fb847254a6 100644
> --- a/drivers/power/supply/qcom_battmgr.c
> +++ b/drivers/power/supply/qcom_battmgr.c
> @@ -21,6 +21,8 @@
>   enum qcom_battmgr_variant {
>   	QCOM_BATTMGR_SM8350,
>   	QCOM_BATTMGR_SC8280XP,
> +	QCOM_BATTMGR_X1E80100,
> +	QCOM_BATTMGR_SM8550,
You're breaking ordering here.

Well the ordering is already broken but, please sort this list 
alphanumerically.

>   };
> 
>   #define BATTMGR_BAT_STATUS		0x1
> @@ -66,6 +68,9 @@ enum qcom_battmgr_variant {
>   #define BATT_RESISTANCE			21
>   #define BATT_POWER_NOW			22
>   #define BATT_POWER_AVG			23
> +#define BATT_CHG_CTRL_EN		24
> +#define BATT_CHG_CTRL_START_THR		25
> +#define BATT_CHG_CTRL_END_THR		26
> 
>   #define BATTMGR_USB_PROPERTY_GET	0x32
>   #define BATTMGR_USB_PROPERTY_SET	0x33
> @@ -90,6 +95,13 @@ enum qcom_battmgr_variant {
>   #define WLS_TYPE			5
>   #define WLS_BOOST_EN			6
> 
> +#define BATTMGR_CHG_CTRL_LIMIT_EN	0x48
> +#define CHARGE_CTRL_START_THR_MIN	50
> +#define CHARGE_CTRL_START_THR_MAX	95
> +#define CHARGE_CTRL_END_THR_MIN		55
> +#define CHARGE_CTRL_END_THR_MAX		100
> +#define CHARGE_CTRL_DELTA_SOC		5
> +
>   struct qcom_battmgr_enable_request {
>   	struct pmic_glink_hdr hdr;
>   	__le32 battery_id;
> @@ -124,6 +136,13 @@ struct qcom_battmgr_discharge_time_request {
>   	__le32 reserved;
>   };
> 
> +struct qcom_battmgr_charge_ctrl_request {
> +	struct pmic_glink_hdr hdr;
> +	__le32 enable;
> +	__le32 target_soc;
> +	__le32 delta_soc;
> +};
> +
>   struct qcom_battmgr_message {
>   	struct pmic_glink_hdr hdr;
>   	union {
> @@ -236,6 +255,8 @@ struct qcom_battmgr_info {
>   	unsigned int capacity_warning;
>   	unsigned int cycle_count;
>   	unsigned int charge_count;
> +	unsigned int charge_ctrl_start;
> +	unsigned int charge_ctrl_end;
>   	char model_number[BATTMGR_STRING_LEN];
>   	char serial_number[BATTMGR_STRING_LEN];
>   	char oem_info[BATTMGR_STRING_LEN];
> @@ -424,6 +445,8 @@ static const u8 sm8350_bat_prop_map[] = {
>   	[POWER_SUPPLY_PROP_RESISTANCE] = BATT_RESISTANCE,
>   	[POWER_SUPPLY_PROP_STATE_OF_HEALTH] = BATT_SOH,
>   	[POWER_SUPPLY_PROP_POWER_NOW] = BATT_POWER_NOW,
> +	[POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD] = BATT_CHG_CTRL_START_THR,
> +	[POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD] = BATT_CHG_CTRL_END_THR,
>   };
> 
>   static int qcom_battmgr_bat_sm8350_update(struct qcom_battmgr *battmgr,
> @@ -494,7 +517,8 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
>   	if (!battmgr->service_up)
>   		return -EAGAIN;
> 
> -	if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
> +	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
> +			battmgr->variant == QCOM_BATTMGR_X1E80100)

Please run your series through checkpatch

0004-power-supply-qcom_battmgr-Add-state_of_health-proper.patch has no 
obvious style problems and is ready for submission.
CHECK: Alignment should match open parenthesis
#95: FILE: drivers/power/supply/qcom_battmgr.c:521:
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)

CHECK: Alignment should match open parenthesis
#137: FILE: drivers/power/supply/qcom_battmgr.c:678:
+	if (soc < CHARGE_CTRL_START_THR_MIN ||
+			soc > CHARGE_CTRL_START_THR_MAX) {

CHECK: Alignment should match open parenthesis
#139: FILE: drivers/power/supply/qcom_battmgr.c:680:
+		dev_err(battmgr->dev, "charge control start threshold exceed range: 
[%u - %u]\n",
+				CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);

CHECK: Alignment should match open parenthesis
#175: FILE: drivers/power/supply/qcom_battmgr.c:716:
+	if (soc < CHARGE_CTRL_END_THR_MIN ||
+			soc > CHARGE_CTRL_END_THR_MAX) {

CHECK: Alignment should match open parenthesis
#177: FILE: drivers/power/supply/qcom_battmgr.c:718:
+		dev_err(battmgr->dev, "charge control end threshold exceed range: [%u 
- %u]\n",
+				CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);

CHECK: Alignment should match open parenthesis
#196: FILE: drivers/power/supply/qcom_battmgr.c:737:
+static int qcom_battmgr_bat_is_writeable(struct power_supply *psy,
+				enum power_supply_property psp)

CHECK: Alignment should match open parenthesis
#210: FILE: drivers/power/supply/qcom_battmgr.c:751:
+static int qcom_battmgr_bat_set_property(struct power_supply *psy,
+			enum power_supply_property psp,

CHECK: Alignment should match open parenthesis
#326: FILE: drivers/power/supply/qcom_battmgr.c:985:
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)

CHECK: Alignment should match open parenthesis
#336: FILE: drivers/power/supply/qcom_battmgr.c:1108:
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)

CHECK: Alignment should match open parenthesis
#367: FILE: drivers/power/supply/qcom_battmgr.c:1527:
+	else if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100)

CHECK: Alignment should match open parenthesis
#396: FILE: drivers/power/supply/qcom_battmgr.c:1606:
+	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
+			battmgr->variant == QCOM_BATTMGR_X1E80100) {


>   		ret = qcom_battmgr_bat_sc8280xp_update(battmgr, psp);
>   	else
>   		ret = qcom_battmgr_bat_sm8350_update(battmgr, psp);
> @@ -599,6 +623,12 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
>   	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
>   		val->intval = battmgr->status.charge_time;
>   		break;
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> +		val->intval = battmgr->info.charge_ctrl_start;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> +		val->intval = battmgr->info.charge_ctrl_end;
> +		break;
>   	case POWER_SUPPLY_PROP_MANUFACTURE_YEAR:
>   		val->intval = battmgr->info.year;
>   		break;
> @@ -624,6 +654,120 @@ static int qcom_battmgr_bat_get_property(struct power_supply *psy,
>   	return 0;
>   }
> 
> +static int qcom_battmgr_set_charge_control(struct qcom_battmgr *battmgr,
> +					   u32 target_soc, u32 delta_soc)
> +{
> +	struct qcom_battmgr_charge_ctrl_request request = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_BATTMGR),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(BATTMGR_CHG_CTRL_LIMIT_EN),
> +		.enable = cpu_to_le32(1),
> +		.target_soc = cpu_to_le32(target_soc),
> +		.delta_soc = cpu_to_le32(delta_soc),
> +	};
> +
> +	return qcom_battmgr_request(battmgr, &request, sizeof(request));
> +}
> +
> +static int qcom_battmgr_set_charge_start_threshold(struct qcom_battmgr *battmgr, int soc)
> +{
> +	u32 target_soc, delta_soc;
> +	int ret;
> +
> +	if (soc < CHARGE_CTRL_START_THR_MIN ||
> +			soc > CHARGE_CTRL_START_THR_MAX) {
> +		dev_err(battmgr->dev, "charge control start threshold exceed range: [%u - %u]\n",
> +				CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
> +		return -EINVAL;
> +	}

'soc' is what - a threshold as far as I can tell.

> +
> +	/*
> +	 * If the new start threshold is larger than the old end threshold,
> +	 * move the end threshold one step (DELTA_SOC) after the new start
> +	 * threshold.
> +	 */
> +	if (soc > battmgr->info.charge_ctrl_end) {
> +		target_soc = soc + CHARGE_CTRL_DELTA_SOC;
> +		target_soc = min_t(u32, target_soc, CHARGE_CTRL_END_THR_MAX);
> +		delta_soc = target_soc - soc;
> +		delta_soc = min_t(u32, delta_soc, CHARGE_CTRL_DELTA_SOC);
> +	} else {
> +		target_soc =  battmgr->info.charge_ctrl_end;
> +		delta_soc = battmgr->info.charge_ctrl_end - soc;
> +	}
> +
> +	mutex_lock(&battmgr->lock);
> +	ret = qcom_battmgr_set_charge_control(battmgr, target_soc, delta_soc);
> +	mutex_unlock(&battmgr->lock);
> +	if (!ret) {
> +		battmgr->info.charge_ctrl_start = soc;
> +		battmgr->info.charge_ctrl_end = target_soc;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qcom_battmgr_set_charge_end_threshold(struct qcom_battmgr *battmgr, int soc)
> +{
> +	u32 delta_soc = CHARGE_CTRL_DELTA_SOC;
> +	int ret;
> +
> +	if (soc < CHARGE_CTRL_END_THR_MIN ||
> +			soc > CHARGE_CTRL_END_THR_MAX) {
> +		dev_err(battmgr->dev, "charge control end threshold exceed range: [%u - %u]\n",
> +				CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);
> +		return -EINVAL;
> +	}
> +
> +	if (battmgr->info.charge_ctrl_start && soc > battmgr->info.charge_ctrl_start)
> +		delta_soc = soc - battmgr->info.charge_ctrl_start;
> +
> +	mutex_lock(&battmgr->lock);
> +	ret = qcom_battmgr_set_charge_control(battmgr, soc, delta_soc);
> +	mutex_unlock(&battmgr->lock);
> +	if (!ret) {
> +		battmgr->info.charge_ctrl_start = soc - delta_soc;
> +		battmgr->info.charge_ctrl_end = soc;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qcom_battmgr_bat_is_writeable(struct power_supply *psy,
> +				enum power_supply_property psp)
> +{
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> +		return 1;
> +	default:
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qcom_battmgr_bat_set_property(struct power_supply *psy,
> +			enum power_supply_property psp,
> +			const union power_supply_propval *pval)
> +{
> +	struct qcom_battmgr *battmgr = power_supply_get_drvdata(psy);
> +
> +	if (!battmgr->service_up)
> +		return -EAGAIN;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> +		return qcom_battmgr_set_charge_start_threshold(battmgr, pval->intval);
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> +		return qcom_battmgr_set_charge_end_threshold(battmgr, pval->intval);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>   static const enum power_supply_property sc8280xp_bat_props[] = {
>   	POWER_SUPPLY_PROP_STATUS,
>   	POWER_SUPPLY_PROP_PRESENT,
> @@ -657,6 +801,43 @@ static const struct power_supply_desc sc8280xp_bat_psy_desc = {
>   	.get_property = qcom_battmgr_bat_get_property,
>   };
> 
> +static const enum power_supply_property x1e80100_bat_props[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_TECHNOLOGY,
> +	POWER_SUPPLY_PROP_CYCLE_COUNT,
> +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_POWER_NOW,
> +	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> +	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_CHARGE_EMPTY,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
> +	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
> +	POWER_SUPPLY_PROP_ENERGY_FULL,
> +	POWER_SUPPLY_PROP_ENERGY_EMPTY,
> +	POWER_SUPPLY_PROP_ENERGY_NOW,
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
> +	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
> +	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
> +	POWER_SUPPLY_PROP_MODEL_NAME,
> +	POWER_SUPPLY_PROP_MANUFACTURER,
> +	POWER_SUPPLY_PROP_SERIAL_NUMBER,
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> +};
> +
> +static const struct power_supply_desc x1e80100_bat_psy_desc = {
> +	.name = "qcom-battmgr-bat",
> +	.type = POWER_SUPPLY_TYPE_BATTERY,
> +	.properties = x1e80100_bat_props,
> +	.num_properties = ARRAY_SIZE(x1e80100_bat_props),
> +	.get_property = qcom_battmgr_bat_get_property,
> +	.set_property = qcom_battmgr_bat_set_property,
> +	.property_is_writeable = qcom_battmgr_bat_is_writeable,
> +};
> +
>   static const enum power_supply_property sm8350_bat_props[] = {
>   	POWER_SUPPLY_PROP_STATUS,
>   	POWER_SUPPLY_PROP_HEALTH,
> @@ -689,6 +870,42 @@ static const struct power_supply_desc sm8350_bat_psy_desc = {
>   	.get_property = qcom_battmgr_bat_get_property,
>   };
> 
> +static const enum power_supply_property sm8550_bat_props[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_CHARGE_TYPE,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +	POWER_SUPPLY_PROP_VOLTAGE_OCV,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_MAX,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_TECHNOLOGY,
> +	POWER_SUPPLY_PROP_CHARGE_COUNTER,
> +	POWER_SUPPLY_PROP_CYCLE_COUNT,
> +	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> +	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_MODEL_NAME,
> +	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
> +	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
> +	POWER_SUPPLY_PROP_RESISTANCE,
> +	POWER_SUPPLY_PROP_STATE_OF_HEALTH,
> +	POWER_SUPPLY_PROP_POWER_NOW,
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> +};
> +
> +static const struct power_supply_desc sm8550_bat_psy_desc = {
> +	.name = "qcom-battmgr-bat",
> +	.type = POWER_SUPPLY_TYPE_BATTERY,
> +	.properties = sm8550_bat_props,
> +	.num_properties = ARRAY_SIZE(sm8550_bat_props),
> +	.get_property = qcom_battmgr_bat_get_property,
> +	.set_property = qcom_battmgr_bat_set_property,
> +	.property_is_writeable = qcom_battmgr_bat_is_writeable,
> +};
> +
>   static int qcom_battmgr_ac_get_property(struct power_supply *psy,
>   					enum power_supply_property psp,
>   					union power_supply_propval *val)
> @@ -764,7 +981,8 @@ static int qcom_battmgr_usb_get_property(struct power_supply *psy,
>   	if (!battmgr->service_up)
>   		return -EAGAIN;
> 
> -	if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
> +	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
> +			battmgr->variant == QCOM_BATTMGR_X1E80100)
>   		ret = qcom_battmgr_bat_sc8280xp_update(battmgr, psp);
>   	else
>   		ret = qcom_battmgr_usb_sm8350_update(battmgr, psp);
> @@ -886,7 +1104,8 @@ static int qcom_battmgr_wls_get_property(struct power_supply *psy,
>   	if (!battmgr->service_up)
>   		return -EAGAIN;
> 
> -	if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
> +	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
> +			battmgr->variant == QCOM_BATTMGR_X1E80100)
>   		ret = qcom_battmgr_bat_sc8280xp_update(battmgr, psp);
>   	else
>   		ret = qcom_battmgr_wls_sm8350_update(battmgr, psp);
> @@ -1196,6 +1415,12 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
>   		case BATT_POWER_NOW:
>   			battmgr->status.power_now = le32_to_cpu(resp->intval.value);
>   			break;
> +		case BATT_CHG_CTRL_START_THR:
> +			battmgr->info.charge_ctrl_start = le32_to_cpu(resp->intval.value);
> +			break;
> +		case BATT_CHG_CTRL_END_THR:
> +			battmgr->info.charge_ctrl_end = le32_to_cpu(resp->intval.value);
> +			break;
>   		default:
>   			dev_warn(battmgr->dev, "unknown property %#x\n", property);
>   			break;
> @@ -1278,6 +1503,7 @@ static void qcom_battmgr_sm8350_callback(struct qcom_battmgr *battmgr,
>   		}
>   		break;
>   	case BATTMGR_REQUEST_NOTIFICATION:
> +	case BATTMGR_CHG_CTRL_LIMIT_EN:
>   		battmgr->error = 0;
>   		break;
>   	default:
> @@ -1297,7 +1523,8 @@ static void qcom_battmgr_callback(const void *data, size_t len, void *priv)
> 
>   	if (opcode == BATTMGR_NOTIFICATION)
>   		qcom_battmgr_notification(battmgr, data, len);
> -	else if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
> +	else if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
> +			battmgr->variant == QCOM_BATTMGR_X1E80100)
>   		qcom_battmgr_sc8280xp_callback(battmgr, data, len);
>   	else
>   		qcom_battmgr_sm8350_callback(battmgr, data, len);
> @@ -1333,7 +1560,8 @@ static void qcom_battmgr_pdr_notify(void *priv, int state)
>   static const struct of_device_id qcom_battmgr_of_variants[] = {
>   	{ .compatible = "qcom,sc8180x-pmic-glink", .data = (void *)QCOM_BATTMGR_SC8280XP },
>   	{ .compatible = "qcom,sc8280xp-pmic-glink", .data = (void *)QCOM_BATTMGR_SC8280XP },
> -	{ .compatible = "qcom,x1e80100-pmic-glink", .data = (void *)QCOM_BATTMGR_SC8280XP },
> +	{ .compatible = "qcom,x1e80100-pmic-glink", .data = (void *)QCOM_BATTMGR_X1E80100 },
> +	{ .compatible = "qcom,sm8550-pmic-glink", .data = (void *)QCOM_BATTMGR_SM8550 },

Please separate compat string addition from functional changes.

>   	/* Unmatched devices falls back to QCOM_BATTMGR_SM8350 */
>   	{}
>   };
> @@ -1343,6 +1571,7 @@ static char *qcom_battmgr_battery[] = { "battery" };
>   static int qcom_battmgr_probe(struct auxiliary_device *adev,
>   			      const struct auxiliary_device_id *id)
>   {
> +	const struct power_supply_desc *psy_desc;
>   	struct power_supply_config psy_cfg_supply = {};
>   	struct power_supply_config psy_cfg = {};
>   	const struct of_device_id *match;
> @@ -1373,8 +1602,14 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
>   	else
>   		battmgr->variant = QCOM_BATTMGR_SM8350;
> 
> -	if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
> -		battmgr->bat_psy = devm_power_supply_register(dev, &sc8280xp_bat_psy_desc, &psy_cfg);
> +	if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
> +			battmgr->variant == QCOM_BATTMGR_X1E80100) {
> +		if (battmgr->variant == QCOM_BATTMGR_X1E80100)
> +			psy_desc = &x1e80100_bat_psy_desc;
> +		else
> +			psy_desc = &sc8280xp_bat_psy_desc;
> +
> +		battmgr->bat_psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
>   		if (IS_ERR(battmgr->bat_psy))
>   			return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
>   					     "failed to register battery power supply\n");
> @@ -1394,7 +1629,12 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev,
>   			return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
>   					     "failed to register wireless charing power supply\n");
>   	} else {
> -		battmgr->bat_psy = devm_power_supply_register(dev, &sm8350_bat_psy_desc, &psy_cfg);
> +		if (battmgr->variant == QCOM_BATTMGR_SM8550)
> +			psy_desc = &sm8550_bat_psy_desc;
> +		else
> +			psy_desc = &sm8350_bat_psy_desc;
> +
> +		battmgr->bat_psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
>   		if (IS_ERR(battmgr->bat_psy))
>   			return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
>   					     "failed to register battery power supply\n");
> 
> --
> 2.34.1
> 
> 
> 

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Fenglin Wu]

Thanks for reviewing the change!

On 5/30/2025 4:48 PM, Bryan O'Donoghue wrote:
> On 30/05/2025 08:35, Fenglin Wu via B4 Relay wrote:
>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>
>> Add charge control support for SM8550 and X1E80100. It's supported
>> with below two power supply properties:
>>
>> charge_control_end_threshold: SOC threshold at which the charging
>> should be terminated.
>>
>> charge_control_start_threshold: SOC threshold at which the charging
>> should be resumed.
>
> Maybe this is very obvious to battery charger experts but what does 
> SOC mean here ?
>
> Reading your patch you pass a "int soc" and compare it to a threshold 
> value, without 'soc' having an obvious meaning.
>
> Its a threshold right ? Why not just call it threshold ?
>
"SOC" stands for battery State of Charge, I will rephrase the commit 
text for better explanation.
>>
>> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>> ---
>>   drivers/power/supply/qcom_battmgr.c | 256 
>> ++++++++++++++++++++++++++++++++++--
>>   1 file changed, 248 insertions(+), 8 deletions(-)
>>
>> -    if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
>> +    if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>
> Please run your series through checkpatch
>
I actually did that before sending the patches out. I run checkpatch 
with below two commands and I saw no issues:

git format -1 xxxx --stdtout | ./script/checkpatch.pl -

b4 prep --check

Can you let me know what specific command that you ran with it?

> 0004-power-supply-qcom_battmgr-Add-state_of_health-proper.patch has no 
> obvious style problems and is ready for submission.
> CHECK: Alignment should match open parenthesis
> #95: FILE: drivers/power/supply/qcom_battmgr.c:521:
> +    if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>
>>
>> +static int qcom_battmgr_set_charge_start_threshold(struct 
>> qcom_battmgr *battmgr, int soc)
>> +{
>> +    u32 target_soc, delta_soc;
>> +    int ret;
>> +
>> +    if (soc < CHARGE_CTRL_START_THR_MIN ||
>> +            soc > CHARGE_CTRL_START_THR_MAX) {
>> +        dev_err(battmgr->dev, "charge control start threshold exceed 
>> range: [%u - %u]\n",
>> +                CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
>> +        return -EINVAL;
>> +    }
>
> 'soc' is what - a threshold as far as I can tell.

I will update it with a more meaningful name

>>
>>       if (opcode == BATTMGR_NOTIFICATION)
>>           qcom_battmgr_notification(battmgr, data, len);
>> -    else if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
>> +    else if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>>           qcom_battmgr_sc8280xp_callback(battmgr, data, len);
>>       else
>>           qcom_battmgr_sm8350_callback(battmgr, data, len);
>> @@ -1333,7 +1560,8 @@ static void qcom_battmgr_pdr_notify(void *priv, 
>> int state)
>>   static const struct of_device_id qcom_battmgr_of_variants[] = {
>>       { .compatible = "qcom,sc8180x-pmic-glink", .data = (void 
>> *)QCOM_BATTMGR_SC8280XP },
>>       { .compatible = "qcom,sc8280xp-pmic-glink", .data = (void 
>> *)QCOM_BATTMGR_SC8280XP },
>> -    { .compatible = "qcom,x1e80100-pmic-glink", .data = (void 
>> *)QCOM_BATTMGR_SC8280XP },
>> +    { .compatible = "qcom,x1e80100-pmic-glink", .data = (void 
>> *)QCOM_BATTMGR_X1E80100 },
>> +    { .compatible = "qcom,sm8550-pmic-glink", .data = (void 
>> *)QCOM_BATTMGR_SM8550 },
>
> Please separate compat string addition from functional changes.
>
The compatible string "qcom,sm8550-pmic-glink" has been present in the 
binding for a while and it was added as a fallback of "qcom,pmic-glink". 
The battmgr function has been also supported well on SM8550 for a while. 
The change here is only specifying a different match data for SM8550 so 
the driver can handle some new features differently. Does it also need 
to add it in a separate change? If so,  this change would be split into 
following 3 patches I think:

1) add QCOM_BATTMGR_SM8550/X1E80100 variants definition in 
qcom_battmgr_variant.

2) add compatible string with corresponding match data for SM8550.

3) add the charge control function support.

>>       /* Unmatched devices falls back to QCOM_BATTMGR_SM8350 */
>>       {}
>>   };
>>
>>
>> -- 
>> 2.34.1
>>
>>
>>
>

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Bryan O'Donoghue]

On 30/05/2025 10:37, Fenglin Wu wrote:
> Thanks for reviewing the change!
> 
> On 5/30/2025 4:48 PM, Bryan O'Donoghue wrote:
>> On 30/05/2025 08:35, Fenglin Wu via B4 Relay wrote:
>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>
>>> Add charge control support for SM8550 and X1E80100. It's supported
>>> with below two power supply properties:
>>>
>>> charge_control_end_threshold: SOC threshold at which the charging
>>> should be terminated.
>>>
>>> charge_control_start_threshold: SOC threshold at which the charging
>>> should be resumed.
>>
>> Maybe this is very obvious to battery charger experts but what does
>> SOC mean here ?
>>
>> Reading your patch you pass a "int soc" and compare it to a threshold
>> value, without 'soc' having an obvious meaning.
>>
>> Its a threshold right ? Why not just call it threshold ?
>>
> "SOC" stands for battery State of Charge, I will rephrase the commit
> text for better explanation.
>>>
>>> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>> ---
>>>    drivers/power/supply/qcom_battmgr.c | 256
>>> ++++++++++++++++++++++++++++++++++--
>>>    1 file changed, 248 insertions(+), 8 deletions(-)
>>>
>>> -    if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
>>> +    if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>>
>> Please run your series through checkpatch
>>
> I actually did that before sending the patches out. I run checkpatch
> with below two commands and I saw no issues:
> 
> git format -1 xxxx --stdtout | ./script/checkpatch.pl -
> 
> b4 prep --check
> 
> Can you let me know what specific command that you ran with it?

do $KERNELPATH/scripts/checkpatch.pl --strict $file;
codespell $file;

> 
>> 0004-power-supply-qcom_battmgr-Add-state_of_health-proper.patch has no
>> obvious style problems and is ready for submission.
>> CHECK: Alignment should match open parenthesis
>> #95: FILE: drivers/power/supply/qcom_battmgr.c:521:
>> +    if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>>
>>>
>>> +static int qcom_battmgr_set_charge_start_threshold(struct
>>> qcom_battmgr *battmgr, int soc)
>>> +{
>>> +    u32 target_soc, delta_soc;
>>> +    int ret;
>>> +
>>> +    if (soc < CHARGE_CTRL_START_THR_MIN ||
>>> +            soc > CHARGE_CTRL_START_THR_MAX) {
>>> +        dev_err(battmgr->dev, "charge control start threshold exceed
>>> range: [%u - %u]\n",
>>> +                CHARGE_CTRL_START_THR_MIN, CHARGE_CTRL_START_THR_MAX);
>>> +        return -EINVAL;
>>> +    }
>>
>> 'soc' is what - a threshold as far as I can tell.
> 
> I will update it with a more meaningful name
> 
>>>
>>>        if (opcode == BATTMGR_NOTIFICATION)
>>>            qcom_battmgr_notification(battmgr, data, len);
>>> -    else if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
>>> +    else if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>>>            qcom_battmgr_sc8280xp_callback(battmgr, data, len);
>>>        else
>>>            qcom_battmgr_sm8350_callback(battmgr, data, len);
>>> @@ -1333,7 +1560,8 @@ static void qcom_battmgr_pdr_notify(void *priv,
>>> int state)
>>>    static const struct of_device_id qcom_battmgr_of_variants[] = {
>>>        { .compatible = "qcom,sc8180x-pmic-glink", .data = (void
>>> *)QCOM_BATTMGR_SC8280XP },
>>>        { .compatible = "qcom,sc8280xp-pmic-glink", .data = (void
>>> *)QCOM_BATTMGR_SC8280XP },
>>> -    { .compatible = "qcom,x1e80100-pmic-glink", .data = (void
>>> *)QCOM_BATTMGR_SC8280XP },
>>> +    { .compatible = "qcom,x1e80100-pmic-glink", .data = (void
>>> *)QCOM_BATTMGR_X1E80100 },
>>> +    { .compatible = "qcom,sm8550-pmic-glink", .data = (void
>>> *)QCOM_BATTMGR_SM8550 },
>>
>> Please separate compat string addition from functional changes.
>>
> The compatible string "qcom,sm8550-pmic-glink" has been present in the
> binding for a while and it was added as a fallback of "qcom,pmic-glink".
> The battmgr function has been also supported well on SM8550 for a while.
> The change here is only specifying a different match data for SM8550 so
> the driver can handle some new features differently. Does it also need
> to add it in a separate change? If so,  this change would be split into
> following 3 patches I think:
> 
> 1) add QCOM_BATTMGR_SM8550/X1E80100 variants definition in
> qcom_battmgr_variant.
> 
> 2) add compatible string with corresponding match data for SM8550.
> 
> 3) add the charge control function support.

For preference compats and functional change should be disjoined IMO.

---
bod

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[György Kurucz]

> Add charge control support for SM8550 and X1E80100.

Thank you for this, tested on my Lenovo Yoga Slim 7x, the limiting works 
well, I finally don't have to worry about leaving my laptop plugged in 
for too long.

One small thing I noticed is that after setting the sysfs values and 
rebooting, they report 0 again. The limiting appears to stay in effect 
though, so it seems that the firmware does keep the values, but Linux 
does not read them back. Indeed, looking at the code, it seems that 
actually reading back the values is only implemented for the SM8550.

Anyway, this is just a small nitpick, this does not really affect the 
functionality, and I would support merging this series regardless of 
whether the read back values are always correct.

György

Re: [PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Dmitry Baryshkov]

On Fri, May 30, 2025 at 03:35:07PM +0800, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> Add state_of_health power supply property to represent battery
> health percentage.
> 
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
>  Documentation/ABI/testing/sysfs-class-power | 10 ++++++++++
>  drivers/power/supply/power_supply_sysfs.c   |  1 +
>  include/linux/power_supply.h                |  1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> index 22a565a6a1c509461b8c483e12975295765121d6..74e0d4d67467500c3cd62da3ae0b2e4a67e77680 100644
> --- a/Documentation/ABI/testing/sysfs-class-power
> +++ b/Documentation/ABI/testing/sysfs-class-power
> @@ -562,6 +562,16 @@ Description:
>  
>  		Valid values: Represented in microohms
>  
> +What:		/sys/class/power_supply/<supply_name>/state_of_health
> +Date:		May 2025
> +Contact:	linux-arm-msm@vger.kernel.org
> +Description:
> +		Reports battery power supply state of health in percentage.
> +
> +		Access: Read
> +
> +		Valid values: 0 - 100 (percent)

What does it mean that battery has 77% of health?

> +
>  **USB Properties**
>  
>  What:		/sys/class/power_supply/<supply_name>/input_current_limit
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index dd829148eb6fda5dcd7eab53fc70f99081763714..12af0d0398822ff23d8970f6bdc8e3ef68081a1d 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -221,6 +221,7 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
>  	POWER_SUPPLY_ATTR(MANUFACTURE_MONTH),
>  	POWER_SUPPLY_ATTR(MANUFACTURE_DAY),
>  	POWER_SUPPLY_ATTR(RESISTANCE),
> +	POWER_SUPPLY_ATTR(STATE_OF_HEALTH),
>  	/* Properties of type `const char *' */
>  	POWER_SUPPLY_ATTR(MODEL_NAME),
>  	POWER_SUPPLY_ATTR(MANUFACTURER),
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index de3e88810e322546470b21258913abc7707c86a7..dd0108940231352ac6c6f0fa962d1ea904d81c7a 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -175,6 +175,7 @@ enum power_supply_property {
>  	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
>  	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
>  	POWER_SUPPLY_PROP_RESISTANCE,
> +	POWER_SUPPLY_PROP_STATE_OF_HEALTH,
>  	/* Properties of type `const char *' */
>  	POWER_SUPPLY_PROP_MODEL_NAME,
>  	POWER_SUPPLY_PROP_MANUFACTURER,
> 
> -- 
> 2.34.1
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/8] power: supply: qcom_battmgr: Add resistance power supply property

[Dmitry Baryshkov]

On Fri, May 30, 2025 at 03:35:08PM +0800, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> Add power supply property to get battery resistance from the battery
> management firmware.
> 
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
>  drivers/power/supply/qcom_battmgr.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Dmitry Baryshkov]

On Fri, May 30, 2025 at 03:35:11PM +0800, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.

Why?

> 
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> index 4c9e78f29523e3d77aacb4299f64ab96f9b1a831..972bec151118f2e20e1f3b4e0c0a8fbbbea7ab90 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> @@ -39,9 +39,11 @@ properties:
>            - enum:
>                - qcom,sm8650-pmic-glink
>                - qcom,sm8750-pmic-glink
> -              - qcom,x1e80100-pmic-glink
>            - const: qcom,sm8550-pmic-glink
>            - const: qcom,pmic-glink
> +      - items:
> +          - const: qcom,x1e80100-pmic-glink
> +          - const: qcom,pmic-glink
>  
>    '#address-cells':
>      const: 1
> 
> -- 
> 2.34.1
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Krzysztof Kozlowski]

On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.

Why?

Do not describe what you do here, it's obvious. We see it from the diff.


Best regards,
Krzysztof

Re: [PATCH v2 8/8] arm64: dts: qcom: x1*: Remove qcom,sm8550-pmic-glink fallback

[Krzysztof Kozlowski]

On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> The "qcom,x1e80100-pmic-glink" is not longer a fallback compatible
> string of "qcom,sm8550-pmic-glink", so remove "qcom,sm8550-pmic-glink"

No, it still is.

> in x1* platform pmic-glink device nodes.

No explanation, no reasoning and this affects users of this DTS.

Best regards,
Krzysztof

Re: [PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Fenglin Wu]

On 6/2/2025 2:17 PM, Dmitry Baryshkov wrote:
> On Fri, May 30, 2025 at 03:35:07PM +0800, Fenglin Wu via B4 Relay wrote:
>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>
>> Add state_of_health power supply property to represent battery
>> health percentage.
>>
>> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>> ---
>>   Documentation/ABI/testing/sysfs-class-power | 10 ++++++++++
>>   drivers/power/supply/power_supply_sysfs.c   |  1 +
>>   include/linux/power_supply.h                |  1 +
>>   3 files changed, 12 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
>> index 22a565a6a1c509461b8c483e12975295765121d6..74e0d4d67467500c3cd62da3ae0b2e4a67e77680 100644
>> --- a/Documentation/ABI/testing/sysfs-class-power
>> +++ b/Documentation/ABI/testing/sysfs-class-power
>> @@ -562,6 +562,16 @@ Description:
>>   
>>   		Valid values: Represented in microohms
>>   
>> +What:		/sys/class/power_supply/<supply_name>/state_of_health
>> +Date:		May 2025
>> +Contact:	linux-arm-msm@vger.kernel.org
>> +Description:
>> +		Reports battery power supply state of health in percentage.
>> +
>> +		Access: Read
>> +
>> +		Valid values: 0 - 100 (percent)
> What does it mean that battery has 77% of health?

I will update this to explain it better:

Reports battery power supply state of health in percentage, indicating that the maximum charge capacity has degraded to that percentage of its original designed capacity.

>> +
>>   **USB Properties**
>>   
>>   What:		/sys/class/power_supply/<supply_name>/input_current_limit
>> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
>> index dd829148eb6fda5dcd7eab53fc70f99081763714..12af0d0398822ff23d8970f6bdc8e3ef68081a1d 100644
>> --- a/drivers/power/supply/power_supply_sysfs.c
>> +++ b/drivers/power/supply/power_supply_sysfs.c
>> @@ -221,6 +221,7 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
>>   	POWER_SUPPLY_ATTR(MANUFACTURE_MONTH),
>>   	POWER_SUPPLY_ATTR(MANUFACTURE_DAY),
>>   	POWER_SUPPLY_ATTR(RESISTANCE),
>> +	POWER_SUPPLY_ATTR(STATE_OF_HEALTH),
>>   	/* Properties of type `const char *' */
>>   	POWER_SUPPLY_ATTR(MODEL_NAME),
>>   	POWER_SUPPLY_ATTR(MANUFACTURER),
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index de3e88810e322546470b21258913abc7707c86a7..dd0108940231352ac6c6f0fa962d1ea904d81c7a 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -175,6 +175,7 @@ enum power_supply_property {
>>   	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
>>   	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
>>   	POWER_SUPPLY_PROP_RESISTANCE,
>> +	POWER_SUPPLY_PROP_STATE_OF_HEALTH,
>>   	/* Properties of type `const char *' */
>>   	POWER_SUPPLY_PROP_MODEL_NAME,
>>   	POWER_SUPPLY_PROP_MANUFACTURER,
>>
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Fenglin Wu]

On 5/30/2025 6:11 PM, Bryan O'Donoghue wrote:
> On 30/05/2025 10:37, Fenglin Wu wrote:
>> Thanks for reviewing the change!
>>
>> On 5/30/2025 4:48 PM, Bryan O'Donoghue wrote:
>>> On 30/05/2025 08:35, Fenglin Wu via B4 Relay wrote:
>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>
>>>> Add charge control support for SM8550 and X1E80100. It's supported
>>>> with below two power supply properties:
>>>>
>>>> charge_control_end_threshold: SOC threshold at which the charging
>>>> should be terminated.
>>>>
>>>> charge_control_start_threshold: SOC threshold at which the charging
>>>> should be resumed.
>>>
>>> Maybe this is very obvious to battery charger experts but what does
>>> SOC mean here ?
>>>
>>> Reading your patch you pass a "int soc" and compare it to a threshold
>>> value, without 'soc' having an obvious meaning.
>>>
>>> Its a threshold right ? Why not just call it threshold ?
>>>
>> "SOC" stands for battery State of Charge, I will rephrase the commit
>> text for better explanation.
>>>>
>>>> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>> ---
>>>>    drivers/power/supply/qcom_battmgr.c | 256
>>>> ++++++++++++++++++++++++++++++++++--
>>>>    1 file changed, 248 insertions(+), 8 deletions(-)
>>>>
>>>> -    if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
>>>> +    if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>>>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>>>
>>> Please run your series through checkpatch
>>>
>> I actually did that before sending the patches out. I run checkpatch
>> with below two commands and I saw no issues:
>>
>> git format -1 xxxx --stdtout | ./script/checkpatch.pl -
>>
>> b4 prep --check
>>
>> Can you let me know what specific command that you ran with it?
>
> do $KERNELPATH/scripts/checkpatch.pl --strict $file;
> codespell $file;
>
Thanks, I will run the commands to check before sending next patch


>>
>>> 0004-power-supply-qcom_battmgr-Add-state_of_health-proper.patch has no
>>> obvious style problems and is ready for submission.
>>> CHECK: Alignment should match open parenthesis
>>> #95: FILE: drivers/power/supply/qcom_battmgr.c:521:
>>> +    if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>>>
>>>>
>>>> +static int qcom_battmgr_set_charge_start_threshold(struct
>>>> qcom_battmgr *battmgr, int soc)
>>>> +{
>>>> +    u32 target_soc, delta_soc;
>>>> +    int ret;
>>>> +
>>>> +    if (soc < CHARGE_CTRL_START_THR_MIN ||
>>>> +            soc > CHARGE_CTRL_START_THR_MAX) {
>>>> +        dev_err(battmgr->dev, "charge control start threshold exceed
>>>> range: [%u - %u]\n",
>>>> +                CHARGE_CTRL_START_THR_MIN, 
>>>> CHARGE_CTRL_START_THR_MAX);
>>>> +        return -EINVAL;
>>>> +    }
>>>
>>> 'soc' is what - a threshold as far as I can tell.
>>
>> I will update it with a more meaningful name
>>
>>>>
>>>>        if (opcode == BATTMGR_NOTIFICATION)
>>>>            qcom_battmgr_notification(battmgr, data, len);
>>>> -    else if (battmgr->variant == QCOM_BATTMGR_SC8280XP)
>>>> +    else if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>>>> +            battmgr->variant == QCOM_BATTMGR_X1E80100)
>>>>            qcom_battmgr_sc8280xp_callback(battmgr, data, len);
>>>>        else
>>>>            qcom_battmgr_sm8350_callback(battmgr, data, len);
>>>> @@ -1333,7 +1560,8 @@ static void qcom_battmgr_pdr_notify(void *priv,
>>>> int state)
>>>>    static const struct of_device_id qcom_battmgr_of_variants[] = {
>>>>        { .compatible = "qcom,sc8180x-pmic-glink", .data = (void
>>>> *)QCOM_BATTMGR_SC8280XP },
>>>>        { .compatible = "qcom,sc8280xp-pmic-glink", .data = (void
>>>> *)QCOM_BATTMGR_SC8280XP },
>>>> -    { .compatible = "qcom,x1e80100-pmic-glink", .data = (void
>>>> *)QCOM_BATTMGR_SC8280XP },
>>>> +    { .compatible = "qcom,x1e80100-pmic-glink", .data = (void
>>>> *)QCOM_BATTMGR_X1E80100 },
>>>> +    { .compatible = "qcom,sm8550-pmic-glink", .data = (void
>>>> *)QCOM_BATTMGR_SM8550 },
>>>
>>> Please separate compat string addition from functional changes.
>>>
>> The compatible string "qcom,sm8550-pmic-glink" has been present in the
>> binding for a while and it was added as a fallback of "qcom,pmic-glink".
>> The battmgr function has been also supported well on SM8550 for a while.
>> The change here is only specifying a different match data for SM8550 so
>> the driver can handle some new features differently. Does it also need
>> to add it in a separate change? If so,  this change would be split into
>> following 3 patches I think:
>>
>> 1) add QCOM_BATTMGR_SM8550/X1E80100 variants definition in
>> qcom_battmgr_variant.
>>
>> 2) add compatible string with corresponding match data for SM8550.
>>
>> 3) add the charge control function support.
>
> For preference compats and functional change should be disjoined IMO.

I understand that adding a new compatible will have to be done in a 
separate change. However, when updating match data of an existing 
compatible due to a new feature, isn't it better to include it within 
the new feature?

let me know if you think that having 3 separate changes above is more 
appropriate.

Thanks

>
> ---
> bod

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Fenglin Wu]

On 5/31/2025 6:36 PM, György Kurucz wrote:
>> Add charge control support for SM8550 and X1E80100.
>
> Thank you for this, tested on my Lenovo Yoga Slim 7x, the limiting 
> works well, I finally don't have to worry about leaving my laptop 
> plugged in for too long.
>
> One small thing I noticed is that after setting the sysfs values and 
> rebooting, they report 0 again. The limiting appears to stay in effect 
> though, so it seems that the firmware does keep the values, but Linux 
> does not read them back. Indeed, looking at the code, it seems that 
> actually reading back the values is only implemented for the SM8550.

Right.

Based on offline information, X1E80100 doesn't support reading back 
those threshold values in battery management firmware, so I can only use 
the cached values for sysfs read.

>
> Anyway, this is just a small nitpick, this does not really affect the 
> functionality, and I would support merging this series regardless of 
> whether the read back values are always correct.
>
> György

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Fenglin Wu]

On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>
>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
> Why?
>
> Do not describe what you do here, it's obvious. We see it from the diff.
>
>
> Best regards,
> Krzysztof

Previously, in qcom_battmgr driver, x1e80100 was specified with a match 
data the same as sc8280xp, also sm8550 was treated a fallback of sm8350 
without the need of a match data.

In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated 
as a fallback of sm8550. There was no issues to make x1e80100 as a 
fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.

In patch [5/8] in this series, in qcom_battmgr driver, it added charge 
control functionality for sm8550 and x1e80100 differently hence 
different match data was specified for them, and it makes x1e80100 ad 
sm8550 incompatible and they need to be treated differently.

I explained this a little bit in the commit text of patch [7/8] in this 
series, I can make similar description in patch [6/8].

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Krzysztof Kozlowski]

On 03/06/2025 08:42, Fenglin Wu wrote:
> 
> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>
>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>> Why?
>>
>> Do not describe what you do here, it's obvious. We see it from the diff.
>>
>>
>> Best regards,
>> Krzysztof
> 
> Previously, in qcom_battmgr driver, x1e80100 was specified with a match 
> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350 
> without the need of a match data.
> 
> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated 
> as a fallback of sm8550. There was no issues to make x1e80100 as a 
> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
> 
> In patch [5/8] in this series, in qcom_battmgr driver, it added charge 
> control functionality for sm8550 and x1e80100 differently hence 
> different match data was specified for them, and it makes x1e80100 ad 
> sm8550 incompatible and they need to be treated differently.

So you break ABI and that's your problem to fix. You cannot make devices
incompatible without good justification.



Best regards,
Krzysztof

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Fenglin Wu]

On 6/3/2025 2:47 PM, Krzysztof Kozlowski wrote:
> On 03/06/2025 08:42, Fenglin Wu wrote:
>> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>
>>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>>> Why?
>>>
>>> Do not describe what you do here, it's obvious. We see it from the diff.
>>>
>>>
>>> Best regards,
>>> Krzysztof
>> Previously, in qcom_battmgr driver, x1e80100 was specified with a match
>> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350
>> without the need of a match data.
>>
>> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated
>> as a fallback of sm8550. There was no issues to make x1e80100 as a
>> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
>>
>> In patch [5/8] in this series, in qcom_battmgr driver, it added charge
>> control functionality for sm8550 and x1e80100 differently hence
>> different match data was specified for them, and it makes x1e80100 ad
>> sm8550 incompatible and they need to be treated differently.
> So you break ABI and that's your problem to fix. You cannot make devices
> incompatible without good justification.

I would say x1e80100 and sm8550 are different and incompatible from a 
battery management firmware support perspective. The x1e80100 follows 
the sc8280xp as a compute platform, whereas the sm8550 follows the 
sm8350 as a mobile platform.

The difference between them was initially ignored because the sm8550 
could use everything that the sm8350 has, and no match data needed to be 
specified for it. However, now the sm8550 has new features that the 
sm8350 doesn't have, requiring us to treat it differently, thus the 
incompatibility was acknowledged.

>
> Best regards,
> Krzysztof

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Krzysztof Kozlowski]

On 03/06/2025 08:59, Fenglin Wu wrote:
> 
> On 6/3/2025 2:47 PM, Krzysztof Kozlowski wrote:
>> On 03/06/2025 08:42, Fenglin Wu wrote:
>>> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>>>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>>
>>>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>>>> Why?
>>>>
>>>> Do not describe what you do here, it's obvious. We see it from the diff.
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>> Previously, in qcom_battmgr driver, x1e80100 was specified with a match
>>> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350
>>> without the need of a match data.
>>>
>>> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated
>>> as a fallback of sm8550. There was no issues to make x1e80100 as a
>>> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
>>>
>>> In patch [5/8] in this series, in qcom_battmgr driver, it added charge
>>> control functionality for sm8550 and x1e80100 differently hence
>>> different match data was specified for them, and it makes x1e80100 ad
>>> sm8550 incompatible and they need to be treated differently.
>> So you break ABI and that's your problem to fix. You cannot make devices
>> incompatible without good justification.
> 
> I would say x1e80100 and sm8550 are different and incompatible from a 
> battery management firmware support perspective. The x1e80100 follows 
> the sc8280xp as a compute platform, whereas the sm8550 follows the 
> sm8350 as a mobile platform.

Not correct arguments for compatibility.

> 
> The difference between them was initially ignored because the sm8550 
> could use everything that the sm8350 has, and no match data needed to be 
> specified for it. However, now the sm8550 has new features that the 
> sm8350 doesn't have, requiring us to treat it differently, thus the 
> incompatibility was acknowledged.

So they are perfectly compatible.

I really do not understand what we are discussing here. Explain in
simple terms of DT spec: what is incompatible that SW cannot use one
interface to handle the other?


Best regards,
Krzysztof

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Fenglin Wu]

On 6/3/2025 3:06 PM, Krzysztof Kozlowski wrote:
> On 03/06/2025 08:59, Fenglin Wu wrote:
>> On 6/3/2025 2:47 PM, Krzysztof Kozlowski wrote:
>>> On 03/06/2025 08:42, Fenglin Wu wrote:
>>>> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>>>>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>>>
>>>>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>>>>> Why?
>>>>>
>>>>> Do not describe what you do here, it's obvious. We see it from the diff.
>>>>>
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>> Previously, in qcom_battmgr driver, x1e80100 was specified with a match
>>>> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350
>>>> without the need of a match data.
>>>>
>>>> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated
>>>> as a fallback of sm8550. There was no issues to make x1e80100 as a
>>>> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
>>>>
>>>> In patch [5/8] in this series, in qcom_battmgr driver, it added charge
>>>> control functionality for sm8550 and x1e80100 differently hence
>>>> different match data was specified for them, and it makes x1e80100 ad
>>>> sm8550 incompatible and they need to be treated differently.
>>> So you break ABI and that's your problem to fix. You cannot make devices
>>> incompatible without good justification.
>> I would say x1e80100 and sm8550 are different and incompatible from a
>> battery management firmware support perspective. The x1e80100 follows
>> the sc8280xp as a compute platform, whereas the sm8550 follows the
>> sm8350 as a mobile platform.
> Not correct arguments for compatibility.
>
>> The difference between them was initially ignored because the sm8550
>> could use everything that the sm8350 has, and no match data needed to be
>> specified for it. However, now the sm8550 has new features that the
>> sm8350 doesn't have, requiring us to treat it differently, thus the
>> incompatibility was acknowledged.
> So they are perfectly compatible.
>
> I really do not understand what we are discussing here. Explain in
> simple terms of DT spec: what is incompatible that SW cannot use one
> interface to handle the other?

1. x1e80100 was a fallback of sc8280xp, it used "sc8280xp_bat_psy_desc" 
when registering the power supply device.

2. sm8550 was a fallback of sm8350, and they all used 
"sm8350_bat_psy_desc" when registering the power supply device.

3. x1e80100 and sm8550 they are incompatible as they are using different 
data structure of "xxx_bat_psy_desc"  and other “psy_desc" too, such as, 
ac/usb/wls.

4. For charge control functionality, it's only supported in the battery 
management firmware in x1e80100 and sm8550 platforms. And the change in 
battmgr driver (patch [5/8]) adds the support by using 2 additional 
power supply properties, which eventually need to be added in the 
"properties" data member of "xxx_bat_psy_desc" when registering power 
supply devices. Hence, "x1e80100_bat_psy_desc" and "sm8550_bat_psy_desc" 
are created and used separately when registering power supply device 
according to the "variant" value defined in the match data.

The main code change is in [5/8], I am pasting a snippet which might 
help to explain this a little bit:

-       if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
-               battmgr->bat_psy = devm_power_supply_register(dev, 
&sc8280xp_bat_psy_desc, &psy_cfg);
+       if (battmgr->variant == QCOM_BATTMGR_SC8280XP || 
battmgr->variant == QCOM_BATTMGR_X1E80100) {
+               if (battmgr->variant == QCOM_BATTMGR_X1E80100)
+                       psy_desc = &x1e80100_bat_psy_desc;
+               else
+                       psy_desc = &sc8280xp_bat_psy_desc;
+
+               battmgr->bat_psy = devm_power_supply_register(dev, 
psy_desc, &psy_cfg);
                 if (IS_ERR(battmgr->bat_psy))
                         return dev_err_probe(dev, 
PTR_ERR(battmgr->bat_psy),
                                              "failed to register 
battery power supply\n");
@@ -1394,7 +1628,12 @@ static int qcom_battmgr_probe(struct 
auxiliary_device *adev,
                         return dev_err_probe(dev, 
PTR_ERR(battmgr->wls_psy),
                                              "failed to register 
wireless charing power supply\n");
         } else {
-               battmgr->bat_psy = devm_power_supply_register(dev, 
&sm8350_bat_psy_desc, &psy_cfg);
+               if (battmgr->variant == QCOM_BATTMGR_SM8550)
+                       psy_desc = &sm8550_bat_psy_desc;
+               else
+                       psy_desc = &sm8350_bat_psy_desc;
+
+               battmgr->bat_psy = devm_power_supply_register(dev, 
psy_desc, &psy_cfg);
                 if (IS_ERR(battmgr->bat_psy))
                         return dev_err_probe(dev, 
PTR_ERR(battmgr->bat_psy),
                                              "failed to register 
battery power supply\n");

>
> Best regards,
> Krzysztof

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Krzysztof Kozlowski]

On 03/06/2025 09:41, Fenglin Wu wrote:
> 
> On 6/3/2025 3:06 PM, Krzysztof Kozlowski wrote:
>> On 03/06/2025 08:59, Fenglin Wu wrote:
>>> On 6/3/2025 2:47 PM, Krzysztof Kozlowski wrote:
>>>> On 03/06/2025 08:42, Fenglin Wu wrote:
>>>>> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>>>>>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>>>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>>>>
>>>>>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>>>>>> Why?
>>>>>>
>>>>>> Do not describe what you do here, it's obvious. We see it from the diff.
>>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>> Previously, in qcom_battmgr driver, x1e80100 was specified with a match
>>>>> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350
>>>>> without the need of a match data.
>>>>>
>>>>> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated
>>>>> as a fallback of sm8550. There was no issues to make x1e80100 as a
>>>>> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
>>>>>
>>>>> In patch [5/8] in this series, in qcom_battmgr driver, it added charge
>>>>> control functionality for sm8550 and x1e80100 differently hence
>>>>> different match data was specified for them, and it makes x1e80100 ad
>>>>> sm8550 incompatible and they need to be treated differently.
>>>> So you break ABI and that's your problem to fix. You cannot make devices
>>>> incompatible without good justification.
>>> I would say x1e80100 and sm8550 are different and incompatible from a
>>> battery management firmware support perspective. The x1e80100 follows
>>> the sc8280xp as a compute platform, whereas the sm8550 follows the
>>> sm8350 as a mobile platform.
>> Not correct arguments for compatibility.
>>
>>> The difference between them was initially ignored because the sm8550
>>> could use everything that the sm8350 has, and no match data needed to be
>>> specified for it. However, now the sm8550 has new features that the
>>> sm8350 doesn't have, requiring us to treat it differently, thus the
>>> incompatibility was acknowledged.
>> So they are perfectly compatible.
>>
>> I really do not understand what we are discussing here. Explain in
>> simple terms of DT spec: what is incompatible that SW cannot use one
>> interface to handle the other?
> 
> 1. x1e80100 was a fallback of sc8280xp, it used "sc8280xp_bat_psy_desc" 


No, that's not true. Read the binding again:

              - qcom,x1e80100-pmic-glink
           - const: qcom,sm8550-pmic-glink

No fallback to sc8280xp.


> when registering the power supply device.
> 
> 2. sm8550 was a fallback of sm8350, and they all used 


Also not true. The remaining fallback is not sm8350.


> "sm8350_bat_psy_desc" when registering the power supply device.
> 
> 3. x1e80100 and sm8550 they are incompatible as they are using different 
> data structure of "xxx_bat_psy_desc"  and other “psy_desc" too, such as, 
> ac/usb/wls.

Look at the driver and bindings now - they are compatible. It looks like
you made it incompatible and now you claim the "they are incompatible".
No, you did it. Look at the driver.



> 
> 4. For charge control functionality, it's only supported in the battery 
> management firmware in x1e80100 and sm8550 platforms. And the change in 
> battmgr driver (patch [5/8]) adds the support by using 2 additional 
> power supply properties, which eventually need to be added in the 
> "properties" data member of "xxx_bat_psy_desc" when registering power 
> supply devices. Hence, "x1e80100_bat_psy_desc" and "sm8550_bat_psy_desc" 
> are created and used separately when registering power supply device 
> according to the "variant" value defined in the match data.
> 
> The main code change is in [5/8], I am pasting a snippet which might 
> help to explain this a little bit:
> 
> -       if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
> -               battmgr->bat_psy = devm_power_supply_register(dev, 
> &sc8280xp_bat_psy_desc, &psy_cfg);
> +       if (battmgr->variant == QCOM_BATTMGR_SC8280XP || 
> battmgr->variant == QCOM_BATTMGR_X1E80100) {
> +               if (battmgr->variant == QCOM_BATTMGR_X1E80100)
> +                       psy_desc = &x1e80100_bat_psy_desc;
> +               else
> +                       psy_desc = &sc8280xp_bat_psy_desc;
> +
> +               battmgr->bat_psy = devm_power_supply_register(dev, 
> psy_desc, &psy_cfg);
>                  if (IS_ERR(battmgr->bat_psy))
>                          return dev_err_probe(dev, 
> PTR_ERR(battmgr->bat_psy),


This explains nothing to me. I think you did not get my questions at all
and just want to push whatever you have in drivers.

Such ping pongs are just tiring, so go back to my previous email, read
it carefully and try harder to understand what compatibility means.


NAK, you are affecting the users and ABI with justification "I make it
now incompatible, so it is incompatible".

Best regards,
Krzysztof

Re: [PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Dmitry Baryshkov]

On Tue, Jun 03, 2025 at 12:50:18PM +0800, Fenglin Wu wrote:
> 
> On 6/2/2025 2:17 PM, Dmitry Baryshkov wrote:
> > On Fri, May 30, 2025 at 03:35:07PM +0800, Fenglin Wu via B4 Relay wrote:
> > > From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> > > 
> > > Add state_of_health power supply property to represent battery
> > > health percentage.
> > > 
> > > Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> > > ---
> > >   Documentation/ABI/testing/sysfs-class-power | 10 ++++++++++
> > >   drivers/power/supply/power_supply_sysfs.c   |  1 +
> > >   include/linux/power_supply.h                |  1 +
> > >   3 files changed, 12 insertions(+)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> > > index 22a565a6a1c509461b8c483e12975295765121d6..74e0d4d67467500c3cd62da3ae0b2e4a67e77680 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-power
> > > +++ b/Documentation/ABI/testing/sysfs-class-power
> > > @@ -562,6 +562,16 @@ Description:
> > >   		Valid values: Represented in microohms
> > > +What:		/sys/class/power_supply/<supply_name>/state_of_health
> > > +Date:		May 2025
> > > +Contact:	linux-arm-msm@vger.kernel.org
> > > +Description:
> > > +		Reports battery power supply state of health in percentage.
> > > +
> > > +		Access: Read
> > > +
> > > +		Valid values: 0 - 100 (percent)
> > What does it mean that battery has 77% of health?
> 
> I will update this to explain it better:
> 
> Reports battery power supply state of health in percentage, indicating that the maximum charge capacity has degraded to that percentage of its original designed capacity.

Which basically means that we don't need it in the first place, as we
can read capacity_full and capacity_full_design (or energy_full /
energy_full_design) and divide one onto another.

> 
> > > +
> > >   **USB Properties**
> > >   What:		/sys/class/power_supply/<supply_name>/input_current_limit
> > > diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> > > index dd829148eb6fda5dcd7eab53fc70f99081763714..12af0d0398822ff23d8970f6bdc8e3ef68081a1d 100644
> > > --- a/drivers/power/supply/power_supply_sysfs.c
> > > +++ b/drivers/power/supply/power_supply_sysfs.c
> > > @@ -221,6 +221,7 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
> > >   	POWER_SUPPLY_ATTR(MANUFACTURE_MONTH),
> > >   	POWER_SUPPLY_ATTR(MANUFACTURE_DAY),
> > >   	POWER_SUPPLY_ATTR(RESISTANCE),
> > > +	POWER_SUPPLY_ATTR(STATE_OF_HEALTH),
> > >   	/* Properties of type `const char *' */
> > >   	POWER_SUPPLY_ATTR(MODEL_NAME),
> > >   	POWER_SUPPLY_ATTR(MANUFACTURER),
> > > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > > index de3e88810e322546470b21258913abc7707c86a7..dd0108940231352ac6c6f0fa962d1ea904d81c7a 100644
> > > --- a/include/linux/power_supply.h
> > > +++ b/include/linux/power_supply.h
> > > @@ -175,6 +175,7 @@ enum power_supply_property {
> > >   	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
> > >   	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
> > >   	POWER_SUPPLY_PROP_RESISTANCE,
> > > +	POWER_SUPPLY_PROP_STATE_OF_HEALTH,
> > >   	/* Properties of type `const char *' */
> > >   	POWER_SUPPLY_PROP_MODEL_NAME,
> > >   	POWER_SUPPLY_PROP_MANUFACTURER,
> > > 
> > > -- 
> > > 2.34.1
> > > 
> > > 

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Dmitry Baryshkov]

On Tue, Jun 03, 2025 at 01:48:11PM +0800, Fenglin Wu wrote:
> 
> On 5/31/2025 6:36 PM, György Kurucz wrote:
> > > Add charge control support for SM8550 and X1E80100.
> > 
> > Thank you for this, tested on my Lenovo Yoga Slim 7x, the limiting works
> > well, I finally don't have to worry about leaving my laptop plugged in
> > for too long.
> > 
> > One small thing I noticed is that after setting the sysfs values and
> > rebooting, they report 0 again. The limiting appears to stay in effect
> > though, so it seems that the firmware does keep the values, but Linux
> > does not read them back. Indeed, looking at the code, it seems that
> > actually reading back the values is only implemented for the SM8550.
> 
> Right.
> 
> Based on offline information, X1E80100 doesn't support reading back those
> threshold values in battery management firmware, so I can only use the
> cached values for sysfs read.

Which limits usablity of the attribute, it is now impossible to identify
whether it is enabled or disabled. Is there a chance of fixing that for
the X1E80100 platform?

> 
> > 
> > Anyway, this is just a small nitpick, this does not really affect the
> > functionality, and I would support merging this series regardless of
> > whether the read back values are always correct.
> > 
> > György

-- 
With best wishes
Dmitry

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Fenglin Wu]

On 6/3/2025 5:34 PM, Krzysztof Kozlowski wrote:
> On 03/06/2025 09:41, Fenglin Wu wrote:
>> On 6/3/2025 3:06 PM, Krzysztof Kozlowski wrote:
>>> On 03/06/2025 08:59, Fenglin Wu wrote:
>>>> On 6/3/2025 2:47 PM, Krzysztof Kozlowski wrote:
>>>>> On 03/06/2025 08:42, Fenglin Wu wrote:
>>>>>> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>>>>>>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>>>>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>>>>>
>>>>>>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>>>>>>> Why?
>>>>>>>
>>>>>>> Do not describe what you do here, it's obvious. We see it from the diff.
>>>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Krzysztof
>>>>>> Previously, in qcom_battmgr driver, x1e80100 was specified with a match
>>>>>> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350
>>>>>> without the need of a match data.
>>>>>>
>>>>>> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated
>>>>>> as a fallback of sm8550. There was no issues to make x1e80100 as a
>>>>>> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
>>>>>>
>>>>>> In patch [5/8] in this series, in qcom_battmgr driver, it added charge
>>>>>> control functionality for sm8550 and x1e80100 differently hence
>>>>>> different match data was specified for them, and it makes x1e80100 ad
>>>>>> sm8550 incompatible and they need to be treated differently.
>>>>> So you break ABI and that's your problem to fix. You cannot make devices
>>>>> incompatible without good justification.
>>>> I would say x1e80100 and sm8550 are different and incompatible from a
>>>> battery management firmware support perspective. The x1e80100 follows
>>>> the sc8280xp as a compute platform, whereas the sm8550 follows the
>>>> sm8350 as a mobile platform.
>>> Not correct arguments for compatibility.
>>>
>>>> The difference between them was initially ignored because the sm8550
>>>> could use everything that the sm8350 has, and no match data needed to be
>>>> specified for it. However, now the sm8550 has new features that the
>>>> sm8350 doesn't have, requiring us to treat it differently, thus the
>>>> incompatibility was acknowledged.
>>> So they are perfectly compatible.
>>>
>>> I really do not understand what we are discussing here. Explain in
>>> simple terms of DT spec: what is incompatible that SW cannot use one
>>> interface to handle the other?
>> 1. x1e80100 was a fallback of sc8280xp, it used "sc8280xp_bat_psy_desc"
>
> No, that's not true. Read the binding again:
>
>                - qcom,x1e80100-pmic-glink
>             - const: qcom,sm8550-pmic-glink
>
> No fallback to sc8280xp.
>
>
>> when registering the power supply device.
>>
>> 2. sm8550 was a fallback of sm8350, and they all used
>
> Also not true. The remaining fallback is not sm8350.
>
>
>> "sm8350_bat_psy_desc" when registering the power supply device.
>>
>> 3. x1e80100 and sm8550 they are incompatible as they are using different
>> data structure of "xxx_bat_psy_desc"  and other “psy_desc" too, such as,
>> ac/usb/wls.
> Look at the driver and bindings now - they are compatible. It looks like
> you made it incompatible and now you claim the "they are incompatible".
> No, you did it. Look at the driver.
>
>
>
>> 4. For charge control functionality, it's only supported in the battery
>> management firmware in x1e80100 and sm8550 platforms. And the change in
>> battmgr driver (patch [5/8]) adds the support by using 2 additional
>> power supply properties, which eventually need to be added in the
>> "properties" data member of "xxx_bat_psy_desc" when registering power
>> supply devices. Hence, "x1e80100_bat_psy_desc" and "sm8550_bat_psy_desc"
>> are created and used separately when registering power supply device
>> according to the "variant" value defined in the match data.
>>
>> The main code change is in [5/8], I am pasting a snippet which might
>> help to explain this a little bit:
>>
>> -       if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
>> -               battmgr->bat_psy = devm_power_supply_register(dev,
>> &sc8280xp_bat_psy_desc, &psy_cfg);
>> +       if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>> battmgr->variant == QCOM_BATTMGR_X1E80100) {
>> +               if (battmgr->variant == QCOM_BATTMGR_X1E80100)
>> +                       psy_desc = &x1e80100_bat_psy_desc;
>> +               else
>> +                       psy_desc = &sc8280xp_bat_psy_desc;
>> +
>> +               battmgr->bat_psy = devm_power_supply_register(dev,
>> psy_desc, &psy_cfg);
>>                   if (IS_ERR(battmgr->bat_psy))
>>                           return dev_err_probe(dev,
>> PTR_ERR(battmgr->bat_psy),
>
> This explains nothing to me. I think you did not get my questions at all
> and just want to push whatever you have in drivers.
>
> Such ping pongs are just tiring, so go back to my previous email, read
> it carefully and try harder to understand what compatibility means.
>
>
> NAK, you are affecting the users and ABI with justification "I make it
> now incompatible, so it is incompatible".
>
> Best regards,
> Krzysztof

Thanks for the explanation with patience. I misunderstood the fallback 
behavior.

I was worried about if the compatible string matching would work 
correctly if both the device node and the driver declared multiple 
identical compatible strings.

I understand now and even if the device node and the driver have defined 
multiple identical compatible strings, the best match which is the most 
specific compatible string will be found.

So in the example below, for X1E80100-CRD, the battmgr driver will 
always match to "qcom,x1e80100-pmic-glink" which is the most specific 
compatible string defined at the beginning of the device node compatible 
string, and the compatibility has not been broken.

In qcom_battmgr driver:

static const struct of_device_id qcom_battmgr_of_variants[] = {
         ...
         { .compatible = "qcom,x1e80100-pmic-glink", .data = (void 
*)QCOM_BATTMGR_X1E80100 },
         { .compatible = "qcom,sm8550-pmic-glink", .data = (void 
*)QCOM_BATTMGR_SM8550 },
         ...
};

In x1-crd.dtsi:

pmic-glink {
           compatible = "qcom,x1e80100-pmic-glink",
                      "qcom,sm8550-pmic-glink",
                      "qcom,pmic-glink";
         ...

}

Let me know if my understanding is correct. I will drop patch 
[6/8],[7/8],[8/8] in next version.

Thanks

Re: [PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Fenglin Wu]

On 6/3/2025 6:35 PM, Dmitry Baryshkov wrote:
>>>> +What:		/sys/class/power_supply/<supply_name>/state_of_health
>>>> +Date:		May 2025
>>>> +Contact:	linux-arm-msm@vger.kernel.org
>>>> +Description:
>>>> +		Reports battery power supply state of health in percentage.
>>>> +
>>>> +		Access: Read
>>>> +
>>>> +		Valid values: 0 - 100 (percent)
>>> What does it mean that battery has 77% of health?
>> I will update this to explain it better:
>>
>> Reports battery power supply state of health in percentage, indicating that the maximum charge capacity has degraded to that percentage of its original designed capacity.
> Which basically means that we don't need it in the first place, as we
> can read capacity_full and capacity_full_design (or energy_full /
> energy_full_design) and divide one onto another.

Hmm, it is true in general to quantify how the battery performance has 
degraded over time. However, estimating and calculating for battery 
state of health is much more complicated I think. I am not an expert, 
but as far as I know, different battery management systems might have 
different algorithms to calculate the battery health and report it in as 
percentage. For example, in Qcom battery management firmware, a "soh" 
parameter is provided as the battery health percentage based on the 
real-time calculations from learning capacity, resistance estimation, etc.

Re: [PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Dmitry Baryshkov]

On Thu, Jun 05, 2025 at 02:08:30PM +0800, Fenglin Wu wrote:
> 
> On 6/3/2025 6:35 PM, Dmitry Baryshkov wrote:
> > > > > +What:		/sys/class/power_supply/<supply_name>/state_of_health
> > > > > +Date:		May 2025
> > > > > +Contact:	linux-arm-msm@vger.kernel.org
> > > > > +Description:
> > > > > +		Reports battery power supply state of health in percentage.
> > > > > +
> > > > > +		Access: Read
> > > > > +
> > > > > +		Valid values: 0 - 100 (percent)
> > > > What does it mean that battery has 77% of health?
> > > I will update this to explain it better:
> > > 
> > > Reports battery power supply state of health in percentage, indicating that the maximum charge capacity has degraded to that percentage of its original designed capacity.
> > Which basically means that we don't need it in the first place, as we
> > can read capacity_full and capacity_full_design (or energy_full /
> > energy_full_design) and divide one onto another.
> 
> Hmm, it is true in general to quantify how the battery performance has
> degraded over time. However, estimating and calculating for battery state of
> health is much more complicated I think. I am not an expert, but as far as I
> know, different battery management systems might have different algorithms
> to calculate the battery health and report it in as percentage. For example,
> in Qcom battery management firmware, a "soh" parameter is provided as the
> battery health percentage based on the real-time calculations from learning
> capacity, resistance estimation, etc.

Ack, this is more than just full / full_design. Please consider
expanding ABI description (though in the vendor-neutral way).

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Konrad Dybcio]

On 6/3/25 12:37 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 03, 2025 at 01:48:11PM +0800, Fenglin Wu wrote:
>>
>> On 5/31/2025 6:36 PM, György Kurucz wrote:
>>>> Add charge control support for SM8550 and X1E80100.
>>>
>>> Thank you for this, tested on my Lenovo Yoga Slim 7x, the limiting works
>>> well, I finally don't have to worry about leaving my laptop plugged in
>>> for too long.
>>>
>>> One small thing I noticed is that after setting the sysfs values and
>>> rebooting, they report 0 again. The limiting appears to stay in effect
>>> though, so it seems that the firmware does keep the values, but Linux
>>> does not read them back. Indeed, looking at the code, it seems that
>>> actually reading back the values is only implemented for the SM8550.
>>
>> Right.
>>
>> Based on offline information, X1E80100 doesn't support reading back those
>> threshold values in battery management firmware, so I can only use the
>> cached values for sysfs read.
> 
> Which limits usablity of the attribute, it is now impossible to identify
> whether it is enabled or disabled. Is there a chance of fixing that for
> the X1E80100 platform?

Is there a chance we store that value in SDAM and can read it back?

Konrad

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Fenglin Wu]

On 6/7/2025 5:46 PM, Konrad Dybcio wrote:
> On 6/3/25 12:37 PM, Dmitry Baryshkov wrote:
>> On Tue, Jun 03, 2025 at 01:48:11PM +0800, Fenglin Wu wrote:
>>> On 5/31/2025 6:36 PM, György Kurucz wrote:
>>>>> Add charge control support for SM8550 and X1E80100.
>>>> Thank you for this, tested on my Lenovo Yoga Slim 7x, the limiting works
>>>> well, I finally don't have to worry about leaving my laptop plugged in
>>>> for too long.
>>>>
>>>> One small thing I noticed is that after setting the sysfs values and
>>>> rebooting, they report 0 again. The limiting appears to stay in effect
>>>> though, so it seems that the firmware does keep the values, but Linux
>>>> does not read them back. Indeed, looking at the code, it seems that
>>>> actually reading back the values is only implemented for the SM8550.
>>> Right.
>>>
>>> Based on offline information, X1E80100 doesn't support reading back those
>>> threshold values in battery management firmware, so I can only use the
>>> cached values for sysfs read.
>> Which limits usablity of the attribute, it is now impossible to identify
>> whether it is enabled or disabled. Is there a chance of fixing that for
>> the X1E80100 platform?
> Is there a chance we store that value in SDAM and can read it back?
>
> Konrad

The thresholds are stored in PMIC SDAM registers by ADSP after receiving 
the set requests, and ADSP reads them back during initialization. This 
is why ADSP retains them upon device reboot.

I spoke with the battery management firmware team, and they have no 
plans to update the battery management firmware for X1E80100 further. 
Consequently, they cannot provide any interfaces to read these 
thresholds through PMIC Glink.

Reading them from the existing SDAM registers requires adding 
"nvmem-cells" DT properties to specify the SDAM registers. However, the 
"pmic_glink.power-supply" device is an auxiliary device created by the 
pmic_glink driver and does not have an associated DT node. Is there any 
method to create a DT node and add DT properties for an auxiliary device?

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Dmitry Baryshkov]

On 09/06/2025 05:39, Fenglin Wu wrote:
> 
> On 6/7/2025 5:46 PM, Konrad Dybcio wrote:
>> On 6/3/25 12:37 PM, Dmitry Baryshkov wrote:
>>> On Tue, Jun 03, 2025 at 01:48:11PM +0800, Fenglin Wu wrote:
>>>> On 5/31/2025 6:36 PM, György Kurucz wrote:
>>>>>> Add charge control support for SM8550 and X1E80100.
>>>>> Thank you for this, tested on my Lenovo Yoga Slim 7x, the limiting 
>>>>> works
>>>>> well, I finally don't have to worry about leaving my laptop plugged in
>>>>> for too long.
>>>>>
>>>>> One small thing I noticed is that after setting the sysfs values and
>>>>> rebooting, they report 0 again. The limiting appears to stay in effect
>>>>> though, so it seems that the firmware does keep the values, but Linux
>>>>> does not read them back. Indeed, looking at the code, it seems that
>>>>> actually reading back the values is only implemented for the SM8550.
>>>> Right.
>>>>
>>>> Based on offline information, X1E80100 doesn't support reading back 
>>>> those
>>>> threshold values in battery management firmware, so I can only use the
>>>> cached values for sysfs read.
>>> Which limits usablity of the attribute, it is now impossible to identify
>>> whether it is enabled or disabled. Is there a chance of fixing that for
>>> the X1E80100 platform?
>> Is there a chance we store that value in SDAM and can read it back?
>>
>> Konrad
> 
> The thresholds are stored in PMIC SDAM registers by ADSP after receiving 
> the set requests, and ADSP reads them back during initialization. This 
> is why ADSP retains them upon device reboot.
> 
> I spoke with the battery management firmware team, and they have no 
> plans to update the battery management firmware for X1E80100 further. 
> Consequently, they cannot provide any interfaces to read these 
> thresholds through PMIC Glink.
> 
> Reading them from the existing SDAM registers requires adding "nvmem- 
> cells" DT properties to specify the SDAM registers. However, the 
> "pmic_glink.power-supply" device is an auxiliary device created by the 
> pmic_glink driver and does not have an associated DT node. Is there any 
> method to create a DT node and add DT properties for an auxiliary device?

Auxiliary-bus devices don't have their OF nodes. Instead they use the 
main device's of node thanks to the call to device_set_of_node_from_dev().

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/8] power: supply: qcom_battmgr: Add charge control support

[Konrad Dybcio]

On 6/9/25 9:17 AM, Dmitry Baryshkov wrote:
> On 09/06/2025 05:39, Fenglin Wu wrote:
>>
>> On 6/7/2025 5:46 PM, Konrad Dybcio wrote:
>>> On 6/3/25 12:37 PM, Dmitry Baryshkov wrote:
>>>> On Tue, Jun 03, 2025 at 01:48:11PM +0800, Fenglin Wu wrote:
>>>>> On 5/31/2025 6:36 PM, György Kurucz wrote:
>>>>>>> Add charge control support for SM8550 and X1E80100.
>>>>>> Thank you for this, tested on my Lenovo Yoga Slim 7x, the limiting works
>>>>>> well, I finally don't have to worry about leaving my laptop plugged in
>>>>>> for too long.
>>>>>>
>>>>>> One small thing I noticed is that after setting the sysfs values and
>>>>>> rebooting, they report 0 again. The limiting appears to stay in effect
>>>>>> though, so it seems that the firmware does keep the values, but Linux
>>>>>> does not read them back. Indeed, looking at the code, it seems that
>>>>>> actually reading back the values is only implemented for the SM8550.
>>>>> Right.
>>>>>
>>>>> Based on offline information, X1E80100 doesn't support reading back those
>>>>> threshold values in battery management firmware, so I can only use the
>>>>> cached values for sysfs read.
>>>> Which limits usablity of the attribute, it is now impossible to identify
>>>> whether it is enabled or disabled. Is there a chance of fixing that for
>>>> the X1E80100 platform?
>>> Is there a chance we store that value in SDAM and can read it back?
>>>
>>> Konrad
>>
>> The thresholds are stored in PMIC SDAM registers by ADSP after receiving the set requests, and ADSP reads them back during initialization. This is why ADSP retains them upon device reboot.
>>
>> I spoke with the battery management firmware team, and they have no plans to update the battery management firmware for X1E80100 further. Consequently, they cannot provide any interfaces to read these thresholds through PMIC Glink.
>>
>> Reading them from the existing SDAM registers requires adding "nvmem- cells" DT properties to specify the SDAM registers. However, the "pmic_glink.power-supply" device is an auxiliary device created by the pmic_glink driver and does not have an associated DT node. Is there any method to create a DT node and add DT properties for an auxiliary device?
> 
> Auxiliary-bus devices don't have their OF nodes. Instead they use the main device's of node thanks to the call to device_set_of_node_from_dev().

i.e. something like this is what we want:

---- socname.dtsi ----

pmic-glink {
	compatible = ...;
	[...]

	nvmem-cells = <&charge_limit_lower>, <&charge_limit_upper>;
	nvmem-cell-names = "charge-limit-lower", "charge-limit-upper";
}

---------------------

if you have better names for these cells than what i put here as
placeholders, go for it - just make sure to also add the -names
counterpart, as we won't be able to rely on just indices in the long run

Konrad

Re: [PATCH v2 6/8] dt-bindings: soc: qcom: pmic-glink: Move X1E80100 out of fallbacks

[Konrad Dybcio]

On 6/4/25 11:40 AM, Fenglin Wu wrote:
> 
> On 6/3/2025 5:34 PM, Krzysztof Kozlowski wrote:
>> On 03/06/2025 09:41, Fenglin Wu wrote:
>>> On 6/3/2025 3:06 PM, Krzysztof Kozlowski wrote:
>>>> On 03/06/2025 08:59, Fenglin Wu wrote:
>>>>> On 6/3/2025 2:47 PM, Krzysztof Kozlowski wrote:
>>>>>> On 03/06/2025 08:42, Fenglin Wu wrote:
>>>>>>> On 6/2/2025 3:40 PM, Krzysztof Kozlowski wrote:
>>>>>>>> On 30/05/2025 09:35, Fenglin Wu via B4 Relay wrote:
>>>>>>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>>>>>>
>>>>>>>>> Move X1E80100 out of the fallbacks of SM8550 in pmic-glink support.
>>>>>>>> Why?
>>>>>>>>
>>>>>>>> Do not describe what you do here, it's obvious. We see it from the diff.
>>>>>>>>
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Krzysztof
>>>>>>> Previously, in qcom_battmgr driver, x1e80100 was specified with a match
>>>>>>> data the same as sc8280xp, also sm8550 was treated a fallback of sm8350
>>>>>>> without the need of a match data.
>>>>>>>
>>>>>>> In ucsi_glink driver, sm8550 had a match data and x1e80100 was treated
>>>>>>> as a fallback of sm8550. There was no issues to make x1e80100 as a
>>>>>>> fallback of sm8550 from both qcom_battmgr and ucsi_glink driver perspective.
>>>>>>>
>>>>>>> In patch [5/8] in this series, in qcom_battmgr driver, it added charge
>>>>>>> control functionality for sm8550 and x1e80100 differently hence
>>>>>>> different match data was specified for them, and it makes x1e80100 ad
>>>>>>> sm8550 incompatible and they need to be treated differently.
>>>>>> So you break ABI and that's your problem to fix. You cannot make devices
>>>>>> incompatible without good justification.
>>>>> I would say x1e80100 and sm8550 are different and incompatible from a
>>>>> battery management firmware support perspective. The x1e80100 follows
>>>>> the sc8280xp as a compute platform, whereas the sm8550 follows the
>>>>> sm8350 as a mobile platform.
>>>> Not correct arguments for compatibility.
>>>>
>>>>> The difference between them was initially ignored because the sm8550
>>>>> could use everything that the sm8350 has, and no match data needed to be
>>>>> specified for it. However, now the sm8550 has new features that the
>>>>> sm8350 doesn't have, requiring us to treat it differently, thus the
>>>>> incompatibility was acknowledged.
>>>> So they are perfectly compatible.
>>>>
>>>> I really do not understand what we are discussing here. Explain in
>>>> simple terms of DT spec: what is incompatible that SW cannot use one
>>>> interface to handle the other?
>>> 1. x1e80100 was a fallback of sc8280xp, it used "sc8280xp_bat_psy_desc"
>>
>> No, that's not true. Read the binding again:
>>
>>                - qcom,x1e80100-pmic-glink
>>             - const: qcom,sm8550-pmic-glink
>>
>> No fallback to sc8280xp.
>>
>>
>>> when registering the power supply device.
>>>
>>> 2. sm8550 was a fallback of sm8350, and they all used
>>
>> Also not true. The remaining fallback is not sm8350.
>>
>>
>>> "sm8350_bat_psy_desc" when registering the power supply device.
>>>
>>> 3. x1e80100 and sm8550 they are incompatible as they are using different
>>> data structure of "xxx_bat_psy_desc"  and other “psy_desc" too, such as,
>>> ac/usb/wls.
>> Look at the driver and bindings now - they are compatible. It looks like
>> you made it incompatible and now you claim the "they are incompatible".
>> No, you did it. Look at the driver.
>>
>>
>>
>>> 4. For charge control functionality, it's only supported in the battery
>>> management firmware in x1e80100 and sm8550 platforms. And the change in
>>> battmgr driver (patch [5/8]) adds the support by using 2 additional
>>> power supply properties, which eventually need to be added in the
>>> "properties" data member of "xxx_bat_psy_desc" when registering power
>>> supply devices. Hence, "x1e80100_bat_psy_desc" and "sm8550_bat_psy_desc"
>>> are created and used separately when registering power supply device
>>> according to the "variant" value defined in the match data.
>>>
>>> The main code change is in [5/8], I am pasting a snippet which might
>>> help to explain this a little bit:
>>>
>>> -       if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {
>>> -               battmgr->bat_psy = devm_power_supply_register(dev,
>>> &sc8280xp_bat_psy_desc, &psy_cfg);
>>> +       if (battmgr->variant == QCOM_BATTMGR_SC8280XP ||
>>> battmgr->variant == QCOM_BATTMGR_X1E80100) {
>>> +               if (battmgr->variant == QCOM_BATTMGR_X1E80100)
>>> +                       psy_desc = &x1e80100_bat_psy_desc;
>>> +               else
>>> +                       psy_desc = &sc8280xp_bat_psy_desc;
>>> +
>>> +               battmgr->bat_psy = devm_power_supply_register(dev,
>>> psy_desc, &psy_cfg);
>>>                   if (IS_ERR(battmgr->bat_psy))
>>>                           return dev_err_probe(dev,
>>> PTR_ERR(battmgr->bat_psy),
>>
>> This explains nothing to me. I think you did not get my questions at all
>> and just want to push whatever you have in drivers.
>>
>> Such ping pongs are just tiring, so go back to my previous email, read
>> it carefully and try harder to understand what compatibility means.
>>
>>
>> NAK, you are affecting the users and ABI with justification "I make it
>> now incompatible, so it is incompatible".
>>
>> Best regards,
>> Krzysztof
> 
> Thanks for the explanation with patience. I misunderstood the fallback behavior.
> 
> I was worried about if the compatible string matching would work correctly if both the device node and the driver declared multiple identical compatible strings.
> 
> I understand now and even if the device node and the driver have defined multiple identical compatible strings, the best match which is the most specific compatible string will be found.
> 
> So in the example below, for X1E80100-CRD, the battmgr driver will always match to "qcom,x1e80100-pmic-glink" which is the most specific compatible string defined at the beginning of the device node compatible string, and the compatibility has not been broken.
> 
> In qcom_battmgr driver:
> 
> static const struct of_device_id qcom_battmgr_of_variants[] = {
>         ...
>         { .compatible = "qcom,x1e80100-pmic-glink", .data = (void *)QCOM_BATTMGR_X1E80100 },
>         { .compatible = "qcom,sm8550-pmic-glink", .data = (void *)QCOM_BATTMGR_SM8550 },
>         ...
> };
> 
> In x1-crd.dtsi:
> 
> pmic-glink {
>           compatible = "qcom,x1e80100-pmic-glink",
>                      "qcom,sm8550-pmic-glink",
>                      "qcom,pmic-glink";
>         ...
> 
> }
> 
> Let me know if my understanding is correct. I will drop patch [6/8],[7/8],[8/8] in next version.

Unless we have some mobile-firmware-specific calls/behaviors that apply to
sm8550, but not to x1e80100 (which I don't believe we do), I think this is
fair

Konrad

Re: [PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Sebastian Reichel]

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

Hi,

On Thu, Jun 05, 2025 at 09:34:14AM +0300, Dmitry Baryshkov wrote:
> On Thu, Jun 05, 2025 at 02:08:30PM +0800, Fenglin Wu wrote:
> > 
> > On 6/3/2025 6:35 PM, Dmitry Baryshkov wrote:
> > > > > > +What:		/sys/class/power_supply/<supply_name>/state_of_health
> > > > > > +Date:		May 2025
> > > > > > +Contact:	linux-arm-msm@vger.kernel.org
> > > > > > +Description:
> > > > > > +		Reports battery power supply state of health in percentage.
> > > > > > +
> > > > > > +		Access: Read
> > > > > > +
> > > > > > +		Valid values: 0 - 100 (percent)
> > > > > What does it mean that battery has 77% of health?
> > > > I will update this to explain it better:
> > > > 
> > > > Reports battery power supply state of health in percentage, indicating that the maximum charge capacity has degraded to that percentage of its original designed capacity.
> > > Which basically means that we don't need it in the first place, as we
> > > can read capacity_full and capacity_full_design (or energy_full /
> > > energy_full_design) and divide one onto another.
> > 
> > Hmm, it is true in general to quantify how the battery performance has
> > degraded over time. However, estimating and calculating for battery state of
> > health is much more complicated I think. I am not an expert, but as far as I
> > know, different battery management systems might have different algorithms
> > to calculate the battery health and report it in as percentage. For example,
> > in Qcom battery management firmware, a "soh" parameter is provided as the
> > battery health percentage based on the real-time calculations from learning
> > capacity, resistance estimation, etc.
> 
> Ack, this is more than just full / full_design. Please consider
> expanding ABI description (though in the vendor-neutral way).

No, Dmitry was right. It is exactly the same.

Estimating the battery state of health information is indeed tricky
and complicated. The reason for that is that estimating and
calculating POWER_SUPPLY_PROP_CHARGE_FULL/POWER_SUPPLY_PROP_ENERGY_FULL
(i.e. not the *_FULL_DESIGN) is complicated :)

Greetings,

-- Sebastian

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

Re: [PATCH v2 1/8] power: supply: core: Add resistance power supply property

[Sebastian Reichel]

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

Hi,

On Fri, May 30, 2025 at 03:35:06PM +0800, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> 
> Some battery drivers provide the ability to export resistance as a
> parameter. Add resistance power supply property for that purpose.

This is missing some information and the naming is bad.

Which resistance (I suppose battery internal resistance)?

That is heavily dependent on the battery temperature. So this needs
to document if this is for the current temperature or for some
specific one.

-- Sebastian

> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
>  Documentation/ABI/testing/sysfs-class-power | 10 ++++++++++
>  drivers/power/supply/power_supply_sysfs.c   |  1 +
>  include/linux/power_supply.h                |  1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> index 560124cc31770cde03bcdbbba0d85a5bd78b15a0..22a565a6a1c509461b8c483e12975295765121d6 100644
> --- a/Documentation/ABI/testing/sysfs-class-power
> +++ b/Documentation/ABI/testing/sysfs-class-power
> @@ -552,6 +552,16 @@ Description:
>  			Integer > 0: representing full cycles
>  			Integer = 0: cycle_count info is not available
>  
> +What:		/sys/class/power_supply/<supply_name>/resistance
> +Date:		May 2025
> +Contact:	linux-arm-msm@vger.kernel.org
> +Description:
> +		Reports the resistance of the battery power supply.
> +
> +		Access: Read
> +
> +		Valid values: Represented in microohms
> +
>  **USB Properties**
>  
>  What:		/sys/class/power_supply/<supply_name>/input_current_limit
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index a438f7983d4f6a832e9d479184c7c35453e1757c..dd829148eb6fda5dcd7eab53fc70f99081763714 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -220,6 +220,7 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
>  	POWER_SUPPLY_ATTR(MANUFACTURE_YEAR),
>  	POWER_SUPPLY_ATTR(MANUFACTURE_MONTH),
>  	POWER_SUPPLY_ATTR(MANUFACTURE_DAY),
> +	POWER_SUPPLY_ATTR(RESISTANCE),
>  	/* Properties of type `const char *' */
>  	POWER_SUPPLY_ATTR(MODEL_NAME),
>  	POWER_SUPPLY_ATTR(MANUFACTURER),
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index c4cb854971f53a244ba7742a15ce7a5515da6199..de3e88810e322546470b21258913abc7707c86a7 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -174,6 +174,7 @@ enum power_supply_property {
>  	POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
>  	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
>  	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
> +	POWER_SUPPLY_PROP_RESISTANCE,
>  	/* Properties of type `const char *' */
>  	POWER_SUPPLY_PROP_MODEL_NAME,
>  	POWER_SUPPLY_PROP_MANUFACTURER,
> 
> -- 
> 2.34.1
> 
> 

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

Re: [PATCH v2 1/8] power: supply: core: Add resistance power supply property

[Fenglin Wu]

On 6/22/2025 9:26 AM, Sebastian Reichel wrote:
> Hi,
>
> On Fri, May 30, 2025 at 03:35:06PM +0800, Fenglin Wu via B4 Relay wrote:
>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>
>> Some battery drivers provide the ability to export resistance as a
>> parameter. Add resistance power supply property for that purpose.
> This is missing some information and the naming is bad.
>
> Which resistance (I suppose battery internal resistance)?
>
> That is heavily dependent on the battery temperature. So this needs
> to document if this is for the current temperature or for some
> specific one.
>
> -- Sebastian

This is battery internal resistance calculated by battery management 
system, using the real-time temperature measured by the thermistor 
inside the battery pack.

I can update the name to something like "rt_internal_resistance" and 
update the description accordingly.

>> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>> ---
>>   Documentation/ABI/testing/sysfs-class-power | 10 ++++++++++
>>   drivers/power/supply/power_supply_sysfs.c   |  1 +
>>   include/linux/power_supply.h                |  1 +
>>   3 files changed, 12 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
>> index 560124cc31770cde03bcdbbba0d85a5bd78b15a0..22a565a6a1c509461b8c483e12975295765121d6 100644
>> --- a/Documentation/ABI/testing/sysfs-class-power
>> +++ b/Documentation/ABI/testing/sysfs-class-power
>> @@ -552,6 +552,16 @@ Description:
>>   			Integer > 0: representing full cycles
>>   			Integer = 0: cycle_count info is not available
>>   
>> +What:		/sys/class/power_supply/<supply_name>/resistance
>> +Date:		May 2025
>> +Contact:	linux-arm-msm@vger.kernel.org
>> +Description:
>> +		Reports the resistance of the battery power supply.
>> +
>> +		Access: Read
>> +
>> +		Valid values: Represented in microohms
>> +
>>   **USB Properties**
>>   
>>   What:		/sys/class/power_supply/<supply_name>/input_current_limit
>> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
>> index a438f7983d4f6a832e9d479184c7c35453e1757c..dd829148eb6fda5dcd7eab53fc70f99081763714 100644
>> --- a/drivers/power/supply/power_supply_sysfs.c
>> +++ b/drivers/power/supply/power_supply_sysfs.c
>> @@ -220,6 +220,7 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
>>   	POWER_SUPPLY_ATTR(MANUFACTURE_YEAR),
>>   	POWER_SUPPLY_ATTR(MANUFACTURE_MONTH),
>>   	POWER_SUPPLY_ATTR(MANUFACTURE_DAY),
>> +	POWER_SUPPLY_ATTR(RESISTANCE),
>>   	/* Properties of type `const char *' */
>>   	POWER_SUPPLY_ATTR(MODEL_NAME),
>>   	POWER_SUPPLY_ATTR(MANUFACTURER),
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index c4cb854971f53a244ba7742a15ce7a5515da6199..de3e88810e322546470b21258913abc7707c86a7 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -174,6 +174,7 @@ enum power_supply_property {
>>   	POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
>>   	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
>>   	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
>> +	POWER_SUPPLY_PROP_RESISTANCE,
>>   	/* Properties of type `const char *' */
>>   	POWER_SUPPLY_PROP_MODEL_NAME,
>>   	POWER_SUPPLY_PROP_MANUFACTURER,
>>
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH v2 1/8] power: supply: core: Add resistance power supply property

[Sebastian Reichel]

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

Hi,

On Mon, Jun 30, 2025 at 04:28:14PM +0800, Fenglin Wu wrote:
> On 6/22/2025 9:26 AM, Sebastian Reichel wrote:
> > On Fri, May 30, 2025 at 03:35:06PM +0800, Fenglin Wu via B4 Relay wrote:
> > > From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> > > 
> > > Some battery drivers provide the ability to export resistance as a
> > > parameter. Add resistance power supply property for that purpose.
> > This is missing some information and the naming is bad.
> > 
> > Which resistance (I suppose battery internal resistance)?
> > 
> > That is heavily dependent on the battery temperature. So this needs
> > to document if this is for the current temperature or for some
> > specific one.
> > 
> > -- Sebastian
> 
> This is battery internal resistance calculated by battery management system,
> using the real-time temperature measured by the thermistor inside the
> battery pack.
> 
> I can update the name to something like "rt_internal_resistance" and update
> the description accordingly.

Your message is kind of mixed signal to me.

If the BMS needs the thermistor to calculate the internal
resistance, it means the data is either not real-time, but
just adopting some fixed value to the current temperature,
or the internal resistance is adopted from the current
temperature to some fixed temperature.

My expectation would be, that the BMS instead actually measures the
internal resistance via ohm's and law and Kirchhoff's voltage law.
So please make sure to understand what data is actually provided by
the BMS for a proper ABI description.

Depending on the description I think 'internal_resistance' is a good
name.

Greetings,

-- Sebastian

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

Re: [PATCH v2 2/8] power: supply: core: Add state_of_health power supply property

[Fenglin Wu]

On 6/22/2025 9:17 AM, Sebastian Reichel wrote:
> Hi,
>
> On Thu, Jun 05, 2025 at 09:34:14AM +0300, Dmitry Baryshkov wrote:
>> On Thu, Jun 05, 2025 at 02:08:30PM +0800, Fenglin Wu wrote:
>>> On 6/3/2025 6:35 PM, Dmitry Baryshkov wrote:
>>>>>>> +What:		/sys/class/power_supply/<supply_name>/state_of_health
>>>>>>> +Date:		May 2025
>>>>>>> +Contact:	linux-arm-msm@vger.kernel.org
>>>>>>> +Description:
>>>>>>> +		Reports battery power supply state of health in percentage.
>>>>>>> +
>>>>>>> +		Access: Read
>>>>>>> +
>>>>>>> +		Valid values: 0 - 100 (percent)
>>>>>> What does it mean that battery has 77% of health?
>>>>> I will update this to explain it better:
>>>>>
>>>>> Reports battery power supply state of health in percentage, indicating that the maximum charge capacity has degraded to that percentage of its original designed capacity.
>>>> Which basically means that we don't need it in the first place, as we
>>>> can read capacity_full and capacity_full_design (or energy_full /
>>>> energy_full_design) and divide one onto another.
>>> Hmm, it is true in general to quantify how the battery performance has
>>> degraded over time. However, estimating and calculating for battery state of
>>> health is much more complicated I think. I am not an expert, but as far as I
>>> know, different battery management systems might have different algorithms
>>> to calculate the battery health and report it in as percentage. For example,
>>> in Qcom battery management firmware, a "soh" parameter is provided as the
>>> battery health percentage based on the real-time calculations from learning
>>> capacity, resistance estimation, etc.
>> Ack, this is more than just full / full_design. Please consider
>> expanding ABI description (though in the vendor-neutral way).
> No, Dmitry was right. It is exactly the same.
>
> Estimating the battery state of health information is indeed tricky
> and complicated. The reason for that is that estimating and
> calculating POWER_SUPPLY_PROP_CHARGE_FULL/POWER_SUPPLY_PROP_ENERGY_FULL
> (i.e. not the *_FULL_DESIGN) is complicated :)
>
> Greetings,
>
> -- Sebastian
Hi Sebastian,

Thanks for the comment.

In the qcom_battmgr driver, both POWER_SUPPLY_PROP_CHARGE_FULL and 
POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN properties are already supported, 
and CHARGE_FULL is used to represent the learned battery capacity 
calculated in Qcom BMS. Additionally, the Qcom BMS calculates a 
comprehensive battery SOH parameter by considering multiple factors, 
such as changes in battery impedance, learned capacity over time, and 
charging/discharging cycles. Such as, for the battery impedance change, 
it is specially important for calculating SOH in scenarios with high 
load or low temperatures, as only part of the CHARGE_FULL capacity may 
be usable due to significant voltage drops, especially in aged batteries.

Hence, we proposed adding "state_of_health" support in power supply ABI 
to expose this parameter provided in Qcom BMS which is different from 
the calculation of charge_full / charge_full_design.

Also, Android battery management code [1] is expecting "state_of_health" 
power supply property and it can use it if it's available.

[1] 
https://android.googlesource.com/platform/system/core/+/refs/heads/main/healthd/BatteryMonitor.cpp#927

Thanks

Fenglin

Re: [PATCH v2 1/8] power: supply: core: Add resistance power supply property

[Fenglin Wu]

On 7/7/2025 8:15 AM, Sebastian Reichel wrote:
> Hi,
>
> On Mon, Jun 30, 2025 at 04:28:14PM +0800, Fenglin Wu wrote:
>> On 6/22/2025 9:26 AM, Sebastian Reichel wrote:
>>> On Fri, May 30, 2025 at 03:35:06PM +0800, Fenglin Wu via B4 Relay wrote:
>>>> From: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>>>
>>>> Some battery drivers provide the ability to export resistance as a
>>>> parameter. Add resistance power supply property for that purpose.
>>> This is missing some information and the naming is bad.
>>>
>>> Which resistance (I suppose battery internal resistance)?
>>>
>>> That is heavily dependent on the battery temperature. So this needs
>>> to document if this is for the current temperature or for some
>>> specific one.
>>>
>>> -- Sebastian
>> This is battery internal resistance calculated by battery management system,
>> using the real-time temperature measured by the thermistor inside the
>> battery pack.
>>
>> I can update the name to something like "rt_internal_resistance" and update
>> the description accordingly.
> Your message is kind of mixed signal to me.
>
> If the BMS needs the thermistor to calculate the internal
> resistance, it means the data is either not real-time, but
> just adopting some fixed value to the current temperature,
> or the internal resistance is adopted from the current
> temperature to some fixed temperature.
>
> My expectation would be, that the BMS instead actually measures the
> internal resistance via ohm's and law and Kirchhoff's voltage law.
> So please make sure to understand what data is actually provided by
> the BMS for a proper ABI description.
>
> Depending on the description I think 'internal_resistance' is a good
> name.
>
> Greetings,
>
> -- Sebastian

Hi Sebastian,

Sorry for causing the confusion. I will try to clear it by explaining 
how the battery resistance is calculated in Qcom BMS.

In Qcom BMS, it uses the Equivalent Series Resistance (ESR) parameter to 
represent the battery’s real-time internal resistance. ESR changes 
dynamically depending on factors like the battery’s state of charge 
(SoC), temperature, charging or discharging status. To estimate ESR 
accurately under different conditions, the BMS uses data obtained from 
characterizing representative battery samples, mapping ESR values across 
various temperatures and SoC levels under charging or discharging 
status. The characterization process with those battery samples on test 
bench would use ohm's law to calculate the battery resistance I think. 
These data points serve as a reference for real-time resistance 
estimation. During operation, the BMS software refers to this data and 
adjusts ESR values according to real-time inputs, especially 
temperature, which is typically measured by a thermistor inside the 
battery pack.

I can use 'internal_resistance' if you think this is good to represent 
this ESR parameter.

Thanks

Fenglin