1:51 a.m.
From: Lukasz Nowak <lnowak(a)tycoint.com>
Changes from V2:
- made AlwaysOnline flow cleaner in gobi.c
- it turns out that AlwaysOnline is already a part of core logic
in src/modem.c which have the same effect as if the driver had
no set_online() function at all. Hence removed changes to
gobi_set_online() as it is never called.
Changes from V1:
- split into multiple smaller commits
- added comments in code, documenting issues with the Telit QMI firmware
- disabled non-working low-power operating modes in additional cases
- use mccmnc string instead of truncating an invalid operator name
Lukasz Nowak (2):
gobi: Do not use low-power modes for some modems
udevng: add Telit LE910 V1 support
plugins/gobi.c | 19 +++++++++++++++++++
plugins/udevng.c | 24 ++++++++++++++++++++----
2 files changed, 39 insertions(+), 4 deletions(-)
--
2.7.4
1:51 a.m.
New subject: [PATCH v3 1/2] gobi: Do not use low-power modes for some modems
From: Lukasz Nowak <lnowak(a)tycoint.com>
Telit QMI modems have a problem with the low-power operating modes.
After entering and leaving such a state, UIM service does not return.
The sim card is still marked as powered-down. The QMI interface does
not have a way to power it back on.
To avoid this, keep modems with the "AlwaysOnline" flag online
in the disable-modem and offline-modem procedures.
---
plugins/gobi.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/plugins/gobi.c b/plugins/gobi.c
index 6a78941..06f906d 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -168,6 +168,16 @@ static void get_oper_mode_cb(struct qmi_result *result, void
*user_data)
data->oper_mode = mode;
+ /*
+ * Telit QMI LTE modem must remain online. If powered down, it also
+ * powers down the sim card, and QMI interface has no way to bring
+ * it back alive.
+ */
+ if (ofono_modem_get_boolean(modem, "AlwaysOnline")) {
+ ofono_modem_set_powered(modem, TRUE);
+ return;
+ }
+
switch (data->oper_mode) {
case QMI_DMS_OPER_MODE_ONLINE:
param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
@@ -353,6 +363,14 @@ static int gobi_disable(struct ofono_modem *modem)
qmi_service_cancel_all(data->dms);
qmi_service_unregister_all(data->dms);
+ /*
+ * Telit QMI modem must remain online. If powered down, it also
+ * powers down the sim card, and QMI interface has no way to bring
+ * it back alive.
+ */
+ if (ofono_modem_get_boolean(modem, "AlwaysOnline"))
+ goto out;
+
param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
QMI_DMS_OPER_MODE_PERSIST_LOW_POWER);
if (!param)
@@ -362,6 +380,7 @@ static int gobi_disable(struct ofono_modem *modem)
power_disable_cb, modem, NULL) > 0)
return -EINPROGRESS;
+out:
shutdown_device(modem);
return -EINPROGRESS;
--
2.7.4
2:08 a.m.
New subject: [PATCH v3 1/2] gobi: Do not use low-power modes for some modems
Hi Lukasz,
On 03/24/2017 09:51 AM, Lukasz Nowak wrote:
From: Lukasz Nowak <lnowak(a)tycoint.com>
Telit QMI modems have a problem with the low-power operating modes.
After entering and leaving such a state, UIM service does not return.
The sim card is still marked as powered-down. The QMI interface does
not have a way to power it back on.
To avoid this, keep modems with the "AlwaysOnline" flag online
in the disable-modem and offline-modem procedures.
---
plugins/gobi.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
Applied, thanks.
Regards,
-Denis
1:51 a.m.
New subject: [PATCH v3 2/2] udevng: add Telit LE910 V1 support
From: Lukasz Nowak <lnowak(a)tycoint.com>
Tested with LE910-SVG and Verizon.
---
plugins/udevng.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 2279bbe..3b15064 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -182,14 +182,15 @@ static gboolean setup_gobi(struct modem_info *modem)
const char *qmi = NULL, *mdm = NULL, *net = NULL;
const char *gps = NULL, *diag = NULL;
GSList *list;
+ gboolean telit = FALSE;
DBG("%s", modem->syspath);
for (list = modem->devices; list; list = list->next) {
struct device_info *info = list->data;
- DBG("%s %s %s %s", info->devnode, info->interface,
- info->number, info->label);
+ DBG("%s %s %s %s %s", info->devnode, info->interface,
+ info->number, info->label, info->subsystem);
if (g_strcmp0(info->interface, "255/255/255") == 0) {
if (info->number == NULL)
@@ -199,10 +200,18 @@ static gboolean setup_gobi(struct modem_info *modem)
else if (g_strcmp0(info->number, "01") == 0)
diag = info->devnode;
else if (g_strcmp0(info->number, "02") == 0)
- mdm = info->devnode;
+ if (g_strcmp0(info->subsystem, "net") == 0)
+ net = info->devnode;
+ else if (g_strcmp0(info->subsystem, "usbmisc") == 0) {
+ telit = TRUE;
+ qmi = info->devnode;
+ } else
+ mdm = info->devnode;
else if (g_strcmp0(info->number, "03") == 0)
gps = info->devnode;
- }
+ } else if (g_strcmp0(info->interface, "255/0/0") == 0)
+ if (g_strcmp0(info->number, "04") == 0)
+ mdm = info->devnode;
}
if (qmi == NULL || mdm == NULL || net == NULL)
@@ -215,6 +224,12 @@ static gboolean setup_gobi(struct modem_info *modem)
ofono_modem_set_string(modem->modem, "Diag", diag);
ofono_modem_set_string(modem->modem, "NetworkInterface", net);
+ if (telit) {
+ /* Telit LE910 V1 modems */
+ ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
+ ofono_modem_set_boolean(modem->modem, "AlwaysOnline", TRUE);
+ }
+
return TRUE;
}
@@ -1168,6 +1183,7 @@ static struct {
{ "mbm", "cdc_ether", "0930" },
{ "mbm", "cdc_ncm", "0930" },
{ "hso", "hso" },
+ { "gobi", "option", "1bc7", "1201" },
{ "gobi", "qmi_wwan" },
{ "gobi", "qcserial" },
{ "sierra", "qmi_wwan", "1199" },
--
2.7.4
2:16 a.m.
New subject: [PATCH v3 2/2] udevng: add Telit LE910 V1 support
Hi Lukasz,
On 03/24/2017 09:51 AM, Lukasz Nowak wrote:
From: Lukasz Nowak <lnowak(a)tycoint.com>
Tested with LE910-SVG and Verizon.
---
plugins/udevng.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 2279bbe..3b15064 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -182,14 +182,15 @@ static gboolean setup_gobi(struct modem_info *modem)
const char *qmi = NULL, *mdm = NULL, *net = NULL;
const char *gps = NULL, *diag = NULL;
GSList *list;
+ gboolean telit = FALSE;
DBG("%s", modem->syspath);
for (list = modem->devices; list; list = list->next) {
struct device_info *info = list->data;
- DBG("%s %s %s %s", info->devnode, info->interface,
- info->number, info->label);
+ DBG("%s %s %s %s %s", info->devnode, info->interface,
+ info->number, info->label, info->subsystem);
if (g_strcmp0(info->interface, "255/255/255") == 0) {
if (info->number == NULL)
@@ -199,10 +200,18 @@ static gboolean setup_gobi(struct modem_info *modem)
else if (g_strcmp0(info->number, "01") == 0)
diag = info->devnode;
else if (g_strcmp0(info->number, "02") == 0)
- mdm = info->devnode;
+ if (g_strcmp0(info->subsystem, "net") == 0)
+ net = info->devnode;
+ else if (g_strcmp0(info->subsystem, "usbmisc") == 0) {
just a small nit: this line is over 80 characters
+ telit = TRUE;
Would it be safer to set this value based on usb major/minor number instead?
+ qmi = info->devnode;
+ } else
+ mdm = info->devnode;
else if (g_strcmp0(info->number, "03") == 0)
gps = info->devnode;
- }
+ } else if (g_strcmp0(info->interface, "255/0/0") == 0)
+ if (g_strcmp0(info->number, "04") == 0)
+ mdm = info->devnode;
}
if (qmi == NULL || mdm == NULL || net == NULL)
@@ -215,6 +224,12 @@ static gboolean setup_gobi(struct modem_info *modem)
ofono_modem_set_string(modem->modem, "Diag", diag);
ofono_modem_set_string(modem->modem, "NetworkInterface", net);
+ if (telit) {
+ /* Telit LE910 V1 modems */
+ ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
+ ofono_modem_set_boolean(modem->modem, "AlwaysOnline", TRUE);
+ }
+
return TRUE;
}
@@ -1168,6 +1183,7 @@ static struct {
{ "mbm", "cdc_ether", "0930" },
{ "mbm", "cdc_ncm", "0930" },
{ "hso", "hso" },
+ { "gobi", "option", "1bc7", "1201" },
{ "gobi", "qmi_wwan" },
{ "gobi", "qcserial" },
{ "sierra", "qmi_wwan", "1199" },
Regards,
-Denis
2:56 a.m.
New subject: [PATCH v3 2/2] udevng: add Telit LE910 V1 support
Hi Lukasz,
I'm working on support for the Quectel UC21 and have some similar
patches to what you have below. Based on that experience, here are some
comments:
On 03/24/2017 03:51 PM, Lukasz Nowak wrote:
From: Lukasz Nowak <lnowak(a)tycoint.com>
Tested with LE910-SVG and Verizon.
---
plugins/udevng.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 2279bbe..3b15064 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -182,14 +182,15 @@ static gboolean setup_gobi(struct modem_info *modem)
const char *qmi = NULL, *mdm = NULL, *net = NULL;
const char *gps = NULL, *diag = NULL;
GSList *list;
+ gboolean telit = FALSE;
DBG("%s", modem->syspath);
for (list = modem->devices; list; list = list->next) {
struct device_info *info = list->data;
- DBG("%s %s %s %s", info->devnode, info->interface,
- info->number, info->label);
+ DBG("%s %s %s %s %s", info->devnode, info->interface,
+ info->number, info->label, info->subsystem);
Put this DBG message change in a separate patch. It's a good idea and I
did the same thing in my tree, but it's unrelated to the telit support.
if (g_strcmp0(info->interface, "255/255/255") == 0) {
if (info->number == NULL)
@@ -199,10 +200,18 @@ static gboolean setup_gobi(struct modem_info *modem)
else if (g_strcmp0(info->number, "01") == 0)
diag = info->devnode;
else if (g_strcmp0(info->number, "02") == 0)
- mdm = info->devnode;
+ if (g_strcmp0(info->subsystem, "net") == 0)
+ net = info->devnode;
+ else if (g_strcmp0(info->subsystem, "usbmisc") == 0) {
+ telit = TRUE;
+ qmi = info->devnode;
+ } else
+ mdm = info->devnode;
else if (g_strcmp0(info->number, "03") == 0)
gps = info->devnode;
- }
+ } else if (g_strcmp0(info->interface, "255/0/0") == 0)
+ if (g_strcmp0(info->number, "04") == 0)
+ mdm = info->devnode;
}
I need almost exactly the same code for the UC21, but the endpoints are
different. What we'll end up with if we do this is an enormous tree of
cases that all span multiple interfaces, endpoints, and subsystems.
Because of that, I'm not sure that setup_gobi is the right place to be
putting this.
The Sierra 7xxx modem does something similar, but uses setup_sierra to
attach the endpoints and then defers to the gobi driver with:
ofono_modem_set_driver(modem->mode, "gobi"); /* around line 270... in
setup_sierra */
I suspect that that's a better approach. I think using setup_gobi makes
little sense because it doesn't provide a clear association to the
hardware vendor and the devices from different vendors don't have a
common configuration.
setup_telit makes some assumptions about endpoint 2 that you might not
want to muck about with... in my opinion, doing a setup_telit_le910() is
not horrid, and better than making a mess of setup_gobi.
if (qmi == NULL || mdm == NULL || net == NULL)
@@ -215,6 +224,12 @@ static gboolean setup_gobi(struct modem_info *modem)
ofono_modem_set_string(modem->modem, "Diag", diag);
ofono_modem_set_string(modem->modem, "NetworkInterface", net);
+ if (telit) {
+ /* Telit LE910 V1 modems */
+ ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
+ ofono_modem_set_boolean(modem->modem, "AlwaysOnline", TRUE);
+ }
+
return TRUE;
}
@@ -1168,6 +1183,7 @@ static struct {
{ "mbm", "cdc_ether", "0930" },
{ "mbm", "cdc_ncm", "0930" },
{ "hso", "hso" },
+ { "gobi", "option", "1bc7", "1201" },
I
don't understand why you are setting "option" here... does the device
not use qmi_wwan?
/Jonas
10:07 a.m.
New subject: [PATCH v3 2/2] udevng: add Telit LE910 V1 support
Hi Jonas,
I think you will be happy with PATCH v4 :)
On 24/03/17 15:56, Jonas Bonn wrote:
Hi Lukasz,
I'm working on support for the Quectel UC21 and have some similar
patches to what you have below. Based on that experience, here are some
comments:
On 03/24/2017 03:51 PM, Lukasz Nowak wrote:
> From: Lukasz Nowak <lnowak(a)tycoint.com>
>
> Tested with LE910-SVG and Verizon.
> ---
> plugins/udevng.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 2279bbe..3b15064 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -182,14 +182,15 @@ static gboolean setup_gobi(struct modem_info *modem)
> const char *qmi = NULL, *mdm = NULL, *net = NULL;
> const char *gps = NULL, *diag = NULL;
> GSList *list;
> + gboolean telit = FALSE;
>
> DBG("%s", modem->syspath);
>
> for (list = modem->devices; list; list = list->next) {
> struct device_info *info = list->data;
>
> - DBG("%s %s %s %s", info->devnode, info->interface,
> - info->number, info->label);
> + DBG("%s %s %s %s %s", info->devnode, info->interface,
> + info->number, info->label, info->subsystem);
Put this DBG message change in a separate patch. It's a good idea and I
did the same thing in my tree, but it's unrelated to the telit support.
>
> if (g_strcmp0(info->interface, "255/255/255") == 0) {
> if (info->number == NULL)
> @@ -199,10 +200,18 @@ static gboolean setup_gobi(struct modem_info *modem)
> else if (g_strcmp0(info->number, "01") == 0)
> diag = info->devnode;
> else if (g_strcmp0(info->number, "02") == 0)
> - mdm = info->devnode;
> + if (g_strcmp0(info->subsystem, "net") == 0)
> + net = info->devnode;
> + else if (g_strcmp0(info->subsystem, "usbmisc") == 0) {
> + telit = TRUE;
> + qmi = info->devnode;
> + } else
> + mdm = info->devnode;
> else if (g_strcmp0(info->number, "03") == 0)
> gps = info->devnode;
> - }
> + } else if (g_strcmp0(info->interface, "255/0/0") == 0)
> + if (g_strcmp0(info->number, "04") == 0)
> + mdm = info->devnode;
> }
I need almost exactly the same code for the UC21, but the endpoints are
different. What we'll end up with if we do this is an enormous tree of
cases that all span multiple interfaces, endpoints, and subsystems.
Because of that, I'm not sure that setup_gobi is the right place to be
putting this.
The Sierra 7xxx modem does something similar, but uses setup_sierra to
attach the endpoints and then defers to the gobi driver with:
ofono_modem_set_driver(modem->mode, "gobi"); /* around line 270... in
setup_sierra */
I suspect that that's a better approach. I think using setup_gobi makes
little sense because it doesn't provide a clear association to the
hardware vendor and the devices from different vendors don't have a
common configuration.
setup_telit makes some assumptions about endpoint 2 that you might not
want to muck about with... in my opinion, doing a setup_telit_le910() is
not horrid, and better than making a mess of setup_gobi.
The reason for using setup_gobi() was a problem with udevng, where
cdc-wdm0 would not have vendor_id:product_id. There was no way to separate
setup functions in vendor_list without vid:pid.
This is fixed in v4 - setup_telitqmi() was added.
>
> if (qmi == NULL || mdm == NULL || net == NULL)
> @@ -215,6 +224,12 @@ static gboolean setup_gobi(struct modem_info *modem)
> ofono_modem_set_string(modem->modem, "Diag", diag);
> ofono_modem_set_string(modem->modem, "NetworkInterface", net);
>
> + if (telit) {
> + /* Telit LE910 V1 modems */
> + ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
> + ofono_modem_set_boolean(modem->modem, "AlwaysOnline", TRUE);
> + }
> +
> return TRUE;
> }
>
> @@ -1168,6 +1183,7 @@ static struct {
> { "mbm", "cdc_ether", "0930" },
> { "mbm", "cdc_ncm", "0930" },
> { "hso", "hso" },
> + { "gobi", "option", "1bc7", "1201" },
I don't understand why you are setting "option" here... does the device
not use qmi_wwan?
Right, thanks for reminding me about this. setup_gobi() requires a modem device.
I didn't want to touch it as i have not other qmi device to test with.
Telit modem has a parallel AT interface using the option driver, so I just
added it there early on.
But I think it is not used anywhere in gobi, so I will try to remove it from
setup_telitqmi on Monday.
Lukasz
/Jonas
_______________________________________________
ofono mailing list
ofono(a)ofono.org
https://lists.ofono.org/mailman/listinfo/ofono
10:15 p.m.
New subject: [PATCH v3 2/2] udevng: add Telit LE910 V1 support
Hi Jonas,
On 24/03/17 23:07, Lukasz Nowak wrote:
Hi Jonas,
I think you will be happy with PATCH v4 :)
On 24/03/17 15:56, Jonas Bonn wrote:
> Hi Lukasz,
>
> I'm working on support for the Quectel UC21 and have some similar
> patches to what you have below. Based on that experience, here are some
> comments:
>
> On 03/24/2017 03:51 PM, Lukasz Nowak wrote:
>> From: Lukasz Nowak <lnowak(a)tycoint.com>
>>
>> Tested with LE910-SVG and Verizon.
>> ---
>> plugins/udevng.c | 24 ++++++++++++++++++++----
>> 1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/plugins/udevng.c b/plugins/udevng.c
>> index 2279bbe..3b15064 100644
>> --- a/plugins/udevng.c
>> +++ b/plugins/udevng.c
>> @@ -182,14 +182,15 @@ static gboolean setup_gobi(struct modem_info *modem)
>> const char *qmi = NULL, *mdm = NULL, *net = NULL;
>> const char *gps = NULL, *diag = NULL;
>> GSList *list;
>> + gboolean telit = FALSE;
>>
>> DBG("%s", modem->syspath);
>>
>> for (list = modem->devices; list; list = list->next) {
>> struct device_info *info = list->data;
>>
>> - DBG("%s %s %s %s", info->devnode, info->interface,
>> - info->number, info->label);
>> + DBG("%s %s %s %s %s", info->devnode, info->interface,
>> + info->number, info->label, info->subsystem);
>
> Put this DBG message change in a separate patch. It's a good idea and I
> did the same thing in my tree, but it's unrelated to the telit support.
>
>>
>> if (g_strcmp0(info->interface, "255/255/255") == 0) {
>> if (info->number == NULL)
>> @@ -199,10 +200,18 @@ static gboolean setup_gobi(struct modem_info *modem)
>> else if (g_strcmp0(info->number, "01") == 0)
>> diag = info->devnode;
>> else if (g_strcmp0(info->number, "02") == 0)
>> - mdm = info->devnode;
>> + if (g_strcmp0(info->subsystem, "net") == 0)
>> + net = info->devnode;
>> + else if (g_strcmp0(info->subsystem, "usbmisc") == 0) {
>> + telit = TRUE;
>> + qmi = info->devnode;
>> + } else
>> + mdm = info->devnode;
>> else if (g_strcmp0(info->number, "03") == 0)
>> gps = info->devnode;
>> - }
>> + } else if (g_strcmp0(info->interface, "255/0/0") == 0)
>> + if (g_strcmp0(info->number, "04") == 0)
>> + mdm = info->devnode;
>> }
>
> I need almost exactly the same code for the UC21, but the endpoints are
> different. What we'll end up with if we do this is an enormous tree of
> cases that all span multiple interfaces, endpoints, and subsystems.
> Because of that, I'm not sure that setup_gobi is the right place to be
> putting this.
>
> The Sierra 7xxx modem does something similar, but uses setup_sierra to
> attach the endpoints and then defers to the gobi driver with:
>
> ofono_modem_set_driver(modem->mode, "gobi"); /* around line 270... in
> setup_sierra */
>
> I suspect that that's a better approach. I think using setup_gobi makes
> little sense because it doesn't provide a clear association to the
> hardware vendor and the devices from different vendors don't have a
> common configuration.
>
> setup_telit makes some assumptions about endpoint 2 that you might not
> want to muck about with... in my opinion, doing a setup_telit_le910() is
> not horrid, and better than making a mess of setup_gobi.
The reason for using setup_gobi() was a problem with udevng, where
cdc-wdm0 would not have vendor_id:product_id. There was no way to separate
setup functions in vendor_list without vid:pid.
This is fixed in v4 - setup_telitqmi() was added.
>
>>
>> if (qmi == NULL || mdm == NULL || net == NULL)
>> @@ -215,6 +224,12 @@ static gboolean setup_gobi(struct modem_info *modem)
>> ofono_modem_set_string(modem->modem, "Diag", diag);
>> ofono_modem_set_string(modem->modem, "NetworkInterface", net);
>>
>> + if (telit) {
>> + /* Telit LE910 V1 modems */
>> + ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
>> + ofono_modem_set_boolean(modem->modem, "AlwaysOnline", TRUE);
>> + }
>> +
>> return TRUE;
>> }
>>
>> @@ -1168,6 +1183,7 @@ static struct {
>> { "mbm", "cdc_ether", "0930" },
>> { "mbm", "cdc_ncm", "0930" },
>> { "hso", "hso" },
>> + { "gobi", "option", "1bc7", "1201" },
> I don't understand why you are setting "option" here... does the device
> not use qmi_wwan?
Right, thanks for reminding me about this. setup_gobi() requires a modem device.
I didn't want to touch it as i have not other qmi device to test with.
Telit modem has a parallel AT interface using the option driver, so I just
added it there early on.
I remember now why this is required. The Telit modem always creates those option devices
alongside qmi_wwan/cdc_wdm, and if I don't list them explicitly mapped as:
{ "telitqmi", "option", "1bc7", "1201" }
then the driver selection will fall back to this:
{ "telit", "option", "1bc7" }
which we don't want.
So the entry in vendor_list[] must stay. I will only remove this:
} else if (g_strcmp0(info->interface, "255/0/0") == 0)
if (g_strcmp0(info->number, "04") == 0)
mdm = info->devnode;
[...]
ofono_modem_set_string(modem->modem, "Modem", mdm);
as it is not doing anything.
Lukasz
But I think it is not used anywhere in gobi, so I will try to remove it from
setup_telitqmi on Monday.
Lukasz
>
> /Jonas
>
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> https://lists.ofono.org/mailman/listinfo/ofono
>
7:59 a.m.
New subject: [PATCH v3 2/2] udevng: add Telit LE910 V1 support
Hi Jonas,
I need almost exactly the same code for the UC21, but the endpoints are
different. What we'll end up with if we do this is an enormous tree of
cases that all span multiple interfaces, endpoints, and subsystems.
Because of that, I'm not sure that setup_gobi is the right place to be
putting this.
The Sierra 7xxx modem does something similar, but uses setup_sierra to
attach the endpoints and then defers to the gobi driver with:
ofono_modem_set_driver(modem->mode, "gobi"); /* around line 270... in
setup_sierra */
I suspect that that's a better approach. I think using setup_gobi makes
little sense because it doesn't provide a clear association to the
hardware vendor and the devices from different vendors don't have a
common configuration.
Good that you're taking a second look, the QMI stuff is not my
specialty. Marcel was the one who added QMI support, so he can correct
me if I'm wrong. But from what I remember the gobi driver was first
written to support gobi 3000 cards from Dell. They were not branded so
the driver was named gobi. Later, Sierra and Huawei came out with
almost the same card with slight differences. Hence setup_sierra and
setup_huawei use a gobi driver for certain families. setup_gobi itself
is probably only relevant to that original dell card.
setup_telit makes some assumptions about endpoint 2 that you might not
want to muck about with... in my opinion, doing a setup_telit_le910() is
not horrid, and better than making a mess of setup_gobi.
+1 on not using setup_gobi. How hard would it be to modify setup_telit
to handle this?
Regards,
-Denis
2:11 a.m.
Hi Lukasz,
On 03/24/2017 09:51 AM, Lukasz Nowak wrote:
From: Lukasz Nowak <lnowak(a)tycoint.com>
Changes from V2:
- made AlwaysOnline flow cleaner in gobi.c
- it turns out that AlwaysOnline is already a part of core logic
in src/modem.c which have the same effect as if the driver had
no set_online() function at all. Hence removed changes to
gobi_set_online() as it is never called.
Ah yes, I completely forgot that someone added the AlwaysOnline override
so you didn't need to implement a brand new driver without .set_online
implemented. Good that you discovered that, sorry for the confusion.
Regards,
-Denis
2003
days inactive
2004
days old
9 comments
3 participants
participants (3)
-
Denis Kenzior -
Jonas Bonn -
Lukasz Nowak