at: make sure we read the full response before processing it

The response can include the command and an error number, so we want to 
only check it contains ERROR, even if it's replying more than just the 
'ERROR' string.

meson.build

@@ -44,6 +44,7 @@ mgr_deps = [
     dependency('glib-2.0'),
     dependency('gio-unix-2.0'),
     dependency('libgpiod'),
+    dependency('libusb-1.0'),
     dependency('mm-glib'),
 ]
 

src/at.c

@@ -31,7 +31,7 @@ static int configure_serial(const char *tty)
     struct termios ttycfg;
     int fd;
 
-    fd = open(tty, O_RDWR | O_NOCTTY);
+    fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
     if (fd > 0) {
         tcgetattr(fd, &ttycfg);
         ttycfg.c_iflag &= ~(IGNBRK | BRKINT | ICRNL | INLCR | PARMRK | INPCK | ISTRIP | IXON);
@@ -143,7 +143,6 @@ static int append_at_command(struct EG25Manager *manager,
     if (!at_cmd)
         return -1;
 
-    at_cmd->retries = 0;
     at_cmd->cmd = g_strdup(cmd);
     if (subcmd)
         at_cmd->subcmd = g_strdup(subcmd);
@@ -157,22 +156,33 @@ static int append_at_command(struct EG25Manager *manager,
     return 0;
 }
 
+#define READ_BUFFER_SIZE 256
+
 static gboolean modem_response(gint fd,
                                GIOCondition event,
                                gpointer data)
 {
     struct EG25Manager *manager = data;
-    char response[256];
+    char response[READ_BUFFER_SIZE*4+1];
-    int ret;
+    char tmp[READ_BUFFER_SIZE];
+    ssize_t ret, pos = 0;
 
     /*
-     * TODO: several reads can be necessary to get the full response, we could
+     * Several reads can be necessary to get the full response, so we loop
-     * loop until we read 0 chars with a reasonable delay between attempts
+     * until we read 0 chars with a reasonable delay between attempts
      * (remember the transfer rate is 115200 here)
      */
-    ret = read(fd, response, sizeof(response));
+    do {
-    if (ret > 0) {
+        ret = read(fd, tmp, sizeof(tmp));
-        response[ret] = 0;
+        if (ret > 0) {
+            memcpy(&response[pos], tmp, ret);
+            pos += ret;
+            usleep(10000);
+        }
+    } while (ret > 0 && pos < (sizeof(response) - 1));
+
+    if (pos > 0) {
+        response[pos] = 0;
         g_strstrip(response);
         if (strlen(response) == 0)
             return TRUE;
@@ -181,7 +191,7 @@ static gboolean modem_response(gint fd,
 
         if (strcmp(response, "RDY") == 0)
             manager->modem_state = EG25_STATE_STARTED;
-        else if (strcmp(response, "ERROR") == 0)
+        else if (strstr(response, "ERROR"))
             retry_at_command(manager);
         else if (strstr(response, "OK"))
             process_at_result(manager, response);
@@ -206,7 +216,8 @@ int at_init(struct EG25Manager *manager)
 void at_destroy(struct EG25Manager *manager)
 {
     g_source_remove(manager->at_source);
-    close(manager->at_fd);
+    if (manager->at_fd > 0)
+        close(manager->at_fd);
 }
 
 void at_sequence_configure(struct EG25Manager *manager)
@@ -248,7 +259,7 @@ void at_sequence_configure(struct EG25Manager *manager)
 
 void at_sequence_suspend(struct EG25Manager *manager)
 {
-    append_at_command(manager, "QGPS", NULL, NULL, "0");
+    append_at_command(manager, "QGPS", NULL, "0", NULL);
     append_at_command(manager, "QCFG", "urc/cache", "1", NULL);
     send_at_command(manager);
 }
@@ -256,7 +267,7 @@ void at_sequence_suspend(struct EG25Manager *manager)
 void at_sequence_resume(struct EG25Manager *manager)
 {
     append_at_command(manager, "QCFG", "urc/cache", "0", NULL);
-    append_at_command(manager, "QGPS", NULL, NULL, "1");
+    append_at_command(manager, "QGPS", NULL, "1", NULL);
     send_at_command(manager);
 }
 

src/gpio.c

@@ -11,6 +11,8 @@
 
 #define MAX_GPIOCHIP_LINES 352
 
+#define GPIO_IDX_INVAL 0xffff
+
 enum {
     GPIO_OUT_DTR = 0,
     GPIO_OUT_PWRKEY,
@@ -20,7 +22,7 @@ enum {
     GPIO_OUT_COUNT
 };
 
-static unsigned gpio_idx_bh[] = {
+static unsigned gpio_out_idx_bh[] = {
     358,
     35,
     68,
@@ -28,7 +30,7 @@ static unsigned gpio_idx_bh[] = {
     232
 };
 
-static unsigned gpio_idx_ce[] = {
+static unsigned gpio_out_idx_ce[] = {
     34,
     35,
     68,
@@ -36,24 +38,34 @@ static unsigned gpio_idx_ce[] = {
     232
 };
 
+enum {
+    GPIO_IN_STATUS = 0,
+    GPIO_IN_COUNT
+};
+
+static unsigned gpio_in_idx_bh[] = {
+    GPIO_IDX_INVAL,
+};
+
+static unsigned gpio_in_idx_ce[] = {
+    233,
+};
+
 int gpio_sequence_poweron(struct EG25Manager *manager)
 {
     gpiod_line_set_value(manager->gpio_out[GPIO_OUT_PWRKEY], 1);
     sleep(1);
     gpiod_line_set_value(manager->gpio_out[GPIO_OUT_PWRKEY], 0);
 
-    g_message("Executed power-on sequence");
+    g_message("Executed power-on/off sequence");
 
     return 0;
 }
 
 int gpio_sequence_shutdown(struct EG25Manager *manager)
 {
-    gpiod_line_set_value(manager->gpio_out[GPIO_OUT_RESET], 1);
     gpiod_line_set_value(manager->gpio_out[GPIO_OUT_DISABLE], 1);
-    gpiod_line_set_value(manager->gpio_out[GPIO_OUT_PWRKEY], 1);
+    gpio_sequence_poweron(manager);
-    sleep(1);
-    gpiod_line_set_value(manager->gpio_out[GPIO_OUT_PWRKEY], 0);
 
     g_message("Executed power-off sequence");
 
@@ -83,7 +95,8 @@ int gpio_sequence_resume(struct EG25Manager *manager)
 int gpio_init(struct EG25Manager *manager)
 {
     int i, ret;
-    unsigned *gpio_idx;
+    unsigned *gpio_in_idx;
+    unsigned *gpio_out_idx;
 
     manager->gpiochip[0] = gpiod_chip_open_by_label(GPIO_CHIP1_LABEL);
     if (!manager->gpiochip[0]) {
@@ -91,53 +104,106 @@ int gpio_init(struct EG25Manager *manager)
         return 1;
     }
 
+    manager->gpiochip[1] = gpiod_chip_open_by_label(GPIO_CHIP2_LABEL);
+    if (!manager->gpiochip[1]) {
+        g_critical("Unable to open GPIO chip " GPIO_CHIP2_LABEL);
+        return 1;
+    }
+
     if (manager->braveheart) {
-        // BH have DTR on the 2nd gpiochip
+        gpio_in_idx = gpio_in_idx_bh;
-        manager->gpiochip[1] = gpiod_chip_open_by_label(GPIO_CHIP2_LABEL);
+        gpio_out_idx = gpio_out_idx_bh;
-        if (!manager->gpiochip[1]) {
-            g_critical("Unable to open GPIO chip " GPIO_CHIP2_LABEL);
-            return 1;
-        }
-        gpio_idx = gpio_idx_bh;
     } else {
-        gpio_idx = gpio_idx_ce;
+        gpio_in_idx = gpio_in_idx_ce;
+        gpio_out_idx = gpio_out_idx_ce;
     }
 
     for (i = 0; i < GPIO_OUT_COUNT; i++) {
         unsigned int offset, chipidx;
 
-        if (gpio_idx[i] < MAX_GPIOCHIP_LINES) {
+        if (gpio_out_idx[i] < MAX_GPIOCHIP_LINES) {
-            offset = gpio_idx[i];
+            offset = gpio_out_idx[i];
             chipidx = 0;
         } else {
-            offset = gpio_idx[i] - MAX_GPIOCHIP_LINES;
+            offset = gpio_out_idx[i] - MAX_GPIOCHIP_LINES;
             chipidx = 1;
         }
 
         manager->gpio_out[i] = gpiod_chip_get_line(manager->gpiochip[chipidx], offset);
         if (!manager->gpio_out[i]) {
-            g_critical("Unable to get line %d", i);
+            g_critical("Unable to get output GPIO %d", i);
             return 1;
         }
 
         ret = gpiod_line_request_output(manager->gpio_out[i], "eg25manager", 0);
         if (ret < 0) {
-            g_critical("Unable to request line %d", i);
+            g_critical("Unable to request output GPIO %d", i);
             return 1;
         }
     }
 
+    for (i = 0; i < GPIO_IN_COUNT; i++) {
+        unsigned int offset, chipidx;
+
+        if (gpio_in_idx[i] == GPIO_IDX_INVAL)
+            continue;
+
+        if (gpio_in_idx[i] < MAX_GPIOCHIP_LINES) {
+            offset = gpio_in_idx[i];
+            chipidx = 0;
+        } else {
+            offset = gpio_in_idx[i] - MAX_GPIOCHIP_LINES;
+            chipidx = 1;
+        }
+
+        manager->gpio_in[i] = gpiod_chip_get_line(manager->gpiochip[chipidx], offset);
+        if (!manager->gpio_in[i]) {
+            g_warning("Unable to get input GPIO %d", i);
+            continue;
+        }
+
+        ret = gpiod_line_request_input(manager->gpio_in[i], "eg25manager");
+        if (ret < 0) {
+            g_warning("Unable to request input GPIO %d", i);
+            manager->gpio_in[i] = NULL;
+        }
+    }
+
     return 0;
 }
 
+gboolean gpio_check_poweroff(struct EG25Manager *manager, gboolean keep_down)
+{
+    if (manager->gpio_in[GPIO_IN_STATUS] &&
+        gpiod_line_get_value(manager->gpio_in[GPIO_IN_STATUS]) == 1) {
+
+        if (keep_down) {
+            // Asserting RESET line to prevent modem from rebooting
+            gpiod_line_set_value(manager->gpio_out[GPIO_OUT_RESET], 1);
+        }
+
+        return TRUE;
+    }
+
+    return FALSE;
+}
+
 void gpio_destroy(struct EG25Manager *manager)
 {
     int i;
 
-    for (i = 0; i < GPIO_OUT_COUNT; i++)
+    for (i = 0; i < GPIO_OUT_COUNT; i++) {
-        gpiod_line_release(manager->gpio_out[i]);
+        if (manager->gpio_out[i])
+            gpiod_line_release(manager->gpio_out[i]);
+    }
 
-    gpiod_chip_close(manager->gpiochip[0]);
+    for (i = 0; i < GPIO_IN_COUNT; i++) {
+        if (manager->gpio_in[i])
+            gpiod_line_release(manager->gpio_in[i]);
+    }
+
+    if (manager->gpiochip[0])
+        gpiod_chip_close(manager->gpiochip[0]);
     if (manager->gpiochip[1])
         gpiod_chip_close(manager->gpiochip[1]);
 }

src/gpio.h

@@ -15,3 +15,5 @@ int gpio_sequence_poweron(struct EG25Manager *state);
 int gpio_sequence_shutdown(struct EG25Manager *state);
 int gpio_sequence_suspend(struct EG25Manager *state);
 int gpio_sequence_resume(struct EG25Manager *state);
+
+gboolean gpio_check_poweroff(struct EG25Manager *manager, gboolean keep_down);

src/manager.c

@@ -16,45 +16,74 @@
 #include <unistd.h>
 
 #include <glib-unix.h>
+#include <libusb.h>
 
-static gboolean quit_timeout_cb(struct EG25Manager *manager)
+#define EG25_USB_VID 0x2c7c
-{
+#define EG25_USB_PID 0x0125
-    g_message("Modem down, quitting...");
-    g_main_loop_quit(manager->loop);
-
-    at_destroy(manager);
-    gpio_destroy(manager);
-
-    return FALSE;
-}
 
 static gboolean quit_app(struct EG25Manager *manager)
 {
+    int i;
+
     g_message("Request to quit...");
 
+    at_destroy(manager);
+    mm_iface_destroy(manager);
+    suspend_destroy(manager);
+
     if (manager->modem_state >= EG25_STATE_STARTED) {
         g_message("Powering down the modem...");
         gpio_sequence_shutdown(manager);
         manager->modem_state = EG25_STATE_FINISHING;
-        /*
+        for (i = 0; i < 30; i++) {
-         * TODO: add a polling function to check STATUS and RI pins state
+            if (gpio_check_poweroff(manager, TRUE))
-         * (that way we could reduce the poweroff delay)
+                break;
-         */
+            sleep(1);
-        g_timeout_add_seconds(30, G_SOURCE_FUNC(quit_timeout_cb), manager);
+        }
     }
+    g_message("Modem down, quitting...");
 
-    mm_iface_destroy(manager);
+    g_main_loop_quit(manager->loop);
-    suspend_destroy(manager);
-    g_bus_unwatch_name(manager->mm_watch);
 
     return FALSE;
 }
 
 static gboolean modem_start(struct EG25Manager *manager)
 {
-    g_message("Starting modem...");
+    ssize_t i, count;
-    gpio_sequence_poweron(manager);
+    gboolean should_boot = TRUE;
-    manager->modem_state = EG25_STATE_POWERED;
+    libusb_context *ctx = NULL;
+    libusb_device **devices = NULL;
+    struct libusb_device_descriptor desc;
+
+    if (manager->braveheart) {
+        // BH don't have the STATUS line connected, so check if USB device is present
+        libusb_init(&ctx);
+
+        count = libusb_get_device_list(ctx, &devices);
+        for (i = 0; i < count; i++) {
+            libusb_get_device_descriptor(devices[i], &desc);
+            if (desc.idVendor == EG25_USB_VID && desc.idProduct == EG25_USB_PID) {
+                g_message("Found corresponding USB device, modem already powered");
+                should_boot = FALSE;
+                break;
+            }
+        }
+
+        libusb_free_device_list(devices, 1);
+        libusb_exit(ctx);
+    } else if (!gpio_check_poweroff(manager, FALSE)) {
+        g_message("STATUS is low, modem already powered");
+        should_boot = FALSE;
+    }
+
+    if (should_boot) {
+        g_message("Starting modem...");
+        gpio_sequence_poweron(manager);
+        manager->modem_state = EG25_STATE_POWERED;
+    } else {
+        manager->modem_state = EG25_STATE_STARTED;
+    }
 
     return FALSE;
 }
@@ -106,14 +135,20 @@ error:
     manager->modem_state = EG25_STATE_RESETTING;
 }
 
-void modem_suspend(struct EG25Manager *manager)
+void modem_suspend_pre(struct EG25Manager *manager)
 {
+    manager->modem_state = EG25_STATE_SUSPENDING;
     gpio_sequence_suspend(manager);
+}
+
+void modem_suspend_post(struct EG25Manager *manager)
+{
     at_sequence_suspend(manager);
 }
 
 void modem_resume_pre(struct EG25Manager *manager)
 {
+    manager->modem_state = EG25_STATE_RESUMING;
     gpio_sequence_resume(manager);
 }
 
@@ -140,7 +175,7 @@ int main(int argc, char *argv[])
         return 1;
     }
     read(fd, compatible, sizeof(compatible));
-    if (strstr(compatible, "pine64,pinephone-1.1"))
+    if (!strstr(compatible, "pine64,pinephone-1.2"))
         manager.braveheart = TRUE;
     close(fd);
 
@@ -156,5 +191,7 @@ int main(int argc, char *argv[])
 
     g_main_loop_run(manager.loop);
 
+    gpio_destroy(&manager);
+
     return 0;
 }

src/manager.h

@@ -46,11 +46,13 @@ struct EG25Manager {
 
     struct gpiod_chip *gpiochip[2];
     struct gpiod_line *gpio_out[5];
+    struct gpiod_line *gpio_in[2];
 };
 
 void modem_configure(struct EG25Manager *data);
 void modem_reset(struct EG25Manager *data);
-void modem_suspend(struct EG25Manager *data);
+void modem_suspend_pre(struct EG25Manager *data);
+void modem_suspend_post(struct EG25Manager *data);
 void modem_resume_pre(struct EG25Manager *data);
 void modem_resume_post(struct EG25Manager *data);
 void modem_update_state(struct EG25Manager *data, MMModemState state);

src/mm-iface.c

@@ -47,6 +47,9 @@ static void add_modem(struct EG25Manager *manager, GDBusObject *object)
         modem_configure(manager);
 
     path = mm_modem_get_device(manager->mm_modem);
+
+    if (manager->modem_usb_id)
+        g_free(manager->modem_usb_id);
     manager->modem_usb_id = g_strdup(strrchr(path, '/') + 1);
 
     gdbus_modem = MM_GDBUS_MODEM(manager->mm_modem);
@@ -129,6 +132,9 @@ static void object_removed_cb(struct EG25Manager *manager, GDBusObject *object)
     path = g_dbus_object_get_object_path(object);
     g_message("ModemManager object `%s' removed", path);
 
+    if (manager->modem_state == EG25_STATE_SUSPENDING)
+        modem_suspend_post(manager);
+
     manager->mm_modem = NULL;
     if (manager->modem_usb_id) {
         g_free(manager->modem_usb_id);
@@ -189,5 +195,12 @@ void mm_iface_init(struct EG25Manager *manager)
 
 void mm_iface_destroy(struct EG25Manager *manager)
 {
-    g_clear_object(&manager->mm_manager);
+    if (manager->mm_manager) {
+        g_clear_object(&manager->mm_manager);
+        manager->mm_manager = NULL;
+    }
+    if (manager->mm_watch != 0) {
+        g_bus_unwatch_name(manager->mm_watch);
+        manager->mm_watch = 0;
+    }
 }

src/suspend.c

@@ -92,13 +92,11 @@ static void signal_cb(GDBusProxy *proxy,
 
     if (is_about_to_suspend) {
         g_message("system is about to suspend");
-        manager->modem_state = EG25_STATE_SUSPENDING;
+        modem_suspend_pre(manager);
-        modem_suspend(manager);
     } else {
         g_message("system is resuming");
         take_inhibitor(manager);
         modem_resume_pre(manager);
-        manager->modem_state = EG25_STATE_RESUMING;
         manager->suspend_source = g_timeout_add_seconds(8, G_SOURCE_FUNC(check_modem_resume), manager);
     }
 }
@@ -114,11 +112,12 @@ static void name_owner_cb(GObject *object,
     g_assert(proxy == manager->suspend_proxy);
 
     owner = g_dbus_proxy_get_name_owner(proxy);
-    if (owner)
+    if (owner) {
         take_inhibitor(manager);
-    else
+        g_free(owner);
+    } else {
         drop_inhibitor(manager);
-    g_free(owner);
+    }
 }
 
 static void on_proxy_acquired(GObject *object,
@@ -139,9 +138,10 @@ static void on_proxy_acquired(GObject *object,
     g_signal_connect(manager->suspend_proxy, "g-signal", G_CALLBACK(signal_cb), manager);
 
     owner = g_dbus_proxy_get_name_owner(manager->suspend_proxy);
-    if (owner)
+    if (owner) {
         take_inhibitor(manager);
-    g_free(owner);
+        g_free(owner);
+    }
 }
 
 void suspend_init(struct EG25Manager *manager)
@@ -156,7 +156,10 @@ void suspend_init(struct EG25Manager *manager)
 void suspend_destroy(struct EG25Manager *manager)
 {
     drop_inhibitor(manager);
-    g_object_unref(manager->suspend_proxy);
+    if (manager->suspend_proxy) {
+        g_object_unref(manager->suspend_proxy);
+        manager->suspend_proxy = NULL;
+    }
 }
 
 void suspend_inhibit(struct EG25Manager *manager, gboolean inhibit)