[coreboot-gerrit] Change in coreboot[master]: soc/intel/cannonlake: Steal no memory for disabled IGD

[Christian Walter (Code Review)]

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39454 )

Change subject: soc/intel/cannonlake: Steal no memory for disabled IGD
......................................................................

Patch Set 13:

(3 comments)

https://review.coreboot.org/c/coreboot/+/39454/13/src/soc/intel/cannonlake/r... 
File src/soc/intel/cannonlake/romstage/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/39454/13/src/soc/intel/cannonlake/r... 
PS13, Line 42: pci_read_config16(SA_DEV_IGD, PCI_VENDOR_ID) == 0xffff))
> if dev and dev->enabled are true, can this still be false? And wouldn't we have to mark the device a […]

If dev, but there is no IGD (Read from PCI_VENDOR_ID == 0xffff) it should be disabled, otherwise it will crash.

https://review.coreboot.org/c/coreboot/+/39454/13/src/soc/intel/cannonlake/r... 
PS13, Line 42: dev
> not needed

Why? We should only do this if dev. If dev is not enabled anyways we do need to read this.

https://review.coreboot.org/c/coreboot/+/39454/13/src/soc/intel/cannonlake/r... 
PS13, Line 98: dev
> "dev" was declared const above...

Seems like a common rule to override this.. 
 e.g.
dev = pcidev_path_on_root(PCH_DEVFN_HDA);
dev = pcidev_path_on_root(SA_DEVFN_IPU);

so either we have to fix all of these or _ignore_ the const.