Skip to content

type-c-service/tps6699x: Fix EPR capable flag not being set - #941

Open
RobertZ2011 wants to merge 3 commits into
OpenDevicePartnership:stable-v0.1.yfrom
RobertZ2011:epr-flag-fix
Open

type-c-service/tps6699x: Fix EPR capable flag not being set#941
RobertZ2011 wants to merge 3 commits into
OpenDevicePartnership:stable-v0.1.yfrom
RobertZ2011:epr-flag-fix

Conversation

@RobertZ2011

@RobertZ2011 RobertZ2011 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This flag was not being set properly. Also add two more useful fields to PortStatus.

@RobertZ2011 RobertZ2011 self-assigned this Aug 7, 2026
@RobertZ2011
RobertZ2011 marked this pull request as ready for review August 7, 2026 16:12
@RobertZ2011
RobertZ2011 requested review from a team as code owners August 7, 2026 16:12
@RobertZ2011
RobertZ2011 requested review from asasine, gjpmsft, jerrysxie, kurtjd, tullom and williampMSFT and a lite review from Copilot August 7, 2026 16:12
@RobertZ2011 RobertZ2011 added bug Something isn't working type-c Related to the type-c service or drivers. labels Aug 7, 2026

Copilot AI 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.

Pull request overview

This PR fixes PortStatus.epr not being populated when using the TPS6699x Type‑C/PD controller driver, by extracting the EPR-capable bit from the partner’s first received Source PDO (the 5V fixed SPR PDO). It also updates the PortStatus field documentation to describe epr as “port partner is EPR capable”. The intent is to make the EPR capability visible to higher layers that consume PortStatus.

Changes:

  • Parse the first received Source PDO as a fixed PDO and use its flags to populate dual_power, unconstrained_power, and the newly-set epr field.
  • Update PortStatus.epr doc comment to reflect “port partner is EPR capable”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
type-c-service/src/driver/tps6699x.rs Sets PortStatus.epr (and refactors related flag extraction) based on the first received fixed Source PDO.
embedded-service/src/type_c/controller.rs Updates the documentation describing what PortStatus.epr represents.

Comment thread embedded-service/src/type_c/controller.rs
asasine
asasine previously approved these changes Aug 7, 2026
Comment thread type-c-service/src/driver/tps6699x.rs Outdated
Comment on lines +379 to +381
port_status.dual_power = fixed_source_pdo.dual_role_power;
port_status.unconstrained_power = fixed_source_pdo.unconstrained_power;
port_status.epr = fixed_source_pdo.epr_capable;

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.

Use an exhaustive pattern to not forget fields by accident.

let Pdo { a, b, c } = pdo; // note: no `..`. Use `field: _` to intentionally discard one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, I've also added dual_data and usb_comms_capable to PortStatus to support more functionality for downstream users. The other ignored fields are either irrelevant (current/voltage is already pulled from the PDO), or doesn't strike me as particularly useful.

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.

What about putting the whole PDO in?

kurtjd
kurtjd previously approved these changes Aug 7, 2026
@RobertZ2011
RobertZ2011 dismissed stale reviews from kurtjd and asasine via 7f74126 August 7, 2026 18:12
@RobertZ2011
RobertZ2011 requested review from asasine and kurtjd August 7, 2026 18:17

@asasine asasine 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.

The whole PDO would avoid ambiguity with what each field refers to, and would save us from needing to revisit this again for yet-another-field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working type-c Related to the type-c service or drivers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants