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

Commit Message

Denis Kenzior May 20, 2024, 7:17 p.m. UTC

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(-)

Comments

[e-mail] May 28, 2024, 8:10 p.m. UTC | #1

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <[e-mail]>:

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!

Patch

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);