From c39000bf93f5123c0935115444f10ab86af852e8 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 18 Dec 2020 00:35:32 +0100 Subject: [PATCH 1/5] suspend: increase modem detection delay by 1s Sometimes it takes just a little bit longer than usual, so this avoids unnecessary modem recovery. --- src/suspend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/suspend.c b/src/suspend.c index 54c4b6a..c15bdd0 100644 --- a/src/suspend.c +++ b/src/suspend.c @@ -99,7 +99,7 @@ static void signal_cb(GDBusProxy *proxy, 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); + manager->suspend_source = g_timeout_add_seconds(9, G_SOURCE_FUNC(check_modem_resume), manager); } } From 74b91c7d58822636f3c439fb06fe4fe6960e0aad Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 18 Dec 2020 00:45:58 +0100 Subject: [PATCH 2/5] manager: make sure we don't reset the modem twice in a row This patch adds a 3s delay when resetting the modem during which we avoid triggering a new reset. This makes sure we don't trigger a reset twice in a row. It also disables any related running timer to avoid being re-triggered unnecessarily. --- src/manager.c | 27 ++++++++++++++++++++++++--- src/manager.h | 9 +++++---- src/mm-iface.c | 5 ++++- src/suspend.c | 1 + 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/manager.c b/src/manager.c index 26e0d67..be72c57 100644 --- a/src/manager.c +++ b/src/manager.c @@ -110,10 +110,27 @@ void modem_configure(struct EG25Manager *manager) at_sequence_configure(manager); } +static gboolean modem_reset_done(struct EG25Manager* manager) +{ + manager->modem_state = EG25_STATE_RESUMING; + manager->reset_timer = 0; + return FALSE; +} + void modem_reset(struct EG25Manager *manager) { int fd, ret, len = strlen(manager->modem_usb_id); + if (manager->reset_timer) + return; + + if (manager->suspend_source) { + g_source_remove(manager->suspend_source); + manager->suspend_source = 0; + } + + manager->modem_state = EG25_STATE_RESETTING; + fd = open("/sys/bus/usb/drivers/usb/unbind", O_WRONLY); if (fd < 0) goto error; @@ -127,16 +144,20 @@ void modem_reset(struct EG25Manager *manager) goto error; ret = write(fd, manager->modem_usb_id, len); if (ret < len) - g_warning("Couldn't unbind modem: wrote %d/%d bytes", ret, len); - + g_warning("Couldn't bind modem: wrote %d/%d bytes", ret, len); close(fd); + /* + * 3s is long enough to make sure the modem has been bound back and + * short enough to ensure it hasn't been acquired by ModemManager + */ + manager->reset_timer = g_timeout_add_seconds(3, G_SOURCE_FUNC(modem_reset_done), manager); + return; error: // Everything else failed, reset the modem at_sequence_reset(manager); - manager->modem_state = EG25_STATE_RESETTING; } void modem_suspend(struct EG25Manager *manager) diff --git a/src/manager.h b/src/manager.h index 714a812..8c12741 100644 --- a/src/manager.h +++ b/src/manager.h @@ -26,6 +26,11 @@ enum EG25State { struct EG25Manager { GMainLoop *loop; + guint reset_timer; + + int at_fd; + guint at_source; + GList *at_cmds; enum EG25State modem_state; gchar *modem_usb_id; @@ -40,10 +45,6 @@ struct EG25Manager { guint suspend_source; - int at_fd; - guint at_source; - GList *at_cmds; - struct gpiod_chip *gpiochip[2]; struct gpiod_line *gpio_out[5]; struct gpiod_line *gpio_in[2]; diff --git a/src/mm-iface.c b/src/mm-iface.c index 0d7e5b6..76c5138 100644 --- a/src/mm-iface.c +++ b/src/mm-iface.c @@ -35,7 +35,10 @@ static void add_modem(struct EG25Manager *manager, GDBusObject *object) g_assert(manager->mm_modem != NULL); if (manager->modem_state == EG25_STATE_RESUMING) { - g_source_remove(manager->suspend_source); + if (manager->suspend_source) { + g_source_remove(manager->suspend_source); + manager->suspend_source = 0; + } modem_resume_post(manager); manager->modem_state = EG25_STATE_CONFIGURED; } diff --git a/src/suspend.c b/src/suspend.c index c15bdd0..4aa5de6 100644 --- a/src/suspend.c +++ b/src/suspend.c @@ -20,6 +20,7 @@ static gboolean check_modem_resume(struct EG25Manager *manager) { g_message("Modem wasn't probed in time, restart it!"); + manager->suspend_source = 0; modem_reset(manager); return FALSE; From 62a07f9c5183b7fcc7db336565e5ca0e50358e9a Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 18 Dec 2020 01:37:06 +0100 Subject: [PATCH 3/5] src: add udev watcher to improve modem recovery Most of the modem issues follow a (incomplete) USB device reset. Instead of relying solely on the existing timer, this patch adds a udev monitor which resets the modem as soon as its associated USB device is reset, which greatly improves recovery time. --- meson.build | 1 + src/manager.c | 3 +++ src/manager.h | 2 ++ src/meson.build | 1 + src/suspend.c | 4 ++++ src/udev.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/udev.h | 12 ++++++++++++ 7 files changed, 64 insertions(+) create mode 100644 src/udev.c create mode 100644 src/udev.h diff --git a/meson.build b/meson.build index ba38ca3..70c6338 100644 --- a/meson.build +++ b/meson.build @@ -43,6 +43,7 @@ endif mgr_deps = [ dependency('glib-2.0'), dependency('gio-unix-2.0'), + dependency('gudev-1.0'), dependency('libgpiod'), dependency('libusb-1.0'), dependency('mm-glib'), diff --git a/src/manager.c b/src/manager.c index be72c57..a7dc2ff 100644 --- a/src/manager.c +++ b/src/manager.c @@ -9,6 +9,7 @@ #include "manager.h" #include "mm-iface.h" #include "suspend.h" +#include "udev.h" #include #include @@ -30,6 +31,7 @@ static gboolean quit_app(struct EG25Manager *manager) at_destroy(manager); mm_iface_destroy(manager); suspend_destroy(manager); + udev_destroy(manager); if (manager->modem_state >= EG25_STATE_STARTED) { g_message("Powering down the modem..."); @@ -202,6 +204,7 @@ int main(int argc, char *argv[]) gpio_init(&manager); mm_iface_init(&manager); suspend_init(&manager); + udev_init(&manager); g_idle_add(G_SOURCE_FUNC(modem_start), &manager); diff --git a/src/manager.h b/src/manager.h index 8c12741..feaca9b 100644 --- a/src/manager.h +++ b/src/manager.h @@ -8,6 +8,7 @@ #include #include +#include #include enum EG25State { @@ -44,6 +45,7 @@ struct EG25Manager { int suspend_inhibit_fd; guint suspend_source; + GUdevClient *udev; struct gpiod_chip *gpiochip[2]; struct gpiod_line *gpio_out[5]; diff --git a/src/meson.build b/src/meson.build index eb4b690..012773c 100644 --- a/src/meson.build +++ b/src/meson.build @@ -12,6 +12,7 @@ executable ( 'manager.c', 'manager.h', 'mm-iface.c', 'mm-iface.h', 'suspend.c', 'suspend.h', + 'udev.c', 'udev.h', ], dependencies : mgr_deps, install : true diff --git a/src/suspend.c b/src/suspend.c index 4aa5de6..92260a9 100644 --- a/src/suspend.c +++ b/src/suspend.c @@ -158,6 +158,10 @@ void suspend_init(struct EG25Manager *manager) void suspend_destroy(struct EG25Manager *manager) { drop_inhibitor(manager); + if (manager->suspend_source) { + g_source_remove(manager->suspend_source); + manager->suspend_source = 0; + } if (manager->suspend_proxy) { g_object_unref(manager->suspend_proxy); manager->suspend_proxy = NULL; diff --git a/src/udev.c b/src/udev.c new file mode 100644 index 0000000..acf10f8 --- /dev/null +++ b/src/udev.c @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2020 Arnaud Ferraris + * + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +#include "udev.h" + +#include + +static void udev_event_cb(GUdevClient *client, gchar *action, GUdevDevice *device, gpointer data) +{ + struct EG25Manager *manager = data; + + if (strcmp(action, "unbind") != 0 || manager->modem_state == EG25_STATE_RESETTING) + return; + + if (strncmp(g_udev_device_get_name(device), manager->modem_usb_id, strlen(manager->modem_usb_id)) == 0 && + manager->reset_timer == 0) { + g_message("Lost modem, resetting..."); + modem_reset(manager); + } +} + +void udev_init (struct EG25Manager *manager) +{ + const char * const subsystems[] = { "usb", NULL }; + + manager->udev = g_udev_client_new(subsystems); + g_signal_connect(manager->udev, "uevent", G_CALLBACK(udev_event_cb), manager); + + return; +} + +void udev_destroy (struct EG25Manager *manager) +{ + if (manager->udev) { + g_object_unref(manager->udev); + manager->udev = NULL; + } +} diff --git a/src/udev.h b/src/udev.h new file mode 100644 index 0000000..bf166b1 --- /dev/null +++ b/src/udev.h @@ -0,0 +1,12 @@ +/* + * Copyright (C) 2020 Arnaud Ferraris + * + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +#pragma once + +#include "manager.h" + +void udev_init (struct EG25Manager *data); +void udev_destroy (struct EG25Manager *data); From 067c01b685b40cfe34776e653aeaf8d32052b7f6 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 18 Dec 2020 01:38:27 +0100 Subject: [PATCH 4/5] manager: rename suspend_source to suspend_timer This makes its role more explicit. --- src/manager.c | 6 +++--- src/manager.h | 2 +- src/mm-iface.c | 6 +++--- src/suspend.c | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/manager.c b/src/manager.c index a7dc2ff..bc08ddb 100644 --- a/src/manager.c +++ b/src/manager.c @@ -126,9 +126,9 @@ void modem_reset(struct EG25Manager *manager) if (manager->reset_timer) return; - if (manager->suspend_source) { - g_source_remove(manager->suspend_source); - manager->suspend_source = 0; + if (manager->suspend_timer) { + g_source_remove(manager->suspend_timer); + manager->suspend_timer = 0; } manager->modem_state = EG25_STATE_RESETTING; diff --git a/src/manager.h b/src/manager.h index feaca9b..c51f32f 100644 --- a/src/manager.h +++ b/src/manager.h @@ -43,7 +43,7 @@ struct EG25Manager { GDBusProxy *suspend_proxy; int suspend_inhibit_fd; - guint suspend_source; + guint suspend_timer; GUdevClient *udev; diff --git a/src/mm-iface.c b/src/mm-iface.c index 76c5138..21f2f8d 100644 --- a/src/mm-iface.c +++ b/src/mm-iface.c @@ -35,9 +35,9 @@ static void add_modem(struct EG25Manager *manager, GDBusObject *object) g_assert(manager->mm_modem != NULL); if (manager->modem_state == EG25_STATE_RESUMING) { - if (manager->suspend_source) { - g_source_remove(manager->suspend_source); - manager->suspend_source = 0; + if (manager->suspend_timer) { + g_source_remove(manager->suspend_timer); + manager->suspend_timer = 0; } modem_resume_post(manager); manager->modem_state = EG25_STATE_CONFIGURED; diff --git a/src/suspend.c b/src/suspend.c index 92260a9..2e1301e 100644 --- a/src/suspend.c +++ b/src/suspend.c @@ -20,7 +20,7 @@ static gboolean check_modem_resume(struct EG25Manager *manager) { g_message("Modem wasn't probed in time, restart it!"); - manager->suspend_source = 0; + manager->suspend_timer = 0; modem_reset(manager); return FALSE; @@ -100,7 +100,7 @@ static void signal_cb(GDBusProxy *proxy, take_inhibitor(manager); modem_resume_pre(manager); manager->modem_state = EG25_STATE_RESUMING; - manager->suspend_source = g_timeout_add_seconds(9, G_SOURCE_FUNC(check_modem_resume), manager); + manager->suspend_timer = g_timeout_add_seconds(9, G_SOURCE_FUNC(check_modem_resume), manager); } } @@ -158,9 +158,9 @@ void suspend_init(struct EG25Manager *manager) void suspend_destroy(struct EG25Manager *manager) { drop_inhibitor(manager); - if (manager->suspend_source) { - g_source_remove(manager->suspend_source); - manager->suspend_source = 0; + if (manager->suspend_timer) { + g_source_remove(manager->suspend_timer); + manager->suspend_timer = 0; } if (manager->suspend_proxy) { g_object_unref(manager->suspend_proxy); From 2a18b1cb0c559d9c3cc7497b6cecdd46c1735e97 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 18 Dec 2020 01:38:58 +0100 Subject: [PATCH 5/5] meson.build: release version 0.1.1 --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 70c6338..cb0d72e 100644 --- a/meson.build +++ b/meson.build @@ -8,7 +8,7 @@ project ( 'eg25manager', 'c', - version : '0.0.6', + version : '0.1.1', license : 'GPLv3+', meson_version : '>= 0.50.0', default_options :