[PATCH] atmodem: Query the list of supported <fac>s from the modem

[PATCH] atmodem: Query the list of supported <fac>s from the modem

[Slava Monich]

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

Not all modems support all <fac>s (particularly, "PS"), let's be polite
and not ask them for the ones they don't support.
---
 drivers/atmodem/sim.c | 69 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 12 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 6395a04..effce6e 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -52,6 +52,7 @@ struct sim_data {
 	unsigned int vendor;
 	guint ready_id;
 	struct at_util_sim_state_query *sim_state_query;
+	const char *passwd_fac[OFONO_SIM_PASSWORD_INVALID];
 };
 
 static const char *crsm_prefix[] = { "+CRSM:", NULL };
@@ -1439,7 +1440,7 @@ static void at_lock_unlock_cb(gboolean ok, GAtResult *result,
 	cb(&error, cbd->data);
 }
 
-static const char *const at_clck_cpwd_fac[] = {
+static const char *const at_clck_cpwd_fac[OFONO_SIM_PASSWORD_INVALID] = {
 	[OFONO_SIM_PASSWORD_SIM_PIN] = "SC",
 	[OFONO_SIM_PASSWORD_SIM_PIN2] = "P2",
 	[OFONO_SIM_PASSWORD_PHSIM_PIN] = "PS",
@@ -1459,13 +1460,13 @@ static void at_pin_enable(struct ofono_sim *sim,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 	int ret;
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+	if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
+				sd->passwd_fac[passwd_type] == NULL)
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
-			at_clck_cpwd_fac[passwd_type], enable ? 1 : 0, passwd);
+			sd->passwd_fac[passwd_type], enable ? 1 : 0, passwd);
 
 	ret = g_at_chat_send(sd->chat, buf, none_prefix,
 				at_lock_unlock_cb, cbd, g_free);
@@ -1490,14 +1491,13 @@ static void at_change_passwd(struct ofono_sim *sim,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 	int ret;
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len ||
-			at_clck_cpwd_fac[passwd_type] == NULL)
+	if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
+			sd->passwd_fac[passwd_type] == NULL)
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
-			at_clck_cpwd_fac[passwd_type], old_passwd, new_passwd);
+			sd->passwd_fac[passwd_type], old_passwd, new_passwd);
 
 	ret = g_at_chat_send(sd->chat, buf, none_prefix,
 				at_lock_unlock_cb, cbd, g_free);
@@ -1550,13 +1550,13 @@ static void at_query_clck(struct ofono_sim *sim,
 	struct sim_data *sd = ofono_sim_get_data(sim);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+	if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
+				sd->passwd_fac[passwd_type] == NULL)
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
-			at_clck_cpwd_fac[passwd_type]);
+			sd->passwd_fac[passwd_type]);
 
 	if (g_at_chat_send(sd->chat, buf, clck_prefix,
 				at_lock_status_cb, cbd, g_free) > 0)
@@ -1568,6 +1568,43 @@ error:
 	CALLBACK_WITH_FAILURE(cb, -1, data);
 }
 
+static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user)
+{
+	struct ofono_sim *sim = user;
+	struct sim_data *sd = ofono_sim_get_data(sim);
+	GAtResultIter iter;
+	const char *fac;
+
+	if (!ok)
+		goto done;
+
+	g_at_result_iter_init(&iter, result);
+
+	/* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */
+	if (!g_at_result_iter_next(&iter, "+CLCK:") ||
+				!g_at_result_iter_open_list(&iter))
+		goto done;
+
+	memset(sd->passwd_fac, 0, sizeof(sd->passwd_fac));
+
+	while (g_at_result_iter_next_string(&iter, &fac)) {
+		int i;
+
+		/* Find it in the list of known <fac>s */
+		for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
+			if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) {
+				DBG("found %s", fac);
+				/* at_clck_cpwd_fac[i] is a static string */
+				sd->passwd_fac[i] = at_clck_cpwd_fac[i];
+				break;
+			}
+		}
+	}
+
+done:
+	ofono_sim_register(sim);
+}
+
 static gboolean at_sim_register(gpointer user)
 {
 	struct ofono_sim *sim = user;
@@ -1591,7 +1628,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
 		g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL);
 
 	ofono_sim_set_data(sim, sd);
-	g_idle_add(at_sim_register, sim);
+
+	/* The default password -> <fac> map */
+	memcpy(sd->passwd_fac, at_clck_cpwd_fac, sizeof(sd->passwd_fac));
+
+	/* Query supported <fac>s */
+	if (!g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
+					at_clck_query_cb, sim, NULL)) {
+		g_idle_add(at_sim_register, sim);
+	}
 
 	return 0;
 }
-- 
1.9.1

Re: [PATCH] atmodem: Query the list of supported <fac>s from the modem

[Denis Kenzior]

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

Hi Slava,

On 10/23/2017 12:57 PM, Slava Monich wrote:
> Not all modems support all <fac>s (particularly, "PS"), let's be polite
> and not ask them for the ones they don't support.
> ---
>   drivers/atmodem/sim.c | 69 ++++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 57 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
> index 6395a04..effce6e 100644
> --- a/drivers/atmodem/sim.c
> +++ b/drivers/atmodem/sim.c
> @@ -52,6 +52,7 @@ struct sim_data {
>   	unsigned int vendor;
>   	guint ready_id;
>   	struct at_util_sim_state_query *sim_state_query;
> +	const char *passwd_fac[OFONO_SIM_PASSWORD_INVALID];

Okay, but why not just make this a bitmap instead?  I think we have like 
15 or 16 SIM passwords, so this could be all of 2 to 4 bytes.  Or use 
idmap...

>   };
>   
>   static const char *crsm_prefix[] = { "+CRSM:", NULL };
> @@ -1439,7 +1440,7 @@ static void at_lock_unlock_cb(gboolean ok, GAtResult *result,
>   	cb(&error, cbd->data);
>   }
>   
> -static const char *const at_clck_cpwd_fac[] = {
> +static const char *const at_clck_cpwd_fac[OFONO_SIM_PASSWORD_INVALID] = {

Is this really needed?  ARRAY_SIZE should still work...

>   	[OFONO_SIM_PASSWORD_SIM_PIN] = "SC",
>   	[OFONO_SIM_PASSWORD_SIM_PIN2] = "P2",
>   	[OFONO_SIM_PASSWORD_PHSIM_PIN] = "PS",
> @@ -1459,13 +1460,13 @@ static void at_pin_enable(struct ofono_sim *sim,
>   	struct cb_data *cbd = cb_data_new(cb, data);
>   	char buf[64];
>   	int ret;
> -	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
>   
> -	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
> +	if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
> +				sd->passwd_fac[passwd_type] == NULL)
>   		goto error;
>   
>   	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
> -			at_clck_cpwd_fac[passwd_type], enable ? 1 : 0, passwd);
> +			sd->passwd_fac[passwd_type], enable ? 1 : 0, passwd);

at_clck_cpwd_fac and passwd_fac contain the same info at a given index 
(if valid).  So this chunk can be omitted.  See using a bitmap above...

>   
>   	ret = g_at_chat_send(sd->chat, buf, none_prefix,
>   				at_lock_unlock_cb, cbd, g_free);
> @@ -1490,14 +1491,13 @@ static void at_change_passwd(struct ofono_sim *sim,
>   	struct cb_data *cbd = cb_data_new(cb, data);
>   	char buf[64];
>   	int ret;
> -	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
>   
> -	if (passwd_type >= len ||
> -			at_clck_cpwd_fac[passwd_type] == NULL)
> +	if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
> +			sd->passwd_fac[passwd_type] == NULL)
>   		goto error;
>   
>   	snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
> -			at_clck_cpwd_fac[passwd_type], old_passwd, new_passwd);
> +			sd->passwd_fac[passwd_type], old_passwd, new_passwd);
>   
>   	ret = g_at_chat_send(sd->chat, buf, none_prefix,
>   				at_lock_unlock_cb, cbd, g_free);
> @@ -1550,13 +1550,13 @@ static void at_query_clck(struct ofono_sim *sim,
>   	struct sim_data *sd = ofono_sim_get_data(sim);
>   	struct cb_data *cbd = cb_data_new(cb, data);
>   	char buf[64];
> -	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
>   
> -	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
> +	if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
> +				sd->passwd_fac[passwd_type] == NULL)
>   		goto error;
>   
>   	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
> -			at_clck_cpwd_fac[passwd_type]);
> +			sd->passwd_fac[passwd_type]);
>   
>   	if (g_at_chat_send(sd->chat, buf, clck_prefix,
>   				at_lock_status_cb, cbd, g_free) > 0)
> @@ -1568,6 +1568,43 @@ error:
>   	CALLBACK_WITH_FAILURE(cb, -1, data);
>   }
>   
> +static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user)
> +{
> +	struct ofono_sim *sim = user;
> +	struct sim_data *sd = ofono_sim_get_data(sim);
> +	GAtResultIter iter;
> +	const char *fac;
> +
> +	if (!ok)
> +		goto done;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	/* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */
> +	if (!g_at_result_iter_next(&iter, "+CLCK:") ||
> +				!g_at_result_iter_open_list(&iter))
> +		goto done;
> +
> +	memset(sd->passwd_fac, 0, sizeof(sd->passwd_fac));
> +
> +	while (g_at_result_iter_next_string(&iter, &fac)) {
> +		int i;
> +
> +		/* Find it in the list of known <fac>s */
> +		for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
> +			if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) {
> +				DBG("found %s", fac);
> +				/* at_clck_cpwd_fac[i] is a static string */
> +				sd->passwd_fac[i] = at_clck_cpwd_fac[i];

All this can do is set an appropriate bit

> +				break;
> +			}
> +		}
> +	}
> +
> +done:
> +	ofono_sim_register(sim);
> +}
> +
>   static gboolean at_sim_register(gpointer user)
>   {
>   	struct ofono_sim *sim = user;
> @@ -1591,7 +1628,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
>   		g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL);
>   
>   	ofono_sim_set_data(sim, sd);
> -	g_idle_add(at_sim_register, sim);
> +
> +	/* The default password -> <fac> map */
> +	memcpy(sd->passwd_fac, at_clck_cpwd_fac, sizeof(sd->passwd_fac));
> +
> +	/* Query supported <fac>s */
> +	if (!g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
> +					at_clck_query_cb, sim, NULL)) {
> +		g_idle_add(at_sim_register, sim);
> +	}

This is a bit weird.  If we fail here just return an error.  Why would 
we proceed trying to initialize the atom driver?

>   
>   	return 0;
>   }
> 

Regards,
-Denis

[PATCH v2] atmodem: Query the list of supported <fac>s from the modem

[Slava Monich]

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

Not all modems support all <fac>s (particularly, "PS"), let's be polite
and not ask them for the ones they don't support.
---
 drivers/atmodem/sim.c | 57 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 6395a04..06bfb90 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -51,6 +51,7 @@ struct sim_data {
 	GAtChat *chat;
 	unsigned int vendor;
 	guint ready_id;
+	guint passwd_type_mask;
 	struct at_util_sim_state_query *sim_state_query;
 };
 
@@ -1459,9 +1460,8 @@ static void at_pin_enable(struct ofono_sim *sim,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 	int ret;
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+	if (!(sd->passwd_type_mask & (1 << passwd_type)))
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
@@ -1490,10 +1490,8 @@ static void at_change_passwd(struct ofono_sim *sim,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 	int ret;
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len ||
-			at_clck_cpwd_fac[passwd_type] == NULL)
+	if (!(sd->passwd_type_mask & (1 << passwd_type)))
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
@@ -1550,9 +1548,8 @@ static void at_query_clck(struct ofono_sim *sim,
 	struct sim_data *sd = ofono_sim_get_data(sim);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+	if (!(sd->passwd_type_mask & (1 << passwd_type)))
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
@@ -1568,13 +1565,42 @@ error:
 	CALLBACK_WITH_FAILURE(cb, -1, data);
 }
 
-static gboolean at_sim_register(gpointer user)
+static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user)
 {
 	struct ofono_sim *sim = user;
+	struct sim_data *sd = ofono_sim_get_data(sim);
+	GAtResultIter iter;
+	const char *fac;
 
-	ofono_sim_register(sim);
+	if (!ok)
+		goto done;
+
+	g_at_result_iter_init(&iter, result);
+
+	/* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */
+	if (!g_at_result_iter_next(&iter, "+CLCK:") ||
+				!g_at_result_iter_open_list(&iter))
+		goto done;
+
+	/* Clear the default mask */
+	sd->passwd_type_mask = 0;
 
-	return FALSE;
+	/* Set the bits for <fac>s that are actually supported */
+	while (g_at_result_iter_next_string(&iter, &fac)) {
+		int i;
+
+		/* Find it in the list of known <fac>s */
+		for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
+			if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) {
+				sd->passwd_type_mask |= (1 << i);
+				DBG("found %s", fac);
+				break;
+			}
+		}
+	}
+
+done:
+	ofono_sim_register(sim);
 }
 
 static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
@@ -1582,6 +1608,7 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
 {
 	GAtChat *chat = data;
 	struct sim_data *sd;
+	int i;
 
 	sd = g_new0(struct sim_data, 1);
 	sd->chat = g_at_chat_clone(chat);
@@ -1591,9 +1618,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
 		g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL);
 
 	ofono_sim_set_data(sim, sd);
-	g_idle_add(at_sim_register, sim);
 
-	return 0;
+	/* <fac>s supported by default */
+	for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++)
+		if (at_clck_cpwd_fac[i])
+			sd->passwd_type_mask |= (1 << i);
+
+	/* Query supported <fac>s */
+	return g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
+				at_clck_query_cb, sim, NULL) ? 0 : -1;
 }
 
 static void at_sim_remove(struct ofono_sim *sim)
-- 
1.9.1

Re: [PATCH v2] atmodem: Query the list of supported <fac>s from the modem

[Denis Kenzior]

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

Hi Slava,

On 10/23/2017 03:21 PM, Slava Monich wrote:
> Not all modems support all <fac>s (particularly, "PS"), let's be polite
> and not ask them for the ones they don't support.
> ---
>   drivers/atmodem/sim.c | 57 ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 45 insertions(+), 12 deletions(-)
> 

drivers/atmodem/sim.c: In function ‘at_clck_query_cb’:
drivers/atmodem/sim.c:1593:17: error: comparison between signed and 
unsigned integer expressions [-Werror=sign-compare]
    for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
                  ^
drivers/atmodem/sim.c: In function ‘at_sim_probe’:
drivers/atmodem/sim.c:1623:16: error: comparison between signed and 
unsigned integer expressions [-Werror=sign-compare]
   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++)
                 ^
drivers/atmodem/sim.c: At top level:
cc1: error: unrecognized command line option ‘-Wno-format-truncation’ 
[-Werror]


Regards,
-Denis

Re: [PATCH v2] atmodem: Query the list of supported <fac>s from the modem

[Slava Monich]

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

On 23/10/17 23:33, Denis Kenzior wrote:
> Hi Slava,
>
> On 10/23/2017 03:21 PM, Slava Monich wrote:
>> Not all modems support all <fac>s (particularly, "PS"), let's be polite
>> and not ask them for the ones they don't support.
>> ---
>>   drivers/atmodem/sim.c | 57 
>> ++++++++++++++++++++++++++++++++++++++++-----------
>>   1 file changed, 45 insertions(+), 12 deletions(-)
>>
>
> drivers/atmodem/sim.c: In function ‘at_clck_query_cb’:
> drivers/atmodem/sim.c:1593:17: error: comparison between signed and 
> unsigned integer expressions [-Werror=sign-compare]
>    for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
>                  ^
> drivers/atmodem/sim.c: In function ‘at_sim_probe’:
> drivers/atmodem/sim.c:1623:16: error: comparison between signed and 
> unsigned integer expressions [-Werror=sign-compare]
>   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++)

Apparently we are using slightly different compilers. Will fix.

-Slava

[PATCH v3] atmodem: Query the list of supported <fac>s from the modem

[Slava Monich]

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

Not all modems support all <fac>s (particularly, "PS"), let's be polite
and not ask them for the ones they don't support.
---
 drivers/atmodem/sim.c | 57 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 6395a04..8665274 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -51,6 +51,7 @@ struct sim_data {
 	GAtChat *chat;
 	unsigned int vendor;
 	guint ready_id;
+	guint passwd_type_mask;
 	struct at_util_sim_state_query *sim_state_query;
 };
 
@@ -1459,9 +1460,8 @@ static void at_pin_enable(struct ofono_sim *sim,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 	int ret;
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+	if (!(sd->passwd_type_mask & (1 << passwd_type)))
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
@@ -1490,10 +1490,8 @@ static void at_change_passwd(struct ofono_sim *sim,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 	int ret;
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len ||
-			at_clck_cpwd_fac[passwd_type] == NULL)
+	if (!(sd->passwd_type_mask & (1 << passwd_type)))
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
@@ -1550,9 +1548,8 @@ static void at_query_clck(struct ofono_sim *sim,
 	struct sim_data *sd = ofono_sim_get_data(sim);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
-	unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-	if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+	if (!(sd->passwd_type_mask & (1 << passwd_type)))
 		goto error;
 
 	snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
@@ -1568,13 +1565,42 @@ error:
 	CALLBACK_WITH_FAILURE(cb, -1, data);
 }
 
-static gboolean at_sim_register(gpointer user)
+static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user)
 {
 	struct ofono_sim *sim = user;
+	struct sim_data *sd = ofono_sim_get_data(sim);
+	GAtResultIter iter;
+	const char *fac;
 
-	ofono_sim_register(sim);
+	if (!ok)
+		goto done;
+
+	g_at_result_iter_init(&iter, result);
+
+	/* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */
+	if (!g_at_result_iter_next(&iter, "+CLCK:") ||
+				!g_at_result_iter_open_list(&iter))
+		goto done;
+
+	/* Clear the default mask */
+	sd->passwd_type_mask = 0;
 
-	return FALSE;
+	/* Set the bits for <fac>s that are actually supported */
+	while (g_at_result_iter_next_string(&iter, &fac)) {
+		unsigned int i;
+
+		/* Find it in the list of known <fac>s */
+		for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
+			if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) {
+				sd->passwd_type_mask |= (1 << i);
+				DBG("found %s", fac);
+				break;
+			}
+		}
+	}
+
+done:
+	ofono_sim_register(sim);
 }
 
 static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
@@ -1582,6 +1608,7 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
 {
 	GAtChat *chat = data;
 	struct sim_data *sd;
+	unsigned int i;
 
 	sd = g_new0(struct sim_data, 1);
 	sd->chat = g_at_chat_clone(chat);
@@ -1591,9 +1618,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
 		g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL);
 
 	ofono_sim_set_data(sim, sd);
-	g_idle_add(at_sim_register, sim);
 
-	return 0;
+	/* <fac>s supported by default */
+	for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++)
+		if (at_clck_cpwd_fac[i])
+			sd->passwd_type_mask |= (1 << i);
+
+	/* Query supported <fac>s */
+	return g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
+				at_clck_query_cb, sim, NULL) ? 0 : -1;
 }
 
 static void at_sim_remove(struct ofono_sim *sim)
-- 
1.9.1

Re: [PATCH v3] atmodem: Query the list of supported <fac>s from the modem

[Denis Kenzior]

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

Hi Slava,

On 10/23/2017 03:52 PM, Slava Monich wrote:
> Not all modems support all <fac>s (particularly, "PS"), let's be polite
> and not ask them for the ones they don't support.
> ---
>   drivers/atmodem/sim.c | 57 ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 45 insertions(+), 12 deletions(-)
> 

Applied, thanks.

Regards,
-Denis