[flashrom] [PATCH] SPI bitbanging core fixups

[Michael Karcher]

Am Freitag, den 16.07.2010, 23:54 +0200 schrieb Carl-Daniel Hailfinger:
> -extern enum bitbang_spi_master bitbang_spi_master;
[...]
> -int bitbang_spi_init(void);
> +int bitbang_spi_init(enum bitbang_spi_master master);
Eliminating global variables... great.

> +/* Clarification about OUTB/OUTW/OUTL argument order:
> + * OUT[BWL](val, port)
> + */
> +
Looks unrelated, good hint though. Both argument orders are established
in different PC compilers.

> -/* Length of half a clock period in usecs */
> -int bitbang_spi_half_period = 0;
> +/* Length of half a clock period in usecs. Default to 1 (500 kHz). */
> +int bitbang_spi_half_period = 1;
Why so slow? SPI was designed to be fast (if you want a slow system, use
I2C instead ;) ). It might be quite sensible on programmers at the
parallel port having some cable length and no driver chip at the flash
piece (maybe that applies to SPIPGM), but that is specific to this
programmer. nVidia on-board stuff should run slow enough without any
programmer_delay calls.

> +/* Note that CS# is active low, so val=0 means the chip is active. */
>  void bitbang_spi_set_cs(int val)
I don't like the name spi_set_cs then, something like spi_set_negcs? To
bad "/" and "#" characters are forbidden in identifiers...

>  	bitbang_spi_set_cs(1);
>  	bitbang_spi_set_sck(0);
> +	bitbang_spi_set_mosi(0);
Initializeing all bits might make sense, looks OK.

>  	buses_supported = CHIP_BUSTYPE_SPI;
Why do you take away LPC/FWH/PARALLEL here? Think nForce SPI, which
could cooperate with nForce LPC just like on Intel chipsets.

>  	/* Never shrink. realloc() calls are expensive. */
>  	if (bufsize > oldbufsize) {
> -		bufout = realloc(bufout, bufsize);
> -		if (!bufout) {
> +		tmp = realloc(bufout, bufsize);
> +		if (!tmp) {
>  			msg_perr("Out of memory!\n");
> +			if (bufout)
> +				free(bufout);
> +			bufout = NULL;
>  			if (bufin)
>  				free(bufin);
> +			bufin = NULL;
> +			oldbufsize = 0;
>  			exit(1);
We can (somehow) handle Intel chipsets rejecting instructions or not
being able to execute some instruction without exit()ing. Why do we need
to call exit here?

Uncommented parts OK.

Regards,
  Michael Karcher