[PATCH] Core logic to support SIM hot swap
Aleksander Morgado
aleksander at aleksander.es
Thu Jun 30 13:54:48 UTC 2016
Hey Carlo!
On Thu, Jun 30, 2016 at 1:36 PM, Carlo Lobrano <c.lobrano at gmail.com> wrote:
> BaseModem
> added reprobe property.
>
> MMDevice
> added logic to recreate the modem if it is set invalid and "to reprobe"
>
> MMBroadbandModem
> * added initialization step for SIM hot swap:
> 1. keep dedicated ports open to listen to modem's unsolicited
> 2. dedicated error management in case of initialization failure due to SIM missing
> * added function to be called in order to act upon SIM insertion/removal:
> 1. close dedicated ports
> 2. set the modem to be reprobed
> 3. disable modem
>
> MMIfaceModem
> * added initialization step for SIM hot swap, if supported by the plugin
> * dedicated error management in case of initialization failure due to SIM missing
> ---
> src/mm-base-modem.c | 26 ++++++++++
> src/mm-base-modem.h | 4 ++
> src/mm-broadband-modem.c | 126 ++++++++++++++++++++++++++++++++++++++++-------
> src/mm-broadband-modem.h | 5 ++
> src/mm-device.c | 15 ++++++
> src/mm-iface-modem.c | 41 +++++++++++++++
> src/mm-iface-modem.h | 9 ++++
> 7 files changed, 209 insertions(+), 17 deletions(-)
>
> diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c
> index 9cd7c5f..ede014b 100644
> --- a/src/mm-base-modem.c
> +++ b/src/mm-base-modem.c
> @@ -47,6 +47,7 @@ enum {
> PROP_VENDOR_ID,
> PROP_PRODUCT_ID,
> PROP_CONNECTION,
> + PROP_REPROBE,
> PROP_LAST
> };
>
> @@ -70,6 +71,7 @@ struct _MMBaseModemPrivate {
>
> gboolean hotplugged;
> gboolean valid;
> + gboolean reprobe;
>
> guint max_timeouts;
>
> @@ -392,6 +394,24 @@ mm_base_modem_set_valid (MMBaseModem *self,
> }
> }
>
> +void
> +mm_base_modem_set_reprobe (MMBaseModem *self,
> + gboolean reprobe)
> +{
> + g_return_if_fail (MM_IS_BASE_MODEM (self));
> +
> + self->priv->reprobe = reprobe;
> +}
> +
> +gboolean
> +mm_base_modem_get_reprobe (MMBaseModem *self)
> +{
> + g_return_val_if_fail (MM_IS_BASE_MODEM (self), FALSE);
> +
> + return self->priv->reprobe;
> +}
> +
> +
> gboolean
> mm_base_modem_get_valid (MMBaseModem *self)
> {
> @@ -1295,6 +1315,9 @@ set_property (GObject *object,
> case PROP_VALID:
> mm_base_modem_set_valid (self, g_value_get_boolean (value));
> break;
> + case PROP_REPROBE:
> + mm_base_modem_set_reprobe (self, g_value_get_boolean (value));
> + break;
> case PROP_MAX_TIMEOUTS:
> self->priv->max_timeouts = g_value_get_uint (value);
> break;
> @@ -1338,6 +1361,9 @@ get_property (GObject *object,
> case PROP_VALID:
> g_value_set_boolean (value, self->priv->valid);
> break;
> + case PROP_REPROBE:
> + g_value_set_boolean (value, self->priv->reprobe);
> + break;
> case PROP_MAX_TIMEOUTS:
> g_value_set_uint (value, self->priv->max_timeouts);
> break;
> diff --git a/src/mm-base-modem.h b/src/mm-base-modem.h
> index 3c0d16f..c4bcdce 100644
> --- a/src/mm-base-modem.h
> +++ b/src/mm-base-modem.h
> @@ -166,6 +166,10 @@ void mm_base_modem_set_valid (MMBaseModem *self,
> gboolean valid);
> gboolean mm_base_modem_get_valid (MMBaseModem *self);
>
> +void mm_base_modem_set_reprobe (MMBaseModem *self,
> + gboolean reprobe);
> +gboolean mm_base_modem_get_reprobe (MMBaseModem *self);
> +
> const gchar *mm_base_modem_get_device (MMBaseModem *self);
> const gchar **mm_base_modem_get_drivers (MMBaseModem *self);
> const gchar *mm_base_modem_get_plugin (MMBaseModem *self);
> diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> index e1fd7ca..38fabed 100644
> --- a/src/mm-broadband-modem.c
> +++ b/src/mm-broadband-modem.c
> @@ -124,6 +124,7 @@ typedef struct _PortsContext PortsContext;
> struct _MMBroadbandModemPrivate {
> /* Broadband modem specific implementation */
> PortsContext *enabled_ports_ctx;
> + PortsContext *sim_hot_swap_ports_ctx;
> gboolean modem_init_run;
>
> /*<--- Modem interface --->*/
> @@ -8317,6 +8318,11 @@ disabling_stopped (MMBroadbandModem *self,
> ports_context_unref (self->priv->enabled_ports_ctx);
> self->priv->enabled_ports_ctx = NULL;
> }
> +
> + if (self->priv->sim_hot_swap_ports_ctx) {
> + ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
> + self->priv->sim_hot_swap_ports_ctx = NULL;
> + }
> return TRUE;
> }
>
> @@ -9435,6 +9441,7 @@ typedef enum {
> INITIALIZE_STEP_IFACE_SIGNAL,
> INITIALIZE_STEP_IFACE_OMA,
> INITIALIZE_STEP_IFACE_FIRMWARE,
> + INITIALIZE_STEP_SIM_HOT_SWAP,
> INITIALIZE_STEP_IFACE_SIMPLE,
> INITIALIZE_STEP_LAST,
> } InitializeStep;
> @@ -9776,6 +9783,38 @@ initialize_step (InitializeContext *ctx)
> ctx);
> return;
>
> + case INITIALIZE_STEP_SIM_HOT_SWAP:
> + {
> + gboolean is_sim_hot_swap_supported = FALSE;
> +
> + g_object_get(MM_BASE_MODEM (ctx->self),
> + IS_SIM_HOT_SWAP_SUPPORTED_PROPERTY,
> + &is_sim_hot_swap_supported,
> + NULL);
> +
> + if (!is_sim_hot_swap_supported) {
> + ctx->self->priv->sim_hot_swap_ports_ctx = NULL;
> + } else {
> + PortsContext *ports;
> + GError *error = NULL;
> +
> + mm_dbg ("Modem supports SIM hot swap. Opening dedicated ports.");
> +
> + ports = g_new0 (PortsContext, 1);
> + ports->ref_count = 1;
> +
> + if (!open_ports_enabling (ctx->self, ports, FALSE, &error)) {
> + g_prefix_error (&error, "Couldn't open ports during Modem SIM hot swap enabling: ");
> + g_simple_async_result_take_error (ctx->result, error);
Shouldn't the context be finished here? You're setting the error in
the GSimpleAsyncResult, but it is not being completed anywhere.
_
> + } else {
> + ctx->self->priv->sim_hot_swap_ports_ctx = ports_context_ref (ports);
> + ports_context_unref (ports);
> + }
> + }
> + }
> + /* Fall down to next step */
> + ctx->step++;
> +
> case INITIALIZE_STEP_IFACE_SIMPLE:
> if (ctx->self->priv->modem_state != MM_MODEM_STATE_FAILED)
> mm_iface_modem_simple_initialize (MM_IFACE_MODEM_SIMPLE (ctx->self));
> @@ -9796,23 +9835,45 @@ initialize_step (InitializeContext *ctx)
> "cannot fully initialize");
> } else {
> /* Fatal SIM, firmware, or modem failure :-( */
> - g_simple_async_result_set_error (ctx->result,
> - MM_CORE_ERROR,
> - MM_CORE_ERROR_WRONG_STATE,
> - "Modem is unusable, "
> - "cannot fully initialize");
> - /* Ensure we only leave the Modem, OMA, and Firmware interfaces
> - * around. A failure could be caused by firmware issues, which
> - * a firmware update, switch, or provisioning could fix.
> - */
> - mm_iface_modem_3gpp_shutdown (MM_IFACE_MODEM_3GPP (ctx->self));
> - mm_iface_modem_3gpp_ussd_shutdown (MM_IFACE_MODEM_3GPP_USSD (ctx->self));
> - mm_iface_modem_cdma_shutdown (MM_IFACE_MODEM_CDMA (ctx->self));
> - mm_iface_modem_location_shutdown (MM_IFACE_MODEM_LOCATION (ctx->self));
> - mm_iface_modem_messaging_shutdown (MM_IFACE_MODEM_MESSAGING (ctx->self));
> - mm_iface_modem_voice_shutdown (MM_IFACE_MODEM_VOICE (ctx->self));
> - mm_iface_modem_time_shutdown (MM_IFACE_MODEM_TIME (ctx->self));
> - mm_iface_modem_simple_shutdown (MM_IFACE_MODEM_SIMPLE (ctx->self));
> + gboolean is_sim_hot_swap_supported = FALSE;
> + MMModemStateFailedReason reason =
> + mm_gdbus_modem_get_state_failed_reason (
> + (MmGdbusModem*)ctx->self->priv->modem_dbus_skeleton);
> +
> + g_object_get(MM_BASE_MODEM (ctx->self),
> + IS_SIM_HOT_SWAP_SUPPORTED_PROPERTY,
> + &is_sim_hot_swap_supported,
> + NULL);
> +
> + if (reason == MM_MODEM_STATE_FAILED_REASON_SIM_MISSING &&
> + is_sim_hot_swap_supported) {
> + mm_info("SIM is missing, but the modem supports SIM hot insertion. Waiting for SIM...");
> + g_simple_async_result_set_error (ctx->result,
> + MM_CORE_ERROR,
> + MM_CORE_ERROR_WRONG_STATE,
> + "Modem is unusable due to SIM missing, "
> + "cannot fully initialize, "
> + "waiting for SIM insertion.");
> + } else {
> + mm_dbg ("SIM is missing and Modem does not support SIM Hot Swap");
> + g_simple_async_result_set_error (ctx->result,
> + MM_CORE_ERROR,
> + MM_CORE_ERROR_WRONG_STATE,
> + "Modem is unusable, "
> + "cannot fully initialize");
> + /* Ensure we only leave the Modem, OMA, and Firmware interfaces
> + * around. A failure could be caused by firmware issues, which
> + * a firmware update, switch, or provisioning could fix.
> + */
> + mm_iface_modem_3gpp_shutdown (MM_IFACE_MODEM_3GPP (ctx->self));
> + mm_iface_modem_3gpp_ussd_shutdown (MM_IFACE_MODEM_3GPP_USSD (ctx->self));
> + mm_iface_modem_cdma_shutdown (MM_IFACE_MODEM_CDMA (ctx->self));
> + mm_iface_modem_location_shutdown (MM_IFACE_MODEM_LOCATION (ctx->self));
> + mm_iface_modem_messaging_shutdown (MM_IFACE_MODEM_MESSAGING (ctx->self));
> + mm_iface_modem_voice_shutdown (MM_IFACE_MODEM_VOICE (ctx->self));
> + mm_iface_modem_time_shutdown (MM_IFACE_MODEM_TIME (ctx->self));
> + mm_iface_modem_simple_shutdown (MM_IFACE_MODEM_SIMPLE (ctx->self));
> + }
> }
> initialize_context_complete_and_free (ctx);
> return;
> @@ -9960,6 +10021,37 @@ mm_broadband_modem_create_device_identifier (MMBroadbandModem *self,
> MM_GDBUS_MODEM (MM_BROADBAND_MODEM (self)->priv->modem_dbus_skeleton))));
> }
>
> +
> +/*****************************************************************************/
> +static void
> +disable_ready (MMBaseModem *self,
> + GAsyncResult *res,
> + gpointer user_data)
How about renaming this as "after_hotswap_event_disable_ready"?
> +{
> + GError *error = NULL;
> + if (!mm_base_modem_disable_finish (self, res, &error)) {
> + mm_err ("Disable modem error: %s", error->message);
GError leaks here.
> + } else {
> + mm_base_modem_set_valid (MM_BASE_MODEM (self), FALSE);
> + }
> +}
> +
> +
> +void mm_broadband_modem_update_sim_hot_swap_status (MMBroadbandModem *self,
> + gboolean is_sim_missing)
> +{
> + if (self->priv->sim_hot_swap_ports_ctx) {
> + mm_dbg ("Releasing SIM hot swap ports context");
> + ports_context_unref (self->priv->sim_hot_swap_ports_ctx);
> + self->priv->sim_hot_swap_ports_ctx = NULL;
> + }
> +
> + mm_base_modem_set_reprobe (MM_BASE_MODEM (self), TRUE);
> + mm_base_modem_disable (MM_BASE_MODEM (self),
> + (GAsyncReadyCallback)disable_ready,
> + NULL);
> +}
> +
> /*****************************************************************************/
>
> MMBroadbandModem *
> diff --git a/src/mm-broadband-modem.h b/src/mm-broadband-modem.h
> index 93ffeb5..ca5bab3 100644
> --- a/src/mm-broadband-modem.h
> +++ b/src/mm-broadband-modem.h
> @@ -127,5 +127,10 @@ gboolean mm_broadband_modem_lock_sms_storages_finish (MMBroadbandModem *self,
> void mm_broadband_modem_unlock_sms_storages (MMBroadbandModem *self,
> gboolean mem1,
> gboolean mem2);
> +/* Helper to update SIM hot swap */
> +void mm_broadband_modem_update_sim_hot_swap_status (MMBroadbandModem *self,
> + gboolean is_sim_missing);
> +
> +
>
> #endif /* MM_BROADBAND_MODEM_H */
> diff --git a/src/mm-device.c b/src/mm-device.c
> index 2456cc8..abc8f30 100644
> --- a/src/mm-device.c
> +++ b/src/mm-device.c
> @@ -489,8 +489,23 @@ modem_valid (MMBaseModem *modem,
> MMDevice *self)
> {
> if (!mm_base_modem_get_valid (modem)) {
> + GDBusObjectManagerServer *object_manager = self->priv->object_manager;
> +
> /* Modem no longer valid */
> mm_device_remove_modem (self);
> +
> + if (mm_base_modem_get_reprobe (modem)) {
> + GError *error = NULL;
> +
> + if (!mm_device_create_modem (self, object_manager, &error)) {
> + mm_warn ("Could not recreate modem for device at '%s': %s",
> + mm_device_get_path(self),
> + error ? error->message : "unknown");
> + g_error_free (error);
> + } else {
> + mm_dbg ("Modem recreated for device '%s'", mm_device_get_path(self));
> + }
> + }
> } else {
> /* Modem now valid, export it, but only if we really have it around.
> * It may happen that the initialization sequence fails because the
> diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
> index 516ed78..9aa6df8 100644
> --- a/src/mm-iface-modem.c
> +++ b/src/mm-iface-modem.c
> @@ -3745,6 +3745,7 @@ typedef enum {
> INITIALIZATION_STEP_DEVICE_ID,
> INITIALIZATION_STEP_SUPPORTED_MODES,
> INITIALIZATION_STEP_SUPPORTED_BANDS,
> + INITIALIZATION_STEP_SIM_HOT_SWAP,
> INITIALIZATION_STEP_SUPPORTED_IP_FAMILIES,
> INITIALIZATION_STEP_POWER_STATE,
> INITIALIZATION_STEP_UNLOCK_REQUIRED,
> @@ -4203,6 +4204,22 @@ load_current_bands_ready (MMIfaceModem *self,
> interface_initialization_step (ctx);
> }
>
> +/*****************************************************************************/
> +/* Setup SIM hot swap (Modem interface) */
> +static void
> +setup_sim_hot_swap_ready (MMIfaceModem *self,
> + GAsyncResult *res,
> + gpointer user_data) {
> + GError *error = NULL;
> +
> + MM_IFACE_MODEM_GET_INTERFACE (self)->setup_sim_hot_swap_finish (self, res, &error);
> + if (error)
> + mm_warn ("SIM hot swap setup failed: '%s'", error->message);
error leaks.
> + else
> + mm_dbg ("SIM hot swap setup succeded");
> +}
> +
> static void
> interface_initialization_step (InitializationContext *ctx)
> {
> @@ -4544,6 +4561,15 @@ interface_initialization_step (InitializationContext *ctx)
> /* Fall down to next step */
> ctx->step++;
>
> + case INITIALIZATION_STEP_SIM_HOT_SWAP:
> + if (MM_IFACE_MODEM_GET_INTERFACE (ctx->self)->setup_sim_hot_swap &&
> + MM_IFACE_MODEM_GET_INTERFACE (ctx->self)->setup_sim_hot_swap_finish) {
> + MM_IFACE_MODEM_GET_INTERFACE (ctx->self)->setup_sim_hot_swap (
> + MM_IFACE_MODEM (ctx->self),
> + (GAsyncReadyCallback) setup_sim_hot_swap_ready,
> + NULL);
A return is missing here.
> + }
And the ctx->step++; is missing here.
> +
> case INITIALIZATION_STEP_UNLOCK_REQUIRED:
> /* Only check unlock required if we were previously not unlocked */
> if (mm_gdbus_modem_get_unlock_required (ctx->skeleton) != MM_MODEM_LOCK_NONE) {
> @@ -4707,6 +4733,21 @@ interface_initialization_step (InitializationContext *ctx)
> ctx->self);
>
> if (ctx->fatal_error) {
> + if (g_error_matches (ctx->fatal_error,
> + MM_MOBILE_EQUIPMENT_ERROR,
> + MM_MOBILE_EQUIPMENT_ERROR_SIM_NOT_INSERTED)) {
> + gboolean is_sim_hot_swap_supported = FALSE;
> +
> + g_object_get(MM_BASE_MODEM (ctx->self),
> + IS_SIM_HOT_SWAP_SUPPORTED_PROPERTY,
> + &is_sim_hot_swap_supported,
> + NULL);
> +
> + if(is_sim_hot_swap_supported) {
A whitespace before the open-parenthesis is missing.
> + mm_iface_modem_update_failed_state (ctx->self,
> + MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
> + }
> + }
> g_simple_async_result_take_error (ctx->result, ctx->fatal_error);
> ctx->fatal_error = NULL;
> } else {
> diff --git a/src/mm-iface-modem.h b/src/mm-iface-modem.h
> index 74ea9f9..eabb0be 100644
> --- a/src/mm-iface-modem.h
> +++ b/src/mm-iface-modem.h
> @@ -36,6 +36,7 @@
> #define MM_IFACE_MODEM_STATE "iface-modem-state"
> #define MM_IFACE_MODEM_SIM "iface-modem-sim"
> #define MM_IFACE_MODEM_BEARER_LIST "iface-modem-bearer-list"
> +#define IS_SIM_HOT_SWAP_SUPPORTED_PROPERTY "is-sim-hot-swap-supported"
Property name should be prefixed with "MM_IFACE_MODEM", something like:
#define MM_IFACE_MODEM_SIM_HOTSWAP_SUPPORTED
"iface-modem-sim-hotswap-supported".
>
> typedef struct _MMIfaceModem MMIfaceModem;
>
> @@ -327,6 +328,14 @@ struct _MMIfaceModem {
> MMBaseBearer * (*create_bearer_finish) (MMIfaceModem *self,
> GAsyncResult *res,
> GError **error);
> + /* Setup SIM hot swap */
> + void (*setup_sim_hot_swap) (MMIfaceModem *self,
> + GAsyncReadyCallback callback,
> + gpointer user_data);
> +
> + gboolean (*setup_sim_hot_swap_finish) (MMIfaceModem *self,
> + GAsyncResult *res,
> + GError **error);
> };
>
> GType mm_iface_modem_get_type (void);
> --
> 2.7.4
>
I see that the property defined in the mm-iface-modem isn't
implemented by any of the base objects, and that is not right. The
property must be implemented by MMBroadbandModem, so that getting the
property doesn't fail (it likely doesn't fail for you in your tests
because you implemented the property in the telit plugin in the next
patch).
--
Aleksander
https://aleksander.es
More information about the ModemManager-devel
mailing list