Re: [PATCH v2 1/3] qmi: add LTE atom driver
Hi Reinhard, So this looks good to me. Since Reinhard has provided invaluable insight >> into how QMI works in the past and seems to have 'insider' knowledge, maybe we can get him to also comment on whether the approach is correct (e.g. whether querying / setting the default profile is equivalent to setting the default LTE attach parameters) or if there's another QMI command that should be used. Hi Denis, unfortunately the assumption that the profile number returned from WDSGetDefaultProfileNumber is used for handling the default LTE attach parameters does not hold in all cases: 1. Huawei ME909u devices are known to use /profile number 16 for LTE attach although WDSGetDefaultProfileNumber returns 1. 2. Some devices like SIMCom SIM7230E will still use their "default default profile number" even if WDSSetDefaultProfileNumber has been used to set it to different value (which is also reported as changed by WDSGetDefaultProfileNumber). Some older devices like Sierra Wireless MC77xx do not implement WDSGetLTEAttachParameters. Okay, so it sounds like we have some vendor quirks to take care of. Since I only have to support a fixed number of QMI devices I still use AT commands and a fixed (16 for the ME909u, 1 otherwise) to handle the default LTE attach parameters. An approach like this may not be suitable for a general framework like Ofono. In theory there's nothing preventing us from using AT commands to implement the lte atom for QMI based devices. You can mix and match atom drivers from different driver trees fairly easily. Another alternative is to introduce vendor quirks for QMI, similar to how we use enumerations defined in drivers/atmodem/vendor.h. Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH v2 1/3] qmi: add LTE atom driver
On Fri, Mar 02, 2018 at 10:36:05AM -0600, Denis Kenzior wrote: > Hi Jonas, > > On 03/02/2018 09:12 AM, Jonas Bonn wrote: > > This patch adds an LTE atom for QMI modems. > > > > This atom sets the APN that the LTE default bearer should use when > > establishing its PDP context. This APN needs to be set on the 'default' > > profile so the atom queries which profile is the default and resets > > it before allowing the APN to be set. > > > > Once configured, the default profile settings are used when the > > modem connects to the network; for this reason, the LTE atom needs > > to be instantiated in post_sim, before the modem is set online. > > --- > > Makefile.am | 1 + > > drivers/qmimodem/lte.c | 266 > > > > drivers/qmimodem/qmimodem.c | 2 + > > drivers/qmimodem/qmimodem.h | 3 + > > 4 files changed, 272 insertions(+) > > create mode 100644 drivers/qmimodem/lte.c > > So this looks good to me. Since Reinhard has provided invaluable insight > into how QMI works in the past and seems to have 'insider' knowledge, maybe > we can get him to also comment on whether the approach is correct (e.g. > whether querying / setting the default profile is equivalent to setting the > default LTE attach parameters) or if there's another QMI command that should > be used. Hi Denis, unfortunately the assumption that the profile number returned from WDSGetDefaultProfileNumber is used for handling the default LTE attach parameters does not hold in all cases: 1. Huawei ME909u devices are known to use /profile number 16 for LTE attach although WDSGetDefaultProfileNumber returns 1. 2. Some devices like SIMCom SIM7230E will still use their "default default profile number" even if WDSSetDefaultProfileNumber has been used to set it to different value (which is also reported as changed by WDSGetDefaultProfileNumber). Some older devices like Sierra Wireless MC77xx do not implement WDSGetLTEAttachParameters. Since I only have to support a fixed number of QMI devices I still use AT commands and a fixed (16 for the ME909u, 1 otherwise) to handle the default LTE attach parameters. An approach like this may not be suitable for a general framework like Ofono. Regards, Reinhard ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH v2 1/3] qmi: add LTE atom driver
Hi Jonas,
On 03/02/2018 09:12 AM, Jonas Bonn wrote:
This patch adds an LTE atom for QMI modems.
This atom sets the APN that the LTE default bearer should use when
establishing its PDP context. This APN needs to be set on the 'default'
profile so the atom queries which profile is the default and resets
it before allowing the APN to be set.
Once configured, the default profile settings are used when the
modem connects to the network; for this reason, the LTE atom needs
to be instantiated in post_sim, before the modem is set online.
---
Makefile.am | 1 +
drivers/qmimodem/lte.c | 266
drivers/qmimodem/qmimodem.c | 2 +
drivers/qmimodem/qmimodem.h | 3 +
4 files changed, 272 insertions(+)
create mode 100644 drivers/qmimodem/lte.c
So this looks good to me. Since Reinhard has provided invaluable
insight into how QMI works in the past and seems to have 'insider'
knowledge, maybe we can get him to also comment on whether the approach
is correct (e.g. whether querying / setting the default profile is
equivalent to setting the default LTE attach parameters) or if there's
another QMI command that should be used.
Otherwise a few minor nitpicks below, I can fix these or you can:
+static void reset_profile_cb(struct qmi_result *result, void *user_data)
+{
+ struct ofono_lte *lte = user_data;
+ uint16_t error;
+
+ DBG("");
+
+ if (qmi_result_set_error(result, &error)) {
+ ofono_error("Reset profile error: %hd", error);
+ }
No {} please
+
+ ofono_lte_register(lte);
+}
+
+static void get_default_profile_cb(struct qmi_result *result, void *user_data)
+{
+ struct ofono_lte *lte = user_data;
+ struct lte_data *ldd = ofono_lte_get_data(lte);
+ uint16_t error;
+ uint8_t index;
+ struct qmi_param* param;
*param
+ struct {
+ uint8_t type;
+ uint8_t index;
+ } __attribute__((packed)) p = {
+ .type = 0, /* 3GPP */
+ };
+
+ DBG("");
+
+ if (qmi_result_set_error(result, &error)) {
+ ofono_error("Get default profile error: %hd", error);
+ goto error;
+ }
+
+ /* Profile index */
+ if (!qmi_result_get_uint8(result, 0x01, &index)) {
+ ofono_error("Failed query default profile");
+ goto error;
+ }
+
+ DBG("Default profile index: %hhd", index);
+
+ ldd->default_profile = index;
+
+ p.index = index;
+
+ param = qmi_param_new();
+ if (!param)
+ goto error;
+
+ /* Profile selector */
+ qmi_param_append(param, 0x01, sizeof(p), &p);
+
+ /* Reset profile */
+ if (qmi_service_send(ldd->wds, 0x4b, param,
+ reset_profile_cb, lte, NULL) > 0)
+ return;
+
+ qmi_param_free(param);
+
+error:
+ ofono_error("Failed to reset profile %hhd", index);
+ ofono_lte_register(lte);
Do you actually want to register here still or remove the atom instead?
+}
+
+
no double empty lines please
+static void create_wds_cb(struct qmi_service *service, void *user_data)
+{
+ struct ofono_lte *lte = user_data;
+ struct lte_data *ldd = ofono_lte_get_data(lte);
+ struct qmi_param* param;
*param
+ struct {
+ uint8_t type;
+ uint8_t family;
+ } __attribute((packed)) p = {
+ .type = 0, /* 3GPP */
+ .family = 0, /* embedded */
+ };
+
+ DBG("");
+
+ if (!service) {
+ ofono_error("Failed to request WDS service");
+ ofono_lte_remove(lte);
+ return;
+ }
+
+ ldd->wds = qmi_service_ref(service);
+
+ /* Query the default profile */
+ param = qmi_param_new();
+ if (!param)
+ goto error;
+
+ /* Profile type */
+ qmi_param_append(param, 0x1, sizeof(p), &p);
+
+ /* Get default profile */
+ if (qmi_service_send(ldd->wds, 0x49, param,
+ get_default_profile_cb, lte, NULL) > 0)
+ return;
+
+ qmi_param_free(param);
+
+error:
+ ofono_error("Failed to query default profile");
+ ofono_lte_register(lte);
+}
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono
