[coreboot-gerrit] Change in coreboot[master]: drivers/spi/spi_flash: Clean up SPI flash probe

Furquan Shaikh (Code Review) gerrit at coreboot.org
Fri May 19 21:23:17 CEST 2017


Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/19708 )

Change subject: drivers/spi/spi_flash: Clean up SPI flash probe
......................................................................


drivers/spi/spi_flash: Clean up SPI flash probe

1. Rename __spi_flash_probe to spi_flash_generic_probe and export it
so that drivers can use it outside spi_flash.c.
2. Make southbridge intel spi driver use spi_flash_generic_probe if
spi_is_multichip returns 0.
3. Add spi_flash_probe to spi_ctrlr structure to allow platforms to
provide specialized probe functions. With this change, the specialized
spi flash probe functions are now associated with a particular spi
ctrlr structure and no longer disconnected from the spi controller.

BUG=b:38330715

Change-Id: I35f3bd8ddc5e71515df3ef0c1c4b1a68ee56bf4b
Signed-off-by: Furquan Shaikh <furquan at chromium.org>
Reviewed-on: https://review.coreboot.org/19708
Tested-by: build bot (Jenkins) <no-reply at coreboot.org>
Reviewed-by: Aaron Durbin <adurbin at chromium.org>
Reviewed-by: Julius Werner <jwerner at chromium.org>
---
M src/drivers/spi/spi_flash.c
M src/include/spi-generic.h
M src/include/spi_flash.h
M src/soc/intel/common/block/fast_spi/fast_spi_flash.c
M src/soc/mediatek/mt8173/flash_controller.c
M src/soc/mediatek/mt8173/include/soc/flash_controller.h
M src/soc/mediatek/mt8173/spi.c
M src/southbridge/intel/common/spi.c
8 files changed, 53 insertions(+), 62 deletions(-)

Approvals:
  Aaron Durbin: Looks good to me, approved
  Julius Werner: Looks good to me, approved
  build bot (Jenkins): Verified



diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index 096d7cd..564d573 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -281,16 +281,7 @@
 };
 #define IDCODE_LEN (IDCODE_CONT_LEN + IDCODE_PART_LEN)
 
-int
-__attribute__((weak)) spi_flash_programmer_probe(const struct spi_slave *spi,
-						 int force,
-						 struct spi_flash *flash)
-{
-	/* Default weak implementation. Do nothing. */
-	return -1;
-}
-
-static int __spi_flash_probe(const struct spi_slave *spi,
+int spi_flash_generic_probe(const struct spi_slave *spi,
 				struct spi_flash *flash)
 {
 	int ret, i, shift;
@@ -330,32 +321,29 @@
 int spi_flash_probe(unsigned int bus, unsigned int cs, struct spi_flash *flash)
 {
 	struct spi_slave spi;
+	int ret = -1;
 
 	if (spi_setup_slave(bus, cs, &spi)) {
 		printk(BIOS_WARNING, "SF: Failed to set up slave\n");
 		return -1;
 	}
 
-	/* Try special programmer probe if any (without force). */
-	if (spi_flash_programmer_probe(&spi, 0, flash) == 0)
-		goto flash_found;
+	/* Try special programmer probe if any. */
+	if (spi.ctrlr->flash_probe)
+		ret = spi.ctrlr->flash_probe(&spi, flash);
 
 	/* If flash is not found, try generic spi flash probe. */
-	if (__spi_flash_probe(&spi, flash) == 0)
-		goto flash_found;
-
-	/* If flash is not yet found, force special programmer probe if any. */
-	if (spi_flash_programmer_probe(&spi, 1, flash) == 0)
-		goto flash_found;
+	if (ret)
+		ret = spi_flash_generic_probe(&spi, flash);
 
 	/* Give up -- nothing more to try if flash is not found. */
-	printk(BIOS_WARNING, "SF: Unsupported manufacturer!\n");
-	return -1;
+	if (ret) {
+		printk(BIOS_WARNING, "SF: Unsupported manufacturer!\n");
+		return -1;
+	}
 
-flash_found:
 	printk(BIOS_INFO, "SF: Detected %s with sector size 0x%x, total 0x%x\n",
 			flash->name, flash->sector_size, flash->size);
-
 	return 0;
 }
 
diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h
index 56353bb..783df0b 100644
--- a/src/include/spi-generic.h
+++ b/src/include/spi-generic.h
@@ -94,6 +94,8 @@
  */
 #define SPI_CTRLR_DEFAULT_MAX_XFER_SIZE	(UINT32_MAX)
 
+struct spi_flash;
+
 /*-----------------------------------------------------------------------
  * Representation of a SPI controller.
  *
@@ -108,6 +110,11 @@
  * deduct_cmd_len:	Whether cmd_len should be deducted from max_xfer_size
  *			when calculating max_data_size
  *
+ * Following member is provided by specialized SPI controllers that are
+ * actually SPI flash controllers.
+ *
+ * flash_probe:	Specialized probe function provided by SPI flash
+ *			controllers.
  */
 struct spi_ctrlr {
 	int (*claim_bus)(const struct spi_slave *slave);
@@ -119,6 +126,8 @@
 			struct spi_op vectors[], size_t count);
 	uint32_t max_xfer_size;
 	bool deduct_cmd_len;
+	int (*flash_probe)(const struct spi_slave *slave,
+				struct spi_flash *flash);
 };
 
 /*-----------------------------------------------------------------------
diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h
index bc0318c..ab8155d 100644
--- a/src/include/spi_flash.h
+++ b/src/include/spi_flash.h
@@ -65,20 +65,19 @@
  * non-zero = error
  */
 int spi_flash_probe(unsigned int bus, unsigned int cs, struct spi_flash *flash);
+
 /*
- * Specialized probing performed by platform. This is a weak function which can
- * be overriden by platform driver.
+ * Generic probing for SPI flash chip based on the different flashes provided.
  *
  * Params:
- * spi   = Pointer to spi_slave structure.
- * force = Indicates if the platform driver can skip specialized probing.
+ * spi   = Pointer to spi_slave structure
  * flash = Pointer to spi_flash structure that needs to be filled.
  *
  * Return value:
- * 0 = success
+ * 0        = success
  * non-zero = error
  */
-int spi_flash_programmer_probe(const struct spi_slave *spi, int force,
+int spi_flash_generic_probe(const struct spi_slave *slave,
 				struct spi_flash *flash);
 
 /* All the following functions return 0 on success and non-zero on error. */
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c
index 7780144..fc36553 100644
--- a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c
+++ b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c
@@ -280,8 +280,8 @@
  * The size of the flash component is always taken from density field in the
  * SFDP table. FLCOMP.C0DEN is no longer used by the Flash Controller.
  */
-int spi_flash_programmer_probe(const struct spi_slave *dev,
-				int force, struct spi_flash *flash)
+static int fast_spi_flash_probe(const struct spi_slave *dev,
+				struct spi_flash *flash)
 {
 	BOILERPLATE_CREATE_CTX(ctx);
 	uint32_t flash_bits;
@@ -362,4 +362,5 @@
 const struct spi_ctrlr fast_spi_flash_ctrlr = {
 	.setup = fast_spi_flash_ctrlr_setup,
 	.max_xfer_size = SPI_CTRLR_DEFAULT_MAX_XFER_SIZE,
+	.flash_probe = fast_spi_flash_probe,
 };
diff --git a/src/soc/mediatek/mt8173/flash_controller.c b/src/soc/mediatek/mt8173/flash_controller.c
index ee950b8..f4c0de4 100644
--- a/src/soc/mediatek/mt8173/flash_controller.c
+++ b/src/soc/mediatek/mt8173/flash_controller.c
@@ -228,14 +228,8 @@
 	return 0;
 }
 
-int spi_flash_programmer_probe(const struct spi_slave *spi,
-			       int force, struct spi_flash *flash)
+int mtk_spi_flash_probe(const struct spi_slave *spi, struct spi_flash *flash)
 {
-	static int done;
-
-	if (done)
-		return 0;
-
 	write32(&mt8173_nor->wrprot, SFLASH_COMMAND_ENABLE);
 	memcpy(&flash->spi, spi, sizeof(*spi));
 	flash->name = "mt8173 flash controller";
@@ -246,6 +240,6 @@
 	flash->sector_size = 0x1000;
 	flash->erase_cmd = SECTOR_ERASE_CMD;
 	flash->size = CONFIG_ROM_SIZE;
-	done = 1;
+
 	return 0;
 }
diff --git a/src/soc/mediatek/mt8173/include/soc/flash_controller.h b/src/soc/mediatek/mt8173/include/soc/flash_controller.h
index 458f357..82d167a 100644
--- a/src/soc/mediatek/mt8173/include/soc/flash_controller.h
+++ b/src/soc/mediatek/mt8173/include/soc/flash_controller.h
@@ -87,4 +87,6 @@
 check_member(mt8173_nor_regs, fdma_end_dadr, 0x724);
 static struct mt8173_nor_regs * const mt8173_nor = (void *)SFLASH_REG_BASE;
 
+int mtk_spi_flash_probe(const struct spi_slave *spi, struct spi_flash *flash);
+
 #endif /* __SOC_MEDIATEK_MT8173_FLASH_CONTROLLER_H__ */
diff --git a/src/soc/mediatek/mt8173/spi.c b/src/soc/mediatek/mt8173/spi.c
index 188bdc2..b8ee423 100644
--- a/src/soc/mediatek/mt8173/spi.c
+++ b/src/soc/mediatek/mt8173/spi.c
@@ -295,6 +295,7 @@
 	.xfer = spi_ctrlr_xfer,
 	.xfer_vector = spi_xfer_two_vectors,
 	.max_xfer_size = 65535,
+	.flash_probe = mtk_spi_flash_probe,
 };
 
 int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave)
diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c
index 110c29c..79b1db7 100644
--- a/src/southbridge/intel/common/spi.c
+++ b/src/southbridge/intel/common/spi.c
@@ -653,20 +653,6 @@
 	return 0;
 }
 
-static const struct spi_ctrlr spi_ctrlr = {
-	.xfer = spi_ctrlr_xfer,
-	.xfer_vector = spi_xfer_two_vectors,
-	.max_xfer_size = member_size(ich9_spi_regs, fdata),
-};
-
-int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave)
-{
-	slave->bus = bus;
-	slave->cs = cs;
-	slave->ctrlr = &spi_ctrlr;
-	return 0;
-}
-
 /* Sets FLA in FADDR to (addr & 0x01FFFFFF) without touching other bits. */
 static void ich_hwseq_set_addr(uint32_t addr)
 {
@@ -899,18 +885,14 @@
 	return 0;
 }
 
-int spi_flash_programmer_probe(const struct spi_slave *spi,
-			       int force, struct spi_flash *flash)
+static int spi_flash_programmer_probe(const struct spi_slave *spi,
+					struct spi_flash *flash)
 {
 	uint32_t flcomp;
 
-	/*
-	 * Perform SPI flash probing only if:
-	 * 1. spi_is_multichip returns 1 or
-	 * 2. Specialized probing is forced by SPI flash driver.
-	 */
-	if (!spi_is_multichip() && !force)
-		return -1;
+	/* Try generic probing first if spi_is_multichip returns 0. */
+	if (!spi_is_multichip() && !spi_flash_generic_probe(spi, flash))
+		return 0;
 
 	memcpy(&flash->spi, spi, sizeof(*spi));
 	flash->name = "Opaque HW-sequencing";
@@ -946,3 +928,18 @@
 
 	return 0;
 }
+
+static const struct spi_ctrlr spi_ctrlr = {
+	.xfer = spi_ctrlr_xfer,
+	.xfer_vector = spi_xfer_two_vectors,
+	.max_xfer_size = member_size(ich9_spi_regs, fdata),
+	.flash_probe = spi_flash_programmer_probe,
+};
+
+int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave)
+{
+	slave->bus = bus;
+	slave->cs = cs;
+	slave->ctrlr = &spi_ctrlr;
+	return 0;
+}

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I35f3bd8ddc5e71515df3ef0c1c4b1a68ee56bf4b
Gerrit-PatchSet: 16
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>



More information about the coreboot-gerrit mailing list