11:28 a.m.
Infineon modem selftest, during ifx_enable().
Two steps trigger, with timeout. In case one fails, modem will not power up.
---
plugins/ifx.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/plugins/ifx.c b/plugins/ifx.c
index 411c012..18250b0 100644
--- a/plugins/ifx.c
+++ b/plugins/ifx.c
@@ -524,7 +524,7 @@ static gboolean mux_timeout_cb(gpointer user_data)
struct ofono_modem *modem = user_data;
struct ifx_data *data = ofono_modem_get_data(modem);
- ofono_error("Timeout with multiplexer setup");
+ ofono_error("Timeout with modem and multiplexer setup");
data->mux_init_timeout = 0;
@@ -539,6 +539,68 @@ static gboolean mux_timeout_cb(gpointer user_data)
return FALSE;
}
+static void dev_ver_selftest_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+
+ struct ofono_modem *modem = user_data;
+ struct ifx_data *data = ofono_modem_get_data(modem);
+
+ if (ok)
+ return;
+
+ ofono_error("at@vers:device_version_id()-FAILED");
+
+ g_at_chat_cancel_all(data->dlcs[AUX_DLC]);
+
+ if (data->mux_init_timeout > 0) {
+ g_source_remove(data->mux_init_timeout);
+ data->mux_init_timeout = 0;
+ }
+
+ if (data->dlcs[AUX_DLC]) {
+ g_at_chat_unref(data->dlcs[AUX_DLC]);
+ data->dlcs[AUX_DLC] = NULL;
+ }
+
+ if (data->device) {
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+ }
+
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static void rtc_gti_selftest_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct ifx_data *data = ofono_modem_get_data(modem);
+
+ if (ok)
+ return;
+
+ ofono_error("at@rtc:rtc_gti_test_verify_32khz()-FAILED");
+
+ g_at_chat_cancel_all(data->dlcs[AUX_DLC]);
+
+ if (data->mux_init_timeout > 0) {
+ g_source_remove(data->mux_init_timeout);
+ data->mux_init_timeout = 0;
+ }
+
+ if (data->dlcs[AUX_DLC]) {
+ g_at_chat_unref(data->dlcs[AUX_DLC]);
+ data->dlcs[AUX_DLC] = NULL;
+ }
+
+ if (data->device) {
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+ }
+
+ ofono_modem_set_powered(modem, FALSE);
+}
static int ifx_enable(struct ofono_modem *modem)
{
struct ifx_data *data = ofono_modem_get_data(modem);
@@ -592,12 +654,23 @@ static int ifx_enable(struct ofono_modem *modem)
g_at_chat_send(chat, "ATE0 +CMEE=1", NULL,
NULL, NULL, NULL);
+ /* Execute Modem Self tests */
+ g_at_chat_send(chat, "at@rtc:rtc_gti_test_verify_32khz()",
+ NULL, rtc_gti_selftest_cb, modem, NULL);
+
+ g_at_chat_send(chat, "at@vers:device_version_id()",
+ NULL, dev_ver_selftest_cb, modem, NULL);
+
+ /* Enable MUX Channels */
data->frame_size = 1509;
g_at_chat_send(chat, "AT+CMUX=0,0,,1509,10,3,30,,", NULL,
mux_setup_cb, modem, NULL);
- data->mux_init_timeout = g_timeout_add_seconds(5, mux_timeout_cb,
+ /* Total Self-test execution time is less than 2s, used 10s
+ * to cover both Self-test and Mux setup time
+ */
+ data->mux_init_timeout = g_timeout_add_seconds(10, mux_timeout_cb,
modem);
data->dlcs[AUX_DLC] = chat;
--
1.7.2.3
12:34 a.m.
Hi Anand,
Infineon modem selftest, during ifx_enable().
Two steps trigger, with timeout. In case one fails, modem will not power up.
so I would prefer if you use git send-email and then use linux.intel.com
as your SMTP server.
That way we are sure that Exchange does not mangle the email and more
important does not mess with your email/author information. We have the
rule that it is firstname lastname and not this weird Outlook stuff with
Lastname first.
And now I going to make some last nitpicks on the style.
---
plugins/ifx.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/plugins/ifx.c b/plugins/ifx.c
index 411c012..18250b0 100644
--- a/plugins/ifx.c
+++ b/plugins/ifx.c
@@ -524,7 +524,7 @@ static gboolean mux_timeout_cb(gpointer user_data)
struct ofono_modem *modem = user_data;
struct ifx_data *data = ofono_modem_get_data(modem);
- ofono_error("Timeout with multiplexer setup");
+ ofono_error("Timeout with modem and multiplexer setup");
data->mux_init_timeout = 0;
@@ -539,6 +539,68 @@ static gboolean mux_timeout_cb(gpointer user_data)
return FALSE;
}
+static void dev_ver_selftest_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
In general the second line should be at least behind the (. So just add
another tab here.
+{
+
Remove this empty line here.
+ struct ofono_modem *modem = user_data;
+ struct ifx_data *data = ofono_modem_get_data(modem);
+
+ if (ok)
+ return;
+
+ ofono_error("at@vers:device_version_id()-FAILED");
+
+ g_at_chat_cancel_all(data->dlcs[AUX_DLC]);
You actually do not need this one. The g_at_chat_unref implies this.
+
+ if (data->mux_init_timeout > 0) {
+ g_source_remove(data->mux_init_timeout);
+ data->mux_init_timeout = 0;
+ }
+
+ if (data->dlcs[AUX_DLC]) {
+ g_at_chat_unref(data->dlcs[AUX_DLC]);
+ data->dlcs[AUX_DLC] = NULL;
+ }
You do not need the if check here. The data->dlcs[AUX_DLC] is always
valid in this callback. If not, then there is a big problem.
+ if (data->device) {
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+ }
Same here. Remove the if check. If data->device is not valid you are in
big trouble.
> +
> + ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +static void rtc_gti_selftest_cb(gboolean ok, GAtResult *result,
> + gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct ifx_data *data = ofono_modem_get_data(modem);
> +
> + if (ok)
> + return;
> +
> + ofono_error("at@rtc:rtc_gti_test_verify_32khz()-FAILED");
> +
> + g_at_chat_cancel_all(data->dlcs[AUX_DLC]);
+
+ if (data->mux_init_timeout > 0) {
+ g_source_remove(data->mux_init_timeout);
+ data->mux_init_timeout = 0;
+ }
+
+ if (data->dlcs[AUX_DLC]) {
+ g_at_chat_unref(data->dlcs[AUX_DLC]);
+ data->dlcs[AUX_DLC] = NULL;
+ }
> +
+ if (data->device) {
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+ }
> +
> + ofono_modem_set_powered(modem, FALSE);
> +}
You need to add an empty line between functions.
And of course everything from the other function applies here as well.
static int ifx_enable(struct ofono_modem *modem)
{
struct ifx_data *data = ofono_modem_get_data(modem);
@@ -592,12 +654,23 @@ static int ifx_enable(struct ofono_modem *modem)
g_at_chat_send(chat, "ATE0 +CMEE=1", NULL,
NULL, NULL, NULL);
+ /* Execute Modem Self tests */
So this read better with /* Execute modem self tests */ instead of this
random capitalization of words. Keep it lowercase and simple ;)
+ g_at_chat_send(chat,
"at@rtc:rtc_gti_test_verify_32khz()",
+ NULL, rtc_gti_selftest_cb, modem, NULL);
+
+ g_at_chat_send(chat, "at@vers:device_version_id()",
+ NULL, dev_ver_selftest_cb, modem, NULL);
+
+ /* Enable MUX Channels */
This reads better as just /* Enable multiplexer */ and no double spaces
please.
data->frame_size = 1509;
g_at_chat_send(chat, "AT+CMUX=0,0,,1509,10,3,30,,", NULL,
mux_setup_cb, modem, NULL);
- data->mux_init_timeout = g_timeout_add_seconds(5, mux_timeout_cb,
+ /* Total Self-test execution time is less than 2s, used 10s
+ * to cover both Self-test and Mux setup time
+ */
Check the M2 coding style for formatting multi-line comments.
/*
* Total self test execution ... 2 seconds. Use
* 10 seconds timeout to cover self tests and multiplexer setup.
*/
+ data->mux_init_timeout = g_timeout_add_seconds(10,
mux_timeout_cb,
modem);
data->dlcs[AUX_DLC] = chat;
Rest looks all fine now. As I said, just some nitpicks.
Regards
Marcel
4243
days inactive
4244
days old
1 comments
2 participants
participants (2)
-
Marcel Holtmann -
Ramakrishna, AnandX