[coreboot-gerrit] Change in coreboot[master]: soc/intel/skylake: Add support for GSPI controller

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/19099 )

Change subject: soc/intel/skylake: Add support for GSPI controller
......................................................................


Patch Set 1:

(3 comments)

https://review.coreboot.org/#/c/19099/1/src/soc/intel/skylake/Kconfig
File src/soc/intel/skylake/Kconfig:

PS1, Line 113: I2C
> I see that we named this i2c. I think we should probably remove this in the
Yes, I will change this.


https://review.coreboot.org/#/c/19099/1/src/soc/intel/skylake/gspi.c
File src/soc/intel/skylake/gspi.c:

PS1, Line 25: __SIMPLE_DEVICE__
> Do we really need 2 sets of functions?
I will revisit this. I had some assumptions about the use f devfn and functions available with and without __SIMPLE_DEVICE__. I will take a look again.


Line 45: 	res = find_resource(dev, PCI_BASE_ADDRESS_0);
> This code is assuming the resources are added prior to using this function.
That is true. Okay, I will add the check that if base address is not set, then set it here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29b1d4d5a6ee4093f2596065ac375c06f17d33ac
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes