Patchwork Use a distinct return code for invalid SPI command length

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2009-07-14 10:01:57
Message ID <4A5C5795.3050602@gmx.net>
Download mbox | patch
Permalink /patch/32/
State Accepted
Commit r653
Headers show

Comments

Carl-Daniel Hailfinger - 2009-07-14 10:01:57
Use a distinct return code for SPI commands with unsupported/invalid length.
Some drivers support only a few combinations of read/write length and
return error otherwise. Having a distinct return code for this error
means we can handle it in upper layers.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Reinauer - 2009-07-14 10:09:05
On 14.07.2009 12:01 Uhr, Carl-Daniel Hailfinger wrote:
> Use a distinct return code for SPI commands with unsupported/invalid length.
> Some drivers support only a few combinations of read/write length and
> return error otherwise. Having a distinct return code for this error
> means we can handle it in upper layers.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>   

Maybe it would be better style to use an enum instead of #defines but
that's a different story.

Acked-by: Stefan Reinauer <stepan@coresystems.de>
Carl-Daniel Hailfinger - 2009-07-14 10:30:50
On 14.07.2009 12:09, Stefan Reinauer wrote:
> On 14.07.2009 12:01 Uhr, Carl-Daniel Hailfinger wrote:
>   
>> Use a distinct return code for SPI commands with unsupported/invalid length.
>> Some drivers support only a few combinations of read/write length and
>> return error otherwise. Having a distinct return code for this error
>> means we can handle it in upper layers.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>  
>>     
>
> Maybe it would be better style to use an enum instead of #defines but
> that's a different story.
>   

There are quite a few #defines which would benefit from being enums, but
here I see no way to have enums count backwards. We could make the error
codes positive values and return -ESOMETHING if we really want to
convert to enums here.

> Acked-by: Stefan Reinauer <stepan@coresystems.de>
>   

Thanks, r653.

Regards,
Carl-Daniel

Patch

Index: flashrom-spi_commandlength/it87spi.c
===================================================================
--- flashrom-spi_commandlength/it87spi.c	(Revision 652)
+++ flashrom-spi_commandlength/it87spi.c	(Arbeitskopie)
@@ -170,7 +170,7 @@ 
 	if (readcnt > 3) {
 		printf("%s called with unsupported readcnt %i.\n",
 		       __FUNCTION__, readcnt);
-		return 1;
+		return SPI_INVALID_LENGTH;
 	}
 	switch (writecnt) {
 	case 1:
@@ -200,7 +200,7 @@ 
 	default:
 		printf("%s called with unsupported writecnt %i.\n",
 		       __FUNCTION__, writecnt);
-		return 1;
+		return SPI_INVALID_LENGTH;
 	}
 	/*
 	 * Start IO, 33 or 16 MHz, readcnt input bytes, writecnt output bytes.
Index: flashrom-spi_commandlength/ft2232_spi.c
===================================================================
--- flashrom-spi_commandlength/ft2232_spi.c	(Revision 652)
+++ flashrom-spi_commandlength/ft2232_spi.c	(Arbeitskopie)
@@ -201,6 +201,9 @@ 
 	unsigned char port_val = 0;
 	int i, ret = 0;
 
+	if (writecnt > 65536 || readcnt > 65536)
+		return SPI_INVALID_LENGTH;
+
 	buf = realloc(buf, writecnt + readcnt + 100);
 	if (!buf) {
 		fprintf(stderr, "Out of memory!\n");
Index: flashrom-spi_commandlength/wbsio_spi.c
===================================================================
--- flashrom-spi_commandlength/wbsio_spi.c	(Revision 652)
+++ flashrom-spi_commandlength/wbsio_spi.c	(Arbeitskopie)
@@ -154,7 +154,8 @@ 
 	if (!mode) {
 		fprintf(stderr, "%s: unsupported command type wr=%d rd=%d\n",
 			__func__, writecnt, readcnt);
-		return 1;
+		/* Command type refers to the number of bytes read/written. */
+		return SPI_INVALID_LENGTH;
 	}
 
 	OUTB(writearr[0], wbsio_spibase);
Index: flashrom-spi_commandlength/spi.h
===================================================================
--- flashrom-spi_commandlength/spi.h	(Revision 652)
+++ flashrom-spi_commandlength/spi.h	(Arbeitskopie)
@@ -108,5 +108,6 @@ 
 /* Error codes */
 #define SPI_INVALID_OPCODE	-2
 #define SPI_INVALID_ADDRESS	-3
+#define SPI_INVALID_LENGTH	-4
 
 #endif		/* !__SPI_H__ */
Index: flashrom-spi_commandlength/sb600spi.c
===================================================================
--- flashrom-spi_commandlength/sb600spi.c	(Revision 652)
+++ flashrom-spi_commandlength/sb600spi.c	(Arbeitskopie)
@@ -114,14 +114,14 @@ 
 
 	if (readcnt > 8) {
 		printf("%s, SB600 SPI controller can not receive %d bytes, "
-		       "which is limited with 8 bytes\n", __func__, readcnt);
-		return 1;
+		       "it is limited to 8 bytes\n", __func__, readcnt);
+		return SPI_INVALID_LENGTH;
 	}
 
 	if (writecnt > 8) {
-		printf("%s, SB600 SPI controller can not sent %d bytes, "
-		       "which is limited with 8 bytes\n", __func__, writecnt);
-		return 1;
+		printf("%s, SB600 SPI controller can not send %d bytes, "
+		       "it is limited to 8 bytes\n", __func__, writecnt);
+		return SPI_INVALID_LENGTH;
 	}
 
 	mmio_writeb(cmd, sb600_spibar + 0);
Index: flashrom-spi_commandlength/ichspi.c
===================================================================
--- flashrom-spi_commandlength/ichspi.c	(Revision 652)
+++ flashrom-spi_commandlength/ichspi.c	(Arbeitskopie)
@@ -599,10 +599,16 @@ 
 {
 	switch (spi_controller) {
 	case SPI_CONTROLLER_VIA:
+		if (datalength > 16)
+			return SPI_INVALID_LENGTH;
 		return ich7_run_opcode(op, offset, datalength, data, 16);
 	case SPI_CONTROLLER_ICH7:
+		if (datalength > 64)
+			return SPI_INVALID_LENGTH;
 		return ich7_run_opcode(op, offset, datalength, data, 64);
 	case SPI_CONTROLLER_ICH9:
+		if (datalength > 64)
+			return SPI_INVALID_LENGTH;
 		return ich9_run_opcode(op, offset, datalength, data);
 	default:
 		printf_debug("%s: unsupported chipset\n", __FUNCTION__);
@@ -686,6 +692,7 @@ 
 		    const unsigned char *writearr, unsigned char *readarr)
 {
 	int a;
+	int result;
 	int opcode_index = -1;
 	const unsigned char cmd = *writearr;
 	OPCODE *opcode;
@@ -728,10 +735,10 @@ 
 		count = readcnt;
 	}
 
-	if (run_opcode(*opcode, addr, count, data) != 0) {
+	result = run_opcode(*opcode, addr, count, data);
+	if (result) {
 		printf_debug("run OPCODE 0x%02x failed\n", opcode->opcode);
-		return 1;
 	}
 
-	return 0;
+	return result;
 }