[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 6:

(4 comments)

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

Line 56: void gspi_early_bar_init(void)
> If this was changed to take the common gspi config structure array then all
Yes, that's a great idea. That way we can keep the code to minimal in SoCs.


https://review.coreboot.org/#/c/19099/6/src/soc/intel/skylake/chip.h
File src/soc/intel/skylake/chip.h:

Line 53: struct skylake_gspi_config {
> Should this type be hoisted into common/gspi? Then the glue code can move t
Done


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

Line 123: }
> I can't help but think we could move all of this to the common gspi code as
Yes, that's right.


https://review.coreboot.org/#/c/19099/6/src/soc/intel/skylake/spi.c
File src/soc/intel/skylake/spi.c:

PS6, Line 46: 2
> Should this 2 be 1 + whatever the macro is which defines the number of buse
It would actually be (1 + (MAX - 1))


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