| Message ID | 2f17ae29e75967b4522b080c275b907622e1d353.1447773299.git.stillcompiling@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 17, 2015 at 07:24:21AM -0800, Joshua Clayton wrote: > default_rx and rx are needlessly different. > Use one buffer, local to transmit() Why? This isn't what I'd expect from black boxing the API, from a userspace point of view the transfer is atomic and in an ideal world we'd be able to do direct to/from memory transfers rather than memcpy() into kernel space which means that userspace should assume the transfers are going on simultaneously even if they don't currently.
On Tuesday, November 17, 2015 05:41:56 PM Mark Brown wrote: > On Tue, Nov 17, 2015 at 07:24:21AM -0800, Joshua Clayton wrote: > > > default_rx and rx are needlessly different. > > Use one buffer, local to transmit() > > Why? This isn't what I'd expect from black boxing the API, from a > userspace point of view the transfer is atomic and in an ideal world > we'd be able to do direct to/from memory transfers rather than memcpy() > into kernel space which means that userspace should assume the transfers > are going on simultaneously even if they don't currently. The important thing here was to get rid of the default_rx buffer. I just noticed that the output can be set up completely within the scope of the transmit function, since the operands are global. But I would be just as happy to set it up at the top level. I'll change this in V2 -- ~Joshua Clayton -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/spi/spidev_test.c b/Documentation/spi/spidev_test.c index 135b3f5..dfe8f47 100644 --- a/Documentation/spi/spidev_test.c +++ b/Documentation/spi/spidev_test.c @@ -46,7 +46,6 @@ uint8_t default_tx[] = { 0xF0, 0x0D, }; -uint8_t default_rx[ARRAY_SIZE(default_tx)] = {0, }; char *input_tx; static void hex_dump(const void *src, size_t length, size_t line_size, char *prefix) @@ -100,10 +99,10 @@ static int unescape(char *_dst, char *_src, size_t len) return ret; } -static void transfer(int fd, uint8_t const *tx, uint8_t const *rx, size_t len) +static void transfer(int fd, uint8_t const *tx, size_t len) { int ret; - + uint8_t *rx = malloc(len); struct spi_ioc_transfer tr = { .tx_buf = (unsigned long)tx, .rx_buf = (unsigned long)rx, @@ -135,6 +134,7 @@ static void transfer(int fd, uint8_t const *tx, uint8_t const *rx, size_t len) if (verbose) hex_dump(tx, len, 32, "TX"); hex_dump(rx, len, 32, "RX"); + free(rx); } static void print_usage(const char *prog) @@ -254,7 +254,6 @@ int main(int argc, char *argv[]) int ret = 0; int fd; uint8_t *tx; - uint8_t *rx; int size; parse_opts(argc, argv); @@ -303,13 +302,11 @@ int main(int argc, char *argv[]) if (input_tx) { size = strlen(input_tx+1); tx = malloc(size); - rx = malloc(size); size = unescape((char *)tx, input_tx, size); - transfer(fd, tx, rx, size); - free(rx); + transfer(fd, tx, size); free(tx); } else { - transfer(fd, default_tx, default_rx, sizeof(default_tx)); + transfer(fd, default_tx, sizeof(default_tx)); } close(fd);
default_rx and rx are needlessly different. Use one buffer, local to transmit() Signed-off-by: Joshua Clayton <stillcompiling@gmail.com> --- Documentation/spi/spidev_test.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)