Change in coreboot[master]: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver
Werner Zeh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43166 )
Change subject: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver ......................................................................
mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver
The common fast SPI driver has a function to set up the SPI OPCODE menu. Use this function here instead of coding it again as it results in the very same register values being written.
TEST=Compare register values in both cases and make sure they match.
Change-Id: I98457a0b0652f746734ee4204e10acd09b6e5fda Signed-off-by: Werner Zeh werner.zeh@siemens.com --- M src/mainboard/siemens/mc_apl1/mainboard.c 1 file changed, 2 insertions(+), 54 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/43166/1
diff --git a/src/mainboard/siemens/mc_apl1/mainboard.c b/src/mainboard/siemens/mc_apl1/mainboard.c index 7c68090..bff97ea 100644 --- a/src/mainboard/siemens/mc_apl1/mainboard.c +++ b/src/mainboard/siemens/mc_apl1/mainboard.c @@ -8,6 +8,7 @@ #include <hwilib.h> #include <i210.h> #include <intelblocks/cpulib.h> +#include <intelblocks/fast_spi.h> #include <intelblocks/systemagent.h> #include <soc/pci_devs.h> #include <string.h> @@ -23,51 +24,6 @@ #define BIOS_MAILBOX_INTERFACE 0x7084 #define RUN_BUSY_STS (1 << 31)
-/* - * SPI Opcode Menu setup for SPIBAR lock down - * should support most common flash chips. - */ -#define SPI_OPMENU_0 0x01 /* WRSR: Write Status Register */ -#define SPI_OPTYPE_0 0x01 /* Write, no address */ - -#define SPI_OPMENU_1 0x02 /* PP: Page Program */ -#define SPI_OPTYPE_1 0x03 /* Write, address required */ - -#define SPI_OPMENU_2 0x03 /* READ: Read Data */ -#define SPI_OPTYPE_2 0x02 /* Read, address required */ - -#define SPI_OPMENU_3 0x05 /* RDSR: Read Status Register */ -#define SPI_OPTYPE_3 0x00 /* Read, no address */ - -#define SPI_OPMENU_4 0x20 /* SE20: Sector Erase 0x20 */ -#define SPI_OPTYPE_4 0x03 /* Write, address required */ - -#define SPI_OPMENU_5 0x9f /* RDID: Read ID */ -#define SPI_OPTYPE_5 0x00 /* Read, no address */ - -#define SPI_OPMENU_6 0xd8 /* BED8: Block Erase 0xd8 */ -#define SPI_OPTYPE_6 0x03 /* Write, address required */ - -#define SPI_OPMENU_7 0x0b /* FAST: Fast Read */ -#define SPI_OPTYPE_7 0x02 /* Read, address required */ - -#define SPI_OPMENU_UPPER ((SPI_OPMENU_7 << 24) | (SPI_OPMENU_6 << 16) | \ - (SPI_OPMENU_5 << 8) | SPI_OPMENU_4) -#define SPI_OPMENU_LOWER ((SPI_OPMENU_3 << 24) | (SPI_OPMENU_2 << 16) | \ - (SPI_OPMENU_1 << 8) | SPI_OPMENU_0) - -#define SPI_OPTYPE ((SPI_OPTYPE_7 << 14) | (SPI_OPTYPE_6 << 12) | \ - (SPI_OPTYPE_5 << 10) | (SPI_OPTYPE_4 << 8) | \ - (SPI_OPTYPE_3 << 6) | (SPI_OPTYPE_2 << 4) | \ - (SPI_OPTYPE_1 << 2) | (SPI_OPTYPE_0)) - -#define SPI_OPPREFIX ((0x50 << 8) | 0x06) /* EWSR and WREN */ - -#define SPIBAR_OFFSET 0x3800 -#define SPI_REG_PREOP_OPTYPE 0xa4 -#define SPI_REG_OPMENU_L 0xa8 -#define SPI_REG_OPMENU_H 0xac - #define SD_CAP_BYP 0x810 #define SD_CAP_BYP_EN 0x5A #define SD_CAP_BYP_REG1 0x814 @@ -233,7 +189,6 @@ { uint16_t cmd = 0; struct device *dev = NULL; - void *spi_base = NULL;
/* Do board specific things */ variant_mainboard_final(); @@ -246,14 +201,7 @@ pci_write_config16(dev, PCI_COMMAND, cmd); } /* Set up SPI OPCODE menu before the controller is locked. */ - dev = PCH_DEV_SPI; - spi_base = (void *)pci_read_config32(dev, PCI_BASE_ADDRESS_0); - if (!spi_base) - return; - write32((spi_base + SPI_REG_PREOP_OPTYPE), - ((SPI_OPTYPE << 16) | SPI_OPPREFIX)); - write32((spi_base + SPI_REG_OPMENU_L), SPI_OPMENU_LOWER); - write32((spi_base + SPI_REG_OPMENU_H), SPI_OPMENU_UPPER); + fast_spi_set_opcode_menu();
/* Set SD-Card speed to HS mode only. */ dev = pcidev_path_on_root(PCH_DEVFN_SDCARD);
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43166 )
Change subject: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver ......................................................................
Patch Set 1: Code-Review+2
I have tested the change on mc_apl5 mainboard and it works.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43166 )
Change subject: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43166/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/mainboard.c:
https://review.coreboot.org/c/coreboot/+/43166/1/src/mainboard/siemens/mc_ap... PS1, Line 204: fast_spi_set_opcode_menu(); This is not seem mainboard specific? Any reason it happens here? BTW who is locking things down?
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43166 )
Change subject: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43166/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/mainboard.c:
https://review.coreboot.org/c/coreboot/+/43166/1/src/mainboard/siemens/mc_ap... PS1, Line 204: fast_spi_set_opcode_menu();
This is not seem mainboard specific? Any reason it happens here? BTW who is locking things down?
Lockdown is handled in FSP. And though it is not mainboard specific there is no need to have it set up unless you want to use the SPI controller to flash you chip from OS using hardware accelerated access. As I cannot take this decision for every usage I enable it just for our mainboards.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43166 )
Change subject: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/43166/1/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/mainboard.c:
https://review.coreboot.org/c/coreboot/+/43166/1/src/mainboard/siemens/mc_ap... PS1, Line 204: fast_spi_set_opcode_menu();
Lockdown is handled in FSP. […]
Done
uwe.poeche@siemens.com has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43166 )
Change subject: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver ......................................................................
Patch Set 1: Code-Review+1
Werner Zeh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43166 )
Change subject: mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver ......................................................................
mb/siemens/mc_apl1: Use OPCODE menu set up of fast SPI driver
The common fast SPI driver has a function to set up the SPI OPCODE menu. Use this function here instead of coding it again as it results in the very same register values being written.
TEST=Compare register values in both cases and make sure they match.
Change-Id: I98457a0b0652f746734ee4204e10acd09b6e5fda Signed-off-by: Werner Zeh werner.zeh@siemens.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43166 Reviewed-by: Mario Scheithauer mario.scheithauer@siemens.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: uwe.poeche@siemens.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/siemens/mc_apl1/mainboard.c 1 file changed, 2 insertions(+), 54 deletions(-)
Approvals: build bot (Jenkins): Verified Mario Scheithauer: Looks good to me, approved Arthur Heymans: Looks good to me, but someone else must approve uwe.poeche@siemens.com: Looks good to me, but someone else must approve
diff --git a/src/mainboard/siemens/mc_apl1/mainboard.c b/src/mainboard/siemens/mc_apl1/mainboard.c index 7c68090..bff97ea 100644 --- a/src/mainboard/siemens/mc_apl1/mainboard.c +++ b/src/mainboard/siemens/mc_apl1/mainboard.c @@ -8,6 +8,7 @@ #include <hwilib.h> #include <i210.h> #include <intelblocks/cpulib.h> +#include <intelblocks/fast_spi.h> #include <intelblocks/systemagent.h> #include <soc/pci_devs.h> #include <string.h> @@ -23,51 +24,6 @@ #define BIOS_MAILBOX_INTERFACE 0x7084 #define RUN_BUSY_STS (1 << 31)
-/* - * SPI Opcode Menu setup for SPIBAR lock down - * should support most common flash chips. - */ -#define SPI_OPMENU_0 0x01 /* WRSR: Write Status Register */ -#define SPI_OPTYPE_0 0x01 /* Write, no address */ - -#define SPI_OPMENU_1 0x02 /* PP: Page Program */ -#define SPI_OPTYPE_1 0x03 /* Write, address required */ - -#define SPI_OPMENU_2 0x03 /* READ: Read Data */ -#define SPI_OPTYPE_2 0x02 /* Read, address required */ - -#define SPI_OPMENU_3 0x05 /* RDSR: Read Status Register */ -#define SPI_OPTYPE_3 0x00 /* Read, no address */ - -#define SPI_OPMENU_4 0x20 /* SE20: Sector Erase 0x20 */ -#define SPI_OPTYPE_4 0x03 /* Write, address required */ - -#define SPI_OPMENU_5 0x9f /* RDID: Read ID */ -#define SPI_OPTYPE_5 0x00 /* Read, no address */ - -#define SPI_OPMENU_6 0xd8 /* BED8: Block Erase 0xd8 */ -#define SPI_OPTYPE_6 0x03 /* Write, address required */ - -#define SPI_OPMENU_7 0x0b /* FAST: Fast Read */ -#define SPI_OPTYPE_7 0x02 /* Read, address required */ - -#define SPI_OPMENU_UPPER ((SPI_OPMENU_7 << 24) | (SPI_OPMENU_6 << 16) | \ - (SPI_OPMENU_5 << 8) | SPI_OPMENU_4) -#define SPI_OPMENU_LOWER ((SPI_OPMENU_3 << 24) | (SPI_OPMENU_2 << 16) | \ - (SPI_OPMENU_1 << 8) | SPI_OPMENU_0) - -#define SPI_OPTYPE ((SPI_OPTYPE_7 << 14) | (SPI_OPTYPE_6 << 12) | \ - (SPI_OPTYPE_5 << 10) | (SPI_OPTYPE_4 << 8) | \ - (SPI_OPTYPE_3 << 6) | (SPI_OPTYPE_2 << 4) | \ - (SPI_OPTYPE_1 << 2) | (SPI_OPTYPE_0)) - -#define SPI_OPPREFIX ((0x50 << 8) | 0x06) /* EWSR and WREN */ - -#define SPIBAR_OFFSET 0x3800 -#define SPI_REG_PREOP_OPTYPE 0xa4 -#define SPI_REG_OPMENU_L 0xa8 -#define SPI_REG_OPMENU_H 0xac - #define SD_CAP_BYP 0x810 #define SD_CAP_BYP_EN 0x5A #define SD_CAP_BYP_REG1 0x814 @@ -233,7 +189,6 @@ { uint16_t cmd = 0; struct device *dev = NULL; - void *spi_base = NULL;
/* Do board specific things */ variant_mainboard_final(); @@ -246,14 +201,7 @@ pci_write_config16(dev, PCI_COMMAND, cmd); } /* Set up SPI OPCODE menu before the controller is locked. */ - dev = PCH_DEV_SPI; - spi_base = (void *)pci_read_config32(dev, PCI_BASE_ADDRESS_0); - if (!spi_base) - return; - write32((spi_base + SPI_REG_PREOP_OPTYPE), - ((SPI_OPTYPE << 16) | SPI_OPPREFIX)); - write32((spi_base + SPI_REG_OPMENU_L), SPI_OPMENU_LOWER); - write32((spi_base + SPI_REG_OPMENU_H), SPI_OPMENU_UPPER); + fast_spi_set_opcode_menu();
/* Set SD-Card speed to HS mode only. */ dev = pcidev_path_on_root(PCH_DEVFN_SDCARD);
-
Arthur Heymans (Code Review) -
Mario Scheithauer (Code Review)
-
Name of user not set (Code Review)
-
Werner Zeh (Code Review)