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@chromium.org Reviewed-on: https://review.coreboot.org/19708 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Julius Werner jwerner@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; +}
-
Furquan Shaikh (Code Review)