From: Denis Kenzior <denkenz@gmail.com>
To: ofono@ofono.org
Subject: Re: [PATCH] sim: Read EFust and EFest
Date: Thu, 22 Jul 2010 11:53:55 -0500 [thread overview]
Message-ID: <4C4877A3.8000600@gmail.com> (raw)
In-Reply-To: <1279710452-32096-1-git-send-email-yang.gu@intel.com>
[-- Attachment #1: Type: text/plain, Size: 8822 bytes --]
Hi Yang,
On 07/21/2010 06:07 AM, Yang Gu wrote:
> ---
> src/sim.c | 77 +++++++++++++++++++++++++++++++++++++++++++-
> src/simutil.h | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 173 insertions(+), 2 deletions(-)
>
> diff --git a/src/sim.c b/src/sim.c
> index 2514e7b..2e304dd 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -95,6 +95,8 @@ struct ofono_sim {
> void *driver_data;
> struct ofono_atom *atom;
> DBusMessage *pending;
> + gboolean service_alloc[88];
> + gboolean service_actv[3];
I really think this is overkill. A gboolean takes up 4 bytes of space,
so this gets a bit expensive fast. Can't you just store the bitmap and
provide a simple utility function to get the enabled flag?
> };
>
> struct msisdn_set_request {
> @@ -1059,6 +1061,77 @@ static void sim_retrieve_imsi(struct ofono_sim *sim)
> sim->driver->read_imsi(sim, sim_imsi_cb, sim);
> }
>
> +static void sim_efest_read_cb(int ok, int length, int record,
> + const unsigned char *data,
> + int record_length, void *userdata)
> +{
> + struct ofono_sim *sim = userdata;
> + int i;
> + int size;
> +
> + if (!ok)
> + return;
> +
> + if (length < 1) {
> + ofono_error("EFest shall contain at least one byte");
> + return;
> + }
> +
> + size = sizeof(sim->service_actv) / sizeof(*sim->service_actv);
> + for (i = 0; i < size; i++) {
> + if (i / 8 >= length)
> + sim->service_actv[i] = 0;
> + else
> + sim->service_actv[i] = (data[i / 8] >> (i % 8)) & 1;
> + }
> +}
> +
> +static void sim_retrieve_efest(struct ofono_sim *sim)
> +{
> + ofono_sim_read(sim, SIM_EFEST_FILEID,
> + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> + sim_efest_read_cb, sim);
> +}
What is the purpose of the above function? Using it inline might be better.
> +
> +static void sim_efust_read_cb(int ok, int length, int record,
> + const unsigned char *data,
> + int record_length, void *userdata)
> +{
> + struct ofono_sim *sim = userdata;
> + int i;
> + int size;
> +
> + if (!ok)
> + return;
> +
> + if (length < 1) {
> + ofono_error("EFust shall contain at least one byte");
> + return;
> + }
> +
> + size = sizeof(sim->service_alloc) / sizeof(*sim->service_alloc);
> + for (i = 0; i < size; i++) {
> + if (i / 8 >= length)
> + sim->service_alloc[i] = 0;
> + else
> + sim->service_alloc[i] = (data[i / 8] >> (i % 8)) & 1;
> + }
> +}
> +
> +static void sim_retrieve_efust(struct ofono_sim *sim)
> +{
> + ofono_sim_read(sim, SIM_EFUST_FILEID,
> + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> + sim_efust_read_cb, sim);
> +}
> +
Again, what is the purpose here? Moving this down into
sim_service_request might be a good idea.
> +static void sim_service_request(struct ofono_sim *sim)
> +{
> + sim_retrieve_efust(sim);
> + sim_retrieve_efest(sim);
> + sim_retrieve_imsi(sim);
You have to be careful here, the sim_op queue is running in parallel
with sim_retrieve_imsi. So if your intention is to delay retrieval of
the IMSI until EFust and EFest are read, then you need to retrieve the
IMSI from the EFest callback.
> +}
> +
> static void sim_pin_query_cb(const struct ofono_error *error,
> enum ofono_sim_password_type pin_type,
> void *data)
> @@ -1088,13 +1161,13 @@ static void sim_pin_query_cb(const struct ofono_error *error,
>
> checkdone:
> if (pin_type == OFONO_SIM_PASSWORD_NONE)
> - sim_retrieve_imsi(sim);
> + sim_service_request(sim);
> }
>
> static void sim_pin_check(struct ofono_sim *sim)
> {
> if (!sim->driver->query_passwd_state) {
> - sim_retrieve_imsi(sim);
> + sim_service_request(sim);
> return;
> }
>
> diff --git a/src/simutil.h b/src/simutil.h
> index 29194ca..76ac433 100644
> --- a/src/simutil.h
> +++ b/src/simutil.h
> @@ -26,9 +26,11 @@ enum sim_fileid {
> SIM_EF_CPHS_MWIS_FILEID = 0x6f11,
> SIM_EF_CPHS_INFORMATION_FILEID = 0x6f16,
> SIM_EF_CPHS_MBDN_FILEID = 0x6f17,
> + SIM_EFUST_FILEID = 0x6f38,
> SIM_EFMSISDN_FILEID = 0x6f40,
> SIM_EFSPN_FILEID = 0x6f46,
> SIM_EFSDN_FILEID = 0x6f49,
> + SIM_EFEST_FILEID = 0x6f56,
> SIM_EFAD_FILEID = 0x6fad,
> SIM_EFPHASE_FILEID = 0x6fae,
> SIM_EFPNN_FILEID = 0x6fc5,
> @@ -53,6 +55,102 @@ enum sim_file_access {
> SIM_FILE_ACCESS_NEVER = 15,
> };
>
> +/* 131.102 Section 4.2.8 */
> +enum usim_alloc_service {
> + USIM_ALLOC_SERVICE_LOCAL_PHONE_BOOK = 0,
> + USIM_ALLOC_SERVICE_FDN = 1,
> + USIM_ALLOC_SERVICE_EXT_2 = 2,
> + USIM_ALLOC_SERVICE_SDN = 3,
> + USIM_ALLOC_SERVICE_EXT_3 = 4,
> + USIM_ALLOC_SERVICE_BDN = 5,
> + USIM_ALLOC_SERVICE_EXT_4 = 6,
> + USIM_ALLOC_SERVICE_OCI_OCT = 7,
> + USIM_ALLOC_SERVICE_ICI_ICT = 8,
> + USIM_ALLOC_SERVICE_SMS = 9,
> + USIM_ALLOC_SERVICE_SMSR = 10,
> + USIM_ALLOC_SERVICE_SMSP = 11,
> + USIM_ALLOC_SERVICE_AOC = 12,
> + USIM_ALLOC_SERVICE_CCP2 = 13,
> + USIM_ALLOC_SERVICE_CBS_ID = 14,
> + USIM_ALLOC_SERVICE_CBS_ID_RANGE = 15,
> + USIM_ALLOC_SERVICE_GROUP_ID_LEVEL_1 = 16,
> + USIM_ALLOC_SERVICE_GROUP_ID_LEVEL_2 = 17,
> + USIM_ALLOC_SERVICE_PROVIDER_NAME = 18,
> + USIM_ALLOC_SERVICE_USER_PLMN = 19,
> + USIM_ALLOC_SERVICE_MSISDN = 20,
> + USIM_ALLOC_SERVICE_IMG = 21,
> + USIM_ALLOC_SERVICE_SOLSA = 22,
> + USIM_ALLOC_SERVICE_PRECEDENCE_PREEMPTION = 23,
> + USIM_ALLOC_SERVICE_EMLPP = 24,
> + USIM_ALLOC_SERVICE_GSM_ACCESS = 26,
> + USIM_ALLOC_SERVICE_DATA_DOWNLOAD_SMS_PP = 27,
> + USIM_ALLOC_SERVICE_DATA_DOWNLOAD_SMS_CB = 28,
> + USIM_ALLOC_SERVICE_CALL_CONTROL_USIM = 29,
> + USIM_ALLOC_SERVICE_MO_SMS_USIM = 30,
> + USIM_ALLOC_SERVICE_RUN_AT_COMMAND = 31,
> + USIM_ALLOC_SERVICE_ENABLED_SERVICE_TABLE = 33,
> + USIM_ALLOC_SERVICE_ACL = 34,
> + USIM_ALLOC_SERVICE_DEPERSONALISATION_CTRL_KEY = 35,
> + USIM_ALLOC_SERVICE_NETWORK_LIST = 36,
> + USIM_ALLOC_SERVICE_GSM_SECURITY_CONTEXT = 37,
> + USIM_ALLOC_SERVICE_CPBCCH = 38,
> + USIM_ALLOC_SERVICE_INVESTIGATION_SCAN = 39,
> + USIM_ALLOC_SERVICE_MEXE = 40,
> + USIM_ALLOC_SERVICE_OPERATOR_PLMN = 41,
> + USIM_ALLOC_SERVICE_HPLMN = 42,
> + USIM_ALLOC_SERVICE_EXT_5 = 43,
> + USIM_ALLOC_SERVICE_PLMN_NETWORK_NAME = 44,
> + USIM_ALLOC_SERVICE_OPERATOR_PLMN_LIST = 45,
> + USIM_ALLOC_SERVICE_MAILBOX_DIALLING_NUMBERS = 46,
> + USIM_ALLOC_SERVICE_MWIS = 47,
> + USIM_ALLOC_SERVICE_CFIS = 48,
> + USIM_ALLOC_SERVICE_PROVIDER_DISPLAY_INFO = 50,
> + USIM_ALLOC_SERVICE_MMS = 51,
> + USIM_ALLOC_SERVICE_EXT_8 = 52,
> + USIM_ALLOC_SERVICE_CALL_CONTROL_GPRS_USIM = 53,
> + USIM_ALLOC_SERVICE_MMS_USER_CONN_PARAM = 54,
> + USIM_ALLOC_SERVICE_NIA = 55,
> + USIM_ALLOC_SERVICE_EFVGCS_EFVGCSS = 56,
> + USIM_ALLOC_SERVICE_EFVBS_EFVBSS = 57,
> + USIM_ALLOC_SERVICE_PSEUDONYM = 58,
> + USIM_ALLOC_SERVICE_USER_PLMN_I_WLAN = 59,
> + USIM_ALLOC_SERVICE_OPERATOR_PLMN_I_WLAN = 60,
> + USIM_ALLOC_SERVICE_USER_WSID = 61,
> + USIM_ALLOC_SERVICE_OPERATOR_WSID = 62,
> + USIM_ALLOC_SERVICE_VGCS_SECURITY = 63,
> + USIM_ALLOC_SERVICE_VBS_SECURITY = 64,
> + USIM_ALLOC_SERVICE_WLAN_REAUTH_ID = 65,
> + USIM_ALLOC_SERVICE_MMS_STORAGE = 66,
> + USIM_ALLOC_SERVICE_GBA = 67,
> + USIM_ALLOC_SERVICE_MBMS_SECURITY = 68,
> + USIM_ALLOC_SERVICE_USSD_APPLICATION_MODE = 69,
> + USIM_ALLOC_SERVICE_EQUIVALENT_HPLMN = 70,
> + USIM_ALLOC_SERVICE_ADDITIONAL_TERMINAL_PROFILE = 71,
> + USIM_ALLOC_SERVICE_EQUIVALENT_HPLMN_IND = 72,
> + USIM_ALLOC_SERVICE_LAST_RPLMN_IND = 73,
> + USIM_ALLOC_SERVICE_OMA_BCAST_SC_PROFILE = 74,
> + USIM_ALLOC_SERVICE_BGA_LOCAL_KEY = 75,
> + USIM_ALLOC_SERVICE_TERMINAL_APPLICATIONS = 76,
> + USIM_ALLOC_SERVICE_PROVIDER_NAME_ICON = 77,
> + USIM_ALLOC_SERVICE_PLMN_NETWORK_NAME_ICON = 78,
> + USIM_ALLOC_SERVICE_CONN_PARAM_USIM_IP = 79,
> + USIM_ALLOC_SERVICE_HOME_I_WLAN_ID_LIST = 80,
> + USIM_ALLOC_SERVICE_I_WLAN_EQUIVALENT_HPLMN_IND = 81,
> + USIM_ALLOC_SERVICE_I_WLAN_HPLMN_PRIORITY_IND = 82,
> + USIM_ALLOC_SERVICE_I_WLAN_LAST_PLMN = 83,
> + USIM_ALLOC_SERVICE_EPS_INFO = 84,
> + USIM_ALLOC_SERVICE_CSG_IND = 85,
> + USIM_ALLOC_SERVICE_CALL_CONTROL_EPS_PDN_USIM = 86,
> + USIM_ALLOC_SERVICE_HPLMN_DIRECT_ACCESS = 87
> +};
> +
So I realize what you're trying to do here, but I think that prefixing
this enum with SIM_UST_SERVICE will be better. Also, since you're at
it, make sure you grab the last couple of enums from 31.102.
> +/* 131.102 Section 4.2.47 */
> +enum usim_actv_service {
> + USIM_ACTV_SERVICE_FDN = 0,
> + USIM_ACTV_SERVICE_BDN = 1,
> + USIM_ACTV_SERVICE_ACL = 2
> +};
> +
Same thing here, SIM_EST_SERVICE would be better.
> #define SIM_EFSPN_DC_HOME_PLMN_BIT 0x1
> #define SIM_EFSPN_DC_ROAMING_SPN_BIT 0x2
>
Regards,
-Denis
prev parent reply other threads:[~2010-07-22 16:53 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-21 11:07 [PATCH] sim: Read EFust and EFest Yang Gu
2010-07-22 16:53 ` Denis Kenzior [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C4877A3.8000600@gmail.com \
--to=denkenz@gmail.com \
--cc=ofono@ofono.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.