[PATCH 1/6] gobi: add / use DeviceProtocol property

[PATCH 1/6] gobi: add / use DeviceProtocol property

[Denis Kenzior]

Instead of using the kernel driver as a hint for whether qrtr or qmi is
being used, introduce an explicit DeviceProtocol property that should be
set by the detection logic to tell the driver whether to use qrtr or
qmux.

Split up QMI modem driver detection and setup logic inside udevng into
setup_qmi_qrtr(), setup_qmi_qmux() and setup_qmi_netdev().
setup_qmi_netdev() takes care of setting up of common networking device
properties, while setup_qmi_qrtr() and setup_qmi_qmux() setup QRTR and
QMUX devices, respectively.

While here, ensure that modem types are handled appropriately.  QRTR is
only available on embedded devices, while QMUX requires USB.
---
 plugins/gobi.c   | 34 ++++++++++++++-------
 plugins/udevng.c | 78 +++++++++++++++++++++++++++++++-----------------
 2 files changed, 75 insertions(+), 37 deletions(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index 8bd2c4060446..d8b78ad6f5b0 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -72,6 +72,11 @@
 #define GOBI_VOICE	(1 << 9)
 #define GOBI_WDA	(1 << 10)
 
+enum qmi_protocol {
+	QMI_PROTOCOL_QMUX,
+	QMI_PROTOCOL_QRTR,
+};
+
 struct gobi_data {
 	struct qmi_device *device;
 	struct qmi_service *dms;
@@ -85,6 +90,7 @@ struct gobi_data {
 	char main_net_name[IFNAMSIZ];
 	uint32_t max_aggregation_size;
 	uint32_t set_powered_id;
+	enum qmi_protocol protocol;
 };
 
 static void gobi_debug(const char *str, void *user_data)
@@ -98,11 +104,22 @@ static int gobi_probe(struct ofono_modem *modem)
 {
 	struct gobi_data *data;
 	const char *kernel_driver;
+	const char *value;
+	enum qmi_protocol protocol;
 
 	DBG("%p", modem);
 
-	data = l_new(struct gobi_data, 1);
+	value = ofono_modem_get_string(modem, "DeviceProtocol");
 
+	if (l_streq0(value, "qrtr"))
+		protocol = QMI_PROTOCOL_QRTR;
+	else if (l_streq0(value, "qmux"))
+		protocol = QMI_PROTOCOL_QMUX;
+	else
+		return -EPROTO;
+
+	data = l_new(struct gobi_data, 1);
+	data->protocol = protocol;
 	kernel_driver = ofono_modem_get_string(modem, "KernelDriver");
 	DBG("kernel_driver: %s", kernel_driver);
 
@@ -421,25 +438,22 @@ static void discover_cb(void *user_data)
 static int gobi_enable(struct ofono_modem *modem)
 {
 	struct gobi_data *data = ofono_modem_get_data(modem);
-	const char *kernel_driver;
+	const char *device;
 	int r;
 
 	DBG("%p", modem);
 
-	kernel_driver = ofono_modem_get_string(modem, "KernelDriver");
-	if (!kernel_driver)
-		return -EINVAL;
-
-	if (!strcmp(kernel_driver, "qrtr"))
+	switch (data->protocol) {
+	case QMI_PROTOCOL_QRTR:
 		data->device = qmi_device_new_qrtr();
-	else {
-		const char *device;
-
+		break;
+	case QMI_PROTOCOL_QMUX:
 		device = ofono_modem_get_string(modem, "Device");
 		if (!device)
 			return -EINVAL;
 
 		data->device = qmi_device_new_qmux(device);
+		break;
 	}
 
 	if (!data->device)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index fb694858aa6c..aa2a216d4247 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -197,20 +197,14 @@ static gboolean setup_hso(struct modem_info *modem)
 	return TRUE;
 }
 
-static int setup_qmi(struct modem_info *modem, const struct device_info *qmi,
-			const struct device_info *net)
+static int setup_qmi_netdev(struct modem_info *modem,
+					const struct device_info *net)
 {
 	const char *attr_value;
 	uint32_t ifindex;
 	int r;
 
-	DBG("qmi: %s net: %s kernel_driver: %s interface_number: %s",
-		qmi->devnode, net->devnode, net->kernel_driver, net->number);
-
-	if (!qmi->kernel_driver)
-		return -EINVAL;
-
-	if (!net->number && modem->type != MODEM_TYPE_EMBEDDED)
+	if (!net->kernel_driver)
 		return -EINVAL;
 
 	attr_value = udev_device_get_sysattr_value(net->udev_device,
@@ -222,30 +216,60 @@ static int setup_qmi(struct modem_info *modem, const struct device_info *qmi,
 	if (r < 0)
 		return r;
 
-	ofono_modem_set_driver(modem->modem, "gobi");
-	ofono_modem_set_string(modem->modem, "Device", qmi->devnode);
 	ofono_modem_set_string(modem->modem, "KernelDriver",
 							net->kernel_driver);
 	ofono_modem_set_string(modem->modem, "NetworkInterface", net->devnode);
-	ofono_modem_set_string(modem->modem, "InterfaceNumber", net->number);
 	ofono_modem_set_integer(modem->modem, "NetworkInterfaceIndex",
 							ifindex);
 
+	return 0;
+}
+
+static int setup_qmi_qmux(struct modem_info *modem,
+				const struct device_info *qmi,
+				const struct device_info *net)
+{
+	DBG("qmi: %s net: %s kernel_driver: %s interface_number: %s",
+		qmi->devnode, net->devnode, net->kernel_driver, net->number);
+
+	if (modem->type != MODEM_TYPE_USB)
+		return -ENOTSUP;
+
+	if (!net->number)
+		return -EINVAL;
+
+	if (!qmi->kernel_driver)
+		return -EINVAL;
+
+	ofono_modem_set_driver(modem->modem, "gobi");
+	ofono_modem_set_string(modem->modem, "Device", qmi->devnode);
+	ofono_modem_set_string(modem->modem, "DeviceProtocol", "qmux");
+	ofono_modem_set_string(modem->modem, "InterfaceNumber", net->number);
+
+	ofono_modem_set_string(modem->modem, "Bus", "usb");
+
+	return setup_qmi_netdev(modem, net);
+}
+
+static int setup_qmi_qrtr(struct modem_info *modem,
+				const struct device_info *net)
+{
+	DBG("net: %s kernel_driver: %s", net->devnode, net->kernel_driver);
+
 	switch (modem->type) {
-	case MODEM_TYPE_USB:
-		ofono_modem_set_string(modem->modem, "Bus", "usb");
-		break;
-	case MODEM_TYPE_PCIE:
-		ofono_modem_set_string(modem->modem, "Bus", "pcie");
-		break;
 	case MODEM_TYPE_EMBEDDED:
 		ofono_modem_set_string(modem->modem, "Bus", "embedded");
 		break;
+	case MODEM_TYPE_USB:
 	case MODEM_TYPE_SERIAL:
-		break;
+	case MODEM_TYPE_PCIE:
+		return -ENOTSUP;
 	}
 
-	return 0;
+	ofono_modem_set_driver(modem->modem, "gobi");
+	ofono_modem_set_string(modem->modem, "DeviceProtocol", "qrtr");
+
+	return setup_qmi_netdev(modem, net);
 }
 
 static gboolean setup_gobi_qrtr_premux(struct modem_info *modem,
@@ -310,7 +334,7 @@ static gboolean setup_gobi_qrtr(struct modem_info *modem)
 		return FALSE;
 	}
 
-	r = setup_qmi(modem, ipa_info, ipa_info);
+	r = setup_qmi_qrtr(modem, ipa_info);
 	if (r < 0)
 		return FALSE;
 
@@ -367,7 +391,7 @@ static gboolean setup_gobi(struct modem_info *modem)
 	DBG("qmi=%s net=%s mdm=%s gps=%s diag=%s",
 			qmi->devnode, net->devnode, mdm, gps, diag);
 
-	if (setup_qmi(modem, qmi, net) < 0)
+	if (setup_qmi_qmux(modem, qmi, net) < 0)
 		return FALSE;
 
 	ofono_modem_set_string(modem->modem, "Modem", mdm);
@@ -432,7 +456,7 @@ static gboolean setup_sierra(struct modem_info *modem)
 	}
 
 	if (qmi != NULL && net != NULL) {
-		if (setup_qmi(modem, qmi, net) < 0)
+		if (setup_qmi_qmux(modem, qmi, net) < 0)
 			return FALSE;
 
 		goto done;
@@ -510,7 +534,7 @@ static gboolean setup_huawei(struct modem_info *modem)
 	}
 
 	if (qmi != NULL && net != NULL) {
-		if (setup_qmi(modem, qmi, net) < 0)
+		if (setup_qmi_qmux(modem, qmi, net) < 0)
 			return FALSE;
 
 		goto done;
@@ -847,7 +871,7 @@ static gboolean setup_telitqmi(struct modem_info *modem)
 	if (qmi == NULL || net == NULL)
 		return FALSE;
 
-	if (setup_qmi(modem, qmi, net) < 0)
+	if (setup_qmi_qmux(modem, qmi, net) < 0)
 		return FALSE;
 
 	if (g_strcmp0(modem->model, "1070"))
@@ -1161,7 +1185,7 @@ static gboolean setup_quectelqmi(struct modem_info *modem)
 
 	DBG("gps=%s aux=%s", gps, aux);
 
-	if (setup_qmi(modem, qmi, net) < 0)
+	if (setup_qmi_qmux(modem, qmi, net) < 0)
 		return FALSE;
 
 	qmap_size = udev_device_get_sysattr_value(net->udev_device,
@@ -1611,7 +1635,7 @@ static gboolean setup_sim7x00(struct modem_info *modem)
 	if (qmi != NULL && net != NULL) {
 		DBG("mdm=%s gps=%s diag=%s", mdm, gps, diag);
 
-		if (setup_qmi(modem, qmi, net) < 0)
+		if (setup_qmi_qmux(modem, qmi, net) < 0)
 			return FALSE;
 
 		ofono_modem_set_string(modem->modem, "Modem", mdm);
-- 
2.45.0

[PATCH 2/6] gobi: Rename KernelDriver to NetworkInterfaceDriver

[Denis Kenzior]

The naming should now more accurately reflect what the driver string
represents.
---
 plugins/gobi.c   | 10 ++++++----
 plugins/udevng.c |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index d8b78ad6f5b0..5bb0b5ace0ca 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -103,9 +103,9 @@ static void gobi_debug(const char *str, void *user_data)
 static int gobi_probe(struct ofono_modem *modem)
 {
 	struct gobi_data *data;
-	const char *kernel_driver;
 	const char *value;
 	enum qmi_protocol protocol;
+	const char *if_driver;
 
 	DBG("%p", modem);
 
@@ -118,12 +118,14 @@ static int gobi_probe(struct ofono_modem *modem)
 	else
 		return -EPROTO;
 
+	if_driver = ofono_modem_get_string(modem,
+						"NetworkInterfaceKernelDriver");
+	DBG("netdev driver: %s", if_driver);
+
 	data = l_new(struct gobi_data, 1);
 	data->protocol = protocol;
-	kernel_driver = ofono_modem_get_string(modem, "KernelDriver");
-	DBG("kernel_driver: %s", kernel_driver);
 
-	if (!strcmp(kernel_driver, "qmi_wwan_q"))
+	if (!strcmp(if_driver, "qmi_wwan_q"))
 		data->using_qmi_wwan_q = true;
 
 	data->main_net_ifindex =
diff --git a/plugins/udevng.c b/plugins/udevng.c
index aa2a216d4247..c7db66238681 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -216,7 +216,7 @@ static int setup_qmi_netdev(struct modem_info *modem,
 	if (r < 0)
 		return r;
 
-	ofono_modem_set_string(modem->modem, "KernelDriver",
+	ofono_modem_set_string(modem->modem, "NetworkInterfaceKernelDriver",
 							net->kernel_driver);
 	ofono_modem_set_string(modem->modem, "NetworkInterface", net->devnode);
 	ofono_modem_set_integer(modem->modem, "NetworkInterfaceIndex",
-- 
2.45.0

[PATCH 3/6] gobi: ensure required properties are provided

[Denis Kenzior]

Ensure that a minimum set of properties required to drive the modem are
provided by the detection framework.  Also, while here, document these
properties.
---
 plugins/gobi.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index 5bb0b5ace0ca..bb4560c43a06 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -100,12 +100,37 @@ static void gobi_debug(const char *str, void *user_data)
 	ofono_info("%s%s", prefix, str);
 }
 
+/*
+ * Probe the modem.  The following modem properties are expected to be set
+ * in order to initialize the driver properly:
+ *
+ * DeviceProtocol
+ *   Can be "qrtr" or "qmux", tells the driver which QMI encapsulation protocol
+ *   is being used.
+ *
+ * NetworkInterface
+ *   The string that contains the 'main' network device.  This can be
+ *   "rmnet_ipa" on SoC systems, or "wwan0" for upstream linux systems.
+ *
+ * NetworkInterfaceIndex
+ *   The index of the main interface given by NetworkInterface
+ *
+ * NetworkInterfaceDriver
+ *   The kernel driver that is being used by the main network device.  Certain
+ *   drivers such as 'qmi_wwan' or 'qmi_wwan_q' are treated specifically.
+ *
+ * Bus
+ *   The bus of the modem.  Values can be "usb", "embedded", or "pci"
+ */
 static int gobi_probe(struct ofono_modem *modem)
 {
 	struct gobi_data *data;
 	const char *value;
 	enum qmi_protocol protocol;
 	const char *if_driver;
+	const char *ifname;
+	int ifindex;
+	const char *bus;
 
 	DBG("%p", modem);
 
@@ -120,7 +145,14 @@ static int gobi_probe(struct ofono_modem *modem)
 
 	if_driver = ofono_modem_get_string(modem,
 						"NetworkInterfaceKernelDriver");
-	DBG("netdev driver: %s", if_driver);
+	ifname = ofono_modem_get_string(modem, "NetworkInterface");
+	ifindex = ofono_modem_get_integer(modem, "NetworkInterfaceIndex");
+	bus = ofono_modem_get_string(modem, "Bus");
+
+	DBG("net: %s[%s](%d) %s", ifname, if_driver, ifindex, bus);
+
+	if (!if_driver || !ifname || !ifindex || !bus)
+		return -EPROTO;
 
 	data = l_new(struct gobi_data, 1);
 	data->protocol = protocol;
@@ -133,8 +165,6 @@ static int gobi_probe(struct ofono_modem *modem)
 	l_strlcpy(data->main_net_name,
 			ofono_modem_get_string(modem, "NetworkInterface"),
 			sizeof(data->main_net_name));
-	DBG("net: %s (%d)", data->main_net_name, data->main_net_ifindex);
-
 	ofono_modem_set_data(modem, data);
 	ofono_modem_set_capabilities(modem, OFONO_MODEM_CAPABILITY_LTE);
 
-- 
2.45.0

[PATCH 4/6] udevng: add and use get_ifname() for netdev nodes

[Denis Kenzior]

For USB devices, 'devnode' member contains the path to the character
device, typically under /dev.  For example, /dev/cdc-wdm0.  For network
devices that are part of the USB composite device, 'devnode' was
overloaded to contain the network interface.  Unfortunately, this code
path / logic was only active when add_device() was called with
MODEM_TYPE_USB.  For non-USB bus devices 'devnode' was always NULL.
This led to NetworkInterface properties being set improperly:

ofonod[4874]: plugins/gobi.c:gobi_probe() net: (null)[mhi_net](2) pcie
ofonod[4874]: plugins/udevng.c:create_modem() could not register modem 'mhi'

Fix this by introducing a new get_ifname() helper which will query the
network device node information for the network interface name as
follows:
	- Query ID_NET_NAME property.  This is used on newer kernels.
	- Query INTERFACE property.  This is used on newer and legacy
	  kernels.
	- Fall back to sysname in case none of the above properties are
	  present.
---
 plugins/udevng.c | 85 +++++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 34 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index c7db66238681..80a57434de78 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -78,6 +78,23 @@ struct serial_device_info {
 	struct udev_device *dev;
 };
 
+static const char *get_ifname(const struct device_info *info)
+{
+	struct udev_device *udev_device = info->udev_device;
+	const char *net_name;
+
+	net_name = udev_device_get_property_value(udev_device, "ID_NET_NAME");
+	if (net_name)
+		return net_name;
+
+	net_name = udev_device_get_property_value(udev_device, "INTERFACE");
+	if (net_name)
+		return net_name;
+
+	/* Fall back to using sysname (M: field in udevadm) */
+	return udev_device_get_sysname(udev_device);
+}
+
 static gboolean setup_isi(struct modem_info *modem)
 {
 	const char *node = NULL;
@@ -96,7 +113,7 @@ static gboolean setup_isi(struct modem_info *modem)
 			if (g_strcmp0(info->interface, "2/254/0") == 0)
 				addr = 16;
 
-			node = info->devnode;
+			node = get_ifname(info);
 		}
 	}
 
@@ -142,7 +159,7 @@ static gboolean setup_mbm(struct modem_info *modem)
 						"gw") == TRUE ||
 				g_str_has_suffix(info->sysattr,
 						"NetworkAdapter") == TRUE) {
-			network = info->devnode;
+			network = get_ifname(info);
 		}
 	}
 
@@ -178,9 +195,12 @@ static gboolean setup_hso(struct modem_info *modem)
 			app = info->devnode;
 		else if (g_strcmp0(info->sysattr, "Modem") == 0)
 			mdm = info->devnode;
-		else if (info->sysattr == NULL &&
-				g_str_has_prefix(info->devnode, "hso") == TRUE)
-			net = info->devnode;
+		else if (!info->sysattr) {
+			const char *net_name = get_ifname(info);
+
+			if (g_str_has_prefix(net_name, "hso"))
+				net = get_ifname(info);
+		}
 	}
 
 	if (ctl == NULL || app == NULL)
@@ -218,7 +238,8 @@ static int setup_qmi_netdev(struct modem_info *modem,
 
 	ofono_modem_set_string(modem->modem, "NetworkInterfaceKernelDriver",
 							net->kernel_driver);
-	ofono_modem_set_string(modem->modem, "NetworkInterface", net->devnode);
+	ofono_modem_set_string(modem->modem, "NetworkInterface",
+							get_ifname(net));
 	ofono_modem_set_integer(modem->modem, "NetworkInterfaceIndex",
 							ifindex);
 
@@ -230,7 +251,8 @@ static int setup_qmi_qmux(struct modem_info *modem,
 				const struct device_info *net)
 {
 	DBG("qmi: %s net: %s kernel_driver: %s interface_number: %s",
-		qmi->devnode, net->devnode, net->kernel_driver, net->number);
+			qmi->devnode, get_ifname(net),
+			net->kernel_driver, net->number);
 
 	if (modem->type != MODEM_TYPE_USB)
 		return -ENOTSUP;
@@ -254,7 +276,7 @@ static int setup_qmi_qmux(struct modem_info *modem,
 static int setup_qmi_qrtr(struct modem_info *modem,
 				const struct device_info *net)
 {
-	DBG("net: %s kernel_driver: %s", net->devnode, net->kernel_driver);
+	DBG("net: %s kernel_driver: %s", get_ifname(net), net->kernel_driver);
 
 	switch (modem->type) {
 	case MODEM_TYPE_EMBEDDED:
@@ -389,7 +411,7 @@ static gboolean setup_gobi(struct modem_info *modem)
 		return FALSE;
 
 	DBG("qmi=%s net=%s mdm=%s gps=%s diag=%s",
-			qmi->devnode, net->devnode, mdm, gps, diag);
+			qmi->devnode, get_ifname(net), mdm, gps, diag);
 
 	if (setup_qmi_qmux(modem, qmi, net) < 0)
 		return FALSE;
@@ -465,10 +487,11 @@ static gboolean setup_sierra(struct modem_info *modem)
 	if (mdm == NULL || net == NULL)
 		return FALSE;
 
-	ofono_modem_set_string(modem->modem, "NetworkInterface", net->devnode);
+	ofono_modem_set_string(modem->modem, "NetworkInterface",
+					get_ifname(net));
 done:
 	DBG("modem=%s app=%s net=%s diag=%s qmi=%s",
-			mdm, app, net->devnode, diag, qmi->devnode);
+			mdm, app, get_ifname(net), diag, qmi->devnode);
 
 	ofono_modem_set_string(modem->modem, "Modem", mdm);
 	ofono_modem_set_string(modem->modem, "App", app);
@@ -543,10 +566,11 @@ static gboolean setup_huawei(struct modem_info *modem)
 	if (mdm == NULL || pcui == NULL)
 		return FALSE;
 
-	ofono_modem_set_string(modem->modem, "NetworkInterface", net->devnode);
+	ofono_modem_set_string(modem->modem, "NetworkInterface",
+					get_ifname(net));
 done:
 	DBG("mdm=%s pcui=%s diag=%s qmi=%s net=%s",
-		mdm, pcui, diag, qmi->devnode, net->devnode);
+		mdm, pcui, diag, qmi->devnode, get_ifname(net));
 
 	ofono_modem_set_string(modem->modem, "Modem", mdm);
 	ofono_modem_set_string(modem->modem, "Pcui", pcui);
@@ -646,11 +670,11 @@ static gboolean setup_icera(struct modem_info *modem)
 				mdm = info->devnode;
 		} else if (g_strcmp0(info->interface, "2/6/0") == 0) {
 			if (g_strcmp0(info->number, "05") == 0)
-				net = info->devnode;
+				net = get_ifname(info);
 			else if (g_strcmp0(info->number, "06") == 0)
-				net = info->devnode;
+				net = get_ifname(info);
 			else if (g_strcmp0(info->number, "07") == 0)
-				net = info->devnode;
+				net = get_ifname(info);
 		}
 	}
 
@@ -823,7 +847,7 @@ static gboolean setup_telit(struct modem_info *modem)
 				gps = info->devnode;
 		} else if (info->sysattr && (g_str_has_suffix(info->sysattr,
 						"CDC NCM") == TRUE)) {
-			net = info->devnode;
+			net = get_ifname(info);
 		}
 	}
 
@@ -1022,7 +1046,7 @@ static gboolean setup_samsung(struct modem_info *modem)
 		if (g_strcmp0(info->interface, "10/0/0") == 0)
 			control = info->devnode;
 		else if (g_strcmp0(info->interface, "255/0/0") == 0)
-			network = info->devnode;
+			network = get_ifname(info);
 	}
 
 	if (control == NULL && network == NULL)
@@ -1249,7 +1273,7 @@ static gboolean setup_mbim(struct modem_info *modem)
 		if (g_strcmp0(subsystem, "usbmisc") == 0) /* cdc-wdm */
 			ctl = info->devnode;
 		else if (g_strcmp0(subsystem, "net") == 0) /* wwan */
-			net = info->devnode;
+			net = get_ifname(info);
 		else if (g_strcmp0(subsystem, "tty") == 0) {
 			if (g_strcmp0(info->number, "02") == 0)
 				atcmd = info->devnode;
@@ -1407,7 +1431,7 @@ static gboolean setup_ublox(struct modem_info *modem)
 				g_strcmp0(info->interface, "2/13/0") == 0 ||
 				g_strcmp0(info->interface, "10/0/0") == 0 ||
 				g_strcmp0(info->interface, "224/1/3") == 0) {
-			net = info->devnode;
+			net = get_ifname(info);
 		}
 	}
 
@@ -1452,7 +1476,7 @@ static gboolean setup_gemalto(struct modem_info *modem)
 			else if (g_strcmp0(info->number, "03") == 0)
 				mdm = info->devnode;
 			else if (g_strcmp0(subsystem, "net") == 0)
-				net = info->devnode;
+				net = get_ifname(info);
 			else if (g_strcmp0(subsystem, "usbmisc") == 0)
 				qmi = info->devnode;
 		}
@@ -1470,9 +1494,9 @@ static gboolean setup_gemalto(struct modem_info *modem)
 		if (g_strcmp0(info->interface, "2/6/0") == 0) {
 			if (g_strcmp0(subsystem, "net") == 0) {
 				if (g_strcmp0(info->number, "0a") == 0)
-					net = info->devnode;
+					net = get_ifname(info);
 				if (g_strcmp0(info->number, "0c") == 0)
-					net2 = info->devnode;
+					net2 = get_ifname(info);
 			}
 		}
 	}
@@ -1535,11 +1559,11 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
 				} else if (g_strcmp0(subsystem, "net")
 									== 0) {
 					if (g_strcmp0(info->number, "06") == 0)
-						net = info->devnode;
+						net = get_ifname(info);
 					if (g_strcmp0(info->number, "08") == 0)
-						net2 = info->devnode;
+						net2 = get_ifname(info);
 					if (g_strcmp0(info->number, "0a") == 0)
-						net3 = info->devnode;
+						net3 = get_ifname(info);
 				}
 			} else {
 				if (g_strcmp0(subsystem, "tty") == 0) {
@@ -1548,7 +1572,7 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
 				} else if (g_strcmp0(subsystem, "net")
 									== 0) {
 					if (g_strcmp0(info->number, "00") == 0)
-						net = info->devnode;
+						net = get_ifname(info);
 				}
 			}
 
@@ -1992,13 +2016,6 @@ static void add_device(const char *modem_syspath, const char *modem_devname,
 
 	if (modem->type == MODEM_TYPE_USB) {
 		devnode = udev_device_get_devnode(device);
-		if (devnode == NULL) {
-			devnode = udev_device_get_property_value(device,
-							"INTERFACE");
-			if (devnode == NULL)
-				return;
-		}
-
 		usb_interface = udev_device_get_parent_with_subsystem_devtype(
 							device, "usb",
 							"usb_interface");
-- 
2.45.0

[PATCH 5/6] udevng: Add mhi subsystem detection

[Denis Kenzior]

For now, only Qualcomm derived firmware devices that support QRTR +
mhi_net are detected.
---
 plugins/udevng.c | 201 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 189 insertions(+), 12 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 80a57434de78..d3ce3d5b3f7c 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -44,6 +44,7 @@ enum modem_type {
 	MODEM_TYPE_SERIAL,
 	MODEM_TYPE_PCIE,
 	MODEM_TYPE_EMBEDDED,
+	MODEM_TYPE_MHI,
 };
 
 struct modem_info {
@@ -282,6 +283,9 @@ static int setup_qmi_qrtr(struct modem_info *modem,
 	case MODEM_TYPE_EMBEDDED:
 		ofono_modem_set_string(modem->modem, "Bus", "embedded");
 		break;
+	case MODEM_TYPE_MHI:
+		ofono_modem_set_string(modem->modem, "Bus", "pcie");
+		break;
 	case MODEM_TYPE_USB:
 	case MODEM_TYPE_SERIAL:
 	case MODEM_TYPE_PCIE:
@@ -1730,6 +1734,42 @@ static gboolean setup_sim76xx(struct modem_info *modem)
 	return TRUE;
 }
 
+static gboolean setup_mhi(struct modem_info *modem)
+{
+	const struct device_info *net = NULL;
+	const struct device_info *qrtr = NULL;
+	GSList *list;
+	int r;
+
+	DBG("%s", modem->syspath);
+
+	for (list = modem->devices; list; list = list->next) {
+		const struct device_info *info = list->data;
+		const char *subsystem =
+				udev_device_get_subsystem(info->udev_device);
+
+		DBG("%s", udev_device_get_syspath(info->udev_device));
+
+		if (l_streq0(udev_device_get_property_value(info->udev_device,
+								"MODALIAS"),
+					"mhi:IPCR"))
+			qrtr = info;
+		else if (l_streq0(subsystem, "net"))
+			net = info;
+	}
+
+	DBG("net: %p, qrtr: %p", net, qrtr);
+
+	if (!net || !qrtr)
+		return FALSE;
+
+	r = setup_qmi_qrtr(modem, net);
+	if (r < 0)
+		return FALSE;
+
+	return TRUE;
+}
+
 static struct {
 	const char *name;
 	gboolean (*setup)(struct modem_info *modem);
@@ -1772,6 +1812,7 @@ static struct {
 	{ "tc65",	setup_tc65		},
 	{ "ehs6",	setup_ehs6		},
 	{ "gobiqrtr",	setup_gobi_qrtr		},
+	{ "mhi",	setup_mhi		},
 	{ }
 };
 
@@ -1823,6 +1864,7 @@ static void destroy_modem(gpointer data)
 	case MODEM_TYPE_USB:
 	case MODEM_TYPE_PCIE:
 	case MODEM_TYPE_EMBEDDED:
+	case MODEM_TYPE_MHI:
 		for (list = modem->devices; list; list = list->next) {
 			struct device_info *info = list->data;
 
@@ -1854,6 +1896,7 @@ static gboolean check_remove(gpointer key, gpointer value, gpointer user_data)
 	switch (modem->type) {
 	case MODEM_TYPE_USB:
 	case MODEM_TYPE_PCIE:
+	case MODEM_TYPE_MHI:
 		for (list = modem->devices; list; list = list->next) {
 			struct device_info *info = list->data;
 			const char *syspath =
@@ -2315,11 +2358,137 @@ static void check_pci_device(struct udev_device *device)
 			device, kernel_driver);
 }
 
+static const struct {
+	const char *driver;
+	uint16_t vend;
+	uint16_t dev;
+	uint16_t subvend;
+	uint16_t subdev;
+} wwan_driver_list[] = {
+	{ "mhi",		0x17cb, 0x0308, },
+	{ }
+};
+
+static int parse_pci_id(const char *id, uint16_t *out_vend, uint16_t *out_dev)
+{
+	_auto_(l_strv_free) char **ids = l_strsplit(id, ':');
+	int r;
+
+	if (!ids || !ids[0] || !ids[1] || ids[2])
+		return -EINVAL;
+
+	r = l_safe_atox16(ids[0], out_vend);
+	if (r < 0)
+		return r;
+
+	r = l_safe_atox16(ids[1], out_dev);
+	if (r < 0)
+		return r;
+
+	return 0;
+}
+
+static bool add_mhi_device(struct udev_device *device,
+						struct udev_device *parent)
+{
+	const char *syspath;
+	const char *kernel_driver;
+	const char *pci_id;
+	const char *pci_subid;
+	uint16_t vend, dev, subvend, subdev;
+	unsigned int i;
+
+	/* Use syspath of the MHI device as the modem path */
+	syspath = udev_device_get_syspath(parent);
+	if (!syspath)
+		return false;
+
+	kernel_driver = udev_device_get_property_value(device, "ID_NET_DRIVER");
+	if (!kernel_driver)
+		kernel_driver = udev_device_get_driver(device);
+
+	/* vid / pid is on the parent of the MHI device */
+	pci_id = udev_device_get_property_value(parent, "PCI_ID");
+	pci_subid = udev_device_get_property_value(parent, "PCI_SUBSYS_ID");
+
+	if (!pci_id || !pci_subid)
+		return false;
+
+	if (parse_pci_id(pci_id, &vend, &dev) < 0)
+		return false;
+
+	if (parse_pci_id(pci_subid, &subvend, &subdev) < 0)
+		return false;
+
+	for (i = 0; wwan_driver_list[i].driver; i++) {
+		if (wwan_driver_list[i].vend != vend)
+			continue;
+
+		if (wwan_driver_list[i].dev != dev)
+			continue;
+
+		if (wwan_driver_list[i].subvend &&
+				wwan_driver_list[i].subvend != subvend)
+			continue;
+
+		if (wwan_driver_list[i].subdev &&
+				wwan_driver_list[i].subdev != subdev)
+			continue;
+
+		add_device(syspath, NULL, wwan_driver_list[i].driver,
+				NULL, NULL, MODEM_TYPE_MHI,
+				device, kernel_driver);
+		return true;
+	}
+
+	return false;
+}
+
+static void check_wwan_device(struct udev_device *device)
+{
+	struct udev_device *parent;
+
+	parent = udev_device_get_parent_with_subsystem_devtype(device,
+								"mhi", NULL);
+	if (!parent)
+		return;
+
+	/* If this is an MHI device, find the MHI parent */
+	parent = udev_device_get_parent(parent);
+	if (!parent)
+		return;
+
+	add_mhi_device(device, parent);
+}
+
+static void check_mhi_device(struct udev_device *device)
+{
+	struct udev_device *parent =
+		udev_device_get_parent_with_subsystem_devtype(device,
+								"pci", NULL);
+
+	if (!parent)
+		return;
+
+	add_mhi_device(device, parent);
+}
+
 static void check_net_device(struct udev_device *device)
 {
 	char path[32];
 	const char *name;
 	const char *iflink;
+	struct udev_device *parent;
+
+	parent = udev_device_get_parent(device);
+	if (parent && l_streq0(udev_device_get_subsystem(parent), "mhi")) {
+		parent = udev_device_get_parent_with_subsystem_devtype(device,
+								"pci", NULL);
+		if (parent)
+			add_mhi_device(device, parent);
+
+		return;
+	}
 
 	name = udev_device_get_sysname(device);
 	if (!l_str_has_prefix(name, "rmnet_"))
@@ -2337,25 +2506,29 @@ static void check_net_device(struct udev_device *device)
 
 static void check_device(struct udev_device *device)
 {
-	const char *bus;
+	const char *subsystem = udev_device_get_subsystem(device);
+	const char *bus = udev_device_get_property_value(device, "ID_BUS");
 
-	bus = udev_device_get_property_value(device, "ID_BUS");
-	if (bus == NULL) {
-		bus = udev_device_get_subsystem(device);
-		if (bus == NULL)
-			return;
+	if (l_streq0(subsystem, "net")) {
+		/* Handle USB-connected network devices in check_usb_device */
+		if (l_streq0(bus, "usb"))
+			check_usb_device(device);
+		else
+			check_net_device(device);
+
+		return;
 	}
 
-	if ((g_str_equal(bus, "usb") == TRUE) ||
-			(g_str_equal(bus, "usbmisc") == TRUE))
+	if (l_streq0(subsystem, "usb") || l_streq0(subsystem, "usbmisc"))
 		check_usb_device(device);
-	else if (g_str_equal(bus, "pci") == TRUE)
+	else if (l_streq0(subsystem, "pci"))
 		check_pci_device(device);
-	else if (g_str_equal(bus, "net") == TRUE)
-		check_net_device(device);
+	else if (l_streq0(subsystem, "wwan"))
+		check_wwan_device(device);
+	else if (l_streq0(subsystem, "mhi"))
+		check_mhi_device(device);
 	else
 		add_serial_device(device);
-
 }
 
 static gboolean create_modem(gpointer key, gpointer value, gpointer user_data)
@@ -2414,6 +2587,8 @@ static void enumerate_devices(struct udev *context)
 	udev_enumerate_add_match_subsystem(enumerate, "net");
 	udev_enumerate_add_match_subsystem(enumerate, "hsi");
 	udev_enumerate_add_match_subsystem(enumerate, "pci");
+	udev_enumerate_add_match_subsystem(enumerate, "wwan");
+	udev_enumerate_add_match_subsystem(enumerate, "mhi");
 
 	udev_enumerate_scan_devices(enumerate);
 
@@ -2539,6 +2714,8 @@ static int detect_init(void)
 							"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_add_match_subsystem_devtype(udev_mon, "wwan", NULL);
+	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "mhi", NULL);
 
 	udev_monitor_filter_update(udev_mon);
 
-- 
2.45.0

[PATCH 6/6] qmi: gprs-context: support bind_mux for pcie devices

[Denis Kenzior]

Add support for PCIe based devices when invoking
QMI_WDS_BIND_MUX_DATA_PORT command.  While here, also refactor the
handling of InterfaceNumber property and make it clear that is is
only used for USB based devices.
---
 drivers/qmimodem/gprs-context.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/qmimodem/gprs-context.c b/drivers/qmimodem/gprs-context.c
index 3a4f2f0717bc..6350d6584d8f 100644
--- a/drivers/qmimodem/gprs-context.c
+++ b/drivers/qmimodem/gprs-context.c
@@ -525,23 +525,29 @@ static void qmi_gprs_context_bind_mux(struct ofono_gprs_context *gc)
 		goto error;
 	}
 
-	interface_number = ofono_modem_get_string(modem, "InterfaceNumber");
-	if (!interface_number && endpoint_info.endpoint_type !=
-					QMI_DATA_ENDPOINT_TYPE_EMBEDDED) {
-		ofono_error("%s: Missing 'InterfaceNumber'",
-					ofono_modem_get_path(modem));
-		goto error;
-	} else if (!interface_number)
-		u8 = 1;	/* Default for embedded modems */
-	else if (l_safe_atox8(interface_number, &u8) < 0) {
-		ofono_error("%s: Invalid InterfaceNumber",
+	switch (endpoint_info.endpoint_type) {
+	case QMI_DATA_ENDPOINT_TYPE_PCIE:
+		endpoint_info.interface_number = 0x04; /* Magic for PCIE */
+		break;
+	case QMI_DATA_ENDPOINT_TYPE_EMBEDDED:
+		endpoint_info.interface_number = 0x01;
+		break;
+	case QMI_DATA_ENDPOINT_TYPE_HSUSB:
+		interface_number = ofono_modem_get_string(modem,
+							"InterfaceNumber");
+		if (!l_safe_atox8(interface_number, &u8)) {
+			endpoint_info.interface_number = u8;
+			break;
+		}
+
+		ofono_error("%s: Missing or invalid 'InterfaceNumber'",
 					ofono_modem_get_path(modem));
+		/* Fall through */
+	default:
 		goto error;
 	}
 
-	endpoint_info.interface_number = u8;
-
-	DBG("interface_number: %d", u8);
+	DBG("interface_number: %d", endpoint_info.interface_number);
 	DBG("mux_id: %hhx", data->mux_id);
 
 	param = qmi_param_new();
-- 
2.45.0

Re: [PATCH 1/6] gobi: add / use DeviceProtocol property

[patchwork-bot+ofono]

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Mon, 20 May 2024 14:17:02 -0500 you wrote:
> Instead of using the kernel driver as a hint for whether qrtr or qmi is
> being used, introduce an explicit DeviceProtocol property that should be
> set by the detection logic to tell the driver whether to use qrtr or
> qmux.
> 
> Split up QMI modem driver detection and setup logic inside udevng into
> setup_qmi_qrtr(), setup_qmi_qmux() and setup_qmi_netdev().
> setup_qmi_netdev() takes care of setting up of common networking device
> properties, while setup_qmi_qrtr() and setup_qmi_qmux() setup QRTR and
> QMUX devices, respectively.
> 
> [...]

Here is the summary with links:
  - [1/6] gobi: add / use DeviceProtocol property
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=cebbc86ce7a7
  - [2/6] gobi: Rename KernelDriver to NetworkInterfaceDriver
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=df18422bafc8
  - [3/6] gobi: ensure required properties are provided
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=b3ff67d78830
  - [4/6] udevng: add and use get_ifname() for netdev nodes
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=6d185edd5066
  - [5/6] udevng: Add mhi subsystem detection
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=cdd58360140a
  - [6/6] qmi: gprs-context: support bind_mux for pcie devices
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=cb16587e46e6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html