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

power: Set XLP_OUT at power off #382

Closed
wants to merge 1 commit into from
Closed

power: Set XLP_OUT at power off #382

wants to merge 1 commit into from

Conversation

crawfxrd
Copy link
Member

Disable VDD3 once at power off instead of setting it every cycle in the LED update logic.

Disable VDD3 once at power off instead of setting it every cycle in the
LED update logic.

Signed-off-by: Tim Crawford <[email protected]>
@crawfxrd crawfxrd requested review from jackpot51 and a team July 21, 2023 19:50
@crawfxrd
Copy link
Member Author

crawfxrd commented Jul 21, 2023

Presumably this is the cause of #320 (comment).

Enabling XLP_OUT probably has to be moved from board_init() to power_on().

@XV-02
Copy link

XV-02 commented Jul 21, 2023

Does this need to be tested in tandem with #320 (comment) ?

In isolation, I'm seeing the power light flash amber for ~3 seconds or less when I hold the power-button to shut the machine off. I can't trigger the power light to flash by short-stroking the power button.

All of these are resolutions of the issues I was seeing in #320 (comment) . I'd need to check other systems before approving however.

Is there any specific expected behaviour I should be looking for while I do that? (Its a very limited change, but I figure best to ask)

@crawfxrd
Copy link
Member Author

Does this need to be tested in tandem with #320 (comment) ?

No, I split this out to determine if this is the cause the incorrect LED behavior.

In isolation, I'm seeing the power light flash amber for ~3 seconds or less when I hold the power-button to shut the machine off.

  • Do you hold it after power off, or release it when it powers off?
  • Does that happen on release firmware?

Sounds like something is being left on. Probably bad.

Is there any specific expected behaviour I should be looking for while I do that?

One test is:

  • Power off unit
  • Remove AC power
  • Apply AC power
  • Ensure power LED does not turn on or blink

I will look into this more next week.

@crawfxrd crawfxrd marked this pull request as draft July 21, 2023 22:42
@XV-02
Copy link

XV-02 commented Jul 21, 2023

* Do you hold it after power off, or release it when it powers off?

* Does that happen on release firmware?

So, I hold the button until the system powered down, and immediately release the button. It blinks there-after.

It does not happen on the currently released firmware.

@XV-02
Copy link

XV-02 commented Jul 24, 2023

One test is:

* Power off unit

* Remove AC power

* Apply AC power

* Ensure power LED does not turn on or blink

Doing this, I notice that, upon returning the AC power, the power light goes from off to solid amber, though this is also true on our current firmware release.

A quick table of status light behaviour is steady-state situations. The results are the same for both released firmware, and with this PR.

AC Battery State Power State ----> Power Status Light Battery Status Light
In Full On Solid Green Solid Green
In Full Suspend Flashing Green Solid Green
In Full Off Solid Amber Solid Green
In Partial On Solid Green Solid Amber
In Partial Suspend Flashing Green Solid Amber
In Partial Off Solid Amber Solid Amber
Out N/A On Solid Green Off
Out N/A Suspend Flashing Green Off
Out N/A Off Off Off

I would also note that the behaviour between released firmware and this PR also differs when powering a system off with shutdown now while on AC:

  • Released: Power light transitions from solid green to solid amber. Battery light remains as it was (amber for charging, green for full)
  • This PR: Power and Battery lights turn off and stay off until AC is unplugged and plugged back in.

@crawfxrd
Copy link
Member Author

XLP_OUT controls VDD3, which is used for standby power. darp8 schematics show it's asserted at AC_IN. So it's not correct to set it after transitioning to S5. It has to be set when in S5 and AC is unplugged.

@crawfxrd crawfxrd closed this Jul 24, 2023
@crawfxrd crawfxrd deleted the xlp-out branch July 24, 2023 18:55
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