[coreboot-gerrit] Change in coreboot[master]: arch: Unify basic cache clearing API

Julius Werner (Code Review) gerrit at coreboot.org
Sat May 20 01:05:51 CEST 2017


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

Change subject: arch: Unify basic cache clearing API
......................................................................


Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/19788/1/src/arch/mips/include/arch/cache.h
File src/arch/mips/include/arch/cache.h:

PS1, Line 44: /* TODO: Global cache API. Implement properly once we finally have a MIPS board
            :    again where we can figure out what exactly these should be doing. */
> I thought the concise multi-line comments should only be used inside functi
It's my understanding that you should in any situation chose the comment style that fits best with the existing code and gives the comment the vertical weight that is appropriate for its content (roughly guided by length). There is no hard "in/out of functions" rule.


https://review.coreboot.org/#/c/19788/1/src/arch/x86/include/arch/cache.h
File src/arch/x86/include/arch/cache.h:

Line 59: 		wbinvd();
> Just call dcache_clean_all()?
Uhh... sure? Doesn't really make a difference?

I guess I can make dcache_clean_all() call dcache_clean_invalidate_all() instead, that would make a little more sense (because WBINVD both cleans and invalidates).


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

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



More information about the coreboot-gerrit mailing list