[flashrom] [PATCH] Bitbanging SPI driver

[Uwe Hermann]

On Thu, Sep 24, 2009 at 01:11:46AM +0200, Carl-Daniel Hailfinger wrote:
> This is the bitbanging SPI driver infrastructure.
> 
> If you want support for a particular piece of hardware, just fill in
> a few functions in spi_bitbang_master_table. That's it.
> On top of this, the RayeR SPI flasher should be supportable in ~20 LOC.
> 
> Tested, trace looks OK.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Acked-by: Uwe Hermann <uwe at hermann-uwe.de>

But please see below.

 
> Index: flashrom-bitbang_spi/flash.h
> ===================================================================
> --- flashrom-bitbang_spi/flash.h	(Revision 731)
> +++ flashrom-bitbang_spi/flash.h	(Arbeitskopie)
> @@ -145,6 +145,21 @@
>  void chip_readn(uint8_t *buf, const chipaddr addr, size_t len);
>  void programmer_delay(int usecs);
>  
> +enum spi_bitbang_master {
> +	SPI_BITBANG_INVALID /* This must always be the last entry. */
> +};
> +
> +extern const int spi_bitbang_master_count;
> +
> +extern enum spi_bitbang_master spi_bitbang_master;
> +
> +struct spi_bitbang_master_entry {
> +	void (*set_cs) (int val);
> +	void (*set_sck) (int val);
> +	void (*set_mosi) (int val);
> +	int (*get_miso) (void);
> +};

You mix bitbang_spi_* and spi_bitbang_* naming in the code, please fix
to use only one of them (bitbang_spi_ IMHO).
Can be an extra patch, no need to fix in this one.


> +/* bitbang_spi.c */
> +extern int bitbang_half_period;

Also, I'd make all functions and global variables prefixed with
"bitbang_spi_" (not just "bitbang_" like this one) for consistency.
"bitbang_" alone is also a bit too generic, thus "bitbang_spi_".


> +extern const struct spi_bitbang_master_entry spi_bitbang_master_table[];
> +int bitbang_spi_init(void);
> +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);


> +void bitbang_set_cs(int val)

bitbang_spi_set_cs

> +void bitbang_set_sck(int val)

bitbang_spi_set_sck

> +void bitbang_set_mosi(int val)

bitbang_spi_set_mosi

> +int bitbang_get_miso(void)

bitbang_spi_set_miso


> +		if ((r = spi_nbyte_program(i, &buf[i], l))) {
> +			fprintf(stderr, "%s: write fail %d\n", __FUNCTION__, r);

__func__ for consistency.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org