Message-ID: <16135e8d.86f9.19619ac8560.Coremail.slark_xiao@163.com>
Date: Wed, 9 Apr 2025 16:30:58 +0800 (CST)
From: "Slark Xiao" <slark_xiao@....com>
To: "Sergey Ryazanov" <ryazanov.s.a@...il.com>
Cc: "Loic Poulain" <loic.poulain@....qualcomm.com>,
	"Johannes Berg" <johannes@...solutions.net>,
	"Andrew Lunn" <andrew+netdev@...n.ch>,
	"Eric Dumazet" <edumazet@...gle.com>,
	"David S . Miller" <davem@...emloft.net>,
	"Jakub Kicinski" <kuba@...nel.org>,
	"Paolo Abeni" <pabeni@...hat.com>, netdev@...r.kernel.org,
	"Muhammad Nuzaihan" <zaihan@...ealasia.net>,
	"Qiang Yu" <quic_qianyu@...cinc.com>,
	"Manivannan Sadhasivam" <manivannan.sadhasivam@...aro.org>,
	"Johan Hovold" <johan@...nel.org>
Subject: Re:Re:[RFC PATCH 4/6] net: wwan: add NMEA port support


Hi Sergey,
Device port /dev/gnss0 is enumerated . Does it be expected?
I can get the NMEA data from this port by cat or minicom command.
But the gpsd.service also can not be initialized normally. It reports:

TriggeredBy: ● gpsd.socket
    Process: 3824 ExecStartPre=/bin/stty speed 115200 -F $DEVICES (code=exited, status=1/FAILURE)
        CPU: 7ms

4月 09 16:04:16 jbd systemd[1]: Starting GPS (Global Positioning System) Daemon...
4月 09 16:04:17 jbd stty[3824]: /bin/stty: /dev/gnss0: Inappropriate ioctl for device
4月 09 16:04:17 jbd systemd[1]: gpsd.service: Control process exited, code=exited, status=1/FAILURE
4月 09 16:04:17 jbd systemd[1]: gpsd.service: Failed with result 'exit-code'.
4月 09 16:04:17 jbd systemd[1]: Failed to start GPS (Global Positioning System) Daemon.

Seems it's not a serial port.
Any advice?

Thanks

At 2025-04-09 11:54:50, "Slark Xiao" <slark_xiao@....com> wrote:
>
>Hi Sergey,
>I saw you add WWAN_PORT_NMEA here. And I have a concern, shall we
>add it into mhi_wwan_ctrl.c for mapping "NMEA" channel in 
>mhi_wwan_ctrl_match_table since previous QDU100 device has added
>NMEA channel(So only "NMEA" channel is valid in pci_generic.c, but not
>"GNSS" in future).
>
>I am testing your patch now. Will update later.
>Thanks
>
>At 2025-04-09 07:31:16, "Sergey Ryazanov" <ryazanov.s.a@...il.com> wrote:
>>Many WWAN modems come with embedded GNSS receiver inside and have a
>>dedicated port to output geopositioning data. On the one hand, the
>>GNSS receiver has little in common with WWAN modem and just shares a
>>host interface and should be exported using the GNSS subsystem. On the
>>other hand, GNSS receiver is not automatically activated and needs a
>>generic WWAN control port (AT, MBIM, etc.) to be turned on. And a user
>>space software needs extra information to find the control port.
>>
>>Introduce the new type of WWAN port - NMEA. When driver asks to register
>>a NMEA port, the core allocates common parent WWAN device as usual, but
>>exports the NMEA port via the GNSS subsystem and acts as a proxy between
>>the device driver and the GNSS subsystem.
>>
>>From the WWAN device driver perspective, a NMEA port is registered as a
>>regular WWAN port without any difference. And the driver interacts only
>>with the WWAN core. From the user space perspective, the NMEA port is a
>>GNSS device which parent can be used to enumerate and select the proper
>>control port for the GNSS receiver management.
>>
>>CC: Slark Xiao <slark_xiao@....com>
>>CC: Muhammad Nuzaihan <zaihan@...ealasia.net>
>>CC: Qiang Yu <quic_qianyu@...cinc.com>
>>CC: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
>>CC: Johan Hovold <johan@...nel.org>
>>Suggested-by: Loic Poulain <loic.poulain@....qualcomm.com>
>>Signed-off-by: Sergey Ryazanov <ryazanov.s.a@...il.com>
>>---
>> drivers/net/wwan/Kconfig     |   1 +
>> drivers/net/wwan/wwan_core.c | 157 ++++++++++++++++++++++++++++++++++-
>> include/linux/wwan.h         |   2 +
>> 3 files changed, 156 insertions(+), 4 deletions(-)
>>
>>diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
>>index 410b0245114e..88df55d78d90 100644
>>--- a/drivers/net/wwan/Kconfig
>>+++ b/drivers/net/wwan/Kconfig
>>@@ -7,6 +7,7 @@ menu "Wireless WAN"
>> 
>> config WWAN
>> 	tristate "WWAN Driver Core"
>>+	depends on GNSS || GNSS = n
>> 	help
>> 	  Say Y here if you want to use the WWAN driver core. This driver
>> 	  provides a common framework for WWAN drivers.
>>diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
>>index 439a57bc2b9c..a30f0c89aa82 100644
>>--- a/drivers/net/wwan/wwan_core.c
>>+++ b/drivers/net/wwan/wwan_core.c
>>@@ -16,6 +16,7 @@
>> #include <linux/types.h>
>> #include <linux/uaccess.h>
>> #include <linux/termios.h>
>>+#include <linux/gnss.h>
>> #include <linux/wwan.h>
>> #include <net/rtnetlink.h>
>> #include <uapi/linux/wwan.h>
>>@@ -89,9 +90,16 @@ struct wwan_port {
>> 			struct ktermios termios;
>> 			int mdmbits;
>> 		} at_data;
>>+		struct gnss_device *gnss;
>> 	};
>> };
>> 
>>+static int wwan_port_op_start(struct wwan_port *port);
>>+static void wwan_port_op_stop(struct wwan_port *port);
>>+static int wwan_port_op_tx(struct wwan_port *port, struct sk_buff *skb,
>>+			   bool nonblock);
>>+static int wwan_wait_tx(struct wwan_port *port, bool nonblock);
>>+
>> static ssize_t index_show(struct device *dev, struct device_attribute *attr, char *buf)
>> {
>> 	struct wwan_device *wwan = to_wwan_dev(dev);
>>@@ -340,6 +348,7 @@ static const struct {
>> 		.name = "MIPC",
>> 		.devsuf = "mipc",
>> 	},
>>+	/* WWAN_PORT_NMEA is exported via the GNSS subsystem */
>> };
>> 
>> static ssize_t type_show(struct device *dev, struct device_attribute *attr,
>>@@ -498,6 +507,132 @@ static void wwan_port_unregister_wwan(struct wwan_port *port)
>> 	device_unregister(&port->dev);
>> }
>> 
>>+#if IS_ENABLED(CONFIG_GNSS)
>>+static int wwan_gnss_open(struct gnss_device *gdev)
>>+{
>>+	return wwan_port_op_start(gnss_get_drvdata(gdev));
>>+}
>>+
>>+static void wwan_gnss_close(struct gnss_device *gdev)
>>+{
>>+	wwan_port_op_stop(gnss_get_drvdata(gdev));
>>+}
>>+
>>+static int wwan_gnss_write(struct gnss_device *gdev, const unsigned char *buf,
>>+			   size_t count)
>>+{
>>+	struct wwan_port *port = gnss_get_drvdata(gdev);
>>+	struct sk_buff *skb, *head = NULL, *tail = NULL;
>>+	size_t frag_len, remain = count;
>>+	int ret;
>>+
>>+	ret = wwan_wait_tx(port, false);
>>+	if (ret)
>>+		return ret;
>>+
>>+	do {
>>+		frag_len = min(remain, port->frag_len);
>>+		skb = alloc_skb(frag_len + port->headroom_len, GFP_KERNEL);
>>+		if (!skb) {
>>+			ret = -ENOMEM;
>>+			goto freeskb;
>>+		}
>>+		skb_reserve(skb, port->headroom_len);
>>+		memcpy(skb_put(skb, frag_len), buf + count - remain, frag_len);
>>+
>>+		if (!head) {
>>+			head = skb;
>>+		} else {
>>+			if (!tail)
>>+				skb_shinfo(head)->frag_list = skb;
>>+			else
>>+				tail->next = skb;
>>+
>>+			tail = skb;
>>+			head->data_len += skb->len;
>>+			head->len += skb->len;
>>+			head->truesize += skb->truesize;
>>+		}
>>+	} while (remain -= frag_len);
>>+
>>+	ret = wwan_port_op_tx(port, head, false);
>>+	if (!ret)
>>+		return count;
>>+
>>+freeskb:
>>+	kfree_skb(head);
>>+	return ret;
>>+}
>>+
>>+static struct gnss_operations wwan_gnss_ops = {
>>+	.open = wwan_gnss_open,
>>+	.close = wwan_gnss_close,
>>+	.write_raw = wwan_gnss_write,
>>+};
>>+
>>+static int wwan_port_register_gnss(struct wwan_port *port)
>>+{
>>+	struct wwan_device *wwandev = to_wwan_dev(port->dev.parent);
>>+	struct gnss_device *gdev;
>>+	int err;
>>+
>>+	gdev = gnss_allocate_device(&wwandev->dev);
>>+	if (!gdev) {
>>+		err = -ENOMEM;
>>+		goto error_destroy_port;
>>+	}
>>+
>>+	/* NB: for now we support only NMEA WWAN port type, so hardcode
>>+	 * the GNSS port type. If more GNSS WWAN port types will be added,
>>+	 * then we should dynamically mapt WWAN port type to GNSS type.
>>+	 */
>>+	gdev->type = GNSS_TYPE_NMEA;
>>+	gdev->ops = &wwan_gnss_ops;
>>+	gnss_set_drvdata(gdev, port);
>>+
>>+	port->gnss = gdev;
>>+
>>+	err = gnss_register_device(gdev);
>>+	if (err) {
>>+		gnss_put_device(gdev);
>>+		goto error_destroy_port;
>>+	}
>>+
>>+	dev_info(&wwandev->dev, "port %s attached\n", dev_name(&gdev->dev));
>>+
>>+	return 0;
>>+
>>+error_destroy_port:
>>+	__wwan_port_destroy(port);
>>+
>>+	return err;
>>+}
>>+
>>+static void wwan_port_unregister_gnss(struct wwan_port *port)
>>+{
>>+	struct wwan_device *wwandev = to_wwan_dev(port->dev.parent);
>>+	struct gnss_device *gdev = port->gnss;
>>+
>>+	dev_info(&wwandev->dev, "port %s disconnected\n", dev_name(&gdev->dev));
>>+
>>+	gnss_deregister_device(gdev);
>>+	gnss_put_device(gdev);
>>+
>>+	__wwan_port_destroy(port);
>>+}
>>+#else
>>+static inline int wwan_port_register_gnss(struct wwan_port *port)
>>+{
>>+	__wwan_port_destroy(port);
>>+	return -EOPNOTSUPP;
>>+}
>>+
>>+static inline void wwan_port_unregister_gnss(struct wwan_port *port)
>>+{
>>+	WARN_ON(1);	/* This handler cannot be called */
>>+}
>>+#endif
>>+
>> struct wwan_port *wwan_create_port(struct device *parent,
>> 				   enum wwan_port_type type,
>> 				   const struct wwan_port_ops *ops,
>>@@ -536,7 +671,11 @@ struct wwan_port *wwan_create_port(struct device *parent,
>> 	port->dev.parent = &wwandev->dev;
>> 	dev_set_drvdata(&port->dev, drvdata);
>> 
>>-	err = wwan_port_register_wwan(port);
>>+	if (port->type == WWAN_PORT_NMEA)
>>+		err = wwan_port_register_gnss(port);
>>+	else
>>+		err = wwan_port_register_wwan(port);
>>+
>> 	if (err)
>> 		goto error_wwandev_remove;
>> 
>>@@ -564,7 +703,10 @@ void wwan_remove_port(struct wwan_port *port)
>> 	wake_up_interruptible(&port->waitqueue);
>> 	skb_queue_purge(&port->rxq);
>> 
>>-	wwan_port_unregister_wwan(port);
>>+	if (port->type == WWAN_PORT_NMEA)
>>+		wwan_port_unregister_gnss(port);
>>+	else
>>+		wwan_port_unregister_wwan(port);
>> 
>> 	/* Release related wwan device */
>> 	wwan_remove_dev(wwandev);
>>@@ -573,8 +715,15 @@ EXPORT_SYMBOL_GPL(wwan_remove_port);
>> 
>> void wwan_port_rx(struct wwan_port *port, struct sk_buff *skb)
>> {
>>-	skb_queue_tail(&port->rxq, skb);
>>-	wake_up_interruptible(&port->waitqueue);
>>+	if (port->type == WWAN_PORT_NMEA) {
>>+#if IS_ENABLED(CONFIG_GNSS)
>>+		gnss_insert_raw(port->gnss, skb->data, skb->len);
>>+#endif
>>+		consume_skb(skb);
>>+	} else {
>>+		skb_queue_tail(&port->rxq, skb);
>>+		wake_up_interruptible(&port->waitqueue);
>>+	}
>> }
>> EXPORT_SYMBOL_GPL(wwan_port_rx);
>> 
>>diff --git a/include/linux/wwan.h b/include/linux/wwan.h
>>index a4d6cc0c9f68..1e0e2cb53579 100644
>>--- a/include/linux/wwan.h
>>+++ b/include/linux/wwan.h
>>@@ -19,6 +19,7 @@
>>  * @WWAN_PORT_FASTBOOT: Fastboot protocol control
>>  * @WWAN_PORT_ADB: ADB protocol control
>>  * @WWAN_PORT_MIPC: MTK MIPC diagnostic interface
>>+ * @WWAN_PORT_NMEA: embedded GNSS receiver with NMEA output
>>  *
>>  * @WWAN_PORT_MAX: Highest supported port types
>>  * @WWAN_PORT_UNKNOWN: Special value to indicate an unknown port type
>>@@ -34,6 +35,7 @@ enum wwan_port_type {
>> 	WWAN_PORT_FASTBOOT,
>> 	WWAN_PORT_ADB,
>> 	WWAN_PORT_MIPC,
>>+	WWAN_PORT_NMEA,
>> 
>> 	/* Add new port types above this line */
>> 
>>-- 
>>2.45.3

Powered by blists - more mailing lists