Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-06-29 Thread Aleksander Morgado
Hey Eric,

+Carlo in CC

Carlo, would also like your opinion on this.

On 28/06/17 23:46, Eric Caruso wrote:
> If an MBIM modem supports unsolicited notifications for
> subscriber ready status, we can use it to detect when SIM cards
> have been removed and reinserted. Upon detection we should re-
> probe the modem so that we can configure it for the new SIM.
> ---
>  src/mm-broadband-modem-mbim.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
> index 0d1126d4..c95255d9 100644
> --- a/src/mm-broadband-modem-mbim.c
> +++ b/src/mm-broadband-modem-mbim.c
> @@ -88,6 +88,9 @@ struct _MMBroadbandModemMbimPrivate {
>  /* Access technology updates */
>  MbimDataClass available_data_classes;
>  MbimDataClass highest_available_data_class;
> +
> +/* For checking whether the SIM has been hot swapped */
> +MbimSubscriberReadyState last_ready_state;
>  };
>  
>  
> /*/
> @@ -2041,8 +2044,18 @@ basic_connect_notification_subscriber_ready_status 
> (MMBroadbandModemMbim *self,
>  if (ready_state == MBIM_SUBSCRIBER_READY_STATE_INITIALIZED)
>  mm_iface_modem_update_own_numbers (MM_IFACE_MODEM (self), 
> telephone_numbers);
>  
> -/* TODO: handle SIM removal using 
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED */
> +if (self->priv->last_ready_state != 
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
> +/* SIM has been removed */
> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
> +
> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
> +} else if (self->priv->last_ready_state == 
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
> +   ready_state != MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
> +/* SIM has been reinserted */
> +mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM 
> (self));
> +}
>  

If I'm not mistaken, whenever a sim insert/removal event is detected, we should 
just call mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger 
a full modem re-probe. In this case the method is only being called for the 
case where the SIM is inserted, not for when the SIM is removed.

> +self->priv->last_ready_state = ready_state;
>  g_strfreev (telephone_numbers);
>  }
>  
> @@ -3165,6 +3178,7 @@ mm_broadband_modem_mbim_new (const gchar *device,
>   MM_BASE_MODEM_PLUGIN, plugin,
>   MM_BASE_MODEM_VENDOR_ID, vendor_id,
>   MM_BASE_MODEM_PRODUCT_ID, product_id,
> + MM_IFACE_MODEM_SIM_HOT_SWAP_SUPPORTED, TRUE,
>   NULL);
>  }
>  
> 


-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-06-29 Thread Aleksander Morgado
On 29/06/17 10:51, Aleksander Morgado wrote:
>> +if (self->priv->last_ready_state != 
>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
>> +/* SIM has been removed */
>> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
>> +
>> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
>> +} else if (self->priv->last_ready_state == 
>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>> +   ready_state != MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) 
>> {
>> +/* SIM has been reinserted */
>> +mm_broadband_modem_update_sim_hot_swap_detected (MM_BROADBAND_MODEM 
>> (self));
>> +}
>>  
> If I'm not mistaken, whenever a sim insert/removal event is detected, we 
> should just call mm_broadband_modem_update_sim_hot_swap_detected(), which 
> will trigger a full modem re-probe. In this case the method is only being 
> called for the case where the SIM is inserted, not for when the SIM is 
> removed.
> 

Also, could you provide MM debug logs showing the SIM card hot insertion and 
the SIM card hot removal?

-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-06-29 Thread Carlo Lobrano
Hi,

> If I'm not mistaken, whenever a sim insert/removal event is detected, we
should just call
> mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger a
full modem re-probe.

yes, I confirm this. *mm_broadband_modem_update_sim_hot_swap_detected* will
trigger full re-probe and disable current modem.

Here is the code in telit plugin


​133 *if* ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status
!= QSS_STATUS_SIM_REMOVED) ||
134 (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
QSS_STATUS_SIM_REMOVED)) {
135 mm_info ("QSS: SIM swap detected");
136 mm_broadband_modem_update_sim_hot_swap_detected (
MM_BROADBAND_MODEM (self));
137 }
​
The if condition is a bit complex here because we can have 4 different QSS
states, but if we are tracing just 2 or them (SIM IN vs SIM OUT), and if
I'm not wrong here, whenever last_ready_state and ready_state differ,
*mm_broadband_modem_update_sim_hot_swap_detected* should be called

if self->priv->last_ready_state != ready_state:
mm_broadband_modem_update_sim_hot_swap_detected (...)


On 29 June 2017 at 11:08, Aleksander Morgado 
wrote:

> On 29/06/17 10:51, Aleksander Morgado wrote:
> >> +if (self->priv->last_ready_state != 
> >> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED
> &&
> >> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
> >> +/* SIM has been removed */
> >> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
> >> +
> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
> >> +} else if (self->priv->last_ready_state ==
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
> >> +   ready_state != 
> >> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED)
> {
> >> +/* SIM has been reinserted */
> >> +mm_broadband_modem_update_sim_hot_swap_detected
> (MM_BROADBAND_MODEM (self));
> >> +}
> >>
> > If I'm not mistaken, whenever a sim insert/removal event is detected, we
> should just call mm_broadband_modem_update_sim_hot_swap_detected(), which
> will trigger a full modem re-probe. In this case the method is only being
> called for the case where the SIM is inserted, not for when the SIM is
> removed.
> >
>
> Also, could you provide MM debug logs showing the SIM card hot insertion
> and the SIM card hot removal?
>
> --
> Aleksander
> https://aleksander.es
>
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-06-29 Thread Eric Caruso
Thanks for taking a look at this!

On Thu, Jun 29, 2017 at 3:22 AM, Carlo Lobrano  wrote:
> Hi,
>
>> If I'm not mistaken, whenever a sim insert/removal event is detected, we
>> should just call
>> mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger a
>> full modem re-probe.

I had imagined that a full re-probe on SIM removal was overkill, and
the discussion in thread
https://lists.freedesktop.org/archives/modemmanager-devel/2014-February/000914.html
seemed to land on moving the modem into the failed state rather than
disabling it. I suppose the re-probe will re-initialize it and it will
get to the failed state because it doesn't have a SIM inserted, so I
can change this if that's what we want.

>
> yes, I confirm this. mm_broadband_modem_update_sim_hot_swap_detected will
> trigger full re-probe and disable current modem.
>
> Here is the code in telit plugin
>
>
> 133 if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
> QSS_STATUS_SIM_REMOVED) ||
> 134 (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
> QSS_STATUS_SIM_REMOVED)) {
> 135 mm_info ("QSS: SIM swap detected");
> 136 mm_broadband_modem_update_sim_hot_swap_detected
> (MM_BROADBAND_MODEM (self));
> 137 }
> The if condition is a bit complex here because we can have 4 different QSS
> states, but if we are tracing just 2 or them (SIM IN vs SIM OUT), and if I'm
> not wrong here, whenever last_ready_state and ready_state differ,
> mm_broadband_modem_update_sim_hot_swap_detected should be called
>
> if self->priv->last_ready_state != ready_state:
> mm_broadband_modem_update_sim_hot_swap_detected (...)
>

If I'm not mistaken, the
basic_connect_notification_subscriber_ready_status callback will
trigger on any change in the subscriber ready state, so this would
catch other state changes such as unlocking the SIM.

>
> On 29 June 2017 at 11:08, Aleksander Morgado 
> wrote:
>>
>> On 29/06/17 10:51, Aleksander Morgado wrote:
>> >> +if (self->priv->last_ready_state !=
>> >> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>> >> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
>> >> +/* SIM has been removed */
>> >> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
>> >> +
>> >> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
>> >> +} else if (self->priv->last_ready_state ==
>> >> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>> >> +   ready_state !=
>> >> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
>> >> +/* SIM has been reinserted */
>> >> +mm_broadband_modem_update_sim_hot_swap_detected
>> >> (MM_BROADBAND_MODEM (self));
>> >> +}
>> >>
>> > If I'm not mistaken, whenever a sim insert/removal event is detected, we
>> > should just call mm_broadband_modem_update_sim_hot_swap_detected(), which
>> > will trigger a full modem re-probe. In this case the method is only being
>> > called for the case where the SIM is inserted, not for when the SIM is
>> > removed.
>> >
>>
>> Also, could you provide MM debug logs showing the SIM card hot insertion
>> and the SIM card hot removal?

Removal puts this in the logs:

2017-06-29T09:52:09.749860-07:00 INFO ModemManager[7755]: 
Received notification (service 'basic-connect', command
'subscriber-ready-status')
2017-06-29T09:52:09.749918-07:00 INFO ModemManager[7755]:  Modem
/org/freedesktop/ModemManager1/Modem/0: state changed (connected ->
failed)
2017-06-29T09:52:09.759901-07:00 INFO ModemManager[7755]: 
Disconnecting bearer '/org/freedesktop/ModemManager1/Bearer/0'
2017-06-29T09:52:09.760052-07:00 INFO ModemManager[7755]:  Modem
/org/freedesktop/ModemManager1/Modem/0: state changed (failed ->
disconnecting)
2017-06-29T09:52:09.760772-07:00 INFO ModemManager[7755]:  Not
enabling periodic signal checks: unsupported
2017-06-29T09:52:09.760832-07:00 INFO ModemManager[7755]: 
Launching disconnection on data port (net/wwan0)

and reinsertion puts this in the logs:

2017-06-29T09:54:02.475438-07:00 INFO ModemManager[7755]: 
Received notification (service 'basic-connect', command
'subscriber-ready-status')
2017-06-29T09:54:02.475481-07:00 INFO ModemManager[7755]: 
Releasing SIM hot swap ports context
2017-06-29T09:54:02.475544-07:00 INFO ModemManager[7755]: 
(ttyACM0) device open count is 1 (close)
2017-06-29T09:54:02.475586-07:00 INFO ModemManager[7755]: 
(ttyACM2) device open count is 1 (close)
2017-06-29T09:54:02.475743-07:00 INFO ModemManager[7755]:  Modem
/org/freedesktop/ModemManager1/Modem/0: state changed (registered ->
disabling)
2017-06-29T09:54:02.476583-07:00 INFO ModemManager[7755]: 
Modem has time capabilities, disabling the Time interface...
2017-06-29T09:54:02.476704-07:00 INFO ModemManager[7755]: 
Modem has messaging capabilities, disabling the Messaging interface...
...
2017-06-29T09:54:02.506030-07:00 INFO ModemManager[7755]:  Modem
/org/freedesktop/ModemManager1/Modem/0: 3GPP Registration state
changed (idle -> unknown)
2017-06-29T09:54

Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-06-30 Thread Aleksander Morgado
On 29/06/17 19:20, Eric Caruso wrote:
> Thanks for taking a look at this!
> 
> On Thu, Jun 29, 2017 at 3:22 AM, Carlo Lobrano  wrote:
>> Hi,
>>
>>> If I'm not mistaken, whenever a sim insert/removal event is detected, we
>>> should just call
>>> mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger a
>>> full modem re-probe.
> 
> I had imagined that a full re-probe on SIM removal was overkill, and
> the discussion in thread
> https://lists.freedesktop.org/archives/modemmanager-devel/2014-February/000914.html
> seemed to land on moving the modem into the failed state rather than
> disabling it. I suppose the re-probe will re-initialize it and it will
> get to the failed state because it doesn't have a SIM inserted, so I
> can change this if that's what we want.
> 

A full reprobe is a bit overkill, yes, but right now that's the only way to not 
only force the modem in Failed state but also make sure all feature interfaces 
get un-exported from DBus. A modem in failed state should only allow very few 
of the feature interfaces exposed, mainly those which would allow to get away 
from a failed state, like e.g. the Firmware interface.

I'd suggest we keep the full reprobe for now, but also investigate the 
possibility of getting in the Failed state with the limited interfaces, as that 
would be much quicker.

>>
>> yes, I confirm this. mm_broadband_modem_update_sim_hot_swap_detected will
>> trigger full re-probe and disable current modem.
>>
>> Here is the code in telit plugin
>>
>>
>> 133 if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
>> QSS_STATUS_SIM_REMOVED) ||
>> 134 (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
>> QSS_STATUS_SIM_REMOVED)) {
>> 135 mm_info ("QSS: SIM swap detected");
>> 136 mm_broadband_modem_update_sim_hot_swap_detected
>> (MM_BROADBAND_MODEM (self));
>> 137 }
>> The if condition is a bit complex here because we can have 4 different QSS
>> states, but if we are tracing just 2 or them (SIM IN vs SIM OUT), and if I'm
>> not wrong here, whenever last_ready_state and ready_state differ,
>> mm_broadband_modem_update_sim_hot_swap_detected should be called
>>
>> if self->priv->last_ready_state != ready_state:
>> mm_broadband_modem_update_sim_hot_swap_detected (...)
>>
> 
> If I'm not mistaken, the
> basic_connect_notification_subscriber_ready_status callback will
> trigger on any change in the subscriber ready state, so this would
> catch other state changes such as unlocking the SIM.
> 

I sure hope we're not reprobing completely on SIM unlock notifications... 
Carlo, could you confirm that?

>>
>> On 29 June 2017 at 11:08, Aleksander Morgado 
>> wrote:
>>>
>>> On 29/06/17 10:51, Aleksander Morgado wrote:
> +if (self->priv->last_ready_state !=
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
> +/* SIM has been removed */
> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
> +
> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
> +} else if (self->priv->last_ready_state ==
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
> +   ready_state !=
> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
> +/* SIM has been reinserted */
> +mm_broadband_modem_update_sim_hot_swap_detected
> (MM_BROADBAND_MODEM (self));
> +}
>
 If I'm not mistaken, whenever a sim insert/removal event is detected, we
 should just call mm_broadband_modem_update_sim_hot_swap_detected(), which
 will trigger a full modem re-probe. In this case the method is only being
 called for the case where the SIM is inserted, not for when the SIM is
 removed.

>>>
>>> Also, could you provide MM debug logs showing the SIM card hot insertion
>>> and the SIM card hot removal?
> 
> Removal puts this in the logs:
> 
> 2017-06-29T09:52:09.749860-07:00 INFO ModemManager[7755]: 
> Received notification (service 'basic-connect', command
> 'subscriber-ready-status')
> 2017-06-29T09:52:09.749918-07:00 INFO ModemManager[7755]:  Modem
> /org/freedesktop/ModemManager1/Modem/0: state changed (connected ->
> failed)
> 2017-06-29T09:52:09.759901-07:00 INFO ModemManager[7755]: 
> Disconnecting bearer '/org/freedesktop/ModemManager1/Bearer/0'
> 2017-06-29T09:52:09.760052-07:00 INFO ModemManager[7755]:  Modem
> /org/freedesktop/ModemManager1/Modem/0: state changed (failed ->
> disconnecting)
> 2017-06-29T09:52:09.760772-07:00 INFO ModemManager[7755]:  Not
> enabling periodic signal checks: unsupported
> 2017-06-29T09:52:09.760832-07:00 INFO ModemManager[7755]: 
> Launching disconnection on data port (net/wwan0)
> 
> and reinsertion puts this in the logs:
> 
> 2017-06-29T09:54:02.475438-07:00 INFO ModemManager[7755]: 
> Received notification (service 'basic-connect', command
> 'subscriber-ready-status')
> 2017-06-29T09:54:02.475481-07:00 INFO ModemM

Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-06-30 Thread Eric Caruso
On Fri, Jun 30, 2017 at 1:00 AM, Aleksander Morgado
 wrote:
> On 29/06/17 19:20, Eric Caruso wrote:
>> Thanks for taking a look at this!
>>
>> On Thu, Jun 29, 2017 at 3:22 AM, Carlo Lobrano  wrote:
>>> Hi,
>>>
 If I'm not mistaken, whenever a sim insert/removal event is detected, we
 should just call
 mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger a
 full modem re-probe.
>>
>> I had imagined that a full re-probe on SIM removal was overkill, and
>> the discussion in thread
>> https://lists.freedesktop.org/archives/modemmanager-devel/2014-February/000914.html
>> seemed to land on moving the modem into the failed state rather than
>> disabling it. I suppose the re-probe will re-initialize it and it will
>> get to the failed state because it doesn't have a SIM inserted, so I
>> can change this if that's what we want.
>>
>
> A full reprobe is a bit overkill, yes, but right now that's the only way to 
> not only force the modem in Failed state but also make sure all feature 
> interfaces get un-exported from DBus. A modem in failed state should only 
> allow very few of the feature interfaces exposed, mainly those which would 
> allow to get away from a failed state, like e.g. the Firmware interface.
>
> I'd suggest we keep the full reprobe for now, but also investigate the 
> possibility of getting in the Failed state with the limited interfaces, as 
> that would be much quicker.

Thanks, I'll do that for now and look into moving it into the failed
state in the future.

>
>>>
>>> yes, I confirm this. mm_broadband_modem_update_sim_hot_swap_detected will
>>> trigger full re-probe and disable current modem.
>>>
>>> Here is the code in telit plugin
>>>
>>>
>>> 133 if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
>>> QSS_STATUS_SIM_REMOVED) ||
>>> 134 (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
>>> QSS_STATUS_SIM_REMOVED)) {
>>> 135 mm_info ("QSS: SIM swap detected");
>>> 136 mm_broadband_modem_update_sim_hot_swap_detected
>>> (MM_BROADBAND_MODEM (self));
>>> 137 }
>>> The if condition is a bit complex here because we can have 4 different QSS
>>> states, but if we are tracing just 2 or them (SIM IN vs SIM OUT), and if I'm
>>> not wrong here, whenever last_ready_state and ready_state differ,
>>> mm_broadband_modem_update_sim_hot_swap_detected should be called
>>>
>>> if self->priv->last_ready_state != ready_state:
>>> mm_broadband_modem_update_sim_hot_swap_detected (...)
>>>
>>
>> If I'm not mistaken, the
>> basic_connect_notification_subscriber_ready_status callback will
>> trigger on any change in the subscriber ready state, so this would
>> catch other state changes such as unlocking the SIM.
>>
>
> I sure hope we're not reprobing completely on SIM unlock notifications... 
> Carlo, could you confirm that?

It doesn't look like the telit QSS code does this since it looks for
specific QSS_STATUS_* states, and prior to this patch the only thing
basic_connect_notification_subscriber_ready_status would do is get the
own numbers off the SIM, so I'm not sure where it would be
reprobing...

>
>>>
>>> On 29 June 2017 at 11:08, Aleksander Morgado 
>>> wrote:

 On 29/06/17 10:51, Aleksander Morgado wrote:
>> +if (self->priv->last_ready_state !=
>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
>> +/* SIM has been removed */
>> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
>> +
>> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
>> +} else if (self->priv->last_ready_state ==
>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>> +   ready_state !=
>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
>> +/* SIM has been reinserted */
>> +mm_broadband_modem_update_sim_hot_swap_detected
>> (MM_BROADBAND_MODEM (self));
>> +}
>>
> If I'm not mistaken, whenever a sim insert/removal event is detected, we
> should just call mm_broadband_modem_update_sim_hot_swap_detected(), which
> will trigger a full modem re-probe. In this case the method is only being
> called for the case where the SIM is inserted, not for when the SIM is
> removed.
>

 Also, could you provide MM debug logs showing the SIM card hot insertion
 and the SIM card hot removal?
>>
>> Removal puts this in the logs:
>>
>> 2017-06-29T09:52:09.749860-07:00 INFO ModemManager[7755]: 
>> Received notification (service 'basic-connect', command
>> 'subscriber-ready-status')
>> 2017-06-29T09:52:09.749918-07:00 INFO ModemManager[7755]:  Modem
>> /org/freedesktop/ModemManager1/Modem/0: state changed (connected ->
>> failed)
>> 2017-06-29T09:52:09.759901-07:00 INFO ModemManager[7755]: 
>> Disconnecting bearer '/org/freedesktop/ModemManager1/Bearer/0'
>> 2017-06-29T09:52:09.760052-07:00 INFO ModemManager[7755]:  Modem
>> /o

Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-06-30 Thread Eric Caruso
On Fri, Jun 30, 2017 at 10:05 AM, Eric Caruso  wrote:
> On Fri, Jun 30, 2017 at 1:00 AM, Aleksander Morgado
>  wrote:
>> On 29/06/17 19:20, Eric Caruso wrote:
>>> Thanks for taking a look at this!
>>>
>>> On Thu, Jun 29, 2017 at 3:22 AM, Carlo Lobrano  wrote:
 Hi,

> If I'm not mistaken, whenever a sim insert/removal event is detected, we
> should just call
> mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger a
> full modem re-probe.
>>>
>>> I had imagined that a full re-probe on SIM removal was overkill, and
>>> the discussion in thread
>>> https://lists.freedesktop.org/archives/modemmanager-devel/2014-February/000914.html
>>> seemed to land on moving the modem into the failed state rather than
>>> disabling it. I suppose the re-probe will re-initialize it and it will
>>> get to the failed state because it doesn't have a SIM inserted, so I
>>> can change this if that's what we want.
>>>
>>
>> A full reprobe is a bit overkill, yes, but right now that's the only way to 
>> not only force the modem in Failed state but also make sure all feature 
>> interfaces get un-exported from DBus. A modem in failed state should only 
>> allow very few of the feature interfaces exposed, mainly those which would 
>> allow to get away from a failed state, like e.g. the Firmware interface.
>>
>> I'd suggest we keep the full reprobe for now, but also investigate the 
>> possibility of getting in the Failed state with the limited interfaces, as 
>> that would be much quicker.
>
> Thanks, I'll do that for now and look into moving it into the failed
> state in the future.

Something interesting happened when I tried this out: when re-probing
the modem on removal, we disable the Modem3gpp interface. This calls
mm_iface_modem_3gpp_disable which in turn calls
cleanup_unsolicited_events. As as result we stop getting
SUBSCRIBER_READY_STATUS notifications and we can no longer get
notified when the SIM is inserted. Simply moving the modem to the
failed state when the SIM card is ejected did not disable the 3gpp
interface so we were still able to get notifications.

However, this is still a problem even for the original patch if a SIM
card is not inserted when the modem is first enabled, so we have to
deal with this anyway. Additionally, if we want to move to the failed
state and disable interfaces we'll probably have the same issue. We
might want to be notified of SUBSCRIBER_INFO events regardless of
whether setup_unsolicited_events or cleanup_unsolicited_events is
called (e.g. we can subscribe to them separately in
setup_sim_hot_swap).

>
>>

 yes, I confirm this. mm_broadband_modem_update_sim_hot_swap_detected will
 trigger full re-probe and disable current modem.

 Here is the code in telit plugin


 133 if ((prev_qss_status == QSS_STATUS_SIM_REMOVED && cur_qss_status !=
 QSS_STATUS_SIM_REMOVED) ||
 134 (prev_qss_status > QSS_STATUS_SIM_REMOVED && cur_qss_status ==
 QSS_STATUS_SIM_REMOVED)) {
 135 mm_info ("QSS: SIM swap detected");
 136 mm_broadband_modem_update_sim_hot_swap_detected
 (MM_BROADBAND_MODEM (self));
 137 }
 The if condition is a bit complex here because we can have 4 different QSS
 states, but if we are tracing just 2 or them (SIM IN vs SIM OUT), and if 
 I'm
 not wrong here, whenever last_ready_state and ready_state differ,
 mm_broadband_modem_update_sim_hot_swap_detected should be called

 if self->priv->last_ready_state != ready_state:
 mm_broadband_modem_update_sim_hot_swap_detected (...)

>>>
>>> If I'm not mistaken, the
>>> basic_connect_notification_subscriber_ready_status callback will
>>> trigger on any change in the subscriber ready state, so this would
>>> catch other state changes such as unlocking the SIM.
>>>
>>
>> I sure hope we're not reprobing completely on SIM unlock notifications... 
>> Carlo, could you confirm that?
>
> It doesn't look like the telit QSS code does this since it looks for
> specific QSS_STATUS_* states, and prior to this patch the only thing
> basic_connect_notification_subscriber_ready_status would do is get the
> own numbers off the SIM, so I'm not sure where it would be
> reprobing...
>
>>

 On 29 June 2017 at 11:08, Aleksander Morgado 
 wrote:
>
> On 29/06/17 10:51, Aleksander Morgado wrote:
>>> +if (self->priv->last_ready_state !=
>>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>>> +ready_state == MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
>>> +/* SIM has been removed */
>>> +mm_iface_modem_update_failed_state (MM_IFACE_MODEM (self),
>>> +
>>> MM_MODEM_STATE_FAILED_REASON_SIM_MISSING);
>>> +} else if (self->priv->last_ready_state ==
>>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED &&
>>> +   ready_state !=
>>> MBIM_SUBSCRIBER_READY_STATE_SIM_NOT_INSERTED) {
>>> +/* SIM h

Re: [PATCH] mm-broadband-modem-mbim: support hot swapping

2017-07-03 Thread Aleksander Morgado
On 30/06/17 20:28, Eric Caruso wrote:
>> If I'm not mistaken, whenever a sim insert/removal event is detected, we
>> should just call
>> mm_broadband_modem_update_sim_hot_swap_detected(), which will trigger a
>> full modem re-probe.
 I had imagined that a full re-probe on SIM removal was overkill, and
 the discussion in thread
 https://lists.freedesktop.org/archives/modemmanager-devel/2014-February/000914.html
 seemed to land on moving the modem into the failed state rather than
 disabling it. I suppose the re-probe will re-initialize it and it will
 get to the failed state because it doesn't have a SIM inserted, so I
 can change this if that's what we want.

>>> A full reprobe is a bit overkill, yes, but right now that's the only way to 
>>> not only force the modem in Failed state but also make sure all feature 
>>> interfaces get un-exported from DBus. A modem in failed state should only 
>>> allow very few of the feature interfaces exposed, mainly those which would 
>>> allow to get away from a failed state, like e.g. the Firmware interface.
>>>
>>> I'd suggest we keep the full reprobe for now, but also investigate the 
>>> possibility of getting in the Failed state with the limited interfaces, as 
>>> that would be much quicker.
>> Thanks, I'll do that for now and look into moving it into the failed
>> state in the future.
> Something interesting happened when I tried this out: when re-probing
> the modem on removal, we disable the Modem3gpp interface. This calls
> mm_iface_modem_3gpp_disable which in turn calls
> cleanup_unsolicited_events. As as result we stop getting
> SUBSCRIBER_READY_STATUS notifications and we can no longer get
> notified when the SIM is inserted. Simply moving the modem to the
> failed state when the SIM card is ejected did not disable the 3gpp
> interface so we were still able to get notifications.
> 
> However, this is still a problem even for the original patch if a SIM
> card is not inserted when the modem is first enabled, so we have to
> deal with this anyway. Additionally, if we want to move to the failed
> state and disable interfaces we'll probably have the same issue. We
> might want to be notified of SUBSCRIBER_INFO events regardless of
> whether setup_unsolicited_events or cleanup_unsolicited_events is
> called (e.g. we can subscribe to them separately in
> setup_sim_hot_swap).
> 

Yes, that is true. Check how the Telit plugin deals with this registering the 
#QSS indications separately.

-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel