[PATCH 0/1] add send_passwd feature to qmimodem sim driver

[PATCH 0/1] add send_passwd feature to qmimodem sim driver

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

Add ability to send PIN to modem (using QMI_UIM_VERIFY_PIN command).
This has been tested on MC7304 and MC7430 modems.
Password state and number of retries can now change after driver
initialization. So query_passwd_state and query_pin_retries are
implemented by asking modem for this information rather than remembering
initial value.
A retry has been implemented on query_passwd_state command. This command
is called just after PIN is entered and return a temporary error
(seen on MC7430).

Christophe Ronco (1):
  MC7430: Add send_passwd feature to qmimodem sim driver

 drivers/qmimodem/sim.c | 278 +++++++++++++++++++++++++++++++++++++++----------
 drivers/qmimodem/uim.h |  11 ++
 2 files changed, 232 insertions(+), 57 deletions(-)

-- 
2.11.0

[PATCH 1/1] MC7430: Add send_passwd feature to qmimodem sim driver

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 11788 bytes --]

drivers/qmimodem/sim.c
 Add send_passwd feature to qmimodem smi driver.
 Password state and number of retries asked using QMI_UIM_GET_CARD_STATUS command
 rather than remembered after initial QMI_UIM_GET_CARD_STATUS command.
 Retry implemented for query_passwd_state if temporary error detected.
drivers/qmimodem/uim.h
 Add VERIFY_PIN message parameters
---
 drivers/qmimodem/sim.c | 278 +++++++++++++++++++++++++++++++++++++++----------
 drivers/qmimodem/uim.h |  11 ++
 2 files changed, 232 insertions(+), 57 deletions(-)

diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
index f61466d5..9a01be2e 100644
--- a/drivers/qmimodem/sim.c
+++ b/drivers/qmimodem/sim.c
@@ -39,17 +39,36 @@
 #define EF_STATUS_INVALIDATED 0
 #define EF_STATUS_VALID 1
 
+/* max number of retry of commands that can temporary fail */
+#define MAX_RETRY_COUNT 100
+
+enum get_card_status_result {
+	GET_CARD_STATUS_RESULT_OK =	0, /* No error */
+	GET_CARD_STATUS_RESULT_ERROR =	1, /* Definitive error */
+	GET_CARD_STATUS_RESULT_TEMP_ERROR =	2 /* Temp error, a retry could work */
+};
+
+/* information from QMI_UIM_GET_CARD_STATUS command */
+struct sim_status {
+	uint8_t card_state;
+	uint8_t app_type;
+	uint8_t passwd_state;
+	int retries[OFONO_SIM_PASSWORD_INVALID];
+};
+
 struct sim_data {
 	struct qmi_device *qmi_dev;
 	struct qmi_service *dms;
 	struct qmi_service *uim;
 	uint32_t event_mask;
-	uint8_t card_state;
 	uint8_t app_type;
-	uint8_t passwd_state;
-	int retries[OFONO_SIM_PASSWORD_INVALID];
+	uint32_t retry_count;
 };
 
+static void qmi_query_passwd_state(struct ofono_sim *sim,
+				ofono_sim_passwd_cb_t cb, void *user_data);
+
+
 static int create_fileid_data(uint8_t app_type, int fileid,
 					const unsigned char *path,
 					unsigned int path_len,
@@ -339,76 +358,52 @@ static void qmi_read_imsi(struct ofono_sim *sim,
 	g_free(cbd);
 }
 
-static void qmi_query_passwd_state(struct ofono_sim *sim,
-				ofono_sim_passwd_cb_t cb, void *user_data)
-{
-	struct sim_data *data = ofono_sim_get_data(sim);
-
-	DBG("passwd state %d", data->passwd_state);
-
-	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
-		CALLBACK_WITH_FAILURE(cb, -1, user_data);
-		return;
-	}
-
-	CALLBACK_WITH_SUCCESS(cb, data->passwd_state, user_data);
-}
-
-static void qmi_query_pin_retries(struct ofono_sim *sim,
-				ofono_sim_pin_retries_cb_t cb, void *user_data)
-{
-	struct sim_data *data = ofono_sim_get_data(sim);
-
-	DBG("passwd state %d", data->passwd_state);
-
-	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
-		CALLBACK_WITH_FAILURE(cb, NULL, user_data);
-		return;
-	}
-
-	CALLBACK_WITH_SUCCESS(cb, data->retries, user_data);
-}
-
-static void card_setup(const struct qmi_uim_slot_info *slot,
+static gboolean get_card_status(const struct qmi_uim_slot_info *slot,
 					const struct qmi_uim_app_info1 *info1,
 					const struct qmi_uim_app_info2 *info2,
-							struct sim_data *data)
+					struct sim_status *sim_stat)
 {
-	data->card_state = slot->card_state;
-	data->app_type = info1->app_type;
+	gboolean need_retry = FALSE;
+	sim_stat->card_state = slot->card_state;
+	sim_stat->app_type = info1->app_type;
 
 	switch (info1->app_state) {
 	case 0x02:	/* PIN1 or UPIN is required */
-		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
 		break;
 	case 0x03:	/* PUK1 or PUK for UPIN is required */
-		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
+		break;
+	case 0x04:	/* Personalization state must be checked. */
+		/* This is temporary, we should retry */
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
+		need_retry = TRUE;
 		break;
 	case 0x07:	/* Ready */
-		data->passwd_state = OFONO_SIM_PASSWORD_NONE;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
 		break;
 	default:
-		data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
+		DBG("info1->app_state:0x%x: OFONO_SIM_PASSWORD_INVALID", info1->app_state);
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
 		break;
 	}
 
-	data->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
-	data->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
 
-	data->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
-	data->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
+	return need_retry;
 }
 
-static void get_card_status_cb(struct qmi_result *result, void *user_data)
+static enum get_card_status_result handle_get_card_status_result(
+		struct qmi_result *result, struct sim_status *sim_stat)
 {
-	struct ofono_sim *sim = user_data;
-	struct sim_data *data = ofono_sim_get_data(sim);
 	const void *ptr;
 	const struct qmi_uim_card_status *status;
 	uint16_t len, offset;
 	uint8_t i;
-
-	DBG("");
+	enum get_card_status_result res = GET_CARD_STATUS_RESULT_ERROR;
 
 	if (qmi_result_set_error(result, NULL))
 		goto done;
@@ -442,12 +437,184 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
 			index = GUINT16_FROM_LE(status->index_gw_pri);
 
 			if ((index & 0xff) == i && (index >> 8) == n)
-				card_setup(slot, info1, info2, data);
+			{
+				if(get_card_status(slot, info1, info2, sim_stat))
+					res = GET_CARD_STATUS_RESULT_TEMP_ERROR; /* need retry */
+				else
+					res = GET_CARD_STATUS_RESULT_OK;
+			}
 		}
 	}
 
 done:
-	switch (data->card_state) {
+	return res;
+}
+
+static void query_passwd_state_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_sim *sim = cbd->user;
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct sim_status sim_stat;
+	enum get_card_status_result res;
+
+	res = handle_get_card_status_result(result, &sim_stat);
+	switch(res)
+	{
+		case GET_CARD_STATUS_RESULT_OK:
+			DBG("passwd state %d", sim_stat.passwd_state);
+			data->retry_count = 0;
+			CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
+			break;
+		case GET_CARD_STATUS_RESULT_TEMP_ERROR:
+			data->retry_count++;
+			if(data->retry_count > MAX_RETRY_COUNT) {
+				DBG("Failed after %d attempts", data->retry_count);
+				data->retry_count = 0;
+				CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+			} else {
+				usleep(20000);
+				DBG("Retry command");
+				qmi_query_passwd_state(sim, cb, cbd->data);
+			}
+			break;
+		default:
+		case GET_CARD_STATUS_RESULT_ERROR:
+			DBG("Command failed");
+			data->retry_count = 0;
+			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+			break;
+	}
+}
+
+static void qmi_query_passwd_state(struct ofono_sim *sim,
+				ofono_sim_passwd_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+
+	DBG("");
+
+	cbd->user = sim;
+
+	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
+					query_passwd_state_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+
+	g_free(cbd);
+}
+
+static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_pin_retries_cb_t cb = cbd->cb;
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if(handle_get_card_status_result(result, &sim_stat))
+	{
+		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+	}
+	CALLBACK_WITH_SUCCESS(cb, sim_stat.retries, cbd->data);
+}
+
+static void qmi_query_pin_retries(struct ofono_sim *sim,
+				ofono_sim_pin_retries_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+
+	DBG("");
+
+	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
+					query_pin_retries_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+
+	g_free(cbd);
+}
+
+static void pin_send_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_lock_unlock_cb_t cb = cbd->cb;
+
+	DBG("");
+
+	if (qmi_result_set_error(result, NULL)) {
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+}
+
+
+static void qmi_pin_send(struct ofono_sim *sim, const char *passwd,
+			ofono_sim_lock_unlock_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+	int passwd_len;
+	struct qmi_param *param;
+	struct qmi_uim_param_message_info *info_data;
+	unsigned char session_info_data[2];
+
+	DBG("");
+
+	if(!passwd)
+		goto error;
+	passwd_len = strlen(passwd);
+	if(passwd_len <= 0 || passwd_len > 0xFF)
+		goto error;
+	
+
+	param = qmi_param_new();
+	if (!param)
+		goto error;
+
+	/* param info */
+	info_data = alloca(2 + passwd_len);
+	info_data->pin_id = 0x01; /* PIN 1 */
+	info_data->length = (uint8_t)passwd_len;
+	memcpy(info_data->pin_value, passwd, passwd_len);
+	qmi_param_append(param, QMI_UIM_PARAM_MESSAGE_INFO, 2 + passwd_len, info_data);
+	/* param Session Information */
+	session_info_data[0] = 0x6;
+	session_info_data[1] = 0x0;
+	qmi_param_append(param, QMI_UIM_PARAM_MESSAGE_SESSION_INFO, 2, session_info_data);
+
+	if (qmi_service_send(data->uim, QMI_UIM_VERIFY_PIN, param,
+					pin_send_cb, cbd, g_free) > 0)
+		return;
+
+error:
+	qmi_param_free(param);
+
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+	g_free(cbd);
+}
+
+static void get_card_status_cb(struct qmi_result *result, void *user_data)
+{
+	struct ofono_sim *sim = user_data;
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if(handle_get_card_status_result(result, &sim_stat))
+		goto done;
+
+done:
+	data->app_type = sim_stat.app_type;
+	switch (sim_stat.card_state) {
 	case 0x00:	/* Absent */
 	case 0x02:	/* Error */
 		break;
@@ -543,12 +710,8 @@ static int qmi_sim_probe(struct ofono_sim *sim,
 
 	data = g_new0(struct sim_data, 1);
 
-	data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
-
-	for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
-		data->retries[i] = -1;
-
 	data->qmi_dev = device;
+	data->retry_count = 0;
 
 	ofono_sim_set_data(sim, data);
 
@@ -590,6 +753,7 @@ static struct ofono_sim_driver driver = {
 	.read_imsi		= qmi_read_imsi,
 	.query_passwd_state	= qmi_query_passwd_state,
 	.query_pin_retries	= qmi_query_pin_retries,
+	.send_passwd		= qmi_pin_send,
 };
 
 void qmi_sim_init(void)
diff --git a/drivers/qmimodem/uim.h b/drivers/qmimodem/uim.h
index 8f123e7d..cd10e684 100644
--- a/drivers/qmimodem/uim.h
+++ b/drivers/qmimodem/uim.h
@@ -25,6 +25,8 @@
 #define QMI_UIM_WRITE_RECORD		35	/* Write a record */
 #define QMI_UIM_GET_FILE_ATTRIBUTES	36	/* Get file attributes */
 
+#define QMI_UIM_VERIFY_PIN		38	/* Verify PIN */
+
 #define QMI_UIM_EVENT_REGISTRATION	46	/* Register for indications */
 #define QMI_UIM_GET_CARD_STATUS		47	/* Get card status */
 
@@ -91,3 +93,12 @@ struct qmi_uim_file_attributes {
 	uint16_t raw_len;
 	uint8_t raw_value[0];
 } __attribute__((__packed__));
+
+/* Verify PIN parameter */
+#define QMI_UIM_PARAM_MESSAGE_SESSION_INFO	0x01
+#define QMI_UIM_PARAM_MESSAGE_INFO	0x02
+struct qmi_uim_param_message_info {
+	uint8_t pin_id;
+	uint8_t length;
+	uint8_t pin_value[0];
+} __attribute__((__packed__));
-- 
2.11.0

Re: [PATCH 1/1] MC7430: Add send_passwd feature to qmimodem sim driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 13037 bytes --]

Hi Christophe,

On 01/03/2017 04:55 AM, Christophe Ronco wrote:
> drivers/qmimodem/sim.c
>  Add send_passwd feature to qmimodem smi driver.
>  Password state and number of retries asked using QMI_UIM_GET_CARD_STATUS command
>  rather than remembered after initial QMI_UIM_GET_CARD_STATUS command.
>  Retry implemented for query_passwd_state if temporary error detected.
> drivers/qmimodem/uim.h
>  Add VERIFY_PIN message parameters

This really sounds like it should be broken into 3-4 commits.  One for 
each thing you're doing up above.

> ---
>  drivers/qmimodem/sim.c | 278 +++++++++++++++++++++++++++++++++++++++----------
>  drivers/qmimodem/uim.h |  11 ++
>  2 files changed, 232 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
> index f61466d5..9a01be2e 100644
> --- a/drivers/qmimodem/sim.c
> +++ b/drivers/qmimodem/sim.c
> @@ -39,17 +39,36 @@
>  #define EF_STATUS_INVALIDATED 0
>  #define EF_STATUS_VALID 1
>
> +/* max number of retry of commands that can temporary fail */
> +#define MAX_RETRY_COUNT 100
> +
> +enum get_card_status_result {
> +	GET_CARD_STATUS_RESULT_OK =	0, /* No error */
> +	GET_CARD_STATUS_RESULT_ERROR =	1, /* Definitive error */
> +	GET_CARD_STATUS_RESULT_TEMP_ERROR =	2 /* Temp error, a retry could work */
> +};
> +
> +/* information from QMI_UIM_GET_CARD_STATUS command */
> +struct sim_status {
> +	uint8_t card_state;
> +	uint8_t app_type;
> +	uint8_t passwd_state;
> +	int retries[OFONO_SIM_PASSWORD_INVALID];
> +};
> +
>  struct sim_data {
>  	struct qmi_device *qmi_dev;
>  	struct qmi_service *dms;
>  	struct qmi_service *uim;
>  	uint32_t event_mask;
> -	uint8_t card_state;
>  	uint8_t app_type;
> -	uint8_t passwd_state;
> -	int retries[OFONO_SIM_PASSWORD_INVALID];
> +	uint32_t retry_count;
>  };
>
> +static void qmi_query_passwd_state(struct ofono_sim *sim,
> +				ofono_sim_passwd_cb_t cb, void *user_data);
> +
> +

No double empty lines please

>  static int create_fileid_data(uint8_t app_type, int fileid,
>  					const unsigned char *path,
>  					unsigned int path_len,
> @@ -339,76 +358,52 @@ static void qmi_read_imsi(struct ofono_sim *sim,
>  	g_free(cbd);
>  }
>
> -static void qmi_query_passwd_state(struct ofono_sim *sim,
> -				ofono_sim_passwd_cb_t cb, void *user_data)
> -{
> -	struct sim_data *data = ofono_sim_get_data(sim);
> -
> -	DBG("passwd state %d", data->passwd_state);
> -
> -	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
> -		CALLBACK_WITH_FAILURE(cb, -1, user_data);
> -		return;
> -	}
> -
> -	CALLBACK_WITH_SUCCESS(cb, data->passwd_state, user_data);
> -}
> -
> -static void qmi_query_pin_retries(struct ofono_sim *sim,
> -				ofono_sim_pin_retries_cb_t cb, void *user_data)
> -{
> -	struct sim_data *data = ofono_sim_get_data(sim);
> -
> -	DBG("passwd state %d", data->passwd_state);
> -
> -	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
> -		CALLBACK_WITH_FAILURE(cb, NULL, user_data);
> -		return;
> -	}
> -
> -	CALLBACK_WITH_SUCCESS(cb, data->retries, user_data);
> -}
> -
> -static void card_setup(const struct qmi_uim_slot_info *slot,
> +static gboolean get_card_status(const struct qmi_uim_slot_info *slot,
>  					const struct qmi_uim_app_info1 *info1,
>  					const struct qmi_uim_app_info2 *info2,
> -							struct sim_data *data)
> +					struct sim_status *sim_stat)
>  {
> -	data->card_state = slot->card_state;
> -	data->app_type = info1->app_type;
> +	gboolean need_retry = FALSE;
> +	sim_stat->card_state = slot->card_state;
> +	sim_stat->app_type = info1->app_type;
>
>  	switch (info1->app_state) {
>  	case 0x02:	/* PIN1 or UPIN is required */
> -		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
>  		break;
>  	case 0x03:	/* PUK1 or PUK for UPIN is required */
> -		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
> +		break;
> +	case 0x04:	/* Personalization state must be checked. */
> +		/* This is temporary, we should retry */
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
> +		need_retry = TRUE;
>  		break;
>  	case 0x07:	/* Ready */
> -		data->passwd_state = OFONO_SIM_PASSWORD_NONE;
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
>  		break;
>  	default:
> -		data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
> +		DBG("info1->app_state:0x%x: OFONO_SIM_PASSWORD_INVALID", info1->app_state);
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
>  		break;
>  	}
>
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
>
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
> +	return need_retry;
>  }
>
> -static void get_card_status_cb(struct qmi_result *result, void *user_data)
> +static enum get_card_status_result handle_get_card_status_result(
> +		struct qmi_result *result, struct sim_status *sim_stat)
>  {
> -	struct ofono_sim *sim = user_data;
> -	struct sim_data *data = ofono_sim_get_data(sim);
>  	const void *ptr;
>  	const struct qmi_uim_card_status *status;
>  	uint16_t len, offset;
>  	uint8_t i;
> -
> -	DBG("");
> +	enum get_card_status_result res = GET_CARD_STATUS_RESULT_ERROR;
>
>  	if (qmi_result_set_error(result, NULL))
>  		goto done;
> @@ -442,12 +437,184 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
>  			index = GUINT16_FROM_LE(status->index_gw_pri);
>
>  			if ((index & 0xff) == i && (index >> 8) == n)
> -				card_setup(slot, info1, info2, data);
> +			{
> +				if(get_card_status(slot, info1, info2, sim_stat))
> +					res = GET_CARD_STATUS_RESULT_TEMP_ERROR; /* need retry */
> +				else
> +					res = GET_CARD_STATUS_RESULT_OK;
> +			}

This is not our coding style.  Please see the Linux Kernel coding style 
& our amendments in doc/coding-style.txt

>  		}
>  	}
>
>  done:
> -	switch (data->card_state) {
> +	return res;
> +}
> +
> +static void query_passwd_state_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_sim_passwd_cb_t cb = cbd->cb;
> +	struct ofono_sim *sim = cbd->user;
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct sim_status sim_stat;
> +	enum get_card_status_result res;
> +
> +	res = handle_get_card_status_result(result, &sim_stat);
> +	switch(res)
> +	{

Not our coding style

> +		case GET_CARD_STATUS_RESULT_OK:
> +			DBG("passwd state %d", sim_stat.passwd_state);
> +			data->retry_count = 0;
> +			CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
> +			break;
> +		case GET_CARD_STATUS_RESULT_TEMP_ERROR:
> +			data->retry_count++;
> +			if(data->retry_count > MAX_RETRY_COUNT) {
> +				DBG("Failed after %d attempts", data->retry_count);
> +				data->retry_count = 0;
> +				CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> +			} else {
> +				usleep(20000);
> +				DBG("Retry command");
> +				qmi_query_passwd_state(sim, cb, cbd->data);
> +			}
> +			break;
> +		default:
> +		case GET_CARD_STATUS_RESULT_ERROR:
> +			DBG("Command failed");
> +			data->retry_count = 0;
> +			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> +			break;
> +	}
> +}
> +
> +static void qmi_query_passwd_state(struct ofono_sim *sim,
> +				ofono_sim_passwd_cb_t cb, void *user_data)
> +{
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +
> +	DBG("");
> +
> +	cbd->user = sim;
> +
> +	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
> +					query_passwd_state_cb, cbd, g_free) > 0)
> +		return;
> +
> +	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> +
> +	g_free(cbd);
> +}
> +
> +static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_sim_pin_retries_cb_t cb = cbd->cb;
> +	struct sim_status sim_stat;
> +
> +	DBG("");
> +
> +	if(handle_get_card_status_result(result, &sim_stat))
> +	{
> +		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
> +	}
> +	CALLBACK_WITH_SUCCESS(cb, sim_stat.retries, cbd->data);
> +}
> +
> +static void qmi_query_pin_retries(struct ofono_sim *sim,
> +				ofono_sim_pin_retries_cb_t cb, void *user_data)
> +{
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +
> +	DBG("");
> +
> +	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
> +					query_pin_retries_cb, cbd, g_free) > 0)
> +		return;
> +
> +	CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
> +
> +	g_free(cbd);
> +}
> +
> +static void pin_send_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_sim_lock_unlock_cb_t cb = cbd->cb;
> +
> +	DBG("");
> +
> +	if (qmi_result_set_error(result, NULL)) {
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);
> +		return;
> +	}
> +
> +	CALLBACK_WITH_SUCCESS(cb, cbd->data);
> +}
> +
> +
> +static void qmi_pin_send(struct ofono_sim *sim, const char *passwd,
> +			ofono_sim_lock_unlock_cb_t cb, void *user_data)
> +{
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +	int passwd_len;
> +	struct qmi_param *param;
> +	struct qmi_uim_param_message_info *info_data;
> +	unsigned char session_info_data[2];
> +
> +	DBG("");
> +
> +	if(!passwd)
> +		goto error;
> +	passwd_len = strlen(passwd);
> +	if(passwd_len <= 0 || passwd_len > 0xFF)
> +		goto error;
> +	
> +

Double empty lines, mixed space/tab indentation.  Please fix this.

> +	param = qmi_param_new();
> +	if (!param)
> +		goto error;
> +
> +	/* param info */
> +	info_data = alloca(2 + passwd_len);
> +	info_data->pin_id = 0x01; /* PIN 1 */
> +	info_data->length = (uint8_t)passwd_len;
> +	memcpy(info_data->pin_value, passwd, passwd_len);
> +	qmi_param_append(param, QMI_UIM_PARAM_MESSAGE_INFO, 2 + passwd_len, info_data);
> +	/* param Session Information */
> +	session_info_data[0] = 0x6;
> +	session_info_data[1] = 0x0;
> +	qmi_param_append(param, QMI_UIM_PARAM_MESSAGE_SESSION_INFO, 2, session_info_data);

Many lines > 80 characters.  Please fix this accordingly.

> +
> +	if (qmi_service_send(data->uim, QMI_UIM_VERIFY_PIN, param,
> +					pin_send_cb, cbd, g_free) > 0)
> +		return;
> +
> +error:
> +	qmi_param_free(param);
> +
> +	CALLBACK_WITH_FAILURE(cb, cbd->data);
> +
> +	g_free(cbd);
> +}
> +
> +static void get_card_status_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct ofono_sim *sim = user_data;
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct sim_status sim_stat;
> +
> +	DBG("");
> +
> +	if(handle_get_card_status_result(result, &sim_stat))
> +		goto done;
> +
> +done:
> +	data->app_type = sim_stat.app_type;
> +	switch (sim_stat.card_state) {
>  	case 0x00:	/* Absent */
>  	case 0x02:	/* Error */
>  		break;
> @@ -543,12 +710,8 @@ static int qmi_sim_probe(struct ofono_sim *sim,
>
>  	data = g_new0(struct sim_data, 1);
>
> -	data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
> -
> -	for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
> -		data->retries[i] = -1;
> -
>  	data->qmi_dev = device;
> +	data->retry_count = 0;
>
>  	ofono_sim_set_data(sim, data);
>
> @@ -590,6 +753,7 @@ static struct ofono_sim_driver driver = {
>  	.read_imsi		= qmi_read_imsi,
>  	.query_passwd_state	= qmi_query_passwd_state,
>  	.query_pin_retries	= qmi_query_pin_retries,
> +	.send_passwd		= qmi_pin_send,
>  };
>
>  void qmi_sim_init(void)
> diff --git a/drivers/qmimodem/uim.h b/drivers/qmimodem/uim.h
> index 8f123e7d..cd10e684 100644
> --- a/drivers/qmimodem/uim.h
> +++ b/drivers/qmimodem/uim.h
> @@ -25,6 +25,8 @@
>  #define QMI_UIM_WRITE_RECORD		35	/* Write a record */
>  #define QMI_UIM_GET_FILE_ATTRIBUTES	36	/* Get file attributes */
>
> +#define QMI_UIM_VERIFY_PIN		38	/* Verify PIN */
> +
>  #define QMI_UIM_EVENT_REGISTRATION	46	/* Register for indications */
>  #define QMI_UIM_GET_CARD_STATUS		47	/* Get card status */
>
> @@ -91,3 +93,12 @@ struct qmi_uim_file_attributes {
>  	uint16_t raw_len;
>  	uint8_t raw_value[0];
>  } __attribute__((__packed__));
> +
> +/* Verify PIN parameter */
> +#define QMI_UIM_PARAM_MESSAGE_SESSION_INFO	0x01
> +#define QMI_UIM_PARAM_MESSAGE_INFO	0x02
> +struct qmi_uim_param_message_info {
> +	uint8_t pin_id;
> +	uint8_t length;
> +	uint8_t pin_value[0];
> +} __attribute__((__packed__));
>

Regards,
-Denis

[PATCH 0/4] Add pin_send feature to qmimodem sim driver

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

Previous patch broken down in 4 patches and aligned with current HEAD
I hope I have fixed the coding rules problems present in my first patch
version.

Christophe Ronco (4):
  qmimodem: get password state from modem
  qmimodem: add pin_send feature
  qmimodem: query_passwd_state can be retried
  udevng: Sierra modems use SIM driver

 drivers/qmimodem/sim.c | 287 +++++++++++++++++++++++++++++++++++++++----------
 drivers/qmimodem/uim.h |  11 ++
 plugins/udevng.c       |   2 -
 3 files changed, 239 insertions(+), 61 deletions(-)

-- 
2.11.0

[PATCH 1/4] qmimodem: get password state from modem

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 7420 bytes --]

Password state and number of retries asked to modem using
QMI_UIM_GET_CARD_STATUS command rather than remembered after initial
QMI_UIM_GET_CARD_STATUS command.
---
 drivers/qmimodem/sim.c | 173 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 115 insertions(+), 58 deletions(-)

diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
index db012bcf..c4c79afe 100644
--- a/drivers/qmimodem/sim.c
+++ b/drivers/qmimodem/sim.c
@@ -39,15 +39,20 @@
 #define EF_STATUS_INVALIDATED 0
 #define EF_STATUS_VALID 1
 
+/* information from QMI_UIM_GET_CARD_STATUS command */
+struct sim_status {
+	uint8_t card_state;
+	uint8_t app_type;
+	uint8_t passwd_state;
+	int retries[OFONO_SIM_PASSWORD_INVALID];
+};
+
 struct sim_data {
 	struct qmi_device *qmi_dev;
 	struct qmi_service *dms;
 	struct qmi_service *uim;
 	uint32_t event_mask;
-	uint8_t card_state;
 	uint8_t app_type;
-	uint8_t passwd_state;
-	int retries[OFONO_SIM_PASSWORD_INVALID];
 };
 
 static int create_fileid_data(uint8_t app_type, int fileid,
@@ -339,76 +344,50 @@ static void qmi_read_imsi(struct ofono_sim *sim,
 	g_free(cbd);
 }
 
-static void qmi_query_passwd_state(struct ofono_sim *sim,
-				ofono_sim_passwd_cb_t cb, void *user_data)
-{
-	struct sim_data *data = ofono_sim_get_data(sim);
-
-	DBG("passwd state %d", data->passwd_state);
-
-	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
-		CALLBACK_WITH_FAILURE(cb, -1, user_data);
-		return;
-	}
-
-	CALLBACK_WITH_SUCCESS(cb, data->passwd_state, user_data);
-}
-
-static void qmi_query_pin_retries(struct ofono_sim *sim,
-				ofono_sim_pin_retries_cb_t cb, void *user_data)
-{
-	struct sim_data *data = ofono_sim_get_data(sim);
-
-	DBG("passwd state %d", data->passwd_state);
-
-	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
-		CALLBACK_WITH_FAILURE(cb, NULL, user_data);
-		return;
-	}
-
-	CALLBACK_WITH_SUCCESS(cb, data->retries, user_data);
-}
-
-static void card_setup(const struct qmi_uim_slot_info *slot,
+static void get_card_status(const struct qmi_uim_slot_info *slot,
 					const struct qmi_uim_app_info1 *info1,
 					const struct qmi_uim_app_info2 *info2,
-							struct sim_data *data)
+					struct sim_status *sim_stat)
 {
-	data->card_state = slot->card_state;
-	data->app_type = info1->app_type;
+	sim_stat->card_state = slot->card_state;
+	sim_stat->app_type = info1->app_type;
 
 	switch (info1->app_state) {
 	case 0x02:	/* PIN1 or UPIN is required */
-		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
 		break;
 	case 0x03:	/* PUK1 or PUK for UPIN is required */
-		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
+		break;
+	case 0x04:	/* Personalization state must be checked. */
+		/* This is temporary, we could retry and get another result */
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
 		break;
 	case 0x07:	/* Ready */
-		data->passwd_state = OFONO_SIM_PASSWORD_NONE;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
 		break;
 	default:
-		data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
+		DBG("info1->app_state:0x%x: OFONO_SIM_PASSWORD_INVALID",
+			info1->app_state);
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
 		break;
 	}
 
-	data->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
-	data->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
 
-	data->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
-	data->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
 }
 
-static void get_card_status_cb(struct qmi_result *result, void *user_data)
+static gboolean handle_get_card_status_result(
+		struct qmi_result *result, struct sim_status *sim_stat)
 {
-	struct ofono_sim *sim = user_data;
-	struct sim_data *data = ofono_sim_get_data(sim);
 	const void *ptr;
 	const struct qmi_uim_card_status *status;
 	uint16_t len, offset;
 	uint8_t i;
-
-	DBG("");
+	gboolean res = FALSE;
 
 	if (qmi_result_set_error(result, NULL))
 		goto done;
@@ -442,14 +421,98 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
 			index = GUINT16_FROM_LE(status->index_gw_pri);
 
 			if ((index & 0xff) == i && (index >> 8) == n)
-				card_setup(slot, info1, info2, data);
+				get_card_status(slot, info1, info2, sim_stat);
 		}
 	}
 
 done:
+	return res;
+}
+
+static void query_passwd_state_cb(struct qmi_result *result,
+					void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if (handle_get_card_status_result(result, &sim_stat)) {
+		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
+}
+
+static void qmi_query_passwd_state(struct ofono_sim *sim,
+				ofono_sim_passwd_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+
+	DBG("");
+
+	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
+					query_passwd_state_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+
+	g_free(cbd);
+}
+
+static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_pin_retries_cb_t cb = cbd->cb;
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if (handle_get_card_status_result(result, &sim_stat)) {
+		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(cb, sim_stat.retries, cbd->data);
+}
+
+static void qmi_query_pin_retries(struct ofono_sim *sim,
+				ofono_sim_pin_retries_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+
+	DBG("");
+
+	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
+					query_pin_retries_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+
+	g_free(cbd);
+}
+
+static void get_card_status_cb(struct qmi_result *result, void *user_data)
+{
+	struct ofono_sim *sim = user_data;
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if (handle_get_card_status_result(result, &sim_stat)) {
+		data->app_type = 0; /* Unknown */
+		ofono_sim_register(sim);
+		return;
+	}
+
 	ofono_sim_register(sim);
 
-	switch (data->card_state) {
+	switch (sim_stat.card_state) {
 	case 0x00:	/* Absent */
 	case 0x02:	/* Error */
 		break;
@@ -536,17 +599,11 @@ static int qmi_sim_probe(struct ofono_sim *sim,
 {
 	struct qmi_device *device = user_data;
 	struct sim_data *data;
-	int i;
 
 	DBG("");
 
 	data = g_new0(struct sim_data, 1);
 
-	data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
-
-	for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
-		data->retries[i] = -1;
-
 	data->qmi_dev = device;
 
 	ofono_sim_set_data(sim, data);
-- 
2.11.0

[PATCH 2/4] qmimodem: add pin_send feature

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 3424 bytes --]

Add ability to send PIN to a QMI modem using QMI_UIM_VERIFY_PIN command.
This has been tested on MC7304 and MC7430 modems.
---
 drivers/qmimodem/sim.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/qmimodem/uim.h | 11 +++++++++
 2 files changed, 76 insertions(+)

diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
index c4c79afe..ec5cc3e6 100644
--- a/drivers/qmimodem/sim.c
+++ b/drivers/qmimodem/sim.c
@@ -496,6 +496,70 @@ static void qmi_query_pin_retries(struct ofono_sim *sim,
 	g_free(cbd);
 }
 
+static void pin_send_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_lock_unlock_cb_t cb = cbd->cb;
+
+	DBG("");
+
+	if (qmi_result_set_error(result, NULL)) {
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+}
+
+static void qmi_pin_send(struct ofono_sim *sim, const char *passwd,
+			ofono_sim_lock_unlock_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+	int passwd_len;
+	struct qmi_param *param;
+	struct qmi_uim_param_message_info *info_data;
+	unsigned char session_info_data[2];
+
+	DBG("");
+
+	if (!passwd)
+		goto error;
+
+	passwd_len = strlen(passwd);
+
+	if (passwd_len <= 0 || passwd_len > 0xFF)
+		goto error;
+
+	param = qmi_param_new();
+	if (!param)
+		goto error;
+
+	/* param info */
+	info_data = alloca(2 + passwd_len);
+	info_data->pin_id = 0x01; /* PIN 1 */
+	info_data->length = (uint8_t) passwd_len;
+	memcpy(info_data->pin_value, passwd, passwd_len);
+	qmi_param_append(param, QMI_UIM_PARAM_MESSAGE_INFO, 2 + passwd_len,
+					info_data);
+	/* param Session Information */
+	session_info_data[0] = 0x6;
+	session_info_data[1] = 0x0;
+	qmi_param_append(param, QMI_UIM_PARAM_MESSAGE_SESSION_INFO, 2,
+					session_info_data);
+
+	if (qmi_service_send(data->uim, QMI_UIM_VERIFY_PIN, param,
+					pin_send_cb, cbd, g_free) > 0)
+		return;
+
+	qmi_param_free(param);
+
+error:
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+	g_free(cbd);
+}
+
 static void get_card_status_cb(struct qmi_result *result, void *user_data)
 {
 	struct ofono_sim *sim = user_data;
@@ -646,6 +710,7 @@ static struct ofono_sim_driver driver = {
 	.read_imsi		= qmi_read_imsi,
 	.query_passwd_state	= qmi_query_passwd_state,
 	.query_pin_retries	= qmi_query_pin_retries,
+	.send_passwd		= qmi_pin_send,
 };
 
 void qmi_sim_init(void)
diff --git a/drivers/qmimodem/uim.h b/drivers/qmimodem/uim.h
index 8f123e7d..cd10e684 100644
--- a/drivers/qmimodem/uim.h
+++ b/drivers/qmimodem/uim.h
@@ -25,6 +25,8 @@
 #define QMI_UIM_WRITE_RECORD		35	/* Write a record */
 #define QMI_UIM_GET_FILE_ATTRIBUTES	36	/* Get file attributes */
 
+#define QMI_UIM_VERIFY_PIN		38	/* Verify PIN */
+
 #define QMI_UIM_EVENT_REGISTRATION	46	/* Register for indications */
 #define QMI_UIM_GET_CARD_STATUS		47	/* Get card status */
 
@@ -91,3 +93,12 @@ struct qmi_uim_file_attributes {
 	uint16_t raw_len;
 	uint8_t raw_value[0];
 } __attribute__((__packed__));
+
+/* Verify PIN parameter */
+#define QMI_UIM_PARAM_MESSAGE_SESSION_INFO	0x01
+#define QMI_UIM_PARAM_MESSAGE_INFO	0x02
+struct qmi_uim_param_message_info {
+	uint8_t pin_id;
+	uint8_t length;
+	uint8_t pin_value[0];
+} __attribute__((__packed__));
-- 
2.11.0

[PATCH 3/4] qmimodem: query_passwd_state can be retried

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 5907 bytes --]

Retry command QMI_UIM_GET_CARD_STATUS during query_passwd_state if a
temporary error status has been detected.
This happens with a MC7430 modem when query_passwd_state is called just
after PIN is entered.
---
 drivers/qmimodem/sim.c | 75 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 61 insertions(+), 14 deletions(-)

diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
index ec5cc3e6..d13dfe2a 100644
--- a/drivers/qmimodem/sim.c
+++ b/drivers/qmimodem/sim.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <string.h>
+#include <unistd.h>
 
 #include <ofono/log.h>
 #include <ofono/modem.h>
@@ -39,6 +40,15 @@
 #define EF_STATUS_INVALIDATED 0
 #define EF_STATUS_VALID 1
 
+/* max number of retry of commands that can temporary fail */
+#define MAX_RETRY_COUNT 100
+
+enum get_card_status_result {
+	GET_CARD_STATUS_RESULT_OK, /* No error */
+	GET_CARD_STATUS_RESULT_ERROR, /* Definitive error */
+	GET_CARD_STATUS_RESULT_TEMP_ERROR, /* error, a retry could work */
+};
+
 /* information from QMI_UIM_GET_CARD_STATUS command */
 struct sim_status {
 	uint8_t card_state;
@@ -53,8 +63,12 @@ struct sim_data {
 	struct qmi_service *uim;
 	uint32_t event_mask;
 	uint8_t app_type;
+	uint32_t retry_count;
 };
 
+static void qmi_query_passwd_state(struct ofono_sim *sim,
+				ofono_sim_passwd_cb_t cb, void *user_data);
+
 static int create_fileid_data(uint8_t app_type, int fileid,
 					const unsigned char *path,
 					unsigned int path_len,
@@ -344,11 +358,13 @@ static void qmi_read_imsi(struct ofono_sim *sim,
 	g_free(cbd);
 }
 
-static void get_card_status(const struct qmi_uim_slot_info *slot,
+/* Return True if a retry could give another (better) result */
+static gboolean get_card_status(const struct qmi_uim_slot_info *slot,
 					const struct qmi_uim_app_info1 *info1,
 					const struct qmi_uim_app_info2 *info2,
 					struct sim_status *sim_stat)
 {
+	gboolean need_retry = FALSE;
 	sim_stat->card_state = slot->card_state;
 	sim_stat->app_type = info1->app_type;
 
@@ -362,6 +378,7 @@ static void get_card_status(const struct qmi_uim_slot_info *slot,
 	case 0x04:	/* Personalization state must be checked. */
 		/* This is temporary, we could retry and get another result */
 		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
+		need_retry = TRUE;
 		break;
 	case 0x07:	/* Ready */
 		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
@@ -378,16 +395,17 @@ static void get_card_status(const struct qmi_uim_slot_info *slot,
 
 	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
 	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
+	return need_retry;
 }
 
-static gboolean handle_get_card_status_result(
+static enum get_card_status_result handle_get_card_status_result(
 		struct qmi_result *result, struct sim_status *sim_stat)
 {
 	const void *ptr;
 	const struct qmi_uim_card_status *status;
 	uint16_t len, offset;
 	uint8_t i;
-	gboolean res = FALSE;
+	enum get_card_status_result res = GET_CARD_STATUS_RESULT_ERROR;
 
 	if (qmi_result_set_error(result, NULL))
 		goto done;
@@ -420,8 +438,13 @@ static gboolean handle_get_card_status_result(
 
 			index = GUINT16_FROM_LE(status->index_gw_pri);
 
-			if ((index & 0xff) == i && (index >> 8) == n)
-				get_card_status(slot, info1, info2, sim_stat);
+			if ((index & 0xff) == i && (index >> 8) == n) {
+				if (get_card_status(slot, info1, info2,
+								sim_stat))
+					res = GET_CARD_STATUS_RESULT_TEMP_ERROR;
+				else
+					res = GET_CARD_STATUS_RESULT_OK;
+			}
 		}
 	}
 
@@ -434,16 +457,36 @@ static void query_passwd_state_cb(struct qmi_result *result,
 {
 	struct cb_data *cbd = user_data;
 	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_sim *sim = cbd->user;
+	struct sim_data *data = ofono_sim_get_data(sim);
 	struct sim_status sim_stat;
-
-	DBG("");
-
-	if (handle_get_card_status_result(result, &sim_stat)) {
+	enum get_card_status_result res;
+
+	res = handle_get_card_status_result(result, &sim_stat);
+	switch (res) {
+	case GET_CARD_STATUS_RESULT_OK:
+		DBG("passwd state %d", sim_stat.passwd_state);
+		data->retry_count = 0;
+		CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
+		break;
+	case GET_CARD_STATUS_RESULT_TEMP_ERROR:
+		data->retry_count++;
+		if (data->retry_count > MAX_RETRY_COUNT) {
+			DBG("Failed after %d attempts", data->retry_count);
+			data->retry_count = 0;
+			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		} else {
+			usleep(20000);
+			DBG("Retry command");
+			qmi_query_passwd_state(sim, cb, cbd->data);
+		}
+		break;
+	case GET_CARD_STATUS_RESULT_ERROR:
+		DBG("Command failed");
+		data->retry_count = 0;
 		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
-		return;
+		break;
 	}
-
-	CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
 }
 
 static void qmi_query_passwd_state(struct ofono_sim *sim,
@@ -454,6 +497,8 @@ static void qmi_query_passwd_state(struct ofono_sim *sim,
 
 	DBG("");
 
+	cbd->user = sim;
+
 	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
 					query_passwd_state_cb, cbd, g_free) > 0)
 		return;
@@ -471,7 +516,8 @@ static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
 
 	DBG("");
 
-	if (handle_get_card_status_result(result, &sim_stat)) {
+	if (handle_get_card_status_result(result, &sim_stat) !=
+					GET_CARD_STATUS_RESULT_OK) {
 		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
 		return;
 	}
@@ -568,7 +614,8 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
 
 	DBG("");
 
-	if (handle_get_card_status_result(result, &sim_stat)) {
+	if (handle_get_card_status_result(result, &sim_stat) !=
+					GET_CARD_STATUS_RESULT_OK) {
 		data->app_type = 0; /* Unknown */
 		ofono_sim_register(sim);
 		return;
-- 
2.11.0

[PATCH 4/4] udevng: Sierra modems use SIM driver

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

---
 plugins/udevng.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 933bf4fa..a2866b64 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -252,8 +252,6 @@ static gboolean setup_sierra(struct modem_info *modem)
 
 	if (qmi != NULL && net != NULL) {
 		ofono_modem_set_driver(modem->modem, "gobi");
-		/* Fixup SIM interface for Sierra QMI devices */
-		ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
 		goto done;
 	}
 
-- 
2.11.0

Re: [PATCH 1/4] qmimodem: get password state from modem

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 8253 bytes --]

Hi Christophe,

On 01/10/2017 02:38 AM, Christophe Ronco wrote:
> Password state and number of retries asked to modem using
> QMI_UIM_GET_CARD_STATUS command rather than remembered after initial
> QMI_UIM_GET_CARD_STATUS command.
> ---
>  drivers/qmimodem/sim.c | 173 ++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 115 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
> index db012bcf..c4c79afe 100644
> --- a/drivers/qmimodem/sim.c
> +++ b/drivers/qmimodem/sim.c
> @@ -39,15 +39,20 @@
>  #define EF_STATUS_INVALIDATED 0
>  #define EF_STATUS_VALID 1
>
> +/* information from QMI_UIM_GET_CARD_STATUS command */
> +struct sim_status {
> +	uint8_t card_state;
> +	uint8_t app_type;
> +	uint8_t passwd_state;
> +	int retries[OFONO_SIM_PASSWORD_INVALID];
> +};
> +
>  struct sim_data {
>  	struct qmi_device *qmi_dev;
>  	struct qmi_service *dms;
>  	struct qmi_service *uim;
>  	uint32_t event_mask;
> -	uint8_t card_state;
>  	uint8_t app_type;
> -	uint8_t passwd_state;
> -	int retries[OFONO_SIM_PASSWORD_INVALID];
>  };
>
>  static int create_fileid_data(uint8_t app_type, int fileid,
> @@ -339,76 +344,50 @@ static void qmi_read_imsi(struct ofono_sim *sim,
>  	g_free(cbd);
>  }
>
> -static void qmi_query_passwd_state(struct ofono_sim *sim,
> -				ofono_sim_passwd_cb_t cb, void *user_data)
> -{
> -	struct sim_data *data = ofono_sim_get_data(sim);
> -
> -	DBG("passwd state %d", data->passwd_state);
> -
> -	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
> -		CALLBACK_WITH_FAILURE(cb, -1, user_data);
> -		return;
> -	}
> -
> -	CALLBACK_WITH_SUCCESS(cb, data->passwd_state, user_data);
> -}
> -
> -static void qmi_query_pin_retries(struct ofono_sim *sim,
> -				ofono_sim_pin_retries_cb_t cb, void *user_data)
> -{
> -	struct sim_data *data = ofono_sim_get_data(sim);
> -
> -	DBG("passwd state %d", data->passwd_state);
> -
> -	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
> -		CALLBACK_WITH_FAILURE(cb, NULL, user_data);
> -		return;
> -	}
> -
> -	CALLBACK_WITH_SUCCESS(cb, data->retries, user_data);
> -}
> -
> -static void card_setup(const struct qmi_uim_slot_info *slot,
> +static void get_card_status(const struct qmi_uim_slot_info *slot,
>  					const struct qmi_uim_app_info1 *info1,
>  					const struct qmi_uim_app_info2 *info2,
> -							struct sim_data *data)
> +					struct sim_status *sim_stat)
>  {
> -	data->card_state = slot->card_state;
> -	data->app_type = info1->app_type;
> +	sim_stat->card_state = slot->card_state;
> +	sim_stat->app_type = info1->app_type;
>
>  	switch (info1->app_state) {
>  	case 0x02:	/* PIN1 or UPIN is required */
> -		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
>  		break;
>  	case 0x03:	/* PUK1 or PUK for UPIN is required */
> -		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
> +		break;
> +	case 0x04:	/* Personalization state must be checked. */
> +		/* This is temporary, we could retry and get another result */
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
>  		break;
>  	case 0x07:	/* Ready */
> -		data->passwd_state = OFONO_SIM_PASSWORD_NONE;
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
>  		break;
>  	default:
> -		data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
> +		DBG("info1->app_state:0x%x: OFONO_SIM_PASSWORD_INVALID",
> +			info1->app_state);
> +		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
>  		break;
>  	}
>
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
>
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
> -	data->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
> +	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
>  }
>
> -static void get_card_status_cb(struct qmi_result *result, void *user_data)
> +static gboolean handle_get_card_status_result(
> +		struct qmi_result *result, struct sim_status *sim_stat)
>  {
> -	struct ofono_sim *sim = user_data;
> -	struct sim_data *data = ofono_sim_get_data(sim);
>  	const void *ptr;
>  	const struct qmi_uim_card_status *status;
>  	uint16_t len, offset;
>  	uint8_t i;
> -
> -	DBG("");
> +	gboolean res = FALSE;
>
>  	if (qmi_result_set_error(result, NULL))
>  		goto done;
> @@ -442,14 +421,98 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
>  			index = GUINT16_FROM_LE(status->index_gw_pri);
>
>  			if ((index & 0xff) == i && (index >> 8) == n)
> -				card_setup(slot, info1, info2, data);
> +				get_card_status(slot, info1, info2, sim_stat);
>  		}
>  	}
>
>  done:
> +	return res;

This function seems to always return false...

> +}
> +
> +static void query_passwd_state_cb(struct qmi_result *result,
> +					void *user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_sim_passwd_cb_t cb = cbd->cb;
> +	struct sim_status sim_stat;
> +
> +	DBG("");
> +
> +	if (handle_get_card_status_result(result, &sim_stat)) {
> +		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> +		return;
> +	}

So this if-condition is never encountered...

> +
> +	CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
> +}
> +
> +static void qmi_query_passwd_state(struct ofono_sim *sim,
> +				ofono_sim_passwd_cb_t cb, void *user_data)
> +{
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +
> +	DBG("");
> +
> +	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
> +					query_passwd_state_cb, cbd, g_free) > 0)
> +		return;
> +
> +	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> +
> +	g_free(cbd);
> +}
> +
> +static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_sim_pin_retries_cb_t cb = cbd->cb;
> +	struct sim_status sim_stat;
> +
> +	DBG("");
> +
> +	if (handle_get_card_status_result(result, &sim_stat)) {

as above

> +		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
> +		return;
> +	}
> +
> +	CALLBACK_WITH_SUCCESS(cb, sim_stat.retries, cbd->data);
> +}
> +
> +static void qmi_query_pin_retries(struct ofono_sim *sim,
> +				ofono_sim_pin_retries_cb_t cb, void *user_data)
> +{
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +
> +	DBG("");
> +
> +	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
> +					query_pin_retries_cb, cbd, g_free) > 0)
> +		return;
> +
> +	CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
> +
> +	g_free(cbd);
> +}
> +
> +static void get_card_status_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct ofono_sim *sim = user_data;
> +	struct sim_data *data = ofono_sim_get_data(sim);
> +	struct sim_status sim_stat;
> +
> +	DBG("");
> +
> +	if (handle_get_card_status_result(result, &sim_stat)) {
> +		data->app_type = 0; /* Unknown */
> +		ofono_sim_register(sim);
> +		return;
> +	}
> +
>  	ofono_sim_register(sim);

I don't understand what happens here.  Is it really the intent to always 
call ofono_sim_register?  If so, the logic should probably be modified 
to reflect that.

>
> -	switch (data->card_state) {
> +	switch (sim_stat.card_state) {
>  	case 0x00:	/* Absent */
>  	case 0x02:	/* Error */
>  		break;
> @@ -536,17 +599,11 @@ static int qmi_sim_probe(struct ofono_sim *sim,
>  {
>  	struct qmi_device *device = user_data;
>  	struct sim_data *data;
> -	int i;
>
>  	DBG("");
>
>  	data = g_new0(struct sim_data, 1);
>
> -	data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
> -
> -	for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
> -		data->retries[i] = -1;
> -
>  	data->qmi_dev = device;
>
>  	ofono_sim_set_data(sim, data);
>

Regards,
-Denis

Re: [PATCH 2/4] qmimodem: add pin_send feature

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

Hi Christophe,

On 01/10/2017 02:38 AM, Christophe Ronco wrote:
> Add ability to send PIN to a QMI modem using QMI_UIM_VERIFY_PIN command.
> This has been tested on MC7304 and MC7430 modems.
> ---
>  drivers/qmimodem/sim.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/qmimodem/uim.h | 11 +++++++++
>  2 files changed, 76 insertions(+)
>

Looks fine to me.

Re: [PATCH 4/4] udevng: Sierra modems use SIM driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 182 bytes --]

Hi Christophe,

On 01/10/2017 02:38 AM, Christophe Ronco wrote:
> ---
>  plugins/udevng.c | 2 --
>  1 file changed, 2 deletions(-)
>

Applied, thanks.

Regards,
-Denis

[PATCH 0/1] Correction of previous patch

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

Hello Denis,

Thanks again for code review. Here is another (I hope better) version
of the patch.
I changed gboolean for bool, because it is used more often than
gboolean in repository code.

Christophe

Christophe Ronco (1):
  qmimodem: get password state from modem

 drivers/qmimodem/sim.c | 178 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 119 insertions(+), 59 deletions(-)

-- 
2.11.0

[PATCH 1/1] qmimodem: get password state from modem

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 7553 bytes --]

Password state and number of retries asked to modem using
QMI_UIM_GET_CARD_STATUS command rather than remembered after initial
QMI_UIM_GET_CARD_STATUS command.
---
 drivers/qmimodem/sim.c | 178 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 119 insertions(+), 59 deletions(-)

diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
index db012bcf..201217e6 100644
--- a/drivers/qmimodem/sim.c
+++ b/drivers/qmimodem/sim.c
@@ -39,15 +39,20 @@
 #define EF_STATUS_INVALIDATED 0
 #define EF_STATUS_VALID 1
 
+/* information from QMI_UIM_GET_CARD_STATUS command */
+struct sim_status {
+	uint8_t card_state;
+	uint8_t app_type;
+	uint8_t passwd_state;
+	int retries[OFONO_SIM_PASSWORD_INVALID];
+};
+
 struct sim_data {
 	struct qmi_device *qmi_dev;
 	struct qmi_service *dms;
 	struct qmi_service *uim;
 	uint32_t event_mask;
-	uint8_t card_state;
 	uint8_t app_type;
-	uint8_t passwd_state;
-	int retries[OFONO_SIM_PASSWORD_INVALID];
 };
 
 static int create_fileid_data(uint8_t app_type, int fileid,
@@ -339,76 +344,50 @@ static void qmi_read_imsi(struct ofono_sim *sim,
 	g_free(cbd);
 }
 
-static void qmi_query_passwd_state(struct ofono_sim *sim,
-				ofono_sim_passwd_cb_t cb, void *user_data)
-{
-	struct sim_data *data = ofono_sim_get_data(sim);
-
-	DBG("passwd state %d", data->passwd_state);
-
-	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
-		CALLBACK_WITH_FAILURE(cb, -1, user_data);
-		return;
-	}
-
-	CALLBACK_WITH_SUCCESS(cb, data->passwd_state, user_data);
-}
-
-static void qmi_query_pin_retries(struct ofono_sim *sim,
-				ofono_sim_pin_retries_cb_t cb, void *user_data)
-{
-	struct sim_data *data = ofono_sim_get_data(sim);
-
-	DBG("passwd state %d", data->passwd_state);
-
-	if (data->passwd_state == OFONO_SIM_PASSWORD_INVALID) {
-		CALLBACK_WITH_FAILURE(cb, NULL, user_data);
-		return;
-	}
-
-	CALLBACK_WITH_SUCCESS(cb, data->retries, user_data);
-}
-
-static void card_setup(const struct qmi_uim_slot_info *slot,
+static void get_card_status(const struct qmi_uim_slot_info *slot,
 					const struct qmi_uim_app_info1 *info1,
 					const struct qmi_uim_app_info2 *info2,
-							struct sim_data *data)
+					struct sim_status *sim_stat)
 {
-	data->card_state = slot->card_state;
-	data->app_type = info1->app_type;
+	sim_stat->card_state = slot->card_state;
+	sim_stat->app_type = info1->app_type;
 
 	switch (info1->app_state) {
 	case 0x02:	/* PIN1 or UPIN is required */
-		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PIN;
 		break;
 	case 0x03:	/* PUK1 or PUK for UPIN is required */
-		data->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_SIM_PUK;
+		break;
+	case 0x04:	/* Personalization state must be checked. */
+		/* This is temporary, we could retry and get another result */
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
 		break;
 	case 0x07:	/* Ready */
-		data->passwd_state = OFONO_SIM_PASSWORD_NONE;
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
 		break;
 	default:
-		data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
+		DBG("info1->app_state:0x%x: OFONO_SIM_PASSWORD_INVALID",
+			info1->app_state);
+		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
 		break;
 	}
 
-	data->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
-	data->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN] = info2->pin1_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK] = info2->puk1_retries;
 
-	data->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
-	data->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
+	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
 }
 
-static void get_card_status_cb(struct qmi_result *result, void *user_data)
+static bool handle_get_card_status_result(
+		struct qmi_result *result, struct sim_status *sim_stat)
 {
-	struct ofono_sim *sim = user_data;
-	struct sim_data *data = ofono_sim_get_data(sim);
 	const void *ptr;
 	const struct qmi_uim_card_status *status;
 	uint16_t len, offset;
 	uint8_t i;
-
-	DBG("");
+	bool res = false;
 
 	if (qmi_result_set_error(result, NULL))
 		goto done;
@@ -441,15 +420,102 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
 
 			index = GUINT16_FROM_LE(status->index_gw_pri);
 
-			if ((index & 0xff) == i && (index >> 8) == n)
-				card_setup(slot, info1, info2, data);
+			if ((index & 0xff) == i && (index >> 8) == n) {
+				get_card_status(slot, info1, info2, sim_stat);
+				res = true;
+			}
 		}
 	}
 
 done:
+	return res;
+}
+
+static void query_passwd_state_cb(struct qmi_result *result,
+					void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if (!handle_get_card_status_result(result, &sim_stat)) {
+		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
+}
+
+static void qmi_query_passwd_state(struct ofono_sim *sim,
+				ofono_sim_passwd_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+
+	DBG("");
+
+	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
+					query_passwd_state_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+
+	g_free(cbd);
+}
+
+static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_sim_pin_retries_cb_t cb = cbd->cb;
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if (!handle_get_card_status_result(result, &sim_stat)) {
+		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(cb, sim_stat.retries, cbd->data);
+}
+
+static void qmi_query_pin_retries(struct ofono_sim *sim,
+				ofono_sim_pin_retries_cb_t cb, void *user_data)
+{
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+
+	DBG("");
+
+	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
+					query_pin_retries_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+
+	g_free(cbd);
+}
+
+static void get_card_status_cb(struct qmi_result *result, void *user_data)
+{
+	struct ofono_sim *sim = user_data;
+	struct sim_data *data = ofono_sim_get_data(sim);
+	struct sim_status sim_stat;
+
+	DBG("");
+
+	if (!handle_get_card_status_result(result, &sim_stat)) {
+		data->app_type = 0;	/* Unknown */
+		sim_stat.card_state = 0x00;	/* Absent */
+	} else {
+		data->app_type = sim_stat.app_type;
+	}
+
 	ofono_sim_register(sim);
 
-	switch (data->card_state) {
+	switch (sim_stat.card_state) {
 	case 0x00:	/* Absent */
 	case 0x02:	/* Error */
 		break;
@@ -536,17 +602,11 @@ static int qmi_sim_probe(struct ofono_sim *sim,
 {
 	struct qmi_device *device = user_data;
 	struct sim_data *data;
-	int i;
 
 	DBG("");
 
 	data = g_new0(struct sim_data, 1);
 
-	data->passwd_state = OFONO_SIM_PASSWORD_INVALID;
-
-	for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
-		data->retries[i] = -1;
-
 	data->qmi_dev = device;
 
 	ofono_sim_set_data(sim, data);
-- 
2.11.0

Re: [PATCH 1/1] qmimodem: get password state from modem

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

Hi Christophe,

On 01/11/2017 04:06 AM, Christophe Ronco wrote:
> Password state and number of retries asked to modem using
> QMI_UIM_GET_CARD_STATUS command rather than remembered after initial
> QMI_UIM_GET_CARD_STATUS command.
> ---
>  drivers/qmimodem/sim.c | 178 +++++++++++++++++++++++++++++++++----------------
>  1 file changed, 119 insertions(+), 59 deletions(-)
>

Applied, thanks.  I also went ahead and applied patch #2 from your 
previous series: "qmimodem: add pin_send feature".  Patch #3 no longer 
applies, so please resubmit that one.

Regards,
-Denis

[PATCH 0/1] Patch #3 resubmitted

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]

I changed again gboolean for bool.

Christophe Ronco (1):
  qmimodem: query_passwd_state can be retried

 drivers/qmimodem/sim.c | 74 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 60 insertions(+), 14 deletions(-)

-- 
2.11.0

[PATCH 1/1] qmimodem: query_passwd_state can be retried

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 5870 bytes --]

Retry command QMI_UIM_GET_CARD_STATUS during query_passwd_state if a
temporary error status has been detected.
This happens with a MC7430 modem when query_passwd_state is called just
after PIN is entered.
---
 drivers/qmimodem/sim.c | 74 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 60 insertions(+), 14 deletions(-)

diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
index 8e5042db..8aac88ba 100644
--- a/drivers/qmimodem/sim.c
+++ b/drivers/qmimodem/sim.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <string.h>
+#include <unistd.h>
 
 #include <ofono/log.h>
 #include <ofono/modem.h>
@@ -39,6 +40,15 @@
 #define EF_STATUS_INVALIDATED 0
 #define EF_STATUS_VALID 1
 
+/* max number of retry of commands that can temporary fail */
+#define MAX_RETRY_COUNT 100
+
+enum get_card_status_result {
+	GET_CARD_STATUS_RESULT_OK, /* No error */
+	GET_CARD_STATUS_RESULT_ERROR, /* Definitive error */
+	GET_CARD_STATUS_RESULT_TEMP_ERROR, /* error, a retry could work */
+};
+
 /* information from QMI_UIM_GET_CARD_STATUS command */
 struct sim_status {
 	uint8_t card_state;
@@ -53,8 +63,12 @@ struct sim_data {
 	struct qmi_service *uim;
 	uint32_t event_mask;
 	uint8_t app_type;
+	uint32_t retry_count;
 };
 
+static void qmi_query_passwd_state(struct ofono_sim *sim,
+				ofono_sim_passwd_cb_t cb, void *user_data);
+
 static int create_fileid_data(uint8_t app_type, int fileid,
 					const unsigned char *path,
 					unsigned int path_len,
@@ -344,11 +358,13 @@ static void qmi_read_imsi(struct ofono_sim *sim,
 	g_free(cbd);
 }
 
-static void get_card_status(const struct qmi_uim_slot_info *slot,
+/* Return true if a retry could give another (better) result */
+static bool get_card_status(const struct qmi_uim_slot_info *slot,
 					const struct qmi_uim_app_info1 *info1,
 					const struct qmi_uim_app_info2 *info2,
 					struct sim_status *sim_stat)
 {
+	bool need_retry = false;
 	sim_stat->card_state = slot->card_state;
 	sim_stat->app_type = info1->app_type;
 
@@ -362,6 +378,7 @@ static void get_card_status(const struct qmi_uim_slot_info *slot,
 	case 0x04:	/* Personalization state must be checked. */
 		/* This is temporary, we could retry and get another result */
 		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
+		need_retry = true;
 		break;
 	case 0x07:	/* Ready */
 		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
@@ -378,16 +395,18 @@ static void get_card_status(const struct qmi_uim_slot_info *slot,
 
 	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
 	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
+
+	return need_retry;
 }
 
-static bool handle_get_card_status_result(
+static enum get_card_status_result handle_get_card_status_result(
 		struct qmi_result *result, struct sim_status *sim_stat)
 {
 	const void *ptr;
 	const struct qmi_uim_card_status *status;
 	uint16_t len, offset;
 	uint8_t i;
-	bool res = false;
+	enum get_card_status_result res = GET_CARD_STATUS_RESULT_ERROR;
 
 	if (qmi_result_set_error(result, NULL))
 		goto done;
@@ -421,8 +440,11 @@ static bool handle_get_card_status_result(
 			index = GUINT16_FROM_LE(status->index_gw_pri);
 
 			if ((index & 0xff) == i && (index >> 8) == n) {
-				get_card_status(slot, info1, info2, sim_stat);
-				res = true;
+				if (get_card_status(slot, info1, info2,
+								sim_stat))
+					res = GET_CARD_STATUS_RESULT_TEMP_ERROR;
+				else
+					res = GET_CARD_STATUS_RESULT_OK;
 			}
 		}
 	}
@@ -436,16 +458,36 @@ static void query_passwd_state_cb(struct qmi_result *result,
 {
 	struct cb_data *cbd = user_data;
 	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_sim *sim = cbd->user;
+	struct sim_data *data = ofono_sim_get_data(sim);
 	struct sim_status sim_stat;
-
-	DBG("");
-
-	if (!handle_get_card_status_result(result, &sim_stat)) {
+	enum get_card_status_result res;
+
+	res = handle_get_card_status_result(result, &sim_stat);
+	switch (res) {
+	case GET_CARD_STATUS_RESULT_OK:
+		DBG("passwd state %d", sim_stat.passwd_state);
+		data->retry_count = 0;
+		CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
+		break;
+	case GET_CARD_STATUS_RESULT_TEMP_ERROR:
+		data->retry_count++;
+		if (data->retry_count > MAX_RETRY_COUNT) {
+			DBG("Failed after %d attempts", data->retry_count);
+			data->retry_count = 0;
+			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		} else {
+			usleep(20000);
+			DBG("Retry command");
+			qmi_query_passwd_state(sim, cb, cbd->data);
+		}
+		break;
+	case GET_CARD_STATUS_RESULT_ERROR:
+		DBG("Command failed");
+		data->retry_count = 0;
 		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
-		return;
+		break;
 	}
-
-	CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
 }
 
 static void qmi_query_passwd_state(struct ofono_sim *sim,
@@ -456,6 +498,8 @@ static void qmi_query_passwd_state(struct ofono_sim *sim,
 
 	DBG("");
 
+	cbd->user = sim;
+
 	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
 					query_passwd_state_cb, cbd, g_free) > 0)
 		return;
@@ -473,7 +517,8 @@ static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
 
 	DBG("");
 
-	if (!handle_get_card_status_result(result, &sim_stat)) {
+	if (handle_get_card_status_result(result, &sim_stat) !=
+					GET_CARD_STATUS_RESULT_OK) {
 		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
 		return;
 	}
@@ -570,7 +615,8 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
 
 	DBG("");
 
-	if (!handle_get_card_status_result(result, &sim_stat)) {
+	if (handle_get_card_status_result(result, &sim_stat) !=
+					GET_CARD_STATUS_RESULT_OK) {
 		data->app_type = 0;	/* Unknown */
 		sim_stat.card_state = 0x00;	/* Absent */
 	} else {
-- 
2.11.0

Re: [PATCH 1/1] qmimodem: query_passwd_state can be retried

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]

Hi Christophe,

All of the patch looks fine, except:

> @@ -436,16 +458,36 @@ static void query_passwd_state_cb(struct qmi_result *result,
>  {
>  	struct cb_data *cbd = user_data;
>  	ofono_sim_passwd_cb_t cb = cbd->cb;
> +	struct ofono_sim *sim = cbd->user;
> +	struct sim_data *data = ofono_sim_get_data(sim);
>  	struct sim_status sim_stat;
> -
> -	DBG("");
> -
> -	if (!handle_get_card_status_result(result, &sim_stat)) {
> +	enum get_card_status_result res;
> +
> +	res = handle_get_card_status_result(result, &sim_stat);
> +	switch (res) {
> +	case GET_CARD_STATUS_RESULT_OK:
> +		DBG("passwd state %d", sim_stat.passwd_state);
> +		data->retry_count = 0;
> +		CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
> +		break;
> +	case GET_CARD_STATUS_RESULT_TEMP_ERROR:
> +		data->retry_count++;
> +		if (data->retry_count > MAX_RETRY_COUNT) {
> +			DBG("Failed after %d attempts", data->retry_count);
> +			data->retry_count = 0;
> +			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> +		} else {
> +			usleep(20000);

You cannot use usleep here.  usleep blocks, so the rest of the daemon is 
unable to process anything else.  So for example, if you have multiple 
modems, usleep would block oFono from receiving / sending any events to 
these, D-Bus API would be blocked, etc.

For an example of how to get around this using GLib timers, see 
drivers/atmodem/atutil.c.

The at_util_sim_state_query (at_util_sim_state_query_new()) is doing 
something extremely similar to what you're trying to do here.

Regards,
-Denis

Re: [PATCH 1/1] qmimodem: query_passwd_state can be retried

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

Hi

On 01/17/2017 04:29 AM, Christophe Ronco wrote:
> Retry command QMI_UIM_GET_CARD_STATUS during query_passwd_state if a
> temporary error status has been detected.
> This happens with a MC7430 modem when query_passwd_state is called just
> after PIN is entered.
> ---
>  drivers/qmimodem/sim.c | 95 ++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
>

Applied, thanks!

Regards,
-Denis

[PATCH 0/1] use g_timeout_add instead of usleep

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 568 bytes --]

Here is another version of the patch using g_timeout_add.
Compare to what is done in at_util_sim_state_query, I choose to wait
only 20ms between attempts (2s in at_util_sim_state_query).
On my modem, I manage to recover from temporary error after 7 to 10
tries (less than 2 seconds).
Let me know if you prefer a bigger wait before retry.

Christophe Ronco (1):
  qmimodem: query_passwd_state can be retried

 drivers/qmimodem/sim.c | 95 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 81 insertions(+), 14 deletions(-)

-- 
2.11.0

[PATCH 1/1] qmimodem: query_passwd_state can be retried

[Christophe Ronco]

[-- Attachment #1: Type: text/plain, Size: 6564 bytes --]

Retry command QMI_UIM_GET_CARD_STATUS during query_passwd_state if a
temporary error status has been detected.
This happens with a MC7430 modem when query_passwd_state is called just
after PIN is entered.
---
 drivers/qmimodem/sim.c | 95 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 81 insertions(+), 14 deletions(-)

diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c
index 8e5042db..602e5296 100644
--- a/drivers/qmimodem/sim.c
+++ b/drivers/qmimodem/sim.c
@@ -39,6 +39,15 @@
 #define EF_STATUS_INVALIDATED 0
 #define EF_STATUS_VALID 1
 
+/* max number of retry of commands that can temporary fail */
+#define MAX_RETRY_COUNT 100
+
+enum get_card_status_result {
+	GET_CARD_STATUS_RESULT_OK, /* No error */
+	GET_CARD_STATUS_RESULT_ERROR, /* Definitive error */
+	GET_CARD_STATUS_RESULT_TEMP_ERROR, /* error, a retry could work */
+};
+
 /* information from QMI_UIM_GET_CARD_STATUS command */
 struct sim_status {
 	uint8_t card_state;
@@ -53,8 +62,13 @@ struct sim_data {
 	struct qmi_service *uim;
 	uint32_t event_mask;
 	uint8_t app_type;
+	uint32_t retry_count;
+	guint poll_source;
 };
 
+static void qmi_query_passwd_state(struct ofono_sim *sim,
+				ofono_sim_passwd_cb_t cb, void *user_data);
+
 static int create_fileid_data(uint8_t app_type, int fileid,
 					const unsigned char *path,
 					unsigned int path_len,
@@ -344,11 +358,13 @@ static void qmi_read_imsi(struct ofono_sim *sim,
 	g_free(cbd);
 }
 
-static void get_card_status(const struct qmi_uim_slot_info *slot,
+/* Return true if a retry could give another (better) result */
+static bool get_card_status(const struct qmi_uim_slot_info *slot,
 					const struct qmi_uim_app_info1 *info1,
 					const struct qmi_uim_app_info2 *info2,
 					struct sim_status *sim_stat)
 {
+	bool need_retry = false;
 	sim_stat->card_state = slot->card_state;
 	sim_stat->app_type = info1->app_type;
 
@@ -362,6 +378,7 @@ static void get_card_status(const struct qmi_uim_slot_info *slot,
 	case 0x04:	/* Personalization state must be checked. */
 		/* This is temporary, we could retry and get another result */
 		sim_stat->passwd_state = OFONO_SIM_PASSWORD_INVALID;
+		need_retry = true;
 		break;
 	case 0x07:	/* Ready */
 		sim_stat->passwd_state = OFONO_SIM_PASSWORD_NONE;
@@ -378,16 +395,18 @@ static void get_card_status(const struct qmi_uim_slot_info *slot,
 
 	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PIN2] = info2->pin2_retries;
 	sim_stat->retries[OFONO_SIM_PASSWORD_SIM_PUK2] = info2->puk2_retries;
+
+	return need_retry;
 }
 
-static bool handle_get_card_status_result(
+static enum get_card_status_result handle_get_card_status_result(
 		struct qmi_result *result, struct sim_status *sim_stat)
 {
 	const void *ptr;
 	const struct qmi_uim_card_status *status;
 	uint16_t len, offset;
 	uint8_t i;
-	bool res = false;
+	enum get_card_status_result res = GET_CARD_STATUS_RESULT_ERROR;
 
 	if (qmi_result_set_error(result, NULL))
 		goto done;
@@ -421,8 +440,11 @@ static bool handle_get_card_status_result(
 			index = GUINT16_FROM_LE(status->index_gw_pri);
 
 			if ((index & 0xff) == i && (index >> 8) == n) {
-				get_card_status(slot, info1, info2, sim_stat);
-				res = true;
+				if (get_card_status(slot, info1, info2,
+								sim_stat))
+					res = GET_CARD_STATUS_RESULT_TEMP_ERROR;
+				else
+					res = GET_CARD_STATUS_RESULT_OK;
 			}
 		}
 	}
@@ -431,21 +453,59 @@ done:
 	return res;
 }
 
+static gboolean query_passwd_state_retry(gpointer userdata)
+{
+	struct cb_data *cbd = userdata;
+	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_sim *sim = cbd->user;
+	struct sim_data *data = ofono_sim_get_data(sim);
+
+	data->poll_source = 0;
+
+	qmi_query_passwd_state(sim, cb, cbd->data);
+
+	return FALSE;
+}
+
 static void query_passwd_state_cb(struct qmi_result *result,
 					void *user_data)
 {
 	struct cb_data *cbd = user_data;
 	ofono_sim_passwd_cb_t cb = cbd->cb;
+	struct ofono_sim *sim = cbd->user;
+	struct sim_data *data = ofono_sim_get_data(sim);
 	struct sim_status sim_stat;
-
-	DBG("");
-
-	if (!handle_get_card_status_result(result, &sim_stat)) {
+	enum get_card_status_result res;
+	struct cb_data *retry_cbd;
+
+	res = handle_get_card_status_result(result, &sim_stat);
+	switch (res) {
+	case GET_CARD_STATUS_RESULT_OK:
+		DBG("passwd state %d", sim_stat.passwd_state);
+		data->retry_count = 0;
+		CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
+		break;
+	case GET_CARD_STATUS_RESULT_TEMP_ERROR:
+		data->retry_count++;
+		if (data->retry_count > MAX_RETRY_COUNT) {
+			DBG("Failed after %d attempts", data->retry_count);
+			data->retry_count = 0;
+			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		} else {
+			DBG("Retry command");
+			retry_cbd = cb_data_new(cb, cbd->data);
+			retry_cbd->user = sim;
+			data->poll_source = g_timeout_add(20,
+						query_passwd_state_retry,
+						retry_cbd);
+		}
+		break;
+	case GET_CARD_STATUS_RESULT_ERROR:
+		DBG("Command failed");
+		data->retry_count = 0;
 		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
-		return;
+		break;
 	}
-
-	CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data);
 }
 
 static void qmi_query_passwd_state(struct ofono_sim *sim,
@@ -456,6 +516,8 @@ static void qmi_query_passwd_state(struct ofono_sim *sim,
 
 	DBG("");
 
+	cbd->user = sim;
+
 	if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL,
 					query_passwd_state_cb, cbd, g_free) > 0)
 		return;
@@ -473,7 +535,8 @@ static void query_pin_retries_cb(struct qmi_result *result, void *user_data)
 
 	DBG("");
 
-	if (!handle_get_card_status_result(result, &sim_stat)) {
+	if (handle_get_card_status_result(result, &sim_stat) !=
+					GET_CARD_STATUS_RESULT_OK) {
 		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
 		return;
 	}
@@ -570,7 +633,8 @@ static void get_card_status_cb(struct qmi_result *result, void *user_data)
 
 	DBG("");
 
-	if (!handle_get_card_status_result(result, &sim_stat)) {
+	if (handle_get_card_status_result(result, &sim_stat) !=
+					GET_CARD_STATUS_RESULT_OK) {
 		data->app_type = 0;	/* Unknown */
 		sim_stat.card_state = 0x00;	/* Absent */
 	} else {
@@ -689,6 +753,9 @@ static void qmi_sim_remove(struct ofono_sim *sim)
 
 	ofono_sim_set_data(sim, NULL);
 
+	if (data->poll_source > 0)
+		g_source_remove(data->poll_source);
+
 	if (data->uim) {
 		qmi_service_unregister_all(data->uim);
 		qmi_service_unref(data->uim);
-- 
2.11.0