[PATCH] huawei: implement modem power up and down
Aleksander Morgado
aleksander at lanedo.com
Fri Aug 2 03:03:57 PDT 2013
Hey,
On 02/08/13 06:44, Ben Chan wrote:
> From: Franko Fang <fangxiaozhi at huawei.com>
>
> This patch is originally developed by:
> Franko Fang <fangxiaozhi at huawei.com>
>
> And then reviewed and updated by:
> Ben Chan <benchan at chromium.org>
Thanks for the update Ben, you fixed most of the stuff I was going to
say in my first review :)
> ---
> plugins/huawei/mm-broadband-modem-huawei.c | 159 +++++++++++++++++++++++++++++
> 1 file changed, 159 insertions(+)
>
> diff --git a/plugins/huawei/mm-broadband-modem-huawei.c b/plugins/huawei/mm-broadband-modem-huawei.c
> index eb13a1b..8bc545b 100644
> --- a/plugins/huawei/mm-broadband-modem-huawei.c
> +++ b/plugins/huawei/mm-broadband-modem-huawei.c
> @@ -67,6 +67,12 @@ typedef enum {
> NDISDUP_SUPPORTED
> } NdisdupSupport;
>
> +typedef enum {
> + RFSWITCH_SUPPORT_UNKNOWN,
> + RFSWITCH_NOT_SUPPORTED,
> + RFSWITCH_SUPPORTED
> +} RfswitchSupport;
> +
> struct _MMBroadbandModemHuaweiPrivate {
> /* Regex for signal quality related notifications */
> GRegex *rssi_regex;
> @@ -92,6 +98,8 @@ struct _MMBroadbandModemHuaweiPrivate {
>
> gboolean sysinfoex_supported;
> gboolean sysinfoex_support_checked;
> +
> + RfswitchSupport rfswitch_supported;
> };
>
> /*****************************************************************************/
> @@ -2536,6 +2544,150 @@ modem_time_load_network_time (MMIfaceModemTime *self,
> }
>
> /*****************************************************************************/
Whiteline here.
> +typedef struct {
> + MMIfaceModem *self;
> + GAsyncReadyCallback callback;
> + gpointer user_data;
> + void (*modem_power) (MMIfaceModem *self,
> + GAsyncReadyCallback callback,
> + gpointer user_data);
> +} PowerManagerContext;
> +
> +static void
> +power_manager_context_complete_and_free (PowerManagerContext *ctx)
Don't name it complete_and_free(), because you are not completing any
GSimpleAsyncResult inside. A better name would be just
"power_manager_context_free().
> +{
> + ctx->modem_power = NULL;
This previous line is not needed.
> + g_slice_free (PowerManagerContext, ctx);
And given that the _free() method will only have one line, just fully
avoid the _free() method and call g_slice_free() directly when it's used.
> +}
> +
> +static void
> +huawei_rfswitch_check_ready (MMBaseModem *self,
> + GAsyncResult *res,
> + PowerManagerContext *ctx)
> +{
> + if (!mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, NULL)) {
> + mm_dbg ("The device can not support ^rfswitch");
> + MM_BROADBAND_MODEM_HUAWEI (self)->priv->rfswitch_supported = RFSWITCH_NOT_SUPPORTED;
> + } else {
> + mm_dbg ("The device support ^rfswitch");
s/support/supports
> + MM_BROADBAND_MODEM_HUAWEI (self)->priv->rfswitch_supported = RFSWITCH_SUPPORTED;
> + }
> +
> + if (ctx->modem_power)
ctx->modem_power will always be != NULL, no need to check it.
> + ctx->modem_power (ctx->self, ctx->callback, ctx->user_data);
> +
> + power_manager_context_complete_and_free (ctx);
Here you can call g_slice_free() directly.
> +}
> +
> +/*****************************************************************************/
> +/* Modem power up (Modem interface) */
> +
> +static void
> +huawei_modem_power_up (MMIfaceModem *self,
> + GAsyncReadyCallback callback,
> + gpointer user_data)
> +{
> + switch (MM_BROADBAND_MODEM_HUAWEI (self)->priv->rfswitch_supported) {
> + case RFSWITCH_SUPPORT_UNKNOWN:
> + {
Open brace in the same line as the case.
> + PowerManagerContext *ctx;
> +
> + ctx = g_slice_new0 (PowerManagerContext);
> + ctx->self = self;
> + ctx->callback = callback;
> + ctx->user_data = user_data;
> + ctx->modem_power = huawei_modem_power_up;
> +
> + mm_base_modem_at_command (MM_BASE_MODEM (self),
> + "^RFSWITCH?",
> + 3,
> + FALSE,
> + (GAsyncReadyCallback)huawei_rfswitch_check_ready,
> + ctx);
> + break;
> + }
> + case RFSWITCH_NOT_SUPPORTED:
> + mm_base_modem_at_command (MM_BASE_MODEM (self),
> + "+CFUN=1",
> + 30,
> + FALSE,
> + callback,
> + user_data);
> + break;
> + case RFSWITCH_SUPPORTED:
> + mm_base_modem_at_command (MM_BASE_MODEM (self),
> + "^RFSWITCH=1",
> + 30,
> + FALSE,
> + callback,
> + user_data);
> + break;
Add a default: break; here.
> + }
And g_assert_not_reached() here.
> +}
> +
> +static gboolean
> +huawei_modem_power_up_finish (MMIfaceModem *self,
> + GAsyncResult *res,
> + GError **error)
> +{
> + return !!mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error);
> +}
> +
> +/*****************************************************************************/
> +/* Modem power down (Modem interface) */
> +
> +static void
> +huawei_modem_power_down (MMIfaceModem *self,
> + GAsyncReadyCallback callback,
> + gpointer user_data)
> +{
> + switch (MM_BROADBAND_MODEM_HUAWEI (self)->priv->rfswitch_supported) {
> + case RFSWITCH_SUPPORT_UNKNOWN:
> + {
Open brace in the same line as the case.
> + PowerManagerContext *ctx;
> +
> + ctx = g_slice_new0 (PowerManagerContext);
> + ctx->self = self;
> + ctx->callback = callback;
> + ctx->user_data = user_data;
> + ctx->modem_power = huawei_modem_power_down;
> +
> + mm_base_modem_at_command (MM_BASE_MODEM (self),
> + "^RFSWITCH?",
> + 3,
> + FALSE,
> + (GAsyncReadyCallback)huawei_rfswitch_check_ready,
> + ctx);
> + break;
> + }
> + case RFSWITCH_NOT_SUPPORTED:
> + mm_base_modem_at_command (MM_BASE_MODEM (self),
> + "+CFUN=0",
> + 30,
> + FALSE,
> + callback,
> + user_data);
Are we sure that +CFUN=0 is good for *all* Huawei modems? Don't they
support +CFUN=4? This is the most critical change being done in the
patch I would say.
> + break;
> + case RFSWITCH_SUPPORTED:
> + mm_base_modem_at_command (MM_BASE_MODEM (self),
> + "^RFSWITCH=0",
> + 30,
> + FALSE,
> + callback,
> + user_data);
> + break;
Add a default: break; here.
> + }
And g_assert_not_reached() here.
> +}
> +
> +static gboolean
> +huawei_modem_power_down_finish (MMIfaceModem *self,
> + GAsyncResult *res,
> + GError **error)
> +{
> + return !!mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, error);
> +}
> +
> +/*****************************************************************************/
> /* Check support (Time interface) */
>
> static gboolean
> @@ -2701,6 +2853,8 @@ mm_broadband_modem_huawei_init (MMBroadbandModemHuawei *self)
>
> self->priv->ndisdup_support = NDISDUP_SUPPORT_UNKNOWN;
>
> + self->priv->rfswitch_supported = RFSWITCH_SUPPORT_UNKNOWN;
> +
> self->priv->sysinfoex_supported = FALSE;
> self->priv->sysinfoex_support_checked = FALSE;
> }
> @@ -2751,6 +2905,11 @@ iface_modem_init (MMIfaceModem *iface)
> iface->load_signal_quality_finish = modem_load_signal_quality_finish;
> iface->create_bearer = huawei_modem_create_bearer;
> iface->create_bearer_finish = huawei_modem_create_bearer_finish;
> +
> + iface->modem_power_up = huawei_modem_power_up;
> + iface->modem_power_up_finish = huawei_modem_power_up_finish;
> + iface->modem_power_down = huawei_modem_power_down;
> + iface->modem_power_down_finish = huawei_modem_power_down_finish;
> }
>
> static void
>
--
Aleksander
More information about the ModemManager-devel
mailing list