[coreboot-gerrit] Change in coreboot[master]: drivers/spi: Add flash probe callbacks to spi_ctrlr structure

[Julius Werner (Code Review)]

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

Change subject: drivers/spi: Add flash probe callbacks to spi_ctrlr structure
......................................................................


Patch Set 8: Code-Review+1

(1 comment)

By the way, another option would be to export __spi_flash_probe() to be callable from other files (maybe call it spi_flash_generic_probe()), and then the controller-specific probe function can just call it first before doing its own thing if it wants to. Might be easier than having the whole "call this first, but only if that callback says so, otherwise try the other thing" logic in the generic code when only a single implementation needs it.

So in spi_flash.c you'd just have:

 if (spi.ctrlr->probe)
   ret = spi.ctrlr.probe(&spi, flash);
 else
   ret = spi_flash_generic_probe(&spi, flash);

and for the Intel southbridge probe you'd have a little wrapper:

 int ret = -1;
 if (!spi_is_multichip)
   ret = spi_flash_generic_probe();
 if (ret)
   ret = actual_intel_specific_probe();
 return ret;

https://review.coreboot.org/#/c/19708/8/src/soc/mediatek/mt8173/flash_controller.c
File src/soc/mediatek/mt8173/flash_controller.c:

Line 248: const struct spi_flash_ctrlr spi_flash_ctrlr = {
nit: isn't it easier to put this in spi.c so you don't need to declare an extern variable in the middle of a C file?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35f3bd8ddc5e71515df3ef0c1c4b1a68ee56bf4b
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: 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