Attention is currently required from: Raul Rangel, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52801 )
Change subject: mb/google/guybrush: Fix S0i3/S3 GPIO configuration ......................................................................
Patch Set 3:
(2 comments)
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52801/comment/6f3c18f9_a2641754 PS1, Line 20: PAD_SCI
I think we need to set this to PAD_GPI instead and make it so the kernel driver can set the wake bits. https://lore.kernel.org/patchwork/patch/1420135/
Yes, that can work. I had set the wake bits in coreboot because those were set for a specific sleep type. But, thinking about it some more, I think it shouldn't matter if S3|S0i3|S4 are set at the same time (at least from a Chrome OS perspective). We support the same wake source from S3 and S0i3. S4 is not used and so the setting doesn't really matter.
Since the OS doesn't have a driver loaded for this, the IRQ never gets enabled, so the wake_status never gets cleared. This results in not being able to stay in S0i3.
That should be handled correctly once you have the appropriate kernel driver loaded for it i.e. gpio-keys driver.
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52801/comment/96290514_0445df6f PS3, Line 40: SOC_SAR_INT_L Is this really a wake source? Probably just a PAD_GPI is okay here?