Hi Varun,
On 06/18/2019 08:05 AM, Varun Gargi wrote:
Adding support for enumerating PCIe types of modems in ofono
---
plugins/udevng.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 168 insertions(+), 22 deletions(-)
mode change 100644 => 100755 plugins/udevng.c
Please don't touch the mode
diff --git a/plugins/udevng.c b/plugins/udevng.c
old mode 100644
new mode 100755
index 4b420dc..79eec42
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -41,6 +41,7 @@
enum modem_type {
MODEM_TYPE_USB,
MODEM_TYPE_SERIAL,
+ MODEM_TYPE_PCIE,
};
struct modem_info {
@@ -1198,26 +1199,39 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
info->interface, info->number, info->label,
info->sysattr, info->subsystem);
- if (g_strcmp0(modem->model,"095a") == 0) {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "06") == 0)
- net = info->devnode;
- if (g_strcmp0(info->number, "08") == 0)
- net2 = info->devnode;
- if (g_strcmp0(info->number, "0a") == 0)
- net3 = info->devnode;
- }
- } else {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "02") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- net = info->devnode;
+ if (g_strcmp0(info->subsystem, "pci") == 0) {
+ mdm = "/dev/iat";
+ net = "/dev/inm0";
+ net2 = "/dev/inm1";
+ net3 = "/dev/inm2";
These seem to be quite hardcoded, what if multiple modems are inserted?
+ ofono_modem_set_string(modem->modem,
+ "CtrlPath", "/PCIE/IOSM/CTRL/1");
+ ofono_modem_set_string(modem->modem, "DataPath",
+ "/PCIE/IOSM/IPS/");
This seems to treat all PCIE modems the same way. Is there a
possibility of this changing in the future? E.g. should you actually
check the vid/pid or limit which devices this applies to some other way?
It would be preferable if we had to 'opt-in' devices along the way,
rather than having to deal with a new device being detected and
categorized incorrectly.
+ } else { /* For USB */
+ if (g_strcmp0(modem->model,"095a") == 0) {
+ if (g_strcmp0(info->subsystem, "tty") == 0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ mdm = info->devnode;
+ } else if (g_strcmp0(info->subsystem, "net") ==
0) {
+ if (g_strcmp0(info->number, "06") == 0)
+ net = info->devnode;
+ if (g_strcmp0(info->number, "08") == 0)
+ net2 = info->devnode;
+ if (g_strcmp0(info->number, "0a") == 0)
+ net3 = info->devnode;
+ }
+ } else {
+ if (g_strcmp0(info->subsystem, "tty") == 0) {
+ if (g_strcmp0(info->number, "02") == 0)
+ mdm = info->devnode;
+ } else if (g_strcmp0(info->subsystem, "net") ==
0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ net = info->devnode;
+ }
}
+ ofono_modem_set_string(modem->modem, "CtrlPath", "/USBCDC/0");
+ ofono_modem_set_string(modem->modem, "DataPath", "/USBHS/NCM/");
Coding style is all wrong here
}
}
@@ -1235,9 +1249,6 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
if (net3)
ofono_modem_set_string(modem->modem, "NetworkInterface3", net3);
- ofono_modem_set_string(modem->modem, "CtrlPath", "/USBCDC/0");
- ofono_modem_set_string(modem->modem, "DataPath", "/USBHS/NCM/");
-
return TRUE;
}
@@ -1406,6 +1417,7 @@ static void destroy_modem(gpointer data)
switch (modem->type) {
case MODEM_TYPE_USB:
+ case MODEM_TYPE_PCIE:
for (list = modem->devices; list; list = list->next) {
struct device_info *info = list->data;
@@ -1436,6 +1448,7 @@ static gboolean check_remove(gpointer key, gpointer value, gpointer user_data)
switch (modem->type) {
case MODEM_TYPE_USB:
+ case MODEM_TYPE_PCIE:
for (list = modem->devices; list; list = list->next) {
struct device_info *info = list->data;
@@ -1565,6 +1578,86 @@ static void add_serial_device(struct udev_device *dev)
modem->serial = info;
}
+static void add_pci_device(const char *syspath, const char *devname,
+ const char *driver, const char *vendor,
+ const char *model, struct udev_device *device)
+{
+ const char *devpath, *devnode , *interface , *number;
+ const char *label, *sysattr, *subsystem;
+ struct modem_info *modem;
+ struct device_info *info;
+ struct udev_device *parent;
+
+ devpath = udev_device_get_syspath(device);
+
+ if (devpath == NULL)
+ return;
+
+ devnode = udev_device_get_devnode(device);
+
+ if (devnode == NULL) {
+ devnode = udev_device_get_property_value(device, "INTERFACE");
+ DBG("devnode = %s", devnode );
+ }
+
+ modem = g_hash_table_lookup(modem_list, syspath);
+
+ if (modem == NULL) {
+ modem = g_try_new0(struct modem_info, 1);
+
+ if (modem == NULL)
+ return;
+
+ modem->type = MODEM_TYPE_PCIE;
+ modem->syspath = g_strdup(syspath);
+ modem->devname = g_strdup(devname);
+ modem->driver = g_strdup(driver);
+ modem->vendor = g_strdup(vendor);
+ modem->model = g_strdup(model);
+ modem->sysattr = get_sysattr(driver);
+
+ g_hash_table_replace(modem_list, modem->syspath, modem);
+ }
+
+ interface = udev_device_get_property_value(device, "INTERFACE");
+ /* If environment variable is not set, get value from attributes (or
parent's ones) */
+ number = udev_device_get_sysattr_value(device, "bInterfaceNumber");
+
+ if (number == NULL) {
+ parent = udev_device_get_parent(device);
+ number = udev_device_get_sysattr_value(parent,
"bInterfaceNumber");
+ }
+
+ label = udev_device_get_property_value(device, "OFONO_LABEL");
+
+ if (!label)
+ label = udev_device_get_property_value(device, "OFONO_LABEL");
+
+ subsystem = udev_device_get_subsystem(device);
+
+ if (modem->sysattr != NULL)
+ sysattr = udev_device_get_sysattr_value(device, modem->sysattr);
+ else
+ sysattr = NULL;
+
+ DBG("%s (%s) %s [%s] ==> %s %s", devnode, driver,
+ interface, number, label, sysattr);
+ info = g_try_new0(struct device_info, 1);
+
+ if (info == NULL)
+ return;
+
+ info->devpath = g_strdup(devpath);
+ info->devnode = g_strdup(devnode);
+ info->interface = g_strdup(interface);
+ info->number = g_strdup(number);
+ info->label = g_strdup(label);
+ info->sysattr = g_strdup(sysattr);
+ info->subsystem = g_strdup(subsystem);
+
+ modem->devices = g_slist_insert_sorted(modem->devices, info,
+ compare_device);
This function seems to be mostly copy-pasted version of add_device with
a few minor variations. Can we commonize any code between these two?
+}
static void add_device(const char *syspath, const char *devname,
const char *driver, const char *vendor,
@@ -1594,8 +1687,10 @@ static void add_device(const char *syspath, const char
*devname,
return;
modem = g_hash_table_lookup(modem_list, syspath);
+
if (modem == NULL) {
modem = g_try_new0(struct modem_info, 1);
+
if (modem == NULL)
return;
Superfluous changes?
@@ -1739,6 +1834,7 @@ static struct {
{ "telit", "cdc_acm", "1bc7", "0036" },
{ "xmm7xxx", "cdc_acm", "8087" },
{ "xmm7xxx", "cdc_ncm", "8087" },
+ { "xmm7xxx", "imc_ipc", "0x8086", "0x7560"},
Something is funky with the formatting here
{ }
};
@@ -1825,6 +1921,53 @@ static void check_usb_device(struct udev_device *device)
add_device(syspath, devname, driver, vendor, model, device);
}
+static void check_pci_device(struct udev_device *device)
+{
+ const char *syspath, *devname , *driver;
+ const char *vendor = NULL, *model = NULL, *drv = NULL;
+ unsigned int i;
+
+ syspath = udev_device_get_syspath(device);
+
+ if (syspath == NULL)
+ return;
+
+ devname = udev_device_get_devnode(device);
+ vendor = udev_device_get_sysattr_value(device, "vendor");
+ model = udev_device_get_sysattr_value(device, "device");
+ driver = udev_device_get_property_value(device, "OFONO_DRIVER");
+ drv = udev_device_get_property_value(device, "DRIVER");
+ DBG("%s [%s:%s]", drv, vendor, model);
+
+ if (vendor == NULL || model == NULL)
+ return;
+
+ for (i = 0; vendor_list[i].driver; i++) {
+ if (( drv != NULL) && (g_str_equal(vendor_list[i].drv, drv) ==
TRUE)) {
+ DBG("vendor_list[%d].drv = %s",i, vendor_list[i].drv );
+ } else {
+ continue;
+ }
Not our style, also {} are not needed
+
+ if (vendor_list[i].vid) {
+ if (!g_str_equal(vendor_list[i].vid, vendor))
+ continue;
+ }
+
+ if (vendor_list[i].pid) {
+ if (!g_str_equal(vendor_list[i].pid, model))
+ continue;
+ }
+ driver = vendor_list[i].driver;
Not our style
+ }
+
+ if (driver == NULL)
+ return;
+ else
+ DBG("Driver Found %s", driver);
+
+ add_pci_device(syspath, devname, driver, vendor, model, device);
+}
doc/coding-style.txt item M1
static void check_device(struct udev_device *device)
{
const char *bus;
@@ -1839,6 +1982,8 @@ static void check_device(struct udev_device *device)
if ((g_str_equal(bus, "usb") == TRUE) ||
(g_str_equal(bus, "usbmisc") == TRUE))
check_usb_device(device);
+ else if (g_str_equal(bus, "pci") == TRUE)
+ check_pci_device(device);
else
add_serial_device(device);
@@ -1899,6 +2044,7 @@ static void enumerate_devices(struct udev *context)
udev_enumerate_add_match_subsystem(enumerate, "usbmisc");
udev_enumerate_add_match_subsystem(enumerate, "net");
udev_enumerate_add_match_subsystem(enumerate, "hsi");
+ udev_enumerate_add_match_subsystem(enumerate, "pci");
udev_enumerate_scan_devices(enumerate);
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono