All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH udev v2 0/5] Device handling cleanup series
@ 2017-03-28 13:18 Jonas Bonn
  2017-03-28 13:18 ` [PATCH udev v2 1/5] udevng: add serial device handling functions Jonas Bonn
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Jonas Bonn @ 2017-03-28 13:18 UTC (permalink / raw)
  To: ofono

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

This is version 2 of the udev handling cleanups.

Changes since v1:

* Reworked patch 1/5 (was 11/15) to handle 'legacy' devices in a simpler
  manner.
* Addressed comments from Denis

Original blurb follows:

This is a series of incremental cleanups to the libudev device handling
code.

- the early patches in the series are all good cleanups that can be
  taken irregardless of whether the last patches are desired
- the last patches are more intrusive as they attempt to move the
  functionality of udev.c into udevng.c

Somebody with access to old hardware really needs to take a look at
this series to see if the migration to udevng works properly.  I do
not have any such hardware, myself.  I suspect that there's not many
such devices in circulation; my hope is that this legacy code works
for now and can be ripped out in time.

In any case, if someone has such old hardware, hopefully these changes
are sufficiently correct that the last bugs can shake themselves out
easily... we'll see.


Jonas Bonn (5):
  udevng: add serial device handling functions
  udevng: match on the hsi subsystem for legacy devices
  udevng: hook up legacy devices
  udevng: get properties from interface
  plugins: remove udev module

 Makefile.am      |   2 -
 plugins/udev.c   | 431 -------------------------------------------------------
 plugins/udevng.c | 266 ++++++++++++++++++++++++++++++++--
 3 files changed, 255 insertions(+), 444 deletions(-)
 delete mode 100644 plugins/udev.c

-- 
2.9.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH udev v2 1/5] udevng: add serial device handling functions
  2017-03-28 13:18 [PATCH udev v2 0/5] Device handling cleanup series Jonas Bonn
@ 2017-03-28 13:18 ` Jonas Bonn
  2017-03-28 16:46   ` Denis Kenzior
  2017-03-28 13:18 ` [PATCH udev v2 2/5] udevng: match on the hsi subsystem for legacy devices Jonas Bonn
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Jonas Bonn @ 2017-03-28 13:18 UTC (permalink / raw)
  To: ofono

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

This adds, but does not hook up, support for simple serial modems.  These
modems generally have only a single device node so are simpler than the
USB devices which generally have different device nodes for different
features.  These modems are currently handled by udev.c, but this
functionality will allow to remove that module completely.

- A new "device_info" type is created called serial_device_info
- the function add_serial_device sets up a modem_info structure and a
  serial_device_info for the device
- A reference to the device's udev node is saved in the device info
- The device driver is retrieved from the OFONO_DRIVER environment variable
  which needs to be set up by some udev rule
- Setup functions are added for these types of devices: a common function
  setup_serial_modem covers the generic (simple) case, whereas modems
  with special requirements are given their own setup functions to handle
  the special bits
- Modem destroy needs to know the "device_info" type in order to clean
  up properly, so a 'type' value is set on these structures for this
  purpose
---
 plugins/udevng.c | 242 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 234 insertions(+), 8 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 7a29be6..7ac185c 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -48,7 +48,13 @@ struct modem_info {
 	const char *sysattr;
 };
 
+enum {
+	DEVICE_TYPE_USB,
+	DEVICE_TYPE_SERIAL,
+} DeviceType;
+
 struct device_info {
+	int type;
 	char *devpath;
 	char *devnode;
 	char *interface;
@@ -58,6 +64,14 @@ struct device_info {
 	char *subsystem;
 };
 
+struct serial_modem_info {
+	int type;
+	char *devpath;
+	char *devnode;
+	char *subsystem;
+	struct udev_device* dev;
+};
+
 static gboolean setup_isi(struct modem_info *modem)
 {
 	const char *node = NULL;
@@ -867,6 +881,100 @@ static gboolean setup_quectel(struct modem_info *modem)
 	return TRUE;
 }
 
+static gboolean setup_serial_modem(struct modem_info* modem)
+{
+	struct serial_modem_info* info;
+
+	info = modem->devices->data;
+
+	ofono_modem_set_string(modem->modem, "Device", info->devnode);
+
+	return TRUE;
+}
+
+static gboolean setup_tc65(struct modem_info* modem)
+{
+	ofono_modem_set_driver(modem->modem, "cinterion");
+
+	return setup_serial_modem(modem);
+}
+
+static gboolean setup_ehs6(struct modem_info* modem)
+{
+	ofono_modem_set_driver(modem->modem, "cinterion");
+
+	return setup_serial_modem(modem);
+}
+
+static gboolean setup_ifx(struct modem_info* modem)
+{
+	struct serial_modem_info* info;
+	const char *value;
+
+	info = modem->devices->data;
+
+	value = udev_device_get_property_value(info->dev, "OFONO_IFX_LDISC");
+	if (value)
+		ofono_modem_set_string(modem->modem, "LineDiscipline", value);
+
+	value = udev_device_get_property_value(info->dev, "OFONO_IFX_AUDIO");
+	if (value)
+		ofono_modem_set_string(modem->modem, "AudioSetting", value);
+
+	value = udev_device_get_property_value(info->dev, "OFONO_IFX_LOOPBACK");
+	if (value)
+		ofono_modem_set_string(modem->modem, "AudioLoopback", value);
+
+	ofono_modem_set_string(modem->modem, "Device", info->devnode);
+
+	return TRUE;
+}
+
+static gboolean setup_wavecom(struct modem_info* modem)
+{
+	struct serial_modem_info* info;
+	const char *value;
+
+	info = modem->devices->data;
+
+	value = udev_device_get_property_value(info->dev,
+						"OFONO_WAVECOM_MODEL");
+	if (value)
+		ofono_modem_set_string(modem->modem, "Model", value);
+
+	ofono_modem_set_string(modem->modem, "Device", info->devnode);
+
+	return TRUE;
+}
+
+static gboolean setup_isi_serial(struct modem_info* modem)
+{
+	struct serial_modem_info* info;
+	const char *value;
+
+	info = modem->devices->data;
+
+	if (g_strcmp0(udev_device_get_subsystem(info->dev), "net") != 0)
+		return FALSE;
+
+	value = udev_device_get_sysattr_value(info->dev, "type");
+	if (g_strcmp0(value, "820") != 0)
+		return FALSE;
+
+	/* OK, we want this device to be a modem */
+	value = udev_device_get_sysname(info->dev);
+	if (value)
+		ofono_modem_set_string(modem->modem, "Interface", value);
+
+	value = udev_device_get_property_value(info->dev, "OFONO_ISI_ADDRESS");
+	if (value)
+		ofono_modem_set_integer(modem->modem, "Address", atoi(value));
+
+	ofono_modem_set_string(modem->modem, "Device", info->devnode);
+
+	return TRUE;
+}
+
 static gboolean setup_ublox(struct modem_info *modem)
 {
 	const char *aux = NULL, *mdm = NULL, *net = NULL;
@@ -997,6 +1105,17 @@ static struct {
 	{ "quectel",	setup_quectel	},
 	{ "ublox",	setup_ublox	},
 	{ "gemalto",	setup_gemalto	},
+	/* Following are non-USB modems */
+	{ "ifx",	setup_ifx		},
+	{ "u8500",	setup_isi_serial	},
+	{ "n900",	setup_isi_serial	},
+	{ "calypso",	setup_serial_modem	},
+	{ "cinterion",	setup_serial_modem	},
+	{ "nokiacdma",	setup_serial_modem	},
+	{ "sim900",	setup_serial_modem	},
+	{ "wavecom",	setup_wavecom		},
+	{ "tc65",	setup_tc65		},
+	{ "ehs6",	setup_ehs6		},
 	{ }
 };
 
@@ -1028,14 +1147,25 @@ static void destroy_modem(gpointer data)
 
 		DBG("%s", info->devnode);
 
-		g_free(info->devpath);
-		g_free(info->devnode);
-		g_free(info->interface);
-		g_free(info->number);
-		g_free(info->label);
-		g_free(info->sysattr);
-		g_free(info->subsystem);
-		g_free(info);
+		if (info->type == DEVICE_TYPE_USB) {
+			g_free(info->devpath);
+			g_free(info->devnode);
+			g_free(info->interface);
+			g_free(info->number);
+			g_free(info->label);
+			g_free(info->sysattr);
+			g_free(info->subsystem);
+			g_free(info);
+		} else if (info->type == DEVICE_TYPE_SERIAL) {
+			struct serial_modem_info* sinfo;
+			sinfo = (struct serial_modem_info*) info;
+			g_free(sinfo->devpath);
+			g_free(sinfo->devnode);
+			g_free(sinfo->subsystem);
+			udev_device_unref(sinfo->dev);
+			g_free(sinfo);
+		}
+
 
 		list->data = NULL;
 	}
@@ -1088,6 +1218,101 @@ static gint compare_device(gconstpointer a, gconstpointer b)
 	return g_strcmp0(info1->number, info2->number);
 }
 
+/*
+ * Here we try to find the "modem device".
+ *
+ * In this variant we identify the "modem device" as simply the device
+ * that has the OFONO_DRIVER property.  If the device node doesn't
+ * have this property itself, then we do a brute force search for it
+ * through the device hierarchy. 
+ *
+ */
+static struct udev_device* get_legacy_modem_device(struct udev_device *dev)
+{
+	const char* driver;
+
+	while (dev) {
+		driver = udev_device_get_property_value(dev, "OFONO_DRIVER");
+		if (driver)
+			return dev;
+		dev = udev_device_get_parent(dev);
+	}
+
+	return NULL; 
+}
+
+/*
+ * Add 'legacy' device
+ *
+ * The term legacy is a bit misleading, but this adds devices according
+ * to the original ofono model.
+ *
+ * - We cannot assume that these are USB devices
+ * - The modem consists of only a single interface
+ * - The device must have an OFONO_DRIVER property from udev
+ */
+static void add_serial_device(struct udev_device *dev)
+{
+	const char *syspath, *devpath, *devname, *devnode;
+	struct modem_info *modem;
+	struct serial_modem_info *info;
+	const char* vendor = NULL;
+	const char* model = NULL;
+	const char *subsystem;
+	struct udev_device* mdev;
+	const char* driver;
+
+	mdev = get_legacy_modem_device(dev);
+	if (!mdev) {
+		DBG("Device is missing required OFONO_DRIVER property");
+		return;
+	}
+
+	driver = udev_device_get_property_value(mdev, "OFONO_DRIVER");
+
+	syspath = udev_device_get_syspath(mdev);
+	devname = udev_device_get_devnode(mdev);
+	devpath = udev_device_get_devpath(mdev);
+
+	devnode = udev_device_get_devnode(dev);
+
+	if (!syspath || !devname || !devpath || !devnode)
+		return;
+
+	modem = g_hash_table_lookup(modem_list, syspath);
+	if (modem == NULL) {
+		modem = g_try_new0(struct modem_info, 1);
+		if (modem == NULL)
+			return;
+
+		modem->syspath = g_strdup(syspath);
+		modem->devname = g_strdup(devname);
+		modem->driver = g_strdup("legacy");
+		modem->vendor = g_strdup(vendor);
+		modem->model = g_strdup(model);
+
+		g_hash_table_replace(modem_list, modem->syspath, modem);
+	}
+
+	subsystem = udev_device_get_subsystem(dev);
+
+	DBG("%s", syspath);
+	DBG("%s", devpath);
+	DBG("%s (%s)", devnode, driver);
+
+	info = g_try_new0(struct serial_modem_info, 1);
+	if (info == NULL)
+		return;
+
+	info->type = DEVICE_TYPE_SERIAL;
+	info->devpath = g_strdup(devpath);
+	info->devnode = g_strdup(devnode);
+	info->subsystem = g_strdup(subsystem);
+	info->dev = udev_device_ref(dev);
+
+	modem->devices = g_slist_append(modem->devices, info);
+}
+
 static void add_device(const char *syspath, const char *devname,
 			const char *driver, const char *vendor,
 			const char *model, struct udev_device *device)
@@ -1164,6 +1389,7 @@ static void add_device(const char *syspath, const char *devname,
 	if (info == NULL)
 		return;
 
+	info->type = DEVICE_TYPE_USB;
 	info->devpath = g_strdup(devpath);
 	info->devnode = g_strdup(devnode);
 	info->interface = g_strdup(interface);
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH udev v2 2/5] udevng: match on the hsi subsystem for legacy devices
  2017-03-28 13:18 [PATCH udev v2 0/5] Device handling cleanup series Jonas Bonn
  2017-03-28 13:18 ` [PATCH udev v2 1/5] udevng: add serial device handling functions Jonas Bonn
@ 2017-03-28 13:18 ` Jonas Bonn
  2017-03-28 13:18 ` [PATCH udev v2 3/5] udevng: hook up " Jonas Bonn
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Jonas Bonn @ 2017-03-28 13:18 UTC (permalink / raw)
  To: ofono

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

---
 plugins/udevng.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 7ac185c..4ff91d5 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1606,6 +1606,7 @@ static void enumerate_devices(struct udev *context)
 	udev_enumerate_add_match_subsystem(enumerate, "usb");
 	udev_enumerate_add_match_subsystem(enumerate, "usbmisc");
 	udev_enumerate_add_match_subsystem(enumerate, "net");
+	udev_enumerate_add_match_subsystem(enumerate, "hsi");
 
 	udev_enumerate_scan_devices(enumerate);
 
@@ -1730,6 +1731,7 @@ static int detect_init(void)
 	udev_monitor_filter_add_match_subsystem_devtype(udev_mon,
 							"usbmisc", NULL);
 	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "net", NULL);
+	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "hsi", NULL);
 
 	udev_monitor_filter_update(udev_mon);
 
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH udev v2 3/5] udevng: hook up legacy devices
  2017-03-28 13:18 [PATCH udev v2 0/5] Device handling cleanup series Jonas Bonn
  2017-03-28 13:18 ` [PATCH udev v2 1/5] udevng: add serial device handling functions Jonas Bonn
  2017-03-28 13:18 ` [PATCH udev v2 2/5] udevng: match on the hsi subsystem for legacy devices Jonas Bonn
@ 2017-03-28 13:18 ` Jonas Bonn
  2017-03-28 13:18 ` [PATCH udev v2 4/5] udevng: get properties from interface Jonas Bonn
  2017-03-28 13:18 ` [PATCH udev v2 5/5] plugins: remove udev module Jonas Bonn
  4 siblings, 0 replies; 8+ messages in thread
From: Jonas Bonn @ 2017-03-28 13:18 UTC (permalink / raw)
  To: ofono

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

...and disable old udev code by shorting it out in it's init() function.

The check_device function is augmented to differentiate between USB
and serial devices:

- if the device sits on a USB bus, the device is handled as before
- if not, an attempt is made to handle the device as a serial device
---
 plugins/udev.c   | 2 ++
 plugins/udevng.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/plugins/udev.c b/plugins/udev.c
index 92451d2..564ede9 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -365,6 +365,8 @@ static void udev_start(void)
 
 static int udev_init(void)
 {
+	return 0;
+
 	devpath_list = g_hash_table_new_full(g_str_hash, g_str_equal,
 						g_free, g_free);
 	if (devpath_list == NULL) {
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 4ff91d5..1e84aa4 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1556,6 +1556,9 @@ 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
+		add_serial_device(device);
+
 }
 
 static gboolean create_modem(gpointer key, gpointer value, gpointer user_data)
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH udev v2 4/5] udevng: get properties from interface
  2017-03-28 13:18 [PATCH udev v2 0/5] Device handling cleanup series Jonas Bonn
                   ` (2 preceding siblings ...)
  2017-03-28 13:18 ` [PATCH udev v2 3/5] udevng: hook up " Jonas Bonn
@ 2017-03-28 13:18 ` Jonas Bonn
  2017-03-28 16:51   ` Denis Kenzior
  2017-03-28 13:18 ` [PATCH udev v2 5/5] plugins: remove udev module Jonas Bonn
  4 siblings, 1 reply; 8+ messages in thread
From: Jonas Bonn @ 2017-03-28 13:18 UTC (permalink / raw)
  To: ofono

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

Device properties are generally on the device, on the USB interface
descriptor, or the on the USB device descriptor.
---
 plugins/udevng.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 1e84aa4..6b76d30 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1317,7 +1317,7 @@ static void add_device(const char *syspath, const char *devname,
 			const char *driver, const char *vendor,
 			const char *model, struct udev_device *device)
 {
-	struct udev_device *intf;
+	struct udev_device *usb_interface;
 	const char *devpath, *devnode, *interface, *number;
 	const char *label, *sysattr, *subsystem;
 	struct modem_info *modem;
@@ -1335,9 +1335,9 @@ static void add_device(const char *syspath, const char *devname,
 			return;
 	}
 
-	intf = udev_device_get_parent_with_subsystem_devtype(device,
+	usb_interface = udev_device_get_parent_with_subsystem_devtype(device,
 						"usb", "usb_interface");
-	if (intf == NULL)
+	if (usb_interface == NULL)
 		return;
 
 	modem = g_hash_table_lookup(modem_list, syspath);
@@ -1373,6 +1373,11 @@ static void add_device(const char *syspath, const char *devname,
 	}
 
 	label = udev_device_get_property_value(device, "OFONO_LABEL");
+	if (!label) {
+		label = udev_device_get_property_value(usb_interface,
+							"OFONO_LABEL");
+	}
+
 	subsystem = udev_device_get_subsystem(device);
 
 	if (modem->sysattr != NULL)
@@ -1495,6 +1500,14 @@ static void check_usb_device(struct udev_device *device)
 	model = udev_device_get_property_value(usb_device, "ID_MODEL_ID");
 
 	driver = udev_device_get_property_value(usb_device, "OFONO_DRIVER");
+	if (!driver) {
+		struct udev_device *usb_interface;
+		usb_interface = udev_device_get_parent_with_subsystem_devtype(
+					device, "usb", "usb_interface");
+		if (usb_interface)
+			driver = udev_device_get_property_value(
+					usb_interface, "OFONO_DRIVER");
+	}
 	if (driver == NULL) {
 		const char *drv;
 		unsigned int i;
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH udev v2 5/5] plugins: remove udev module
  2017-03-28 13:18 [PATCH udev v2 0/5] Device handling cleanup series Jonas Bonn
                   ` (3 preceding siblings ...)
  2017-03-28 13:18 ` [PATCH udev v2 4/5] udevng: get properties from interface Jonas Bonn
@ 2017-03-28 13:18 ` Jonas Bonn
  4 siblings, 0 replies; 8+ messages in thread
From: Jonas Bonn @ 2017-03-28 13:18 UTC (permalink / raw)
  To: ofono

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

This functionality has been moved into the udevng module and was
already disabled in an earlier patch.
---
 Makefile.am    |   2 -
 plugins/udev.c | 433 ---------------------------------------------------------
 2 files changed, 435 deletions(-)
 delete mode 100644 plugins/udev.c

diff --git a/Makefile.am b/Makefile.am
index d9f9b08..b232f97 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -106,8 +106,6 @@ gril_sources = gril/gril.h gril/gril.c \
 btio_sources = btio/btio.h btio/btio.c
 
 if UDEV
-builtin_modules += udev
-builtin_sources += plugins/udev.c
 builtin_cflags += @UDEV_CFLAGS@
 builtin_libadd += @UDEV_LIBS@
 
diff --git a/plugins/udev.c b/plugins/udev.c
deleted file mode 100644
index 564ede9..0000000
--- a/plugins/udev.c
+++ /dev/null
@@ -1,433 +0,0 @@
-/*
- *
- *  oFono - Open Source Telephony
- *
- *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License version 2 as
- *  published by the Free Software Foundation.
- *
- *  This program 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 General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <errno.h>
-#include <ctype.h>
-#include <stdlib.h>
-
-#include <libudev.h>
-
-#include <glib.h>
-#include <string.h>
-
-#define OFONO_API_SUBJECT_TO_CHANGE
-#include <ofono/plugin.h>
-#include <ofono/modem.h>
-#include <ofono/log.h>
-
-static GSList *modem_list = NULL;
-static GHashTable *devpath_list = NULL;
-
-static struct ofono_modem *find_modem(const char *devpath)
-{
-	GSList *list;
-
-	for (list = modem_list; list; list = list->next) {
-		struct ofono_modem *modem = list->data;
-		const char *path = ofono_modem_get_string(modem, "Path");
-
-		if (g_strcmp0(devpath, path) == 0)
-			return modem;
-	}
-
-	return NULL;
-}
-
-/*
- * Here we try to find the "modem device".
- *
- * In this variant we identify the "modem device" as simply the device
- * that has the OFONO_DRIVER property.  If the device node doesn't
- * have this property itself, then we do a brute force search for it
- * through the device hierarchy.
- *
- */
-static struct udev_device* get_modem_device(struct udev_device *dev)
-{
-	const char* driver;
-
-	while (dev) {
-		driver = udev_device_get_property_value(dev, "OFONO_DRIVER");
-		if (driver)
-			return dev;
-		dev = udev_device_get_parent(dev);
-	}
-
-	return NULL;
-}
-
-static const char *get_serial(struct udev_device *udev_device)
-{
-	const char *serial;
-
-	serial = udev_device_get_property_value(udev_device, "ID_SERIAL_SHORT");
-
-	if (serial != NULL) {
-		unsigned int i, len = strlen(serial);
-
-		for (i = 0; i < len; i++) {
-			if (!g_ascii_isalnum(serial[i]))
-				return NULL;
-		}
-	}
-
-	return serial;
-}
-
-static void __add_common(struct ofono_modem *modem,
-					struct udev_device *udev_device)
-{
-	const char *devnode;
-
-	DBG("modem %p", modem);
-
-	devnode = udev_device_get_devnode(udev_device);
-	ofono_modem_set_string(modem, "Device", devnode);
-
-	ofono_modem_register(modem);
-}
-
-static void add_ifx(struct ofono_modem *modem, struct udev_device *dev)
-{
-	const char *value;
-
-	value = udev_device_get_property_value(dev, "OFONO_IFX_LDISC");
-	if (value)
-		ofono_modem_set_string(modem, "LineDiscipline", value);
-	value = udev_device_get_property_value(dev, "OFONO_IFX_AUDIO");
-	if (value)
-		ofono_modem_set_string(modem, "AudioSetting", value);
-	value = udev_device_get_property_value(dev, "OFONO_IFX_LOOPBACK");
-	if (value)
-		ofono_modem_set_string(modem, "AudioLoopback", value);
-
-	__add_common(modem, dev);
-}
-
-static void add_isi(struct ofono_modem *modem,
-					struct udev_device *udev_device)
-{
-	const char *ifname, *type, *addr;
-
-	DBG("modem %p", modem);
-
-	if (ofono_modem_get_string(modem, "Interface"))
-		return;
-
-	addr = udev_device_get_property_value(udev_device, "OFONO_ISI_ADDRESS");
-	if (addr != NULL)
-		ofono_modem_set_integer(modem, "Address", atoi(addr));
-
-	if (g_strcmp0(udev_device_get_subsystem(udev_device), "net") != 0)
-		return;
-
-	type = udev_device_get_sysattr_value(udev_device, "type");
-	if (g_strcmp0(type, "820") != 0)
-		return;
-
-	ifname = udev_device_get_sysname(udev_device);
-	ofono_modem_set_string(modem, "Interface", ifname);
-
-	DBG("interface %s", ifname);
-
-	ofono_modem_register(modem);
-}
-
-static void add_wavecom(struct ofono_modem *modem, struct udev_device *dev)
-{
-	const char *value;
-
-	value = udev_device_get_property_value(dev, "OFONO_WAVECOM_MODEL");
-	if (value)
-		ofono_modem_set_string(modem, "Model", value);
-
-	__add_common(modem, dev);
-}
-
-static void add_modem(struct udev_device *udev_device)
-{
-	struct ofono_modem *modem;
-	const char *devpath, *curpath, *driver;
-	struct udev_device *mdev;
-
-	mdev = get_modem_device(udev_device);
-	if (!mdev) {
-		DBG("Device has no OFONO_DRIVER property");
-		return;
-	}
-
-	driver = udev_device_get_property_value(mdev, "OFONO_DRIVER");
-
-	if(g_strcmp0(driver, "tc65") == 0)
-		driver = "cinterion";
-	if(g_strcmp0(driver, "ehs6") == 0)
-		driver = "cinterion";
-
-	devpath = udev_device_get_devpath(mdev);
-	if (devpath == NULL)
-		return;
-
-	modem = find_modem(devpath);
-	if (modem == NULL) {
-		const char *serial = get_serial(mdev);
-
-		modem = ofono_modem_create(serial, driver);
-		if (modem == NULL)
-			return;
-
-		ofono_modem_set_string(modem, "Path", devpath);
-
-		modem_list = g_slist_prepend(modem_list, modem);
-	}
-
-	curpath = udev_device_get_devpath(udev_device);
-	if (curpath == NULL)
-		return;
-
-	DBG("%s (%s)", curpath, driver);
-
-	g_hash_table_insert(devpath_list, g_strdup(curpath), g_strdup(devpath));
-
-	if (g_strcmp0(driver, "ifx") == 0)
-		add_ifx(modem, udev_device);
-	else if (g_strcmp0(driver, "u8500") == 0)
-		add_isi(modem, udev_device);
-	else if (g_strcmp0(driver, "n900") == 0)
-		add_isi(modem, udev_device);
-	else if (g_strcmp0(driver, "calypso") == 0)
-		__add_common(modem, udev_device);
-	else if (g_strcmp0(driver, "cinterion") == 0)
-		__add_common(modem, udev_device);
-	else if (g_strcmp0(driver, "nokiacdma") == 0)
-		__add_common(modem, udev_device);
-	else if (g_strcmp0(driver, "sim900") == 0)
-		__add_common(modem, udev_device);
-	else if (g_strcmp0(driver, "wavecom") == 0)
-		add_wavecom(modem, udev_device);
-}
-
-static gboolean devpath_remove(gpointer key, gpointer value, gpointer user_data)
-{
-	const char *path = value;
-	const char *devpath = user_data;
-
-	DBG("%s -> %s", path, devpath);
-
-	return g_str_equal(path, devpath);
-}
-
-static void remove_modem(struct udev_device *udev_device)
-{
-	struct ofono_modem *modem;
-	const char *curpath = udev_device_get_devpath(udev_device);
-	char *devpath, *remove;
-
-	if (curpath == NULL)
-		return;
-
-	DBG("%s", curpath);
-
-	devpath = g_hash_table_lookup(devpath_list, curpath);
-	if (devpath == NULL)
-		return;
-
-	modem = find_modem(devpath);
-	if (modem == NULL)
-		return;
-
-	modem_list = g_slist_remove(modem_list, modem);
-
-	ofono_modem_remove(modem);
-
-	DBG("%s", devpath);
-
-	remove = g_strdup(devpath);
-
-	g_hash_table_foreach_remove(devpath_list, devpath_remove, remove);
-
-	g_free(remove);
-}
-
-static void enumerate_devices(struct udev *context)
-{
-	struct udev_enumerate *enumerate;
-	struct udev_list_entry *entry;
-
-	enumerate = udev_enumerate_new(context);
-	if (enumerate == NULL)
-		return;
-
-	udev_enumerate_add_match_subsystem(enumerate, "tty");
-	udev_enumerate_add_match_subsystem(enumerate, "net");
-	udev_enumerate_add_match_subsystem(enumerate, "hsi");
-
-	udev_enumerate_scan_devices(enumerate);
-
-	entry = udev_enumerate_get_list_entry(enumerate);
-	while (entry) {
-		const char *syspath = udev_list_entry_get_name(entry);
-		struct udev_device *device;
-
-		device = udev_device_new_from_syspath(context, syspath);
-		if (device != NULL) {
-			add_modem(device);
-
-			udev_device_unref(device);
-		}
-
-		entry = udev_list_entry_get_next(entry);
-	}
-
-	udev_enumerate_unref(enumerate);
-}
-
-static struct udev *udev_ctx;
-static struct udev_monitor *udev_mon;
-static guint udev_watch = 0;
-
-static gboolean udev_event(GIOChannel *channel, GIOCondition cond,
-							gpointer user_data)
-{
-	struct udev_device *device;
-	const char *action;
-
-	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
-		ofono_warn("Error with udev monitor channel");
-		udev_watch = 0;
-		return FALSE;
-	}
-
-	device = udev_monitor_receive_device(udev_mon);
-	if (device == NULL)
-		return TRUE;
-
-	action = udev_device_get_action(device);
-	if (action == NULL)
-		goto done;
-
-	if (g_str_equal(action, "add") == TRUE)
-		add_modem(device);
-	else if (g_str_equal(action, "remove") == TRUE)
-		remove_modem(device);
-
-done:
-	udev_device_unref(device);
-
-	return TRUE;
-}
-
-static void udev_start(void)
-{
-	GIOChannel *channel;
-	int fd;
-
-	if (udev_monitor_enable_receiving(udev_mon) < 0) {
-		ofono_error("Failed to enable udev monitor");
-		return;
-	}
-
-	enumerate_devices(udev_ctx);
-
-	fd = udev_monitor_get_fd(udev_mon);
-
-	channel = g_io_channel_unix_new(fd);
-	if (channel == NULL)
-		return;
-
-	udev_watch = g_io_add_watch(channel,
-				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-							udev_event, NULL);
-
-	g_io_channel_unref(channel);
-}
-
-static int udev_init(void)
-{
-	return 0;
-
-	devpath_list = g_hash_table_new_full(g_str_hash, g_str_equal,
-						g_free, g_free);
-	if (devpath_list == NULL) {
-		ofono_error("Failed to create udev path list");
-		return -ENOMEM;
-	}
-
-	udev_ctx = udev_new();
-	if (udev_ctx == NULL) {
-		ofono_error("Failed to create udev context");
-		g_hash_table_destroy(devpath_list);
-		return -EIO;
-	}
-
-	udev_mon = udev_monitor_new_from_netlink(udev_ctx, "udev");
-	if (udev_mon == NULL) {
-		ofono_error("Failed to create udev monitor");
-		g_hash_table_destroy(devpath_list);
-		udev_unref(udev_ctx);
-		udev_ctx = NULL;
-		return -EIO;
-	}
-
-	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "tty", NULL);
-	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "net", NULL);
-	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "hsi", NULL);
-
-	udev_monitor_filter_update(udev_mon);
-
-	udev_start();
-
-	return 0;
-}
-
-static void udev_exit(void)
-{
-	GSList *list;
-
-	if (udev_watch > 0)
-		g_source_remove(udev_watch);
-
-	for (list = modem_list; list; list = list->next) {
-		struct ofono_modem *modem = list->data;
-
-		ofono_modem_remove(modem);
-	}
-
-	g_slist_free(modem_list);
-	modem_list = NULL;
-
-	g_hash_table_destroy(devpath_list);
-	devpath_list = NULL;
-
-	if (udev_ctx == NULL)
-		return;
-
-	udev_monitor_filter_remove(udev_mon);
-
-	udev_monitor_unref(udev_mon);
-	udev_unref(udev_ctx);
-}
-
-OFONO_PLUGIN_DEFINE(udev, "udev hardware detection", VERSION,
-			OFONO_PLUGIN_PRIORITY_DEFAULT, udev_init, udev_exit)
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH udev v2 1/5] udevng: add serial device handling functions
  2017-03-28 13:18 ` [PATCH udev v2 1/5] udevng: add serial device handling functions Jonas Bonn
@ 2017-03-28 16:46   ` Denis Kenzior
  0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2017-03-28 16:46 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

On 03/28/2017 08:18 AM, Jonas Bonn wrote:
> This adds, but does not hook up, support for simple serial modems.  These
> modems generally have only a single device node so are simpler than the
> USB devices which generally have different device nodes for different
> features.  These modems are currently handled by udev.c, but this
> functionality will allow to remove that module completely.
>
> - A new "device_info" type is created called serial_device_info
> - the function add_serial_device sets up a modem_info structure and a
>   serial_device_info for the device
> - A reference to the device's udev node is saved in the device info
> - The device driver is retrieved from the OFONO_DRIVER environment variable
>   which needs to be set up by some udev rule
> - Setup functions are added for these types of devices: a common function
>   setup_serial_modem covers the generic (simple) case, whereas modems
>   with special requirements are given their own setup functions to handle
>   the special bits
> - Modem destroy needs to know the "device_info" type in order to clean
>   up properly, so a 'type' value is set on these structures for this
>   purpose
> ---
>  plugins/udevng.c | 242 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 234 insertions(+), 8 deletions(-)

Just FYI, when applying I get:
Applying: udevng: add serial device handling functions
.git/rebase-apply/patch:201: trailing whitespace.
  * through the device hierarchy.
.git/rebase-apply/patch:215: trailing whitespace.
	return NULL;
fatal: 2 lines add whitespace errors.
Patch failed at 0001 udevng: add serial device handling functions
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Please configure your editor to squash these

>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 7a29be6..7ac185c 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -48,7 +48,13 @@ struct modem_info {
>  	const char *sysattr;
>  };
>
> +enum {
> +	DEVICE_TYPE_USB,
> +	DEVICE_TYPE_SERIAL,
> +} DeviceType;
> +
>  struct device_info {
> +	int type;
>  	char *devpath;
>  	char *devnode;
>  	char *interface;
> @@ -58,6 +64,14 @@ struct device_info {
>  	char *subsystem;
>  };
>
> +struct serial_modem_info {

serial_device_info

> +	int type;
> +	char *devpath;
> +	char *devnode;
> +	char *subsystem;
> +	struct udev_device* dev;
> +};
> +

Also, instead of the casting ugliness you have, why don't you just make 
this into a proper abstraction.  E.g. something like:

enum modem_type {
	MODEM_TYPE_USB,
	MODEM_TYPE_SERIAL
},

struct modem_info {
	...
	enum modem_type type;
	union {
		GSList *devices;
		struct serial_device_info *serial;
	}
};

>  static gboolean setup_isi(struct modem_info *modem)
>  {
>  	const char *node = NULL;
> @@ -867,6 +881,100 @@ static gboolean setup_quectel(struct modem_info *modem)
>  	return TRUE;
>  }
>
> +static gboolean setup_serial_modem(struct modem_info* modem)
> +{
> +	struct serial_modem_info* info;
> +
> +	info = modem->devices->data;

Then this simply becomes modem->serial

> +
> +	ofono_modem_set_string(modem->modem, "Device", info->devnode);
> +
> +	return TRUE;
> +}
> +
> +static gboolean setup_tc65(struct modem_info* modem)
> +{
> +	ofono_modem_set_driver(modem->modem, "cinterion");
> +
> +	return setup_serial_modem(modem);
> +}
> +
> +static gboolean setup_ehs6(struct modem_info* modem)
> +{
> +	ofono_modem_set_driver(modem->modem, "cinterion");
> +
> +	return setup_serial_modem(modem);
> +}
> +
> +static gboolean setup_ifx(struct modem_info* modem)
> +{
> +	struct serial_modem_info* info;
> +	const char *value;
> +
> +	info = modem->devices->data;
> +
> +	value = udev_device_get_property_value(info->dev, "OFONO_IFX_LDISC");
> +	if (value)
> +		ofono_modem_set_string(modem->modem, "LineDiscipline", value);
> +
> +	value = udev_device_get_property_value(info->dev, "OFONO_IFX_AUDIO");
> +	if (value)
> +		ofono_modem_set_string(modem->modem, "AudioSetting", value);
> +
> +	value = udev_device_get_property_value(info->dev, "OFONO_IFX_LOOPBACK");
> +	if (value)
> +		ofono_modem_set_string(modem->modem, "AudioLoopback", value);
> +
> +	ofono_modem_set_string(modem->modem, "Device", info->devnode);
> +
> +	return TRUE;
> +}
> +
> +static gboolean setup_wavecom(struct modem_info* modem)
> +{
> +	struct serial_modem_info* info;
> +	const char *value;
> +
> +	info = modem->devices->data;
> +
> +	value = udev_device_get_property_value(info->dev,
> +						"OFONO_WAVECOM_MODEL");
> +	if (value)
> +		ofono_modem_set_string(modem->modem, "Model", value);
> +
> +	ofono_modem_set_string(modem->modem, "Device", info->devnode);
> +
> +	return TRUE;
> +}
> +
> +static gboolean setup_isi_serial(struct modem_info* modem)
> +{
> +	struct serial_modem_info* info;
> +	const char *value;
> +
> +	info = modem->devices->data;
> +
> +	if (g_strcmp0(udev_device_get_subsystem(info->dev), "net") != 0)
> +		return FALSE;
> +
> +	value = udev_device_get_sysattr_value(info->dev, "type");
> +	if (g_strcmp0(value, "820") != 0)
> +		return FALSE;
> +
> +	/* OK, we want this device to be a modem */
> +	value = udev_device_get_sysname(info->dev);
> +	if (value)
> +		ofono_modem_set_string(modem->modem, "Interface", value);
> +
> +	value = udev_device_get_property_value(info->dev, "OFONO_ISI_ADDRESS");
> +	if (value)
> +		ofono_modem_set_integer(modem->modem, "Address", atoi(value));
> +
> +	ofono_modem_set_string(modem->modem, "Device", info->devnode);
> +
> +	return TRUE;
> +}
> +
>  static gboolean setup_ublox(struct modem_info *modem)
>  {
>  	const char *aux = NULL, *mdm = NULL, *net = NULL;
> @@ -997,6 +1105,17 @@ static struct {
>  	{ "quectel",	setup_quectel	},
>  	{ "ublox",	setup_ublox	},
>  	{ "gemalto",	setup_gemalto	},
> +	/* Following are non-USB modems */
> +	{ "ifx",	setup_ifx		},
> +	{ "u8500",	setup_isi_serial	},
> +	{ "n900",	setup_isi_serial	},
> +	{ "calypso",	setup_serial_modem	},
> +	{ "cinterion",	setup_serial_modem	},
> +	{ "nokiacdma",	setup_serial_modem	},
> +	{ "sim900",	setup_serial_modem	},
> +	{ "wavecom",	setup_wavecom		},
> +	{ "tc65",	setup_tc65		},
> +	{ "ehs6",	setup_ehs6		},
>  	{ }
>  };
>
> @@ -1028,14 +1147,25 @@ static void destroy_modem(gpointer data)
>
>  		DBG("%s", info->devnode);
>
> -		g_free(info->devpath);
> -		g_free(info->devnode);
> -		g_free(info->interface);
> -		g_free(info->number);
> -		g_free(info->label);
> -		g_free(info->sysattr);
> -		g_free(info->subsystem);
> -		g_free(info);
> +		if (info->type == DEVICE_TYPE_USB) {
> +			g_free(info->devpath);
> +			g_free(info->devnode);
> +			g_free(info->interface);
> +			g_free(info->number);
> +			g_free(info->label);
> +			g_free(info->sysattr);
> +			g_free(info->subsystem);
> +			g_free(info);

This needs to be put into a separate function, e.g. device_info_free()

> +		} else if (info->type == DEVICE_TYPE_SERIAL) {
> +			struct serial_modem_info* sinfo;
> +			sinfo = (struct serial_modem_info*) info;
> +			g_free(sinfo->devpath);
> +			g_free(sinfo->devnode);
> +			g_free(sinfo->subsystem);
> +			udev_device_unref(sinfo->dev);
> +			g_free(sinfo);

serial_device_info_free()

> +		}
> +
>
>  		list->data = NULL;
>  	}
> @@ -1088,6 +1218,101 @@ static gint compare_device(gconstpointer a, gconstpointer b)
>  	return g_strcmp0(info1->number, info2->number);
>  }
>
> +/*
> + * Here we try to find the "modem device".
> + *
> + * In this variant we identify the "modem device" as simply the device
> + * that has the OFONO_DRIVER property.  If the device node doesn't
> + * have this property itself, then we do a brute force search for it
> + * through the device hierarchy.
> + *
> + */
> +static struct udev_device* get_legacy_modem_device(struct udev_device *dev)
> +{
> +	const char* driver;
> +
> +	while (dev) {
> +		driver = udev_device_get_property_value(dev, "OFONO_DRIVER");
> +		if (driver)
> +			return dev;
> +		dev = udev_device_get_parent(dev);

item M1

> +	}
> +
> +	return NULL;
> +}
> +
> +/*
> + * Add 'legacy' device
> + *
> + * The term legacy is a bit misleading, but this adds devices according
> + * to the original ofono model.
> + *
> + * - We cannot assume that these are USB devices
> + * - The modem consists of only a single interface
> + * - The device must have an OFONO_DRIVER property from udev
> + */
> +static void add_serial_device(struct udev_device *dev)
> +{
> +	const char *syspath, *devpath, *devname, *devnode;
> +	struct modem_info *modem;
> +	struct serial_modem_info *info;
> +	const char* vendor = NULL;
> +	const char* model = NULL;
> +	const char *subsystem;
> +	struct udev_device* mdev;
> +	const char* driver;
> +
> +	mdev = get_legacy_modem_device(dev);
> +	if (!mdev) {
> +		DBG("Device is missing required OFONO_DRIVER property");
> +		return;
> +	}
> +
> +	driver = udev_device_get_property_value(mdev, "OFONO_DRIVER");
> +
> +	syspath = udev_device_get_syspath(mdev);
> +	devname = udev_device_get_devnode(mdev);
> +	devpath = udev_device_get_devpath(mdev);
> +
> +	devnode = udev_device_get_devnode(dev);
> +
> +	if (!syspath || !devname || !devpath || !devnode)
> +		return;
> +
> +	modem = g_hash_table_lookup(modem_list, syspath);
> +	if (modem == NULL) {
> +		modem = g_try_new0(struct modem_info, 1);
> +		if (modem == NULL)
> +			return;
> +
> +		modem->syspath = g_strdup(syspath);
> +		modem->devname = g_strdup(devname);
> +		modem->driver = g_strdup("legacy");
> +		modem->vendor = g_strdup(vendor);
> +		modem->model = g_strdup(model);
> +
> +		g_hash_table_replace(modem_list, modem->syspath, modem);
> +	}
> +
> +	subsystem = udev_device_get_subsystem(dev);
> +
> +	DBG("%s", syspath);
> +	DBG("%s", devpath);
> +	DBG("%s (%s)", devnode, driver);
> +
> +	info = g_try_new0(struct serial_modem_info, 1);
> +	if (info == NULL)
> +		return;
> +
> +	info->type = DEVICE_TYPE_SERIAL;
> +	info->devpath = g_strdup(devpath);
> +	info->devnode = g_strdup(devnode);
> +	info->subsystem = g_strdup(subsystem);
> +	info->dev = udev_device_ref(dev);
> +
> +	modem->devices = g_slist_append(modem->devices, info);
> +}
> +
>  static void add_device(const char *syspath, const char *devname,
>  			const char *driver, const char *vendor,
>  			const char *model, struct udev_device *device)
> @@ -1164,6 +1389,7 @@ static void add_device(const char *syspath, const char *devname,
>  	if (info == NULL)
>  		return;
>
> +	info->type = DEVICE_TYPE_USB;
>  	info->devpath = g_strdup(devpath);
>  	info->devnode = g_strdup(devnode);
>  	info->interface = g_strdup(interface);
>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH udev v2 4/5] udevng: get properties from interface
  2017-03-28 13:18 ` [PATCH udev v2 4/5] udevng: get properties from interface Jonas Bonn
@ 2017-03-28 16:51   ` Denis Kenzior
  0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2017-03-28 16:51 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

On 03/28/2017 08:18 AM, Jonas Bonn wrote:
> Device properties are generally on the device, on the USB interface
> descriptor, or the on the USB device descriptor.
> ---
>  plugins/udevng.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> @@ -1373,6 +1373,11 @@ static void add_device(const char *syspath, const char *devname,
>  	}
>
>  	label = udev_device_get_property_value(device, "OFONO_LABEL");
> +	if (!label) {
> +		label = udev_device_get_property_value(usb_interface,
> +							"OFONO_LABEL");
> +	}
> +

We prefer not to have {} for single expression if/while/do/for blocks

>  	subsystem = udev_device_get_subsystem(device);
>
>  	if (modem->sysattr != NULL)
> @@ -1495,6 +1500,14 @@ static void check_usb_device(struct udev_device *device)
>  	model = udev_device_get_property_value(usb_device, "ID_MODEL_ID");
>
>  	driver = udev_device_get_property_value(usb_device, "OFONO_DRIVER");
> +	if (!driver) {
> +		struct udev_device *usb_interface;
> +		usb_interface = udev_device_get_parent_with_subsystem_devtype(
> +					device, "usb", "usb_interface");
> +		if (usb_interface)
> +			driver = udev_device_get_property_value(
> +					usb_interface, "OFONO_DRIVER");

Might be cleaner written as:
		struct udev_device *usb_interface =
			udev_device_get_parent_with_subsystem_devtype(
					device, "usb", "usb_interface");

		if (usb_interface)
			driver = udev_device_get_property_value(
					usb_interface, "OFONO_DRIVER");

> +	}
>  	if (driver == NULL) {

item M1.  Separate the two if statements

>  		const char *drv;
>  		unsigned int i;
>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-03-28 16:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 13:18 [PATCH udev v2 0/5] Device handling cleanup series Jonas Bonn
2017-03-28 13:18 ` [PATCH udev v2 1/5] udevng: add serial device handling functions Jonas Bonn
2017-03-28 16:46   ` Denis Kenzior
2017-03-28 13:18 ` [PATCH udev v2 2/5] udevng: match on the hsi subsystem for legacy devices Jonas Bonn
2017-03-28 13:18 ` [PATCH udev v2 3/5] udevng: hook up " Jonas Bonn
2017-03-28 13:18 ` [PATCH udev v2 4/5] udevng: get properties from interface Jonas Bonn
2017-03-28 16:51   ` Denis Kenzior
2017-03-28 13:18 ` [PATCH udev v2 5/5] plugins: remove udev module Jonas Bonn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.