[dartuino][bootloader] Changes as per code review.

app/moot/fsboot.c

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -33,7 +34,7 @@
 
 #define MAX_FPATH_LEN 64
 
-#define LOCAL_TRACE 1
+#define LOCAL_TRACE 0
 
 // Attempt to boot from the filesystem.
 void attempt_fs_boot(void)

app/moot/include/app/moot/fsboot.h

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -21,10 +22,7 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-
+#pragma once
-
-#ifndef APP_MOOT_FS_BOOT_H_
-#define APP_MOOT_FS_BOOT_H_
 
 // The platform/target should implement this routine by mouting the default
 // filesystem and returning a string that points to the mount point. If NULL is
@@ -32,5 +30,3 @@
 // boot or that (2) mounting the default filesystem failed in which case the
 // system proceeds to boot without FSBoot.
 void attempt_fs_boot(void);
-
-#endif  // APP_MOOT_FS_BOOT_H_

app/moot/include/app/moot/moot.h

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -21,12 +22,10 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-#ifndef APP_MOOT_MOOT_H_
+#pragma once
-#define APP_MOOT_MOOT_H_
 
 typedef enum {
     BOOT_NOW,
     NEXT_BOOT_STRATEGY
 } next_boot_action_t;
 
-#endif  // APP_MOOT_MOOT_H_

app/moot/include/app/moot/stubs.h

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -21,8 +22,7 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-#ifndef APP_MOOT_STUBS_H_
+#pragma once
-#define APP_MOOT_STUBS_H_
 
 #include <sys/types.h>
 
@@ -46,5 +46,3 @@ extern const moot_sysinfo_t moot_system_info;
 // populated to reflect the path at which the FS was mounted and the name of
 // the BIO device that hosts the FS.
 status_t moot_mount_default_fs(char **mount_path, char **device_name);
-
-#endif  // APP_MOOT_STUBS_H_

app/moot/include/app/moot/usbboot.h

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files

app/moot/moot.c

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -66,6 +67,6 @@ static void moot_entry(const struct app_descriptor *app, void *args)
 }
 
 APP_START(moot)
-.init = moot_init,
+ .init = moot_init,
  .entry = moot_entry,
-  APP_END
+APP_END

app/moot/stubs.c

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -21,8 +22,8 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-#include <compiler.h>
 #include <app/moot/stubs.h>
+#include <compiler.h>
 #include <err.h>
 
 // Fail by default. System must override this.

app/moot/usbboot.c

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -49,11 +50,12 @@
 
 // How long should we wait for activity on USB before we continue to boot?
 #define USB_BOOT_TIMEOUT (3000)
+#define USB_READ_TIMEOUT (10000)
 
 static const uint8_t if_descriptor[] = {
     0x09,           /* length */
     INTERFACE,      /* type */
-    0x01,           /* interface num */ // TODO(gkalsi)
+    0x01,           /* interface num */
     0x00,           /* alternates */
     0x02,           /* endpoint count */
     0xff,           /* interface class */
@@ -64,7 +66,7 @@ static const uint8_t if_descriptor[] = {
     /* endpoint 1 IN */
     0x07,           /* length */
     ENDPOINT,       /* type */
-    0x1 | 0x80,     /* address: 1 IN */ // TODO(gkalsi)
+    0x1 | 0x80,     /* address: 1 IN */
     0x02,           /* type: bulk */
     W(64),          /* max packet size: 64 */
     00,             /* interval */
@@ -72,7 +74,7 @@ static const uint8_t if_descriptor[] = {
     /* endpoint 1 OUT */
     0x07,           /* length */
     ENDPOINT,       /* type */
-    0x1,            /* address: 1 OUT */ // TODO(gkalsi)
+    0x1,            /* address: 1 OUT */
     0x02,           /* type: bulk */
     W(64),          /* max packet size: 64 */
     00,             /* interval */
@@ -94,6 +96,7 @@ static const uint8_t if_descriptor[] = {
 #define USB_RESP_ERR_ERASE_SYS_FLASH  (0xFFF3)
 #define USB_RESP_ERR_WRITE_SYS_FLASH  (0xFFF4)
 #define USB_RESP_CANT_FIND_BUILDSIG   (0xFFF5)
+#define USB_RESP_USB_READ_ERROR       (0xFFF6)
 
 /* Bootloader commands */
 #define USB_CMD_FLASH    (0x01)
@@ -114,20 +117,23 @@ typedef struct cmd_response {
 
 // USB Functions
 static void usb_xmit(void *data, size_t len);
-static status_t usb_recv(void *data, size_t len, lk_time_t timeout, size_t *actual);
+static status_t usb_recv(void *data, size_t len, lk_time_t timeout,
+                         size_t *actual);
 static status_t usb_xmit_cplt_cb(ep_t endpoint, usbc_transfer_t *t);
 static status_t usb_recv_cplt_cb(ep_t endpoint, usbc_transfer_t *t);
 static status_t usb_register_cb(
     void *cookie, usb_callback_op_t op, const union usb_callback_args *args);
 
-static uint8_t buffer[4096];
 static event_t txevt = EVENT_INITIAL_VALUE(txevt, 0, EVENT_FLAG_AUTOUNSIGNAL);
 static event_t rxevt = EVENT_INITIAL_VALUE(rxevt, 0, EVENT_FLAG_AUTOUNSIGNAL);
-static volatile bool online = false;
+static volatile bool usb_online = false;
 
 // Command processor that handles USB boot commands.
-static bool handle(const void *data, const size_t n, cmd_response_t *resp)
+static bool handle_usb_cmd(
-{
+    const void *data, const size_t n, cmd_response_t *resp
+) {
+    static uint8_t buffer[4096];
+
     DEBUG_ASSERT(resp);
 
     resp->magic = RESP_MAGIC;
@@ -164,10 +170,11 @@ static bool handle(const void *data, const size_t n, cmd_response_t *resp)
                 break;
             }
 
-            ssize_t n_bytes_erased = bio_erase(dev, moot_system_info.system_offset, image_length);
+            ssize_t n_bytes_erased =
+                bio_erase(dev, moot_system_info.system_offset, image_length);
             if (n_bytes_erased < image_length) {
                 resp->code = USB_RESP_ERR_ERASE_SYS_FLASH;
-                break;
+                goto close_and_exit;
             }
 
             // Signal to the host to start sending the image over.
@@ -181,12 +188,21 @@ static bool handle(const void *data, const size_t n, cmd_response_t *resp)
                                (ssize_t)sizeof(buffer) : image_length;
 
                 size_t bytes_received;
-                usb_recv(buffer, xfer, INFINITE_TIME, &bytes_received);
+                st = usb_recv(buffer, xfer, USB_READ_TIMEOUT, &bytes_received);
+                if (st != NO_ERROR) {
+                    resp->code = USB_RESP_USB_READ_ERROR;
+                    goto close_and_exit;
+                }
+
+                if (xfer != (ssize_t)bytes_received) {
+                    resp->code = USB_RESP_BAD_DATA_LEN;
+                    goto close_and_exit;
+                }
 
                 ssize_t written = bio_write(dev, buffer, addr, bytes_received);
                 if (written != (ssize_t)bytes_received) {
                     resp->code = USB_RESP_ERR_WRITE_SYS_FLASH;
-                    goto finish;
+                    goto close_and_exit;
                 }
 
                 addr += written;
@@ -195,6 +211,8 @@ static bool handle(const void *data, const size_t n, cmd_response_t *resp)
 
             resp->code = USB_RESP_NO_ERROR;
 
+close_and_exit:
+            bio_close(dev);
             break;
         case USB_CMD_BOOT:
             resp->code = USB_RESP_NO_ERROR;
@@ -237,23 +255,26 @@ finish:
 }
 
 void init_usb_boot(void)
+{
+    usb_register_callback(&usb_register_cb, NULL);
+}
+
+void append_usb_interfaces(void)
 {
     usb_append_interface_lowspeed(if_descriptor, sizeof(if_descriptor));
     usb_append_interface_highspeed(if_descriptor, sizeof(if_descriptor));
-
-    usb_register_callback(&usb_register_cb, NULL);
 }
 
 void attempt_usb_boot(void)
 {
-    uint8_t *buf = malloc(USB_XFER_SIZE);
+    static uint8_t buf[USB_XFER_SIZE];
 
     lk_time_t start = current_time();
     lk_time_t timeout = USB_BOOT_TIMEOUT;
     size_t bytes_received;
 
     while (current_time() - start < timeout) {
-        if (!online) {
+        if (!usb_online) {
             thread_yield();
             continue;
         }
@@ -264,7 +285,7 @@ void attempt_usb_boot(void)
         } else if (r == NO_ERROR) {
             // Somebody tried to talk to us over USB, they own the boot now.
             cmd_response_t response;
-            bool should_boot = handle(buf, bytes_received, &response);
+            bool should_boot = handle_usb_cmd(buf, bytes_received, &response);
             usb_xmit((void *)&response, sizeof(response));
             timeout = INFINITE_TIME;
             if (should_boot) {
@@ -274,7 +295,6 @@ void attempt_usb_boot(void)
     }
 
 finish:
-    free(buf);
     return;
 }
 
@@ -287,7 +307,7 @@ static status_t usb_register_cb(
     if (op == USB_CB_ONLINE) {
         usbc_setup_endpoint(1, USB_IN, 0x40);
         usbc_setup_endpoint(1, USB_OUT, 0x40);
-        online = true;
+        usb_online = true;
     }
     return NO_ERROR;
 }
@@ -319,8 +339,9 @@ static void usb_xmit(void *data, size_t len)
     event_wait(&txevt);
 }
 
-static status_t usb_recv(void *data, size_t len, lk_time_t timeout, size_t *actual)
+static status_t usb_recv(
-{
+    void *data, size_t len, lk_time_t timeout, size_t *actual
+) {
     usbc_transfer_t transfer = {
         .callback = &usb_recv_cplt_cb,
         .result = 0,
@@ -334,7 +355,6 @@ static status_t usb_recv(void *data, size_t len, lk_time_t timeout, size_t *actu
     status_t res = event_wait_timeout(&rxevt, timeout);
 
     if (res != NO_ERROR) {
-        // TODO(gkalsi): Cancel the USB txn?
         return res;
     }
 

project/dartuinoP0-bootloader.mk

@@ -7,4 +7,4 @@ MODULES += \
   app/moot \
   lib/version \
   lib/buildsig \
-  target/dartuinoP0/bootloader
+  target/dartuinoP0/projects/bootloader

project/dartuinoP0-test.mk

@@ -8,6 +8,9 @@ include project/virtual/fs.mk
 
 DISPLAY_PANEL_TYPE ?= LS013B7DH06
 
+MODULES += \
+  target/dartuinoP0/projects/system
+
 MODULE_DEPS += \
     app/accelerometer \
 

target/dartuinoP0/projects/bootloader/bootloader_stubs.c

@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files
@@ -30,7 +31,8 @@ static char bootloader_mount_point[] = "/spifs";
 #include <app/moot/stubs.h>
 #include <stdio.h>
 
-#define BOOTLOADER_LENGTH_KB (64)
+#define BOOTLOADER_SIZE_KB (64)
+#define SYSTEM_FLASH_SIZE_KB (1024)
 
 status_t moot_mount_default_fs(char **mount_path, char **device_name)
 {
@@ -42,8 +44,8 @@ status_t moot_mount_default_fs(char **mount_path, char **device_name)
 const moot_sysinfo_t moot_system_info = {
     .sys_base_addr = 0x00210000,
     .btldr_offset = 0x0,
-    .bootloader_len = 1024 * BOOTLOADER_LENGTH_KB,
+    .bootloader_len = 1024 * BOOTLOADER_SIZE_KB,
-    .system_offset = 1024 * BOOTLOADER_LENGTH_KB,
+    .system_offset = 1024 * BOOTLOADER_SIZE_KB,
-    .system_len = (1024 * (1024 - BOOTLOADER_LENGTH_KB)),
+    .system_len = (1024 * (SYSTEM_FLASH_SIZE_KB - BOOTLOADER_SIZE_KB)),
     .system_flash_name = bootloader_primary_flash_name,
 };

target/dartuinoP0/projects/system/rules.mk

@@ -0,0 +1,8 @@
+LOCAL_DIR := $(GET_LOCAL_DIR)
+
+MODULE := $(LOCAL_DIR)
+
+MODULE_SRCS += \
+	$(LOCAL_DIR)/system_stubs.c
+
+include make/module.mk

target/dartuinoP0/projects/system/system_stubs.c

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ * Author: gkalsi@google.com (Gurjant Kalsi)
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files
+ * (the "Software"), to deal in the Software without restriction,
+ * including without limitation the rights to use, copy, modify, merge,
+ * publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+ * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+ * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+void append_usb_interfaces(void)
+{
+	// no-op
+}

target/dartuinoP0/usb.c

@@ -36,6 +36,8 @@
 #define W(w) (w & 0xff), (w >> 8)
 #define W3(w) (w & 0xff), ((w >> 8) & 0xff), ((w >> 16) & 0xff)
 
+extern void append_usb_interfaces(void);
+
 /* top level device descriptor */
 static const uint8_t dev_descr[] = {
     0x12,           /* descriptor length */
@@ -98,7 +100,9 @@ usb_config config = {
 void target_usb_setup(void)
 {
     usb_setup(&config);
+
     printf("appending interfaces\n");
+    append_usb_interfaces();
 
     usb_add_string("LK", 1);
     usb_add_string("LK Industries", 2);

tools/moot/mtldr.py

@@ -1,5 +1,6 @@
 """
- Copyright (c) 2016 Gurjant Kalsi <me@gurjantkalsi.com>
+ Copyright 2016 Google Inc. All Rights Reserved.
+ Author: gkalsi@google.com (Gurjant Kalsi)
 
  Permission is hereby granted, free of charge, to any person obtaining
  a copy of this software and associated documentation files
@@ -59,6 +60,7 @@ class Retcode:
     err_erase_sys_flash = (0xFFF3)
     err_write_sys_flash = (0xFFF4)
     cant_find_buildsig  = (0xFFF5)
+    usb_read_error      = (0xFFF6)
 
 
 class CommandParam: