Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30890 )
Change subject: autoport: Add support for Haswell-LynxPoint platform ......................................................................
Patch Set 13:
(11 comments)
Now use the full struct for most of LP GPIO because the old one breaks things.
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/haswell.go File util/autoport/haswell.go:
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/haswell.go@3... PS11, Line 39: FormatInt32(refclk / 128 / (inteltool.IGD[0xc8254] >> 16))
This results in a division by zero on desktop boards.
Done
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/haswell.go@1... PS11, Line 109: File: "drivers/intel/gma/acpi/default_brightness_levels.asl",
FIXME: This is not necessary if the board doesn't have backlight
Comment added.
https://review.coreboot.org/c/coreboot/+/30890/1/util/autoport/lynxpoint.go File util/autoport/lynxpoint.go:
https://review.coreboot.org/c/coreboot/+/30890/1/util/autoport/lynxpoint.go@... PS1, Line 2:
A lot of this code (especially the GPIO stuff) is the same as bd82x6x. […]
Done
https://review.coreboot.org/c/coreboot/+/30890/10/util/autoport/lynxpoint.go File util/autoport/lynxpoint.go:
https://review.coreboot.org/c/coreboot/+/30890/10/util/autoport/lynxpoint.go... PS10, Line 401: // the lid is open by default.
Please use consistent comments in generated code
Done
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/lynxpoint.go File util/autoport/lynxpoint.go:
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/lynxpoint.go... PS11, Line 188: DSDTDefine{ : Key: "BRIGHTNESS_UP", : Value: "\_SB.PCI0.GFX0.INCB", : }, : DSDTDefine{ : Key: "BRIGHTNESS_DOWN", : Value: "\_SB.PCI0.GFX0.DECB", : }, : DSDTDefine{ : Key: "ACPI_VIDEO_DEVICE", : Value: "\_SB.PCI0.GFX0", : })
I meant all three values.
Done
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/lynxpoint.go... PS11, Line 240: SATA Controller 1
SATA Controller (AHCI)
Done
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/lynxpoint.go... PS11, Line 242: SATA Controller 2
SATA Controller (Legacy)
Done
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/lynxpoint.go... PS11, Line 250: PCI-LPC bridge
Drop this part, it will result in "LPC bridge PCI-LPC bridge"
Done
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/lynxpoint.go... PS11, Line 426: : /* Disable USB ports in S3 by default */ : gnvs->s3u0 = 0; : gnvs->s3u1 = 0; : : /* Disable USB ports in S5 by default */ : gnvs->s5u0 = 0; : gnvs->s5u1 = 0;
Drop these
Done
https://review.coreboot.org/c/coreboot/+/30890/11/util/autoport/lynxpoint.go... PS11, Line 435: // the lid is open by default.
/* The lid is open by default. […]
Done
https://review.coreboot.org/c/coreboot/+/30890/12/util/autoport/lynxpoint.go File util/autoport/lynxpoint.go:
https://review.coreboot.org/c/coreboot/+/30890/12/util/autoport/lynxpoint.go... PS12, Line 128: switch g {
The indentation is rather odd here
Now I've used gofmt to reformat the code.