Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46352 )
Change subject: soc/intel/broadwell: Drop reg-script usage from bootblock PCH init ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/46352/2/src/soc/intel/broadwell/boo... File src/soc/intel/broadwell/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/46352/2/src/soc/intel/broadwell/boo... PS2, Line 59: /* Enable GPIO functionality. */ Want to comment the other lines too? (kidding)
https://review.coreboot.org/c/coreboot/+/46352/2/src/soc/intel/broadwell/boo... PS2, Line 63: pci_write_config16(PCH_DEV_LPC, LPC_IO_DEC, 0x0010); This doesn't look like BAR setup.
https://review.coreboot.org/c/coreboot/+/46352/2/src/soc/intel/broadwell/boo... PS2, Line 79: Spurious space
https://review.coreboot.org/c/coreboot/+/46352/2/src/soc/intel/broadwell/boo... PS2, Line 84: It's affected by posted write syndrome. That's neither helpful for people who understand posting writes nor for those who don't.
How about something like this:
/* Reading it back guarantees that the write is done before we use the configured base address below. */
https://review.coreboot.org/c/coreboot/+/46352/2/src/soc/intel/broadwell/boo... PS2, Line 85: RCBA32(HPTC); Why use a (void) cast above to make it clear that the value is discarded but not here?