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

hx20: restore RW firmware access #16

Open
wants to merge 4 commits into
base: hx20-hx30
Choose a base branch
from

Conversation

DHowett
Copy link
Contributor

@DHowett DHowett commented Jun 27, 2022

This pull request comprises three bug fixes.

1. lfw: restore RW firmware access by using the right pin mux

The alternate function table for LFW on the hx20 contained a few
incorrect values that resulted in LFW being unable to read from flash,
which disabled the use of the RW (and in some cases the RO) firmware
image.

  1. SHD_IO0 is function 1 on the MEC152x, not function 2.
  2. SHD_IO2 and SHD_IO3 were not included in the alternate function table.

The alternate function table from hx20/gpio.inc is correct, but it is
missing the SPI alternate for SHD_GLK/GPIO_0056.

In the future, we may want to move to model where hx20/gpio.inc includes
hx20/lfw/gpio.inc and common pin configurations are shared.

2. Remember whether we cut power to ourselves, and re-check during boot

On the hx20 (and presumably hx30,) a design issue prevents us from
hibernating the EC properly. Therefore, every time we bring the machine
down we cut power instead of hibernating. That results in the next boot
being a complete reset.

There is code in lfw that checks whether the current boot is due to a
watchdog reset or a power-on reset and if it is, clears the image type
back to EC_IMAGE_UNKNOWN. Due to that design issue, the hx20 EC is
always in POR/VTR or WDT on startup.

By storing whether the last shutdown was graceful/intended and checking
it before resetting the image type, we can work around this issue.

3. Make sure we fire SHUTDOWN_COMPLETE hooks when the AP goes down

This ensures that the hook in common/system.c that triggers an
"at-shutdown" reboot from RO to RW fires.

The x86 power sequencer in $/power fires this hook, but we don't.

Without this, ectool reboot_ec RW at-shutdown doesn't work.

4. mchp/lfw: make sure we keep the EC on for long operations

Over the past six months with this patch set, I've observed one
troubling behavior: the laptop would not always boot when it was powered
completely off (with the EC off as well) and I clicked the power button.

It turns out that the power buttons control VCI_IN[01] and that the
VBAT-powered control interface is configured to drive GPIO250 as VCI_OUT
based on the status of those inputs. Now, GPIO250 is also known as EC_ON
on hx20/30: it controls whether 3VL_EC is on.

The MEC1521 manual recommends the following example of using VCI_OUT on
a "mobile platform" (abbreviated):

  1. A coin cell battery is installed, powering VBAT
  2. The power button on VCI_IN0# is pushed causing VCI_OUT to be
    asserted, powering the VTR rail
  3. The EC reconfigures VCI so that firmware controls the VCI_OUT pin.

Now, the EC is doing this over in vci_task (hx20/hx30), which is long
after LFW has started and jumped to main firmware. The problem is,
restoring access to RO and RW images in the prior three commits causes
us to try to read from flash on startup.

Reading from flash is slightly slower, so we extend the period of time
between step 2 and 3 where 3VL_EC is only driven by VCI_IN0#. When the
user releases the power button during that window, 3VL_EC is cut and the
EC shuts down.

This patch fixes the issue by asserting firmware control of VCI_OUT as
early as is safe in lfw.

@DHowett

This comment was marked as resolved.

@DHowett

This comment was marked as outdated.

@DHowett

This comment was marked as resolved.

@DHowett DHowett changed the title hx20/lfw: restore RW firmware access by using the right pin mux hx20: restore RW firmware access Jul 30, 2022
@DHowett
Copy link
Contributor Author

DHowett commented Jul 30, 2022

I've updated this patch set with my tested and working proposal. It now properly (1) reboots into RW, (2) boots into RW from cold-off, and (3) can reboot into RO/RW at-shutdown

@DHowett DHowett marked this pull request as draft August 1, 2022 00:55
@DHowett DHowett marked this pull request as ready for review January 16, 2023 21:15
@DHowett

This comment was marked as outdated.

@DHowett DHowett marked this pull request as draft January 17, 2023 01:36
@DHowett DHowett changed the base branch from hx20 to hx20-hx30 January 23, 2023 00:41
@github-actions
Copy link

github-actions bot commented Jan 23, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@DHowett

This comment was marked as outdated.

The alternate function table for LFW on the hx20 contained a few
incorrect values that resulted in LFW being unable to read from flash,
which disabled the use of the RW (and in some cases the RO) firmware
image.

1. SHD_IO0 is function 1 on the MEC152x, not function 2.
2. SHD_IO2 and SHD_IO3 were not included in the alternate function table.

The alternate function table from hx20/gpio.inc is correct, but it is
missing the SPI alternate for SHD_GLK/GPIO_0056.

In the future, we may want to move to model where hx20/gpio.inc includes
hx20/lfw/gpio.inc and common pin configurations are shared.
On the hx20 (and presumably hx30,) a design issue prevents us from
hibernating the EC properly. Therefore, every time we bring the machine
down we cut power instead of hibernating. That results in the next boot
being a complete reset.

There is code in lfw that checks whether the current boot is due to a
watchdog reset or a power-on reset and if it is, clears the image type
back to EC_IMAGE_UNKNOWN. Due to that design issue, the hx20 EC is
*always* in POR/VTR or WDT on startup.

By storing whether the last shutdown was graceful/intended and checking
it before resetting the image type, we can work around this issue.
This ensures that the hook in common/system.c that triggers an
"at-shutdown" reboot from RO to RW fires.

Without this, `ectool reboot_ec RW at-shutdown` doesn't work.
@DHowett
Copy link
Contributor Author

DHowett commented Jan 23, 2023

Nailed it. Here's the explanation -

mchp/lfw: make sure we keep the EC on for long operations

Over the past six months with this patch set, I've observed one
troubling behavior: the laptop would not always boot when it was powered
completely off (with the EC off as well) and I clicked the power button.

It turns out that the power buttons control VCI_IN[01] and that the
VBAT-powered control interface is configured to drive GPIO250 as VCI_OUT
based on the status of those inputs. Now, GPIO250 is also known as EC_ON
on hx20/30: it controls whether 3VL_EC is on.

The MEC1521 manual recommends the following example of using VCI_OUT on
a "mobile platform" (abbreviated):

  1. A coin cell battery is installed, powering VBAT
  2. The power button on VCI_IN0# is pushed causing VCI_OUT to be
    asserted, powering the VTR rail
  3. The EC reconfigures VCI so that firmware controls the VCI_OUT pin.

Now, the EC is doing this over in vci_task (hx20/hx30), which is long
after LFW has started and jumped to main firmware. The problem is,
restoring access to RO and RW images in the prior three commits causes
us to try to read from flash on startup.

Reading from flash is slightly slower, so we extend the period of time
between step 2 and 3 where 3VL_EC is only driven by VCI_IN0#. When the
user releases the power button during that window, 3VL_EC is cut and the
EC shuts down.

This patch fixes the issue by asserting firmware control of VCI_OUT as
early as is safe in lfw.

@DHowett DHowett marked this pull request as ready for review January 23, 2023 15:59
* power button. This ensures that we can stay on long enough to read
* from SPI flash.
*/
gpio_reset(GPIO_EC_ON);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might seem counterintuitive, but gpio_reset is documented as turning off alternative states for the GPIO and asserting the flags set in gpio.inc; this results in it being re-muxed as GPIO and pulled high.

Over the past six months with this patch set, I've observed one
troubling behavior: the laptop would not always boot when it was powered
completely off (with the EC off as well) and I clicked the power button.

It turns out that the power buttons control VCI_IN[01] and that the
VBAT-powered control interface is configured to drive GPIO250 as VCI_OUT
based on the status of those inputs. Now, GPIO250 is also known as EC_ON
on hx20/30: it controls whether 3VL_EC is on.

The MEC1521 manual recommends the following example of using VCI_OUT on
a "mobile platform" (abbreviated):
1. A coin cell battery is installed, powering VBAT
2. The power button on VCI_IN0# is pushed causing VCI_OUT to be
   asserted, powering the VTR rail
3. The EC reconfigures VCI so that firmware controls the VCI_OUT pin.

Now, the EC is doing this over in vci_task (hx20/hx30), which is long
after LFW has started and jumped to main firmware. The problem is,
restoring access to RO and RW images in the prior three commits causes
us to try to read from flash on startup.

Reading from flash is slightly slower, so we extend the period of time
between step 2 and 3 where 3VL_EC is only driven by VCI_IN0#. When the
user releases the power button during that window, 3VL_EC is cut and the
EC shuts down.

This patch fixes the issue by asserting firmware control of VCI_OUT as
early as is safe in lfw.
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