[coreboot-gerrit] Patch set updated for coreboot: spi: Get rid of flash_programmer_probe in spi_slave structure
Furquan Shaikh (furquan@google.com)
gerrit at coreboot.org
Mon Nov 21 07:29:26 CET 2016
Furquan Shaikh (furquan at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17465
-gerrit
commit 1532efec362e6a5d6f2b1af6b4c98ec4471d11df
Author: Furquan Shaikh <furquan at chromium.org>
Date: Thu Nov 17 20:38:07 2016 -0800
spi: Get rid of flash_programmer_probe in spi_slave structure
flash_programmer_probe is a property of the spi flash driver and does
not belong in the spi_slave structure. Thus, make
spi_flash_programmer_probe a callback from the spi_flash_probe
function. Logic still remains the same as before (order matters):
1. Try spi_flash_programmer_probe without force option
2. Try generic flash probing
3. Try spi_flash_programmer_probe with force option
If none of the above steps work, fail probing. Flash controller is
expected to honor force option to decide whether to perform specialized
probing or to defer to generic probing.
BUG=None
BRANCH=None
TEST=Compiles successfully
Change-Id: I4163593eea034fa044ec2216e56d0ea3fbc86c7d
Signed-off-by: Furquan Shaikh <furquan at chromium.org>
---
src/drivers/spi/spi_flash.c | 81 ++++++++++++----------
src/include/spi-generic.h | 4 --
src/include/spi_flash.h | 1 +
src/soc/intel/apollolake/spi.c | 4 +-
src/soc/intel/skylake/flash_controller.c | 4 +-
src/soc/mediatek/mt8173/flash_controller.c | 2 +-
.../mediatek/mt8173/include/soc/flash_controller.h | 1 -
src/soc/mediatek/mt8173/spi.c | 2 -
src/southbridge/intel/common/spi.c | 13 ++--
9 files changed, 59 insertions(+), 53 deletions(-)
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index baa1f1a..b2fdab9 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -296,46 +296,39 @@ static struct {
};
#define IDCODE_LEN (IDCODE_CONT_LEN + IDCODE_PART_LEN)
+struct spi_flash *
+__attribute__((weak)) spi_flash_programmer_probe(struct spi_slave *spi,
+ int force)
+{
+ /* Default weak implementation. Do nothing. */
+ return NULL;
+}
-/* Public API implementations. */
-struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs)
+static struct spi_flash *__spi_flash_probe(struct spi_slave *spi)
{
- struct spi_slave *spi;
- struct spi_flash *flash = NULL;
int ret, i, shift;
u8 idcode[IDCODE_LEN], *idp;
-
- spi = spi_setup_slave(bus, cs);
- if (!spi) {
- printk(BIOS_WARNING, "SF: Failed to set up slave\n");
- return NULL;
- }
-
- if (spi->force_programmer_specific && spi->programmer_specific_probe) {
- flash = spi->programmer_specific_probe (spi);
- if (!flash)
- goto err_read_id;
- goto flash_detected;
- }
+ struct spi_flash *flash = NULL;
/* Read the ID codes */
ret = spi_flash_cmd(spi, CMD_READ_ID, idcode, sizeof(idcode));
if (ret)
- goto err_read_id;
+ return NULL;
-#if CONFIG_DEBUG_SPI_FLASH
- printk(BIOS_SPEW, "SF: Got idcode: ");
- for (i = 0; i < sizeof(idcode); i++)
- printk(BIOS_SPEW, "%02x ", idcode[i]);
- printk(BIOS_SPEW, "\n");
-#endif
+ if (IS_ENABLED(CONFIG_DEBUG_SPI_FLASH)) {
+ printk(BIOS_SPEW, "SF: Got idcode: ");
+ for (i = 0; i < sizeof(idcode); i++)
+ printk(BIOS_SPEW, "%02x ", idcode[i]);
+ printk(BIOS_SPEW, "\n");
+ }
/* count the number of continuation bytes */
- for (shift = 0, idp = idcode;
- shift < IDCODE_CONT_LEN && *idp == 0x7f;
+ for (shift = 0, idp = idcode; shift < IDCODE_CONT_LEN && *idp == 0x7f;
++shift, ++idp)
continue;
+ printk(BIOS_INFO, "Manufacturer: %02x\n", *idp);
+
/* search the table for matches in shift and id */
for (i = 0; i < ARRAY_SIZE(flashes); ++i)
if (flashes[i].shift == shift && flashes[i].idcode == *idp) {
@@ -345,15 +338,37 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs)
break;
}
- if (!flash && spi->programmer_specific_probe) {
- flash = spi->programmer_specific_probe (spi);
+ return flash;
+}
+
+struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs)
+{
+ struct spi_slave *spi;
+ struct spi_flash *flash;
+
+ spi = spi_setup_slave(bus, cs);
+ if (!spi) {
+ printk(BIOS_WARNING, "SF: Failed to set up slave\n");
+ return NULL;
}
+
+ /* Try special programmer probe if any (without force). */
+ flash = spi_flash_programmer_probe(spi, 0);
+
+ /* If flash is not found, try generic spi flash probe. */
+ if (!flash)
+ flash = __spi_flash_probe(spi);
+
+ /* If flash is not yet found, force special programmer probe if any. */
+ if (!flash)
+ flash = spi_flash_programmer_probe(spi, 1);
+
+ /* Give up -- nothing more to try if flash is not found. */
if (!flash) {
- printk(BIOS_WARNING, "SF: Unsupported manufacturer %02x\n", *idp);
- goto err_manufacturer_probe;
+ printk(BIOS_WARNING, "SF: Unsupported manufacturer!\n");
+ return NULL;
}
-flash_detected:
printk(BIOS_INFO, "SF: Detected %s with sector size 0x%x, total 0x%x\n",
flash->name, flash->sector_size, flash->size);
@@ -366,10 +381,6 @@ flash_detected:
spi_flash_dev = flash;
return flash;
-
-err_manufacturer_probe:
-err_read_id:
- return NULL;
}
int spi_flash_read(const struct spi_flash *flash, u32 offset, size_t len,
diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h
index eacb092..e57f56d 100644
--- a/src/include/spi-generic.h
+++ b/src/include/spi-generic.h
@@ -27,16 +27,12 @@
/*-----------------------------------------------------------------------
* Representation of a SPI slave, i.e. what we're communicating with.
*
- * Drivers are expected to extend this with controller-specific data.
- *
* bus: ID of the bus that the slave is attached to.
* cs: ID of the chip select connected to the slave.
*/
struct spi_slave {
unsigned int bus;
unsigned int cs;
- int force_programmer_specific;
- struct spi_flash * (*programmer_specific_probe) (struct spi_slave *spi);
};
/*-----------------------------------------------------------------------
diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h
index 5cb6829..83af288 100644
--- a/src/include/spi_flash.h
+++ b/src/include/spi_flash.h
@@ -45,6 +45,7 @@ void lb_spi_flash(struct lb_header *header);
/* SPI Flash Driver Public API */
struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs);
+struct spi_flash *spi_flash_programmer_probe(struct spi_slave *spi, int force);
/* All the following functions return 0 on success and non-zero on error. */
int spi_flash_read(const struct spi_flash *flash, u32 offset, size_t len,
diff --git a/src/soc/intel/apollolake/spi.c b/src/soc/intel/apollolake/spi.c
index 018572c..4da292a 100644
--- a/src/soc/intel/apollolake/spi.c
+++ b/src/soc/intel/apollolake/spi.c
@@ -354,7 +354,7 @@ static struct spi_flash boot_flash CAR_GLOBAL;
* 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.
*/
-static struct spi_flash *nuclear_flash_probe(struct spi_slave *spi)
+struct spi_flash *spi_flash_programmer_probe(struct spi_slave *spi, int force)
{
BOILERPLATE_CREATE_CTX(ctx);
struct spi_flash *flash;
@@ -403,8 +403,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
slave->bus = bus;
slave->cs = cs;
- slave->programmer_specific_probe = nuclear_flash_probe;
- slave->force_programmer_specific = 1;
return slave;
}
diff --git a/src/soc/intel/skylake/flash_controller.c b/src/soc/intel/skylake/flash_controller.c
index 340e555..0d75c3f 100644
--- a/src/soc/intel/skylake/flash_controller.c
+++ b/src/soc/intel/skylake/flash_controller.c
@@ -346,7 +346,7 @@ int pch_hwseq_read_status(const struct spi_flash *flash, u8 *reg)
static struct spi_slave boot_spi CAR_GLOBAL;
static struct spi_flash boot_flash CAR_GLOBAL;
-static struct spi_flash *spi_flash_hwseq_probe(struct spi_slave *spi)
+struct spi_flash *spi_flash_programmer_probe(struct spi_slave *spi, int force)
{
struct spi_flash *flash;
@@ -381,8 +381,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
slave->bus = bus;
slave->cs = cs;
- slave->force_programmer_specific = 1;
- slave->programmer_specific_probe = spi_flash_hwseq_probe;
return slave;
}
diff --git a/src/soc/mediatek/mt8173/flash_controller.c b/src/soc/mediatek/mt8173/flash_controller.c
index dc64d40..3799c6a 100644
--- a/src/soc/mediatek/mt8173/flash_controller.c
+++ b/src/soc/mediatek/mt8173/flash_controller.c
@@ -233,7 +233,7 @@ static int nor_erase(const struct spi_flash *flash, u32 offset, size_t len)
return 0;
}
-struct spi_flash *mt8173_nor_flash_probe(struct spi_slave *spi)
+struct spi_flash *spi_flash_programmer_probe(struct spi_slave *spi, int force)
{
static struct spi_flash flash = {0};
diff --git a/src/soc/mediatek/mt8173/include/soc/flash_controller.h b/src/soc/mediatek/mt8173/include/soc/flash_controller.h
index 1d2ac32..458f357 100644
--- a/src/soc/mediatek/mt8173/include/soc/flash_controller.h
+++ b/src/soc/mediatek/mt8173/include/soc/flash_controller.h
@@ -87,5 +87,4 @@ struct mt8173_nor_regs {
check_member(mt8173_nor_regs, fdma_end_dadr, 0x724);
static struct mt8173_nor_regs * const mt8173_nor = (void *)SFLASH_REG_BASE;
-struct spi_flash *mt8173_nor_flash_probe(struct spi_slave *spi);
#endif /* __SOC_MEDIATEK_MT8173_FLASH_CONTROLLER_H__ */
diff --git a/src/soc/mediatek/mt8173/spi.c b/src/soc/mediatek/mt8173/spi.c
index c35b1fc..d8639e0 100644
--- a/src/soc/mediatek/mt8173/spi.c
+++ b/src/soc/mediatek/mt8173/spi.c
@@ -173,8 +173,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
case CONFIG_BOOT_DEVICE_SPI_FLASH_BUS:
slave.bus = bus;
slave.cs = cs;
- slave.force_programmer_specific = 1;
- slave.programmer_specific_probe = &mt8173_nor_flash_probe;
return &slave;
default:
die ("wrong bus number.\n");
diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c
index e041422..36102c2 100644
--- a/src/southbridge/intel/common/spi.c
+++ b/src/southbridge/intel/common/spi.c
@@ -66,7 +66,6 @@
#endif /* !__SMM__ */
static int spi_is_multichip(void);
-static struct spi_flash *spi_flash_hwseq(struct spi_slave *spi);
typedef struct spi_slave ich_spi_slave;
@@ -299,8 +298,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
slave->bus = bus;
slave->cs = cs;
- slave->force_programmer_specific = spi_is_multichip ();
- slave->programmer_specific_probe = spi_flash_hwseq;
return slave;
}
@@ -920,11 +917,19 @@ static int ich_hwseq_write(const struct spi_flash *flash, u32 addr, size_t len,
}
-static struct spi_flash *spi_flash_hwseq(struct spi_slave *spi)
+struct spi_flash *spi_flash_programmer_probe(struct spi_slave *spi, int force)
{
struct spi_flash *flash = NULL;
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 NULL;
+
flash = malloc(sizeof(*flash));
if (!flash) {
printk(BIOS_WARNING, "SF: Failed to allocate memory\n");
More information about the coreboot-gerrit
mailing list