[fix-isi-udev PATCH 0/3] Improve udev with isi

[fix-isi-udev PATCH 0/3] Improve udev with isi

[Pekka.Pessi]

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

Hello all,

Here are three patches for udev. For a reason or another, the current
ofono.rules does not work very well in Ubuntu 10.04. The "isigen" driver
is available when the device is added, but not when the udev devices are
enumerated.

[fix-isi-udev PATCH 1/3] udev: do not use post-143 libudev features

Patch 1/3 provides a replacement function for
udev_device_get_property_value(), which is not available in libudev
version 143 required by configure.ac.

[fix-isi-udev PATCH 2/3] udev: refactor add_isi

Patch 2/3 models the add_isi like other add_* functions: it gathers the
information from device nodes and registers the modem when all the
required information is present. Also, it registers the modem only when
a correct phonet device with Ethernet type 820 is found.

[fix-isi-udev PATCH 3/3] ofono.rules: assign isigen driver by USB ids

Patch 3/3 provides a udev rule that detects the isigen driver using the
USB vendor (and product) ids, just like the other modem rules.

--Pekka

[fix-isi-udev PATCH 1/3] udev: do not use post-143 libudev features

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

There is no udev_device_get_property_value() in libudev version 143.
---
 plugins/udev.c |   38 ++++++++++++++++----------------------
 1 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/plugins/udev.c b/plugins/udev.c
index 3c2c09c..d506234 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -55,38 +55,34 @@ static struct ofono_modem *find_modem(const char *devpath)
 	return NULL;
 }
 
-static const char *get_driver(struct udev_device *udev_device)
+static const char *get_property(struct udev_device *device,
+				char const *property_name)
 {
 	struct udev_list_entry *entry;
-	const char *driver = NULL;
 
-	entry = udev_device_get_properties_list_entry(udev_device);
+	entry = udev_device_get_properties_list_entry(device);
 	while (entry) {
 		const char *name = udev_list_entry_get_name(entry);
 
-		if (g_strcmp0(name, "OFONO_DRIVER") == 0)
-			driver = udev_list_entry_get_value(entry);
+		if (g_strcmp0(name, property_name) == 0)
+			return udev_list_entry_get_value(entry);
 
 		entry = udev_list_entry_get_next(entry);
 	}
 
-	return driver;
+	return NULL;
 }
 
-static const char *get_serial(struct udev_device *udev_device)
+static const char *get_driver(struct udev_device *udev_device)
 {
-	struct udev_list_entry *entry;
-	const char *serial = NULL;
-
-	entry = udev_device_get_properties_list_entry(udev_device);
-	while (entry) {
-		const char *name = udev_list_entry_get_name(entry);
+	return get_property(udev_device, "OFONO_DRIVER");
+}
 
-		if (g_strcmp0(name, "ID_SERIAL_SHORT") == 0)
-			serial = udev_list_entry_get_value(entry);
+static const char *get_serial(struct udev_device *udev_device)
+{
+	const char *serial;
 
-		entry = udev_list_entry_get_next(entry);
-	}
+	serial = get_property(udev_device, "ID_SERIAL_SHORT");
 
 	if (serial != NULL) {
 		unsigned int i, len = strlen(serial);
@@ -147,8 +143,7 @@ static void add_mbm(struct ofono_modem *modem,
 			g_str_has_suffix(desc, "Mini-Card Network Adapter") ||
 			g_str_has_suffix(desc, "Broadband Network Adapter") ||
 			g_str_has_suffix(desc, "Minicard NetworkAdapter")) {
-		devnode = udev_device_get_property_value(udev_device,
-								"INTERFACE");
+		devnode = get_property(udev_device, "INTERFACE");
 		ofono_modem_set_string(modem, NETWORK_INTERFACE, devnode);
 	} else {
 		return;
@@ -194,8 +189,7 @@ static void add_hso(struct ofono_modem *modem,
 		else if (g_str_has_suffix(type, "Control") == TRUE)
 			ofono_modem_set_string(modem, CONTROL_PORT, devnode);
 	} else if (g_str_equal(subsystem, "net") == TRUE) {
-		devnode = udev_device_get_property_value(udev_device,
-								"INTERFACE");
+		devnode = get_property(udev_device, "INTERFACE");
 		ofono_modem_set_string(modem, NETWORK_INTERFACE, devnode);
 	} else {
 		return;
@@ -435,7 +429,7 @@ static void add_isi(struct ofono_modem *modem,
 
 	DBG("interface %s", ifname);
 
-	addr = udev_device_get_property_value(udev_device, "OFONO_ISI_ADDRESS");
+	addr = get_property(udev_device, "OFONO_ISI_ADDRESS");
 	if (addr != NULL)
 		ofono_modem_set_integer(modem, "Address", atoi(addr));
 
-- 
1.7.0.4

[fix-isi-udev PATCH 2/3] udev: refactor add_isi

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Gather OFONO_ISI_ADDRESS from same udev device node as OFONO_DRIVER.
---
 plugins/udev.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/plugins/udev.c b/plugins/udev.c
index d506234..a352b90 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -420,19 +420,29 @@ static void add_nokia(struct ofono_modem *modem,
 static void add_isi(struct ofono_modem *modem,
 					struct udev_device *udev_device)
 {
-	const char *ifname, *addr;
+	const char *ifname, *type, *addr;
 
 	DBG("modem %p", modem);
 
-	ifname = udev_device_get_sysname(udev_device);
-	ofono_modem_set_string(modem, "Interface", ifname);
-
-	DBG("interface %s", ifname);
+	if (ofono_modem_get_string(modem, "Interface"))
+		return;
 
 	addr = get_property(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);
 }
 
-- 
1.7.0.4

[fix-isi-udev PATCH 3/3] ofono.rules: assign isigen driver by USB ids

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Now using the USB vendor and product IDs like the rest of the oFono
drivers.

For reasons left unclear, the SUBSYSTEMS rule did not work consistently
in Ubuntu 10.04.
---
 plugins/ofono.rules |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/plugins/ofono.rules b/plugins/ofono.rules
index 6517757..ccae688 100644
--- a/plugins/ofono.rules
+++ b/plugins/ofono.rules
@@ -342,9 +342,6 @@ SUBSYSTEM!="net", GOTO="ofono_isi_end"
 ATTRS{type}!="820", GOTO="ofono_isi_end"
 KERNELS=="gadget", GOTO="ofono_isi_end"
 
-# Generic / PC Suite mode
-SUBSYSTEMS=="usb", ENV{OFONO_DRIVER}="isigen", ENV{OFONO_ISI_ADDRESS}="16"
-
 # Nokia N900 modem
 SUBSYSTEMS=="hsi", ENV{OFONO_DRIVER}="n900", ENV{OFONO_ISI_ADDRESS}="108"
 
@@ -404,6 +401,9 @@ ATTRS{idVendor}=="0930", ATTRS{idProduct}=="130b", ENV{OFONO_DRIVER}="mbm"
 ATTRS{idVendor}=="0930", ATTRS{idProduct}=="130c", ENV{OFONO_DRIVER}="mbm"
 ATTRS{idVendor}=="0930", ATTRS{idProduct}=="1311", ENV{OFONO_DRIVER}="mbm"
 
+# Nokia default (with PC Suite)
+ATTRS{idVendor}=="0421", ENV{OFONO_DRIVER}="isigen", ENV{OFONO_ISI_ADDRESS}="16"
+
 # Nokia Internet Stick CS-10
 ATTRS{idVendor}=="0421", ATTRS{idProduct}=="060e", ENV{OFONO_DRIVER}="nokia"
 
-- 
1.7.0.4

Re: [fix-isi-udev PATCH 3/3] ofono.rules: assign isigen driver by USB ids

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

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




On Mon,  4 Oct 2010 16:11:06 +0300, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Now using the USB vendor and product IDs like the rest of the oFono
> drivers.
> 
> For reasons left unclear, the SUBSYSTEMS rule did not work consistently
> in Ubuntu 10.04.

The network device is not part of the USB subsystem, only its parent
device.

> +# Nokia default (with PC Suite)
> +ATTRS{idVendor}=="0421", ENV{OFONO_DRIVER}="isigen",
> ENV{OFONO_ISI_ADDRESS}="16"
> +

This will match the whole USB gadget. How do you get the Phonet interface
from that?

I would simply match on Ethernet type 820 and kernel name usbpnXXX.

-- 
Rémi Denis-Courmont
http://www.remlab.net
http://fi.linkedin.com/in/remidenis

Re: [fix-isi-udev PATCH 3/3] ofono.rules: assign isigen driver by USB ids

[Pekka Pessi]

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

Hi,

2010/10/8 Rémi Denis-Courmont <remi@remlab.net>:
>> Now using the USB vendor and product IDs like the rest of the oFono
>> drivers.
>>
>> For reasons left unclear, the SUBSYSTEMS rule did not work consistently
>> in Ubuntu 10.04.
>
> The network device is not part of the USB subsystem, only its parent
> device.

Yes, the last S in SUBSYSTEMS refer to a SUBSYSTEM of a parent.

>> +# Nokia default (with PC Suite)
>> +ATTRS{idVendor}=="0421", ENV{OFONO_DRIVER}="isigen",
>> ENV{OFONO_ISI_ADDRESS}="16"
>> +
>
> This will match the whole USB gadget.

That is the point. We don't want one modem with at command tty and
another with phonet interface.

> How do you get the Phonet interface
> from that?

> I would simply match on Ethernet type 820 and

isi_add() does the Ethernet type matching. For a reason or another
Lucid udev rules fails to match the type sysattr when enumerating
existing devices.

> kernel name usbpnXXX.

The interface name is ignored as an expert earlier advised. ;)

-- 
Pekka.Pessi mail at nokia.com

Re: [fix-isi-udev PATCH 0/3] Improve udev with isi

[Aki Niemi]

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

Hi Pekka,

On Mon, 2010-10-04 at 15:11 +0200, ext Pekka.Pessi(a)nokia.com wrote:
> [fix-isi-udev PATCH 1/3] udev: do not use post-143 libudev features
> [fix-isi-udev PATCH 2/3] udev: refactor add_isi

These two were applied. Thanks!

> [fix-isi-udev PATCH 3/3] ofono.rules: assign isigen driver by USB ids

This one was left dangling; let's see if we can come up with a better
way to match the modems driven by cdc_phonet. (Although, admittedly this
same approach is already used with some AT modems.)

Cheers,
Aki

Re: [fix-isi-udev PATCH 0/3] Improve udev with isi

[Marcel Holtmann]

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

Hi Aki,

> > [fix-isi-udev PATCH 1/3] udev: do not use post-143 libudev features
> > [fix-isi-udev PATCH 2/3] udev: refactor add_isi
> 
> These two were applied. Thanks!
> 
> > [fix-isi-udev PATCH 3/3] ofono.rules: assign isigen driver by USB ids
> 
> This one was left dangling; let's see if we can come up with a better
> way to match the modems driven by cdc_phonet. (Although, admittedly this
> same approach is already used with some AT modems.)

that is a leftover from the time where we only looking for TTY devices.
In that case that was fine. Now this needs to be fixed as well.

Regards

Marcel

Re: [fix-isi-udev PATCH 0/3] Improve udev with isi

[Alexander Kanavin]

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

On 07.10.2010 22:41, ext Aki Niemi wrote:
>> [fix-isi-udev PATCH 3/3] ofono.rules: assign isigen driver by USB ids
>
> This one was left dangling; let's see if we can come up with a better
> way to match the modems driven by cdc_phonet. (Although, admittedly this
> same approach is already used with some AT modems.)

I suppose you could match by usb interface class and subclass? (02 and 254 
respectively -  the first is standardized, but the second seems to be just 
made up by Nokia at some point, so we better keep the vendor id too)

Alex