Bug #171

Thinkpad X220: usb_always_on broken by 9554b26f9fd608

Added by Nathaniel Roach 7 months ago. Updated 24 days ago.

Status:NewStart date:08/07/2018
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

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.

firmware.diff Magnifier - diff broken.log working.log (15.3 KB) Nathaniel Roach, 08/07/2018 04:41 AM

ectool - broken ectool -d output (858 Bytes) Nathaniel Roach, 08/07/2018 04:42 AM

ectool - working ectool -d output (858 Bytes) Nathaniel Roach, 08/07/2018 04:43 AM

ectool-after.txt Magnifier (858 Bytes) Anton Kindestam, 01/27/2019 09:42 PM

nvram-after.txt Magnifier (373 Bytes) Anton Kindestam, 01/27/2019 09:42 PM

ectool-before.txt Magnifier (858 Bytes) Anton Kindestam, 01/27/2019 09:42 PM

nvram-before.txt Magnifier (366 Bytes) Anton Kindestam, 01/27/2019 09:42 PM

History

#1 Updated by Nathaniel Roach 7 months ago

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

#2 Updated by Anton Kindestam about 1 month 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.

#3 Updated by Patrick Rudolph about 1 month 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?

#4 Updated by Nathaniel Roach about 1 month 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.

#5 Updated by Anton Kindestam 25 days 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.

#6 Updated by Nathaniel Roach 25 days 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?

#7 Updated by Anton Kindestam 24 days 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.

Also available in: Atom PDF