Re: [flashrom] [PATCH] kill central list of SPI programmers

[Carl-Daniel Hailfinger]

Auf 18.07.2010 17:20, Michael Karcher schrieb:
> This version really compiles. Forgot the forward declaration after
> moving code initially.
>
> Signed-off-by: Michael Karcher

I like most ideas in there, and have some minor nitpicks.

Using a registration function would allow us to handle multiple SPI
programmers on one board (e.g. one in the EC and one in the
southbridge), but some infrastructure changes elsewhere would be needed.
We need that to handle stuff like the Google Cr-48 netbook sanely.
One of my post-0.9.4 goals is to register programmer drivers for every
bus, not just SPI. The idea is to be able to handle multiple flash chips
behind LPC as well, e.g. if they have different base addresses. And
stuff like switching between Dual BIOS chips with a GPIO line could be
implemented under that scheme as well.
> diff --git a/buspirate_spi.c b/buspirate_spi.c
> index 55e71c2..36eaa9a 100644
> --- a/buspirate_spi.c
> +++ b/buspirate_spi.c
> @@ -44,6 +44,8 @@ static int buspirate_serialport_setup(char *dev)
>  #define sp_flush_incoming(...) 0
>  #endif
>
> +static const struct spi_programmer spi_programmer_buspirate;
>
> static int buspirate_sendrecv(unsigned char *buf, unsigned int writecnt, unsigned int readcnt)
>  {
>    int i, ret = 0;
> @@ -210,7 +212,7 @@ int buspirate_spi_init(void)
>    }
>
> buses_supported = CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_BUSPIRATE;
> spi_programmer = &spi_programmer_buspirate;

You assign the spi_programmer variable directly. While that is a
straightforward replacement of the current code, it would be cool if you
could use a registration function instead:
register_spi_programmer(&spi_programmer_buspirate);
Depending on whether modification of the programmer-specific info is
desired, it may also be possible to have that registration function copy
the struct so later calls can make changes even if the original
parameter is const.
> return 0;
>  }
> @@ -250,7 +252,7 @@ int buspirate_spi_shutdown(void)
>    return 0;
>  }
>
> -int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> +static int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    	const unsigned char *writearr, unsigned char *readarr)
>  {
>    static unsigned char *buf = NULL;
> @@ -304,12 +306,20 @@ int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    return ret;
>  }
>
> -int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    return spi_read_chunked(flash, buf, start, len, 12);
>  }
>
> -int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    return spi_write_chunked(flash, buf, start, len, 12);
>  }
>
> +static const struct spi_programmer spi_programmer_buspirate = {
> .type = SPI_CONTROLLER_BUSPIRATE,
> .command = buspirate_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = buspirate_spi_read,
> .write_256 = buspirate_spi_write_256,
>
> +};
> diff --git a/dediprog.c b/dediprog.c
> index 0b92983..c21c801 100644
> --- a/dediprog.c
> +++ b/dediprog.c
> @@ -25,6 +25,7 @@
>
> #define DEFAULT_TIMEOUT 3000
>  static usb_dev_handle *dediprog_handle;
> +static const struct spi_programmer spi_programmer_dediprog;
>
> #if 0
>  /* Might be useful for other pieces of code as well. */
> @@ -145,14 +146,14 @@ static int dediprog_set_spi_speed(uint16_t speed)
>  }
>  #endif
>
> -int dediprog_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int dediprog_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    msg_pspew("%s, start=0x%x, len=0x%x\n", __func__, start, len);
>    /* Chosen read length is 16 bytes for now. */
>    return spi_read_chunked(flash, buf, start, len, 16);
>  }
>
> -int dediprog_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> +static int dediprog_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    		const unsigned char *writearr, unsigned char *readarr)
>  {
>    int ret;
> @@ -318,7 +319,7 @@ int dediprog_init(void)
>    	return 1;
>
> buses_supported = CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_DEDIPROG;
> spi_programmer = &spi_programmer_dediprog;

Registration function. Same for all subsequent occurences.
> /* RE leftover, leave in until the driver is complete. */
>  #if 0
> @@ -394,3 +395,11 @@ int dediprog_shutdown(void)
>    }
>    return 0;
>  }
>
> +static const struct spi_programmer spi_programmer_dediprog = {
> .type = SPI_CONTROLLER_DEDIPROG,
> .command = dediprog_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = dediprog_spi_read,
> .write_256 = spi_chip_write_1_new,
>
> +};
> diff --git a/dummyflasher.c b/dummyflasher.c
> index 241dcee..b1bbcf2 100644
> --- a/dummyflasher.c
> +++ b/dummyflasher.c
> @@ -25,6 +25,8 @@
>  #include "flash.h"
>  #include "chipdrivers.h"
>
> +static const struct spi_programmer spi_programmer_dummy;
>
> static void tolower_string(char *str)
>  {
>    for (; *str != '\0'; str++)
> @@ -59,7 +61,7 @@ int dummy_init(void)
>    }
>    if (strstr(bustext, "spi")) {
>    	buses_supported |= CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_DUMMY;
> spi_programmer = &spi_programmer_dummy;
> msg_pdbg("Enabling support for %s flash.\n", "SPI");
> }
> if (buses_supported == CHIP_BUSTYPE_NONE)
>
> @@ -140,7 +142,7 @@ void dummy_chip_readn(uint8_t *buf, const chipaddr addr, size_t len)
>    return;
>  }
>
> -int dummy_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> +static int dummy_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    	      const unsigned char *writearr, unsigned char *readarr)
>  {
>    int i;
> @@ -161,7 +163,7 @@ int dummy_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    return 0;
>  }
>
> -int dummy_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int dummy_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    /* Maximum read length is unlimited, use 64kB. */
>    return spi_read_chunked(flash, buf, start, len, 64 * 1024);
> @@ -171,7 +173,15 @@ int dummy_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> never be successful, and the current frontend refuses to write in that case.
> Other frontends may allow writing even for non-detected chips, though.
>
> */
> -int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    return spi_write_chunked(flash, buf, start, len, 256);
>  }
>
> +static const struct spi_programmer spi_programmer_dummy	= {
> .type = SPI_CONTROLLER_DUMMY,
> .command = dummy_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = dummy_spi_read,
> .write_256 = dummy_spi_write_256,
>
> +};
> diff --git a/flash.h b/flash.h
> index 8ebea29..fb06207 100644
> --- a/flash.h
> +++ b/flash.h
> @@ -452,10 +452,6 @@ uint8_t dummy_chip_readb(const chipaddr addr);
>  uint16_t dummy_chip_readw(const chipaddr addr);
>  uint32_t dummy_chip_readl(const chipaddr addr);
>  void dummy_chip_readn(uint8_t *buf, const chipaddr addr, size_t len);
> -int dummy_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>       const unsigned char *writearr, unsigned char *readarr);
>
> -int dummy_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
> -int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
>  #endif
>
> /* nic3com.c */
> @@ -527,9 +523,6 @@ extern const struct pcidev_status ata_hpt[];
>  #define FTDI_FT2232H 0x6010
>  #define FTDI_FT4232H 0x6011
>  int ft2232_spi_init(void);
> -int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
> -int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
> -int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
>
> /* bitbang_spi.c */
>  int bitbang_spi_init(const struct bitbang_spi_master *master, int halfperiod);
> @@ -544,15 +537,10 @@ struct buspirate_spispeeds {
>  };
>  int buspirate_spi_init(void);
>  int buspirate_spi_shutdown(void);
> -int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
> -int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
> -int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
>
> /* dediprog.c */
>  int dediprog_init(void);
>  int dediprog_shutdown(void);
> -int dediprog_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
> -int dediprog_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
>
> /* flashrom.c */
>  enum write_granularity {
> @@ -627,32 +615,18 @@ int handle_romentries(uint8_t *buffer, struct flashchip *flash);
>
> /* spi.c */
>  enum spi_controller {
> SPI_CONTROLLER_NONE,
>
> -#if CONFIG_INTERNAL == 1
> -#if defined(__i386__) || defined(__x86_64__)
>    SPI_CONTROLLER_ICH7,
>    SPI_CONTROLLER_ICH9,
>    SPI_CONTROLLER_IT87XX,
>    SPI_CONTROLLER_SB600,
>    SPI_CONTROLLER_VIA,
>    SPI_CONTROLLER_WBSIO,
> -#endif
> -#endif
> -#if CONFIG_FT2232_SPI == 1
>    SPI_CONTROLLER_FT2232,
> -#endif
> -#if CONFIG_DUMMY == 1
>    SPI_CONTROLLER_DUMMY,
> -#endif
> -#if CONFIG_BUSPIRATE_SPI == 1
>    SPI_CONTROLLER_BUSPIRATE,
> -#endif
> -#if CONFIG_DEDIPROG == 1
>    SPI_CONTROLLER_DEDIPROG,
> -#endif
> SPI_CONTROLLER_INVALID /* This must always be the last entry. */

Could you leave the #ifdefs in place for now?
> };
> -extern const int spi_programmer_count;
>
> struct spi_command {
>    unsigned int writecnt;
>    unsigned int readcnt;
> @@ -660,6 +634,7 @@ struct spi_command {
>    unsigned char *readarr;
>  };
>  struct spi_programmer {
> enum spi_controller type;
> int (*command)(unsigned int writecnt, unsigned int readcnt,
>   const unsigned char *writearr, unsigned char *readarr);
> int (*multicommand)(struct spi_command *cmds);
>
> @@ -669,8 +644,8 @@ struct spi_programmer {
>    int (*write_256)(struct flashchip *flash, uint8_t *buf, int start, int len);
>  };
>
> -extern enum spi_controller spi_controller;
> -extern const struct spi_programmer spi_programmer[];
> +const struct spi_programmer *spi_programmer;
>
> int spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    	const unsigned char *writearr, unsigned char *readarr);
>  int spi_send_multicommand(struct spi_command *cmds);
> @@ -684,11 +659,6 @@ extern uint32_t ichspi_bbar;
>  void ich_init_spi(struct pci_dev *dev, uint32_t base, void * rcrb, 
>                    int ich_generation);
>  int via_init_spi(struct pci_dev * dev);
> -int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>     const unsigned char *writearr, unsigned char *readarr);
>
> -int ich_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
> -int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len);
> -int ich_spi_send_multicommand(struct spi_command *cmds);
>
> /* it87spi.c */
>  void enter_conf_mode_ite(uint16_t port);
> @@ -696,23 +666,12 @@ void exit_conf_mode_ite(uint16_t port);
>  struct superio probe_superio_ite(void);
>  int init_superio_ite(void);
>  int it87spi_init(void);
> -int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> 	const unsigned char *writearr, unsigned char *readarr);
>
> -int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len);
> -int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
>
> /* sb600spi.c */
>  void sb600_probe_spi(struct pci_dev * dev);
> -int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>       const unsigned char *writearr, unsigned char *readarr);
>
> -int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
> -int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
>
> /* wbsio_spi.c */
>  int wbsio_check_for_spi(void);
> -int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>       const unsigned char *writearr, unsigned char *readarr);
>
> -int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
>
> /* serprog.c */
>  int serprog_init(void);
> diff --git a/flashrom.c b/flashrom.c
> index fe173a7..dc262d5 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -1354,10 +1354,6 @@ int selfcheck(void)
>    	msg_gerr("Programmer table miscompilation!\n");
>    	ret = 1;
>    }
> if (spi_programmer_count - 1 != SPI_CONTROLLER_INVALID) {
> msg_gerr("SPI programmer table miscompilation!\n");
> ret = 1;
> }
> for (flash = flashchips; flash && flash->name; flash++)
> if (selfcheck_eraseblocks(flash))
> 	ret = 1;
>
> diff --git a/ft2232_spi.c b/ft2232_spi.c
> index c8711e9..b45ba42 100644
> --- a/ft2232_spi.c
> +++ b/ft2232_spi.c
> @@ -45,6 +45,7 @@
>  #define BITMODE_BITBANG_NORMAL	1
>  #define BITMODE_BITBANG_SPI	2
>
> +static const struct spi_programmer spi_programmer_ft2232;
>  static struct ftdi_context ftdic_context;
>
> static int send_buf(struct ftdi_context *ftdic, const unsigned char *buf, int size)
> @@ -192,7 +193,7 @@ int ft2232_spi_init(void)
>    // msg_pdbg("\nft2232 chosen\n");
>
> buses_supported = CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_FT2232;
>
> spi_programmer = &spi_programmer_ft2232;
>
> return 0;
>
> }
> @@ -293,4 +294,12 @@ int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int l
>    return spi_write_chunked(flash, buf, start, len, 256);
>  }
>
> +static const struct spi_programmer spi_programmer_ft2232 = {
> .type = SPI_CONTROLLER_FT2232,
> .command = ft2232_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = ft2232_spi_read,
> .write_256 = ft2232_spi_write_256,
>
> +};
>
> #endif
> diff --git a/ichspi.c b/ichspi.c
> index 246f3ae..ae9a5f4 100644
> --- a/ichspi.c
> +++ b/ichspi.c
> @@ -106,6 +106,9 @@ static int ichspi_lock = 0;
>  uint32_t ichspi_bbar = 0;
>
> static void *ich_spibar = NULL;
> +static const struct spi_programmer spi_programmer_ich7;
> +static const struct spi_programmer spi_programmer_ich9;
> +static const struct spi_programmer spi_programmer_via;
>
> typedef struct _OPCODE {
>    uint8_t opcode;		//This commands spi opcode
> @@ -234,7 +237,7 @@ static int generate_opcodes(OPCODES * op)
>    	return -1;
>    }
> switch (spi_controller) {
> switch (spi_programmer->type) {

We need to decide whether we want to access spi_programmer directly. It
works in your patch, but if we go the route towards multiple SPI
programmers on one board, we have to be sure to access the correct
spi_programmer (maybe array member, maybe something else).
If you have any neat ideas, please come forward. Otherwise I'd say your
code is a good solution for now, and once we add support for registering
multiple programmers, we'll notice this location because compilation
will fail.
> case SPI_CONTROLLER_ICH7:
>    case SPI_CONTROLLER_VIA:
>    	preop = REGREAD16(ICH7_REG_PREOP);
> @@ -309,7 +312,7 @@ int program_opcodes(OPCODES * op)
>    }
>
> msg_pdbg("\n%s: preop=%04x optype=%04x opmenu=%08x%08x\n", __func__, preop, optype, opmenu[0], opmenu[1]);
> switch (spi_controller) {
> switch (spi_programmer->type) {
> case SPI_CONTROLLER_ICH7:
> case SPI_CONTROLLER_VIA:
> REGWRITE16(ICH7_REG_PREOP, preop);
>
> @@ -337,7 +340,7 @@ int program_opcodes(OPCODES * op)
>   */
>  void ich_set_bbar(uint32_t minaddr)
>  {
> switch (spi_controller) {
> switch (spi_programmer->type) {
> case SPI_CONTROLLER_ICH7:
> mmio_writel(minaddr, ich_spibar + 0x50);
> ichspi_bbar = mmio_readl(ich_spibar + 0x50);
>
> @@ -448,19 +451,19 @@ void ich_init_spi(struct pci_dev *dev, uint32_t base, void * rcrb,
>    switch (ich_generation) {
>    case 7:
>    	buses_supported = CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_ICH7;
> spi_programmer = &spi_programmer_ich7;
> spibar_offset = 0x3020;
> break;
> case 8:
> buses_supported = CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_ICH9;
> spi_programmer = &spi_programmer_ich9;
> spibar_offset = 0x3020;
> break;
> case 9:
> case 10:
> default:		/* Future version might behave the same */
> buses_supported = CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_ICH9;
> spi_programmer = &spi_programmer_ich9;
> spibar_offset = 0x3800;
> break;
> }
>
> @@ -471,7 +474,7 @@ void ich_init_spi(struct pci_dev *dev, uint32_t base, void * rcrb,
>    /* Assign Virtual Address */
>    ich_spibar = rcrb + spibar_offset;
> switch (spi_controller) {
> switch (spi_programmer->type) {
> case SPI_CONTROLLER_ICH7:
> msg_pdbg("0x00: 0x%04x     (SPIS)\n",
> 	     mmio_readw(ich_spibar + 0));
>
> @@ -600,7 +603,7 @@ int via_init_spi(struct pci_dev * dev)
>
> /* Not sure if it speaks all these bus protocols. */
>    buses_supported = CHIP_BUSTYPE_LPC | CHIP_BUSTYPE_FWH | CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_VIA;
>
> spi_programmer = &spi_programmer_via;
> ich_init_opcodes();
>
> return 0;
>
> @@ -848,7 +851,7 @@ static int ich9_run_opcode(OPCODE op, uint32_t offset,
>  static int run_opcode(OPCODE op, uint32_t offset,
>    	      uint8_t datalength, uint8_t * data)
>  {
> switch (spi_controller) {
> switch (spi_programmer->type) {
> case SPI_CONTROLLER_VIA:
> if (datalength > 16) {
> 	msg_perr("%s: Internal command size error for "
>
> @@ -881,27 +884,27 @@ static int run_opcode(OPCODE op, uint32_t offset,
>    return -1;
>  }
>
> -int ich_spi_read(struct flashchip *flash, uint8_t * buf, int start, int len)
> +static int ich_spi_read(struct flashchip *flash, uint8_t * buf, int start, int len)
>  {
>    int maxdata = 64;
> if (spi_controller == SPI_CONTROLLER_VIA)
>
> if (spi_programmer->type == SPI_CONTROLLER_VIA)
> maxdata = 16;
>
> return spi_read_chunked(flash, buf, start, len, maxdata);
>
> }
>
> -int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len)
> +static int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len)
>  {
>    int maxdata = 64;
> if (spi_controller == SPI_CONTROLLER_VIA)
>
> if (spi_programmer->type == SPI_CONTROLLER_VIA)
> maxdata = 16;
>
> return spi_write_chunked(flash, buf, start, len, maxdata);
>
> }
>
> -int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> +static int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    	    const unsigned char *writearr, unsigned char *readarr)
>  {
>    int result;
> @@ -966,7 +969,7 @@ int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>        opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) {
>    	addr = (writearr[1] << 16) |
>    	    (writearr[2] << 8) | (writearr[3] << 0);
> switch (spi_controller) {
> switch (spi_programmer->type) {
> case SPI_CONTROLLER_ICH7:
> case SPI_CONTROLLER_ICH9:
> 	if (addr < ichspi_bbar) {
>
> @@ -1001,7 +1004,7 @@ int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    return result;
>  }
>
> -int ich_spi_send_multicommand(struct spi_command *cmds)
> +static int ich_spi_send_multicommand(struct spi_command *cmds)
>  {
>    int ret = 0;
>    int i;
> @@ -1071,4 +1074,28 @@ int ich_spi_send_multicommand(struct spi_command *cmds)
>    return ret;
>  }
>
> +static const struct spi_programmer spi_programmer_ich7 = {
>    .type = SPI_CONTROLLER_ICH7,
> .command = ich_spi_send_command,
> .multicommand = ich_spi_send_multicommand,
> .read = ich_spi_read,
> .write_256 = ich_spi_write_256,
>
> +};
>
> +static const struct spi_programmer spi_programmer_ich9 = {
> .type = SPI_CONTROLLER_ICH9,
> .command = ich_spi_send_command,
> .multicommand = ich_spi_send_multicommand,
> .read = ich_spi_read,
> .write_256 = ich_spi_write_256,
>
> +};
>
> +static const struct spi_programmer spi_programmer_via = {
> .type = SPI_CONTROLLER_VIA,
> .command = ich_spi_send_command,
> .multicommand = ich_spi_send_multicommand,
> .read = ich_spi_read,
> .write_256 = ich_spi_write_256,
>
> +};
>
> #endif
> diff --git a/it87spi.c b/it87spi.c
> index cf3d3b9..dddb5a9 100644
> --- a/it87spi.c
> +++ b/it87spi.c
> @@ -34,6 +34,8 @@
>  #define ITE_SUPERIO_PORT1	0x2e
>  #define ITE_SUPERIO_PORT2	0x4e
>
> +static const struct spi_programmer spi_programmer_it87xx;
>
> uint16_t it8716f_flashport = 0;
>  /* use fast 33MHz SPI (<>0) or slow 16MHz (0) */
>  static int fast_spi = 1;
> @@ -175,7 +177,7 @@ static uint16_t it87spi_probe(uint16_t port)
>    it8716f_flashport = flashport;
>    if (buses_supported & CHIP_BUSTYPE_SPI)
>    	msg_pdbg("Overriding chipset SPI with IT87 SPI.\n");
> spi_controller = SPI_CONTROLLER_IT87XX;
> spi_programmer = &spi_programmer_it87xx;
> buses_supported |= CHIP_BUSTYPE_SPI;
> return 0;
>
> }
> @@ -227,7 +229,7 @@ int it87spi_init(void)
> commands with the address in inverse wire order. That's why the register
> ordering in case 4 and 5 may seem strange.
>
> */
> -int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> +static int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    		const unsigned char *writearr, unsigned char *readarr)
>  {
>    uint8_t busy, writeenc;
> @@ -320,7 +322,7 @@ static int it8716f_spi_page_program(struct flashchip *flash, uint8_t *buf, int s
> IT8716F only allows maximum of 512 kb SPI mapped to LPC memory cycles
> Need to read this big flash using firmware cycles 3 byte at a time.
>
> */
> -int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    int total_size = 1024 * flash->total_size;
>    fast_spi = 0;
> @@ -334,7 +336,7 @@ int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int
>    return 0;
>  }
>
> -int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    /*
>     * IT8716F only allows maximum of 512 kb SPI chip size for memory
> @@ -371,4 +373,12 @@ int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start,
>    return 0;
>  }
>
> +static const struct spi_programmer spi_programmer_it87xx = {
> .type = SPI_CONTROLLER_IT87XX,
> .command = it8716f_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = it8716f_spi_chip_read,
> .write_256 = it8716f_spi_chip_write_256,
>
> +};
>
> #endif
> diff --git a/sb600spi.c b/sb600spi.c
> index e42b20b..866e309 100644
> --- a/sb600spi.c
> +++ b/sb600spi.c
> @@ -41,6 +41,7 @@
>   */
>
> static uint8_t *sb600_spibar = NULL;
> +static const struct spi_programmer spi_programmer_sb600;
>
> void sb600_probe_spi(struct pci_dev * dev)
>  {
> @@ -114,16 +115,16 @@ void sb600_probe_spi(struct pci_dev * dev)
>    	return;
>
> buses_supported |= CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_SB600;
> spi_programmer = &spi_programmer_sb600;
>
> }
>
> -int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    /* Maximum read length is 8 bytes. */
>    return spi_read_chunked(flash, buf, start, len, 8);
>  }
>
> -int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    return spi_write_chunked(flash, buf, start, len, 5);
>  }
> @@ -144,7 +145,7 @@ static void execute_command(void)
>    	;
>  }
>
> -int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> +static int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    	      const unsigned char *writearr, unsigned char *readarr)
>  {
>    int count;
> @@ -227,4 +228,12 @@ int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    return 0;
>  }
>
> +static const struct spi_programmer spi_programmer_sb600 = {
> .type = SPI_CONTROLLER_SB600,
> .command = sb600_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = sb600_spi_read,
> .write_256 = sb600_spi_write_256,
>
> +};
>
> #endif
> diff --git a/spi.c b/spi.c
> index 2b64463..dc4da9f 100644
> --- a/spi.c
> +++ b/spi.c
> @@ -27,129 +27,34 @@
>  #include "chipdrivers.h"
>  #include "spi.h"
>
> -enum spi_controller spi_controller = SPI_CONTROLLER_NONE;
>
> void spi_prettyprint_status_register(struct flashchip *flash);
>
> -const struct spi_programmer spi_programmer[] = {
> { /* SPI_CONTROLLER_NONE */
> .command = NULL,
> .multicommand = NULL,
> .read = NULL,
> .write_256 = NULL,
> },
>
> -#if CONFIG_INTERNAL == 1
> -#if defined(__i386__) || defined(__x86_64__)
> { /* SPI_CONTROLLER_ICH7 */
> .command = ich_spi_send_command,
> .multicommand = ich_spi_send_multicommand,
> .read = ich_spi_read,
> .write_256 = ich_spi_write_256,
> },
> { /* SPI_CONTROLLER_ICH9 */
> .command = ich_spi_send_command,
> .multicommand = ich_spi_send_multicommand,
> .read = ich_spi_read,
> .write_256 = ich_spi_write_256,
> },
> { /* SPI_CONTROLLER_IT87XX */
> .command = it8716f_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = it8716f_spi_chip_read,
> .write_256 = it8716f_spi_chip_write_256,
> },
> { /* SPI_CONTROLLER_SB600 */
> .command = sb600_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = sb600_spi_read,
> .write_256 = sb600_spi_write_256,
> },
> { /* SPI_CONTROLLER_VIA */
> .command = ich_spi_send_command,
> .multicommand = ich_spi_send_multicommand,
> .read = ich_spi_read,
> .write_256 = ich_spi_write_256,
> },
> { /* SPI_CONTROLLER_WBSIO */
> .command = wbsio_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = wbsio_spi_read,
> .write_256 = spi_chip_write_1_new,
> },
>
> -#endif
> -#endif
>
> -#if CONFIG_FT2232_SPI == 1
> { /* SPI_CONTROLLER_FT2232 */
> .command = ft2232_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = ft2232_spi_read,
> .write_256 = ft2232_spi_write_256,
> },
>
> -#endif
>
> -#if CONFIG_DUMMY == 1
> { /* SPI_CONTROLLER_DUMMY */
> .command = dummy_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = dummy_spi_read,
> .write_256 = dummy_spi_write_256,
> },
>
> -#endif
>
> -#if CONFIG_BUSPIRATE_SPI == 1
> { /* SPI_CONTROLLER_BUSPIRATE */
> .command = buspirate_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = buspirate_spi_read,
> .write_256 = buspirate_spi_write_256,
> },
>
> -#endif
>
> -#if CONFIG_DEDIPROG == 1
> { /* SPI_CONTROLLER_DEDIPROG */
> .command = dediprog_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = dediprog_spi_read,
> .write_256 = spi_chip_write_1_new,
> },
>
> -#endif
> {}, /* This entry corresponds to SPI_CONTROLLER_INVALID. */
>
> -};
>
> -const int spi_programmer_count = ARRAY_SIZE(spi_programmer);
> +const struct spi_programmer *spi_programmer;

If someone forces a SPI access on a programmer which does not support
SPI, this will explode AFAICS because it means following an
uninitialized pointer to check for NULL in a struct member which does
not exist.
> int spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    	const unsigned char *writearr, unsigned char *readarr)
>  {
> if (!spi_programmer[spi_controller].command) {
> if (!spi_programmer->command) {
> msg_perr("%s called, but SPI is unsupported on this "
> 	 "hardware. Please report a bug at "
> 	 "[e-mail]\n", __func__);
> return 1;
> }
> return spi_programmer[spi_controller].command(writecnt, readcnt,
> return spi_programmer->command(writecnt, readcnt,
> 				      writearr, readarr);
>
> }
>
> int spi_send_multicommand(struct spi_command *cmds)
>  {
> if (!spi_programmer[spi_controller].multicommand) {
> if (!spi_programmer->multicommand) {
> msg_perr("%s called, but SPI is unsupported on this "
> 	 "hardware. Please report a bug at "
> 	 "[e-mail]\n", __func__);
> return 1;
> }
> return spi_programmer[spi_controller].multicommand(cmds);
> return spi_programmer->multicommand(cmds);
>
> }
>
> int default_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> @@ -183,14 +88,14 @@ int default_spi_send_multicommand(struct spi_command *cmds)
>
> int spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
> if (!spi_programmer[spi_controller].read) {
> if (!spi_programmer->read) {
> msg_perr("%s called, but SPI read is unsupported on this "
> 	 "hardware. Please report a bug at "
> 	 "[e-mail]\n", __func__);
> return 1;
> }
> return spi_programmer[spi_controller].read(flash, buf, start, len);
> return spi_programmer->read(flash, buf, start, len);
>
> }
>
> /*
> @@ -202,14 +107,14 @@ int spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  /* real chunksize is up to 256, logical chunksize is 256 */
>  int spi_chip_write_256_new(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
> if (!spi_programmer[spi_controller].write_256) {
> if (!spi_programmer->write_256) {
> msg_perr("%s called, but SPI page write is unsupported on this "
> 	 "hardware. Please report a bug at "
> 	 "[e-mail]\n", __func__);
> return 1;
> }
> return spi_programmer[spi_controller].write_256(flash, buf, start, len);
> return spi_programmer->write_256(flash, buf, start, len);
>
> }
>
> /* Wrapper function until the generic code is converted to partial writes. */
> @@ -239,7 +144,7 @@ int spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
>   */
>  uint32_t spi_get_valid_read_addr(void)
>  {
> switch (spi_controller) {
> switch (spi_programmer->type) {
>
> #if CONFIG_INTERNAL == 1
>  #if defined(__i386__) || defined(__x86_64__)
>    case SPI_CONTROLLER_ICH7:
> diff --git a/spi25.c b/spi25.c
> index 32bb73c..d9dae35 100644
> --- a/spi25.c
> +++ b/spi25.c
> @@ -177,7 +177,7 @@ int probe_spi_rdid4(struct flashchip *flash)
>    /* Some SPI controllers do not support commands with writecnt=1 and
>     * readcnt=4.
>     */
> switch (spi_controller) {
> switch (spi_programmer->type) {
>
> #if CONFIG_INTERNAL == 1
>  #if defined(__i386__) || defined(__x86_64__)
>    case SPI_CONTROLLER_IT87XX:
> @@ -1019,7 +1019,7 @@ int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len)
>    	.readarr	= NULL,
>    }};
> switch (spi_controller) {
> switch (spi_programmer->type) {
>
> #if CONFIG_INTERNAL == 1
>  #if defined(__i386__) || defined(__x86_64__)
>    case SPI_CONTROLLER_IT87XX:
> diff --git a/wbsio_spi.c b/wbsio_spi.c
> index b8f8b38..6288340 100644
> --- a/wbsio_spi.c
> +++ b/wbsio_spi.c
> @@ -28,6 +28,7 @@
>  #define WBSIO_PORT2	0x4e
>
> static uint16_t wbsio_spibase = 0;
> +static const struct spi_programmer spi_programmer_wbsio;
>
> static uint16_t wbsio_get_spibase(uint16_t port)
>  {
> @@ -68,7 +69,7 @@ int wbsio_check_for_spi(void)
>    msg_pspew("\nwbsio_spibase = 0x%x\n", wbsio_spibase);
>
> buses_supported |= CHIP_BUSTYPE_SPI;
> spi_controller = SPI_CONTROLLER_WBSIO;
> spi_programmer = &spi_programmer_wbsio;
> msg_pdbg("%s: Winbond saved on 4 register bits so max chip size is "
> "1024 KB!\n", __func__);
> max_rom_decode.spi = 1024 * 1024;
>
> @@ -96,7 +97,7 @@ int wbsio_check_for_spi(void)
> Would one more byte of RAM in the chip (to get all 24 bits) really make
> such a big difference?
>
> */
> -int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt,
> +static int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    	      const unsigned char *writearr, unsigned char *readarr)
>  {
>    int i;
> @@ -180,9 +181,18 @@ int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt,
>    return 0;
>  }
>
> -int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> +static int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>    return read_memmapped(flash, buf, start, len);
>  }
>
> +static const struct spi_programmer spi_programmer_wbsio = {
> .type = SPI_CONTROLLER_WBSIO,
> .command = wbsio_spi_send_command,
> .multicommand = default_spi_send_multicommand,
> .read = wbsio_spi_read,
> .write_256 = spi_chip_write_1_new,
>
> +};
>
> #endif

Looks good otherwise.

Regards,
Carl-Daniel