Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29917 )
Change subject: src: Fix - Remove unused variables ......................................................................
Patch Set 23:
(6 comments)
https://review.coreboot.org/#/c/29917/23/src/cpu/amd/family_10h-family_15h/i... File src/cpu/amd/family_10h-family_15h/init_cpus.c:
https://review.coreboot.org/#/c/29917/23/src/cpu/amd/family_10h-family_15h/i... PS23, Line 1070: ) I think it would be preferable to add {} for readability.
https://review.coreboot.org/#/c/29917/23/src/northbridge/intel/haswell/ramin... File src/northbridge/intel/haswell/raminit.c:
https://review.coreboot.org/#/c/29917/23/src/northbridge/intel/haswell/ramin... PS23, Line 203: are we sure that this read is not necessary?
https://review.coreboot.org/#/c/29917/23/src/northbridge/intel/pineview/earl... File src/northbridge/intel/pineview/early_init.c:
https://review.coreboot.org/#/c/29917/23/src/northbridge/intel/pineview/earl... PS23, Line 125: reg32 = MCHBAR32(0x30); : reg32 = 0x21800; : MCHBAR32(0x30) = reg32; : DMIBAR32(0x2c) = 0x86000040; : pci_write_config32(PCI_DEV(0, 0x1e, 0), 0x18, 0x00020200); : pci_write_config32(PCI_DEV(0, 0x1e, 0), 0x18, 0x00000000); : : early_graphics_setup(); : : reg32 = MCHBAR32(0x40); : reg32 = 0x0; : MCHBAR32(0x40) = reg32; : reg32 = MCHBAR32(0x40); : reg32 = 0x8; : MCHBAR32(0x40) = reg32; This is unrelated to the description and doesn't make the code more readable
https://review.coreboot.org/#/c/29917/23/src/northbridge/intel/x4x/early_ini... File src/northbridge/intel/x4x/early_init.c:
https://review.coreboot.org/#/c/29917/23/src/northbridge/intel/x4x/early_ini... PS23, Line 243: This read might be necessary.
https://review.coreboot.org/#/c/29917/23/src/northbridge/intel/x4x/early_ini... PS23, Line 226: reg16 = RCBA16(0x1a8); : reg16 &= ~0x3; : RCBA16(0x1a8) = reg16; This assumes that the register reads the same value that was written. Did you verify that that is the case?
https://review.coreboot.org/#/c/29917/23/src/soc/intel/fsp_baytrail/romstage... File src/soc/intel/fsp_baytrail/romstage/romstage.c:
https://review.coreboot.org/#/c/29917/23/src/soc/intel/fsp_baytrail/romstage... PS23, Line 253: if (!cbmem_was_initted) : printk(BIOS_ERR, "cbmem init failed\n"); unrelated to description
-
Stefan Reinauer (Code Review)