Re: [PATCH v2 1/2] net: Add MHI Endpoint network driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/7/2023 10:27 AM, Jeffrey Hugo wrote:
On 6/7/2023 9:24 AM, Manivannan Sadhasivam wrote:
Add a network driver for the Modem Host Interface (MHI) endpoint devices
that provides network interfaces to the PCIe based Qualcomm endpoint
devices supporting MHI bus. This driver allows the MHI endpoint devices to
establish IP communication with the host machines (x86, ARM64) over MHI
bus.

The driver currently supports only IP_SW0 MHI channel that can be used
to route IP traffic from the endpoint CPU to host machine.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
---
  drivers/net/Kconfig      |   9 ++
  drivers/net/Makefile     |   1 +
  drivers/net/mhi_ep_net.c | 331 +++++++++++++++++++++++++++++++++++++++
  3 files changed, 341 insertions(+)
  create mode 100644 drivers/net/mhi_ep_net.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 368c6f5b327e..36b628e2e49f 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -452,6 +452,15 @@ config MHI_NET
        QCOM based WWAN modems for IP or QMAP/rmnet protocol (like SDX55).
        Say Y or M.
+config MHI_EP_NET
+    tristate "MHI Endpoint network driver"
+    depends on MHI_BUS_EP
+    help
+      This is the network driver for MHI bus implementation in endpoint
+      devices. It is used provide the network interface for QCOM endpoint
+      devices such as SDX55 modems.
+      Say Y or M.

What will the module be called if "m" is selected?

+
  endif # NET_CORE
  config SUNGEM_PHY
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index e26f98f897c5..b8e706a4150e 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_NLMON) += nlmon.o
  obj-$(CONFIG_NET_VRF) += vrf.o
  obj-$(CONFIG_VSOCKMON) += vsockmon.o
  obj-$(CONFIG_MHI_NET) += mhi_net.o
+obj-$(CONFIG_MHI_EP_NET) += mhi_ep_net.o
  #
  # Networking Drivers
diff --git a/drivers/net/mhi_ep_net.c b/drivers/net/mhi_ep_net.c
new file mode 100644
index 000000000000..0d7939caefc7
--- /dev/null
+++ b/drivers/net/mhi_ep_net.c
@@ -0,0 +1,331 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MHI Endpoint Network driver
+ *
+ * Based on drivers/net/mhi_net.c
+ *
+ * Copyright (c) 2023, Linaro Ltd.
+ * Author: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
+ */
+
+#include <linux/if_arp.h>
+#include <linux/mhi_ep.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/u64_stats_sync.h>
+
+#define MHI_NET_MIN_MTU        ETH_MIN_MTU
+#define MHI_NET_MAX_MTU        0xffff

ETH_MAX_MTU ?

Personal preference thing. If you think 0xffff is really the superior option, so be it. Personally, it takes me a second to figure out that is 64k - 1 and then relate it to the MHI packet size limit. Also seems really odd with this line of code right next to, and related to, ETH_MIN_MTU. Feels like a non-magic number here will make things more maintainable.

Alternatively move MHI_MAX_MTU out of host/internal.h into something that is convenient for this driver to include and use? It is a fundamental constant for the MHI protocol, we just haven't yet had a need for it to be used outside of the MHI bus implementation code.

+
+struct mhi_ep_net_stats {
+    u64_stats_t rx_packets;
+    u64_stats_t rx_bytes;
+    u64_stats_t rx_errors;
+    u64_stats_t tx_packets;
+    u64_stats_t tx_bytes;
+    u64_stats_t tx_errors;
+    u64_stats_t tx_dropped;
+    struct u64_stats_sync tx_syncp;
+    struct u64_stats_sync rx_syncp;
+};
+
+struct mhi_ep_net_dev {
+    struct mhi_ep_device *mdev;
+    struct net_device *ndev;
+    struct mhi_ep_net_stats stats;
+    struct workqueue_struct *xmit_wq;
+    struct work_struct xmit_work;
+    struct sk_buff_head tx_buffers;
+    spinlock_t tx_lock; /* Lock for protecting tx_buffers */
+    u32 mru;
+};
+
+static void mhi_ep_net_dev_process_queue_packets(struct work_struct *work)
+{
+    struct mhi_ep_net_dev *mhi_ep_netdev = container_of(work,
+            struct mhi_ep_net_dev, xmit_work);

Looks like this can fit all on one line to me.






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux