Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270 as this value mus...

[el-coder-sb (Code Review)]

el-coder-sb has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270 as this value must contain the address and cmd bytes to really make use of 256 raw data page write.
......................................................................

ft2232_spi.c - change the chunksize from 256 to 270 as this value must contain the address and cmd bytes to really make use of 256 raw data page write.

For details check:
https://mail.coreboot.org/hyperkitty/list/[e-mail]/message/W2HU...

Signed-off-by: Simon Buhrow 
Change-Id: Iac067a23025e9df053ab9cd4e82a98de70046c18
---
M ft2232_spi.c
1 file changed, 417 insertions(+), 417 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39632/1

diff --git a/ft2232_spi.c b/ft2232_spi.c
index 1a5b2fe..62cfabe 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -29,60 +29,60 @@
 /* This is not defined in libftdi.h <0.20 (c7e4c09e68cfa6f5e112334aa1b3bb23401c8dc7 to be exact).
  * Some tests indicate that his is the only change that it is needed to support the FT232H in flashrom. */
 #if !defined(HAVE_FT232H)
-#define TYPE_232H	6
+#define TYPE_232H   6
 #endif

/* Please keep sorted by vendor ID, then device ID. */

-#define FTDI_VID		0x0403
-#define FTDI_FT2232H_PID	0x6010
-#define FTDI_FT4232H_PID	0x6011
-#define FTDI_FT232H_PID		0x6014
-#define TIAO_TUMPA_PID		0x8a98
-#define TIAO_TUMPA_LITE_PID	0x8a99
-#define AMONTEC_JTAGKEY_PID	0xCFF8
+#define FTDI_VID        0x0403
+#define FTDI_FT2232H_PID    0x6010
+#define FTDI_FT4232H_PID    0x6011
+#define FTDI_FT232H_PID     0x6014
+#define TIAO_TUMPA_PID      0x8a98
+#define TIAO_TUMPA_LITE_PID 0x8a99
+#define AMONTEC_JTAGKEY_PID 0xCFF8

-#define GOEPEL_VID		0x096C
-#define GOEPEL_PICOTAP_PID	0x1449
+#define GOEPEL_VID      0x096C
+#define GOEPEL_PICOTAP_PID  0x1449

-#define FIC_VID			0x1457
-#define OPENMOKO_DBGBOARD_PID	0x5118
+#define FIC_VID         0x1457
+#define OPENMOKO_DBGBOARD_PID   0x5118

-#define OLIMEX_VID		0x15BA
-#define OLIMEX_ARM_OCD_PID	0x0003
-#define OLIMEX_ARM_TINY_PID	0x0004
-#define OLIMEX_ARM_OCD_H_PID	0x002B
-#define OLIMEX_ARM_TINY_H_PID	0x002A
+#define OLIMEX_VID      0x15BA
+#define OLIMEX_ARM_OCD_PID  0x0003
+#define OLIMEX_ARM_TINY_PID 0x0004
+#define OLIMEX_ARM_OCD_H_PID    0x002B
+#define OLIMEX_ARM_TINY_H_PID   0x002A

-#define GOOGLE_VID		0x18D1
-#define GOOGLE_SERVO_PID	0x5001
-#define GOOGLE_SERVO_V2_PID0	0x5002
-#define GOOGLE_SERVO_V2_PID1	0x5003
+#define GOOGLE_VID      0x18D1
+#define GOOGLE_SERVO_PID    0x5001
+#define GOOGLE_SERVO_V2_PID0    0x5002
+#define GOOGLE_SERVO_V2_PID1    0x5003

const struct dev_entry devs_ft2232spi[] = {
-	{FTDI_VID, FTDI_FT2232H_PID, OK, "FTDI", "FT2232H"},
-	{FTDI_VID, FTDI_FT4232H_PID, OK, "FTDI", "FT4232H"},
-	{FTDI_VID, FTDI_FT232H_PID, OK, "FTDI", "FT232H"},
-	{FTDI_VID, TIAO_TUMPA_PID, OK, "TIAO", "USB Multi-Protocol Adapter"},
-	{FTDI_VID, TIAO_TUMPA_LITE_PID, OK, "TIAO", "USB Multi-Protocol Adapter Lite"},
-	{FTDI_VID, AMONTEC_JTAGKEY_PID, OK, "Amontec", "JTAGkey"},
-	{GOEPEL_VID, GOEPEL_PICOTAP_PID, OK, "GOEPEL", "PicoTAP"},
-	{GOOGLE_VID, GOOGLE_SERVO_PID, OK, "Google", "Servo"},
-	{GOOGLE_VID, GOOGLE_SERVO_V2_PID0, OK, "Google", "Servo V2 Legacy"},
-	{GOOGLE_VID, GOOGLE_SERVO_V2_PID1, OK, "Google", "Servo V2"},
-	{FIC_VID, OPENMOKO_DBGBOARD_PID, OK, "FIC", "OpenMoko Neo1973 Debug board (V2+)"},
-	{OLIMEX_VID, OLIMEX_ARM_OCD_PID, OK, "Olimex", "ARM-USB-OCD"},
-	{OLIMEX_VID, OLIMEX_ARM_TINY_PID, OK, "Olimex", "ARM-USB-TINY"},
-	{OLIMEX_VID, OLIMEX_ARM_OCD_H_PID, OK, "Olimex", "ARM-USB-OCD-H"},
-	{OLIMEX_VID, OLIMEX_ARM_TINY_H_PID, OK, "Olimex", "ARM-USB-TINY-H"},
+    {FTDI_VID, FTDI_FT2232H_PID, OK, "FTDI", "FT2232H"},
+    {FTDI_VID, FTDI_FT4232H_PID, OK, "FTDI", "FT4232H"},
+    {FTDI_VID, FTDI_FT232H_PID, OK, "FTDI", "FT232H"},
+    {FTDI_VID, TIAO_TUMPA_PID, OK, "TIAO", "USB Multi-Protocol Adapter"},
+    {FTDI_VID, TIAO_TUMPA_LITE_PID, OK, "TIAO", "USB Multi-Protocol Adapter Lite"},
+    {FTDI_VID, AMONTEC_JTAGKEY_PID, OK, "Amontec", "JTAGkey"},
+    {GOEPEL_VID, GOEPEL_PICOTAP_PID, OK, "GOEPEL", "PicoTAP"},
+    {GOOGLE_VID, GOOGLE_SERVO_PID, OK, "Google", "Servo"},
+    {GOOGLE_VID, GOOGLE_SERVO_V2_PID0, OK, "Google", "Servo V2 Legacy"},
+    {GOOGLE_VID, GOOGLE_SERVO_V2_PID1, OK, "Google", "Servo V2"},
+    {FIC_VID, OPENMOKO_DBGBOARD_PID, OK, "FIC", "OpenMoko Neo1973 Debug board (V2+)"},
+    {OLIMEX_VID, OLIMEX_ARM_OCD_PID, OK, "Olimex", "ARM-USB-OCD"},
+    {OLIMEX_VID, OLIMEX_ARM_TINY_PID, OK, "Olimex", "ARM-USB-TINY"},
+    {OLIMEX_VID, OLIMEX_ARM_OCD_H_PID, OK, "Olimex", "ARM-USB-OCD-H"},
+    {OLIMEX_VID, OLIMEX_ARM_TINY_H_PID, OK, "Olimex", "ARM-USB-TINY-H"},

-	{0},
+    {0},
 };

#define DEFAULT_DIVISOR 2

-#define BITMODE_BITBANG_NORMAL	1
-#define BITMODE_BITBANG_SPI	2
+#define BITMODE_BITBANG_NORMAL  1
+#define BITMODE_BITBANG_SPI 2

/* The variables cs_bits and pindir store the values for the "set data bits low byte" MPSSE command that
  * sets the initial state and the direction of the I/O pins. The pin offsets are as follows:
@@ -108,442 +108,442 @@

static const char *get_ft2232_devicename(int ft2232_vid, int ft2232_type)
 {
-	int i;
-	for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) {
-		if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid))
-				return devs_ft2232spi[i].device_name;
-	}
-	return "unknown device";
+    int i;
+    for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) {
+        if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid))
+                return devs_ft2232spi[i].device_name;
+    }
+    return "unknown device";
 }

static const char *get_ft2232_vendorname(int ft2232_vid, int ft2232_type)
 {
-	int i;
-	for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) {
-		if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid))
-				return devs_ft2232spi[i].vendor_name;
-	}
-	return "unknown vendor";
+    int i;
+    for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) {
+        if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid))
+                return devs_ft2232spi[i].vendor_name;
+    }
+    return "unknown vendor";
 }

static int send_buf(struct ftdi_context *ftdic, const unsigned char *buf,
-		    int size)
+            int size)
 {
-	int r;
-	r = ftdi_write_data(ftdic, (unsigned char *) buf, size);
-	if (r < 0) {
-		msg_perr("ftdi_write_data: %d, %s\n", r, ftdi_get_error_string(ftdic));
-		return 1;
-	}
-	return 0;
+    int r;
+    r = ftdi_write_data(ftdic, (unsigned char *) buf, size);
+    if (r < 0) {
+        msg_perr("ftdi_write_data: %d, %s\n", r, ftdi_get_error_string(ftdic));
+        return 1;
+    }
+    return 0;
 }

static int get_buf(struct ftdi_context *ftdic, const unsigned char *buf,
-		   int size)
+           int size)
 {
-	int r;
+    int r;

-	while (size > 0) {
-		r = ftdi_read_data(ftdic, (unsigned char *) buf, size);
-		if (r < 0) {
-			msg_perr("ftdi_read_data: %d, %s\n", r, ftdi_get_error_string(ftdic));
-			return 1;
-		}
-		buf += r;
-		size -= r;
-	}
-	return 0;
+    while (size > 0) {
+        r = ftdi_read_data(ftdic, (unsigned char *) buf, size);
+        if (r < 0) {
+            msg_perr("ftdi_read_data: %d, %s\n", r, ftdi_get_error_string(ftdic));
+            return 1;
+        }
+        buf += r;
+        size -= r;
+    }
+    return 0;
 }

static int ft2232_spi_send_command(struct flashctx *flash,
-				   unsigned int writecnt, unsigned int readcnt,
-				   const unsigned char *writearr,
-				   unsigned char *readarr);
+                   unsigned int writecnt, unsigned int readcnt,
+                   const unsigned char *writearr,
+                   unsigned char *readarr);

static const struct spi_master spi_master_ft2232 = {
-	.features	= SPI_MASTER_4BA,
-	.max_data_read	= 64 * 1024,
-	.max_data_write	= 256,
-	.command	= ft2232_spi_send_command,
-	.multicommand	= default_spi_send_multicommand,
-	.read		= default_spi_read,
-	.write_256	= default_spi_write_256,
-	.write_aai	= default_spi_write_aai,
+    .features   = SPI_MASTER_4BA,
+    .max_data_read  = 64 * 1024,
+    .max_data_write = 256,
+    .command    = ft2232_spi_send_command,
+    .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. */
 int ft2232_spi_init(void)
 {
-	int ret = 0;
-	struct ftdi_context *ftdic = &ftdic_context;
-	unsigned char buf[512];
-	int ft2232_vid = FTDI_VID;
-	int ft2232_type = FTDI_FT4232H_PID;
-	int channel_count = 4; /* Stores the number of channels of the device. */
-	enum ftdi_interface ft2232_interface = INTERFACE_A;
-	/*
-	 * The 'H' chips can run with an internal clock of either 12 MHz or 60 MHz,
-	 * but the non-H chips can only run at 12 MHz. We enable the divide-by-5
-	 * prescaler on the former to run on the same speed.
-	 */
-	uint8_t clock_5x = 1;
-	/* In addition to the prescaler mentioned above there is also another
-	 * configurable one on all versions of the chips. Its divisor div can be
-	 * set by a 16 bit value x according to the following formula:
-	 * div = (1 + x) * 2 <-> x = div / 2 - 1
-	 * Hence the expressible divisors are all even numbers between 2 and
-	 * 2^17 (=131072) resulting in SCK frequencies of 6 MHz down to about
-	 * 92 Hz for 12 MHz inputs.
-	 */
-	uint32_t divisor = DEFAULT_DIVISOR;
-	int f;
-	char *arg;
-	double mpsse_clk;
+    int ret = 0;
+    struct ftdi_context *ftdic = &ftdic_context;
+    unsigned char buf[512];
+    int ft2232_vid = FTDI_VID;
+    int ft2232_type = FTDI_FT4232H_PID;
+    int channel_count = 4; /* Stores the number of channels of the device. */
+    enum ftdi_interface ft2232_interface = INTERFACE_A;
+    /*
+     * The 'H' chips can run with an internal clock of either 12 MHz or 60 MHz,
+     * but the non-H chips can only run at 12 MHz. We enable the divide-by-5
+     * prescaler on the former to run on the same speed.
+     */
+    uint8_t clock_5x = 1;
+    /* In addition to the prescaler mentioned above there is also another
+     * configurable one on all versions of the chips. Its divisor div can be
+     * set by a 16 bit value x according to the following formula:
+     * div = (1 + x) * 2 <-> x = div / 2 - 1
+     * Hence the expressible divisors are all even numbers between 2 and
+     * 2^17 (=131072) resulting in SCK frequencies of 6 MHz down to about
+     * 92 Hz for 12 MHz inputs.
+     */
+    uint32_t divisor = DEFAULT_DIVISOR;
+    int f;
+    char *arg;
+    double mpsse_clk;

-	arg = extract_programmer_param("type");
-	if (arg) {
-		if (!strcasecmp(arg, "2232H")) {
-			ft2232_type = FTDI_FT2232H_PID;
-			channel_count = 2;
-		} else if (!strcasecmp(arg, "4232H")) {
-			ft2232_type = FTDI_FT4232H_PID;
-			channel_count = 4;
-		} else if (!strcasecmp(arg, "232H")) {
-			ft2232_type = FTDI_FT232H_PID;
-			channel_count = 1;
-		} else if (!strcasecmp(arg, "jtagkey")) {
-			ft2232_type = AMONTEC_JTAGKEY_PID;
-			channel_count = 2;
-			/* JTAGkey(2) needs to enable its output via Bit4 / GPIOL0
-			*  value: 0x18  OE=high, CS=high, DI=low, DO=low, SK=low
-			*    dir: 0x1b  OE=output, CS=output, DI=input, DO=output, SK=output */
-			cs_bits = 0x18;
-			pindir = 0x1b;
-		} else if (!strcasecmp(arg, "picotap")) {
-			ft2232_vid = GOEPEL_VID;
-			ft2232_type = GOEPEL_PICOTAP_PID;
-			channel_count = 2;
-		} else if (!strcasecmp(arg, "tumpa")) {
-			/* Interface A is SPI1, B is SPI2. */
-			ft2232_type = TIAO_TUMPA_PID;
-			channel_count = 2;
-		} else if (!strcasecmp(arg, "tumpalite")) {
-			/* Only one channel is used on lite edition */
-			ft2232_type = TIAO_TUMPA_LITE_PID;
-			channel_count = 1;
-		} else if (!strcasecmp(arg, "busblaster")) {
-			/* In its default configuration it is a jtagkey clone */
-			ft2232_type = FTDI_FT2232H_PID;
-			channel_count = 2;
-			cs_bits = 0x18;
-			pindir = 0x1b;
-		} else if (!strcasecmp(arg, "openmoko")) {
-			ft2232_vid = FIC_VID;
-			ft2232_type = OPENMOKO_DBGBOARD_PID;
-			channel_count = 2;
-		} else if (!strcasecmp(arg, "arm-usb-ocd")) {
-			ft2232_vid = OLIMEX_VID;
-			ft2232_type = OLIMEX_ARM_OCD_PID;
-			channel_count = 2;
-			/* arm-usb-ocd(-h) has an output buffer that needs to be enabled by pulling ADBUS4 low.
-			*  value: 0x08  #OE=low, CS=high, DI=low, DO=low, SK=low
-			*    dir: 0x1b  #OE=output, CS=output, DI=input, DO=output, SK=output */
-			cs_bits = 0x08;
-			pindir = 0x1b;
-		} else if (!strcasecmp(arg, "arm-usb-tiny")) {
-			ft2232_vid = OLIMEX_VID;
-			ft2232_type = OLIMEX_ARM_TINY_PID;
-			channel_count = 2;
-		} else if (!strcasecmp(arg, "arm-usb-ocd-h")) {
-			ft2232_vid = OLIMEX_VID;
-			ft2232_type = OLIMEX_ARM_OCD_H_PID;
-			channel_count = 2;
-			/* See arm-usb-ocd */
-			cs_bits = 0x08;
-			pindir = 0x1b;
-		} else if (!strcasecmp(arg, "arm-usb-tiny-h")) {
-			ft2232_vid = OLIMEX_VID;
-			ft2232_type = OLIMEX_ARM_TINY_H_PID;
-			channel_count = 2;
-		} else if (!strcasecmp(arg, "google-servo")) {
-			ft2232_vid = GOOGLE_VID;
-			ft2232_type = GOOGLE_SERVO_PID;
-		} else if (!strcasecmp(arg, "google-servo-v2")) {
-			ft2232_vid = GOOGLE_VID;
-			ft2232_type = GOOGLE_SERVO_V2_PID1;
-			/* Default divisor is too fast, and chip ID fails */
-			divisor = 6;
-		} else if (!strcasecmp(arg, "google-servo-v2-legacy")) {
-			ft2232_vid = GOOGLE_VID;
-			ft2232_type = GOOGLE_SERVO_V2_PID0;
-		} else if (!strcasecmp(arg, "flyswatter")) {
-			ft2232_type = FTDI_FT2232H_PID;
-			channel_count = 2;
-			/* Flyswatter and Flyswatter-2 require GPIO bits 0x80
-			 * and 0x40 to be driven low to enable output buffers */
-			pindir = 0xcb;
-		} else {
-			msg_perr("Error: Invalid device type specified.\n");
-			free(arg);
-			return -1;
-		}
-	}
-	free(arg);
+    arg = extract_programmer_param("type");
+    if (arg) {
+        if (!strcasecmp(arg, "2232H")) {
+            ft2232_type = FTDI_FT2232H_PID;
+            channel_count = 2;
+        } else if (!strcasecmp(arg, "4232H")) {
+            ft2232_type = FTDI_FT4232H_PID;
+            channel_count = 4;
+        } else if (!strcasecmp(arg, "232H")) {
+            ft2232_type = FTDI_FT232H_PID;
+            channel_count = 1;
+        } else if (!strcasecmp(arg, "jtagkey")) {
+            ft2232_type = AMONTEC_JTAGKEY_PID;
+            channel_count = 2;
+            /* JTAGkey(2) needs to enable its output via Bit4 / GPIOL0
+            *  value: 0x18  OE=high, CS=high, DI=low, DO=low, SK=low
+            *    dir: 0x1b  OE=output, CS=output, DI=input, DO=output, SK=output */
+            cs_bits = 0x18;
+            pindir = 0x1b;
+        } else if (!strcasecmp(arg, "picotap")) {
+            ft2232_vid = GOEPEL_VID;
+            ft2232_type = GOEPEL_PICOTAP_PID;
+            channel_count = 2;
+        } else if (!strcasecmp(arg, "tumpa")) {
+            /* Interface A is SPI1, B is SPI2. */
+            ft2232_type = TIAO_TUMPA_PID;
+            channel_count = 2;
+        } else if (!strcasecmp(arg, "tumpalite")) {
+            /* Only one channel is used on lite edition */
+            ft2232_type = TIAO_TUMPA_LITE_PID;
+            channel_count = 1;
+        } else if (!strcasecmp(arg, "busblaster")) {
+            /* In its default configuration it is a jtagkey clone */
+            ft2232_type = FTDI_FT2232H_PID;
+            channel_count = 2;
+            cs_bits = 0x18;
+            pindir = 0x1b;
+        } else if (!strcasecmp(arg, "openmoko")) {
+            ft2232_vid = FIC_VID;
+            ft2232_type = OPENMOKO_DBGBOARD_PID;
+            channel_count = 2;
+        } else if (!strcasecmp(arg, "arm-usb-ocd")) {
+            ft2232_vid = OLIMEX_VID;
+            ft2232_type = OLIMEX_ARM_OCD_PID;
+            channel_count = 2;
+            /* arm-usb-ocd(-h) has an output buffer that needs to be enabled by pulling ADBUS4 low.
+            *  value: 0x08  #OE=low, CS=high, DI=low, DO=low, SK=low
+            *    dir: 0x1b  #OE=output, CS=output, DI=input, DO=output, SK=output */
+            cs_bits = 0x08;
+            pindir = 0x1b;
+        } else if (!strcasecmp(arg, "arm-usb-tiny")) {
+            ft2232_vid = OLIMEX_VID;
+            ft2232_type = OLIMEX_ARM_TINY_PID;
+            channel_count = 2;
+        } else if (!strcasecmp(arg, "arm-usb-ocd-h")) {
+            ft2232_vid = OLIMEX_VID;
+            ft2232_type = OLIMEX_ARM_OCD_H_PID;
+            channel_count = 2;
+            /* See arm-usb-ocd */
+            cs_bits = 0x08;
+            pindir = 0x1b;
+        } else if (!strcasecmp(arg, "arm-usb-tiny-h")) {
+            ft2232_vid = OLIMEX_VID;
+            ft2232_type = OLIMEX_ARM_TINY_H_PID;
+            channel_count = 2;
+        } else if (!strcasecmp(arg, "google-servo")) {
+            ft2232_vid = GOOGLE_VID;
+            ft2232_type = GOOGLE_SERVO_PID;
+        } else if (!strcasecmp(arg, "google-servo-v2")) {
+            ft2232_vid = GOOGLE_VID;
+            ft2232_type = GOOGLE_SERVO_V2_PID1;
+            /* Default divisor is too fast, and chip ID fails */
+            divisor = 6;
+        } else if (!strcasecmp(arg, "google-servo-v2-legacy")) {
+            ft2232_vid = GOOGLE_VID;
+            ft2232_type = GOOGLE_SERVO_V2_PID0;
+        } else if (!strcasecmp(arg, "flyswatter")) {
+            ft2232_type = FTDI_FT2232H_PID;
+            channel_count = 2;
+            /* Flyswatter and Flyswatter-2 require GPIO bits 0x80
+             * and 0x40 to be driven low to enable output buffers */
+            pindir = 0xcb;
+        } else {
+            msg_perr("Error: Invalid device type specified.\n");
+            free(arg);
+            return -1;
+        }
+    }
+    free(arg);

-	arg = extract_programmer_param("port");
-	if (arg) {
-		switch (toupper((unsigned char)*arg)) {
-		case 'A':
-			ft2232_interface = INTERFACE_A;
-			break;
-		case 'B':
-			ft2232_interface = INTERFACE_B;
-			if (channel_count < 2)
-				channel_count = -1;
-			break;
-		case 'C':
-			ft2232_interface = INTERFACE_C;
-			if (channel_count < 3)
-				channel_count = -1;
-			break;
-		case 'D':
-			ft2232_interface = INTERFACE_D;
-			if (channel_count < 4)
-				channel_count = -1;
-			break;
-		default:
-			channel_count = -1;
-			break;
-		}
-		if (channel_count < 0 || strlen(arg) != 1) {
-			msg_perr("Error: Invalid channel/port/interface specified: "%s".\n", arg);
-			free(arg);
-			return -2;
-		}
-	}
-	free(arg);
+    arg = extract_programmer_param("port");
+    if (arg) {
+        switch (toupper((unsigned char)*arg)) {
+        case 'A':
+            ft2232_interface = INTERFACE_A;
+            break;
+        case 'B':
+            ft2232_interface = INTERFACE_B;
+            if (channel_count < 2)
+                channel_count = -1;
+            break;
+        case 'C':
+            ft2232_interface = INTERFACE_C;
+            if (channel_count < 3)
+                channel_count = -1;
+            break;
+        case 'D':
+            ft2232_interface = INTERFACE_D;
+            if (channel_count < 4)
+                channel_count = -1;
+            break;
+        default:
+            channel_count = -1;
+            break;
+        }
+        if (channel_count < 0 || strlen(arg) != 1) {
+            msg_perr("Error: Invalid channel/port/interface specified: "%s".\n", arg);
+            free(arg);
+            return -2;
+        }
+    }
+    free(arg);

-	arg = extract_programmer_param("divisor");
-	if (arg && strlen(arg)) {
-		unsigned int temp = 0;
-		char *endptr;
-		temp = strtoul(arg, &endptr, 10);
-		if (*endptr || temp < 2 || temp > 131072 || temp & 0x1) {
-			msg_perr("Error: Invalid SPI frequency divisor specified: "%s".\n"
-				 "Valid are even values between 2 and 131072.\n", arg);
-			free(arg);
-			return -2;
-		}
-		divisor = (uint32_t)temp;
-	}
-	free(arg);
+    arg = extract_programmer_param("divisor");
+    if (arg && strlen(arg)) {
+        unsigned int temp = 0;
+        char *endptr;
+        temp = strtoul(arg, &endptr, 10);
+        if (*endptr || temp < 2 || temp > 131072 || temp & 0x1) {
+            msg_perr("Error: Invalid SPI frequency divisor specified: "%s".\n"
+                 "Valid are even values between 2 and 131072.\n", arg);
+            free(arg);
+            return -2;
+        }
+        divisor = (uint32_t)temp;
+    }
+    free(arg);

-	/* Allows setting multiple GPIOL states, for example: csgpiol=012 */
-	arg = extract_programmer_param("csgpiol");
-	if (arg) {
-		unsigned int ngpios = strlen(arg);
-		for (unsigned int i = 0; i <= ngpios; i++) {
-			int temp = arg[i] - '0';
-			if (ngpios == 0 || (ngpios != i && (temp < 0 || temp > 3))) {
-				msg_perr("Error: Invalid GPIOLs specified: "%s".\n"
-					 "Valid values are numbers between 0 and 3. "
-					 "Multiple GPIOLs can be specified.\n", arg);
-				free(arg);
-				return -2;
-			} else {
-				unsigned int pin = temp + 4;
-				cs_bits |= 1 << pin;
-				pindir |= 1 << pin;
-			}
-		}
-	}
-	free(arg);
+    /* Allows setting multiple GPIOL states, for example: csgpiol=012 */
+    arg = extract_programmer_param("csgpiol");
+    if (arg) {
+        unsigned int ngpios = strlen(arg);
+        for (unsigned int i = 0; i <= ngpios; i++) {
+            int temp = arg[i] - '0';
+            if (ngpios == 0 || (ngpios != i && (temp < 0 || temp > 3))) {
+                msg_perr("Error: Invalid GPIOLs specified: "%s".\n"
+                     "Valid values are numbers between 0 and 3. "
+                     "Multiple GPIOLs can be specified.\n", arg);
+                free(arg);
+                return -2;
+            } else {
+                unsigned int pin = temp + 4;
+                cs_bits |= 1 << pin;
+                pindir |= 1 << pin;
+            }
+        }
+    }
+    free(arg);

-	msg_pdbg("Using device type %s %s ",
-		 get_ft2232_vendorname(ft2232_vid, ft2232_type),
-		 get_ft2232_devicename(ft2232_vid, ft2232_type));
-	msg_pdbg("channel %s.\n",
-		 (ft2232_interface == INTERFACE_A) ? "A" :
-		 (ft2232_interface == INTERFACE_B) ? "B" :
-		 (ft2232_interface == INTERFACE_C) ? "C" : "D");
+    msg_pdbg("Using device type %s %s ",
+         get_ft2232_vendorname(ft2232_vid, ft2232_type),
+         get_ft2232_devicename(ft2232_vid, ft2232_type));
+    msg_pdbg("channel %s.\n",
+         (ft2232_interface == INTERFACE_A) ? "A" :
+         (ft2232_interface == INTERFACE_B) ? "B" :
+         (ft2232_interface == INTERFACE_C) ? "C" : "D");

-	if (ftdi_init(ftdic) < 0) {
-		msg_perr("ftdi_init failed.\n");
-		return -3;
-	}
+    if (ftdi_init(ftdic) < 0) {
+        msg_perr("ftdi_init failed.\n");
+        return -3;
+    }

-	if (ftdi_set_interface(ftdic, ft2232_interface) < 0) {
-		msg_perr("Unable to select channel (%s).\n", ftdi_get_error_string(ftdic));
-	}
+    if (ftdi_set_interface(ftdic, ft2232_interface) < 0) {
+        msg_perr("Unable to select channel (%s).\n", ftdi_get_error_string(ftdic));
+    }

-	arg = extract_programmer_param("serial");
-	f = ftdi_usb_open_desc(ftdic, ft2232_vid, ft2232_type, NULL, arg);
-	free(arg);
+    arg = extract_programmer_param("serial");
+    f = ftdi_usb_open_desc(ftdic, ft2232_vid, ft2232_type, NULL, arg);
+    free(arg);

-	if (f < 0 && f != -5) {
-		msg_perr("Unable to open FTDI device: %d (%s).\n", f, ftdi_get_error_string(ftdic));
-		return -4;
-	}
+    if (f < 0 && f != -5) {
+        msg_perr("Unable to open FTDI device: %d (%s).\n", f, ftdi_get_error_string(ftdic));
+        return -4;
+    }

-	if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H && ftdic->type != TYPE_232H) {
-		msg_pdbg("FTDI chip type %d is not high-speed.\n", ftdic->type);
-		clock_5x = 0;
-	}
+    if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H && ftdic->type != TYPE_232H) {
+        msg_pdbg("FTDI chip type %d is not high-speed.\n", ftdic->type);
+        clock_5x = 0;
+    }

-	if (ftdi_usb_reset(ftdic) < 0) {
-		msg_perr("Unable to reset FTDI device (%s).\n", ftdi_get_error_string(ftdic));
-	}
+    if (ftdi_usb_reset(ftdic) < 0) {
+        msg_perr("Unable to reset FTDI device (%s).\n", ftdi_get_error_string(ftdic));
+    }

-	if (ftdi_set_latency_timer(ftdic, 2) < 0) {
-		msg_perr("Unable to set latency timer (%s).\n", ftdi_get_error_string(ftdic));
-	}
+    if (ftdi_set_latency_timer(ftdic, 2) < 0) {
+        msg_perr("Unable to set latency timer (%s).\n", ftdi_get_error_string(ftdic));
+    }

-	if (ftdi_write_data_set_chunksize(ftdic, 256)) {
-		msg_perr("Unable to set chunk size (%s).\n", ftdi_get_error_string(ftdic));
-	}
+    if (ftdi_write_data_set_chunksize(ftdic, 270)) {
+        msg_perr("Unable to set chunk size (%s).\n", ftdi_get_error_string(ftdic));
+    }

-	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0) {
-		msg_perr("Unable to set bitmode to SPI (%s).\n", ftdi_get_error_string(ftdic));
-	}
+    if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0) {
+        msg_perr("Unable to set bitmode to SPI (%s).\n", ftdi_get_error_string(ftdic));
+    }

-	if (clock_5x) {
-		msg_pdbg("Disable divide-by-5 front stage\n");
-		buf[0] = 0x8a; /* Disable divide-by-5. DIS_DIV_5 in newer libftdi */
-		if (send_buf(ftdic, buf, 1)) {
-			ret = -5;
-			goto ftdi_err;
-		}
-		mpsse_clk = 60.0;
-	} else {
-		mpsse_clk = 12.0;
-	}
+    if (clock_5x) {
+        msg_pdbg("Disable divide-by-5 front stage\n");
+        buf[0] = 0x8a; /* Disable divide-by-5. DIS_DIV_5 in newer libftdi */
+        if (send_buf(ftdic, buf, 1)) {
+            ret = -5;
+            goto ftdi_err;
+        }
+        mpsse_clk = 60.0;
+    } else {
+        mpsse_clk = 12.0;
+    }

-	msg_pdbg("Set clock divisor\n");
-	buf[0] = TCK_DIVISOR;
-	buf[1] = (divisor / 2 - 1) & 0xff;
-	buf[2] = ((divisor / 2 - 1) >> 8) & 0xff;
-	if (send_buf(ftdic, buf, 3)) {
-		ret = -6;
-		goto ftdi_err;
-	}
+    msg_pdbg("Set clock divisor\n");
+    buf[0] = TCK_DIVISOR;
+    buf[1] = (divisor / 2 - 1) & 0xff;
+    buf[2] = ((divisor / 2 - 1) >> 8) & 0xff;
+    if (send_buf(ftdic, buf, 3)) {
+        ret = -6;
+        goto ftdi_err;
+    }

-	msg_pdbg("MPSSE clock: %f MHz, divisor: %u, SPI clock: %f MHz\n",
-		 mpsse_clk, divisor, (double)(mpsse_clk / divisor));
+    msg_pdbg("MPSSE clock: %f MHz, divisor: %u, SPI clock: %f MHz\n",
+         mpsse_clk, divisor, (double)(mpsse_clk / divisor));

-	/* Disconnect TDI/DO to TDO/DI for loopback. */
-	msg_pdbg("No loopback of TDI/DO TDO/DI\n");
-	buf[0] = LOOPBACK_END;
-	if (send_buf(ftdic, buf, 1)) {
-		ret = -7;
-		goto ftdi_err;
-	}
+    /* Disconnect TDI/DO to TDO/DI for loopback. */
+    msg_pdbg("No loopback of TDI/DO TDO/DI\n");
+    buf[0] = LOOPBACK_END;
+    if (send_buf(ftdic, buf, 1)) {
+        ret = -7;
+        goto ftdi_err;
+    }

-	msg_pdbg("Set data bits\n");
-	buf[0] = SET_BITS_LOW;
-	buf[1] = cs_bits;
-	buf[2] = pindir;
-	if (send_buf(ftdic, buf, 3)) {
-		ret = -8;
-		goto ftdi_err;
-	}
+    msg_pdbg("Set data bits\n");
+    buf[0] = SET_BITS_LOW;
+    buf[1] = cs_bits;
+    buf[2] = pindir;
+    if (send_buf(ftdic, buf, 3)) {
+        ret = -8;
+        goto ftdi_err;
+    }

-	register_spi_master(&spi_master_ft2232);
+    register_spi_master(&spi_master_ft2232);

-	return 0;
+    return 0;

ftdi_err:
-	if ((f = ftdi_usb_close(ftdic)) < 0) {
-		msg_perr("Unable to close FTDI device: %d (%s)\n", f, ftdi_get_error_string(ftdic));
-	}
-	return ret;
+    if ((f = ftdi_usb_close(ftdic)) < 0) {
+        msg_perr("Unable to close FTDI device: %d (%s)\n", f, ftdi_get_error_string(ftdic));
+    }
+    return ret;
 }

/* Returns 0 upon success, a negative number upon errors. */
 static int ft2232_spi_send_command(struct flashctx *flash,
-				   unsigned int writecnt, unsigned int readcnt,
-				   const unsigned char *writearr,
-				   unsigned char *readarr)
+                   unsigned int writecnt, unsigned int readcnt,
+                   const unsigned char *writearr,
+                   unsigned char *readarr)
 {
-	struct ftdi_context *ftdic = &ftdic_context;
-	static unsigned char *buf = NULL;
-	/* failed is special. We use bitwise ops, but it is essentially bool. */
-	int i = 0, ret = 0, failed = 0;
-	int bufsize;
-	static int oldbufsize = 0;
+    struct ftdi_context *ftdic = &ftdic_context;
+    static unsigned char *buf = NULL;
+    /* failed is special. We use bitwise ops, but it is essentially bool. */
+    int i = 0, ret = 0, failed = 0;
+    int bufsize;
+    static int oldbufsize = 0;

-	if (writecnt > 65536 || readcnt > 65536)
-		return SPI_INVALID_LENGTH;
+    if (writecnt > 65536 || readcnt > 65536)
+        return SPI_INVALID_LENGTH;

-	/* buf is not used for the response from the chip. */
-	bufsize = max(writecnt + 9, 260 + 9);
-	/* Never shrink. realloc() calls are expensive. */
-	if (bufsize > oldbufsize) {
-		buf = realloc(buf, bufsize);
-		if (!buf) {
-			msg_perr("Out of memory!\n");
-			/* TODO: What to do with buf? */
-			return SPI_GENERIC_ERROR;
-		}
-		oldbufsize = bufsize;
-	}
+    /* buf is not used for the response from the chip. */
+    bufsize = max(writecnt + 9, 260 + 9);
+    /* Never shrink. realloc() calls are expensive. */
+    if (bufsize > oldbufsize) {
+        buf = realloc(buf, bufsize);
+        if (!buf) {
+            msg_perr("Out of memory!\n");
+            /* TODO: What to do with buf? */
+            return SPI_GENERIC_ERROR;
+        }
+        oldbufsize = bufsize;
+    }

-	/*
-	 * Minimize USB transfers by packing as many commands as possible
-	 * together. If we're not expecting to read, we can assert CS#, write,
-	 * and deassert CS# all in one shot. If reading, we do three separate
-	 * operations.
-	 */
-	msg_pspew("Assert CS#\n");
-	buf[i++] = SET_BITS_LOW;
-	buf[i++] = ~ 0x08 & cs_bits; /* assert CS (3rd) bit only */
-	buf[i++] = pindir;
+    /*
+     * Minimize USB transfers by packing as many commands as possible
+     * together. If we're not expecting to read, we can assert CS#, write,
+     * and deassert CS# all in one shot. If reading, we do three separate
+     * operations.
+     */
+    msg_pspew("Assert CS#\n");
+    buf[i++] = SET_BITS_LOW;
+    buf[i++] = ~ 0x08 & cs_bits; /* assert CS (3rd) bit only */
+    buf[i++] = pindir;

-	if (writecnt) {
-		buf[i++] = MPSSE_DO_WRITE | MPSSE_WRITE_NEG;
-		buf[i++] = (writecnt - 1) & 0xff;
-		buf[i++] = ((writecnt - 1) >> 8) & 0xff;
-		memcpy(buf + i, writearr, writecnt);
-		i += writecnt;
-	}
+    if (writecnt) {
+        buf[i++] = MPSSE_DO_WRITE | MPSSE_WRITE_NEG;
+        buf[i++] = (writecnt - 1) & 0xff;
+        buf[i++] = ((writecnt - 1) >> 8) & 0xff;
+        memcpy(buf + i, writearr, writecnt);
+        i += writecnt;
+    }

-	/*
-	 * Optionally terminate this batch of commands with a
-	 * read command, then do the fetch of the results.
-	 */
-	if (readcnt) {
-		buf[i++] = MPSSE_DO_READ;
-		buf[i++] = (readcnt - 1) & 0xff;
-		buf[i++] = ((readcnt - 1) >> 8) & 0xff;
-		ret = send_buf(ftdic, buf, i);
-		failed = ret;
-		/* We can't abort here, we still have to deassert CS#. */
-		if (ret)
-			msg_perr("send_buf failed before read: %i\n", ret);
-		i = 0;
-		if (ret == 0) {
-			/*
-			 * FIXME: This is unreliable. There's no guarantee that
-			 * we read the response directly after sending the read
-			 * command. We may be scheduled out etc.
-			 */
-			ret = get_buf(ftdic, readarr, readcnt);
-			failed |= ret;
-			/* We can't abort here either. */
-			if (ret)
-				msg_perr("get_buf failed: %i\n", ret);
-		}
-	}
+    /*
+     * Optionally terminate this batch of commands with a
+     * read command, then do the fetch of the results.
+     */
+    if (readcnt) {
+        buf[i++] = MPSSE_DO_READ;
+        buf[i++] = (readcnt - 1) & 0xff;
+        buf[i++] = ((readcnt - 1) >> 8) & 0xff;
+        ret = send_buf(ftdic, buf, i);
+        failed = ret;
+        /* We can't abort here, we still have to deassert CS#. */
+        if (ret)
+            msg_perr("send_buf failed before read: %i\n", ret);
+        i = 0;
+        if (ret == 0) {
+            /*
+             * FIXME: This is unreliable. There's no guarantee that
+             * we read the response directly after sending the read
+             * command. We may be scheduled out etc.
+             */
+            ret = get_buf(ftdic, readarr, readcnt);
+            failed |= ret;
+            /* We can't abort here either. */
+            if (ret)
+                msg_perr("get_buf failed: %i\n", ret);
+        }
+    }

-	msg_pspew("De-assert CS#\n");
-	buf[i++] = SET_BITS_LOW;
-	buf[i++] = cs_bits;
-	buf[i++] = pindir;
-	ret = send_buf(ftdic, buf, i);
-	failed |= ret;
-	if (ret)
-		msg_perr("send_buf failed at end: %i\n", ret);
+    msg_pspew("De-assert CS#\n");
+    buf[i++] = SET_BITS_LOW;
+    buf[i++] = cs_bits;
+    buf[i++] = pindir;
+    ret = send_buf(ftdic, buf, i);
+    failed |= ret;
+    if (ret)
+        msg_perr("send_buf failed at end: %i\n", ret);

-	return failed ? -1 : 0;
+    return failed ? -1 : 0;
 }

#endif

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270 as this value must contain the address and cmd bytes to really make use of 256 raw data page write.
......................................................................

Patch Set 1:

Why did all tabs get replaced with spaces?

[el-coder-sb (Code Review)]

el-coder-sb has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270 as this value must contain the address and cmd bytes to really make use of 256 raw data page write.
......................................................................

Patch Set 1:

I am sorry for the tab-replacing! That was my geany editor. I noticed it after push and as I am a very newbie to git and gerrit I was not able to correct it!
If you can tell me how to correct it in a clean way, just let me know. I will do so.
The only change I wanted to do is in line 401.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270 as this value must contain the address and cmd bytes to really make use of 256 raw data page write.
......................................................................

Patch Set 1:
> Patch Set 1:
>
> I am sorry for the tab-replacing! That was my geany editor. I noticed it after push and as I am a very newbie to git and gerrit I was not able to correct it!
> If you can tell me how to correct it in a clean way, just let me know. I will do so.
> The only change I wanted to do is in line 401.

You can tell geany to not change the formatting of the files when saving.

As this is a small change, just remove this commit from your local tree and re-create it again. Gerrit identifies this change with this line in the commit message:

Change-Id: Iac067a23025e9df053ab9cd4e82a98de70046c18

To push an updated version of this change, just make a commit which contains this line in the commit message, and pull it again.

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270. Tabs right now.

[el-coder-sb (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/39632

to look at the new patch set (#2).

Change subject: ft2232_spi.c - change the chunksize from 256 to 270. Tabs right now.
......................................................................

ft2232_spi.c - change the chunksize from 256 to 270. Tabs right now.

For details check:
https://mail.coreboot.org/hyperkitty/list/[e-mail]/message/W2HU...

Signed-off-by: Simon Buhrow 
Change-Id: Iac067a23025e9df053ab9cd4e82a98de70046c18
---
M ft2232_spi.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39632/2

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270. This is to reall...

[el-coder-sb (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/39632

to look at the new patch set (#3).

Change subject: ft2232_spi.c - change the chunksize from 256 to 270. This is to really make use of page write time advantage. Because the Chunksize must be 256Byte raw data plus the address and cmd bytes.
......................................................................

ft2232_spi.c - change the chunksize from 256 to 270.
This is to really make use of page write time advantage.
Because the Chunksize must be 256Byte raw data plus the address and cmd bytes.

For details check:
https://mail.coreboot.org/hyperkitty/list/[e-mail]/message/W2HU...

Signed-off-by: Simon Buhrow 
Change-Id: Iac067a23025e9df053ab9cd4e82a98de70046c18
---
M ft2232_spi.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39632/3

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270.

[el-coder-sb (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/39632

to look at the new patch set (#4).

Change subject: ft2232_spi.c - change the chunksize from 256 to 270.
......................................................................

ft2232_spi.c - change the chunksize from 256 to 270.

This is to really make use of page write time advantage.
Because the Chunksize must be 256Byte raw data plus the address and cmd bytes.

For details check:
https://mail.coreboot.org/hyperkitty/list/[e-mail]/message/W2HU...

Signed-off-by: Simon Buhrow 
Change-Id: Iac067a23025e9df053ab9cd4e82a98de70046c18
---
M ft2232_spi.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39632/4

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270.

[el-coder-sb (Code Review)]

el-coder-sb has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270.
......................................................................

Patch Set 4:

Thanks Angel,
now I got it :-)

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270.
......................................................................

Patch Set 4: Code-Review+1

(1 comment)

https://review.coreboot.org/c/flashrom/+/39632/4/ft2232_spi.c 
File ft2232_spi.c:

https://review.coreboot.org/c/flashrom/+/39632/4/ft2232_spi.c@400 
PS4, Line 400: 
Could we at least have a comment here explaining where this 270 comes from?

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270.
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/flashrom/+/39632/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/39632/4//COMMIT_MSG@7 
PS4, Line 7: ft2232_spi.c - change the chunksize from 256 to 270.
Nit: do not use a period at the end of a commit summary. Also, use a colon: instead of a hyphen:

ft2232_spi.c: change the chunksize from 256 to 270

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270.

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270.
......................................................................

Patch Set 4: Code-Review+1

This seems fine, however I wonder if it will be better to set the chunksize in ft2232_spi_send_command() where we have flash context and can tell if we're dealing with a large chip with a 4-byte address, since that will require adding another byte to the chunksize?

Change in flashrom[master]: ft2232_spi.c - change the chunksize from 256 to 270.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c - change the chunksize from 256 to 270.
......................................................................

Patch Set 4:
> Patch Set 4: Code-Review+1
>
> This seems fine, however I wonder if it will be better to set the chunksize in ft2232_spi_send_command() where we have flash context and can tell if we're dealing with a large chip with a 4-byte address, since that will require adding another byte to the chunksize?

I am trying it out, it does seem to be faster (it might be placebo effect, though: didn't measure it accurately). I have to check if there's a struct we could use sizeof() on.

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[el-coder-sb (Code Review)]

Hello build bot (Jenkins), David Hendricks, Angel Pons,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/39632

to look at the new patch set (#5).

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

ft2232_spi.c: change the chunksize from 256 to 270

This is to really make use of page write time advantage.
Because the Chunksize must be 256Byte raw data plus the address and cmd bytes.

For details check:
https://mail.coreboot.org/hyperkitty/list/[e-mail]/message/W2HU...

Signed-off-by: Simon Buhrow 
Change-Id: Iac067a23025e9df053ab9cd4e82a98de70046c18
---
M ft2232_spi.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39632/5

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[el-coder-sb (Code Review)]

el-coder-sb has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

Patch Set 5:
> Patch Set 4: Code-Review+1
>
> This seems fine, however I wonder if it will be better to set the chunksize in ft2232_spi_send_command() where we have flash context and can tell if we're dealing with a large chip with a 4-byte address, since that will require adding another byte to the chunksize?

Sure, there might be a better place for this where the value can be set depending on the used address mode and may be further cmd configs. 
With regard to the 4-byte addresse: I chose the 270 as it is for 3-byte and 4-byte addressing. 269 would be only for 3-byte addressing.

Furthermore IMHO the chunksize is not that crititcal unless it is to small (as it is at the moment with the 256 Bytes). If everything is done right up to calling ftdi_write_data() of the libftdi the chunksize is not critical/necessary. Theoretically it could be the default 4096, I think. But 270 is some more reasonable.

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

Patch Set 5:

(1 comment)
> Patch Set 5:
> > Patch Set 4: Code-Review+1
> >
> > This seems fine, however I wonder if it will be better to set the chunksize in ft2232_spi_send_command() where we have flash context and can tell if we're dealing with a large chip with a 4-byte address, since that will require adding another byte to the chunksize?
>
> Sure, there might be a better place for this where the value can be set depending on the used address mode and may be further cmd configs. 
> With regard to the 4-byte addresse: I chose the 270 as it is for 3-byte and 4-byte addressing. 269 would be only for 3-byte addressing.
>
> Furthermore IMHO the chunksize is not that crititcal unless it is to small (as it is at the moment with the 256 Bytes). If everything is done right up to calling ftdi_write_data() of the libftdi the chunksize is not critical/necessary. Theoretically it could be the default 4096, I think. But 270 is some more reasonable.

Right, I agree with your reasoning.

https://review.coreboot.org/c/flashrom/+/39632/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/39632/4//COMMIT_MSG@7 
PS4, Line 7: ft2232_spi.c - change the chunksize from 256 to 270.
> Nit: do not use a period at the end of a commit summary. Also, use a colon: instead of a hyphen: […]

Done

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

Patch Set 5: Code-Review+2
> Patch Set 5:
>
> (1 comment)
> > Patch Set 5:
> > > Patch Set 4: Code-Review+1
> > >
> > > This seems fine, however I wonder if it will be better to set the chunksize in ft2232_spi_send_command() where we have flash context and can tell if we're dealing with a large chip with a 4-byte address, since that will require adding another byte to the chunksize?
> >
> > Sure, there might be a better place for this where the value can be set depending on the used address mode and may be further cmd configs. 
> > With regard to the 4-byte addresse: I chose the 270 as it is for 3-byte and 4-byte addressing. 269 would be only for 3-byte addressing.
> >
> > Furthermore IMHO the chunksize is not that crititcal unless it is to small (as it is at the moment with the 256 Bytes). If everything is done right up to calling ftdi_write_data() of the libftdi the chunksize is not critical/necessary. Theoretically it could be the default 4096, I think. But 270 is some more reasonable.
>
> Right, I agree with your reasoning.

Sounds good to me as well!

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

Patch Set 5: Code-Review+2

Could you please rebase this change? The Verified score got lost, it seems...

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[el-coder-sb (Code Review)]

el-coder-sb has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

Patch Set 5:
> Patch Set 5: Code-Review+2
>
> Could you please rebase this change? The Verified score got lost, it seems...

Is that to me? - If yes, I´m sorry, I have no idea how to rebase...

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 5: Code-Review+2
> >
> > Could you please rebase this change? The Verified score got lost, it seems...
>
> Is that to me? - If yes, I´m sorry, I have no idea how to rebase...

Yes, it was to you. There's a button in the gerrit UI that says Rebase, choose "rebase on top of the master branch"

Change in flashrom[master]: ft2232_spi.c: change the chunksize from 256 to 270

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/39632 )

Change subject: ft2232_spi.c: change the chunksize from 256 to 270
......................................................................

ft2232_spi.c: change the chunksize from 256 to 270

This is to really make use of page write time advantage.
Because the Chunksize must be 256Byte raw data plus the address and cmd bytes.

For details check:
https://mail.coreboot.org/hyperkitty/list/[e-mail]/message/W2HU...

Signed-off-by: Simon Buhrow 
Change-Id: Iac067a23025e9df053ab9cd4e82a98de70046c18
Reviewed-on: https://review.coreboot.org/c/flashrom/+/39632
Tested-by: build bot (Jenkins) 
Reviewed-by: David Hendricks 
Reviewed-by: Angel Pons 
---
M ft2232_spi.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  David Hendricks: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/ft2232_spi.c b/ft2232_spi.c
index 1a5b2fe..520eb6e 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -398,7 +398,7 @@
    	msg_perr("Unable to set latency timer (%s).\n", ftdi_get_error_string(ftdic));
    }

-	if (ftdi_write_data_set_chunksize(ftdic, 256)) {
+	if (ftdi_write_data_set_chunksize(ftdic, 270)) {
    	msg_perr("Unable to set chunk size (%s).\n", ftdi_get_error_string(ftdic));
    }