Re: [PATCH v4 1/3] huawei: detect possible secondary device

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

Hi Kalle,

>  struct huawei_data {
> -	GAtChat *chat;
> +	GAtChat *chat, *event;
>  };
> 

Minor nitpick, but we prefer:
GAtChat *chat;
GAtChat *event;

in structures.

> @@ -80,6 +81,11 @@ static void huawei_debug(const char *str, void
>  *user_data) ofono_info("%s", str);
>  }
> 
> +static void huawei_event_debug(const char *str, void *user_data)
> +{
> +	ofono_info("* %s", str);
> +}
> +

There is no need for a separate function here.  user_data can be used for 
this:

const char *prefix = user_data;
ofono_info("%s: %s", prefix, str);

> +	modem_device = ofono_modem_get_string(modem, "Device");
> +	event_device = ofono_modem_get_string(modem, "SecondaryDevice");
> +
> +	if (modem_device == NULL || event_device == NULL)
> +		return -EINVAL;
> +
> +	data->chat = create_port(modem_device);
> +
> +	if (data->chat == NULL)
>  		return -EIO;
> 
>  	g_at_chat_add_terminator(data->chat, "COMMAND NOT SUPPORT", -1, FALSE);
> @@ -120,6 +140,20 @@ static int huawei_enable(struct ofono_modem *modem)
>  	if (getenv("OFONO_AT_DEBUG"))
>  		g_at_chat_set_debug(data->chat, huawei_debug, NULL);
> 
> +	data->event = create_port(modem_device);
> +

Event port using the modem device again?

> +	if (data->event) {
> +		g_at_chat_cancel_all(data->event);
> +		g_at_chat_unregister_all(data->event);
> +	}
> +

You might want to unref event here instead of cfun_disable

Regards,
-Denis