Re: [MM master] Re: [MM06] [PATCH] sierra: use +CFUN=4 for powering down

[Dan Williams <dcbw redhat com>]

On Tue, 2012-09-18 at 22:15 +0200, Marius Kotsbak wrote:
> 2012/9/10 Aleksander Morgado <aleksander lanedo com>
>         
>         >> See patch attached.
>         >>
>         >> sierra: use +CFUN=4 for powering down
>         >>
>         >>  plugins/mm-modem-sierra-gsm.c |   32
>         ++++++++++++++++++++++++++++++++
>         >>  1 file changed, 32 insertions(+)
>         >
>         > This also needs to be intregrated in the master branch
>         version. Attached
>         > is my attempt to do it, but it seems not to work. I see no
>         effect of
>         > disabling the modem with "mmcli -m X -d".
>         
>         
>         See review below.
>         > 0001-sierra-use-CFUN-4-for-powering-down.patch
>         
>         >
>         >
>         >>From ac69eb9a08f821efb674f54292ee90707ca81e29 Mon Sep 17
>         00:00:00 2001
>         > From: "Marius B. Kotsbak" <marius kotsbak com>
>         > Date: Sun, 9 Sep 2012 18:06:30 +0200
>         > Subject: [PATCH] sierra: use +CFUN=4 for powering down
>         >
>         > ---
>         >  plugins/sierra/mm-broadband-modem-sierra.c |   11
>         +++++++----
>         >  1 file changed, 7 insertions(+), 4 deletions(-)
>         >
>         > diff --git a/plugins/sierra/mm-broadband-modem-sierra.c
>         b/plugins/sierra/mm-broadband-modem-sierra.c
>         > index 49c2e22..c8e8f3e 100644
>         > --- a/plugins/sierra/mm-broadband-modem-sierra.c
>         > +++ b/plugins/sierra/mm-broadband-modem-sierra.c
>         > @@ -527,11 +527,14 @@ modem_power_down (MMIfaceModem *self,
>         >          return;
>         >      }
>         >
>         > -    /* For 3GPP modems we should call parent's power down,
>         but there is no
>         > -     * such power down command in MMBroadbandModem, so just
>         finish here. */
>         > -    g_simple_async_result_set_op_res_gboolean (result,
>         TRUE);
>         > -    g_simple_async_result_complete_in_idle (result);
>         >      g_object_unref (result);
>         > +
>         > +    mm_base_modem_at_command (MM_BASE_MODEM (self),
>         > +                              "+CFUN=4",
>         > +                              3,
>         > +                              FALSE,
>         > +                              callback,
>         > +                              user_data);
>         >  }
>         >
>         
>         There are 2 main ways to handle the async command execution
>         when an AT
>         command is issued:
>          1) If only the AT command can be issued, just run
>         base_modem_at_command() passing callback and user_data. In
>         that case you
>         don't need the _ready() method and you can just run
>         base_modem_at_command_finish() in the async method finish().
>          2) If the logic of the async command may end up not issuing
>         an AT
>         command, you need to create a GSimpleAsyncResult, and use a
>         specific
>         ready() method passed as GAsyncReadyCallback in
>         base_modem_at_command().
>         In this case, your new ready() method is responsible for
>         calling
>         base_modem_at_command_finish() and setting the simple async
>         result in
>         the GSimpleAsyncResult. Your async method finish would then
>         just get the
>         result or the error from the GSimpleAsyncResult.
>         
>         Now, before your patch, this async method was using approach
>         2, as the
>         async would end up issuing an AT command if it was a CDMA
>         modem, but
>         just returning in idle if it was a 3GPP modem. After the
>         patch, you're
>         merging both approaches, which is wrong.
>         
>         Given that now the async command will always issue an AT
>         command, you
>         can use the approach to pass the callback/user_data directly
>         in *both*
>         the at_command() methods instead of providing new custom
>         ready()
>         methods, modifying the async method finish() as well so that
>         it calls
>         _at_command_finish() itself.
>         
> 
> Thanks for the explanation. It is quite complicated, but I think I
> understand it now. Updated patch attached. This time it works with the
> "mmcli -m X -d" (but not with the init power down patch).

Yeah, !pcstate is a CDMA-only command; CFUN=4 is the right one for
GSM/UMTS/LTE devices.  Pushed, thanks!

Dan