fw: Build with tkey-libs

Build firmware, testfw and testapp using tkey-libs:

  https://github.com/tillitis/tkey-libs

In an effort not to have more or less identical code maintained in two
places, use tkey-libs when developing firmware, testfw and the
firmware testapp, too.

You can place the Git directory directly under hw/application_fpga
and then an ordinary make should work.

Or build with:

  make LIBDIR=/path/to/tkey-libs

Co-authored-by: Mikael Ågren <mikael@tillitis.se>

hw/application_fpga/Makefile

@@ -41,6 +41,8 @@ OBJCOPY ?= llvm-objcopy
 
 CC = clang
 
+LIBDIR ?= tkey-libs
+
 CFLAGS = \
 	-target riscv32-unknown-none-elf \
 	-march=rv32iczmmul \
@@ -59,7 +61,9 @@ CFLAGS = \
 	-Wpedantic \
 	-Wno-language-extension-token \
 	-flto \
-	-g
+	-g \
+	-I $(LIBDIR)/include \
+	-I $(LIBDIR)
 
 AS = clang
 
@@ -113,20 +117,12 @@ PICORV32_SRCS = \
 	$(P)/core/picorv32/rtl/picorv32.v
 
 FIRMWARE_DEPS = \
-	$(P)/fw/tk1_mem.h \
+	$(P)/fw/tk1/proto.h
-	$(P)/fw/tk1/types.h \
-	$(P)/fw/tk1/lib.h \
-	$(P)/fw/tk1/proto.h \
-	$(P)/fw/tk1/assert.h \
-	$(P)/fw/tk1/led.h
 
 FIRMWARE_OBJS = \
 	$(P)/fw/tk1/main.o \
 	$(P)/fw/tk1/start.o \
 	$(P)/fw/tk1/proto.o \
-	$(P)/fw/tk1/lib.o \
-	$(P)/fw/tk1/assert.o \
-	$(P)/fw/tk1/led.o \
 	$(P)/fw/tk1/blake2s/blake2s.o \
 	$(P)/fw/tk1/syscall_enable.o \
 	$(P)/fw/tk1/syscall_handler.o
@@ -134,18 +130,12 @@ FIRMWARE_OBJS = \
 FIRMWARE_SOURCES = \
 	$(P)/fw/tk1/main.c \
 	$(P)/fw/tk1/proto.c \
-	$(P)/fw/tk1/lib.c \
-	$(P)/fw/tk1/assert.c \
-	$(P)/fw/tk1/led.c \
 	$(P)/fw/tk1/blake2s/blake2s.c \
 	$(P)/fw/tk1/syscall_handler.c
 
 TESTFW_OBJS = \
 	$(P)/fw/testfw/main.o \
-	$(P)/fw/testfw/start.o \
+	$(P)/fw/testfw/start.o
-	$(P)/fw/tk1/proto.o \
-	$(P)/fw/tk1/assert.o \
-	$(P)/fw/tk1/lib.o
 
 #-------------------------------------------------------------------
 # All: Complete build of HW and FW.
@@ -184,19 +174,27 @@ secret:
 #-------------------------------------------------------------------
 LDFLAGS = \
 	-T $(P)/fw/tk1/firmware.lds \
-	-Wl,--cref,-M
+	-Wl,--cref,-M \
+	-L $(LIBDIR) -lcommon
+
+# Common libraries the firmware and testfw depend on. See
+# https://github.com/tillitis/tkey-libs/
+.PHONY: tkey-libs
+tkey-libs:
+	make -C $(LIBDIR)
 
 $(FIRMWARE_OBJS): $(FIRMWARE_DEPS)
 $(TESTFW_OBJS): $(FIRMWARE_DEPS)
 
-firmware.elf: $(FIRMWARE_OBJS) $(P)/fw/tk1/firmware.lds
+#firmware.elf: CFLAGS += -DTKEY_DEBUG
+firmware.elf: tkey-libs $(FIRMWARE_OBJS) $(P)/fw/tk1/firmware.lds
 	$(CC) $(CFLAGS) $(FIRMWARE_OBJS) $(LDFLAGS) -o $@ > $(basename $@).map
 
 simfirmware.elf: CFLAGS += -DSIMULATION
 simfirmware.elf: $(FIRMWARE_OBJS) $(P)/fw/tk1/firmware.lds
 	$(CC) $(CFLAGS) $(FIRMWARE_OBJS) $(LDFLAGS) -o $@ > $(basename $@).map
 
-qemu_firmware.elf: CFLAGS += -DQEMU_CONSOLE
+qemu_firmware.elf: CFLAGS += -DQEMU_DEBUG
 qemu_firmware.elf: firmware.elf
 	mv firmware.elf qemu_firmware.elf
 
@@ -227,7 +225,7 @@ splint:
 		-fullinitblock \
 		$(FIRMWARE_SOURCES)
 
-testfw.elf: $(TESTFW_OBJS) $(P)/fw/tk1/firmware.lds
+testfw.elf: tkey-libs $(TESTFW_OBJS) $(P)/fw/tk1/firmware.lds
 	$(CC) $(CFLAGS) $(TESTFW_OBJS) $(LDFLAGS) -o $@ > $(basename $@).map
 
 # Generate a fake BRAM file that will be filled in later after place-n-route

hw/application_fpga/fw/README.md

@@ -314,7 +314,7 @@ Currently supported syscalls:
 Standing in `hw/application_fpga/` you can run `make firmware.elf` to
 build just the firmware. You don't need all the FPGA development
 tools. See [the Developer Handbook](https://dev.tillitis.se/tools/)
-for the tools you need. The easiest is probably to use your OCI image,
+for the tools you need. The easiest is probably to use our OCI image,
 `ghcr.io/tillitis/tkey-builder`.
 
 [Our version of qemu](https://dev.tillitis.se/tools/#qemu-emulator) is
@@ -322,11 +322,28 @@ also useful for debugging the firmware. You can attach GDB, use
 breakpoints, et cetera.
 
 There is a special make target for QEMU: `qemu_firmware.elf`, which
-sets `-DQEMU_CONSOLE`, so you can use plain debug prints using the
+sets `-DQEMU_DEBUG`, so you can debug prints using the `debug_*()`
-helper functions in `lib.c` like `htif_puts()` `htif_putinthex()`
+functions. Note that these functions are only usable in QEMU and that
-`htif_hexdump()` and friends. Note that these functions are only
+you might need to `make clean` before building, if you have already
-usable in qemu and that you might need to `make clean` before
+built before.
-building, if you have already built before.
+
+If you want debug prints to show up on the special TKey HID debug
+endpoint instead, define `-DTKEY_DEBUG`.
+
+Note that if you use `TKEY_DEBUG` you *must* have something listening
+on the corresponding HID device. It's usually the last HID device
+created. On Linux, for instance, this means the last reported hidraw
+in `dmesg` is the one you should do `cat /dev/hidrawX` on.
+
+### tkey-libs
+
+Most of the utility functions that the firmware use lives in
+`tkey-libs`. The canonical place where you can find tkey-libs is at:
+
+  https://github.com/tillitis/tkey-libs
+
+but we have vendored it in for firmware use in `../tkey-libs`. See top
+README for how to update.
 
 ### Test firmware
 

hw/application_fpga/fw/testapp/Makefile

@@ -1,4 +1,5 @@
 P := $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
+LIBDIR ?= ../../tkey-libs
 OBJCOPY ?= llvm-objcopy
 CC = clang
 CFLAGS = \
@@ -21,7 +22,9 @@ CFLAGS = \
 	-Wno-language-extension-token \
 	-Werror \
 	-flto \
-	-g
+	-g \
+	-I $(LIBDIR)/include \
+	-I $(LIBDIR)
 
 AS = clang
 
@@ -31,7 +34,9 @@ ASFLAGS = \
 	-mabi=ilp32 \
 	-mno-relax
 
-LDFLAGS=-T $(P)/app.lds
+LDFLAGS = \
+	-T $(P)/app.lds \
+	-L $(LIBDIR) -lcommon
 
 .PHONY: all
 all: testapp.bin
@@ -41,6 +46,10 @@ all: testapp.bin
 	$(OBJCOPY) --input-target=elf32-littleriscv --output-target=binary $^ $@
 	chmod a-x $@
 
+.PHONY: tkey-libs
+tkey-libs:
+	make -C $(LIBDIR)
+
 TESTAPP_FMTFILES = \
 	$(P)/main.c \
 	$(P)/syscall.h
@@ -48,13 +57,9 @@ TESTAPP_FMTFILES = \
 TESTAPP_OBJS = \
 	$(P)/main.o \
 	$(P)/crt0.o \
-	$(P)/syscall.o \
+	$(P)/syscall.o
-	$(P)/../tk1/assert.o \
-	$(P)/../tk1/led.o \
-	$(P)/../tk1/lib.o \
-	$(P)/../tk1/proto.o
 
-testapp.elf: $(TESTAPP_OBJS)
+testapp.elf: tkey-libs $(TESTAPP_OBJS)
 	$(CC) $(CFLAGS) $(TESTAPP_OBJS) $(LDFLAGS) -o $@
 
 .PHONY: fmt

hw/application_fpga/fw/testapp/main.c

@@ -3,11 +3,14 @@
  * SPDX-License-Identifier: GPL-2.0-only
  */
 
-#include "../tk1/led.h"
+#include <stdint.h>
-#include "../tk1/lib.h"
+#include <tkey/assert.h>
+#include <tkey/io.h>
+#include <tkey/led.h>
+#include <tkey/lib.h>
+
 #include "../tk1/proto.h"
 #include "../tk1/syscall_num.h"
-#include "../tk1/types.h"
 #include "../tk1_mem.h"
 #include "syscall.h"
 
@@ -33,74 +36,10 @@ volatile uint32_t *trng_entropy     = (volatile uint32_t *)TK1_MMIO_TRNG_ENTROPY
 #define UDI_WORDS 2
 #define CDI_WORDS 8
 
-static void write_with_header(const uint8_t *buf, size_t nbytes, enum mode mode)
-{
-	// Append USB Mode Protocol header:
-	//   1 byte mode
-	//   1 byte length
-	writebyte(mode);
-	writebyte(nbytes);
-
-	for (int i = 0; i < nbytes; i++) {
-		writebyte(buf[i]);
-	}
-}
-
-static void write(const uint8_t *buf, size_t nbytes)
-{
-	uint8_t len;
-
-	while (nbytes > 0) {
-		// We split the data into chunks that will fit in the
-		// USB Mode Protocol with some spare change.
-		len =
-		    nbytes < USBMODE_PACKET_SIZE ? nbytes : USBMODE_PACKET_SIZE;
-
-		write_with_header((const uint8_t *)buf, len, MODE_CDC);
-
-		buf += len;
-		nbytes -= len;
-	}
-}
-
-unsigned strlen(const char *str)
-{
-	const char *s;
-
-	for (s = str; *s; ++s)
-		;
-
-	return (s - str);
-}
-
-void puts(char *buf)
-{
-	size_t nbytes = strlen(buf);
-
-	write((const uint8_t *)buf, nbytes);
-}
-
-void hex(uint8_t buf[2], const uint8_t c)
-{
-	unsigned int upper = (c >> 4) & 0xf;
-	unsigned int lower = c & 0xf;
-
-	buf[0] = upper < 10 ? '0' + upper : 'a' - 10 + upper;
-	buf[1] = lower < 10 ? '0' + lower : 'a' - 10 + lower;
-}
-
-void puthex(uint8_t c)
-{
-	uint8_t buf[2];
-
-	hex(buf, c);
-	write(buf, 2);
-}
-
 void puthexn(uint8_t *p, int n)
 {
 	for (int i = 0; i < n; i++) {
-		puthex(p[i]);
+		puthex(IO_CDC, p[i]);
 	}
 }
 
@@ -123,33 +62,41 @@ uint32_t wait_timer_tick(uint32_t last_timer)
 
 void failmsg(char *s)
 {
-	puts("FAIL: ");
+	puts(IO_CDC, "FAIL: ");
-	puts(s);
+	puts(IO_CDC, s);
-	puts("\r\n");
+	puts(IO_CDC, "\r\n");
 }
 
 int main(void)
 {
 	uint8_t in = 0;
-	uint8_t mode = 0;
+	uint8_t available = 0;
-	uint8_t mode_bytes_left = 0;
+	enum ioend endpoint = IO_NONE;
 
-	set_led(LED_BLUE);
+	led_set(LED_BLUE);
 
 	// Wait for terminal program and a character to be typed
-	in = readbyte(&mode, &mode_bytes_left);
+	if (readselect(IO_CDC, &endpoint, &available) < 0) {
+		// readselect failed! I/O broken? Just redblink.
+		assert(1 == 2);
+	}
 
-	puts("\r\nI'm testapp on:");
+	if (read(IO_CDC, &in, 1, 1) < 0) {
+		// read failed! I/O broken? Just redblink.
+		assert(1 == 2);
+	}
+
+	puts(IO_CDC, "\r\nI'm testapp on:");
 	// Output the TK1 core's NAME0 and NAME1
 	uint32_t name;
 	wordcpy_s(&name, 1, (void *)tk1name0, 1);
 	reverseword(&name);
-	write((const uint8_t *)&name, 4);
+	write(IO_CDC, (const uint8_t *)&name, 4);
-	puts(" ");
+	puts(IO_CDC, " ");
 	wordcpy_s(&name, 1, (void *)tk1name1, 1);
 	reverseword(&name);
-	write((const uint8_t *)&name, 4);
+	write(IO_CDC, (const uint8_t *)&name, 4);
-	puts("\r\n");
+	puts(IO_CDC, "\r\n");
 
 	uint32_t zeros[8];
 	memset(zeros, 0, 8 * 4);
@@ -194,9 +141,9 @@ int main(void)
 	}
 
 	// Should NOT be able to reset Tkey from app mode
-	puts("\r\nTesting system reset...");
+	puts(IO_CDC, "\r\nTesting system reset...");
 	*system_reset = 1;
-	puts("done.\r\n");
+	puts(IO_CDC, "done.\r\n");
 
 	// Test FW_RAM.
 	*fw_ram = 0x21;
@@ -205,7 +152,7 @@ int main(void)
 		anyfailed = 1;
 	}
 
-	puts("\r\nTesting timer... 3");
+	puts(IO_CDC, "\r\nTesting timer... 3");
 	// Matching clock at 21 MHz, giving us timer in seconds
 	*timer_prescaler = 21 * 1000000;
 
@@ -216,7 +163,7 @@ int main(void)
 	while (*timer_status & (1 << TK1_MMIO_TIMER_STATUS_RUNNING_BIT)) {
 	}
 	// Now timer has expired and is ready to run again
-	puts(" 2");
+	puts(IO_CDC, " 2");
 
 	// Test to interrupt a timer - and reads from timer register
 	// Starting 10s timer and interrupting it in 3s...
@@ -229,7 +176,7 @@ int main(void)
 
 	// Stop the timer
 	*timer_ctrl = (1 << TK1_MMIO_TIMER_CTRL_STOP_BIT);
-	puts(" 1. done.\r\n");
+	puts(IO_CDC, " 1. done.\r\n");
 
 	if (*timer_status & (1 << TK1_MMIO_TIMER_STATUS_RUNNING_BIT)) {
 		failmsg("Timer didn't stop");
@@ -242,14 +189,14 @@ int main(void)
 	}
 
 	// Check and display test results.
-	puts("\r\n--> ");
+	puts(IO_CDC, "\r\n--> ");
 	if (anyfailed) {
-		puts("Some test FAILED!\r\n");
+		puts(IO_CDC, "Some test FAILED!\r\n");
 	} else {
-		puts("All tests passed.\r\n");
+		puts(IO_CDC, "All tests passed.\r\n");
 	}
 
-	puts("\r\nHere are 256 bytes from the TRNG:\r\n");
+	puts(IO_CDC, "\r\nHere are 256 bytes from the TRNG:\r\n");
 	for (int j = 0; j < 8; j++) {
 		for (int i = 0; i < 8; i++) {
 			while ((*trng_status &
@@ -257,21 +204,28 @@ int main(void)
 			}
 			uint32_t rnd = *trng_entropy;
 			puthexn((uint8_t *)&rnd, 4);
-			puts(" ");
+			puts(IO_CDC, " ");
 		}
-		puts("\r\n");
+		puts(IO_CDC, "\r\n");
 	}
-	puts("\r\n");
+	puts(IO_CDC, "\r\n");
 
-	puts("Now echoing what you type...Type + to reset device\r\n");
+	puts(IO_CDC, "Now echoing what you type...Type + to reset device\r\n");
 	for (;;) {
-		in = readbyte(&mode, &mode_bytes_left);
+		if (readselect(IO_CDC, &endpoint, &available) < 0) {
+			// readselect failed! I/O broken? Just redblink.
+			assert(1 == 2);
+		}
+
+		if (read(IO_CDC, &in, 1, 1) < 0) {
+			// read failed! I/O broken? Just redblink.
+			assert(1 == 2);
+		}
+
 		if (in == '+') {
 			syscall(TK1_SYSCALL_RESET, 0);
 		}
 
-		writebyte(MODE_CDC);
+		write(IO_CDC, &in, 1);
-		writebyte(1);
-		writebyte(in);
 	}
 }

hw/application_fpga/fw/testapp/syscall.h

@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: 2024 Tillitis AB <tillitis.se>
 // SPDX-License-Identifier: BSD-2-Clause
 
-#include "../tk1/types.h"
+#include <stdint.h>
 
 #ifndef TKEY_APP_SYSCALL_H
 #define TKEY_APP_SYSCALL_H

hw/application_fpga/fw/testfw/main.c

@@ -3,9 +3,13 @@
  * SPDX-License-Identifier: GPL-2.0-only
  */
 
-#include "../tk1/lib.h"
+#include <stddef.h>
+#include <stdint.h>
+#include <tkey/assert.h>
+#include <tkey/io.h>
+#include <tkey/lib.h>
+
 #include "../tk1/proto.h"
-#include "../tk1/types.h"
 #include "../tk1_mem.h"
 
 #define USBMODE_PACKET_SIZE 64
@@ -30,107 +34,13 @@ volatile uint32_t *trng_entropy     = (volatile uint32_t *)TK1_MMIO_TRNG_ENTROPY
 #define UDI_WORDS 2
 #define CDI_WORDS 8
 
-void *memcpy(void *dest, const void *src, size_t n)
-{
-	uint8_t *src_byte = (uint8_t *)src;
-	uint8_t *dest_byte = (uint8_t *)dest;
-
-	for (int i = 0; i < n; i++) {
-		dest_byte[i] = src_byte[i];
-	}
-
-	return dest;
-}
-
-static void write_with_header(const uint8_t *buf, size_t nbytes, enum mode mode)
-{
-	// Append USB Mode Protocol header:
-	//   1 byte mode
-	//   1 byte length
-	writebyte(mode);
-	writebyte(nbytes);
-
-	for (int i = 0; i < nbytes; i++) {
-		writebyte(buf[i]);
-	}
-}
-
-static void write(const uint8_t *buf, size_t nbytes)
-{
-	uint8_t len;
-
-	while (nbytes > 0) {
-		// We split the data into chunks that will fit in the
-		// USB Mode Protocol with some spare change.
-		len =
-		    nbytes < USBMODE_PACKET_SIZE ? nbytes : USBMODE_PACKET_SIZE;
-
-		write_with_header((const uint8_t *)buf, len, MODE_CDC);
-
-		buf += len;
-		nbytes -= len;
-	}
-}
-
-unsigned strlen(const char *str)
-{
-	const char *s;
-
-	for (s = str; *s; ++s)
-		;
-
-	return (s - str);
-}
-
-void puts(char *buf)
-{
-	size_t nbytes = strlen(buf);
-
-	write((const uint8_t *)buf, nbytes);
-}
-
-void hex(uint8_t buf[2], const uint8_t c)
-{
-	unsigned int upper = (c >> 4) & 0xf;
-	unsigned int lower = c & 0xf;
-
-	buf[0] = upper < 10 ? '0' + upper : 'a' - 10 + upper;
-	buf[1] = lower < 10 ? '0' + lower : 'a' - 10 + lower;
-}
-
-void puthex(uint8_t c)
-{
-	uint8_t buf[2];
-
-	hex(buf, c);
-	write(buf, 2);
-}
-
 void puthexn(uint8_t *p, int n)
 {
 	for (int i = 0; i < n; i++) {
-		puthex(p[i]);
+		puthex(IO_CDC, p[i]);
 	}
 }
 
-void hexdump(void *buf, int len)
-{
-	uint8_t *byte_buf = (uint8_t *)buf;
-
-	for (int i = 0; i < len; i++) {
-		puthex(byte_buf[i]);
-		if (i % 2 == 1) {
-			puts(" ");
-		}
-
-		if (i != 1 && i % 16 == 1) {
-			puts("\r\n");
-		}
-	}
-
-	puts("\r\n");
-}
-
 void reverseword(uint32_t *wordp)
 {
 	*wordp = ((*wordp & 0xff000000) >> 24) | ((*wordp & 0x00ff0000) >> 8) |
@@ -166,19 +76,19 @@ int check_fwram_zero_except(unsigned int offset, uint8_t expected_val)
 		if (i == offset) {
 			if (val != expected_val) {
 				failed_now = 1;
-				puts("  wrong value at: ");
+				puts(IO_CDC, "  wrong value at: ");
 			}
 		} else {
 			if (val != 0) {
 				failed_now = 1;
-				puts("  not zero at: ");
+				puts(IO_CDC, "  not zero at: ");
 			}
 		}
 		if (failed_now) {
 			failed = 1;
 			reverseword(&addr);
 			puthexn((uint8_t *)&addr, 4);
-			puts("\r\n");
+			puts(IO_CDC, "\r\n");
 		}
 	}
 	return failed;
@@ -186,16 +96,16 @@ int check_fwram_zero_except(unsigned int offset, uint8_t expected_val)
 
 void failmsg(char *s)
 {
-	puts("FAIL: ");
+	puts(IO_CDC, "FAIL: ");
-	puts(s);
+	puts(IO_CDC, s);
-	puts("\r\n");
+	puts(IO_CDC, "\r\n");
 }
 
 int main(void)
 {
 	uint8_t in = 0;
-	uint8_t mode = 0;
+	uint8_t available = 0;
-	uint8_t mode_bytes_left = 0;
+	enum ioend endpoint = IO_NONE;
 
 	// Hard coded test UDS in ../../data/uds.hex
 	// clang-format off
@@ -212,19 +122,27 @@ int main(void)
 	// clang-format on
 
 	// Wait for terminal program and a character to be typed
-	in = readbyte(&mode, &mode_bytes_left);
+	if (readselect(IO_CDC, &endpoint, &available) < 0) {
+		// readselect failed! I/O broken? Just redblink.
+		assert(1 == 2);
+	}
 
-	puts("\r\nI'm testfw on:");
+	if (read(IO_CDC, &in, 1, 1) < 0) {
+		// read failed! I/O broken? Just redblink.
+		assert(1 == 2);
+	}
+
+	puts(IO_CDC, "\r\nI'm testfw on:");
 	// Output the TK1 core's NAME0 and NAME1
 	uint32_t name;
 	wordcpy_s(&name, 1, (void *)tk1name0, 1);
 	reverseword(&name);
-	write((const uint8_t *)&name, 4);
+	write(IO_CDC, (const uint8_t *)&name, 4);
-	puts(" ");
+	puts(IO_CDC, " ");
 	wordcpy_s(&name, 1, (void *)tk1name1, 1);
 	reverseword(&name);
-	write((const uint8_t *)&name, 4);
+	write(IO_CDC, (const uint8_t *)&name, 4);
-	puts("\r\n");
+	puts(IO_CDC, "\r\n");
 
 	uint32_t zeros[8];
 	memset(zeros, 0, 8 * 4);
@@ -240,11 +158,11 @@ int main(void)
 		anyfailed = 1;
 	}
 
-	puts("\r\nUDS: ");
+	puts(IO_CDC, "\r\nUDS: ");
 	for (int i = 0; i < UDS_WORDS * 4; i++) {
-		puthex(((uint8_t *)uds_local)[i]);
+		puthex(IO_CDC, ((uint8_t *)uds_local)[i]);
 	}
-	puts("\r\n");
+	puts(IO_CDC, "\r\n");
 	if (!memeq(uds_local, uds_test, UDS_WORDS * 4)) {
 		failmsg("UDS not equal to test UDS");
 		anyfailed = 1;
@@ -287,7 +205,7 @@ int main(void)
 	}
 
 	// Test FW_RAM.
-	puts("\r\nTesting FW_RAM (takes 50s on hw)...\r\n");
+	puts(IO_CDC, "\r\nTesting FW_RAM (takes 50s on hw)...\r\n");
 	for (unsigned int i = 0; i < TK1_MMIO_FW_RAM_SIZE; i++) {
 		zero_fwram();
 		*(volatile uint8_t *)(TK1_MMIO_FW_RAM_BASE + i) = 0x42;
@@ -297,7 +215,7 @@ int main(void)
 		}
 	}
 
-	puts("\r\nTesting timer... 3");
+	puts(IO_CDC, "\r\nTesting timer... 3");
 	// Matching clock at 24 MHz, giving us timer in seconds
 	*timer_prescaler = 24 * 1000000;
 
@@ -308,7 +226,7 @@ int main(void)
 	while (*timer_status & (1 << TK1_MMIO_TIMER_STATUS_RUNNING_BIT)) {
 	}
 	// Now timer has expired and is ready to run again
-	puts(" 2");
+	puts(IO_CDC, " 2");
 
 	// Test to interrupt a timer - and reads from timer register
 	// Starting 10s timer and interrupting it in 3s...
@@ -321,7 +239,7 @@ int main(void)
 
 	// Stop the timer
 	*timer_ctrl = (1 << TK1_MMIO_TIMER_CTRL_STOP_BIT);
-	puts(" 1. done.\r\n");
+	puts(IO_CDC, " 1. done.\r\n");
 
 	if (*timer_status & (1 << TK1_MMIO_TIMER_STATUS_RUNNING_BIT)) {
 		failmsg("Timer didn't stop");
@@ -334,14 +252,14 @@ int main(void)
 	}
 
 	// Check and display test results.
-	puts("\r\n--> ");
+	puts(IO_CDC, "\r\n--> ");
 	if (anyfailed) {
-		puts("Some test FAILED!\r\n");
+		puts(IO_CDC, "Some test FAILED!\r\n");
 	} else {
-		puts("All tests passed.\r\n");
+		puts(IO_CDC, "All tests passed.\r\n");
 	}
 
-	puts("\r\nHere are 256 bytes from the TRNG:\r\n");
+	puts(IO_CDC, "\r\nHere are 256 bytes from the TRNG:\r\n");
 
 	for (int j = 0; j < 8; j++) {
 		for (int i = 0; i < 8; i++) {
@@ -350,21 +268,28 @@ int main(void)
 			}
 			uint32_t rnd = *trng_entropy;
 			puthexn((uint8_t *)&rnd, 4);
-			puts(" ");
+			puts(IO_CDC, " ");
 		}
-		puts("\r\n");
+		puts(IO_CDC, "\r\n");
 	}
-	puts("\r\n");
+	puts(IO_CDC, "\r\n");
 
-	puts("Now echoing what you type...Type + to reset device\r\n");
+	puts(IO_CDC, "Now echoing what you type...Type + to reset device\r\n");
 	for (;;) {
-		in = readbyte(&mode, &mode_bytes_left);
+		if (readselect(IO_CDC, &endpoint, &available) < 0) {
+			// readselect failed! I/O broken? Just redblink.
+			assert(1 == 2);
+		}
+
+		if (read(IO_CDC, &in, 1, 1) < 0) {
+			// read failed! I/O broken? Just redblink.
+			assert(1 == 2);
+		}
+
 		if (in == '+') {
 			*system_reset = 1;
 		}
 
-		writebyte(MODE_CDC);
+		write(IO_CDC, &in, 1);
-		writebyte(1);
-		writebyte(in);
 	}
 }

hw/application_fpga/fw/tk1/assert.c

@@ -1,29 +0,0 @@
-/*
- * Copyright (C) 2022, 2023 - Tillitis AB
- * SPDX-License-Identifier: GPL-2.0-only
- */
-
-#include "assert.h"
-#include "lib.h"
-
-void assert_fail(const char *assertion, const char *file, unsigned int line,
-		 const char *function)
-{
-	htif_puts("assert: ");
-	htif_puts(assertion);
-	htif_puts(" ");
-	htif_puts(file);
-	htif_puts(":");
-	htif_putinthex(line);
-	htif_puts(" ");
-	htif_puts(function);
-	htif_lf();
-
-#ifndef S_SPLINT_S
-	// Force illegal instruction to halt CPU
-	asm volatile("unimp");
-#endif
-
-	// Not reached
-	__builtin_unreachable();
-}

hw/application_fpga/fw/tk1/assert.h

@@ -1,15 +0,0 @@
-/*
- * Copyright (C) 2022, 2023 - Tillitis AB
- * SPDX-License-Identifier: GPL-2.0-only
- */
-
-#ifndef ASSERT_H
-#define ASSERT_H
-
-#define assert(expr)                                                           \
-	((expr) ? (void)(0) : assert_fail(#expr, __FILE__, __LINE__, __func__))
-
-void assert_fail(const char *assertion, const char *file, unsigned int line,
-		 const char *function);
-
-#endif

hw/application_fpga/fw/tk1/blake2s/blake2s.c

@@ -6,8 +6,8 @@
 // A simple blake2s Reference Implementation.
 //======================================================================
 
-#include "../types.h"
+#include <stdint.h>
-#include "../lib.h"
+
 #include "blake2s.h"
 
 // Dummy printf() for verbose mode

hw/application_fpga/fw/tk1/blake2s/blake2s.h

@@ -4,7 +4,8 @@
 #ifndef BLAKE2S_H
 #define BLAKE2S_H
 
-#include "../types.h"
+#include <stdint.h>
+#include <stddef.h>
 
 // state context
 typedef struct {

hw/application_fpga/fw/tk1/led.c

@@ -1,15 +0,0 @@
-/*
- * Copyright (C) 2022, 2023 - Tillitis AB
- * SPDX-License-Identifier: GPL-2.0-only
- */
-
-#include "led.h"
-#include "../tk1_mem.h"
-#include "types.h"
-
-static volatile uint32_t *led = (volatile uint32_t *)TK1_MMIO_TK1_LED;
-
-void set_led(uint32_t led_value)
-{
-	*led = led_value;
-}

hw/application_fpga/fw/tk1/led.h

@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 2022, 2023 - Tillitis AB
- * SPDX-License-Identifier: GPL-2.0-only
- */
-
-#ifndef LED_H
-#define LED_H
-
-#include "../tk1_mem.h"
-#include "types.h"
-
-// clang-format off
-#define LED_BLACK 0
-#define LED_RED   (1 << TK1_MMIO_TK1_LED_R_BIT)
-#define LED_GREEN (1 << TK1_MMIO_TK1_LED_G_BIT)
-#define LED_BLUE  (1 << TK1_MMIO_TK1_LED_B_BIT)
-#define LED_WHITE (LED_RED | LED_GREEN | LED_BLUE)
-// clang-format on
-
-void set_led(uint32_t led_value);
-#endif

hw/application_fpga/fw/tk1/lib.c

@@ -1,164 +0,0 @@
-/*
- * Copyright (C) 2022-2024 - Tillitis AB
- * SPDX-License-Identifier: GPL-2.0-only
- */
-
-#include "lib.h"
-#include "assert.h"
-#include "types.h"
-
-#ifdef QEMU_CONSOLE
-struct {
-	uint32_t arr[2];
-} static volatile tohost __attribute__((section(".htif")));
-struct {
-	uint32_t arr[2];
-} /*@unused@*/ static volatile fromhost __attribute__((section(".htif")));
-
-static void htif_send(uint8_t dev, uint8_t cmd, int64_t data)
-{
-	/* endian neutral encoding with ordered 32-bit writes */
-	union {
-		uint32_t arr[2];
-		uint64_t val;
-	} encode = {.val = (uint64_t)dev << 56 | (uint64_t)cmd << 48 | data};
-	tohost.arr[0] = encode.arr[0];
-	tohost.arr[1] = encode.arr[1];
-}
-
-static void htif_set_tohost(uint8_t dev, uint8_t cmd, int64_t data)
-{
-	/* send data with specified device and command */
-	while (tohost.arr[0]) {
-#ifndef S_SPLINT_S
-		asm volatile("" : : "r"(fromhost.arr[0]));
-		asm volatile("" : : "r"(fromhost.arr[1]));
-#endif
-	}
-	htif_send(dev, cmd, data);
-}
-
-static void htif_putchar(char ch)
-{
-	htif_set_tohost((uint8_t)1, (uint8_t)1, (int64_t)ch & 0xff);
-}
-
-void htif_puts(const char *s)
-{
-	while (*s != '\0')
-		htif_putchar(*s++);
-}
-
-void htif_hexdump(void *buf, int len)
-{
-	uint8_t *byte_buf = (uint8_t *)buf;
-
-	for (int i = 0; i < len; i++) {
-		htif_puthex(byte_buf[i]);
-		if (i % 2 == 1) {
-			(void)htif_putchar(' ');
-		}
-
-		if (i != 1 && i % 16 == 1) {
-			htif_lf();
-		}
-	}
-
-	htif_lf();
-}
-
-void htif_putc(char ch)
-{
-	htif_putchar(ch);
-}
-
-void htif_lf(void)
-{
-	htif_putchar('\n');
-}
-
-void htif_puthex(uint8_t c)
-{
-	unsigned int upper = (c >> 4) & 0xf;
-	unsigned int lower = c & 0xf;
-
-	htif_putchar(upper < 10 ? '0' + upper : 'a' - 10 + upper);
-	htif_putchar(lower < 10 ? '0' + lower : 'a' - 10 + lower);
-}
-
-void htif_putinthex(const uint32_t n)
-{
-	uint8_t *buf = (uint8_t *)&n;
-
-	htif_puts("0x");
-	for (int i = 3; i > -1; i--) {
-		htif_puthex(buf[i]);
-	}
-}
-#endif
-
-void *memset(void *dest, int c, unsigned n)
-{
-	uint8_t *s = dest;
-
-	for (; n; n--, s++)
-		*s = (uint8_t)c;
-
-	/*@ -temptrans @*/
-	return dest;
-}
-
-void memcpy_s(void *dest, size_t destsize, const void *src, size_t n)
-{
-	assert(dest != NULL);
-	assert(src != NULL);
-	assert(destsize >= n);
-
-	uint8_t *src_byte = (uint8_t *)src;
-	uint8_t *dest_byte = (uint8_t *)dest;
-
-	for (size_t i = 0; i < n; i++) {
-		/*@ -nullderef @*/
-		/* splint complains that dest_byte and src_byte can be
-		 * NULL, but it seems it doesn't understand assert.
-		 * See above.
-		 */
-		dest_byte[i] = src_byte[i];
-	}
-}
-
-void wordcpy_s(void *dest, size_t destsize, const void *src, size_t n)
-{
-	assert(dest != NULL);
-	assert(src != NULL);
-	assert(destsize >= n);
-
-	uint32_t *src_word = (uint32_t *)src;
-	uint32_t *dest_word = (uint32_t *)dest;
-
-	for (size_t i = 0; i < n; i++) {
-		dest_word[i] = src_word[i];
-	}
-}
-
-int memeq(void *dest, const void *src, size_t n)
-{
-	uint8_t *src_byte = (uint8_t *)src;
-	uint8_t *dest_byte = (uint8_t *)dest;
-	int res = -1;
-
-	for (size_t i = 0; i < n; i++) {
-		if (dest_byte[i] != src_byte[i]) {
-			res = 0;
-		}
-	}
-
-	return res;
-}
-
-void secure_wipe(void *v, size_t n)
-{
-	volatile uint8_t *p = (volatile uint8_t *)v;
-	while (n--)
-		*p++ = 0;
-}

hw/application_fpga/fw/tk1/lib.h

@@ -1,32 +0,0 @@
-/*
- * Copyright (C) 2022-2024 - Tillitis AB
- * SPDX-License-Identifier: GPL-2.0-only
- */
-
-#ifndef LIB_H
-#define LIB_H
-
-#include "types.h"
-
-#ifdef QEMU_CONSOLE
-void htif_putc(char ch);
-void htif_lf(void);
-void htif_puthex(uint8_t c);
-void htif_putinthex(const uint32_t n);
-void htif_puts(const char *s);
-void htif_hexdump(void *buf, int len);
-#else
-#define htif_putc(ch)
-#define htif_lf(void)
-#define htif_puthex(c)
-#define htif_putinthex(n)
-#define htif_puts(s)
-#define htif_hexdump(buf, len)
-#endif /* QEMU_CONSOLE */
-
-void *memset(void *dest, int c, unsigned n);
-void memcpy_s(void *dest, size_t destsize, const void *src, size_t n);
-void wordcpy_s(void *dest, size_t destsize, const void *src, size_t n);
-int memeq(void *dest, const void *src, size_t n);
-void secure_wipe(void *v, size_t n);
-#endif

hw/application_fpga/fw/tk1/main.c

@@ -3,14 +3,19 @@
  * SPDX-License-Identifier: GPL-2.0-only
  */
 
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <tkey/assert.h>
+#include <tkey/debug.h>
+#include <tkey/lib.h>
+
 #include "../tk1_mem.h"
-#include "assert.h"
+
 #include "blake2s/blake2s.h"
-#include "lib.h"
 #include "proto.h"
 #include "state.h"
 #include "syscall_enable.h"
-#include "types.h"
 
 // clang-format off
 static volatile uint32_t *uds              = (volatile uint32_t *)TK1_MMIO_UDS_FIRST;
@@ -36,7 +41,7 @@ struct context {
 	uint32_t left;	    // Bytes left to receive
 	uint8_t digest[32]; // Program digest
 	uint8_t *loadaddr;  // Where we are currently loading a TKey program
-	uint8_t use_uss;    // Use USS?
+	bool use_uss;	    // Use USS?
 	uint8_t uss[32];    // User Supplied Secret, if any
 };
 
@@ -60,26 +65,26 @@ static void scramble_ram(void);
 
 static void print_hw_version(void)
 {
-	htif_puts("Hello, I'm firmware with");
+	debug_puts("Hello, I'm firmware with");
-	htif_puts(" tk1_name0:");
+	debug_puts(" tk1_name0:");
-	htif_putinthex(*name0);
+	debug_putinthex(*name0);
-	htif_puts(" tk1_name1:");
+	debug_puts(" tk1_name1:");
-	htif_putinthex(*name1);
+	debug_putinthex(*name1);
-	htif_puts(" tk1_version:");
+	debug_puts(" tk1_version:");
-	htif_putinthex(*ver);
+	debug_putinthex(*ver);
-	htif_lf();
+	debug_lf();
 }
 
 static void print_digest(uint8_t *md)
 {
-	htif_puts("The app digest:\n");
+	debug_puts("The app digest:\n");
 	for (int j = 0; j < 4; j++) {
 		for (int i = 0; i < 8; i++) {
-			htif_puthex(md[i + 8 * j]);
+			debug_puthex(md[i + 8 * j]);
 		}
-		htif_lf();
+		debug_lf();
 	}
-	htif_lf();
+	debug_lf();
 }
 
 static uint32_t rnd_word(void)
@@ -151,20 +156,20 @@ static enum state initial_commands(const struct frame_header *hdr,
 				   const uint8_t *cmd, enum state state,
 				   struct context *ctx)
 {
-	uint8_t rsp[CMDLEN_MAXBYTES] = {0};
+	uint8_t rsp[CMDSIZE] = {0};
 
 	switch (cmd[0]) {
 	case FW_CMD_NAME_VERSION:
-		htif_puts("cmd: name-version\n");
+		debug_puts("cmd: name-version\n");
 		if (hdr->len != 1) {
 			// Bad length
 			state = FW_STATE_FAIL;
 			break;
 		}
 
-		copy_name(rsp, CMDLEN_MAXBYTES, *name0);
+		copy_name(rsp, CMDSIZE, *name0);
-		copy_name(&rsp[4], CMDLEN_MAXBYTES - 4, *name1);
+		copy_name(&rsp[4], CMDSIZE - 4, *name1);
-		wordcpy_s(&rsp[8], CMDLEN_MAXBYTES / 4 - 2, (void *)ver, 1);
+		wordcpy_s(&rsp[8], CMDSIZE - 8, (void *)ver, 1);
 
 		fwreply(*hdr, FW_RSP_NAME_VERSION, rsp);
 		// still initial state
@@ -173,7 +178,7 @@ static enum state initial_commands(const struct frame_header *hdr,
 	case FW_CMD_GET_UDI: {
 		uint32_t udi_words[2];
 
-		htif_puts("cmd: get-udi\n");
+		debug_puts("cmd: get-udi\n");
 		if (hdr->len != 1) {
 			// Bad length
 			state = FW_STATE_FAIL;
@@ -182,7 +187,7 @@ static enum state initial_commands(const struct frame_header *hdr,
 
 		rsp[0] = STATUS_OK;
 		wordcpy_s(&udi_words, 2, (void *)udi, 2);
-		memcpy_s(&rsp[1], CMDLEN_MAXBYTES - 1, &udi_words, 2 * 4);
+		memcpy_s(&rsp[1], CMDSIZE - 1, &udi_words, 2 * 4);
 		fwreply(*hdr, FW_RSP_GET_UDI, rsp);
 		// still initial state
 		break;
@@ -191,7 +196,7 @@ static enum state initial_commands(const struct frame_header *hdr,
 	case FW_CMD_LOAD_APP: {
 		uint32_t local_app_size;
 
-		htif_puts("cmd: load-app(size, uss)\n");
+		debug_puts("cmd: load-app(size, uss)\n");
 		if (hdr->len != 128) {
 			// Bad length
 			state = FW_STATE_FAIL;
@@ -202,9 +207,9 @@ static enum state initial_commands(const struct frame_header *hdr,
 		local_app_size =
 		    cmd[1] + (cmd[2] << 8) + (cmd[3] << 16) + (cmd[4] << 24);
 
-		htif_puts("app size: ");
+		debug_puts("app size: ");
-		htif_putinthex(local_app_size);
+		debug_putinthex(local_app_size);
-		htif_lf();
+		debug_lf();
 
 		if (local_app_size == 0 || local_app_size > TK1_APP_MAX_SIZE) {
 			rsp[0] = STATUS_BAD;
@@ -218,10 +223,10 @@ static enum state initial_commands(const struct frame_header *hdr,
 		// Do we have a USS at all?
 		if (cmd[5] != 0) {
 			// Yes
-			ctx->use_uss = TRUE;
+			ctx->use_uss = true;
 			memcpy_s(ctx->uss, 32, &cmd[6], 32);
 		} else {
-			ctx->use_uss = FALSE;
+			ctx->use_uss = false;
 		}
 
 		rsp[0] = STATUS_OK;
@@ -237,9 +242,9 @@ static enum state initial_commands(const struct frame_header *hdr,
 	}
 
 	default:
-		htif_puts("Got unknown firmware cmd: 0x");
+		debug_puts("Got unknown firmware cmd: 0x");
-		htif_puthex(cmd[0]);
+		debug_puthex(cmd[0]);
-		htif_lf();
+		debug_lf();
 		state = FW_STATE_FAIL;
 		break;
 	}
@@ -251,12 +256,12 @@ static enum state loading_commands(const struct frame_header *hdr,
 				   const uint8_t *cmd, enum state state,
 				   struct context *ctx)
 {
-	uint8_t rsp[CMDLEN_MAXBYTES] = {0};
+	uint8_t rsp[CMDSIZE] = {0};
 	uint32_t nbytes = 0;
 
 	switch (cmd[0]) {
 	case FW_CMD_LOAD_APP_DATA:
-		htif_puts("cmd: load-app-data\n");
+		debug_puts("cmd: load-app-data\n");
 		if (hdr->len != 128) {
 			// Bad length
 			state = FW_STATE_FAIL;
@@ -278,9 +283,9 @@ static enum state loading_commands(const struct frame_header *hdr,
 			blake2s_ctx b2s_ctx = {0};
 			int blake2err = 0;
 
-			htif_puts("Fully loaded ");
+			debug_puts("Fully loaded ");
-			htif_putinthex(*app_size);
+			debug_putinthex(*app_size);
-			htif_lf();
+			debug_lf();
 
 			// Compute Blake2S digest of the app,
 			// storing it for FW_STATE_RUN
@@ -293,8 +298,7 @@ static enum state loading_commands(const struct frame_header *hdr,
 			// And return the digest in final
 			// response
 			rsp[0] = STATUS_OK;
-			memcpy_s(&rsp[1], CMDLEN_MAXBYTES - 1, &ctx->digest,
+			memcpy_s(&rsp[1], CMDSIZE - 1, &ctx->digest, 32);
-				 32);
 			fwreply(*hdr, FW_RSP_LOAD_APP_DATA_READY, rsp);
 
 			state = FW_STATE_RUN;
@@ -307,9 +311,9 @@ static enum state loading_commands(const struct frame_header *hdr,
 		break;
 
 	default:
-		htif_puts("Got unknown firmware cmd: 0x");
+		debug_puts("Got unknown firmware cmd: 0x");
-		htif_puthex(cmd[0]);
+		debug_puthex(cmd[0]);
-		htif_lf();
+		debug_lf();
 		state = FW_STATE_FAIL;
 		break;
 	}
@@ -324,10 +328,10 @@ static void run(const struct context *ctx)
 	// CDI = hash(uds, hash(app), uss)
 	compute_cdi(ctx->digest, ctx->use_uss, ctx->uss);
 
-	htif_puts("Flipping to app mode!\n");
+	debug_puts("Flipping to app mode!\n");
-	htif_puts("Jumping to ");
+	debug_puts("Jumping to ");
-	htif_putinthex(*app_addr);
+	debug_putinthex(*app_addr);
-	htif_lf();
+	debug_lf();
 
 	// Clear the firmware stack
 	// clang-format off
@@ -400,7 +404,7 @@ int main(void)
 {
 	struct context ctx = {0};
 	struct frame_header hdr = {0};
-	uint8_t cmd[CMDLEN_MAXBYTES] = {0};
+	uint8_t cmd[CMDSIZE] = {0};
 	enum state state = FW_STATE_INITIAL;
 
 	print_hw_version();
@@ -411,10 +415,7 @@ int main(void)
 	 */
 	ctx.loadaddr = (uint8_t *)TK1_RAM_BASE;
 	/*@+mustfreeonly@*/
-	ctx.use_uss = FALSE;
+	ctx.use_uss = false;
-
-	uint8_t mode = 0;
-	uint8_t mode_bytes_left = 0;
 
 	scramble_ram();
 
@@ -425,18 +426,19 @@ int main(void)
 	for (;;) {
 		switch (state) {
 		case FW_STATE_INITIAL:
-			if (readcommand(&hdr, cmd, state, &mode,
+			if (readcommand(&hdr, cmd, state) == -1) {
-					&mode_bytes_left) == -1) {
 				state = FW_STATE_FAIL;
 				break;
 			}
 
+			debug_puts("cmd: \n");
+			debug_hexdump(cmd, hdr.len);
+
 			state = initial_commands(&hdr, cmd, state, &ctx);
 			break;
 
 		case FW_STATE_LOADING:
-			if (readcommand(&hdr, cmd, state, &mode,
+			if (readcommand(&hdr, cmd, state) == -1) {
-					&mode_bytes_left) == -1) {
 				state = FW_STATE_FAIL;
 				break;
 			}
@@ -451,9 +453,9 @@ int main(void)
 		case FW_STATE_FAIL:
 			// fallthrough
 		default:
-			htif_puts("firmware state 0x");
+			debug_puts("firmware state 0x");
-			htif_puthex(state);
+			debug_puthex(state);
-			htif_lf();
+			debug_lf();
 			assert(1 == 2);
 			break; // Not reached
 		}

hw/application_fpga/fw/tk1/proto.c

@@ -3,27 +3,19 @@
  * SPDX-License-Identifier: GPL-2.0-only
  */
 
-#include "proto.h"
+#include <stdint.h>
-#include "../tk1_mem.h"
+#include <tkey/assert.h>
-#include "assert.h"
+#include <tkey/debug.h>
-#include "led.h"
+#include <tkey/io.h>
-#include "lib.h"
+#include <tkey/led.h>
-#include "state.h"
+#include <tkey/lib.h>
-#include "types.h"
 
-// clang-format off
+#include "proto.h"
-static volatile uint32_t *can_rx = (volatile uint32_t *)TK1_MMIO_UART_RX_STATUS;
+#include "state.h"
-static volatile uint32_t *rx =     (volatile uint32_t *)TK1_MMIO_UART_RX_DATA;
-static volatile uint32_t *can_tx = (volatile uint32_t *)TK1_MMIO_UART_TX_STATUS;
-static volatile uint32_t *tx =     (volatile uint32_t *)TK1_MMIO_UART_TX_DATA;
-// clang-format on
 
 static uint8_t genhdr(uint8_t id, uint8_t endpoint, uint8_t status,
 		      enum cmdlen len);
 static int parseframe(uint8_t b, struct frame_header *hdr);
-static void write(uint8_t *buf, size_t nbytes);
-static int read(uint8_t *buf, size_t bufsize, size_t nbytes, uint8_t *mode,
-		uint8_t *mode_bytes_left);
 static size_t bytelen(enum cmdlen cmdlen);
 
 static uint8_t genhdr(uint8_t id, uint8_t endpoint, uint8_t status,
@@ -32,29 +24,59 @@ static uint8_t genhdr(uint8_t id, uint8_t endpoint, uint8_t status,
 	return (id << 5) | (endpoint << 3) | (status << 2) | len;
 }
 
-int readcommand(struct frame_header *hdr, uint8_t *cmd, int state,
+int readcommand(struct frame_header *hdr, uint8_t *cmd, int state)
-		uint8_t *mode, uint8_t *mode_bytes_left)
 {
 	uint8_t in = 0;
+	uint8_t available = 0;
+	enum ioend endpoint = IO_NONE;
 
-	set_led((state == FW_STATE_LOADING) ? LED_BLACK : LED_WHITE);
+	led_set((state == FW_STATE_LOADING) ? LED_BLACK : LED_WHITE);
-	in = readbyte(mode, mode_bytes_left);
 
-	if (parseframe(in, hdr) == -1) {
+	debug_puts("readcommand\n");
-		htif_puts("Couldn't parse header\n");
+
+	if (readselect(IO_CDC, &endpoint, &available) < 0) {
 		return -1;
 	}
 
-	(void)memset(cmd, 0, CMDLEN_MAXBYTES);
+	if (read(IO_CDC, &in, 1, 1) == -1) {
-	// Now we know the size of the cmd frame, read it all
-	if (read(cmd, CMDLEN_MAXBYTES, hdr->len, mode, mode_bytes_left) != 0) {
-		htif_puts("read: buffer overrun\n");
 		return -1;
 	}
 
+	debug_puts("read 1 byte\n");
+
+	if (parseframe(in, hdr) == -1) {
+		debug_puts("Couldn't parse header\n");
+		return -1;
+	}
+
+	debug_puts("parseframe succeeded\n");
+
+	(void)memset(cmd, 0, CMDSIZE);
+
+	// Now we know the size of the cmd frame, read it all
+	uint8_t n = 0;
+	while (n < hdr->len) {
+		// Wait for something to be available
+		if (readselect(IO_CDC, &endpoint, &available) < 0) {
+			return -1;
+		}
+
+		// Read as much as is available of what we expect
+		available = available > hdr->len ? hdr->len : available;
+
+		assert(n < CMDSIZE);
+		int n_bytes_read =
+		    read(IO_CDC, &cmd[n], CMDSIZE - n, available);
+		if (n_bytes_read < 0) {
+			return -1;
+		}
+
+		n += n_bytes_read;
+	}
+
 	// Is it for us?
 	if (hdr->endpoint != DST_FW) {
-		htif_puts("Message not meant for us\n");
+		debug_puts("Message not meant for us\n");
 		return -1;
 	}
 
@@ -86,6 +108,7 @@ void fwreply(struct frame_header hdr, enum fwcmd rspcode, uint8_t *buf)
 {
 	size_t nbytes = 0;
 	enum cmdlen len = 0;	// length covering (rspcode + length of buf)
+	uint8_t frame[1 + 128]; // Frame header + longest response
 
 	switch (rspcode) {
 	case FW_RSP_NAME_VERSION:
@@ -109,95 +132,23 @@ void fwreply(struct frame_header hdr, enum fwcmd rspcode, uint8_t *buf)
 		break;
 
 	default:
-		htif_puts("fwreply(): Unknown response code: 0x");
+		debug_puts("fwreply(): Unknown response code: 0x");
-		htif_puthex(rspcode);
+		debug_puthex(rspcode);
-		htif_lf();
+		debug_lf();
 		return;
 	}
 
 	nbytes = bytelen(len);
 
-	// Mode Protocol Header
-	writebyte(MODE_CDC);
-	writebyte(2);
-
 	// Frame Protocol Header
-	writebyte(genhdr(hdr.id, hdr.endpoint, 0x0, len));
+	frame[0] = genhdr(hdr.id, hdr.endpoint, 0x0, len);
+	// App protocol header
+	frame[1] = rspcode;
 
-	// FW protocol header
+	// Payload
-	writebyte(rspcode);
+	memcpy(&frame[2], buf, nbytes - 1);
-	nbytes--;
 
-	while (nbytes > 0) {
+	write(IO_CDC, frame, 1 + nbytes);
-		// Limit transfers to 64 bytes (2 byte header + 62 byte data) to
-		// fit in a single USB frame.
-		size_t tx_count = nbytes > 62 ? 62 : nbytes;
-		// Mode Protocol Header
-		writebyte(MODE_CDC);
-		writebyte(tx_count & 0xff);
-
-		// Data
-		write(buf, tx_count);
-		nbytes -= tx_count;
-		buf += tx_count;
-	}
-}
-
-void writebyte(uint8_t b)
-{
-	for (;;) {
-		if (*can_tx) {
-			*tx = b;
-			return;
-		}
-	}
-}
-
-static void write(uint8_t *buf, size_t nbytes)
-{
-	for (int i = 0; i < nbytes; i++) {
-		writebyte(buf[i]);
-	}
-}
-
-uint8_t readbyte_(void)
-{
-	for (;;) {
-		if (*can_rx) {
-			uint32_t b = *rx;
-			return b;
-		}
-	}
-}
-
-uint8_t readbyte(uint8_t *mode, uint8_t *mode_bytes_left)
-{
-	if (*mode_bytes_left == 0) {
-		*mode = readbyte_();
-		if (*mode != MODE_CDC) {
-			htif_puts("We only support MODE_CDC\n");
-			assert(1 == 2);
-		} else {
-			*mode_bytes_left = readbyte_();
-		}
-	}
-	uint8_t b = readbyte_();
-	*mode_bytes_left -= 1;
-	return b;
-}
-
-static int read(uint8_t *buf, size_t bufsize, size_t nbytes, uint8_t *mode,
-		uint8_t *mode_bytes_left)
-{
-	if (nbytes > bufsize) {
-		return -1;
-	}
-
-	for (int n = 0; n < nbytes; n++) {
-		buf[n] = readbyte(mode, mode_bytes_left);
-	}
-
-	return 0;
 }
 
 // bytelen returns the number of bytes a cmdlen takes

hw/application_fpga/fw/tk1/proto.h

@@ -3,17 +3,12 @@
  * SPDX-License-Identifier: GPL-2.0-only
  */
 
-#include "types.h"
+#include <stddef.h>
+#include <stdint.h>
 
 #ifndef PROTO_H
 #define PROTO_H
 
-enum mode {
-	MODE_TKEYCTRL = 0x20,
-	MODE_CDC = 0x40,
-	MODE_HID = 0x80,
-};
-
 enum endpoints {
 	DST_HW_IFPGA,
 	DST_HW_AFPGA,
@@ -28,7 +23,7 @@ enum cmdlen {
 	LEN_128
 };
 
-#define CMDLEN_MAXBYTES 128
+#define CMDSIZE 128
 
 // clang-format off
 enum fwcmd {
@@ -57,9 +52,6 @@ struct frame_header {
 };
 
 /*@ -exportlocal @*/
-void writebyte(uint8_t b);
-uint8_t readbyte(uint8_t *mode, uint8_t *mode_bytes_left);
 void fwreply(struct frame_header hdr, enum fwcmd rspcode, uint8_t *buf);
-int readcommand(struct frame_header *hdr, uint8_t *cmd, int state,
+int readcommand(struct frame_header *hdr, uint8_t *cmd, int state);
-		uint8_t *mode, uint8_t *mode_bytes_left);
 #endif

hw/application_fpga/fw/tk1/syscall_handler.c

@@ -3,10 +3,11 @@
  * SPDX-License-Identifier: GPL-2.0-only
  */
 
-#include "../tk1/assert.h"
+#include <stdint.h>
-#include "../tk1/led.h"
+#include <tkey/assert.h>
+#include <tkey/led.h>
+
 #include "../tk1/syscall_num.h"
-#include "../tk1/types.h"
 
 // clang-format off
 static volatile uint32_t *system_reset = (volatile uint32_t *)TK1_MMIO_TK1_SYSTEM_RESET;
@@ -20,7 +21,7 @@ int32_t syscall_handler(uint32_t number, uint32_t arg1)
 		*system_reset = 1;
 		return 0;
 	case TK1_SYSCALL_SET_LED:
-		set_led(arg1);
+		led_set(arg1);
 		return 0;
 	case TK1_SYSCALL_GET_VIDPID:
 		// UDI is 2 words: VID/PID & serial. Return just the

hw/application_fpga/fw/tk1/types.h

@@ -1,22 +0,0 @@
-/*
- * Copyright (C) 2022 - Tillitis AB
- * SPDX-License-Identifier: GPL-2.0-only
- */
-
-#ifndef TYPES_H
-#define TYPES_H
-
-typedef unsigned int uintptr_t;
-typedef unsigned long long uint64_t;
-typedef unsigned int uint32_t;
-typedef int int32_t;
-typedef long long int64_t;
-typedef unsigned char uint8_t;
-typedef unsigned long size_t;
-
-#define NULL ((char *)0)
-
-#define FALSE 0
-#define TRUE !FALSE
-
-#endif