[PATCH] Add DEVICE_OPEN_FLAG_DATA_FORMAT
Dan Williams
dcbw at redhat.com
Fri Sep 7 09:28:47 PDT 2012
On Thu, 2012-09-06 at 15:35 -0400, Robert Shade wrote:
> Ah, looks like we are. I didn't go to that level of granularity
> because it didn't seem like our current drivers supported any other
> mode.
>
> The QMI drivers from Sierra Wireless (modified versions of the
> CodeAurora drivers), explicitly set it to these values regardless of
> the QMI_CTL version and will fail if unsuccessful. I can say that
> SetDataFormat is at least in v1.4 of QMI_CTL.
Yeah, I was working on this earlier this week for the Pantech UML290,
for which the Windows drivers also set RawIP + no-QOS. Our Linux
drivers expect 802.3 + no-QOS.
The reason I did the separate flags are:
1) at some point, maybe we do want to allow RawIP or QoS headers; I
think QoS headers are most likely because we might want to handle that
better in the future. I'm not sure we have a case for RawIP yet, until
we find a device that really doesn't support DHCP on the network port.
Plus we don't actually have RawIP support in the kernel yet.
2) The Set Data Format command requires both TLVs, so we do need to
allow userspace to pass both options in, which requires that:
3) We don't want probing to change the port, so we need to the flags to
affirmatively specify whether to change or not. That, in conjunction
with (2) caused me to break them out separately.
In the end, our patches are quite similar, and Aleksander could take
either one. I put my enums into qmi-enums.h, because we're trying not
to expose the CTL service to clients (they don't really need it) and
there these enums are mostly private due to that; thus it didn't make a
lot of sense to me to create a new qmi-enums-ctl.h just for stuff that's
supposed to be private.
Dan
> On Thu, Sep 6, 2012 at 2:24 PM, Aleksander Morgado
> <aleksander at lanedo.com> wrote:
> > Hey Robert,
> >
> >> Some devices (like Sierra's MC7700 w/the latest AT&T firmware), default
> >> to a packet data format that's incompatable with qmi_wwan/usbnet. This results
> >> in us being unable to get a DHCP address or otherwise use the virtual ethernet
> >> port. Add DEVICE_OPEN_FLAG_DATA_FORMAT to set the data format
> >> to default (no-QoS) and data mode to ethernet when opening.
> >>
> >
> > Seems you and Dan are colliding here; he's now doing something almost
> > equivalent:
> >
> > http://cgit.freedesktop.org/libqmi/commit/?h=dcbw&id=a714a1244abf7465bade94f052f7f676e1eccc86
> >
> > I'll let you both discuss about the implementation details. Seems Dan
> > did split the different options into different flag values for
> > qmi_device_open().
> >
> > One of the things that I think is worth considering is that the
> > qmi_device_open() shouldn't fail if the "Set Data Format" command fails;
> > specially as we don't know in which CTL version it was introduced.
> >
> >
> >> ---
> >> cli/qmicli.c | 7 ++++
> >> data/qmi-service-ctl.json | 26 ++++++++++++++++
> >> libqmi-glib/Makefile.am | 13 +++++--
> >> libqmi-glib/qmi-device.c | 69 +++++++++++++++++++++++++++++++++++++++++++
> >> libqmi-glib/qmi-device.h | 4 ++-
> >> libqmi-glib/qmi-enums-ctl.h | 55 ++++++++++++++++++++++++++++++++++
> >> libqmi-glib/qmi-enums.h | 1 +
> >> 7 files changed, 170 insertions(+), 5 deletions(-)
> >> create mode 100644 libqmi-glib/qmi-enums-ctl.h
> >>
> >> diff --git a/cli/qmicli.c b/cli/qmicli.c
> >> index fd0d6c6..c91bfcd 100644
> >> --- a/cli/qmicli.c
> >> +++ b/cli/qmicli.c
> >> @@ -49,6 +49,7 @@ static gchar *device_str;
> >> static gchar *device_set_instance_id_str;
> >> static gboolean device_open_version_info_flag;
> >> static gboolean device_open_sync_flag;
> >> +static gboolean device_open_data_format_flag;
> >> static gchar *client_cid_str;
> >> static gboolean client_no_release_cid_flag;
> >> static gboolean verbose_flag;
> >> @@ -72,6 +73,10 @@ static GOptionEntry main_entries[] = {
> >> "Run sync operation when opening device",
> >> NULL
> >> },
> >> + { "device-open-data-format", 0, 0, G_OPTION_ARG_NONE,
> >> &device_open_data_format_flag,
> >> + "Set data format when opening device",
> >> + NULL
> >> + },
> >> { "client-cid", 0, 0, G_OPTION_ARG_STRING, &client_cid_str,
> >> "Use the given CID, don't allocate a new one",
> >> "[CID]"
> >> @@ -409,6 +414,8 @@ device_new_ready (GObject *unused,
> >> open_flags |= QMI_DEVICE_OPEN_FLAGS_VERSION_INFO;
> >> if (device_open_sync_flag)
> >> open_flags |= QMI_DEVICE_OPEN_FLAGS_SYNC;
> >> + if (device_open_data_format_flag)
> >> + open_flags |= QMI_DEVICE_OPEN_FLAGS_DATA_FORMAT;
> >>
> >> /* Open the device */
> >> qmi_device_open (device,
> >> diff --git a/data/qmi-service-ctl.json b/data/qmi-service-ctl.json
> >> index c789bff..2ef7895 100644
> >> --- a/data/qmi-service-ctl.json
> >> +++ b/data/qmi-service-ctl.json
> >> @@ -109,6 +109,32 @@
> >> "prerequisites": [ { "common-ref" : "Success" } ] } ] },
> >>
> >> // *********************************************************************************
> >> + { "name" : "Set Data Format",
> >> + "type" : "Message",
> >> + "service" : "CTL",
> >> + "id" : "0x0026",
> >> + "input" : [ { "name" : "Data Format",
> >> + "id" : "0x01",
> >> + "mandatory" : "yes",
> >> + "type" : "TLV",
> >> + "format" : "guint8",
> >> + "public-format" : "QmiCtlDataFormat" },
> >> + { "name" : "Data Mode",
> >> + "id" : "0x10",
> >> + "mandatory" : "no",
> >> + "type" : "TLV",
> >> + "format" : "guint16",
> >> + "public-format" : "QmiCtlDataMode" } ],
> >> + "output" : [ { "common-ref" : "Operation Result" },
> >> + { "name" : "Data Mode",
> >> + "id" : "0x10",
> >> + "mandatory" : "no",
> >> + "type" : "TLV",
> >> + "format" : "guint16",
> >> + "public-format" : "QmiCtlDataMode",
> >> + "prerequisites": [ { "common-ref" : "Success" } ] } ] },
> >> +
> >> + // *********************************************************************************
> >> { "name" : "Sync",
> >> "type" : "Message",
> >> "service" : "CTL",
> >> diff --git a/libqmi-glib/Makefile.am b/libqmi-glib/Makefile.am
> >> index b429777..ea32de8 100644
> >> --- a/libqmi-glib/Makefile.am
> >> +++ b/libqmi-glib/Makefile.am
> >> @@ -31,10 +31,10 @@ qmi-error-quarks.c: qmi-errors.h qmi-error-types.h
> >> $(top_srcdir)/build-aux/templ
> >> qmi-errors.h > $@
> >>
> >> # Enum/Flag types
> >> -ENUMS = qmi-enums.h qmi-enums-wds.h qmi-enums-dms.h qmi-enums-nas.h
> >> +ENUMS = qmi-enums.h qmi-enums-wds.h qmi-enums-dms.h qmi-enums-nas.h
> >> qmi-enums-ctl.h
> >> qmi-enum-types.h: $(ENUMS)
> >> $(top_srcdir)/build-aux/templates/qmi-enum-types-template.h
> >> $(AM_V_GEN) $(GLIB_MKENUMS) \
> >> - --fhead "#ifndef __LIBQMI_GLIB_ENUM_TYPES_H__\n#define
> >> __LIBQMI_GLIB_ENUM_TYPES_H__\n#include \"qmi-enums.h\"\n#include
> >> \"qmi-enums-wds.h\"\n#include \"qmi-enums-dms.h\"\n#include
> >> \"qmi-enums-nas.h\"\n" \
> >> + --fhead "#ifndef __LIBQMI_GLIB_ENUM_TYPES_H__\n#define
> >> __LIBQMI_GLIB_ENUM_TYPES_H__\n#include \"qmi-enums.h\"\n#include
> >> \"qmi-enums-wds.h\"\n#include \"qmi-enums-dms.h\"\n#include
> >> \"qmi-enums-nas.h\"\n#include \"qmi-enums-ctl.h\"\n" \
> >> --template $(top_srcdir)/build-aux/templates/qmi-enum-types-template.h \
> >> --ftail "#endif /* __LIBQMI_GLIB_ENUM_TYPES_H__ */\n" \
> >> $(ENUMS) > $@
> >> @@ -108,7 +108,7 @@ qmi-nas.stamp:
> >> $(top_srcdir)/data/qmi-service-nas.json $(top_srcdir)/build-aux/q
> >> qmi-device.c: qmi-error-types.h qmi-enum-types.h qmi-ctl.h qmi-dms.h
> >> qmi-wds.h qmi-nas.h
> >> qmi-client.c: qmi-error-types.h qmi-enum-types.h
> >> qmi-message.c: qmi-error-types.h qmi-enum-types.h qmi-ctl.h qmi-dms.h
> >> qmi-wds.h qmi-nas.h
> >> -qmi-ctl.h: qmi-ctl.stamp
> >> +qmi-ctl.h: qmi-ctl.stamp qmi-enums-ctl.h
> >> qmi-ctl.c: qmi-error-types.h qmi-enum-types.h qmi-ctl.h
> >> qmi-dms.h: qmi-dms.stamp qmi-enums-dms.h
> >> qmi-dms.c: qmi-error-types.h qmi-enum-types.h qmi-flags64-types.h qmi-dms.h
> >> @@ -124,6 +124,7 @@ libqmi_glib_la_SOURCES = \
> >> qmi-enums-wds.h qmi-enums-wds.c \
> >> qmi-enums-dms.h \
> >> qmi-enums-nas.h \
> >> + qmi-enums-ctl.h \
> >> qmi-enums.h qmi-enum-types.h qmi-enum-types.c qmi-flags64-types.h
> >> qmi-flags64-types.c \
> >> qmi-utils.h qmi-utils.c \
> >> qmi-message.h qmi-message.c \
> >> @@ -147,12 +148,16 @@ include_HEADERS = \
> >> qmi-message.h \
> >> qmi-device.h \
> >> qmi-client.h \
> >> - qmi-ctl.h \
> >> + qmi-enums-ctl.h qmi-ctl.h \
> >> qmi-enums-dms.h qmi-flags64-dms.h qmi-dms.h \
> >> qmi-enums-wds.h qmi-wds.h \
> >> qmi-enums-nas.h qmi-flags64-nas.h qmi-nas.h
> >>
> >> CLEANFILES = \
> >> + qmi-error-types.h qmi-error-types.c \
> >> + qmi-error-quarks.c \
> >> + qmi-enum-types.h qmi-enum-types.c \
> >> + qmi-flags64-types.h qmi-flags64-types.c \
> >> qmi-ctl.h qmi-ctl.c qmi-ctl.stamp \
> >> qmi-dms.h qmi-dms.c qmi-dms.stamp \
> >> qmi-wds.h qmi-wds.c qmi-wds.stamp \
> >> diff --git a/libqmi-glib/qmi-device.c b/libqmi-glib/qmi-device.c
> >> index 56d2122..7d6a844 100644
> >> --- a/libqmi-glib/qmi-device.c
> >> +++ b/libqmi-glib/qmi-device.c
> >> @@ -1433,6 +1433,46 @@ version_info_ready (QmiClientCtl *client_ctl,
> >> }
> >>
> >> static void
> >> +data_format_ready (QmiClientCtl *client_ctl,
> >> + GAsyncResult *res,
> >> + DeviceOpenContext *ctx)
> >> +{
> >> + QmiMessageCtlSetDataFormatOutput *output;
> >> + GError *error = NULL;
> >> + QmiCtlDataMode data_mode;
> >> +
> >> + /* Check result of the async operation */
> >> + output = qmi_client_ctl_set_data_format_finish (client_ctl, res, &error);
> >> + if (!output) {
> >> + g_simple_async_result_take_error (ctx->result, error);
> >> + device_open_context_complete_and_free (ctx);
> >> + return;
> >> + }
> >> +
> >> + /* Check result of the QMI operation */
> >> + if (!qmi_message_ctl_set_data_format_output_get_result (output, &error)) {
> >> + g_simple_async_result_take_error (ctx->result, error);
> >> + device_open_context_complete_and_free (ctx);
> >> + qmi_message_ctl_set_data_format_output_unref (output);
> >> + return;
> >> + }
> >> +
> >> + if (!qmi_message_ctl_set_data_format_output_get_data_mode(output,
> >> &data_mode, &error)) {
> >> + g_simple_async_result_take_error (ctx->result, error);
> >> + device_open_context_complete_and_free (ctx);
> >> + qmi_message_ctl_set_data_format_output_unref (output);
> >> + return;
> >> + }
> >> +
> >> + g_debug ("[%s] Data Format set to default, Mode: %s",
> >> + qmi_ctl_data_mode_get_string(data_mode));
> >> +
> >> + /* Keep on with next flags */
> >> + process_open_flags (ctx);
> >> + qmi_message_ctl_set_data_format_output_unref (output);
> >> +}
> >> +
> >> +static void
> >> process_open_flags (DeviceOpenContext *ctx)
> >> {
> >> /* Query version info? */
> >> @@ -1466,6 +1506,35 @@ process_open_flags (DeviceOpenContext *ctx)
> >> return;
> >> }
> >>
> >> + /* Set Data Mode? */
> >> + if (ctx->flags & QMI_DEVICE_OPEN_FLAGS_DATA_FORMAT) {
> >> + QmiMessageCtlSetDataFormatInput *input =
> >> qmi_message_ctl_set_data_format_input_new();
> >> +
> >> + g_debug ("[%s] Setting Data Format...",
> >> + ctx->self->priv->path_display);
> >> + ctx->flags &= ~QMI_DEVICE_OPEN_FLAGS_DATA_FORMAT;
> >> +
> >> + qmi_message_ctl_set_data_format_input_set_data_format(
> >> + input,
> >> + QMI_CTL_DATA_FORMAT_DEFAULT,
> >> + NULL);
> >> +
> >> + qmi_message_ctl_set_data_format_input_set_data_mode(
> >> + input,
> >> + QMI_CTL_DATA_MODE_ETHERNET,
> >> + NULL);
> >> +
> >> + qmi_client_ctl_set_data_format (ctx->self->priv->client_ctl,
> >> + input,
> >> + ctx->timeout,
> >> + ctx->cancellable,
> >> + (GAsyncReadyCallback)data_format_ready,
> >> + ctx);
> >> +
> >> + qmi_message_ctl_set_data_format_input_unref(input);
> >> + return;
> >> + }
> >> +
> >> /* No more flags to process, done we are */
> >> g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> >> device_open_context_complete_and_free (ctx);
> >> diff --git a/libqmi-glib/qmi-device.h b/libqmi-glib/qmi-device.h
> >> index 6e04562..38a241c 100644
> >> --- a/libqmi-glib/qmi-device.h
> >> +++ b/libqmi-glib/qmi-device.h
> >> @@ -75,13 +75,15 @@ gboolean qmi_device_is_open (QmiDevice *self);
> >> * @QMI_DEVICE_OPEN_FLAGS_NONE: No flags.
> >> * @QMI_DEVICE_OPEN_FLAGS_VERSION_INFO: Run version info check when opening.
> >> * @QMI_DEVICE_OPEN_FLAGS_SYNC: Synchronize with endpoint once the
> >> device is open. Will release any previously allocated client ID.
> >> + * @QMI_DEVICE_OPEN_FLAGS_DATA_MODE: Set Data mode once the device is open.
> >> *
> >> * Flags to specify which actions to be performed when the device is open.
> >> */
> >> typedef enum {
> >> QMI_DEVICE_OPEN_FLAGS_NONE = 0,
> >> QMI_DEVICE_OPEN_FLAGS_VERSION_INFO = 1 << 0,
> >> - QMI_DEVICE_OPEN_FLAGS_SYNC = 1 << 1
> >> + QMI_DEVICE_OPEN_FLAGS_SYNC = 1 << 1,
> >> + QMI_DEVICE_OPEN_FLAGS_DATA_FORMAT = 1 << 2
> >> } QmiDeviceOpenFlags;
> >>
> >> void qmi_device_open (QmiDevice *self,
> >> diff --git a/libqmi-glib/qmi-enums-ctl.h b/libqmi-glib/qmi-enums-ctl.h
> >> new file mode 100644
> >> index 0000000..7f06eff
> >> --- /dev/null
> >> +++ b/libqmi-glib/qmi-enums-ctl.h
> >> @@ -0,0 +1,55 @@
> >> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
> >> +/*
> >> + * libqmi-glib -- GLib/GIO based library to control QMI devices
> >> + *
> >> + * This library is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU Lesser General Public
> >> + * License as published by the Free Software Foundation; either
> >> + * version 2 of the License, or (at your option) any later version.
> >> + *
> >> + * This library is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >> + * Lesser General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU Lesser General Public
> >> + * License along with this library; if not, write to the
> >> + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> >> + * Boston, MA 02110-1301 USA.
> >> + *
> >> + * Copyright (C) 2012 Lanedo GmbH <aleksander at lanedo.com>
> >> + */
> >> +
> >> +#ifndef _LIBQMI_GLIB_QMI_ENUMS_CTL_H_
> >> +#define _LIBQMI_GLIB_QMI_ENUMS_CTL_H_
> >> +
> >> +#include <glib.h>
> >> +
> >> +/*****************************************************************************/
> >> +/* Helper enums for the 'QMI CTL Set Data Format' message */
> >> +
> >> +/**
> >> + * QmiCtlDataFormat:
> >> + * @QMI_CTL_DATA_FORMAT_DEFAULT: Default
> >> + * @QMI_CTL_DATA_FORMAT_QOS: QoS Header Present
> >> + *
> >> + * Data format.
> >> + */
> >> +typedef enum {
> >> + QMI_CTL_DATA_FORMAT_DEFAULT = 0,
> >> + QMI_CTL_DATA_FORMAT_QOS = 1
> >> +} QmiCtlDataFormat;
> >> +
> >> +/**
> >> + * QmiCtlDataMode:
> >> + * @QMI_CTL_DATA_MODE_RAW_IP: Raw IP Mode
> >> + * @QMI_CTL_DATA_MODE_ETHERNET: Ethernet Mode
> >> + *
> >> + * Data mode.
> >> + */
> >> +typedef enum {
> >> + QMI_CTL_DATA_MODE_RAW_IP = 0x02,
> >> + QMI_CTL_DATA_MODE_ETHERNET = 0x01
> >> +} QmiCtlDataMode;
> >> +
> >> +#endif /* _LIBQMI_GLIB_QMI_ENUMS_CTL_H_ */
> >> diff --git a/libqmi-glib/qmi-enums.h b/libqmi-glib/qmi-enums.h
> >> index 5710eba..8129193 100644
> >> --- a/libqmi-glib/qmi-enums.h
> >> +++ b/libqmi-glib/qmi-enums.h
> >> @@ -21,6 +21,7 @@
> >> */
> >>
> >> #include "qmi-enums-wds.h"
> >> +#include "qmi-enums-ctl.h"
> >>
> >> #ifndef _LIBQMI_GLIB_QMI_ENUMS_H_
> >> #define _LIBQMI_GLIB_QMI_ENUMS_H_
> >>
> >
> >
> > --
> > Aleksander
> _______________________________________________
> libqmi-devel mailing list
> libqmi-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libqmi-devel
More information about the libqmi-devel
mailing list