Skip to contentSkip to conversation

spi: Add file i/o to spidev_test

PATCH Superseded Joshua Clayton 4 participants 35 messages Nov 17, 2015 – Nov 23, 2015 5 messages not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover spi: Add file i/o to spidev_test

Add to spidev_test, the ability to read from and write to a file. Clean up code along the way.
As I finished, I found that a request has been made to move spidev_test from Documentation to tools, so I did that as well.

Joshua Clayton (8):
Documentation/spi/spidev_test.c: use one rx buffer Documentation/spi/spidev_test.c: clean up input_tx Documentation/spi/spidev_test.c: accept input from a file Documentation/spi/spidev_test.c: output to a file Documentation/spi/spidev_test.c: check error Documentation/spi/spidev_test.c: fix whitespace tools/Makefile: minor whitespace cleanup
spi: Move spi code from Documentation to tools

 Documentation/Makefile          |   2 +-
 Documentation/spi/Makefile      |   8 -
 Documentation/spi/spidev_fdx.c  | 158 -----------------
 Documentation/spi/spidev_test.c | 318 ---------------------------------
 tools/Makefile                  |  43 ++++-
 tools/spi/Makefile              |   4 +
 tools/spi/spidev_fdx.c          | 158 +++++++++++++++++
 tools/spi/spidev_test.c         | 385 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 582 insertions(+), 494 deletions(-)
 delete mode 100644 Documentation/spi/Makefile
 delete mode 100644 Documentation/spi/spidev_fdx.c
 delete mode 100644 Documentation/spi/spidev_test.c
 create mode 100644 tools/spi/Makefile
 create mode 100644 tools/spi/spidev_fdx.c
 create mode 100644 tools/spi/spidev_test.c
Signature · 6 lines
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw

PATCH 1/8 Documentation/spi/spidev_test.c: use one rx buffer

default_rx and rx are needlessly different.
Use one buffer, local to transmit()

---
 Documentation/spi/spidev_test.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
Documentation/spi/spidev_test.c +5 -8
index 135b3f5..dfe8f47 100644
@@ -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);
Signature · 1 line
2.5.0
#4 lore raw

PATCH 2/8 Documentation/spi/spidev_test.c: clean up input_tx

In reply to Joshua Clayton

Put input from string into its own function.

---
 Documentation/spi/spidev_test.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
Documentation/spi/spidev_test.c +13 -10
index dfe8f47..1ed9110 100644
@@ -249,12 +249,20 @@ static void parse_opts(int argc, char *argv[])
 	}
 }
 
+static void transfer_escaped_string(int fd, char *str)
+{
+	size_t size = strlen(str + 1);
+	uint8_t *tx = malloc(size);
+
+	size = unescape((char *)tx, str, size);
+	transfer(fd, tx, size);
+	free(tx);
+}
+
 int main(int argc, char *argv[])
 {
 	int ret = 0;
 	int fd;
-	uint8_t *tx;
-	int size;
 
 	parse_opts(argc, argv);
 
@@ -299,15 +307,10 @@ int main(int argc, char *argv[])
 	printf("bits per word: %d\n", bits);
 	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
-	if (input_tx) {
-		size = strlen(input_tx+1);
-		tx = malloc(size);
-		size = unescape((char *)tx, input_tx, size);
-		transfer(fd, tx, size);
-		free(tx);
-	} else {
+	if (input_tx)
+		transfer_escaped_string(fd, input_tx);
+	else
 		transfer(fd, default_tx, sizeof(default_tx));
-	}
 
 	close(fd);
 
Signature · 1 line
2.5.0
#8 lore raw

PATCH 3/8 Documentation/spi/spidev_test.c: accept input from a file

In reply to Joshua Clayton

Add input file support to facilitate testing larger data.

---
 Documentation/spi/spidev_test.c | 42 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)
Documentation/spi/spidev_test.c +41 -1
index 1ed9110..ef812ad 100644
@@ -19,6 +19,7 @@
 #include <getopt.h>
 #include <fcntl.h>
 #include <sys/ioctl.h>
+#include <sys/stat.h>
 #include <linux/types.h>
 #include <linux/spi/spidev.h>
 
@@ -33,6 +34,7 @@ static void pabort(const char *s)
 static const char *device = "/dev/spidev1.1";
 static uint32_t mode;
 static uint8_t bits = 8;
+static char *input_file;
 static uint32_t speed = 500000;
 static uint16_t delay;
 static int verbose;
@@ -144,6 +146,7 @@ static void print_usage(const char *prog)
 	     "  -s --speed    max speed (Hz)\n"
 	     "  -d --delay    delay (usec)\n"
 	     "  -b --bpw      bits per word \n"
+	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
 	     "  -l --loop     loopback\n"
 	     "  -H --cpha     clock phase\n"
 	     "  -O --cpol     clock polarity\n"
@@ -167,6 +170,7 @@ static void parse_opts(int argc, char *argv[])
 			{ "speed",   1, 0, 's' },
 			{ "delay",   1, 0, 'd' },
 			{ "bpw",     1, 0, 'b' },
+			{ "input",   1, 0, 'i' },
 			{ "loop",    0, 0, 'l' },
 			{ "cpha",    0, 0, 'H' },
 			{ "cpol",    0, 0, 'O' },
@@ -182,7 +186,8 @@ static void parse_opts(int argc, char *argv[])
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:lHOLC3NR24p:v", lopts, NULL);
+		c = getopt_long(argc, argv, "D:s:d:b:i:lHOLC3NR24p:v",
+				lopts, NULL);
 
 		if (c == -1)
 			break;
@@ -200,6 +205,9 @@ static void parse_opts(int argc, char *argv[])
 		case 'b':
 			bits = atoi(optarg);
 			break;
+		case 'i':
+			input_file = optarg;
+			break;
 		case 'l':
 			mode |= SPI_LOOP;
 			break;
@@ -259,6 +267,33 @@ static void transfer_escaped_string(int fd, char *str)
 	free(tx);
 }
 
+static void transfer_file(int fd, char *filename)
+{
+	ssize_t bytes;
+	struct stat sb;
+	int tx_fd;
+	uint8_t *tx;
+
+	if (stat(filename, &sb) == -1)
+		pabort("can't stat input file");
+
+	if (sb.st_size > 4096)
+		pabort("input file exceeds spidev's 4k limit");
+
+	tx_fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		pabort("can't open input file");
+
+	tx = malloc(sb.st_size);
+	bytes = read(tx_fd, tx, sb.st_size);
+	if (bytes != sb.st_size)
+		pabort("failed to read input file");
+
+	transfer(fd, tx, sb.st_size);
+	free(tx);
+	close(tx_fd);
+}
+
 int main(int argc, char *argv[])
 {
 	int ret = 0;
@@ -307,8 +342,13 @@ int main(int argc, char *argv[])
 	printf("bits per word: %d\n", bits);
 	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
+	if (input_tx && input_file)
+		pabort("only one of -p and --input may be selected");
+
 	if (input_tx)
 		transfer_escaped_string(fd, input_tx);
+	else if (input_file)
+		transfer_file(fd, input_file);
 	else
 		transfer(fd, default_tx, sizeof(default_tx));
 
Signature · 1 line
2.5.0
#12 lore raw

PATCH 4/8 Documentation/spi/spidev_test.c: output to a file

In reply to Joshua Clayton

For testing of larger data transfers, output unmodified data directly to a file.

---
 Documentation/spi/spidev_test.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)
Documentation/spi/spidev_test.c +24 -2
index ef812ad..273f667 100644
@@ -35,6 +35,7 @@ static const char *device = "/dev/spidev1.1";
 static uint32_t mode;
 static uint8_t bits = 8;
 static char *input_file;
+static char *output_file;
 static uint32_t speed = 500000;
 static uint16_t delay;
 static int verbose;
@@ -104,6 +105,7 @@ static int unescape(char *_dst, char *_src, size_t len)
 static void transfer(int fd, uint8_t const *tx, size_t len)
 {
 	int ret;
+	int out_fd;
 	uint8_t *rx = malloc(len);
 	struct spi_ioc_transfer tr = {
 		.tx_buf = (unsigned long)tx,
@@ -135,7 +137,22 @@ static void transfer(int fd, uint8_t const *tx, size_t len)
 
 	if (verbose)
 		hex_dump(tx, len, 32, "TX");
-	hex_dump(rx, len, 32, "RX");
+
+	if (output_file) {
+		out_fd = open(output_file, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+		if (out_fd < 0)
+			pabort("could not open output file");
+
+		ret = write(out_fd, rx, len);
+		if (ret != len)
+			pabort("not all bytes written to utput file");
+
+		close(out_fd);
+	}
+
+	if (verbose || !output_file)
+		hex_dump(rx, len, 32, "RX");
+
 	free(rx);
 }
 
@@ -147,6 +164,7 @@ static void print_usage(const char *prog)
 	     "  -d --delay    delay (usec)\n"
 	     "  -b --bpw      bits per word \n"
 	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
+	     "  -o --output   output data to a file (e.g. \"results.bin\")\n"
 	     "  -l --loop     loopback\n"
 	     "  -H --cpha     clock phase\n"
 	     "  -O --cpol     clock polarity\n"
@@ -171,6 +189,7 @@ static void parse_opts(int argc, char *argv[])
 			{ "delay",   1, 0, 'd' },
 			{ "bpw",     1, 0, 'b' },
 			{ "input",   1, 0, 'i' },
+			{ "output",  1, 0, 'o' },
 			{ "loop",    0, 0, 'l' },
 			{ "cpha",    0, 0, 'H' },
 			{ "cpol",    0, 0, 'O' },
@@ -186,7 +205,7 @@ static void parse_opts(int argc, char *argv[])
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:i:lHOLC3NR24p:v",
+		c = getopt_long(argc, argv, "D:s:d:b:i:o:lHOLC3NR24p:v",
 				lopts, NULL);
 
 		if (c == -1)
@@ -208,6 +227,9 @@ static void parse_opts(int argc, char *argv[])
 		case 'i':
 			input_file = optarg;
 			break;
+		case 'o':
+			output_file = optarg;
+			break;
 		case 'l':
 			mode |= SPI_LOOP;
 			break;
Signature · 6 lines
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#13 lore raw

PATCH 5/8 Documentation/spi/spidev_test.c: check error

In reply to Joshua Clayton

Check the result of sscanf to verify a result was found. report and error and abort if pattern was not found.

---
 Documentation/spi/spidev_test.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Documentation/spi/spidev_test.c +5 -1
index 273f667..2ff5fe8 100644
@@ -85,13 +85,17 @@ static void hex_dump(const void *src, size_t length, size_t line_size, char *pre
 static int unescape(char *_dst, char *_src, size_t len)
 {
 	int ret = 0;
+	int match;
 	char *src = _src;
 	char *dst = _dst;
 	unsigned int ch;
 
 	while (*src) {
 		if (*src == '\\' && *(src+1) == 'x') {
-			sscanf(src + 2, "%2x", &ch);
+			match = sscanf(src + 2, "%2x", &ch);
+			if (!match)
+				pabort("malformed input string");
+
 			src += 4;
 			*dst++ = (unsigned char)ch;
 		} else {
Signature · 6 lines
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#14 lore raw

PATCH 6/8 Documentation/spi/spidev_test.c: fix whitespace

In reply to Joshua Clayton
---
 Documentation/spi/spidev_test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Documentation/spi/spidev_test.c +3 -2
index 2ff5fe8..bf6070c 100644
@@ -51,7 +51,8 @@ uint8_t default_tx[] = {
 
 char *input_tx;
 
-static void hex_dump(const void *src, size_t length, size_t line_size, char *prefix)
+static void hex_dump(const void *src, size_t length, size_t line_size,
+		     char *prefix)
 {
 	int i = 0;
 	const unsigned char *address = src;
@@ -166,7 +167,7 @@ static void print_usage(const char *prog)
 	puts("  -D --device   device to use (default /dev/spidev1.1)\n"
 	     "  -s --speed    max speed (Hz)\n"
 	     "  -d --delay    delay (usec)\n"
-	     "  -b --bpw      bits per word \n"
+	     "  -b --bpw      bits per word\n"
 	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
 	     "  -o --output   output data to a file (e.g. \"results.bin\")\n"
 	     "  -l --loop     loopback\n"
Signature · 1 line
2.5.0
#15 lore raw

PATCH 7/8 tools/Makefile: minor whitespace cleanup

In reply to Joshua Clayton
---
 tools/Makefile | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)
tools/Makefile +30 -7
index 9a617ad..428fb4d 100644
@@ -80,10 +80,21 @@ turbostat_install x86_energy_perf_policy_install:
 tmon_install:
 	$(call descend,thermal/$(@:_install=),install)
 
-install: acpi_install cgroup_install cpupower_install hv_install firewire_install lguest_install \
-		perf_install selftests_install turbostat_install usb_install \
-		virtio_install vm_install net_install x86_energy_perf_policy_install \
-	tmon
+install: acpi_install \
+		cgroup_install \
+		cpupower_install \
+		hv_install \
+		firewire_install \
+		lguest_install \
+		perf_install \
+		selftests_install \
+		tmon \
+		turbostat_install \
+		usb_install \
+		virtio_install \
+		vm_install \
+		net_install \
+		x86_energy_perf_policy_install
 
 acpi_clean:
 	$(call descend,power/acpi,clean)
@@ -112,8 +123,20 @@ turbostat_clean x86_energy_perf_policy_clean:
 tmon_clean:
 	$(call descend,thermal/tmon,clean)
 
-clean: acpi_clean cgroup_clean cpupower_clean hv_clean firewire_clean lguest_clean \
-		perf_clean selftests_clean turbostat_clean usb_clean virtio_clean \
-		vm_clean net_clean x86_energy_perf_policy_clean tmon_clean
+clean: acpi_clean \
+		cgroup_clean \
+		cpupower_clean \
+		hv_clean \
+		firewire_clean \
+		lguest_clean \
+		perf_clean \
+		selftests_clean \
+		tmon_clean \
+		turbostat_clean \
+		usb_clean \
+		virtio_clean \
+		vm_clean \
+		net_clean \
+		x86_energy_perf_policy_clean
 
 .PHONY: FORCE
Signature · 1 line
2.5.0
#18 lore raw

PATCH 8/8 spi: Move spi code from Documentation to tools

In reply to Joshua Clayton

Jon Corbet requested this code moved with the last changeset,
https://lkml.org/lkml/2015/3/1/144,
but the patch was not applied because it missed the Makefile. Moved spidev_test, spidev_fdx and their Makefile infrastructure.

---
 Documentation/Makefile          |   2 +-
 Documentation/spi/Makefile      |   8 -
 Documentation/spi/spidev_fdx.c  | 158 -----------------
 Documentation/spi/spidev_test.c | 385 ----------------------------------------
 tools/Makefile                  |   6 +-
 tools/spi/Makefile              |   4 +
 tools/spi/spidev_fdx.c          | 158 +++++++++++++++++
 tools/spi/spidev_test.c         | 385 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 552 insertions(+), 554 deletions(-)
 delete mode 100644 Documentation/spi/Makefile
 delete mode 100644 Documentation/spi/spidev_fdx.c
 delete mode 100644 Documentation/spi/spidev_test.c
 create mode 100644 tools/spi/Makefile
 create mode 100644 tools/spi/spidev_fdx.c
 create mode 100644 tools/spi/spidev_test.c
Documentation/Makefile +1 -1
index 6883a1b..f642cb9 100644
@@ -1,4 +1,4 @@
 subdir-y := accounting arm auxdisplay blackfin connector \
 	filesystems filesystems ia64 laptops mic misc-devices \
-	networking pcmcia prctl ptp spi timers vDSO video4linux \
+	networking pcmcia prctl ptp timers vDSO video4linux \
 	watchdog
Documentation/spi/Makefile deleted +0 -8
deleted file mode 100644
index efa2558..0000000
@@ -1,8 +0,0 @@
-# List of programs to build
-hostprogs-y := spidev_test spidev_fdx
-
-# Tell kbuild to always build the programs
-always := $(hostprogs-y)
-
-HOSTCFLAGS_spidev_test.o += -I$(objtree)/usr/include
-HOSTCFLAGS_spidev_fdx.o += -I$(objtree)/usr/include
Documentation/spi/spidev_fdx.c deleted +0 -158

158 lines not on this page · raw message

Documentation/spi/spidev_test.c deleted +0 -385

385 lines not on this page · raw message

tools/Makefile +4 -2
index 428fb4d..864762a 100644
@@ -11,6 +11,7 @@ help:
 	@echo '  lguest     - a minimal 32-bit x86 hypervisor'
 	@echo '  perf       - Linux performance measurement and analysis tool'
 	@echo '  selftests  - various kernel selftests'
+	@echo '  spi        - spi tools'
 	@echo '  turbostat  - Intel CPU idle stats and freq reporting tool'
 	@echo '  usb        - USB testing tools'
 	@echo '  virtio     - vhost test module'
@@ -41,7 +42,7 @@ acpi: FORCE
 cpupower: FORCE
 	$(call descend,power/$@)
 
-cgroup firewire hv guest usb virtio vm net: FORCE
+cgroup firewire hv guest spi usb virtio vm net: FORCE
 	$(call descend,$@)
 
 liblockdep: FORCE
@@ -102,7 +103,7 @@ acpi_clean:
 cpupower_clean:
 	$(call descend,power/cpupower,clean)
 
-cgroup_clean hv_clean firewire_clean lguest_clean usb_clean virtio_clean vm_clean net_clean:
+cgroup_clean hv_clean firewire_clean lguest_clean spi_clean usb_clean virtio_clean vm_clean net_clean:
 	$(call descend,$(@:_clean=),clean)
 
 liblockdep_clean:
@@ -131,6 +132,7 @@ clean: acpi_clean \
 		lguest_clean \
 		perf_clean \
 		selftests_clean \
+		spi_clean \
 		tmon_clean \
 		turbostat_clean \
 		usb_clean \
tools/spi/Makefile new file +4 -0
new file mode 100644
index 0000000..cd0db62
@@ -0,0 +1,4 @@
+all: spidev_test spidev_fdx
+
+clean:
+	$(RM) spidev_test spidev_fdx
tools/spi/spidev_fdx.c new file +158 -0

158 lines not on this page · raw message

tools/spi/spidev_test.c new file +385 -0

385 lines not on this page · raw message

Signature · 1 line
2.5.0
maintainer
#20 lore raw
In reply to Joshua Clayton
Quoted 6 lines · Joshua Clayton

Documentation/spi/spidev_test.c: use one rx buffer Documentation/spi/spidev_test.c: clean up input_tx Documentation/spi/spidev_test.c: accept input from a file Documentation/spi/spidev_test.c: output to a file Documentation/spi/spidev_test.c: check error Documentation/spi/spidev_test.c: fix whitespace

Please use subjct lines reflecting the style for the subsystem.

#21 lore raw
Quoted 10 lines · Mark Brown
Quoted 6 lines · Joshua Clayton

Documentation/spi/spidev_test.c: use one rx buffer Documentation/spi/spidev_test.c: clean up input_tx Documentation/spi/spidev_test.c: accept input from a file Documentation/spi/spidev_test.c: output to a file Documentation/spi/spidev_test.c: check error Documentation/spi/spidev_test.c: fix whitespace

Please use subjct lines reflecting the style for the subsystem.

OK. Will do.
Assuming "spi: spidev_test: yadda..."
It was a bit unclear due to the location of the file.

Signature · 1 line
~Joshua Clayton
maintainer
#22 lore raw
Quoted 10 lines · Joshua Clayton
Quoted 8 lines · Mark Brown
Quoted 6 lines · Joshua Clayton

Documentation/spi/spidev_test.c: use one rx buffer Documentation/spi/spidev_test.c: clean up input_tx Documentation/spi/spidev_test.c: accept input from a file Documentation/spi/spidev_test.c: output to a file Documentation/spi/spidev_test.c: check error Documentation/spi/spidev_test.c: fix whitespace

Quoted 4 lines
Quoted 1 line

Please use subjct lines reflecting the style for the subsystem.

OK. Will do.
Assuming "spi: spidev_test: yadda..."

Something like that.

Quoted 1 line

It was a bit unclear due to the location of the file.

It's not really the location that you should be focusing on, it's other
commits in the file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151117/a48258ac/attachment.sig>

maintainer
#2 lore raw

Re: [PATCH 1/8] Documentation/spi/spidev_test.c: use one rx buffer

In reply to Joshua Clayton
Quoted 2 lines · Joshua Clayton

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.

maintainer
#5 lore raw

Re: [PATCH 2/8] Documentation/spi/spidev_test.c: clean up input_tx

In reply to Joshua Clayton
Quoted 1 line · Joshua Clayton

Put input from string into its own function.

Again, why are we doing this? I'm having a hard time seeing what the gain is, the amount of code being moved is tiny.

maintainer
#16 lore raw

Re: [PATCH 7/8] tools/Makefile: minor whitespace cleanup

In reply to Joshua Clayton
Quoted 22 lines · Joshua Clayton
---
 tools/Makefile | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)
tools/Makefile +5 -4
index 9a617ad..428fb4d 100644
@@ -80,10 +80,21 @@ turbostat_install x86_energy_perf_policy_install:
 tmon_install:
 	$(call descend,thermal/$(@:_install=),install)
 
-install: acpi_install cgroup_install cpupower_install hv_install firewire_install lguest_install \
-		perf_install selftests_install turbostat_install usb_install \
-		virtio_install vm_install net_install x86_energy_perf_policy_install \
-	tmon
+install: acpi_install \
+		cgroup_install \
+		cpupower_install \
+		hv_install \
+		firewire_install \

tools/Makefile turbostat_install x86_energy_perf_policy_install: · line 97

Quoted 10 lines
tools/Makefile +10 -0
+		lguest_install \
+		perf_install \
+		selftests_install \
+		tmon \
+		turbostat_install \
+		usb_install \
+		virtio_install \
+		vm_install \
+		net_install \
+		x86_energy_perf_policy_install

This isn't a whitespace cleanup, this is a substantial reindentation of the file :( Please ensure your changelogs are accurate and in general try to avoid mixing this sort of invasive stylistic change in with other patch serieses, it reduces the potential for conflicts.

maintainer
#19 lore raw

Re: [PATCH 8/8] spi: Move spi code from Documentation to tools

In reply to Joshua Clayton
Quoted 4 lines · Joshua Clayton

Jon Corbet requested this code moved with the last changeset,
https://lkml.org/lkml/2015/3/1/144,
but the patch was not applied because it missed the Makefile. Moved spidev_test, spidev_fdx and their Makefile infrastructure.

Trivial moves like this should generally come early rather than later in the series so that they are easier to apply, as this is at the end of the series it depends on the entire series.

#9 lore raw

Re: [PATCH 3/8] Documentation/spi/spidev_test.c: accept input from a file

In reply to Joshua Clayton
Quoted 69 lines · Joshua Clayton

Add input file support to facilitate testing larger data.

---
  Documentation/spi/spidev_test.c | 42 ++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 41 insertions(+), 1 deletion(-)
Documentation/spi/spidev_test.c +11 -1
index 1ed9110..ef812ad 100644
@@ -19,6 +19,7 @@
  #include <getopt.h>
  #include <fcntl.h>
  #include <sys/ioctl.h>
+#include <sys/stat.h>
  #include <linux/types.h>
  #include <linux/spi/spidev.h>
 
@@ -33,6 +34,7 @@ static void pabort(const char *s)
  static const char *device = "/dev/spidev1.1";
  static uint32_t mode;
  static uint8_t bits = 8;
+static char *input_file;
  static uint32_t speed = 500000;
  static uint16_t delay;
  static int verbose;
@@ -144,6 +146,7 @@ static void print_usage(const char *prog)
  	     "  -s --speed    max speed (Hz)\n"
  	     "  -d --delay    delay (usec)\n"
  	     "  -b --bpw      bits per word \n"
+	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
  	     "  -l --loop     loopback\n"
  	     "  -H --cpha     clock phase\n"
  	     "  -O --cpol     clock polarity\n"
@@ -167,6 +170,7 @@ static void parse_opts(int argc, char *argv[])
  			{ "speed",   1, 0, 's' },
  			{ "delay",   1, 0, 'd' },
  			{ "bpw",     1, 0, 'b' },
+			{ "input",   1, 0, 'i' },
  			{ "loop",    0, 0, 'l' },
  			{ "cpha",    0, 0, 'H' },
  			{ "cpol",    0, 0, 'O' },
@@ -182,7 +186,8 @@ static void parse_opts(int argc, char *argv[])
  		};
  		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:lHOLC3NR24p:v", lopts, NULL);
+		c = getopt_long(argc, argv, "D:s:d:b:i:lHOLC3NR24p:v",
+				lopts, NULL);
 
  		if (c == -1)
  			break;
@@ -200,6 +205,9 @@ static void parse_opts(int argc, char *argv[])
  		case 'b':
  			bits = atoi(optarg);
  			break;
+		case 'i':
+			input_file = optarg;
+			break;
  		case 'l':
  			mode |= SPI_LOOP;
  			break;
@@ -259,6 +267,33 @@ static void transfer_escaped_string(int fd, char *str)
  	free(tx);
  }
 
+static void transfer_file(int fd, char *filename)
+{

Documentation/spi/spidev_test.c static void transfer_escaped_string(int fd, char *str) · line 281

Quoted 10 lines
Documentation/spi/spidev_test.c +10 -0
+	ssize_t bytes;
+	struct stat sb;
+	int tx_fd;
+	uint8_t *tx;
+
+	if (stat(filename, &sb) == -1)
+		pabort("can't stat input file");
+
+	if (sb.st_size > 4096)
+		pabort("input file exceeds spidev's 4k limit");

This is not a true. IIRC PAGE_SIZE is the default buffer size for spidev, but can be changed using bufsiz module parameter. Just 'insmod spidev bufsiz=X', where X is number of bytes.

Documentation/spi/spidev_test.c static void transfer_escaped_string(int fd, char *str) · line 287

Quoted 6 lines
Documentation/spi/spidev_test.c +6 -0
+
+	tx_fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		pabort("can't open input file");
+
+	tx = malloc(sb.st_size);

It would be good to check new allocations for fail.

Quoted 27 lines
Documentation/spi/spidev_test.c +14 -0
+	bytes = read(tx_fd, tx, sb.st_size);
+	if (bytes != sb.st_size)
+		pabort("failed to read input file");
+
+	transfer(fd, tx, sb.st_size);
+	free(tx);
+	close(tx_fd);
+}
+
  int main(int argc, char *argv[])
  {
  	int ret = 0;
@@ -307,8 +342,13 @@ int main(int argc, char *argv[])
  	printf("bits per word: %d\n", bits);
  	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
+	if (input_tx && input_file)
+		pabort("only one of -p and --input may be selected");
+
  	if (input_tx)
  		transfer_escaped_string(fd, input_tx);
+	else if (input_file)
+		transfer_file(fd, input_file);
  	else
  		transfer(fd, default_tx, sizeof(default_tx));
 
maintainer
#10 lore raw

Re: [PATCH 3/8] Documentation/spi/spidev_test.c: accept input from a file

Quoted 8 lines · Anton Bondarenko

Documentation/spi/spidev_test.c static void transfer_escaped_string(int fd, char *str) · line 281

Quoted 2 lines · Joshua Clayton
Documentation/spi/spidev_test.c +2 -0
+	if (sb.st_size > 4096)
+		pabort("input file exceeds spidev's 4k limit");

This is not a true. IIRC PAGE_SIZE is the default buffer size for spidev, but can be changed using bufsiz module parameter.
Just 'insmod spidev bufsiz=X', where X is number of bytes.

Right, there's also various options for changing PAGE_SIZE on some architectures. I was going to go and check what we actually do here but in general it does seem better to just let the kernel worry about validating things like this - it needs to do that anyway and it means that if someone improves the kernel code to accept larger buffers then the tool will automatically be able to use them instead of requiring people to remember to separately update the tool.

#3 lore raw

Re: [PATCH 1/8] Documentation/spi/spidev_test.c: use one rx buffer

In reply to Mark Brown
Quoted 10 lines · Mark Brown
Quoted 2 lines · Joshua Clayton

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

#6 lore raw

Re: [PATCH 2/8] Documentation/spi/spidev_test.c: clean up input_tx

In reply to Mark Brown
Quoted 5 lines · Mark Brown
Quoted 1 line · Joshua Clayton

Put input from string into its own function.

Again, why are we doing this? I'm having a hard time seeing what the gain is, the amount of code being moved is tiny.

It takes some clutter out of main() whose scope is limited to that little block of code, and because in the next patch we add another (larger)function to the if/else block.

I don't know if it is valid to say "look at the next commit" for justification, but That is the reason.

Signature · 1 line
~Joshua Clayton
#11 lore raw

Re: [PATCH 3/8] Documentation/spi/spidev_test.c: accept input from a file

In reply to Anton Bondarenko
Quoted 6 lines · Anton Bondarenko

Documentation/spi/spidev_test.c static void transfer_escaped_string(int fd, char *str) · line 281

Quoted 2 lines · Joshua Clayton
Documentation/spi/spidev_test.c +2 -0
+	if (sb.st_size > 4096)
+		pabort("input file exceeds spidev's 4k limit");

This is not a true. IIRC PAGE_SIZE is the default buffer size for spidev, but can be changed using bufsiz module parameter. Just 'insmod spidev bufsiz=X', where X is number of bytes.

You are right. I will drop this. As Mark suggests. The ioctl gives a nice error code if the buffer is too big.

...

Quoted 2 lines

Documentation/spi/spidev_test.c static void transfer_escaped_string(int fd, char *str) · line 287

Quoted 1 line
Documentation/spi/spidev_test.c +1 -0
+	tx = malloc(sb.st_size);

It would be good to check new allocations for fail.

I will add a check for this.

Signature · 1 line
~Joshua Clayton
#17 lore raw

Re: [PATCH 7/8] tools/Makefile: minor whitespace cleanup

In reply to Mark Brown
Quoted 30 lines · Mark Brown
Quoted 29 lines · Joshua Clayton
---
 tools/Makefile | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)
tools/Makefile +12 -4
index 9a617ad..428fb4d 100644
@@ -80,10 +80,21 @@ turbostat_install x86_energy_perf_policy_install:
 tmon_install:
 	$(call descend,thermal/$(@:_install=),install)
 
-install: acpi_install cgroup_install cpupower_install hv_install firewire_install lguest_install \
-		perf_install selftests_install turbostat_install usb_install \
-		virtio_install vm_install net_install x86_energy_perf_policy_install \
-	tmon
+install: acpi_install \
+		cgroup_install \
+		cpupower_install \
+		hv_install \
+		firewire_install \
+		lguest_install \
+		perf_install \
+		selftests_install \
+		tmon \
+		turbostat_install \
+		usb_install \
+		virtio_install \
Quoted 8 lines

tools/Makefile turbostat_install x86_energy_perf_policy_install: · line 97

Quoted 3 lines
tools/Makefile +3 -0
+		vm_install \
+		net_install \
+		x86_energy_perf_policy_install

This isn't a whitespace cleanup, this is a substantial reindentation of the file :( Please ensure your changelogs are accurate and in general try to avoid mixing this sort of invasive stylistic change in with other patch serieses, it reduces the potential for conflicts.

Um.
Inability to resist Makefile cleanup is a weakness of mine. It is a tangent from the rest of the series so I'll drop it.

Signature · 1 line
~Joshua Clayton
maintainer
#7 lore raw

Re: [PATCH 2/8] Documentation/spi/spidev_test.c: clean up input_tx

In reply to Joshua Clayton

On Tue, Nov 17, 2015 at 11:21:12AM -0800, Joshua Clayton wrote:

Please fix your mail client to word wrap within paragraphs at something substantially less than 80 columns. Doing this makes your messages much easier to read and reply to.

Quoted 5 lines

It takes some clutter out of main() whose scope is limited to that little block of code, and because in the next patch we add another (larger)function to the if/else block.

I don't know if it is valid to say "look at the next commit" for justification, but That is the reason.

That's totally fine - just say that it's to support future changes in
this area. It's good to split out mechanical changes like this from the
more complex changes, you just need to say why they're happening.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151117/0a2f7f03/attachment.sig>

#23 lore raw

v2 cover spi: Add file i/o to spidev_test

In reply to Joshua Clayton

Add to spidev_test, the ability to read from and write to a file. Clean up code along the way.
As I finished, I found that a request has been made to move spidev_test from Documentation to tools, so I did that as well.

Thanks to Mark Brown and Anton Bodarenko for feedback on v1

v2 Changes:
- Move spidev_test to tools in the first commit instead of the last.
- Drop the patch that formats tools/Makefile
- Drop the patch that changes rx to a local variable in transfer()
- Check the result of malloc everywhere it is called.
- Change all the patch subjects to spi: spidev_test
- Removed a check of the spi transfer buffer size, in favor of letting the kernel give an error in the ioctl if too big.

Joshua Clayton (6):
spi: Move spi code from Documentation to tools
spi: spidev_test: transfer_escaped_string function
spi: spidev_test: accept input from a file
spi: spidev_test: output to a file
spi: spidev_test: check error
spi: spidev_test: fix whitespace

 Documentation/Makefile          |   2 +-
 Documentation/spi/Makefile      |   8 -
 Documentation/spi/spidev_fdx.c  | 158 ----------------
 Documentation/spi/spidev_test.c | 318 --------------------------------
 tools/Makefile                  |   7 +-
 tools/spi/Makefile              |   4 +
 tools/spi/spidev_fdx.c          | 158 ++++++++++++++++
 tools/spi/spidev_test.c         | 399 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 566 insertions(+), 488 deletions(-)
 delete mode 100644 Documentation/spi/Makefile
 delete mode 100644 Documentation/spi/spidev_fdx.c
 delete mode 100644 Documentation/spi/spidev_test.c
 create mode 100644 tools/spi/Makefile
 create mode 100644 tools/spi/spidev_fdx.c
 create mode 100644 tools/spi/spidev_test.c
Signature · 6 lines
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#24 lore raw

PATCH v2 1/6 spi: Move spi code from Documentation to tools

Jon Corbet requested this code moved with the last changeset,
https://lkml.org/lkml/2015/3/1/144,
but the patch was not applied because it missed the Makefile. Moved spidev_test, spidev_fdx and their Makefile infrastructure.

---
 Documentation/Makefile          |   2 +-
 Documentation/spi/Makefile      |   8 -
 Documentation/spi/spidev_fdx.c  | 158 --------------------
 Documentation/spi/spidev_test.c | 318 ----------------------------------------
 tools/Makefile                  |   7 +-
 tools/spi/Makefile              |   4 +
 tools/spi/spidev_fdx.c          | 158 ++++++++++++++++++++
 tools/spi/spidev_test.c         | 318 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 485 insertions(+), 488 deletions(-)
 delete mode 100644 Documentation/spi/Makefile
 delete mode 100644 Documentation/spi/spidev_fdx.c
 delete mode 100644 Documentation/spi/spidev_test.c
 create mode 100644 tools/spi/Makefile
 create mode 100644 tools/spi/spidev_fdx.c
 create mode 100644 tools/spi/spidev_test.c
Documentation/Makefile +1 -1
index 6883a1b..f642cb9 100644
@@ -1,4 +1,4 @@
 subdir-y := accounting arm auxdisplay blackfin connector \
 	filesystems filesystems ia64 laptops mic misc-devices \
-	networking pcmcia prctl ptp spi timers vDSO video4linux \
+	networking pcmcia prctl ptp timers vDSO video4linux \
 	watchdog
Documentation/spi/Makefile deleted +0 -8
deleted file mode 100644
index efa2558..0000000
@@ -1,8 +0,0 @@
-# List of programs to build
-hostprogs-y := spidev_test spidev_fdx
-
-# Tell kbuild to always build the programs
-always := $(hostprogs-y)
-
-HOSTCFLAGS_spidev_test.o += -I$(objtree)/usr/include
-HOSTCFLAGS_spidev_fdx.o += -I$(objtree)/usr/include
Documentation/spi/spidev_fdx.c deleted +0 -158

158 lines not on this page · raw message

Documentation/spi/spidev_test.c deleted +0 -318

318 lines not on this page · raw message

tools/Makefile +4 -3
index 9a617ad..d238a96 100644
@@ -11,6 +11,7 @@ help:
 	@echo '  lguest     - a minimal 32-bit x86 hypervisor'
 	@echo '  perf       - Linux performance measurement and analysis tool'
 	@echo '  selftests  - various kernel selftests'
+	@echo '  spi        - spi tools'
 	@echo '  turbostat  - Intel CPU idle stats and freq reporting tool'
 	@echo '  usb        - USB testing tools'
 	@echo '  virtio     - vhost test module'
@@ -41,7 +42,7 @@ acpi: FORCE
 cpupower: FORCE
 	$(call descend,power/$@)
 
-cgroup firewire hv guest usb virtio vm net: FORCE
+cgroup firewire hv guest spi usb virtio vm net: FORCE
 	$(call descend,$@)
 
 liblockdep: FORCE
@@ -91,7 +92,7 @@ acpi_clean:
 cpupower_clean:
 	$(call descend,power/cpupower,clean)
 
-cgroup_clean hv_clean firewire_clean lguest_clean usb_clean virtio_clean vm_clean net_clean:
+cgroup_clean hv_clean firewire_clean lguest_clean spi_clean usb_clean virtio_clean vm_clean net_clean:
 	$(call descend,$(@:_clean=),clean)
 
 liblockdep_clean:
@@ -113,7 +114,7 @@ tmon_clean:
 	$(call descend,thermal/tmon,clean)
 
 clean: acpi_clean cgroup_clean cpupower_clean hv_clean firewire_clean lguest_clean \
-		perf_clean selftests_clean turbostat_clean usb_clean virtio_clean \
+		perf_clean selftests_clean spi_clean turbostat_clean usb_clean virtio_clean \
 		vm_clean net_clean x86_energy_perf_policy_clean tmon_clean
 
 .PHONY: FORCE
tools/spi/Makefile new file +4 -0
new file mode 100644
index 0000000..cd0db62
@@ -0,0 +1,4 @@
+all: spidev_test spidev_fdx
+
+clean:
+	$(RM) spidev_test spidev_fdx
tools/spi/spidev_fdx.c new file +158 -0

158 lines not on this page · raw message

tools/spi/spidev_test.c new file +318 -0

318 lines not on this page · raw message

Signature · 1 line
2.5.0
#25 lore raw

PATCH v2 2/6 spi: spidev_test: transfer_escaped_string function

In reply to Joshua Clayton

Move the input_tx code into its own small function.
This cleans up some variables from main() that are used only here. While we are at it, check malloc calls instead of assuming they succeed.

---
 tools/spi/spidev_test.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)
tools/spi/spidev_test.c +23 -13
index 135b3f5..f9d2957 100644
@@ -249,13 +249,30 @@ static void parse_opts(int argc, char *argv[])
 	}
 }
 
+static void transfer_escaped_string(int fd, char *str)
+{
+	size_t size = strlen(str + 1);
+	uint8_t *tx;
+	uint8_t *rx;
+
+	tx = malloc(size);
+	if (!tx)
+		pabort("can't allocate tx buffer");
+
+	rx = malloc(size);
+	if (!rx)
+		pabort("can't allocate rx buffer");
+
+	size = unescape((char *)tx, str, size);
+	transfer(fd, tx, rx, size);
+	free(rx);
+	free(tx);
+}
+
 int main(int argc, char *argv[])
 {
 	int ret = 0;
 	int fd;
-	uint8_t *tx;
-	uint8_t *rx;
-	int size;
 
 	parse_opts(argc, argv);
 
@@ -300,17 +317,10 @@ int main(int argc, char *argv[])
 	printf("bits per word: %d\n", bits);
 	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
-	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);
-		free(tx);
-	} else {
+	if (input_tx)
+		transfer_escaped_string(fd, input_tx);
+	else
 		transfer(fd, default_tx, default_rx, sizeof(default_tx));
-	}
 
 	close(fd);
 
Signature · 1 line
2.5.0
#28 lore raw

PATCH v2 3/6 spi: spidev_test: accept input from a file

In reply to Joshua Clayton

Add input file support to facilitate testing larger data.

---
 tools/spi/spidev_test.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)
tools/spi/spidev_test.c +47 -1
index f9d2957..71a45a4 100644
@@ -19,6 +19,7 @@
 #include <getopt.h>
 #include <fcntl.h>
 #include <sys/ioctl.h>
+#include <sys/stat.h>
 #include <linux/types.h>
 #include <linux/spi/spidev.h>
 
@@ -33,6 +34,7 @@ static void pabort(const char *s)
 static const char *device = "/dev/spidev1.1";
 static uint32_t mode;
 static uint8_t bits = 8;
+static char *input_file;
 static uint32_t speed = 500000;
 static uint16_t delay;
 static int verbose;
@@ -144,6 +146,7 @@ static void print_usage(const char *prog)
 	     "  -s --speed    max speed (Hz)\n"
 	     "  -d --delay    delay (usec)\n"
 	     "  -b --bpw      bits per word \n"
+	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
 	     "  -l --loop     loopback\n"
 	     "  -H --cpha     clock phase\n"
 	     "  -O --cpol     clock polarity\n"
@@ -167,6 +170,7 @@ static void parse_opts(int argc, char *argv[])
 			{ "speed",   1, 0, 's' },
 			{ "delay",   1, 0, 'd' },
 			{ "bpw",     1, 0, 'b' },
+			{ "input",   1, 0, 'i' },
 			{ "loop",    0, 0, 'l' },
 			{ "cpha",    0, 0, 'H' },
 			{ "cpol",    0, 0, 'O' },
@@ -182,7 +186,8 @@ static void parse_opts(int argc, char *argv[])
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:lHOLC3NR24p:v", lopts, NULL);
+		c = getopt_long(argc, argv, "D:s:d:b:i:lHOLC3NR24p:v",
+				lopts, NULL);
 
 		if (c == -1)
 			break;
@@ -200,6 +205,9 @@ static void parse_opts(int argc, char *argv[])
 		case 'b':
 			bits = atoi(optarg);
 			break;
+		case 'i':
+			input_file = optarg;
+			break;
 		case 'l':
 			mode |= SPI_LOOP;
 			break;
@@ -269,6 +277,39 @@ static void transfer_escaped_string(int fd, char *str)
 	free(tx);
 }
 
+static void transfer_file(int fd, char *filename)
+{
+	ssize_t bytes;
+	struct stat sb;
+	int tx_fd;
+	uint8_t *tx;
+	uint8_t *rx;
+
+	if (stat(filename, &sb) == -1)
+		pabort("can't stat input file");
+
+	tx_fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		pabort("can't open input file");
+
+	tx = malloc(sb.st_size);
+	if (!tx)
+		pabort("can't allocate tx buffer");
+
+	rx = malloc(sb.st_size);
+	if (!rx)
+		pabort("can't allocate rx buffer");
+
+	bytes = read(tx_fd, tx, sb.st_size);
+	if (bytes != sb.st_size)
+		pabort("failed to read input file");
+
+	transfer(fd, tx, rx, sb.st_size);
+	free(rx);
+	free(tx);
+	close(tx_fd);
+}
+
 int main(int argc, char *argv[])
 {
 	int ret = 0;
@@ -317,8 +358,13 @@ int main(int argc, char *argv[])
 	printf("bits per word: %d\n", bits);
 	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
+	if (input_tx && input_file)
+		pabort("only one of -p and --input may be selected");
+
 	if (input_tx)
 		transfer_escaped_string(fd, input_tx);
+	else if (input_file)
+		transfer_file(fd, input_file);
 	else
 		transfer(fd, default_tx, default_rx, sizeof(default_tx));
 
Signature · 1 line
2.5.0
#31 lore raw

PATCH v2 4/6 spi: spidev_test: output to a file

In reply to Joshua Clayton

For testing of larger data transfers, output unmodified data directly to a file.

---
 tools/spi/spidev_test.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
tools/spi/spidev_test.c +23 -3
index 71a45a4..02fc3a4 100644
@@ -35,6 +35,7 @@ static const char *device = "/dev/spidev1.1";
 static uint32_t mode;
 static uint8_t bits = 8;
 static char *input_file;
+static char *output_file;
 static uint32_t speed = 500000;
 static uint16_t delay;
 static int verbose;
@@ -105,7 +106,7 @@ static int unescape(char *_dst, char *_src, size_t len)
 static void transfer(int fd, uint8_t const *tx, uint8_t const *rx, size_t len)
 {
 	int ret;
-
+	int out_fd;
 	struct spi_ioc_transfer tr = {
 		.tx_buf = (unsigned long)tx,
 		.rx_buf = (unsigned long)rx,
@@ -136,7 +137,21 @@ 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");
+
+	if (output_file) {
+		out_fd = open(output_file, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+		if (out_fd < 0)
+			pabort("could not open output file");
+
+		ret = write(out_fd, rx, len);
+		if (ret != len)
+			pabort("not all bytes written to utput file");
+
+		close(out_fd);
+	}
+
+	if (verbose || !output_file)
+		hex_dump(rx, len, 32, "RX");
 }
 
 static void print_usage(const char *prog)
@@ -147,6 +162,7 @@ static void print_usage(const char *prog)
 	     "  -d --delay    delay (usec)\n"
 	     "  -b --bpw      bits per word \n"
 	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
+	     "  -o --output   output data to a file (e.g. \"results.bin\")\n"
 	     "  -l --loop     loopback\n"
 	     "  -H --cpha     clock phase\n"
 	     "  -O --cpol     clock polarity\n"
@@ -171,6 +187,7 @@ static void parse_opts(int argc, char *argv[])
 			{ "delay",   1, 0, 'd' },
 			{ "bpw",     1, 0, 'b' },
 			{ "input",   1, 0, 'i' },
+			{ "output",  1, 0, 'o' },
 			{ "loop",    0, 0, 'l' },
 			{ "cpha",    0, 0, 'H' },
 			{ "cpol",    0, 0, 'O' },
@@ -186,7 +203,7 @@ static void parse_opts(int argc, char *argv[])
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:i:lHOLC3NR24p:v",
+		c = getopt_long(argc, argv, "D:s:d:b:i:o:lHOLC3NR24p:v",
 				lopts, NULL);
 
 		if (c == -1)
@@ -208,6 +225,9 @@ static void parse_opts(int argc, char *argv[])
 		case 'i':
 			input_file = optarg;
 			break;
+		case 'o':
+			output_file = optarg;
+			break;
 		case 'l':
 			mode |= SPI_LOOP;
 			break;
Signature · 6 lines
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#34 lore raw

PATCH v2 5/6 spi: spidev_test: check error

In reply to Joshua Clayton

Check the result of sscanf to verify a result was found. report and error and abort if pattern was not found.

---
 tools/spi/spidev_test.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/spi/spidev_test.c +5 -1
index 02fc3a4..322370d 100644
@@ -86,13 +86,17 @@ static void hex_dump(const void *src, size_t length, size_t line_size, char *pre
 static int unescape(char *_dst, char *_src, size_t len)
 {
 	int ret = 0;
+	int match;
 	char *src = _src;
 	char *dst = _dst;
 	unsigned int ch;
 
 	while (*src) {
 		if (*src == '\\' && *(src+1) == 'x') {
-			sscanf(src + 2, "%2x", &ch);
+			match = sscanf(src + 2, "%2x", &ch);
+			if (!match)
+				pabort("malformed input string");
+
 			src += 4;
 			*dst++ = (unsigned char)ch;
 		} else {
Signature · 1 line
2.5.0
#37 lore raw

PATCH v2 6/6 spi: spidev_test: fix whitespace

In reply to Joshua Clayton
---
 tools/spi/spidev_test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
tools/spi/spidev_test.c +3 -2
index 322370d..eddfc33 100644
@@ -52,7 +52,8 @@ uint8_t default_tx[] = {
 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)
+static void hex_dump(const void *src, size_t length, size_t line_size,
+		     char *prefix)
 {
 	int i = 0;
 	const unsigned char *address = src;
@@ -164,7 +165,7 @@ static void print_usage(const char *prog)
 	puts("  -D --device   device to use (default /dev/spidev1.1)\n"
 	     "  -s --speed    max speed (Hz)\n"
 	     "  -d --delay    delay (usec)\n"
-	     "  -b --bpw      bits per word \n"
+	     "  -b --bpw      bits per word\n"
 	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
 	     "  -o --output   output data to a file (e.g. \"results.bin\")\n"
 	     "  -l --loop     loopback\n"
Signature · 1 line
2.5.0
#39 lore raw

Applied "spi: spidev_test: fix whitespace" to the spi tree

In reply to a message not in the archive

The patch

spi: spidev_test: fix whitespace

has been applied to the spi tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From b2cfc90428f1c24ce4920f06b584bd05c62741e6 Mon Sep 17 00:00:00 2001

From: Joshua Clayton <[email protected]>
Date: Wed, 18 Nov 2015 14:30:42 -0800
Subject: [PATCH] spi: spidev_test: fix whitespace

---
 tools/spi/spidev_test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
tools/spi/spidev_test.c +3 -2
index 322370dbc770..eddfc336a960 100644
@@ -52,7 +52,8 @@ uint8_t default_tx[] = {
 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)
+static void hex_dump(const void *src, size_t length, size_t line_size,
+		     char *prefix)
 {
 	int i = 0;
 	const unsigned char *address = src;
@@ -164,7 +165,7 @@ static void print_usage(const char *prog)
 	puts("  -D --device   device to use (default /dev/spidev1.1)\n"
 	     "  -s --speed    max speed (Hz)\n"
 	     "  -d --delay    delay (usec)\n"
-	     "  -b --bpw      bits per word \n"
+	     "  -b --bpw      bits per word\n"
 	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
 	     "  -o --output   output data to a file (e.g. \"results.bin\")\n"
 	     "  -l --loop     loopback\n"
Signature · 6 lines
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#36 lore raw

Applied "spi: spidev_test: check error" to the spi tree

In reply to a message not in the archive

The patch

spi: spidev_test: check error

has been applied to the spi tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From a20874f78be633b9e3a505d5f191735188b110a5 Mon Sep 17 00:00:00 2001

From: Joshua Clayton <[email protected]>
Date: Wed, 18 Nov 2015 14:30:41 -0800
Subject: [PATCH] spi: spidev_test: check error

Check the result of sscanf to verify a result was found. report and error and abort if pattern was not found.

---
 tools/spi/spidev_test.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/spi/spidev_test.c +5 -1
index 02fc3a44901b..322370dbc770 100644
@@ -86,13 +86,17 @@ static void hex_dump(const void *src, size_t length, size_t line_size, char *pre
 static int unescape(char *_dst, char *_src, size_t len)
 {
 	int ret = 0;
+	int match;
 	char *src = _src;
 	char *dst = _dst;
 	unsigned int ch;
 
 	while (*src) {
 		if (*src == '\\' && *(src+1) == 'x') {
-			sscanf(src + 2, "%2x", &ch);
+			match = sscanf(src + 2, "%2x", &ch);
+			if (!match)
+				pabort("malformed input string");
+
 			src += 4;
 			*dst++ = (unsigned char)ch;
 		} else {
Signature · 6 lines
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#33 lore raw

Applied "spi: spidev_test: output to a file" to the spi tree

In reply to a message not in the archive

The patch

spi: spidev_test: output to a file

has been applied to the spi tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 983b27886a3c7f6eff4e987ddba932da74703f4e Mon Sep 17 00:00:00 2001

From: Joshua Clayton <[email protected]>
Date: Wed, 18 Nov 2015 14:30:40 -0800
Subject: [PATCH] spi: spidev_test: output to a file

For testing of larger data transfers, output unmodified data directly to a file.

---
 tools/spi/spidev_test.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)
tools/spi/spidev_test.c +23 -3
index 71a45a4d7d49..02fc3a44901b 100644
@@ -35,6 +35,7 @@ static const char *device = "/dev/spidev1.1";
 static uint32_t mode;
 static uint8_t bits = 8;
 static char *input_file;
+static char *output_file;
 static uint32_t speed = 500000;
 static uint16_t delay;
 static int verbose;
@@ -105,7 +106,7 @@ static int unescape(char *_dst, char *_src, size_t len)
 static void transfer(int fd, uint8_t const *tx, uint8_t const *rx, size_t len)
 {
 	int ret;
-
+	int out_fd;
 	struct spi_ioc_transfer tr = {
 		.tx_buf = (unsigned long)tx,
 		.rx_buf = (unsigned long)rx,
@@ -136,7 +137,21 @@ 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");
+
+	if (output_file) {
+		out_fd = open(output_file, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+		if (out_fd < 0)
+			pabort("could not open output file");
+
+		ret = write(out_fd, rx, len);
+		if (ret != len)
+			pabort("not all bytes written to utput file");
+
+		close(out_fd);
+	}
+
+	if (verbose || !output_file)
+		hex_dump(rx, len, 32, "RX");
 }
 
 static void print_usage(const char *prog)
@@ -147,6 +162,7 @@ static void print_usage(const char *prog)
 	     "  -d --delay    delay (usec)\n"
 	     "  -b --bpw      bits per word \n"
 	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
+	     "  -o --output   output data to a file (e.g. \"results.bin\")\n"
 	     "  -l --loop     loopback\n"
 	     "  -H --cpha     clock phase\n"
 	     "  -O --cpol     clock polarity\n"
@@ -171,6 +187,7 @@ static void parse_opts(int argc, char *argv[])
 			{ "delay",   1, 0, 'd' },
 			{ "bpw",     1, 0, 'b' },
 			{ "input",   1, 0, 'i' },
+			{ "output",  1, 0, 'o' },
 			{ "loop",    0, 0, 'l' },
 			{ "cpha",    0, 0, 'H' },
 			{ "cpol",    0, 0, 'O' },
@@ -186,7 +203,7 @@ static void parse_opts(int argc, char *argv[])
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:i:lHOLC3NR24p:v",
+		c = getopt_long(argc, argv, "D:s:d:b:i:o:lHOLC3NR24p:v",
 				lopts, NULL);
 
 		if (c == -1)
@@ -208,6 +225,9 @@ static void parse_opts(int argc, char *argv[])
 		case 'i':
 			input_file = optarg;
 			break;
+		case 'o':
+			output_file = optarg;
+			break;
 		case 'l':
 			mode |= SPI_LOOP;
 			break;
Signature · 6 lines
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#30 lore raw

Applied "spi: spidev_test: accept input from a file" to the spi tree

In reply to a message not in the archive

The patch

spi: spidev_test: accept input from a file

has been applied to the spi tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 7af475a5b55e8e47327818e8133fe22204c1fc4d Mon Sep 17 00:00:00 2001

From: Joshua Clayton <[email protected]>
Date: Wed, 18 Nov 2015 14:30:39 -0800
Subject: [PATCH] spi: spidev_test: accept input from a file

Add input file support to facilitate testing larger data.

---
 tools/spi/spidev_test.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)
tools/spi/spidev_test.c +47 -1
index f9d2957e538b..71a45a4d7d49 100644
@@ -19,6 +19,7 @@
 #include <getopt.h>
 #include <fcntl.h>
 #include <sys/ioctl.h>
+#include <sys/stat.h>
 #include <linux/types.h>
 #include <linux/spi/spidev.h>
 
@@ -33,6 +34,7 @@ static void pabort(const char *s)
 static const char *device = "/dev/spidev1.1";
 static uint32_t mode;
 static uint8_t bits = 8;
+static char *input_file;
 static uint32_t speed = 500000;
 static uint16_t delay;
 static int verbose;
@@ -144,6 +146,7 @@ static void print_usage(const char *prog)
 	     "  -s --speed    max speed (Hz)\n"
 	     "  -d --delay    delay (usec)\n"
 	     "  -b --bpw      bits per word \n"
+	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
 	     "  -l --loop     loopback\n"
 	     "  -H --cpha     clock phase\n"
 	     "  -O --cpol     clock polarity\n"
@@ -167,6 +170,7 @@ static void parse_opts(int argc, char *argv[])
 			{ "speed",   1, 0, 's' },
 			{ "delay",   1, 0, 'd' },
 			{ "bpw",     1, 0, 'b' },
+			{ "input",   1, 0, 'i' },
 			{ "loop",    0, 0, 'l' },
 			{ "cpha",    0, 0, 'H' },
 			{ "cpol",    0, 0, 'O' },
@@ -182,7 +186,8 @@ static void parse_opts(int argc, char *argv[])
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:lHOLC3NR24p:v", lopts, NULL);
+		c = getopt_long(argc, argv, "D:s:d:b:i:lHOLC3NR24p:v",
+				lopts, NULL);
 
 		if (c == -1)
 			break;
@@ -200,6 +205,9 @@ static void parse_opts(int argc, char *argv[])
 		case 'b':
 			bits = atoi(optarg);
 			break;
+		case 'i':
+			input_file = optarg;
+			break;
 		case 'l':
 			mode |= SPI_LOOP;
 			break;
@@ -269,6 +277,39 @@ static void transfer_escaped_string(int fd, char *str)
 	free(tx);
 }
 
+static void transfer_file(int fd, char *filename)
+{
+	ssize_t bytes;
+	struct stat sb;
+	int tx_fd;
+	uint8_t *tx;
+	uint8_t *rx;
+
+	if (stat(filename, &sb) == -1)
+		pabort("can't stat input file");
+
+	tx_fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		pabort("can't open input file");
+
+	tx = malloc(sb.st_size);
+	if (!tx)
+		pabort("can't allocate tx buffer");
+
+	rx = malloc(sb.st_size);
+	if (!rx)
+		pabort("can't allocate rx buffer");
+
+	bytes = read(tx_fd, tx, sb.st_size);
+	if (bytes != sb.st_size)
+		pabort("failed to read input file");
+
+	transfer(fd, tx, rx, sb.st_size);
+	free(rx);
+	free(tx);
+	close(tx_fd);
+}
+
 int main(int argc, char *argv[])
 {
 	int ret = 0;
@@ -317,8 +358,13 @@ int main(int argc, char *argv[])
 	printf("bits per word: %d\n", bits);
 	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
+	if (input_tx && input_file)
+		pabort("only one of -p and --input may be selected");
+
 	if (input_tx)
 		transfer_escaped_string(fd, input_tx);
+	else if (input_file)
+		transfer_file(fd, input_file);
 	else
 		transfer(fd, default_tx, default_rx, sizeof(default_tx));
 
Signature · 6 lines
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#27 lore raw

Applied "spi: spidev_test: transfer_escaped_string function" to the spi tree

In reply to a message not in the archive

The patch

spi: spidev_test: transfer_escaped_string function

has been applied to the spi tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 5c437a401b824399b6fa7342c66b675ebb7af43e Mon Sep 17 00:00:00 2001

From: Joshua Clayton <[email protected]>
Date: Wed, 18 Nov 2015 14:30:38 -0800
Subject: [PATCH] spi: spidev_test: transfer_escaped_string function

Move the input_tx code into its own small function.
This cleans up some variables from main() that are used only here. While we are at it, check malloc calls instead of assuming they succeed.

---
 tools/spi/spidev_test.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)
tools/spi/spidev_test.c +23 -13
index 135b3f592b83..f9d2957e538b 100644
@@ -249,13 +249,30 @@ static void parse_opts(int argc, char *argv[])
 	}
 }
 
+static void transfer_escaped_string(int fd, char *str)
+{
+	size_t size = strlen(str + 1);
+	uint8_t *tx;
+	uint8_t *rx;
+
+	tx = malloc(size);
+	if (!tx)
+		pabort("can't allocate tx buffer");
+
+	rx = malloc(size);
+	if (!rx)
+		pabort("can't allocate rx buffer");
+
+	size = unescape((char *)tx, str, size);
+	transfer(fd, tx, rx, size);
+	free(rx);
+	free(tx);
+}
+
 int main(int argc, char *argv[])
 {
 	int ret = 0;
 	int fd;
-	uint8_t *tx;
-	uint8_t *rx;
-	int size;
 
 	parse_opts(argc, argv);
 
@@ -300,17 +317,10 @@ int main(int argc, char *argv[])
 	printf("bits per word: %d\n", bits);
 	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
-	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);
-		free(tx);
-	} else {
+	if (input_tx)
+		transfer_escaped_string(fd, input_tx);
+	else
 		transfer(fd, default_tx, default_rx, sizeof(default_tx));
-	}
 
 	close(fd);
 
Signature · 6 lines
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html