[PATCH] Core logic to support SIM hot swap
Carlo Lobrano
carlo.lobrano at telit.com
Thu Jun 30 14:56:33 UTC 2016
On giovedì 30 giugno 2016 15:54:48 CEST Aleksander Morgado wrote:
> 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.
> _
Uhm, you're right. So, basically unref PortsContext and leave a log about the error would be enough?
> > + } 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"?
>
Agree
> > +{
> > + 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).
>
I did tried without the plugin part, but I didn't see that problem.
To implement the property in MMBroadbandModem, do you mean like PROP_MODEM_SIM? I can use that as example?
Thanks,
Carlo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Image_01.jpg
Type: image/jpg
Size: 20835 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0003.jpg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: autosig_Image_01.jpg
Type: image/jpg
Size: 2771 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0004.jpg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: autosig_Image_07.gif
Type: image/gif
Size: 1089 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0005.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: autosig_Image_04.gif
Type: image/gif
Size: 1059 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0006.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: autosig_Image_05.gif
Type: image/gif
Size: 643 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0007.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: autosig_Image_06.gif
Type: image/gif
Size: 1091 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0008.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: autosig_Image_02.JPG
Type: image/jpg
Size: 8465 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0005.jpg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: autosig_Image_03.gif
Type: image/gif
Size: 1111 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160630/3bb3d960/attachment-0009.gif>
More information about the ModemManager-devel
mailing list