Hi Christophe, On 01/19/2017 02:15 AM, Christophe Ronco wrote: > Add a way to get and set data format expected by kernel device driver. > This is inspired by what is done in qmicli (package libqmi). > It does not use QMI protocol but a sysfs exported by kernel driver. > To use this feature, kernel version must be equal or more than 4.5. > --- > drivers/qmimodem/qmi.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/qmimodem/qmi.h | 10 +++ > 2 files changed, 210 insertions(+) > > diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c > index 0080f250..bf3c7407 100644 > --- a/drivers/qmimodem/qmi.c > +++ b/drivers/qmimodem/qmi.c > @@ -26,6 +26,8 @@ > #define _GNU_SOURCE > #include > #include > +#include > +#include > #include > #include > #include > @@ -33,6 +35,8 @@ > > #include > > +#include > + > #include "qmi.h" > #include "ctl.h" > > @@ -1234,6 +1238,202 @@ bool qmi_device_shutdown(struct qmi_device *device, qmi_shutdown_func_t func, > return true; > } > > +static bool get_device_file_name(struct qmi_device *device, > + char *file_name, int size) > +{ > + pid_t pid; > + char temp[100]; > + ssize_t result; > + > + if (size <= 0) > + return false; > + > + pid = getpid(); > + > + snprintf(temp, 100, "/proc/%d/fd/%d", (int) pid, device->fd); > + temp[99] = 0; > + > + result = readlink(temp, file_name, size - 1); > + > + if ((result == -1) || (result >= size - 1)) { Due to operator precedence, both sets of inner parens are not really necessary. > + DBG("Error %d in readlink", errno); > + return false; > + } > + > + file_name[result] = 0; > + > + return true; > +} > + > +static char *get_first_dir_in_directory(char *dir_path) > +{ > + DIR *dir; > + struct dirent *dir_entry; > + char *dir_name = NULL; > + > + dir = opendir(dir_path); > + > + if (!dir) > + return NULL; > + > + dir_entry = readdir(dir); > + > + while ((dir_entry != NULL)) { > + if ((dir_entry->d_type == DT_DIR) && > + (strcmp(dir_entry->d_name, ".") != 0) && > + (strcmp(dir_entry->d_name, "..") != 0)) { inner parens should not be necessary here either. > + dir_name = strdup(dir_entry->d_name); Might as well use g_strdup here for consistency > + break; > + } > + > + dir_entry = readdir(dir); > + } > + > + closedir(dir); > + return dir_name; > +} > + > +static char *get_device_iface_name(struct qmi_device *device) Can we name this get_device_interface? > +{ > + gchar * driver_names[] = { "usbmisc", "usb" }; > + guint i; use char & unsigned int here. We try to avoid GLib types whenever possible. > + char file_path[100]; Any reason this isn't PATH_MAX? What's the file_path expected to be? > + char *file_name; > + char *int_name = NULL; > + > + if (!get_device_file_name(device, file_path, 100)) 100 -> sizeof(file_path)? > + return NULL; > + > + file_name = basename(file_path); > + > + for (i = 0; i < G_N_ELEMENTS(driver_names) && !int_name; i++) { > + gchar *sysfs_path; > + > + sysfs_path = g_strdup_printf("/sys/class/%s/%s/device/net/", > + driver_names[i], file_name); > + int_name = get_first_dir_in_directory(sysfs_path); > + g_free(sysfs_path); > + } > + > + return int_name; > +} > + > +enum qmi_device_expected_data_format qmi_device_get_expected_data_format( > + struct qmi_device *device) > +{ > + gchar *sysfs_path = NULL; char * > + char *int_name = NULL; > + FILE *f = NULL; We don't use FILE operations and prefer to use the low-level versions (open/close/read/write) > + gchar value; char > + enum qmi_device_expected_data_format expected = > + QMI_DEVICE_EXPECTED_DATA_FORMAT_UNKNOWN; > + > + if (!device) > + goto done; > + > + int_name = get_device_iface_name(device); How about a better name here. E.g. interface = get_device_interface(device) > + > + if (!int_name) { > + DBG("Error while getting interface name"); > + goto done; > + } > + > + /* Build sysfs file path and open it */ > + sysfs_path = g_strdup_printf("/sys/class/net/%s/qmi/raw_ip", int_name); > + > + f = fopen(sysfs_path, "r"); > + if (f == NULL) { > + /* maybe not supported by kernel */ > + DBG("Error %d in fopen(%s)", errno, sysfs_path); > + goto done; > + } > + > + if (fread(&value, 1, 1, f) != 1) { > + DBG("Error %d in fread(%s)", errno, sysfs_path); > + goto done; > + } Can we replace fopen/fread by open/read? > + > + if (value == 'Y') > + expected = QMI_DEVICE_EXPECTED_DATA_FORMAT_RAW_IP; > + else if (value == 'N') > + expected = QMI_DEVICE_EXPECTED_DATA_FORMAT_802_3; > + else > + DBG("Unexpected sysfs file contents"); > + > +done: > + if (f) > + fclose(f); > + > + if (sysfs_path) > + g_free(sysfs_path); > + > + if (int_name) > + free(int_name); g_free if you use g_strdup. > + > + return expected; > +} > + > +bool qmi_device_set_expected_data_format(struct qmi_device *device, > + enum qmi_device_expected_data_format format) > +{ > + bool res = false; > + gchar *sysfs_path = NULL; char > + char *int_name = NULL; char *interface > + FILE *f = NULL; > + gchar value; char > + > + if (!device) > + goto done; > + > + switch (format) { > + case QMI_DEVICE_EXPECTED_DATA_FORMAT_802_3: > + value = 'N'; > + break; > + case QMI_DEVICE_EXPECTED_DATA_FORMAT_RAW_IP: > + value = 'Y'; > + break; > + default: > + DBG("Unhandled firmat: %d", (int) format); > + goto done; > + } > + > + int_name = get_device_iface_name(device); interface = ? > + > + if (!int_name) { > + DBG("Error while getting interface name"); > + goto done; > + } > + > + /* Build sysfs file path and open it */ > + sysfs_path = g_strdup_printf("/sys/class/net/%s/qmi/raw_ip", int_name); > + > + f = fopen(sysfs_path, "w"); > + if (f == NULL) { > + /* maybe not supported by kernel */ > + DBG("Error %d in fopen(%s)", errno, sysfs_path); > + goto done; > + } > + > + if (fwrite(&value, 1, 1, f) != 1) { > + DBG("Error %d in fwrite(%s)", errno, sysfs_path); > + goto done; > + } fopen, fwrite, fclose -> open/write/close > + > + res = true; > + > +done: > + if (f) > + fclose(f); > + > + if (sysfs_path) > + g_free(sysfs_path); > + > + if (int_name) > + free(int_name); > + > + return res; > +} > + > struct qmi_param *qmi_param_new(void) > { > struct qmi_param *param; > diff --git a/drivers/qmimodem/qmi.h b/drivers/qmimodem/qmi.h > index dca115c4..ac19fe01 100644 > --- a/drivers/qmimodem/qmi.h > +++ b/drivers/qmimodem/qmi.h > @@ -47,6 +47,12 @@ > #define QMI_SERVICE_RMS 225 /* Remote management service */ > #define QMI_SERVICE_OMA 226 /* OMA device management service */ > > +enum qmi_device_expected_data_format { > + QMI_DEVICE_EXPECTED_DATA_FORMAT_UNKNOWN, > + QMI_DEVICE_EXPECTED_DATA_FORMAT_802_3, > + QMI_DEVICE_EXPECTED_DATA_FORMAT_RAW_IP, > +}; > + > struct qmi_version { > uint8_t type; > uint16_t major; > @@ -82,6 +88,10 @@ bool qmi_device_discover(struct qmi_device *device, qmi_discover_func_t func, > bool qmi_device_shutdown(struct qmi_device *device, qmi_shutdown_func_t func, > void *user_data, qmi_destroy_func_t destroy); > > +enum qmi_device_expected_data_format qmi_device_get_expected_data_format( > + struct qmi_device *device); > +bool qmi_device_set_expected_data_format(struct qmi_device *device, > + enum qmi_device_expected_data_format format); > > struct qmi_param; > > Regards, -Denis