Change in coreboot[master]: soc/intel/cannonlake: Make use of is_devfn_enabled() function

[Subrata Banik (Code Review)]

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55328 )

Change subject: soc/intel/cannonlake: Make use of is_devfn_enabled() function
......................................................................

Patch Set 2:

(6 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/55328/comment/14ce4ae5_c7c7ff9c 
PS1, Line 15: built
> nit: build

Ack

https://review.coreboot.org/c/coreboot/+/55328/comment/00a02e78_82eac723 
PS1, Line 17:
> nit: Drop blank line?

Ack

File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/55328/comment/dcb61ef8_6fa945fb 
PS1, Line 315: 			dev->enabled = 0;
> This also disabled the device in the devicetree

Ack

https://review.coreboot.org/c/coreboot/+/55328/comment/302c0cae_05f9af72 
PS1, Line 150: 	if (params->SataEnable) {
> Strictly speaking, this changes behavior when the device exists in the devicetree but is disabled: […]

If SataEnable UPD itself is disable then FSP doesn't honour other SATA related UPDs like mode etc. Hence, as you said, mostly its safe with this CL.

https://review.coreboot.org/c/coreboot/+/55328/comment/5c6e9780_614e342a 
PS1, Line 173: if (params->PchLanEnable) {
             : 		if (config->s0ix_enable && params->PchLanEnable) {
> Hmmm, do you notice anything weird here? A redundant check, maybe? 😉 […]

Ack

https://review.coreboot.org/c/coreboot/+/55328/comment/0ae834ae_2720cdb3 
PS1, Line 481: if (CONFIG(RUN_FSP_GOP) && is_devfn_enabled(SA_DEVFN_IGD))
             : 		params->PeiGraphicsPeimInit = 1;
             : 	else
             : 		params->PeiGraphicsPeimInit = 0;
> Or maybe not, it fits on JSL

Ack