[coreboot-gerrit] Change in coreboot[master]: soc/intel/skylake: Cleanup code by using common FAST_SPI module

[Barnali Sarkar (Code Review)]

Barnali Sarkar has posted comments on this change. ( https://review.coreboot.org/19055 )

Change subject: soc/intel/skylake: Cleanup code by using common FAST_SPI module
......................................................................


Patch Set 1:

(10 comments)

https://review.coreboot.org/#/c/19055/1/src/soc/intel/skylake/bootblock/cpu.c
File src/soc/intel/skylake/bootblock/cpu.c:

PS1, Line 30: #include <intelblocks/fast_spi.h>
> alphabetic order?
ok, will fix


https://review.coreboot.org/#/c/19055/1/src/soc/intel/skylake/bootblock/pch.c
File src/soc/intel/skylake/bootblock/pch.c:

PS1, Line 57: PCH_DEV_SPI
> use device_t local there.
ok


https://review.coreboot.org/#/c/19055/1/src/soc/intel/skylake/finalize.c
File src/soc/intel/skylake/finalize.c:

PS1, Line 33: #include <intelblocks/fast_spi.h>
> order?
ok


PS1, Line 92: 	fast_spi_set_opcode_menu();
            : 	fast_spi_lock_bar();
> why removed comments as well?
Added the comment in the function definitions in fast_spi.c.
Ok, will add there here as well.


PS1, Line 160: 		fast_spi_set_bios_interface_lock_down();
> put comments
ok


https://review.coreboot.org/#/c/19055/1/src/soc/intel/skylake/flash_controller.c
File src/soc/intel/skylake/flash_controller.c:

PS1, Line 27: #include <intelblocks/fast_spi_def.h>
> alphabetic order
ok


https://review.coreboot.org/#/c/19055/1/src/soc/intel/skylake/include/soc/pch.h
File src/soc/intel/skylake/include/soc/pch.h:

PS1, Line 38: //u32 pch_read_soft_strap(int id);
> why commented code?
will remove it


https://review.coreboot.org/#/c/19055/1/src/soc/intel/skylake/pch.c
File src/soc/intel/skylake/pch.c:

PS1, Line 27: #include <intelblocks/fast_spi.h>
> alphabetic order
ok


PS1, Line 39: /*u32 pch_read_soft_strap(int id)
            : {
            : 	uint32_t fdoc;
            : 	void *spibar = get_fast_spi_bar();
            : 
            : 	fdoc = read32(spibar + SPIBAR_FDOC);
            : 	fdoc &= ~0x00007ffc;
            : 	write32(spibar + SPIBAR_FDOC, fdoc);
            : 
            : 	fdoc |= 0x00004000;
            : 	fdoc |= id * 4;
            : 	write32(spibar + SPIBAR_FDOC, fdoc);
            : 
            : 	return read32(spibar + SPIBAR_FDOD);
            : }*/
> commented code? why?
will remove, sorry


https://review.coreboot.org/#/c/19055/1/src/soc/intel/skylake/romstage/romstage.c
File src/soc/intel/skylake/romstage/romstage.c:

PS1, Line 41: #include <intelblocks/fast_spi.h>
> alphabetic order
ok


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4fc90504d322db70ed4ea644b1593cc0605b5fe8
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Barnali Sarkar <barnali.sarkar at intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra at intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan at intel.com>
Gerrit-Reviewer: Barnali Sarkar <barnali.sarkar at intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein at intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams at intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi at intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha at intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik at intel.com>
Gerrit-HasComments: Yes