platform_ServoPowerStateController tests 'power_state:reset' incorrectly.
Reported by
jrbarnette@chromium.org,
Apr 17 2018
|
||||||
Issue descriptionA (not so) recent change to platform_ServoPowerStateController now allows two distinct, incompatible behaviors of 'power_state:reset': The control may leave the DUT powered on or off, based on FAFT-controlled settings. That's incorrect: The required behavior of 'power_state:reset' is that the DUT is reset and then turned on, regardless of how the underlying hardware implements 'cold_reset'. Existing repair code in Autotest relies on this. The test needs to be reverted back to its prior state. Any board/model that provides the broken 'power_state:reset' behavior needs to be updated to conform to the requirement. In practice that means that hdctools must change to turn the DUT on if it's left off after asserting cold reset. I believe (but I haven't confirmed) that fizz is the only affected hardware. The work to fix this problem should be rolled in with the fix for bug 831907 , since it's basically the same code.
,
Apr 18 2018
I have seen the similar error from monroe testing as well. http://haoweiw-moblab1.cros.corp.google.com/results/32-moblab/192.168.231.101/
,
Apr 18 2018
> I have seen the similar error from monroe testing as well. Oh, right. The change was targeted mostly at Chromeboxes. Since older Chromeboxes implemented power_state:reset correctly, the test change broke them.
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/a4674ca5825cbb9776ca1f3509e69245862188d5 commit a4674ca5825cbb9776ca1f3509e69245862188d5 Author: Nick Sanders <nsanders@chromium.org> Date: Thu Apr 19 05:31:05 2018 Revert "[autotest] Separate reset test cases for box, base and book" This reverts commit 0e7beadc08a767b1d110aef222b72461669c8b16. platform_ServoPowerStateController specifies power_state behavior expected by the autotest lab. This change allowed and occasionally required incompatible behavior. BUG= chromium:834063 TEST=test_that platform_ServoPowerStateController_USBUnplugged Change-Id: I2240b1bd27cf24a7d9015faa47be9b9a1fe87b42 Signed-off-by: Nick Sanders <nsanders@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1017882 Reviewed-by: Richard Barnette <jrbarnette@chromium.org> Reviewed-by: Shelley Chen <shchen@chromium.org> [modify] https://crrev.com/a4674ca5825cbb9776ca1f3509e69245862188d5/server/site_tests/platform_ServoPowerStateController/platform_ServoPowerStateController.py [modify] https://crrev.com/a4674ca5825cbb9776ca1f3509e69245862188d5/server/cros/faft/config/fizz.py
,
Apr 20 2018
Looks like this fixed breaks power_state:rec. power_state:rec didn't stay at recovery screen.. DUT turns back on after it is turned off. https://pantheon.corp.google.com/storage/browser/chromeos-moblab-englab-mtv/results/haoweiw-moblab1/54:ab:3a:e1:57:0b/ae2e2d0a443611e8910d54ab3ae1570b/3-moblab/192.168.231.100/
,
Apr 20 2018
> power_state:rec didn't stay at recovery screen.. DUT turns back on after it is turned off. That was probably a pre-existing bug in servod. It's not caused by fixing the test. At most, the test wasn't finding this because it failed earlier. The logs say that the failure was on scarlet. Does this happen on all scarlet DUTs, or just the one?
,
Apr 20 2018
I can try couple more, from physical vision, I can say scarlet unit could not boot from USB attached on Servo.
,
Apr 20 2018
,
Apr 20 2018
> Well, second DUT failed at the same way. OK. The failures aren't associated with this bug; work with nsanders@ on a new bug, or use the existing bug relating to scarlet, servo, and CCD.
,
Apr 20 2018
I marked this as fixed but still need couple more test run to verify.
,
Apr 20 2018
> I marked this as fixed but still need couple more test run to verify. Wait, no. This bug was about more than the test; 'power_state:reset' for fizz is broken, because the test change was introduced to allow fizz to do the wrong thing... We need a plan/bug to address the fizz problem.
,
Apr 24 2018
I see more platform are affected. monroe: https://pantheon.corp.google.com/storage/browser/chromeos-moblab-englab-mtv/results/haoweiw-moblab1/54:ab:3a:e1:57:0b/19494a9a32d811e8a6e8ea7d22e41126/36-moblab/192.168.231.101/ panther: https://pantheon.corp.google.com/storage/browser/chromeos-moblab-englab-mtv/results/haoweiw-moblab1/54:ab:3a:e1:57:0b/ae2e2d0a443611e8910d54ab3ae1570b/15-moblab/192.168.231.101/
,
Apr 25 2018
Posted CL for power_state:reset on fizz here: https://chromium-review.googlesource.com/c/chromiumos/third_party/hdctools/+/1028853
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/0b07b975215c41ef117fb17be5c6bb18b67002eb commit 0b07b975215c41ef117fb17be5c6bb18b67002eb Author: Shelley Chen <shchen@google.com> Date: Fri Apr 27 04:10:14 2018 cros_ec_hardrec_pbinitidle_power: force power on after power_state:reset Certain boards (Fizz and derivatives) will retain their state even after a power_state:reset call. For example, if the DUT is off prior to power_state:reset, it will remain off after the call. However, autotest does not expect this and expects the DUT to be online after the power_state:reset call. Creating a new cros_ec_hardrec_pbinitidle_power driver that inherits from cros_ec_hardrec_power that is only used for Fizz at the moment in order to minimize interference with other boards. BUG=b:78461317, chromium:834063 BRANCH=None TEST=test_that -b fizz IP f:.*platform_ServoPowerStateController/control.usb Change-Id: I2eccb803da55863766bdc0d9872247b3476c5b74 Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://chromium-review.googlesource.com/1028853 Commit-Ready: Shelley Chen <shchen@chromium.org> Tested-by: Shelley Chen <shchen@chromium.org> Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> Reviewed-by: Richard Barnette <jrbarnette@google.com> [modify] https://crrev.com/0b07b975215c41ef117fb17be5c6bb18b67002eb/servo/data/servo_fizz_overlay.xml [modify] https://crrev.com/0b07b975215c41ef117fb17be5c6bb18b67002eb/servo/drv/__init__.py [add] https://crrev.com/0b07b975215c41ef117fb17be5c6bb18b67002eb/servo/drv/cros_ec_hardrec_pbinitidle_power.py
,
Apr 27 2018
Should we schedule a labstation update to apply this change?
,
Apr 27 2018
> Should we schedule a labstation update to apply this change? I'll be filing a separate bug to get this change deployed to the lab. We need to wait for the change to get through the canary first, though.
,
Apr 27 2018
I've filed bug 837679 for the lab update. Let's declare "Fixed" for now. "Verified" should wait until we see fizz passing the test in the lab.
,
May 9 2018
From haoweiw@, sounds like it's still not fixed
,
May 9 2018
labstation is updated to desired build. $ ssh root@chromeos4-haoweiw-labstation1 "cat /etc/lsb-release | grep BUILDER" CHROMEOS_RELEASE_BUILDER_PATH=guado_labstation-release/R68-10623.0.0
,
May 9 2018
The logs look like the test was using the old, broken test version. That is, the DUT did the right thing, but the test thought it should do the wrong thing. Probably, you need to update your moblab to something with the recent test fix.
,
May 9 2018
What is the proper build with recent test fix to moblab?
,
May 10 2018
> What is the proper build with recent test fix to moblab?
It's the CL in comment #4:
http://crrev.com/c/1017882
Crosland info:
https://crosland.corp.google.com/cl?q=chromium:1017882
That says build R68-10598.0.0 or later.
,
Dec 5
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by jrbarnette@chromium.org
, Apr 17 2018