SPI bitbanging on devices which speak SPI natively has a dual-use problem: We need to shut down normal SPI operations to do the bitbanging ourselves. Once we're done, it makes a lot of sense to reenable "normal" SPI operations again. Add request_bus/release_bus functions to struct bitbang_spi_master. Add a bitbang shutdown function (not used yet). Change MCP SPI and Intel NIC SPI to use the new request/release bus infrastructure.
There are multiple possible strategies for bus request/release: - Request at the start of a SPI command, release immediately afterwards. - Request at the start of a SPI multicommand, release once all commands of the multicommand are done. - Request on programmer init, release on shutdown. Each strategy has its own advantages. For now, we will stay with the first strategy which worked fine so far.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/bitbang_spi.c =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/bitbang_spi.c (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/bitbang_spi.c (Arbeitskopie) @@ -53,6 +53,18 @@ return bitbang_spi_master->get_miso(); }
+static void bitbang_spi_request_bus(void) +{ + if (bitbang_spi_master->request_bus) + bitbang_spi_master->request_bus(); +} + +static void bitbang_spi_release_bus(void) +{ + if (bitbang_spi_master->release_bus) + bitbang_spi_master->release_bus(); +} + int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod) { /* BITBANG_SPI_INVALID is 0, so if someone forgot to initialize ->type, @@ -61,19 +73,44 @@ */ if (!master || master->type == BITBANG_SPI_INVALID || !master->set_cs || !master->set_sck || !master->set_mosi || !master->get_miso) { - msg_perr("Incomplete bitbanging SPI master setting! Please " - "report a bug at flashrom@flashrom.org\n"); + msg_perr("Incomplete SPI bitbang master setting! " + "Please report a bug at flashrom@flashrom.org\n"); return 1; } + if (bitbang_spi_master) { + msg_perr("SPI bitbang master already initialized! " + "Please report a bug at flashrom@flashrom.org\n"); + return 1; + } + bitbang_spi_master = master; bitbang_spi_half_period = halfperiod;
+ /* FIXME: Run bitbang_spi_request_bus here or in programmer init? */ bitbang_spi_set_cs(1); bitbang_spi_set_sck(0); bitbang_spi_set_mosi(0); return 0; }
+int bitbang_spi_shutdown(const struct bitbang_spi_master *master) +{ + if (!bitbang_spi_master) { + msg_perr("Shutting down an uninitialized SPI bitbang master! " + "Please report a bug at flashrom@flashrom.org\n"); + return 1; + } + if (master != bitbang_spi_master) { + msg_perr("Shutting down a mismatched SPI bitbang master! " + "Please report a bug at flashrom@flashrom.org\n"); + return 1; + } + + /* FIXME: Run bitbang_spi_release_bus here or per command? */ + bitbang_spi_master = NULL; + return 0; +} + static uint8_t bitbang_spi_readwrite_byte(uint8_t val) { uint8_t ret = 0; @@ -96,6 +133,11 @@ { int i;
+ /* FIXME: Run bitbang_spi_request_bus here or in programmer init? + * Requesting and releasing the SPI bus is handled in here to allow the + * programmer to use its own SPI engine for native accesses. + */ + bitbang_spi_request_bus(); bitbang_spi_set_cs(0); for (i = 0; i < writecnt; i++) bitbang_spi_readwrite_byte(writearr[i]); @@ -105,6 +147,8 @@ programmer_delay(bitbang_spi_half_period); bitbang_spi_set_cs(1); programmer_delay(bitbang_spi_half_period); + /* FIXME: Run bitbang_spi_release_bus here or in programmer init? */ + bitbang_spi_release_bus();
return 0; } Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/nicintel_spi.c =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/nicintel_spi.c (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/nicintel_spi.c (Arbeitskopie) @@ -91,20 +91,10 @@ { uint32_t tmp;
- /* - * Requesting and releasing the SPI bus is handled in here to allow - * the chipset to use its own SPI engine for native reads. - */ - if (val == 0) - nicintel_request_spibus(); - tmp = pci_mmio_readl(nicintel_spibar + FLA); tmp &= ~(1 << FL_CS); tmp |= (val << FL_CS); pci_mmio_writel(tmp, nicintel_spibar + FLA); - - if (val == 1) - nicintel_release_spibus(); }
static void nicintel_bitbang_set_sck(int val) @@ -142,6 +132,8 @@ .set_sck = nicintel_bitbang_set_sck, .set_mosi = nicintel_bitbang_set_mosi, .get_miso = nicintel_bitbang_get_miso, + .request_bus = nicintel_request_spibus, + .release_bus = nicintel_release_spibus, };
int nicintel_spi_init(void) Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/mcp6x_spi.c =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/mcp6x_spi.c (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/mcp6x_spi.c (Arbeitskopie) @@ -66,18 +66,9 @@
static void mcp6x_bitbang_set_cs(int val) { - /* Requesting and releasing the SPI bus is handled in here to allow the - * chipset to use its own SPI engine for native reads. - */ - if (val == 0) - mcp6x_request_spibus(); - mcp_gpiostate &= ~(1 << MCP6X_SPI_CS); mcp_gpiostate |= (val << MCP6X_SPI_CS); mmio_writeb(mcp_gpiostate, mcp6x_spibar + 0x530); - - if (val == 1) - mcp6x_release_spibus(); }
static void mcp6x_bitbang_set_sck(int val) @@ -106,6 +97,8 @@ .set_sck = mcp6x_bitbang_set_sck, .set_mosi = mcp6x_bitbang_set_mosi, .get_miso = mcp6x_bitbang_get_miso, + .request_bus = mcp6x_request_spibus, + .release_bus = mcp6x_release_spibus, };
int mcp6x_spi_init(int want_spi) Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/programmer.h =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/programmer.h (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/programmer.h (Arbeitskopie) @@ -131,6 +131,8 @@ void (*set_sck) (int val); void (*set_mosi) (int val); int (*get_miso) (void); + void (*request_bus) (void); + void (*release_bus) (void); };
#if CONFIG_INTERNAL == 1 @@ -443,6 +445,7 @@
/* bitbang_spi.c */ int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod); +int bitbang_spi_shutdown(const struct bitbang_spi_master *master); int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int bitbang_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
On Tue, Sep 14, 2010 at 03:59:49AM +0200, Carl-Daniel Hailfinger wrote:
SPI bitbanging on devices which speak SPI natively has a dual-use problem: We need to shut down normal SPI operations to do the bitbanging ourselves. Once we're done, it makes a lot of sense to reenable "normal" SPI operations again. Add request_bus/release_bus functions to struct bitbang_spi_master. Add a bitbang shutdown function (not used yet). Change MCP SPI and Intel NIC SPI to use the new request/release bus infrastructure.
There are multiple possible strategies for bus request/release:
- Request at the start of a SPI command, release immediately afterwards.
- Request at the start of a SPI multicommand, release once all commands
of the multicommand are done.
- Request on programmer init, release on shutdown.
Each strategy has its own advantages. For now, we will stay with the first strategy which worked fine so far.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Uwe Hermann uwe@hermann-uwe.de
int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod) { /* BITBANG_SPI_INVALID is 0, so if someone forgot to initialize ->type, @@ -61,19 +73,44 @@ */ if (!master || master->type == BITBANG_SPI_INVALID || !master->set_cs || !master->set_sck || !master->set_mosi || !master->get_miso) {
msg_perr("Incomplete bitbanging SPI master setting! Please ""report a bug at flashrom@flashrom.org\n");
msg_perr("Incomplete SPI bitbang master setting! ""Please report a bug at flashrom@flashrom.org\n");
Replace "! " with "!\n" please to make the output fit in 80 chars/line. Ditto for a few other msg_* functions below.
Or maybe "Incomplete SPI bitbang master setting, please report a bug!", not sure if we need to mention the email address every time, it's listed in the manpage and also not hard to find out anyway.
Uwe.
On 14.09.2010 14:31, Uwe Hermann wrote:
On Tue, Sep 14, 2010 at 03:59:49AM +0200, Carl-Daniel Hailfinger wrote:
SPI bitbanging on devices which speak SPI natively has a dual-use problem: We need to shut down normal SPI operations to do the bitbanging ourselves. Once we're done, it makes a lot of sense to reenable "normal" SPI operations again. Add request_bus/release_bus functions to struct bitbang_spi_master. Add a bitbang shutdown function (not used yet). Change MCP SPI and Intel NIC SPI to use the new request/release bus infrastructure.
There are multiple possible strategies for bus request/release:
- Request at the start of a SPI command, release immediately afterwards.
- Request at the start of a SPI multicommand, release once all commands
of the multicommand are done.
- Request on programmer init, release on shutdown.
Each strategy has its own advantages. For now, we will stay with the first strategy which worked fine so far.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Uwe Hermann uwe@hermann-uwe.de
int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod) { /* BITBANG_SPI_INVALID is 0, so if someone forgot to initialize ->type, @@ -61,19 +73,44 @@ */ if (!master || master->type == BITBANG_SPI_INVALID || !master->set_cs || !master->set_sck || !master->set_mosi || !master->get_miso) {
msg_perr("Incomplete bitbanging SPI master setting! Please ""report a bug at flashrom@flashrom.org\n");
msg_perr("Incomplete SPI bitbang master setting! ""Please report a bug at flashrom@flashrom.org\n");Replace "! " with "!\n" please to make the output fit in 80 chars/line. Ditto for a few other msg_* functions below.
Or maybe "Incomplete SPI bitbang master setting, please report a bug!", not sure if we need to mention the email address every time, it's listed in the manpage and also not hard to find out anyway.
Thanks for the review. I kept the email address for now. We will have to establish a common guideline for bug reporting instructions, and then make sure the whole codebase adheres to it. New patch below.
SPI bitbanging on devices which speak SPI natively has a dual-use problem: We need to shut down normal SPI operations to do the bitbanging ourselves. Once we're done, it makes a lot of sense to reenable "normal" SPI operations again. Add request_bus/release_bus functions to struct bitbang_spi_master. Add a bitbang shutdown function (not used yet). Change MCP SPI and Intel NIC SPI to use the new request/release bus infrastructure.
There are multiple possible strategies for bus request/release: - Request at the start of a SPI command, release immediately afterwards. - Request at the start of a SPI multicommand, release once all commands of the multicommand are done. - Request on programmer init, release on shutdown. Each strategy has its own advantages. For now, we will stay with the first strategy which worked fine so far.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/bitbang_spi.c =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/bitbang_spi.c (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/bitbang_spi.c (Arbeitskopie) @@ -53,6 +53,18 @@ return bitbang_spi_master->get_miso(); }
+static void bitbang_spi_request_bus(void) +{ + if (bitbang_spi_master->request_bus) + bitbang_spi_master->request_bus(); +} + +static void bitbang_spi_release_bus(void) +{ + if (bitbang_spi_master->release_bus) + bitbang_spi_master->release_bus(); +} + int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod) { /* BITBANG_SPI_INVALID is 0, so if someone forgot to initialize ->type, @@ -61,19 +73,44 @@ */ if (!master || master->type == BITBANG_SPI_INVALID || !master->set_cs || !master->set_sck || !master->set_mosi || !master->get_miso) { - msg_perr("Incomplete bitbanging SPI master setting! Please " - "report a bug at flashrom@flashrom.org\n"); + msg_perr("Incomplete SPI bitbang master setting!\n" + "Please report a bug at flashrom@flashrom.org\n"); return 1; } + if (bitbang_spi_master) { + msg_perr("SPI bitbang master already initialized!\n" + "Please report a bug at flashrom@flashrom.org\n"); + return 1; + } + bitbang_spi_master = master; bitbang_spi_half_period = halfperiod;
+ /* FIXME: Run bitbang_spi_request_bus here or in programmer init? */ bitbang_spi_set_cs(1); bitbang_spi_set_sck(0); bitbang_spi_set_mosi(0); return 0; }
+int bitbang_spi_shutdown(const struct bitbang_spi_master *master) +{ + if (!bitbang_spi_master) { + msg_perr("Shutting down an uninitialized SPI bitbang master!\n" + "Please report a bug at flashrom@flashrom.org\n"); + return 1; + } + if (master != bitbang_spi_master) { + msg_perr("Shutting down a mismatched SPI bitbang master!\n" + "Please report a bug at flashrom@flashrom.org\n"); + return 1; + } + + /* FIXME: Run bitbang_spi_release_bus here or per command? */ + bitbang_spi_master = NULL; + return 0; +} + static uint8_t bitbang_spi_readwrite_byte(uint8_t val) { uint8_t ret = 0; @@ -96,6 +133,11 @@ { int i;
+ /* FIXME: Run bitbang_spi_request_bus here or in programmer init? + * Requesting and releasing the SPI bus is handled in here to allow the + * programmer to use its own SPI engine for native accesses. + */ + bitbang_spi_request_bus(); bitbang_spi_set_cs(0); for (i = 0; i < writecnt; i++) bitbang_spi_readwrite_byte(writearr[i]); @@ -105,6 +147,8 @@ programmer_delay(bitbang_spi_half_period); bitbang_spi_set_cs(1); programmer_delay(bitbang_spi_half_period); + /* FIXME: Run bitbang_spi_release_bus here or in programmer init? */ + bitbang_spi_release_bus();
return 0; } Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/nicintel_spi.c =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/nicintel_spi.c (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/nicintel_spi.c (Arbeitskopie) @@ -91,20 +91,10 @@ { uint32_t tmp;
- /* - * Requesting and releasing the SPI bus is handled in here to allow - * the chipset to use its own SPI engine for native reads. - */ - if (val == 0) - nicintel_request_spibus(); - tmp = pci_mmio_readl(nicintel_spibar + FLA); tmp &= ~(1 << FL_CS); tmp |= (val << FL_CS); pci_mmio_writel(tmp, nicintel_spibar + FLA); - - if (val == 1) - nicintel_release_spibus(); }
static void nicintel_bitbang_set_sck(int val) @@ -142,6 +132,8 @@ .set_sck = nicintel_bitbang_set_sck, .set_mosi = nicintel_bitbang_set_mosi, .get_miso = nicintel_bitbang_get_miso, + .request_bus = nicintel_request_spibus, + .release_bus = nicintel_release_spibus, };
int nicintel_spi_init(void) Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/mcp6x_spi.c =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/mcp6x_spi.c (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/mcp6x_spi.c (Arbeitskopie) @@ -66,18 +66,9 @@
static void mcp6x_bitbang_set_cs(int val) { - /* Requesting and releasing the SPI bus is handled in here to allow the - * chipset to use its own SPI engine for native reads. - */ - if (val == 0) - mcp6x_request_spibus(); - mcp_gpiostate &= ~(1 << MCP6X_SPI_CS); mcp_gpiostate |= (val << MCP6X_SPI_CS); mmio_writeb(mcp_gpiostate, mcp6x_spibar + 0x530); - - if (val == 1) - mcp6x_release_spibus(); }
static void mcp6x_bitbang_set_sck(int val) @@ -106,6 +97,8 @@ .set_sck = mcp6x_bitbang_set_sck, .set_mosi = mcp6x_bitbang_set_mosi, .get_miso = mcp6x_bitbang_get_miso, + .request_bus = mcp6x_request_spibus, + .release_bus = mcp6x_release_spibus, };
int mcp6x_spi_init(int want_spi) Index: flashrom-bitbang_spi_init_errorcheck_request_release_bus/programmer.h =================================================================== --- flashrom-bitbang_spi_init_errorcheck_request_release_bus/programmer.h (Revision 1164) +++ flashrom-bitbang_spi_init_errorcheck_request_release_bus/programmer.h (Arbeitskopie) @@ -131,6 +131,8 @@ void (*set_sck) (int val); void (*set_mosi) (int val); int (*get_miso) (void); + void (*request_bus) (void); + void (*release_bus) (void); };
#if CONFIG_INTERNAL == 1 @@ -443,6 +445,7 @@
/* bitbang_spi.c */ int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod); +int bitbang_spi_shutdown(const struct bitbang_spi_master *master); int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int bitbang_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
On 15.09.2010 02:10, Carl-Daniel Hailfinger wrote:
On 14.09.2010 14:31, Uwe Hermann wrote:
Acked-by: Uwe Hermann uwe@hermann-uwe.de
SPI bitbanging on devices which speak SPI natively has a dual-use problem: We need to shut down normal SPI operations to do the bitbanging ourselves. Once we're done, it makes a lot of sense to reenable "normal" SPI operations again. Add request_bus/release_bus functions to struct bitbang_spi_master. Add a bitbang shutdown function (not used yet). Change MCP SPI and Intel NIC SPI to use the new request/release bus infrastructure.
There are multiple possible strategies for bus request/release:
- Request at the start of a SPI command, release immediately afterwards.
- Request at the start of a SPI multicommand, release once all commands
of the multicommand are done.
- Request on programmer init, release on shutdown.
Each strategy has its own advantages. For now, we will stay with the first strategy which worked fine so far.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
I reused your Ack.
Acked-by: Uwe Hermann uwe@hermann-uwe.de
Thanks, committed in r1171.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Uwe Hermann