Skip to contentSkip to conversation

FSL SPI read fix.

PATCH In review Dale Smith 6 participants 9 messages Sep 26, 2012 – Dec 14, 2015 SPI

lore
Share & tools
lore

Dale Smith

author
#0 lore raw

PATCH 1/1 FSL SPI read fix.

The fsl spi engine is non functional when reading from a device. This patch fixes it.

Note that none of the other spi interfaces parse through the datastream looking for 0x0b bytes.

-Dale

drivers/spi/fsl_espi.c +0 -0
index a1ebd33..737719b 100644
@@ -291,17 +291,10 @@ int spi_xfer(struct spi_slave *slave, unsigned

int bitlen, const void *data_out,
debug("***spi_xfer:...%08x readed\n", tmpdin);
}
}
- if (data_in) {
- memcpy(data_in, buffer + 2 * cmd_len, tran_len);
- if (*buffer == 0x0b) {
- data_in += tran_len;
- data_len -= tran_len;
- *(int *)buffer += tran_len;
- }
- }
spi_cs_deactivate(slave);
}
-
+ if (data_in)
+ memcpy(data_in, buffer + rx_offset, len);
free(buffer);
return 0;
}

#1 lore raw

Hi,

Can you please update the commit header and logic of the code w.r.t current master tree.

also please use proper commit body.

--
Thanks,
Jagan.

Quoted 36 lines · Dale Smith

The fsl spi engine is non functional when reading from a device. This patch fixes it.

Note that none of the other spi interfaces parse through the datastream looking for 0x0b bytes.

(quoted diff) +0 -1
-Dale
 
  }
drivers/spi/fsl_espi.c +2 -9
index a1ebd33..737719b 100644
@@ -291,17 +291,10 @@ int spi_xfer(struct spi_slave *slave, unsigned
 int bitlen, const void *data_out,
  				debug("***spi_xfer:...%08x readed\n", tmpdin);
  			}
  		}
-		if (data_in) {
-			memcpy(data_in, buffer + 2 * cmd_len, tran_len);
-			if (*buffer == 0x0b) {
-				data_in += tran_len;
-				data_len -= tran_len;
-				*(int *)buffer += tran_len;
-			}
-		}
  		spi_cs_deactivate(slave);
  	}
-
+	if (data_in)
+		memcpy(data_in, buffer + rx_offset, len);
  	free(buffer);
  	return 0;
#2 lore raw

PATCH 1/1 FSL espi: Return all data read from device unmodified.

Signed-off-by Dale P. Smith <[email protected]>
---
 drivers/spi/fsl_espi.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)
drivers/spi/fsl_espi.c +2 -8
index 28609ee..bd9dc64 100644
@@ -287,17 +287,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *data_out,
 				debug("***spi_xfer:...%08x readed\n", tmpdin);
 			}
 		}
-		if (data_in) {
-			memcpy(data_in, buffer + 2 * cmd_len, tran_len);
-			if (*buffer == 0x0b) {
-				data_in += tran_len;
-				data_len -= tran_len;
-				*(int *)buffer += tran_len;
-			}
-		}
 		spi_cs_deactivate(slave);
 	}
 
+	if (data_in)
+		memcpy(data_in, buffer + rx_offset, len);
 	free(buffer);
 	return 0;
 }
Signature · 1 line
1.7.2.5
#3 lore raw

PATCH 1/1 FSL espi: Return all data read from device unmodified.

Hi,

Thanks for sending this.

Few comments.
1. Please use subject prefix with version number this case it should be "PATH v3"
2. Commit header should be stand' one as all follows.
" spi: fsl_espi: "

Quoted 19 lines · Dale P. Smith
Signed-off-by Dale P. Smith <[email protected]>
---
 drivers/spi/fsl_espi.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)
drivers/spi/fsl_espi.c +0 -5
index 28609ee..bd9dc64 100644
@@ -287,17 +287,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *data_out,
                                debug("***spi_xfer:...%08x readed\n", tmpdin);
                        }
                }
-               if (data_in) {
-                       memcpy(data_in, buffer + 2 * cmd_len, tran_len);
-                       if (*buffer == 0x0b) {
-                               data_in += tran_len;
-                               data_len -= tran_len;

drivers/spi/fsl_espi.c int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *data_out, · line 296

Quoted 10 lines
drivers/spi/fsl_espi.c +2 -3
-                               *(int *)buffer += tran_len;
-                       }
-               }
                spi_cs_deactivate(slave);
        }
 
+       if (data_in)
+               memcpy(data_in, buffer + rx_offset, len);
        free(buffer);
        return 0;

Have tested this recently?

Thanks,
Jagan.

#4 lore raw

PATCH 1/1 FSL espi: Return all data read from device unmodified.

Hi Jagan, Dale,

Quoted 7 lines · Jagan Teki

Hi,

Thanks for sending this.

Few comments.
1. Please use subject prefix with version number this case it should be "PATH v3"

Plus if it is v3, then it should have patch history too.

Amicalement,

Signature · 1 line
Albert.
#5 lore raw

PATCH 1/1 FSL espi: Return all data read from device unmodified.

Quoted 7 lines · Albert ARIBAUD

Hi Jagan, Dale,

Quoted 2 lines · Jagan Teki

Hi,

Quoted 8 lines
Quoted 5 lines

Thanks for sending this.

Few comments.
1. Please use subject prefix with version number this case it should be "PATH v3"

Plus if it is v3, then it should have patch history too.

Thanks Amicalement,

Hi Dale,

Please refer these 3 points for sending your v3.
I am just pointing to a link for sending version patches ignore this if you know it already
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

--
Thanks,
Jagan.

#6 lore raw

PATCH v3 1/1 spi: fsl_espi: Return all data read from device unmodified

In reply to Dale P. Smith
Signed-off-by Dale P. Smith <[email protected]>

---
Changes for v2:
- First attempt at using git format-patch
Changes for v3:
- Fix subject.
- Add changelog

 drivers/spi/fsl_espi.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/spi/fsl_espi.c +2 -8
index b1586d1..c84a7ea 100644
@@ -345,17 +345,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *data_out,
 				}
 			}
 		}
-		if (data_in) {
-			memcpy(data_in, buffer + 2 * cmd_len, tran_len);
-			if (*buffer == 0x0b) {
-				data_in += tran_len;
-				data_len -= tran_len;
-				*(int *)buffer += tran_len;
-			}
-		}
 		spi_cs_deactivate(slave);
 	}
 
+	if (data_in)
+		memcpy(data_in, buffer + rx_offset, len);
 	free(buffer);
 	return 0;
 }
Signature · 1 line
2.1.4
#7 lore raw

PATCH v3 1/1 spi: fsl_espi: Return all data read from device unmodified

Quoted 7 lines · Dale P. Smith
Signed-off-by Dale P. Smith <[email protected]>

---
Changes for v2:
???- First attempt at using git format-patch
Changes for v3:
???- Fix subject.
???- Add changelog

While this is a step in the right direction, this driver needs a rewrite.
- The malloc/memcpy crap need to go.
- TXing 4 bytes a time while while word size is still one byte makes Not full/Not empty ? HW flags useless.
There is no real maintainer of this driver though so I am afraid nobody will do make this happen.

Quoted 28 lines

?drivers/spi/fsl_espi.c | 10 ++--------
?1 file changed, 2 insertions(+), 8 deletions(-)

drivers/spi/fsl_espi.c +0 -8
index b1586d1..c84a7ea 100644
@@ -345,17 +345,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *data_out,
 ?				}
 ?			}
 ?		}
-		if (data_in) {
-			memcpy(data_in, buffer + 2 * cmd_len, tran_len);
-			if (*buffer == 0x0b) {
-				data_in += tran_len;
-				data_len -= tran_len;
-				*(int *)buffer += tran_len;
-			}
-		}
 ?		spi_cs_deactivate(slave);
 ?	}

?

drivers/spi/fsl_espi.c +2 -0
+	if (data_in)
+		memcpy(data_in, buffer + rx_offset, len);

? free(buffer);
? return 0;
?}

#8 lore raw

PATCH v3 1/1 spi: fsl_espi: Return all data read from device unmodified

Quoted 9 lines · Joakim Tjernlund
Quoted 7 lines · Dale P. Smith
Signed-off-by Dale P. Smith <[email protected]>

---
Changes for v2:
- First attempt at using git format-patch
Changes for v3:
- Fix subject.
- Add changelog

Quoted 5 lines

While this is a step in the right direction, this driver needs a rewrite.
- The malloc/memcpy crap need to go.
- TXing 4 bytes a time while while word size is still one byte makes Not full/Not empty HW flags useless.
There is no real maintainer of this driver though so I am afraid nobody will do make this happen.

Please try to write it in driver model format, let me know for any help.

thanks!

Signature · 1 line
Jagan.