Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33549 )
Change subject: sb/intel/bd82x6x/lpc: Setup default LPC decode ranges ......................................................................
Patch Set 4:
(6 comments)
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/asrock/b75pro... File src/mainboard/asrock/b75pro3-m/romstage.c:
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/asrock/b75pro... PS4, Line 29: LPT_LPC_EN
this decode is missing in the common code
Hmm missed that. Thx
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/google/link/r... File src/mainboard/google/link/romstage.c:
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/google/link/r... PS4, Line 39: GAMEL_LPC_EN
is this really used? if not, this can also be dropped
I can do that in a followup patch.
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/intel/dcp847s... File src/mainboard/intel/dcp847ske/early_southbridge.c:
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/intel/dcp847s... PS4, Line 41: pci_write_config32(PCI_DEV(0, 0x1f, 0), LPC_GEN1_DEC, 0x00fc0a01);
this line shouldn't be dropped
yes, this is done via the devicetree now.
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/intel/emerald... File src/mainboard/intel/emeraldlake2/romstage.c:
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/intel/emerald... PS4, Line 33: pci_devfn_t dev = PCH_LPC_DEV; : : /* Enable SuperIO + PS/2 Keyboard/Mouse */ : u16 lpc_config = CNF1_LPC_EN | CNF2_LPC_EN | KBC_LPC_EN; : pci_write_config16(dev, LPC_EN, lpc_config); : : /* Enable COM1 */ : if (sio1007_enable_uart_at(SIO_PORT)) { : pci_write_config16(dev, LPC_EN, : lpc_config | COMA_LPC_EN);
this can be dropped
sio1007_enable_uart_at(SIO_PORT) still needs to be called I think. I think the intention is to not set up COMA decoding if that failed to somehow avoid a slow boot?
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/roda/rv11/var... File src/mainboard/roda/rv11/variants/rw11/romstage.c:
https://review.coreboot.org/c/coreboot/+/33549/4/src/mainboard/roda/rv11/var... PS4, Line 35: COMB_LPC_EN
if we always enable com B, this could also be dropped
I don't mind doing that.
https://review.coreboot.org/c/coreboot/+/33549/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/early_pch.c:
https://review.coreboot.org/c/coreboot/+/33549/4/src/southbridge/intel/bd82x... PS4, Line 264: | KBC_LPC_EN | MC_LPC_EN;
| LPT_LPC_EN and | COMB_LPC_EN ?
fine by me