Barnali Sarkar has posted comments on this change. ( https://review.coreboot.org/18557 )
Change subject: soc/intel/common/block: [WIP]Add Intel common FAST_SPI code ......................................................................
Patch Set 18:
(4 comments)
https://review.coreboot.org/#/c/18557/17/src/soc/intel/common/block/fast_spi... File src/soc/intel/common/block/fast_spi/fast_spi.c:
Line 22:
Why is the below function exposed?
Currently, since I have only pushed the SPI library upto bootblock, I need to make this function as GLOBAL since there are other places (like flash_controller.c, flash_ctrlr.c files), which calls this function to get the FAST_SPI BAR. So, I have made it non-static to avoid compilation issues as of now. In future, when the complete library will be formed and when we wont require this function from user side, we can make it Local to this file. I will put a /*TODO*/ comment here for now, to ensure that this gets addressed in future.
https://review.coreboot.org/#/c/18557/18/src/soc/intel/common/block/fast_spi... File src/soc/intel/common/block/fast_spi/fast_spi.c:
Line 49:
Why is there a new line here and below but there isn't one in the bios inte
will fix this.
PS18, Line 157: device_t dev
This feedback still isn't addressed.
will fix in the next patch-set.
https://review.coreboot.org/#/c/18557/18/src/soc/intel/common/block/include/... File src/soc/intel/common/block/include/intelblocks/fast_spi_def.h:
Line 17: #define SOC_INTEL_COMMON_BLOCK_FAST_SPI_DEF_H
Why does this file need to be in include/intelblocks/fast_spi_def.h?
I have included it here since, in flash_controller.c file, we have lot of these MACRO definitions used as of now (which will be part of this library after we cover romstage part). But now, we need to include this header file from flash_controller.c to get it built. So, if we keep it local to the "fast_spi" directory, then in flash_controller.c file, we have to include like this - #include <../common/block/fast_spi/fast_spi_def.h> which doesn't look good, I thought.
I will make a /*TODO*/ comment in this file, to ensure to change this to block/fast_spi/ directory after the complete library is formed.