2024-06-14 09:57:09

by Vanillan Wang

[permalink] [raw]
Subject: [net-next v1] net: wwan: t7xx: Add debug port

From: Jinjian Song <[email protected]>

Add support for userspace to switch on the debug port(ADB,MIPC).
- ADB port: /dev/ccci_sap_adb
- MIPC port: /dev/ttyMIPC0

Switch on debug port:
- debug: 'echo debug > /sys/bus/pci/devices/${bdf}/t7xx_mode

Switch off debug port:
- normal: 'echo normal > /sys/bus/pci/devices/${bdf}/t7xx_mode

Signed-off-by: Jinjian Song <[email protected]>
---
drivers/net/wwan/t7xx/Makefile | 1 +
drivers/net/wwan/t7xx/t7xx_pci.c | 7 +
drivers/net/wwan/t7xx/t7xx_pci.h | 2 +
drivers/net/wwan/t7xx/t7xx_port.h | 16 ++
drivers/net/wwan/t7xx/t7xx_port_debug.c | 354 ++++++++++++++++++++++++
drivers/net/wwan/t7xx/t7xx_port_proxy.c | 45 ++-
drivers/net/wwan/t7xx/t7xx_port_proxy.h | 2 +
7 files changed, 426 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/wwan/t7xx/t7xx_port_debug.c

diff --git a/drivers/net/wwan/t7xx/Makefile b/drivers/net/wwan/t7xx/Makefile
index 2652cd00504e..b9684fd46d76 100644
--- a/drivers/net/wwan/t7xx/Makefile
+++ b/drivers/net/wwan/t7xx/Makefile
@@ -11,6 +11,7 @@ mtk_t7xx-y:= t7xx_pci.o \
t7xx_port_proxy.o \
t7xx_port_ctrl_msg.o \
t7xx_port_wwan.o \
+ t7xx_port_debug.o \
t7xx_hif_dpmaif.o \
t7xx_hif_dpmaif_tx.o \
t7xx_hif_dpmaif_rx.o \
diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c
index e0b1e7a616ca..6b18460d626c 100644
--- a/drivers/net/wwan/t7xx/t7xx_pci.c
+++ b/drivers/net/wwan/t7xx/t7xx_pci.c
@@ -41,6 +41,7 @@
#include "t7xx_pcie_mac.h"
#include "t7xx_reg.h"
#include "t7xx_state_monitor.h"
+#include "t7xx_port_proxy.h"

#define T7XX_PCI_IREG_BASE 0
#define T7XX_PCI_EREG_BASE 2
@@ -59,6 +60,8 @@ static const char * const t7xx_mode_names[] = {
[T7XX_FASTBOOT_SWITCHING] = "fastboot_switching",
[T7XX_FASTBOOT_DOWNLOAD] = "fastboot_download",
[T7XX_FASTBOOT_DUMP] = "fastboot_dump",
+ [T7XX_DEBUG] = "debug",
+ [T7XX_NORMAL] = "normal",
};

static_assert(ARRAY_SIZE(t7xx_mode_names) == T7XX_MODE_LAST);
@@ -82,6 +85,10 @@ static ssize_t t7xx_mode_store(struct device *dev,
} else if (index == T7XX_RESET) {
WRITE_ONCE(t7xx_dev->mode, T7XX_RESET);
t7xx_acpi_pldr_func(t7xx_dev);
+ } else if (index == T7XX_DEBUG) {
+ t7xx_proxy_port_debug(t7xx_dev, true);
+ } else if (index == T7XX_NORMAL) {
+ t7xx_proxy_port_debug(t7xx_dev, false);
}

return count;
diff --git a/drivers/net/wwan/t7xx/t7xx_pci.h b/drivers/net/wwan/t7xx/t7xx_pci.h
index 49a11586d8d8..bdcadeb035e0 100644
--- a/drivers/net/wwan/t7xx/t7xx_pci.h
+++ b/drivers/net/wwan/t7xx/t7xx_pci.h
@@ -50,6 +50,8 @@ enum t7xx_mode {
T7XX_FASTBOOT_SWITCHING,
T7XX_FASTBOOT_DOWNLOAD,
T7XX_FASTBOOT_DUMP,
+ T7XX_DEBUG,
+ T7XX_NORMAL,
T7XX_MODE_LAST, /* must always be last */
};

diff --git a/drivers/net/wwan/t7xx/t7xx_port.h b/drivers/net/wwan/t7xx/t7xx_port.h
index f74d3bab810d..aa9b0df1f730 100644
--- a/drivers/net/wwan/t7xx/t7xx_port.h
+++ b/drivers/net/wwan/t7xx/t7xx_port.h
@@ -27,6 +27,7 @@
#include <linux/types.h>
#include <linux/wait.h>
#include <linux/wwan.h>
+#include <linux/cdev.h>

#include "t7xx_hif_cldma.h"
#include "t7xx_pci.h"
@@ -42,6 +43,8 @@ enum port_ch {
/* to AP */
PORT_CH_AP_CONTROL_RX = 0x1000,
PORT_CH_AP_CONTROL_TX = 0x1001,
+ PORT_CH_AP_ADB_RX = 0x100a,
+ PORT_CH_AP_ADB_TX = 0x100b,

/* to MD */
PORT_CH_CONTROL_RX = 0x2000,
@@ -100,6 +103,16 @@ struct t7xx_port_conf {
struct port_ops *ops;
char *name;
enum wwan_port_type port_type;
+ bool debug;
+ char *class_name;
+ unsigned char baseminor;
+};
+
+struct t7xx_cdev {
+ struct t7xx_port *port;
+ struct cdev cdev;
+ dev_t dev_num;
+ struct class *dev_class;
};

struct t7xx_port {
@@ -134,6 +147,9 @@ struct t7xx_port {
struct {
struct rchan *relaych;
} log;
+ struct {
+ struct t7xx_cdev *debug_port;
+ } debug;
};
};

diff --git a/drivers/net/wwan/t7xx/t7xx_port_debug.c b/drivers/net/wwan/t7xx/t7xx_port_debug.c
new file mode 100644
index 000000000000..8c94a72f210d
--- /dev/null
+++ b/drivers/net/wwan/t7xx/t7xx_port_debug.c
@@ -0,0 +1,354 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021, MediaTek Inc.
+ * Copyright (c) 2024, Fibocom Wireless Inc.
+ *
+ * Authors: Jinjian Song <[email protected]>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cdev.h>
+#include <linux/mutex.h>
+#include <linux/poll.h>
+#include <linux/skbuff.h>
+#include <linux/spinlock.h>
+
+#include "t7xx_state_monitor.h"
+#include "t7xx_port.h"
+#include "t7xx_port_proxy.h"
+
+static __poll_t port_char_poll(struct file *fp, struct poll_table_struct *poll)
+{
+ struct t7xx_port *port;
+ __poll_t mask = 0;
+
+ port = fp->private_data;
+ poll_wait(fp, &port->rx_wq, poll);
+
+ spin_lock_irq(&port->rx_wq.lock);
+ if (!skb_queue_empty(&port->rx_skb_list))
+ mask |= EPOLLIN | EPOLLRDNORM;
+ spin_unlock_irq(&port->rx_wq.lock);
+
+ return mask;
+}
+
+/**
+ * port_char_open() - open char port
+ * @inode: pointer to inode structure
+ * @file: pointer to file structure
+ *
+ * Open a char port using pre-defined md_ccci_ports structure in port_proxy
+ *
+ * Return: 0 for success, -EINVAL for failure
+ */
+static int port_char_open(struct inode *inode, struct file *file)
+{
+ struct t7xx_cdev *t7xx_debug;
+ struct t7xx_port *port;
+
+ t7xx_debug = container_of(inode->i_cdev, struct t7xx_cdev, cdev);
+ port = t7xx_debug->port;
+
+ if (!port)
+ return -EINVAL;
+
+ port->port_conf->ops->enable_chl(port);
+ atomic_inc(&port->usage_cnt);
+
+ file->private_data = port;
+
+ return nonseekable_open(inode, file);
+}
+
+static int port_char_close(struct inode *inode, struct file *file)
+{
+ struct t7xx_port *port;
+ struct sk_buff *skb;
+
+ port = file->private_data;
+
+ /* decrease usage count, so when we ask again,
+ * the packet can be dropped in recv_request.
+ */
+ atomic_dec(&port->usage_cnt);
+ port->port_conf->ops->disable_chl(port);
+
+ /* purge RX request list */
+ spin_lock_irq(&port->rx_wq.lock);
+ while ((skb = __skb_dequeue(&port->rx_skb_list)) != NULL)
+ dev_kfree_skb(skb);
+ spin_unlock_irq(&port->rx_wq.lock);
+
+ return 0;
+}
+
+static ssize_t port_char_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+ bool full_req_done = false;
+ struct t7xx_port *port;
+ int ret = 0, read_len;
+ struct sk_buff *skb;
+
+ port = file->private_data;
+
+ spin_lock_irq(&port->rx_wq.lock);
+ if (skb_queue_empty(&port->rx_skb_list)) {
+ if (file->f_flags & O_NONBLOCK) {
+ spin_unlock_irq(&port->rx_wq.lock);
+ return -EAGAIN;
+ }
+
+ ret = wait_event_interruptible_locked_irq(port->rx_wq,
+ !skb_queue_empty(&port->rx_skb_list));
+ if (ret == -ERESTARTSYS) {
+ spin_unlock_irq(&port->rx_wq.lock);
+ return -EINTR;
+ }
+ }
+ skb = skb_peek(&port->rx_skb_list);
+
+ if (count >= skb->len) {
+ read_len = skb->len;
+ full_req_done = true;
+ __skb_unlink(skb, &port->rx_skb_list);
+ } else {
+ read_len = count;
+ }
+
+ spin_unlock_irq(&port->rx_wq.lock);
+ if (copy_to_user(buf, skb->data, read_len)) {
+ dev_err(port->dev, "Read on %s, copy to user failed, %d/%zu\n",
+ port->port_conf->name, read_len, count);
+ ret = -EFAULT;
+ }
+
+ skb_pull(skb, read_len);
+ if (full_req_done)
+ dev_kfree_skb(skb);
+
+ return ret ? ret : read_len;
+}
+
+static ssize_t port_char_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ unsigned int header_len = sizeof(struct ccci_header);
+ size_t offset, txq_mtu, chunk_len = 0;
+ struct t7xx_port *port;
+ struct sk_buff *skb;
+ bool blocking;
+ int ret;
+
+ port = file->private_data;
+
+ blocking = !(file->f_flags & O_NONBLOCK);
+ if (!blocking)
+ return -EAGAIN;
+
+ if (!port->chan_enable)
+ return -EINVAL;
+
+ txq_mtu = t7xx_get_port_mtu(port);
+ if (txq_mtu < 0)
+ return -EINVAL;
+
+ for (offset = 0; offset < count; offset += chunk_len) {
+ chunk_len = min(count - offset, txq_mtu - header_len);
+
+ skb = __dev_alloc_skb(chunk_len + header_len, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ ret = copy_from_user(skb_put(skb, chunk_len), buf + offset, chunk_len);
+
+ if (ret) {
+ dev_kfree_skb(skb);
+ return -EFAULT;
+ }
+
+ ret = t7xx_port_send_skb(port, skb, 0, 0);
+ if (ret) {
+ if (ret == -EBUSY && !blocking)
+ ret = -EAGAIN;
+ dev_kfree_skb_any(skb);
+ return ret;
+ }
+ }
+
+ return count;
+}
+
+static int t7xx_cdev_init(struct t7xx_port *port)
+{
+ struct t7xx_cdev *t7xx_debug;
+ struct device *dev;
+
+ dev = &port->t7xx_dev->pdev->dev;
+
+ t7xx_debug = devm_kzalloc(dev, sizeof(*t7xx_debug), GFP_KERNEL);
+ if (!t7xx_debug)
+ return -ENOMEM;
+
+ t7xx_debug->port = port;
+ port->debug.debug_port = t7xx_debug;
+
+ return 0;
+}
+
+static void t7xx_cdev_uninit(struct t7xx_port *port)
+{
+ struct device *dev;
+
+ if (!port->debug.debug_port)
+ return;
+
+ dev = &port->t7xx_dev->pdev->dev;
+
+ devm_kfree(dev, port->debug.debug_port);
+ port->debug.debug_port = NULL;
+}
+
+static const struct file_operations char_fops = {
+ .owner = THIS_MODULE,
+ .open = &port_char_open,
+ .read = &port_char_read,
+ .write = &port_char_write,
+ .release = &port_char_close,
+ .poll = &port_char_poll,
+};
+
+static int port_char_init(struct t7xx_port *port)
+{
+ const struct t7xx_port_conf *port_conf = port->port_conf;
+ struct t7xx_cdev *t7xx_debug;
+ struct device *dev;
+ int ret;
+
+ if (port->debug.debug_port)
+ return 0;
+
+ t7xx_cdev_init(port);
+
+ t7xx_debug = port->debug.debug_port;
+
+ port->rx_length_th = RX_QUEUE_MAXLEN;
+
+ ret = alloc_chrdev_region(&t7xx_debug->dev_num, port_conf->baseminor, 1, "t7xx_cdev");
+ if (ret) {
+ dev_err(port->dev, "Alloc chrdev region failed, ret=%d\n", ret);
+ return ret;
+ }
+
+ cdev_init(&t7xx_debug->cdev, &char_fops);
+ t7xx_debug->cdev.owner = THIS_MODULE;
+
+ ret = cdev_add(&t7xx_debug->cdev, t7xx_debug->dev_num, 1);
+ if (ret) {
+ dev_err(port->dev, "Add cdev failed, ret=%d\n", ret);
+ goto err_cdev_add;
+ }
+
+ t7xx_debug->dev_class = class_create(port_conf->class_name);
+ if (IS_ERR(t7xx_debug->dev_class)) {
+ ret = PTR_ERR(t7xx_debug->dev_class);
+ dev_err(port->dev, "Create class failed, ret=%d\n", ret);
+ goto err_class_create;
+ }
+
+ dev = device_create(t7xx_debug->dev_class, NULL, t7xx_debug->dev_num,
+ NULL, port->port_conf->name);
+ if (IS_ERR(dev)) {
+ ret = PTR_ERR(dev);
+ dev_err(port->dev, "Create device failed, ret=%d\n", ret);
+ goto err_device_create;
+ }
+
+ port->debug.debug_port->cdev = t7xx_debug->cdev;
+ t7xx_debug->port = port;
+
+ return 0;
+
+err_device_create:
+ class_destroy(t7xx_debug->dev_class);
+err_class_create:
+ cdev_del(&t7xx_debug->cdev);
+err_cdev_add:
+ unregister_chrdev_region(t7xx_debug->dev_num, 1);
+ return ret;
+}
+
+static void port_char_uninit(struct t7xx_port *port)
+{
+ struct t7xx_cdev *t7xx_debug;
+ unsigned long flags;
+ struct sk_buff *skb;
+
+ if (!port->debug.debug_port)
+ return;
+
+ t7xx_debug = port->debug.debug_port;
+
+ device_destroy(t7xx_debug->dev_class, t7xx_debug->dev_num);
+ class_destroy(t7xx_debug->dev_class);
+ cdev_del(&t7xx_debug->cdev);
+ unregister_chrdev_region(t7xx_debug->dev_num, 1);
+
+ t7xx_cdev_uninit(port);
+
+ spin_lock_irqsave(&port->rx_wq.lock, flags);
+ while ((skb = __skb_dequeue(&port->rx_skb_list)) != NULL)
+ dev_kfree_skb(skb);
+ spin_unlock_irqrestore(&port->rx_wq.lock, flags);
+}
+
+static int port_char_recv_skb(struct t7xx_port *port, struct sk_buff *skb)
+{
+ const struct t7xx_port_conf *port_conf = port->port_conf;
+ unsigned long flags;
+
+ if (!atomic_read(&port->usage_cnt) || !port->chan_enable) {
+ dev_dbg_ratelimited(port->dev, "Port %s is not opened, drop packets\n",
+ port_conf->name);
+ return -ENETDOWN;
+ }
+
+ spin_lock_irqsave(&port->rx_wq.lock, flags);
+ if (port->rx_skb_list.qlen >= port->rx_length_th) {
+ spin_unlock_irqrestore(&port->rx_wq.lock, flags);
+ return -ENOBUFS;
+ }
+
+ __skb_queue_tail(&port->rx_skb_list, skb);
+ spin_unlock_irqrestore(&port->rx_wq.lock, flags);
+
+ wake_up_all(&port->rx_wq);
+
+ return 0;
+}
+
+static int port_char_enable_chl(struct t7xx_port *port)
+{
+ spin_lock(&port->port_update_lock);
+ port->chan_enable = true;
+ spin_unlock(&port->port_update_lock);
+
+ return 0;
+}
+
+static int port_char_disable_chl(struct t7xx_port *port)
+{
+ spin_lock(&port->port_update_lock);
+ port->chan_enable = false;
+ spin_unlock(&port->port_update_lock);
+
+ return 0;
+}
+
+struct port_ops debug_port_ops = {
+ .init = &port_char_init,
+ .recv_skb = &port_char_recv_skb,
+ .uninit = &port_char_uninit,
+ .enable_chl = &port_char_enable_chl,
+ .disable_chl = &port_char_disable_chl,
+};
diff --git a/drivers/net/wwan/t7xx/t7xx_port_proxy.c b/drivers/net/wwan/t7xx/t7xx_port_proxy.c
index 7d6388bf1d7c..3dd87f25124e 100644
--- a/drivers/net/wwan/t7xx/t7xx_port_proxy.c
+++ b/drivers/net/wwan/t7xx/t7xx_port_proxy.c
@@ -39,6 +39,8 @@

#define Q_IDX_CTRL 0
#define Q_IDX_MBIM 2
+#define Q_IDX_MIPC 2
+#define Q_IDX_ADB 3
#define Q_IDX_AT_CMD 5

#define INVALID_SEQ_NUM GENMASK(15, 0)
@@ -100,6 +102,28 @@ static const struct t7xx_port_conf t7xx_port_conf[] = {
.path_id = CLDMA_ID_AP,
.ops = &ctl_port_ops,
.name = "t7xx_ap_ctrl",
+ }, {
+ .tx_ch = PORT_CH_AP_ADB_TX,
+ .rx_ch = PORT_CH_AP_ADB_RX,
+ .txq_index = Q_IDX_ADB,
+ .rxq_index = Q_IDX_ADB,
+ .path_id = CLDMA_ID_AP,
+ .ops = &debug_port_ops,
+ .name = "ccci_sap_adb",
+ .debug = true,
+ .class_name = "t7xx_adb",
+ .baseminor = 100,
+ }, {
+ .tx_ch = PORT_CH_MIPC_TX,
+ .rx_ch = PORT_CH_MIPC_RX,
+ .txq_index = Q_IDX_MIPC,
+ .rxq_index = Q_IDX_MIPC,
+ .path_id = CLDMA_ID_MD,
+ .ops = &debug_port_ops,
+ .name = "ttyCMIPC0",
+ .debug = true,
+ .class_name = "t7xx_mipc",
+ .baseminor = 101,
},
};

@@ -505,13 +529,32 @@ static void t7xx_proxy_init_all_ports(struct t7xx_modem *md)
spin_lock_init(&port->port_update_lock);
port->chan_enable = false;

- if (port_conf->ops && port_conf->ops->init)
+ if (!port_conf->debug && port_conf->ops && port_conf->ops->init)
port_conf->ops->init(port);
}

t7xx_proxy_setup_ch_mapping(port_prox);
}

+void t7xx_proxy_port_debug(struct t7xx_pci_dev *t7xx_dev, bool show)
+{
+ struct port_proxy *port_prox = t7xx_dev->md->port_prox;
+ struct t7xx_port *port;
+ int i;
+
+ for_each_proxy_port(i, port, port_prox) {
+ const struct t7xx_port_conf *port_conf = port->port_conf;
+
+ spin_lock_init(&port->port_update_lock);
+ if (port_conf->debug && port_conf->ops && port_conf->ops->init) {
+ if (show)
+ port_conf->ops->init(port);
+ else
+ port_conf->ops->uninit(port);
+ }
+ }
+}
+
void t7xx_port_proxy_set_cfg(struct t7xx_modem *md, enum port_cfg_id cfg_id)
{
struct port_proxy *port_prox = md->port_prox;
diff --git a/drivers/net/wwan/t7xx/t7xx_port_proxy.h b/drivers/net/wwan/t7xx/t7xx_port_proxy.h
index 7f5706811445..5bf958824aa8 100644
--- a/drivers/net/wwan/t7xx/t7xx_port_proxy.h
+++ b/drivers/net/wwan/t7xx/t7xx_port_proxy.h
@@ -93,11 +93,13 @@ struct ctrl_msg_header {
/* Port operations mapping */
extern struct port_ops wwan_sub_port_ops;
extern struct port_ops ctl_port_ops;
+extern struct port_ops debug_port_ops;

#ifdef CONFIG_WWAN_DEBUGFS
extern struct port_ops t7xx_trace_port_ops;
#endif

+void t7xx_proxy_port_debug(struct t7xx_pci_dev *t7xx_dev, bool show);
void t7xx_port_proxy_reset(struct port_proxy *port_prox);
void t7xx_port_proxy_uninit(struct port_proxy *port_prox);
int t7xx_port_proxy_init(struct t7xx_modem *md);
--
2.34.1



2024-06-14 15:25:14

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [net-next v1] net: wwan: t7xx: Add debug port

On Fri, 14 Jun 2024 17:49:51 +0800 Vanillan Wang wrote:
> From: Jinjian Song <[email protected]>
>
> Add support for userspace to switch on the debug port(ADB,MIPC).
> - ADB port: /dev/ccci_sap_adb
> - MIPC port: /dev/ttyMIPC0
>
> Switch on debug port:
> - debug: 'echo debug > /sys/bus/pci/devices/${bdf}/t7xx_mode
>
> Switch off debug port:
> - normal: 'echo normal > /sys/bus/pci/devices/${bdf}/t7xx_mode

You need to provide more detail on what it does and how it's used.

> + txq_mtu = t7xx_get_port_mtu(port);
> + if (txq_mtu < 0)
> + return -EINVAL;

drivers/net/wwan/t7xx/t7xx_port_debug.c:153:5-12: WARNING: Unsigned expression compared with zero: txq_mtu < 0
--
pw-bot: cr

2024-06-15 01:46:39

by Sergey Ryazanov

[permalink] [raw]
Subject: Re: [net-next v1] net: wwan: t7xx: Add debug port

On 14.06.2024 12:49, Vanillan Wang wrote:
> From: Jinjian Song <[email protected]>
>
> Add support for userspace to switch on the debug port(ADB,MIPC).
> - ADB port: /dev/ccci_sap_adb
> - MIPC port: /dev/ttyMIPC0

NAK

The WWAN subsystem was purposely introducted to get rid of this mess of
char devices and their implementation in drivers. If you want to export
a port, which type is not available in the WWAN subsystem, then
introduce a new port to the subsystem and then register it from a driver.

--
Sergey