[coreboot-gerrit] Patch set updated for coreboot: spi: Clean up SPI driver interface
Furquan Shaikh (furquan@google.com)
gerrit at coreboot.org
Tue Nov 22 00:31:45 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/17561
-gerrit
commit 3b75114eb4590070861b6915c9ccf05e7d5a3126
Author: Furquan Shaikh <furquan at chromium.org>
Date: Mon Nov 21 11:41:24 2016 -0800
spi: Clean up SPI driver interface
1. Add new structures to define:
a. SPI device (spi_slave) -- Name is kept similar to current spi
slave to allow easy transition from older interface to new
interface.
b. SPI controller (spi_ctrlr) - SPI controller structure defined by
chipset.
c. SPI controller to bus mapping (spi_ctrlr_buses) - Map to indicate
what buses are managed by a SPI controller. SoC is expected to define
this mapping for all available SPI buses.
2. Add spi_device_init function to allow users to initialize a
particular SPI device based on the bus and chip select number of the
device.
3. All platforms using this new interface will have to define a mapping
between the SPI buses and controllers that allows spi_device_init to
associate the right controller with a device.
BUG=chrome-os-partner:59832
BRANCH=None
TEST=Compiles successfully for reef with SPI_GENERIC option selected.
Change-Id: Ia6f47941b786299f4d823895898ffb1b36e02f73
Signed-off-by: Furquan Shaikh <furquan at chromium.org>
---
src/drivers/spi/Kconfig | 6 ++++
src/drivers/spi/Makefile.inc | 7 ++++
src/drivers/spi/spi_flash.c | 78 ++++++++++++++++++++++++++++++++----------
src/drivers/spi/spi_generic.c | 45 ++++++++++++++++++++++++
src/include/spi-generic.h | 79 ++++++++++++++++++++++++++++++++++++++-----
5 files changed, 189 insertions(+), 26 deletions(-)
diff --git a/src/drivers/spi/Kconfig b/src/drivers/spi/Kconfig
index b55de58..d30c4eb 100644
--- a/src/drivers/spi/Kconfig
+++ b/src/drivers/spi/Kconfig
@@ -13,6 +13,12 @@
## GNU General Public License for more details.
##
+config SPI_GENERIC
+ bool
+ default n
+ help
+ Use SPI generic interface for communication with SPI devices.
+
config COMMON_CBFS_SPI_WRAPPER
bool
default n
diff --git a/src/drivers/spi/Makefile.inc b/src/drivers/spi/Makefile.inc
index 92baf62..bb2a56e 100644
--- a/src/drivers/spi/Makefile.inc
+++ b/src/drivers/spi/Makefile.inc
@@ -21,6 +21,7 @@ bootblock-$(CONFIG_SPI_FLASH_SST) += sst.c
bootblock-$(CONFIG_SPI_FLASH_STMICRO) += stmicro.c
bootblock-$(CONFIG_SPI_FLASH_WINBOND) += winbond.c
bootblock-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.c
+bootblock-$(CONFIG_SPI_GENERIC) += spi_generic.c
romstage-$(CONFIG_COMMON_CBFS_SPI_WRAPPER) += cbfs_spi.c
romstage-$(CONFIG_SPI_FLASH) += spi_flash.c
@@ -36,6 +37,7 @@ romstage-$(CONFIG_SPI_FLASH_SST) += sst.c
romstage-$(CONFIG_SPI_FLASH_STMICRO) += stmicro.c
romstage-$(CONFIG_SPI_FLASH_WINBOND) += winbond.c
romstage-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.c
+romstage-$(CONFIG_SPI_GENERIC) += spi_generic.c
verstage-$(CONFIG_COMMON_CBFS_SPI_WRAPPER) += cbfs_spi.c
verstage-$(CONFIG_SPI_FLASH) += spi_flash.c
@@ -51,6 +53,7 @@ verstage-$(CONFIG_SPI_FLASH_SST) += sst.c
verstage-$(CONFIG_SPI_FLASH_STMICRO) += stmicro.c
verstage-$(CONFIG_SPI_FLASH_WINBOND) += winbond.c
verstage-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.c
+verstage-$(CONFIG_SPI_GENERIC) += spi_generic.c
ramstage-$(CONFIG_COMMON_CBFS_SPI_WRAPPER) += cbfs_spi.c
ramstage-$(CONFIG_SPI_FLASH) += spi_flash.c
@@ -66,6 +69,7 @@ ramstage-$(CONFIG_SPI_FLASH_SST) += sst.c
ramstage-$(CONFIG_SPI_FLASH_STMICRO) += stmicro.c
ramstage-$(CONFIG_SPI_FLASH_WINBOND) += winbond.c
ramstage-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.c
+ramstage-$(CONFIG_SPI_GENERIC) += spi_generic.c
ifeq ($(CONFIG_SPI_FLASH_SMM),y)
# SPI flash driver interface
@@ -84,4 +88,7 @@ smm-$(CONFIG_SPI_FLASH_SST) += sst.c
smm-$(CONFIG_SPI_FLASH_STMICRO) += stmicro.c
smm-$(CONFIG_SPI_FLASH_WINBOND) += winbond.c
smm-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.c
+
+# SPI generic driver interface
+smm-$(CONFIG_SPI_GENERIC) += spi_generic.c
endif
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index b2fdab9..1d086be 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -32,6 +32,14 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
cmd[3] = addr >> 0;
}
+#if IS_ENABLED(CONFIG_SPI_GENERIC)
+typedef int (*spi_xfer_fnptr)(const struct spi_slave *, const void *,
+ unsigned int, void *, unsigned int);
+#else
+typedef int (*spi_xfer_fnptr)(struct spi_slave *, const void *, unsigned int,
+ void *, unsigned int);
+#endif
+
/*
* If atomic sequencing is used, the cycle type is known to the SPI
* controller so that it can perform consecutive transfers and arbitrate
@@ -45,37 +53,65 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
* spi_xfer() at once and let the controller handle the details. Otherwise
* we will write all output bytes first and then read if necessary.
*
- * FIXME: This really should be abstracted better, but that will
- * require overhauling the entire SPI infrastructure.
*/
+static int do_spi_flash_xfer(spi_xfer_fnptr xfer_fn, struct spi_slave *spi,
+ const void *dout, unsigned int bytes_out,
+ void *din, unsigned int bytes_in)
+{
+ if (IS_ENABLED(CONFIG_SPI_ATOMIC_SEQUENCING)) {
+ if (xfer_fn(spi, dout, bytes_out, din, bytes_in))
+ return 1;
+ return 0;
+ }
+
+ if (dout && bytes_out)
+ if (xfer_fn(spi, dout, bytes_out, NULL, 0))
+ return 1;
+ if (din && bytes_in)
+ if (xfer_fn(spi, NULL, 0, din, bytes_in))
+ return 1;
+
+ return 0;
+}
+
+#if IS_ENABLED(CONFIG_SPI_GENERIC)
static int do_spi_flash_cmd(struct spi_slave *spi, const void *dout,
unsigned int bytes_out, void *din, unsigned int bytes_in)
{
+ const struct spi_ctrlr *ctrlr = spi->ctrlr;
+ spi_xfer_fnptr xfer_fn;
int ret = 1;
- if (spi_claim_bus(spi))
+ if (ctrlr->claim_bus && ctrlr->claim_bus(spi))
return ret;
-#if CONFIG_SPI_ATOMIC_SEQUENCING == 1
- if (spi_xfer(spi, dout, bytes_out, din, bytes_in) < 0)
- goto done;
+ assert(ctrlr->xfer);
+ xfer_fn = ctrlr->xfer;
+ ret = do_spi_flash_xfer(xfer_fn, spi, dout, bytes_out, din, bytes_in);
+
+ if (ctrlr->release_bus)
+ ctrlr->release_bus(spi);
+
+ return ret;
+}
+
#else
- if (dout && bytes_out) {
- if (spi_xfer(spi, dout, bytes_out, NULL, 0) < 0)
- goto done;
- }
+static int do_spi_flash_cmd(struct spi_slave *spi, const void *dout,
+ unsigned int bytes_out, void *din, unsigned int bytes_in)
+{
+ spi_xfer_fnptr xfer_fn;
+ int ret = 1;
- if (din && bytes_in) {
- if (spi_xfer(spi, NULL, 0, din, bytes_in) < 0)
- goto done;
- }
-#endif
+ if (spi_claim_bus(spi))
+ return ret;
+
+ xfer_fn = spi_xfer;
+ ret = do_spi_flash_xfer(xfer_fn, spi, dout, bytes_out, din, bytes_in);
- ret = 0;
-done:
spi_release_bus(spi);
return ret;
}
+#endif
int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len)
{
@@ -346,7 +382,15 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs)
struct spi_slave *spi;
struct spi_flash *flash;
+#if IS_ENABLED(CONFIG_SPI_GENERIC)
+ struct spi_slave dev;
+ spi = &dev;
+ if (spi_device_init(bus, cs, spi))
+ spi = NULL;
+#else
spi = spi_setup_slave(bus, cs);
+#endif
+
if (!spi) {
printk(BIOS_WARNING, "SF: Failed to set up slave\n");
return NULL;
diff --git a/src/drivers/spi/spi_generic.c b/src/drivers/spi/spi_generic.c
new file mode 100644
index 0000000..1190a60
--- /dev/null
+++ b/src/drivers/spi/spi_generic.c
@@ -0,0 +1,45 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright 2016 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <spi-generic.h>
+#include <string.h>
+
+void spi_init(void)
+{
+ /* Do nothing. */
+}
+
+int spi_device_init(unsigned int bus, unsigned int cs, struct spi_slave *dev)
+{
+ size_t i;
+
+ memset(dev, 0, sizeof(*dev));
+
+ for (i = 0; i < spi_ctrlr_bus_map_count; i++) {
+ if ((spi_ctrlr_bus_map[i].bus_start <= bus) &&
+ (spi_ctrlr_bus_map[i].bus_end >= bus)) {
+ dev->ctrlr = spi_ctrlr_bus_map[i].ctrlr;
+ break;
+ }
+ }
+
+ if (dev->ctrlr == NULL)
+ return -1;
+
+ dev->bus = bus;
+ dev->cs = cs;
+ return dev->ctrlr->setup(dev);
+}
diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h
index e57f56d..d19e5fd 100644
--- a/src/include/spi-generic.h
+++ b/src/include/spi-generic.h
@@ -17,6 +17,7 @@
#define _SPI_GENERIC_H_
#include <stdint.h>
+#include <stddef.h>
/* Controller-specific definitions: */
@@ -25,21 +26,84 @@
#define SPI_OPCODE_FAST_READ 0x0b
/*-----------------------------------------------------------------------
- * Representation of a SPI slave, i.e. what we're communicating with.
+ * Initialization, must be called once on start up.
+ *
+ */
+void spi_init(void);
+unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len);
+
+#if IS_ENABLED(CONFIG_SPI_GENERIC)
+
+struct spi_ctrlr;
+
+/*-----------------------------------------------------------------------
+ * Representation of a SPI device, i.e. what we're communicating with.
*
* bus: ID of the bus that the slave is attached to.
* cs: ID of the chip select connected to the slave.
+ * ctrlr: Pointer to SPI controller structure managing this device.
*/
struct spi_slave {
- unsigned int bus;
- unsigned int cs;
+ unsigned int bus;
+ unsigned int cs;
+ const struct spi_ctrlr *ctrlr;
};
/*-----------------------------------------------------------------------
- * Initialization, must be called once on start up.
+ * Representation of a SPI controller
*
+ * setup: Callback to setup given SPI device.
+ * xfer: Callback to transfer data with given SPI device.
+ * claim_bus: Callback to claim SPI bus connected to given device.
+ * release_bus: Callback to release SPI bus connected to given device.
*/
-void spi_init(void);
+struct spi_ctrlr {
+ int (*setup)(const struct spi_slave *dev);
+ int (*xfer)(const struct spi_slave *dev, const void *dout,
+ unsigned int bytes_out, void *din, unsigned int bytes_in);
+ int (*claim_bus)(const struct spi_slave *dev);
+ int (*release_bus)(const struct spi_slave *dev);
+};
+
+/*-----------------------------------------------------------------------
+ * Structure defining mapping of SPI buses to controller.
+ *
+ * ctrlr: Pointer to controller structure managing the given SPI buses.
+ * bus_start: Start bus number managed by the controller.
+ * bus_end: End bus number manager by the controller.
+ */
+struct spi_ctrlr_buses {
+ const struct spi_ctrlr *ctrlr;
+ unsigned int bus_start;
+ unsigned int bus_end;
+};
+
+/*-----------------------------------------------------------------------
+ * Initialize a SPI device identified by the bus and chip select number.
+ * Internally identifies the SPI controller that manages this bus and calls
+ * setup for the device.
+ *
+ * Fills in spi_slave structure with required information and returns 0 on
+ * success and -1 on error.
+ */
+int spi_device_init(unsigned int bus, unsigned int cs, struct spi_slave *dev);
+
+/* Mapping of SPI buses to controllers - should be defined by platform. */
+extern const struct spi_ctrlr_buses spi_ctrlr_bus_map[];
+extern const size_t spi_ctrlr_bus_map_count;
+
+#else
+
+/*-----------------------------------------------------------------------
+ * Representation of a SPI slave, i.e. what we're communicating with.
+ *
+ * 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;
+};
/*-----------------------------------------------------------------------
* Set up communications parameters for a SPI slave.
@@ -99,10 +163,6 @@ void spi_release_bus(struct spi_slave *slave);
int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bytesout,
void *din, unsigned int bytesin);
-
-
-unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len);
-
/*-----------------------------------------------------------------------
* Write 8 bits, then read 8 bits.
* slave: The SPI slave we're communicating with
@@ -124,5 +184,6 @@ static inline int spi_w8r8(struct spi_slave *slave, unsigned char byte)
ret = spi_xfer(slave, dout, 2, din, 2);
return ret < 0 ? ret : din[1];
}
+#endif
#endif /* _SPI_GENERIC_H_ */
More information about the coreboot-gerrit
mailing list