Skip to content
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.

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

Creates Charging Subsystem #2

Closed
wants to merge 3 commits into from
Closed

Creates Charging Subsystem #2

wants to merge 3 commits into from

Conversation

rriveramcrus
Copy link
Collaborator

Adds a charging subsystem to the Zephyr kernel akin to the fuel gauge subsystem. Introduces a sample sbs charger driver with basic testing framework.

@rriveramcrus rriveramcrus force-pushed the charger branch 4 times, most recently from 801f60c to bf71ab9 Compare April 3, 2023 15:49
Add initial charger driver API with the most basic of native_posix
driver tests.

Signed-off-by: Ricardo Rivera-Matos <[email protected]>
@xyzzy42
Copy link

xyzzy42 commented Apr 6, 2023

Currently charger drivers use the Sensor API. Which allows for reading specific data channels, setting properties, and getting events.

It seems like this has only a method to get property values. Is this really any different than what is done now with sensor channels?

There doesn't appear to be any way to get notified of events. Change state changing, error conditions, etc.

@rriveramcrus
Copy link
Collaborator Author

Hey @xyzzy42, the MVP here is to port the Linux sbs_charger and create a dedicated charging API. I kept it as small as possible for the eventual upstream PR and only introduced the charging API props that would be used by the sbs_charger.

Once the basic API is upstreamed, subsequent PRs for more feature rich charging ICs would grow the API. The charging API is intended to pair with the recently upstream fuel gauge API to create a complete power supply subsystem in Zephyr as is done in Linux. (zephyrproject-rtos#44847)

I couldn't find an example of a charging IC driver in the sensor subsystem, but I could find a couple of fuel gauge ICs (max17055, max17262, bq247xx). Could you share the charging IC driver you are referring to?

@xyzzy42
Copy link

xyzzy42 commented Apr 6, 2023

I wrote a BQ25180 driver using the Sensor API. Which supports more controls than those here and also supports charging events. While existing drivers aren't really chargers, they end up sharing some of the same fields as chargers.

I don't quite see the point of making a new API that is just a subset of an existing one and solves no problems, just to say there is a new API. New APIs add complexity and more code.

There should be a problem statement. Existing methods have these limitations. And then the proposed method should then solve those limitations.

@rriveramcrus
Copy link
Collaborator Author

The BQ25180 is a very simple charging IC aimed at IoT and wearables, so I agree that the added complexity would probably not benefit that application and would be well served by the sensor API.
Let's remember though that ZephyrOS exists in more complicated applications such as the EC on a notebook motherboard. In an application such as that there are often several components in the charging subsystem that need to interact with each other. For example, a USB PD controller talking to a fuel gauge to request a CC/CV negotiation to then select between a main or potential side car charging ICs. These interactions between ICs from potentially different vendors could be served by common code down the line. An example of this would be the "power_supply_get_property_from_supplier()" function in the Linux power supply class.
It could become quite messy to leverage the sensor API to integrate a charging IC such as BQ25790 or BQ25730 (aimed at notebooks). I tried to do so with an upcoming IC we are developing here (#1) and ran into some of these issues and some concern from stakeholders with shoehorning the driver into the sensor API. After receiving this feedback I felt it was necessary for these mission critical drivers to live in their own subsystem.

@rriveramcrus
Copy link
Collaborator Author

@xyzzy42, this discussion wrt to Zephyr architecture is valuable and I would hate for it to get lost on our Cirrus Zephyr OS fork. I will put together an RFC on zephyrproject/zephyr and share the link with you here once it is available.

@rriveramcrus
Copy link
Collaborator Author

@xyzzy42, here is the RFC: zephyrproject-rtos#56635

Adds a sample sbs charger driver and basics tests.

Signed-off-by: Ricardo Rivera-Matos <[email protected]>
Adds a short stub doc as a placeholder for future documentation in the
charger API.

Signed-off-by: Ricardo Rivera-Matos <[email protected]>
@rriveramcrus rriveramcrus marked this pull request as ready for review April 7, 2023 20:46
@rriveramcrus
Copy link
Collaborator Author

Moving discussion to zephyrproject-rtos#56666

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants