[PATCH v3 00/14] greybus: introduce CPC as transport layer

[PATCH v3 00/14] greybus: introduce CPC as transport layer

[Damien Riégel]

Hi,

This patchset brings support for Silicon Labs' CPC protocol as transport
layer for Greybus. CPC stands for Co-Processor Communication and
currently exists as a userspace daemon [1] that enables multiple
applications on a coprocessor to share a single physical link to a Linux
host, using a protocol loosely based on HDLC [2].

While the userspace implementations serves its purpose, it has some
redundancies with Greybus that makes it not very suitable for kernel
integration as-is, and so the protocol has been modified to better fit
with Greybus. Even though kernel and userspace implementations share the
same name, they are not the same protocol and are not compatible. The
kernel integration with Greybus is intended to superseed the userspace
implementation.

CPC is introduced as a module that sits between Greybus and CPC Host
Device Drivers implementations, like SDIO or SPI. This patchset includes
SDIO as physical layer but the protocol is not final and might change,
it's mostly there to showcase all the elements.

        +----------------------------------------------------+
        |                      Greybus                       |
        +----------------------------------------------------+
                                 /|\
                                  |
                                 \|/
        +----------------------------------------------------+
        |                        CPC                         |
        +----------------------------------------------------+
              /|\                /|\                /|\
               |                  |                  |
              \|/                \|/                \|/
          +----------+       +---------+       +-----------+
          |   SDIO   |       |   SPI   |       |   Others  |
          +----------+       +---------+       +-----------+

CPC implements some of the features of Unipro that Greybus relies upon,
like reliable transmission. CPC takes care of detecting transmission
errors and retransmit frames if necessary, but that feature is not part
of the RFC to keep it concise. There's also a flow-control
feature, preventing sending messages to already full cports.

In order to implement these features, a 4-byte header is prepended to
Greybus messages, making the whole header 12 bytes (Greybus header is 8
bytes).

This RFC starts by implementing a shim layer between physical bus
drivers (like SDIO and SPI) and Greybus, and progressively add more
elements to it to make it useful in its own right. Finally, an SDIO
driver is added to enable the communication with a remote device.

[1] https://github.com/SiliconLabs/cpc-daemon
[2] https://en.wikipedia.org/wiki/High-Level_Data_Link_Control

Changes in v3:
  - addressed Jerome's review comments, mostly for SDIO driver
  - rewrote cover letter and commit messages across the series

Changes in v2:
  - addressed review comments and errors reported by kernel bot
  - for SDIO driver, remove padding between headers and payloads when
    aggregating packets together

Damien Riégel (13):
  greybus: cpc: add minimal CPC Host Device infrastructure
  greybus: cpc: introduce CPC cport structure
  greybus: cpc: use socket buffers instead of gb_message in TX path
  greybus: cpc: pack cport ID in Greybus header
  greybus: cpc: switch RX path to socket buffers
  greybus: cpc: introduce CPC header structure
  greybus: cpc: account for CPC header size in RX and TX path
  greybus: cpc: add and validate sequence numbers
  greybus: cpc: acknowledge all incoming messages
  greybus: cpc: use holding queue instead of sending out immediately
  greybus: cpc: honour remote's RX window
  greybus: cpc: let host device drivers dequeue TX frames
  greybus: cpc: add private data pointer in CPC Host Device

Gabriel Beaulieu (1):
  greybus: cpc: add CPC SDIO host driver

 MAINTAINERS                    |   6 +
 drivers/greybus/Kconfig        |   2 +
 drivers/greybus/Makefile       |   2 +
 drivers/greybus/cpc/Kconfig    |  24 ++
 drivers/greybus/cpc/Makefile   |   9 +
 drivers/greybus/cpc/cpc.h      |  75 ++++++
 drivers/greybus/cpc/cport.c    | 112 ++++++++
 drivers/greybus/cpc/header.c   | 136 ++++++++++
 drivers/greybus/cpc/header.h   |  52 ++++
 drivers/greybus/cpc/host.c     | 319 ++++++++++++++++++++++
 drivers/greybus/cpc/host.h     |  63 +++++
 drivers/greybus/cpc/protocol.c | 170 ++++++++++++
 drivers/greybus/cpc/sdio.c     | 480 +++++++++++++++++++++++++++++++++
 13 files changed, 1450 insertions(+)
 create mode 100644 drivers/greybus/cpc/Kconfig
 create mode 100644 drivers/greybus/cpc/Makefile
 create mode 100644 drivers/greybus/cpc/cpc.h
 create mode 100644 drivers/greybus/cpc/cport.c
 create mode 100644 drivers/greybus/cpc/header.c
 create mode 100644 drivers/greybus/cpc/header.h
 create mode 100644 drivers/greybus/cpc/host.c
 create mode 100644 drivers/greybus/cpc/host.h
 create mode 100644 drivers/greybus/cpc/protocol.c
 create mode 100644 drivers/greybus/cpc/sdio.c

-- 
2.52.0

[PATCH v3 01/14] greybus: cpc: add minimal CPC Host Device infrastructure

[Damien Riégel]

CPC stands for Co-Processor Communication and currently exists as a
userspace daemon [1]. It enables multiple applications running on a
coprocessor to share a single physical link to a Linux host.

CPC userspace and kernel implementations differ, as the kernel
implementation relies on Greybus. So even though both share the "CPC"
name, they are not the same protocol and are not intended to be
compatible with each other. Kernel integration is intended to superseed
the userspace daemon.

The point of the kernel integration is gain access to more physical
links, like SDIO, and eventually add Greybus drivers that integrate into
kernel stacks like Wi-Fi and Bluetooth.

The first step to add CPC support with Greybus is to add a very minimal
module for CPC Host Devices. For now, this module only proxies calls to
the Greybus Host Device API and does nothing useful, but further commits
will use this base to build upon.

[1] https://github.com/SiliconLabs/cpc-daemon

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
Changes in v3:
  - rewrite commit message and Kconfig help message to better explain
    what CPC is

 MAINTAINERS                  |  6 +++
 drivers/greybus/Kconfig      |  2 +
 drivers/greybus/Makefile     |  2 +
 drivers/greybus/cpc/Kconfig  | 12 ++++++
 drivers/greybus/cpc/Makefile |  6 +++
 drivers/greybus/cpc/host.c   | 84 ++++++++++++++++++++++++++++++++++++
 drivers/greybus/cpc/host.h   | 40 +++++++++++++++++
 7 files changed, 152 insertions(+)
 create mode 100644 drivers/greybus/cpc/Kconfig
 create mode 100644 drivers/greybus/cpc/Makefile
 create mode 100644 drivers/greybus/cpc/host.c
 create mode 100644 drivers/greybus/cpc/host.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6d1de82e6dc..56daf9ec310 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10774,6 +10774,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
 F:	drivers/greybus/gb-beagleplay.c
 
+GREYBUS CPC DRIVERS
+M:	Damien Riégel <damien.riegel@silabs.com>
+R:	Silicon Labs Kernel Team <linux-devel@silabs.com>
+S:	Supported
+F:	drivers/greybus/cpc/*
+
 GREYBUS SUBSYSTEM
 M:	Johan Hovold <johan@kernel.org>
 M:	Alex Elder <elder@kernel.org>
diff --git a/drivers/greybus/Kconfig b/drivers/greybus/Kconfig
index c3f056d28b0..565a0fdcb2c 100644
--- a/drivers/greybus/Kconfig
+++ b/drivers/greybus/Kconfig
@@ -30,6 +30,8 @@ config GREYBUS_BEAGLEPLAY
 	  To compile this code as a module, chose M here: the module
 	  will be called gb-beagleplay.ko
 
+source "drivers/greybus/cpc/Kconfig"
+
 config GREYBUS_ES2
 	tristate "Greybus ES3 USB host controller"
 	depends on USB
diff --git a/drivers/greybus/Makefile b/drivers/greybus/Makefile
index d986e94f889..92fe1d62691 100644
--- a/drivers/greybus/Makefile
+++ b/drivers/greybus/Makefile
@@ -21,6 +21,8 @@ ccflags-y += -I$(src)
 obj-$(CONFIG_GREYBUS_BEAGLEPLAY)	+= gb-beagleplay.o
 
 # Greybus Host controller drivers
+obj-$(CONFIG_GREYBUS_CPC)	+= cpc/
+
 gb-es2-y := es2.o
 
 obj-$(CONFIG_GREYBUS_ES2)	+= gb-es2.o
diff --git a/drivers/greybus/cpc/Kconfig b/drivers/greybus/cpc/Kconfig
new file mode 100644
index 00000000000..0e72383381a
--- /dev/null
+++ b/drivers/greybus/cpc/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config GREYBUS_CPC
+	tristate "Greybus CPC driver"
+	help
+	  CPC (Co-Processor Communication) is a protocol for sharing a single
+	  physical link to a coprocessor between multiple users. CPC adds
+	  checksumming, retransmissions and flow control to Greybus to transmit
+	  messages as reliably as possible.
+
+	  To compile this code as a module, chose M here: the module will be
+	  called gb-cpc.ko
diff --git a/drivers/greybus/cpc/Makefile b/drivers/greybus/cpc/Makefile
new file mode 100644
index 00000000000..490982a0ff5
--- /dev/null
+++ b/drivers/greybus/cpc/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+gb-cpc-y := host.o
+
+# CPC core
+obj-$(CONFIG_GREYBUS_CPC)	+= gb-cpc.o
diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
new file mode 100644
index 00000000000..80516517ff6
--- /dev/null
+++ b/drivers/greybus/cpc/host.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/err.h>
+#include <linux/greybus.h>
+#include <linux/module.h>
+
+#include "host.h"
+
+static struct cpc_host_device *gb_hd_to_cpc_hd(struct gb_host_device *hd)
+{
+	return (struct cpc_host_device *)&hd->hd_priv;
+}
+
+static int cpc_gb_message_send(struct gb_host_device *gb_hd, u16 cport_id,
+			       struct gb_message *message, gfp_t gfp_mask)
+{
+	struct cpc_host_device *cpc_hd = gb_hd_to_cpc_hd(gb_hd);
+
+	return cpc_hd->driver->message_send(cpc_hd, cport_id, message, gfp_mask);
+}
+
+static void cpc_gb_message_cancel(struct gb_message *message)
+{
+	/* Not implemented */
+}
+
+static struct gb_hd_driver cpc_gb_driver = {
+	.hd_priv_size = sizeof(struct cpc_host_device),
+	.message_send = cpc_gb_message_send,
+	.message_cancel = cpc_gb_message_cancel,
+};
+
+struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent)
+{
+	struct cpc_host_device *cpc_hd;
+	struct gb_host_device *hd;
+
+	if ((!driver->message_send) || (!driver->message_cancel)) {
+		dev_err(parent, "missing mandatory callbacks\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	hd = gb_hd_create(&cpc_gb_driver, parent, GB_CPC_MSG_SIZE_MAX, GB_CPC_NUM_CPORTS);
+	if (IS_ERR(hd))
+		return (struct cpc_host_device *)hd;
+
+	cpc_hd = gb_hd_to_cpc_hd(hd);
+	cpc_hd->gb_hd = hd;
+	cpc_hd->driver = driver;
+
+	return cpc_hd;
+}
+EXPORT_SYMBOL_GPL(cpc_hd_create);
+
+int cpc_hd_add(struct cpc_host_device *cpc_hd)
+{
+	return gb_hd_add(cpc_hd->gb_hd);
+}
+EXPORT_SYMBOL_GPL(cpc_hd_add);
+
+void cpc_hd_put(struct cpc_host_device *cpc_hd)
+{
+	return gb_hd_put(cpc_hd->gb_hd);
+}
+EXPORT_SYMBOL_GPL(cpc_hd_put);
+
+void cpc_hd_del(struct cpc_host_device *cpc_hd)
+{
+	return gb_hd_del(cpc_hd->gb_hd);
+}
+EXPORT_SYMBOL_GPL(cpc_hd_del);
+
+void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u16 cport_id, u8 *data, size_t length)
+{
+	greybus_data_rcvd(cpc_hd->gb_hd, cport_id, data, length);
+}
+EXPORT_SYMBOL_GPL(cpc_hd_rcvd);
+
+MODULE_DESCRIPTION("Greybus over CPC");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Silicon Laboratories, Inc.");
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
new file mode 100644
index 00000000000..f55feb303f4
--- /dev/null
+++ b/drivers/greybus/cpc/host.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_HOST_H
+#define __CPC_HOST_H
+
+#include <linux/device.h>
+#include <linux/greybus.h>
+#include <linux/types.h>
+
+#define GB_CPC_MSG_SIZE_MAX 4096
+#define GB_CPC_NUM_CPORTS 8
+
+struct cpc_host_device;
+
+struct cpc_hd_driver {
+	int (*message_send)(struct cpc_host_device *hd, u16 dest_cport_id,
+			    struct gb_message *message, gfp_t gfp_mask);
+	void (*message_cancel)(struct gb_message *message);
+};
+
+/**
+ * struct cpc_host_device - CPC host device.
+ * @gb_hd: pointer to Greybus Host Device this device belongs to.
+ * @driver: driver operations.
+ */
+struct cpc_host_device {
+	struct gb_host_device *gb_hd;
+	const struct cpc_hd_driver *driver;
+};
+
+struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent);
+int cpc_hd_add(struct cpc_host_device *cpc_hd);
+void cpc_hd_put(struct cpc_host_device *cpc_hd);
+void cpc_hd_del(struct cpc_host_device *cpc_hd);
+void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u16 cport_id, u8 *data, size_t length);
+
+#endif
-- 
2.52.0

[PATCH v3 02/14] greybus: cpc: introduce CPC cport structure

[Damien Riégel]

A number of CPC features, like retransmission or remote's receive
window, are cport-based. In order to prepare for these features,
introduce a CPC CPort context structure.

The CPC Host Device module now implements cport_allocate and
cport_release callbacks in order to allocate and release these
structures when requested by Greybus module.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
Changes in v3:
  - simplify cpc_hd_cport_allocate so that cport with ID "i"is at offset
    "i" in cpc_hd->cports array

 drivers/greybus/cpc/Makefile |   2 +-
 drivers/greybus/cpc/cpc.h    |  29 +++++++++
 drivers/greybus/cpc/cport.c  |  37 +++++++++++
 drivers/greybus/cpc/host.c   | 115 ++++++++++++++++++++++++++++++++++-
 drivers/greybus/cpc/host.h   |  12 ++++
 5 files changed, 193 insertions(+), 2 deletions(-)
 create mode 100644 drivers/greybus/cpc/cpc.h
 create mode 100644 drivers/greybus/cpc/cport.c

diff --git a/drivers/greybus/cpc/Makefile b/drivers/greybus/cpc/Makefile
index 490982a0ff5..3d50f8c5473 100644
--- a/drivers/greybus/cpc/Makefile
+++ b/drivers/greybus/cpc/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-gb-cpc-y := host.o
+gb-cpc-y := cport.o host.o
 
 # CPC core
 obj-$(CONFIG_GREYBUS_CPC)	+= gb-cpc.o
diff --git a/drivers/greybus/cpc/cpc.h b/drivers/greybus/cpc/cpc.h
new file mode 100644
index 00000000000..3915a7fbc4f
--- /dev/null
+++ b/drivers/greybus/cpc/cpc.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_H
+#define __CPC_H
+
+#include <linux/device.h>
+#include <linux/greybus.h>
+#include <linux/types.h>
+
+/**
+ * struct cpc_cport - CPC cport
+ * @id: cport ID
+ * @cpc_hd: pointer to the CPC host device this cport belongs to
+ */
+struct cpc_cport {
+	u16 id;
+
+	struct cpc_host_device *cpc_hd;
+};
+
+struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask);
+void cpc_cport_release(struct cpc_cport *cport);
+
+int cpc_cport_message_send(struct cpc_cport *cport, struct gb_message *message, gfp_t gfp_mask);
+
+#endif
diff --git a/drivers/greybus/cpc/cport.c b/drivers/greybus/cpc/cport.c
new file mode 100644
index 00000000000..88bdb2f8182
--- /dev/null
+++ b/drivers/greybus/cpc/cport.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include "cpc.h"
+#include "host.h"
+
+/**
+ * cpc_cport_alloc() - Allocate and initialize CPC cport.
+ * @cport_id: cport ID.
+ * @gfp_mask: GFP mask for allocation.
+ *
+ * Return: Pointer to allocated and initialized cpc_cport, or NULL on failure.
+ */
+struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask)
+{
+	struct cpc_cport *cport;
+
+	cport = kzalloc(sizeof(*cport), gfp_mask);
+	if (!cport)
+		return NULL;
+
+	cport->id = cport_id;
+
+	return cport;
+}
+
+void cpc_cport_release(struct cpc_cport *cport)
+{
+	kfree(cport);
+}
+
+int cpc_cport_message_send(struct cpc_cport *cport, struct gb_message *message, gfp_t gfp_mask)
+{
+	return cport->cpc_hd->driver->message_send(cport->cpc_hd, cport->id, message, gfp_mask);
+}
diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 80516517ff6..3dda5b06590 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -7,6 +7,7 @@
 #include <linux/greybus.h>
 #include <linux/module.h>
 
+#include "cpc.h"
 #include "host.h"
 
 static struct cpc_host_device *gb_hd_to_cpc_hd(struct gb_host_device *hd)
@@ -14,12 +15,101 @@ static struct cpc_host_device *gb_hd_to_cpc_hd(struct gb_host_device *hd)
 	return (struct cpc_host_device *)&hd->hd_priv;
 }
 
+static struct cpc_cport *cpc_hd_get_cport_unlocked(struct cpc_host_device *cpc_hd, u16 cport_id)
+{
+	if (cport_id >= ARRAY_SIZE(cpc_hd->cports))
+		return NULL;
+
+	return cpc_hd->cports[cport_id];
+}
+
+static struct cpc_cport *cpc_hd_get_cport(struct cpc_host_device *cpc_hd, u16 cport_id)
+{
+	struct cpc_cport *cport;
+
+	mutex_lock(&cpc_hd->lock);
+	cport = cpc_hd_get_cport_unlocked(cpc_hd, cport_id);
+	mutex_unlock(&cpc_hd->lock);
+
+	return cport;
+}
+
+static int cpc_hd_message_send(struct cpc_host_device *cpc_hd, u16 cport_id,
+			       struct gb_message *message, gfp_t gfp_mask)
+{
+	struct cpc_cport *cport;
+
+	cport = cpc_hd_get_cport(cpc_hd, cport_id);
+	if (!cport) {
+		dev_err(cpc_hd_dev(cpc_hd), "message_send: cport %u not found\n", cport_id);
+		return -EINVAL;
+	}
+
+	return cpc_cport_message_send(cport, message, gfp_mask);
+}
+
+static int cpc_hd_cport_allocate(struct cpc_host_device *cpc_hd, int cport_id, unsigned long flags)
+{
+	struct cpc_cport *cport;
+	int ret;
+
+	mutex_lock(&cpc_hd->lock);
+	if (cport_id < 0) {
+		for (cport_id = 0; cport_id < ARRAY_SIZE(cpc_hd->cports); cport_id++) {
+			if (cpc_hd->cports[cport_id] == NULL)
+				break;
+		}
+	}
+
+	if (cport_id >= ARRAY_SIZE(cpc_hd->cports)) {
+		ret = -ENOSPC;
+		goto unlock;
+	}
+
+	cport = cpc_hd_get_cport_unlocked(cpc_hd, cport_id);
+	if (cport) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	cport = cpc_cport_alloc(cport_id, GFP_KERNEL);
+	if (!cport) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	cport->cpc_hd = cpc_hd;
+
+	cpc_hd->cports[cport_id] = cport;
+	ret = cport_id;
+
+unlock:
+	mutex_unlock(&cpc_hd->lock);
+
+	return ret;
+}
+
+static void cpc_hd_cport_release(struct cpc_host_device *cpc_hd, u16 cport_id)
+{
+	struct cpc_cport *cport;
+
+	mutex_lock(&cpc_hd->lock);
+
+	cport = cpc_hd->cports[cport_id];
+	if (cport) {
+		cpc_cport_release(cport);
+		cpc_hd->cports[cport_id] = NULL;
+	}
+
+	mutex_unlock(&cpc_hd->lock);
+}
+
 static int cpc_gb_message_send(struct gb_host_device *gb_hd, u16 cport_id,
 			       struct gb_message *message, gfp_t gfp_mask)
 {
 	struct cpc_host_device *cpc_hd = gb_hd_to_cpc_hd(gb_hd);
 
-	return cpc_hd->driver->message_send(cpc_hd, cport_id, message, gfp_mask);
+	return cpc_hd_message_send(cpc_hd, cport_id, message, gfp_mask);
 }
 
 static void cpc_gb_message_cancel(struct gb_message *message)
@@ -27,12 +117,33 @@ static void cpc_gb_message_cancel(struct gb_message *message)
 	/* Not implemented */
 }
 
+static int cpc_gb_cport_allocate(struct gb_host_device *gb_hd, int cport_id, unsigned long flags)
+{
+	struct cpc_host_device *cpc_hd = gb_hd_to_cpc_hd(gb_hd);
+
+	return cpc_hd_cport_allocate(cpc_hd, cport_id, flags);
+}
+
+static void cpc_gb_cport_release(struct gb_host_device *gb_hd, u16 cport_id)
+{
+	struct cpc_host_device *cpc_hd = gb_hd_to_cpc_hd(gb_hd);
+
+	return cpc_hd_cport_release(cpc_hd, cport_id);
+}
+
 static struct gb_hd_driver cpc_gb_driver = {
 	.hd_priv_size = sizeof(struct cpc_host_device),
 	.message_send = cpc_gb_message_send,
 	.message_cancel = cpc_gb_message_cancel,
+	.cport_allocate = cpc_gb_cport_allocate,
+	.cport_release = cpc_gb_cport_release,
 };
 
+static void cpc_hd_init(struct cpc_host_device *cpc_hd)
+{
+	mutex_init(&cpc_hd->lock);
+}
+
 struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent)
 {
 	struct cpc_host_device *cpc_hd;
@@ -51,6 +162,8 @@ struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct devic
 	cpc_hd->gb_hd = hd;
 	cpc_hd->driver = driver;
 
+	cpc_hd_init(cpc_hd);
+
 	return cpc_hd;
 }
 EXPORT_SYMBOL_GPL(cpc_hd_create);
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
index f55feb303f4..c3f2f56a939 100644
--- a/drivers/greybus/cpc/host.h
+++ b/drivers/greybus/cpc/host.h
@@ -8,11 +8,13 @@
 
 #include <linux/device.h>
 #include <linux/greybus.h>
+#include <linux/mutex.h>
 #include <linux/types.h>
 
 #define GB_CPC_MSG_SIZE_MAX 4096
 #define GB_CPC_NUM_CPORTS 8
 
+struct cpc_cport;
 struct cpc_host_device;
 
 struct cpc_hd_driver {
@@ -25,12 +27,22 @@ struct cpc_hd_driver {
  * struct cpc_host_device - CPC host device.
  * @gb_hd: pointer to Greybus Host Device this device belongs to.
  * @driver: driver operations.
+ * @lock: mutex to synchronize access to cport array.
+ * @cports: array of cport pointers allocated by Greybus core.
  */
 struct cpc_host_device {
 	struct gb_host_device *gb_hd;
 	const struct cpc_hd_driver *driver;
+
+	struct mutex lock; /* Synchronize access to cports */
+	struct cpc_cport *cports[GB_CPC_NUM_CPORTS];
 };
 
+static inline struct device *cpc_hd_dev(struct cpc_host_device *cpc_hd)
+{
+	return &cpc_hd->gb_hd->dev;
+}
+
 struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent);
 int cpc_hd_add(struct cpc_host_device *cpc_hd);
 void cpc_hd_put(struct cpc_host_device *cpc_hd);
-- 
2.52.0

[PATCH v3 03/14] greybus: cpc: use socket buffers instead of gb_message in TX path

[Damien Riégel]

CPC comes with its own header, whose support is added in a future commit
of this series. Without skb, the CPC host device drivers have to get two
pointers to get a full packet: one pointer to the CPC header and one
pointer to the GB message.

In order to make implementations simpler, convert the GB message into an
SKB in the `message_send()` operation. That way, CPC internally works
only with SKBs, and only convert back to GB messages when notifying
Greybus core that a message has been transmitted or received.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
 drivers/greybus/cpc/cpc.h   | 11 +++++++++-
 drivers/greybus/cpc/cport.c | 11 ++++++++--
 drivers/greybus/cpc/host.c  | 41 ++++++++++++++++++++++++++++++++++---
 drivers/greybus/cpc/host.h  |  7 ++++---
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/drivers/greybus/cpc/cpc.h b/drivers/greybus/cpc/cpc.h
index 3915a7fbc4f..d9f8f60913a 100644
--- a/drivers/greybus/cpc/cpc.h
+++ b/drivers/greybus/cpc/cpc.h
@@ -24,6 +24,15 @@ struct cpc_cport {
 struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask);
 void cpc_cport_release(struct cpc_cport *cport);
 
-int cpc_cport_message_send(struct cpc_cport *cport, struct gb_message *message, gfp_t gfp_mask);
+int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb);
+
+struct cpc_skb_cb {
+	struct cpc_cport *cport;
+
+	/* Keep track of the GB message the skb originates from */
+	struct gb_message *gb_message;
+};
+
+#define CPC_SKB_CB(__skb) ((struct cpc_skb_cb *)&((__skb)->cb[0]))
 
 #endif
diff --git a/drivers/greybus/cpc/cport.c b/drivers/greybus/cpc/cport.c
index 88bdb2f8182..ed0b8e8b0d7 100644
--- a/drivers/greybus/cpc/cport.c
+++ b/drivers/greybus/cpc/cport.c
@@ -31,7 +31,14 @@ void cpc_cport_release(struct cpc_cport *cport)
 	kfree(cport);
 }
 
-int cpc_cport_message_send(struct cpc_cport *cport, struct gb_message *message, gfp_t gfp_mask)
+/**
+ * cpc_cport_transmit() - Transmit skb over cport.
+ * @cport: cport.
+ * @skb: skb to be transmitted.
+ */
+int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb)
 {
-	return cport->cpc_hd->driver->message_send(cport->cpc_hd, cport->id, message, gfp_mask);
+	struct cpc_host_device *cpc_hd = cport->cpc_hd;
+
+	return cpc_hd_send_skb(cpc_hd, skb);
 }
diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 3dda5b06590..a3acfc9bfca 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -6,6 +6,7 @@
 #include <linux/err.h>
 #include <linux/greybus.h>
 #include <linux/module.h>
+#include <linux/skbuff.h>
 
 #include "cpc.h"
 #include "host.h"
@@ -38,6 +39,8 @@ static int cpc_hd_message_send(struct cpc_host_device *cpc_hd, u16 cport_id,
 			       struct gb_message *message, gfp_t gfp_mask)
 {
 	struct cpc_cport *cport;
+	struct sk_buff *skb;
+	unsigned int size;
 
 	cport = cpc_hd_get_cport(cpc_hd, cport_id);
 	if (!cport) {
@@ -45,7 +48,18 @@ static int cpc_hd_message_send(struct cpc_host_device *cpc_hd, u16 cport_id,
 		return -EINVAL;
 	}
 
-	return cpc_cport_message_send(cport, message, gfp_mask);
+	size = sizeof(*message->header) + message->payload_size;
+	skb = alloc_skb(size, gfp_mask);
+	if (!skb)
+		return -ENOMEM;
+
+	/* Header and payload are already contiguous in Greybus message */
+	skb_put_data(skb, message->buffer, sizeof(*message->header) + message->payload_size);
+
+	CPC_SKB_CB(skb)->cport = cport;
+	CPC_SKB_CB(skb)->gb_message = message;
+
+	return cpc_cport_transmit(cport, skb);
 }
 
 static int cpc_hd_cport_allocate(struct cpc_host_device *cpc_hd, int cport_id, unsigned long flags)
@@ -149,8 +163,8 @@ struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct devic
 	struct cpc_host_device *cpc_hd;
 	struct gb_host_device *hd;
 
-	if ((!driver->message_send) || (!driver->message_cancel)) {
-		dev_err(parent, "missing mandatory callbacks\n");
+	if (!driver->transmit) {
+		dev_err(parent, "missing mandatory callback\n");
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -186,12 +200,33 @@ void cpc_hd_del(struct cpc_host_device *cpc_hd)
 }
 EXPORT_SYMBOL_GPL(cpc_hd_del);
 
+void cpc_hd_message_sent(struct sk_buff *skb, int status)
+{
+	struct cpc_host_device *cpc_hd = CPC_SKB_CB(skb)->cport->cpc_hd;
+	struct gb_host_device *hd = cpc_hd->gb_hd;
+
+	greybus_message_sent(hd, CPC_SKB_CB(skb)->gb_message, status);
+}
+EXPORT_SYMBOL_GPL(cpc_hd_message_sent);
+
 void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u16 cport_id, u8 *data, size_t length)
 {
 	greybus_data_rcvd(cpc_hd->gb_hd, cport_id, data, length);
 }
 EXPORT_SYMBOL_GPL(cpc_hd_rcvd);
 
+/**
+ * cpc_hd_send_skb() - Queue a socket buffer for transmission.
+ * @cpc_hd: Host device to send SKB over.
+ * @skb: SKB to send.
+ */
+int cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb)
+{
+	const struct cpc_hd_driver *drv = cpc_hd->driver;
+
+	return drv->transmit(cpc_hd, skb);
+}
+
 MODULE_DESCRIPTION("Greybus over CPC");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Silicon Laboratories, Inc.");
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
index c3f2f56a939..191b5e394a6 100644
--- a/drivers/greybus/cpc/host.h
+++ b/drivers/greybus/cpc/host.h
@@ -18,9 +18,7 @@ struct cpc_cport;
 struct cpc_host_device;
 
 struct cpc_hd_driver {
-	int (*message_send)(struct cpc_host_device *hd, u16 dest_cport_id,
-			    struct gb_message *message, gfp_t gfp_mask);
-	void (*message_cancel)(struct gb_message *message);
+	int (*transmit)(struct cpc_host_device *hd, struct sk_buff *skb);
 };
 
 /**
@@ -48,5 +46,8 @@ int cpc_hd_add(struct cpc_host_device *cpc_hd);
 void cpc_hd_put(struct cpc_host_device *cpc_hd);
 void cpc_hd_del(struct cpc_host_device *cpc_hd);
 void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u16 cport_id, u8 *data, size_t length);
+void cpc_hd_message_sent(struct sk_buff *skb, int status);
+
+int cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
 
 #endif
-- 
2.52.0

[PATCH v3 04/14] greybus: cpc: pack cport ID in Greybus header

[Damien Riégel]

Take advantage of the padding bytes present in the Greybus header to
store the CPort ID and minimize overhead. This technique is already used
by the es2 driver.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
Changes in v2:
  - clear pad bytes when unpacking CPort ID

 drivers/greybus/cpc/cpc.h   |  3 +++
 drivers/greybus/cpc/cport.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/greybus/cpc/host.c  | 13 ++++++++++++-
 drivers/greybus/cpc/host.h  |  2 +-
 4 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/greybus/cpc/cpc.h b/drivers/greybus/cpc/cpc.h
index d9f8f60913a..62597957814 100644
--- a/drivers/greybus/cpc/cpc.h
+++ b/drivers/greybus/cpc/cpc.h
@@ -24,6 +24,9 @@ struct cpc_cport {
 struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask);
 void cpc_cport_release(struct cpc_cport *cport);
 
+void cpc_cport_pack(struct gb_operation_msg_hdr *gb_hdr, u16 cport_id);
+u16 cpc_cport_unpack(struct gb_operation_msg_hdr *gb_hdr);
+
 int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb);
 
 struct cpc_skb_cb {
diff --git a/drivers/greybus/cpc/cport.c b/drivers/greybus/cpc/cport.c
index ed0b8e8b0d7..2c73d8e724e 100644
--- a/drivers/greybus/cpc/cport.c
+++ b/drivers/greybus/cpc/cport.c
@@ -3,6 +3,9 @@
  * Copyright (c) 2025, Silicon Laboratories, Inc.
  */
 
+#include <linux/unaligned.h>
+#include <linux/skbuff.h>
+
 #include "cpc.h"
 #include "host.h"
 
@@ -31,6 +34,32 @@ void cpc_cport_release(struct cpc_cport *cport)
 	kfree(cport);
 }
 
+/**
+ * cpc_cport_pack() - Pack CPort ID into Greybus Operation Message header.
+ * @gb_hdr: Greybus operation message header.
+ * @cport_id: CPort ID to pack.
+ */
+void cpc_cport_pack(struct gb_operation_msg_hdr *gb_hdr, u16 cport_id)
+{
+	put_unaligned_le16(cport_id, gb_hdr->pad);
+}
+
+/**
+ * cpc_cport_unpack() - Unpack CPort ID from Greybus Operation Message header.
+ * @gb_hdr: Greybus operation message header.
+ *
+ * Return: CPort ID packed in the header.
+ */
+u16 cpc_cport_unpack(struct gb_operation_msg_hdr *gb_hdr)
+{
+	u16 cport_id = get_unaligned_le16(gb_hdr->pad);
+
+	// Clear padding bytes
+	put_unaligned_le16(0, gb_hdr->pad);
+
+	return cport_id;
+}
+
 /**
  * cpc_cport_transmit() - Transmit skb over cport.
  * @cport: cport.
@@ -39,6 +68,11 @@ void cpc_cport_release(struct cpc_cport *cport)
 int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb)
 {
 	struct cpc_host_device *cpc_hd = cport->cpc_hd;
+	struct gb_operation_msg_hdr *gb_hdr;
+
+	/* Inject cport ID in Greybus header */
+	gb_hdr = (struct gb_operation_msg_hdr *)skb->data;
+	cpc_cport_pack(gb_hdr, cport->id);
 
 	return cpc_hd_send_skb(cpc_hd, skb);
 }
diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index a3acfc9bfca..10b0529dc4e 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -209,8 +209,19 @@ void cpc_hd_message_sent(struct sk_buff *skb, int status)
 }
 EXPORT_SYMBOL_GPL(cpc_hd_message_sent);
 
-void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u16 cport_id, u8 *data, size_t length)
+void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u8 *data, size_t length)
 {
+	struct gb_operation_msg_hdr *gb_hdr;
+	u16 cport_id;
+
+	/* Prevent an out-of-bound access if called with non-sensical parameters. */
+	if (!data || length < sizeof(*gb_hdr))
+		return;
+
+	/* Retrieve cport ID that was packed in Greybus header */
+	gb_hdr = (struct gb_operation_msg_hdr *)data;
+	cport_id = cpc_cport_unpack(gb_hdr);
+
 	greybus_data_rcvd(cpc_hd->gb_hd, cport_id, data, length);
 }
 EXPORT_SYMBOL_GPL(cpc_hd_rcvd);
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
index 191b5e394a6..2e568bac44e 100644
--- a/drivers/greybus/cpc/host.h
+++ b/drivers/greybus/cpc/host.h
@@ -45,7 +45,7 @@ struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct devic
 int cpc_hd_add(struct cpc_host_device *cpc_hd);
 void cpc_hd_put(struct cpc_host_device *cpc_hd);
 void cpc_hd_del(struct cpc_host_device *cpc_hd);
-void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u16 cport_id, u8 *data, size_t length);
+void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u8 *data, size_t length);
 void cpc_hd_message_sent(struct sk_buff *skb, int status);
 
 int cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
-- 
2.52.0

[PATCH v3 05/14] greybus: cpc: switch RX path to socket buffers

[Damien Riégel]

For symmetry, also convert the RX path to use socket buffers instead of
u8* buffers. Before this commit CPC host device drivers were responsible
for allocating and freeing the buffers.

Now they are only responsible for allocating the SKBs and pass it to the
upper layer, the CPC "core" module will take of converting the SKBs into
a buffer that can be consumed by Greybus' core and releasing the SKBs.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
 drivers/greybus/cpc/host.c | 13 ++++++++-----
 drivers/greybus/cpc/host.h |  2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 10b0529dc4e..66c4d7fd0b8 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -209,20 +209,23 @@ void cpc_hd_message_sent(struct sk_buff *skb, int status)
 }
 EXPORT_SYMBOL_GPL(cpc_hd_message_sent);
 
-void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u8 *data, size_t length)
+void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, struct sk_buff *skb)
 {
 	struct gb_operation_msg_hdr *gb_hdr;
 	u16 cport_id;
 
 	/* Prevent an out-of-bound access if called with non-sensical parameters. */
-	if (!data || length < sizeof(*gb_hdr))
-		return;
+	if (skb->len < sizeof(*gb_hdr))
+		goto free_skb;
 
 	/* Retrieve cport ID that was packed in Greybus header */
-	gb_hdr = (struct gb_operation_msg_hdr *)data;
+	gb_hdr = (struct gb_operation_msg_hdr *)skb->data;
 	cport_id = cpc_cport_unpack(gb_hdr);
 
-	greybus_data_rcvd(cpc_hd->gb_hd, cport_id, data, length);
+	greybus_data_rcvd(cpc_hd->gb_hd, cport_id, skb->data, skb->len);
+
+free_skb:
+	kfree_skb(skb);
 }
 EXPORT_SYMBOL_GPL(cpc_hd_rcvd);
 
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
index 2e568bac44e..cc835f5298b 100644
--- a/drivers/greybus/cpc/host.h
+++ b/drivers/greybus/cpc/host.h
@@ -45,7 +45,7 @@ struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct devic
 int cpc_hd_add(struct cpc_host_device *cpc_hd);
 void cpc_hd_put(struct cpc_host_device *cpc_hd);
 void cpc_hd_del(struct cpc_host_device *cpc_hd);
-void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, u8 *data, size_t length);
+void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
 void cpc_hd_message_sent(struct sk_buff *skb, int status);
 
 int cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
-- 
2.52.0

[PATCH v3 06/14] greybus: cpc: introduce CPC header structure

[Damien Riégel]

CPC main features are reliable transmission and remote's receive window
management. To implement these features, an additional header is needed.
This header is prepended to all Greybus messages before going on the
wire.

Reliable transmission: to make transmission reliable, messages are
sequenced and acknowledged. That constitutes two bytes of the header,
one for the sequence number, one for the acknowledgment number. If a
message is not acked in a timely manner, a retransmission mechanism will
attempt another transmission. That mechanism will be implemented in a
future patch set.

Remote's receive window: the remote advertises the number of reception
buffers that are available on this cport. The other peer must take care
of not sending more messages than advertised by the remote. This is a
sort of flow control. That accounts for one byte in the header.

The remaining byte carries some flags. For instance, there is a flag to
indicate if it's a CPC message or a Greybus message.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
Changes in v3:
  - remove CPC_HEADER_SIZE and GREYBUS_HEADER_SIZE macros

Changes in v2:
  - Minor tweaks to structure documentation

 drivers/greybus/cpc/header.h | 41 ++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 drivers/greybus/cpc/header.h

diff --git a/drivers/greybus/cpc/header.h b/drivers/greybus/cpc/header.h
new file mode 100644
index 00000000000..f65a608a650
--- /dev/null
+++ b/drivers/greybus/cpc/header.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_HEADER_H
+#define __CPC_HEADER_H
+
+#include <linux/greybus.h>
+#include <linux/types.h>
+
+#define CPC_HEADER_MAX_RX_WINDOW U8_MAX
+
+/**
+ * struct cpc header - Representation of CPC header.
+ * @ctrl_flags: contains the type of frame and other control flags.
+ * @recv_wnd: number of buffers that the cport can receive without blocking.
+ * @seq: sequence number.
+ * @ack: acknowledge number, indicate to the remote the next sequence number
+ *	 this peer expects to see.
+ *
+ * Each peer can confirm reception of frames by setting the acknowledgment number to the next frame
+ * it expects to see, i.e. setting the ack number to X effectively acknowledges frames with sequence
+ * number up to X-1.
+ *
+ * CPC is designed around the concept that each cport has its pool of reception buffers. The number
+ * of buffers in a pool is advertised to the remote via the @recv_wnd attribute. This acts as
+ * software flow-control, and a peer shall not send frames to a remote if the @recv_wnd is zero.
+ *
+ * The eighth-bit (0x80) of the control byte indicates if the frame targets CPC or Greybus. If the
+ * bit is set, the frame should be interpreted as a CPC control frame. For simplicity, control
+ * frames have the same encoding as Greybus frames.
+ */
+struct cpc_header {
+	__u8 ctrl_flags;
+	__u8 recv_wnd;
+	__u8 seq;
+	__u8 ack;
+} __packed;
+
+#endif
-- 
2.52.0

[PATCH v3 07/14] greybus: cpc: account for CPC header size in RX and TX path

[Damien Riégel]

Account that a CPC header is prepended to every frame in the RX and TX
path. For now, nothing is done with that headroom but at least bytes are
reserved for it.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
 drivers/greybus/cpc/host.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 66c4d7fd0b8..759322759bd 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -9,6 +9,7 @@
 #include <linux/skbuff.h>
 
 #include "cpc.h"
+#include "header.h"
 #include "host.h"
 
 static struct cpc_host_device *gb_hd_to_cpc_hd(struct gb_host_device *hd)
@@ -48,11 +49,13 @@ static int cpc_hd_message_send(struct cpc_host_device *cpc_hd, u16 cport_id,
 		return -EINVAL;
 	}
 
-	size = sizeof(*message->header) + message->payload_size;
+	size = sizeof(struct cpc_header) + sizeof(*message->header) + message->payload_size;
 	skb = alloc_skb(size, gfp_mask);
 	if (!skb)
 		return -ENOMEM;
 
+	skb_reserve(skb, sizeof(struct cpc_header));
+
 	/* Header and payload are already contiguous in Greybus message */
 	skb_put_data(skb, message->buffer, sizeof(*message->header) + message->payload_size);
 
@@ -215,9 +218,11 @@ void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, struct sk_buff *skb)
 	u16 cport_id;
 
 	/* Prevent an out-of-bound access if called with non-sensical parameters. */
-	if (skb->len < sizeof(*gb_hdr))
+	if (skb->len < (sizeof(*gb_hdr) + sizeof(struct cpc_header)))
 		goto free_skb;
 
+	skb_pull(skb, sizeof(struct cpc_header));
+
 	/* Retrieve cport ID that was packed in Greybus header */
 	gb_hdr = (struct gb_operation_msg_hdr *)skb->data;
 	cport_id = cpc_cport_unpack(gb_hdr);
-- 
2.52.0

[PATCH v3 08/14] greybus: cpc: add and validate sequence numbers

[Damien Riégel]

The first step in making the CPC header actually do something is to add
the sequence number to outgoing messages and validate that incoming
frames are received in order.

At this stage, the driver doesn't send standalone acks, so if a message
with Sequence X is received, the remote will not be acknowledged until a
message targeting that CPort comes from the Greybus layer. Only then the
driver will ack with acknowledgedment number of X + 1.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
 drivers/greybus/cpc/Makefile   |  2 +-
 drivers/greybus/cpc/cpc.h      | 20 +++++++++++++++
 drivers/greybus/cpc/cport.c    | 25 ++++++++++++++++++
 drivers/greybus/cpc/header.c   | 17 ++++++++++++
 drivers/greybus/cpc/header.h   |  2 ++
 drivers/greybus/cpc/host.c     | 13 +++++++---
 drivers/greybus/cpc/protocol.c | 47 ++++++++++++++++++++++++++++++++++
 7 files changed, 121 insertions(+), 5 deletions(-)
 create mode 100644 drivers/greybus/cpc/header.c
 create mode 100644 drivers/greybus/cpc/protocol.c

diff --git a/drivers/greybus/cpc/Makefile b/drivers/greybus/cpc/Makefile
index 3d50f8c5473..c4b530d27a3 100644
--- a/drivers/greybus/cpc/Makefile
+++ b/drivers/greybus/cpc/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-gb-cpc-y := cport.o host.o
+gb-cpc-y := cport.o header.o host.o protocol.o
 
 # CPC core
 obj-$(CONFIG_GREYBUS_CPC)	+= gb-cpc.o
diff --git a/drivers/greybus/cpc/cpc.h b/drivers/greybus/cpc/cpc.h
index 62597957814..87b54a4fd34 100644
--- a/drivers/greybus/cpc/cpc.h
+++ b/drivers/greybus/cpc/cpc.h
@@ -8,17 +8,32 @@
 
 #include <linux/device.h>
 #include <linux/greybus.h>
+#include <linux/mutex.h>
 #include <linux/types.h>
 
+struct sk_buff;
+
 /**
  * struct cpc_cport - CPC cport
  * @id: cport ID
  * @cpc_hd: pointer to the CPC host device this cport belongs to
+ * @lock: mutex to synchronize accesses to tcb and other attributes
+ * @tcb: Transmission Control Block
  */
 struct cpc_cport {
 	u16 id;
 
 	struct cpc_host_device *cpc_hd;
+	struct mutex lock; /* Synchronize access to state variables */
+
+	/*
+	 * @ack: current acknowledge number
+	 * @seq: current sequence number
+	 */
+	struct {
+		u8 ack;
+		u8 seq;
+	} tcb;
 };
 
 struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask);
@@ -34,8 +49,13 @@ struct cpc_skb_cb {
 
 	/* Keep track of the GB message the skb originates from */
 	struct gb_message *gb_message;
+
+	u8 seq;
 };
 
 #define CPC_SKB_CB(__skb) ((struct cpc_skb_cb *)&((__skb)->cb[0]))
 
+void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack);
+void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb);
+
 #endif
diff --git a/drivers/greybus/cpc/cport.c b/drivers/greybus/cpc/cport.c
index 2c73d8e724e..7041a6a8a36 100644
--- a/drivers/greybus/cpc/cport.c
+++ b/drivers/greybus/cpc/cport.c
@@ -9,6 +9,16 @@
 #include "cpc.h"
 #include "host.h"
 
+/**
+ * cpc_cport_tcb_reset() - Reset cport's TCB to initial values.
+ * @cport: cport pointer
+ */
+static void cpc_cport_tcb_reset(struct cpc_cport *cport)
+{
+	cport->tcb.ack = 0;
+	cport->tcb.seq = 0;
+}
+
 /**
  * cpc_cport_alloc() - Allocate and initialize CPC cport.
  * @cport_id: cport ID.
@@ -25,6 +35,9 @@ struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask)
 		return NULL;
 
 	cport->id = cport_id;
+	cpc_cport_tcb_reset(cport);
+
+	mutex_init(&cport->lock);
 
 	return cport;
 }
@@ -69,10 +82,22 @@ int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb)
 {
 	struct cpc_host_device *cpc_hd = cport->cpc_hd;
 	struct gb_operation_msg_hdr *gb_hdr;
+	u8 ack;
 
 	/* Inject cport ID in Greybus header */
 	gb_hdr = (struct gb_operation_msg_hdr *)skb->data;
 	cpc_cport_pack(gb_hdr, cport->id);
 
+	mutex_lock(&cport->lock);
+
+	CPC_SKB_CB(skb)->seq = cport->tcb.seq;
+
+	cport->tcb.seq++;
+	ack = cport->tcb.ack;
+
+	mutex_unlock(&cport->lock);
+
+	cpc_protocol_prepare_header(skb, ack);
+
 	return cpc_hd_send_skb(cpc_hd, skb);
 }
diff --git a/drivers/greybus/cpc/header.c b/drivers/greybus/cpc/header.c
new file mode 100644
index 00000000000..62946d6077e
--- /dev/null
+++ b/drivers/greybus/cpc/header.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include "header.h"
+
+/**
+ * cpc_header_get_seq() - Get the sequence number.
+ * @hdr: CPC header.
+ *
+ * Return: Sequence number.
+ */
+u8 cpc_header_get_seq(const struct cpc_header *hdr)
+{
+	return hdr->seq;
+}
diff --git a/drivers/greybus/cpc/header.h b/drivers/greybus/cpc/header.h
index f65a608a650..5196422380e 100644
--- a/drivers/greybus/cpc/header.h
+++ b/drivers/greybus/cpc/header.h
@@ -38,4 +38,6 @@ struct cpc_header {
 	__u8 ack;
 } __packed;
 
+u8 cpc_header_get_seq(const struct cpc_header *hdr);
+
 #endif
diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 759322759bd..2c1b5d02ec2 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -215,19 +215,24 @@ EXPORT_SYMBOL_GPL(cpc_hd_message_sent);
 void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, struct sk_buff *skb)
 {
 	struct gb_operation_msg_hdr *gb_hdr;
+	struct cpc_cport *cport;
 	u16 cport_id;
 
 	/* Prevent an out-of-bound access if called with non-sensical parameters. */
 	if (skb->len < (sizeof(*gb_hdr) + sizeof(struct cpc_header)))
 		goto free_skb;
 
-	skb_pull(skb, sizeof(struct cpc_header));
-
 	/* Retrieve cport ID that was packed in Greybus header */
-	gb_hdr = (struct gb_operation_msg_hdr *)skb->data;
+	gb_hdr = (struct gb_operation_msg_hdr *)(skb->data + sizeof(struct cpc_header));
 	cport_id = cpc_cport_unpack(gb_hdr);
 
-	greybus_data_rcvd(cpc_hd->gb_hd, cport_id, skb->data, skb->len);
+	cport = cpc_hd_get_cport(cpc_hd, cport_id);
+	if (!cport) {
+		dev_warn(cpc_hd_dev(cpc_hd), "cport %u not allocated\n", cport_id);
+		goto free_skb;
+	}
+
+	cpc_protocol_on_data(cport, skb);
 
 free_skb:
 	kfree_skb(skb);
diff --git a/drivers/greybus/cpc/protocol.c b/drivers/greybus/cpc/protocol.c
new file mode 100644
index 00000000000..8f0ac6dfa11
--- /dev/null
+++ b/drivers/greybus/cpc/protocol.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/skbuff.h>
+
+#include "cpc.h"
+#include "header.h"
+#include "host.h"
+
+void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack)
+{
+	struct cpc_header *hdr;
+
+	skb_push(skb, sizeof(*hdr));
+
+	hdr = (struct cpc_header *)skb->data;
+	memset(hdr, 0, sizeof(*hdr));
+
+	hdr->ack = ack;
+	hdr->seq = CPC_SKB_CB(skb)->seq;
+}
+
+void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb)
+{
+	struct cpc_header *cpc_hdr = (struct cpc_header *)skb->data;
+	u8 seq = cpc_header_get_seq(cpc_hdr);
+	bool expected_seq = false;
+
+	mutex_lock(&cport->lock);
+
+	expected_seq = seq == cport->tcb.ack;
+	if (expected_seq)
+		cport->tcb.ack++;
+	else
+		dev_warn_ratelimited(cpc_hd_dev(cport->cpc_hd),
+				     "unexpected seq: %u, expected seq: %u\n", seq, cport->tcb.ack);
+
+	mutex_unlock(&cport->lock);
+
+	if (expected_seq) {
+		skb_pull(skb, sizeof(*cpc_hdr));
+
+		greybus_data_rcvd(cport->cpc_hd->gb_hd, cport->id, skb->data, skb->len);
+	}
+}
-- 
2.52.0

[PATCH v3 09/14] greybus: cpc: acknowledge all incoming messages

[Damien Riégel]

Currently, CPC doesn't send messages on its own, it only prepends its
header to outgoing messages. This can lead to messages not being
acknowledged, for instance in the case of an SVC Ping

	Host				Device

  SVC Ping (seq=X, ack=Y)
				  SVC Ping Reply (seq=Y, ack=X+1)

The "Ping Reply" is never acknowledged at the CPC level, which can lead
to retransmissions, or worst the device might think the link is broken
and attempt to recover.

To prevent that scenario, an ack mechanism is implemented in the most
straightforward manner: send an ACK to all incoming messages. Here, two
flags need to be added:
 - First, a flag is needed to differentiate between pure CPC frames,
   that are only meaningful at the CPC level, and regular Greybus
   operations. This flag is called "CONTROL". Currently there is only
   one type of control frame, the standalone ack. Control messages have
   the same format as Greybus operations.
 - Second, ack themselves should not be acked, so to determine if a
   message should be acked or not, a REQUEST_ACK flag is added.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
Changes in v2:
  - add missing cpu_to_le16 conversion when setting message size

 drivers/greybus/cpc/cpc.h      |  3 ++
 drivers/greybus/cpc/cport.c    |  1 +
 drivers/greybus/cpc/header.c   | 41 +++++++++++++++++++++++++
 drivers/greybus/cpc/header.h   |  3 ++
 drivers/greybus/cpc/protocol.c | 55 +++++++++++++++++++++++++++++-----
 5 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/drivers/greybus/cpc/cpc.h b/drivers/greybus/cpc/cpc.h
index 87b54a4fd34..725fd7f4afc 100644
--- a/drivers/greybus/cpc/cpc.h
+++ b/drivers/greybus/cpc/cpc.h
@@ -51,6 +51,9 @@ struct cpc_skb_cb {
 	struct gb_message *gb_message;
 
 	u8 seq;
+
+#define CPC_SKB_FLAG_REQ_ACK (1 << 0)
+	u8 cpc_flags;
 };
 
 #define CPC_SKB_CB(__skb) ((struct cpc_skb_cb *)&((__skb)->cb[0]))
diff --git a/drivers/greybus/cpc/cport.c b/drivers/greybus/cpc/cport.c
index 7041a6a8a36..847cc8ebe41 100644
--- a/drivers/greybus/cpc/cport.c
+++ b/drivers/greybus/cpc/cport.c
@@ -91,6 +91,7 @@ int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb)
 	mutex_lock(&cport->lock);
 
 	CPC_SKB_CB(skb)->seq = cport->tcb.seq;
+	CPC_SKB_CB(skb)->cpc_flags = CPC_SKB_FLAG_REQ_ACK;
 
 	cport->tcb.seq++;
 	ack = cport->tcb.ack;
diff --git a/drivers/greybus/cpc/header.c b/drivers/greybus/cpc/header.c
index 62946d6077e..8875a6fed26 100644
--- a/drivers/greybus/cpc/header.c
+++ b/drivers/greybus/cpc/header.c
@@ -3,8 +3,25 @@
  * Copyright (c) 2025, Silicon Laboratories, Inc.
  */
 
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+
 #include "header.h"
 
+#define CPC_HEADER_CONTROL_IS_CONTROL_MASK BIT(7)
+#define CPC_HEADER_CONTROL_REQ_ACK_MASK BIT(6)
+
+/**
+ * cpc_header_is_control() - Identify if this is a control frame.
+ * @hdr: CPC header.
+ *
+ * Return: True if this is a control frame, false if this a Greybus frame.
+ */
+bool cpc_header_is_control(const struct cpc_header *hdr)
+{
+	return hdr->ctrl_flags & CPC_HEADER_CONTROL_IS_CONTROL_MASK;
+}
+
 /**
  * cpc_header_get_seq() - Get the sequence number.
  * @hdr: CPC header.
@@ -15,3 +32,27 @@ u8 cpc_header_get_seq(const struct cpc_header *hdr)
 {
 	return hdr->seq;
 }
+
+/**
+ * cpc_header_get_req_ack() - Get the request acknowledge frame flag.
+ * @hdr: CPC header.
+ *
+ * Return: Request acknowledge frame flag.
+ */
+bool cpc_header_get_req_ack(const struct cpc_header *hdr)
+{
+	return FIELD_GET(CPC_HEADER_CONTROL_REQ_ACK_MASK, hdr->ctrl_flags);
+}
+
+/**
+ * cpc_header_encode_ctrl_flags() - Encode parameters into the control byte.
+ * @control: True if CPC control frame, false if Greybus frame.
+ * @req_ack: Frame flag indicating a request to be acknowledged.
+ *
+ * Return: Encoded control byte.
+ */
+u8 cpc_header_encode_ctrl_flags(bool control, bool req_ack)
+{
+	return FIELD_PREP(CPC_HEADER_CONTROL_IS_CONTROL_MASK, control) |
+	       FIELD_PREP(CPC_HEADER_CONTROL_REQ_ACK_MASK, req_ack);
+}
diff --git a/drivers/greybus/cpc/header.h b/drivers/greybus/cpc/header.h
index 5196422380e..19612012b19 100644
--- a/drivers/greybus/cpc/header.h
+++ b/drivers/greybus/cpc/header.h
@@ -38,6 +38,9 @@ struct cpc_header {
 	__u8 ack;
 } __packed;
 
+bool cpc_header_is_control(const struct cpc_header *hdr);
 u8 cpc_header_get_seq(const struct cpc_header *hdr);
+bool cpc_header_get_req_ack(const struct cpc_header *hdr);
+u8 cpc_header_encode_ctrl_flags(bool control, bool req_ack);
 
 #endif
diff --git a/drivers/greybus/cpc/protocol.c b/drivers/greybus/cpc/protocol.c
index 8f0ac6dfa11..97db70a53b0 100644
--- a/drivers/greybus/cpc/protocol.c
+++ b/drivers/greybus/cpc/protocol.c
@@ -9,6 +9,11 @@
 #include "header.h"
 #include "host.h"
 
+static bool cpc_skb_is_sequenced(struct sk_buff *skb)
+{
+	return CPC_SKB_CB(skb)->cpc_flags & CPC_SKB_FLAG_REQ_ACK;
+}
+
 void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack)
 {
 	struct cpc_header *hdr;
@@ -20,26 +25,62 @@ void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack)
 
 	hdr->ack = ack;
 	hdr->seq = CPC_SKB_CB(skb)->seq;
+	hdr->ctrl_flags = cpc_header_encode_ctrl_flags(!CPC_SKB_CB(skb)->gb_message,
+						       cpc_skb_is_sequenced(skb));
+}
+
+static void cpc_protocol_queue_ack(struct cpc_cport *cport, u8 ack)
+{
+	struct gb_operation_msg_hdr *gb_hdr;
+	struct sk_buff *skb;
+
+	skb = alloc_skb(sizeof(struct cpc_header) + sizeof(*gb_hdr), GFP_KERNEL);
+	if (!skb)
+		return;
+
+	skb_reserve(skb, sizeof(struct cpc_header));
+
+	gb_hdr = skb_put(skb, sizeof(*gb_hdr));
+	memset(gb_hdr, 0, sizeof(*gb_hdr));
+
+	/* In the CPC Operation Header, only the size and cport_id matter for ACKs. */
+	gb_hdr->size = cpu_to_le16(sizeof(*gb_hdr));
+	cpc_cport_pack(gb_hdr, cport->id);
+
+	cpc_protocol_prepare_header(skb, ack);
+
+	cpc_hd_send_skb(cport->cpc_hd, skb);
 }
 
 void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb)
 {
 	struct cpc_header *cpc_hdr = (struct cpc_header *)skb->data;
+	bool require_ack = cpc_header_get_req_ack(cpc_hdr);
 	u8 seq = cpc_header_get_seq(cpc_hdr);
 	bool expected_seq = false;
+	u8 ack;
 
 	mutex_lock(&cport->lock);
 
-	expected_seq = seq == cport->tcb.ack;
-	if (expected_seq)
-		cport->tcb.ack++;
-	else
-		dev_warn_ratelimited(cpc_hd_dev(cport->cpc_hd),
-				     "unexpected seq: %u, expected seq: %u\n", seq, cport->tcb.ack);
+	if (require_ack) {
+		expected_seq = seq == cport->tcb.ack;
+		if (expected_seq)
+			cport->tcb.ack++;
+		else
+			dev_warn_ratelimited(cpc_hd_dev(cport->cpc_hd),
+					     "unexpected seq: %u, expected seq: %u\n",
+					     seq, cport->tcb.ack);
+	}
+
+	ack = cport->tcb.ack;
 
 	mutex_unlock(&cport->lock);
 
-	if (expected_seq) {
+	/* Ack no matter if the sequence was valid or not, to resync with remote */
+	if (require_ack)
+		cpc_protocol_queue_ack(cport, ack);
+
+	if (expected_seq && !cpc_header_is_control(cpc_hdr)) {
 		skb_pull(skb, sizeof(*cpc_hdr));
 
 		greybus_data_rcvd(cport->cpc_hd->gb_hd, cport->id, skb->data, skb->len);
-- 
2.52.0

[PATCH v3 10/14] greybus: cpc: use holding queue instead of sending out immediately

[Damien Riégel]

A feature of CPC is the RX window. This value, advertised in every
packet, indicates to the remote peer how many reception buffers are
available for that CPort. The remote peer must not send more messages
than advertised, as these extra messages are almost guaranteed to be
dropped by the receiver.  This is a bit similar to TCP's receive window,
but expressed in a number of messages instead of bytes.

As the first step to handle remote's RX window, store the skb in a
sk_buff_head list, instead of sending a message immediately when pushed
by Greybus. skbs are still sent out straight away for now, but a future
commit will make sure they stay in the holding queue instead of being
sent out immediately if the remote's RX window is too small.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
 drivers/greybus/cpc/cpc.h      | 10 ++++++----
 drivers/greybus/cpc/cport.c    | 21 ++++++++++++---------
 drivers/greybus/cpc/host.c     |  4 +++-
 drivers/greybus/cpc/protocol.c | 25 ++++++++++++++++++++++++-
 4 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/drivers/greybus/cpc/cpc.h b/drivers/greybus/cpc/cpc.h
index 725fd7f4afc..f1669585c45 100644
--- a/drivers/greybus/cpc/cpc.h
+++ b/drivers/greybus/cpc/cpc.h
@@ -9,15 +9,15 @@
 #include <linux/device.h>
 #include <linux/greybus.h>
 #include <linux/mutex.h>
+#include <linux/skbuff.h>
 #include <linux/types.h>
 
-struct sk_buff;
-
 /**
  * struct cpc_cport - CPC cport
  * @id: cport ID
  * @cpc_hd: pointer to the CPC host device this cport belongs to
  * @lock: mutex to synchronize accesses to tcb and other attributes
+ * @holding_queue: list of frames queued to be sent
  * @tcb: Transmission Control Block
  */
 struct cpc_cport {
@@ -26,6 +26,8 @@ struct cpc_cport {
 	struct cpc_host_device *cpc_hd;
 	struct mutex lock; /* Synchronize access to state variables */
 
+	struct sk_buff_head holding_queue;
+
 	/*
 	 * @ack: current acknowledge number
 	 * @seq: current sequence number
@@ -42,7 +44,7 @@ void cpc_cport_release(struct cpc_cport *cport);
 void cpc_cport_pack(struct gb_operation_msg_hdr *gb_hdr, u16 cport_id);
 u16 cpc_cport_unpack(struct gb_operation_msg_hdr *gb_hdr);
 
-int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb);
+void cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb);
 
 struct cpc_skb_cb {
 	struct cpc_cport *cport;
@@ -58,7 +60,7 @@ struct cpc_skb_cb {
 
 #define CPC_SKB_CB(__skb) ((struct cpc_skb_cb *)&((__skb)->cb[0]))
 
-void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack);
 void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb);
+void __cpc_protocol_write_head(struct cpc_cport *cport);
 
 #endif
diff --git a/drivers/greybus/cpc/cport.c b/drivers/greybus/cpc/cport.c
index 847cc8ebe41..91c39856e22 100644
--- a/drivers/greybus/cpc/cport.c
+++ b/drivers/greybus/cpc/cport.c
@@ -7,7 +7,6 @@
 #include <linux/skbuff.h>
 
 #include "cpc.h"
-#include "host.h"
 
 /**
  * cpc_cport_tcb_reset() - Reset cport's TCB to initial values.
@@ -38,15 +37,23 @@ struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask)
 	cpc_cport_tcb_reset(cport);
 
 	mutex_init(&cport->lock);
+	skb_queue_head_init(&cport->holding_queue);
 
 	return cport;
 }
 
 void cpc_cport_release(struct cpc_cport *cport)
 {
+	skb_queue_purge(&cport->holding_queue);
 	kfree(cport);
 }
 
+static void cpc_cport_queue_skb(struct cpc_cport *cport, struct sk_buff *skb)
+{
+	__skb_header_release(skb);
+	__skb_queue_tail(&cport->holding_queue, skb);
+}
+
 /**
  * cpc_cport_pack() - Pack CPort ID into Greybus Operation Message header.
  * @gb_hdr: Greybus operation message header.
@@ -78,11 +85,9 @@ u16 cpc_cport_unpack(struct gb_operation_msg_hdr *gb_hdr)
  * @cport: cport.
  * @skb: skb to be transmitted.
  */
-int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb)
+void cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb)
 {
-	struct cpc_host_device *cpc_hd = cport->cpc_hd;
 	struct gb_operation_msg_hdr *gb_hdr;
-	u8 ack;
 
 	/* Inject cport ID in Greybus header */
 	gb_hdr = (struct gb_operation_msg_hdr *)skb->data;
@@ -94,11 +99,9 @@ int cpc_cport_transmit(struct cpc_cport *cport, struct sk_buff *skb)
 	CPC_SKB_CB(skb)->cpc_flags = CPC_SKB_FLAG_REQ_ACK;
 
 	cport->tcb.seq++;
-	ack = cport->tcb.ack;
+
+	cpc_cport_queue_skb(cport, skb);
+	__cpc_protocol_write_head(cport);
 
 	mutex_unlock(&cport->lock);
-
-	cpc_protocol_prepare_header(skb, ack);
-
-	return cpc_hd_send_skb(cpc_hd, skb);
 }
diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 2c1b5d02ec2..225f9342cd2 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -62,7 +62,9 @@ static int cpc_hd_message_send(struct cpc_host_device *cpc_hd, u16 cport_id,
 	CPC_SKB_CB(skb)->cport = cport;
 	CPC_SKB_CB(skb)->gb_message = message;
 
-	return cpc_cport_transmit(cport, skb);
+	cpc_cport_transmit(cport, skb);
+
+	return 0;
 }
 
 static int cpc_hd_cport_allocate(struct cpc_host_device *cpc_hd, int cport_id, unsigned long flags)
diff --git a/drivers/greybus/cpc/protocol.c b/drivers/greybus/cpc/protocol.c
index 97db70a53b0..4cda71994d8 100644
--- a/drivers/greybus/cpc/protocol.c
+++ b/drivers/greybus/cpc/protocol.c
@@ -14,7 +14,7 @@ static bool cpc_skb_is_sequenced(struct sk_buff *skb)
 	return CPC_SKB_CB(skb)->cpc_flags & CPC_SKB_FLAG_REQ_ACK;
 }
 
-void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack)
+static void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack)
 {
 	struct cpc_header *hdr;
 
@@ -86,3 +86,26 @@ void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb)
 		greybus_data_rcvd(cport->cpc_hd->gb_hd, cport->id, skb->data, skb->len);
 	}
 }
+
+static void __cpc_protocol_write_skb(struct cpc_cport *cport, struct sk_buff *skb, u8 ack)
+{
+	cpc_protocol_prepare_header(skb, ack);
+
+	cpc_hd_send_skb(cport->cpc_hd, skb);
+}
+
+/* Write skbs at the head of holding queue */
+void __cpc_protocol_write_head(struct cpc_cport *cport)
+{
+	struct sk_buff *skb;
+	u8 ack;
+
+	ack = cport->tcb.ack;
+
+	/* For each SKB in the holding queue, clone it and pass it to lower layer */
+	while ((skb = skb_peek(&cport->holding_queue))) {
+		skb_unlink(skb, &cport->holding_queue);
+
+		__cpc_protocol_write_skb(cport, skb, ack);
+	}
+}
-- 
2.52.0

[PATCH v3 11/14] greybus: cpc: honour remote's RX window

[Damien Riégel]

The RX window indicates how many reception buffers the peer has
available for that cport. The other peer must not send more messages
than that window, or the chances of those messages being lost is very
high, leading to retransmissions and poor performance.

The RX window is associated with the ack number, and indicates the valid
range of sequence number the other peer can use:

	Ack		RX window		Valid Sequence Range

	X		0			None
	X		1			X
	X		2			X -> X+1

So everytime an ack is received, the driver evaluates if the valid
sequence range has changed and if so pops message from its holding queue.

SKBs used to be passed directly to the driver for transmission. Now SBKs
still need to be passed to driver for transmission, but CPC also needs
to keep track of them, to be able to trigger a retransmission if they
are not acked in a timely manner. As SKBs cannot be in two lists at the
same time, drivers now get cloned SKBs.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
Changes in v2:
  - fix documentation of cpc_header_number_in_window which documented a
    unexisting parameter `end`
  - simplify implementation of cpc_header_get_frames_acked_count

 drivers/greybus/cpc/cpc.h      |  9 ++++
 drivers/greybus/cpc/cport.c    |  5 +++
 drivers/greybus/cpc/header.c   | 78 ++++++++++++++++++++++++++++++++++
 drivers/greybus/cpc/header.h   |  6 +++
 drivers/greybus/cpc/host.c     |  9 ----
 drivers/greybus/cpc/host.h     |  1 -
 drivers/greybus/cpc/protocol.c | 71 ++++++++++++++++++++++++++++---
 7 files changed, 163 insertions(+), 16 deletions(-)

diff --git a/drivers/greybus/cpc/cpc.h b/drivers/greybus/cpc/cpc.h
index f1669585c45..e8cbe916630 100644
--- a/drivers/greybus/cpc/cpc.h
+++ b/drivers/greybus/cpc/cpc.h
@@ -18,6 +18,7 @@
  * @cpc_hd: pointer to the CPC host device this cport belongs to
  * @lock: mutex to synchronize accesses to tcb and other attributes
  * @holding_queue: list of frames queued to be sent
+ * @retx_queue: list of frames sent and waiting for acknowledgment
  * @tcb: Transmission Control Block
  */
 struct cpc_cport {
@@ -27,12 +28,20 @@ struct cpc_cport {
 	struct mutex lock; /* Synchronize access to state variables */
 
 	struct sk_buff_head holding_queue;
+	struct sk_buff_head retx_queue;
 
 	/*
+	 * @send_wnd: send window, maximum number of frames that the remote can accept.
+	 *            TX frames should have a sequence in the range [send_una; send_una + send_wnd]
+	 * @send_nxt: send next, the next sequence number that will be used for transmission
+	 * @send_una: send unacknowledged, the oldest unacknowledged sequence number
 	 * @ack: current acknowledge number
 	 * @seq: current sequence number
 	 */
 	struct {
+		u8 send_wnd;
+		u8 send_nxt;
+		u8 send_una;
 		u8 ack;
 		u8 seq;
 	} tcb;
diff --git a/drivers/greybus/cpc/cport.c b/drivers/greybus/cpc/cport.c
index 91c39856e22..c05d89709f0 100644
--- a/drivers/greybus/cpc/cport.c
+++ b/drivers/greybus/cpc/cport.c
@@ -16,6 +16,9 @@ static void cpc_cport_tcb_reset(struct cpc_cport *cport)
 {
 	cport->tcb.ack = 0;
 	cport->tcb.seq = 0;
+	cport->tcb.send_nxt = 0;
+	cport->tcb.send_una = 0;
+	cport->tcb.send_wnd = 1;
 }
 
 /**
@@ -38,12 +41,14 @@ struct cpc_cport *cpc_cport_alloc(u16 cport_id, gfp_t gfp_mask)
 
 	mutex_init(&cport->lock);
 	skb_queue_head_init(&cport->holding_queue);
+	skb_queue_head_init(&cport->retx_queue);
 
 	return cport;
 }
 
 void cpc_cport_release(struct cpc_cport *cport)
 {
+	skb_queue_purge(&cport->retx_queue);
 	skb_queue_purge(&cport->holding_queue);
 	kfree(cport);
 }
diff --git a/drivers/greybus/cpc/header.c b/drivers/greybus/cpc/header.c
index 8875a6fed26..dfcd5581133 100644
--- a/drivers/greybus/cpc/header.c
+++ b/drivers/greybus/cpc/header.c
@@ -22,6 +22,17 @@ bool cpc_header_is_control(const struct cpc_header *hdr)
 	return hdr->ctrl_flags & CPC_HEADER_CONTROL_IS_CONTROL_MASK;
 }
 
+/**
+ * cpc_header_get_recv_wnd() - Get the receive window.
+ * @hdr: CPC header.
+ *
+ * Return: Receive window.
+ */
+u8 cpc_header_get_recv_wnd(const struct cpc_header *hdr)
+{
+	return hdr->recv_wnd;
+}
+
 /**
  * cpc_header_get_seq() - Get the sequence number.
  * @hdr: CPC header.
@@ -33,6 +44,17 @@ u8 cpc_header_get_seq(const struct cpc_header *hdr)
 	return hdr->seq;
 }
 
+/**
+ * cpc_header_get_ack() - Get the acknowledge number.
+ * @hdr: CPC header.
+ *
+ * Return: Acknowledge number.
+ */
+u8 cpc_header_get_ack(const struct cpc_header *hdr)
+{
+	return hdr->ack;
+}
+
 /**
  * cpc_header_get_req_ack() - Get the request acknowledge frame flag.
  * @hdr: CPC header.
@@ -56,3 +78,59 @@ u8 cpc_header_encode_ctrl_flags(bool control, bool req_ack)
 	return FIELD_PREP(CPC_HEADER_CONTROL_IS_CONTROL_MASK, control) |
 	       FIELD_PREP(CPC_HEADER_CONTROL_REQ_ACK_MASK, req_ack);
 }
+
+/**
+ * cpc_header_get_frames_acked_count() - Get frames to be acknowledged.
+ * @seq: Current sequence number of the endpoint.
+ * @ack: Acknowledge number of the received frame.
+ *
+ * Return: Frames to be acknowledged.
+ */
+u8 cpc_header_get_frames_acked_count(u8 seq, u8 ack)
+{
+	return ack - seq;
+}
+
+/**
+ * cpc_header_number_in_window() - Test if a number is within a window.
+ * @start: Start of the window.
+ * @wnd: Window size.
+ * @n: Number to be tested.
+ *
+ * Given the start of the window and its size, test if the number is
+ * in the range [start; start + wnd).
+ *
+ * @return True if start <= n <= start + wnd - 1 (modulo 256), otherwise false.
+ */
+bool cpc_header_number_in_window(u8 start, u8 wnd, u8 n)
+{
+	u8 end;
+
+	if (wnd == 0)
+		return false;
+
+	end = start + wnd - 1;
+
+	return cpc_header_number_in_range(start, end, n);
+}
+
+/**
+ * cpc_header_number_in_range() - Test if a number is between start and end (included).
+ * @start: Lowest limit.
+ * @end: Highest limit inclusively.
+ * @n: Number to be tested.
+ *
+ * @return True if start <= n <= end (modulo 256), otherwise false.
+ */
+bool cpc_header_number_in_range(u8 start, u8 end, u8 n)
+{
+	if (end >= start) {
+		if (n < start || n > end)
+			return false;
+	} else {
+		if (n > end && n < start)
+			return false;
+	}
+
+	return true;
+}
diff --git a/drivers/greybus/cpc/header.h b/drivers/greybus/cpc/header.h
index 19612012b19..906bf45660d 100644
--- a/drivers/greybus/cpc/header.h
+++ b/drivers/greybus/cpc/header.h
@@ -39,8 +39,14 @@ struct cpc_header {
 } __packed;
 
 bool cpc_header_is_control(const struct cpc_header *hdr);
+u8 cpc_header_get_recv_wnd(const struct cpc_header *hdr);
 u8 cpc_header_get_seq(const struct cpc_header *hdr);
+u8 cpc_header_get_ack(const struct cpc_header *hdr);
 bool cpc_header_get_req_ack(const struct cpc_header *hdr);
 u8 cpc_header_encode_ctrl_flags(bool control, bool req_ack);
 
+u8 cpc_header_get_frames_acked_count(u8 seq, u8 ack);
+bool cpc_header_number_in_window(u8 start, u8 wnd, u8 n);
+bool cpc_header_number_in_range(u8 start, u8 end, u8 n);
+
 #endif
diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 225f9342cd2..9a0f8158504 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -205,15 +205,6 @@ void cpc_hd_del(struct cpc_host_device *cpc_hd)
 }
 EXPORT_SYMBOL_GPL(cpc_hd_del);
 
-void cpc_hd_message_sent(struct sk_buff *skb, int status)
-{
-	struct cpc_host_device *cpc_hd = CPC_SKB_CB(skb)->cport->cpc_hd;
-	struct gb_host_device *hd = cpc_hd->gb_hd;
-
-	greybus_message_sent(hd, CPC_SKB_CB(skb)->gb_message, status);
-}
-EXPORT_SYMBOL_GPL(cpc_hd_message_sent);
-
 void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, struct sk_buff *skb)
 {
 	struct gb_operation_msg_hdr *gb_hdr;
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
index cc835f5298b..8f05877b2be 100644
--- a/drivers/greybus/cpc/host.h
+++ b/drivers/greybus/cpc/host.h
@@ -46,7 +46,6 @@ int cpc_hd_add(struct cpc_host_device *cpc_hd);
 void cpc_hd_put(struct cpc_host_device *cpc_hd);
 void cpc_hd_del(struct cpc_host_device *cpc_hd);
 void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
-void cpc_hd_message_sent(struct sk_buff *skb, int status);
 
 int cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
 
diff --git a/drivers/greybus/cpc/protocol.c b/drivers/greybus/cpc/protocol.c
index 4cda71994d8..4afaf0eb95f 100644
--- a/drivers/greybus/cpc/protocol.c
+++ b/drivers/greybus/cpc/protocol.c
@@ -14,7 +14,7 @@ static bool cpc_skb_is_sequenced(struct sk_buff *skb)
 	return CPC_SKB_CB(skb)->cpc_flags & CPC_SKB_FLAG_REQ_ACK;
 }
 
-static void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack)
+static void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack, u8 recv_window)
 {
 	struct cpc_header *hdr;
 
@@ -24,6 +24,7 @@ static void cpc_protocol_prepare_header(struct sk_buff *skb, u8 ack)
 	memset(hdr, 0, sizeof(*hdr));
 
 	hdr->ack = ack;
+	hdr->recv_wnd = recv_window;
 	hdr->seq = CPC_SKB_CB(skb)->seq;
 	hdr->ctrl_flags = cpc_header_encode_ctrl_flags(!CPC_SKB_CB(skb)->gb_message,
 						       cpc_skb_is_sequenced(skb));
@@ -47,11 +48,47 @@ static void cpc_protocol_queue_ack(struct cpc_cport *cport, u8 ack)
 	gb_hdr->size = cpu_to_le16(sizeof(*gb_hdr));
 	cpc_cport_pack(gb_hdr, cport->id);
 
-	cpc_protocol_prepare_header(skb, ack);
+	cpc_protocol_prepare_header(skb, ack, CPC_HEADER_MAX_RX_WINDOW);
 
 	cpc_hd_send_skb(cport->cpc_hd, skb);
 }
 
+static void __cpc_protocol_receive_ack(struct cpc_cport *cport, u8 recv_wnd, u8 ack)
+{
+	struct gb_host_device *gb_hd = cport->cpc_hd->gb_hd;
+	struct sk_buff *skb;
+	u8 acked_frames;
+
+	cport->tcb.send_wnd = recv_wnd;
+
+	skb = skb_peek(&cport->retx_queue);
+	if (!skb)
+		return;
+
+	/* Return if no frame to ACK. */
+	if (!cpc_header_number_in_range(cport->tcb.send_una, cport->tcb.send_nxt, ack))
+		return;
+
+	/* Calculate how many frames will be ACK'd. */
+	acked_frames = cpc_header_get_frames_acked_count(CPC_SKB_CB(skb)->seq, ack);
+
+	for (u8 i = 0; i < acked_frames; i++) {
+		skb = skb_dequeue(&cport->retx_queue);
+		if (!skb) {
+			dev_err_ratelimited(cpc_hd_dev(cport->cpc_hd),
+					    "pending ack queue shorter than expected");
+			break;
+		}
+
+		if (CPC_SKB_CB(skb)->gb_message)
+			greybus_message_sent(gb_hd, CPC_SKB_CB(skb)->gb_message, 0);
+
+		kfree_skb(skb);
+
+		cport->tcb.send_una++;
+	}
+}
+
 void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb)
 {
 	struct cpc_header *cpc_hdr = (struct cpc_header *)skb->data;
@@ -62,6 +99,9 @@ void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb)
 
 	mutex_lock(&cport->lock);
 
+	__cpc_protocol_receive_ack(cport, cpc_header_get_recv_wnd(cpc_hdr),
+				   cpc_header_get_ack(cpc_hdr));
+
 	if (require_ack) {
 		expected_seq = seq == cport->tcb.ack;
 		if (expected_seq)
@@ -74,6 +114,8 @@ void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb)
 
 	ack = cport->tcb.ack;
 
+	__cpc_protocol_write_head(cport);
+
 	mutex_unlock(&cport->lock);
 
 	/* Ack no matter if the sequence was valid or not, to resync with remote */
@@ -87,9 +129,10 @@ void cpc_protocol_on_data(struct cpc_cport *cport, struct sk_buff *skb)
 	}
 }
 
-static void __cpc_protocol_write_skb(struct cpc_cport *cport, struct sk_buff *skb, u8 ack)
+static void __cpc_protocol_write_skb(struct cpc_cport *cport, struct sk_buff *skb, u8 ack,
+				     u8 recv_window)
 {
-	cpc_protocol_prepare_header(skb, ack);
+	cpc_protocol_prepare_header(skb, ack, recv_window);
 
 	cpc_hd_send_skb(cport->cpc_hd, skb);
 }
@@ -98,14 +141,30 @@ static void __cpc_protocol_write_skb(struct cpc_cport *cport, struct sk_buff *sk
 void __cpc_protocol_write_head(struct cpc_cport *cport)
 {
 	struct sk_buff *skb;
-	u8 ack;
+	u8 ack, send_una, send_wnd;
 
 	ack = cport->tcb.ack;
+	send_una = cport->tcb.send_una;
+	send_wnd = cport->tcb.send_wnd;
 
 	/* For each SKB in the holding queue, clone it and pass it to lower layer */
 	while ((skb = skb_peek(&cport->holding_queue))) {
+		struct sk_buff *out_skb;
+
+		/* Skip this skb if it must be acked but the remote has no room for it. */
+		if (!cpc_header_number_in_window(send_una, send_wnd, CPC_SKB_CB(skb)->seq))
+			break;
+
+		/* Clone and send out the skb */
+		out_skb = skb_clone(skb, GFP_KERNEL);
+		if (!out_skb)
+			return;
+
 		skb_unlink(skb, &cport->holding_queue);
 
-		__cpc_protocol_write_skb(cport, skb, ack);
+		__cpc_protocol_write_skb(cport, out_skb, ack, CPC_HEADER_MAX_RX_WINDOW);
+
+		cport->tcb.send_nxt++;
+		skb_queue_tail(&cport->retx_queue, skb);
 	}
 }
-- 
2.52.0

[PATCH v3 12/14] greybus: cpc: let host device drivers dequeue TX frames

[Damien Riégel]

This lets the CPC host device drivers dequeue frames when it's
convenient for them to do so, instead of forcing each to them to
implement a queue to store pending skbs.

The callback is changed from `transmit` to `wake_tx` and let CPC core
notify these drivers when there is something to transmit.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
 drivers/greybus/cpc/host.c | 74 ++++++++++++++++++++++++++++++++++++--
 drivers/greybus/cpc/host.h | 12 +++++--
 2 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index 9a0f8158504..d5ceb657fa9 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -161,6 +161,7 @@ static struct gb_hd_driver cpc_gb_driver = {
 static void cpc_hd_init(struct cpc_host_device *cpc_hd)
 {
 	mutex_init(&cpc_hd->lock);
+	skb_queue_head_init(&cpc_hd->tx_queue);
 }
 
 struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent)
@@ -168,7 +169,7 @@ struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct devic
 	struct cpc_host_device *cpc_hd;
 	struct gb_host_device *hd;
 
-	if (!driver->transmit) {
+	if (!driver->wake_tx) {
 		dev_err(parent, "missing mandatory callback\n");
 		return ERR_PTR(-EINVAL);
 	}
@@ -237,13 +238,80 @@ EXPORT_SYMBOL_GPL(cpc_hd_rcvd);
  * @cpc_hd: Host device to send SKB over.
  * @skb: SKB to send.
  */
-int cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb)
+void cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb)
 {
 	const struct cpc_hd_driver *drv = cpc_hd->driver;
 
-	return drv->transmit(cpc_hd, skb);
+	mutex_lock(&cpc_hd->lock);
+	skb_queue_tail(&cpc_hd->tx_queue, skb);
+	mutex_unlock(&cpc_hd->lock);
+
+	drv->wake_tx(cpc_hd);
 }
 
+/**
+ * cpc_hd_tx_queue_empty() - Check if transmit queue is empty.
+ * @cpc_hd: CPC Host Device.
+ *
+ * Return: True if transmit queue is empty, false otherwise.
+ */
+bool cpc_hd_tx_queue_empty(struct cpc_host_device *cpc_hd)
+{
+	bool empty;
+
+	mutex_lock(&cpc_hd->lock);
+	empty = skb_queue_empty(&cpc_hd->tx_queue);
+	mutex_unlock(&cpc_hd->lock);
+
+	return empty;
+}
+EXPORT_SYMBOL_GPL(cpc_hd_tx_queue_empty);
+
+/**
+ * cpc_hd_dequeue() - Get the next SKB that was queued for transmission.
+ * @cpc_hd: CPC Host Device.
+ *
+ * Get an SKB that was previously queued by cpc_hd_send_skb().
+ *
+ * Return: An SKB, or %NULL if queue was empty.
+ */
+struct sk_buff *cpc_hd_dequeue(struct cpc_host_device *cpc_hd)
+{
+	struct sk_buff *skb;
+
+	mutex_lock(&cpc_hd->lock);
+	skb = skb_dequeue(&cpc_hd->tx_queue);
+	mutex_unlock(&cpc_hd->lock);
+
+	return skb;
+}
+EXPORT_SYMBOL_GPL(cpc_hd_dequeue);
+
+/**
+ * cpc_hd_dequeue_many() - Get the next max_frames SKBs that were queued for transmission.
+ * @cpc_hd: CPC host device.
+ * @frame_list: Caller-provided sk_buff_head to fill with dequeued frames.
+ * @max_frames: Maximum number of frames to dequeue.
+ *
+ * Return: Number of frames actually dequeued.
+ */
+u32 cpc_hd_dequeue_many(struct cpc_host_device *cpc_hd, struct sk_buff_head *frame_list,
+			unsigned int max_frames)
+{
+	struct sk_buff *skb;
+	unsigned int count = 0;
+
+	mutex_lock(&cpc_hd->lock);
+	while (count < max_frames && (skb = skb_dequeue(&cpc_hd->tx_queue))) {
+		skb_queue_tail(frame_list, skb);
+		count++;
+	}
+	mutex_unlock(&cpc_hd->lock);
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(cpc_hd_dequeue_many);
+
 MODULE_DESCRIPTION("Greybus over CPC");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Silicon Laboratories, Inc.");
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
index 8f05877b2be..ee6a86de309 100644
--- a/drivers/greybus/cpc/host.h
+++ b/drivers/greybus/cpc/host.h
@@ -9,6 +9,7 @@
 #include <linux/device.h>
 #include <linux/greybus.h>
 #include <linux/mutex.h>
+#include <linux/skbuff.h>
 #include <linux/types.h>
 
 #define GB_CPC_MSG_SIZE_MAX 4096
@@ -18,7 +19,7 @@ struct cpc_cport;
 struct cpc_host_device;
 
 struct cpc_hd_driver {
-	int (*transmit)(struct cpc_host_device *hd, struct sk_buff *skb);
+	int (*wake_tx)(struct cpc_host_device *cpc_hd);
 };
 
 /**
@@ -34,6 +35,8 @@ struct cpc_host_device {
 
 	struct mutex lock; /* Synchronize access to cports */
 	struct cpc_cport *cports[GB_CPC_NUM_CPORTS];
+
+	struct sk_buff_head tx_queue;
 };
 
 static inline struct device *cpc_hd_dev(struct cpc_host_device *cpc_hd)
@@ -47,6 +50,11 @@ void cpc_hd_put(struct cpc_host_device *cpc_hd);
 void cpc_hd_del(struct cpc_host_device *cpc_hd);
 void cpc_hd_rcvd(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
 
-int cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
+void cpc_hd_send_skb(struct cpc_host_device *cpc_hd, struct sk_buff *skb);
+
+bool cpc_hd_tx_queue_empty(struct cpc_host_device *cpc_hd);
+struct sk_buff *cpc_hd_dequeue(struct cpc_host_device *cpc_hd);
+u32 cpc_hd_dequeue_many(struct cpc_host_device *cpc_hd, struct sk_buff_head *frame_list,
+			unsigned int max_frames);
 
 #endif
-- 
2.52.0

[PATCH v3 13/14] greybus: cpc: add private data pointer in CPC Host Device

[Damien Riégel]

Add a private data pointer when creating a CPC Host Device. This lets
the host device drivers get back their context more easily in the
callbacks.

Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
 drivers/greybus/cpc/host.c | 4 +++-
 drivers/greybus/cpc/host.h | 5 ++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/greybus/cpc/host.c b/drivers/greybus/cpc/host.c
index d5ceb657fa9..5c11f9c6bd5 100644
--- a/drivers/greybus/cpc/host.c
+++ b/drivers/greybus/cpc/host.c
@@ -164,7 +164,8 @@ static void cpc_hd_init(struct cpc_host_device *cpc_hd)
 	skb_queue_head_init(&cpc_hd->tx_queue);
 }
 
-struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent)
+struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent,
+				      void *priv)
 {
 	struct cpc_host_device *cpc_hd;
 	struct gb_host_device *hd;
@@ -181,6 +182,7 @@ struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct devic
 	cpc_hd = gb_hd_to_cpc_hd(hd);
 	cpc_hd->gb_hd = hd;
 	cpc_hd->driver = driver;
+	cpc_hd->priv = priv;
 
 	cpc_hd_init(cpc_hd);
 
diff --git a/drivers/greybus/cpc/host.h b/drivers/greybus/cpc/host.h
index ee6a86de309..4bb7339b394 100644
--- a/drivers/greybus/cpc/host.h
+++ b/drivers/greybus/cpc/host.h
@@ -37,6 +37,8 @@ struct cpc_host_device {
 	struct cpc_cport *cports[GB_CPC_NUM_CPORTS];
 
 	struct sk_buff_head tx_queue;
+
+	void *priv;
 };
 
 static inline struct device *cpc_hd_dev(struct cpc_host_device *cpc_hd)
@@ -44,7 +46,8 @@ static inline struct device *cpc_hd_dev(struct cpc_host_device *cpc_hd)
 	return &cpc_hd->gb_hd->dev;
 }
 
-struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent);
+struct cpc_host_device *cpc_hd_create(struct cpc_hd_driver *driver, struct device *parent,
+				      void *priv);
 int cpc_hd_add(struct cpc_host_device *cpc_hd);
 void cpc_hd_put(struct cpc_host_device *cpc_hd);
 void cpc_hd_del(struct cpc_host_device *cpc_hd);
-- 
2.52.0

[PATCH v3 14/14] greybus: cpc: add CPC SDIO host driver

[Damien Riégel]

From: Gabriel Beaulieu <gabriel.beaulieu@silabs.com>

This introduces a new module gb-cpc-sdio, in order to communicate with a
Greybus CPC device over SDIO.

Most of the complexity stems from aggregation: packets are aggregated to
minimize the number of CMD53s. In the first block, the first le32 is the
number of packets in this transfer. Immediately after that are all the
packet headers (CPC + Greybus). This lets the device process all the
headers in a single interrupt, and prepare the ADMA descriptors for all
the payloads in one go.

Payloads start at the beginning of the second block and are concatained.
Their lengths must be 32-bit aligned, so the driver takes care of adding
and removing padding if necessary.

Signed-off-by: Gabriel Beaulieu <gabriel.beaulieu@silabs.com>
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
Changes in v3:
  - rework defines to group together address-related defines and remove
    orphaned value
  - remove trivial comments
  - all RX and TX are now done from the workqueue. In previous
    iterations, transfers could either be done from the threaded IRQ or
    from the workqueue.
  - remove erroneous SDIO VID/PID
  - remove padding between headers and payloads when aggregating

Changes in v2:
  - change formatting from %lu to %zu when printing size_t's
  - remove "/**" kernel-doc marker for static functions not actually
    using the kernel-doc format
  - reduce header inclusion list
  - use reverse christmas tree variable declarations consistently
  - update aggregation functions to try to be more legible
  - use define instead of constant value 0x0C for the address where to
    read the number of bytes the device wants to send

 drivers/greybus/cpc/Kconfig  |  12 +
 drivers/greybus/cpc/Makefile |   3 +
 drivers/greybus/cpc/sdio.c   | 480 +++++++++++++++++++++++++++++++++++
 3 files changed, 495 insertions(+)
 create mode 100644 drivers/greybus/cpc/sdio.c

diff --git a/drivers/greybus/cpc/Kconfig b/drivers/greybus/cpc/Kconfig
index 0e72383381a..50f4c27666d 100644
--- a/drivers/greybus/cpc/Kconfig
+++ b/drivers/greybus/cpc/Kconfig
@@ -10,3 +10,15 @@ config GREYBUS_CPC
 
 	  To compile this code as a module, chose M here: the module will be
 	  called gb-cpc.ko
+
+config GREYBUS_CPC_SDIO
+	tristate "Greybus CPC over SDIO"
+	depends on GREYBUS_CPC && MMC
+	help
+	  This driver provides Greybus CPC host support for devices
+	  connected via SDIO interface.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called gb-cpc-sdio.
+
+	  If unsure, say N.
diff --git a/drivers/greybus/cpc/Makefile b/drivers/greybus/cpc/Makefile
index c4b530d27a3..3296536e86d 100644
--- a/drivers/greybus/cpc/Makefile
+++ b/drivers/greybus/cpc/Makefile
@@ -4,3 +4,6 @@ gb-cpc-y := cport.o header.o host.o protocol.o
 
 # CPC core
 obj-$(CONFIG_GREYBUS_CPC)	+= gb-cpc.o
+
+gb-cpc-sdio-y := sdio.o
+obj-$(CONFIG_GREYBUS_CPC_SDIO)	+= gb-cpc-sdio.o
diff --git a/drivers/greybus/cpc/sdio.c b/drivers/greybus/cpc/sdio.c
new file mode 100644
index 00000000000..34dfa7c1db1
--- /dev/null
+++ b/drivers/greybus/cpc/sdio.c
@@ -0,0 +1,480 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/kthread.h>
+#include <linux/mmc/sdio_func.h>
+#include <linux/mmc/sdio_ids.h>
+#include <linux/skbuff.h>
+#include <linux/workqueue.h>
+
+#include "cpc.h"
+#include "header.h"
+#include "host.h"
+
+#define GB_CPC_SDIO_MSG_SIZE_MAX 4096
+#define GB_CPC_SDIO_BLOCK_SIZE 256
+#define GB_CPC_SDIO_ALIGNMENT 4
+#define CPC_FRAME_HEADER_SIZE (sizeof(struct cpc_header) + sizeof(struct gb_operation_msg_hdr))
+
+#define GB_CPC_SDIO_ADDR_FIFO		0x00
+#define GB_CPC_SDIO_ADDR_RX_BYTES_CNT	0x0C
+
+enum cpc_sdio_flags {
+	CPC_SDIO_FLAG_RX_PENDING,
+	CPC_SDIO_FLAG_SHUTDOWN,
+};
+
+struct cpc_sdio {
+	struct cpc_host_device *cpc_hd;
+	struct device *dev;
+	struct sdio_func *func;
+
+	struct work_struct rxtx_work;
+	unsigned long flags;
+
+	wait_queue_head_t event_queue;
+	u8 max_aggregation;
+};
+
+struct frame_header {
+	struct cpc_header cpc;
+	struct gb_operation_msg_hdr gb;
+} __packed;
+
+static inline struct cpc_sdio *cpc_hd_to_cpc_sdio(struct cpc_host_device *cpc_hd)
+{
+	return (struct cpc_sdio *)cpc_hd->priv;
+}
+
+static int gb_cpc_sdio_wake_tx(struct cpc_host_device *cpc_hd)
+{
+	struct cpc_sdio *ctx = cpc_hd_to_cpc_sdio(cpc_hd);
+
+	if (test_bit(CPC_SDIO_FLAG_SHUTDOWN, &ctx->flags))
+		return 0;
+
+	schedule_work(&ctx->rxtx_work);
+
+	return 0;
+}
+
+/*
+ * Return the memory requirement in bytes for the aggregated frame aligned to the block size.
+ * All headers are contained in a dedicated block. Then payloads start at the beginning of second
+ * block. Each individual payload must be 4-byte aligned, and the total must be block-aligned.
+ */
+static size_t cpc_sdio_get_aligned_size(struct cpc_sdio *ctx, struct sk_buff_head *frame_list)
+{
+	struct sk_buff *frame;
+	size_t size = 0;
+
+	skb_queue_walk(frame_list, frame)
+		size += ALIGN(frame->len, GB_CPC_SDIO_ALIGNMENT);
+
+	size = ALIGN(size, GB_CPC_SDIO_BLOCK_SIZE);
+
+	return size;
+}
+
+static size_t cpc_sdio_build_aggregated_frame(struct cpc_sdio *ctx,
+					      struct sk_buff_head *frame_list,
+					      unsigned char **buffer)
+{
+	unsigned char *tx_buff;
+	struct sk_buff *frame;
+	__le32 *frame_count;
+	size_t xfer_size;
+	unsigned int i = 0;
+
+	xfer_size = cpc_sdio_get_aligned_size(ctx, frame_list);
+
+	tx_buff = kmalloc(xfer_size, GFP_KERNEL);
+	if (!tx_buff)
+		return 0;
+
+	frame_count = (__le32 *)tx_buff;
+	*frame_count = cpu_to_le32(skb_queue_len(frame_list));
+	i += sizeof(*frame_count);
+
+	/* First step is to aggregate all headers together */
+	skb_queue_walk(frame_list, frame) {
+		struct frame_header *fh = (struct frame_header *)&tx_buff[i];
+
+		memcpy(fh, frame->data, sizeof(*fh));
+		i += sizeof(*fh);
+	}
+
+	skb_queue_walk(frame_list, frame) {
+		size_t payload_len, padding_len;
+
+		if (frame->len <= CPC_FRAME_HEADER_SIZE)
+			continue;
+
+		payload_len = frame->len - CPC_FRAME_HEADER_SIZE;
+		memcpy(&tx_buff[i], &frame->data[CPC_FRAME_HEADER_SIZE], payload_len);
+		i += payload_len;
+
+		padding_len = ALIGN(payload_len, GB_CPC_SDIO_ALIGNMENT) - payload_len;
+		if (padding_len) {
+			memset(&tx_buff[i], 0, padding_len);
+			i += padding_len;
+		}
+	}
+
+	*buffer = tx_buff;
+
+	return xfer_size;
+}
+
+static bool cpc_sdio_get_payload_size(struct cpc_sdio *ctx, const struct frame_header *header,
+				      size_t *payload_size)
+{
+	size_t gb_size;
+
+	gb_size = le16_to_cpu(header->gb.size);
+
+	if (gb_size < sizeof(header->gb)) {
+		dev_dbg(ctx->dev, "Invalid Greybus header size: %zu\n", gb_size);
+		return false;
+	}
+
+	if (gb_size > (GB_CPC_SDIO_MSG_SIZE_MAX + sizeof(header->gb))) {
+		dev_dbg(ctx->dev, "Payload size exceeds maximum: %zu\n", gb_size);
+		return false;
+	}
+
+	*payload_size = gb_size - sizeof(header->gb);
+
+	return true;
+}
+
+/*
+ * Process aggregated frame
+ * Reconstructed frame layout:
+ * +-----+-----+-----+------+------+------+------+-------+---------+
+ * | CPC Header (4B) | Size | OpID | Type | Stat | CPort | Payload |
+ * +-----+-----+-----+------+------+------+------+-------+---------+
+ */
+static void cpc_sdio_process_aggregated_frame(struct cpc_sdio *ctx, unsigned char *aggregated_frame,
+					      unsigned int frame_len)
+{
+	const unsigned char *payload_start;
+	const struct frame_header *header;
+	unsigned int payload_offset;
+	size_t aligned_payload_size;
+	struct sk_buff *rx_skb;
+	__le32 frame_count_le;
+	size_t payload_size;
+	size_t frame_size;
+	u32 frame_count;
+
+	frame_count_le = *((__le32 *)aggregated_frame);
+	frame_count = le32_to_cpu(frame_count_le);
+
+	if (frame_count > ctx->max_aggregation) {
+		dev_warn(ctx->dev,
+			 "Process aggregated frame: frame count %u exceeds negotiated maximum %u\n",
+			 frame_count, ctx->max_aggregation);
+		return;
+	}
+
+	/* Header starts at block 0 after frame count */
+	header = (struct frame_header *)&aggregated_frame[sizeof(frame_count_le)];
+
+	/* Payloads start at block 1 (after complete block 0) */
+	payload_offset = (frame_count * CPC_FRAME_HEADER_SIZE) + sizeof(frame_count_le);
+
+	for (unsigned int i = 0; i < frame_count; i++) {
+		payload_start = &aggregated_frame[payload_offset];
+
+		if (!cpc_sdio_get_payload_size(ctx, header, &payload_size)) {
+			dev_err(ctx->dev,
+				"Process aggregated frame: failed to get payload size, aborting.\n");
+			return;
+		}
+
+		aligned_payload_size = ALIGN(payload_size, GB_CPC_SDIO_ALIGNMENT);
+
+		if (payload_offset + aligned_payload_size > frame_len) {
+			dev_err(ctx->dev,
+				"Process aggregated frame: payload is out of buffer boundary, aborting at frame %u\n",
+				i);
+			return;
+		}
+
+		frame_size = CPC_FRAME_HEADER_SIZE + payload_size;
+
+		rx_skb = alloc_skb(frame_size, GFP_KERNEL);
+		if (!rx_skb)
+			return;
+
+		memcpy(skb_put(rx_skb, CPC_FRAME_HEADER_SIZE), header, CPC_FRAME_HEADER_SIZE);
+
+		if (payload_size > 0)
+			memcpy(skb_put(rx_skb, payload_size), payload_start, payload_size);
+
+		cpc_hd_rcvd(ctx->cpc_hd, rx_skb);
+
+		header++;
+		payload_offset += aligned_payload_size;
+	}
+}
+
+static u32 cpc_sdio_get_rx_num_bytes(struct sdio_func *func, int *err)
+{
+	unsigned int rx_num_block_addr = GB_CPC_SDIO_ADDR_RX_BYTES_CNT;
+
+	return sdio_readl(func, rx_num_block_addr, err);
+}
+
+static void gb_cpc_sdio_rx(struct cpc_sdio *ctx)
+{
+	unsigned char *rx_buff;
+	size_t data_len;
+	int err;
+
+	sdio_claim_host(ctx->func);
+	data_len = cpc_sdio_get_rx_num_bytes(ctx->func, &err);
+
+	if (err) {
+		dev_err(ctx->dev, "failed to obtain byte count (%d)\n", err);
+		goto release_host;
+	}
+
+	if (data_len < sizeof(u32) + CPC_FRAME_HEADER_SIZE) {
+		dev_err(ctx->dev, "failed to obtain enough bytes for a header (%zu)\n", data_len);
+		goto release_host;
+	}
+
+	rx_buff = kmalloc(data_len, GFP_KERNEL);
+	if (!rx_buff)
+		goto release_host;
+
+	err = sdio_readsb(ctx->func, rx_buff, GB_CPC_SDIO_ADDR_FIFO, data_len);
+	sdio_release_host(ctx->func);
+
+	if (err) {
+		dev_err(ctx->dev, "failed to sdio_readsb (%d)\n", err);
+		goto free_rx_skb;
+	}
+
+	if (data_len < GB_CPC_SDIO_BLOCK_SIZE) {
+		dev_err(ctx->dev, "received %zd bytes, expected at least %u bytes\n", data_len,
+			GB_CPC_SDIO_BLOCK_SIZE);
+		goto free_rx_skb;
+	}
+
+	cpc_sdio_process_aggregated_frame(ctx, rx_buff, data_len);
+
+free_rx_skb:
+	kfree(rx_buff);
+
+	return;
+
+release_host:
+	sdio_release_host(ctx->func);
+}
+
+static int gb_cpc_sdio_tx(struct cpc_sdio *ctx)
+{
+	struct sk_buff_head frame_list;
+	unsigned char *tx_buff;
+	size_t tx_len;
+	int pkt_sent;
+	int err;
+
+	skb_queue_head_init(&frame_list);
+
+	cpc_hd_dequeue_many(ctx->cpc_hd, &frame_list, ctx->max_aggregation);
+
+	if (skb_queue_empty(&frame_list))
+		return 0;
+
+	tx_len = cpc_sdio_build_aggregated_frame(ctx, &frame_list, &tx_buff);
+	if (!tx_len) {
+		dev_err(ctx->dev, "failed to build aggregated frame\n");
+		goto cleanup_frames;
+	}
+
+	sdio_claim_host(ctx->func);
+	err = sdio_writesb(ctx->func, GB_CPC_SDIO_ADDR_FIFO, tx_buff, tx_len);
+	sdio_release_host(ctx->func);
+
+	kfree(tx_buff);
+
+cleanup_frames:
+	pkt_sent = skb_queue_len(&frame_list);
+	skb_queue_purge(&frame_list);
+
+	return pkt_sent;
+}
+
+static void gb_cpc_sdio_rxtx_work(struct work_struct *work)
+{
+	struct cpc_sdio *ctx = container_of(work, struct cpc_sdio, rxtx_work);
+	int num_tx = 0;
+
+	do {
+		if (test_and_clear_bit(CPC_SDIO_FLAG_RX_PENDING, &ctx->flags))
+			gb_cpc_sdio_rx(ctx);
+
+		num_tx = gb_cpc_sdio_tx(ctx);
+	} while (num_tx);
+}
+
+static struct cpc_hd_driver cpc_sdio_driver = {
+	.wake_tx = gb_cpc_sdio_wake_tx,
+};
+
+static int cpc_sdio_init(struct sdio_func *func)
+{
+	unsigned char rx_data_ready_irq_en_bit = BIT(0);
+	unsigned int irq_enable_addr = 0x09;
+	int err;
+
+	sdio_writeb(func, rx_data_ready_irq_en_bit, irq_enable_addr, &err);
+
+	return err;
+}
+
+static void cpc_sdio_irq_handler(struct sdio_func *func)
+{
+	unsigned int rx_data_pending_irq_bit = 0;
+	unsigned int irq_status_addr = 0x08;
+	unsigned long int_status;
+	struct cpc_sdio *ctx;
+	struct device *dev;
+	int err;
+
+	ctx = sdio_get_drvdata(func);
+	dev = &func->dev;
+
+	int_status = sdio_readb(func, irq_status_addr, &err);
+	if (err)
+		return;
+
+	if (__test_and_clear_bit(rx_data_pending_irq_bit, &int_status)) {
+		sdio_writeb(func, BIT(rx_data_pending_irq_bit), irq_status_addr, &err);
+		if (err)
+			return;
+
+		set_bit(CPC_SDIO_FLAG_RX_PENDING, &ctx->flags);
+		schedule_work(&ctx->rxtx_work);
+	}
+
+	if (int_status)
+		dev_err_once(dev, "unhandled interrupt from the device (%ld)\n", int_status);
+}
+
+static int cpc_sdio_probe(struct sdio_func *func, const struct sdio_device_id *id)
+{
+	struct cpc_host_device *cpc_hd;
+	struct cpc_sdio *ctx;
+	int err;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	cpc_hd = cpc_hd_create(&cpc_sdio_driver, &func->dev, ctx);
+	if (IS_ERR(cpc_hd)) {
+		kfree(ctx);
+		return PTR_ERR(cpc_hd);
+	}
+
+	ctx->cpc_hd = cpc_hd;
+	ctx->dev = cpc_hd_dev(cpc_hd);
+	ctx->func = func;
+	ctx->max_aggregation = 1;
+
+	INIT_WORK(&ctx->rxtx_work, gb_cpc_sdio_rxtx_work);
+
+	sdio_set_drvdata(func, ctx);
+
+	sdio_claim_host(func);
+
+	err = sdio_enable_func(func);
+	if (err)
+		goto release_host;
+
+	err = sdio_set_block_size(func, GB_CPC_SDIO_BLOCK_SIZE);
+	if (err)
+		goto disable_func;
+
+	err = cpc_sdio_init(func);
+	if (err)
+		goto disable_func;
+
+	err = sdio_claim_irq(func, cpc_sdio_irq_handler);
+	if (err)
+		goto disable_func;
+
+	err = cpc_hd_add(cpc_hd);
+	if (err)
+		goto release_irq;
+
+	sdio_release_host(func);
+
+	return 0;
+
+release_irq:
+	sdio_release_irq(func);
+
+disable_func:
+	sdio_disable_func(func);
+
+release_host:
+	sdio_release_host(func);
+	cpc_hd_put(cpc_hd);
+	kfree(ctx);
+
+	return err;
+}
+
+static void cpc_sdio_remove(struct sdio_func *func)
+{
+	struct cpc_sdio *ctx = sdio_get_drvdata(func);
+	struct cpc_host_device *cpc_hd = ctx->cpc_hd;
+
+	set_bit(CPC_SDIO_FLAG_SHUTDOWN, &ctx->flags);
+
+
+	sdio_claim_host(func);
+	sdio_release_irq(func);
+
+	cancel_work_sync(&ctx->rxtx_work);
+
+	sdio_disable_func(func);
+	sdio_release_host(func);
+
+	cpc_hd_del(cpc_hd);
+	cpc_hd_put(cpc_hd);
+
+	kfree(ctx);
+}
+
+static const struct sdio_device_id sdio_ids[] = {
+	/* No official ID */
+	{ SDIO_DEVICE(0x0000, 0x1000), },
+	{ },
+};
+MODULE_DEVICE_TABLE(sdio, sdio_ids);
+
+static struct sdio_driver gb_cpc_sdio_driver = {
+	.name     = KBUILD_MODNAME,
+	.id_table = sdio_ids,
+	.probe    = cpc_sdio_probe,
+	.remove   = cpc_sdio_remove,
+	.drv = {
+		.owner = THIS_MODULE,
+		.name  = KBUILD_MODNAME,
+	},
+};
+
+module_sdio_driver(gb_cpc_sdio_driver);
+
+MODULE_DESCRIPTION("Greybus Host Driver for Silicon Labs devices using SDIO");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Damien Riégel <damien.riegel@silabs.com>");
-- 
2.52.0

Re: [PATCH v3 14/14] greybus: cpc: add CPC SDIO host driver

[kernel test robot]

Hi Damien,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.19 next-20260212]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Damien-Ri-gel/greybus-cpc-add-minimal-CPC-Host-Device-infrastructure/20260212-232259
base:   linus/master
patch link:    https://lore.kernel.org/r/20260212144352.93043-15-damien.riegel%40silabs.com
patch subject: [PATCH v3 14/14] greybus: cpc: add CPC SDIO host driver
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260213/202602130822.5syxoFy2-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260213/202602130822.5syxoFy2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602130822.5syxoFy2-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/greybus/cpc/sdio.c: In function 'gb_cpc_sdio_tx':
>> drivers/greybus/cpc/sdio.c:286:13: warning: variable 'err' set but not used [-Wunused-but-set-variable]
     286 |         int err;
         |             ^~~


vim +/err +286 drivers/greybus/cpc/sdio.c

   279	
   280	static int gb_cpc_sdio_tx(struct cpc_sdio *ctx)
   281	{
   282		struct sk_buff_head frame_list;
   283		unsigned char *tx_buff;
   284		size_t tx_len;
   285		int pkt_sent;
 > 286		int err;
   287	
   288		skb_queue_head_init(&frame_list);
   289	
   290		cpc_hd_dequeue_many(ctx->cpc_hd, &frame_list, ctx->max_aggregation);
   291	
   292		if (skb_queue_empty(&frame_list))
   293			return 0;
   294	
   295		tx_len = cpc_sdio_build_aggregated_frame(ctx, &frame_list, &tx_buff);
   296		if (!tx_len) {
   297			dev_err(ctx->dev, "failed to build aggregated frame\n");
   298			goto cleanup_frames;
   299		}
   300	
   301		sdio_claim_host(ctx->func);
   302		err = sdio_writesb(ctx->func, GB_CPC_SDIO_ADDR_FIFO, tx_buff, tx_len);
   303		sdio_release_host(ctx->func);
   304	
   305		kfree(tx_buff);
   306	
   307	cleanup_frames:
   308		pkt_sent = skb_queue_len(&frame_list);
   309		skb_queue_purge(&frame_list);
   310	
   311		return pkt_sent;
   312	}
   313	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 10/14] greybus: cpc: use holding queue instead of sending out immediately

[kernel test robot]

Hi Damien,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.19 next-20260212]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Damien-Ri-gel/greybus-cpc-add-minimal-CPC-Host-Device-infrastructure/20260212-232259
base:   linus/master
patch link:    https://lore.kernel.org/r/20260212144352.93043-11-damien.riegel%40silabs.com
patch subject: [PATCH v3 10/14] greybus: cpc: use holding queue instead of sending out immediately
config: arm-randconfig-r133-20260213 (https://download.01.org/0day-ci/archive/20260213/202602131458.8YjgR17Z-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260213/202602131458.8YjgR17Z-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602131458.8YjgR17Z-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

ERROR: modpost: "skb_put" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_queue_purge_reason" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "__alloc_skb" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_pull" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "sk_skb_reason_drop" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_push" [drivers/greybus/cpc/gb-cpc.ko] undefined!
>> ERROR: modpost: "skb_unlink" [drivers/greybus/cpc/gb-cpc.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 14/14] greybus: cpc: add CPC SDIO host driver

[Jérôme Pouiller]

Hello Damien,

On Thursday 12 February 2026 15:43:52 Central European Standard Time Damien Riégel wrote:
> From: Gabriel Beaulieu <gabriel.beaulieu@silabs.com>
> 
> This introduces a new module gb-cpc-sdio, in order to communicate with a
> Greybus CPC device over SDIO.
> 
> Most of the complexity stems from aggregation: packets are aggregated to
> minimize the number of CMD53s. In the first block, the first le32 is the
> number of packets in this transfer. Immediately after that are all the
> packet headers (CPC + Greybus). This lets the device process all the
> headers in a single interrupt, and prepare the ADMA descriptors for all
> the payloads in one go.
> 
> Payloads start at the beginning of the second block and are concatained.
> Their lengths must be 32-bit aligned, so the driver takes care of adding
> and removing padding if necessary.
> 
> Signed-off-by: Gabriel Beaulieu <gabriel.beaulieu@silabs.com>
> Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
> ---
> Changes in v3:
>   - rework defines to group together address-related defines and remove
>     orphaned value
>   - remove trivial comments
>   - all RX and TX are now done from the workqueue. In previous
>     iterations, transfers could either be done from the threaded IRQ or
>     from the workqueue.
>   - remove erroneous SDIO VID/PID
>   - remove padding between headers and payloads when aggregating
> 
> Changes in v2:
>   - change formatting from %lu to %zu when printing size_t's
>   - remove "/**" kernel-doc marker for static functions not actually
>     using the kernel-doc format
>   - reduce header inclusion list
>   - use reverse christmas tree variable declarations consistently
>   - update aggregation functions to try to be more legible
>   - use define instead of constant value 0x0C for the address where to
>     read the number of bytes the device wants to send
> 
>  drivers/greybus/cpc/Kconfig  |  12 +
>  drivers/greybus/cpc/Makefile |   3 +
>  drivers/greybus/cpc/sdio.c   | 480 +++++++++++++++++++++++++++++++++++
>  3 files changed, 495 insertions(+)
>  create mode 100644 drivers/greybus/cpc/sdio.c
> 
[...]
> +static const struct sdio_device_id sdio_ids[] = {
> +	/* No official ID */
> +	{ SDIO_DEVICE(0x0000, 0x1000), },

Can you provide some details about how it's work? I assume Silabs sells
OEM products and each vendor has to set their own VID/PID, that's it?

I assume Silabs also has samples products. How it works for them? Is
there a default VID/PID or the sample firmware won't compile until the
user changes the VID/PID?

In any case, I believe we can't publish a driver with VID = 0.

(BTW, I suggest to include linux-mmc@vger.kernel.org as recipient of
this PR).

-- 
Jérôme Pouiller

Re: [PATCH v3 14/14] greybus: cpc: add CPC SDIO host driver

[Damien Riégel]

On Fri Feb 13, 2026 at 4:35 AM EST, Jérôme Pouiller wrote:
> Hello Damien,
>
> On Thursday 12 February 2026 15:43:52 Central European Standard Time Damien Riégel wrote:
>> From: Gabriel Beaulieu <gabriel.beaulieu@silabs.com>
>> 
>> This introduces a new module gb-cpc-sdio, in order to communicate with a
>> Greybus CPC device over SDIO.
>> 
>> Most of the complexity stems from aggregation: packets are aggregated to
>> minimize the number of CMD53s. In the first block, the first le32 is the
>> number of packets in this transfer. Immediately after that are all the
>> packet headers (CPC + Greybus). This lets the device process all the
>> headers in a single interrupt, and prepare the ADMA descriptors for all
>> the payloads in one go.
>> 
>> Payloads start at the beginning of the second block and are concatained.
>> Their lengths must be 32-bit aligned, so the driver takes care of adding
>> and removing padding if necessary.
>> 
>> Signed-off-by: Gabriel Beaulieu <gabriel.beaulieu@silabs.com>
>> Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
>> ---
>> Changes in v3:
>>   - rework defines to group together address-related defines and remove
>>     orphaned value
>>   - remove trivial comments
>>   - all RX and TX are now done from the workqueue. In previous
>>     iterations, transfers could either be done from the threaded IRQ or
>>     from the workqueue.
>>   - remove erroneous SDIO VID/PID
>>   - remove padding between headers and payloads when aggregating
>> 
>> Changes in v2:
>>   - change formatting from %lu to %zu when printing size_t's
>>   - remove "/**" kernel-doc marker for static functions not actually
>>     using the kernel-doc format
>>   - reduce header inclusion list
>>   - use reverse christmas tree variable declarations consistently
>>   - update aggregation functions to try to be more legible
>>   - use define instead of constant value 0x0C for the address where to
>>     read the number of bytes the device wants to send
>> 
>>  drivers/greybus/cpc/Kconfig  |  12 +
>>  drivers/greybus/cpc/Makefile |   3 +
>>  drivers/greybus/cpc/sdio.c   | 480 +++++++++++++++++++++++++++++++++++
>>  3 files changed, 495 insertions(+)
>>  create mode 100644 drivers/greybus/cpc/sdio.c
>> 
> [...]
>> +static const struct sdio_device_id sdio_ids[] = {
>> +	/* No official ID */
>> +	{ SDIO_DEVICE(0x0000, 0x1000), },
>
> Can you provide some details about how it's work? I assume Silabs sells
> OEM products and each vendor has to set their own VID/PID, that's it?
>
> I assume Silabs also has samples products. How it works for them? Is
> there a default VID/PID or the sample firmware won't compile until the
> user changes the VID/PID?

That's not something we have defined yet, but I think we could at least
define one VID/PID for "Greybus/CPC compatible devices", that every
vendor could use if they don't make any changes to the protocol. Each
vendor would have to create the manifest based on their products (like
enable WiFi for product X, enable WiFi & Bluetooth for product Y).

> In any case, I believe we can't publish a driver with VID = 0.

Noted, the patchset can't be applied as is. I'll check what the process
is to assign a VID/PID.

> (BTW, I suggest to include linux-mmc@vger.kernel.org as recipient of
> this PR).

I decided to wait a bit to avoid creating noise on linux-mmc,
considering this patchset is still in early review phase, but I'll add
that list for future versions.

thanks,
damien

Re: [PATCH v3 11/14] greybus: cpc: honour remote's RX window

[kernel test robot]

Hi Damien,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.19 next-20260213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Damien-Ri-gel/greybus-cpc-add-minimal-CPC-Host-Device-infrastructure/20260212-232259
base:   linus/master
patch link:    https://lore.kernel.org/r/20260212144352.93043-12-damien.riegel%40silabs.com
patch subject: [PATCH v3 11/14] greybus: cpc: honour remote's RX window
config: arm-randconfig-r133-20260213 (https://download.01.org/0day-ci/archive/20260214/202602140508.Xj7ai7J1-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260214/202602140508.Xj7ai7J1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602140508.Xj7ai7J1-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

ERROR: modpost: "skb_put" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_dequeue" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_queue_purge_reason" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "__alloc_skb" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_queue_tail" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_pull" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "sk_skb_reason_drop" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_push" [drivers/greybus/cpc/gb-cpc.ko] undefined!
ERROR: modpost: "skb_unlink" [drivers/greybus/cpc/gb-cpc.ko] undefined!
>> ERROR: modpost: "skb_clone" [drivers/greybus/cpc/gb-cpc.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 14/14] greybus: cpc: add CPC SDIO host driver

[Damien Riégel]

On Fri Feb 13, 2026 at 11:00 AM EST, Damien Riégel wrote:
> On Fri Feb 13, 2026 at 4:35 AM EST, Jérôme Pouiller wrote:
>> In any case, I believe we can't publish a driver with VID = 0.
>
> Noted, the patchset can't be applied as is. I'll check what the process
> is to assign a VID/PID.

I'm still trying to figure out internally how I can get a VID/PID for
this driver. We thought we had a valid vendor ID we could use, based on
a JEDEC ID that we already have assigned, but upon further investigation
I don't think we have the right to derive an SDIO VID from that JEDEC
ID, so the whole process might take a bit of time.

In the meantime, may I suggest that we continue the review as if a valid
VID/PID was assigned? You can keep your "Reviewed-by" until we actually
have valid IDs. If we have to get in touch with external organizations,
it might take a few weeks or even months to get sorted, I have no idea
how long that process will be.

Alternatively, I could just drop the SDIO VID/PID and just rely on device
tree instead? That leaves the question of what the compatible string
should be. Rob Herring doesn't want a generic "silabs,greybus" string,
it must be a real product and we don't have that yet.

I'd be happy to take any input that helps make progress :)


thanks,
damien