[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))
> There MASK parameter isn't needed for your use case. It's just making thing
Yes, I took the macro as it is. Removing MASK is a good idea.
But we maybe will end up with __SIMPLE_DEVICE__ where this macro will be unneeded anymore.


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

PS3, Line 18: #include <device/device.h>
            : #include <device/pci.h>
            : #include <console/console.h>
            : #include <arch/io.h>
            : #include <cpu/cpu.h>
            : #include <cpu/x86/cache.h>
            : #include <cpu/x86/smm.h>
            : #include <string.h>
            : #include <soc/iomap.h>
            : #include <soc/lpc.h>
            : #include <soc/smm.h>
> Are all these headers needed?
I will check the includes.

Regarding __SIMPLE_DEVICE__:
The access to the PCI config space of these weird devices is not needed while SMIs are handled but only in the relocation handler. I can use __SIMPLE_DEVICE__ just for fsp_broadwell_de/smmrelocate.c And in this case I will need to move all other PCI accesses to simple device, but that's OK for me.


-- 
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