Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35018 )
Change subject: soc/amd/common/block: Create new SPI code ......................................................................
Patch Set 6:
(11 comments)
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... File src/soc/amd/common/block/spi/fch_spi_ctrl.c:
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 27: extern const struct spi_flash_ops fch_spi_flash_ops;
Gotcha. Will do.
Done
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 29: struct spi_data ctrl_spi_data;
Would need to create functions to return needed parameters. […]
Done
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 216: int do_fch_spi_flash_cmd(const void *dout, size_t bytes_out, void *din, size_t bytes_in)
Will do.
Done
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 238: int fch_spi_flash_cmd(u8 cmd, void *response, size_t len)
Will do.
It's also used on inline command enable_write (see fch_spi.h). Done anyway.
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 248: * The following table holds all device probe functions
On original code (drivers/spi/spi_flash.c) it was done this way. I have no problem either way.
Done, though size is not returned as const.
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 312:
understood... if I create a separate file for the table.
Done
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 379: save_flash_data
Will do.
Done
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 380: flash->ops = &fch_spi_flash_ops;
Will do.
Done
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 381: if (idp[0] == VENDOR_ID_SST) {
non_standard is only set here, no where else. […]
Done the move above.
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... File src/soc/amd/common/block/spi/fch_spi_flash.c:
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 230: const struct spi_flash_ops fch_spi_flash_ops = {
will do.
Done
https://review.coreboot.org/c/coreboot/+/35018/6/src/soc/amd/common/block/sp... PS6, Line 236:
will do.
Done