driver: fuel_gauge/sy24561: add driver #93525

franckduriez · 2025-07-22T16:23:44Z

Add driver for silergy sy24561 fuel gauge

github-actions · 2025-07-22T16:24:25Z

Hello @franckduriez, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

DBS06

First of all, thanks for your contribution, we really appreciate that and overall it looks well done!

I also looked inside the datasheet and as far as I see you implemented the two most basic functions. Consider exposing more properties, e.g. temperature compensation (you mentioned that in the silergy,sy24561.yaml), alarms/alerts, STATUS Register (which indicates if the battery will be charged/discharged), etc., which are available on this IC.
Especially the temperature compensation seems quite interesting, as according to the datasheet this increases the SOC accuracy.

Please also add an e.g. sy24561-emulation, for example emul_lc709203f.c, emul_max17048, or emul_bq27z746.c and also a test (look here), because this helps a lot to avoid possible future breaking changes and further increase the test coverage and quality of Zephyr in general.

drivers/fuel_gauge/sy24561/sy24561.c

DBS06 · 2025-07-23T07:13:30Z

drivers/fuel_gauge/sy24561/sy24561.c

+#include <zephyr/kernel.h>
+#include <zephyr/logging/log.h>
+#include <zephyr/sys/byteorder.h>
+


You are using the PRIu8 macro here LOG_DBG("RSOC: %" PRIu8 "%%", val->relative_state_of_charge);, it would be better to also include inttypes.h explicitly, even if it seems to be included implicitly by some other header.

Suggested change

#include <inttypes.h>

DBS06 · 2025-07-23T07:21:11Z

drivers/fuel_gauge/sy24561/sy24561.c

+		return -ENODEV;
+	}
+
+	val->voltage = (((sys_be16_to_cpu(voltage) * 2500 / 0x1000)) + 2500);


These are magic numbers, I know its taken directly from the datasheet, but at least write a comment that the scale and offset (2500) and/or the whole formula are taken from the datasheet so its clear where the magic numbers and formula come from.

Furthermore, do the sys_be16_to_cpu(voltage) inside of sy24561_read_reg) as I suggested.

DBS06 · 2025-07-23T07:30:01Z

drivers/fuel_gauge/sy24561/sy24561.c

+static int sy24561_read_reg(const struct device *dev, uint16_t *output, uint8_t addr)
+{
+	const struct sy24561_config *config = dev->config;
+
+	return i2c_write_read_dt(&config->i2c, &addr, sizeof(addr), output, sizeof(*output));
+}


Suggested change

static int sy24561_read_reg(const struct device *dev, uint16_t *output, uint8_t addr)

{

const struct sy24561_config *config = dev->config;

return i2c_write_read_dt(&config->i2c, &addr, sizeof(addr), output, sizeof(*output));

}

static int sy24561_read_reg(const struct device *dev, uint8_t reg, uint16_t *value)

{

uint8_t buffer[2];

const struct sy24561_config *config = dev->config;

int ret = i2c_write_read_dt(&config->i2c, &reg, sizeof(reg), buffer, sizeof(buffer));

if (ret) {

LOG_ERR("i2c_write_read failed (reg 0x%02x): %d", reg, ret);

return ret;

}

*value = sys_get_be16(buf);

return ret;

DBS06 · 2025-07-23T07:32:58Z

drivers/fuel_gauge/sy24561/sy24561.c

+		return -ENODEV;
+	}
+
+	val->relative_state_of_charge = 100 * sys_be16_to_cpu(soc) / 0xffff;


Same here, these are magic numbers, I know its taken directly from the datasheet, but at least add a comment that the formula with its values are taken from the datasheet so its clear where the magic numbers and formula come from.

Furthermore, do the sys_be16_to_cpu(soc) inside of sy24561_read_reg() as I suggested.

DBS06 · 2025-07-23T07:38:37Z

drivers/fuel_gauge/sy24561/sy24561.c

+	case FUEL_GAUGE_RELATIVE_STATE_OF_CHARGE:
+		return sy24561_get_soc(dev, val);
+	default:
+		LOG_ERR("Not supported");


If you add a logging error be more specific, e.g.:

Suggested change

LOG_ERR("Not supported");

LOG_ERR("Property %d not supported", prop);

franckduriez · 2025-07-23T16:18:37Z

Hello @DBS06,

I tried to integrate your remarks, except for the emulation and test that I will make later.
Can you give me some feedback on the fuel gauge property usage? I made some comments when things were not clear to me.

For the new registers added in the latest version, I will be able to test them only once I receive my first PCB prototypes.

DBS06

Hi @franckduriez

I reviewed it again. First, don't be scared by the amount of requested changes, most of them are basically the same! I see that you invested a lot of time and effort, which is great!

except for the emulation and test that I will make later.

Ok

Can you give me some feedback on the fuel gauge property usage? I made some comments when things were not clear to me.

I don't see your comments, where are they? Nevertheless, I think I gave you feedback with my new review.

For the new registers added in the latest version, I will be able to test them only once I receive my first PCB prototypes.

Do you mean the ones you recently added? I wrote "consider", if you can't test them, it would have been okay to not to implement them right now and do and update later when you can. If you write a test, you can test the new added properties over them (test-driven-development). Another possibility would be to just wait until you receive your PCB, its up to you.

Kr

DBS06 · 2025-07-24T06:37:31Z

drivers/fuel_gauge/sy24561/sy24561.c

+
+	sys_put_be16(value, buffer + 1);
+
+	int const ret = i2c_write_read_dt(&config->i2c, &reg, sizeof(reg), NULL, 0);


Suggested change

int const ret = i2c_write_read_dt(&config->i2c, &reg, sizeof(reg), NULL, 0);

int ret = i2c_write_dt(&config->i2c, buffer, sizeof(buffer));

DBS06 · 2025-07-24T06:39:58Z

drivers/fuel_gauge/sy24561/sy24561.c

+		return -ENODEV;
+	}
+
+	// Scaling formula taken from datasheet at page 5


Suggested change

// Scaling formula taken from datasheet at page 5

/* Scaling formula taken from datasheet at page 5 */

And for all occurrences

DBS06 · 2025-07-24T06:44:48Z

drivers/fuel_gauge/sy24561/sy24561.c

+{
+	switch (prop) {
+	case FUEL_GAUGE_VOLTAGE:
+		return sy24561_get_voltage(dev, val);


It's better to pass the dedicated value from union fuel_gauge_prop_val *val instead of the whole union.
Note: Voltage is expected to be in [µV]

Suggested change

return sy24561_get_voltage(dev, val);

return sy24561_get_voltage(dev, &val->voltage);

DBS06 · 2025-07-24T07:25:44Z

drivers/fuel_gauge/sy24561/sy24561.c

+	return 0;
+}
+
+static int sy24561_set_temperature(const struct device *dev, union fuel_gauge_prop_val val)


You will receive the temperature in deci Kelvin [0.1 °K] not in [°C]
see here

Futhermore, change the interface to:

Suggested change

static int sy24561_set_temperature(const struct device *dev, union fuel_gauge_prop_val val)

static int sy24561_set_temperature(const struct device *dev, uint16_t temperature)

DBS06 · 2025-07-24T07:50:20Z

drivers/fuel_gauge/sy24561/sy24561.c

+	int16_t const temperature = val.temperature;
+
+	// This comes from datasheet at page 5
+	int16_t const temperature_max = 60;
+	int16_t const temperature_min = -20;
+
+	if (temperature > temperature_max) {
+		LOG_ERR("Temperature should be <= %" PRIi16, temperature_max);
+		return -EINVAL;
+	}
+
+	if (temperature > temperature_min) {
+		LOG_ERR("Temperature should be >= %" PRIi16, temperature_min);
+		return -EINVAL;
+	}
+
+	// This comes from datasheet at page 5 and 6
+	uint16_t const temperature_mask = 0xff;
+	uint16_t const temperature_reg_value = 40 + temperature;


It would be easier to work with deci Kelvin (avoids converting from unsigned to signed and back), compensate values which are out of the temperature range of the IC, and in the end convert it to °C+40.

Add #define DECI_KELVIN_TO_CELSIUS 2732 at the top of the file, to the other constants.

Suggested change

int16_t const temperature = val.temperature;

// This comes from datasheet at page 5

int16_t const temperature_max = 60;

int16_t const temperature_min = -20;

if (temperature > temperature_max) {

LOG_ERR("Temperature should be <= %" PRIi16, temperature_max);

return -EINVAL;

}

if (temperature > temperature_min) {

LOG_ERR("Temperature should be >= %" PRIi16, temperature_min);

return -EINVAL;

}

// This comes from datasheet at page 5 and 6

uint16_t const temperature_mask = 0xff;

uint16_t const temperature_reg_value = 40 + temperature;

/* This is taken from datasheet at page 5 */

uint16_t const temperature_min = 2532; /* in [0.1°K] == -20[°C] */

uint16_t const temperature_max = 3332; /* in [0.1°K] == 60[°C] */

if (temperature > temperature_max) {

LOG_WRN("Temperature above max temperature of 60°C forced to 60°C);

temperature = temperature_max;

}

if (temperature < temperature_min) {

LOG_WRN("Temperature below min temperature of -20°C forced to -20°C);

temperature = temperature_min;

}

/* convert deci-Kelvin [0.1°K] to Celsius + 40°C offset; constants are taken from datasheet at page 5 and 6 */

uint16_t const temperature_reg_value = ((temperature + 400 - DECI_KELVIN_TO_CELSIUS) / 10);

uint16_t const temperature_mask = 0xff;

DBS06 · 2025-07-24T09:45:58Z

drivers/fuel_gauge/sy24561/sy24561.c

+{
+	uint16_t voltage = 0;
+
+	if (sy24561_read_reg(dev, SY24561_REG_VBAT, &voltage) != 0) {


Suggested change

if (sy24561_read_reg(dev, SY24561_REG_VBAT, &voltage) != 0) {

if (sy24561_read_reg(dev, SY24561_REG_VBAT, &tmp_voltage) != 0) {

DBS06 · 2025-07-24T09:49:48Z

drivers/fuel_gauge/sy24561/sy24561.c

+	}
+
+	// Scaling formula taken from datasheet at page 5
+	val->voltage = (((voltage * 2500 / 0x1000)) + 2500);


Changes according to interface change.
Note: FUEL_GAUGE_VOLTAGE expects voltage in µV, here you pass it back in mV.

Suggested change

val->voltage = (((voltage * 2500 / 0x1000)) + 2500);

*voltage = (((tmp_voltage * 2500 / 0x1000)) + 2500) * 1000;

DBS06 · 2025-07-24T09:51:30Z

drivers/fuel_gauge/sy24561/sy24561.c

+	// This comes from datasheet at page 6
+	uint16_t charging = IS_BIT_SET(status, 8);
+
+	val->current_direction = charging;


Change according to interface change

Suggested change

val->current_direction = charging;

*current_direction = charging;

DBS06 · 2025-07-24T09:52:45Z

drivers/fuel_gauge/sy24561/sy24561.c

+	return 0;
+}
+
+static int sy24561_get_status(const struct device *dev, union fuel_gauge_prop_val *val)


It's better to pass the dedicated value from union fuel_gauge_prop_val *val instead of the whole union.

Suggested change

static int sy24561_get_status(const struct device *dev, union fuel_gauge_prop_val *val)

static int sy24561_get_status(const struct device *dev, uint16_t *fg_status)

DBS06 · 2025-07-24T09:53:28Z

drivers/fuel_gauge/sy24561/sy24561.c

+	// This comes from datasheet at page 6
+	uint16_t alarm = IS_BIT_SET(config, 5);
+
+	val->fg_status = alarm;


Change according to interface change

Suggested change

val->fg_status = alarm;

*fg_status = alarm;

DBS06 · 2025-07-24T10:48:35Z

drivers/fuel_gauge/sy24561/sy24561.c

+
+LOG_MODULE_REGISTER(SY24561, CONFIG_FUEL_GAUGE_LOG_LEVEL);
+
+enum {


Suggested change

enum {

enum sy24561_regs {

DBS06 · 2025-07-24T10:55:54Z

drivers/fuel_gauge/sy24561/sy24561.c

+static int sy24561_get_buffer_prop(const struct device *dev, fuel_gauge_prop_t prop, void *dst,
+				   size_t dst_len)
+{
+	int rc = 0;
+
+	switch (prop) {
+	case FUEL_GAUGE_MANUFACTURER_NAME:
+		if (dst_len == sizeof(struct sbs_gauge_manufacturer_name)) {
+			struct sbs_gauge_manufacturer_name *const output = dst;
+			char const manufacturer[] = "Sylergy";
+			output->manufacturer_name_length = sizeof(manufacturer) - 1;
+			memcpy(output->manufacturer_name, manufacturer,
+			       output->manufacturer_name_length);
+		} else {
+			rc = -EINVAL;
+		}
+		break;
+	case FUEL_GAUGE_DEVICE_NAME:
+		if (dst_len == sizeof(struct sbs_gauge_device_name)) {
+			struct sbs_gauge_device_name *const output = dst;
+			char const name[] = "SY24561";
+			output->device_name_length = sizeof(name) - 1;
+			memcpy(output->device_name, name, output->device_name_length);
+		} else {
+			rc = -EINVAL;
+		}
+		break;
+	case FUEL_GAUGE_DEVICE_CHEMISTRY:
+		if (dst_len == sizeof(struct sbs_gauge_device_chemistry)) {
+			struct sbs_gauge_device_chemistry *const output = dst;
+			char const chem[] = "LION";
+			output->device_chemistry_length = sizeof(chem) - 1;
+			memcpy(output->device_chemistry, chem, output->device_chemistry_length);
+		} else {
+			rc = -EINVAL;
+		}
+		break;
+	default:
+		LOG_ERR("Property %d not supported", (int)prop);
+		rc = -ENOTSUP;
+	}
+
+	return rc;
+}


If it is not a property which can be read from the chip, don't do it! Therefore, the whole function can be removed.

Suggested change

static int sy24561_get_buffer_prop(const struct device *dev, fuel_gauge_prop_t prop, void *dst,

size_t dst_len)

{

int rc = 0;

switch (prop) {

case FUEL_GAUGE_MANUFACTURER_NAME:

if (dst_len == sizeof(struct sbs_gauge_manufacturer_name)) {

struct sbs_gauge_manufacturer_name *const output = dst;

char const manufacturer[] = "Sylergy";

output->manufacturer_name_length = sizeof(manufacturer) - 1;

memcpy(output->manufacturer_name, manufacturer,

output->manufacturer_name_length);

} else {

rc = -EINVAL;

}

break;

case FUEL_GAUGE_DEVICE_NAME:

if (dst_len == sizeof(struct sbs_gauge_device_name)) {

struct sbs_gauge_device_name *const output = dst;

char const name[] = "SY24561";

output->device_name_length = sizeof(name) - 1;

memcpy(output->device_name, name, output->device_name_length);

} else {

rc = -EINVAL;

}

break;

case FUEL_GAUGE_DEVICE_CHEMISTRY:

if (dst_len == sizeof(struct sbs_gauge_device_chemistry)) {

struct sbs_gauge_device_chemistry *const output = dst;

char const chem[] = "LION";

output->device_chemistry_length = sizeof(chem) - 1;

memcpy(output->device_chemistry, chem, output->device_chemistry_length);

} else {

rc = -EINVAL;

}

break;

default:

LOG_ERR("Property %d not supported", (int)prop);

rc = -ENOTSUP;

}

return rc;

}

Add driver for silergy sy24561 fuel gauge Signed-off-by: Franck Duriez <franck.lucien.duriez@gmail.com>

Add emulator for fuel gauge SY24561 Signed-off-by: Franck Duriez <franck.lucien.duriez@gmail.com>

Add test for fuel gauge SY24561 Signed-off-by: Franck Duriez <franck.lucien.duriez@gmail.com>

franckduriez · 2025-07-25T19:21:13Z

Hello @DBS06

I added the emulator and test.
I also was able to test the full driver on an operational PCB

Let me know if changes are still required

sonarqubecloud · 2025-07-25T19:45:26Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

zephyrbot added the area: Fuel Gauge label Jul 22, 2025

zephyrbot requested review from aaronemassey, DBS06 and teburd July 22, 2025 16:24

zephyrbot assigned aaronemassey and teburd Jul 22, 2025

franckduriez force-pushed the driver-fuelgauge-sy24561 branch 2 times, most recently from 7c66c3f to d4403da Compare July 22, 2025 18:31

DBS06 requested changes Jul 23, 2025

View reviewed changes

franckduriez force-pushed the driver-fuelgauge-sy24561 branch from d4403da to b609396 Compare July 23, 2025 16:12

DBS06 requested changes Jul 24, 2025

View reviewed changes

DBS06 reviewed Jul 24, 2025

View reviewed changes

franckduriez force-pushed the driver-fuelgauge-sy24561 branch from b609396 to ccb2684 Compare July 25, 2025 16:36

franckduriez added 3 commits July 25, 2025 21:17

driver: fuel_gauge/sy24561: add driver

f350717

Add driver for silergy sy24561 fuel gauge Signed-off-by: Franck Duriez <franck.lucien.duriez@gmail.com>

driver: fuel_gauge/sy24561: add emulator

96aed3b

Add emulator for fuel gauge SY24561 Signed-off-by: Franck Duriez <franck.lucien.duriez@gmail.com>

test: driver/fuel_gauge/sy24561: add test

b303902

Add test for fuel gauge SY24561 Signed-off-by: Franck Duriez <franck.lucien.duriez@gmail.com>

franckduriez force-pushed the driver-fuelgauge-sy24561 branch from ccb2684 to b303902 Compare July 25, 2025 19:19

-static int sy24561_read_reg(const struct device *dev, uint16_t *output, uint8_t addr)
-{
-	const struct sy24561_config *config = dev->config;
-	return i2c_write_read_dt(&config->i2c, &addr, sizeof(addr), output, sizeof(*output));
-}
+static int sy24561_read_reg(const struct device *dev, uint8_t reg, uint16_t *value)
+{
+	uint8_t buffer[2];
+	const struct sy24561_config *config = dev->config;
+	int ret = i2c_write_read_dt(&config->i2c, &reg, sizeof(reg), buffer, sizeof(buffer));
+	if (ret) {
+		LOG_ERR("i2c_write_read failed (reg 0x%02x): %d", reg, ret);
+		return ret;
+	}
+	*value = sys_get_be16(buf);
+	return ret;

	LOG_ERR("Not supported");
	LOG_ERR("Property %d not supported", prop);


		sys_put_be16(value, buffer + 1);

		int const ret = i2c_write_read_dt(&config->i2c, &reg, sizeof(reg), NULL, 0);

	int const ret = i2c_write_read_dt(&config->i2c, &reg, sizeof(reg), NULL, 0);
	int ret = i2c_write_dt(&config->i2c, buffer, sizeof(buffer));

	// Scaling formula taken from datasheet at page 5
	/* Scaling formula taken from datasheet at page 5 */

	return sy24561_get_voltage(dev, val);
	return sy24561_get_voltage(dev, &val->voltage);

	static int sy24561_set_temperature(const struct device *dev, union fuel_gauge_prop_val val)
	static int sy24561_set_temperature(const struct device *dev, uint16_t temperature)

	if (sy24561_read_reg(dev, SY24561_REG_VBAT, &voltage) != 0) {
	if (sy24561_read_reg(dev, SY24561_REG_VBAT, &tmp_voltage) != 0) {

	val->voltage = (((voltage * 2500 / 0x1000)) + 2500);
	voltage = (((tmp_voltage 2500 / 0x1000)) + 2500) * 1000;

	val->current_direction = charging;
	*current_direction = charging;

	static int sy24561_get_status(const struct device dev, union fuel_gauge_prop_val val)
	static int sy24561_get_status(const struct device dev, uint16_t fg_status)


		LOG_MODULE_REGISTER(SY24561, CONFIG_FUEL_GAUGE_LOG_LEVEL);

		enum {

driver: fuel_gauge/sy24561: add driver #93525

Are you sure you want to change the base?

driver: fuel_gauge/sy24561: add driver #93525

Conversation

franckduriez commented Jul 22, 2025

Uh oh!

github-actions bot commented Jul 22, 2025

Uh oh!

DBS06 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DBS06 Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DBS06 Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DBS06 Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franckduriez commented Jul 23, 2025

Uh oh!

DBS06 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DBS06 Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franckduriez commented Jul 25, 2025

Uh oh!

sonarqubecloud bot commented Jul 25, 2025

Quality Gate passed

Uh oh!

Uh oh!

DBS06 Jul 23, 2025 •

edited

Loading

DBS06 Jul 23, 2025 •

edited

Loading

DBS06 Jul 23, 2025 •

edited

Loading

DBS06 Jul 24, 2025 •

edited

Loading