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

[Furquan Shaikh (Code Review)]

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

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


Patch Set 8:

(6 comments)

https://review.coreboot.org/#/c/19098/8/src/soc/intel/common/block/gspi/gspi.c
File src/soc/intel/common/block/gspi/gspi.c:

PS8, Line 104: bar
> (bar) + (bus) * 4 * KiB
Done


Line 243: {
> Should the entire structure be memset() to 0 here?
In xfer case, I was actually setting the other params before calling this. But, since this function is called init, it might make sense to do that later and have a memset() to 0 here.


PS8, Line 266: assert
> I'm surprised the compiler isn't complaining about using this name since as
woops.. that is true. I will use a different name.


Line 344: 	return (ref_clk_mhz/gspi_clk_mhz - 1) & SSCR0_SCR_MASK;
> DIV_ROUNDUP? Otherwise you can result in a faster clock than you requested.
Done


Line 422: 	 * Set m and n to 1, so that this divider acts as a pass-through.
> It seems like the M/N clock divider gets you closer to the target reference
There is no recommendation within the guide, but M/N does give us more control. I will go with this m=n=1 for now and as a follow-up push a better way to calculate m/n values.


Line 623: 	return __gspi_xfer(p);
> Once a byte is written to the fifo the clocks start pumping per bit?
That's correct.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0eb91eba2c523be457fee8922c44fb500a9fa140
Gerrit-PatchSet: 8
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