| Message ID | 2269874.vFx2qVVIhK@adam-laptop-hp (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | RFC - Add AT interface to qmi device | expand |
Hi Adam, On 4/30/24 4:49 PM, Adam Pigg wrote: > Hi > > I would appreciate views on this patch I need for the Pinephone modem. This > adds code to the gobi plugin and qmi_device to get a handle to the GAtChat Aux > interface. The voicecall and sms drivers can then use this interface to > trigger call/sms notification. From an architectural perspective this is unfortunate. QMI atom drivers (drivers/qmimodem/*) should not know anything about AT command drivers (drivers/atmodem/*) or GAtChat and vice versa. Doing so keeps dependencies to a minimum and allows oFono to be compiled without AT/ISI/RIL/MBIM support for example. For modem drivers, I can see the need / usecase of driving both QMI and AT / whatever ports together simultaneously. Such modem drivers would then depend on both QMI / AT / whatever support being available. > > This is done in addition to the QMI interface, because it seems that > _sometimes_ the modem does not always send the QMI notification, or it is > missed due to the phone being in a deep sleep state. The AT interface is > always triggered. Have you considered simply using AT commands for calls and SMS? Regards, -Denis
Hi, * Denis Kenzior <denkenz@gmail.com> [700101 02:00]: > Hi Adam, > > On 4/30/24 4:49 PM, Adam Pigg wrote: > > Hi > > > > I would appreciate views on this patch I need for the Pinephone modem. This > > adds code to the gobi plugin and qmi_device to get a handle to the GAtChat Aux > > interface. The voicecall and sms drivers can then use this interface to > > trigger call/sms notification. > > From an architectural perspective this is unfortunate. QMI atom drivers > (drivers/qmimodem/*) should not know anything about AT command drivers > (drivers/atmodem/*) or GAtChat and vice versa. Doing so keeps dependencies > to a minimum and allows oFono to be compiled without AT/ISI/RIL/MBIM support > for example. > > For modem drivers, I can see the need / usecase of driving both QMI and AT / > whatever ports together simultaneously. Such modem drivers would then > depend on both QMI / AT / whatever support being available. > > > > > This is done in addition to the QMI interface, because it seems that > > _sometimes_ the modem does not always send the QMI notification, or it is > > missed due to the phone being in a deep sleep state. The AT interface is > > always triggered. > > Have you considered simply using AT commands for calls and SMS? At least on Droid4 the ttyUSB AT interface is bit buggy.. But there may be a generic way to trigger lost QMI events based on the USB AT interface notifications. What we accidentally discovered with the Maemo Droid4 modem patches was that triggering a dummy QMI call on serial (muxed TS 27.010) notifications would trigger the lost QMI notifications :) So we have the call below in the out of tree patches to kick the USB QMI modem based on the AT interface notifications: int mot_qmi_trigger_events(struct ofono_modem *modem) { struct motmdm_data *data = ofono_modem_get_data(modem); if (data->wms == NULL) return -ENODEV; return qmi_service_send(data->wms, QMI_WMS_GET_SMSC_ADDR, NULL, NULL, NULL, g_free); }; Ivaylo mentioned a while back that the notifications also appear on the ttyUSB AT interface, so maybe the same code would work for Pinephone and Droid4. Not sure if Ivaylo already has some related patches. Regards, Tony
Hi Tony, >> >> Have you considered simply using AT commands for calls and SMS? > > At least on Droid4 the ttyUSB AT interface is bit buggy.. But there may be > a generic way to trigger lost QMI events based on the USB AT interface > notifications. Hmm, okay. > > What we accidentally discovered with the Maemo Droid4 modem patches was > that triggering a dummy QMI call on serial (muxed TS 27.010) notifications Can you clarify? QMI call on the QMI character device or on the AT port itself? > would trigger the lost QMI notifications :) > > So we have the call below in the out of tree patches to kick the USB QMI > modem based on the AT interface notifications: > > int mot_qmi_trigger_events(struct ofono_modem *modem) > { > struct motmdm_data *data = ofono_modem_get_data(modem); > > if (data->wms == NULL) > return -ENODEV; > > return qmi_service_send(data->wms, QMI_WMS_GET_SMSC_ADDR, NULL, > NULL, NULL, g_free); > }; > I'm fine with this approach if the handling of this quirk is done by the modem driver. The modem driver can manage both QMI and AT ports. What I want to avoid is coupling between technologies in the atom drivers themselves. In other words, QMI SMS driver should not depend on anything related to AT commands. Regards, -Denis
* Denis Kenzior <denkenz@gmail.com> [240520 18:58]: > > > Have you considered simply using AT commands for calls and SMS? > > > > At least on Droid4 the ttyUSB AT interface is bit buggy.. But there may be > > a generic way to trigger lost QMI events based on the USB AT interface > > notifications. > > Hmm, okay. > > > > > What we accidentally discovered with the Maemo Droid4 modem patches was > > that triggering a dummy QMI call on serial (muxed TS 27.010) notifications > > Can you clarify? QMI call on the QMI character device or on the AT port itself? I think any QMI command on the USB QMI interface will trigger the pending USB QMI notifications. > > int mot_qmi_trigger_events(struct ofono_modem *modem) > > { > > struct motmdm_data *data = ofono_modem_get_data(modem); > > > > if (data->wms == NULL) > > return -ENODEV; > > > > return qmi_service_send(data->wms, QMI_WMS_GET_SMSC_ADDR, NULL, > > NULL, NULL, g_free); > > }; > > > > I'm fine with this approach if the handling of this quirk is done by the > modem driver. The modem driver can manage both QMI and AT ports. What I > want to avoid is coupling between technologies in the atom drivers > themselves. In other words, QMI SMS driver should not depend on anything > related to AT commands. OK yes doing it as needed from the modem driver makes sense. Note that there may not even be a need for proper AT commands in this case. It may be possible to use just ell directly, and just kick the USB QMI interface on any output from the ttyUSB AT interface basically. Not sure if any thing needs to be acked on the ttyUSB AT interface. Regards, Tony
diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c index 69a0e535..d94c5498 100644 --- a/drivers/qmimodem/qmi.c +++ b/drivers/qmimodem/qmi.c @@ -39,6 +39,7 @@ #include <ell/ell.h> #include <ofono/log.h> +#include <gatchat.h> #include "qmi.h" #include "ctl.h" @@ -106,6 +107,7 @@ struct qmi_device { bool writer_active : 1; bool shutting_down : 1; bool destroyed : 1; + GAtChat *atmodem; }; struct qmi_device_qmux { @@ -1053,6 +1055,22 @@ bool qmi_device_has_service(struct qmi_device *device, uint16_t type) return __find_service_info_by_type(device, type); } +void qmi_device_set_atmodem(struct qmi_device *device, GAtChat* atmodem) +{ + if (device == NULL) + return; + + device->atmodem = atmodem; +} + +GAtChat* qmi_device_get_atmodem(struct qmi_device *device) +{ + if (device == NULL) + return NULL; + + return device->atmodem; +} + struct discover_data { struct discovery super; struct qmi_device *device; diff --git a/drivers/qmimodem/qmi.h b/drivers/qmimodem/qmi.h index 0075f738..7bc423aa 100644 --- a/drivers/qmimodem/qmi.h +++ b/drivers/qmimodem/qmi.h @@ -21,6 +21,7 @@ #include <stdbool.h> #include <stdint.h> +#include <gatchat.h> #define QMI_SERVICE_CONTROL 0 /* Control service */ #define QMI_SERVICE_WDS 1 /* Wireless data service */ @@ -77,6 +78,9 @@ typedef void (*qmi_destroy_func_t)(void *user_data); struct qmi_device; +void qmi_device_set_atmodem(struct qmi_device *device, GAtChat* atmodem); +GAtChat* qmi_device_get_atmodem(struct qmi_device *device); + typedef void (*qmi_debug_func_t)(const char *str, void *user_data); typedef void (*qmi_shutdown_func_t)(void *user_data); typedef void (*qmi_discover_func_t)(void *user_data); diff --git a/drivers/qmimodem/sms.c b/drivers/qmimodem/sms.c index c6252d9d..f8882ab9 100644 --- a/drivers/qmimodem/sms.c +++ b/drivers/qmimodem/sms.c @@ -30,6 +30,8 @@ #include <ofono/modem.h> #include <ofono/sms.h> +#include <gatchat.h> + #include "qmi.h" #include "wms.h" #include "util.h" @@ -776,6 +778,15 @@ static void create_wms_cb(struct qmi_service *service, void *user_data) ofono_sms_register(sms); } +static void qmi_sms_at_notify(GAtResult *result, gpointer user_data) +{ + struct ofono_sms *sms = user_data; + + DBG(""); + + get_msg_list(sms); +} + static int qmi_sms_probe(struct ofono_sms *sms, unsigned int vendor, void *user_data) { @@ -790,6 +801,12 @@ static int qmi_sms_probe(struct ofono_sms *sms, qmi_service_create(device, QMI_SERVICE_WMS, create_wms_cb, sms, NULL); + GAtChat* atmodem = qmi_device_get_atmodem(device); + if (atmodem) { + guint ret = g_at_chat_register(atmodem, "+CMTI:", qmi_sms_at_notify, FALSE, sms, NULL); + DBG("SMS AT CHAT REGISTER %d", ret); + } + return 0; } diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c index c6383a56..4b01b784 100644 --- a/drivers/qmimodem/voicecall.c +++ b/drivers/qmimodem/voicecall.c @@ -24,12 +24,15 @@ #include <config.h> #endif +#include <ofono.h> #include <ofono/log.h> #include <ofono/modem.h> #include <ofono/voicecall.h> #include <src/common.h> #include <ell/ell.h> +#include <gatchat.h> + #include "voice.h" #include "qmi.h" @@ -750,6 +753,17 @@ static void create_voice_cb(struct qmi_service *service, void *user_data) ofono_voicecall_register(vc); } +static void qmi_voicecall_ring_notify(GAtResult *result, gpointer user_data) { + struct ofono_voicecall *vc = user_data; + struct voicecall_data *data = ofono_voicecall_get_data(vc); + + DBG("DETECTED AT RING"); + + qmi_service_send(data->voice, QMI_VOICE_ALL_CALL_STATUS_IND, NULL, + all_call_status_ind, vc, NULL); + +} + static int qmi_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor, void *user_data) { @@ -766,6 +780,15 @@ static int qmi_voicecall_probe(struct ofono_voicecall *vc, qmi_service_create(device, QMI_SERVICE_VOICE, create_voice_cb, vc, NULL); + //Get a handle to the modem serial interface to detect RING signals + //Sometimes the QMI notify doesnt come through if the phone is + //asleep + GAtChat* atmodem = qmi_device_get_atmodem(device); + if (atmodem) { + guint ret = g_at_chat_register(atmodem, "RING", qmi_voicecall_ring_notify, FALSE, vc, NULL); + DBG("VOICE AT CHAT REGISTER %d", ret); + } + return 0; } diff --git a/plugins/gobi.c b/plugins/gobi.c index 550ce787..4a4d9d1b 100644 --- a/plugins/gobi.c +++ b/plugins/gobi.c @@ -29,6 +29,7 @@ #include <stdlib.h> #include <stdio.h> #include <net/if.h> +#include <gatchat.h> #define OFONO_API_SUBJECT_TO_CHANGE #include <ofono/plugin.h> @@ -60,6 +61,12 @@ #include <drivers/qmimodem/wda.h> #include <drivers/qmimodem/util.h> +//Define function from atutil.h as it cant be uncluded +GAtChat *at_util_open_device(struct ofono_modem *modem, const char *key, + GAtDebugFunc debug_func, char *debug_prefix, + char *tty_option, ...); + + #define GOBI_DMS (1 << 0) #define GOBI_NAS (1 << 1) #define GOBI_WMS (1 << 2) @@ -448,6 +455,15 @@ static int gobi_enable(struct ofono_modem *modem) if (getenv("OFONO_QMI_DEBUG")) qmi_device_set_debug(data->device, gobi_debug, "QMI: "); + //Get a handle to an AT interface if one was found, and attach it to the qmi_device + GAtChat *atchat = at_util_open_device(modem, "Aux", NULL, "Aux: ", NULL); + if (!atchat) { + DBG("No Aux"); + } else { + DBG("Have atmodem"); + } + qmi_device_set_atmodem(data->device, atchat); //Will set to NULL if doesnt exist + r = qmi_device_discover(data->device, discover_cb, modem, NULL); if (!r) return -EINPROGRESS; diff --git a/plugins/udevng.c b/plugins/udevng.c index b9d115f1..3f72ec3f 100644 --- a/plugins/udevng.c +++ b/plugins/udevng.c @@ -2085,6 +2085,12 @@ static void check_usb_device(struct udev_device *device) const char *syspath, *devname, *driver; const char *vendor = NULL, *model = NULL; const char *kernel_driver; + const char* devsyspath = udev_device_get_syspath(device); + + if (devsyspath == NULL) { + return; + } + DBG("devsyspath: %s", devsyspath); usb_device = udev_device_get_parent_with_subsystem_devtype(device,
Hi I would appreciate views on this patch I need for the Pinephone modem. This adds code to the gobi plugin and qmi_device to get a handle to the GAtChat Aux interface. The voicecall and sms drivers can then use this interface to trigger call/sms notification. This is done in addition to the QMI interface, because it seems that _sometimes_ the modem does not always send the QMI notification, or it is missed due to the phone being in a deep sleep state. The AT interface is always triggered. I do not expect to upstream this, but there may be useful parts, or you may be able to suggest another method? Thanks Adam "usb", "usb_device"); @@ -2156,10 +2162,39 @@ static void check_usb_device(struct udev_device *device) if (driver == NULL) return; - } - add_device(syspath, devname, driver, vendor, model, MODEM_TYPE_USB, + add_device(syspath, devname, driver, vendor, model, MODEM_TYPE_USB, device, kernel_driver); + + return; + } + + //Now handle device as opposed to parent device + driver = udev_device_get_property_value(device, "ID_USB_DRIVER"); + DBG("driver: %s", driver); + + for (unsigned int i = 0; vendor_list[i].driver; i++) { + if (g_str_equal(vendor_list[i].drv, driver) == FALSE) + continue; + + if (vendor_list[i].vid) { + if (!g_str_equal(vendor_list[i].vid, vendor)) + continue; + } + + if (vendor_list[i].pid) { + if (!g_str_equal(vendor_list[i].pid, model)) + continue; + } + + driver = vendor_list[i].driver; + } + + if (driver == NULL) + return; + + add_device(devsyspath, devname, driver, vendor, model, MODEM_TYPE_USB, + device, kernel_driver); } static const struct {