Change in coreboot[master]: drivers/spi/spi_flash.c: Add SPI vendor IDs

[Richard Spiegel (Code Review)]

Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35240 )

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

drivers/spi/spi_flash.c: Add SPI vendor IDs

Currently SPI vendor IDs are magic numbers in spi_flash.c. These definitions
are needed for AMD's fch_spi. So add the definitions to spi_generic.h and use
it at spi_flash.c

BUG=b:136595978
TEST=Build test of several platforms that don't use stoneyridge. Build and boot
grunt (using stoneyridge new fch_spi).

Change-Id: Ie39485d8c092151db8c9d88afaf02e19c507c93f
Signed-off-by: Richard Spiegel 
---
M src/drivers/spi/spi_flash.c
M src/include/spi-generic.h
2 files changed, 25 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/35240/1

diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index e2485ab..a7b10a8 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -100,7 +100,6 @@
 #pragma GCC diagnostic push
 #if defined(__GNUC__) && !defined(__clang__)
 #pragma GCC diagnostic ignored "-Wstack-usage="
-#pragma GCC diagnostic ignored "-Wvla"
 #endif
 int spi_flash_cmd_write(const struct spi_slave *spi, const u8 *cmd,
    		size_t cmd_len, const void *data, size_t data_len)
@@ -282,38 +281,38 @@
 } flashes[] = {
    /* Keep it sorted by define name */
 #if CONFIG(SPI_FLASH_AMIC)
-	{ 0, 0x37, spi_flash_probe_amic, },
+	{ 0, VENDOR_ID_AMIC, spi_flash_probe_amic, },
 #endif
 #if CONFIG(SPI_FLASH_ATMEL)
-	{ 0, 0x1f, spi_flash_probe_atmel, },
+	{ 0, VENDOR_ID_ATMEL, spi_flash_probe_atmel, },
 #endif
 #if CONFIG(SPI_FLASH_EON)
-	{ 0, 0x1c, spi_flash_probe_eon, },
+	{ 0, VENDOR_ID_EON, spi_flash_probe_eon, },
 #endif
 #if CONFIG(SPI_FLASH_GIGADEVICE)
-	{ 0, 0xc8, spi_flash_probe_gigadevice, },
+	{ 0, VENDOR_ID_GIGADEVICE, spi_flash_probe_gigadevice, },
 #endif
 #if CONFIG(SPI_FLASH_MACRONIX)
-	{ 0, 0xc2, spi_flash_probe_macronix, },
+	{ 0, VENDOR_ID_MACRONIX, spi_flash_probe_macronix, },
 #endif
 #if CONFIG(SPI_FLASH_SPANSION)
-	{ 0, 0x01, spi_flash_probe_spansion, },
+	{ 0, VENDOR_ID_SPANSION, spi_flash_probe_spansion, },
 #endif
 #if CONFIG(SPI_FLASH_SST)
-	{ 0, 0xbf, spi_flash_probe_sst, },
+	{ 0, VENDOR_ID_SST, spi_flash_probe_sst, },
 #endif
 #if CONFIG(SPI_FLASH_STMICRO)
-	{ 0, 0x20, spi_flash_probe_stmicro, },
+	{ 0, VENDOR_ID_STMICRO, spi_flash_probe_stmicro, },
 #endif
 #if CONFIG(SPI_FLASH_WINBOND)
-	{ 0, 0xef, spi_flash_probe_winbond, },
+	{ 0, VENDOR_ID_WINBOND, spi_flash_probe_winbond, },
 #endif
    /* Keep it sorted by best detection */
 #if CONFIG(SPI_FLASH_STMICRO)
-	{ 0, 0xff, spi_flash_probe_stmicro, },
+	{ 0, VENDOR_ID_STMICRO_FF, spi_flash_probe_stmicro, },
 #endif
 #if CONFIG(SPI_FLASH_ADESTO)
-	{ 0, 0x1f, spi_flash_probe_adesto, },
+	{ 0, VENDOR_ID_ADESTO, spi_flash_probe_adesto, },
 #endif
 };
 #define IDCODE_LEN (IDCODE_CONT_LEN + IDCODE_PART_LEN)
diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h
index ffd3d2d..2f26dc5 100644
--- a/src/include/spi-generic.h
+++ b/src/include/spi-generic.h
@@ -28,6 +28,20 @@
 #include <stdint.h>
 #include <stddef.h>

+/* SPI vendor IDs */
+
+#define VENDOR_ID_ADESTO		0x1f
+#define VENDOR_ID_AMIC			0x37
+#define VENDOR_ID_ATMEL			0x1f
+#define VENDOR_ID_EON			0x1c
+#define VENDOR_ID_GIGADEVICE		0xc8
+#define VENDOR_ID_MACRONIX		0xc2
+#define VENDOR_ID_SPANSION		0x01
+#define VENDOR_ID_SST			0xbf
+#define VENDOR_ID_STMICRO		0x20
+#define VENDOR_ID_STMICRO_FF		0xff
+#define VENDOR_ID_WINBOND		0xef
+
 /* Controller-specific definitions: */

struct spi_ctrlr;

[Richard Spiegel (Code Review)]

Hello Felix Held, Marshall Dawson, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35240

to look at the new patch set (#2).

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

drivers/spi/spi_flash.c: Add SPI vendor IDs

Currently SPI vendor IDs are magic numbers in spi_flash.c. These definitions
are needed for AMD's fch_spi. So add the definitions to spi_generic.h and use
it at spi_flash.c

BUG=b:136595978
TEST=Build test of several platforms that don't use stoneyridge. Build and boot
grunt (using stoneyridge new fch_spi).

Change-Id: Ie39485d8c092151db8c9d88afaf02e19c507c93f
Signed-off-by: Richard Spiegel 
---
M src/drivers/spi/spi_flash.c
M src/include/spi-generic.h
2 files changed, 25 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/35240/2

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35240 )

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

Patch Set 2: Code-Review+2

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35240 )

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35240/2/src/include/spi-generic.h 
File src/include/spi-generic.h:

https://review.coreboot.org/c/coreboot/+/35240/2/src/include/spi-generic.h@3... 
PS2, Line 32: 
I could do without the blank line, and match the style in the rest of the file.  (Line 45 heads a whole bunch of definitions, so it doesn't count.)

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35240 )

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35240/2/src/include/spi-generic.h 
File src/include/spi-generic.h:

https://review.coreboot.org/c/coreboot/+/35240/2/src/include/spi-generic.h@3... 
PS2, Line 32:
> I could do without the blank line, and match the style in the rest of the file. […]

Done

[Richard Spiegel (Code Review)]

Hello Felix Held, Marshall Dawson, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35240

to look at the new patch set (#3).

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

drivers/spi/spi_flash.c: Add SPI vendor IDs

Currently SPI vendor IDs are magic numbers in spi_flash.c. These definitions
are needed for AMD's fch_spi. So add the definitions to spi_generic.h and use
it at spi_flash.c

BUG=b:136595978
TEST=Build test of several platforms that don't use stoneyridge. Build and boot
grunt (using stoneyridge new fch_spi).

Change-Id: Ie39485d8c092151db8c9d88afaf02e19c507c93f
Signed-off-by: Richard Spiegel 
---
M src/drivers/spi/spi_flash.c
M src/include/spi-generic.h
2 files changed, 24 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/35240/3

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35240 )

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

Patch Set 3: Code-Review+2

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35240 )

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

Patch Set 3: Code-Review+2

[Martin Roth (Code Review)]

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35240 )

Change subject: drivers/spi/spi_flash.c: Add SPI vendor IDs
......................................................................

drivers/spi/spi_flash.c: Add SPI vendor IDs

Currently SPI vendor IDs are magic numbers in spi_flash.c. These definitions
are needed for AMD's fch_spi. So add the definitions to spi_generic.h and use
it at spi_flash.c

BUG=b:136595978
TEST=Build test of several platforms that don't use stoneyridge. Build and boot
grunt (using stoneyridge new fch_spi).

Change-Id: Ie39485d8c092151db8c9d88afaf02e19c507c93f
Signed-off-by: Richard Spiegel 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35240
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Held 
Reviewed-by: Martin Roth 
---
M src/drivers/spi/spi_flash.c
M src/include/spi-generic.h
2 files changed, 24 insertions(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved
  Felix Held: Looks good to me, approved

diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index e2485ab..5dbe1f4 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -282,38 +282,38 @@
 } flashes[] = {
    /* Keep it sorted by define name */
 #if CONFIG(SPI_FLASH_AMIC)
-	{ 0, 0x37, spi_flash_probe_amic, },
+	{ 0, VENDOR_ID_AMIC, spi_flash_probe_amic, },
 #endif
 #if CONFIG(SPI_FLASH_ATMEL)
-	{ 0, 0x1f, spi_flash_probe_atmel, },
+	{ 0, VENDOR_ID_ATMEL, spi_flash_probe_atmel, },
 #endif
 #if CONFIG(SPI_FLASH_EON)
-	{ 0, 0x1c, spi_flash_probe_eon, },
+	{ 0, VENDOR_ID_EON, spi_flash_probe_eon, },
 #endif
 #if CONFIG(SPI_FLASH_GIGADEVICE)
-	{ 0, 0xc8, spi_flash_probe_gigadevice, },
+	{ 0, VENDOR_ID_GIGADEVICE, spi_flash_probe_gigadevice, },
 #endif
 #if CONFIG(SPI_FLASH_MACRONIX)
-	{ 0, 0xc2, spi_flash_probe_macronix, },
+	{ 0, VENDOR_ID_MACRONIX, spi_flash_probe_macronix, },
 #endif
 #if CONFIG(SPI_FLASH_SPANSION)
-	{ 0, 0x01, spi_flash_probe_spansion, },
+	{ 0, VENDOR_ID_SPANSION, spi_flash_probe_spansion, },
 #endif
 #if CONFIG(SPI_FLASH_SST)
-	{ 0, 0xbf, spi_flash_probe_sst, },
+	{ 0, VENDOR_ID_SST, spi_flash_probe_sst, },
 #endif
 #if CONFIG(SPI_FLASH_STMICRO)
-	{ 0, 0x20, spi_flash_probe_stmicro, },
+	{ 0, VENDOR_ID_STMICRO, spi_flash_probe_stmicro, },
 #endif
 #if CONFIG(SPI_FLASH_WINBOND)
-	{ 0, 0xef, spi_flash_probe_winbond, },
+	{ 0, VENDOR_ID_WINBOND, spi_flash_probe_winbond, },
 #endif
    /* Keep it sorted by best detection */
 #if CONFIG(SPI_FLASH_STMICRO)
-	{ 0, 0xff, spi_flash_probe_stmicro, },
+	{ 0, VENDOR_ID_STMICRO_FF, spi_flash_probe_stmicro, },
 #endif
 #if CONFIG(SPI_FLASH_ADESTO)
-	{ 0, 0x1f, spi_flash_probe_adesto, },
+	{ 0, VENDOR_ID_ADESTO, spi_flash_probe_adesto, },
 #endif
 };
 #define IDCODE_LEN (IDCODE_CONT_LEN + IDCODE_PART_LEN)
diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h
index ffd3d2d..c8dbb67 100644
--- a/src/include/spi-generic.h
+++ b/src/include/spi-generic.h
@@ -28,6 +28,19 @@
 #include <stdint.h>
 #include <stddef.h>

+/* SPI vendor IDs */
+#define VENDOR_ID_ADESTO			0x1f
+#define VENDOR_ID_AMIC				0x37
+#define VENDOR_ID_ATMEL				0x1f
+#define VENDOR_ID_EON				0x1c
+#define VENDOR_ID_GIGADEVICE			0xc8
+#define VENDOR_ID_MACRONIX			0xc2
+#define VENDOR_ID_SPANSION			0x01
+#define VENDOR_ID_SST				0xbf
+#define VENDOR_ID_STMICRO			0x20
+#define VENDOR_ID_STMICRO_FF			0xff
+#define VENDOR_ID_WINBOND			0xef
+
 /* Controller-specific definitions: */

struct spi_ctrlr;