[PATCH] sim: showing lock state with call meter

[PATCH] sim: showing lock state with call meter

[Jussi Kangas]

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

---
Hi,

On Mon, 2011-02-21 at 17:21 +0200, Denis Kenzior wrote:

> It seems to be showing the states correctly except in case when you make
> > PIN2 blocked using call meter API. If PIN2 goes blocked when using
> > callmeter nothing is shown in API. (My patch did not do that either.
> > Plan was to fix it in separate patch)
> > 
> 
> Please do.  Most modems do not support PIN2 entry via CPIN, and
> implement this using a vendor extension.  PIN2 / PUK2 support seems to
> be a rather recent addition to the specs.
> 
Here it is. Also after this patch having PUK2 blocked does not prevent
going online anymore in start up and it does not hide the interfaces if
PIN2 goes blocked during usage.

> There seems to be also some problem when API shows PUK2 required and PIN
> > and PIN2 locked. Reset-pin opens the PUK2 but API does not get the
> > response it's looking for and oFono needs to be booted before states are
> > showed correctly again. I suspect that's a separate issue however and
> > not caused by this patch. 
> > 
> 
> I'm having trouble visualizing this one.  Can you provide an AT debug
> log to illustrate?
> 
I found the problem already. Looks like modem problem which has nothing to
do with having multiple locks on/blocked or with your change. Reseting
the PIN2 does not seem to trigger expected event in startup.

Br,
Jussi

 include/sim.h    |    2 ++
 src/call-meter.c |   27 +++++++++++++++++++++++++++
 src/sim.c        |    8 ++++----
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/include/sim.h b/include/sim.h
index 412ae44..a56056e 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -227,6 +227,8 @@ unsigned int ofono_sim_add_file_watch(struct ofono_sim_context *context,
 void ofono_sim_remove_file_watch(struct ofono_sim_context *context,
 					unsigned int id);
 
+void sim_pin_check(struct ofono_sim *sim);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/call-meter.c b/src/call-meter.c
index 0789935..61678d8 100644
--- a/src/call-meter.c
+++ b/src/call-meter.c
@@ -335,11 +335,20 @@ static void set_acm_max_query_callback(const struct ofono_error *error,
 static void set_acm_max_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_call_meter *cm = data;
+	struct ofono_atom *sim_atom;
+	struct ofono_sim *sim = NULL;
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		DBG("Setting acm_max failed");
 		__ofono_dbus_pending_reply(&cm->pending,
 					__ofono_error_failed(cm->pending));
+		sim_atom = __ofono_modem_find_atom(
+			__ofono_atom_get_modem(cm->atom),
+				OFONO_ATOM_TYPE_SIM);
+		if (!sim_atom)
+			return;
+		sim = __ofono_atom_get_data(sim_atom);
+		sim_pin_check(sim);
 		return;
 	}
 
@@ -396,11 +405,20 @@ static void set_puct_query_callback(const struct ofono_error *error,
 static void set_puct_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_call_meter *cm = data;
+	struct ofono_atom *sim_atom;
+	struct ofono_sim *sim = NULL;
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		DBG("setting puct failed");
 		__ofono_dbus_pending_reply(&cm->pending,
 					__ofono_error_failed(cm->pending));
+		sim_atom = __ofono_modem_find_atom(
+			__ofono_atom_get_modem(cm->atom),
+				OFONO_ATOM_TYPE_SIM);
+		if (!sim_atom)
+			return;
+		sim = __ofono_atom_get_data(sim_atom);
+		sim_pin_check(sim);
 		return;
 	}
 
@@ -593,11 +611,20 @@ static void reset_acm_query_callback(const struct ofono_error *error, int value,
 static void acm_reset_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_call_meter *cm = data;
+	struct ofono_atom *sim_atom;
+	struct ofono_sim *sim = NULL;
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		DBG("reseting acm failed");
 		__ofono_dbus_pending_reply(&cm->pending,
 					__ofono_error_failed(cm->pending));
+		sim_atom = __ofono_modem_find_atom(
+			__ofono_atom_get_modem(cm->atom),
+				OFONO_ATOM_TYPE_SIM);
+		if (!sim_atom)
+			return;
+		sim = __ofono_atom_get_data(sim_atom);
+		sim_pin_check(sim);
 		return;
 	}
 
diff --git a/src/sim.c b/src/sim.c
index c39269d..08236f2 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -49,7 +49,6 @@
 static GSList *g_drivers = NULL;
 
 static void sim_own_numbers_update(struct ofono_sim *sim);
-static void sim_pin_check(struct ofono_sim *sim);
 
 struct ofono_sim {
 	/* Contents of the SIM file system, in rough initialization order */
@@ -2231,8 +2230,9 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 						&pin_name);
 	}
 
-	if (pin_type != OFONO_SIM_PASSWORD_NONE &&
-			sim->state == OFONO_SIM_STATE_READY) {
+	if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
+			sim->state == OFONO_SIM_STATE_READY) &&
+				(pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {
 		/* Force the sim state out of READY */
 		sim_free_main_state(sim);
 
@@ -2247,7 +2247,7 @@ checkdone:
 		sim_initialize_after_pin(sim);
 }
 
-static void sim_pin_check(struct ofono_sim *sim)
+void sim_pin_check(struct ofono_sim *sim)
 {
 	if (sim->driver->query_passwd_state == NULL) {
 		sim_initialize_after_pin(sim);
-- 
1.7.1

Re: [PATCH] sim: showing lock state with call meter

[Denis Kenzior]

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

Hi Jussi,

>  include/sim.h    |    2 ++
>  src/call-meter.c |   27 +++++++++++++++++++++++++++
>  src/sim.c        |    8 ++++----
>  3 files changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sim.h b/include/sim.h
> index 412ae44..a56056e 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -227,6 +227,8 @@ unsigned int ofono_sim_add_file_watch(struct ofono_sim_context *context,
>  void ofono_sim_remove_file_watch(struct ofono_sim_context *context,
>  					unsigned int id);
>  
> +void sim_pin_check(struct ofono_sim *sim);
> +

Please name this __ofono_sim_recheck_pin(struct ofono_sim *sim);

>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/src/call-meter.c b/src/call-meter.c
> index 0789935..61678d8 100644
> --- a/src/call-meter.c
> +++ b/src/call-meter.c
> @@ -335,11 +335,20 @@ static void set_acm_max_query_callback(const struct ofono_error *error,
>  static void set_acm_max_callback(const struct ofono_error *error, void *data)
>  {
>  	struct ofono_call_meter *cm = data;
> +	struct ofono_atom *sim_atom;
> +	struct ofono_sim *sim = NULL;
>  
>  	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>  		DBG("Setting acm_max failed");
>  		__ofono_dbus_pending_reply(&cm->pending,
>  					__ofono_error_failed(cm->pending));
> +		sim_atom = __ofono_modem_find_atom(
> +			__ofono_atom_get_modem(cm->atom),
> +				OFONO_ATOM_TYPE_SIM);
> +		if (!sim_atom)

doc/coding-style.txt M1, and M13

> +			return;
> +		sim = __ofono_atom_get_data(sim_atom);

doc/coding-style.txt M1

> +		sim_pin_check(sim);
>  		return;
>  	}
>

And please factor out this code into a separate function, you repeat it
three times.

> @@ -396,11 +405,20 @@ static void set_puct_query_callback(const struct ofono_error *error,
>  static void set_puct_callback(const struct ofono_error *error, void *data)
>  {
>  	struct ofono_call_meter *cm = data;
> +	struct ofono_atom *sim_atom;
> +	struct ofono_sim *sim = NULL;
>  
>  	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>  		DBG("setting puct failed");
>  		__ofono_dbus_pending_reply(&cm->pending,
>  					__ofono_error_failed(cm->pending));
> +		sim_atom = __ofono_modem_find_atom(
> +			__ofono_atom_get_modem(cm->atom),
> +				OFONO_ATOM_TYPE_SIM);
> +		if (!sim_atom)
> +			return;
> +		sim = __ofono_atom_get_data(sim_atom);
> +		sim_pin_check(sim);
>  		return;
>  	}
>  
> @@ -593,11 +611,20 @@ static void reset_acm_query_callback(const struct ofono_error *error, int value,
>  static void acm_reset_callback(const struct ofono_error *error, void *data)
>  {
>  	struct ofono_call_meter *cm = data;
> +	struct ofono_atom *sim_atom;
> +	struct ofono_sim *sim = NULL;
>  
>  	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>  		DBG("reseting acm failed");
>  		__ofono_dbus_pending_reply(&cm->pending,
>  					__ofono_error_failed(cm->pending));
> +		sim_atom = __ofono_modem_find_atom(
> +			__ofono_atom_get_modem(cm->atom),
> +				OFONO_ATOM_TYPE_SIM);
> +		if (!sim_atom)
> +			return;
> +		sim = __ofono_atom_get_data(sim_atom);
> +		sim_pin_check(sim);
>  		return;
>  	}
>  
> diff --git a/src/sim.c b/src/sim.c
> index c39269d..08236f2 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -49,7 +49,6 @@
>  static GSList *g_drivers = NULL;
>  
>  static void sim_own_numbers_update(struct ofono_sim *sim);
> -static void sim_pin_check(struct ofono_sim *sim);
>  
>  struct ofono_sim {
>  	/* Contents of the SIM file system, in rough initialization order */
> @@ -2231,8 +2230,9 @@ static void sim_pin_query_cb(const struct ofono_error *error,
>  						&pin_name);
>  	}
>  
> -	if (pin_type != OFONO_SIM_PASSWORD_NONE &&
> -			sim->state == OFONO_SIM_STATE_READY) {
> +	if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
> +			sim->state == OFONO_SIM_STATE_READY) &&
> +				(pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {

I don't see how this can work.  You need to check for pin_type != NONE,
PIN2 and PUK2 AND state == READY here.  This also only covers the case
of the PIN2 or PUK2 being triggered when call-meter is active.  You do
not take care of the case where PIN2 or PUK2 are already required during
sim initialization.

>  		/* Force the sim state out of READY */
>  		sim_free_main_state(sim);
>  
> @@ -2247,7 +2247,7 @@ checkdone:
>  		sim_initialize_after_pin(sim);
>  }
>  
> -static void sim_pin_check(struct ofono_sim *sim)
> +void sim_pin_check(struct ofono_sim *sim)
>  {
>  	if (sim->driver->query_passwd_state == NULL) {
>  		sim_initialize_after_pin(sim);

Regards,
-Denis

Re: [PATCH] sim: showing lock state with call meter

[Jussi Kangas]

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

Hi,

On Wed, 2011-02-23 at 08:15 +0200, Denis Kenzior wrote:
> >  
> > -	if (pin_type != OFONO_SIM_PASSWORD_NONE &&
> > -			sim->state == OFONO_SIM_STATE_READY) {
> > +	if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
> > +			sim->state == OFONO_SIM_STATE_READY) &&
> > +				(pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {
> 
> I don't see how this can work.  You need to check for pin_type != NONE,
> PIN2 and PUK2 AND state == READY here.  This also only covers the case
> of the PIN2 or PUK2 being triggered when call-meter is active.  You do
> not take care of the case where PIN2 or PUK2 are already required during
> sim initialization.

Hmm. It seems to work here. If I try reset call meter with wrong
password I get pin_type OFONO_SIM_PASSWORD_SIM_PIN2 and
OFONO_SIM_STATE_READY. APIs stay up and SIM manager shows pin2 locked
and pin2 required. (Which is kinda wrong I admit, u cannot unlock the
pin2, pin2 should always be visible in LockedPins or not at all, I would
prefer the later. However it is showed as it is showed and it was
working that way already before this patch). If I run same case three
times I get same values but API shows that puk2 is required and all
API:s stay up. If I run the case with correct password it does not come
to this code at all all. 

PIN2 cannot be required in startup. It is only required if u do
something with certain features like fdn or call meter. PUK2 can be
blocked in startup all right but it does not matter from startup point
of view. If I start the oFono with PIN2 blocked I get pin_type
OFONO_SIM_PASSWORD_SIM_PIN2 and OFONO_SIM_STATE_INSERTED states and API
shows pin2 locked and puk2 required as required. (does not seem work if
I boot up the modem and not just oFono though. Looks like modem bug
where it does not read voluntarily PIN2 state.)

Br,
-Jussi 

Re: [PATCH] sim: showing lock state with call meter

[Denis Kenzior]

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

Hi Jussi,

On 02/23/2011 09:24 AM, Jussi Kangas wrote:
> Hi,
> 
> On Wed, 2011-02-23 at 08:15 +0200, Denis Kenzior wrote:
>>>  
>>> -	if (pin_type != OFONO_SIM_PASSWORD_NONE &&
>>> -			sim->state == OFONO_SIM_STATE_READY) {
>>> +	if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
>>> +			sim->state == OFONO_SIM_STATE_READY) &&
>>> +				(pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {
>>
>> I don't see how this can work.  You need to check for pin_type != NONE,
>> PIN2 and PUK2 AND state == READY here.  This also only covers the case
>> of the PIN2 or PUK2 being triggered when call-meter is active.  You do
>> not take care of the case where PIN2 or PUK2 are already required during
>> sim initialization.
> 
> Hmm. It seems to work here. If I try reset call meter with wrong
> password I get pin_type OFONO_SIM_PASSWORD_SIM_PIN2 and
> OFONO_SIM_STATE_READY. APIs stay up and SIM manager shows pin2 locked
> and pin2 required. (Which is kinda wrong I admit, u cannot unlock the
> pin2, pin2 should always be visible in LockedPins or not at all, I would
> prefer the later. However it is showed as it is showed and it was
> working that way already before this patch). If I run same case three
> times I get same values but API shows that puk2 is required and all
> API:s stay up. If I run the case with correct password it does not come
> to this code at all all. 

What I'm saying here is that it would be safer to do it like this:

switch (pin_type) {
case OFONO_SIM_PASSWORD_NONE:
case OFONO_SIM_PASSWORD_SIM_PIN2:
case OFONO_SIM_PASSWORD_SIM_PUK2:
	break;
default:
	if (sim->state == OFONO_SIM_STATE_READY)
		...
}

We have to check for the PUK2 case above since we might be in PUK2
required state already.  If we fail to unlock the PUK2, then
	pin_type = puk2pin(pin_type);
will never be executed.

And I'm fine with your suggestion of blacklisting PIN2 from LockedPins,
but lets handle this separately.

> 
> PIN2 cannot be required in startup. It is only required if u do
> something with certain features like fdn or call meter. PUK2 can be
> blocked in startup all right but it does not matter from startup point
> of view. If I start the oFono with PIN2 blocked I get pin_type
> OFONO_SIM_PASSWORD_SIM_PIN2 and OFONO_SIM_STATE_INSERTED states and API
> shows pin2 locked and puk2 required as required. (does not seem work if
> I boot up the modem and not just oFono though. Looks like modem bug
> where it does not read voluntarily PIN2 state.)
> 

Again, my point here is that you have this code at the bottom of the
function:

        if (pin_type == OFONO_SIM_PASSWORD_NONE)
                sim_initialize_after_pin(sim);

If you want the SIM initialization to proceed then you need to add a
check for SIM PIN2/PUK2.

Regards,
-Denis

Re: [PATCH] sim: showing lock state with call meter

[Jussi Kangas]

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

Hi,

On Thu, 2011-02-24 at 22:17 +0200, Denis Kenzior wrote: 

> We have to check for the PUK2 case above since we might be in PUK2
> required state already.  If we fail to unlock the PUK2, then
>       pin_type = puk2pin(pin_type);
> will never be executed.
> 
> And I'm fine with your suggestion of blacklisting PIN2 from
LockedPins,
> but lets handle this separately.

Yes let's handle that as separate issue if at all. What I was trying to 
say was that there was no situation in my testings where PUK2 would be 
required in this part of code. If PUK2 is required pin_type is still
PIN2 here. Situation you describe has to happen with other modems.

> Again, my point here is that you have this code at the bottom of the
> function:
> 
>         if (pin_type == OFONO_SIM_PASSWORD_NONE)
>                 sim_initialize_after_pin(sim);
> 
> If you want the SIM initialization to proceed then you need to add a
> check for SIM PIN2/PUK2.

You are writing about adding PIN2 and PUK2 to this check? I rechecked 
my AT command API specification and PIN2 and PUK2 cannot be required in
startup. (Speculation about bug in my previous mail was wrong) And if
PIN2 or PUK2 is required in other time there is no need for
sim_initialization. You must be writing about situation where oFono is
run workstation environment and restarted without booting the modem? Ok,
I'll change that but I think I'll add a check to prevent sim
initialization in sim state READY situation. 

What is your general opinion about making amends in oFono code to enable
nice working in workstation? My previous hotswap patch does not work if
you disable modem without restarting oFono. It lacks initialization of
have_sim parameter in init_sim_reporting method. 

Br,
-Jussi

[PATCH] sim: showing lock state with call meter

[Jussi Kangas]

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

---

Hi,

Here is the fixed patch for enabling the showing of pin2 blocked 
situation in SIM API when lock state changes after using call meter
API.

Br,
Jussi 

 include/sim.h    |    2 ++
 src/call-meter.c |   19 +++++++++++++++++++
 src/sim.c        |   46 +++++++++++++++++++++++++++++++---------------
 3 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/include/sim.h b/include/sim.h
index 412ae44..ab3a57f 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -227,6 +227,8 @@ unsigned int ofono_sim_add_file_watch(struct ofono_sim_context *context,
 void ofono_sim_remove_file_watch(struct ofono_sim_context *context,
 					unsigned int id);
 
+void __ofono_sim_recheck_pin(struct ofono_sim *sim);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/call-meter.c b/src/call-meter.c
index 0789935..c6a85aa 100644
--- a/src/call-meter.c
+++ b/src/call-meter.c
@@ -332,6 +332,22 @@ static void set_acm_max_query_callback(const struct ofono_error *error,
 	set_acm_max(cm, value);
 }
 
+static void check_pin2_state(struct ofono_call_meter *cm)
+{
+	struct ofono_atom *sim_atom;
+	struct ofono_sim *sim = NULL;
+
+	sim_atom = __ofono_modem_find_atom(
+		__ofono_atom_get_modem(cm->atom),
+			OFONO_ATOM_TYPE_SIM);
+
+	if (sim_atom == NULL)
+		return;
+
+	sim = __ofono_atom_get_data(sim_atom);
+	__ofono_sim_recheck_pin(sim);
+}
+
 static void set_acm_max_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_call_meter *cm = data;
@@ -340,6 +356,7 @@ static void set_acm_max_callback(const struct ofono_error *error, void *data)
 		DBG("Setting acm_max failed");
 		__ofono_dbus_pending_reply(&cm->pending,
 					__ofono_error_failed(cm->pending));
+		check_pin2_state(cm);
 		return;
 	}
 
@@ -401,6 +418,7 @@ static void set_puct_callback(const struct ofono_error *error, void *data)
 		DBG("setting puct failed");
 		__ofono_dbus_pending_reply(&cm->pending,
 					__ofono_error_failed(cm->pending));
+		check_pin2_state(cm);
 		return;
 	}
 
@@ -598,6 +616,7 @@ static void acm_reset_callback(const struct ofono_error *error, void *data)
 		DBG("reseting acm failed");
 		__ofono_dbus_pending_reply(&cm->pending,
 					__ofono_error_failed(cm->pending));
+		check_pin2_state(cm);
 		return;
 	}
 
diff --git a/src/sim.c b/src/sim.c
index c39269d..8eafa1f 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -49,7 +49,6 @@
 static GSList *g_drivers = NULL;
 
 static void sim_own_numbers_update(struct ofono_sim *sim);
-static void sim_pin_check(struct ofono_sim *sim);
 
 struct ofono_sim {
 	/* Contents of the SIM file system, in rough initialization order */
@@ -624,7 +623,7 @@ static void sim_unlock_cb(const struct ofono_error *error, void *data)
 		DBusMessage *reply = __ofono_error_failed(sim->pending);
 
 		__ofono_dbus_pending_reply(&sim->pending, reply);
-		sim_pin_check(sim);
+		__ofono_sim_recheck_pin(sim);
 
 		return;
 	}
@@ -640,7 +639,7 @@ static void sim_lock_cb(const struct ofono_error *error, void *data)
 		DBusMessage *reply = __ofono_error_failed(sim->pending);
 
 		__ofono_dbus_pending_reply(&sim->pending, reply);
-		sim_pin_check(sim);
+		__ofono_sim_recheck_pin(sim);
 
 		return;
 	}
@@ -711,7 +710,7 @@ static void sim_change_pin_cb(const struct ofono_error *error, void *data)
 		__ofono_dbus_pending_reply(&sim->pending,
 				__ofono_error_failed(sim->pending));
 
-		sim_pin_check(sim);
+		__ofono_sim_recheck_pin(sim);
 
 		return;
 	}
@@ -776,7 +775,7 @@ static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 
 	__ofono_dbus_pending_reply(&sim->pending, reply);
 
-	sim_pin_check(sim);
+	__ofono_sim_recheck_pin(sim);
 }
 
 static DBusMessage *sim_enter_pin(DBusConnection *conn, DBusMessage *msg,
@@ -1846,7 +1845,7 @@ skip_efpl:
 						DBUS_TYPE_STRING,
 						&sim->language_prefs);
 
-	sim_pin_check(sim);
+	__ofono_sim_recheck_pin(sim);
 }
 
 static void sim_iccid_read_cb(int ok, int length, int record,
@@ -2231,23 +2230,40 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 						&pin_name);
 	}
 
-	if (pin_type != OFONO_SIM_PASSWORD_NONE &&
-			sim->state == OFONO_SIM_STATE_READY) {
-		/* Force the sim state out of READY */
-		sim_free_main_state(sim);
+	switch (pin_type) {
+	case OFONO_SIM_PASSWORD_NONE:
+	case OFONO_SIM_PASSWORD_SIM_PIN2:
+	case OFONO_SIM_PASSWORD_SIM_PUK2:
+		break;
+	default:
+		if (sim->state == OFONO_SIM_STATE_READY) {
+			/* Force the sim state out of READY */
+			sim_free_main_state(sim);
 
-		sim->state = OFONO_SIM_STATE_INSERTED;
-		__ofono_modem_sim_reset(__ofono_atom_get_modem(sim->atom));
+			sim->state = OFONO_SIM_STATE_INSERTED;
+			__ofono_modem_sim_reset(
+				__ofono_atom_get_modem(sim->atom));
+		}
+		break;
 	}
 
 	sim_pin_retries_check(sim);
 
 checkdone:
-	if (pin_type == OFONO_SIM_PASSWORD_NONE)
+	switch (pin_type) {
+	case OFONO_SIM_PASSWORD_SIM_PIN2:
+	case OFONO_SIM_PASSWORD_SIM_PUK2:
+		if (sim->state == OFONO_SIM_STATE_READY)
+			break;
+	case OFONO_SIM_PASSWORD_NONE:
 		sim_initialize_after_pin(sim);
+		break;
+	default:
+		break;
+	}
 }
 
-static void sim_pin_check(struct ofono_sim *sim)
+void __ofono_sim_recheck_pin(struct ofono_sim *sim)
 {
 	if (sim->driver->query_passwd_state == NULL) {
 		sim_initialize_after_pin(sim);
@@ -2579,6 +2595,6 @@ void __ofono_sim_refresh(struct ofono_sim *sim, GSList *file_list,
 		 * Start initialization procedure from after EFiccid,
 		 * EFli and EFpl are retrieved.
 		 */
-		sim_pin_check(sim);
+		__ofono_sim_recheck_pin(sim);
 	}
 }
-- 
1.7.1

Re: [PATCH] sim: showing lock state with call meter

[Denis Kenzior]

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

Hi Jussi,

On 02/25/2011 07:19 AM, Jussi Kangas wrote:
> Hi,
> 
> On Thu, 2011-02-24 at 22:17 +0200, Denis Kenzior wrote: 
> 
>> We have to check for the PUK2 case above since we might be in PUK2
>> required state already.  If we fail to unlock the PUK2, then
>>       pin_type = puk2pin(pin_type);
>> will never be executed.
>>
>> And I'm fine with your suggestion of blacklisting PIN2 from
> LockedPins,
>> but lets handle this separately.
> 
> Yes let's handle that as separate issue if at all. What I was trying to 
> say was that there was no situation in my testings where PUK2 would be 
> required in this part of code. If PUK2 is required pin_type is still
> PIN2 here. Situation you describe has to happen with other modems.
>

Looking at the code there might be possibilities when pin_type is in
fact PUK2.  The puk2pin code is executed only if pin_type does not match
sim->pin_type.  So if you're already in PUK2 state, and fail to unlock
again, you need to check against PUK2, not PIN2.

>> Again, my point here is that you have this code at the bottom of the
>> function:
>>
>>         if (pin_type == OFONO_SIM_PASSWORD_NONE)
>>                 sim_initialize_after_pin(sim);
>>
>> If you want the SIM initialization to proceed then you need to add a
>> check for SIM PIN2/PUK2.
> 
> You are writing about adding PIN2 and PUK2 to this check? I rechecked 
> my AT command API specification and PIN2 and PUK2 cannot be required in
> startup. (Speculation about bug in my previous mail was wrong) And if
> PIN2 or PUK2 is required in other time there is no need for
> sim_initialization. You must be writing about situation where oFono is
> run workstation environment and restarted without booting the modem? Ok,
> I'll change that but I think I'll add a check to prevent sim
> initialization in sim state READY situation. 
> 
> What is your general opinion about making amends in oFono code to enable
> nice working in workstation? My previous hotswap patch does not work if
> you disable modem without restarting oFono. It lacks initialization of
> have_sim parameter in init_sim_reporting method. 
> 

We need to make every reasonable effort to make oFono work in these
cases.  We can't assume that the driver can reset the modem properly
when powering up.  And in fact most USB data sticks on the market do not
support modem resets short of pulling the hardware.  If the modem
firmware is buggy, then there's obviously nothing we can do.

Regards,
-Denis

Re: [PATCH] sim: showing lock state with call meter

[Denis Kenzior]

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

Hi Jussi,

On 02/25/2011 07:20 AM, Jussi Kangas wrote:
> ---
> 
> Hi,
> 
> Here is the fixed patch for enabling the showing of pin2 blocked 
> situation in SIM API when lock state changes after using call meter
> API.
> 
> Br,
> Jussi 
> 
>  include/sim.h    |    2 ++
>  src/call-meter.c |   19 +++++++++++++++++++
>  src/sim.c        |   46 +++++++++++++++++++++++++++++++---------------
>  3 files changed, 52 insertions(+), 15 deletions(-)
> 

I applied this patch, but broke it up into three separate commits:

9007bf63927b7008129a7704a8fd0649fed67065
c5b321e768a9f36ba55dc9ce39d8f28c527a57eb
07c7308581c921ad1ff70903c8534ac01e62b549

Few more comments below:

>  
>  checkdone:
> -	if (pin_type == OFONO_SIM_PASSWORD_NONE)
> +	switch (pin_type) {
> +	case OFONO_SIM_PASSWORD_SIM_PIN2:
> +	case OFONO_SIM_PASSWORD_SIM_PUK2:
> +		if (sim->state == OFONO_SIM_STATE_READY)
> +			break;
> +	case OFONO_SIM_PASSWORD_NONE:
>  		sim_initialize_after_pin(sim);
> +		break;

If we correctly enter PIN2 or unblock PIN2, do we go back to the READY
state?  If so, we might want to avoid running the initialization
procedure above.

> +	default:
> +		break;
> +	}
>  }

Regards,
-Denis