Skip to content

drivers: charger: add driver for bq25713#86791

Merged
kartben merged 1 commit into
zephyrproject-rtos:mainfrom
sophiekovalevsky:feature-add-bq25713-driver
Apr 10, 2025
Merged

drivers: charger: add driver for bq25713#86791
kartben merged 1 commit into
zephyrproject-rtos:mainfrom
sophiekovalevsky:feature-add-bq25713-driver

Conversation

@sophiekovalevsky

Copy link
Copy Markdown
Contributor

Create a driver implementation for the battery charge controller TI BQ25713.

It includes the ability to enable / disable the controller and also to setup max current and voltage charge parameters at initialization time but also at run time.

@rriveramcrus

rriveramcrus commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

image

Hello, I'll take a deeper look at this PR next week but I do want to point out that BQ25710 and BQ25713 are identical parts except that BQ25710 is SMBus and BQ25713 is I2C.

It would be really nice if the core code for the driver could be re-used and the communication code be specific to the respective chip. There are some examples in Zephyr of SPI/I2C drivers that handle sharing common code, I cannot think of any I2C/SMBus drivers off of the top of my head though.

@sophiekovalevsky

Copy link
Copy Markdown
Contributor Author

image

Hello, I'll take a deeper look at this PR next week but I do want to point out that BQ25710 and BQ25713 are identical parts except that BQ25710 is SMBus and BQ25713 is I2C.

It would be really nice if the core code for the driver could be re-used and the communication code be specific to the respective chip. There are some examples in Zephyr of SPI/I2C drivers that handle sharing common code, I cannot think of any I2C/SMBus drivers off of the top of my head though.

Thanks for checking this so quickly. I will check what examples are available at the moment. I just started with bq25713 because thats the version im having at the moment to test, but definitely i can prepare the driver so it can be used with BQ25710.

@carlescufi carlescufi requested a review from tmon-nordic March 10, 2025 15:35
Comment thread drivers/charger/charger_bq25713.c Outdated
Comment thread drivers/charger/charger_bq25713.c Outdated
Comment thread drivers/charger/charger_bq25713.c Outdated
Comment thread drivers/charger/charger_bq25713.c Outdated
Comment thread drivers/charger/charger_bq25713.c Outdated
@sophiekovalevsky

Copy link
Copy Markdown
Contributor Author

@rriveramcrus all suggestions have been introduced in the code.

Regarding making this driver broader to include bq25710 i guess i did not find any so far that would take into consideration that register mapping between these two series are different.

Regarding i2c and smbus transaction, i think using the I2C bus would work the same as for this device on smbus.

@teburd teburd requested a review from albertofloyd March 14, 2025 18:11
@rriveramcrus

Copy link
Copy Markdown
Contributor

Hello @sophiekovalevsky, just wanted to give you an update. I am checking with a couple of folks that may be impacted if we decide to keep the driver you have developed as-is rather than add BQ25710 support. Thank you for your patience in the meantime.

@tmon-nordic tmon-nordic removed their request for review March 19, 2025 07:24
@tmon-nordic

Copy link
Copy Markdown
Contributor

The code looks good to me, but I am not using any board that would have bq25713 on it. I am removing my request for review because bq25713 is battery charger chip that can be configured to adhere to USB specifications with regards to slew rate and VBUS voltage, but itself bq25713 does not process any USB Battery Charging signalling (that happens on D+/D- lines to determine maximum current draw).

Comment thread drivers/charger/charger_bq25713.c Outdated
Create a driver implementation for the battery charge controller
TI BQ25713.

It includes the ability to enable / disable the controller and also
to setup max current and voltage charge parameters at initialization
time but also at run time.

On the other hand, it is possible to assign / obtain input voltage
and current regulation.

Signed-off-by: Kiara Navarro <knavarro@paltatech.com>
@rriveramcrus

Copy link
Copy Markdown
Contributor

@albertofloyd, do you have any concerns wrt the SMBus variant of this chip (BQ25710) not being included initially?

@sophiekovalevsky

Copy link
Copy Markdown
Contributor Author

@rriveramcrus it seems @albertofloyd may not be available to review this? I already request some help but did not get any answer, then, i dont know what should be the procedure based on current state. Any feedback is appreciate, thanks.

@rriveramcrus rriveramcrus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sophiekovalevsky, the driver looks good to me. I think we can proceed and deal with integrating the SMBus variants down the line. Thank you for your contribution!

@kartben kartben requested a review from Copilot April 10, 2025 15:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.

Files not reviewed (4)
  • drivers/charger/CMakeLists.txt: Language not supported
  • drivers/charger/Kconfig: Language not supported
  • drivers/charger/Kconfig.bq25713: Language not supported
  • tests/drivers/build_all/charger/i2c.dtsi: Language not supported

Comment thread dts/bindings/charger/ti,bq25713.yaml
Comment thread dts/bindings/charger/ti,bq25713.yaml

@kartben kartben left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm - thanks for the PR!

@sophiekovalevsky sophiekovalevsky deleted the feature-add-bq25713-driver branch September 15, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Charger platform: TI SimpleLink Texas Instruments SimpleLink MCU

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants