[coreboot-gerrit] Change in coreboot[master]: fsp_broadwell_de: Add SMM code

[Werner Zeh (Code Review)]

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/19145 )

Change subject: fsp_broadwell_de: Add SMM code
......................................................................


Patch Set 3:

(2 comments)

https://review.coreboot.org/#/c/19145/3/src/soc/intel/fsp_broadwell_de/include/soc/pci_devs.h
File src/soc/intel/fsp_broadwell_de/include/soc/pci_devs.h:

Line 127: 				  ((WHERE) & 0xFFF)) & ~MASK))
> Why do you need to copy this macro here?
I only found this macro in src/arch/x86/pci_ops_mmconf.c. It is not exported via header file and is only used in this file. Is there a decent way to use it in this file as well?


https://review.coreboot.org/#/c/19145/3/src/soc/intel/fsp_broadwell_de/smmrelocate.c
File src/soc/intel/fsp_broadwell_de/smmrelocate.c:

Line 167: 						SMM_FEATURE_CONTROL, 3);
> Why not create the device dynamically early in ramstage? Or put the device 
There is only this two placec where access to only this register is needed...and that only in the case where SMM is selected. I thought it is not worth it creating the device.
Is there a reason you do not like this access?


-- 
To view, visit https://review.coreboot.org/19145
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I461a14d411aedefdb0cb54ae43b91103a80a4f6a
Gerrit-PatchSet: 3
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Werner Zeh <werner.zeh at siemens.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer at siemens.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh at siemens.com>
Gerrit-Reviewer: York Yang <york.yang at intel.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes