Re: [PATCH_v0 2/3] sim: Don't use sim file system for CDMA sim

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Guillaume,

On 11/16/2011 08:49 AM, Guillaume Zajac wrote:
> ---
>  src/sim.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 57 insertions(+), 9 deletions(-)
> 
> diff --git a/src/sim.c b/src/sim.c
> index d1d55c5..9ba3483 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -102,6 +102,8 @@ struct ofono_sim {
>  	unsigned char *iidf_image;
>  	unsigned int *iidf_watch_ids;
>  
> +	gboolean cdma_sim;
> +
>  	DBusMessage *pending;
>  	const struct ofono_sim_driver *driver;
>  	void *driver_data;
> @@ -1380,7 +1382,8 @@ static void sim_set_ready(struct ofono_sim *sim)
>  
>  	sim->state = OFONO_SIM_STATE_READY;
>  
> -	sim_fs_check_version(sim->simfs);
> +	if (sim->simfs)
> +		sim_fs_check_version(sim->simfs);
>  
>  	call_state_watches(sim);
>  }
> @@ -2527,6 +2530,47 @@ struct ofono_sim *ofono_sim_create(struct ofono_modem *modem,
>  	return sim;
>  }
>  
> +struct ofono_sim *ofono_cdma_sim_create(struct ofono_modem *modem,
> +					unsigned int vendor,
> +					const char *driver,
> +					void *data)
> +{
> +	struct ofono_sim *sim;
> +	GSList *l;
> +	int i;
> +
> +	if (driver == NULL)
> +		return NULL;
> +
> +	sim = g_try_new0(struct ofono_sim, 1);
> +
> +	if (sim == NULL)
> +		return NULL;
> +
> +	sim->cdma_sim = TRUE;
> +	sim->phase = OFONO_SIM_PHASE_UNKNOWN;
> +	sim->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_SIM,
> +						sim_remove, sim);
> +
> +	for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
> +		sim->pin_retries[i] = -1;
> +
> +	for (l = g_drivers; l; l = l->next) {
> +		const struct ofono_sim_driver *drv = l->data;
> +
> +		if (g_strcmp0(drv->name, driver))
> +			continue;
> +
> +		if (drv->probe(sim, vendor, data) < 0)
> +			continue;
> +
> +		sim->driver = drv;
> +		break;
> +	}
> +
> +	return sim;
> +}
> +
>  static void emulator_cnum_cb(struct ofono_emulator *em,
>  			struct ofono_emulator_request *req, void *userdata)
>  {
> @@ -2591,19 +2635,23 @@ void ofono_sim_register(struct ofono_sim *sim)
>  	}
>  
>  	ofono_modem_add_interface(modem, OFONO_SIM_MANAGER_INTERFACE);
> -	sim->state_watches = __ofono_watchlist_new(g_free);
> -	sim->simfs = sim_fs_new(sim, sim->driver);
>  
> +	sim->state_watches = __ofono_watchlist_new(g_free);
>  	__ofono_atom_register(sim->atom, sim_unregister);
>  
> -	ofono_sim_add_state_watch(sim, sim_ready, sim, NULL);
> +	if (sim->cdma_sim == FALSE) {
> +		sim->simfs = sim_fs_new(sim, sim->driver);
>  
> -	if (sim->state > OFONO_SIM_STATE_NOT_PRESENT)
> -		sim_initialize(sim);
> +		ofono_sim_add_state_watch(sim, sim_ready, sim, NULL);
> +
> +		if (sim->state > OFONO_SIM_STATE_NOT_PRESENT)
> +			sim_initialize(sim);
>  
> -	sim->hfp_watch = __ofono_modem_add_atom_watch(modem,
> -					OFONO_ATOM_TYPE_EMULATOR_HFP,
> -					emulator_hfp_watch, sim, NULL);
> +		sim->hfp_watch = __ofono_modem_add_atom_watch(modem,
> +						OFONO_ATOM_TYPE_EMULATOR_HFP,
> +						emulator_hfp_watch, sim, NULL);
> +	} else
> +		sim_retrieve_imsi(sim);

I am really unhappy with this approach.  The public API is pretty much
dead on arrival long term, and doesn't even handle immediate issues
(e.g. what if the PIN is locked?).

Perhaps implementing a huawei cdma specific sim driver with only the
IMSI operation implemented might be a better 'hack' at this point.
However, ideally the core should be able to figure out the SIM type from
simply reading the relevant EFs.

>  }
>  
>  void ofono_sim_remove(struct ofono_sim *sim)

Regards,
-Denis