Re: [PATCH 4/5] xmm7modem: handling of dual sim single active feature

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Antara,

On 03/28/2019 07:06 AM, Antara Borwankar wrote:
> Handled DSSA use case for xmm7modem. Added driver function
> to switch between available card slots for SIM.
> 
> Only one SIM will be active at a given time. On calling this
> function the active SIM will be removed and the card slot made
> inactive and SIM in the other card slot will be made active.
> 
> Use case is similar to a SIM swap/change where one SIM is replaced
> another SIM in the same slot.
> ---
>   drivers/atmodem/sim.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 114 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
> index 5f66a09..5fdfe30 100644
> --- a/drivers/atmodem/sim.c
> +++ b/drivers/atmodem/sim.c
> @@ -75,6 +75,7 @@ static const char *cuad_prefix[] = { "+CUAD:", NULL };
>   static const char *ccho_prefix[] = { "+CCHO:", NULL };
>   static const char *crla_prefix[] = { "+CRLA:", NULL };
>   static const char *cgla_prefix[] = { "+CGLA:", NULL };
> +static const char *xcmscsc_prefix[] = { "+XCMSCSC:", NULL};
>   static const char *none_prefix[] = { NULL };
>   
>   static void append_file_path(char *buf, const unsigned char *path,
> @@ -1160,6 +1161,7 @@ static void at_pin_retries_query(struct ofono_sim *sim,
>   	DBG("");
>   
>   	switch (sd->vendor) {
> +	case OFONO_VENDOR_XMM:
>   	case OFONO_VENDOR_IFX:
>   		if (g_at_chat_send(sd->chat, "AT+XPINCNT", xpincnt_prefix,
>   					xpincnt_cb, cbd, g_free) > 0)
> @@ -1920,6 +1922,76 @@ static void at_logical_access(struct ofono_sim *sim, int session_id,
>   	CALLBACK_WITH_FAILURE(cb, NULL, 0, data);
>   }
>   
> +static void xcmscsc_query_cb(gboolean ok, GAtResult *result, gpointer user)
> +{
> +	struct ofono_sim *sim = user;
> +	struct sim_data *sd = ofono_sim_get_data(sim);
> +	GAtResultIter iter;
> +	int active_slot;
> +
> +	if (!ok)
> +		goto done;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "+XCMSCSC:"))
> +		goto done;
> +
> +	g_at_result_iter_skip_next(&iter);
> +	g_at_result_iter_skip_next(&iter);
> +
> +	g_at_result_iter_next_number(&iter, &active_slot);
> +
> +	/* set active SIM slot */
> +	ofono_sim_set_active_card_slot(sim, active_slot+1);
> +
> +done:
> +	/* Query supported <fac>s */
> +	g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
> +				at_clck_query_cb, sim, NULL);
> +}
> +
> +static void at_xcmscsc_test_cb(gboolean ok, GAtResult *result, gpointer user)
> +{
> +	struct ofono_sim *sim = user;
> +	struct sim_data *sd = ofono_sim_get_data(sim);
> +	GAtResultIter iter;
> +	int card_slot_count;
> +
> +	if (!ok)
> +		goto done;

It seems pointless to try and query +XCMSCSC if this command failed...

> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "+XCMSCSC:"))
> +		goto done;
> +
> +	g_at_result_iter_skip_next(&iter);
> +	g_at_result_iter_skip_next(&iter);
> +
> +	if (!g_at_result_iter_open_list(&iter))
> +		goto done;
> +
> +	g_at_result_iter_skip_next(&iter);
> +
> +	if (!g_at_result_iter_next_number(&iter, &card_slot_count))
> +		goto done;
> +
> +	/* Set num slots */
> +	ofono_sim_set_card_slot_count(sim, card_slot_count+1);
> +
> +done:
> +	/* enable reporting of MSIM remap status information
> +	   and enable automatic acceptance of MSIM Remap
> +	   acknowledgement. */
> +	g_at_chat_send(sd->chat, "AT+XCMSRS=2", none_prefix,
> +					NULL, NULL, NULL);
> +
> +	/* Query active card slot */
> +	g_at_chat_send(sd->chat, "AT+XCMSCSC?", xcmscsc_prefix,
> +				xcmscsc_query_cb, sim, NULL);

e.g. these are probably only relevant on the success path.  On failure 
the logical thing is to skip directly to +CLCK processing

> +}
> +
>   static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
>   				void *data)
>   {
> @@ -1938,6 +2010,14 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
>   		if (at_clck_cpwd_fac[i])
>   			sd->passwd_type_mask |= (1 << i);
>   
> +	/* set default values */
> +	ofono_sim_set_card_slot_count(sim, 1);
> +	ofono_sim_set_active_card_slot(sim, 1);
> +

This needs to be taken care of the core.  Otherwise the drivers you're 
not touching are going to cause weirdness.

> +	if (sd->vendor == OFONO_VENDOR_XMM)
> +		return g_at_chat_send(sd->chat, "AT+XCMSCSC=?", xcmscsc_prefix,
> +			at_xcmscsc_test_cb, sim, NULL) ? 0 : -1;
> +
>   	/* Query supported <fac>s */
>   	return g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
>   				at_clck_query_cb, sim, NULL) ? 0 : -1;
> @@ -1957,6 +2037,38 @@ static void at_sim_remove(struct ofono_sim *sim)
>   	g_free(sd);
>   }
>   
> +static void xcmscsc_cb(gboolean ok, GAtResult *result,
> +		gpointer user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_sim_set_active_card_slot_cb_t cb = cbd->cb;
> +	struct ofono_error error;
> +
> +	decode_at_error(&error, g_at_result_final_response(result));
> +
> +	if (cb)
> +		cb(&error, cbd->data);
> +}
> +
> +static void at_set_active_card_slot(struct ofono_sim *sim, unsigned int index,
> +			ofono_sim_set_active_card_slot_cb_t cb, void *data)
> +{
> +	struct sim_data *sd = ofono_sim_get_data(sim);
> +	struct cb_data *cbd = cb_data_new(cb, data);
> +	char cmd[43];
> +

You still need to take care of returning a not implemented error in case 
the vendor != VENDOR_XMM.  e.g. perhaps a CME error 4 or equivalent.

> +	/* Enable single SIM mode for indicated card slot id */
> +	snprintf(cmd, sizeof(cmd), "AT+XCMSCSC=1,0,%u,1", index);
> +
> +	if (g_at_chat_send(sd->chat, cmd, none_prefix, xcmscsc_cb,
> +			cbd, g_free) > 0)
> +		return;
> +
> +	g_free(cbd);
> +
> +	CALLBACK_WITH_FAILURE(cb, data);
> +}
> +
>   static const struct ofono_sim_driver driver = {
>   	.name			= "atmodem",
>   	.probe			= at_sim_probe,
> @@ -1982,7 +2094,8 @@ static const struct ofono_sim_driver driver = {
>   	.session_read_binary	= at_session_read_binary,
>   	.session_read_record	= at_session_read_record,
>   	.session_read_info	= at_session_read_info,
> -	.logical_access		= at_logical_access
> +	.logical_access		= at_logical_access,
> +	.set_active_card_slot	= at_set_active_card_slot
>   };
>   
>   static const struct ofono_sim_driver driver_noef = {
> 

Regards,
-Denis