Skip to content

drivers: sensor: npm13xx_charger: improve sample fetching #94546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 15, 2025
Merged

drivers: sensor: npm13xx_charger: improve sample fetching #94546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions drivers/mfd/mfd_npm13xx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,16 @@ int mfd_npm13xx_reg_write(const struct device *dev, uint8_t base, uint8_t offset
return i2c_write_dt(&config->i2c, buff, sizeof(buff));
}

int mfd_npm13xx_reg_write2(const struct device *dev, uint8_t base, uint8_t offset, uint8_t data1,
uint8_t data2)
int mfd_npm13xx_reg_write_burst(const struct device *dev, uint8_t base, uint8_t offset, void *data,
size_t len)
{
const struct mfd_npm13xx_config *config = dev->config;
uint8_t buff[] = {base, offset, data1, data2};
struct i2c_msg msg[2] = {
{.buf = (uint8_t []){base, offset}, .len = 2, .flags = I2C_MSG_WRITE},
{.buf = data, .len = len, .flags = I2C_MSG_WRITE | I2C_MSG_STOP},
};

return i2c_write_dt(&config->i2c, buff, sizeof(buff));
return i2c_transfer_dt(&config->i2c, msg, 2);
}

int mfd_npm13xx_reg_update(const struct device *dev, uint8_t base, uint8_t offset, uint8_t data,
Expand Down
68 changes: 32 additions & 36 deletions drivers/sensor/nordic/npm13xx_charger/npm13xx_charger.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ struct npm13xx_charger_data {
#define ADC_OFFSET_RESULTS 0x10U
#define ADC_OFFSET_IBAT_EN 0x24U

#define ADC_CONV_TIME_US 250U

/* nPM13xx VBUS register offsets */
#define VBUS_OFFSET_ILIMUPDATE 0x00U
#define VBUS_OFFSET_ILIM 0x01U
Expand Down Expand Up @@ -302,6 +304,17 @@ int npm13xx_charger_sample_fetch(const struct device *dev, enum sensor_channel c
struct npm13xx_charger_data *data = dev->data;
struct adc_results_t results;
int ret;
k_timepoint_t conv_done;

/* Trigger current+voltage, NTC and die temp measurements (four in total) */
ret = mfd_npm13xx_reg_write_burst(config->mfd, ADC_BASE, ADC_OFFSET_TASK_VBAT,
(uint8_t []){1U, 1U, 1U}, 3U);
Comment on lines +310 to +311
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The compound literal array should be assigned to a named constant for better readability and maintainability. Consider defining a constant like static const uint8_t adc_trigger_pattern[] = {1U, 1U, 1U};

Copilot uses AI. Check for mistakes.

if (ret != 0) {
return ret;
}

/* Set timepoint for conversion and read status registers in the meantime */
conv_done = sys_timepoint_calc(K_USEC(ADC_CONV_TIME_US * 4));

/* Read charge status and error reason */
ret = mfd_npm13xx_reg_read(config->mfd, CHGR_BASE, CHGR_OFFSET_CHG_STAT, &data->status);
Expand All @@ -314,6 +327,14 @@ int npm13xx_charger_sample_fetch(const struct device *dev, enum sensor_channel c
return ret;
}

/* Read vbus status */
ret = mfd_npm13xx_reg_read(config->mfd, VBUS_BASE, VBUS_OFFSET_STATUS, &data->vbus_stat);
if (ret != 0) {
return ret;
}

k_sleep(sys_timepoint_timeout(conv_done));

/* Read adc results */
ret = mfd_npm13xx_reg_read_burst(config->mfd, ADC_BASE, ADC_OFFSET_RESULTS, &results,
sizeof(results));
Expand All @@ -327,21 +348,6 @@ int npm13xx_charger_sample_fetch(const struct device *dev, enum sensor_channel c
data->current = adc_get_res(results.msb_ibat, results.lsb_b, ADC_LSB_IBAT_SHIFT);
data->ibat_stat = results.ibat_stat;

/* Trigger ntc and die temperature measurements */
ret = mfd_npm13xx_reg_write2(config->mfd, ADC_BASE, ADC_OFFSET_TASK_TEMP, 1U, 1U);
if (ret != 0) {
return ret;
}

/* Trigger current and voltage measurement */
ret = mfd_npm13xx_reg_write(config->mfd, ADC_BASE, ADC_OFFSET_TASK_VBAT, 1U);
if (ret != 0) {
return ret;
}

/* Read vbus status */
ret = mfd_npm13xx_reg_read(config->mfd, VBUS_BASE, VBUS_OFFSET_STATUS, &data->vbus_stat);

return ret;
}

Expand All @@ -354,9 +360,10 @@ static int set_ntc_thresholds(const struct npm13xx_charger_config *const config)
/* Ref: Datasheet Figure 14: Equation for battery temperature */
uint16_t code = (1024 * res) / (res + config->thermistor_ohms);

int ret = mfd_npm13xx_reg_write2(
int ret = mfd_npm13xx_reg_write_burst(
config->mfd, CHGR_BASE, CHGR_OFFSET_NTC_TEMPS + (idx * 2U),
code >> NTCTEMP_MSB_SHIFT, code & NTCTEMP_LSB_MASK);
(uint8_t []){code >> NTCTEMP_MSB_SHIFT, code & NTCTEMP_LSB_MASK},
Comment on lines +363 to +365
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The compound literal array should be assigned to a named variable for better readability. Consider creating a temporary array variable like uint8_t ntc_data[] = {code >> NTCTEMP_MSB_SHIFT, code & NTCTEMP_LSB_MASK};

Copilot uses AI. Check for mistakes.

2U);

if (ret != 0) {
return ret;
Expand All @@ -377,9 +384,10 @@ static int set_dietemp_thresholds(const struct npm13xx_charger_config *const con
DIETEMP_FACTOR_DIV;
uint16_t code = DIV_ROUND_CLOSEST(numerator, DIETEMP_FACTOR_MUL);

int ret = mfd_npm13xx_reg_write2(
int ret = mfd_npm13xx_reg_write_burst(
config->mfd, CHGR_BASE, CHGR_OFFSET_DIE_TEMPS + (idx * 2U),
code >> DIETEMP_MSB_SHIFT, code & DIETEMP_LSB_MASK);
(uint8_t []){code >> DIETEMP_MSB_SHIFT, code & DIETEMP_LSB_MASK},
Comment on lines +387 to +389
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The compound literal array should be assigned to a named variable for better readability. Consider creating a temporary array variable like uint8_t temp_data[] = {code >> DIETEMP_MSB_SHIFT, code & DIETEMP_LSB_MASK};

Copilot uses AI. Check for mistakes.

2U);

if (ret != 0) {
return ret;
Expand Down Expand Up @@ -582,16 +590,16 @@ int npm13xx_charger_init(const struct device *dev)
ret = mfd_npm13xx_reg_write(config->mfd, CHGR_BASE, CHGR_OFFSET_ISET, idx);
} else {
/* Set charge current MSB and LSB and discharge limit for nPM1300 */
ret = mfd_npm13xx_reg_write2(config->mfd, CHGR_BASE, CHGR_OFFSET_ISET, idx / 2U,
idx & 1U);
ret = mfd_npm13xx_reg_write_burst(config->mfd, CHGR_BASE, CHGR_OFFSET_ISET,
(uint8_t []){idx / 2U, idx & 1U}, 2U);
Comment on lines +593 to +594
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The compound literal array should be assigned to a named variable for better readability. Consider creating a temporary array variable like uint8_t charge_data[] = {idx / 2U, idx & 1U};

Suggested change
ret = mfd_npm13xx_reg_write_burst(config->mfd, CHGR_BASE, CHGR_OFFSET_ISET,
(uint8_t []){idx / 2U, idx & 1U}, 2U);
uint8_t charge_data[] = {idx / 2U, idx & 1U};
ret = mfd_npm13xx_reg_write_burst(config->mfd, CHGR_BASE, CHGR_OFFSET_ISET,
charge_data, 2U);

Copilot uses AI. Check for mistakes.

if (ret != 0) {
return ret;
}

ret = mfd_npm13xx_reg_write2(
ret = mfd_npm13xx_reg_write_burst(
config->mfd, CHGR_BASE, CHGR_OFFSET_ISET_DISCHG,
npm1300_discharge_limits[config->dischg_limit_idx] / 2U,
npm1300_discharge_limits[config->dischg_limit_idx] & 1U);
(uint8_t []){npm1300_discharge_limits[config->dischg_limit_idx] / 2U,
npm1300_discharge_limits[config->dischg_limit_idx] & 1U}, 2U);
Comment on lines +599 to +602
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The compound literal array should be assigned to a named variable for better readability. Consider creating a temporary array variable like uint8_t discharge_data[] = {npm1300_discharge_limits[config->dischg_limit_idx] / 2U, npm1300_discharge_limits[config->dischg_limit_idx] & 1U};

Copilot uses AI. Check for mistakes.

}
if (ret != 0) {
return ret;
Expand Down Expand Up @@ -628,18 +636,6 @@ int npm13xx_charger_init(const struct device *dev)
return ret;
}

/* Trigger current and voltage measurement */
ret = mfd_npm13xx_reg_write(config->mfd, ADC_BASE, ADC_OFFSET_TASK_VBAT, 1U);
if (ret != 0) {
return ret;
}

/* Trigger ntc and die temperature measurements */
ret = mfd_npm13xx_reg_write2(config->mfd, ADC_BASE, ADC_OFFSET_TASK_TEMP, 1U, 1U);
if (ret != 0) {
return ret;
}

/* Enable automatic temperature measurements during charging */
ret = mfd_npm13xx_reg_write(config->mfd, ADC_BASE, ADC_OFFSET_TASK_AUTO, 1U);
if (ret != 0) {
Expand Down
12 changes: 6 additions & 6 deletions include/zephyr/drivers/mfd/npm13xx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,18 @@ int mfd_npm13xx_reg_read(const struct device *dev, uint8_t base, uint8_t offset,
int mfd_npm13xx_reg_write(const struct device *dev, uint8_t base, uint8_t offset, uint8_t data);

/**
* @brief Write two registers to npm13xx
* @brief Write multiple registers to npm13xx
*
* @param dev npm13xx mfd device
* @param base Register base address (bits 15..8 of 16-bit address)
* @param offset Register offset address (bits 7..0 of 16-bit address)
* @param data1 first byte of data to write
* @param data2 second byte of data to write
* @param offset First register offset address (bits 7..0 of 16-bit address)
* @param data Pointer to buffer to write
* @param len Number of bytes to write
* @retval 0 If successful
* @retval -errno In case of any bus error (see i2c_write_dt())
*/
int mfd_npm13xx_reg_write2(const struct device *dev, uint8_t base, uint8_t offset, uint8_t data1,
uint8_t data2);
int mfd_npm13xx_reg_write_burst(const struct device *dev, uint8_t base, uint8_t offset, void *data,
size_t len);

/**
* @brief Update selected bits in npm13xx register
Expand Down