The MHI WWWAN control driver allows MHI Qcom based modems to expose
different modem control protocols/ports to userspace, so that userspace
modem tools or daemon (e.g. ModemManager) can control WWAN config
and state (APN config, SMS, provider selection...). A Qcom based
modem can expose one or several of the following protocols:
- AT: Well known AT commands interactive protocol (microcom, minicom...)
- MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
- QMI: Qcom MSM/Modem Interface (libqmi, qmicli)
- QCDM: Qcom Modem diagnostic interface (libqcdm)
- FIREHOSE: XML-based protocol for Modem firmware management
(qmi-firmware-update)
The different interfaces are exposed as character devices, in the same
way as for USB modem variants (known as modem 'ports').
Note that this patch is mostly a rework of the earlier MHI UCI
tentative that was a generic interface for accessing MHI bus from
userspace. As suggested, this new version is WWAN specific and is
dedicated to only expose channels used for controlling a modem, and
for which related opensource user support exist. Other MHI channels
not fitting the requirements will request either to be plugged to
the right Linux subsystem (when available) or to be discussed as a
new MHI driver (e.g AI accelerator, WiFi debug channels, etc...).
This change introduces a new drivers/net/wwan directory, aiming to
be the common place for WWAN drivers.
Co-developed-by: Hemant Kumar <[email protected]>
Signed-off-by: Hemant Kumar <[email protected]>
Signed-off-by: Loic Poulain <[email protected]>
---
v2: update copyright (2021)
v3: Move driver to dedicated drivers/net/wwan directory
drivers/net/Kconfig | 2 +
drivers/net/Makefile | 1 +
drivers/net/wwan/Kconfig | 26 ++
drivers/net/wwan/Makefile | 6 +
drivers/net/wwan/mhi_wwan_ctrl.c | 559 +++++++++++++++++++++++++++++++++++++++
5 files changed, 594 insertions(+)
create mode 100644 drivers/net/wwan/Kconfig
create mode 100644 drivers/net/wwan/Makefile
create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 1ebb4b9..28b18f2 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -501,6 +501,8 @@ source "drivers/net/wan/Kconfig"
source "drivers/net/ieee802154/Kconfig"
+source "drivers/net/wwan/Kconfig"
+
config XEN_NETDEV_FRONTEND
tristate "Xen network device frontend driver"
depends on XEN
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index f4990ff..5da6424 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_SUNGEM_PHY) += sungem_phy.o
obj-$(CONFIG_WAN) += wan/
obj-$(CONFIG_WLAN) += wireless/
obj-$(CONFIG_IEEE802154) += ieee802154/
+obj-$(CONFIG_WWAN) += wwan/
obj-$(CONFIG_VMXNET3) += vmxnet3/
obj-$(CONFIG_XEN_NETDEV_FRONTEND) += xen-netfront.o
diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
new file mode 100644
index 0000000..643aa10
--- /dev/null
+++ b/drivers/net/wwan/Kconfig
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Wireless WAN device configuration
+#
+
+menuconfig WWAN
+ bool "Wireless WAN"
+ help
+ This section contains Wireless WAN driver configurations.
+
+if WWAN
+
+config MHI_WWAN_CTRL
+ tristate "MHI WWAN control driver for QCOM based PCIe modems"
+ depends on MHI_BUS
+ help
+ MHI WWAN CTRL allow QCOM based PCIe modems to expose different modem
+ control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
+ and FIREHOSE. These protocols can be accessed directly from userspace
+ (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
+ libqcdm...).
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_wwan_ctrl.
+
+endif # WWAN
diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
new file mode 100644
index 0000000..994a80b
--- /dev/null
+++ b/drivers/net/wwan/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the Linux WWAN device drivers.
+#
+
+obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
new file mode 100644
index 0000000..3904cd0
--- /dev/null
+++ b/drivers/net/wwan/mhi_wwan_ctrl.c
@@ -0,0 +1,559 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2021, The Linux Foundation. All rights reserved.*/
+
+#include <linux/kernel.h>
+#include <linux/mhi.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/poll.h>
+
+#define MHI_WWAN_CTRL_DRIVER_NAME "mhi_wwan_ctrl"
+#define MHI_WWAN_CTRL_MAX_MINORS 128
+
+static DEFINE_IDR(mhi_wwan_ctrl_idr);
+static DEFINE_MUTEX(mhi_wwan_ctrl_drv_lock);
+static struct class *mhi_wwan_ctrl_class;
+static int mhi_wwan_ctrl_major;
+
+/* WWAN interface Protocols used by user-space for controlling a modem */
+#define MHI_WWAN_CTRL_PROTO_AT "AT" /* AT command protocol */
+#define MHI_WWAN_CTRL_PROTO_MBIM "MBIM" /* Mobile Broadband Interface Model control protocol */
+#define MHI_WWAN_CTRL_PROTO_QMUX "QMI" /* Qcom modem/MSM interface for modem control */
+#define MHI_WWAN_CTRL_PROTO_QCDM "DIAG" /* Qcom Modem diagnostic interface */
+#define MHI_WWAN_CTRL_PROTO_FIREHOSE "FIREHOSE" /* XML based command protocol */
+
+/* MHI wwan device flags */
+#define MHI_WWAN_DL_CAP BIT(0)
+#define MHI_WWAN_UL_CAP BIT(1)
+#define MHI_WWAN_CONNECTED BIT(2)
+
+struct mhi_wwan_buf {
+ struct list_head node;
+ void *data;
+ size_t len;
+ size_t consumed;
+};
+
+struct mhi_wwan_dev {
+ unsigned int minor;
+ size_t mtu;
+
+ struct mhi_device *mhi_dev;
+ /* Protect against concurrent MHI device accesses (start, stop, open, close) */
+ struct mutex mhi_dev_lock;
+ unsigned int mhi_dev_open_count;
+
+ wait_queue_head_t ul_wq;
+ wait_queue_head_t dl_wq;
+
+ /* Protect download buf queue against concurent update (read/xfer_cb) */
+ spinlock_t dl_queue_lock;
+ struct list_head dl_queue;
+
+ /* For serializing write/queueing to a same MHI channel */
+ struct mutex write_lock;
+
+ unsigned long flags;
+
+ /* kref is used to safely track and release a mhi_wwan_dev instance,
+ * shared between mhi device probe/remove and user open/close.
+ */
+ struct kref ref_count;
+};
+
+static void mhi_wwan_ctrl_dev_release(struct kref *ref)
+{
+ struct mhi_wwan_dev *wwandev = container_of(ref, struct mhi_wwan_dev, ref_count);
+ struct mhi_wwan_buf *buf_itr, *tmp;
+
+ /* Release non consumed buffers */
+ list_for_each_entry_safe(buf_itr, tmp, &wwandev->dl_queue, node) {
+ list_del(&buf_itr->node);
+ kfree(buf_itr->data);
+ }
+
+ mutex_destroy(&wwandev->mhi_dev_lock);
+ mutex_destroy(&wwandev->write_lock);
+
+ kfree(wwandev);
+}
+
+static int mhi_wwan_ctrl_queue_inbound(struct mhi_wwan_dev *wwandev)
+{
+ struct mhi_device *mhi_dev = wwandev->mhi_dev;
+ struct device *dev = &mhi_dev->dev;
+ int nr_desc, i, ret = -EIO;
+ struct mhi_wwan_buf *ubuf;
+ void *buf;
+
+ /*
+ * skip queuing without error if dl channel is not supported. This
+ * allows open to succeed for udev, supporting ul only channel.
+ */
+ if (!wwandev->mhi_dev->dl_chan)
+ return 0;
+
+ nr_desc = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
+
+ for (i = 0; i < nr_desc; i++) {
+ buf = kmalloc(wwandev->mtu + sizeof(*ubuf), GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* save mhi_wwan_buf info at the end of buf */
+ ubuf = buf + wwandev->mtu;
+ ubuf->data = buf;
+
+ ret = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, buf, wwandev->mtu, MHI_EOT);
+ if (ret) {
+ kfree(buf);
+ dev_err(dev, "Failed to queue buffer %d\n", i);
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+static int mhi_wwan_ctrl_open(struct inode *inode, struct file *filp)
+{
+ unsigned int minor = iminor(inode);
+ struct mhi_wwan_dev *wwandev = NULL;
+ int ret = 0;
+
+ /* Retrieve corresponding mhi_wwan_dev and get a reference */
+ mutex_lock(&mhi_wwan_ctrl_drv_lock);
+ wwandev = idr_find(&mhi_wwan_ctrl_idr, minor);
+ if (!wwandev) {
+ mutex_unlock(&mhi_wwan_ctrl_drv_lock);
+ return -ENODEV;
+ }
+ kref_get(&wwandev->ref_count);
+ mutex_unlock(&mhi_wwan_ctrl_drv_lock);
+
+ /* Start MHI channel(s) if not yet started and fill RX queue */
+ mutex_lock(&wwandev->mhi_dev_lock);
+ if (!wwandev->mhi_dev_open_count++) {
+ ret = mhi_prepare_for_transfer(wwandev->mhi_dev);
+ if (!ret)
+ ret = mhi_wwan_ctrl_queue_inbound(wwandev);
+ if (ret)
+ wwandev->mhi_dev_open_count--;
+ }
+ mutex_unlock(&wwandev->mhi_dev_lock);
+
+ if (ret)
+ return ret;
+
+ filp->private_data = wwandev;
+
+ /* stream-like non-seekable file descriptor */
+ stream_open(inode, filp);
+
+ return 0;
+}
+
+static int mhi_wwan_ctrl_release(struct inode *inode, struct file *file)
+{
+ struct mhi_wwan_dev *wwandev = file->private_data;
+
+ /* Stop the channels, if not already destroyed */
+ mutex_lock(&wwandev->mhi_dev_lock);
+ if (!(--wwandev->mhi_dev_open_count) && wwandev->mhi_dev)
+ mhi_unprepare_from_transfer(wwandev->mhi_dev);
+ mutex_unlock(&wwandev->mhi_dev_lock);
+
+ file->private_data = NULL;
+
+ kref_put(&wwandev->ref_count, mhi_wwan_ctrl_dev_release);
+
+ return 0;
+}
+
+static __poll_t mhi_wwan_ctrl_poll(struct file *file, poll_table *wait)
+{
+ struct mhi_wwan_dev *wwandev = file->private_data;
+ __poll_t mask = 0;
+
+ poll_wait(file, &wwandev->ul_wq, wait);
+ poll_wait(file, &wwandev->dl_wq, wait);
+
+ /* Any buffer in the DL queue ? */
+ spin_lock_bh(&wwandev->dl_queue_lock);
+ if (!list_empty(&wwandev->dl_queue))
+ mask |= EPOLLIN | EPOLLRDNORM;
+ spin_unlock_bh(&wwandev->dl_queue_lock);
+
+ /* If MHI queue is not full, write is possible */
+ mutex_lock(&wwandev->mhi_dev_lock);
+ if (!wwandev->mhi_dev)
+ mask = EPOLLERR | EPOLLHUP;
+ else if (!mhi_queue_is_full(wwandev->mhi_dev, DMA_TO_DEVICE))
+ mask |= EPOLLOUT | EPOLLWRNORM;
+ mutex_unlock(&wwandev->mhi_dev_lock);
+
+ return mask;
+}
+
+static ssize_t mhi_wwan_ctrl_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *offp)
+{
+ struct mhi_wwan_dev *wwandev = file->private_data;
+ size_t bytes_xfered = 0;
+ void *kbuf = NULL;
+ int ret;
+
+ if (!test_bit(MHI_WWAN_UL_CAP, &wwandev->flags))
+ return -EOPNOTSUPP;
+
+ if (!buf || !count)
+ return -EINVAL;
+
+ /* Serialize MHI queueing */
+ if (mutex_lock_interruptible(&wwandev->write_lock))
+ return -EINTR;
+
+ while (count) {
+ size_t xfer_size;
+
+ /* Wait for available transfer descriptor */
+ ret = wait_event_interruptible(wwandev->ul_wq,
+ !test_bit(MHI_WWAN_CONNECTED, &wwandev->flags) ||
+ !mhi_queue_is_full(wwandev->mhi_dev, DMA_TO_DEVICE));
+ if (ret)
+ break;
+
+ if (!test_bit(MHI_WWAN_CONNECTED, &wwandev->flags)) {
+ ret = -EPIPE;
+ break;
+ }
+
+ xfer_size = min_t(size_t, count, wwandev->mtu);
+ kbuf = kmalloc(xfer_size, GFP_KERNEL);
+ if (!kbuf) {
+ ret = -ENOMEM;
+ break;
+ }
+
+ ret = copy_from_user(kbuf, buf, xfer_size);
+ if (ret)
+ break;
+
+ /* Add buffer to MHI queue */
+ ret = mhi_queue_buf(wwandev->mhi_dev, DMA_TO_DEVICE, kbuf,
+ xfer_size, MHI_EOT);
+ if (ret)
+ break;
+
+ bytes_xfered += xfer_size;
+ count -= xfer_size;
+ buf += xfer_size;
+ kbuf = NULL;
+ }
+
+ mutex_unlock(&wwandev->write_lock);
+
+ kfree(kbuf);
+
+ return ret ? ret : bytes_xfered;
+}
+
+static int mhi_wwan_ctrl_recycle_ubuf(struct mhi_wwan_dev *wwandev,
+ struct mhi_wwan_buf *ubuf)
+{
+ int ret;
+
+ mutex_lock(&wwandev->mhi_dev_lock);
+
+ if (!wwandev->mhi_dev) {
+ ret = -ENODEV;
+ goto exit_unlock;
+ }
+
+ ret = mhi_queue_buf(wwandev->mhi_dev, DMA_FROM_DEVICE, ubuf->data,
+ wwandev->mtu, MHI_EOT);
+
+exit_unlock:
+ mutex_unlock(&wwandev->mhi_dev_lock);
+
+ return ret;
+}
+
+static ssize_t mhi_wwan_ctrl_read(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct mhi_wwan_dev *wwandev = file->private_data;
+ bool recycle_buf = false;
+ struct mhi_wwan_buf *ubuf;
+ size_t copy_len;
+ char *copy_ptr;
+ int ret = 0;
+
+ if (!test_bit(MHI_WWAN_DL_CAP, &wwandev->flags))
+ return -EOPNOTSUPP;
+
+ if (!buf)
+ return -EINVAL;
+
+ spin_lock_irq(&wwandev->dl_queue_lock);
+
+ /* Wait for at least one buffer in the dl queue (or device removal) */
+ ret = wait_event_interruptible_lock_irq(wwandev->dl_wq,
+ !list_empty(&wwandev->dl_queue) ||
+ !test_bit(MHI_WWAN_CONNECTED, &wwandev->flags),
+ wwandev->dl_queue_lock);
+ if (ret) {
+ goto err_unlock;
+ } else if (!test_bit(MHI_WWAN_CONNECTED, &wwandev->flags)) {
+ ret = -EPIPE;
+ goto err_unlock;
+ }
+
+ ubuf = list_first_entry_or_null(&wwandev->dl_queue, struct mhi_wwan_buf, node);
+ if (!ubuf) {
+ ret = -EIO;
+ goto err_unlock;
+ }
+
+ /* Consume the buffer */
+ copy_len = min_t(size_t, count, ubuf->len - ubuf->consumed);
+ copy_ptr = ubuf->data + ubuf->consumed;
+ ubuf->consumed += copy_len;
+
+ /* Remove buffer from the DL queue if entirely consumed */
+ if (ubuf->consumed == ubuf->len) {
+ list_del(&ubuf->node);
+ recycle_buf = true;
+ }
+
+ spin_unlock_irq(&wwandev->dl_queue_lock);
+
+ ret = copy_to_user(buf, copy_ptr, copy_len);
+ if (ret)
+ return -EFAULT;
+
+ if (recycle_buf) {
+ /* Give the buffer back to MHI queue */
+ ret = mhi_wwan_ctrl_recycle_ubuf(wwandev, ubuf);
+ if (ret) /* unable to recycle, release */
+ kfree(ubuf->data);
+ }
+
+ return copy_len;
+
+err_unlock:
+ spin_unlock_irq(&wwandev->dl_queue_lock);
+
+ return ret;
+}
+
+static const struct file_operations mhidev_fops = {
+ .owner = THIS_MODULE,
+ .open = mhi_wwan_ctrl_open,
+ .release = mhi_wwan_ctrl_release,
+ .read = mhi_wwan_ctrl_read,
+ .write = mhi_wwan_ctrl_write,
+ .poll = mhi_wwan_ctrl_poll,
+};
+
+static void mhi_ul_xfer_cb(struct mhi_device *mhi_dev,
+ struct mhi_result *mhi_result)
+{
+ struct mhi_wwan_dev *wwandev = dev_get_drvdata(&mhi_dev->dev);
+ struct device *dev = &mhi_dev->dev;
+
+ dev_dbg(dev, "%s: status: %d xfer_len: %zu\n", __func__,
+ mhi_result->transaction_status, mhi_result->bytes_xferd);
+
+ kfree(mhi_result->buf_addr);
+
+ /* Some space available in the 'upload' queue, advertise that */
+ if (!mhi_result->transaction_status)
+ wake_up_interruptible(&wwandev->ul_wq);
+}
+
+static void mhi_dl_xfer_cb(struct mhi_device *mhi_dev,
+ struct mhi_result *mhi_result)
+{
+ struct mhi_wwan_dev *wwandev = dev_get_drvdata(&mhi_dev->dev);
+ struct mhi_wwan_buf *ubuf;
+
+ dev_dbg(&mhi_dev->dev, "%s: status: %d receive_len: %zu\n", __func__,
+ mhi_result->transaction_status, mhi_result->bytes_xferd);
+
+ if (mhi_result->transaction_status &&
+ mhi_result->transaction_status != -EOVERFLOW) {
+ kfree(mhi_result->buf_addr);
+ return;
+ }
+
+ /* ubuf is placed at the end of the buffer (cf mhi_wwan_ctrl_queue_inbound) */
+ ubuf = mhi_result->buf_addr + wwandev->mtu;
+
+ /* paranoia, should never happen */
+ if (WARN_ON(mhi_result->buf_addr != ubuf->data)) {
+ kfree(mhi_result->buf_addr);
+ return;
+ }
+
+ ubuf->data = mhi_result->buf_addr;
+ ubuf->len = mhi_result->bytes_xferd;
+ ubuf->consumed = 0;
+
+ spin_lock_bh(&wwandev->dl_queue_lock);
+ list_add_tail(&ubuf->node, &wwandev->dl_queue);
+ spin_unlock_bh(&wwandev->dl_queue_lock);
+
+ wake_up_interruptible(&wwandev->dl_wq);
+}
+
+static int mhi_wwan_ctrl_probe(struct mhi_device *mhi_dev,
+ const struct mhi_device_id *id)
+{
+ struct mhi_wwan_dev *wwandev;
+ struct device *dev;
+ int index, err;
+
+ /* Create mhi_wwan data context */
+ wwandev = kzalloc(sizeof(*wwandev), GFP_KERNEL);
+ if (!wwandev)
+ return -ENOMEM;
+
+ /* Retrieve index */
+ mutex_lock(&mhi_wwan_ctrl_drv_lock);
+ index = idr_alloc(&mhi_wwan_ctrl_idr, wwandev, 0,
+ MHI_WWAN_CTRL_MAX_MINORS, GFP_KERNEL);
+ mutex_unlock(&mhi_wwan_ctrl_drv_lock);
+ if (index < 0) {
+ err = index;
+ goto err_free_wwandev;
+ }
+
+ /* Init mhi_wwan data */
+ kref_init(&wwandev->ref_count);
+ mutex_init(&wwandev->mhi_dev_lock);
+ mutex_init(&wwandev->write_lock);
+ init_waitqueue_head(&wwandev->ul_wq);
+ init_waitqueue_head(&wwandev->dl_wq);
+ spin_lock_init(&wwandev->dl_queue_lock);
+ INIT_LIST_HEAD(&wwandev->dl_queue);
+ wwandev->mhi_dev = mhi_dev;
+ wwandev->minor = index;
+ wwandev->mtu = min_t(size_t, id->driver_data, MHI_MAX_MTU);
+ set_bit(MHI_WWAN_CONNECTED, &wwandev->flags);
+
+ if (mhi_dev->dl_chan)
+ set_bit(MHI_WWAN_DL_CAP, &wwandev->flags);
+ if (mhi_dev->ul_chan)
+ set_bit(MHI_WWAN_UL_CAP, &wwandev->flags);
+
+ dev_set_drvdata(&mhi_dev->dev, wwandev);
+
+ /* Creates a new device and registers it with sysfs */
+ dev = device_create(mhi_wwan_ctrl_class, &mhi_dev->dev,
+ MKDEV(mhi_wwan_ctrl_major, index), wwandev,
+ "wwan_%s", dev_name(&mhi_dev->dev));
+ if (IS_ERR(dev)) {
+ err = PTR_ERR(dev);
+ goto err_free_idr;
+ }
+
+ return 0;
+
+err_free_idr:
+ mutex_lock(&mhi_wwan_ctrl_drv_lock);
+ idr_remove(&mhi_wwan_ctrl_idr, wwandev->minor);
+ mutex_unlock(&mhi_wwan_ctrl_drv_lock);
+err_free_wwandev:
+ kfree(wwandev);
+ dev_set_drvdata(&mhi_dev->dev, NULL);
+
+ return err;
+};
+
+static void mhi_wwan_ctrl_remove(struct mhi_device *mhi_dev)
+{
+ struct mhi_wwan_dev *wwandev = dev_get_drvdata(&mhi_dev->dev);
+
+ dev_set_drvdata(&mhi_dev->dev, NULL);
+
+ mutex_lock(&mhi_wwan_ctrl_drv_lock);
+ idr_remove(&mhi_wwan_ctrl_idr, wwandev->minor);
+ mutex_unlock(&mhi_wwan_ctrl_drv_lock);
+
+ clear_bit(MHI_WWAN_CONNECTED, &wwandev->flags);
+ device_destroy(mhi_wwan_ctrl_class, MKDEV(mhi_wwan_ctrl_major, wwandev->minor));
+
+ /* Unlink mhi_dev from mhi_wwan_dev */
+ mutex_lock(&wwandev->mhi_dev_lock);
+ wwandev->mhi_dev = NULL;
+ mutex_unlock(&wwandev->mhi_dev_lock);
+
+ /* wake up any blocked user */
+ wake_up_interruptible(&wwandev->dl_wq);
+ wake_up_interruptible(&wwandev->ul_wq);
+
+ kref_put(&wwandev->ref_count, mhi_wwan_ctrl_dev_release);
+}
+
+/* .driver_data stores max mtu */
+static const struct mhi_device_id mhi_wwan_ctrl_match_table[] = {
+ { .chan = MHI_WWAN_CTRL_PROTO_AT, .driver_data = 4096 },
+ { .chan = MHI_WWAN_CTRL_PROTO_MBIM, .driver_data = 4096 },
+ { .chan = MHI_WWAN_CTRL_PROTO_QMUX, .driver_data = 4096 },
+ { .chan = MHI_WWAN_CTRL_PROTO_QCDM, .driver_data = MHI_MAX_MTU },
+ { .chan = MHI_WWAN_CTRL_PROTO_FIREHOSE, .driver_data = MHI_MAX_MTU },
+ {},
+};
+MODULE_DEVICE_TABLE(mhi, mhi_wwan_ctrl_match_table);
+
+static struct mhi_driver mhi_wwan_ctrl_driver = {
+ .id_table = mhi_wwan_ctrl_match_table,
+ .remove = mhi_wwan_ctrl_remove,
+ .probe = mhi_wwan_ctrl_probe,
+ .ul_xfer_cb = mhi_ul_xfer_cb,
+ .dl_xfer_cb = mhi_dl_xfer_cb,
+ .driver = {
+ .name = MHI_WWAN_CTRL_DRIVER_NAME,
+ },
+};
+
+static int __init mhi_wwan_ctrl_init(void)
+{
+ int ret;
+
+ ret = register_chrdev(0, MHI_WWAN_CTRL_DRIVER_NAME, &mhidev_fops);
+ if (ret < 0)
+ return ret;
+
+ mhi_wwan_ctrl_major = ret;
+ mhi_wwan_ctrl_class = class_create(THIS_MODULE, MHI_WWAN_CTRL_DRIVER_NAME);
+ if (IS_ERR(mhi_wwan_ctrl_class)) {
+ unregister_chrdev(mhi_wwan_ctrl_major, MHI_WWAN_CTRL_DRIVER_NAME);
+ return PTR_ERR(mhi_wwan_ctrl_class);
+ }
+
+ ret = mhi_driver_register(&mhi_wwan_ctrl_driver);
+ if (ret) {
+ class_destroy(mhi_wwan_ctrl_class);
+ unregister_chrdev(mhi_wwan_ctrl_major, MHI_WWAN_CTRL_DRIVER_NAME);
+ }
+
+ return ret;
+}
+
+static void __exit mhi_wwan_ctrl_exit(void)
+{
+ mhi_driver_unregister(&mhi_wwan_ctrl_driver);
+ class_destroy(mhi_wwan_ctrl_class);
+ unregister_chrdev(mhi_wwan_ctrl_major, MHI_WWAN_CTRL_DRIVER_NAME);
+ idr_destroy(&mhi_wwan_ctrl_idr);
+}
+
+module_init(mhi_wwan_ctrl_init);
+module_exit(mhi_wwan_ctrl_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MHI WWAN CTRL Driver");
+MODULE_AUTHOR("Hemant Kumar <[email protected]>");
+MODULE_AUTHOR("Loic Poulain <[email protected]>");
--
2.7.4
On Tue, Mar 09, 2021 at 09:42:16AM +0100, Loic Poulain wrote:
> The MHI WWWAN control driver allows MHI Qcom based modems to expose
> different modem control protocols/ports to userspace, so that userspace
> modem tools or daemon (e.g. ModemManager) can control WWAN config
> and state (APN config, SMS, provider selection...). A Qcom based
> modem can expose one or several of the following protocols:
> - AT: Well known AT commands interactive protocol (microcom, minicom...)
> - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> - QMI: Qcom MSM/Modem Interface (libqmi, qmicli)
> - QCDM: Qcom Modem diagnostic interface (libqcdm)
> - FIREHOSE: XML-based protocol for Modem firmware management
> (qmi-firmware-update)
>
> The different interfaces are exposed as character devices, in the same
> way as for USB modem variants (known as modem 'ports').
>
> Note that this patch is mostly a rework of the earlier MHI UCI
> tentative that was a generic interface for accessing MHI bus from
> userspace. As suggested, this new version is WWAN specific and is
> dedicated to only expose channels used for controlling a modem, and
> for which related opensource user support exist. Other MHI channels
> not fitting the requirements will request either to be plugged to
> the right Linux subsystem (when available) or to be discussed as a
> new MHI driver (e.g AI accelerator, WiFi debug channels, etc...).
>
> This change introduces a new drivers/net/wwan directory, aiming to
> be the common place for WWAN drivers.
>
> Co-developed-by: Hemant Kumar <[email protected]>
> Signed-off-by: Hemant Kumar <[email protected]>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> v2: update copyright (2021)
> v3: Move driver to dedicated drivers/net/wwan directory
>
> drivers/net/Kconfig | 2 +
> drivers/net/Makefile | 1 +
> drivers/net/wwan/Kconfig | 26 ++
> drivers/net/wwan/Makefile | 6 +
> drivers/net/wwan/mhi_wwan_ctrl.c | 559 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 594 insertions(+)
> create mode 100644 drivers/net/wwan/Kconfig
> create mode 100644 drivers/net/wwan/Makefile
> create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 1ebb4b9..28b18f2 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -501,6 +501,8 @@ source "drivers/net/wan/Kconfig"
>
> source "drivers/net/ieee802154/Kconfig"
>
> +source "drivers/net/wwan/Kconfig"
> +
> config XEN_NETDEV_FRONTEND
> tristate "Xen network device frontend driver"
> depends on XEN
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index f4990ff..5da6424 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_SUNGEM_PHY) += sungem_phy.o
> obj-$(CONFIG_WAN) += wan/
> obj-$(CONFIG_WLAN) += wireless/
> obj-$(CONFIG_IEEE802154) += ieee802154/
> +obj-$(CONFIG_WWAN) += wwan/
>
> obj-$(CONFIG_VMXNET3) += vmxnet3/
> obj-$(CONFIG_XEN_NETDEV_FRONTEND) += xen-netfront.o
> diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> new file mode 100644
> index 0000000..643aa10
> --- /dev/null
> +++ b/drivers/net/wwan/Kconfig
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Wireless WAN device configuration
> +#
> +
> +menuconfig WWAN
> + bool "Wireless WAN"
> + help
> + This section contains Wireless WAN driver configurations.
> +
> +if WWAN
> +
> +config MHI_WWAN_CTRL
> + tristate "MHI WWAN control driver for QCOM based PCIe modems"
> + depends on MHI_BUS
> + help
> + MHI WWAN CTRL allow QCOM based PCIe modems to expose different modem
> + control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
> + and FIREHOSE. These protocols can be accessed directly from userspace
> + (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> + libqcdm...).
> +
> + To compile this driver as a module, choose M here: the module will be
> + called mhi_wwan_ctrl.
> +
> +endif # WWAN
> diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> new file mode 100644
> index 0000000..994a80b
> --- /dev/null
> +++ b/drivers/net/wwan/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the Linux WWAN device drivers.
> +#
> +
> +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> new file mode 100644
> index 0000000..3904cd0
> --- /dev/null
> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> @@ -0,0 +1,559 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2018-2021, The Linux Foundation. All rights reserved.*/
> +
> +#include <linux/kernel.h>
> +#include <linux/mhi.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/poll.h>
> +
> +#define MHI_WWAN_CTRL_DRIVER_NAME "mhi_wwan_ctrl"
So a driver name is the same as the class that is being created?
That feels wrong, shouldn't the "class" be wwan?
> +#define MHI_WWAN_CTRL_MAX_MINORS 128
Why so many?
thanks,
greg k-h
Hi Loic,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Loic-Poulain/net-Add-Qcom-WWAN-control-driver/20210309-163643
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git d310ec03a34e92a77302edb804f7d68ee4f01ba0
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f4fcd3ed7ac5f29a28988eed9f5516f874073802
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Loic-Poulain/net-Add-Qcom-WWAN-control-driver/20210309-163643
git checkout f4fcd3ed7ac5f29a28988eed9f5516f874073802
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:10,
from drivers/net/wwan/mhi_wwan_ctrl.c:4:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/m68k/include/asm/page_mm.h:174:49: warning: ordered comparison of pointer with null pointer [-Wextra]
174 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void *)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/scatterlist.h:137:2: note: in expansion of macro 'BUG_ON'
137 | BUG_ON(!virt_addr_valid(buf));
| ^~~~~~
include/linux/scatterlist.h:137:10: note: in expansion of macro 'virt_addr_valid'
137 | BUG_ON(!virt_addr_valid(buf));
| ^~~~~~~~~~~~~~~
In file included from include/linux/kernel.h:14,
from drivers/net/wwan/mhi_wwan_ctrl.c:4:
drivers/net/wwan/mhi_wwan_ctrl.c: In function 'mhi_wwan_ctrl_probe':
>> drivers/net/wwan/mhi_wwan_ctrl.c:442:48: error: 'MHI_MAX_MTU' undeclared (first use in this function); did you mean 'ETH_MAX_MTU'?
442 | wwandev->mtu = min_t(size_t, id->driver_data, MHI_MAX_MTU);
| ^~~~~~~~~~~
include/linux/minmax.h:18:39: note: in definition of macro '__typecheck'
18 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^
include/linux/minmax.h:42:24: note: in expansion of macro '__safe_cmp'
42 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~
include/linux/minmax.h:110:27: note: in expansion of macro '__careful_cmp'
110 | #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <)
| ^~~~~~~~~~~~~
drivers/net/wwan/mhi_wwan_ctrl.c:442:17: note: in expansion of macro 'min_t'
442 | wwandev->mtu = min_t(size_t, id->driver_data, MHI_MAX_MTU);
| ^~~~~
drivers/net/wwan/mhi_wwan_ctrl.c:442:48: note: each undeclared identifier is reported only once for each function it appears in
442 | wwandev->mtu = min_t(size_t, id->driver_data, MHI_MAX_MTU);
| ^~~~~~~~~~~
include/linux/minmax.h:18:39: note: in definition of macro '__typecheck'
18 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^
include/linux/minmax.h:42:24: note: in expansion of macro '__safe_cmp'
42 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~
include/linux/minmax.h:110:27: note: in expansion of macro '__careful_cmp'
110 | #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <)
| ^~~~~~~~~~~~~
drivers/net/wwan/mhi_wwan_ctrl.c:442:17: note: in expansion of macro 'min_t'
442 | wwandev->mtu = min_t(size_t, id->driver_data, MHI_MAX_MTU);
| ^~~~~
include/linux/minmax.h:42:2: error: first argument to '__builtin_choose_expr' not a constant
42 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:110:27: note: in expansion of macro '__careful_cmp'
110 | #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <)
| ^~~~~~~~~~~~~
drivers/net/wwan/mhi_wwan_ctrl.c:442:17: note: in expansion of macro 'min_t'
442 | wwandev->mtu = min_t(size_t, id->driver_data, MHI_MAX_MTU);
| ^~~~~
drivers/net/wwan/mhi_wwan_ctrl.c: At top level:
>> drivers/net/wwan/mhi_wwan_ctrl.c:504:53: error: 'MHI_MAX_MTU' undeclared here (not in a function); did you mean 'ETH_MAX_MTU'?
504 | { .chan = MHI_WWAN_CTRL_PROTO_QCDM, .driver_data = MHI_MAX_MTU },
| ^~~~~~~~~~~
| ETH_MAX_MTU
vim +442 drivers/net/wwan/mhi_wwan_ctrl.c
409
410 static int mhi_wwan_ctrl_probe(struct mhi_device *mhi_dev,
411 const struct mhi_device_id *id)
412 {
413 struct mhi_wwan_dev *wwandev;
414 struct device *dev;
415 int index, err;
416
417 /* Create mhi_wwan data context */
418 wwandev = kzalloc(sizeof(*wwandev), GFP_KERNEL);
419 if (!wwandev)
420 return -ENOMEM;
421
422 /* Retrieve index */
423 mutex_lock(&mhi_wwan_ctrl_drv_lock);
424 index = idr_alloc(&mhi_wwan_ctrl_idr, wwandev, 0,
425 MHI_WWAN_CTRL_MAX_MINORS, GFP_KERNEL);
426 mutex_unlock(&mhi_wwan_ctrl_drv_lock);
427 if (index < 0) {
428 err = index;
429 goto err_free_wwandev;
430 }
431
432 /* Init mhi_wwan data */
433 kref_init(&wwandev->ref_count);
434 mutex_init(&wwandev->mhi_dev_lock);
435 mutex_init(&wwandev->write_lock);
436 init_waitqueue_head(&wwandev->ul_wq);
437 init_waitqueue_head(&wwandev->dl_wq);
438 spin_lock_init(&wwandev->dl_queue_lock);
439 INIT_LIST_HEAD(&wwandev->dl_queue);
440 wwandev->mhi_dev = mhi_dev;
441 wwandev->minor = index;
> 442 wwandev->mtu = min_t(size_t, id->driver_data, MHI_MAX_MTU);
443 set_bit(MHI_WWAN_CONNECTED, &wwandev->flags);
444
445 if (mhi_dev->dl_chan)
446 set_bit(MHI_WWAN_DL_CAP, &wwandev->flags);
447 if (mhi_dev->ul_chan)
448 set_bit(MHI_WWAN_UL_CAP, &wwandev->flags);
449
450 dev_set_drvdata(&mhi_dev->dev, wwandev);
451
452 /* Creates a new device and registers it with sysfs */
453 dev = device_create(mhi_wwan_ctrl_class, &mhi_dev->dev,
454 MKDEV(mhi_wwan_ctrl_major, index), wwandev,
455 "wwan_%s", dev_name(&mhi_dev->dev));
456 if (IS_ERR(dev)) {
457 err = PTR_ERR(dev);
458 goto err_free_idr;
459 }
460
461 return 0;
462
463 err_free_idr:
464 mutex_lock(&mhi_wwan_ctrl_drv_lock);
465 idr_remove(&mhi_wwan_ctrl_idr, wwandev->minor);
466 mutex_unlock(&mhi_wwan_ctrl_drv_lock);
467 err_free_wwandev:
468 kfree(wwandev);
469 dev_set_drvdata(&mhi_dev->dev, NULL);
470
471 return err;
472 };
473
474 static void mhi_wwan_ctrl_remove(struct mhi_device *mhi_dev)
475 {
476 struct mhi_wwan_dev *wwandev = dev_get_drvdata(&mhi_dev->dev);
477
478 dev_set_drvdata(&mhi_dev->dev, NULL);
479
480 mutex_lock(&mhi_wwan_ctrl_drv_lock);
481 idr_remove(&mhi_wwan_ctrl_idr, wwandev->minor);
482 mutex_unlock(&mhi_wwan_ctrl_drv_lock);
483
484 clear_bit(MHI_WWAN_CONNECTED, &wwandev->flags);
485 device_destroy(mhi_wwan_ctrl_class, MKDEV(mhi_wwan_ctrl_major, wwandev->minor));
486
487 /* Unlink mhi_dev from mhi_wwan_dev */
488 mutex_lock(&wwandev->mhi_dev_lock);
489 wwandev->mhi_dev = NULL;
490 mutex_unlock(&wwandev->mhi_dev_lock);
491
492 /* wake up any blocked user */
493 wake_up_interruptible(&wwandev->dl_wq);
494 wake_up_interruptible(&wwandev->ul_wq);
495
496 kref_put(&wwandev->ref_count, mhi_wwan_ctrl_dev_release);
497 }
498
499 /* .driver_data stores max mtu */
500 static const struct mhi_device_id mhi_wwan_ctrl_match_table[] = {
501 { .chan = MHI_WWAN_CTRL_PROTO_AT, .driver_data = 4096 },
502 { .chan = MHI_WWAN_CTRL_PROTO_MBIM, .driver_data = 4096 },
503 { .chan = MHI_WWAN_CTRL_PROTO_QMUX, .driver_data = 4096 },
> 504 { .chan = MHI_WWAN_CTRL_PROTO_QCDM, .driver_data = MHI_MAX_MTU },
505 { .chan = MHI_WWAN_CTRL_PROTO_FIREHOSE, .driver_data = MHI_MAX_MTU },
506 {},
507 };
508 MODULE_DEVICE_TABLE(mhi, mhi_wwan_ctrl_match_table);
509
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Greg,
On Tue, 9 Mar 2021 at 10:35, Greg KH <[email protected]> wrote:
>
> On Tue, Mar 09, 2021 at 09:42:16AM +0100, Loic Poulain wrote:
> > The MHI WWWAN control driver allows MHI Qcom based modems to expose
> > different modem control protocols/ports to userspace, so that userspace
> > modem tools or daemon (e.g. ModemManager) can control WWAN config
> > and state (APN config, SMS, provider selection...). A Qcom based
> > modem can expose one or several of the following protocols:
> > - AT: Well known AT commands interactive protocol (microcom, minicom...)
> > - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> > - QMI: Qcom MSM/Modem Interface (libqmi, qmicli)
> > - QCDM: Qcom Modem diagnostic interface (libqcdm)
> > - FIREHOSE: XML-based protocol for Modem firmware management
> > (qmi-firmware-update)
> >
> > The different interfaces are exposed as character devices, in the same
> > way as for USB modem variants (known as modem 'ports').
> >
> > Note that this patch is mostly a rework of the earlier MHI UCI
> > tentative that was a generic interface for accessing MHI bus from
> > userspace. As suggested, this new version is WWAN specific and is
> > dedicated to only expose channels used for controlling a modem, and
> > for which related opensource user support exist. Other MHI channels
> > not fitting the requirements will request either to be plugged to
> > the right Linux subsystem (when available) or to be discussed as a
> > new MHI driver (e.g AI accelerator, WiFi debug channels, etc...).
> >
> > This change introduces a new drivers/net/wwan directory, aiming to
> > be the common place for WWAN drivers.
> >
> > Co-developed-by: Hemant Kumar <[email protected]>
> > Signed-off-by: Hemant Kumar <[email protected]>
> > Signed-off-by: Loic Poulain <[email protected]>
> > ---
> > v2: update copyright (2021)
> > v3: Move driver to dedicated drivers/net/wwan directory
> >
> > drivers/net/Kconfig | 2 +
> > drivers/net/Makefile | 1 +
> > drivers/net/wwan/Kconfig | 26 ++
> > drivers/net/wwan/Makefile | 6 +
> > drivers/net/wwan/mhi_wwan_ctrl.c | 559 +++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 594 insertions(+)
> > create mode 100644 drivers/net/wwan/Kconfig
> > create mode 100644 drivers/net/wwan/Makefile
> > create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index 1ebb4b9..28b18f2 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -501,6 +501,8 @@ source "drivers/net/wan/Kconfig"
> >
> > source "drivers/net/ieee802154/Kconfig"
> >
> > +source "drivers/net/wwan/Kconfig"
> > +
> > config XEN_NETDEV_FRONTEND
> > tristate "Xen network device frontend driver"
> > depends on XEN
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index f4990ff..5da6424 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -68,6 +68,7 @@ obj-$(CONFIG_SUNGEM_PHY) += sungem_phy.o
> > obj-$(CONFIG_WAN) += wan/
> > obj-$(CONFIG_WLAN) += wireless/
> > obj-$(CONFIG_IEEE802154) += ieee802154/
> > +obj-$(CONFIG_WWAN) += wwan/
> >
> > obj-$(CONFIG_VMXNET3) += vmxnet3/
> > obj-$(CONFIG_XEN_NETDEV_FRONTEND) += xen-netfront.o
> > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> > new file mode 100644
> > index 0000000..643aa10
> > --- /dev/null
> > +++ b/drivers/net/wwan/Kconfig
> > @@ -0,0 +1,26 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# Wireless WAN device configuration
> > +#
> > +
> > +menuconfig WWAN
> > + bool "Wireless WAN"
> > + help
> > + This section contains Wireless WAN driver configurations.
> > +
> > +if WWAN
> > +
> > +config MHI_WWAN_CTRL
> > + tristate "MHI WWAN control driver for QCOM based PCIe modems"
> > + depends on MHI_BUS
> > + help
> > + MHI WWAN CTRL allow QCOM based PCIe modems to expose different modem
> > + control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
> > + and FIREHOSE. These protocols can be accessed directly from userspace
> > + (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> > + libqcdm...).
> > +
> > + To compile this driver as a module, choose M here: the module will be
> > + called mhi_wwan_ctrl.
> > +
> > +endif # WWAN
> > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> > new file mode 100644
> > index 0000000..994a80b
> > --- /dev/null
> > +++ b/drivers/net/wwan/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Makefile for the Linux WWAN device drivers.
> > +#
> > +
> > +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> > new file mode 100644
> > index 0000000..3904cd0
> > --- /dev/null
> > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> > @@ -0,0 +1,559 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (c) 2018-2021, The Linux Foundation. All rights reserved.*/
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/mhi.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/poll.h>
> > +
> > +#define MHI_WWAN_CTRL_DRIVER_NAME "mhi_wwan_ctrl"
>
> So a driver name is the same as the class that is being created?
>
> That feels wrong, shouldn't the "class" be wwan?
The driver does not aim to be THE wwan implementation, given the
heterogeneity of WWAN interfaces, so 'wwan' is probably too generic
for this bus/vendor specific driver. But since we create a new wwan
subdir, maybe we should create a minimal wwan_sysfs.c, that would
initially just offer a common class for all WWAN devices (wwan or
wwan-ports), as a first step to if not standardize, at least group
such devices under the same hat. Otherwise, we can just use the misc
class... Any thoughts?
>
> > +#define MHI_WWAN_CTRL_MAX_MINORS 128
>
> Why so many?
Right, it's not valid anymore, I'm going to change that.
Thanks,
Loic
On Tue, Mar 09, 2021 at 11:28:49AM +0100, Loic Poulain wrote:
> Hi Greg,
>
> On Tue, 9 Mar 2021 at 10:35, Greg KH <[email protected]> wrote:
> >
> > On Tue, Mar 09, 2021 at 09:42:16AM +0100, Loic Poulain wrote:
> > > The MHI WWWAN control driver allows MHI Qcom based modems to expose
> > > different modem control protocols/ports to userspace, so that userspace
> > > modem tools or daemon (e.g. ModemManager) can control WWAN config
> > > and state (APN config, SMS, provider selection...). A Qcom based
> > > modem can expose one or several of the following protocols:
> > > - AT: Well known AT commands interactive protocol (microcom, minicom...)
> > > - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> > > - QMI: Qcom MSM/Modem Interface (libqmi, qmicli)
> > > - QCDM: Qcom Modem diagnostic interface (libqcdm)
> > > - FIREHOSE: XML-based protocol for Modem firmware management
> > > (qmi-firmware-update)
> > >
> > > The different interfaces are exposed as character devices, in the same
> > > way as for USB modem variants (known as modem 'ports').
> > >
> > > Note that this patch is mostly a rework of the earlier MHI UCI
> > > tentative that was a generic interface for accessing MHI bus from
> > > userspace. As suggested, this new version is WWAN specific and is
> > > dedicated to only expose channels used for controlling a modem, and
> > > for which related opensource user support exist. Other MHI channels
> > > not fitting the requirements will request either to be plugged to
> > > the right Linux subsystem (when available) or to be discussed as a
> > > new MHI driver (e.g AI accelerator, WiFi debug channels, etc...).
> > >
> > > This change introduces a new drivers/net/wwan directory, aiming to
> > > be the common place for WWAN drivers.
> > >
> > > Co-developed-by: Hemant Kumar <[email protected]>
> > > Signed-off-by: Hemant Kumar <[email protected]>
> > > Signed-off-by: Loic Poulain <[email protected]>
> > > ---
> > > v2: update copyright (2021)
> > > v3: Move driver to dedicated drivers/net/wwan directory
> > >
> > > drivers/net/Kconfig | 2 +
> > > drivers/net/Makefile | 1 +
> > > drivers/net/wwan/Kconfig | 26 ++
> > > drivers/net/wwan/Makefile | 6 +
> > > drivers/net/wwan/mhi_wwan_ctrl.c | 559 +++++++++++++++++++++++++++++++++++++++
> > > 5 files changed, 594 insertions(+)
> > > create mode 100644 drivers/net/wwan/Kconfig
> > > create mode 100644 drivers/net/wwan/Makefile
> > > create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
> > >
> > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > > index 1ebb4b9..28b18f2 100644
> > > --- a/drivers/net/Kconfig
> > > +++ b/drivers/net/Kconfig
> > > @@ -501,6 +501,8 @@ source "drivers/net/wan/Kconfig"
> > >
> > > source "drivers/net/ieee802154/Kconfig"
> > >
> > > +source "drivers/net/wwan/Kconfig"
> > > +
> > > config XEN_NETDEV_FRONTEND
> > > tristate "Xen network device frontend driver"
> > > depends on XEN
> > > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > > index f4990ff..5da6424 100644
> > > --- a/drivers/net/Makefile
> > > +++ b/drivers/net/Makefile
> > > @@ -68,6 +68,7 @@ obj-$(CONFIG_SUNGEM_PHY) += sungem_phy.o
> > > obj-$(CONFIG_WAN) += wan/
> > > obj-$(CONFIG_WLAN) += wireless/
> > > obj-$(CONFIG_IEEE802154) += ieee802154/
> > > +obj-$(CONFIG_WWAN) += wwan/
> > >
> > > obj-$(CONFIG_VMXNET3) += vmxnet3/
> > > obj-$(CONFIG_XEN_NETDEV_FRONTEND) += xen-netfront.o
> > > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> > > new file mode 100644
> > > index 0000000..643aa10
> > > --- /dev/null
> > > +++ b/drivers/net/wwan/Kconfig
> > > @@ -0,0 +1,26 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +#
> > > +# Wireless WAN device configuration
> > > +#
> > > +
> > > +menuconfig WWAN
> > > + bool "Wireless WAN"
> > > + help
> > > + This section contains Wireless WAN driver configurations.
> > > +
> > > +if WWAN
> > > +
> > > +config MHI_WWAN_CTRL
> > > + tristate "MHI WWAN control driver for QCOM based PCIe modems"
> > > + depends on MHI_BUS
> > > + help
> > > + MHI WWAN CTRL allow QCOM based PCIe modems to expose different modem
> > > + control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
> > > + and FIREHOSE. These protocols can be accessed directly from userspace
> > > + (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> > > + libqcdm...).
> > > +
> > > + To compile this driver as a module, choose M here: the module will be
> > > + called mhi_wwan_ctrl.
> > > +
> > > +endif # WWAN
> > > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> > > new file mode 100644
> > > index 0000000..994a80b
> > > --- /dev/null
> > > +++ b/drivers/net/wwan/Makefile
> > > @@ -0,0 +1,6 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Makefile for the Linux WWAN device drivers.
> > > +#
> > > +
> > > +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> > > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > new file mode 100644
> > > index 0000000..3904cd0
> > > --- /dev/null
> > > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > @@ -0,0 +1,559 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/* Copyright (c) 2018-2021, The Linux Foundation. All rights reserved.*/
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/mhi.h>
> > > +#include <linux/mod_devicetable.h>
> > > +#include <linux/module.h>
> > > +#include <linux/poll.h>
> > > +
> > > +#define MHI_WWAN_CTRL_DRIVER_NAME "mhi_wwan_ctrl"
> >
> > So a driver name is the same as the class that is being created?
> >
> > That feels wrong, shouldn't the "class" be wwan?
>
> The driver does not aim to be THE wwan implementation, given the
> heterogeneity of WWAN interfaces, so 'wwan' is probably too generic
> for this bus/vendor specific driver. But since we create a new wwan
> subdir, maybe we should create a minimal wwan_sysfs.c, that would
> initially just offer a common class for all WWAN devices (wwan or
> wwan-ports), as a first step to if not standardize, at least group
> such devices under the same hat. Otherwise, we can just use the misc
> class... Any thoughts?
Why isn't this a good api for all wwan devices? Do you think that this
will not work for others?
A common class would be good, if they all work the same with regards to
a user/kernel api, otherwise it's pointless and not needed :)
And if we are back to the "custom user/kernel api just for this one
driver", then yes, the misc api is the easiest and simplest to use, but
I would wish for better than that for the first wwan driver...
thanks,
greg k-h
On 3/9/2021 3:33 AM, Greg KH wrote:
> On Tue, Mar 09, 2021 at 11:28:49AM +0100, Loic Poulain wrote:
>> Hi Greg,
>>
>> On Tue, 9 Mar 2021 at 10:35, Greg KH <[email protected]> wrote:
>>>
>>> On Tue, Mar 09, 2021 at 09:42:16AM +0100, Loic Poulain wrote:
>>>> The MHI WWWAN control driver allows MHI Qcom based modems to expose
>>>> different modem control protocols/ports to userspace, so that userspace
>>>> modem tools or daemon (e.g. ModemManager) can control WWAN config
>>>> and state (APN config, SMS, provider selection...). A Qcom based
>>>> modem can expose one or several of the following protocols:
>>>> - AT: Well known AT commands interactive protocol (microcom, minicom...)
>>>> - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
>>>> - QMI: Qcom MSM/Modem Interface (libqmi, qmicli)
>>>> - QCDM: Qcom Modem diagnostic interface (libqcdm)
>>>> - FIREHOSE: XML-based protocol for Modem firmware management
>>>> (qmi-firmware-update)
>>>>
>>>> The different interfaces are exposed as character devices, in the same
>>>> way as for USB modem variants (known as modem 'ports').
>>>>
>>>> Note that this patch is mostly a rework of the earlier MHI UCI
>>>> tentative that was a generic interface for accessing MHI bus from
>>>> userspace. As suggested, this new version is WWAN specific and is
>>>> dedicated to only expose channels used for controlling a modem, and
>>>> for which related opensource user support exist. Other MHI channels
>>>> not fitting the requirements will request either to be plugged to
>>>> the right Linux subsystem (when available) or to be discussed as a
>>>> new MHI driver (e.g AI accelerator, WiFi debug channels, etc...).
>>>>
>>>> This change introduces a new drivers/net/wwan directory, aiming to
>>>> be the common place for WWAN drivers.
>>>>
>>>> Co-developed-by: Hemant Kumar <[email protected]>
>>>> Signed-off-by: Hemant Kumar <[email protected]>
>>>> Signed-off-by: Loic Poulain <[email protected]>
>>>> ---
>>>> v2: update copyright (2021)
>>>> v3: Move driver to dedicated drivers/net/wwan directory
>>>>
>>>> drivers/net/Kconfig | 2 +
>>>> drivers/net/Makefile | 1 +
>>>> drivers/net/wwan/Kconfig | 26 ++
>>>> drivers/net/wwan/Makefile | 6 +
>>>> drivers/net/wwan/mhi_wwan_ctrl.c | 559 +++++++++++++++++++++++++++++++++++++++
>>>> 5 files changed, 594 insertions(+)
>>>> create mode 100644 drivers/net/wwan/Kconfig
>>>> create mode 100644 drivers/net/wwan/Makefile
>>>> create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
>>>>
>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>>> index 1ebb4b9..28b18f2 100644
>>>> --- a/drivers/net/Kconfig
>>>> +++ b/drivers/net/Kconfig
>>>> @@ -501,6 +501,8 @@ source "drivers/net/wan/Kconfig"
>>>>
>>>> source "drivers/net/ieee802154/Kconfig"
>>>>
>>>> +source "drivers/net/wwan/Kconfig"
>>>> +
>>>> config XEN_NETDEV_FRONTEND
>>>> tristate "Xen network device frontend driver"
>>>> depends on XEN
>>>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
>>>> index f4990ff..5da6424 100644
>>>> --- a/drivers/net/Makefile
>>>> +++ b/drivers/net/Makefile
>>>> @@ -68,6 +68,7 @@ obj-$(CONFIG_SUNGEM_PHY) += sungem_phy.o
>>>> obj-$(CONFIG_WAN) += wan/
>>>> obj-$(CONFIG_WLAN) += wireless/
>>>> obj-$(CONFIG_IEEE802154) += ieee802154/
>>>> +obj-$(CONFIG_WWAN) += wwan/
>>>>
>>>> obj-$(CONFIG_VMXNET3) += vmxnet3/
>>>> obj-$(CONFIG_XEN_NETDEV_FRONTEND) += xen-netfront.o
>>>> diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
>>>> new file mode 100644
>>>> index 0000000..643aa10
>>>> --- /dev/null
>>>> +++ b/drivers/net/wwan/Kconfig
>>>> @@ -0,0 +1,26 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only
>>>> +#
>>>> +# Wireless WAN device configuration
>>>> +#
>>>> +
>>>> +menuconfig WWAN
>>>> + bool "Wireless WAN"
>>>> + help
>>>> + This section contains Wireless WAN driver configurations.
>>>> +
>>>> +if WWAN
>>>> +
>>>> +config MHI_WWAN_CTRL
>>>> + tristate "MHI WWAN control driver for QCOM based PCIe modems"
>>>> + depends on MHI_BUS
>>>> + help
>>>> + MHI WWAN CTRL allow QCOM based PCIe modems to expose different modem
>>>> + control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
>>>> + and FIREHOSE. These protocols can be accessed directly from userspace
>>>> + (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
>>>> + libqcdm...).
>>>> +
>>>> + To compile this driver as a module, choose M here: the module will be
>>>> + called mhi_wwan_ctrl.
>>>> +
>>>> +endif # WWAN
>>>> diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
>>>> new file mode 100644
>>>> index 0000000..994a80b
>>>> --- /dev/null
>>>> +++ b/drivers/net/wwan/Makefile
>>>> @@ -0,0 +1,6 @@
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +#
>>>> +# Makefile for the Linux WWAN device drivers.
>>>> +#
>>>> +
>>>> +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
>>>> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
>>>> new file mode 100644
>>>> index 0000000..3904cd0
>>>> --- /dev/null
>>>> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
>>>> @@ -0,0 +1,559 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/* Copyright (c) 2018-2021, The Linux Foundation. All rights reserved.*/
>>>> +
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/mhi.h>
>>>> +#include <linux/mod_devicetable.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/poll.h>
>>>> +
>>>> +#define MHI_WWAN_CTRL_DRIVER_NAME "mhi_wwan_ctrl"
>>>
>>> So a driver name is the same as the class that is being created?
>>>
>>> That feels wrong, shouldn't the "class" be wwan?
>>
>> The driver does not aim to be THE wwan implementation, given the
>> heterogeneity of WWAN interfaces, so 'wwan' is probably too generic
>> for this bus/vendor specific driver. But since we create a new wwan
>> subdir, maybe we should create a minimal wwan_sysfs.c, that would
>> initially just offer a common class for all WWAN devices (wwan or
>> wwan-ports), as a first step to if not standardize, at least group
>> such devices under the same hat. Otherwise, we can just use the misc
>> class... Any thoughts?
>
> Why isn't this a good api for all wwan devices? Do you think that this
> will not work for others?
>
> A common class would be good, if they all work the same with regards to
> a user/kernel api, otherwise it's pointless and not needed :)
>
> And if we are back to the "custom user/kernel api just for this one
> driver", then yes, the misc api is the easiest and simplest to use, but
> I would wish for better than that for the first wwan driver...
I'm thinking this doesn't fit with the misc api due to the number of
device minors that could be expected to be consumed.
Each device supported by this driver is going to create 2-5 chardevs.
Having two devices in a system is common for "endusers". Development,
manufacturing, and test (including the community, not just talking
Qualcomm here) commonly have 12+ of these devices in a system. 12 * 5 =
60. Thats a lot of misc minor numbers to chew up just from one driver
given that the limit of dynamic minors is 128. Looking at a random x86
server that I have which could be used for such a usecase already has 30
misc minor numbers used, and this particular server has a fresh distro
install on it. I would expect that number to go up as it gets
provisioned for use.
I guess, the question to you is, how many misc minor numbers is "too
much" for a single driver to expect to consume?
--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
On Tue, Mar 09, 2021 at 09:01:11AM -0700, Jeffrey Hugo wrote:
> On 3/9/2021 3:33 AM, Greg KH wrote:
> > On Tue, Mar 09, 2021 at 11:28:49AM +0100, Loic Poulain wrote:
> > > Hi Greg,
> > >
> > > On Tue, 9 Mar 2021 at 10:35, Greg KH <[email protected]> wrote:
> > > >
> > > > On Tue, Mar 09, 2021 at 09:42:16AM +0100, Loic Poulain wrote:
> > > > > The MHI WWWAN control driver allows MHI Qcom based modems to expose
> > > > > different modem control protocols/ports to userspace, so that userspace
> > > > > modem tools or daemon (e.g. ModemManager) can control WWAN config
> > > > > and state (APN config, SMS, provider selection...). A Qcom based
> > > > > modem can expose one or several of the following protocols:
> > > > > - AT: Well known AT commands interactive protocol (microcom, minicom...)
> > > > > - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> > > > > - QMI: Qcom MSM/Modem Interface (libqmi, qmicli)
> > > > > - QCDM: Qcom Modem diagnostic interface (libqcdm)
> > > > > - FIREHOSE: XML-based protocol for Modem firmware management
> > > > > (qmi-firmware-update)
> > > > >
> > > > > The different interfaces are exposed as character devices, in the same
> > > > > way as for USB modem variants (known as modem 'ports').
> > > > >
> > > > > Note that this patch is mostly a rework of the earlier MHI UCI
> > > > > tentative that was a generic interface for accessing MHI bus from
> > > > > userspace. As suggested, this new version is WWAN specific and is
> > > > > dedicated to only expose channels used for controlling a modem, and
> > > > > for which related opensource user support exist. Other MHI channels
> > > > > not fitting the requirements will request either to be plugged to
> > > > > the right Linux subsystem (when available) or to be discussed as a
> > > > > new MHI driver (e.g AI accelerator, WiFi debug channels, etc...).
> > > > >
> > > > > This change introduces a new drivers/net/wwan directory, aiming to
> > > > > be the common place for WWAN drivers.
> > > > >
> > > > > Co-developed-by: Hemant Kumar <[email protected]>
> > > > > Signed-off-by: Hemant Kumar <[email protected]>
> > > > > Signed-off-by: Loic Poulain <[email protected]>
> > > > > ---
> > > > > v2: update copyright (2021)
> > > > > v3: Move driver to dedicated drivers/net/wwan directory
> > > > >
> > > > > drivers/net/Kconfig | 2 +
> > > > > drivers/net/Makefile | 1 +
> > > > > drivers/net/wwan/Kconfig | 26 ++
> > > > > drivers/net/wwan/Makefile | 6 +
> > > > > drivers/net/wwan/mhi_wwan_ctrl.c | 559 +++++++++++++++++++++++++++++++++++++++
> > > > > 5 files changed, 594 insertions(+)
> > > > > create mode 100644 drivers/net/wwan/Kconfig
> > > > > create mode 100644 drivers/net/wwan/Makefile
> > > > > create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
> > > > >
> > > > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > > > > index 1ebb4b9..28b18f2 100644
> > > > > --- a/drivers/net/Kconfig
> > > > > +++ b/drivers/net/Kconfig
> > > > > @@ -501,6 +501,8 @@ source "drivers/net/wan/Kconfig"
> > > > >
> > > > > source "drivers/net/ieee802154/Kconfig"
> > > > >
> > > > > +source "drivers/net/wwan/Kconfig"
> > > > > +
> > > > > config XEN_NETDEV_FRONTEND
> > > > > tristate "Xen network device frontend driver"
> > > > > depends on XEN
> > > > > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > > > > index f4990ff..5da6424 100644
> > > > > --- a/drivers/net/Makefile
> > > > > +++ b/drivers/net/Makefile
> > > > > @@ -68,6 +68,7 @@ obj-$(CONFIG_SUNGEM_PHY) += sungem_phy.o
> > > > > obj-$(CONFIG_WAN) += wan/
> > > > > obj-$(CONFIG_WLAN) += wireless/
> > > > > obj-$(CONFIG_IEEE802154) += ieee802154/
> > > > > +obj-$(CONFIG_WWAN) += wwan/
> > > > >
> > > > > obj-$(CONFIG_VMXNET3) += vmxnet3/
> > > > > obj-$(CONFIG_XEN_NETDEV_FRONTEND) += xen-netfront.o
> > > > > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> > > > > new file mode 100644
> > > > > index 0000000..643aa10
> > > > > --- /dev/null
> > > > > +++ b/drivers/net/wwan/Kconfig
> > > > > @@ -0,0 +1,26 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > > +#
> > > > > +# Wireless WAN device configuration
> > > > > +#
> > > > > +
> > > > > +menuconfig WWAN
> > > > > + bool "Wireless WAN"
> > > > > + help
> > > > > + This section contains Wireless WAN driver configurations.
> > > > > +
> > > > > +if WWAN
> > > > > +
> > > > > +config MHI_WWAN_CTRL
> > > > > + tristate "MHI WWAN control driver for QCOM based PCIe modems"
> > > > > + depends on MHI_BUS
> > > > > + help
> > > > > + MHI WWAN CTRL allow QCOM based PCIe modems to expose different modem
> > > > > + control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
> > > > > + and FIREHOSE. These protocols can be accessed directly from userspace
> > > > > + (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> > > > > + libqcdm...).
> > > > > +
> > > > > + To compile this driver as a module, choose M here: the module will be
> > > > > + called mhi_wwan_ctrl.
> > > > > +
> > > > > +endif # WWAN
> > > > > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> > > > > new file mode 100644
> > > > > index 0000000..994a80b
> > > > > --- /dev/null
> > > > > +++ b/drivers/net/wwan/Makefile
> > > > > @@ -0,0 +1,6 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +#
> > > > > +# Makefile for the Linux WWAN device drivers.
> > > > > +#
> > > > > +
> > > > > +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> > > > > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > > > new file mode 100644
> > > > > index 0000000..3904cd0
> > > > > --- /dev/null
> > > > > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > > > @@ -0,0 +1,559 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/* Copyright (c) 2018-2021, The Linux Foundation. All rights reserved.*/
> > > > > +
> > > > > +#include <linux/kernel.h>
> > > > > +#include <linux/mhi.h>
> > > > > +#include <linux/mod_devicetable.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/poll.h>
> > > > > +
> > > > > +#define MHI_WWAN_CTRL_DRIVER_NAME "mhi_wwan_ctrl"
> > > >
> > > > So a driver name is the same as the class that is being created?
> > > >
> > > > That feels wrong, shouldn't the "class" be wwan?
> > >
> > > The driver does not aim to be THE wwan implementation, given the
> > > heterogeneity of WWAN interfaces, so 'wwan' is probably too generic
> > > for this bus/vendor specific driver. But since we create a new wwan
> > > subdir, maybe we should create a minimal wwan_sysfs.c, that would
> > > initially just offer a common class for all WWAN devices (wwan or
> > > wwan-ports), as a first step to if not standardize, at least group
> > > such devices under the same hat. Otherwise, we can just use the misc
> > > class... Any thoughts?
> >
> > Why isn't this a good api for all wwan devices? Do you think that this
> > will not work for others?
> >
> > A common class would be good, if they all work the same with regards to
> > a user/kernel api, otherwise it's pointless and not needed :)
> >
> > And if we are back to the "custom user/kernel api just for this one
> > driver", then yes, the misc api is the easiest and simplest to use, but
> > I would wish for better than that for the first wwan driver...
>
> I'm thinking this doesn't fit with the misc api due to the number of device
> minors that could be expected to be consumed.
That's why I asked how many minors do you need :)
> Each device supported by this driver is going to create 2-5 chardevs. Having
> two devices in a system is common for "endusers". Development,
> manufacturing, and test (including the community, not just talking Qualcomm
> here) commonly have 12+ of these devices in a system. 12 * 5 = 60. Thats a
> lot of misc minor numbers to chew up just from one driver given that the
> limit of dynamic minors is 128. Looking at a random x86 server that I have
> which could be used for such a usecase already has 30 misc minor numbers
> used, and this particular server has a fresh distro install on it. I would
> expect that number to go up as it gets provisioned for use.
Look at a phone these days, I see way more misc devices used than just
"30" :(
> I guess, the question to you is, how many misc minor numbers is "too much"
> for a single driver to expect to consume?
If you expect more than 10, I would say to use a real major number. But
be explicit as to what you are expecting here, it was not obvious at
all.
thanks,
greg k-h