Re: [flashrom] [PATCH] RayeR speedups, variable port

[Michael Karcher]

Am Mittwoch, den 21.07.2010, 13:51 +0200 schrieb Carl-Daniel Hailfinger:
> Speed up RayeR SPIPGM driver in flashrom by a factor of 2.
>
> Allow specification of an alternate base port with
> flashrom -p rayer_spi:lptport=0x278
> Any port number is allowed as long as it is nonzero, below 65536 and a
> multiple of four.
>
> Untested, should work.
>
> Martin, this one should hopefully reach the speed of SPIPGM.exe for
> reads. I'll ask Idwer to provide a DOS binary for you.
>
> Signed-off-by: Carl-Daniel Hailfinger

Basically I think this patch is OK.
> +.B lptport
> +parameter to specify an alternate base I/O address with the
> +.sp
> +.B "  flashrom -p rayer_spi:lptport=portnum"
> +.sp
> +syntax where
> +.B portnum
> +is the I/O port number of your parallel port which must be a multiple of 4.

Could you avoid using the term "port" for "I/O port number", but use
something like "base address", "io address" or the like instead. Just to
make double sure that people don't confuse the number of the "LPT port",
namely "LPT1" with the I/O base address of typically 0x378.

So like

----
.B iobase
parameter to specifiy an alternate base I/O address with the
.sp
.B "  flashrom -p rayer_spi:iobase=baseaddr"
.sp
syntax where
.B baseaddr
is the base I/O address of the parallel port, which must be a multiple
of four. Make sure to not forget the "0x" prefix for hexadecimal port
addresses.
----

You can leave off the last sentence if you think it is redundant.
> -void rayer_bitbang_set_cs(int val)
> +/* Cached value of last byte sent. */
> +static uint8_t lpt_outval;

I would call it "lpt_databyte", but lpt_outval sounds good enough too.
Pick whatever you like more.
> tmp = strtoul(portpos, &endptr, 0);
> /* Port 0, port >0x1000, unaligned ports and garbage strings

cut&pasto. must be >= 0x10000
>  * are rejected.
>  */
> if (!tmp || (tmp >= 0x10000) || (tmp & 0x3) ||
>     (*endptr != '\0')) {

Remaining stuff OK.

Regards,
  Michael Karcher