[flashrom] [PATCH] Let the programmer driver decide how to do AAI transfers

[Carl-Daniel Hailfinger]

Hi Nico,

thanks for your patch.

Am 13.06.2012 11:21 schrieb Nico Huber:
> Currently spi_aai_write() is implemented without an abstraction mechanism
> for the programmer driver. This adds another function pointer 'write_aai'
> to struct spi_programmer, which is set to default_spi_write_aai (renamed
> spi_aai_write) for all programmers for now.
>
> A patch which utilises this abstraction in the dediprog driver will
> follow.

This abstraction makes sense.


> Signed-off-by: Nico Huber <nico.huber at secunet.com>

Looks pretty good, I think we can commit the next iteration straight away.


> Index: it87spi.c
> ===================================================================
> --- it87spi.c	(Revision 1541)
> +++ it87spi.c	(Arbeitskopie)
> @@ -120,6 +120,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= it8716f_spi_chip_read,
>  	.write_256	= it8716f_spi_chip_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  static uint16_t it87spi_probe(uint16_t port)
> Index: bitbang_spi.c
> ===================================================================
> --- bitbang_spi.c	(Revision 1541)
> +++ bitbang_spi.c	(Arbeitskopie)
> @@ -71,6 +71,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= default_spi_read,
>  	.write_256	= default_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  #if 0 // until it is needed
> Index: serprog.c
> ===================================================================
> --- serprog.c	(Revision 1541)
> +++ serprog.c	(Arbeitskopie)
> @@ -313,6 +313,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= serprog_spi_read,
>  	.write_256	= default_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  static void serprog_chip_writeb(const struct flashctx *flash, uint8_t val,
> Index: dummyflasher.c
> ===================================================================
> --- dummyflasher.c	(Revision 1541)
> +++ dummyflasher.c	(Arbeitskopie)
> @@ -127,6 +127,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= default_spi_read,
>  	.write_256	= dummy_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  static const struct par_programmer par_programmer_dummy = {
> Index: ichspi.c
> ===================================================================
> --- ichspi.c	(Revision 1541)
> +++ ichspi.c	(Arbeitskopie)
> @@ -1521,6 +1521,7 @@
>  	.multicommand = ich_spi_send_multicommand,
>  	.read = default_spi_read,
>  	.write_256 = default_spi_write_256,
> +	.write_aai = default_spi_write_aai,
>  };
>  
>  static const struct spi_programmer spi_programmer_ich9 = {
> @@ -1531,6 +1532,7 @@
>  	.multicommand = ich_spi_send_multicommand,
>  	.read = default_spi_read,
>  	.write_256 = default_spi_write_256,
> +	.write_aai = default_spi_write_aai,
>  };
>  
>  static const struct opaque_programmer opaque_programmer_ich_hwseq = {
> @@ -1838,6 +1840,7 @@
>  	.multicommand = ich_spi_send_multicommand,
>  	.read = default_spi_read,
>  	.write_256 = default_spi_write_256,
> +	.write_aai = default_spi_write_aai,
>  };
>  
>  int via_init_spi(struct pci_dev *dev)
> Index: dediprog.c
> ===================================================================
> --- dediprog.c	(Revision 1541)
> +++ dediprog.c	(Arbeitskopie)
> @@ -709,6 +709,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= dediprog_spi_read,
>  	.write_256	= dediprog_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  static int dediprog_shutdown(void *data)
> Index: spi25.c
> ===================================================================
> --- spi25.c	(Revision 1541)
> +++ spi25.c	(Arbeitskopie)
> @@ -1069,8 +1069,8 @@
>  	return 0;
>  }
>  
> -int spi_aai_write(struct flashctx *flash, uint8_t *buf, unsigned int start,
> -		  unsigned int len)
> +int default_spi_write_aai(struct flashctx *flash, uint8_t *buf,
> +			  unsigned int start, unsigned int len)

Minor formatting nitpick:
The newly introduced 112 column limit makes it possible to keep this on
one line.


>  {
>  	uint32_t pos = start;
>  	int result;
> Index: it85spi.c
> ===================================================================
> --- it85spi.c	(Revision 1541)
> +++ it85spi.c	(Arbeitskopie)
> @@ -283,6 +283,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= default_spi_read,
>  	.write_256	= default_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  int it85xx_spi_init(struct superio s)
> Index: buspirate_spi.c
> ===================================================================
> --- buspirate_spi.c	(Revision 1541)
> +++ buspirate_spi.c	(Arbeitskopie)
> @@ -124,6 +124,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= default_spi_read,
>  	.write_256	= default_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  static const struct buspirate_spispeeds spispeeds[] = {
> Index: linux_spi.c
> ===================================================================
> --- linux_spi.c	(Revision 1541)
> +++ linux_spi.c	(Arbeitskopie)
> @@ -54,6 +54,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= linux_spi_read,
>  	.write_256	= linux_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  int linux_spi_init(void)
> Index: spi.c
> ===================================================================
> --- spi.c	(Revision 1541)
> +++ spi.c	(Arbeitskopie)
> @@ -161,6 +161,12 @@
>  	}
>  }
>  
> +int spi_aai_write(struct flashctx *flash, uint8_t *buf,
> +		  unsigned int start, unsigned int len)
> +{
> +	return flash->pgm->spi.write_aai(flash, buf, start, len);
> +}
> +
>  int register_spi_programmer(const struct spi_programmer *pgm)
>  {
>  	struct registered_programmer rpgm;

register_spi_programmer() needs an additional check for non-null
.write_aai member.


> Index: ft2232_spi.c
> ===================================================================
> --- ft2232_spi.c	(Revision 1541)
> +++ ft2232_spi.c	(Arbeitskopie)
> @@ -148,6 +148,7 @@
>  	.multicommand	= default_spi_send_multicommand,
>  	.read		= default_spi_read,
>  	.write_256	= default_spi_write_256,
> +	.write_aai	= default_spi_write_aai,
>  };
>  
>  /* Returns 0 upon success, a negative number upon errors. */
> Index: wbsio_spi.c
> ===================================================================
> --- wbsio_spi.c	(Revision 1541)
> +++ wbsio_spi.c	(Arbeitskopie)
> @@ -75,6 +75,7 @@
>  	.multicommand = default_spi_send_multicommand,
>  	.read = wbsio_spi_read,
>  	.write_256 = spi_chip_write_1,
> +	.write_aai = default_spi_write_aai,
>  };
>  
>  int wbsio_check_for_spi(void)
> Index: sb600spi.c
> ===================================================================
> --- sb600spi.c	(Revision 1541)
> +++ sb600spi.c	(Arbeitskopie)
> @@ -201,6 +201,7 @@
>  	.multicommand = default_spi_send_multicommand,
>  	.read = default_spi_read,
>  	.write_256 = default_spi_write_256,
> +	.write_aai = default_spi_write_aai,
>  };
>  
>  int sb600_probe_spi(struct pci_dev *dev)
> Index: programmer.h
> ===================================================================
> --- programmer.h	(Revision 1541)
> +++ programmer.h	(Arbeitskopie)
> @@ -530,6 +530,8 @@
>  	/* Optimized functions for this programmer */
>  	int (*read)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
>  	int (*write_256)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
> +	int (*write_aai)(struct flashctx *flash, uint8_t *buf,
> +			 unsigned int start, unsigned int len);

Admittedly we should write down our coding style requirements somewhere.
Header files are an exception to the 112 column rule. To keep complete
function signatures grepable (as single lines), header files have no
column limit.


>  	const void *data;
>  };
>  
> @@ -538,6 +540,8 @@
>  int default_spi_send_multicommand(struct flashctx *flash, struct spi_command *cmds);
>  int default_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
>  int default_spi_write_256(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
> +int default_spi_write_aai(struct flashctx *flash, uint8_t *buf,
> +			  unsigned int start, unsigned int len);
>  int register_spi_programmer(const struct spi_programmer *programmer);
>  
>  /* The following enum is needed by ich_descriptor_tool and ich* code. */

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/