firmware/main: Cleanup pass on the led control code

Signed-off-by: Sylvain Munaut <tnt@246tNt.com>

firmware/main/led_ctrl.c

@@ -13,20 +13,28 @@
 
 #include "config.h"
 #include "pmu.h"
+#include "utils.h"
 
 
+/* ------------------------------------------------------------------------ */
+/* Hardware definitions                                                     */
+/* ------------------------------------------------------------------------ */
+
 /*
- * 31            24  23                16 15               8 7                  0
- * --------------------------------------------------------------------------------
- * |     stop[0]     |      start[0]      |        /         |     anode_sel      |
- * --------------------------------------------------------------------------------
- * |     stop[2]     |      start[2]      |      stop[1]     |      start[1]      |
- * --------------------------------------------------------------------------------
+ * Subframe format:
+ *
+ *  31            24 23             16 15             8 7               0
+ * -----------------------------------------------------------------------
+ * |    stop[0]     |    start[0]     |       /        |    anode_sel    |
+ * -----------------------------------------------------------------------
+ * |    stop[2]     |    start[2]     |    stop[1]     |    start[1]     |
+ * -----------------------------------------------------------------------
  */
 
 #define LED_N_FRAMES	16
 #define LED_N_SUBFRAMES	16
 
+
 struct led_subframe {
 	union {
 		struct {
@@ -52,6 +60,7 @@ struct wb_led_ctrl {
 	} frame[LED_N_FRAMES];
 } __attribute__((packed,aligned(4)));
 
+
 #define LED_CSR_TRIG_CLR	(1 << 31)
 #define LED_CSR_TRIG_ENA	(1 << 30)
 #define LED_CSR_TRIG_FRAME(n)	(((n) & 0xf) << 24)
@@ -64,23 +73,11 @@ struct wb_led_ctrl {
 static volatile struct wb_led_ctrl * const led_regs = (void*)(LED_CTRL_BASE);
 
 
-static struct {
-	uint8_t  frame_nxt;
-	uint32_t time;
-	uint16_t leds[14*3];
-	struct led_subframe subframe[LED_N_SUBFRAMES+1];
-} g_led;
-
-
-
-static uint32_t rng = 1234;
-static uint16_t rand16()
-{
-	rng = (rng * 22695477) + 1;
-	return rng >> 12;
-}
-
+/* ------------------------------------------------------------------------ */
+/* CIE luminosity                                                           */
+/* ------------------------------------------------------------------------ */
 
+/* Maps every PWM [1-256] to a perceived luminosity value [0-65535] */
 static const uint16_t cie[] = {
 	 1156,  3468,  5703,  7651,  9253, 10628, 11843, 12938,
 	13938, 14863, 15724, 16533, 17295, 18018, 18705, 19362,
@@ -116,16 +113,7 @@ static const uint16_t cie[] = {
 	64785, 64886, 64986, 65087, 65187, 65287, 65386, 65485,
 };
 
-static const int k_scale[] = {
-	189,	/* Emerald */
-	256,	/* Pink */
-	236,	/* Blue */
-};
-
-#define K0	1	/* Pink */
-#define K1	0	/* Emerald */
-#define K2	2	/* Blue */
-
+/* Converts a requested luminosity value to PWM length */
 static uint16_t
 cie_lum2pwm(uint16_t lum)
 {
@@ -146,12 +134,13 @@ cie_lum2pwm(uint16_t lum)
 }
 
 
+/* ------------------------------------------------------------------------ */
+/* Animation                                                                */
+/* ------------------------------------------------------------------------ */
 
 static void
 _led_render(uint32_t frame, uint16_t *leds)
 {
-
-#if 1
 	/* Decrease luminosoty so they all fade out in 15 second */
 	for (int i=0; i<42; i++)
 	{
@@ -174,23 +163,30 @@ _led_render(uint32_t frame, uint16_t *leds)
 		i = (rand16() % 42);
 		leds[i] = 35000;
 	}
-
-#else
-	for (int i=0; i<42; i+=3)
-	{
-		leds[i+0] = 35000;	/* Emerald */
-		leds[i+1] = 35000;	/* Pink */
-		leds[i+2] = 35000;	/* Blue */
-	}
-	
-#endif
 }
 
+
+/* ------------------------------------------------------------------------ */
+/* Frame mapping                                                            */
+/* ------------------------------------------------------------------------ */
+
+/* Mapping priority */
+#define K0	1	/* Pink */
+#define K1	0	/* Emerald */
+#define K2	2	/* Blue */
+
+/* Scaling factor to match intensity */
+static const int k_scale[] = {
+	189,	/* Emerald */
+	256,	/* Pink */
+	236,	/* Blue */
+};
+
 static void
-_led_map_one(int sf, int anode, uint16_t *pwm)
+_led_map_one(struct led_subframe *subframe, int anode, uint16_t *pwm)
 {
 	/* Select anode */
-	g_led.subframe[sf].anode = anode;
+	subframe->anode = anode;
 
 	/* Check various cases */
 	if (pwm[0] + pwm[1] + pwm[2] <= 128)
@@ -201,8 +197,8 @@ _led_map_one(int sf, int anode, uint16_t *pwm)
 
 		for (int k=0; k<3; k++) {
 			if (pwm[k] > 0) {
-				g_led.subframe[sf].cathode[k].start = o;
-				g_led.subframe[sf].cathode[k].stop  = o + pwm[k] - 1;
+				subframe->cathode[k].start = o;
+				subframe->cathode[k].stop  = o + pwm[k] - 1;
 				o += pwm[k] + s;
 			}
 		}
@@ -214,21 +210,21 @@ _led_map_one(int sf, int anode, uint16_t *pwm)
 		/* K0 split, K1 & K2 overlap */
 			/* K0 starting at 0 */
 		if (pwm[K0] > 0) {
-			g_led.subframe[sf].cathode[K0].start = o;
-			g_led.subframe[sf].cathode[K0].stop  = o + pwm[K0] - 1;
+			subframe->cathode[K0].start = o;
+			subframe->cathode[K0].stop  = o + pwm[K0] - 1;
 			o += pwm[K0];
 		}
 
 			/* K1 starts right after K0 */
 		if (pwm[K1] > 0) {
-			g_led.subframe[sf].cathode[K1].start = o;
-			g_led.subframe[sf].cathode[K1].stop  = o + pwm[K1] - 1;
+			subframe->cathode[K1].start = o;
+			subframe->cathode[K1].stop  = o + pwm[K1] - 1;
 		}
 
 			/* K2 aligned to the end */
 		if (pwm[K2] > 0) {
-			g_led.subframe[sf].cathode[K2].start = 0x7f - pwm[K2] + 1;
-			g_led.subframe[sf].cathode[K2].stop  = 0x7f;
+			subframe->cathode[K2].start = 0x7f - pwm[K2] + 1;
+			subframe->cathode[K2].stop  = 0x7f;
 		}
 	}
 	else
@@ -236,25 +232,25 @@ _led_map_one(int sf, int anode, uint16_t *pwm)
 		/* Default, K0 on one side, then K1 & K2 full overlap on the other */
 			/* K0 aligned at the end */
 		if (pwm[K0] > 0) {
-			g_led.subframe[sf].cathode[K0].start = 0x7f - pwm[K0] + 1;
-			g_led.subframe[sf].cathode[K0].stop  = 0x7f;
+			subframe->cathode[K0].start = 0x7f - pwm[K0] + 1;
+			subframe->cathode[K0].stop  = 0x7f;
 		}
 
 			/* K1 & K2 overlapping at beginning */
 		if (pwm[K1] > 0) {
-			g_led.subframe[sf].cathode[K1].start = 0;
-			g_led.subframe[sf].cathode[K1].stop  = pwm[K1] - 1;
+			subframe->cathode[K1].start = 0;
+			subframe->cathode[K1].stop  = pwm[K1] - 1;
 		}
 
 		if (pwm[K2] > 0) {
-			g_led.subframe[sf].cathode[K2].start = 0;
-			g_led.subframe[sf].cathode[K2].stop  = pwm[K2] - 1;
+			subframe->cathode[K2].start = 0;
+			subframe->cathode[K2].stop  = pwm[K2] - 1;
 		}
 	}
 }
 
 static void
-_led_map(void)
+_led_map(struct led_subframe *subframes, uint16_t *leds)
 {
 	uint16_t pwm[42];
 	int k, a, l;
@@ -262,14 +258,14 @@ _led_map(void)
 
 	/* Pre-clear */
 	for (int sf=0; sf<16; sf++) {
-		g_led.subframe[sf].w[0] = 0x007f0000;
-		g_led.subframe[sf].w[1] = 0x007f007f;
+		subframes[sf].w[0] = 0x007f0000;
+		subframes[sf].w[1] = 0x007f007f;
 	}
 
 	/* Convert all linear brightness to pwm times */
 	for (a=0, l=0; a<14; a++)
 		for (k=0; k<3; k++, l++)
-			pwm[l] = (cie_lum2pwm(g_led.leds[l]) * k_scale[k]) >> 8;
+			pwm[l] = (cie_lum2pwm(leds[l]) * k_scale[k]) >> 8;
 
 	/* Attempt packing */
 	sf_l =  0;	/* Low boundary */
@@ -290,31 +286,46 @@ _led_map(void)
 				p1[i] = (pwm[l+i] + 1) >> 1;
 			}
 
-			_led_map_one(sf_l++, a, p0);
-			_led_map_one(--sf_h, a, p1);
+			_led_map_one(&subframes[sf_l++], a, p0);
+			_led_map_one(&subframes[--sf_h], a, p1);
 		}
 		else if ((pwm[l+0] > 0) ||
 		         (pwm[l+1] > 0) ||
 		         (pwm[l+2] > 0))
 		{
 			/* Map all in one subframe */
-			_led_map_one(sf_l++, a, &pwm[l]);
+			_led_map_one(&subframes[sf_l++], a, &pwm[l]);
 		}
 	}
 }
 
+
+/* ------------------------------------------------------------------------ */
+/* Frame mapping                                                            */
+/* ------------------------------------------------------------------------ */
+
+static struct {
+	uint8_t  frame_nxt;
+	uint32_t time;
+	uint16_t leds[14*3];
+	struct led_subframe subframe[LED_N_SUBFRAMES];
+} g_led;
+
+
 static bool
 _led_fill(void)
 {
+	int f0, f1;
 	int frame_limit;
 	bool work = false;
 
-	int f0, f1;
+	/* "Safe" read from hardware */
 	do {
 		f0 = LED_CSR_GET_FRAME(led_regs->csr);
 		f1 = LED_CSR_GET_FRAME(led_regs->csr);
 	} while (f0 != f1);
 
+	/* Compute the first frame we can't write to */
 	frame_limit = (f0 - 1) & (LED_N_FRAMES - 1);
 
 	/* Fill to catch up */
@@ -324,7 +335,7 @@ _led_fill(void)
 		_led_render(g_led.time++, g_led.leds);
 
 		/* Convert to subframe */
-		_led_map();
+		_led_map(g_led.subframe, g_led.leds);
 
 		/* Copy to hardware */
 		for (int subframe=0; subframe<LED_N_SUBFRAMES; subframe++) {
@@ -340,9 +351,17 @@ _led_fill(void)
 	return work;
 }
 
+
+/* ------------------------------------------------------------------------ */
+/* External API                                                             */
+/* ------------------------------------------------------------------------ */
+
 void
 led_init(void)
 {
+	/* Shutdown hardware */
+	led_regs->csr = 0;
+
 	/* Clear internal state */
 	memset(&g_led, 0x00, sizeof(g_led));
 
@@ -353,18 +372,6 @@ led_init(void)
 			led_regs->frame[frame].subframe[subframe].w[1] = 0x007f007f;
 		}
 	}
-
-	/* Test frames */
-	for (int frame=0; frame<LED_N_FRAMES; frame++) {
-		led_regs->frame[frame].subframe[0].w[0] = 0x007f0001;
-		led_regs->frame[frame].subframe[0].w[1] = 0x007f007f;
-
-		led_regs->frame[frame].subframe[1].w[0] = 0x7f000006;
-		led_regs->frame[frame].subframe[1].w[1] = 0x7f007f00;
-
-		led_regs->frame[frame].subframe[2].w[0] = 0x007f0001;
-		led_regs->frame[frame].subframe[2].w[1] = 0x007f007f;
-	}
 }
 
 void

firmware/main/main.c

@@ -18,11 +18,15 @@
 #include "utils.h"
 #include "usb_dev.h"
 
+
+/* ------------------------------------------------------------------------ */
+/* USB                                                                      */
+/* ------------------------------------------------------------------------ */
+
 extern const struct usb_stack_descriptors app_stack_desc;
 
-
 static void
-serial_no_init()
+usb_serial_no_init(void)
 {
 	uint8_t buf[8];
 	char *id, *desc;
@@ -42,6 +46,44 @@ serial_no_init()
 		desc[2 + (i << 1)] = id[i];
 }
 
+static bool
+usb_run(void)
+{
+	/* Check current power state */
+	bool powered     = pmu_is_vbus_present();
+	bool usb_running = pmu_usb_state();
+
+	/* Power USB on/off accordingly */
+	if (powered && !usb_running)
+	{
+		/* Re-enable USB clock */
+		pmu_usb_enable();
+
+		/* Stack init (got reset) */
+		usb_init(&app_stack_desc);
+		usb_dfu_rt_init();
+		usb_dev_init();
+		usb_connect();
+
+		/* We're up ! */
+		usb_running = true;
+	}
+	else if (!powered & usb_running)
+	{
+		/* Shutdown USB clock (and reset USB core) */
+		pmu_usb_disable();
+
+		/* USB not active, don't try to access it */
+		usb_running = false;
+	}
+
+	/* Run USB if need be */
+	if (usb_running)
+		usb_poll();
+
+	return usb_running;
+}
+
 void
 usb_dfu_rt_cb_reboot(void)
 {
@@ -52,36 +94,46 @@ usb_dfu_rt_cb_reboot(void)
 	pmu_sys_reboot(1);
 }
 
-//#define USB 1
+
+/* ------------------------------------------------------------------------ */
+/* Main                                                                     */
+/* ------------------------------------------------------------------------ */
+
 void main()
 {
 	/* SPI */
 	spi_init();
-	serial_no_init();
+	usb_serial_no_init();
+
+	/* Clear events */
+	pmu_get_buttons();
 
 	/* Enable LEDs */
 	led_init();
 	led_start();
 
-	/* Enable USB directly */
-//#define USB
-#ifdef USB
-	pmu_usb_enable();
-	usb_init(&app_stack_desc);
-	usb_dfu_rt_init();
-	usb_dev_init();
-	usb_connect();
-#endif
-
 	/* Main loop */
 	while (1)
 	{
-#ifdef USB
-		/* USB poll */
-		usb_poll();
-		led_poll(false);
-#else
-		led_poll(true);
-#endif
+		/* Run USB */
+		bool usb_running = usb_run();
+
+		/* Check buttons */
+		uint8_t btn = pmu_get_buttons();
+
+		if (btn & BTN_1_SHORT) {
+			/* FIXME */
+		} else if (btn & BTN_1_LONG) {
+			/* FIXME */
+		}
+
+		if (btn & BTN_2_SHORT) {
+			/* FIXME */
+		} else if (btn & BTN_2_LONG) {
+			/* FIXME */
+		}
+
+		/* LED */
+		led_poll(!usb_running);
 	}
 }

firmware/main/pmu.c

@@ -14,17 +14,22 @@ struct wb_pmu {
 	uint32_t stat;
 } __attribute__((packed,aligned(4)));
 
-#define PMU_CTRL_SYS_SHUTDOWN	(1 << 7)
-#define PMU_CTRL_SYS_SUSPEND	(1 << 6)
-#define PMU_CTRL_USB_OFF	(1 << 5)
-#define PMU_CTRL_USB_ON		(1 << 4)
-#define PMU_CTRL_BOOT_EXEC	(1 << 2)
-#define PMU_CTRL_BOOT_SEL(n)	(((n) & 3) << 0)
+#define PMU_CTRL_BOOT_EXEC	(1 << 18)
+#define PMU_CTRL_BOOT_SEL(n)	(((n) & 3) << 16)
+#define PMU_CTRL_SYS_SHUTDOWN	(1 << 11)
+#define PMU_CTRL_SYS_SUSPEND	(1 << 10)
+#define PMU_CTRL_USB_OFF	(1 <<  9)
+#define PMU_CTRL_USB_ON		(1 <<  8)
+#define PMU_CTRL_BTN2_ACK       (1 <<  3)
+#define PMU_CTRL_BTN1_ACK       (1 <<  1)
 
-#define PMU_STAT_USB		(1 << 4)
-#define PMU_STAT_CHARGING	(1 << 3)
-#define PMU_STAT_VBUS		(1 << 2)
-#define PMU_STAT_BTN_MSK	(3 << 0)
+#define PMU_STAT_USB		(1 <<  8)
+#define PMU_STAT_VBUS		(1 <<  5)
+#define PMU_STAT_CHARGING	(1 <<  4)
+#define PMU_STAT_BTN2_EVT	(1 <<  3)
+#define PMU_STAT_BTN2_LONG	(1 <<  2)
+#define PMU_STAT_BTN1_EVT	(1 <<  1)
+#define PMU_STAT_BTN1_LONG	(1 <<  0)
 
 
 static volatile struct wb_pmu * const pmu_regs= (void*)(PMU_BASE);
@@ -90,5 +95,27 @@ pmu_is_vbus_present(void)
 uint8_t
 pmu_get_buttons(void)
 {
-	return pmu_regs->stat & PMU_STAT_BTN_MSK;
+	uint32_t btn;
+	uint32_t ack = 0;
+	uint8_t  rv = 0;
+
+	/* Read status */
+	btn = pmu_regs->stat;
+
+	/* Translate to events */
+	if (btn & PMU_STAT_BTN1_EVT) {
+		rv |= (btn & PMU_STAT_BTN1_LONG) ? BTN_1_LONG : BTN_1_SHORT;
+		ack |= PMU_CTRL_BTN1_ACK;
+	}
+
+	if (btn & PMU_STAT_BTN2_EVT) {
+		rv |= (btn & PMU_STAT_BTN2_LONG) ? BTN_2_LONG : BTN_2_SHORT;
+		ack |= PMU_CTRL_BTN2_ACK;
+	}
+
+	/* Ack */
+	if (ack)
+		pmu_regs->ctrl = ack;
+
+	return rv;
 }

firmware/main/pmu.h

@@ -22,4 +22,9 @@ bool    pmu_usb_state(void);
 bool    pmu_is_charging(void);
 bool    pmu_is_vbus_present(void);
 
+#define BTN_2_LONG	(1 << 3)
+#define BTN_2_SHORT	(1 << 2)
+#define BTN_1_LONG	(1 << 1)
+#define BTN_1_SHORT	(1 << 0)
+
 uint8_t pmu_get_buttons(void);

firmware/main/utils.c

@@ -44,3 +44,12 @@ hexval(char c)
 	else
 		return 0;
 }
+
+
+uint16_t
+rand16()
+{
+	static uint32_t rng = 1234;
+        rng = (rng * 22695477) + 1;
+        return rng >> 12;
+}

firmware/main/utils.h

@@ -11,3 +11,5 @@
 
 char *hexstr(void *d, int n, bool space);
 uint8_t hexval(char c);
+
+uint16_t rand16();

gateware/data/top-xmas-snoopy.pcf

@@ -12,8 +12,8 @@ set_io -nowarn -pullup no usb_dn 18
 set_io -nowarn -pullup no usb_pu 21
 
 # Power
-set_io -nowarn -pullup yes pwr_usb_n 45
-set_io -nowarn -pullup yes pwr_chg_n 44
+set_io -nowarn -pullup yes -pullup_resistor 100K pwr_usb_n 45
+set_io -nowarn -pullup yes -pullup_resistor 100K pwr_chg_n 44
 set_io -nowarn -pullup no  pwr_off   47
 
 # Buttons

gateware/rtl/pmu.v

@@ -41,33 +41,28 @@ module pmu (
 	input  wire rst
 );
 
-	integer i;
-
 	// Signals
 	// -------
 
 	// Bus interface
-	wire      bus_clr_rd;
-	wire      bus_clr_wr;
-	wire      bus_is_suspend;
-	reg [3:0] bus_ack_dly;
-	reg       bus_we;
+	wire        bus_clr_rd;
+	wire        bus_clr_wr;
+	wire        bus_is_suspend;
+	reg   [3:0] bus_ack_dly;
+	reg         bus_we;
 
 	// Control signals
-	reg       ctrl_sys_shutdown;
-	reg       ctrl_sys_suspend;
-	reg       ctrl_usb_off;
-	reg       ctrl_usb_on;
+	reg         ctrl_sys_shutdown;
+	reg         ctrl_sys_suspend;
+	reg         ctrl_usb_off;
+	reg         ctrl_usb_on;
+	reg   [1:0] ctrl_btn_ack;
+	wire  [1:0] stat_btn [0:1];
 
 	// Buttons
 	wire        btn_clk;
-	wire  [1:0] btn_iob;
-	reg   [2:0] btn_sync [0:1];
-	reg   [4:0] btn_cnt  [0:1];
-	reg   [1:0] btn_val;
-	reg   [1:0] btn_val_wb;
-	reg  [16:0] btn_long_cnt [0:1];
-	reg   [1:0] btn_long;
+	wire  [1:0] btn_trigged;
+	wire  [1:0] btn_verylong;
 
 	// Reboot
 	reg         boot_wb_now;
@@ -90,14 +85,14 @@ module pmu (
 	// -------------
 
 	// Clears
-	assign bus_clr_rd = wb_ack | ~wb_cyc;
-	assign bus_clr_wr = wb_ack | ~wb_cyc | ~wb_we;
+	assign bus_clr_rd =  wb_ack | ~wb_cyc;
+	assign bus_clr_wr =  wb_ack | ~wb_cyc | ~wb_we;
 
 	// Ack
 	always @(posedge clk)
 		wb_ack <= wb_cyc & (~bus_is_suspend | bus_ack_dly[3]) & ~wb_ack;
 
-	assign bus_is_suspend = wb_we & wb_wdata[6];
+	assign bus_is_suspend = wb_we & wb_wdata[10];
 
 	always @(posedge clk)
 		if (bus_clr_wr)
@@ -113,10 +108,11 @@ module pmu (
 			wb_rdata <= {
 				27'd0,
 				usb_ena,
+				2'b00,
 				stat_pwr_usb,
 				stat_pwr_chg,
-				btn_val_wb[1],
-				btn_val_wb[0]
+				stat_btn[1],
+				stat_btn[0]
 			};
 
 	// Writes
@@ -127,12 +123,16 @@ module pmu (
 			ctrl_sys_suspend  <= 1'b0;
 			ctrl_usb_off      <= 1'b0;
 			ctrl_usb_on       <= 1'b0;
+			ctrl_btn_ack[1]   <= 1'b0;
+			ctrl_btn_ack[0]   <= 1'b0;
 		end else begin
 			bus_we            <= 1'b1;
-			ctrl_sys_shutdown <= wb_wdata[7];
-			ctrl_sys_suspend  <= wb_wdata[6];
-			ctrl_usb_off      <= wb_wdata[5];
-			ctrl_usb_on       <= wb_wdata[4];
+			ctrl_sys_shutdown <= wb_wdata[11];
+			ctrl_sys_suspend  <= wb_wdata[10];
+			ctrl_usb_off      <= wb_wdata[ 9];
+			ctrl_usb_on       <= wb_wdata[ 8];
+			ctrl_btn_ack[1]   <= wb_wdata[ 3];
+			ctrl_btn_ack[0]   <= wb_wdata[ 1];
 		end
 
 	always @(posedge clk or posedge rst)
@@ -140,8 +140,8 @@ module pmu (
 			boot_wb_now <= 1'b0;
 			boot_wb_sel <= 2'b00;
 		end else if (bus_we) begin
-			boot_wb_now <= wb_wdata[2];
-			boot_wb_sel <= wb_wdata[1:0];
+			boot_wb_now <= wb_wdata[18];
+			boot_wb_sel <= wb_wdata[17:16];
 		end
 
 
@@ -156,57 +156,18 @@ module pmu (
 		.CLKLF   (btn_clk)
 	);
 
-	// IOBs
-	SB_IO #(
-		.PIN_TYPE(6'b000001), // PIN_OUTPUT_NONE, PIN_INPUT
-		.PULLUP(1'b1),
-		.IO_STANDARD("SB_LVCMOS")
-	) btn_iob_I[1:0] (
-		.PACKAGE_PIN (btn),
-		.D_IN_0      (btn_iob)
+	// Submodule
+	pmu_button btn_I[1:0] (
+		.si_stat      ({stat_btn[1], stat_btn[0]}),
+		.si_ack       (ctrl_btn_ack),
+		.si_clk       (clk),
+		.si_rst       (rst),
+		.btn_pad      (btn),
+		.btn_trigged  (btn_trigged),
+		.btn_verylong (btn_verylong),
+		.btn_clk      (btn_clk)
 	);
 
-	// Synchronizer
-	initial
-		for (i=0; i<2; i=i+1)
-			btn_sync[i] = 1;
-
-	always @(posedge btn_clk)
-		for (i=0; i<2; i=i+1)
-			btn_sync[i] <= { btn_sync[i][1:0], btn_iob[i] };
-
-	// Small counter for debounce
-	initial
-		for (i=0; i<2; i=i+1)
-			btn_cnt[i] = 0;
-
-	always @(posedge btn_clk)
-		for (i=0; i<2; i=i+1)
-			btn_cnt[i] <= btn_sync[i][2] ? (btn_cnt[i] + {4'h0, btn_cnt[i][4]}) : 5'h10;
-
-	always @(*)
-		for (i=0; i<2; i=i+1)
-			btn_val[i] = btn_cnt[i][4];
-
-	// Synchronize value in sys clock domain
-	always @(posedge clk)
-		for (i=0; i<2; i=i+1)
-			btn_val_wb[i] <= btn_val[i];
-
-	// Long counter to detect very long presses
-	// ( ~6.5 sec )
-	initial
-		for (i=0; i<2; i=i+1)
-			btn_long_cnt[i] = 0;
-
-	always @(posedge btn_clk)
-		for (i=0; i<2; i=i+1)
-			btn_long_cnt[i] <= (btn_long_cnt[i] + btn_val[i]) & {17{btn_val[i]}};
-
-	always @(*)
-		for (i=0; i<2; i=i+1)
-			btn_long[i] = btn_long_cnt[i][16];
-
 
 	// Reboot
 	// ------
@@ -214,7 +175,7 @@ module pmu (
 	// Command from wb or button force
 	always @(*)
 	begin
-		if (btn_long[0]) begin
+		if (btn_verylong[0]) begin
 			boot_now = 1'b1;
 			boot_sel = 2'b01;
 		end else begin
@@ -252,13 +213,13 @@ module pmu (
 	end
 
 	// Enable system clock on wakeup
-	assign sys_start = wakeup;
+	assign sys_start = wakeup | |btn_trigged;
 
 	// Disable system clock on requests
 	assign sys_stop  = ctrl_sys_suspend;
 
 	// Power-off on long press and on requests
-	assign pwr_off_trig = btn_long[1] | ctrl_sys_shutdown;
+	assign pwr_off_trig = btn_verylong[1] | ctrl_sys_shutdown;
 
 	always @(posedge pwr_off_trig or posedge rst)
 		if (rst)
@@ -276,3 +237,170 @@ module pmu (
 			usb_ena <= (usb_ena | ctrl_usb_on) & ~ctrl_usb_off;
 
 endmodule // pmu
+
+
+module pmu_button (
+	// System interface
+	output wire [1:0] si_stat,
+	input  wire       si_ack,
+	input  wire       si_clk,
+	input  wire       si_rst,
+
+	// Button side
+	input  wire       btn_pad,
+	output wire       btn_trigged,
+	output wire       btn_verylong,
+	input  wire       btn_clk
+);
+
+	// Signals
+	// -------
+
+	// Basic state
+	wire        btn_iob;
+	reg   [2:0] btn_sync     = 3'b111;
+
+	reg   [4:0] btn_cnt      = 5'd0;
+	wire        btn_val;
+
+	reg  [16:0] btn_long_cnt = 16'd0;
+	wire        btn_long;
+
+	// System Interface
+		// System side
+	reg        sis_ack_toggle = 1'b0;
+	reg  [1:0] sis_trig_sync;
+	reg  [1:0] sis_state;
+
+	localparam [1:0]
+		STS_IDLE       = 2'b00,
+		STS_TRIG_SHORT = 2'b10,
+		STS_TRIG_LONG  = 2'b11,
+		STS_WAIT_REL   = 2'b01;
+
+		// Button side
+	reg  [2:0] sib_ack_sync;
+	wire       sib_ack;
+	reg  [3:0] sib_state;
+
+	localparam [2:0]
+		STB_IDLE       = 3'b000,
+		STB_ARMED      = 3'b101,
+		STB_TRIG_SHORT = 3'b110,
+		STB_TRIG_LONG  = 3'b111,
+		STB_WAIT_REL   = 3'b100;
+
+
+	// Basic state
+	// -----------
+
+	// IOBs
+	SB_IO #(
+		.PIN_TYPE(6'b000001), // PIN_OUTPUT_NONE, PIN_INPUT
+		.PULLUP(1'b1),
+		.IO_STANDARD("SB_LVCMOS")
+	) btn_iob_I (
+		.PACKAGE_PIN (btn_pad),
+		.D_IN_0      (btn_iob)
+	);
+
+	// Synchronizer
+	always @(posedge btn_clk)
+		btn_sync <= { btn_sync[1:0], btn_iob };
+
+	// Small counter for debounce
+	always @(posedge btn_clk)
+		btn_cnt <= btn_sync[2] ? (btn_cnt + {4'h0, btn_cnt[4]}) : 5'h10;
+
+	assign btn_val = btn_cnt[4];
+
+	// Long counter to detect long and very long presses
+	// ( ~6.5 sec )
+	always @(posedge btn_clk)
+		btn_long_cnt <= (btn_long_cnt + btn_val) & {17{btn_val}};
+
+	assign btn_long     = btn_long_cnt[13];
+	assign btn_verylong = btn_long_cnt[16];
+
+
+	// System Interface [Sys Domain]
+	// ----------------
+
+	// Ack toggle
+	always @(posedge si_clk)
+		sis_ack_toggle <= sis_ack_toggle ^ (si_ack & sis_state[1]);
+
+	// Synchronizer for trig signal
+	always @(posedge si_clk)
+		sis_trig_sync <= { sis_trig_sync[0], sib_state[3] };
+
+	// State
+	always @(posedge si_clk)
+	begin
+		case (sis_state)
+		STS_IDLE:
+			if (sis_trig_sync[1])
+				sis_state <= sib_state[0] ? STS_TRIG_LONG : STS_TRIG_SHORT;
+		STS_TRIG_SHORT:
+			if (si_ack)
+				sis_state <= STS_WAIT_REL;
+		STS_TRIG_LONG:
+			if (si_ack)
+				sis_state <= STS_WAIT_REL;
+		STS_WAIT_REL:
+			if (~sis_trig_sync[1])
+				sis_state <= STS_IDLE;
+		endcase
+
+		if (si_rst)
+			sis_state <= STS_IDLE;
+	end
+
+	assign si_stat = { sis_state[1], sis_state[1] & sis_state[0] };
+
+
+	// System Interface [Button Domain]
+	// ----------------
+
+	// Ack sync / detect
+	always @(posedge btn_clk)
+		sib_ack_sync <= { ^sib_ack_sync[1:0], sib_ack_sync[0], sis_ack_toggle };
+
+	assign sib_ack = sib_ack_sync[2];
+
+	// State
+	always @(posedge btn_clk)
+	begin
+		// 3rd bit is always delayed version of trigger
+		sib_state[3] <= sib_state[1];
+
+		// Next state
+		case (sib_state[2:0])
+		STB_IDLE:
+			if (btn_val)
+				sib_state[2:0] <= STB_ARMED;
+
+		STB_ARMED:
+			if (btn_long)
+				sib_state[2:0] <= STB_TRIG_LONG;
+			else if (!btn_val)
+				sib_state[2:0] <= STB_TRIG_SHORT;
+
+		STB_TRIG_SHORT:
+			if (sib_ack)
+				sib_state[2:0] <= STB_WAIT_REL;
+
+		STB_TRIG_LONG:
+			if (sib_ack)
+				sib_state[2:0] <= STB_WAIT_REL;
+
+		STB_WAIT_REL:
+			if (!btn_val)
+				sib_state[2:0] <= STB_IDLE;
+		endcase
+	end
+
+	// External trig indicator
+	assign btn_trigged = sib_state[3];
+
+endmodule // pmu_button

gateware/rtl/sysmgr_1.v

@@ -176,8 +176,8 @@ module sysmgr (
 	// Synch triggers
 	always @(posedge clk_base)
 	begin
-		sys_start_s <= { ~sys_start_s[1] & sys_start_s[0], sys_start_s[0], sys_start };
-		sys_stop_s  <= {  ~sys_stop_s[1] &  sys_stop_s[0],  sys_stop_s[0], sys_stop  };
+		sys_start_s <= {  sys_start_s[1],                  sys_start_s[0], sys_start };
+		sys_stop_s  <= { ~sys_stop_s[1] &  sys_stop_s[0],  sys_stop_s[0],  sys_stop  };
 	end
 
 	always @(posedge clk_base)

gateware/rtl/sysmgr_2.v

@@ -170,8 +170,8 @@ module sysmgr (
 	// Synch triggers
 	always @(posedge clk_base)
 	begin
-		sys_start_s <= { ~sys_start_s[1] & sys_start_s[0], sys_start_s[0], sys_start };
-		sys_stop_s  <= {  ~sys_stop_s[1] &  sys_stop_s[0],  sys_stop_s[0], sys_stop  };
+		sys_start_s <= {  sys_start_s[1],                  sys_start_s[0], sys_start };
+		sys_stop_s  <= { ~sys_stop_s[1] &  sys_stop_s[0],  sys_stop_s[0],  sys_stop  };
 	end
 
 	always @(posedge clk_base)

gateware/rtl/sysmgr_3.v

@@ -166,8 +166,8 @@ module sysmgr (
 	// Synch triggers
 	always @(posedge clk_base)
 	begin
-		sys_start_s <= { ~sys_start_s[1] & sys_start_s[0], sys_start_s[0], sys_start };
-		sys_stop_s  <= {  ~sys_stop_s[1] &  sys_stop_s[0],  sys_stop_s[0], sys_stop  };
+		sys_start_s <= {  sys_start_s[1],                  sys_start_s[0], sys_start };
+		sys_stop_s  <= { ~sys_stop_s[1] &  sys_stop_s[0],  sys_stop_s[0],  sys_stop  };
 	end
 
 	always @(posedge clk_base)

gateware/rtl/top.v

@@ -326,7 +326,7 @@ module top (
 	// Unused
 	// ------
 
-	assign hp_p = clk_sys;
+	assign hp_p = 1'bz;
 	assign hp_n = 1'bz;
 
 endmodule // top

hardware/xs-ctrl/ecn.md

@@ -29,6 +29,15 @@ Two of the boards were assembled and tested.
     * The `CHG_FPGA_n` line through the LED
     * The `SENSE` line from the voltage divider
 
+* Voltages on the USB lines (most importantly through the pull-up) end
+  up leaking on the 5V rail through the ESD protection diode, which in
+  turn causes the `SENSE` line to be triggered ...
+
+  I think ideally the pull-up from the FPGA should actually go through
+  and external NMOS that enables pull-up from 5V rail.
+
+* The 5V rail doesn't drain when disconnected.
+
 * The soft-off circuit wouldn't actually turn-off.
 
 * When the battery is not connected, the system can't be operated. The
@@ -60,6 +69,11 @@ boards.
   may need an active discharge resistor to make sure the gate discharges
   when USB is disconnected. Not tested.
 
+* Cut the 5V rail from the ESD diode to prevent voltages on the data line
+  to leak to an un-powered 5V rail.
+
+* Added a 1M pulldown on 5V rail so it discharges.
+
 * Replaced C14 with a 2.2uF cap in parallel with a 100k resistor.
   This makes the `PWR_EN` line only raise to 1.6V which is enough to enable
   the regulator but makes it easier to discharge without it re-triggering.