Re: [PATCH net-next v8 1/2] net: Add a WWAN subsystem
From: Greg KH <gregkh@linuxfoundation.org>
Date: 2021-04-02 14:03:00
Also in:
linux-arm-msm, lkml
On Fri, Apr 02, 2021 at 04:06:36PM +0200, Loic Poulain wrote:
This change introduces initial support for a WWAN framework. Given the complexity and heterogeneity of existing WWAN hardwares and interfaces, there is no strict definition of what a WWAN device is and how it should be represented. It's often a collection of multiple devices that perform the global WWAN feature (netdev, tty, chardev, etc). One usual way to expose modem controls and configuration is via high level protocols such as the well known AT command protocol, MBIM or QMI. The USB modems started to expose that as character devices, and user daemons such as ModemManager learnt how to deal with them. This initial version adds the concept of WWAN port, which can be created by any driver to expose one of these protocols. The WWAN core takes care of the generic part, including character device management, and rely on port operations to received and submit protocol data. Since the different components/devices do no necesserarly know about each others, and can be created/removed in different orders, the WWAN core ensures that all WAN ports that contribute to the 'whole' WWAN feature are grouped under the same virtual WWAN device, relying on the provided parent device (e.g. mhi controller, USB device). It's a 'trick' I copied from Johannes's earlier WWAN subsystem proposal. This initial version is purposely minimalist, it's essentially moving the generic part of the previously proposed mhi_wwan_ctrl driver inside a common WWAN framework, but the implementation is open and flexible enough to allow extension for further drivers. Signed-off-by: Loic Poulain <redacted>
Always run checkpatch before sending stuff off :( Anyway, one thing did stand out:
quoted hunk ↗ jump to hunk
--- /dev/null +++ b/include/linux/wwan.h@@ -0,0 +1,127 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* Copyright (c) 2021, Linaro Ltd <loic.poulain@linaro.org> */ + +#ifndef __WWAN_H +#define __WWAN_H + +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/skbuff.h> + +/** + * enum wwan_port_type - WWAN port types + * @WWAN_PORT_AT: AT commands + * @WWAN_PORT_MBIM: Mobile Broadband Interface Model control + * @WWAN_PORT_QMI: Qcom modem/MSM interface for modem control + * @WWAN_PORT_QCDM: Qcom Modem diagnostic interface + * @WWAN_PORT_FIREHOSE: XML based command protocol + * @WWAN_PORT_MAX + */ +enum wwan_port_type { + WWAN_PORT_AT, + WWAN_PORT_MBIM, + WWAN_PORT_QMI, + WWAN_PORT_QCDM, + WWAN_PORT_FIREHOSE, + WWAN_PORT_MAX, +}; + +/** + * struct wwan_port - The structure that defines a WWAN port + * @type: Port type + * @start_count: Port start counter + * @flags: Store port state and capabilities + * @ops: Pointer to WWAN port operations + * @ops_lock: Protect port ops + * @dev: Underlying device + * @rxq: Buffer inbound queue + * @waitqueue: The waitqueue for port fops (read/write/poll) + */ +struct wwan_port { + enum wwan_port_type type; + unsigned int start_count; + unsigned long flags; + const struct wwan_port_ops *ops; + struct mutex ops_lock; + struct device dev; + struct sk_buff_head rxq; + wait_queue_head_t waitqueue; +};
No need to put the actual definition of struct wwan_port in this .h file, keep it private in your .c file to keep wwan drivers from poking around in it where they shouldn't be :) thanks, greg k-h