Project

General

Profile

Actions

Bug #171

open

Thinkpad X220: usb_always_on broken by 9554b26f9fd608

Added by Nathaniel Roach over 5 years ago. Updated about 5 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Start date:
08/07/2018
Due date:
% Done:

0%

Estimated time:
Affected versions:
Needs backport to:
Affected hardware:
Affected OS:

Description

Reverting the commit 9554b26f9fd608 (vboot: Fix linking error with USE_OPTION_TABLE enabled) causes the usb_always_on setting to not work - the port remains un-powered while in suspend or off.

Reverting the commit builds and the port resumes functioning per the setting. Changing the setting via nvramcui sticks, but does not change the behaviour.

From what I can tell, it does not have any affect on the other nvram settings (Intel GPU reports correct amount of RAM, WWAN is turned on, no power beeps).

Attached is a diff from the /sys/firmware/log (same as cbmem -c) from when it was working to not working, the only differences in the image is that the working image has a custom floppyimg and a SLIC table.

Also attached is the ectool -d outputs.

I've attempted a small amount of debugging, by adding printks to the failure checks added in the patch, and as far as I can tell none of them are firing.


Files

firmware.diff (15.3 KB) firmware.diff diff broken.log working.log Nathaniel Roach, 08/07/2018 04:41 AM
ectool (858 Bytes) ectool broken ectool -d output Nathaniel Roach, 08/07/2018 04:42 AM
ectool (858 Bytes) ectool working ectool -d output Nathaniel Roach, 08/07/2018 04:43 AM
ectool-after.txt (858 Bytes) ectool-after.txt Anton Kindestam, 01/27/2019 09:42 PM
nvram-after.txt (373 Bytes) nvram-after.txt Anton Kindestam, 01/27/2019 09:42 PM
ectool-before.txt (858 Bytes) ectool-before.txt Anton Kindestam, 01/27/2019 09:42 PM
nvram-before.txt (366 Bytes) nvram-before.txt Anton Kindestam, 01/27/2019 09:42 PM
Actions #1

Updated by Nathaniel Roach over 5 years ago

*More correctly, unless you revert the commit, the port does not work

Actions #2

Updated by Anton Kindestam about 5 years ago

I can report also having this issue.
Reverting 9554b26f9fd608 fixed it for me, and is my temporary solution.

It seems like commit 4f4322dd68575402e099e2dfda057687388f064e (which actually mentions this issue) was not enough to resolve this issue for me.

Actions #3

Updated by Patrick Rudolph about 5 years ago

I tested it on master and don't find an issue with get_option() on an Ivy bridge system. Can you investigate to see what the problem is?

Actions #4

Updated by Nathaniel Roach about 5 years ago

Anton Kindestam wrote:

I can report also having this issue.
Reverting 9554b26f9fd608 fixed it for me, and is my temporary solution.

It seems like commit 4f4322dd68575402e099e2dfda057687388f064e (which actually mentions this issue) was not enough to resolve this issue for me.

Hi Anton,

Can you give me some more detail? I wrote the https://review.coreboot.org/c/coreboot/+/29565 patch originally. Can you

  • Run ectool -d and save it's output to a file
  • Set the option through nvramcui or nvramtool
  • Reboot
  • Confirm that the setting applied
  • Run ectool -d again, saving to a different file

And then attach the files here?

Patrick Rudolph wrote:

I tested it on master and don't find an issue with get_option() on an Ivy bridge system. Can you investigate to see what the problem is?

IIRC (because it was a little while ago when I initially debugged the patches) it's due to the changes in 9554... not allowing get_option() to work in SMM, which is where the original code sets the EC flag. I think it is mentioned in a comment in 9554, but I might be confused.

Updated by Anton Kindestam about 5 years ago

Sorry for the late reply.

I have now tested it on coreboot-4.9-514-gdacf083c2f. It seems it only works when using the yellow USB connector, but not any of the other two. I had it working on those as well earlier (by reverting 9554b26f9fd608), unless I'm severely misremembering. Alas, reverting that doesn't seem to do the trick any longer, and I have since rebased...

Files attached. Before is before enabling usb_always_on, after is after enabling it.

Actions #6

Updated by Nathaniel Roach about 5 years ago

Anton Kindestam wrote:

Sorry for the late reply.

I have now tested it on coreboot-4.9-514-gdacf083c2f. It seems it only works when using the yellow USB connector, but not any of the other two. I had it working on those as well earlier (by reverting 9554b26f9fd608), unless I'm severely misremembering. Alas, reverting that doesn't seem to do the trick any longer, and I have since rebased...

Files attached. Before is before enabling usb_always_on, after is after enabling it.

I'm not sure how you got it to work with the other ports initially, as the schematics show a specialised USB charge controller (U20 in the bottom left) only on that yellow port: https://i.imgur.com/XfsHQpC.png
The two lines entering it in the bottom left appear to be what we're controlling in the EC, and as the charge controller doesn't hook up to the other ports, there must have been some other reason they were powered.

Those before/afters you posted don't appear to have the correct EC flag set - is that with my patch reverted or just the normal tree un-touched?

Actions #7

Updated by Anton Kindestam about 5 years ago

It is with the normal tree un-touched. Specifically at commit dacf083c2f

I think I will have to do some testing with an old coreboot build which I believe worked and with stock firmware, to see if I can have the black ports charge while suspended.

Actions

Also available in: Atom PDF