Attention is currently required from: Sean Rhodes.
Paul Menzel has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83879?usp=email )
Change subject: ec/starlabs/merlin: Add Intel Virtual Button driver ......................................................................
Patch Set 3:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83879/comment/984f8baa_d02a7cfd?usp... : PS3, Line 15: Ubuntu Which version?
https://review.coreboot.org/c/coreboot/+/83879/comment/7c552914_0bdbf459?usp... : PS3, Line 16: show show*n*
https://review.coreboot.org/c/coreboot/+/83879/comment/bfa0c2e1_3339c64a?usp... : PS3, Line 15: Tested on `starlite_adl` with Ubuntu, by checking the : virtual keyboard is show when the tablet is undocked : and hidden when docked. Can it be tested by looking at some logs?
https://review.coreboot.org/c/coreboot/+/83879/comment/ab2f2b4b_fc6c5aff?usp... : PS3, Line 9: Add Intel Virtual Button driver which is used to report : to the OS whether a tablet is docked or undocked. : : This is currently only used on `mb/starlite_adl` so the : GPIO is hardcoded to GPP_F15 for now. : : Tested on `starlite_adl` with Ubuntu, by checking the : virtual keyboard is show when the tablet is undocked : and hidden when docked. Please try to use 72 characters per line.
File src/ec/starlabs/merlin/acpi/battery.asl:
PS3: This is from a different commit, isn’t it?
File src/ec/starlabs/merlin/acpi/dock.asl:
PS3: There is already:
1. `src/mainboard/purism/librem_jsl/acpi/vbtn.asl` 2. `src/ec/google/wilco/acpi/vbtn.asl`
Should it be at least named like this? Any possibilities to generalize the driver?
File src/ec/starlabs/merlin/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/83879/comment/2ea24e94_aef2f756?usp... : PS3, Line 53: #if CONFIG(SOC_INTEL_COMMON) : \LIDS = 0x03 : #endif Please comment in the commit message why this is done, or add a comment?
File src/ec/starlabs/merlin/acpi/hid.asl:
https://review.coreboot.org/c/coreboot/+/83879/comment/d14cea88_9f8bc1b8?usp... : PS3, Line 352: Case (0x08) : { : Return (_SB.PCI0.LPCB.EC.VBTN.VGBS()) : } Please mention this in the commit message?