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

[RayeR]

Hi,
I did some speed test on 4MB SPI flash:

read:
spipgm 2:41
flashrom 2:44

erase
spipgm 0:21
flashrom 3:34

So you can see the read speed has improved :)

Martin
> > Great. Please note that the parameter changed from
> > -p rayer_bitbang_spi
> > to
> > -p rayer_spi
> > because the short form is faster to type, and users do not care at all
> > about bitbanging.

[PATCH] RayeR speedups, variable port

[Carl-Daniel Hailfinger]

Hi Martin,

On 25.07.2010 20:51, RayeR wrote:
> I did some speed test on 4MB SPI flash:
>
> read:
> spipgm 2:41
> flashrom 2:44
>
> erase
> spipgm 0:21
> flashrom 3:34
>
> So you can see the read speed has improved :)

Yes, great. I think we will eventually be able to specify --noverify and
get the same erase speed as spipgm. Right now --noverify is ignored for
erase.
Could you please send an Acked-by: line so I can commit this?
Thanks!

Regards,
Carl-Daniel

[PATCH] RayeR speedups, variable port

[Carl-Daniel Hailfinger]

New version, addresses all review comments.
I would love to have this in 0.9.3.

Speed up RayeR SPIPGM driver in flashrom by a factor of 2.

Allow specification of an alternate base address with
flashrom -p rayer_spi:iobase=0x278
Any base address is allowed as long as it is nonzero, below 65536 and a
multiple of four.

Read speed is now on par with original spipgm.exe.

Signed-off-by: Carl-Daniel Hailfinger 
Tested-by: Martin Rehak

Index: flashrom-bitbang_spi_rayer_faster/rayer_spi.c
===================================================================
--- flashrom-bitbang_spi_rayer_faster/rayer_spi.c	(Revision 1184)
+++ flashrom-bitbang_spi_rayer_faster/rayer_spi.c	(Arbeitskopie)
@@ -30,6 +30,7 @@
  */
 #if defined(__i386__) || defined(__x86_64__)

+#include <stdlib.h>
 #include "flash.h"
 #include "programmer.h"

@@ -43,45 +44,33 @@
 /* Pins for slave->master direction */
 #define SPI_MISO_PIN 6

-static int lpt_iobase;
+static uint16_t lpt_iobase;

-/* FIXME: All rayer_bitbang_set_* functions could use caching of the value
- * stored at port lpt_iobase to avoid unnecessary INB. In theory, only one
- * INB(lpt_iobase) would be needed on programmer init to get the initial
- * value.
- */
+/* Cached value of last byte sent. */
+static uint8_t lpt_outbyte;

-void rayer_bitbang_set_cs(int val)
+static void rayer_bitbang_set_cs(int val)
 {
-	uint8_t tmp;
-
-	tmp = INB(lpt_iobase);
-	tmp &= ~(1 << SPI_CS_PIN);
-	tmp |= (val << SPI_CS_PIN);
-	OUTB(tmp, lpt_iobase);
+	lpt_outbyte &= ~(1 << SPI_CS_PIN);
+	lpt_outbyte |= (val << SPI_CS_PIN);
+	OUTB(lpt_outbyte, lpt_iobase);
 }

-void rayer_bitbang_set_sck(int val)
+static void rayer_bitbang_set_sck(int val)
 {
-	uint8_t tmp;
-
-	tmp = INB(lpt_iobase);
-	tmp &= ~(1 << SPI_SCK_PIN);
-	tmp |= (val << SPI_SCK_PIN);
-	OUTB(tmp, lpt_iobase);
+	lpt_outbyte &= ~(1 << SPI_SCK_PIN);
+	lpt_outbyte |= (val << SPI_SCK_PIN);
+	OUTB(lpt_outbyte, lpt_iobase);
 }

-void rayer_bitbang_set_mosi(int val)
+static void rayer_bitbang_set_mosi(int val)
 {
-	uint8_t tmp;
-
-	tmp = INB(lpt_iobase);
-	tmp &= ~(1 << SPI_MOSI_PIN);
-	tmp |= (val << SPI_MOSI_PIN);
-	OUTB(tmp, lpt_iobase);
+	lpt_outbyte &= ~(1 << SPI_MOSI_PIN);
+	lpt_outbyte |= (val << SPI_MOSI_PIN);
+	OUTB(lpt_outbyte, lpt_iobase);
 }

-int rayer_bitbang_get_miso(void)
+static int rayer_bitbang_get_miso(void)
 {
    uint8_t tmp;

@@ -100,16 +89,49 @@

int rayer_spi_init(void)
 {
-	/* Pick a default value for now. */
-	lpt_iobase = 0x378;
+	char *portpos = NULL;

-	msg_pdbg("Using port 0x%x as I/O base for parallel port access.\n",
+	/* Non-default port requested? */
+	portpos = extract_programmer_param("iobase");
+	if (portpos) {
+		char *endptr = NULL;
+		unsigned long tmp;
+		tmp = strtoul(portpos, &endptr, 0);
+		/* Port 0, port >0x10000, unaligned ports and garbage strings
+		 * are rejected.
+		 */
+		if (!tmp || (tmp >= 0x10000) || (tmp & 0x3) ||
+		    (*endptr != '\0')) {
+			/* Using ports below 0x100 is a really bad idea, and
+			 * should only be done if no port between 0x100 and
+			 * 0xfffc works due to routing issues.
+			 */
+			msg_perr("Error: iobase= specified, but the I/O base "
+				 "given was invalid.\nIt must be a multiple of "
+				 "0x4 and lie between 0x100 and 0xfffc.\n");
+			free(portpos);
+			return 1;
+		} else {
+			lpt_iobase = (uint16_t)tmp;
+			msg_pinfo("Non-default I/O base requested. This will "
+				  "not change the hardware settings.\n");
+		}
+	} else {
+		/* Pick a default value for the I/O base. */
+		lpt_iobase = 0x378;
+	}
+	free(portpos);
+	
+	msg_pdbg("Using address 0x%x as I/O base for parallel port access.\n",
    	 lpt_iobase);

get_io_perms();

-	/* 1 usec halfperiod delay for now. */
-	if (bitbang_spi_init(&bitbang_spi_master_rayer, 1))
+	/* Get the initial value before writing to any line. */
+	lpt_outbyte = INB(lpt_iobase);
+
+	/* Zero halfperiod delay. */
+	if (bitbang_spi_init(&bitbang_spi_master_rayer, 0))
    	return 1;

buses_supported = CHIP_BUSTYPE_SPI;
Index: flashrom-bitbang_spi_rayer_faster/flashrom.8
===================================================================
--- flashrom-bitbang_spi_rayer_faster/flashrom.8	(Revision 1184)
+++ flashrom-bitbang_spi_rayer_faster/flashrom.8	(Arbeitskopie)
@@ -397,7 +397,19 @@
 (in Hz). The default is the maximum frequency of 8 MHz.
 .TP
 .BR "rayer_spi " programmer
-No parameters defined yet. More information about the hardware is available at
+The default I/O base address used for the parallel port is 0x378 and you can use
+the optional
+.B iobase
+parameter to specify an alternate base I/O address with the
+.sp
+.B "  flashrom -p rayer_spi:iobase=baseaddr"
+.sp
+syntax where
+.B baseaddr
+is base I/O port address of the parallel port, which must be a multiple of
+four. Make sure to not forget the "0x" prefix for hexadecimal port addresses.
+.sp
+More information about the hardware is available at
 http://rayer.ic.cz/elektro/spipgm.htm
 .SH EXIT STATUS
 flashrom exits with 0 on success, 1 on most failures but with 2 if /dev/mem

[PATCH] RayeR speedups, variable port

[Michael Karcher]

Am Montag, den 04.10.2010, 13:08 +0200 schrieb Carl-Daniel Hailfinger:
> Signed-off-by: Carl-Daniel Hailfinger 
> Tested-by: Martin Rehak

Looks fine to me.

Acked-by: Michael Karcher

Regards,
  Michael Karcher

[PATCH] RayeR speedups, variable port

[Carl-Daniel Hailfinger]

On 05.10.2010 19:32, Michael Karcher wrote:
> Am Montag, den 04.10.2010, 13:08 +0200 schrieb Carl-Daniel Hailfinger:
> > Signed-off-by: Carl-Daniel Hailfinger 
> > Tested-by: Martin Rehak
>
> Looks fine to me.
>
> Acked-by: Michael Karcher

Thanks, committed in r1188.

Regards,
Carl-Daniel